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

Manually populating doesn't work for embedded discriminators in embedded array #8273

Closed
saveman71 opened this issue Oct 23, 2019 · 8 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@saveman71
Copy link
Contributor

saveman71 commented Oct 23, 2019

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Manually populated as described in the docs doesn't work for discriminators.

Repro script (sorry a bit long):

// Modules
const Promise = require('bluebird');
const mongoose = require('mongoose');
mongoose.Promise = Promise;
const Schema = mongoose.Schema;

console.log(mongoose.version);

// DB
let connect = mongoose.connect('mongodb://localhost:27018/test');

// Models
const AddonSchema = new Schema({
  title: String,
});
const Addon = mongoose.model('Addon', AddonSchema);
const AddonItemSchema = new Schema({
  addon: { type: Schema.Types.ObjectId, ref: 'Addon' },
  quantity: Number,
});

const ProductSchema = new Schema({
  title: String,
});
const Product = mongoose.model('Product', ProductSchema);
const ProductItemSchema = new Schema({
  product: { type: Schema.Types.ObjectId, ref: 'Product' },
});

const OrderItemSchema = new mongoose.Schema({
  quantity: Number,
}, {discriminatorKey: '__t'});
const OrderItem = mongoose.model('OrderItem', OrderItemSchema);


const OrderSchema = new Schema({
  product: { type: Schema.Types.ObjectId, ref: 'Product' },
  noDiscriminator: {
    type: [{
      product: { type: Schema.Types.ObjectId, ref: 'Product' },
    }],
    default: [],
  },
  discriminatorNoArray: {
    type: OrderItemSchema,
  },
  items: {
    type: [OrderItemSchema],
    default: [],
  },
});

const docArray = OrderSchema.path('items');

const ProductItem = docArray
  .discriminator('ProductItem', ProductItemSchema);
const AddonItem = docArray
  .discriminator('AddonItem', AddonItemSchema);

OrderSchema.path('discriminatorNoArray').discriminator('ProductItem', ProductItemSchema);
OrderSchema.path('discriminatorNoArray').discriminator('AddonItem', AddonItemSchema);

const Order = mongoose.model('Order', OrderSchema);

// Create test data
connect.then(async () => {
  mongoose.connection.db.dropDatabase();

  const addon = await Addon({title: 'Addon title'}).save();
  const product = await Product({title: 'Product title'}).save();

  // Create order with embedded items
  const unsaved = new Order({
    product,
    noDiscriminator: [
      {
        product,
      }
    ],
    discriminatorNoArray: {
      product,
      quantity: 42,
      __t: 'ProductItem',
    },
    items: [
      {
        addon,
        quantity: 42,
        __t: 'AddonItem',
      },
      {
        product,
        quantity: 42,
        __t: 'ProductItem',
      }
    ]
  });

  // Doesn't show manually populated in items
  console.log(unsaved);

  await unsaved.save();

  // Find order, populate addon of items
  const order = await Order.findOne({}).exec();

  order.product = product;
  order.noDiscriminator[0].product = product;
  order.discriminatorNoArray.product = product;
  order.items[0].addon = addon;
  order.items[0].addon = addon;

  // Doesn't show manually populated in items
  console.log(order);
});

What are the versions of Node.js, Mongoose and MongoDB you are using?

Node.js v8.11.0
Mongoose v5.7.6

@saveman71
Copy link
Contributor Author

saveman71 commented Oct 26, 2019

I've tried to debug this a bit:

here's my unit test (i added an other working product in the root of Order just to tests things):

      it.only('with discriminators in embedded arrays (gh-8273)', function(done) {
        const ProductSchema = new Schema({
          title: String,
        });
        const Product = mongoose.model('gh8273_Product', ProductSchema);
        const ProductItemSchema = new Schema({
          product: { type: Schema.Types.ObjectId, ref: 'gh8273_Product' },
        });

        const OrderItemSchema = new Schema({}, {discriminatorKey: '__t'});

        const OrderSchema = new Schema({
          product: { type: Schema.Types.ObjectId, ref: 'gh8273_Product' },
          items: [OrderItemSchema],
        });

        OrderSchema.path('items').discriminator('ProductItem', ProductItemSchema);
        const Order = mongoose.model('Order', OrderSchema);

        const product = new Product({title: 'Product title'});

        const order = new Order({
          product,
          items: [
            {
              product,
              __t: 'ProductItem',
            },
          ]
        });
        console.log(order);
        assert.ok(order.items[0].product.title);

        done();
      });

I've isolated it to the fact that during the setting process, the subdocument is casted to the object id ref, which is supposed to check if it's populated to, i guess, either allow the document to pass through or to just keep the id.

Cast calls SchemaType._isRef, which calls populated() on the owner document, here with the path of items.product which seems (?) wrong, as this should be a more "advanced" check, as i understand it as it's an array.

The check fails and this set operation is casted to the not populated version.

@saveman71
Copy link
Contributor Author

From what I got the populated state is stored in the "local" scope of the embedded item here

mongoose/lib/document.js

Lines 1132 to 1135 in 9549015

if (refMatches && val instanceof Document) {
this.populated(path, val._id, { [populateModelSymbol]: val.constructor });
didPopulate = true;
}

Then here

mongoose/lib/schematype.js

Lines 1266 to 1268 in 9549015

const path = doc.$__fullPath(self.path);
const owner = doc.ownerDocument ? doc.ownerDocument() : doc;
ref = owner.populated(path);

The population is checked against the full path (items.product) and against the owner document that doesn't know about that population.

@saveman71
Copy link
Contributor Author

@vkarpov15 anything I can do to help from there? I feel i'm lacking a deep understanding of how things work to submit a patch

@saveman71
Copy link
Contributor Author

Also, this seems very similar to #3575

@vkarpov15 vkarpov15 added this to the 5.7.8 milestone Nov 2, 2019
@vkarpov15 vkarpov15 added has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue 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 2, 2019
@vkarpov15
Copy link
Collaborator

@saveman71 thanks for the test script, that was very helpful. You were right about the item.populated and SchemaType._isRef() bit. I opened up a new issue #8298 to track refactoring that.

@vkarpov15
Copy link
Collaborator

In the meantime, the fix for this issue will be in 5.7.8

@saveman71
Copy link
Contributor Author

Thanks for the fix! 🎉

Just a small question, regarding dfde779. Wouldn't

mongoose/lib/document.js

Lines 1136 to 1139 in dfde779

if (refMatches && val instanceof Document) {
this.populated(path, val._id, { [populateModelSymbol]: val.constructor });
didPopulate = true;
}
still be run anyway? Isn't there a side effect?

@vkarpov15
Copy link
Collaborator

@saveman71 yes it will be run, but this.populated(path, val._id) will set populated() on the nested subdoc, not the top-level doc.

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
Development

No branches or pull requests

2 participants