Merge pull request #13544 from hasezoey/fix1731

feat(model): add create option "immediateError"

lib/model.js

@@ -478,7 +478,6 @@ function generateVersionError(doc, modifiedPaths) {
  *     newProduct === product; // true
  *
  * @param {Object} [options] options optional options
- * @param {Boolean} [options.ordered] saves the docs in series rather than parallel.
  * @param {Session} [options.session=null] the [session](https://www.mongodb.com/docs/manual/reference/server-sessions/) associated with this save operation. If not specified, defaults to the [document's associated session](https://mongoosejs.com/docs/api/document.html#Document.prototype.session()).
  * @param {Object} [options.safe] (DEPRECATED) overrides [schema's safe option](https://mongoosejs.com/docs/guide.html#safe). Use the `w` option instead.
  * @param {Boolean} [options.validateBeforeSave] set to false to save without validating.
@@ -2809,6 +2808,8 @@ Model.findByIdAndRemove = function(id, options) {
  *
  * @param {Array|Object} docs Documents to insert, as a spread or array
  * @param {Object} [options] Options passed down to `save()`. To specify `options`, `docs` **must** be an array, not a spread. See [Model.save](https://mongoosejs.com/docs/api/model.html#Model.prototype.save()) for available options.
+ * @param {Boolean} [options.ordered] saves the docs in series rather than parallel.
+ * @param {Boolean} [options.aggregateErrors] Aggregate Errors instead of throwing the first one that occurs. Default: false
  * @return {Promise}
  * @api public
  */
@@ -2865,27 +2866,41 @@ Model.create = async function create(doc, options) {
     return Array.isArray(doc) ? [] : null;
   }
   let res = [];
+  const immediateError = typeof options.aggregateErrors === 'boolean' ? !options.aggregateErrors : true;
+
+  delete options.aggregateErrors; // dont pass on the option to "$save"
+
   if (options.ordered) {
     for (let i = 0; i < args.length; i++) {
-      const doc = args[i];
-      const Model = this.discriminators && doc[discriminatorKey] != null ?
-        this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) :
-        this;
-      if (Model == null) {
-        throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` +
+      try {
+        const doc = args[i];
+        const Model = this.discriminators && doc[discriminatorKey] != null ?
+          this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) :
+          this;
+        if (Model == null) {
+          throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` +
           `found for model "${this.modelName}"`);
-      }
-      let toSave = doc;
-      if (!(toSave instanceof Model)) {
-        toSave = new Model(toSave);
-      }
+        }
+        let toSave = doc;
+        if (!(toSave instanceof Model)) {
+          toSave = new Model(toSave);
+        }
 
-      await toSave.$save(options);
-      res.push(toSave);
+        await toSave.$save(options);
+        res.push(toSave);
+      } catch (err) {
+        if (!immediateError) {
+          res.push(err);
+        } else {
+          throw err;
+        }
+      }
     }
     return res;
   } else {
-    res = await Promise.all(args.map(async doc => {
+    // ".bind(Promise)" is required, otherwise results in "TypeError: Promise.allSettled called on non-object"
+    const promiseType = !immediateError ? Promise.allSettled.bind(Promise) : Promise.all.bind(Promise);
+    let p = promiseType(args.map(async doc => {
       const Model = this.discriminators && doc[discriminatorKey] != null ?
         this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) :
         this;
@@ -2903,6 +2918,13 @@ Model.create = async function create(doc, options) {
 
       return toSave;
     }));
+
+    // chain the mapper, only if "allSettled" is used
+    if (!immediateError) {
+      p = p.then(presult => presult.map(v => v.status === 'fulfilled' ? v.value : v.reason));
+    }
+
+    res = await p;
   }
 
 

test/model.create.test.js

@@ -197,6 +197,52 @@ describe('model', function() {
         const docs = await Test.find();
         assert.equal(docs.length, 5);
       });
+
+      it('should return the first error immediately if "aggregateErrors" is not explicitly set (ordered)', async function() {
+        const testSchema = new Schema({ name: { type: String, required: true } });
+
+        const TestModel = db.model('gh1731-1', testSchema);
+
+        const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true }).then(null).catch(err => err);
+
+        assert.ok(res instanceof mongoose.Error.ValidationError);
+      });
+
+      it('should not return errors immediately if "aggregateErrors" is "true" (ordered)', async function() {
+        const testSchema = new Schema({ name: { type: String, required: true } });
+
+        const TestModel = db.model('gh1731-2', testSchema);
+
+        const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true, aggregateErrors: true });
+
+        assert.equal(res.length, 3);
+        assert.ok(res[0] instanceof mongoose.Document);
+        assert.ok(res[1] instanceof mongoose.Error.ValidationError);
+        assert.ok(res[2] instanceof mongoose.Document);
+      });
+    });
+
+    it('should return the first error immediately if "aggregateErrors" is not explicitly set', async function() {
+      const testSchema = new Schema({ name: { type: String, required: true } });
+
+      const TestModel = db.model('gh1731-3', testSchema);
+
+      const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], {}).then(null).catch(err => err);
+
+      assert.ok(res instanceof mongoose.Error.ValidationError);
+    });
+
+    it('should not return errors immediately if "aggregateErrors" is "true"', async function() {
+      const testSchema = new Schema({ name: { type: String, required: true } });
+
+      const TestModel = db.model('gh1731-4', testSchema);
+
+      const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { aggregateErrors: true });
+
+      assert.equal(res.length, 3);
+      assert.ok(res[0] instanceof mongoose.Document);
+      assert.ok(res[1] instanceof mongoose.Error.ValidationError);
+      assert.ok(res[2] instanceof mongoose.Document);
     });
   });
 });

