diff --git a/packages/@n8n/api-types/src/dto/data-store/update-data-store-row.dto.ts b/packages/@n8n/api-types/src/dto/data-store/update-data-store-row.dto.ts new file mode 100644 index 0000000000..981e94b8cb --- /dev/null +++ b/packages/@n8n/api-types/src/dto/data-store/update-data-store-row.dto.ts @@ -0,0 +1,22 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +import { + dataStoreColumnNameSchema, + dataStoreColumnValueSchema, +} from '../../schemas/data-store.schema'; + +const updateDataStoreRowShape = { + filter: z + .record(dataStoreColumnNameSchema, dataStoreColumnValueSchema) + .refine((obj) => Object.keys(obj).length > 0, { + message: 'filter must not be empty', + }), + data: z + .record(dataStoreColumnNameSchema, dataStoreColumnValueSchema) + .refine((obj) => Object.keys(obj).length > 0, { + message: 'data must not be empty', + }), +}; + +export class UpdateDataStoreRowDto extends Z.class(updateDataStoreRowShape) {} diff --git a/packages/@n8n/api-types/src/dto/data-store/upsert-data-store-rows.dto.ts b/packages/@n8n/api-types/src/dto/data-store/upsert-data-store-rows.dto.ts index 1626588f7e..385ce39539 100644 --- a/packages/@n8n/api-types/src/dto/data-store/upsert-data-store-rows.dto.ts +++ b/packages/@n8n/api-types/src/dto/data-store/upsert-data-store-rows.dto.ts @@ -1,12 +1,13 @@ import { z } from 'zod'; import { Z } from 'zod-class'; -import { dataStoreColumnNameSchema } from '../../schemas/data-store.schema'; - -const dataStoreValueSchema = z.union([z.string(), z.number(), z.boolean(), z.date(), z.null()]); +import { + dataStoreColumnNameSchema, + dataStoreColumnValueSchema, +} from '../../schemas/data-store.schema'; const upsertDataStoreRowsShape = { - rows: z.array(z.record(dataStoreValueSchema)), + rows: z.array(z.record(dataStoreColumnNameSchema, dataStoreColumnValueSchema)), matchFields: z.array(dataStoreColumnNameSchema).min(1), }; diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 5aacc2c5ed..57cc9cec56 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -82,6 +82,7 @@ export { OidcConfigDto } from './oidc/config.dto'; export { CreateDataStoreDto } from './data-store/create-data-store.dto'; export { UpdateDataStoreDto } from './data-store/update-data-store.dto'; +export { UpdateDataStoreRowDto } from './data-store/update-data-store-row.dto'; export { UpsertDataStoreRowsDto } from './data-store/upsert-data-store-rows.dto'; export { ListDataStoreQueryDto } from './data-store/list-data-store-query.dto'; export { ListDataStoreContentQueryDto } from './data-store/list-data-store-content-query.dto'; 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 e77112161a..af8a4d9a19 100644 --- a/packages/@n8n/api-types/src/schemas/data-store.schema.ts +++ b/packages/@n8n/api-types/src/schemas/data-store.schema.ts @@ -57,5 +57,10 @@ export const dateTimeSchema = z .transform((s) => new Date(s)) .pipe(z.date()); -// Dates are received as date strings and validated before insertion -export const dataStoreColumnValueSchema = z.union([z.string(), z.number(), z.boolean(), z.null()]); +export const dataStoreColumnValueSchema = z.union([ + z.string(), + z.number(), + z.boolean(), + z.null(), + z.date(), +]); 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 cfe7028d2c..c13459d50d 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 @@ -8,11 +8,12 @@ import { import type { Project, User } from '@n8n/db'; import { ProjectRepository, QueryFailedError } from '@n8n/db'; import { Container } from '@n8n/di'; +import { DateTime } from 'luxon'; + import { createDataStore } from '@test-integration/db/data-stores'; import { createOwner, createMember, createAdmin } from '@test-integration/db/users'; import type { SuperAgentTest } from '@test-integration/types'; import * as utils from '@test-integration/utils'; -import { DateTime } from 'luxon'; import { DataStoreColumnRepository } from '../data-store-column.repository'; import { DataStoreRowsRepository } from '../data-store-rows.repository'; @@ -1834,9 +1835,12 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/insert', () => { .send(payload) .expect(200); - const rowsInDb = await dataStoreRowsRepository.getManyAndCount(toTableName(dataStore.id), {}); - expect(rowsInDb.count).toBe(1); - expect(rowsInDb.data[0]).toMatchObject(payload.data[0]); + const readResponse = await authMemberAgent + .get(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject(payload.data[0]); }); test('should insert rows if user has project:admin role in team project', async () => { @@ -2759,3 +2763,467 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/upsert', () => { expect(rowsInDb.data[2]).toMatchObject(payload.rows[1]); }); }); + +describe('PATCH /projects/:projectId/data-stores/:dataStoreId/rows', () => { + test('should not update row when project does not exist', async () => { + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authOwnerAgent + .patch('/projects/non-existing-id/data-stores/some-data-store-id/rows') + .send(payload) + .expect(403); + }); + + test('should not update row when data store does not exist', async () => { + const project = await createTeamProject('test project', owner); + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authOwnerAgent + .patch(`/projects/${project.id}/data-stores/non-existing-id/rows`) + .send(payload) + .expect(404); + }); + + test("should not update row in another user's personal project data store", async () => { + const dataStore = await createDataStore(ownerProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authMemberAgent + .patch(`/projects/${ownerProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(403); + }); + + test('should not update row if user has project:viewer role in team project', async () => { + const project = await createTeamProject('test project', owner); + await linkUserToProject(member, project, 'project:viewer'); + const dataStore = await createDataStore(project, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authMemberAgent + .patch(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(403); + }); + + test('should update row if user has project:editor role in team project', async () => { + const project = await createTeamProject('test project', owner); + await linkUserToProject(member, project, 'project:editor'); + const dataStore = await createDataStore(project, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authMemberAgent + .patch(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ id: 1, name: 'Alice', age: 31 }); + }); + + test('should update row if user has project:admin role in team project', async () => { + const project = await createTeamProject('test project', owner); + await linkUserToProject(admin, project, 'project:admin'); + const dataStore = await createDataStore(project, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authAdminAgent + .patch(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authAdminAgent + .get(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ id: 1, name: 'Alice', age: 31 }); + }); + + test('should update row if user is owner in team project', async () => { + const project = await createTeamProject('test project', owner); + const dataStore = await createDataStore(project, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authOwnerAgent + .patch(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authOwnerAgent + .get(`/projects/${project.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ id: 1, name: 'Alice', age: 31 }); + }); + + test('should update row in personal project', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, + }; + + await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ id: 1, name: 'Alice', age: 31 }); + }); + + test('should update row by id filter', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [ + { name: 'Alice', age: 30 }, + { name: 'Bob', age: 25 }, + ], + }); + + const payload = { + filter: { id: 1 }, + data: { age: 31 }, + }; + + await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(2); + expect(readResponse.body.data.data).toEqual( + expect.arrayContaining([ + { id: 1, name: 'Alice', age: 31 }, + { id: 2, name: 'Bob', age: 25 }, + ]), + ); + }); + + test('should update row with multiple filter conditions', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'department', type: 'string' }, + ], + data: [ + { name: 'Alice', age: 30, department: 'Engineering' }, + { name: 'Alice', age: 25, department: 'Marketing' }, + { name: 'Bob', age: 30, department: 'Engineering' }, + ], + }); + + const payload = { + filter: { name: 'Alice', age: 30 }, + data: { department: 'Management' }, + }; + + await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(3); + expect(readResponse.body.data.data).toEqual( + expect.arrayContaining([ + { id: 1, name: 'Alice', age: 30, department: 'Management' }, + { id: 2, name: 'Alice', age: 25, department: 'Marketing' }, + { id: 3, name: 'Bob', age: 30, department: 'Engineering' }, + ]), + ); + }); + + test('should return true when no rows match the filter', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Charlie' }, + data: { age: 25 }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + expect(response.body.data).toBe(true); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ name: 'Alice', age: 30 }); + }); + + test('should fail when filter is empty', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [{ name: 'name', type: 'string' }], + }); + + const payload = { + filter: {}, + data: { name: 'Updated' }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('filter must not be empty'); + }); + + test('should fail when data is empty', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [{ name: 'name', type: 'string' }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: {}, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('data must not be empty'); + }); + + test('should fail when data contains invalid column names', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [{ name: 'name', type: 'string' }], + data: [{ name: 'Alice' }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { invalidColumn: 'value' }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('unknown column'); + }); + + test('should fail when filter contains invalid column names', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [{ name: 'name', type: 'string' }], + data: [{ name: 'Alice' }], + }); + + const payload = { + filter: { invalidColumn: 'Alice' }, + data: { name: 'Updated' }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('unknown column'); + }); + + test('should validate data types in filter', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { age: 'invalid_number' }, + data: { name: 'Updated' }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('does not match column type'); + }); + + test('should validate data types in data', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + data: [{ name: 'Alice', age: 30 }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 'invalid_number' }, + }; + + const response = await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(400); + + expect(response.body.message).toContain('does not match column type'); + }); + + test('should allow partial updates', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'active', type: 'boolean' }, + ], + data: [{ name: 'Alice', age: 30, active: true }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { age: 31 }, // Only updating age, not name or active + }; + + await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ + name: 'Alice', + age: 31, + active: true, + }); + }); + + test('should handle date values in updates', async () => { + const dataStore = await createDataStore(memberProject, { + columns: [ + { name: 'name', type: 'string' }, + { name: 'birthdate', type: 'date' }, + ], + data: [{ name: 'Alice', birthdate: '2000-01-01T00:00:00.000Z' }], + }); + + const payload = { + filter: { name: 'Alice' }, + data: { birthdate: '1995-05-15T12:30:00.000Z' }, + }; + + await authMemberAgent + .patch(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .send(payload) + .expect(200); + + const readResponse = await authMemberAgent + .get(`/projects/${memberProject.id}/data-stores/${dataStore.id}/rows`) + .expect(200); + + expect(readResponse.body.data.count).toBe(1); + expect(readResponse.body.data.data[0]).toMatchObject({ + name: 'Alice', + birthdate: '1995-05-15T12:30:00.000Z', + }); + }); +}); 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 c8ff611ba5..9412427b49 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 @@ -1323,6 +1323,315 @@ describe('dataStore', () => { }); }); + describe('updateRow', () => { + it('should update an existing row with matching filter', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'active', type: 'boolean' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [ + { name: 'Alice', age: 30, active: true }, + { name: 'Bob', age: 25, active: false }, + ]); + + // ACT + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: { age: 31, active: false }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual( + expect.arrayContaining([ + { id: 1, name: 'Alice', age: 31, active: false }, + { id: 2, name: 'Bob', age: 25, active: false }, + ]), + ); + }); + + it('should be able to update by id', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'active', type: 'boolean' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [ + { name: 'Alice', age: 30, active: true }, + { name: 'Bob', age: 25, active: false }, + ]); + + // ACT + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { id: 1 }, + data: { name: 'Alicia', age: 31, active: false }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual( + expect.arrayContaining([ + { id: 1, name: 'Alicia', age: 31, active: false }, + { id: 2, name: 'Bob', age: 25, active: false }, + ]), + ); + }); + + it('should update row with multiple filter conditions', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'department', type: 'string' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [ + { name: 'Alice', age: 30, department: 'Engineering' }, + { name: 'Alice', age: 25, department: 'Marketing' }, + { name: 'Bob', age: 30, department: 'Engineering' }, + ]); + + // ACT + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice', age: 30 }, + data: { department: 'Management' }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual( + expect.arrayContaining([ + { id: 1, name: 'Alice', age: 30, department: 'Management' }, + { id: 2, name: 'Alice', age: 25, department: 'Marketing' }, + { id: 3, name: 'Bob', age: 30, department: 'Engineering' }, + ]), + ); + }); + + it('should return true when no rows match the filter', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice', age: 30 }]); + + // ACT + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Charlie' }, + data: { age: 25 }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual([{ id: 1, name: 'Alice', age: 30 }]); + }); + + it('should throw validation error when filters are empty', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice', age: 30 }]); + + // ACT + const result = dataStoreService.updateRow(dataStoreId, project1.id, { + filter: {}, + data: { name: 'Alice', age: 31 }, + }); + + // ASSERT + await expect(result).rejects.toThrow( + new DataStoreValidationError('Filter columns must not be empty for updateRow'), + ); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual([{ id: 1, name: 'Alice', age: 30 }]); + }); + + it('should throw validation error when data is empty', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice', age: 30 }]); + + // ACT + const result = dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: {}, + }); + + // ASSERT + await expect(result).rejects.toThrow( + new DataStoreValidationError('Data columns must not be empty for updateRow'), + ); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual([{ id: 1, name: 'Alice', age: 30 }]); + }); + + it('should fail when data store does not exist', async () => { + // ACT & ASSERT + const result = dataStoreService.updateRow('non-existent-id', project1.id, { + filter: { name: 'Alice' }, + data: { age: 25 }, + }); + + await expect(result).rejects.toThrow(DataStoreNotFoundError); + }); + + it('should fail when data contains invalid column names', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [{ name: 'name', type: 'string' }], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice' }]); + + // ACT & ASSERT + const result = dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: { invalidColumn: 'value' }, + }); + + await expect(result).rejects.toThrow(DataStoreValidationError); + }); + + it('should fail when filter contains invalid column names', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [{ name: 'name', type: 'string' }], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice' }]); + + // ACT & ASSERT + const result = dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { invalidColumn: 'Alice' }, + data: { name: 'Bob' }, + }); + + await expect(result).rejects.toThrow(DataStoreValidationError); + }); + + it('should fail when data contains invalid type values', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [{ name: 'Alice', age: 30 }]); + + // ACT & ASSERT + const result = dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: { age: 'not-a-number' }, + }); + + await expect(result).rejects.toThrow(DataStoreValidationError); + }); + + it('should allow partial data updates', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + { name: 'active', type: 'boolean' }, + ], + }); + + await dataStoreService.insertRows(dataStoreId, project1.id, [ + { name: 'Alice', age: 30, active: true }, + ]); + + // ACT - only update age, leaving name and active unchanged + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: { age: 31 }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual([{ id: 1, name: 'Alice', age: 31, active: true }]); + }); + + it('should handle date column updates correctly', async () => { + // ARRANGE + const { id: dataStoreId } = await dataStoreService.createDataStore(project1.id, { + name: 'dataStore', + columns: [ + { name: 'name', type: 'string' }, + { name: 'birthDate', type: 'date' }, + ], + }); + + const initialDate = new Date('1990-01-01'); + await dataStoreService.insertRows(dataStoreId, project1.id, [ + { name: 'Alice', birthDate: initialDate }, + ]); + + // ACT + const newDate = new Date('1991-02-02'); + const result = await dataStoreService.updateRow(dataStoreId, project1.id, { + filter: { name: 'Alice' }, + data: { birthDate: newDate.toISOString() }, + }); + + // ASSERT + expect(result).toBe(true); + + const { data } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {}); + expect(data).toEqual([{ id: 1, name: 'Alice', birthDate: newDate.toISOString() }]); + }); + }); + describe('getManyRowsAndCount', () => { it('retrieves rows correctly', async () => { // ARRANGE 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 f8fb8f3731..65159e48ac 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,6 @@ import { addColumnQuery, deleteColumnQuery, buildInsertQuery, - buildUpdateQuery, splitRowsByExistence, } from '../utils/sql-utils'; @@ -113,79 +112,6 @@ describe('sql-utils', () => { }); }); - describe('buildUpdateQuery', () => { - it('should generate a valid SQL update query with one match field', () => { - const tableName = 'data_store_user_abc'; - const row = { name: 'Alice', age: 30, city: 'Paris' }; - const columns = [ - { id: 1, name: 'name', type: 'string' }, - { id: 2, name: 'age', type: 'number' }, - { id: 3, name: 'city', type: 'string' }, - ]; - const matchFields = ['name']; - - const [query, parameters] = buildUpdateQuery(tableName, row, columns, matchFields); - - expect(query).toBe('UPDATE "data_store_user_abc" SET "age" = ?, "city" = ? WHERE "name" = ?'); - expect(parameters).toEqual([30, 'Paris', 'Alice']); - }); - - it('should generate a valid SQL update query with multiple match fields', () => { - const tableName = 'data_store_user_abc'; - const row = { name: 'Alice', age: 30, city: 'Paris' }; - const columns = [ - { id: 1, name: 'name', type: 'string' }, - { id: 2, name: 'age', type: 'number' }, - { id: 3, name: 'city', type: 'string' }, - ]; - const matchFields = ['name', 'city']; - - const [query, parameters] = buildUpdateQuery(tableName, row, columns, matchFields); - - expect(query).toBe( - 'UPDATE "data_store_user_abc" SET "age" = ? WHERE "name" = ? AND "city" = ?', - ); - expect(parameters).toEqual([30, 'Alice', 'Paris']); - }); - - it('should return empty query and parameters if row is empty', () => { - const tableName = 'data_store_user_abc'; - const row = {}; - const matchFields = ['id']; - - const [query, parameters] = buildUpdateQuery(tableName, row, [], matchFields); - - expect(query).toBe(''); - expect(parameters).toEqual([]); - }); - - it('should return empty query and parameters if matchFields is empty', () => { - const tableName = 'data_store_user_abc'; - const row = { name: 'Alice', age: 30 }; - const columns = [ - { id: 1, name: 'name', type: 'string' }, - { id: 2, name: 'age', type: 'number' }, - ]; - const matchFields: string[] = []; - - const [query, parameters] = buildUpdateQuery(tableName, row, columns, matchFields); - - expect(query).toBe(''); - expect(parameters).toEqual([]); - }); - - it('should return empty query and parameters if only matchFields are present in row', () => { - const tableName = 'data_store_user_abc'; - const row = { id: 1 }; - const matchFields = ['id']; - - const [query, parameters] = buildUpdateQuery(tableName, row, [], matchFields); - - expect(query).toBe(''); - expect(parameters).toEqual([]); - }); - }); - 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 23b39ce8fc..a4c59705a4 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,20 +13,21 @@ import { QueryRunner, SelectQueryBuilder, } from '@n8n/typeorm'; +import { DataStoreRows } from 'n8n-workflow'; import { DataStoreColumn } from './data-store-column.entity'; import { addColumnQuery, + buildColumnTypeMap, buildInsertQuery, - buildUpdateQuery, deleteColumnQuery, getPlaceholder, + normalizeValue, quoteIdentifier, splitRowsByExistence, toDslColumns, toTableName, } from './utils/sql-utils'; -import { DataStoreRows } from 'n8n-workflow'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type QueryBuilder = SelectQueryBuilder; @@ -70,13 +71,8 @@ export class DataStoreRowsRepository { dto: UpsertDataStoreRowsDto, columns: DataStoreColumn[], ) { - const dbType = this.dataSource.options.type; const { rows, matchFields } = dto; - if (rows.length === 0) { - return false; - } - const { rowsToInsert, rowsToUpdate } = await this.fetchAndSplitRowsByExistence( tableName, matchFields, @@ -89,15 +85,48 @@ export class DataStoreRowsRepository { if (rowsToUpdate.length > 0) { for (const row of rowsToUpdate) { - // TypeORM cannot infer the columns for a dynamic table name, so we use a raw query - const [query, parameters] = buildUpdateQuery(tableName, row, columns, matchFields, dbType); - await this.dataSource.query(query, parameters); + const updateKeys = Object.keys(row).filter((key) => !matchFields.includes(key)); + if (updateKeys.length === 0) { + return true; + } + + const setData = Object.fromEntries(updateKeys.map((key) => [key, row[key]])); + const whereData = Object.fromEntries(matchFields.map((key) => [key, row[key]])); + + await this.updateRow(tableName, setData, whereData, columns); } } return true; } + async updateRow( + tableName: DataStoreUserTableName, + setData: Record, + whereData: Record, + columns: DataStoreColumn[], + ) { + const dbType = this.dataSource.options.type; + const columnTypeMap = buildColumnTypeMap(columns); + + const queryBuilder = this.dataSource.createQueryBuilder().update(tableName); + + const setValues: Record = {}; + for (const [key, value] of Object.entries(setData)) { + setValues[key] = normalizeValue(value, columnTypeMap[key], dbType); + } + + queryBuilder.set(setValues); + + const normalizedWhereData: Record = {}; + for (const [field, value] of Object.entries(whereData)) { + normalizedWhereData[field] = normalizeValue(value, columnTypeMap[field], dbType); + } + queryBuilder.where(normalizedWhereData); + + await queryBuilder.execute(); + } + async deleteRows(tableName: DataStoreUserTableName, ids: number[]) { if (ids.length === 0) { 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 6a9592080b..940800b97a 100644 --- a/packages/cli/src/modules/data-store/data-store.controller.ts +++ b/packages/cli/src/modules/data-store/data-store.controller.ts @@ -7,6 +7,7 @@ import { ListDataStoreQueryDto, MoveDataStoreColumnDto, UpdateDataStoreDto, + UpdateDataStoreRowDto, UpsertDataStoreRowsDto, } from '@n8n/api-types'; import { AuthenticatedRequest } from '@n8n/db'; @@ -22,6 +23,11 @@ import { RestController, } from '@n8n/decorators'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ConflictError } from '@/errors/response-errors/conflict.error'; +import { InternalServerError } from '@/errors/response-errors/internal-server.error'; +import { NotFoundError } from '@/errors/response-errors/not-found.error'; + import { DataStoreService } from './data-store.service'; import { DataStoreColumnNameConflictError } from './errors/data-store-column-name-conflict.error'; import { DataStoreColumnNotFoundError } from './errors/data-store-column-not-found.error'; @@ -29,11 +35,6 @@ import { DataStoreNameConflictError } from './errors/data-store-name-conflict.er import { DataStoreNotFoundError } from './errors/data-store-not-found.error'; import { DataStoreValidationError } from './errors/data-store-validation.error'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { ConflictError } from '@/errors/response-errors/conflict.error'; -import { InternalServerError } from '@/errors/response-errors/internal-server.error'; -import { NotFoundError } from '@/errors/response-errors/not-found.error'; - @RestController('/projects/:projectId/data-stores') export class DataStoreController { constructor(private readonly dataStoreService: DataStoreService) {} @@ -279,6 +280,29 @@ export class DataStoreController { } } + @Patch('/:dataStoreId/rows') + @ProjectScope('dataStore:writeRow') + async updateDataStoreRow( + req: AuthenticatedRequest<{ projectId: string }>, + _res: Response, + @Param('dataStoreId') dataStoreId: string, + @Body dto: UpdateDataStoreRowDto, + ) { + try { + return await this.dataStoreService.updateRow(dataStoreId, req.params.projectId, dto); + } catch (e: unknown) { + if (e instanceof DataStoreNotFoundError) { + throw new NotFoundError(e.message); + } else if (e instanceof DataStoreValidationError) { + throw new BadRequestError(e.message); + } else if (e instanceof Error) { + throw new InternalServerError(e.message, e); + } else { + throw e; + } + } + } + @Delete('/:dataStoreId/rows') @ProjectScope('dataStore:writeRow') async deleteDataStoreRows( 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 f3ed12db18..7ae8d996b5 100644 --- a/packages/cli/src/modules/data-store/data-store.service.ts +++ b/packages/cli/src/modules/data-store/data-store.service.ts @@ -7,9 +7,11 @@ import type { DataStoreListOptions, UpsertDataStoreRowsDto, UpdateDataStoreDto, + UpdateDataStoreRowDto, } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { Service } from '@n8n/di'; +import type { DataStoreRow, DataStoreRows } from 'n8n-workflow'; import { DataStoreColumnRepository } from './data-store-column.repository'; import { DataStoreRowsRepository } from './data-store-rows.repository'; @@ -19,7 +21,6 @@ import { DataStoreNameConflictError } from './errors/data-store-name-conflict.er import { DataStoreNotFoundError } from './errors/data-store-not-found.error'; import { DataStoreValidationError } from './errors/data-store-validation.error'; import { toTableName, normalizeRows } from './utils/sql-utils'; -import { DataStoreRows } from 'n8n-workflow'; @Service() export class DataStoreService { @@ -139,18 +140,78 @@ export class DataStoreService { await this.validateDataStoreExists(dataStoreId, projectId); await this.validateRows(dataStoreId, dto.rows); + if (dto.rows.length === 0) { + throw new DataStoreValidationError('No rows provided for upsertRows'); + } + const columns = await this.dataStoreColumnRepository.getColumns(dataStoreId); return await this.dataStoreRowsRepository.upsertRows(toTableName(dataStoreId), dto, columns); } + async updateRow(dataStoreId: string, projectId: string, dto: UpdateDataStoreRowDto) { + await this.validateDataStoreExists(dataStoreId, projectId); + + const columns = await this.dataStoreColumnRepository.getColumns(dataStoreId); + if (columns.length === 0) { + throw new DataStoreValidationError( + 'No columns found for this data store or data store not found', + ); + } + + const { data, filter } = dto; + if (!filter || Object.keys(filter).length === 0) { + throw new DataStoreValidationError('Filter columns must not be empty for updateRow'); + } + if (!data || Object.keys(data).length === 0) { + throw new DataStoreValidationError('Data columns must not be empty for updateRow'); + } + + this.validateRowsWithColumns([filter], columns, true, true); + this.validateRowsWithColumns([data], columns, true, false); + + await this.dataStoreRowsRepository.updateRow(toTableName(dataStoreId), data, filter, columns); + return true; + } + async deleteRows(dataStoreId: string, projectId: string, ids: number[]) { await this.validateDataStoreExists(dataStoreId, projectId); return await this.dataStoreRowsRepository.deleteRows(toTableName(dataStoreId), ids); } - private async validateRows(dataStoreId: string, rows: DataStoreRows): Promise { + private validateRowsWithColumns( + rows: DataStoreRows, + columns: Array<{ name: string; type: string }>, + allowPartial = false, + includeSystemColumns = false, + ): void { + // Include system columns like 'id' if requested + const allColumns = includeSystemColumns + ? [{ name: 'id', type: 'number' }, ...columns] + : columns; + const columnNames = new Set(allColumns.map((x) => x.name)); + const columnTypeMap = new Map(allColumns.map((x) => [x.name, x.type])); + for (const row of rows) { + const keys = Object.keys(row); + if (!allowPartial && columnNames.size !== keys.length) { + throw new DataStoreValidationError('mismatched key count'); + } + for (const key of keys) { + if (!columnNames.has(key)) { + throw new DataStoreValidationError('unknown column name'); + } + this.validateCell(row, key, columnTypeMap); + } + } + } + + private async validateRows( + dataStoreId: string, + rows: DataStoreRows, + allowPartial = false, + includeSystemColumns = false, + ): Promise { const columns = await this.dataStoreColumnRepository.getColumns(dataStoreId); if (columns.length === 0) { throw new DataStoreValidationError( @@ -158,56 +219,49 @@ export class DataStoreService { ); } - const columnNames = new Set(columns.map((x) => x.name)); - const columnTypeMap = new Map(columns.map((x) => [x.name, x.type])); - for (const row of rows) { - const keys = Object.keys(row); - if (columns.length !== keys.length) { - throw new DataStoreValidationError('mismatched key count'); - } - for (const key of keys) { - if (!columnNames.has(key)) { - throw new DataStoreValidationError('unknown column name'); - } - const cell = row[key]; - if (cell === null) continue; - switch (columnTypeMap.get(key)) { - case 'boolean': - if (typeof cell !== 'boolean') { - throw new DataStoreValidationError( - `value '${cell.toString()}' does not match column type 'boolean'`, - ); - } - break; - case 'date': - if (typeof cell === 'string') { - const validated = dateTimeSchema.safeParse(cell); - if (validated.success) { - row[key] = validated.data.toISOString(); - break; - } - } else if (cell instanceof Date) { - row[key] = cell.toISOString(); - break; - } + this.validateRowsWithColumns(rows, columns, allowPartial, includeSystemColumns); + } - throw new DataStoreValidationError(`value '${cell}' does not match column type 'date'`); - case 'string': - if (typeof cell !== 'string') { - throw new DataStoreValidationError( - `value '${cell.toString()}' does not match column type 'string'`, - ); - } - break; - case 'number': - if (typeof cell !== 'number') { - throw new DataStoreValidationError( - `value '${cell.toString()}' does not match column type 'number'`, - ); - } - break; + private validateCell(row: DataStoreRow, key: string, columnTypeMap: Map) { + const cell = row[key]; + if (cell === null) return; + + const columnType = columnTypeMap.get(key); + switch (columnType) { + case 'boolean': + if (typeof cell !== 'boolean') { + throw new DataStoreValidationError( + `value '${String(cell)}' does not match column type 'boolean'`, + ); } - } + break; + case 'date': + if (typeof cell === 'string') { + const validated = dateTimeSchema.safeParse(cell); + if (validated.success) { + row[key] = validated.data.toISOString(); + break; + } + } else if (cell instanceof Date) { + row[key] = cell.toISOString(); + break; + } + + throw new DataStoreValidationError(`value '${cell}' does not match column type 'date'`); + case 'string': + if (typeof cell !== 'string') { + throw new DataStoreValidationError( + `value '${String(cell)}' does not match column type 'string'`, + ); + } + break; + case 'number': + if (typeof cell !== 'number') { + throw new DataStoreValidationError( + `value '${String(cell)}' does not match column type 'number'`, + ); + } + break; } } 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 5c111650b4..f5599669dd 100644 --- a/packages/cli/src/modules/data-store/utils/sql-utils.ts +++ b/packages/cli/src/modules/data-store/utils/sql-utils.ts @@ -129,48 +129,6 @@ export function buildInsertQuery( return [query, parameters]; } -export function buildUpdateQuery( - tableName: DataStoreUserTableName, - row: Record, - columns: Array<{ name: string; type: string }>, - matchFields: string[], - dbType: DataSourceOptions['type'] = 'sqlite', -): [string, unknown[]] { - if (Object.keys(row).length === 0 || matchFields.length === 0) { - return ['', []]; - } - - const updateKeys = Object.keys(row).filter((key) => !matchFields.includes(key)); - if (updateKeys.length === 0) { - return ['', []]; - } - - const quotedTableName = quoteIdentifier(tableName, dbType); - const columnTypeMap = buildColumnTypeMap(columns); - - const parameters: unknown[] = []; - let placeholderIndex = 1; - - const setClause = updateKeys - .map((key) => { - const value = normalizeValue(row[key], columnTypeMap[key], dbType); - parameters.push(value); - return `${quoteIdentifier(key, dbType)} = ${getPlaceholder(placeholderIndex++, dbType)}`; - }) - .join(', '); - - const whereClause = matchFields - .map((key) => { - const value = normalizeValue(row[key], columnTypeMap[key], dbType); - parameters.push(value); - return `${quoteIdentifier(key, dbType)} = ${getPlaceholder(placeholderIndex++, dbType)}`; - }) - .join(' AND '); - - const query = `UPDATE ${quotedTableName} SET ${setClause} WHERE ${whereClause}`; - return [query, parameters]; -} - export function splitRowsByExistence( existing: Array>, matchFields: string[], @@ -251,7 +209,7 @@ export function normalizeRows(rows: DataStoreRows, columns: DataStoreColumn[]) { }); } -function normalizeValue( +export function normalizeValue( value: unknown, columnType: string | undefined, dbType: DataSourceOptions['type'], @@ -282,7 +240,7 @@ export function getPlaceholder(index: number, dbType: DataSourceOptions['type']) return dbType.includes('postgres') ? `$${index}` : '?'; } -function buildColumnTypeMap( +export function buildColumnTypeMap( columns: Array<{ name: string; type: string }>, ): Record { return Object.fromEntries(columns.map((col) => [col.name, col.type])); diff --git a/packages/workflow/src/data-store.types.ts b/packages/workflow/src/data-store.types.ts index 3a19883b0e..6de3f285b5 100644 --- a/packages/workflow/src/data-store.types.ts +++ b/packages/workflow/src/data-store.types.ts @@ -72,7 +72,9 @@ export type AddDataStoreColumnOptions = Pick & export type DataStoreColumnJsType = string | number | boolean | Date; -export type DataStoreRows = Array>; +export type DataStoreRow = Record; + +export type DataStoreRows = DataStoreRow[]; // APIs for a data store service operating on a specific projectId export interface IDataStoreProjectAggregateService {