From 1e4d08b87aa0bf5fda792506fd837b4584038dfb Mon Sep 17 00:00:00 2001 From: ArtemHoruzhenko Date: Tue, 1 Sep 2026 10:36:59 +0300 Subject: [PATCH 1/4] feat(api): add pipeline draft CRUD endpoints for RDI instances Adds a pipeline_draft table and list/get/create/update/delete endpoints under rdi/:id/pipeline-drafts, so RDI pipeline work-in- progress can be persisted per instance. Drafts are orphan-removed via a DB-level ON DELETE CASCADE FK to the rdi table (mirroring the query-library module's FK pattern), and data is encrypted at rest via the existing ModelEncryptor. The data column is stored as a JSON string (required since encryption only operates on strings) but exposed as a real object at the API model layer via the existing DataAsJsonString decorator, the same mechanism already used by AiQueryMessageEntity.steps. --- redisinsight/api/config/ormconfig.ts | 2 + .../migration/1784100000000-pipeline-draft.ts | 27 ++ redisinsight/api/migration/index.ts | 2 + .../rdi/__tests__/pipeline-draft.factory.ts | 33 +++ .../rdi/dto/create.pipeline-draft.dto.ts | 9 + redisinsight/api/src/modules/rdi/dto/index.ts | 2 + .../rdi/dto/update.pipeline-draft.dto.ts | 6 + .../rdi/entities/pipeline-draft.entity.ts | 43 +++ .../api/src/modules/rdi/models/index.ts | 1 + .../src/modules/rdi/models/pipeline-draft.ts | 42 +++ .../rdi/pipeline-draft.controller.spec.ts | 147 ++++++++++ .../modules/rdi/pipeline-draft.controller.ts | 111 +++++++ .../rdi/pipeline-draft.service.spec.ts | 175 ++++++++++++ .../src/modules/rdi/pipeline-draft.service.ts | 60 ++++ .../api/src/modules/rdi/rdi.module.ts | 10 + .../local.pipeline-draft.repository.spec.ts | 270 ++++++++++++++++++ .../local.pipeline-draft.repository.ts | 165 +++++++++++ .../repository/pipeline-draft.repository.ts | 34 +++ 18 files changed, 1139 insertions(+) create mode 100644 redisinsight/api/migration/1784100000000-pipeline-draft.ts create mode 100644 redisinsight/api/src/modules/rdi/__tests__/pipeline-draft.factory.ts create mode 100644 redisinsight/api/src/modules/rdi/dto/create.pipeline-draft.dto.ts create mode 100644 redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts create mode 100644 redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts create mode 100644 redisinsight/api/src/modules/rdi/models/pipeline-draft.ts create mode 100644 redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts create mode 100644 redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts create mode 100644 redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts create mode 100644 redisinsight/api/src/modules/rdi/pipeline-draft.service.ts create mode 100644 redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts create mode 100644 redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts create mode 100644 redisinsight/api/src/modules/rdi/repository/pipeline-draft.repository.ts diff --git a/redisinsight/api/config/ormconfig.ts b/redisinsight/api/config/ormconfig.ts index 140074b2c8..4e2f72dd0c 100644 --- a/redisinsight/api/config/ormconfig.ts +++ b/redisinsight/api/config/ormconfig.ts @@ -19,6 +19,7 @@ import { FeaturesConfigEntity } from 'src/modules/feature/entities/features-conf import { CloudDatabaseDetailsEntity } from 'src/modules/cloud/database/entities/cloud-database-details.entity'; import { CloudCapiKeyEntity } from 'src/modules/cloud/capi-key/entity/cloud-capi-key.entity'; import { RdiEntity } from 'src/modules/rdi/entities/rdi.entity'; +import { PipelineDraftEntity } from 'src/modules/rdi/entities/pipeline-draft.entity'; import { AiQueryMessageEntity } from 'src/modules/ai/query/entities/ai-query.message.entity'; import { CloudSessionEntity } from 'src/modules/cloud/session/entities/cloud.session.entity'; import { DatabaseSettingsEntity } from 'src/modules/database-settings/entities/database-setting.entity'; @@ -54,6 +55,7 @@ const ormConfig = { CloudDatabaseDetailsEntity, CloudCapiKeyEntity, RdiEntity, + PipelineDraftEntity, AiQueryMessageEntity, CloudSessionEntity, DatabaseSettingsEntity, diff --git a/redisinsight/api/migration/1784100000000-pipeline-draft.ts b/redisinsight/api/migration/1784100000000-pipeline-draft.ts new file mode 100644 index 0000000000..43caab5463 --- /dev/null +++ b/redisinsight/api/migration/1784100000000-pipeline-draft.ts @@ -0,0 +1,27 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class PipelineDraft1784100000000 implements MigrationInterface { + name = 'PipelineDraft1784100000000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `CREATE TABLE "pipeline_draft" ( + "id" varchar PRIMARY KEY NOT NULL, + "rdiInstanceId" varchar NOT NULL, + "data" text NOT NULL, + "encryption" varchar, + "createdAt" datetime NOT NULL DEFAULT (datetime('now')), + "updatedAt" datetime NOT NULL DEFAULT (datetime('now')), + CONSTRAINT "FK_pipeline_draft_rdiInstanceId" FOREIGN KEY ("rdiInstanceId") REFERENCES "rdi" ("id") ON DELETE CASCADE ON UPDATE NO ACTION + )`, + ); + await queryRunner.query( + `CREATE INDEX "IDX_pipeline_draft_rdiInstanceId" ON "pipeline_draft" ("rdiInstanceId")`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`DROP INDEX "IDX_pipeline_draft_rdiInstanceId"`); + await queryRunner.query(`DROP TABLE "pipeline_draft"`); + } +} diff --git a/redisinsight/api/migration/index.ts b/redisinsight/api/migration/index.ts index 15e8de3dba..448433b1da 100644 --- a/redisinsight/api/migration/index.ts +++ b/redisinsight/api/migration/index.ts @@ -60,6 +60,7 @@ import { DatabaseIsProduction1778758000000 } from './1778758000000-database-isPr import { Environment1779000000000 } from './1779000000000-database-environment'; import { DropDatabaseIsProduction1779000000001 } from './1779000000001-drop-database-isProduction'; import { DatabaseConnectionFamily1784000000000 } from './1784000000000-database-connection-family'; +import { PipelineDraft1784100000000 } from './1784100000000-pipeline-draft'; import { AzureVerifyServerCert1785100000000 } from './1785100000000-azure-verify-server-cert'; export default [ @@ -125,5 +126,6 @@ export default [ Environment1779000000000, DropDatabaseIsProduction1779000000001, DatabaseConnectionFamily1784000000000, + PipelineDraft1784100000000, AzureVerifyServerCert1785100000000, ]; diff --git a/redisinsight/api/src/modules/rdi/__tests__/pipeline-draft.factory.ts b/redisinsight/api/src/modules/rdi/__tests__/pipeline-draft.factory.ts new file mode 100644 index 0000000000..0e090e7583 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/__tests__/pipeline-draft.factory.ts @@ -0,0 +1,33 @@ +import { Factory } from 'fishery'; +import { faker } from '@faker-js/faker'; +import { PipelineDraft } from '../models/pipeline-draft'; +import { PipelineDraftEntity } from '../entities/pipeline-draft.entity'; +import { CreatePipelineDraftDto } from '../dto/create.pipeline-draft.dto'; + +export const pipelineDraftFactory = Factory.define(() => ({ + id: faker.string.uuid(), + rdiInstanceId: faker.string.uuid(), + data: { [faker.word.noun()]: faker.word.words(3) }, + createdAt: faker.date.recent(), + updatedAt: faker.date.recent(), +})); + +// Entity-level `data` is the persisted string form (what DataAsJsonString +// produces before encryption), not the parsed object the API model exposes. +export const pipelineDraftEntityFactory = Factory.define( + () => { + const draft = pipelineDraftFactory.build(); + + return { + ...draft, + data: JSON.stringify(draft.data), + }; + }, +); + +export const createPipelineDraftDtoFactory = + Factory.define(() => { + const { data } = pipelineDraftFactory.build(); + + return { data }; + }); diff --git a/redisinsight/api/src/modules/rdi/dto/create.pipeline-draft.dto.ts b/redisinsight/api/src/modules/rdi/dto/create.pipeline-draft.dto.ts new file mode 100644 index 0000000000..876679deba --- /dev/null +++ b/redisinsight/api/src/modules/rdi/dto/create.pipeline-draft.dto.ts @@ -0,0 +1,9 @@ +import { OmitType } from '@nestjs/swagger'; +import { PipelineDraft } from 'src/modules/rdi/models'; + +export class CreatePipelineDraftDto extends OmitType(PipelineDraft, [ + 'id', + 'rdiInstanceId', + 'createdAt', + 'updatedAt', +] as const) {} diff --git a/redisinsight/api/src/modules/rdi/dto/index.ts b/redisinsight/api/src/modules/rdi/dto/index.ts index 9ccc103249..8840ce27e5 100644 --- a/redisinsight/api/src/modules/rdi/dto/index.ts +++ b/redisinsight/api/src/modules/rdi/dto/index.ts @@ -1,5 +1,7 @@ export * from './create.rdi.dto'; export * from './update.rdi.dto'; +export * from './create.pipeline-draft.dto'; +export * from './update.pipeline-draft.dto'; export * from './rdi.dry-run.job.dto'; export * from './rdi.dry-run.job.response.dto'; export * from './rdi-test-connections.response.dto'; diff --git a/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts b/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts new file mode 100644 index 0000000000..a65fd14b17 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts @@ -0,0 +1,6 @@ +import { PartialType } from '@nestjs/swagger'; +import { CreatePipelineDraftDto } from 'src/modules/rdi/dto/create.pipeline-draft.dto'; + +export class UpdatePipelineDraftDto extends PartialType( + CreatePipelineDraftDto, +) {} diff --git a/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts b/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts new file mode 100644 index 0000000000..2ec4b099bd --- /dev/null +++ b/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts @@ -0,0 +1,43 @@ +import { + Column, + CreateDateColumn, + Entity, + JoinColumn, + ManyToOne, + PrimaryGeneratedColumn, + UpdateDateColumn, +} from 'typeorm'; +import { Expose } from 'class-transformer'; +import { DataAsJsonString } from 'src/common/decorators'; +import { RdiEntity } from 'src/modules/rdi/entities/rdi.entity'; + +@Entity('pipeline_draft') +export class PipelineDraftEntity { + @Expose() + @PrimaryGeneratedColumn('uuid') + id: string; + + @Expose() + @Column({ nullable: false }) + rdiInstanceId: string; + + @ManyToOne(() => RdiEntity, { nullable: false, onDelete: 'CASCADE' }) + @JoinColumn({ name: 'rdiInstanceId' }) + rdiInstance?: RdiEntity; + + @Column({ nullable: false, type: 'text' }) + @DataAsJsonString() + @Expose() + data: string; + + @Column({ nullable: true }) + encryption?: string; + + @Expose() + @CreateDateColumn() + createdAt: Date; + + @Expose() + @UpdateDateColumn() + updatedAt: Date; +} diff --git a/redisinsight/api/src/modules/rdi/models/index.ts b/redisinsight/api/src/modules/rdi/models/index.ts index aae76c1d2d..0dfd46698c 100644 --- a/redisinsight/api/src/modules/rdi/models/index.ts +++ b/redisinsight/api/src/modules/rdi/models/index.ts @@ -1,5 +1,6 @@ export * from './rdi.client.metadata'; export * from './rdi'; +export * from './pipeline-draft'; export * from './rdi-pipeline'; export * from './rdi-dry-run'; export * from './rdi-statistics'; diff --git a/redisinsight/api/src/modules/rdi/models/pipeline-draft.ts b/redisinsight/api/src/modules/rdi/models/pipeline-draft.ts new file mode 100644 index 0000000000..1bdef758df --- /dev/null +++ b/redisinsight/api/src/modules/rdi/models/pipeline-draft.ts @@ -0,0 +1,42 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { Expose } from 'class-transformer'; +import { IsNotEmpty, IsObject } from 'class-validator'; + +export class PipelineDraft { + @ApiProperty({ + description: 'Pipeline draft id.', + type: String, + }) + @Expose() + id: string; + + @ApiProperty({ + description: 'Id of the RDI instance this draft belongs to.', + type: String, + }) + @Expose() + rdiInstanceId: string; + + @ApiProperty({ + description: 'Draft data as a JSON object. Structure is not validated.', + type: Object, + }) + @Expose() + @IsNotEmpty() + @IsObject() + data: object; + + @ApiProperty({ + description: 'Time the draft was created.', + type: Date, + }) + @Expose() + createdAt: Date; + + @ApiProperty({ + description: 'Time the draft was last updated.', + type: Date, + }) + @Expose() + updatedAt: Date; +} diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts new file mode 100644 index 0000000000..ff9136ae38 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts @@ -0,0 +1,147 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { faker } from '@faker-js/faker'; +import { NotFoundException } from '@nestjs/common'; +import { mockSessionMetadata } from 'src/__mocks__'; +import { PipelineDraftController } from './pipeline-draft.controller'; +import { PipelineDraftService } from './pipeline-draft.service'; +import { pipelineDraftFactory } from './__tests__/pipeline-draft.factory'; + +const mockRdiInstanceId = faker.string.uuid(); + +const mockRdiClientMetadata = { + sessionMetadata: mockSessionMetadata, + id: mockRdiInstanceId, +}; + +const mockPipelineDraftService = () => ({ + create: jest.fn(), + list: jest.fn(), + get: jest.fn(), + update: jest.fn(), + delete: jest.fn(), +}); + +describe('PipelineDraftController', () => { + let controller: PipelineDraftController; + let service: ReturnType; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [PipelineDraftController], + providers: [ + { + provide: PipelineDraftService, + useFactory: mockPipelineDraftService, + }, + ], + }).compile(); + + controller = module.get(PipelineDraftController); + service = module.get(PipelineDraftService); + }); + + describe('create', () => { + it('should create a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + service.create.mockResolvedValueOnce(draft); + + const result = await controller.create(mockRdiClientMetadata as any, { + data: draft.data, + }); + + expect(result).toEqual(draft); + expect(service.create).toHaveBeenCalledWith( + mockSessionMetadata, + mockRdiInstanceId, + { data: draft.data }, + ); + }); + }); + + describe('list', () => { + it('should return list of drafts', async () => { + const drafts = pipelineDraftFactory.buildList(3, { + rdiInstanceId: mockRdiInstanceId, + }); + service.list.mockResolvedValueOnce(drafts); + + const result = await controller.list(mockRdiClientMetadata as any); + + expect(result).toEqual(drafts); + expect(service.list).toHaveBeenCalledWith( + mockSessionMetadata, + mockRdiInstanceId, + ); + }); + }); + + describe('get', () => { + it('should return a single draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + service.get.mockResolvedValueOnce(draft); + + const result = await controller.get( + mockRdiClientMetadata as any, + draft.id, + ); + + expect(result).toEqual(draft); + }); + + it('should throw NotFoundException when draft not found', async () => { + service.get.mockRejectedValueOnce(new NotFoundException()); + + await expect( + controller.get(mockRdiClientMetadata as any, faker.string.uuid()), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('update', () => { + it('should update a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + const updatedData = { updated: true }; + const updatedDraft = { ...draft, data: updatedData }; + service.update.mockResolvedValueOnce(updatedDraft); + + const result = await controller.update( + mockRdiClientMetadata as any, + draft.id, + { data: updatedData }, + ); + + expect(result).toEqual(updatedDraft); + }); + }); + + describe('delete', () => { + it('should delete a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + service.delete.mockResolvedValueOnce(undefined); + + await controller.delete(mockRdiClientMetadata as any, draft.id); + + expect(service.delete).toHaveBeenCalledWith( + mockSessionMetadata, + mockRdiInstanceId, + draft.id, + ); + }); + + it('should throw NotFoundException when draft not found', async () => { + service.delete.mockRejectedValueOnce(new NotFoundException()); + + await expect( + controller.delete(mockRdiClientMetadata as any, faker.string.uuid()), + ).rejects.toThrow(NotFoundException); + }); + }); +}); diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts new file mode 100644 index 0000000000..d58d8c7fbf --- /dev/null +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts @@ -0,0 +1,111 @@ +import { + Body, + ClassSerializerInterceptor, + Controller, + Delete, + Get, + Param, + Patch, + Post, + UseInterceptors, + UsePipes, + ValidationPipe, +} from '@nestjs/common'; +import { ApiTags } from '@nestjs/swagger'; +import { ApiEndpoint } from 'src/decorators/api-endpoint.decorator'; +import { RequestRdiClientMetadata } from 'src/modules/rdi/decorators'; +import { PipelineDraft, RdiClientMetadata } from 'src/modules/rdi/models'; +import { PipelineDraftService } from 'src/modules/rdi/pipeline-draft.service'; +import { + CreatePipelineDraftDto, + UpdatePipelineDraftDto, +} from 'src/modules/rdi/dto'; + +@ApiTags('RDI') +@UsePipes(new ValidationPipe({ transform: true })) +@UseInterceptors(ClassSerializerInterceptor) +@Controller('rdi/:id/pipeline-drafts') +export class PipelineDraftController { + constructor(private readonly service: PipelineDraftService) {} + + @Post() + @ApiEndpoint({ + description: 'Create a pipeline draft', + statusCode: 201, + responses: [{ status: 201, type: PipelineDraft }], + }) + async create( + @RequestRdiClientMetadata() rdiClientMetadata: RdiClientMetadata, + @Body() dto: CreatePipelineDraftDto, + ): Promise { + return this.service.create( + rdiClientMetadata.sessionMetadata, + rdiClientMetadata.id, + dto, + ); + } + + @Get() + @ApiEndpoint({ + description: 'List pipeline drafts for an RDI instance', + responses: [{ status: 200, type: PipelineDraft, isArray: true }], + }) + async list( + @RequestRdiClientMetadata() rdiClientMetadata: RdiClientMetadata, + ): Promise { + return this.service.list( + rdiClientMetadata.sessionMetadata, + rdiClientMetadata.id, + ); + } + + @Get('/:draftId') + @ApiEndpoint({ + description: 'Get a pipeline draft by id', + responses: [{ status: 200, type: PipelineDraft }], + }) + async get( + @RequestRdiClientMetadata() rdiClientMetadata: RdiClientMetadata, + @Param('draftId') draftId: string, + ): Promise { + return this.service.get( + rdiClientMetadata.sessionMetadata, + rdiClientMetadata.id, + draftId, + ); + } + + @Patch('/:draftId') + @ApiEndpoint({ + description: 'Update a pipeline draft', + responses: [{ status: 200, type: PipelineDraft }], + }) + async update( + @RequestRdiClientMetadata() rdiClientMetadata: RdiClientMetadata, + @Param('draftId') draftId: string, + @Body() dto: UpdatePipelineDraftDto, + ): Promise { + return this.service.update( + rdiClientMetadata.sessionMetadata, + rdiClientMetadata.id, + draftId, + dto, + ); + } + + @Delete('/:draftId') + @ApiEndpoint({ + description: 'Delete a pipeline draft', + responses: [{ status: 200 }], + }) + async delete( + @RequestRdiClientMetadata() rdiClientMetadata: RdiClientMetadata, + @Param('draftId') draftId: string, + ): Promise { + return this.service.delete( + rdiClientMetadata.sessionMetadata, + rdiClientMetadata.id, + draftId, + ); + } +} diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts new file mode 100644 index 0000000000..137bd61d48 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts @@ -0,0 +1,175 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { faker } from '@faker-js/faker'; +import { NotFoundException } from '@nestjs/common'; +import { mockSessionMetadata } from 'src/__mocks__'; +import { PipelineDraftService } from './pipeline-draft.service'; +import { PipelineDraftRepository } from './repository/pipeline-draft.repository'; +import { + pipelineDraftFactory, + createPipelineDraftDtoFactory, +} from './__tests__/pipeline-draft.factory'; + +const mockRdiInstanceId = faker.string.uuid(); + +const mockPipelineDraftRepository = () => ({ + create: jest.fn(), + list: jest.fn(), + get: jest.fn(), + update: jest.fn(), + delete: jest.fn(), +}); + +describe('PipelineDraftService', () => { + let service: PipelineDraftService; + let repository: ReturnType; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + PipelineDraftService, + { + provide: PipelineDraftRepository, + useFactory: mockPipelineDraftRepository, + }, + ], + }).compile(); + + service = module.get(PipelineDraftService); + repository = module.get(PipelineDraftRepository); + }); + + describe('create', () => { + it('should create a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + const dto = createPipelineDraftDtoFactory.build({ data: draft.data }); + repository.create.mockResolvedValueOnce(draft); + + const result = await service.create( + mockSessionMetadata, + mockRdiInstanceId, + dto, + ); + + expect(result).toEqual(draft); + expect(repository.create).toHaveBeenCalledWith( + mockSessionMetadata, + mockRdiInstanceId, + dto, + ); + }); + }); + + describe('list', () => { + it('should return list of drafts', async () => { + const drafts = pipelineDraftFactory.buildList(3, { + rdiInstanceId: mockRdiInstanceId, + }); + repository.list.mockResolvedValueOnce(drafts); + + const result = await service.list(mockSessionMetadata, mockRdiInstanceId); + + expect(result).toEqual(drafts); + }); + + it('should return empty list', async () => { + repository.list.mockResolvedValueOnce([]); + + const result = await service.list(mockSessionMetadata, mockRdiInstanceId); + + expect(result).toEqual([]); + }); + }); + + describe('get', () => { + it('should return a single draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + repository.get.mockResolvedValueOnce(draft); + + const result = await service.get( + mockSessionMetadata, + mockRdiInstanceId, + draft.id, + ); + + expect(result).toEqual(draft); + }); + + it('should throw NotFoundException if draft not found', async () => { + repository.get.mockRejectedValueOnce(new NotFoundException()); + + await expect( + service.get( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + ), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('update', () => { + it('should update a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + const updatedData = { updated: true }; + const updatedDraft = { ...draft, data: updatedData }; + repository.update.mockResolvedValueOnce(updatedDraft); + + const result = await service.update( + mockSessionMetadata, + mockRdiInstanceId, + draft.id, + { data: updatedData }, + ); + + expect(result).toEqual(updatedDraft); + }); + + it('should throw NotFoundException if draft not found', async () => { + repository.update.mockRejectedValueOnce(new NotFoundException()); + + await expect( + service.update( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + { data: {} }, + ), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('delete', () => { + it('should delete a pipeline draft', async () => { + const draft = pipelineDraftFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + repository.delete.mockResolvedValueOnce(undefined); + + await service.delete(mockSessionMetadata, mockRdiInstanceId, draft.id); + + expect(repository.delete).toHaveBeenCalledWith( + mockSessionMetadata, + mockRdiInstanceId, + draft.id, + ); + }); + + it('should throw NotFoundException if draft not found', async () => { + repository.delete.mockRejectedValueOnce(new NotFoundException()); + + await expect( + service.delete( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + ), + ).rejects.toThrow(NotFoundException); + }); + }); +}); diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts new file mode 100644 index 0000000000..f381ea36f0 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts @@ -0,0 +1,60 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { SessionMetadata } from 'src/common/models'; +import { PipelineDraft } from 'src/modules/rdi/models'; +import { PipelineDraftRepository } from 'src/modules/rdi/repository/pipeline-draft.repository'; +import { + CreatePipelineDraftDto, + UpdatePipelineDraftDto, +} from 'src/modules/rdi/dto'; + +@Injectable() +export class PipelineDraftService { + private logger = new Logger('PipelineDraftService'); + + constructor(private readonly repository: PipelineDraftRepository) {} + + async create( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + dto: CreatePipelineDraftDto, + ): Promise { + this.logger.debug('Creating pipeline draft', sessionMetadata); + return this.repository.create(sessionMetadata, rdiInstanceId, dto); + } + + async list( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + ): Promise { + this.logger.debug('Listing pipeline drafts', sessionMetadata); + return this.repository.list(sessionMetadata, rdiInstanceId); + } + + async get( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise { + this.logger.debug(`Getting pipeline draft ${id}`, sessionMetadata); + return this.repository.get(sessionMetadata, rdiInstanceId, id); + } + + async update( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + dto: UpdatePipelineDraftDto, + ): Promise { + this.logger.debug(`Updating pipeline draft ${id}`, sessionMetadata); + return this.repository.update(sessionMetadata, rdiInstanceId, id, dto); + } + + async delete( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise { + this.logger.debug(`Deleting pipeline draft ${id}`, sessionMetadata); + return this.repository.delete(sessionMetadata, rdiInstanceId, id); + } +} diff --git a/redisinsight/api/src/modules/rdi/rdi.module.ts b/redisinsight/api/src/modules/rdi/rdi.module.ts index 33ca239d1e..092c8b1f56 100644 --- a/redisinsight/api/src/modules/rdi/rdi.module.ts +++ b/redisinsight/api/src/modules/rdi/rdi.module.ts @@ -12,6 +12,10 @@ import { RdiAnalytics } from 'src/modules/rdi/rdi.analytics'; import { RdiPipelineAnalytics } from 'src/modules/rdi/rdi-pipeline.analytics'; import { RdiStatisticsController } from 'src/modules/rdi/rdi-statistics.controller'; import { RdiStatisticsService } from 'src/modules/rdi/rdi-statistics.service'; +import { PipelineDraftController } from 'src/modules/rdi/pipeline-draft.controller'; +import { PipelineDraftService } from 'src/modules/rdi/pipeline-draft.service'; +import { PipelineDraftRepository } from 'src/modules/rdi/repository/pipeline-draft.repository'; +import { LocalPipelineDraftRepository } from 'src/modules/rdi/repository/local.pipeline-draft.repository'; @Module({}) export class RdiModule { @@ -22,11 +26,13 @@ export class RdiModule { RdiController, RdiPipelineController, RdiStatisticsController, + PipelineDraftController, ], providers: [ RdiService, RdiPipelineService, RdiStatisticsService, + PipelineDraftService, RdiClientProvider, RdiClientStorage, RdiClientFactory, @@ -36,6 +42,10 @@ export class RdiModule { provide: RdiRepository, useClass: rdiRepository, }, + { + provide: PipelineDraftRepository, + useClass: LocalPipelineDraftRepository, + }, ], }; } diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts new file mode 100644 index 0000000000..719a80161c --- /dev/null +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts @@ -0,0 +1,270 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { faker } from '@faker-js/faker'; +import { instanceToPlain, plainToInstance } from 'class-transformer'; +import { NotFoundException } from '@nestjs/common'; +import { mockSessionMetadata } from 'src/__mocks__'; +import { EncryptionService } from 'src/modules/encryption/encryption.service'; +import { PipelineDraftEntity } from '../entities/pipeline-draft.entity'; +import { LocalPipelineDraftRepository } from './local.pipeline-draft.repository'; +import { + pipelineDraftEntityFactory, + createPipelineDraftDtoFactory, +} from '../__tests__/pipeline-draft.factory'; + +const mockRdiInstanceId = faker.string.uuid(); + +const mockEncryptResult = { + data: 'encrypted_data', + encryption: 'KEYTAR', +}; + +const mockEncryptionServiceFactory = jest.fn(() => ({ + getAvailableEncryptionStrategies: jest.fn(), + isEncryptionAvailable: jest.fn().mockResolvedValue(true), + encrypt: jest.fn().mockResolvedValue(mockEncryptResult), + decrypt: jest.fn().mockImplementation((data) => data), + getEncryptionStrategy: jest.fn(), +})); + +const mockRepository = () => ({ + save: jest.fn().mockImplementation((entity) => ({ + ...entity, + id: entity.id || faker.string.uuid(), + })), + find: jest.fn(), + findOneBy: jest.fn(), + delete: jest.fn().mockResolvedValue({ affected: 1 }), +}); + +describe('LocalPipelineDraftRepository', () => { + let repository: LocalPipelineDraftRepository; + let typeormRepo: ReturnType; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LocalPipelineDraftRepository, + { + provide: getRepositoryToken(PipelineDraftEntity), + useFactory: mockRepository, + }, + { + provide: EncryptionService, + useFactory: mockEncryptionServiceFactory, + }, + ], + }).compile(); + + repository = module.get(LocalPipelineDraftRepository); + typeormRepo = module.get(getRepositoryToken(PipelineDraftEntity)); + }); + + describe('create', () => { + it('should create and return a pipeline draft', async () => { + const dto = createPipelineDraftDtoFactory.build(); + + const result = await repository.create( + mockSessionMetadata, + mockRdiInstanceId, + dto, + ); + + expect(result).toBeDefined(); + expect(typeormRepo.save).toHaveBeenCalled(); + }); + + it('should store the entity data column as a JSON string, not the raw object', () => { + // DataAsJsonString() is what lets the entity persist `data` as text + // while every layer above the entity (API model, DTOs, controllers) + // works with a real object - verified directly here, independent of + // encryption, since the encryption mock above doesn't round-trip data. + const original = { foo: 'bar', nested: { a: 1 } }; + + const entity = plainToInstance(PipelineDraftEntity, { data: original }); + expect(typeof entity.data).toBe('string'); + expect(entity.data).toBe(JSON.stringify(original)); + + const plain = instanceToPlain(entity); + expect(plain.data).toEqual(original); + }); + }); + + describe('list', () => { + it('should return list of drafts for the rdi instance', async () => { + const entities = pipelineDraftEntityFactory.buildList(2, { + rdiInstanceId: mockRdiInstanceId, + }); + typeormRepo.find.mockResolvedValueOnce(entities); + + const result = await repository.list( + mockSessionMetadata, + mockRdiInstanceId, + ); + + expect(result).toHaveLength(2); + expect(typeormRepo.find).toHaveBeenCalledWith({ + where: { rdiInstanceId: mockRdiInstanceId }, + order: { createdAt: 'ASC' }, + }); + }); + + it('should return empty list', async () => { + typeormRepo.find.mockResolvedValueOnce([]); + + const result = await repository.list( + mockSessionMetadata, + mockRdiInstanceId, + ); + + expect(result).toEqual([]); + }); + + it('should exclude entities that fail to decrypt', async () => { + const decryptionError = new Error('decryption failed'); + mockEncryptionServiceFactory.mockReturnValueOnce({ + getAvailableEncryptionStrategies: jest.fn(), + isEncryptionAvailable: jest.fn().mockResolvedValue(true), + encrypt: jest.fn().mockResolvedValue(mockEncryptResult), + decrypt: jest.fn().mockRejectedValue(decryptionError), + getEncryptionStrategy: jest.fn(), + }); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LocalPipelineDraftRepository, + { + provide: getRepositoryToken(PipelineDraftEntity), + useFactory: mockRepository, + }, + { + provide: EncryptionService, + useFactory: mockEncryptionServiceFactory, + }, + ], + }).compile(); + + const failingRepository = module.get(LocalPipelineDraftRepository); + const failingTypeormRepo = module.get( + getRepositoryToken(PipelineDraftEntity), + ); + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + encryption: 'KEYTAR', + }); + failingTypeormRepo.find.mockResolvedValueOnce([entity]); + + const result = await failingRepository.list( + mockSessionMetadata, + mockRdiInstanceId, + ); + + expect(result).toEqual([]); + }); + }); + + describe('get', () => { + it('should return a single draft', async () => { + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + typeormRepo.findOneBy.mockResolvedValueOnce(entity); + + const result = await repository.get( + mockSessionMetadata, + mockRdiInstanceId, + entity.id, + ); + + expect(result).toBeDefined(); + expect(typeormRepo.findOneBy).toHaveBeenCalledWith({ + id: entity.id, + rdiInstanceId: mockRdiInstanceId, + }); + }); + + it('should throw NotFoundException when draft not found', async () => { + typeormRepo.findOneBy.mockResolvedValueOnce(null); + + await expect( + repository.get( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + ), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('update', () => { + it('should update an existing draft', async () => { + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + typeormRepo.findOneBy.mockResolvedValueOnce(entity); + + const result = await repository.update( + mockSessionMetadata, + mockRdiInstanceId, + entity.id, + { data: { updated: true } }, + ); + + expect(result).toBeDefined(); + expect(typeormRepo.findOneBy).toHaveBeenCalledWith({ + id: entity.id, + rdiInstanceId: mockRdiInstanceId, + }); + expect(typeormRepo.save).toHaveBeenCalled(); + }); + + it('should throw NotFoundException when draft not found', async () => { + typeormRepo.findOneBy.mockResolvedValueOnce(null); + + await expect( + repository.update( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + { data: {} }, + ), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('delete', () => { + it('should delete a draft', async () => { + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + typeormRepo.findOneBy.mockResolvedValueOnce(entity); + + await repository.delete( + mockSessionMetadata, + mockRdiInstanceId, + entity.id, + ); + + expect(typeormRepo.findOneBy).toHaveBeenCalledWith({ + id: entity.id, + rdiInstanceId: mockRdiInstanceId, + }); + expect(typeormRepo.delete).toHaveBeenCalledWith({ + id: entity.id, + rdiInstanceId: mockRdiInstanceId, + }); + }); + + it('should throw NotFoundException when draft not found', async () => { + typeormRepo.findOneBy.mockResolvedValueOnce(null); + + await expect( + repository.delete( + mockSessionMetadata, + mockRdiInstanceId, + faker.string.uuid(), + ), + ).rejects.toThrow(NotFoundException); + }); + }); +}); diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts new file mode 100644 index 0000000000..618fd8facd --- /dev/null +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts @@ -0,0 +1,165 @@ +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; +import { filter, isNull, isUndefined, omitBy } from 'lodash'; +import { plainToInstance } from 'class-transformer'; +import { EncryptionService } from 'src/modules/encryption/encryption.service'; +import { ModelEncryptor } from 'src/modules/encryption/model.encryptor'; +import { classToClass } from 'src/utils'; +import { SessionMetadata } from 'src/common/models'; +import { PipelineDraftEntity } from 'src/modules/rdi/entities/pipeline-draft.entity'; +import { PipelineDraft } from 'src/modules/rdi/models'; +import { PipelineDraftRepository } from 'src/modules/rdi/repository/pipeline-draft.repository'; + +@Injectable() +export class LocalPipelineDraftRepository extends PipelineDraftRepository { + private logger = new Logger('LocalPipelineDraftRepository'); + + private readonly modelEncryptor: ModelEncryptor; + + constructor( + @InjectRepository(PipelineDraftEntity) + private readonly repository: Repository, + private readonly encryptionService: EncryptionService, + ) { + super(); + this.modelEncryptor = new ModelEncryptor(this.encryptionService, ['data']); + } + + async create( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + data: Partial, + ): Promise { + this.logger.debug('Creating pipeline draft', sessionMetadata); + + const entity = plainToInstance(PipelineDraftEntity, { + ...data, + rdiInstanceId, + }); + + const saved = await this.repository.save( + await this.modelEncryptor.encryptEntity(entity), + ); + + this.logger.debug('Pipeline draft created', sessionMetadata); + + return classToClass( + PipelineDraft, + await this.modelEncryptor.decryptEntity(saved, true), + ); + } + + async list( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + ): Promise { + this.logger.debug('Getting pipeline drafts', sessionMetadata); + + const entities = await this.repository.find({ + where: { rdiInstanceId }, + order: { createdAt: 'ASC' }, + }); + + const decryptedEntities = await Promise.all( + entities.map(async (entity): Promise => { + try { + return await this.modelEncryptor.decryptEntity(entity); + } catch (e) { + return null; + } + }), + ); + + return filter(decryptedEntities, (entity) => !isNull(entity)).map( + (entity) => classToClass(PipelineDraft, entity), + ); + } + + async get( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise { + this.logger.debug('Getting pipeline draft', sessionMetadata); + + const entity = await this.repository.findOneBy({ id, rdiInstanceId }); + + if (!entity) { + this.logger.error( + `Pipeline draft with id:${id} and rdiInstanceId:${rdiInstanceId} was not found`, + sessionMetadata, + ); + throw new NotFoundException(`Pipeline draft with id ${id} was not found`); + } + + this.logger.debug('Succeed to get pipeline draft', sessionMetadata); + + return classToClass( + PipelineDraft, + await this.modelEncryptor.decryptEntity(entity, true), + ); + } + + async update( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + data: Partial, + ): Promise { + this.logger.debug('Updating pipeline draft', sessionMetadata); + + const existing = await this.repository.findOneBy({ id, rdiInstanceId }); + + if (!existing) { + this.logger.error( + `Pipeline draft with id:${id} and rdiInstanceId:${rdiInstanceId} was not found`, + sessionMetadata, + ); + throw new NotFoundException(`Pipeline draft with id ${id} was not found`); + } + + const decrypted = await this.modelEncryptor.decryptEntity(existing, true); + const updateData = omitBy(data, isUndefined); + + const merged = plainToInstance(PipelineDraftEntity, { + ...decrypted, + ...updateData, + id, + rdiInstanceId, + }); + + const saved = await this.repository.save( + await this.modelEncryptor.encryptEntity(merged), + ); + + this.logger.debug('Pipeline draft updated', sessionMetadata); + + return classToClass( + PipelineDraft, + await this.modelEncryptor.decryptEntity(saved, true), + ); + } + + async delete( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise { + this.logger.debug('Deleting pipeline draft', sessionMetadata); + + const existing = await this.repository.findOneBy({ id, rdiInstanceId }); + + if (!existing) { + this.logger.error( + `Pipeline draft with id:${id} and rdiInstanceId:${rdiInstanceId} was not found`, + sessionMetadata, + ); + throw new NotFoundException(`Pipeline draft with id ${id} was not found`); + } + + await this.repository.delete({ id, rdiInstanceId }); + + this.logger.debug('Pipeline draft deleted', sessionMetadata); + } +} diff --git a/redisinsight/api/src/modules/rdi/repository/pipeline-draft.repository.ts b/redisinsight/api/src/modules/rdi/repository/pipeline-draft.repository.ts new file mode 100644 index 0000000000..59fd822755 --- /dev/null +++ b/redisinsight/api/src/modules/rdi/repository/pipeline-draft.repository.ts @@ -0,0 +1,34 @@ +import { SessionMetadata } from 'src/common/models'; +import { PipelineDraft } from 'src/modules/rdi/models'; + +export abstract class PipelineDraftRepository { + abstract create( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + data: Partial, + ): Promise; + + abstract list( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + ): Promise; + + abstract get( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise; + + abstract update( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + data: Partial, + ): Promise; + + abstract delete( + sessionMetadata: SessionMetadata, + rdiInstanceId: string, + id: string, + ): Promise; +} From e81e79aa6505a3ab885c1c3688263ecf5dc7e9b2 Mon Sep 17 00:00:00 2001 From: ArtemHoruzhenko Date: Tue, 8 Sep 2026 19:41:57 +0300 Subject: [PATCH 2/4] fix(rdi): address PR review findings on pipeline draft CRUD Enables ValidationPipe whitelisting on the pipeline-draft controller so an unexpected id field on a create/update body can no longer overwrite an unrelated draft via TypeORM's upsert-on-save behavior. Fixes update() double-encoding the persisted data column when a PATCH omits data: the already-JSON-stringified value was being routed back through @DataAsJsonString(), stringifying it a second time. --- .../rdi/pipeline-draft.controller.spec.ts | 16 ++++++++++++++- .../modules/rdi/pipeline-draft.controller.ts | 2 +- .../local.pipeline-draft.repository.spec.ts | 20 +++++++++++++++++++ .../local.pipeline-draft.repository.ts | 14 ++++++++++--- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts index ff9136ae38..458408868e 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts @@ -1,10 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { faker } from '@faker-js/faker'; -import { NotFoundException } from '@nestjs/common'; +import { NotFoundException, ValidationPipe } from '@nestjs/common'; import { mockSessionMetadata } from 'src/__mocks__'; import { PipelineDraftController } from './pipeline-draft.controller'; import { PipelineDraftService } from './pipeline-draft.service'; import { pipelineDraftFactory } from './__tests__/pipeline-draft.factory'; +import { CreatePipelineDraftDto } from './dto'; const mockRdiInstanceId = faker.string.uuid(); @@ -58,6 +59,19 @@ describe('PipelineDraftController', () => { { data: draft.data }, ); }); + + it('should strip fields not declared on the create dto, such as an injected id', async () => { + const pipe = new ValidationPipe({ transform: true, whitelist: true }); + const draft = pipelineDraftFactory.build(); + + const transformed = await pipe.transform( + { data: draft.data, id: faker.string.uuid() }, + { type: 'body', metatype: CreatePipelineDraftDto }, + ); + + expect(transformed).toEqual({ data: draft.data }); + expect(transformed.id).toBeUndefined(); + }); }); describe('list', () => { diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts index d58d8c7fbf..ba855d2dde 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.ts @@ -22,7 +22,7 @@ import { } from 'src/modules/rdi/dto'; @ApiTags('RDI') -@UsePipes(new ValidationPipe({ transform: true })) +@UsePipes(new ValidationPipe({ transform: true, whitelist: true })) @UseInterceptors(ClassSerializerInterceptor) @Controller('rdi/:id/pipeline-drafts') export class PipelineDraftController { diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts index 719a80161c..8987bf77ed 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts @@ -40,6 +40,7 @@ const mockRepository = () => ({ describe('LocalPipelineDraftRepository', () => { let repository: LocalPipelineDraftRepository; let typeormRepo: ReturnType; + let encryptionService: ReturnType; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -58,6 +59,7 @@ describe('LocalPipelineDraftRepository', () => { repository = module.get(LocalPipelineDraftRepository); typeormRepo = module.get(getRepositoryToken(PipelineDraftEntity)); + encryptionService = module.get(EncryptionService); }); describe('create', () => { @@ -230,6 +232,24 @@ describe('LocalPipelineDraftRepository', () => { ), ).rejects.toThrow(NotFoundException); }); + + it('should not re-encode the existing data when the patch omits it', async () => { + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + }); + typeormRepo.findOneBy.mockResolvedValueOnce(entity); + + await repository.update( + mockSessionMetadata, + mockRdiInstanceId, + entity.id, + {}, + ); + + // the value handed to encryption is the entity's single-encoded JSON + // string, unchanged - not JSON.stringify'd a second time on top of it + expect(encryptionService.encrypt).toHaveBeenCalledWith(entity.data); + }); }); describe('delete', () => { diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts index 618fd8facd..effdeb76ee 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts @@ -120,14 +120,22 @@ export class LocalPipelineDraftRepository extends PipelineDraftRepository { } const decrypted = await this.modelEncryptor.decryptEntity(existing, true); - const updateData = omitBy(data, isUndefined); + const { data: updatedData, ...restUpdateData } = omitBy(data, isUndefined); + const { data: decryptedData, ...restDecrypted } = decrypted; + // `decryptedData` is already the entity's JSON-string representation of + // `data`, so it must be assigned directly rather than passed through + // plainToInstance - going through @DataAsJsonString() again would + // JSON.stringify an already-stringified value const merged = plainToInstance(PipelineDraftEntity, { - ...decrypted, - ...updateData, + ...restDecrypted, + ...restUpdateData, id, rdiInstanceId, }); + merged.data = isUndefined(updatedData) + ? decryptedData + : JSON.stringify(updatedData); const saved = await this.repository.save( await this.modelEncryptor.encryptEntity(merged), From 94817cc9486cddaaaef6cf358f4d4afd3c6d3455 Mon Sep 17 00:00:00 2001 From: ArtemHoruzhenko Date: Tue, 8 Sep 2026 20:23:10 +0300 Subject: [PATCH 3/4] fix(rdi): address further PR review findings on pipeline draft CRUD Rejects null data on PATCH instead of silently treating it as omitted (PartialType's IsOptional skips validation for null the same as undefined). Validates the parent RDI instance exists before creating a draft, translating a missing/deleted instance into a 404 instead of an uncaught foreign-key constraint error. Declares the entity's index to match the migration so schema-diff tooling doesn't see it as extraneous. Stops tolerating decrypt failures in update(), since silently swallowing them would overwrite valid ciphertext with null. --- .../rdi/dto/update.pipeline-draft.dto.ts | 18 +++++-- .../rdi/entities/pipeline-draft.entity.ts | 2 + .../rdi/pipeline-draft.controller.spec.ts | 13 ++++- .../rdi/pipeline-draft.service.spec.ts | 21 +++++++++ .../src/modules/rdi/pipeline-draft.service.ts | 16 ++++++- .../local.pipeline-draft.repository.spec.ts | 47 ++++++++++++++++++- .../local.pipeline-draft.repository.ts | 10 ++-- 7 files changed, 112 insertions(+), 15 deletions(-) diff --git a/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts b/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts index a65fd14b17..37b0b64b29 100644 --- a/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts +++ b/redisinsight/api/src/modules/rdi/dto/update.pipeline-draft.dto.ts @@ -1,6 +1,14 @@ -import { PartialType } from '@nestjs/swagger'; -import { CreatePipelineDraftDto } from 'src/modules/rdi/dto/create.pipeline-draft.dto'; +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsNotEmpty, IsObject, ValidateIf } from 'class-validator'; -export class UpdatePipelineDraftDto extends PartialType( - CreatePipelineDraftDto, -) {} +export class UpdatePipelineDraftDto { + @ApiPropertyOptional({ + description: 'Draft data as a JSON object. Structure is not validated.', + type: Object, + }) + // skip validation only when omitted, not when explicitly null + @ValidateIf((dto) => dto.data !== undefined) + @IsNotEmpty() + @IsObject() + data?: object; +} diff --git a/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts b/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts index 2ec4b099bd..2e9c0d3580 100644 --- a/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts +++ b/redisinsight/api/src/modules/rdi/entities/pipeline-draft.entity.ts @@ -2,6 +2,7 @@ import { Column, CreateDateColumn, Entity, + Index, JoinColumn, ManyToOne, PrimaryGeneratedColumn, @@ -18,6 +19,7 @@ export class PipelineDraftEntity { id: string; @Expose() + @Index('IDX_pipeline_draft_rdiInstanceId') @Column({ nullable: false }) rdiInstanceId: string; diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts index 458408868e..ca6af35eb5 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.controller.spec.ts @@ -5,7 +5,7 @@ import { mockSessionMetadata } from 'src/__mocks__'; import { PipelineDraftController } from './pipeline-draft.controller'; import { PipelineDraftService } from './pipeline-draft.service'; import { pipelineDraftFactory } from './__tests__/pipeline-draft.factory'; -import { CreatePipelineDraftDto } from './dto'; +import { CreatePipelineDraftDto, UpdatePipelineDraftDto } from './dto'; const mockRdiInstanceId = faker.string.uuid(); @@ -132,6 +132,17 @@ describe('PipelineDraftController', () => { expect(result).toEqual(updatedDraft); }); + + it('should reject null data instead of treating it as an omitted field', async () => { + const pipe = new ValidationPipe({ transform: true, whitelist: true }); + + await expect( + pipe.transform( + { data: null }, + { type: 'body', metatype: UpdatePipelineDraftDto }, + ), + ).rejects.toThrow(); + }); }); describe('delete', () => { diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts index 137bd61d48..b7a19b6193 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts @@ -4,6 +4,7 @@ import { NotFoundException } from '@nestjs/common'; import { mockSessionMetadata } from 'src/__mocks__'; import { PipelineDraftService } from './pipeline-draft.service'; import { PipelineDraftRepository } from './repository/pipeline-draft.repository'; +import { RdiRepository } from './repository/rdi.repository'; import { pipelineDraftFactory, createPipelineDraftDtoFactory, @@ -19,9 +20,14 @@ const mockPipelineDraftRepository = () => ({ delete: jest.fn(), }); +const mockRdiRepository = () => ({ + get: jest.fn().mockResolvedValue({ id: mockRdiInstanceId }), +}); + describe('PipelineDraftService', () => { let service: PipelineDraftService; let repository: ReturnType; + let rdiRepository: ReturnType; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -31,11 +37,16 @@ describe('PipelineDraftService', () => { provide: PipelineDraftRepository, useFactory: mockPipelineDraftRepository, }, + { + provide: RdiRepository, + useFactory: mockRdiRepository, + }, ], }).compile(); service = module.get(PipelineDraftService); repository = module.get(PipelineDraftRepository); + rdiRepository = module.get(RdiRepository); }); describe('create', () => { @@ -59,6 +70,16 @@ describe('PipelineDraftService', () => { dto, ); }); + + it('should throw NotFoundException when the rdi instance does not exist', async () => { + rdiRepository.get.mockResolvedValueOnce(null); + const dto = createPipelineDraftDtoFactory.build(); + + await expect( + service.create(mockSessionMetadata, mockRdiInstanceId, dto), + ).rejects.toThrow(NotFoundException); + expect(repository.create).not.toHaveBeenCalled(); + }); }); describe('list', () => { diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts index f381ea36f0..abe6d0c3ac 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts @@ -1,7 +1,8 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; import { SessionMetadata } from 'src/common/models'; import { PipelineDraft } from 'src/modules/rdi/models'; import { PipelineDraftRepository } from 'src/modules/rdi/repository/pipeline-draft.repository'; +import { RdiRepository } from 'src/modules/rdi/repository/rdi.repository'; import { CreatePipelineDraftDto, UpdatePipelineDraftDto, @@ -11,7 +12,10 @@ import { export class PipelineDraftService { private logger = new Logger('PipelineDraftService'); - constructor(private readonly repository: PipelineDraftRepository) {} + constructor( + private readonly repository: PipelineDraftRepository, + private readonly rdiRepository: RdiRepository, + ) {} async create( sessionMetadata: SessionMetadata, @@ -19,6 +23,14 @@ export class PipelineDraftService { dto: CreatePipelineDraftDto, ): Promise { this.logger.debug('Creating pipeline draft', sessionMetadata); + + const rdi = await this.rdiRepository.get(rdiInstanceId); + if (!rdi) { + throw new NotFoundException( + `RDI instance with id ${rdiInstanceId} was not found`, + ); + } + return this.repository.create(sessionMetadata, rdiInstanceId, dto); } diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts index 8987bf77ed..753fbb2927 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts @@ -246,10 +246,53 @@ describe('LocalPipelineDraftRepository', () => { {}, ); - // the value handed to encryption is the entity's single-encoded JSON - // string, unchanged - not JSON.stringify'd a second time on top of it expect(encryptionService.encrypt).toHaveBeenCalledWith(entity.data); }); + + it('should not save when the existing draft fails to decrypt, instead of wiping it with null', async () => { + const decryptionError = new Error('decryption failed'); + mockEncryptionServiceFactory.mockReturnValueOnce({ + getAvailableEncryptionStrategies: jest.fn(), + isEncryptionAvailable: jest.fn().mockResolvedValue(true), + encrypt: jest.fn().mockResolvedValue(mockEncryptResult), + decrypt: jest.fn().mockRejectedValue(decryptionError), + getEncryptionStrategy: jest.fn(), + }); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LocalPipelineDraftRepository, + { + provide: getRepositoryToken(PipelineDraftEntity), + useFactory: mockRepository, + }, + { + provide: EncryptionService, + useFactory: mockEncryptionServiceFactory, + }, + ], + }).compile(); + + const failingRepository = module.get(LocalPipelineDraftRepository); + const failingTypeormRepo = module.get( + getRepositoryToken(PipelineDraftEntity), + ); + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + encryption: 'KEYTAR', + }); + failingTypeormRepo.findOneBy.mockResolvedValueOnce(entity); + + await expect( + failingRepository.update( + mockSessionMetadata, + mockRdiInstanceId, + entity.id, + {}, + ), + ).rejects.toThrow(decryptionError); + expect(failingTypeormRepo.save).not.toHaveBeenCalled(); + }); }); describe('delete', () => { diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts index effdeb76ee..5a4bec892d 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts @@ -119,14 +119,14 @@ export class LocalPipelineDraftRepository extends PipelineDraftRepository { throw new NotFoundException(`Pipeline draft with id ${id} was not found`); } - const decrypted = await this.modelEncryptor.decryptEntity(existing, true); + // must not ignore decryption errors here, or a failure would fall + // through to `decryptedData` below and overwrite valid ciphertext with null + const decrypted = await this.modelEncryptor.decryptEntity(existing); const { data: updatedData, ...restUpdateData } = omitBy(data, isUndefined); const { data: decryptedData, ...restDecrypted } = decrypted; - // `decryptedData` is already the entity's JSON-string representation of - // `data`, so it must be assigned directly rather than passed through - // plainToInstance - going through @DataAsJsonString() again would - // JSON.stringify an already-stringified value + // decryptedData is already a JSON string; plainToInstance would run it + // through @DataAsJsonString() again and double-encode it const merged = plainToInstance(PipelineDraftEntity, { ...restDecrypted, ...restUpdateData, From e20ca94fb77a02ae7a4a37eaec4ace86ef0f0ab7 Mon Sep 17 00:00:00 2001 From: ArtemHoruzhenko Date: Tue, 8 Sep 2026 20:44:19 +0300 Subject: [PATCH 4/4] fix(rdi): address more PR review findings on pipeline draft CRUD Excludes drafts whose data decrypt returned null due to an encryption strategy mismatch, rather than only handling decrypt exceptions - the plain strategy returns null instead of throwing when the stored value was encrypted with a different strategy. Uses the existence-only RDI lookup for the parent check in create(), so a draft can still be created even if the RDI instance's stored credentials can no longer be decrypted. --- .../rdi/pipeline-draft.service.spec.ts | 8 ++++ .../src/modules/rdi/pipeline-draft.service.ts | 2 +- .../local.pipeline-draft.repository.spec.ts | 41 +++++++++++++++++++ .../local.pipeline-draft.repository.ts | 7 ++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts index b7a19b6193..4dbfb96dc5 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.spec.ts @@ -80,6 +80,14 @@ describe('PipelineDraftService', () => { ).rejects.toThrow(NotFoundException); expect(repository.create).not.toHaveBeenCalled(); }); + + it('should check rdi existence without decrypting its credentials', async () => { + const dto = createPipelineDraftDtoFactory.build(); + + await service.create(mockSessionMetadata, mockRdiInstanceId, dto); + + expect(rdiRepository.get).toHaveBeenCalledWith(mockRdiInstanceId, true); + }); }); describe('list', () => { diff --git a/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts index abe6d0c3ac..45eb78e8ea 100644 --- a/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts +++ b/redisinsight/api/src/modules/rdi/pipeline-draft.service.ts @@ -24,7 +24,7 @@ export class PipelineDraftService { ): Promise { this.logger.debug('Creating pipeline draft', sessionMetadata); - const rdi = await this.rdiRepository.get(rdiInstanceId); + const rdi = await this.rdiRepository.get(rdiInstanceId, true); if (!rdi) { throw new NotFoundException( `RDI instance with id ${rdiInstanceId} was not found`, diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts index 753fbb2927..d5486ec3bb 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.spec.ts @@ -163,6 +163,47 @@ describe('LocalPipelineDraftRepository', () => { expect(result).toEqual([]); }); + + it('should exclude entities whose decrypt returns null instead of throwing (strategy mismatch)', async () => { + mockEncryptionServiceFactory.mockReturnValueOnce({ + getAvailableEncryptionStrategies: jest.fn(), + isEncryptionAvailable: jest.fn().mockResolvedValue(true), + encrypt: jest.fn().mockResolvedValue(mockEncryptResult), + decrypt: jest.fn().mockResolvedValue(null), + getEncryptionStrategy: jest.fn(), + }); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LocalPipelineDraftRepository, + { + provide: getRepositoryToken(PipelineDraftEntity), + useFactory: mockRepository, + }, + { + provide: EncryptionService, + useFactory: mockEncryptionServiceFactory, + }, + ], + }).compile(); + + const mismatchedRepository = module.get(LocalPipelineDraftRepository); + const mismatchedTypeormRepo = module.get( + getRepositoryToken(PipelineDraftEntity), + ); + const entity = pipelineDraftEntityFactory.build({ + rdiInstanceId: mockRdiInstanceId, + encryption: 'KEYTAR', + }); + mismatchedTypeormRepo.find.mockResolvedValueOnce([entity]); + + const result = await mismatchedRepository.list( + mockSessionMetadata, + mockRdiInstanceId, + ); + + expect(result).toEqual([]); + }); }); describe('get', () => { diff --git a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts index 5a4bec892d..58701937b9 100644 --- a/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts +++ b/redisinsight/api/src/modules/rdi/repository/local.pipeline-draft.repository.ts @@ -71,9 +71,10 @@ export class LocalPipelineDraftRepository extends PipelineDraftRepository { }), ); - return filter(decryptedEntities, (entity) => !isNull(entity)).map( - (entity) => classToClass(PipelineDraft, entity), - ); + return filter( + decryptedEntities, + (entity) => !isNull(entity) && !isNull(entity.data), + ).map((entity) => classToClass(PipelineDraft, entity)); } async get(