From f4c436aa210b32d71c7f9ef0a83ef05abcdfa6ec Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 17 Jul 2022 20:03:15 -0400 Subject: [PATCH 1/3] feat(document): add `$inc()` helper that increments numeric paths Fix #11915 --- lib/document.js | 69 ++++++++++++++ lib/helpers/firstKey.js | 8 ++ lib/internal.js | 1 + lib/model.js | 26 ++++-- test/document.test.js | 196 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 291 insertions(+), 9 deletions(-) create mode 100644 lib/helpers/firstKey.js diff --git a/lib/document.js b/lib/document.js index 72c12c75442..1c7b4f72537 100644 --- a/lib/document.js +++ b/lib/document.js @@ -18,6 +18,7 @@ const ValidatorError = require('./error/validator'); const VirtualType = require('./virtualtype'); const $__hasIncludedChildren = require('./helpers/projection/hasIncludedChildren'); const promiseOrCallback = require('./helpers/promiseOrCallback'); +const castNumber = require('./cast/number'); const cleanModifiedSubpaths = require('./helpers/document/cleanModifiedSubpaths'); const compile = require('./helpers/document/compile').compile; const defineKey = require('./helpers/document/compile').defineKey; @@ -1665,6 +1666,12 @@ Document.prototype.$__set = function(pathToMark, path, options, constructing, pa schema, val, priorVal); if (shouldModify) { + if (this.$__.primitiveAtomics && this.$__.primitiveAtomics[path]) { + delete this.$__.primitiveAtomics[path]; + if (Object.keys(this.$__.primitiveAtomics).length === 0) { + delete this.$__.primitiveAtomics; + } + } this.markModified(pathToMark); // handle directly setting arrays (gh-1126) @@ -1734,6 +1741,68 @@ Document.prototype.$__getValue = function(path) { return utils.getValue(path, this._doc); }; +/** + * Increments the numeric value at `path` by the given `val`. + * When you call `save()` on this document, Mongoose will send a + * [`$inc`](https://www.mongodb.com/docs/manual/reference/operator/update/inc/) + * as opposed to a `$set`. + * + * #### Example: + * const schema = new Schema({ counter: Number }); + * const Test = db.model('Test', schema); + * + * const doc = await Test.create({ counter: 0 }); + * doc.$inc('counter', 2); + * await doc.save(); // Sends a `{ $inc: { counter: 2 } }` to MongoDB + * doc.counter; // 2 + * + * doc.counter += 2; + * await doc.save(); // Sends a `{ $set: { counter: 2 } }` to MongoDB + * + * @param {String|Array} path path or paths to update + * @param {Number} val increment `path` by this value + * @return {Document} this + */ + +Document.prototype.$inc = function $inc(path, val) { + if (val == null) { + val = 1; + } + + if (Array.isArray(path)) { + path.forEach((p) => this.$inc(p, val)); + return this; + } + + const schemaType = this.$__path(path); + if (schemaType == null) { + if (this.$__.strictMode === 'throw') { + throw new StrictModeError(path); + } else if (this.$__.strictMode === true) { + return this; + } + } else if (schemaType.instance !== 'Number') { + this.invalidate(path, new MongooseError.CastError(schemaType.instance, val, path)); + return this; + } + + try { + val = castNumber(val); + } catch (err) { + this.invalidate(path, new MongooseError.CastError('number', val, path, err)); + } + + const currentValue = this.$__getValue(path); + + this.$__setValue(path, currentValue + val); + + this.$__.primitiveAtomics = this.$__.primitiveAtomics || {}; + this.$__.primitiveAtomics[path] = { $inc: val }; + this.markModified(path); + + return this; +}; + /** * Sets a raw value for a path (no casting, setters, transformations) * diff --git a/lib/helpers/firstKey.js b/lib/helpers/firstKey.js new file mode 100644 index 00000000000..4495d2a8860 --- /dev/null +++ b/lib/helpers/firstKey.js @@ -0,0 +1,8 @@ +'use strict'; + +module.exports = function firstKey(obj) { + if (obj == null) { + return null; + } + return Object.keys(obj)[0]; +}; diff --git a/lib/internal.js b/lib/internal.js index a31714825f9..9cd55ef9d8b 100644 --- a/lib/internal.js +++ b/lib/internal.js @@ -28,6 +28,7 @@ InternalCache.prototype._id = undefined; InternalCache.prototype.ownerDocument = undefined; InternalCache.prototype.populate = undefined; // what we want to populate in this doc InternalCache.prototype.populated = undefined;// the _ids that have been populated +InternalCache.prototype.primitiveAtomics = undefined; /** * If `false`, this document was not the result of population. diff --git a/lib/model.js b/lib/model.js index 26fb9a511a0..78dfa03701b 100644 --- a/lib/model.js +++ b/lib/model.js @@ -35,6 +35,7 @@ const castBulkWrite = require('./helpers/model/castBulkWrite'); const createPopulateQueryFilter = require('./helpers/populate/createPopulateQueryFilter'); const getDefaultBulkwriteResult = require('./helpers/getDefaultBulkwriteResult'); const discriminator = require('./helpers/model/discriminator'); +const firstKey = require('./helpers/firstKey'); const each = require('./helpers/each'); const get = require('./helpers/get'); const getConstructorName = require('./helpers/getConstructorName'); @@ -588,6 +589,7 @@ function operand(self, where, delta, data, val, op) { case '$pullAll': case '$push': case '$addToSet': + case '$inc': break; default: // nothing to do @@ -763,15 +765,21 @@ Model.prototype.$__delta = function() { value = value.toObject(); operand(this, where, delta, data, value); } else { - value = utils.clone(value, { - depopulate: true, - transform: false, - virtuals: false, - getters: false, - omitUndefined: true, - _isNested: true - }); - operand(this, where, delta, data, value); + if (this.$__.primitiveAtomics && this.$__.primitiveAtomics[data.path] != null) { + const val = this.$__.primitiveAtomics[data.path]; + const op = firstKey(val); + operand(this, where, delta, data, val[op], op); + } else { + value = utils.clone(value, { + depopulate: true, + transform: false, + virtuals: false, + getters: false, + omitUndefined: true, + _isNested: true + }); + operand(this, where, delta, data, value); + } } } diff --git a/test/document.test.js b/test/document.test.js index 160b14fbe3d..341f90d33d9 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11551,6 +11551,202 @@ describe('document', function() { assert.ok(err); assert.ok(err.errors['testProp.testSubProp.nested.from']); }); + + describe('$inc (gh-11915)', function() { + describe('top-level path', function() { + let Test; + + beforeEach(function() { + const schema = new Schema({ + counter: Number + }); + Test = db.model('Test', schema); + }); + + it('sends a $inc command for a given path', async function() { + await Test.create({ counter: 0 }); + const doc = await Test.findOne(); + assert.strictEqual(doc.counter, 0); + const doc2 = await Test.findOne(); + + doc2.counter = 1; + await doc2.save(); + + doc.$inc('counter'); + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.counter, 2); + }); + + it('works as a $set if the document is new', async function() { + const doc = new Test({ counter: 0 }); + doc.$inc('counter', 2); + assert.equal(doc.counter, 2); + + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.counter, 2); + }); + + it('treats as a $set if set after $inc', async function() { + await Test.create({ counter: 0 }); + const doc = await Test.findOne(); + + doc.$inc('counter', 2); + doc.counter = 5; + assert.deepStrictEqual(doc.getChanges(), { $set: { counter: 5 } }); + await doc.save(); + + const res = await Test.findOne(); + assert.equal(res.counter, 5); + }); + + it('tries to cast to number', async function() { + await Test.create({ counter: 0 }); + const doc = await Test.findOne(); + + doc.$inc('counter', '2'); + assert.deepStrictEqual(doc.getChanges(), { $inc: { counter: 2 } }); + await doc.save(); + + const res = await Test.findOne(); + assert.equal(res.counter, 2); + }); + + it('stores CastError if can\'t convert to number', async function() { + await Test.create({ counter: 0 }); + const doc = await Test.findOne(); + + doc.$inc('counter', 'foobar'); + const err = await doc.save().then(() => null, err => err); + assert.ok(err); + assert.equal(err.errors['counter'].name, 'CastError'); + }); + }); + + describe('nested paths', function() { + let Test; + + beforeEach(function() { + const schema = new Schema({ + nested: { + counter: Number + } + }); + Test = db.model('Test', schema); + }); + + it('handles nested paths', async function() { + await Test.create({ nested: { counter: 0 } }); + const doc = await Test.findOne(); + + doc.$inc('nested.counter', 2); + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.nested.counter, 2); + }); + + it('treats as $set if overwriting nested path', async function() { + await Test.create({ nested: { counter: 0 } }); + const doc = await Test.findOne(); + + doc.$inc('nested.counter', 2); + doc.nested.counter += 3; + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.nested.counter, 5); + }); + }); + + describe('subdocuments', function() { + let Test; + + beforeEach(function() { + const schema = new Schema({ + subdoc: new Schema({ + counter: Number + }) + }); + Test = db.model('Test', schema); + }); + + it('handles paths underneath subdocuments', async function() { + await Test.create({ subdoc: { counter: 0 } }); + const doc = await Test.findOne(); + + doc.$inc('subdoc.counter', 2); + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.subdoc.counter, 2); + }); + + it('treats as a $set if setting subdocument after $inc', async function() { + await Test.create({ subdoc: { counter: 0 } }); + const doc = await Test.findOne(); + + doc.$inc('subdoc.counter', 2); + doc.subdoc = { counter: 5 }; + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.subdoc.counter, 5); + }); + }); + + describe('document array', function() { + let Test; + + beforeEach(function() { + const schema = new Schema({ + docArr: [{ counter: Number }] + }); + Test = db.model('Test', schema); + }); + + it('handles paths underneath subdocuments', async function() { + await Test.create({ docArr: [{ counter: 0 }] }); + const doc = await Test.findOne(); + + doc.docArr[0].$inc('counter'); + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.docArr[0].counter, 1); + }); + + it('works on pushed subdocs', async function() { + await Test.create({ docArr: [] }); + const doc = await Test.findOne(); + + doc.docArr.push({ counter: 0 }); + doc.docArr[0].$inc('counter'); + await doc.save(); + + const res = await Test.findById(doc); + assert.equal(res.docArr[0].counter, 1); + }); + }); + + it('stores CastError if trying to $inc a non-numeric path', async function() { + const schema = new Schema({ + prop: String + }); + const Test = db.model('Test', schema); + + await Test.create({ prop: '' }); + const doc = await Test.findOne(); + + doc.$inc('prop', 2); + const err = await doc.save().then(() => null, err => err); + assert.ok(err); + assert.equal(err.errors['prop'].name, 'CastError'); + }); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From 130337d5fece6bd5bd073bd700b1191749493b82 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 17 Jul 2022 20:09:03 -0400 Subject: [PATCH 2/3] feat(types): add `$inc()` to type definitions --- types/document.d.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/types/document.d.ts b/types/document.d.ts index adf170cc42a..44c74022141 100644 --- a/types/document.d.ts +++ b/types/document.d.ts @@ -43,6 +43,13 @@ declare module 'mongoose' { /** Returns an array of all populated documents associated with the query */ $getPopulatedDocs(): Document[]; + /** + * Increments the numeric value at `path` by the given `val`. + * When you call `save()` on this document, Mongoose will send a + * `$inc` as opposed to a `$set`. + */ + $inc(path: string | string[], val?: number): this; + /** * Returns true if the given path is nullish or only contains empty objects. * Useful for determining whether this subdoc will get stripped out by the From 00cf1c7752715fa9ebb42b1e5fcdba71e51fcaa5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 18 Jul 2022 10:44:50 -0400 Subject: [PATCH 3/3] Update test/document.test.js Co-authored-by: Hafez --- test/document.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/document.test.js b/test/document.test.js index 341f90d33d9..26f253079a1 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11572,7 +11572,7 @@ describe('document', function() { doc2.counter = 1; await doc2.save(); - doc.$inc('counter'); + doc.$inc('counter', 1); await doc.save(); const res = await Test.findById(doc);