fix: mysql delete/remove/destroy with limit and orders (#272)

Co-authored-by: JimmyDaddy <[email protected]>

src/adapters/sequelize.js

@@ -319,7 +319,9 @@ module.exports = Bone => {
     // proxy to class.destroy({ individualHooks=false }) see https://github.com/sequelize/sequelize/blob/4063c2ab627ad57919d5b45cc7755f077a69fa5e/lib/model.js#L2895  before(after)BulkDestroy
     static async bulkDestroy(options = {}) {
       const { where, force } = options;
-      return await this.remove(where || {}, force, { ...options });
+      const spell = this._remove(where || {}, force, { ...options });
+      translateOptions(spell, options);
+      return spell;
     }
 
     // EXISTS

src/bone.js

@@ -789,7 +789,7 @@ class Bone {
       }, opts);
       return result;
     }
-    return await Model.remove(condition, forceDelete, { hooks: false, ...opts });
+    return await Model._remove(condition, forceDelete, opts);
   }
 
   /**
@@ -1454,6 +1454,24 @@ class Bone {
    * @return {Spell}
    */
   static remove(conditions, forceDelete = false, options) {
+    return this._remove(conditions, forceDelete, options);
+  }
+
+  /**
+   * private method for internal calling
+   * Remove any record that matches `conditions`.
+   * - If `forceDelete` is true, `DELETE` records from database permanently.
+   * - If not, update `deletedAt` attribute with current date.
+   * - If `forceDelete` isn't true and `deleteAt` isn't around, throw an Error.
+   * @example
+   * Post.remove({ title: 'Leah' })         // mark Post { title: 'Leah' } as deleted
+   * Post.remove({ title: 'Leah' }, true)   // delete Post { title: 'Leah' }
+   * Post.remove({}, true)                  // delete all data of posts
+   * @param {Object}  conditions
+   * @param {boolean} forceDelete
+   * @return {Spell}
+   */
+  static _remove(conditions, forceDelete = false, options) {
     const { deletedAt } = this.timestamps;
     if (forceDelete !== true && this.attributes[deletedAt]) {
       return Bone.update.call(this, conditions, { [deletedAt]: new Date() }, {

src/drivers/mysql/spellbook.js

@@ -75,4 +75,19 @@ module.exports = {
 
     return result;
   },
+  /**
+   * DELETE ... ORDER BY ...LIMIT
+   * @param {Spell} spell 
+   */
+  formatDelete(spell) {
+    const result = spellbook.formatDelete.call(this, spell);
+    const { rowCount, orders } = spell;
+    const chunks = [];
+
+    if (orders.length > 0) chunks.push(`ORDER BY ${this.formatOrders(spell, orders).join(', ')}`);
+    if (rowCount > 0) chunks.push(`LIMIT ${rowCount}`);
+    if (chunks.length > 0) result.sql += ` ${chunks.join(' ')}`;
+
+    return result;
+  }
 };

test/integration/suite/basics.test.js

@@ -975,6 +975,47 @@ describe('=> Basic', () => {
         await tagMap.remove();
       }, /Error: instance is not persisted yet./);
     });
+
+    it('remove with limit and order(mysql only)', async function () {
+      if (Post.driver.type !== 'mysql') return;
+
+      const posts = await Promise.all([
+        await Post.create({ title: 'Book of Tyrael', word_count: 20 }),
+        await Post.create({ title: 'Book of Cain', word_count: 10 }),
+        await Post.create({ title: 'Book of Cain', word_count: 30 }),
+        await Post.create({ title: 'Book of Cain', word_count: 40 }),
+        await Post.create({ title: 'Book of Cain', word_count: 50 }),
+        await Post.create({ title: 'Book of Cain', word_count: 60 }),
+      ]);
+
+      let deleteCount = await Post.remove({}).limit(2).order('word_count');
+      assert.equal(deleteCount, 2);
+      const post1 = await Post.findOne('id = ?', posts[0].id).unparanoid;
+      assert(post1.deletedAt);
+
+      deleteCount = await Post.remove({ 
+        word_count: {
+          $gte: 0
+        }
+      }, false).limit(3).order('id DESC');;
+      assert.equal(deleteCount, 3);
+      let post6 = await Post.findOne('id = ?', posts[5].id).unparanoid;
+      assert(post6.deletedAt);
+      let post4 = await Post.findOne('id = ?', posts[3].id).unparanoid;
+      assert(post4.deletedAt);
+
+      deleteCount = await Post.remove( {
+        word_count: {
+            $gte: 0
+          }
+        },
+        true,
+      ).limit(3).order('id DESC');
+      post6 = await Post.findOne('id = ?', posts[5].id).unparanoid;
+      assert(!post6);
+      post4 = await Post.findOne('id = ?', posts[3].id).unparanoid;
+      assert(!post4);
+    });
   });
 
   describe('=> Bulk', () => {

test/unit/adapters/sequelize.test.js

@@ -1883,10 +1883,9 @@ describe('Transaction', function() {
   });
 });
 
