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

When saving a model with bulkSave, the values in the discriminator schema are not updated. #13907

Closed
2 tasks done
0skgc opened this issue Sep 29, 2023 · 1 comment · Fixed by #13959
Closed
2 tasks done
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@0skgc
Copy link

0skgc commented Sep 29, 2023

Prerequisites

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

Mongoose version

7.5.3

Node.js version

18.16.0

MongoDB server version

6.0.10

Typescript version (if applicable)

5.2.2

Description

When saving a model with bulkSave, the properties of the discriminator schema are not updated.
When I try the same update with model.prototype.save, it is updated, so is this a bug with bulkSave?

I followed the internal process of bulkSave in the debugger, and it looks like updates to paths that are not in the base schema (updates to the discriminator schema) are deleted in the walkUpdatePath function in castUpdate.js.

Steps to Reproduce

Test code that can reproduce the problem

import mongoose from "mongoose"
import { beforeEach, test, expect } from "vitest"

const schema = new mongoose.Schema({ value: String }, { discriminatorKey: "type" })
const typeASchema = new mongoose.Schema({ aValue: String })
schema.discriminator("A", typeASchema)

type IBaseTest = mongoose.InferSchemaType<typeof schema>
type ITypeATest = IBaseTest & { type: "A" } & mongoose.InferSchemaType<typeof typeASchema>
type ITest = ITypeATest

const TestModel = mongoose.model<ITest>("Test", schema)

describe("mongoose buksSave test", () => {
  beforeEach(async () => {
    await TestModel.deleteMany({})
  })

  const testData: ITypeATest = { value: "initValue", type: "A", aValue: "initAValue" }

  test("bulkSave", async () => {
    // Arrange
    const doc = await TestModel.create<ITypeATest>(testData)

    // Act
    doc.value = "updatedValue1"
    doc.aValue = "updatedValue2"
    await TestModel.bulkSave([doc])

    // Assert (!!!aValue is not updated)
    const findDoc = await TestModel.findById(doc._id)
    expect(findDoc).toMatchInlineSnapshot(`
      {
        "__v": 0,
        "_id": "${doc._id.toString()}",
        "aValue": "initAValue",
        "type": "A",
        "value": "updatedValue1",
      }
    `)
  })

  test("save", async () => {
    // Arrange
    const doc = await TestModel.create<ITypeATest>(testData)

    // Act
    doc.value = "updatedValue1"
    doc.aValue = "updatedValue2"
    await doc.save()

    // Assert (both value and aValue are updated)
    const findDoc = await TestModel.findById(doc._id)
    expect(findDoc).toMatchInlineSnapshot(`
      {
        "__v": 0,
        "_id": "${doc._id.toString()}",
        "aValue": "updatedValue2",
        "type": "A",
        "value": "updatedValue1",
      }
    `)
  })
})

Expected Behavior

bulkSave also updates the properties of the discriminator in the same way as save.

@vkarpov15 vkarpov15 added this to the 7.5.5 milestone Oct 1, 2023
@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 1, 2023
@IslandRhythms
Copy link
Collaborator

const schema = new mongoose.Schema({ value: String }, { discriminatorKey: "type" })
const typeASchema = new mongoose.Schema({ aValue: String })
schema.discriminator("A", typeASchema)

const TestModel = mongoose.model("Test", schema);


async function run() {
  await mongoose.connect('mongodb://127.0.0.1:27017');
  await mongoose.connection.dropDatabase();

  const testData = { value: "initValue", type: "A", aValue: "initAValue" }
    // Arrange
    const doc = await TestModel.create(testData)

    // Act
    doc.value = "updatedValue1"
    doc.aValue = "updatedValue2"
    await TestModel.bulkSave([doc])

    // Assert (!!!aValue is not updated)
    const findDoc = await TestModel.findById(doc._id)
    /*
      {
        "__v": 0,
        "_id": "${doc._id.toString()}",
        "aValue": "initAValue",
        "type": "A",
        "value": "updatedValue1",
      }
    */
    console.log('what is findDoc', findDoc);

    const doc2 = await TestModel.create(testData);
    doc2.value = "updatedValue1"
    doc2.aValue = "updatedValue2"
    await doc2.save()
    /*
      {
        "__v": 0,
        "_id": "${doc._id.toString()}",
        "aValue": "updatedValue2",
        "type": "A",
        "value": "updatedValue1",
      }
    */
    const findDoc2 = await TestModel.findById(doc2._id);
    console.log('what is findDoc2', findDoc2);
}

run();

@IslandRhythms IslandRhythms 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 Oct 2, 2023
vkarpov15 added a commit that referenced this issue Oct 9, 2023
vkarpov15 added a commit that referenced this issue Oct 10, 2023
fix(model): make bulkSave() save changes in discriminator paths if calling bulkSave() on base model
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