feat(database): perform data validation before the update/create operation (#2681)
* chore: test * chore: tmp commit * feat: value guard check
This commit is contained in:
		
							parent
							
								
									dfe77ca2fb
								
							
						
					
					
						commit
						56d1d1b85f
					
				@ -2,21 +2,58 @@ import { Collection, Database, mockDatabase } from '@nocobase/database';
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
describe('update', () => {
 | 
					describe('update', () => {
 | 
				
			||||||
  let db: Database;
 | 
					  let db: Database;
 | 
				
			||||||
  let User: Collection;
 | 
					  let Post: Collection;
 | 
				
			||||||
  let Tag: Collection;
 | 
					  let Tag: Collection;
 | 
				
			||||||
 | 
					  let PostTag: Collection;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  afterEach(async () => {
 | 
					  afterEach(async () => {
 | 
				
			||||||
    await db.close();
 | 
					    await db.close();
 | 
				
			||||||
  });
 | 
					  });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  beforeEach(async () => {
 | 
					  beforeEach(async () => {
 | 
				
			||||||
    db = mockDatabase();
 | 
					    db = mockDatabase();
 | 
				
			||||||
    User = db.collection({
 | 
					
 | 
				
			||||||
 | 
					    await db.clean({ drop: true });
 | 
				
			||||||
 | 
					    PostTag = db.collection({
 | 
				
			||||||
 | 
					      name: 'post_tag',
 | 
				
			||||||
 | 
					      fields: [
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					          type: 'bigInt',
 | 
				
			||||||
 | 
					          name: 'id',
 | 
				
			||||||
 | 
					          primaryKey: true,
 | 
				
			||||||
 | 
					          autoIncrement: true,
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					          type: 'belongsTo',
 | 
				
			||||||
 | 
					          name: 'post',
 | 
				
			||||||
 | 
					          target: 'posts',
 | 
				
			||||||
 | 
					          foreignKey: 'post_id',
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					          type: 'belongsTo',
 | 
				
			||||||
 | 
					          name: 'tag',
 | 
				
			||||||
 | 
					          target: 'tags',
 | 
				
			||||||
 | 
					          foreignKey: 'tag_id',
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					      ],
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    Post = db.collection({
 | 
				
			||||||
      name: 'posts',
 | 
					      name: 'posts',
 | 
				
			||||||
      fields: [
 | 
					      fields: [
 | 
				
			||||||
        { type: 'string', name: 'title' },
 | 
					        { type: 'string', name: 'title' },
 | 
				
			||||||
        {
 | 
					        {
 | 
				
			||||||
          type: 'belongsToMany',
 | 
					          type: 'belongsToMany',
 | 
				
			||||||
          name: 'tags',
 | 
					          name: 'tags',
 | 
				
			||||||
 | 
					          through: 'post_tag',
 | 
				
			||||||
 | 
					          foreignKey: 'post_id',
 | 
				
			||||||
 | 
					          otherKey: 'tag_id',
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					          type: 'hasMany',
 | 
				
			||||||
 | 
					          name: 'posts_tags',
 | 
				
			||||||
 | 
					          foreignKey: 'post_id',
 | 
				
			||||||
 | 
					          target: 'post_tag',
 | 
				
			||||||
        },
 | 
					        },
 | 
				
			||||||
      ],
 | 
					      ],
 | 
				
			||||||
    });
 | 
					    });
 | 
				
			||||||
@ -31,6 +68,9 @@ describe('update', () => {
 | 
				
			|||||||
        {
 | 
					        {
 | 
				
			||||||
          type: 'belongsToMany',
 | 
					          type: 'belongsToMany',
 | 
				
			||||||
          name: 'posts',
 | 
					          name: 'posts',
 | 
				
			||||||
 | 
					          through: 'post_tag',
 | 
				
			||||||
 | 
					          foreignKey: 'tag_id',
 | 
				
			||||||
 | 
					          otherKey: 'post_id',
 | 
				
			||||||
        },
 | 
					        },
 | 
				
			||||||
      ],
 | 
					      ],
 | 
				
			||||||
    });
 | 
					    });
 | 
				
			||||||
@ -38,6 +78,110 @@ describe('update', () => {
 | 
				
			|||||||
    await db.sync();
 | 
					    await db.sync();
 | 
				
			||||||
  });
 | 
					  });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  it('should throw error when update data conflicted', async () => {
 | 
				
			||||||
 | 
					    const t1 = await Tag.repository.create({
 | 
				
			||||||
 | 
					      values: {
 | 
				
			||||||
 | 
					        name: 't1',
 | 
				
			||||||
 | 
					      },
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					    const t2 = await Tag.repository.create({
 | 
				
			||||||
 | 
					      values: {
 | 
				
			||||||
 | 
					        name: 't2',
 | 
				
			||||||
 | 
					      },
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					    const post1 = await Post.repository.create({
 | 
				
			||||||
 | 
					      values: {
 | 
				
			||||||
 | 
					        title: 'p1',
 | 
				
			||||||
 | 
					        tags: [t1.get('id')],
 | 
				
			||||||
 | 
					      },
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    const postTag = await PostTag.repository.findOne();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    let error;
 | 
				
			||||||
 | 
					    try {
 | 
				
			||||||
 | 
					      await Post.repository.update({
 | 
				
			||||||
 | 
					        filterByTk: post1.get('id'),
 | 
				
			||||||
 | 
					        values: {
 | 
				
			||||||
 | 
					          posts_tags: [
 | 
				
			||||||
 | 
					            {
 | 
				
			||||||
 | 
					              id: postTag.get('id'),
 | 
				
			||||||
 | 
					              tag: t1.get('id'),
 | 
				
			||||||
 | 
					              post: post1.get('id'),
 | 
				
			||||||
 | 
					            },
 | 
				
			||||||
 | 
					          ],
 | 
				
			||||||
 | 
					          tags: [t2.get('id')],
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					    } catch (err) {
 | 
				
			||||||
 | 
					      error = err;
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					    expect(error).toBeDefined();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    let error2;
 | 
				
			||||||
 | 
					    try {
 | 
				
			||||||
 | 
					      await Post.repository.update({
 | 
				
			||||||
 | 
					        filterByTk: post1.get('id'),
 | 
				
			||||||
 | 
					        values: {
 | 
				
			||||||
 | 
					          tags: [t2.get('id')],
 | 
				
			||||||
 | 
					          posts_tags: [
 | 
				
			||||||
 | 
					            {
 | 
				
			||||||
 | 
					              id: postTag.get('id'),
 | 
				
			||||||
 | 
					              tag: t1.get('id'),
 | 
				
			||||||
 | 
					              post: post1.get('id'),
 | 
				
			||||||
 | 
					            },
 | 
				
			||||||
 | 
					          ],
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					    } catch (err) {
 | 
				
			||||||
 | 
					      error2 = err;
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					    expect(error2).toBeDefined();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    const User = db.collection({
 | 
				
			||||||
 | 
					      name: 'users',
 | 
				
			||||||
 | 
					      fields: [
 | 
				
			||||||
 | 
					        { type: 'string', name: 'name' },
 | 
				
			||||||
 | 
					        { type: 'hasMany', name: 'posts', target: 'posts', foreignKey: 'user_id' },
 | 
				
			||||||
 | 
					      ],
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    await db.sync();
 | 
				
			||||||
 | 
					    const u1 = await User.repository.create({
 | 
				
			||||||
 | 
					      values: {
 | 
				
			||||||
 | 
					        name: 'u1',
 | 
				
			||||||
 | 
					        posts: [post1.get('id')],
 | 
				
			||||||
 | 
					      },
 | 
				
			||||||
 | 
					    });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    let error3;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    try {
 | 
				
			||||||
 | 
					      await User.repository.update({
 | 
				
			||||||
 | 
					        filterByTk: u1.get('id'),
 | 
				
			||||||
 | 
					        values: {
 | 
				
			||||||
 | 
					          posts: [
 | 
				
			||||||
 | 
					            {
 | 
				
			||||||
 | 
					              id: post1.get('id'),
 | 
				
			||||||
 | 
					              tags: [t2.get('id')],
 | 
				
			||||||
 | 
					              posts_tags: [
 | 
				
			||||||
 | 
					                {
 | 
				
			||||||
 | 
					                  id: postTag.get('id'),
 | 
				
			||||||
 | 
					                  tag: t1.get('id'),
 | 
				
			||||||
 | 
					                  post: post1.get('id'),
 | 
				
			||||||
 | 
					                },
 | 
				
			||||||
 | 
					              ],
 | 
				
			||||||
 | 
					            },
 | 
				
			||||||
 | 
					          ],
 | 
				
			||||||
 | 
					        },
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					    } catch (err) {
 | 
				
			||||||
 | 
					      error3 = err;
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    expect(error3).toBeDefined();
 | 
				
			||||||
 | 
					  });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  it('should update tags to null', async () => {
 | 
					  it('should update tags to null', async () => {
 | 
				
			||||||
    await db.getRepository('posts').create({
 | 
					    await db.getRepository('posts').create({
 | 
				
			||||||
      values: {
 | 
					      values: {
 | 
				
			||||||
 | 
				
			|||||||
@ -2,6 +2,7 @@ import lodash from 'lodash';
 | 
				
			|||||||
import { ModelStatic } from 'sequelize';
 | 
					import { ModelStatic } from 'sequelize';
 | 
				
			||||||
import { Model } from './model';
 | 
					import { Model } from './model';
 | 
				
			||||||
import { AssociationKeysToBeUpdate, BlackList, WhiteList } from './repository';
 | 
					import { AssociationKeysToBeUpdate, BlackList, WhiteList } from './repository';
 | 
				
			||||||
 | 
					import { isPlainObject } from '@nocobase/utils/src';
 | 
				
			||||||
 | 
					
 | 
				
			||||||
type UpdateValueItem = string | number | UpdateValues;
 | 
					type UpdateValueItem = string | number | UpdateValues;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@ -58,6 +59,42 @@ export class UpdateGuard {
 | 
				
			|||||||
    this.blackList = blackList;
 | 
					    this.blackList = blackList;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  checkValues(values) {
 | 
				
			||||||
 | 
					    const dfs = (values, model) => {
 | 
				
			||||||
 | 
					      const associations = model.associations;
 | 
				
			||||||
 | 
					      const belongsToManyThroughNames = [];
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      const associationValueKeys = Object.keys(associations).filter((key) => {
 | 
				
			||||||
 | 
					        return Object.keys(values).includes(key);
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      const belongsToManyValueKeys = associationValueKeys.filter((key) => {
 | 
				
			||||||
 | 
					        return associations[key].associationType === 'BelongsToMany';
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      const hasManyValueKeys = associationValueKeys.filter((key) => {
 | 
				
			||||||
 | 
					        return associations[key].associationType === 'HasMany';
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      for (const belongsToManyKey of belongsToManyValueKeys) {
 | 
				
			||||||
 | 
					        const association = associations[belongsToManyKey];
 | 
				
			||||||
 | 
					        const through = association.through.model;
 | 
				
			||||||
 | 
					        belongsToManyThroughNames.push(through.name);
 | 
				
			||||||
 | 
					      }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      for (const hasManyKey of hasManyValueKeys) {
 | 
				
			||||||
 | 
					        const association = associations[hasManyKey];
 | 
				
			||||||
 | 
					        if (belongsToManyThroughNames.includes(association.target.name)) {
 | 
				
			||||||
 | 
					          throw new Error(
 | 
				
			||||||
 | 
					            `HasMany association ${hasManyKey} cannot be used with BelongsToMany association ${association.target.name} with same through model`,
 | 
				
			||||||
 | 
					          );
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
 | 
					      }
 | 
				
			||||||
 | 
					    };
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    dfs(values, this.model);
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  /**
 | 
					  /**
 | 
				
			||||||
   * Sanitize values by whitelist blacklist
 | 
					   * Sanitize values by whitelist blacklist
 | 
				
			||||||
   * @param values
 | 
					   * @param values
 | 
				
			||||||
@ -69,6 +106,8 @@ export class UpdateGuard {
 | 
				
			|||||||
      throw new Error('please set model first');
 | 
					      throw new Error('please set model first');
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    this.checkValues(values);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    const associations = this.model.associations;
 | 
					    const associations = this.model.associations;
 | 
				
			||||||
    const associationsValues = lodash.pick(values, Object.keys(associations));
 | 
					    const associationsValues = lodash.pick(values, Object.keys(associations));
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
		Reference in New Issue
	
	Block a user