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 <chareice@live.com>

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;
     }
   };

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();
+  });
+});