diff --git a/packages/cli/src/modules/data-store/__tests__/data-store.controller.test.ts b/packages/cli/src/modules/data-store/__tests__/data-store.controller.test.ts index c13459d50d..6e138442b0 100644 --- a/packages/cli/src/modules/data-store/__tests__/data-store.controller.test.ts +++ b/packages/cli/src/modules/data-store/__tests__/data-store.controller.test.ts @@ -1830,17 +1830,18 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${project.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); - const readResponse = await authMemberAgent - .get(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) - .expect(200); + expect(response.body).toEqual({ + data: [1], + }); - expect(readResponse.body.data.count).toBe(1); - expect(readResponse.body.data.data[0]).toMatchObject(payload.data[0]); + const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {}); + expect(rowsInDb.count).toBe(1); + expect(rowsInDb.data[0]).toMatchObject(payload.data[0]); }); test('should insert rows if user has project:admin role in team project', async () => { @@ -1869,11 +1870,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authAdminAgent + const response = await authAdminAgent .post(`/projects/${project.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {}); expect(rowsInDb.count).toBe(1); expect(rowsInDb.data[0]).toMatchObject(payload.data[0]); @@ -1902,11 +1907,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {}); expect(rowsInDb.count).toBe(1); expect(rowsInDb.data[0]).toMatchObject(payload.data[0]); @@ -1968,11 +1977,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const readResponse = await authMemberAgent .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) .expect(200); @@ -2012,11 +2025,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const readResponse = await authMemberAgent .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) .expect(200); @@ -2048,11 +2065,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const readResponse = await authMemberAgent .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) .expect(200); @@ -2099,11 +2120,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const readResponse = await authMemberAgent .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) .expect(200); @@ -2145,11 +2170,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { ], }; - await authMemberAgent + const response = await authMemberAgent .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) .send(payload) .expect(200); + expect(response.body).toEqual({ + data: [1], + }); + const readResponse = await authMemberAgent .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) .expect(200); @@ -2157,6 +2186,63 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { expect(readResponse.body.data.count).toBe(1); expect(readResponse.body.data.data[0]).toMatchObject(payload.data[0]); }); + + test('should insert multiple rows', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { + name: 'a', + type: 'string', + }, + { + name: 'b', + type: 'number', + }, + ], + }); + + const payload = { + data: [ + { + a: 'first', + b: 1, + }, + { + a: 'second', + b: 2, + }, + { + a: 'third', + b: 3, + }, + ], + }; + + const first = await authMemberAgent + .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) + .send(payload) + .expect(200); + + expect(first.body).toEqual({ + data: [1, 2, 3], + }); + + const second = await authMemberAgent + .post(`/projects/${memberProject.id}/data-stores/${dataStore.id}/insert`) + .send(payload) + .expect(200); + + expect(second.body).toEqual({ + data: [4, 5, 6], + }); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(6); + expect(readResponse.body.data.data).toMatchObject([...payload.data, ...payload.data]); + }); }); describe('DELETE /projects/:projectId/data-stores/:dataStoreId/rows', () => { 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 9412427b49..1005ae5b43 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 @@ -457,12 +457,14 @@ describe('dataStore', () => { ], }); - await dataStoreService.insertRows(dataStoreId, project1.id, [ + const results = await dataStoreService.insertRows(dataStoreId, project1.id, [ { name: 'Alice', age: 30 }, { name: 'Bob', age: 25 }, { name: 'Charlie', age: 35 }, ]); + expect(results).toEqual([1, 2, 3]); + // ACT const newColumn = await dataStoreService.addColumn(dataStoreId, project1.id, { name: 'email', @@ -487,9 +489,10 @@ describe('dataStore', () => { ]); // Verify we can insert new rows with the new column - await dataStoreService.insertRows(dataStoreId, project1.id, [ + const newRow = await dataStoreService.insertRows(dataStoreId, project1.id, [ { name: 'David', age: 28, email: 'david@example.com' }, ]); + expect(newRow).toEqual([4]); const finalData = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); expect(finalData.count).toBe(4); @@ -973,7 +976,7 @@ describe('dataStore', () => { const result = await dataStoreService.insertRows(dataStoreId, project1.id, rows); // ASSERT - expect(result).toBe(true); + expect(result).toEqual([1, 2, 3, 4]); const { count, data } = await dataStoreService.getManyRowsAndCount( dataStoreId, @@ -1004,7 +1007,10 @@ describe('dataStore', () => { }); // Insert initial row - await dataStoreService.insertRows(dataStoreId, project1.id, [{ c1: 1, c2: 'foo' }]); + const initial = await dataStoreService.insertRows(dataStoreId, project1.id, [ + { c1: 1, c2: 'foo' }, + ]); + expect(initial).toEqual([1]); // Attempt to insert a row with the same primary key const result = await dataStoreService.insertRows(dataStoreId, project1.id, [ @@ -1012,7 +1018,7 @@ describe('dataStore', () => { ]); // ASSERT - expect(result).toBe(true); + expect(result).toEqual([2]); const { count, data } = await dataStoreRowsRepository.getManyAndCount( toTableName(dataStoreId), @@ -1026,6 +1032,47 @@ describe('dataStore', () => { ]); }); + it('return correct IDs even after deletions', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'myDataStore', + columns: [ + { name: 'c1', type: 'number' }, + { name: 'c2', type: 'string' }, + ], + }); + + // Insert initial row + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, [ + { c1: 1, c2: 'foo' }, + { c1: 2, c2: 'bar' }, + ]); + expect(ids).toEqual([1, 2]); + + await dataStoreService.deleteRows(dataStoreId, project1.id, [ids[0]]); + + // Insert a new row + const result = await dataStoreService.insertRows(dataStoreId, project1.id, [ + { c1: 1, c2: 'baz' }, + { c1: 2, c2: 'faz' }, + ]); + + // ASSERT + expect(result).toEqual([3, 4]); + + const { count, data } = await dataStoreRowsRepository.getManyAndCount( + toTableName(dataStoreId), + {}, + ); + + expect(count).toEqual(3); + expect(data).toEqual([ + { c1: 2, c2: 'bar', id: 2 }, + { c1: 1, c2: 'baz', id: 3 }, + { c1: 2, c2: 'faz', id: 4 }, + ]); + }); + it('rejects a mismatched row with extra column', async () => { // ARRANGE const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { @@ -1185,9 +1232,10 @@ describe('dataStore', () => { }); // Insert initial row - await dataStoreService.insertRows(dataStoreId, project1.id, [ + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, [ { pid: '1995-111a', fullName: 'Alice', age: 30 }, ]); + expect(ids).toEqual([1]); // ACT const result = await dataStoreService.upsertRows(dataStoreId, project1.id, { @@ -1219,9 +1267,10 @@ describe('dataStore', () => { }); // Insert initial row - await dataStoreService.insertRows(dataStoreId, project1.id, [ + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, [ { pid: '1995-111a', fullName: 'Alice', age: 30 }, ]); + expect(ids).toEqual([1]); // ACT const result = await dataStoreService.upsertRows(dataStoreId, project1.id, { @@ -1258,11 +1307,12 @@ describe('dataStore', () => { const { id: dataStoreId } = dataStore; // Insert test rows - await dataStoreService.insertRows(dataStoreId, project1.id, [ + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, [ { name: 'Alice', age: 30 }, { name: 'Bob', age: 25 }, { name: 'Charlie', age: 35 }, ]); + expect(ids).toEqual([1, 2, 3]); // Get initial data to find row IDs const initialData = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); @@ -1310,7 +1360,8 @@ describe('dataStore', () => { const { id: dataStoreId } = dataStore; // Insert one row - await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice' }]); + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice' }]); + expect(ids).toEqual([1]); // ACT - Try to delete existing and non-existing IDs const result = await dataStoreService.deleteRows(dataStoreId, project1.id, [1, 999, 1000]); @@ -1651,7 +1702,8 @@ describe('dataStore', () => { { c1: 5, c2: true, c3: new Date(2), c4: 'hello.' }, ]; - await dataStoreService.insertRows(dataStoreId, project1.id, rows); + const ids = await dataStoreService.insertRows(dataStoreId, project1.id, rows); + expect(ids).toEqual([1, 2, 3]); // ACT const result = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); 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 65159e48ac..d481808131 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 @@ -1,11 +1,6 @@ import type { DataStoreRows } from 'n8n-workflow'; -import { - addColumnQuery, - deleteColumnQuery, - buildInsertQuery, - splitRowsByExistence, -} from '../utils/sql-utils'; +import { addColumnQuery, deleteColumnQuery, splitRowsByExistence } from '../utils/sql-utils'; describe('sql-utils', () => { describe('addColumnQuery', () => { @@ -57,61 +52,6 @@ describe('sql-utils', () => { }); }); - describe('buildInsertQuery', () => { - it('should generate a valid SQL query for inserting rows into a table', () => { - const tableName = 'data_store_user_abc'; - const columns = [ - { name: 'name', type: 'string' }, - { name: 'age', type: 'number' }, - ]; - const rows = [ - { name: 'Alice', age: 30 }, - { name: 'Bob', age: 25 }, - ]; - - const [query, parameters] = buildInsertQuery(tableName, rows, columns, 'postgres'); - - expect(query).toBe( - 'INSERT INTO "data_store_user_abc" ("name", "age") VALUES ($1, $2), ($3, $4)', - ); - expect(parameters).toEqual(['Alice', 30, 'Bob', 25]); - }); - - it('should return an empty query and parameters when rows are empty', () => { - const tableName = 'data_store_user_abc'; - const rows: [] = []; - - const [query, parameters] = buildInsertQuery(tableName, rows, []); - - expect(query).toBe(''); - expect(parameters).toEqual([]); - }); - - it('should return an empty query and parameters when rows have no keys', () => { - const tableName = 'data_store_user_abc'; - const rows = [{}]; - - const [query, parameters] = buildInsertQuery(tableName, rows, []); - - expect(query).toBe(''); - expect(parameters).toEqual([]); - }); - - it('should replace T and Z for MySQL', () => { - const tableName = 'data_store_user_abc'; - const columns = [{ name: 'participatedAt', type: 'date' }]; - const rows = [ - { participatedAt: new Date('2021-01-01') }, - { participatedAt: new Date('2021-01-02') }, - ]; - - const [query, parameters] = buildInsertQuery(tableName, rows, columns, 'mysql'); - - expect(query).toBe('INSERT INTO `data_store_user_abc` (`participatedAt`) VALUES (?), (?)'); - expect(parameters).toEqual(['2021-01-01 00:00:00.000', '2021-01-02 00:00:00.000']); - }); - }); - describe('splitRowsByExistence', () => { it('should correctly separate rows into insert and update based on matchFields', () => { const existing = [ 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 a4c59705a4..8e5fc9f42e 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 @@ -13,14 +13,14 @@ import { QueryRunner, SelectQueryBuilder, } from '@n8n/typeorm'; -import { DataStoreRows } from 'n8n-workflow'; +import { DataStoreColumnJsType, DataStoreRows } from 'n8n-workflow'; import { DataStoreColumn } from './data-store-column.entity'; import { addColumnQuery, buildColumnTypeMap, - buildInsertQuery, deleteColumnQuery, + extractInsertedIds, getPlaceholder, normalizeValue, quoteIdentifier, @@ -58,12 +58,38 @@ export class DataStoreRowsRepository { rows: DataStoreRows, columns: DataStoreColumn[], ) { - const dbType = this.dataSource.options.type; - await this.dataSource.query.apply( - this.dataSource, - buildInsertQuery(tableName, rows, columns, dbType), - ); - return true; + const insertedIds: number[] = []; + + // We insert one by one as the default behavior of returning the last inserted ID + // is consistent, whereas getting all inserted IDs when inserting multiple values is + // surprisingly awkward without Entities, e.g. `RETURNING id` explicitly does not aggregate + // and the `identifiers` array output of `execute()` is empty + for (const row of rows) { + const dbType = this.dataSource.options.type; + + for (const column of columns) { + row[column.name] = normalizeValue(row[column.name], column.type, dbType); + } + + const query = this.dataSource + .createQueryBuilder() + .insert() + .into( + tableName, + columns.map((c) => c.name), + ) + .values(row); + + if (dbType === 'postgres' || dbType === 'mariadb') { + query.returning('id'); + } + + const result = await query.execute(); + + insertedIds.push(...extractInsertedIds(result.raw, dbType)); + } + + return insertedIds; } async upsertRows( @@ -102,8 +128,8 @@ export class DataStoreRowsRepository { async updateRow( tableName: DataStoreUserTableName, - setData: Record, - whereData: Record, + setData: Record, + whereData: Record, columns: DataStoreColumn[], ) { const dbType = this.dataSource.options.type; @@ -111,14 +137,14 @@ export class DataStoreRowsRepository { const queryBuilder = this.dataSource.createQueryBuilder().update(tableName); - const setValues: Record = {}; + const setValues: Record = {}; for (const [key, value] of Object.entries(setData)) { setValues[key] = normalizeValue(value, columnTypeMap[key], dbType); } queryBuilder.set(setValues); - const normalizedWhereData: Record = {}; + const normalizedWhereData: Record = {}; for (const [field, value] of Object.entries(whereData)) { normalizedWhereData[field] = normalizeValue(value, columnTypeMap[field], dbType); } 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 940800b97a..4b3f593c2d 100644 --- a/packages/cli/src/modules/data-store/data-store.controller.ts +++ b/packages/cli/src/modules/data-store/data-store.controller.ts @@ -234,6 +234,9 @@ export class DataStoreController { } } + /** + * @returns the IDs of the inserted rows + */ @Post('/:dataStoreId/insert') @ProjectScope('dataStore:writeRow') async appendDataStoreRows( 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 f5599669dd..5bb8281113 100644 --- a/packages/cli/src/modules/data-store/utils/sql-utils.ts +++ b/packages/cli/src/modules/data-store/utils/sql-utils.ts @@ -5,7 +5,8 @@ import { } from '@n8n/api-types'; import { DslColumn } from '@n8n/db'; import type { DataSourceOptions } from '@n8n/typeorm'; -import { UnexpectedError, type DataStoreRows } from 'n8n-workflow'; +import type { DataStoreColumnJsType, DataStoreRows } from 'n8n-workflow'; +import { UnexpectedError } from 'n8n-workflow'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -97,38 +98,6 @@ export function deleteColumnQuery( return `ALTER TABLE ${quotedTableName} DROP COLUMN ${quoteIdentifier(column, dbType)}`; } -export function buildInsertQuery( - tableName: DataStoreUserTableName, - rows: DataStoreRows, - columns: Array<{ name: string; type: string }>, - dbType: DataSourceOptions['type'] = 'sqlite', -): [string, unknown[]] { - if (rows.length === 0 || Object.keys(rows[0]).length === 0) { - return ['', []]; - } - - const keys = Object.keys(rows[0]); - const quotedKeys = keys.map((key) => quoteIdentifier(key, dbType)).join(', '); - const quotedTableName = quoteIdentifier(tableName, dbType); - - const columnTypeMap = buildColumnTypeMap(columns); - const parameters: unknown[] = []; - const valuePlaceholders: string[] = []; - let placeholderIndex = 1; - - for (const row of rows) { - const rowPlaceholders = keys.map((key) => { - const value = normalizeValue(row[key], columnTypeMap[key], dbType); - parameters.push(value); - return getPlaceholder(placeholderIndex++, dbType); - }); - valuePlaceholders.push(`(${rowPlaceholders.join(', ')})`); - } - - const query = `INSERT INTO ${quotedTableName} (${quotedKeys}) VALUES ${valuePlaceholders.join(', ')}`; - return [query, parameters]; -} - export function splitRowsByExistence( existing: Array>, matchFields: string[], @@ -172,6 +141,51 @@ export function toTableName(dataStoreId: string): DataStoreUserTableName { return `data_store_user_${dataStoreId}`; } +type WithInsertId = { insertId: number }; +type WithRowId = { id: number }; + +const isArrayOf = (data: unknown, itemGuard: (x: unknown) => x is T): data is T[] => + Array.isArray(data) && data.every(itemGuard); + +const isNumber = (value: unknown): value is number => { + return typeof value === 'number' && Number.isFinite(value); +}; + +function hasInsertId(data: unknown): data is WithInsertId { + return typeof data === 'object' && data !== null && 'insertId' in data && isNumber(data.insertId); +} + +function hasRowId(data: unknown): data is WithRowId { + return typeof data === 'object' && data !== null && 'id' in data && isNumber(data.id); +} + +export function extractInsertedIds(raw: unknown, dbType: DataSourceOptions['type']): number[] { + switch (dbType) { + case 'postgres': + case 'mariadb': { + if (!isArrayOf(raw, hasRowId)) { + throw new UnexpectedError( + 'Expected INSERT INTO raw to be { id: number }[] on Postgres or MariaDB', + ); + } + return raw.map((r) => r.id); + } + case 'mysql': { + if (!hasInsertId(raw)) { + throw new UnexpectedError('Expected INSERT INTO raw.insertId: number for MySQL'); + } + return [raw.insertId]; + } + case 'sqlite': + default: { + if (!isNumber(raw)) { + throw new UnexpectedError('Expected INSERT INTO raw to be a number for SQLite'); + } + return [raw]; + } + } +} + export function normalizeRows(rows: DataStoreRows, columns: DataStoreColumn[]) { const typeMap = new Map(columns.map((col) => [col.name, col.type])); return rows.map((row) => { @@ -210,30 +224,24 @@ export function normalizeRows(rows: DataStoreRows, columns: DataStoreColumn[]) { } export function normalizeValue( - value: unknown, + value: DataStoreColumnJsType | null, columnType: string | undefined, dbType: DataSourceOptions['type'], -): unknown { +): DataStoreColumnJsType | null { if (['mysql', 'mariadb'].includes(dbType)) { if (columnType === 'date') { - if ( - value instanceof Date || - (typeof value === 'string' && value.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/)) - ) { - return toMySQLDateTimeString(value); + if (value instanceof Date) { + return value; + } else if (typeof value === 'string') { + const date = new Date(value); + if (!isNaN(date.getTime())) { + return date; + } } } } - return value; -} -function toMySQLDateTimeString(date: Date | string, convertFromDate = true): string { - const dateString = convertFromDate - ? date instanceof Date - ? date.toISOString() - : date - : (date as string); - return dateString.replace('T', ' ').replace('Z', ''); + return value; } export function getPlaceholder(index: number, dbType: DataSourceOptions['type']): string { diff --git a/packages/workflow/src/data-store.types.ts b/packages/workflow/src/data-store.types.ts index 6de3f285b5..3ddfde62ed 100644 --- a/packages/workflow/src/data-store.types.ts +++ b/packages/workflow/src/data-store.types.ts @@ -73,7 +73,6 @@ export type AddDataStoreColumnOptions = Pick & export type DataStoreColumnJsType = string | number | boolean | Date; export type DataStoreRow = Record; - export type DataStoreRows = DataStoreRow[]; // APIs for a data store service operating on a specific projectId @@ -102,7 +101,7 @@ export interface IDataStoreProjectService { dto: Partial, ): Promise<{ count: number; data: DataStoreRows }>; - insertRows(rows: DataStoreRows): Promise; + insertRows(rows: DataStoreRows): Promise; upsertRows(options: UpsertDataStoreRowsOptions): Promise; }