feat: restrict instance methods if no primary key

src/model.js

@@ -895,7 +895,6 @@ Specify a different name for either index to resolve this issue.`);
       underscored: false,
       paranoid: false,
       rejectOnEmpty: false,
-      whereCollection: null,
       schema: '',
       schemaDelimiter: '',
       defaultScope: {},
@@ -1778,9 +1777,6 @@ Specify a different name for either index to resolve this issue.`);
       options.originalAttributes = this._injectDependentVirtualAttributes(options.attributes);
     }
 
-    // whereCollection is used for non-primary key updates
-    this.options.whereCollection = options.where || null;
-
     Utils.mapFinderOptions(options, this);
 
     options = this._paranoidClause(this, options);
@@ -3513,16 +3509,27 @@ Instead of specifying a Model, either:
    * @returns {object}
    */
   where(checkVersion) {
+    if (this.constructor.primaryKeyAttributes.length === 0) {
+      throw new Error(
+        `This model instance method needs to be able to identify the entity in a stable way, but the model does not have a primary key attribute definition. Either add a primary key to this model, or use one of the following alternatives:
+
+- instance methods "save", "update", "decrement", "increment": Use the static "update" method instead.
+- instance method "reload": Use the static "findOne" method instead.
+- instance methods "destroy" and "restore": use the static "destroy" and "restore" methods instead.
+        `.trim(),
+      );
+    }
+
     const where = this.constructor.primaryKeyAttributes.reduce((result, attribute) => {
+      if (_.isNil(this.get(attribute, { raw: true }))) {
+        throw new TypeError(`This model instance method needs to be able to identify the entity in a stable way, but this model instance is missing the value of its primary key "${attribute}". Make sure that attribute was not excluded when retrieving the model from the database.`);
+      }
+
       result[attribute] = this.get(attribute, { raw: true });
 
       return result;
     }, {});
 
-    if (_.size(where) === 0) {
-      return this.constructor.options.whereCollection;
-    }
-
     const versionAttr = this.constructor._versionAttribute;
     if (checkVersion && versionAttr) {
       where[versionAttr] = this.get(versionAttr, { raw: true });

test/integration/instance/decrement.test.js

@@ -186,5 +186,13 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
 
       await expect(User.findByPk(1)).to.eventually.have.property('updatedAt').equalTime(oldDate);
     });
+
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({});
+      await expect(instance.decrement()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
   });
 });

test/integration/instance/destroy.test.js

@@ -248,6 +248,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       expect(user.username).to.equal('bar');
     });
 
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({});
+      await expect(instance.destroy()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+
     it('allows sql logging of delete statements', async function () {
       const UserDelete = this.sequelize.define('UserDelete', {
         name: DataTypes.STRING,

test/integration/instance/increment.test.js

@@ -118,6 +118,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       expect(user2.aNumber).to.be.equal(1);
     });
 
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({});
+      await expect(instance.increment()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+
     it('should still work right with other concurrent updates', async function () {
       const user1 = await this.User.findByPk(1);
       // Select the user again (simulating a concurrent query)

test/integration/instance/reload.test.js

@@ -107,6 +107,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       expect(updatedUser.username).to.equal('Doe John');
     });
 
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({});
+      await expect(instance.reload()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+
     it('should support updating a subset of attributes', async function () {
       const user1 = await this.User.create({
         aNumber: 1,

test/integration/instance/restore.test.js

@@ -0,0 +1,36 @@
+'use strict';
+
+const chai = require('chai');
+
+const expect = chai.expect;
+const Support = require('../support');
+const { DataTypes } = require('@sequelize/core');
+const sinon = require('sinon');
+
+const current = Support.sequelize;
+
+describe(Support.getTestDialectTeaser('Instance'), () => {
+  before(function () {
+    this.clock = sinon.useFakeTimers();
+  });
+
+  afterEach(function () {
+    this.clock.reset();
+  });
+
+  after(function () {
+    this.clock.restore();
+  });
+
+  describe('restore', () => {
+    it('is disallowed if no primary key is present', async function () {
+      const User = this.sequelize.define('User', {
+        name: { type: DataTypes.STRING },
+      }, { noPrimaryKey: true, paranoid: true });
+      await User.sync({ force: true });
+
+      const instance = await User.create({});
+      await expect(instance.restore()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+  });
+});

test/integration/instance/save.test.js

@@ -129,6 +129,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       });
     });
 
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({}, { isNewRecord: true });
+      await expect(instance.save()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+
     describe('hooks', () => {
       it('should update attributes added in hooks when default fields are used', async function () {
         const User = this.sequelize.define(`User${Support.rand()}`, {

test/integration/instance/update.test.js

@@ -182,6 +182,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       expect(user.validateSideEffect).not.to.be.ok;
     });
 
+    it('is disallowed if no primary key is present', async function () {
+      const Foo = this.sequelize.define('Foo', {}, { noPrimaryKey: true });
+      await Foo.sync({ force: true });
+
+      const instance = await Foo.create({});
+      await expect(instance.update()).to.be.rejectedWith('but the model does not have a primary key attribute definition.');
+    });
+
     describe('hooks', () => {
       it('should update attributes added in hooks when default fields are used', async function () {
         const User = this.sequelize.define(`User${Support.rand()}`, {

test/unit/instance/decrement.test.js

@@ -11,6 +11,13 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('decrement', () => {
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
+
+      await expect(instance.decrement()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
+    });
+
     describe('options tests', () => {
       let stub; let instance;
       const Model = current.define('User', {

test/unit/instance/destroy.test.js

@@ -11,6 +11,13 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('destroy', () => {
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
+
+      await expect(instance.destroy()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
+    });
+
     describe('options tests', () => {
       let stub; let instance;
       const Model = current.define('User', {

test/unit/instance/increment.test.js

@@ -11,6 +11,13 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('increment', () => {
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
+
+      await expect(instance.increment()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
+    });
+
     describe('options tests', () => {
       let stub; let instance;
       const Model = current.define('User', {

test/unit/instance/reload.test.js

@@ -11,6 +11,13 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('reload', () => {
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
+
+      await expect(instance.reload()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
+    });
+
     describe('options tests', () => {
       let stub; let instance;
       const Model = current.define('User', {

test/unit/instance/restore.test.js

@@ -11,6 +11,13 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('restore', () => {
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
+
+      await expect(instance.restore()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
+    });
+
     describe('options tests', () => {
       let stub; let instance;
       const Model = current.define('User', {

test/unit/instance/save.test.js

@@ -11,11 +11,11 @@ const sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), () => {
   describe('save', () => {
-    it('should disallow saves if no primary key values is present', async () => {
-      const Model = current.define('User', {});
-      const instance = Model.build({}, { isNewRecord: false });
+    it('is not allowed if the instance does not have a primary key defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({});
 
-      await expect(instance.save()).to.be.rejected;
+      await expect(instance.save()).to.be.rejectedWith('but this model instance is missing the value of its primary key');
     });
 
     describe('options tests', () => {

test/unit/instance/update.test.js

@@ -0,0 +1,19 @@
+'use strict';
+
+const chai = require('chai');
+
+const expect = chai.expect;
+const Support   = require('../support');
+
+const current   = Support.sequelize;
+
+describe(Support.getTestDialectTeaser('Instance'), () => {
+  describe('update', () => {
+    it('is not allowed if the primary key is not defined', async () => {
+      const User = current.define('User', {});
+      const instance = User.build({}, { isNewRecord: false });
+
+      await expect(instance.update()).to.be.rejectedWith('You attempted to save an instance with no primary key, this is not allowed since');
+    });
+  });
+});