refactor: change sort strategy from offset to targetId (#37)

* refactor: change sort strategy from offset to targetId

* fix: remove unnecessary query to optimize performance

packages/actions/src/__tests__/index.ts

@@ -85,6 +85,11 @@ resourcer.define({
   name: 'posts.comments',
   actions: actions.associate,
 });
+resourcer.define({
+  type: 'hasMany',
+  name: 'users.posts',
+  actions: actions.associate,
+});
 resourcer.define({
   type: 'belongsTo',
   name: 'posts.user',

packages/actions/src/__tests__/sort.test.ts

@@ -26,7 +26,7 @@ describe('get', () => {
   
   afterAll(() => db.close());
 
-  describe.only('sort value initialization', () => {
+  describe('sort value initialization', () => {
     it('initialization by bulkCreate', async () => {
       const Post = db.getModel('posts');
       const posts = await Post.findAll({
@@ -85,188 +85,165 @@ describe('get', () => {
   });
 
   describe('sort in whole table', () => {
-    it('init sort value', async () => {
-      const Post = db.getModel('posts');
-    });
-
-    it('move id=1 by offset=1', async () => {
-      const Post = db.getModel('posts');
+    it('move id=1 to position at id=2', async () => {
       await agent
         .post('/posts:sort/1')
         .send({
-          offset: 1,
+          field: 'sort',
+          targetId: 2,
         });
 
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(1);
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(0);
+      const Post = db.getModel('posts');
+      const posts = await Post.findAll({
+        attributes: ['id', 'sort'],
+        order: [['id', 'ASC']]
+      });
+      expect(posts.map(item => item.get())).toEqual([
+        { id: 1, sort: 2 },
+        { id: 2, sort: 1 },
+        { id: 3, sort: 3 },
+        { id: 4, sort: 4 },
+        { id: 5, sort: 5 },
+        { id: 6, sort: 6 },
+        { id: 7, sort: 7 },
+        { id: 8, sort: 8 },
+        { id: 9, sort: 9 },
+        { id: 10, sort: 10 }
+      ]);
     });
 
-    it('move id=1 by offset=9', async () => {
+    it('move id=2 to position at id=1', async () => {
+      await agent
+        .post('/posts:sort/2')
+        .send({
+          field: 'sort',
+          targetId: 1,
+        });
+
       const Post = db.getModel('posts');
+      const posts = await Post.findAll({
+        attributes: ['id', 'sort'],
+        order: [['id', 'ASC']]
+      });
+      expect(posts.map(item => item.get())).toEqual([
+        { id: 1, sort: 2 },
+        { id: 2, sort: 1 },
+        { id: 3, sort: 3 },
+        { id: 4, sort: 4 },
+        { id: 5, sort: 5 },
+        { id: 6, sort: 6 },
+        { id: 7, sort: 7 },
+        { id: 8, sort: 8 },
+        { id: 9, sort: 9 },
+        { id: 10, sort: 10 }
+      ]);
+    });
+
+    it('move id=1 to position at id=10', async () => {
       await agent
         .post('/posts:sort/1')
         .send({
-          offset: 9,
+          field: 'sort',
+          targetId: 10,
         });
 
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(9);
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(0);
-
-      const post10 = await Post.findByPk(10);
-      expect(post10.get('sort')).toBe(8);
-
-      const post11 = await Post.findByPk(11);
-      expect(post11.get('sort')).toBe(10);
-    });
-
-    it('move id=1 by offset=-1', async () => {
       const Post = db.getModel('posts');
-      await agent
-        .post('/posts:sort/1')
-        .send({
-          offset: -1,
-        });
-
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(0);
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(1);
-    });
-
-    it('move id=2 by offset=8', async () => {
-      const Post = db.getModel('posts');
-      await agent
-        .post('/posts:sort/2')
-        .send({
-          offset: 8,
-        });
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(9);
-
-      const post10 = await Post.findByPk(10);
-      expect(post10.get('sort')).toBe(8);
-    });
-
-    it('move id=2 by offset=-1', async () => {
-      const Post = db.getModel('posts');
-      await agent
-        .post('/posts:sort/2')
-        .send({
-          offset: -1,
-        });
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(0);
-
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(1);
-    });
-
-    it('move id=2 by offset=Infinity', async () => {
-      const Post = db.getModel('posts');
-      await agent
-        .post('/posts:sort/2')
-        .send({
-          offset: 'Infinity',
-        });
-
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(0);
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(10);
-
-      const post10 = await Post.findByPk(10);
-      expect(post10.get('sort')).toBe(9);
-    });
-
-    it('move id=2 by offset=-Infinity', async () => {
-      const Post = db.getModel('posts');
-      await agent
-        .post('/posts:sort/2')
-        .send({
-          offset: '-Infinity',
-        });
-
-      const post1 = await Post.findByPk(1);
-      expect(post1.get('sort')).toBe(0);
-
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(-1);
+      const posts = await Post.findAll({
+        attributes: ['id', 'sort'],
+        order: [['id', 'ASC']]
+      });
+      expect(posts.map(item => item.get())).toEqual([
+        { id: 1, sort: 10 },
+        { id: 2, sort: 1 },
+        { id: 3, sort: 2 },
+        { id: 4, sort: 3 },
+        { id: 5, sort: 4 },
+        { id: 6, sort: 5 },
+        { id: 7, sort: 6 },
+        { id: 8, sort: 7 },
+        { id: 9, sort: 8 },
+        { id: 10, sort: 9 }
+      ]);
     });
   });
 
   describe('sort in filtered scope', () => {
-    it('move id=1 by offset=3 in scope filter[status]=publish', async () => {
-      try {
-        await agent
-          .post('/posts:sort/1?filter[status]=publish')
-          .send({
-            offset: 3,
-          });
-      } catch (error) {
-        expect(error).toBeDefined();
-      }
-    });
-
-    // 在 scope 中的排序无所谓值是否与其他不在 scope 中的重复。
-    it('move id=2 by offset=3 in scope filter[status]=publish', async () => {
-      const Post = db.getModel('posts');
+    it('move id=2 to position at id=8 (same scope value)', async () => {
       await agent
-        .post('/posts:sort/2?filter[status]=publish')
+        .post('/posts:sort/2')
         .send({
-          offset: 3,
-        });
-      
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(7);
-    });
-
-    it('move id=2 by offset=Infinity in scope filter[status]=publish', async () => {
-      await agent
-        .post('/posts:sort/2?filter[status]=publish')
-        .send({
-          offset: 'Infinity',
+          field: 'sort_in_status',
+          targetId: 8,
         });
 
       const Post = db.getModel('posts');
-      const post2 = await Post.findByPk(2);
-      expect(post2.get('sort')).toBe(10);
+      const posts = await Post.findAll({
+        where: {
+          status: 'publish'
+        },
+        attributes: ['id', 'sort_in_status'],
+        order: [['id', 'ASC']]
+      });
+      expect(posts.map(item => item.get())).toEqual([
+        { id: 2, sort_in_status: 4 },
+        { id: 4, sort_in_status: 1 },
+        { id: 6, sort_in_status: 2 },
+        { id: 8, sort_in_status: 3 },
+        { id: 10, sort_in_status: 5 }
+      ]);
+    });
+
+    it('move id=1 to position at id=8 (different scope value)', async () => {
+      await agent
+        .post('/posts:sort/1')
+        .send({
+          field: 'sort_in_status',
+          targetId: 8,
+        });
+      
+      const Post = db.getModel('posts');
+      const posts = await Post.findAll({
+        where: {
+          status: 'publish'
+        },
+        attributes: ['id', 'sort_in_status'],
+        order: [['id', 'ASC']]
+      });
+      expect(posts.map(item => item.get())).toEqual([
+        { id: 1, sort_in_status: 4 },
+        { id: 2, sort_in_status: 1 },
+        { id: 4, sort_in_status: 2 },
+        { id: 6, sort_in_status: 3 },
+        { id: 8, sort_in_status: 5 },
+        { id: 10, sort_in_status: 6 }
+      ]);
     });
   });
 
   describe('associations', () => {
     describe('hasMany', () => {
-      it('sort only 1 item in group will never change', async () => {
+      it('move id=1 to position at id=3 (different scope value)', async () => {
         await agent
-          .post('/posts/2/comments:sort/1')
+          .post('/users/1/posts:sort/1')
           .send({
-            offset: 1
+            field: 'sort_in_user',
+            targetId: 3,
           });
 
-        const Comment = db.getModel('comments');
-        const comment1 = await Comment.findByPk(1);
-        expect(comment1.get('sort')).toBe(0);
-      });
+        const Post = db.getModel('posts');
+        const posts = await Post.findAll({
+          where: {
+            user_id: 3
+          },
+          attributes: ['id', 'sort_in_user'],
+          order: [['id', 'ASC']]
+        });
 
-      it('/posts/5/comments:sort/7', async () => {
-        await agent
-          .post('/posts/5/comments:sort/7')
-          .send({
-            offset: 1
-          });
-        
-        const Comment = db.getModel('comments');
-        const comment7 = await Comment.findByPk(7);
-        expect(comment7.get('sort')).toBe(1);
+        expect(posts.map(item => item.get())).toEqual([
+          { id: 1, sort_in_user: 1 },
+          { id: 3, sort_in_user: 2 },
+          { id: 10, sort_in_user: 3 },
+        ]);
       });
     });
   });

packages/actions/src/actions/common.ts

@@ -1,3 +1,5 @@
+import { Utils, Op } from 'sequelize';
+import _ from 'lodash';
 import { Context, Next } from '.';
 import {
   Model, 
@@ -5,10 +7,9 @@ import {
   HASMANY,
   BELONGSTO,
   BELONGSTOMANY,
+  whereCompare
 } from '@nocobase/database';
 import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '@nocobase/resourcer';
-import { Utils, Op } from 'sequelize';
-import _ from 'lodash';
 import { filterByFields } from '../utils';
 
 /**
@@ -373,7 +374,6 @@ export async function sort(ctx: Context, next: Next) {
     associatedName,
     associatedKey,
     associated,
-    filter = {},
     values
   } = ctx.action.params;
 
@@ -390,114 +390,94 @@ export async function sort(ctx: Context, next: Next) {
   const Model = ctx.db.getModel(resourceName);
   const table = ctx.db.getTable(resourceName);
 
-  if (!values.offset) {
+  const { field, targetId } = values;
+  if (!values.field || typeof targetId === 'undefined') {
     return next();
   }
-  const [primaryField] = Model.primaryKeyAttributes;
-  const sortField = values.field || table.getOptions().sortField || 'sort';
-
-  // offset 的有效值为：整型 | 'Infinity' | '-Infinity'
-  const offset = Number(values.offset);
-  const sign = offset < 0 ? {
-    op: Op.lte,
-    order: 'DESC',
-    direction: 1,
-    extremum: 'min'
-  } : {
-    op: Op.gte,
-    order: 'ASC',
-    direction: -1,
-    extremum: 'max'
-  };
+  const sortField = table.getField(field);
+  if (!sortField) {
+    return next();
+  }
+  const { primaryKeyAttribute } = Model;
+  const { name: sortAttr, scope = [] } = sortField.options;
 
   const transaction = await ctx.db.sequelize.transaction();
 
-  const { where = {} } = Model.parseApiJson({ filter });
+  const where = {};
   if (associated && resourceField instanceof HASMANY) {
     where[resourceField.options.foreignKey] = associatedKey;
   }
 
   // 找到操作对象
-  const operand = await Model.findOne({
-    // 这里增加 where 条件是要求如果有 filter 条件，就应该在同条件的组中排序，不是同条件组的报错处理。
+  const source = await Model.findOne({
     where: {
       ...where,
-      [primaryField]: resourceKey
+      [primaryKeyAttribute]: resourceKey
     },
     transaction
   });
-
-  if (!operand) {
+  if (!source) {
     await transaction.rollback();
-    // TODO: 错误需要后面统一处理
-    throw new Error(`resource(${resourceKey}) with filter does not exist`);
+    throw new Error(`resource(${resourceKey}) does not exist`);
   }
 
-  let target;
+  const target = await Model.findByPk(targetId, { transaction });
 
-  // 如果是有限的变动值
-  if (Number.isFinite(offset)) {
-    const absChange = Math.abs(offset);
-    const group = await Model.findAll({
-      where: {
-        ...where,
-        [primaryField]: {
-          [Op.ne]: resourceKey
-        },
-        [sortField]: {
-          [sign.op]: operand[sortField]
-        }
-      },
-      limit: absChange,
-      // offset: 0,
-      attributes: [primaryField, sortField],
-      order: [
-        [sortField, sign.order]
-      ],
-      transaction
+  if (!target) {
+    await transaction.rollback();
+    throw new Error(`resource(${targetId}) does not exist`);
+  }
+
+  const sourceScopeWhere = source.getScopeWhere(scope);
+  const targetScopeWhere = target.getScopeWhere(scope);
+  const sameScope = whereCompare(sourceScopeWhere, targetScopeWhere);
+
+  let increment;
+  const updateWhere = { ...targetScopeWhere };
+  if (sameScope) {
+    const direction = source[sortAttr] < target[sortAttr] ? {
+      sourceOp: Op.gt,
+      targetOp: Op.lte,
+      increment: -1
+    } : {
+      sourceOp: Op.lt,
+      targetOp: Op.gte,
+      increment: 1
+    };
+
+    increment = direction.increment;
+
+    Object.assign(updateWhere, {
+      [sortAttr]: {
+        [direction.sourceOp]: source[sortAttr],
+        [direction.targetOp]: target[sortAttr]
+      }
+    });
+  } else {
+    increment = 1;
+    Object.assign(updateWhere, {
+      [sortAttr]: {
+        [Op.gte]: target[sortAttr]
+      }
     });
-
-    if (!group.length) {
-      // 如果变动范围内的元素数比范围小
-      // 说明全部数据不足一页
-      // target = group[0][priorityKey] - sign.direction;
-      // 没有元素无需变动
-      await transaction.commit();
-      ctx.body = operand;
-      return next();
-    }
-    
-    // 如果变动范围内都有元素（可能出现 limit 范围内元素不足的情况）
-    if (group.length === absChange) {
-      target = group[group.length - 1][sortField];
-
-      await Model.increment(sortField, {
-        by: sign.direction,
-        where: {
-          [primaryField]: {
-            [Op.in]: group.map(item => item[primaryField])
-          }
-        },
-        transaction
-      });
-    }
   }
-  // 如果要求置顶或沉底(未在上一过程中计算出目标值)
-  if (typeof target === 'undefined') {
-    target = await Model[sign.extremum](sortField, {
-      where,
-      transaction
-    }) - sign.direction;
-  }
-  await operand.update({
-    [sortField]: target
+
+  await Model.increment(sortAttr, {
+    by: increment,
+    where: updateWhere,
+    transaction
+  });
+
+  await source.update({
+    [sortAttr]: target[sortAttr],
+    ...targetScopeWhere
   }, {
     transaction
   });
 
   await transaction.commit();
 
-  ctx.body = operand;
+  ctx.body = source;
 
   await next();
 }

packages/database/src/model.ts

@@ -259,7 +259,7 @@ export abstract class Model extends SequelizeModel {
     return data;
   }
 
-  getScopeWhere(scope: string[]) {
+  getScopeWhere(scope: string[] = []) {
     const Model = this.constructor as ModelCtor<Model>;
     const table = this.database.getTable(this.constructor.name);
     const associations = table.getAssociations();