From 7084e8d35fd7c5ff22c66edcad98550c538c766f Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Thu, 4 Jul 2024 21:48:51 +0800 Subject: [PATCH 1/4] chore(ci): release database after closed (#4819) * chore(ci): release database after closed * chore: clean database * chore: test --- .github/workflows/nocobase-test-backend.yml | 2 +- .../core/test/src/scripts/test-db-creator.ts | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nocobase-test-backend.yml b/.github/workflows/nocobase-test-backend.yml index 6731cf64e0..6fb34b1ff8 100644 --- a/.github/workflows/nocobase-test-backend.yml +++ b/.github/workflows/nocobase-test-backend.yml @@ -111,7 +111,7 @@ jobs: DB_SCHEMA: ${{ matrix.schema }} COLLECTION_MANAGER_SCHEMA: ${{ matrix.collection_schema }} DB_TEST_DISTRIBUTOR_PORT: 23450 - DB_TEST_PREFIX: test_ + DB_TEST_PREFIX: test timeout-minutes: 60 mysql-test: diff --git a/packages/core/test/src/scripts/test-db-creator.ts b/packages/core/test/src/scripts/test-db-creator.ts index f76289a19e..edd7358b4a 100644 --- a/packages/core/test/src/scripts/test-db-creator.ts +++ b/packages/core/test/src/scripts/test-db-creator.ts @@ -36,6 +36,11 @@ abstract class BaseClient { await this._createDB(name); this.createdDBs.add(name); + + // remove db after 3 minutes + setTimeout(async () => { + await this.removeDB(name); + }, 3 * 60 * 1000); } async releaseAll() { @@ -51,6 +56,16 @@ abstract class BaseClient { this.createdDBs.delete(name); } } + + async removeDB(name: string) { + if (!this._client) { + return; + } + if (this.createdDBs.has(name)) { + await this._removeDB(name); + this.createdDBs.delete(name); + } + } } class PostgresClient extends BaseClient { @@ -156,8 +171,9 @@ const server = http.createServer((req, res) => { res.end(JSON.stringify({ error })); }); } else if (trimmedPath === 'release') { + const name = parsedUrl.query.name as string | undefined; dbClient - .releaseAll() + .removeDB(name) .then(() => { res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(); From 5dc626d2d67cfa79076aca6a3a30372c0fcd8e05 Mon Sep 17 00:00:00 2001 From: chenos Date: Fri, 5 Jul 2024 06:47:21 +0800 Subject: [PATCH 2/4] fix: target key value is empty (#4796) --- .../database/src/__tests__/target-key.test.ts | 232 ++++++++++++++++++ .../core/database/src/update-associations.ts | 4 + packages/core/database/src/update-guard.ts | 4 +- 3 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 packages/core/database/src/__tests__/target-key.test.ts diff --git a/packages/core/database/src/__tests__/target-key.test.ts b/packages/core/database/src/__tests__/target-key.test.ts new file mode 100644 index 0000000000..4c315ef003 --- /dev/null +++ b/packages/core/database/src/__tests__/target-key.test.ts @@ -0,0 +1,232 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { Database } from '../database'; +import { mockDatabase } from './index'; + +describe('targetKey', () => { + let db: Database; + + beforeEach(async () => { + db = mockDatabase(); + await db.clean({ drop: true }); + }); + + afterEach(async () => { + await db.close(); + }); + + test('default targetKey', async () => { + db.collection({ + name: 'a1', + fields: [ + { + type: 'hasMany', + name: 'b1', + target: 'b1', + }, + ], + }); + db.collection({ + name: 'b1', + fields: [], + }); + await db.sync(); + const r1 = db.getRepository('a1'); + const r2 = db.getRepository('b1'); + const b1 = await r2.create({ + values: {}, + }); + await r1.create({ + values: { + name: 'a1', + b1: [b1.toJSON()], + }, + }); + const b1r = await b1.reload(); + expect(b1r.a1Id).toBe(b1.id); + }); + + test('targetKey=code', async () => { + db.collection({ + name: 'a1', + fields: [ + { + type: 'hasMany', + name: 'b1', + target: 'b1', + targetKey: 'code', + }, + ], + }); + db.collection({ + name: 'b1', + fields: [ + { + type: 'string', + name: 'code', + }, + ], + }); + await db.sync(); + const r1 = db.getRepository('a1'); + const r2 = db.getRepository('b1'); + const b1 = await r2.create({ + values: {}, + }); + await r1.create({ + values: { + name: 'a1', + b1: [b1.toJSON()], + }, + }); + const b1r = await b1.reload(); + expect(b1r.a1Id).toBe(b1.id); + }); + + test('should throw an error', async () => { + db.collection({ + name: 'a1', + fields: [ + { + type: 'hasMany', + name: 'b1', + target: 'b1', + targetKey: 'code', + }, + ], + }); + db.collection({ + name: 'b1', + fields: [ + { + type: 'string', + name: 'code', + unique: true, + }, + ], + }); + await db.sync(); + const r1 = db.getRepository('a1'); + const r2 = db.getRepository('b1'); + const b1 = await r2.create({ + values: {}, + }); + await expect(async () => { + await r1.create({ + values: { + name: 'a1', + b1: [b1.toJSON()], + }, + }); + }).rejects.toThrowError('code field value is empty'); + }); + + test('should find by code', async () => { + db.collection({ + name: 'a1', + fields: [ + { + type: 'hasMany', + name: 'b1', + target: 'b1', + targetKey: 'code', + }, + ], + }); + db.collection({ + name: 'b1', + fields: [ + { + type: 'string', + name: 'code', + unique: true, + }, + ], + }); + await db.sync(); + const r1 = db.getRepository('a1'); + const r2 = db.getRepository('b1'); + const b1 = await r2.create({ + values: { + code: 'code1', + }, + }); + await r1.create({ + values: { + name: 'a1', + b1: [b1.toJSON()], + }, + }); + const b1r = await b1.reload(); + expect(b1r.a1Id).toBe(b1.id); + }); + + test('should find by a1Code and code', async () => { + db.collection({ + name: 'a1', + fields: [ + { + type: 'string', + name: 'code', + unique: true, + }, + { + type: 'hasMany', + name: 'b1', + target: 'b1', + sourceKey: 'code', + foreignKey: 'a1Code', + targetKey: 'code', + }, + ], + }); + db.collection({ + name: 'b1', + indexes: [ + { + type: 'UNIQUE', + fields: ['a1Code', 'code'], + }, + ], + fields: [ + { + type: 'string', + name: 'a1Code', + }, + { + type: 'string', + name: 'code', + }, + ], + }); + await db.sync(); + const r1 = db.getRepository('a1'); + const r2 = db.getRepository('b1'); + await r2.create({ + values: { + code: 'b1', + }, + }); + const b1 = await r2.create({ + values: { + code: 'b1', + }, + }); + await r1.create({ + values: { + code: 'a1', + b1: [b1.toJSON()], + }, + }); + const b1r = await b1.reload(); + expect(b1r.a1Code).toBe('a1'); + expect(b1r.code).toBe('b1'); + }); +}); diff --git a/packages/core/database/src/update-associations.ts b/packages/core/database/src/update-associations.ts index 14629cb1a3..d7889fc60c 100644 --- a/packages/core/database/src/update-associations.ts +++ b/packages/core/database/src/update-associations.ts @@ -490,6 +490,10 @@ export async function updateMultipleAssociation( accessorOptions['through'] = throughValue; } + if (pk !== targetKey && !isUndefinedOrNull(item[pk]) && isUndefinedOrNull(item[targetKey])) { + throw new Error(`${targetKey} field value is empty`); + } + if (isUndefinedOrNull(item[targetKey])) { // create new record const instance = await model[createAccessor](item, accessorOptions); diff --git a/packages/core/database/src/update-guard.ts b/packages/core/database/src/update-guard.ts index 80dacd7ea3..12d0e84df0 100644 --- a/packages/core/database/src/update-guard.ts +++ b/packages/core/database/src/update-guard.ts @@ -157,8 +157,8 @@ export class UpdateGuard { return value; } - const associationKeyName = (associationObj).targetKey - ? (associationObj).targetKey + const associationKeyName = associationObj?.['options']?.targetKey + ? associationObj['options'].targetKey : associationObj.target.primaryKeyAttribute; if (value[associationKeyName]) { From a1b2c25a946931fe454cda52fd44ec5c017fd4e9 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Fri, 5 Jul 2024 08:46:35 +0800 Subject: [PATCH 3/4] feat: beforeAddDataSource hook (#4810) --- .../src/__tests__/data-source-manager.test.ts | 18 ++++++++++++++++++ .../src/data-source-manager.ts | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/core/data-source-manager/src/__tests__/data-source-manager.test.ts b/packages/core/data-source-manager/src/__tests__/data-source-manager.test.ts index 8170bd3a2f..701bd7e191 100644 --- a/packages/core/data-source-manager/src/__tests__/data-source-manager.test.ts +++ b/packages/core/data-source-manager/src/__tests__/data-source-manager.test.ts @@ -165,6 +165,24 @@ describe('example', () => { await app.destroy(); }); + it('should call beforeAddDataSource hook', async () => { + const hook = vi.fn(); + + const app = await createMockServer({ + acl: false, + resourcer: { + prefix: '/api/', + }, + name: 'update-filter', + }); + + app.dataSourceManager.beforeAddDataSource(hook); + // it should be called on main datasource + expect(hook).toBeCalledTimes(1); + + await app.destroy(); + }); + it('should register every datasource instance', async () => { const hook = vi.fn(); diff --git a/packages/core/data-source-manager/src/data-source-manager.ts b/packages/core/data-source-manager/src/data-source-manager.ts index 01fbe0abb0..74532ffe07 100644 --- a/packages/core/data-source-manager/src/data-source-manager.ts +++ b/packages/core/data-source-manager/src/data-source-manager.ts @@ -21,6 +21,7 @@ export class DataSourceManager { factory: DataSourceFactory = new DataSourceFactory(); protected middlewares = []; private onceHooks: Array = []; + private beforeAddHooks: Array = []; constructor(public options = {}) { this.dataSources = new Map(); @@ -32,6 +33,10 @@ export class DataSourceManager { } async add(dataSource: DataSource, options: any = {}) { + for (const hook of this.beforeAddHooks) { + hook(dataSource); + } + await dataSource.load(options); this.dataSources.set(dataSource.name, dataSource); @@ -71,6 +76,13 @@ export class DataSourceManager { return this.factory.create(type, options); } + beforeAddDataSource(hook: DataSourceHook) { + this.beforeAddHooks.push(hook); + for (const dataSource of this.dataSources.values()) { + hook(dataSource); + } + } + afterAddDataSource(hook: DataSourceHook) { this.addHookAndRun(hook); } From cfcf9291dce67fe415c118f3451d9f8b780578d7 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Fri, 5 Jul 2024 08:49:24 +0800 Subject: [PATCH 4/4] fix(database): should not add field when binding error (#4804) * fix(database): should not add field when binding error * fix: test * chore: test * chore: test * chore: test * chore: test * Update nocobase-test-backend.yml * chore: test --------- Co-authored-by: Chareice --- packages/core/database/src/collection.ts | 8 ++ .../src/server/__tests__/field-load.test.ts | 91 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 packages/plugins/@nocobase/plugin-data-source-main/src/server/__tests__/field-load.test.ts diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 6404aa3ea6..688b5c8a18 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -48,6 +48,8 @@ function EnsureAtomicity(target: any, propertyKey: string, descriptor: PropertyD const model = this.model; const beforeAssociationKeys = Object.keys(model.associations); const beforeRawAttributes = Object.keys(model.rawAttributes); + const fieldName = args[0]; + const beforeField = this.getField(fieldName); try { return originalMethod.apply(this, args); @@ -64,6 +66,12 @@ function EnsureAtomicity(target: any, propertyKey: string, descriptor: PropertyD for (const key of createdRawAttributes) { delete this.model.rawAttributes[key]; } + + // remove field created in this method + if (!beforeField) { + this.removeField(fieldName); + } + throw error; } }; diff --git a/packages/plugins/@nocobase/plugin-data-source-main/src/server/__tests__/field-load.test.ts b/packages/plugins/@nocobase/plugin-data-source-main/src/server/__tests__/field-load.test.ts new file mode 100644 index 0000000000..960e7ca8fd --- /dev/null +++ b/packages/plugins/@nocobase/plugin-data-source-main/src/server/__tests__/field-load.test.ts @@ -0,0 +1,91 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { Database, Field, Repository } from '@nocobase/database'; +import { Application } from '@nocobase/server'; +import { createApp } from '.'; + +class MockField extends Field { + get dataType() { + return 'mock'; + } + + bind() { + throw new Error('MockField not implemented.'); + } +} + +describe('load field', async () => { + let db: Database; + let app: Application; + + let collectionRepository: Repository; + + let fieldsRepository: Repository; + + beforeEach(async () => { + app = await createApp({ + database: { + tablePrefix: '', + }, + }); + + db = app.db; + db.registerFieldTypes({ + mock: MockField, + }); + + collectionRepository = db.getCollection('collections').repository; + fieldsRepository = db.getCollection('fields').repository; + }); + + afterEach(async () => { + await app.destroy(); + }); + + it('should not in collection when binding error', async () => { + const collection = await collectionRepository.create({ + values: { + name: 'test1', + fields: [ + { + type: 'bigInt', + name: 'id', + }, + ], + }, + }); + await collection.load(); + expect(db.hasCollection('test1')).toBeTruthy(); + try { + await db.sequelize.transaction(async (transaction) => { + const field = await fieldsRepository.create({ + values: { + name: 'mock', + collectionName: 'test1', + type: 'mock', + }, + transaction, + }); + await field.load({ transaction }); + }); + } catch (error) { + expect(error.message).toBe('MockField not implemented.'); + } + + const instance = await fieldsRepository.findOne({ + filter: { + name: 'mock', + }, + }); + expect(instance).toBeFalsy(); + const field = db.getCollection('test1').getField('mock'); + expect(field).toBeUndefined(); + }); +});