diff --git a/packages/@n8n/api-types/src/schemas/data-store.schema.ts b/packages/@n8n/api-types/src/schemas/data-store.schema.ts index be3119d1781..639578774a2 100644 --- a/packages/@n8n/api-types/src/schemas/data-store.schema.ts +++ b/packages/@n8n/api-types/src/schemas/data-store.schema.ts @@ -85,5 +85,5 @@ export interface IDataStoreService { dataStoreId: string, dto: Partial, ): Promise<{ count: number; data: DataStoreRows[] }>; - appendRows(dataStoreId: string, rows: DataStoreRows): Promise; + insertRows(dataStoreId: string, rows: DataStoreRows): Promise; } diff --git a/packages/cli/src/modules/data-store/__tests__/data-store.service.test.ts b/packages/cli/src/modules/data-store/__tests__/data-store.service.test.ts index b8763e7c0cf..6e52cc29dab 100644 --- a/packages/cli/src/modules/data-store/__tests__/data-store.service.test.ts +++ b/packages/cli/src/modules/data-store/__tests__/data-store.service.test.ts @@ -2,9 +2,10 @@ import { createTeamProject, testDb, testModules } from '@n8n/backend-test-utils' import { Project } from '@n8n/db'; import { Container } from '@n8n/di'; +import { DataStoreRowsRepository } from '../data-store-rows.repository'; import type { DataStoreEntity } from '../data-store.entity'; import { DataStoreRepository } from '../data-store.repository'; -import { DataStoreService } from '../data-store.service'; +import { DataStoreService, toTableName } from '../data-store.service'; beforeAll(async () => { await testModules.loadModules(['data-store']); @@ -23,10 +24,12 @@ afterAll(async () => { describe('dataStore', () => { let dataStoreService: DataStoreService; let dataStoreRepository: DataStoreRepository; + let dataStoreRowsRepository: DataStoreRowsRepository; beforeAll(() => { dataStoreService = Container.get(DataStoreService); dataStoreRepository = Container.get(DataStoreRepository); + dataStoreRowsRepository = Container.get(DataStoreRowsRepository); }); let project1: Project; @@ -556,8 +559,8 @@ describe('dataStore', () => { }); }); - describe('appendRows', () => { - it('appends a row to an existing table', async () => { + describe('insertRows', () => { + it('inserts rows into an existing table', async () => { // ARRANGE await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); await dataStoreService.addColumn(dataStore1.id, { name: 'c2', type: 'boolean' }); @@ -565,14 +568,56 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); // ACT - const result = await dataStoreService.appendRows(dataStore1.id, [ + const rows = [ { c1: 3, c2: true, c3: new Date(), c4: 'hello?' }, { c1: 4, c2: false, c3: new Date(), c4: 'hello!' }, { c1: 5, c2: true, c3: new Date(), c4: 'hello.' }, - ]); + ]; + const result = await dataStoreService.insertRows(dataStore1.id, rows); // ASSERT expect(result).toBe(true); + + const { count, data } = await dataStoreRowsRepository.getManyAndCount( + toTableName(dataStore1.id), + {}, + ); + expect(count).toEqual(3); + expect(data).toEqual( + rows.map((row, i) => ({ + ...row, + id: i + 1, + c2: row.c2 ? 1 : 0, // booleans are stored as numbers in the db + })), + ); + }); + + it('inserts a row even if it matches with the existing one', async () => { + // ARRANGE + await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); + await dataStoreService.addColumn(dataStore1.id, { name: 'c2', type: 'string' }); + + // Insert initial row + await dataStoreService.insertRows(dataStore1.id, [{ c1: 1, c2: 'foo' }]); + + // Attempt to insert a row with the same primary key + const result = await dataStoreService.insertRows(dataStore1.id, [{ c1: 1, c2: 'foo' }]); + + // ASSERT + expect(result).toBe(true); + + const { count, data } = await dataStoreRowsRepository.getManyAndCount( + toTableName(dataStore1.id), + {}, + ); + + console.log(data); + + expect(count).toEqual(2); + expect(data).toEqual([ + { c1: 1, c2: 'foo', id: 1 }, + { c1: 1, c2: 'foo', id: 2 }, + ]); }); it('rejects a mismatched row with extra column', async () => { @@ -583,7 +628,7 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); // ACT - const result = dataStoreService.appendRows(dataStore1.id, [ + const result = dataStoreService.insertRows(dataStore1.id, [ { c1: 3, c2: true, c3: new Date(), c4: 'hello?' }, { cWrong: 3, c1: 4, c2: true, c3: new Date(), c4: 'hello?' }, ]); @@ -591,6 +636,7 @@ describe('dataStore', () => { // ASSERT await expect(result).rejects.toThrow('mismatched key count'); }); + it('rejects a mismatched row with missing column', async () => { // ARRANGE await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); @@ -599,7 +645,7 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); // ACT - const result = dataStoreService.appendRows(dataStore1.id, [ + const result = dataStoreService.insertRows(dataStore1.id, [ { c1: 3, c2: true, c3: new Date(), c4: 'hello?' }, { c2: true, c3: new Date(), c4: 'hello?' }, ]); @@ -607,6 +653,7 @@ describe('dataStore', () => { // ASSERT await expect(result).rejects.toThrow('mismatched key count'); }); + it('rejects a mismatched row with replaced column', async () => { // ARRANGE await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); @@ -615,7 +662,7 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); // ACT - const result = dataStoreService.appendRows(dataStore1.id, [ + const result = dataStoreService.insertRows(dataStore1.id, [ { c1: 3, c2: true, c3: new Date(), c4: 'hello?' }, { cWrong: 3, c2: true, c3: new Date(), c4: 'hello?' }, ]); @@ -623,6 +670,7 @@ describe('dataStore', () => { // ASSERT await expect(result).rejects.toThrow('unknown column name'); }); + it('rejects unknown data store id', async () => { // ARRANGE await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); @@ -631,7 +679,7 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); // ACT - const result = dataStoreService.appendRows('this is not an id', [ + const result = dataStoreService.insertRows('this is not an id', [ { c1: 3, c2: true, c3: new Date(), c4: 'hello?' }, { cWrong: 3, c2: true, c3: new Date(), c4: 'hello?' }, ]); @@ -641,23 +689,25 @@ describe('dataStore', () => { 'no columns found for this data store or data store not found', ); }); + it('rejects on empty column list', async () => { // ARRANGE // ACT - const result = dataStoreService.appendRows('this is not an id', [{}, {}]); + const result = dataStoreService.insertRows('this is not an id', [{}, {}]); // ASSERT await expect(result).rejects.toThrow( 'no columns found for this data store or data store not found', ); }); + it('fails on type mismatch', async () => { // ARRANGE await dataStoreService.addColumn(dataStore1.id, { name: 'c1', type: 'number' }); // ACT - const result = dataStoreService.appendRows(dataStore1.id, [{ c1: 3 }, { c1: true }]); + const result = dataStoreService.insertRows(dataStore1.id, [{ c1: 3 }, { c1: true }]); // ASSERT await expect(result).rejects.toThrow("value 'true' does not match column type 'number'"); @@ -672,7 +722,7 @@ describe('dataStore', () => { await dataStoreService.addColumn(dataStore1.id, { name: 'c3', type: 'date' }); await dataStoreService.addColumn(dataStore1.id, { name: 'c4', type: 'string' }); - await dataStoreService.appendRows(dataStore1.id, [ + await dataStoreService.insertRows(dataStore1.id, [ { c1: 3, c2: true, c3: new Date(0), c4: 'hello?' }, { c1: 4, c2: false, c3: new Date(1), c4: 'hello!' }, { c1: 5, c2: true, c3: new Date(2), c4: 'hello.' }, diff --git a/packages/cli/src/modules/data-store/__tests__/sql-utils.test.ts b/packages/cli/src/modules/data-store/__tests__/sql-utils.test.ts index 86674cca75d..2ffa347d34e 100644 --- a/packages/cli/src/modules/data-store/__tests__/sql-utils.test.ts +++ b/packages/cli/src/modules/data-store/__tests__/sql-utils.test.ts @@ -4,7 +4,7 @@ import { createUserTableQuery, addColumnQuery, deleteColumnQuery, - insertIntoQuery, + buildInsertQuery, } from '../utils/sql-utils'; describe('sql-utils', () => { @@ -56,7 +56,7 @@ describe('sql-utils', () => { }); }); - describe('insertIntoQuery', () => { + describe('buildInsertQuery', () => { it('should generate a valid SQL query for inserting rows into a table', () => { const tableName = 'data_store_user_abc'; const rows = [ @@ -64,9 +64,9 @@ describe('sql-utils', () => { { name: 'Bob', age: 25 }, ]; - const [query, parameters] = insertIntoQuery(tableName, rows); + const [query, parameters] = buildInsertQuery(tableName, rows); - expect(query).toBe('INSERT INTO data_store_user_abc (name,age) VALUES (?,?),(?,?)'); + expect(query).toBe('INSERT INTO data_store_user_abc ("name", "age") VALUES (?,?),(?,?)'); expect(parameters).toEqual(['Alice', 30, 'Bob', 25]); }); @@ -74,7 +74,7 @@ describe('sql-utils', () => { const tableName = 'data_store_user_abc'; const rows: [] = []; - const [query, parameters] = insertIntoQuery(tableName, rows); + const [query, parameters] = buildInsertQuery(tableName, rows); expect(query).toBe(''); expect(parameters).toEqual([]); @@ -84,7 +84,7 @@ describe('sql-utils', () => { const tableName = 'data_store_user_abc'; const rows = [{}]; - const [query, parameters] = insertIntoQuery(tableName, rows); + const [query, parameters] = buildInsertQuery(tableName, rows); expect(query).toBe(''); expect(parameters).toEqual([]); diff --git a/packages/cli/src/modules/data-store/data-store-rows.repository.ts b/packages/cli/src/modules/data-store/data-store-rows.repository.ts index dc8434ea8ed..6774ef17f8f 100644 --- a/packages/cli/src/modules/data-store/data-store-rows.repository.ts +++ b/packages/cli/src/modules/data-store/data-store-rows.repository.ts @@ -6,7 +6,7 @@ import type { import { Service } from '@n8n/di'; import { DataSource, SelectQueryBuilder } from '@n8n/typeorm'; -import { insertIntoQuery } from './utils/sql-utils'; +import { buildInsertQuery } from './utils/sql-utils'; // type QueryBuilder = SelectQueryBuilder>; type QueryBuilder = SelectQueryBuilder; @@ -38,8 +38,8 @@ function getConditionSQL(filter: ListDataStoreContentQueryDto['filter']['filters export class DataStoreRowsRepository { constructor(private dataSource: DataSource) {} - async appendRows(tableName: DataStoreUserTableName, rows: DataStoreRows) { - await this.dataSource.query(...insertIntoQuery(tableName, rows)); + async insertRows(tableName: DataStoreUserTableName, rows: DataStoreRows) { + await this.dataSource.query(...buildInsertQuery(tableName, rows)); return true; } diff --git a/packages/cli/src/modules/data-store/data-store.controller.ts b/packages/cli/src/modules/data-store/data-store.controller.ts index ef8b61ce28f..7a9ccb3931f 100644 --- a/packages/cli/src/modules/data-store/data-store.controller.ts +++ b/packages/cli/src/modules/data-store/data-store.controller.ts @@ -126,7 +126,7 @@ export class DataStoreController { return await this.dataStoreService.getManyRowsAndCount(dataStoreId, dto); } - @Post('/:dataStoreId/append') + @Post('/:dataStoreId/insert') @ProjectScope('dataStore:writeRow') async appendDataStoreRows( _req: AuthenticatedRequest<{ projectId: string }>, @@ -134,6 +134,6 @@ export class DataStoreController { @Param('dataStoreId') dataStoreId: string, @Body dto: DataStoreRows, ) { - return await this.dataStoreService.appendRows(dataStoreId, dto); + return await this.dataStoreService.insertRows(dataStoreId, dto); } } diff --git a/packages/cli/src/modules/data-store/data-store.service.ts b/packages/cli/src/modules/data-store/data-store.service.ts index cbb4d7955bf..654f5cca4de 100644 --- a/packages/cli/src/modules/data-store/data-store.service.ts +++ b/packages/cli/src/modules/data-store/data-store.service.ts @@ -19,7 +19,7 @@ import { DataStoreColumnRepository } from './data-store-column.repository'; import { DataStoreRowsRepository } from './data-store-rows.repository'; import { DataStoreRepository } from './data-store.repository'; -function toTableName(dataStoreId: string): DataStoreUserTableName { +export function toTableName(dataStoreId: string): DataStoreUserTableName { return `data_store_user_${dataStoreId}`; } @@ -312,12 +312,12 @@ export class DataStoreService implements IDataStoreService { return true; } - async appendRows(dataStoreId: string, rows: DataStoreRows) { + async insertRows(dataStoreId: string, rows: DataStoreRows) { const validationResult = await this.validateRows(dataStoreId, rows); if (!validationResult) { return validationResult; } - return await this.dataStoreRowsRepository.appendRows(toTableName(dataStoreId), rows); + return await this.dataStoreRowsRepository.insertRows(toTableName(dataStoreId), rows); } } diff --git a/packages/cli/src/modules/data-store/utils/sql-utils.ts b/packages/cli/src/modules/data-store/utils/sql-utils.ts index 2202033a5d6..0ea0fd480e7 100644 --- a/packages/cli/src/modules/data-store/utils/sql-utils.ts +++ b/packages/cli/src/modules/data-store/utils/sql-utils.ts @@ -82,9 +82,10 @@ export function deleteColumnQuery(tableName: DataStoreUserTableName, column: str return `ALTER TABLE ${tableName} DROP COLUMN \`${column}\``; } -export function insertIntoQuery( +export function buildInsertQuery( tableName: DataStoreUserTableName, rows: DataStoreRows, + dbType: DataSourceOptions['type'] = 'sqlite', ): [string, unknown[]] { if (rows.length === 0) { return ['', []]; @@ -96,13 +97,31 @@ export function insertIntoQuery( return ['', []]; } + const quotedKeys = keys.map((key) => quoteIdentifier(key, dbType)).join(', '); + const wildcards = keys.map((_) => '?').join(','); const rowsQuery = Array(rows.length).fill(`(${wildcards})`).join(','); const parameters = Array(rows.length * keys.length); + for (let i = 0; i < keys.length; ++i) { for (let j = 0; j < rows.length; ++j) { parameters[j * keys.length + i] = rows[j][keys[i]]; } } - return [`INSERT INTO ${tableName} (${keys.join(',')}) VALUES ${rowsQuery}`, parameters]; + const query = `INSERT INTO ${tableName} (${quotedKeys}) VALUES ${rowsQuery}`; + + return [query, parameters]; +} + +function quoteIdentifier(name: string, dbType: DataSourceOptions['type']): string { + switch (dbType) { + case 'postgres': + case 'aurora-postgres': + case 'sqlite': + case 'sqlite-pooled': + case 'better-sqlite3': + return `"${name}"`; + default: + return `\`${name}\``; + } }