From 4e41e630ac0e4082f812f108ea28fb4a2702f6f3 Mon Sep 17 00:00:00 2001 From: Junyi Date: Mon, 7 Dec 2020 11:54:23 +0800 Subject: [PATCH] Feature: action fields options for create/update (#32) * feat: add fields options for create/update actions * test: add test case for json * fix: minor update for reviews * fix: test case * fix: change fields filter strategy for create/update * feat: add transaction for create/update --- .../actions/src/__tests__/actions/create1.ts | 16 +++ .../actions/src/__tests__/actions/create2.ts | 10 ++ .../actions/src/__tests__/actions/update1.ts | 16 +++ .../actions/src/__tests__/actions/update2.ts | 10 ++ packages/actions/src/__tests__/create.test.ts | 55 +++++++- packages/actions/src/__tests__/index.ts | 20 ++- .../actions/src/__tests__/tables/posts.ts | 6 +- packages/actions/src/__tests__/update.test.ts | 124 ++++++++++++++++-- packages/actions/src/actions/common.ts | 87 ++++++------ .../actions/src/middlewares/associated.ts | 48 +++---- packages/actions/src/utils.ts | 10 ++ 11 files changed, 319 insertions(+), 83 deletions(-) create mode 100644 packages/actions/src/__tests__/actions/create1.ts create mode 100644 packages/actions/src/__tests__/actions/create2.ts create mode 100644 packages/actions/src/__tests__/actions/update1.ts create mode 100644 packages/actions/src/__tests__/actions/update2.ts create mode 100644 packages/actions/src/utils.ts diff --git a/packages/actions/src/__tests__/actions/create1.ts b/packages/actions/src/__tests__/actions/create1.ts new file mode 100644 index 000000000..e9a3b25a2 --- /dev/null +++ b/packages/actions/src/__tests__/actions/create1.ts @@ -0,0 +1,16 @@ +import { ActionOptions } from '@nocobase/resourcer'; +import { create } from '../../actions/common'; + +export default { + defaultValues: { + meta: { + location: 'Kunming' + } + }, + + fields: { + except: ['sort'] + }, + + handler: create +} as ActionOptions; diff --git a/packages/actions/src/__tests__/actions/create2.ts b/packages/actions/src/__tests__/actions/create2.ts new file mode 100644 index 000000000..ba2a11352 --- /dev/null +++ b/packages/actions/src/__tests__/actions/create2.ts @@ -0,0 +1,10 @@ +import { ActionOptions } from '@nocobase/resourcer'; +import { create } from '../../actions/common'; + +export default { + fields: { + only: ['title'] + }, + + handler: create +} as ActionOptions; diff --git a/packages/actions/src/__tests__/actions/update1.ts b/packages/actions/src/__tests__/actions/update1.ts new file mode 100644 index 000000000..5c3d1ac89 --- /dev/null +++ b/packages/actions/src/__tests__/actions/update1.ts @@ -0,0 +1,16 @@ +import { ActionOptions } from '@nocobase/resourcer'; +import { update } from '../../actions/common'; + +export default { + defaultValues: { + meta: { + location: 'Kunming' + } + }, + + fields: { + except: ['title'] + }, + + handler: update +} as ActionOptions; diff --git a/packages/actions/src/__tests__/actions/update2.ts b/packages/actions/src/__tests__/actions/update2.ts new file mode 100644 index 000000000..7c499408b --- /dev/null +++ b/packages/actions/src/__tests__/actions/update2.ts @@ -0,0 +1,10 @@ +import { ActionOptions } from '@nocobase/resourcer'; +import { update } from '../../actions/common'; + +export default { + fields: { + only: ['title'] + }, + + handler: update +} as ActionOptions; diff --git a/packages/actions/src/__tests__/create.test.ts b/packages/actions/src/__tests__/create.test.ts index d4e4f4c73..eb4dd4f40 100644 --- a/packages/actions/src/__tests__/create.test.ts +++ b/packages/actions/src/__tests__/create.test.ts @@ -9,14 +9,57 @@ describe('create', () => { afterAll(() => db.close()); - describe('common', () => { - it('create', async () => { + describe('single', () => { + it('create with hasMany items', async () => { const response = await agent .post('/posts') .send({ title: 'title1', + comments: [ + { content: 'content1' }, + { content: 'content2' }, + ] }); expect(response.body.title).toBe('title1'); + + const createdPost = await agent.get(`/posts/${response.body.id}?fields=comments`); + expect(createdPost.body.comments.length).toBe(2); + }); + + it('create with defaultValues by custom action', async () => { + const response = await agent + .post('/posts:create1') + .send({ + title: 'title1', + }); + expect(response.body.meta).toEqual({ location: 'Kunming' }); + }); + + it('create with options.fields.except by custom action', async () => { + const response = await agent + .post('/posts:create1') + .send({ + title: 'title1', + sort: 100 + }); + expect(response.body.sort).toBe(null); + }); + + it('create with options.fields.only by custom action', async () => { + const response = await agent + .post('/posts:create2') + .send({ + title: 'title1', + meta: { a: 1 } + }); + expect(response.body.title).toBe('title1'); + expect(response.body.meta).toBe(null); + + const result = await agent + .get(`/posts/${response.body.id}`); + + expect(result.body.title).toBe('title1'); + expect(result.body.meta).toBe(null); }); }); @@ -31,6 +74,14 @@ describe('create', () => { }); expect(response.body.post_id).toBe(post.id); expect(response.body.content).toBe('content1'); + + const comments = await agent + .get('/comments?fields=id,content'); + expect(comments.body.count).toBe(1); + expect(comments.body.rows).toEqual([{ + id: 1, + content: 'content1' + }]); }); }); }); diff --git a/packages/actions/src/__tests__/index.ts b/packages/actions/src/__tests__/index.ts index e278478ad..ef04e0910 100644 --- a/packages/actions/src/__tests__/index.ts +++ b/packages/actions/src/__tests__/index.ts @@ -11,6 +11,10 @@ import Resourcer from '@nocobase/resourcer'; import associated from '../middlewares/associated'; import actions from '..'; +import create1 from './actions/create1'; +import create2 from './actions/create2'; +import update1 from './actions/update1'; +import update2 from './actions/update2'; function getTestKey() { const { id } = require.main; @@ -52,9 +56,23 @@ const tableFiles = glob.sync(`${resolve(__dirname, './tables')}/*.ts`); // resourcer 在内存中是单例，需要谨慎使用 export const resourcer = new Resourcer(); resourcer.use(associated); -resourcer.registerActionHandlers({...actions.associate, ...actions.common}); +resourcer.registerActionHandlers({...actions.associate, ...actions.common }); resourcer.define({ name: 'posts', + actions: { + ...actions.common, + create1, + create2, + update1, + update2 + }, +}); +resourcer.define({ + name: 'comments', + actions: actions.common, +}); +resourcer.define({ + name: 'users', actions: actions.common, }); resourcer.define({ diff --git a/packages/actions/src/__tests__/tables/posts.ts b/packages/actions/src/__tests__/tables/posts.ts index f16abd3b1..70600f14e 100644 --- a/packages/actions/src/__tests__/tables/posts.ts +++ b/packages/actions/src/__tests__/tables/posts.ts @@ -22,7 +22,7 @@ export default { name: 'user', }, { - type: 'hasmany', + type: 'hasMany', name: 'comments', }, { @@ -32,6 +32,10 @@ export default { { type: 'integer', name: 'sort' + }, + { + type: 'json', + name: 'meta' } ], hooks: { diff --git a/packages/actions/src/__tests__/update.test.ts b/packages/actions/src/__tests__/update.test.ts index 97965a305..534dae6a8 100644 --- a/packages/actions/src/__tests__/update.test.ts +++ b/packages/actions/src/__tests__/update.test.ts @@ -9,24 +9,130 @@ describe('update', () => { afterAll(() => db.close()); - it('common1', async () => { - const Post = db.getModel('posts'); - const post = await Post.create(); - const response = await agent - .put(`/posts/${post.id}`).send({ - title: 'title11112222' + describe('common', () => { + it('basic', async () => { + const Post = db.getModel('posts'); + const post = await Post.create(); + const response = await agent + .put(`/posts/${post.id}`).send({ + title: 'title11112222' + }); + expect(response.body.title).toBe('title11112222'); + }); + + it('update json field by replacing', async () => { + const Post = db.getModel('posts'); + const post = await Post.create({ meta: { a: 1, b: 'c', c: { d: false } } }); + const updated = await agent + .put(`/posts/${post.id}`).send({ + meta: {} + }); + expect(updated.body.meta).toEqual({}); + }); + + it.skip('update json field by path based update', async () => { + const Post = db.getModel('posts'); + const post = await Post.create({ meta: { a: 1, b: 'c', c: { d: false } } }); + const updated = await agent + .put(`/posts/${post.id}?options[json]=merge`).send({ + meta: { + b: 'b', + c: { d: true } + } + }); + // console.log(updated.body); + }); + + // TODO(question): json 字段的覆盖/合并策略 + it.skip('update with fields overwrite default values', async () => { + const Post = db.getModel('posts'); + const post = await Post.create(); + const response = await agent + .put(`/posts:update1/${post.id}`).send({ + meta: { a: 1 }, + }); + expect(response.body.meta).toEqual({ a: 1 }); + + const result = await agent + .get(`/posts/${post.id}`); + + expect(result.body.meta).toEqual({ a: 1 }); + }); + + // TODO(bug): action 的默认值处理时机不对 + it.skip('update with different fields to default values', async () => { + const Post = db.getModel('posts'); + const post = await Post.create({ + meta: { location: 'Beijing' } }); - expect(response.body.title).toBe('title11112222'); + const response = await agent + .put(`/posts:update1/${post.id}`).send({ + meta: { a: 1 }, + }); + expect(response.body.meta).toEqual({ location: 'Beijing', a: 1 }); + }); + + it('update with options.fields.expect in action', async () => { + const Post = db.getModel('posts'); + const post = await Post.create(); + const response = await agent + .put(`/posts:update1/${post.id}`).send({ + title: 'title11112222', + }); + expect(response.body.title).toBe(null); + expect(response.body.meta).toEqual({ + location: 'Kunming' + }); + + const result = await agent + .get(`/posts/${post.id}`); + + expect(result.body.title).toBe(null); + expect(result.body.meta).toEqual({ + location: 'Kunming' + }); + }); + + it('update with options.fields.only in action', async () => { + const Post = db.getModel('posts'); + const post = await Post.create(); + const response = await agent + .put(`/posts:update2/${post.id}`).send({ + title: 'title11112222', + meta: { a: 1 } + }); + expect(response.body.title).toBe('title11112222'); + expect(response.body.meta).toBe(null); + + const result = await agent + .get(`/posts/${post.id}`); + + expect(result.body.title).toBe('title11112222'); + expect(result.body.meta).toBe(null); + }); }); - it('hasOne1', async () => { + it('hasOne', async () => { + const User = db.getModel('users'); + const user = await User.create(); + await user.updateAssociations({ + profile: { email: 'email1122' } + }); + const response = await agent + .put(`/users/${user.id}/profile`).send({ + email: 'email1111', + }); + expect(response.body.email).toEqual('email1111'); + }); + + it('hasOne without exist target', async () => { const User = db.getModel('users'); const user = await User.create(); const response = await agent .put(`/users/${user.id}/profile`).send({ email: 'email1122', }); - expect(response.body.email).toEqual('email1122'); + expect(response.body).toEqual({}); }); it('hasMany1', async () => { diff --git a/packages/actions/src/actions/common.ts b/packages/actions/src/actions/common.ts index 80c3e1fad..d860c34f8 100644 --- a/packages/actions/src/actions/common.ts +++ b/packages/actions/src/actions/common.ts @@ -1,8 +1,9 @@ import { Context, Next } from '.'; -import { Relation, Model, Field, HasOne, HasMany, BelongsTo, BelongsToMany } from '@nocobase/database'; +import { Relation, Model, HasOne, HasMany, BelongsTo, BelongsToMany } from '@nocobase/database'; import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '@nocobase/resourcer'; import { Utils, Op } from 'sequelize'; import _ from 'lodash'; +import { filterByFields } from '../utils'; /** * 查询数据列表 @@ -103,26 +104,32 @@ export async function create(ctx: Context, next: Next) { associated, resourceField, associatedName, - values, - } = ctx.action.params as { associated: Model, associatedName: string, resourceField: Relation, values: any }; + resourceName, + values: data, + fields + } = ctx.action.params; + const values = filterByFields(data, fields); + const transaction = await ctx.db.sequelize.transaction(); + let model: Model; if (associated && resourceField) { const AssociatedModel = ctx.db.getModel(associatedName); if (!(associated instanceof AssociatedModel)) { throw new Error(`${associatedName} associated model invalid`); } - const create = resourceField.getAccessors().create; - const model: Model = await associated[create](values, { context: ctx }); - await model.updateAssociations(values, { context: ctx }); + const { create } = resourceField.getAccessors(); + // @ts-ignore + model = await associated[create](values, { transaction, context: ctx }); + await model.updateAssociations(values, { transaction, context: ctx }); ctx.body = model; } else { - const { resourceName } = ctx.action.params; - const Model = ctx.db.getModel(resourceName); + const ResourceModel = ctx.db.getModel(resourceName); // @ts-ignore - const model = await Model.create(values, { context: ctx }); + model = await ResourceModel.create(values, { transaction, context: ctx }); // @ts-ignore - await model.updateAssociations(values, { context: ctx }); + await model.updateAssociations(values, { transaction, context: ctx }); ctx.body = model; } + await transaction.commit(); await next(); } @@ -197,44 +204,42 @@ export async function get(ctx: Context, next: Next) { export async function update(ctx: Context, next: Next) { const { associated, - resourceField, associatedName, - } = ctx.action.params as { - associated: Model, - associatedName: string, - resourceField: Relation, - values: any, - }; + resourceField, + resourceName, + resourceKey, + // TODO(question): 这个属性从哪设置的？ + resourceKeyAttribute, + fields, + values: data + } = ctx.action.params; + const values = filterByFields(data, fields); + const transaction = await ctx.db.sequelize.transaction(); if (associated && resourceField) { const AssociatedModel = ctx.db.getModel(associatedName); if (!(associated instanceof AssociatedModel)) { + await transaction.rollback(); throw new Error(`${associatedName} associated model invalid`); } - const {get: getAccessor, create: createAccessor, add: addAccessor} = resourceField.getAccessors(); - const { resourceKey, resourceKeyAttribute, fields = [], values } = ctx.action.params; - const TargetModel = ctx.db.getModel(resourceField.getTarget()); - const options = TargetModel.parseApiJson({ - fields, - }); + const { get: getAccessor } = resourceField.getAccessors(); if (resourceField instanceof HasOne || resourceField instanceof BelongsTo) { - let model: Model = await associated[getAccessor]({ ...options, context: ctx }); + let model: Model = await associated[getAccessor]({ transaction, context: ctx }); if (model) { // @ts-ignore - await model.update(values, { context: ctx }); - } else if (!model && resourceField instanceof HasOne) { - model = await associated[createAccessor](values, { context: ctx }); + await model.update(values, { transaction, context: ctx }); + await model.updateAssociations(values, { transaction, context: ctx }); + ctx.body = model; } - await model.updateAssociations(values, { context: ctx }); - ctx.body = model; } else if (resourceField instanceof HasMany || resourceField instanceof BelongsToMany) { + const TargetModel = ctx.db.getModel(resourceField.getTarget()); const [model]: Model[] = await associated[getAccessor]({ - ...options, where: { [resourceKeyAttribute || resourceField.options.targetKey || TargetModel.primaryKeyAttribute]: resourceKey, }, + transaction, context: ctx, }); - + if (resourceField instanceof BelongsToMany) { const throughName = resourceField.getThroughName(); if (typeof values[throughName] === 'object') { @@ -246,39 +251,37 @@ export async function update(ctx: Context, next: Next) { [foreignKey]: associated[sourceKey], [otherKey]: resourceKey, }, + transaction }); - await through.update(throughValues); - await through.updateAssociations(throughValues); + await through.updateAssociations(throughValues, { transaction, context: ctx }); + await through.update(throughValues, { transaction, context: ctx }); delete values[throughName]; } } if (!_.isEmpty(values)) { // @ts-ignore - await model.update(values, { context: ctx }); - await model.updateAssociations(values, { context: ctx }); + await model.update(values, { transaction, context: ctx }); + await model.updateAssociations(values, { transaction, context: ctx }); } ctx.body = model; } } else { - const { resourceName, resourceKey, resourceKeyAttribute, fields = [], values } = ctx.action.params; const Model = ctx.db.getModel(resourceName); - const options = Model.parseApiJson({ - fields, - }); const model = await Model.findOne({ - ...options, where: { [resourceKeyAttribute || Model.primaryKeyAttribute]: resourceKey, }, // @ts-ignore hooks 里添加 context context: ctx, + transaction }); // @ts-ignore - await model.update(values, { context: ctx }); + await model.update(values, { transaction, context: ctx }); // @ts-ignore - await model.updateAssociations(values, { context: ctx }); + await model.updateAssociations(values, { transaction, context: ctx }); ctx.body = model; } + await transaction.commit(); await next(); } diff --git a/packages/actions/src/middlewares/associated.ts b/packages/actions/src/middlewares/associated.ts index 2220ad5ad..6407750e5 100644 --- a/packages/actions/src/middlewares/associated.ts +++ b/packages/actions/src/middlewares/associated.ts @@ -20,37 +20,29 @@ export async function associated(ctx: Context, next: Next) { const Model = ctx.db.getModel(associatedName); const field = ctx.db.getTable(associatedName).getField(resourceName); - let model: Model; + let key: string; - if (field instanceof HasOne) { - model = await Model.findOne({ - where: { - [field.options.sourceKey]: associatedKey, - } - }); - } else if (field instanceof HasMany) { - model = await Model.findOne({ - where: { - [field.options.sourceKey]: associatedKey, - } - }); - } else if (field instanceof BelongsTo) { - model = await Model.findOne({ - where: { - [Model.primaryKeyAttribute]: associatedKey, - } - }); - } else if (field instanceof BelongsToMany) { - model = await Model.findOne({ - where: { - [field.options.sourceKey]: associatedKey, - } - }); + switch (true) { + case field instanceof BelongsTo: + key = field.options.targetKey || Model.primaryKeyAttribute; + break; + case field instanceof HasOne: + case field instanceof HasMany: + case field instanceof BelongsToMany: + key = field.options.sourceKey; + break; } - if (model) { - ctx.action.setParam('associated', model); - ctx.action.setParam('resourceField', field); + if (key) { + const model = await Model.findOne({ + where: { + [key]: associatedKey, + } + }); + if (model) { + ctx.action.setParam('associated', model); + ctx.action.setParam('resourceField', field); + } } await next(); diff --git a/packages/actions/src/utils.ts b/packages/actions/src/utils.ts new file mode 100644 index 000000000..ef48f84e6 --- /dev/null +++ b/packages/actions/src/utils.ts @@ -0,0 +1,10 @@ +import _ from 'lodash'; + +export function filterByFields(data: any, fields: any = {}): any { + const { + only = Array.isArray(fields) ? fields : null, + except = [] + } = fields; + + return _.omit(only ? _.pick(data, only): data, except); +}