diff --git a/lib/model.js b/lib/model.js index 7e983427e3c..88eff90b945 100644 --- a/lib/model.js +++ b/lib/model.js @@ -309,8 +309,14 @@ Model.prototype.$__handleSave = function(options, callback) { callback(null, ret); }); } else { - this.$__reset(); - callback(); + this.constructor.exists(this.$__where()) + .then((documentExists)=>{ + if (!documentExists) throw new DocumentNotFoundError(this.$__where(),this.constructor.modelName); + + this.$__reset(); + callback(); + }) + .catch(callback); return; } diff --git a/test/document.test.js b/test/document.test.js index e3ee5e00ab4..a87e805b6b2 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -25,6 +25,7 @@ const SchemaType = mongoose.SchemaType; const ValidatorError = SchemaType.ValidatorError; const ValidationError = mongoose.Document.ValidationError; const MongooseError = mongoose.Error; +const DocumentNotFoundError = mongoose.Error.DocumentNotFoundError; /** * Test Document constructor. @@ -1939,6 +1940,95 @@ describe('document', function() { }); }); + describe('gh-8371', function() { + it('setting isNew to true makes save tries to insert a new document (gh-8371)', function() { + return co(function*() { + const personSchema = new Schema({ name: String }); + const Person = db.model('gh8371-A', personSchema); + + const createdPerson = yield Person.create({name:'Hafez'}); + const removedPerson = yield Person.findOneAndRemove({_id:createdPerson._id}); + + removedPerson.isNew = true; + + yield removedPerson.save(); + + const foundPerson = yield Person.findOne({_id:removedPerson._id}); + assert.ok(foundPerson); + }); + }); + + it('setting isNew to true throws an error when a document already exists (gh-8371)', function() { + return co(function*() { + const personSchema = new Schema({ name: String }); + const Person = db.model('gh8371-B', personSchema); + + const createdPerson = yield Person.create({name:'Hafez'}); + + createdPerson.isNew = true; + + let threw = false; + try { + yield createdPerson.save(); + } + catch (err) { + threw = true; + assert.equal(err.code, 11000); + } + + assert.equal(threw,true); + }); + }); + + it('saving a document with no changes, throws an error when document is not found', function() { + return co(function*() { + const personSchema = new Schema({ name: String }); + const Person = db.model('gh8371-C', personSchema); + + const person = yield Person.create({name:'Hafez'}); + + yield Person.deleteOne({_id:person._id}); + + let threw = false; + try { + yield person.save(); + } + catch (err) { + assert.equal(err instanceof DocumentNotFoundError, true); + assert.equal(err.message,`No document found for query "{ _id: ${person._id} }" on model "gh8371-C"`); + threw = true; + } + + assert.equal(threw,true); + }); + }); + + it('saving a document with changes, throws an error when document is not found', function() { + return co(function*() { + const personSchema = new Schema({ name: String }); + const Person = db.model('gh8371-D', personSchema); + + const person = yield Person.create({name:'Hafez'}); + + yield Person.deleteOne({_id:person._id}); + + person.name = 'Different Name'; + + let threw = false; + try { + yield person.save(); + } + catch (err) { + assert.equal(err instanceof DocumentNotFoundError,true); + assert.equal(err.message,`No document found for query "{ _id: ${person._id} }" on model "gh8371-D"`); + threw = true; + } + + assert.equal(threw,true); + }); + }); + }); + it('properly calls queue functions (gh-2856)', function(done) { const personSchema = new mongoose.Schema({ name: String diff --git a/test/model.discriminator.querying.test.js b/test/model.discriminator.querying.test.js index 83c55da1a8c..61db4e8344a 100644 --- a/test/model.discriminator.querying.test.js +++ b/test/model.discriminator.querying.test.js @@ -177,16 +177,10 @@ describe('model', function() { }); describe('discriminator model only finds documents of its type', function() { - let impressionEvent, conversionEvent1, conversionEvent2; - - before(function() { - impressionEvent = new ImpressionEvent({name: 'Impression event'}); - conversionEvent1 = new ConversionEvent({name: 'Conversion event 1', revenue: 1}); - conversionEvent2 = new ConversionEvent({name: 'Conversion event 2', revenue: 2}); - }); describe('using "ModelDiscriminator#findById"', function() { it('to find a document of the appropriate discriminator', function(done) { + const impressionEvent = new ImpressionEvent({name: 'Impression event'}); impressionEvent.save(function(err) { assert.ifError(err); @@ -217,6 +211,9 @@ describe('model', function() { describe('using "ModelDiscriminator#find"', function() { it('to find documents of the appropriate discriminator', function(done) { + const impressionEvent = new ImpressionEvent({name: 'Impression event'}); + const conversionEvent1 = new ConversionEvent({name: 'Conversion event 1', revenue: 1}); + const conversionEvent2 = new ConversionEvent({name: 'Conversion event 2', revenue: 2}); impressionEvent.save(function(err) { assert.ifError(err); conversionEvent1.save(function(err) {