Deleting both the original object and its reference in a pre-delete hook

[revosw]

Background

Our team broke up a heavily nested document into multiple documents. Originally, all ventilators were embedded inside the house.

const house = {
  house: {
    ventilators: [
      {
        defects: [
          { comment: "Yep I think it's broken", reported: 16827384783 },
          { comment: "Ms Herringer got psychotic and smashed the ventilator with a chair for the 3rd time this week", reported: 16828575644 },
        ]
      },
      {
        defects: [
          { comment: "Dent on left side", reported: 16827752433 },
        ]
      },
    ]
  }
}

However, now all ventilators are referenced instead.

const house = {
  house: {
    ventilators: [
      ObjectId("62765ac9c76bdc1496d0b0ad"),
      ObjectId("62765f29c76bdc1796d8bd72")
    ]
  }
}

Our schemas/models kind of looked like this before

const houseSchema = new Schema({
  ventilators: {
    type: [Ventilator],
    required: false,
  },
}
const House = model(houseSchema)

const ventilatorSchema = new Schema({
  defects: {
    type: [Defect],
    required: true,
  },
}
const Ventilator = model(ventilatorSchema)

But now the house schema uses a reference instead, and we added a "houseId" field in the ventilator schema

const houseSchema = new Schema({
  ventilators: {
    type: [mongoose.Types.ObjectId],
    ref: 'Ventilator'
    required: false,
  },
}

const ventilatorSchema = new Schema({
  houseId: {
    type: mongoose.Types.ObjectId,
    required: true
  }
  defects: {
    type: [Defect],
    required: true,
  },
}

The problem

We have an endpoint in express for deleting a ventilator. As I understand it, MongoDB has no cascading delete, in the same way SQL has. If you delete the ventilator...

Ventilator.deleteOne({ _id: ventilatorId })

...you gotta remember to delete the reference as well

House.updateOne({ _id: houseId }, { $pull: { ventilators: ventilatorId } })

This works! However, although one of our unit tests are currently able to catch the mistake of not deleting the reference, this looks like a fragile solution. I tried moving House.updateOne inside the ventilatorSchema.pre('deleteOne') hook instead. But here's the question - how do I even know which house to delete the ventilator reference from?

ventilatorSchema.pre('deleteOne', async function (next) {
  // this._conditions is an object containing all the key-value pairs supplied in the deleteOne filter.
  // Because I supplied the id of the ventilator inside `Ventilator.deleteOne()`, I can access the _id
  await House.updateOne({ _id: ???? }, { $pull: { ventilator: this._conditions._id } })
  next()
})

Because this inside the deleteOne hook is a Query and not a Document, I cannot just access the houseId by this.houseId. I tried adding {document: true} as an option, but that just made the hook not run at all.

//        Does not work! Hook isn't run at all
//                                        V
ventilatorSchema.pre('deleteOne', {document: true}, async function (next) {
  await House.updateOne({ _id: ???? }, { $pull: { ventilator: this._conditions._id } })
  next()
})

If this had been the document, I would be able to fish out houseId from the Ventilator document. However, I must rely on performing yet another query, Ventilator.findById(), to retrieve the houseId

ventilatorSchema.pre('deleteOne', async function (next) {
  const ventilator = await Ventilator.findById(this._conditions._id)
  await House.updateOne({ _id: ventilator.houseId }, { $pull: { ventilator: this._conditions._id } })
  next()
})

And this works! Whenever I delete a ventilator, the reference is guaranteed to be deleted as well - no matter how I refactor the ventilator deletion http handler later. But does my code make sense? Is there some glaringly obvious optimization I have missed?

[revosw]

An alternative method could be to pass in not just _id, but also houseId when calling deleteOne

Ventilator.deleteOne({ _id: ventilatorId, houseId: houseId })

Because houseId is now added to the _conditions object, only a single call is needed in the hook now

ventilatorSchema.pre('deleteOne', {document: true}, async function (next) {
  await House.updateOne({ _id: this._conditions.houseId }, { $pull: { ventilator: this._conditions._id } })
  next()
})

But the downside is that if you accidentally refactor Ventilator.deleteOne({ _id: ventilatorId, houseId: houseId }) into Ventilator.deleteOne({ _id: ventilatorId }), now you will have no idea that the reference is not being deleted, and it is far from obvious why that is the case

[vkarpov15]

The better approach would be to not have houseSchema contain a ventilators array. We typically recommend representing one-to-many relationships with the "many" side pointing to the "one" side. So ventilators should have a houseId, but houses should not have ventilators. Can you use that approach or would that cause issues for you?

[revosw]

That sounds like a reasonable and good approach! Thanks 😎