-describe('Model.update with order, limit (mysql only)', () => {
-
+describe('mysql only', () => {
   const Spine = sequelize(Bone);
-
+  
   class Post extends Spine {
     static get table() {
       return 'articles';
@@ -1908,39 +1907,79 @@ describe('Model.update with order, limit (mysql only)', () => {
     Bone.driver = null;
   });
 
-  it('should work', async () => {
-
-    let i = 0;
-    while (i <= 5) {
-      await Post.create({ title: 'Throne' });
-      i += 1;
-    }
-    await Post.update({ title: 'Game' }, {
-      where: {},
-      limit: 2,
-      order: 'id ASC',
-      silent: true,
+  describe('Model.update with order, limit (mysql only)', () => {
+
+    it('should work', async () => {
+
+      let i = 0;
+      while (i <= 5) {
+        await Post.create({ title: 'Throne' });
+        i += 1;
+      }
+      await Post.update({ title: 'Game' }, {
+        where: {},
+        limit: 2,
+        order: 'id ASC',
+        silent: true,
+      });
+      let allPosts = await Post.findAll({ order: 'id ASC' });
+      assert.equal(allPosts[0].title, 'Game');
+      assert.equal(allPosts[1].title, 'Game');
+      assert.equal(allPosts[2].title, 'Throne');
+      assert.equal(allPosts[3].title, 'Throne');
+
+      await Post.bulkUpdate({ title: 'Pilot' }, {
+        where: {},
+        limit: 2,
+        order: 'id ASC',
+        silent: true,
+      });
+      allPosts = await Post.findAll({ order: 'id ASC' });
+      assert.equal(allPosts[0].title, 'Pilot');
+      assert.equal(allPosts[1].title, 'Pilot');
+      assert.equal(allPosts[2].title, 'Throne');
+      assert.equal(allPosts[3].title, 'Throne');
+      await Post.truncate();
     });
-    let allPosts = await Post.findAll({ order: 'id ASC' });
-    assert.equal(allPosts[0].title, 'Game');
-    assert.equal(allPosts[1].title, 'Game');
-    assert.equal(allPosts[2].title, 'Throne');
-    assert.equal(allPosts[3].title, 'Throne');
+  });
 
-    await Post.bulkUpdate({ title: 'Pilot' }, {
-      where: {},
-      limit: 2,
-      order: 'id ASC',
-      silent: true,
+  describe('Model.destroy with order, limit (mysql only)', () => {
+
+    it('should work', async () => {
+
+      let i = 0;
+      const posts = [];
+      while (i <= 5) {
+        posts.push(await Post.create({ title: 'Throne' }));
+        i += 1;
+      }
+      let deleteCount = await Post.destroy({
+        where: {},
+        limit: 2,
+        order: 'id ASC',
+        silent: true,
+      });
+      assert.equal(deleteCount, 2);
+      const p1 = await Post.findByPk(posts[0].id, { paranoid: false });
+      assert(p1.deletedAt);
+      const p2 = await Post.findByPk(posts[0].id, { paranoid: false });
+      assert(p2.deletedAt);
+      deleteCount = await Post.destroy({
+        where: {},
+        limit: 3,
+        order: 'id DESC',
+        silent: true,
+        force: true,
+      });
+      assert.equal(deleteCount, 3);
+      const p3 = await Post.findByPk(posts[3].id, { paranoid: false });
+      assert.deepEqual(p3, null);
+      const p4 = await Post.findByPk(posts[4].id, { paranoid: false });
+      assert.deepEqual(p4, null);
+      const p5 = await Post.findByPk(posts[5].id, { paranoid: false });
+      assert.deepEqual(p5, null);
+      await Post.truncate();
     });
-    allPosts = await Post.findAll({ order: 'id ASC' });
-    assert.equal(allPosts[0].title, 'Pilot');
-    assert.equal(allPosts[1].title, 'Pilot');
-    assert.equal(allPosts[2].title, 'Throne');
-    assert.equal(allPosts[3].title, 'Throne');
-
-
   });
-
-
 });
+