fix: AJV error type (#2466)

* return InvalidParamsException when AJV fails * update tests * update CEE integration tests
betagouv · Apr 10, 2024 · 0ec6e9a · 0ec6e9a
1 parent b31cc90
commit 0ec6e9a
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 85 deletions.
diff --git a/api/src/ilos/validator/AjvValidator.spec.ts b/api/src/ilos/validator/AjvValidator.spec.ts
@@ -1,6 +1,6 @@
+import { ConfigInterfaceResolver, RPCException } from '@ilos/common';
 import anyTest, { TestFn } from 'ava';
 import sinon from 'sinon';
-import { ConfigInterfaceResolver } from '@ilos/common';
 
 import { AjvValidator } from './AjvValidator';
 
@@ -34,7 +34,7 @@ class FakeObject {
   }
 }
 
-test('Json Schema provider: should work', async (t) => {
+test('should work', async (t) => {
   const schema = {
     $schema: 'http://json-schema.org/draft-07/schema#',
     $id: 'myschema',
@@ -52,7 +52,7 @@ test('Json Schema provider: should work', async (t) => {
   t.true(result);
 });
 
-test('Json Schema provider: should raise exception if data unvalid', async (t) => {
+test('should raise exception on invalid data', async (t) => {
   const schema = {
     $schema: 'http://json-schema.org/draft-07/schema#',
     $id: 'myschema',
@@ -66,11 +66,12 @@ test('Json Schema provider: should raise exception if data unvalid', async (t) =
   };
 
   t.context.provider.registerValidator(schema, FakeObject);
-  const err = await t.throwsAsync(async () => t.context.provider.validate(new FakeObject({ hello: 1 })));
-  t.is(err.message, 'data/hello must be string');
+  const err: RPCException = await t.throwsAsync(async () => t.context.provider.validate(new FakeObject({ hello: 1 })));
+  t.is(err.message, 'Invalid params');
+  t.is(err.rpcError.data[0], '/hello: must be string');
 });
 
-test('Json Schema provider: should work with ref', async (t) => {
+test('should work with ref', async (t) => {
   const subSchema = {
     $schema: 'http://json-schema.org/draft-07/schema#',
     $id: 'myschema.world',
@@ -99,7 +100,7 @@ test('Json Schema provider: should work with ref', async (t) => {
   t.true(result);
 });
 
-test('Json Schema provider: should work with inherance', async (t) => {
+test('should work with inheritance', async (t) => {
   class FakeObjectExtended extends FakeObject {}
 
   const schema = {

diff --git a/api/src/ilos/validator/AjvValidator.ts b/api/src/ilos/validator/AjvValidator.ts
@@ -1,10 +1,16 @@
 import * as ajv from 'ajv';
-import ajvKeywords from 'ajv-keywords';
-import addFormats from 'ajv-formats';
 import ajvErrors from 'ajv-errors';
+import addFormats from 'ajv-formats';
+import ajvKeywords from 'ajv-keywords';
 import jsonSchemaSecureJson from 'ajv/lib/refs/json-schema-secure.json';
 
-import { provider, ConfigInterfaceResolver, NewableType, ValidatorInterface } from '@ilos/common';
+import {
+  ConfigInterfaceResolver,
+  InvalidParamsException,
+  NewableType,
+  ValidatorInterface,
+  provider,
+} from '@ilos/common';
 
 @provider()
 export class AjvValidator implements ValidatorInterface {
@@ -97,12 +103,18 @@ export class AjvValidator implements ValidatorInterface {
       throw new Error(`No schema provided for this type (${resolver})`);
     }
     const validator = this.bindings.get(resolver);
-    const valid = await validator(data);
-    if (!valid) throw new Error(this.ajv.errorsText(validator.errors));
+    const valid = validator(data);
+
+    if (!valid) throw new InvalidParamsException(this.mapErrors(validator.errors));
 
     return true;
   }
 
+  protected mapErrors(errors: ajv.ErrorObject[]): string[] {
+    if (!errors || !Array.isArray(errors)) return [];
+    return errors.map((error) => `${error.instancePath}: ${error.message}`);
+  }
+
   protected addFormat(name: string, format: ajv.Format): ValidatorInterface {
     this.ajv.addFormat(name, format);
     return this;

diff --git a/api/src/ilos/validator/ValidatorMiddleware.ts b/api/src/ilos/validator/ValidatorMiddleware.ts
@@ -1,25 +1,18 @@
 import {
-  middleware,
-  ValidatorInterfaceResolver,
-  ParamsType,
   ContextType,
-  ResultType,
   MiddlewareInterface,
-  InvalidParamsException,
+  ParamsType,
+  ResultType,
+  ValidatorInterfaceResolver,
+  middleware,
 } from '@ilos/common';
 
 @middleware()
 export class ValidatorMiddleware implements MiddlewareInterface {
   constructor(private validator: ValidatorInterfaceResolver) {}
 
   async process(params: ParamsType, context: ContextType, next: Function, schema: string): Promise<ResultType> {
-    try {
-      await this.validator.validate(params, schema);
-    } catch (e) {
-      console.error(e);
-      throw new InvalidParamsException(e.message);
-    }
-
+    await this.validator.validate(params, schema);
     return next(params, context);
   }
 }
diff --git a/api/src/pdc/services/acquisition/actions/CreateJourneyAction.spec.ts b/api/src/pdc/services/acquisition/actions/CreateJourneyAction.spec.ts
@@ -1,12 +1,12 @@
+import { InvalidParamsException, ParseErrorException, ValidatorInterfaceResolver } from '@ilos/common';
+import { CarpoolAcquisitionService } from '@pdc/providers/carpool';
 import test from 'ava';
 import sinon, { SinonStubbedInstance } from 'sinon';
-import { CreateJourneyAction } from './CreateJourneyAction';
-import { AcquisitionRepositoryProvider } from '../providers/AcquisitionRepositoryProvider';
-import { ParseErrorException, ValidatorInterfaceResolver } from '@ilos/common';
 import { AcquisitionErrorStageEnum, AcquisitionStatusEnum } from '../interfaces/AcquisitionRepositoryProviderInterface';
-import { CarpoolAcquisitionService } from '@pdc/providers/carpool';
+import { AcquisitionRepositoryProvider } from '../providers/AcquisitionRepositoryProvider';
+import { CreateJourneyAction } from './CreateJourneyAction';
 
-function bootstap(): {
+function bootstrap(): {
   action: CreateJourneyAction;
   repository: SinonStubbedInstance<AcquisitionRepositoryProvider>;
   validator: SinonStubbedInstance<ValidatorInterfaceResolver>;
@@ -15,11 +15,12 @@ function bootstap(): {
   const validator = sinon.createStubInstance(ValidatorInterfaceResolver);
   const service = sinon.createStubInstance(CarpoolAcquisitionService);
   const action = new CreateJourneyAction(repository, validator, service);
+
   return { action, repository, validator };
 }
 
 test('should return repository data if validator not fail', async (t) => {
-  const { action, repository, validator } = bootstap();
+  const { action, repository, validator } = bootstrap();
   const created_at = new Date();
   const operator_journey_id = '1';
   const repositoryResponse = [{ operator_journey_id, created_at }];
@@ -44,7 +45,6 @@ test('should return repository data if validator not fail', async (t) => {
     },
   };
   const result = await action.call(inputData);
-  t.log(result);
   t.deepEqual(result, { operator_journey_id, created_at });
   const { api_version, ...payload } = inputData.params;
   t.true(validator.validate.calledOnceWith(payload));
@@ -61,12 +61,12 @@ test('should return repository data if validator not fail', async (t) => {
 });
 
 test('should fail if validator fail', async (t) => {
-  const { action, repository, validator } = bootstap();
+  const { action, repository, validator } = bootstrap();
   const created_at = new Date();
   const operator_journey_id = '1';
   const repositoryResponse = [{ operator_journey_id, created_at }];
   repository.createOrUpdateMany.resolves(repositoryResponse);
-  const validatorError = new Error('wrong');
+  const validatorError = new InvalidParamsException('wrong');
   validator.validate.rejects(validatorError);
   const inputData = {
     method: '',
@@ -105,7 +105,7 @@ test('should fail if validator fail', async (t) => {
 });
 
 test('should fail if date validation fail', async (t) => {
-  const { action, repository, validator } = bootstap();
+  const { action, repository, validator } = bootstrap();
   const created_at = new Date();
   const operator_journey_id = '1';
   const repositoryResponse = [{ operator_journey_id, created_at }];
@@ -152,7 +152,7 @@ test('should fail if date validation fail', async (t) => {
 });
 
 test('should fail if journey already registred', async (t) => {
-  const { action, repository, validator } = bootstap();
+  const { action, repository, validator } = bootstrap();
   const operator_journey_id = '1';
   repository.createOrUpdateMany.resolves([]);
   const inputData = {

diff --git a/api/src/pdc/services/acquisition/actions/CreateJourneyAction.ts b/api/src/pdc/services/acquisition/actions/CreateJourneyAction.ts
@@ -38,6 +38,7 @@ export class CreateJourneyAction extends AbstractAction {
     // validate the payload manually to log rejected journeys
     try {
       await this.validate(apiVersionString, payload);
+
       const acquisitions = await this.repository.createOrUpdateMany([
         {
           operator_id,
@@ -48,9 +49,11 @@ export class CreateJourneyAction extends AbstractAction {
           payload,
         },
       ]);
+
       if (acquisitions.length !== 1) {
         throw new ConflictException('Journey already registered');
       }
+
       if (env.or_false('APP_ENABLE_CARPOOL_V2')) {
         await this.acquisitionService.registerRequest({
           api_version,
@@ -90,15 +93,18 @@ export class CreateJourneyAction extends AbstractAction {
           incentives: payload.incentives,
         });
       }
+
       return {
         operator_journey_id: acquisitions[0].operator_journey_id,
         created_at: acquisitions[0].created_at,
       };
     } catch (e) {
+      console.error(e.message, { operator_journey_id, operator_id });
+
       if (e instanceof ConflictException) {
         throw e;
       }
-      console.error(e.message, { journey_id: operator_journey_id, operator_id });
+
       await this.repository.createOrUpdateMany([
         {
           operator_id,
@@ -112,6 +118,7 @@ export class CreateJourneyAction extends AbstractAction {
           errors: [e],
         },
       ]);
+
       throw e;
     }
   }

diff --git a/api/src/pdc/services/cee/actions/ImportCeeAction.integration.spec.ts b/api/src/pdc/services/cee/actions/ImportCeeAction.integration.spec.ts
@@ -1,15 +1,15 @@
-import anyTest, { TestFn } from 'ava';
-import { handlerMacro, HandlerMacroContext, makeDbBeforeAfter, DbContext } from '@pdc/providers/test';
-import { ServiceProvider } from '../ServiceProvider';
-import { ParamsInterface, ResultInterface, handlerConfig } from '@shared/cee/importApplication.contract';
 import { ContextType } from '@ilos/common';
+import { PostgresConnection } from '@ilos/connection-postgres';
+import { DbContext, HandlerMacroContext, handlerMacro, makeDbBeforeAfter } from '@pdc/providers/test';
 import {
   ceeJourneyTypeEnumSchema,
   lastNameTruncSchema,
   phoneTruncSchema,
   timestampSchema,
 } from '@shared/cee/common/ceeSchema';
-import { PostgresConnection } from '@ilos/connection-postgres';
+import { ParamsInterface, ResultInterface, handlerConfig } from '@shared/cee/importApplication.contract';
+import anyTest, { TestFn } from 'ava';
+import { ServiceProvider } from '../ServiceProvider';
 
 const { before, after, success, error } = handlerMacro<ParamsInterface, ResultInterface>(
   ServiceProvider,
@@ -54,7 +54,7 @@ test.serial(
   [],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, 'data must NOT have fewer than 1 items');
+    t.is(e.rpcError?.data[0], ': must NOT have fewer than 1 items');
   },
   defaultContext,
 );
@@ -64,7 +64,7 @@ test.serial(
   [{ ...defaultPayload, last_name_trunc: 'abcd' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/last_name_trunc ${lastNameTruncSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/last_name_trunc: ${lastNameTruncSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -74,7 +74,7 @@ test.serial(
   [{ ...defaultPayload, journey_type: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/journey_type ${ceeJourneyTypeEnumSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/journey_type: ${ceeJourneyTypeEnumSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -84,7 +84,7 @@ test.serial(
   [{ ...defaultPayload, datetime: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/datetime ${timestampSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/datetime: ${timestampSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -94,7 +94,7 @@ test.serial(
   [{ ...defaultPayload, phone_trunc: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/phone_trunc ${phoneTruncSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/phone_trunc: ${phoneTruncSchema.errorMessage}`);
   },
   defaultContext,
 );

diff --git a/api/src/pdc/services/cee/actions/ImportCeeAction.ts b/api/src/pdc/services/cee/actions/ImportCeeAction.ts
@@ -28,7 +28,7 @@ export class ImportCeeAction extends AbstractAction {
     const operator_id = getOperatorIdOrFail(context);
     const data = params.map((d, i) => ({
       ...d,
-      datetime: getDateOrFail(d.datetime, `data/${i}/datetime ${timestampSchema.errorMessage}`),
+      datetime: getDateOrFail(d.datetime, `data/${i}/datetime: ${timestampSchema.errorMessage}`),
     }));
 
     const result: ResultInterface = {

diff --git a/api/src/pdc/services/cee/actions/ImportCeeIdentityAction.integration.spec.ts b/api/src/pdc/services/cee/actions/ImportCeeIdentityAction.integration.spec.ts
@@ -1,15 +1,15 @@
-import anyTest, { TestFn } from 'ava';
-import { handlerMacro, HandlerMacroContext, makeDbBeforeAfter, DbContext } from '@pdc/providers/test';
-import { ServiceProvider } from '../ServiceProvider';
-import { ParamsInterface, ResultInterface, handlerConfig } from '@shared/cee/importApplicationIdentity.contract';
 import { ContextType } from '@ilos/common';
+import { PostgresConnection } from '@ilos/connection-postgres';
+import { DbContext, HandlerMacroContext, handlerMacro, makeDbBeforeAfter } from '@pdc/providers/test';
 import {
   ceeJourneyTypeEnumSchema,
   lastNameTruncSchema,
   phoneTruncSchema,
   timestampSchema,
 } from '@shared/cee/common/ceeSchema';
-import { PostgresConnection } from '@ilos/connection-postgres';
+import { ParamsInterface, ResultInterface, handlerConfig } from '@shared/cee/importApplicationIdentity.contract';
+import anyTest, { TestFn } from 'ava';
+import { ServiceProvider } from '../ServiceProvider';
 
 const { before, after, error } = handlerMacro<ParamsInterface, ResultInterface>(ServiceProvider, handlerConfig);
 const { before: dbBefore, after: dbAfter } = makeDbBeforeAfter();
@@ -53,7 +53,7 @@ test.serial(
   [],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, 'data must NOT have fewer than 1 items');
+    t.is(e.rpcError?.data[0], ': must NOT have fewer than 1 items');
   },
   defaultContext,
 );
@@ -63,7 +63,7 @@ test.serial(
   [{ ...defaultPayload, last_name_trunc: 'abcd' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/last_name_trunc ${lastNameTruncSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/last_name_trunc: ${lastNameTruncSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -73,7 +73,7 @@ test.serial(
   [{ ...defaultPayload, journey_type: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/journey_type ${ceeJourneyTypeEnumSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/journey_type: ${ceeJourneyTypeEnumSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -83,7 +83,7 @@ test.serial(
   [{ ...defaultPayload, datetime: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/datetime ${timestampSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/datetime: ${timestampSchema.errorMessage}`);
   },
   defaultContext,
 );
@@ -93,7 +93,7 @@ test.serial(
   [{ ...defaultPayload, phone_trunc: 'bip' }],
   (e: any, t) => {
     t.is(e.message, 'Invalid params');
-    t.is(e.rpcError?.data, `data/0/phone_trunc ${phoneTruncSchema.errorMessage}`);
+    t.is(e.rpcError?.data[0], `/0/phone_trunc: ${phoneTruncSchema.errorMessage}`);
   },
   defaultContext,
 );