diff --git a/packages/cli/src/evaluation/metric.schema.ts b/packages/cli/src/evaluation/metric.schema.ts new file mode 100644 index 0000000000..fb186c50df --- /dev/null +++ b/packages/cli/src/evaluation/metric.schema.ts @@ -0,0 +1,13 @@ +import { z } from 'zod'; + +export const testMetricCreateRequestBodySchema = z + .object({ + name: z.string().min(1).max(255), + }) + .strict(); + +export const testMetricPatchRequestBodySchema = z + .object({ + name: z.string().min(1).max(255), + }) + .strict(); diff --git a/packages/cli/src/evaluation/metrics.controller.ts b/packages/cli/src/evaluation/metrics.controller.ts new file mode 100644 index 0000000000..af8f5c0408 --- /dev/null +++ b/packages/cli/src/evaluation/metrics.controller.ts @@ -0,0 +1,130 @@ +import express from 'express'; + +import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; +import { Delete, Get, Patch, Post, RestController } from '@/decorators'; +import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { + testMetricCreateRequestBodySchema, + testMetricPatchRequestBodySchema, +} from '@/evaluation/metric.schema'; +import { getSharedWorkflowIds } from '@/public-api/v1/handlers/workflows/workflows.service'; + +import { TestDefinitionService } from './test-definition.service.ee'; +import { TestMetricsRequest } from './test-definitions.types.ee'; + +@RestController('/evaluation/test-definitions') +export class TestMetricsController { + constructor( + private readonly testDefinitionService: TestDefinitionService, + private readonly testMetricRepository: TestMetricRepository, + ) {} + + // This method is used in multiple places in the controller to get the test definition + // (or just check that it exists and the user has access to it). + private async getTestDefinition( + req: + | TestMetricsRequest.GetOne + | TestMetricsRequest.GetMany + | TestMetricsRequest.Patch + | TestMetricsRequest.Delete + | TestMetricsRequest.Create, + ) { + const { testDefinitionId } = req.params; + + const userAccessibleWorkflowIds = await getSharedWorkflowIds(req.user, ['workflow:read']); + + const testDefinition = await this.testDefinitionService.findOne( + testDefinitionId, + userAccessibleWorkflowIds, + ); + + if (!testDefinition) throw new NotFoundError('Test definition not found'); + + return testDefinition; + } + + @Get('/:testDefinitionId/metrics') + async getMany(req: TestMetricsRequest.GetMany) { + const { testDefinitionId } = req.params; + + await this.getTestDefinition(req); + + return await this.testMetricRepository.find({ + where: { testDefinition: { id: testDefinitionId } }, + }); + } + + @Get('/:testDefinitionId/metrics/:id') + async getOne(req: TestMetricsRequest.GetOne) { + const { id: metricId, testDefinitionId } = req.params; + + await this.getTestDefinition(req); + + const metric = await this.testMetricRepository.findOne({ + where: { id: metricId, testDefinition: { id: testDefinitionId } }, + }); + + if (!metric) throw new NotFoundError('Metric not found'); + + return metric; + } + + @Post('/:testDefinitionId/metrics') + async create(req: TestMetricsRequest.Create, res: express.Response) { + const bodyParseResult = testMetricCreateRequestBodySchema.safeParse(req.body); + if (!bodyParseResult.success) { + res.status(400).json({ errors: bodyParseResult.error.errors }); + return; + } + + const testDefinition = await this.getTestDefinition(req); + + const metric = this.testMetricRepository.create({ + ...req.body, + testDefinition, + }); + + return await this.testMetricRepository.save(metric); + } + + @Patch('/:testDefinitionId/metrics/:id') + async patch(req: TestMetricsRequest.Patch, res: express.Response) { + const { id: metricId, testDefinitionId } = req.params; + + const bodyParseResult = testMetricPatchRequestBodySchema.safeParse(req.body); + if (!bodyParseResult.success) { + res.status(400).json({ errors: bodyParseResult.error.errors }); + return; + } + + await this.getTestDefinition(req); + + const metric = await this.testMetricRepository.findOne({ + where: { id: metricId, testDefinition: { id: testDefinitionId } }, + }); + + if (!metric) throw new NotFoundError('Metric not found'); + + await this.testMetricRepository.update(metricId, bodyParseResult.data); + + // Respond with the updated metric + return await this.testMetricRepository.findOneBy({ id: metricId }); + } + + @Delete('/:testDefinitionId/metrics/:id') + async delete(req: TestMetricsRequest.GetOne) { + const { id: metricId, testDefinitionId } = req.params; + + await this.getTestDefinition(req); + + const metric = await this.testMetricRepository.findOne({ + where: { id: metricId, testDefinition: { id: testDefinitionId } }, + }); + + if (!metric) throw new NotFoundError('Metric not found'); + + await this.testMetricRepository.delete(metricId); + + return { success: true }; + } +} diff --git a/packages/cli/src/evaluation/test-definitions.types.ee.ts b/packages/cli/src/evaluation/test-definitions.types.ee.ts index 2b784b10ef..939a37b949 100644 --- a/packages/cli/src/evaluation/test-definitions.types.ee.ts +++ b/packages/cli/src/evaluation/test-definitions.types.ee.ts @@ -33,3 +33,33 @@ export declare namespace TestDefinitionsRequest { type Run = AuthenticatedRequest; } + +// ---------------------------------- +// /test-definitions/:testDefinitionId/metrics +// ---------------------------------- + +export declare namespace TestMetricsRequest { + namespace RouteParams { + type TestDefinitionId = { + testDefinitionId: string; + }; + + type TestMetricId = { + id: string; + }; + } + + type GetOne = AuthenticatedRequest; + + type GetMany = AuthenticatedRequest; + + type Create = AuthenticatedRequest; + + type Patch = AuthenticatedRequest< + RouteParams.TestDefinitionId & RouteParams.TestMetricId, + {}, + { name: string } + >; + + type Delete = AuthenticatedRequest; +} diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index e0572ab215..a21ad98ac2 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -64,6 +64,7 @@ import '@/executions/executions.controller'; import '@/external-secrets/external-secrets.controller.ee'; import '@/license/license.controller'; import '@/evaluation/test-definitions.controller.ee'; +import '@/evaluation/metrics.controller'; import '@/workflows/workflow-history/workflow-history.controller.ee'; import '@/workflows/workflows.controller'; diff --git a/packages/cli/test/integration/evaluation/metrics.api.test.ts b/packages/cli/test/integration/evaluation/metrics.api.test.ts new file mode 100644 index 0000000000..d10bb106a5 --- /dev/null +++ b/packages/cli/test/integration/evaluation/metrics.api.test.ts @@ -0,0 +1,381 @@ +import { Container } from 'typedi'; + +import type { TestDefinition } from '@/databases/entities/test-definition.ee'; +import type { User } from '@/databases/entities/user'; +import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; +import { TestDefinitionRepository } from '@/databases/repositories/test-definition.repository.ee'; +import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; +import { createUserShell } from '@test-integration/db/users'; +import { createWorkflow } from '@test-integration/db/workflows'; +import * as testDb from '@test-integration/test-db'; +import type { SuperAgentTest } from '@test-integration/types'; +import * as utils from '@test-integration/utils'; + +let authOwnerAgent: SuperAgentTest; +let workflowUnderTest: WorkflowEntity; +let otherWorkflow: WorkflowEntity; +let testDefinition: TestDefinition; +let otherTestDefinition: TestDefinition; +let ownerShell: User; + +const testServer = utils.setupTestServer({ endpointGroups: ['evaluation'] }); + +beforeAll(async () => { + ownerShell = await createUserShell('global:owner'); + authOwnerAgent = testServer.authAgentFor(ownerShell); +}); + +beforeEach(async () => { + await testDb.truncate(['TestDefinition', 'TestMetric']); + + workflowUnderTest = await createWorkflow({ name: 'workflow-under-test' }, ownerShell); + + testDefinition = Container.get(TestDefinitionRepository).create({ + name: 'test', + workflow: { id: workflowUnderTest.id }, + }); + await Container.get(TestDefinitionRepository).save(testDefinition); + + otherWorkflow = await createWorkflow({ name: 'other-workflow' }); + + otherTestDefinition = Container.get(TestDefinitionRepository).create({ + name: 'other-test', + workflow: { id: otherWorkflow.id }, + }); + await Container.get(TestDefinitionRepository).save(otherTestDefinition); +}); + +describe('GET /evaluation/test-definitions/:testDefinitionId/metrics', () => { + test('should retrieve empty list of metrics for a test definition', async () => { + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${testDefinition.id}/metrics`, + ); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data.length).toBe(0); + }); + + test('should retrieve metrics for a test definition', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const newMetric2 = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-2', + }); + await Container.get(TestMetricRepository).save(newMetric2); + + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${testDefinition.id}/metrics`, + ); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data.length).toBe(2); + expect(resp.body.data).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: expect.any(String), + name: 'metric-1', + }), + expect.objectContaining({ + id: expect.any(String), + name: 'metric-2', + }), + ]), + ); + }); + + test('should return 404 if test definition does not exist', async () => { + const resp = await authOwnerAgent.get('/evaluation/test-definitions/999/metrics'); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if test definition is not accessible to the user', async () => { + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${otherTestDefinition.id}/metrics`, + ); + + expect(resp.statusCode).toBe(404); + }); +}); + +describe('GET /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => { + test('should retrieve a metric for a test definition', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`, + ); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data).toEqual( + expect.objectContaining({ + id: newMetric.id, + name: 'metric-1', + }), + ); + }); + + test('should return 404 if metric does not exist', async () => { + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${testDefinition.id}/metrics/999`, + ); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if metric is not accessible to the user', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: otherTestDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent.get( + `/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`, + ); + + expect(resp.statusCode).toBe(404); + }); +}); + +describe('POST /evaluation/test-definitions/:testDefinitionId/metrics', () => { + test('should create a metric for a test definition', async () => { + const resp = await authOwnerAgent + .post(`/evaluation/test-definitions/${testDefinition.id}/metrics`) + .send({ + name: 'metric-1', + }); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data).toEqual( + expect.objectContaining({ + id: expect.any(String), + name: 'metric-1', + }), + ); + + const metrics = await Container.get(TestMetricRepository).find({ + where: { testDefinition: { id: testDefinition.id } }, + }); + expect(metrics.length).toBe(1); + expect(metrics[0].name).toBe('metric-1'); + }); + + test('should return 400 if name is missing', async () => { + const resp = await authOwnerAgent + .post(`/evaluation/test-definitions/${testDefinition.id}/metrics`) + .send({}); + + expect(resp.statusCode).toBe(400); + expect(resp.body.errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'invalid_type', + message: 'Required', + path: ['name'], + }), + ]), + ); + }); + + test('should return 400 if name is not a string', async () => { + const resp = await authOwnerAgent + .post(`/evaluation/test-definitions/${testDefinition.id}/metrics`) + .send({ + name: 123, + }); + + expect(resp.statusCode).toBe(400); + expect(resp.body.errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'invalid_type', + message: 'Expected string, received number', + path: ['name'], + }), + ]), + ); + }); + + test('should return 404 if test definition does not exist', async () => { + const resp = await authOwnerAgent.post('/evaluation/test-definitions/999/metrics').send({ + name: 'metric-1', + }); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if test definition is not accessible to the user', async () => { + const resp = await authOwnerAgent + .post(`/evaluation/test-definitions/${otherTestDefinition.id}/metrics`) + .send({ + name: 'metric-1', + }); + + expect(resp.statusCode).toBe(404); + }); +}); + +describe('PATCH /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => { + test('should update a metric for a test definition', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent + .patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`) + .send({ + name: 'metric-2', + }); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data).toEqual( + expect.objectContaining({ + id: newMetric.id, + name: 'metric-2', + }), + ); + + const metrics = await Container.get(TestMetricRepository).find({ + where: { testDefinition: { id: testDefinition.id } }, + }); + expect(metrics.length).toBe(1); + expect(metrics[0].name).toBe('metric-2'); + }); + + test('should return 400 if name is missing', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent + .patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`) + .send({}); + + expect(resp.statusCode).toBe(400); + expect(resp.body.errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'invalid_type', + message: 'Required', + path: ['name'], + }), + ]), + ); + }); + + test('should return 400 if name is not a string', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent + .patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`) + .send({ + name: 123, + }); + + expect(resp.statusCode).toBe(400); + expect(resp.body.errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'invalid_type', + message: 'Expected string, received number', + }), + ]), + ); + }); + + test('should return 404 if metric does not exist', async () => { + const resp = await authOwnerAgent + .patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/999`) + .send({ + name: 'metric-1', + }); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if test definition does not exist', async () => { + const resp = await authOwnerAgent.patch('/evaluation/test-definitions/999/metrics/999').send({ + name: 'metric-1', + }); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if metric is not accessible to the user', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: otherTestDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent + .patch(`/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`) + .send({ + name: 'metric-2', + }); + + expect(resp.statusCode).toBe(404); + }); +}); + +describe('DELETE /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => { + test('should delete a metric for a test definition', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: testDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent.delete( + `/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`, + ); + + expect(resp.statusCode).toBe(200); + expect(resp.body.data).toEqual({ success: true }); + + const metrics = await Container.get(TestMetricRepository).find({ + where: { testDefinition: { id: testDefinition.id } }, + }); + expect(metrics.length).toBe(0); + }); + + test('should return 404 if metric does not exist', async () => { + const resp = await authOwnerAgent.delete( + `/evaluation/test-definitions/${testDefinition.id}/metrics/999`, + ); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if metric is not accessible to the user', async () => { + const newMetric = Container.get(TestMetricRepository).create({ + testDefinition: { id: otherTestDefinition.id }, + name: 'metric-1', + }); + await Container.get(TestMetricRepository).save(newMetric); + + const resp = await authOwnerAgent.delete( + `/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`, + ); + + expect(resp.statusCode).toBe(404); + }); +}); diff --git a/packages/cli/test/integration/shared/test-db.ts b/packages/cli/test/integration/shared/test-db.ts index e85825115e..5b70db3be2 100644 --- a/packages/cli/test/integration/shared/test-db.ts +++ b/packages/cli/test/integration/shared/test-db.ts @@ -75,6 +75,7 @@ const repositories = [ 'SharedWorkflow', 'Tag', 'TestDefinition', + 'TestMetric', 'User', 'Variables', 'Webhook', diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index b1e7d8a93c..208e05e0f1 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -279,6 +279,7 @@ export const setupTestServer = ({ break; case 'evaluation': + await import('@/evaluation/metrics.controller'); await import('@/evaluation/test-definitions.controller.ee'); break; }