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

Throw when setting a key name that match the discriminator key name #12517

Closed
2 tasks done
abarriel opened this issue Oct 4, 2022 · 5 comments · Fixed by #12534
Closed
2 tasks done

Throw when setting a key name that match the discriminator key name #12517

abarriel opened this issue Oct 4, 2022 · 5 comments · Fixed by #12534
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@abarriel
Copy link
Contributor

abarriel commented Oct 4, 2022

Prerequisites

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

Mongoose version

6.6.3

Node.js version

16

MongoDB server version

4.9.1

Description

Hello, I have a discriminator key to kind in the root doc but I also have a subSchema which contains a field name kind.
When I am setting the sub field Kind, it throw an error.

Steps to Reproduce

const mongoose = require("mongoose");

mongoose.set("strict", "throw");

const options = { discriminatorKey: "kind"};

const EventSchema = new mongoose.Schema(
  { name: String, kind: String, animals: { kind: String, world: String } },
  options
);

const Event = mongoose.model("Event", EventSchema);

const ClickedLinkEvent = Event.discriminator(
  "ClickedLink",
  new mongoose.Schema({ url: String }, options)
);

async function main() {
  await mongoose.connect("mongodb://localhost:27017");

  const eventDoc = await Event.findByIdAndUpdate(
    { _id: mongoose.Types.ObjectId() },
    {
      $set: {
        name: "name",
        animals: { kind: "kind", world: "world" },
      },
    },
    {
    }
  );

  console.log(eventDoc);
}

main();

"Can't modify discriminator key \"kind\" on discriminator model"

Expected Behavior

No response

@abarriel
Copy link
Contributor Author

abarriel commented Oct 4, 2022

We should use prefix imo.
castUpdate.js#L172

const prefix = pref ? pref + '.' : '';

@hasezoey
Copy link
Collaborator

hasezoey commented Oct 4, 2022

it is not a error, because the discriminator-type can be changed via updates to the document (and mongoose will allow that), if you dont want this, maybe consider setting the path to be immutable via a index

@hasezoey hasezoey added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Oct 4, 2022
@abarriel
Copy link
Contributor Author

abarriel commented Oct 4, 2022

This is an error, I am not changing the discriminator key here @hasezoey
I am changing a key which happens to be the same name as a discriminator key on top level.
I am updating animals.kind and not kind

@hasezoey
Copy link
Collaborator

hasezoey commented Oct 4, 2022

I am changing a key which happens to be the same name as a discriminator key on top level.
I am updating animals.kind and not kind

didnt see that, thanks for clarifying

@hasezoey
Copy link
Collaborator

hasezoey commented Oct 4, 2022

can reproduce

Code
// NodeJS: 18.8.0
// MongoDB: 5.0 (Docker)
// Typescript 4.8.4
import * as mongoose from 'mongoose'; // mongoose@6.6.3

mongoose.set('strict', 'throw');

const options = { discriminatorKey: 'kind' };

const EventSchema = new mongoose.Schema({ name: String, kind: String, animals: { kind: String, world: String } }, options);

const Event = mongoose.model('Event', EventSchema);

const ClickedLinkEvent = Event.discriminator('ClickedLink', new mongoose.Schema({ url: String }, options));

(async () => {
  await mongoose.connect(`mongodb://localhost:27017/`, {
    dbName: 'verifyMASTER',
  });

  const eventDoc = await Event.findByIdAndUpdate(
    { _id: new mongoose.Types.ObjectId() },
    {
      $set: {
        name: 'name',
        // error "Can't modify discriminator key "kind" on discriminator model", even though its a different "kind"
        animals: { kind: 'kind', world: 'world' },
      },
    },
    {}
  );

  console.log(eventDoc);

  await mongoose.disconnect();
})();

Reproduction Repository / Branch: https://github.com/typegoose/typegoose-testing/tree/mongooseGh12517

output:

/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/helpers/query/castUpdate.js:201
        const err = new Error('Can\'t modify discriminator key "' + key + '" on discriminator model');
                    ^

Error: Can't modify discriminator key "kind" on discriminator model
    at walkUpdatePath (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/helpers/query/castUpdate.js:201:21)
    at walkUpdatePath (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/helpers/query/castUpdate.js:307:20)
    at castUpdate (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/helpers/query/castUpdate.js:97:7)
    at model.Query._castUpdate (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/query.js:5104:10)
    at Query._findAndModify (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/query.js:3989:29)
    at model.Query.<anonymous> (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/query.js:3470:8)
    at model.Query._wrappedThunk [as _findOneAndUpdate] (/mnt/projects/nodejs/typegoose-testing/node_modules/mongoose/lib/helpers/query/wrapThunk.js:29:8)
    at /mnt/projects/nodejs/typegoose-testing/node_modules/kareem/index.js:426:25
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

@hasezoey hasezoey added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Oct 4, 2022
@vkarpov15 vkarpov15 added this to the 6.6.6 milestone Oct 5, 2022
lpizzinidev added a commit to lpizzinidev/mongoose that referenced this issue Oct 6, 2022
lpizzinidev added a commit to lpizzinidev/mongoose that referenced this issue Oct 10, 2022
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
3 participants