Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-occurrence of embedded discriminator not calling pre('validate') (discriminator defined without path) #14961

Closed
2 tasks done
mvhayman-afs opened this issue Oct 14, 2024 · 0 comments · Fixed by #15001
Closed
2 tasks done
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@mvhayman-afs
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

8.7.1

Node.js version

20

MongoDB server version

6.9.0

Typescript version (if applicable)

No response

Description

Thanks guys for all your hard work on Mongoose!

I'm upgrading from 7.6.5 to 8.7.1.

I'm having an issue where pre('validate') isn't called on a discriminated schema embedded in an array.

Many of our discriminators are defined at the root level, not under a subkey.

This test passes in 7.6.5:

const assert = require('assert')
const sinon = require('sinon')
const mongoose = require('mongoose')

const Schema = mongoose.Schema

describe('Mongoose Discriminators', () => {
   it('Middleware should succesfully be invoked on base and discriminators', async () => {
      const eventSchema = new Schema(
         { message: String },
         { discriminatorKey: 'kind', _id: false }
      )

      const clickedSchema = new Schema({
         element: String
      }, { _id: false })

      // This is the discriminator which we will use to test middleware

      const purchasedSchema = new Schema({
         product: String
      }, { _id: false })

      // Setup some spies to test whether the middleware functions execute

      const eventPreValidateSpy = sinon.spy()
      const eventPostValidateSpy = sinon.spy()
      const eventPreSaveSpy = sinon.spy()
      const eventPostSaveSpy = sinon.spy()

      eventSchema.pre('validate', function (next) {
         eventPreValidateSpy()
         next()
      })

      eventSchema.post('validate', function (doc) {
         eventPostValidateSpy()
      })

      eventSchema.pre('save', function (next) {
         eventPreSaveSpy()
         next()
      })

      eventSchema.post('save', function (doc) {
         eventPostSaveSpy()
      })

      const purchasedPreValidateSpy = sinon.spy()
      const purchasedPostValidateSpy = sinon.spy()
      const purchasedPreSaveSpy = sinon.spy()
      const purchasedPostSaveSpy = sinon.spy()

      purchasedSchema.pre('validate', function (next) {
         purchasedPreValidateSpy()
         next()
      })

      purchasedSchema.post('validate', function (doc) {
         purchasedPostValidateSpy()
      })

      purchasedSchema.pre('save', function (next) {
         purchasedPreSaveSpy()
         next()
      })

      purchasedSchema.post('save', function (doc) {
         purchasedPostSaveSpy()
      })

      // Register the discriminators (note no path key, but defined on `eventSchema` instead)

      eventSchema.discriminator('Clicked', clickedSchema)
      eventSchema.discriminator('Purchased', purchasedSchema)

      const trackSchema = new Schema({
         event: eventSchema
      })

      // Test

      const MyModel = mongoose.model('track', trackSchema)
      const doc = new MyModel({
         event: {
            kind: 'Purchased',
            message: 'Test'
         }
      })

      await doc.save()
      assert.equal(doc.event.message, 'Test')
      assert.equal(doc.event.kind, 'Purchased')
      assert.ok(!doc.event.product)

      // These assertions fail

      assert(eventPreValidateSpy.calledOnce, 'base pre-validate middleware was not called')
      assert(eventPostValidateSpy.calledOnce, 'base post-validate middleware was not called')
      assert(eventPreSaveSpy.calledOnce, 'base pre-save middleware was not called')
      assert(eventPostSaveSpy.calledOnce, 'base post-save middleware was not called')

      assert(purchasedPreValidateSpy.calledOnce, 'discriminator pre-validate middleware was not called')
      assert(purchasedPostValidateSpy.calledOnce, 'discriminator post-validate middleware was not called')
      assert(purchasedPreSaveSpy.calledOnce, 'discriminator pre-save middleware was not called')
      assert(purchasedPostSaveSpy.calledOnce, 'discriminator post-save middleware was not called')
   })
})

But it fails in 8.7.1.

Note that this works if you define

trackSchema.path('event').discriminator('Clicked', clickedSchema)
trackSchema.path('event').discriminator('Purchased', purchasedSchema)

But we don't have this luxury, as we're not using subkeys.

Original test shamelessly stolen from here:
#5706 (comment)

With changes:

  • trackedSchema moved to the end as recommended in the docs (i.e. define pre hooks on schema before including in parent)
  • remove path key and define discriminators directly on eventSchema

Steps to Reproduce

{
  "name": "mongoose-test-project",
  "version": "1.0.0",
  "main": "index.js",
  "scripts": {
    "test": "mocha"
  },
  "dependencies": {
    "mongodb": "6.9.0",
    "mongoose": "8.7.1"
  },
  "devDependencies": {
    "chai": "^5.1.1",
    "mocha": "^10.2.0",
    "sinon": "^19.0.2"
  },
  "author": "",
  "license": "ISC"
}

vs

{
  "name": "mongoose-test-project",
  "version": "1.0.0",
  "main": "index.js",
  "scripts": {
    "test": "mocha"
  },
  "dependencies": {
    "mongodb": "5.9.0",
    "mongoose": "7.6.5"
  },
  "devDependencies": {
    "chai": "^5.1.1",
    "mocha": "^10.2.0",
    "sinon": "^19.0.2"
  },
}

Expected Behavior

No response

@vkarpov15 vkarpov15 added this to the 8.7.3 milestone Oct 20, 2024
@vkarpov15 vkarpov15 added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Oct 20, 2024
@vkarpov15 vkarpov15 modified the milestones: 8.7.3, 8.7.4 Oct 25, 2024
@vkarpov15 vkarpov15 added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Nov 1, 2024
vkarpov15 added a commit that referenced this issue Nov 1, 2024
… top level model so middleware applies correctly

Fix #14961
vkarpov15 added a commit that referenced this issue Nov 4, 2024
fix: apply embedded discriminators to subdoc schemas before compiling top level model so middleware applies correctly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
2 participants