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

Model.bulkWrite updateOne fails to update discriminator key with overwriteDiscriminatorKey set to true #15040

Open
2 tasks done
lcrosetto opened this issue Nov 15, 2024 · 2 comments · May be fixed by #15046
Open
2 tasks done
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@lcrosetto
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.8.1

Node.js version

20.18.0

MongoDB server version

8.0.1

Typescript version (if applicable)

No response

Description

When updating the discriminator key in a document with schema discriminators, and setting the overwriteDiscriminatorKey option, the update succeeds when done with Model.updateOne() but fails when the same update is done by calling Model.bulkWrite() with the updateOne op.

Steps to Reproduce

The following script will fail to update the discriminator key:

      const conn = mongoose.createConnection(..);

      const dSchema1 = new mongoose.Schema({
        field1: String,
      });
      const dSchema2 = new mongoose.Schema({
        field2: String,
      });
      const baseSchema = new mongoose.Schema({
        field: String,
        key: String,
      }, {discriminatorKey: 'key'});
      const type1Key = 'Type1';
      const type2Key = 'Type2';

      baseSchema.discriminator(type1Key, dSchema1);
      baseSchema.discriminator(type2Key, dSchema2);

      const TestModel = conn.model('Test', baseSchema);

      const test = new TestModel({
        field: 'base field',
        key: type1Key,
        field1: 'field1',
      });
      const r1 = await test.save();
      assert.equal(r1.field1, 'field1');
      assert.equal(r1.key, type1Key);

      const field2 = 'field2';
      await TestModel.bulkWrite([{
        updateOne: {
          filter: { _id: r1._id },
          update: {
            key: type2Key,
            field2,
          },
          overwriteDiscriminatorKey: true,
        },
      }]);

      const r2 = await TestModel.findById(r1._id);
      assert.equal(r2.key, type2Key);
      assert.equal(r2.field2, field2);

      conn.deleteModel('Test');

However, if the call to bulkWrite() is replaced with updateOne() the operation will succeed:

      const field2 = 'field2';
      await TestModel.updateOne(
        { _id: r1._id },
        {
          key: type2Key,
          field2,
        },
        {overwriteDiscriminatorKey: true},
      );
      const r2 = await TestModel.findById(r1._id);
      assert.equal(r2.key, type2Key);
      assert.equal(r2.field2, field2);

Expected Behavior

Overwriting the discriminator key should succeed with Model.bulkWrite() as the same request does when done with Model.updateOne().

@lcrosetto
Copy link
Author

lcrosetto commented Nov 15, 2024

I have gotten the overwriteDiscriminatorKey option to work with Model.bulkWrite() updateOne with the following changes:

op['updateOne']['update'] = castUpdate(model.schema, update, {
strict: strict,
upsert: op['updateOne'].upsert,
arrayFilters: op['updateOne'].arrayFilters
}, model, op['updateOne']['filter']);

        op['updateOne']['update'] = castUpdate(model.schema, update, {
          strict: strict,
          upsert: op['updateOne'].upsert,
          arrayFilters: op['updateOne'].arrayFilters,
          overwriteDiscriminatorKey: op['updateOne'].overwriteDiscriminatorKey
        }, model, op['updateOne']['filter']);

const model = decideModelByObject(originalModel, op['updateOne']['filter']);
const schema = model.schema;
const strict = options.strict != null ? options.strict : model.schema.options.strict;
const update = clone(op['updateOne']['update']);
_addDiscriminatorToObject(schema, op['updateOne']['filter']);

        const update = clone(op['updateOne']['update']);
        const {overwriteDiscriminatorKey} = op['updateOne'];

        const model = decideModelByObject(originalModel, update, op['updateOne']['filter'], {overwriteDiscriminatorKey});
        const schema = model.schema;
        const strict = options.strict != null ? options.strict : model.schema.options.strict;
        const {discriminatorKey} = schema.options;


        if (discriminatorKey && (!overwriteDiscriminatorKey || update[discriminatorKey] == null)) {
          _addDiscriminatorToObject(schema, op['updateOne']['filter']);
        }

function decideModelByObject(model, object) {
const discriminatorKey = model.schema.options.discriminatorKey;
if (object != null && object.hasOwnProperty(discriminatorKey)) {
model = getDiscriminatorByValue(model.discriminators, object[discriminatorKey]) || model;
}
return model;
}

function decideModelByObject(model, object, filter, options) {
  const discriminatorKey = model.schema.options.discriminatorKey;
  let key;
  if (options?.overwriteDiscriminatorKey && object?.[discriminatorKey] != null && model.discriminators) {
    key = object[discriminatorKey];
  } else if (filter != null && filter.hasOwnProperty(discriminatorKey)) {
    key = filter[discriminatorKey];
  }
  if (key) model = getDiscriminatorByValue(model.discriminators, key) || model;
  return model;
}

Changes may be needed for other ops as well.

@lcrosetto
Copy link
Author

The above changes determine the schema in a similar way to query._castUpdate()

vkarpov15 added a commit that referenced this issue Nov 19, 2024
…ateOne and updateMany, allow inferring discriminator key from update

Fix #15040
@vkarpov15 vkarpov15 added this to the 8.8.3 milestone Nov 19, 2024
@vkarpov15 vkarpov15 added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Nov 19, 2024
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