fix: Overhaul DataTableRowWithId type and include createdAt, insertedAt in getMany return (no-changelog) (#18787)

This commit is contained in:
Charlie Kolb
2025-08-26 16:16:12 +02:00
committed by GitHub
parent 46432da41b
commit af2157f384
9 changed files with 180 additions and 48 deletions

View File

@@ -209,7 +209,7 @@ describe('DataStoreProxyService', () => {
);
await dataStoreOperations.insertRows(rows);
expect(dataStoreServiceMock.insertRows).toBeCalledWith('dataStore-id', PROJECT_ID, rows);
expect(dataStoreServiceMock.insertRows).toBeCalledWith('dataStore-id', PROJECT_ID, rows, true);
});
it('should call upsertRows with correct parameters', async () => {
@@ -225,6 +225,11 @@ describe('DataStoreProxyService', () => {
);
await dataStoreOperations.upsertRows(options);
expect(dataStoreServiceMock.upsertRows).toBeCalledWith('dataStore-id', PROJECT_ID, options);
expect(dataStoreServiceMock.upsertRows).toBeCalledWith(
'dataStore-id',
PROJECT_ID,
options,
true,
);
});
});

View File

@@ -2048,11 +2048,15 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => {
id: 1,
first: 'first row',
second: 'some value',
createdAt: expect.any(String),
updatedAt: expect.any(String),
},
{
id: 2,
first: 'another row',
second: 'another value',
createdAt: expect.any(String),
updatedAt: expect.any(String),
},
],
});
@@ -3038,9 +3042,27 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/upsert', () => {
expect(result.body.data).toEqual(
expect.arrayContaining([
{ id: 1, first: 'test row', second: 'updated value' },
{ id: 2, first: 'test row', second: 'updated value' },
{ id: 3, first: 'new row', second: 'new value' },
{
id: 1,
first: 'test row',
second: 'updated value',
createdAt: expect.any(String),
updatedAt: expect.any(String),
},
{
id: 2,
first: 'test row',
second: 'updated value',
createdAt: expect.any(String),
updatedAt: expect.any(String),
},
{
id: 3,
first: 'new row',
second: 'new value',
createdAt: expect.any(String),
updatedAt: expect.any(String),
},
]),
);
});

View File

@@ -1111,9 +1111,33 @@ describe('dataStore', () => {
true,
);
expect(ids).toEqual([
{ id: 1, c1: 1, c2: 'foo', c3: true, c4: now },
{ id: 2, c1: 2, c2: 'bar', c3: false, c4: now },
{ id: 3, c1: null, c2: null, c3: null, c4: null },
{
id: 1,
c1: 1,
c2: 'foo',
c3: true,
c4: now,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
},
{
id: 2,
c1: 2,
c2: 'bar',
c3: false,
c4: now,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
},
{
id: 3,
c1: null,
c2: null,
c3: null,
c4: null,
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
},
]);
});
@@ -1489,6 +1513,8 @@ describe('dataStore', () => {
age: 31,
pid: '1995-111a',
birthday: new Date('1995-01-01'),
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
},
{
id: 2,
@@ -1496,6 +1522,8 @@ describe('dataStore', () => {
age: 30,
pid: '1992-222b',
birthday: new Date('1992-01-01'),
createdAt: expect.any(Date),
updatedAt: expect.any(Date),
},
]),
);
@@ -1725,11 +1753,11 @@ describe('dataStore', () => {
expect(updatedRows[0].createdAt).not.toBeNull();
expect(updatedRows[0].updatedAt).not.toBeNull();
expect(initialRows[0].updatedAt).not.toBeNull();
expect(new Date(updatedRows[0].updatedAt as string).getTime()).toBeGreaterThan(
new Date(initialRows[0].updatedAt as string).getTime(),
expect(new Date(updatedRows[0].updatedAt).getTime()).toBeGreaterThan(
new Date(initialRows[0].updatedAt).getTime(),
);
expect(new Date(updatedRows[0].updatedAt as string).getTime()).toBeGreaterThan(
new Date(updatedRows[0].createdAt as string).getTime(),
expect(new Date(updatedRows[0].updatedAt).getTime()).toBeGreaterThan(
new Date(updatedRows[0].createdAt).getTime(),
);
});

View File

@@ -125,11 +125,15 @@ export class DataStoreProxyService implements DataStoreProxyProvider {
},
async insertRows(rows: DataStoreRows) {
return await dataStoreService.insertRows(dataStoreId, projectId, rows);
return await dataStoreService.insertRows(dataStoreId, projectId, rows, true);
},
async upsertRows(options: UpsertDataStoreRowsOptions) {
return await dataStoreService.upsertRows(dataStoreId, projectId, options);
return await dataStoreService.upsertRows(dataStoreId, projectId, options, true);
},
async deleteRows(ids: number[]) {
return await dataStoreService.deleteRows(dataStoreId, projectId, ids);
},
};
}

