From 4437d8edc93451e7e302f99aada56c0ee0a3f7af Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 29 Nov 2023 10:49:45 -0500 Subject: [PATCH 1/3] fix: allow adding discriminators using `Schema.prototype.discriminator()` to subdocuments after defining parent schema Fix #14109 --- lib/index.js | 10 +++++++ lib/schema/SubdocumentPath.js | 5 ---- lib/schema/documentarray.js | 8 +----- test/document.test.js | 51 +++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/lib/index.js b/lib/index.js index 3e7bf3709ef..c6cba262b8e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -629,6 +629,16 @@ Mongoose.prototype._model = function(name, schema, collection, options) { } } + for (const path of Object.keys(schema.paths)) { + const schemaType = schema.paths[path]; + if (!schemaType.schema || !schemaType.schema._applyDiscriminators) { + continue; + } + for (const disc of schemaType.schema._applyDiscriminators.keys()) { + schemaType.discriminator(disc, schemaType.schema._applyDiscriminators.get(disc)); + } + } + return model; }; diff --git a/lib/schema/SubdocumentPath.js b/lib/schema/SubdocumentPath.js index 2edba587321..7e77d71a0d9 100644 --- a/lib/schema/SubdocumentPath.js +++ b/lib/schema/SubdocumentPath.js @@ -55,11 +55,6 @@ function SubdocumentPath(schema, path, options) { this.$isSingleNested = true; this.base = schema.base; SchemaType.call(this, path, options, 'Embedded'); - if (schema._applyDiscriminators != null && !options?._skipApplyDiscriminators) { - for (const disc of schema._applyDiscriminators.keys()) { - this.discriminator(disc, schema._applyDiscriminators.get(disc)); - } - } } /*! diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js index b0cef6401a2..7f8a39a04d6 100644 --- a/lib/schema/documentarray.js +++ b/lib/schema/documentarray.js @@ -88,12 +88,6 @@ function DocumentArrayPath(key, schema, options, schemaOptions) { this.$embeddedSchemaType.caster = this.Constructor; this.$embeddedSchemaType.schema = this.schema; - - if (schema._applyDiscriminators != null && !options?._skipApplyDiscriminators) { - for (const disc of schema._applyDiscriminators.keys()) { - this.discriminator(disc, schema._applyDiscriminators.get(disc)); - } - } } /** @@ -528,7 +522,7 @@ DocumentArrayPath.prototype.cast = function(value, doc, init, prev, options) { DocumentArrayPath.prototype.clone = function() { const options = Object.assign({}, this.options); - const schematype = new this.constructor(this.path, this.schema, { ...options, _skipApplyDiscriminators: true }, this.schemaOptions); + const schematype = new this.constructor(this.path, this.schema, options, this.schemaOptions); schematype.validators = this.validators.slice(); if (this.requiredValidator !== undefined) { schematype.requiredValidator = this.requiredValidator; diff --git a/test/document.test.js b/test/document.test.js index b000de90fd8..16b565e38c0 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12657,6 +12657,57 @@ describe('document', function() { ); }); + it('handles embedded discriminators defined using Schema.prototype.discriminator after defining schema (gh-14109) (gh-13898)', async function() { + const baseNestedDiscriminated = new Schema({ + type: { type: Number, required: true } + }, { discriminatorKey: 'type' }); + + class BaseClass { + whoAmI() { + return 'I am baseNestedDiscriminated'; + } + } + BaseClass.type = 1; + + baseNestedDiscriminated.loadClass(BaseClass); + + class NumberTyped extends BaseClass { + whoAmI() { + return 'I am NumberTyped'; + } + } + NumberTyped.type = 3; + + class StringTyped extends BaseClass { + whoAmI() { + return 'I am StringTyped'; + } + } + StringTyped.type = 4; + + const containsNestedSchema = new Schema({ + nestedDiscriminatedTypes: { type: [baseNestedDiscriminated], required: true } + }); + + // After `containsNestedSchema`, in #13898 test these were before `containsNestedSchema` + baseNestedDiscriminated.discriminator(1, new Schema({}).loadClass(NumberTyped)); + baseNestedDiscriminated.discriminator('3', new Schema({}).loadClass(StringTyped)); + + class ContainsNested { + whoAmI() { + return 'I am ContainsNested'; + } + } + containsNestedSchema.loadClass(ContainsNested); + + const Test = db.model('Test', containsNestedSchema); + const instance = await Test.create({ type: 1, nestedDiscriminatedTypes: [{ type: 1 }, { type: '3' }] }); + assert.deepStrictEqual( + instance.nestedDiscriminatedTypes.map(i => i.whoAmI()), + ['I am NumberTyped', 'I am StringTyped'] + ); + }); + it('can use `collection` as schema name (gh-13956)', async function() { const schema = new mongoose.Schema({ name: String, collection: String }); const Test = db.model('Test', schema); From 159d70581743c186ca6ac22f45db48d4780c3d1c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 29 Nov 2023 11:28:28 -0500 Subject: [PATCH 2/3] fix: apply embedded discriminators recursively so fix for #14109 works on discriminators underneath subdocs --- .../applyEmbeddedDiscriminators.js | 23 ++++++++ lib/index.js | 11 +--- test/document.test.js | 52 +++++++++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 lib/helpers/discriminator/applyEmbeddedDiscriminators.js diff --git a/lib/helpers/discriminator/applyEmbeddedDiscriminators.js b/lib/helpers/discriminator/applyEmbeddedDiscriminators.js new file mode 100644 index 00000000000..930e1c86c13 --- /dev/null +++ b/lib/helpers/discriminator/applyEmbeddedDiscriminators.js @@ -0,0 +1,23 @@ +'use strict'; + +module.exports = applyEmbeddedDiscriminators; + +function applyEmbeddedDiscriminators(schema, seen = new WeakSet()) { + if (seen.has(schema)) { + return; + } + seen.add(schema); + for (const path of Object.keys(schema.paths)) { + const schemaType = schema.paths[path]; + if (!schemaType.schema) { + continue; + } + applyEmbeddedDiscriminators(schemaType.schema, seen); + if (!schemaType.schema._applyDiscriminators) { + continue; + } + for (const disc of schemaType.schema._applyDiscriminators.keys()) { + schemaType.discriminator(disc, schemaType.schema._applyDiscriminators.get(disc)); + } + } +} diff --git a/lib/index.js b/lib/index.js index c6cba262b8e..90547f1e95b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -32,6 +32,7 @@ const sanitizeFilter = require('./helpers/query/sanitizeFilter'); const isBsonType = require('./helpers/isBsonType'); const MongooseError = require('./error/mongooseError'); const SetOptionError = require('./error/setOptionError'); +const applyEmbeddedDiscriminators = require('./helpers/discriminator/applyEmbeddedDiscriminators'); const defaultMongooseSymbol = Symbol.for('mongoose:default'); @@ -629,15 +630,7 @@ Mongoose.prototype._model = function(name, schema, collection, options) { } } - for (const path of Object.keys(schema.paths)) { - const schemaType = schema.paths[path]; - if (!schemaType.schema || !schemaType.schema._applyDiscriminators) { - continue; - } - for (const disc of schemaType.schema._applyDiscriminators.keys()) { - schemaType.discriminator(disc, schemaType.schema._applyDiscriminators.get(disc)); - } - } + applyEmbeddedDiscriminators(schema); return model; }; diff --git a/test/document.test.js b/test/document.test.js index 16b565e38c0..0e7ee27a510 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12708,6 +12708,58 @@ describe('document', function() { ); }); + it('handles embedded discriminators on nested path defined using Schema.prototype.discriminator (gh-14109) (gh-13898)', async function() { + const baseNestedDiscriminated = new Schema({ + type: { type: Number, required: true } + }, { discriminatorKey: 'type' }); + + class BaseClass { + whoAmI() { + return 'I am baseNestedDiscriminated'; + } + } + BaseClass.type = 1; + + baseNestedDiscriminated.loadClass(BaseClass); + + class NumberTyped extends BaseClass { + whoAmI() { + return 'I am NumberTyped'; + } + } + NumberTyped.type = 3; + + class StringTyped extends BaseClass { + whoAmI() { + return 'I am StringTyped'; + } + } + StringTyped.type = 4; + + const containsNestedSchema = new Schema({ + nestedDiscriminatedTypes: { type: [baseNestedDiscriminated], required: true } + }); + + baseNestedDiscriminated.discriminator(1, new Schema({}).loadClass(NumberTyped)); + baseNestedDiscriminated.discriminator('3', new Schema({}).loadClass(StringTyped)); + + const l2Schema = new Schema({ l3: containsNestedSchema }); + const l1Schema = new Schema({ l2: l2Schema }); + + const Test = db.model('Test', l1Schema); + const instance = await Test.create({ + l2: { + l3: { + nestedDiscriminatedTypes: [{ type: 1 }, { type: '3' }] + } + } + }); + assert.deepStrictEqual( + instance.l2.l3.nestedDiscriminatedTypes.map(i => i.whoAmI()), + ['I am NumberTyped', 'I am StringTyped'] + ); + }); + it('can use `collection` as schema name (gh-13956)', async function() { const schema = new mongoose.Schema({ name: String, collection: String }); const Test = db.model('Test', schema); From 16e53086f85f0915fd4f72460f40a072c9babd34 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 30 Nov 2023 21:24:25 -0500 Subject: [PATCH 3/3] test: remove unnecessary code in tests for #14109 --- test/document.test.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index 0e7ee27a510..f6862905519 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12667,7 +12667,6 @@ describe('document', function() { return 'I am baseNestedDiscriminated'; } } - BaseClass.type = 1; baseNestedDiscriminated.loadClass(BaseClass); @@ -12676,14 +12675,12 @@ describe('document', function() { return 'I am NumberTyped'; } } - NumberTyped.type = 3; class StringTyped extends BaseClass { whoAmI() { return 'I am StringTyped'; } } - StringTyped.type = 4; const containsNestedSchema = new Schema({ nestedDiscriminatedTypes: { type: [baseNestedDiscriminated], required: true } @@ -12718,7 +12715,6 @@ describe('document', function() { return 'I am baseNestedDiscriminated'; } } - BaseClass.type = 1; baseNestedDiscriminated.loadClass(BaseClass); @@ -12727,14 +12723,12 @@ describe('document', function() { return 'I am NumberTyped'; } } - NumberTyped.type = 3; class StringTyped extends BaseClass { whoAmI() { return 'I am StringTyped'; } } - StringTyped.type = 4; const containsNestedSchema = new Schema({ nestedDiscriminatedTypes: { type: [baseNestedDiscriminated], required: true }