Moving sub document within document creates weird validation error

[hardcodet]

Prerequisites

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

Mongoose version

6.1.0

Node.js version

16.x

MongoDB server version

Atlas (latest)

Typescript version (if applicable)

4.8.2

Description

(Note: Posted dirty workaround as an answer to this post)

I used to have a document that was tracking deployments, and I used to move a "pending" deployment to the "distributed" deployment field by simply setting pendingDeployment to null, and assigning the deployment to distributedDeployment

before:

{
  pendingDeployment: { id: "xxx", status: "abc" },
  distributedDeployment: null
}

after:

{
  pendingDeployment: null,
  distributedDeployment:  { id: "xxx", status: "abc" }
}

Since we introduced concurrent deployments, I replaced pendingDeployment by an array. The schema is slightly different, but the process is the same:

{
  pendingDeployments: [{ id: "xxx", status: "abc" }],
  distributedDeployment: null
}

// remove from array
ArrayUtil.removeFirst(target.deployments.pendingDeployments, d => d.id === deployment.id);

// reassign the document and adjust the status
deployments.distributedDeployment = deployment; // this one becomes the new active deployment
deployment.status = SnapshotDeploymentStatus.Deployed;

// update in MongoDb
await this.deploymentTargetRepository.update(target);

However, this crashes now with the following validation error:

DeploymentTarget validation failed: deployments.pendingDeployments.0.status: Path pendingDeployments.0.status is required.

The problem is that the pendingDeployments array is empty at this point! Strange enough: Even if I do not assign that deployment to the distributedDeployment field, I'm still getting the validation error. It appears that the deployment is still tracked as part of the array, and changing the status flag sort of messes up that update.

Steps to Reproduce

Create a subdocument that looks similar to this. Foo should contain a mandatory field I guess.

{
  fooArray: Foo[];
  singleFoo: Foo;
}

1. assign a subdocumente to the array and store the document
2. retrieve the document, remove the foo from the array, and assign it to singleFoo. Also change the value of the mandatory field.
3. Try to update the document

Expected Behavior

Changes should just be saved just like when the moved document wasn't in an array before.

[hardcodet]

I also found a dirty workaround for the issue. The fact that this works probably confirms that it's a bug. The only change is that I moved the line that changes the status above the line that removes the subdocument from the array:

before (as above):

// remove from array
ArrayUtil.removeFirst(target.deployments.pendingDeployments, d => d.id === deployment.id);

// reassign the document and adjust the status
deployments.distributedDeployment = deployment; // this one becomes the new active deployment
deployment.status = SnapshotDeploymentStatus.Deployed;

// update in MongoDb
await this.deploymentTargetRepository.update(target);

after:

// adjust the status before removing the subdocument from the array
deployment.status = SnapshotDeploymentStatus.Deployed;

// remove from array
ArrayUtil.removeFirst(target.deployments.pendingDeployments, d => d.id === deployment.id);

// reassign the document and adjust the status
deployments.distributedDeployment = deployment; // this one becomes the new active deployment

// update in MongoDb
await this.deploymentTargetRepository.update(target);

[vkarpov15]

@hardcodet I'm unable to repro, below script looks like it works as expected:

'use strict';

const mongoose = require('mongoose');

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

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

  const Test = mongoose.model('Test', mongoose.Schema({
    pendingDeployments: [{ id: String, status: String }],
    distributedDeployment: mongoose.Schema({ id: String, status: String })
  }));

  const { _id } = await Test.create({
    pendingDeployments: [{ id: '123', status: 'pending' }, { id: '456', status: 'pending' }],
    distributedDeployment: null
  });

  let doc = await Test.findById(_id);
  doc.pendingDeployments.splice(0, 1);
  doc.distributedDeployment = { id: '123', status: 'deployed' };
  await doc.save();

  console.log(await Test.findById(_id));
}

Output:

$ node gh-12761.js 
{
  _id: new ObjectId("638e17f74fe4020000f7f215"),
  pendingDeployments: [
    {
      id: '456',
      status: 'pending',
      _id: new ObjectId("638e17f74fe4020000f7f217")
    }
  ],
  distributedDeployment: {
    id: '123',
    status: 'deployed',
    _id: new ObjectId("638e17f74fe4020000f7f21c")
  },
  __v: 1
}
^C

Can you please provide the implementation of ArrayUtil.removeFirst and this.deploymentTargetRepository.update? There's a few ways to remove an element from an array in JavaScript.

[hardcodet]

Hi @vkarpov15

The big difference is that I'm actually moving the same object from the array into the field, while you are essentially deleting an item from the array, and then creating a new object (that looks the same which you then assign to the distributedDeployment field.

However, I tried to repro based on your script on my local test system, which is running version 6.6.5 and failed. I then ran the test on the affected system, which is running 6.1.0, which gave me the repro again. So I guess that issue was fixed in one of the recent releases :)