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

Data loss pushing items to list when sorting list and saving multiple times #12656

Closed
2 tasks done
johnpeb opened this issue Nov 4, 2022 · 2 comments · Fixed by #12672
Closed
2 tasks done

Data loss pushing items to list when sorting list and saving multiple times #12656

johnpeb opened this issue Nov 4, 2022 · 2 comments · Fixed by #12672
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@johnpeb
Copy link
Contributor

johnpeb commented Nov 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.7.0

Node.js version

16.13.1

MongoDB server version

4.2.8

Typescript version (if applicable)

4.8.4

Description

The provided test case shows data loss pushing items to list/subcollection when sorting the subcollection AND saving multiple times.

If the orderBy() lines are omitted, the result is correct OR if the first doc.save() is omitted, the result is correct.

This test case shows data loss, but a similar flow causes duplicate items to be inserted into the array on the 2nd save.

Steps to Reproduce

import * as assert from 'assert'
import { orderBy } from 'lodash'
import { Schema } from 'mongoose'
import * as mongoose from 'mongoose'

run().catch(err => console.log(err))

/**
 * npx ts-node scripts/mongoose.ts
 */
async function run (): Promise<void> {
  await mongoose.connect('mongodb://localhost:27017/test-mongoose', {})
  await mongoose.connection.dropDatabase()
  mongoose.set('debug', true)

  const Test = mongoose.model('Test', new Schema({
    list: [{
      // _id: false,
      a: Number
    }]
  }))

  await Test.create({ list: [{ a: 1, b: 11 }] })

  let doc = await Test.findOne()
  doc.list.push({ a: 2 })
  doc.list = orderBy(doc.list, 'a', 'asc') // Same behavior if line replaced with: doc.list = [...doc.list]
  await doc.save()

  doc.list.push({ a: 3 })
  doc.list = orderBy(doc.list, 'a', 'asc') // Same behavior if line replaced with: doc.list = [...doc.list]
  await doc.save()

  doc = await Test.findOne()
  assert(doc.list.length === 3, `Got ${JSON.stringify(
    // @ts-expect-error toObject() not typed
    doc.list.toObject()
  )}`)

  process.exit()
}

Actual output. Note that the {a: 3} element is missing. If the first doc.save() is omitted the output is correct.

% npx ts-node scripts/mongoose.ts
Mongoose: tests.insertOne({ list: [ { a: 1, _id: new ObjectId("6365530081dd159228ce5c94") } ], _id: new ObjectId("6365530081dd159228ce5c93"), __v: 0}, { session: null })
Mongoose: tests.findOne({}, { projection: {} })
Mongoose: tests.updateOne({ _id: new ObjectId("6365530081dd159228ce5c93") }, { '$push': { list: { '$each': [ { a: 2, _id: new ObjectId("6365530081dd159228ce5c98") } ] } }, '$inc': { __v: 1 }}, { session: null })
Mongoose: tests.findOne({ _id: new ObjectId("6365530081dd159228ce5c93") }, { session: null, projection: { _id: 1 } })
Mongoose: tests.findOne({}, { projection: {} })
AssertionError [ERR_ASSERTION]: Got [{"a":1,"_id":"6365530081dd159228ce5c94"},{"a":2,"_id":"6365530081dd159228ce5c98"}]
    at /Users/johnpeberdy/Documents/Code/phenix/scripts/mongoose.ts:47:3
    at Generator.next (<anonymous>)
    at fulfilled (/Users/johnpeberdy/Documents/Code/phenix/scripts/mongoose.ts:5:58)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Expected Behavior

The result should not change if the first doc.save() is omitted. If this use case is not supported, then it should fail loudly.

@johnpeb
Copy link
Contributor Author

johnpeb commented Nov 4, 2022

Workaround: replace doc.list.push(x) with doc.list = [...doc.list, x]

@hasezoey hasezoey added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Nov 8, 2022
@vkarpov15 vkarpov15 added this to the 6.7.3 milestone Nov 9, 2022
@vkarpov15 vkarpov15 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 Nov 9, 2022
vkarpov15 added a commit that referenced this issue Nov 12, 2022
fix(document): handle setting array to itself after saving and pushing a new value
@johnpeb
Copy link
Contributor Author

johnpeb commented Nov 14, 2022

Thanks @vkarpov15 !

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