diff --git a/lib/document.js b/lib/document.js index 568fe653719..4d228c3b82b 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 applyDefaults = require('./helpers/document/applyDefaults'); const cleanModifiedSubpaths = require('./helpers/document/cleanModifiedSubpaths'); const compile = require('./helpers/document/compile').compile; @@ -1567,6 +1568,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) @@ -1636,6 +1643,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 d7fe08d5b87..c4445c254d6 100644 --- a/lib/internal.js +++ b/lib/internal.js @@ -29,6 +29,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 9de649739e4..36eb73bbdab 100644 --- a/lib/model.js +++ b/lib/model.js @@ -37,6 +37,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'); @@ -591,6 +592,7 @@ function operand(self, where, delta, data, val, op) { case '$pullAll': case '$push': case '$addToSet': + case '$inc': break; default: // nothing to do @@ -766,15 +768,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..26f253079a1 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', 1); + 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() { 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