test/types/create.test.ts

@@ -1,4 +1,4 @@
-import { Schema, model, Types, CallbackError } from 'mongoose';
+import { Schema, model, Types, HydratedDocument } from 'mongoose';
 import { expectError, expectType } from 'tsd';
 
 const schema = new Schema({ name: { type: 'String' } });
@@ -118,3 +118,10 @@ Test.insertMany({ _id: new Types.ObjectId('000000000000000000000000'), name: 'te
   (await Test.create([{ name: 'test' }]))[0];
   (await Test.create({ name: 'test' }))._id;
 })();
+
+async function createWithAggregateErrors() {
+  expectType<(HydratedDocument<ITest>)[]>(await Test.create([{}]));
+  expectType<(HydratedDocument<ITest> | Error)[]>(await Test.create([{}], { aggregateErrors: true }));
+}
+
+createWithAggregateErrors();

types/models.d.ts

@@ -123,7 +123,6 @@ declare module 'mongoose' {
     SessionOption {
     checkKeys?: boolean;
     j?: boolean;
-    ordered?: boolean;
     safe?: boolean | WriteConcern;
     timestamps?: boolean | QueryTimestampsConfig;
     validateBeforeSave?: boolean;
@@ -132,6 +131,11 @@ declare module 'mongoose' {
     wtimeout?: number;
   }
 
+  interface CreateOptions extends SaveOptions {
+    ordered?: boolean;
+    aggregateErrors?: boolean;
+  }
+
   interface RemoveOptions extends SessionOption, Omit<mongodb.DeleteOptions, 'session'> {}
 
   const Model: Model<any>;
@@ -217,7 +221,8 @@ declare module 'mongoose' {
     >;
 
     /** Creates a new document or documents */
-    create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options?: SaveOptions): Promise<THydratedDocumentType[]>;
+    create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options: CreateOptions & { aggregateErrors: true }): Promise<(THydratedDocumentType | Error)[]>;
+    create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options?: CreateOptions): Promise<THydratedDocumentType[]>;
     create<DocContents = AnyKeys<TRawDocType>>(doc: DocContents | TRawDocType): Promise<THydratedDocumentType>;
     create<DocContents = AnyKeys<TRawDocType>>(...docs: Array<TRawDocType | DocContents>): Promise<THydratedDocumentType[]>;