mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-21 12:51:16 +08:00
fix(core): Recreate data table backing tables on entity import (#29454)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
94bf3db438
commit
6bca1fa26f
@@ -6,6 +6,7 @@ import { mock } from 'jest-mock-extended';
|
||||
import type { Cipher } from 'n8n-core';
|
||||
|
||||
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import type { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
|
||||
import type { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
|
||||
import { ImportService } from '../import.service';
|
||||
@@ -24,6 +25,9 @@ jest.mock('@n8n/db', () => ({
|
||||
CredentialsRepository: mock<CredentialsRepository>(),
|
||||
TagRepository: mock<TagRepository>(),
|
||||
DataSource: mock<DataSource>(),
|
||||
// `DataTableColumn` (transitively imported by import.service) extends
|
||||
// `WithTimestampsAndStringId`; provide a no-op so the class evaluates.
|
||||
WithTimestampsAndStringId: class {},
|
||||
}));
|
||||
|
||||
jest.mock('@/active-workflow-manager', () => ({
|
||||
@@ -40,6 +44,7 @@ describe('ImportService', () => {
|
||||
let mockCipher: Cipher;
|
||||
let mockActiveWorkflowManager: ActiveWorkflowManager;
|
||||
let mockWorkflowIndexService: WorkflowIndexService;
|
||||
let mockDataTableDDLService: DataTableDDLService;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
@@ -52,6 +57,7 @@ describe('ImportService', () => {
|
||||
mockCipher = mock<Cipher>();
|
||||
mockActiveWorkflowManager = mock<ActiveWorkflowManager>();
|
||||
mockWorkflowIndexService = mock<WorkflowIndexService>();
|
||||
mockDataTableDDLService = mock<DataTableDDLService>();
|
||||
|
||||
// Set up cipher mock
|
||||
mockCipher.decryptV2 = jest.fn(async (data: string) =>
|
||||
@@ -87,6 +93,8 @@ describe('ImportService', () => {
|
||||
mockEntityManager.query = jest.fn().mockResolvedValue(undefined);
|
||||
mockEntityManager.insert = jest.fn().mockResolvedValue(undefined);
|
||||
mockEntityManager.upsert = jest.fn().mockResolvedValue(undefined);
|
||||
// Passthrough so tests can read column fields off the result.
|
||||
mockEntityManager.create = jest.fn().mockImplementation((_entity, data) => data);
|
||||
|
||||
// Mock transaction method
|
||||
mockDataSource.transaction = jest.fn().mockImplementation(async (callback) => {
|
||||
@@ -101,6 +109,7 @@ describe('ImportService', () => {
|
||||
mockCipher,
|
||||
mockActiveWorkflowManager,
|
||||
mockWorkflowIndexService,
|
||||
mockDataTableDDLService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -971,4 +980,119 @@ describe('ImportService', () => {
|
||||
expect(mockLogger.info).toHaveBeenCalledWith('✅ Successfully decompressed entities.zip');
|
||||
});
|
||||
});
|
||||
|
||||
describe('dropExistingDataTableUserTables', () => {
|
||||
it('should drop dynamic tables for every entry in the destination registry', async () => {
|
||||
mockEntityManager.query = jest.fn().mockResolvedValue([{ id: 'abc' }, { id: 'xyz' }]);
|
||||
|
||||
await importService.dropExistingDataTableUserTables(mockEntityManager);
|
||||
|
||||
expect(mockEntityManager.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining('SELECT id FROM "data_table"'),
|
||||
);
|
||||
expect(mockDataTableDDLService.dropTable).toHaveBeenCalledTimes(2);
|
||||
expect(mockDataTableDDLService.dropTable).toHaveBeenCalledWith('abc', mockEntityManager);
|
||||
expect(mockDataTableDDLService.dropTable).toHaveBeenCalledWith('xyz', mockEntityManager);
|
||||
});
|
||||
|
||||
it('should silently skip when the registry is missing on the destination', async () => {
|
||||
mockEntityManager.query = jest.fn().mockRejectedValue(new Error('table not found'));
|
||||
|
||||
await expect(
|
||||
importService.dropExistingDataTableUserTables(mockEntityManager),
|
||||
).resolves.not.toThrow();
|
||||
|
||||
expect(mockDataTableDDLService.dropTable).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should respect the table prefix when querying the registry', async () => {
|
||||
// @ts-expect-error overriding for the test
|
||||
mockDataSource.options = { type: 'sqlite', entityPrefix: 'n8n_' };
|
||||
mockEntityManager.query = jest.fn().mockResolvedValue([]);
|
||||
|
||||
await importService.dropExistingDataTableUserTables(mockEntityManager);
|
||||
|
||||
expect(mockEntityManager.query).toHaveBeenCalledWith(
|
||||
expect.stringContaining('"n8n_data_table"'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('recreateDataTableUserTablesFromRegistry', () => {
|
||||
it('should recreate every backing table referenced in the imported registry', async () => {
|
||||
mockEntityManager.query = jest
|
||||
.fn()
|
||||
.mockResolvedValueOnce([{ id: 'abc' }, { id: 'xyz' }]) // SELECT id FROM data_table
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'col-1', dataTableId: 'abc', name: 'foo', type: 'string', index: 0 },
|
||||
{ id: 'col-2', dataTableId: 'xyz', name: 'bar', type: 'number', index: 0 },
|
||||
]); // SELECT cols
|
||||
|
||||
await importService.recreateDataTableUserTablesFromRegistry(mockEntityManager);
|
||||
|
||||
expect(mockDataTableDDLService.dropTable).toHaveBeenCalledTimes(2);
|
||||
expect(mockDataTableDDLService.createTableWithColumns).toHaveBeenCalledWith(
|
||||
'abc',
|
||||
expect.arrayContaining([expect.objectContaining({ name: 'foo', type: 'string' })]),
|
||||
mockEntityManager,
|
||||
);
|
||||
expect(mockDataTableDDLService.createTableWithColumns).toHaveBeenCalledWith(
|
||||
'xyz',
|
||||
expect.arrayContaining([expect.objectContaining({ name: 'bar', type: 'number' })]),
|
||||
mockEntityManager,
|
||||
);
|
||||
});
|
||||
|
||||
it('should sort columns by index before recreating the backing table', async () => {
|
||||
mockEntityManager.query = jest
|
||||
.fn()
|
||||
.mockResolvedValueOnce([{ id: 'abc' }])
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'col-2', dataTableId: 'abc', name: 'b', type: 'string', index: 1 },
|
||||
{ id: 'col-1', dataTableId: 'abc', name: 'a', type: 'string', index: 0 },
|
||||
]);
|
||||
|
||||
await importService.recreateDataTableUserTablesFromRegistry(mockEntityManager);
|
||||
|
||||
const call = (mockDataTableDDLService.createTableWithColumns as jest.Mock).mock.calls[0];
|
||||
const [, sortedColumns] = call;
|
||||
expect((sortedColumns as Array<{ name: string }>).map((c) => c.name)).toEqual(['a', 'b']);
|
||||
});
|
||||
|
||||
it('should drop existing tables before recreating to make the operation idempotent', async () => {
|
||||
mockEntityManager.query = jest
|
||||
.fn()
|
||||
.mockResolvedValueOnce([{ id: 'abc' }])
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'col-1', dataTableId: 'abc', name: 'foo', type: 'string', index: 0 },
|
||||
]);
|
||||
|
||||
await importService.recreateDataTableUserTablesFromRegistry(mockEntityManager);
|
||||
|
||||
const dropCallOrder = (mockDataTableDDLService.dropTable as jest.Mock).mock
|
||||
.invocationCallOrder[0];
|
||||
const createCallOrder = (mockDataTableDDLService.createTableWithColumns as jest.Mock).mock
|
||||
.invocationCallOrder[0];
|
||||
expect(dropCallOrder).toBeLessThan(createCallOrder);
|
||||
});
|
||||
|
||||
it('should skip silently when the registry is missing', async () => {
|
||||
mockEntityManager.query = jest.fn().mockRejectedValue(new Error('relation does not exist'));
|
||||
|
||||
await expect(
|
||||
importService.recreateDataTableUserTablesFromRegistry(mockEntityManager),
|
||||
).resolves.not.toThrow();
|
||||
|
||||
expect(mockDataTableDDLService.createTableWithColumns).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should no-op when the registry is empty', async () => {
|
||||
mockEntityManager.query = jest.fn().mockResolvedValueOnce([]);
|
||||
|
||||
await importService.recreateDataTableUserTablesFromRegistry(mockEntityManager);
|
||||
|
||||
expect(mockDataTableDDLService.createTableWithColumns).not.toHaveBeenCalled();
|
||||
expect(mockDataTableDDLService.dropTable).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,6 +25,8 @@ import { z } from 'zod';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import type { IWorkflowWithVersionMetadata } from '@/interfaces';
|
||||
import { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
import { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
|
||||
import { DataTableColumn } from '@/modules/data-table/data-table-column.entity';
|
||||
|
||||
@Service()
|
||||
export class ImportService {
|
||||
@@ -57,6 +59,7 @@ export class ImportService {
|
||||
private readonly cipher: Cipher,
|
||||
private readonly activeWorkflowManager: ActiveWorkflowManager,
|
||||
private readonly workflowIndexService: WorkflowIndexService,
|
||||
private readonly dataTableDDLService: DataTableDDLService,
|
||||
) {}
|
||||
|
||||
async initRecords() {
|
||||
@@ -410,6 +413,10 @@ export class ImportService {
|
||||
if (truncateTables) {
|
||||
this.logger.info('\n🗑️ Truncating tables before import...');
|
||||
|
||||
// Drop dynamic data-table user tables first; once the registry is
|
||||
// truncated we have no way to enumerate them.
|
||||
await this.dropExistingDataTableUserTables(transactionManager);
|
||||
|
||||
this.logger.info(`Found ${tableNames.length} tables to truncate: ${tableNames.join(', ')}`);
|
||||
|
||||
await Promise.all(
|
||||
@@ -437,6 +444,10 @@ export class ImportService {
|
||||
customEncryptionKey,
|
||||
);
|
||||
|
||||
// After the data_table / data_table_column registry rows are imported,
|
||||
// recreate the dynamic backing tables (empty) so the imported tables work.
|
||||
await this.recreateDataTableUserTablesFromRegistry(transactionManager);
|
||||
|
||||
if (!skipTogglingForeignKeyConstraints) {
|
||||
await this.enableForeignKeyConstraints(transactionManager);
|
||||
}
|
||||
@@ -606,6 +617,101 @@ export class ImportService {
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop every dynamic data-table user backing table referenced in the destination's
|
||||
* `data_table` registry. Called before truncating registry rows so that backing
|
||||
* tables don't end up orphaned.
|
||||
*/
|
||||
async dropExistingDataTableUserTables(transactionManager: EntityManager): Promise<void> {
|
||||
const tablePrefix = this.dataSource.options.entityPrefix || '';
|
||||
const dataTableTableName = `${tablePrefix}data_table`;
|
||||
|
||||
let existing: Array<{ id: string }>;
|
||||
try {
|
||||
existing = await transactionManager.query(
|
||||
`SELECT id FROM ${this.dataSource.driver.escape(dataTableTableName)}`,
|
||||
);
|
||||
} catch (error) {
|
||||
this.logger.info(
|
||||
` ⚠️ ${dataTableTableName} registry not found, skipping dynamic-table cleanup...`,
|
||||
{ error },
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (existing.length === 0) return;
|
||||
|
||||
this.logger.info(`🗑️ Dropping ${existing.length} existing data-table backing table(s)...`);
|
||||
for (const { id } of existing) {
|
||||
await this.dataTableDDLService.dropTable(id, transactionManager);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recreate dynamic data-table backing tables for every entry in the imported
|
||||
* `data_table` registry. The export bug (ADO-5143) means archives produced
|
||||
* before this fix only contain registry rows; without recreating the backing
|
||||
* tables the imported data tables would reference tables that don't exist on
|
||||
* the destination.
|
||||
*
|
||||
* Tables are dropped before recreation so the operation is idempotent and
|
||||
* handles stale tables left over from a partially-failed prior import.
|
||||
*/
|
||||
async recreateDataTableUserTablesFromRegistry(transactionManager: EntityManager): Promise<void> {
|
||||
const tablePrefix = this.dataSource.options.entityPrefix || '';
|
||||
const dataTableTableName = `${tablePrefix}data_table`;
|
||||
const dataTableColumnTableName = `${tablePrefix}data_table_column`;
|
||||
|
||||
let dataTables: Array<{ id: string }>;
|
||||
try {
|
||||
dataTables = await transactionManager.query(
|
||||
`SELECT id FROM ${this.dataSource.driver.escape(dataTableTableName)}`,
|
||||
);
|
||||
} catch (error) {
|
||||
this.logger.info(
|
||||
` ⚠️ ${dataTableTableName} registry not present; skipping data-table backing-table recreation.`,
|
||||
{ error },
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (dataTables.length === 0) return;
|
||||
|
||||
const escapedColumnTable = this.dataSource.driver.escape(dataTableColumnTableName);
|
||||
const escapedDataTableId = this.dataSource.driver.escape('dataTableId');
|
||||
const escapedIndex = this.dataSource.driver.escape('index');
|
||||
|
||||
const columnRows: Array<{
|
||||
id: string;
|
||||
dataTableId: string;
|
||||
name: string;
|
||||
type: 'string' | 'number' | 'boolean' | 'date';
|
||||
index: number;
|
||||
}> = await transactionManager.query(
|
||||
`SELECT id, ${escapedDataTableId}, name, type, ${escapedIndex} FROM ${escapedColumnTable}`,
|
||||
);
|
||||
|
||||
const columnsByDataTableId = new Map<string, DataTableColumn[]>();
|
||||
for (const row of columnRows) {
|
||||
const list = columnsByDataTableId.get(row.dataTableId) ?? [];
|
||||
list.push(transactionManager.create(DataTableColumn, row));
|
||||
columnsByDataTableId.set(row.dataTableId, list);
|
||||
}
|
||||
|
||||
this.logger.info(`\n📚 Recreating ${dataTables.length} data-table backing table(s)...`);
|
||||
|
||||
for (const { id: dataTableId } of dataTables) {
|
||||
const cols = (columnsByDataTableId.get(dataTableId) ?? [])
|
||||
.slice()
|
||||
.sort((a, b) => a.index - b.index);
|
||||
|
||||
await this.dataTableDDLService.dropTable(dataTableId, transactionManager);
|
||||
await this.dataTableDDLService.createTableWithColumns(dataTableId, cols, transactionManager);
|
||||
}
|
||||
|
||||
this.logger.info(`✅ Recreated ${dataTables.length} data-table backing table(s)`);
|
||||
}
|
||||
|
||||
async disableForeignKeyConstraints(transactionManager: EntityManager) {
|
||||
const disableCommand = this.foreignKeyCommands.disable[this.dataSource.options.type];
|
||||
|
||||
|
||||
@@ -69,6 +69,7 @@ describe('ImportService', () => {
|
||||
mock(),
|
||||
mockActiveWorkflowManager,
|
||||
mockWorkflowIndexService,
|
||||
mock(),
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user