View File

@@ -6,8 +6,10 @@ import { DataSource, DataSourceOptions, QueryRunner, SelectQueryBuilder, In } fr
import {
DataStoreColumnJsType,
DataStoreRows,
DataStoreRowWithId,
DataStoreRowReturn,
UnexpectedError,
DataStoreRowsReturn,
DATA_TABLE_SYSTEM_COLUMNS,
} from 'n8n-workflow';
import { DataStoreColumn } from './data-store-column.entity';
@@ -116,20 +118,29 @@ export class DataStoreRowsRepository {
return `${tablePrefix}data_store_user_${dataStoreId}`;
}
async insertRows<T extends boolean | undefined>(
dataStoreId: string,
rows: DataStoreRows,
columns: DataStoreColumn[],
returnData?: T,
): Promise<Array<T extends true ? DataStoreRowReturn : Pick<DataStoreRowReturn, 'id'>>>;
async insertRows(
dataStoreId: string,
rows: DataStoreRows,
columns: DataStoreColumn[],
returnData: boolean = false,
) {
const inserted: DataStoreRowWithId[] = [];
returnData?: boolean,
): Promise<Array<DataStoreRowReturn | Pick<DataStoreRowReturn, 'id'>>> {
const inserted: Array<Pick<DataStoreRowReturn, 'id'>> = [];
const dbType = this.dataSource.options.type;
const useReturning = dbType === 'postgres' || dbType === 'mariadb';
const table = this.toTableName(dataStoreId);
const columnNames = columns.map((c) => c.name);
const escapedColumns = columns.map((c) => this.dataSource.driver.escape(c.name));
const selectColumns = ['id', ...escapedColumns];
const escapedSystemColumns = DATA_TABLE_SYSTEM_COLUMNS.map((x) =>
this.dataSource.driver.escape(x),
);
const selectColumns = [...escapedSystemColumns, ...escapedColumns];
// 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
@@ -153,7 +164,9 @@ export class DataStoreRowsRepository {
const result = await query.execute();
if (useReturning) {
const returned = normalizeRows(extractReturningData(result.raw), columns);
const returned = returnData
? normalizeRows(extractReturningData(result.raw), columns)
: extractInsertedIds(result.raw, dbType).map((id) => ({ id }));
inserted.push.apply(inserted, returned);
continue;
}
@@ -169,11 +182,7 @@ export class DataStoreRowsRepository {
continue;
}
const insertedRows = (await this.getManyByIds(
dataStoreId,
ids,
columns,
)) as DataStoreRowWithId[];
const insertedRows = await this.getManyByIds(dataStoreId, ids, columns);
inserted.push(...insertedRows);
}
@@ -193,7 +202,10 @@ export class DataStoreRowsRepository {
const table = this.toTableName(dataStoreId);
const escapedColumns = columns.map((c) => this.dataSource.driver.escape(c.name));
const selectColumns = ['id', ...escapedColumns];
const escapedSystemColumns = DATA_TABLE_SYSTEM_COLUMNS.map((x) =>
this.dataSource.driver.escape(x),
);
const selectColumns = [...escapedSystemColumns, ...escapedColumns];
for (const column of columns) {
if (column.name in setData) {
@@ -204,7 +216,7 @@ export class DataStoreRowsRepository {
}
}
let affectedRows: DataStoreRowWithId[] = [];
let affectedRows: Array<Pick<DataStoreRowReturn, 'id'>> = [];
if (!useReturning && returnData) {
// Only Postgres supports RETURNING statement on updates (with our typeorm),
// on other engines we must query the list of updates rows later by ID
@@ -239,20 +251,28 @@ export class DataStoreRowsRepository {
}
// TypeORM cannot infer the columns for a dynamic table name, so we use a raw query
async upsertRows<T extends boolean | undefined>(
dataStoreId: string,
matchFields: string[],
rows: DataStoreRows,
columns: DataStoreColumn[],
returnData?: T,
): Promise<T extends true ? DataStoreRowReturn[] : true>;
async upsertRows(
dataStoreId: string,
matchFields: string[],
rows: DataStoreRows,
columns: DataStoreColumn[],
returnData = false,
returnData?: boolean,
) {
returnData = returnData ?? false;
const { rowsToInsert, rowsToUpdate } = await this.fetchAndSplitRowsByExistence(
dataStoreId,
matchFields,
rows,
);
const output: DataStoreRowWithId[] = [];
const output: DataStoreRowReturn[] = [];
if (rowsToInsert.length > 0) {
const result = await this.insertRows(dataStoreId, rowsToInsert, columns, returnData);
@@ -335,7 +355,7 @@ export class DataStoreRowsRepository {
async getManyAndCount(dataStoreId: string, dto: ListDataStoreContentQueryDto) {
const [countQuery, query] = this.getManyQuery(dataStoreId, dto);
const data: DataStoreRows = await query.select('*').getRawMany();
const data: DataStoreRowsReturn = await query.select('*').getRawMany();
const countResult = await countQuery.select('COUNT(*) as count').getRawOne<{
count: number | string | null;
}>();
@@ -347,7 +367,10 @@ export class DataStoreRowsRepository {
async getManyByIds(dataStoreId: string, ids: number[], columns: DataStoreColumn[]) {
const table = this.toTableName(dataStoreId);
const escapedColumns = columns.map((c) => this.dataSource.driver.escape(c.name));
const selectColumns = ['id', ...escapedColumns];
const escapedSystemColumns = DATA_TABLE_SYSTEM_COLUMNS.map((x) =>
this.dataSource.driver.escape(x),
);
const selectColumns = [...escapedSystemColumns, ...escapedColumns];
if (ids.length === 0) {
return [];
@@ -358,7 +381,7 @@ export class DataStoreRowsRepository {
.select(selectColumns)
.from(table, 'dataStore')
.where({ id: In(ids) })
.getRawMany<DataStoreRowWithId>();
.getRawMany<DataStoreRowReturn>();
return normalizeRows(updatedRows, columns);
}

View File

@@ -34,6 +34,7 @@ import { DataStoreColumnNotFoundError } from './errors/data-store-column-not-fou
import { DataStoreNameConflictError } from './errors/data-store-name-conflict.error';
import { DataStoreNotFoundError } from './errors/data-store-not-found.error';
import { DataStoreValidationError } from './errors/data-store-validation.error';
import { DataStoreRowReturn } from 'n8n-workflow';
@RestController('/projects/:projectId/data-stores')
export class DataStoreController {
@@ -237,6 +238,12 @@ export class DataStoreController {
/**
* @returns the IDs of the inserted rows
*/
async appendDataStoreRows<T extends boolean | undefined>(
req: AuthenticatedRequest<{ projectId: string }>,
_res: Response,
dataStoreId: string,
dto: AddDataStoreRowsDto & { returnData?: T },
): Promise<Array<T extends true ? DataStoreRowReturn : Pick<DataStoreRowReturn, 'id'>>>;
@Post('/:dataStoreId/insert')
@ProjectScope('dataStore:writeRow')
async appendDataStoreRows(

View File

@@ -11,7 +11,7 @@ import type {
} from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
import { Service } from '@n8n/di';
import type { DataStoreRow, DataStoreRows } from 'n8n-workflow';
import type { DataStoreRow, DataStoreRowReturn, DataStoreRows } from 'n8n-workflow';
import { DataStoreColumnRepository } from './data-store-column.repository';
import { DataStoreRowsRepository } from './data-store-rows.repository';
@@ -126,11 +126,17 @@ export class DataStoreService {
return await this.dataStoreColumnRepository.getColumns(dataStoreId);
}
async insertRows<T extends boolean | undefined>(
dataStoreId: string,
projectId: string,
rows: DataStoreRows,
returnData?: T,
): Promise<Array<T extends true ? DataStoreRowReturn : Pick<DataStoreRowReturn, 'id'>>>;
async insertRows(
dataStoreId: string,
projectId: string,
rows: DataStoreRows,
returnData: boolean = false,
returnData?: boolean,
) {
await this.validateDataStoreExists(dataStoreId, projectId);
await this.validateRows(dataStoreId, rows);
@@ -139,6 +145,12 @@ export class DataStoreService {
return await this.dataStoreRowsRepository.insertRows(dataStoreId, rows, columns, returnData);
}
async upsertRows<T extends boolean | undefined>(
dataStoreId: string,
projectId: string,
dto: Omit<UpsertDataStoreRowsDto, 'returnData'>,
returnData?: T,
): Promise<T extends true ? DataStoreRowReturn[] : true>;
async upsertRows(
dataStoreId: string,
projectId: string,

View File

@@ -5,13 +5,18 @@ import {
} from '@n8n/api-types';
import { DslColumn } from '@n8n/db';
import type { DataSourceOptions } from '@n8n/typeorm';
import type { DataStoreColumnJsType, DataStoreRows, DataStoreRowWithId } from 'n8n-workflow';
import type {
DataStoreColumnJsType,
DataStoreRows,
DataStoreRowReturn,
DataStoreRowsReturn,
} from 'n8n-workflow';
import { UnexpectedError } from 'n8n-workflow';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import type { DataStoreUserTableName } from '../data-store.types';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
export function toDslColumns(columns: DataStoreCreateColumnSchema[]): DslColumn[] {
return columns.map((col) => {
const name = new DslColumn(col.name.trim());
@@ -138,7 +143,6 @@ export function quoteIdentifier(name: string, dbType: DataSourceOptions['type'])
}
type WithInsertId = { insertId: number };
type WithRowId = { id: number };
const isArrayOf = <T>(data: unknown, itemGuard: (x: unknown) => x is T): data is T[] =>
Array.isArray(data) && data.every(itemGuard);
@@ -147,18 +151,35 @@ const isNumber = (value: unknown): value is number => {
return typeof value === 'number' && Number.isFinite(value);
};
const isDate = (value: unknown): value is Date => {
return value instanceof Date;
};
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 {
function hasRowReturnData(data: unknown): data is DataStoreRowReturn {
return (
typeof data === 'object' &&
data !== null &&
'id' in data &&
isNumber(data.id) &&
'createdAt' in data &&
isDate(data.createdAt) &&
'updatedAt' in data &&
isDate(data.updatedAt)
);
}
function hasRowId(data: unknown): data is Pick<DataStoreRowReturn, 'id'> {
return typeof data === 'object' && data !== null && 'id' in data && isNumber(data.id);
}
export function extractReturningData(raw: unknown): DataStoreRowWithId[] {
if (!isArrayOf(raw, hasRowId)) {
export function extractReturningData(raw: unknown): DataStoreRowReturn[] {
if (!isArrayOf(raw, hasRowReturnData)) {
throw new UnexpectedError(
'Expected INSERT INTO raw to be { id: number }[] on Postgres or MariaDB',
`Expected INSERT INTO raw to be { id: number; createdAt: string; updatedAt: string }[] on Postgres or MariaDB. Is '${JSON.stringify(raw)}'`,
);
}
@@ -171,7 +192,7 @@ export function extractInsertedIds(raw: unknown, dbType: DataSourceOptions['type
case 'mariadb': {
if (!isArrayOf(raw, hasRowId)) {
throw new UnexpectedError(
'Expected INSERT INTO raw to be { id: number }[] on Postgres or MariaDB',
`Expected INSERT INTO raw to be { id: number }[] on Postgres or MariaDB. Is '${JSON.stringify(raw)}'`,
);
}
return raw.map((r) => r.id);
@@ -192,7 +213,7 @@ export function extractInsertedIds(raw: unknown, dbType: DataSourceOptions['type
}
}
export function normalizeRows(rows: DataStoreRows, columns: DataStoreColumn[]) {
export function normalizeRows(rows: DataStoreRowsReturn, columns: DataStoreColumn[]) {
// we need to normalize system dates as well
const systemColumns = [
{ name: 'createdAt', type: 'date' },

View File

@@ -72,9 +72,17 @@ export type AddDataStoreColumnOptions = Pick<DataStoreColumn, 'name' | 'type'> &
export type DataStoreColumnJsType = string | number | boolean | Date | null;
export const DATA_TABLE_SYSTEM_COLUMNS = ['id', 'createdAt', 'updatedAt'] as const;
export type DataStoreRowReturnBase = {
id: number;
createdAt: Date;
updatedAt: Date;
};
export type DataStoreRow = Record<string, DataStoreColumnJsType>;
export type DataStoreRows = DataStoreRow[];
export type DataStoreRowWithId = DataStoreRow & { id: number };
export type DataStoreRowReturn = DataStoreRow & DataStoreRowReturnBase;
export type DataStoreRowsReturn = DataStoreRowReturn[];
// APIs for a data store service operating on a specific projectId
export interface IDataStoreProjectAggregateService {
@@ -100,9 +108,11 @@ export interface IDataStoreProjectService {
getManyRowsAndCount(
dto: Partial<ListDataStoreRowsOptions>,
): Promise<{ count: number; data: DataStoreRows }>;
): Promise<{ count: number; data: DataStoreRowsReturn }>;
insertRows(rows: DataStoreRows): Promise<Array<{ id: number }>>;
insertRows(rows: DataStoreRows): Promise<DataStoreRowReturn[]>;
upsertRows(options: UpsertDataStoreRowsOptions): Promise<boolean | DataStoreRowWithId[]>;
upsertRows(options: UpsertDataStoreRowsOptions): Promise<DataStoreRowReturn[]>;
deleteRows(ids: number[]): Promise<boolean>;
}