From f15e7d11b10978739c607c3554131018da04c9f0 Mon Sep 17 00:00:00 2001 From: sbriat Date: Fri, 25 Aug 2023 15:16:33 +0200 Subject: [PATCH] use strict null checks --- package-lock.json | 8 +- package.json | 2 +- .../commands/create-ad/create-ad.command.ts | 2 +- .../ad/infrastructure/ad.repository.ts | 8 +- .../application/types/default-params.type.ts | 4 +- .../geography/core/domain/route.entity.ts | 73 +++---------------- .../geography/core/domain/route.types.ts | 2 +- .../geography/infrastructure/geodesic.ts | 1 + .../infrastructure/graphhopper-georouter.ts | 34 +++++---- src/modules/geography/route.mapper.ts | 30 +++----- .../unit/infrastructure/geodesic.spec.ts | 4 +- .../geography/tests/unit/route.mapper.spec.ts | 8 -- .../has-valid-position-indexes.validator.ts | 16 ++-- .../grpc-controllers/dtos/waypoint.dto.ts | 5 +- ...s-valid-position-indexes.validator.spec.ts | 57 ++++++--------- tsconfig.json | 2 +- 16 files changed, 91 insertions(+), 165 deletions(-) diff --git a/package-lock.json b/package-lock.json index 27bd203..32000a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@grpc/proto-loader": "^0.7.6", "@liaoliaots/nestjs-redis": "^9.0.5", "@mobicoop/configuration-module": "^1.2.0", - "@mobicoop/ddd-library": "^1.1.0", + "@mobicoop/ddd-library": "file:../../packages/dddlibrary", "@mobicoop/health-module": "^2.0.0", "@mobicoop/message-broker-module": "^1.2.0", "@nestjs/axios": "^2.0.0", @@ -1505,9 +1505,9 @@ } }, "node_modules/@mobicoop/ddd-library": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@mobicoop/ddd-library/-/ddd-library-1.1.0.tgz", - "integrity": "sha512-x4X7j2CJYZQPDZgLuZP5TFk59fle1wTPdX++Z2YyD7VwwV+yOmVvMIRfTyLRFUTzLObrd6FKs8mh+g59n9jUlA==", + "version": "1.1.1", + "resolved": "file:../../packages/dddlibrary", + "license": "AGPL", "dependencies": { "@nestjs/event-emitter": "^1.4.2", "@nestjs/microservices": "^9.4.0", diff --git a/package.json b/package.json index ed61f84..ab45121 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "@grpc/proto-loader": "^0.7.6", "@liaoliaots/nestjs-redis": "^9.0.5", "@mobicoop/configuration-module": "^1.2.0", - "@mobicoop/ddd-library": "^1.1.0", + "@mobicoop/ddd-library": "file:../../packages/dddlibrary", "@mobicoop/health-module": "^2.0.0", "@mobicoop/message-broker-module": "^1.2.0", "@nestjs/axios": "^2.0.0", diff --git a/src/modules/ad/core/application/commands/create-ad/create-ad.command.ts b/src/modules/ad/core/application/commands/create-ad/create-ad.command.ts index 3b1e695..5d9839f 100644 --- a/src/modules/ad/core/application/commands/create-ad/create-ad.command.ts +++ b/src/modules/ad/core/application/commands/create-ad/create-ad.command.ts @@ -18,7 +18,7 @@ export class CreateAdCommand extends Command { constructor(props: CommandProps) { super(props); - this.id = props.id; + this.id = props.id as string; this.driver = props.driver; this.passenger = props.passenger; this.frequency = props.frequency; diff --git a/src/modules/ad/infrastructure/ad.repository.ts b/src/modules/ad/infrastructure/ad.repository.ts index ae07fd7..cd10089 100644 --- a/src/modules/ad/infrastructure/ad.repository.ts +++ b/src/modules/ad/infrastructure/ad.repository.ts @@ -18,10 +18,10 @@ export type AdBaseModel = { seatsProposed: number; seatsRequested: number; strict: boolean; - driverDuration: number; - driverDistance: number; - passengerDuration: number; - passengerDistance: number; + driverDuration?: number; + driverDistance?: number; + passengerDuration?: number; + passengerDistance?: number; fwdAzimuth: number; backAzimuth: number; createdAt: Date; diff --git a/src/modules/geography/core/application/types/default-params.type.ts b/src/modules/geography/core/application/types/default-params.type.ts index 12ea88e..ba61d39 100644 --- a/src/modules/geography/core/application/types/default-params.type.ts +++ b/src/modules/geography/core/application/types/default-params.type.ts @@ -1,4 +1,4 @@ export type DefaultParams = { - GEOROUTER_TYPE: string; - GEOROUTER_URL: string; + GEOROUTER_TYPE?: string; + GEOROUTER_URL?: string; }; diff --git a/src/modules/geography/core/domain/route.entity.ts b/src/modules/geography/core/domain/route.entity.ts index 46177b7..2930482 100644 --- a/src/modules/geography/core/domain/route.entity.ts +++ b/src/modules/geography/core/domain/route.entity.ts @@ -25,8 +25,9 @@ export class RouteEntity extends AggregateRoot { } catch (e: any) { throw e; } - let driverRoute: Route; - let passengerRoute: Route; + let baseRoute: Route; + let driverRoute: Route | undefined; + let passengerRoute: Route | undefined; if (routes.some((route: Route) => route.type == PathType.GENERIC)) { driverRoute = passengerRoute = routes.find( (route: Route) => route.type == PathType.GENERIC, @@ -41,22 +42,21 @@ export class RouteEntity extends AggregateRoot { ? routes.find((route: Route) => route.type == PathType.PASSENGER) : undefined; } + if (driverRoute) { + baseRoute = driverRoute; + } else { + baseRoute = passengerRoute as Route; + } const routeProps: RouteProps = { driverDistance: driverRoute?.distance, driverDuration: driverRoute?.duration, passengerDistance: passengerRoute?.distance, passengerDuration: passengerRoute?.duration, - fwdAzimuth: driverRoute - ? driverRoute.fwdAzimuth - : passengerRoute.fwdAzimuth, - backAzimuth: driverRoute - ? driverRoute.backAzimuth - : passengerRoute.backAzimuth, - distanceAzimuth: driverRoute - ? driverRoute.distanceAzimuth - : passengerRoute.distanceAzimuth, + fwdAzimuth: baseRoute.fwdAzimuth, + backAzimuth: baseRoute.backAzimuth, + distanceAzimuth: baseRoute.distanceAzimuth, waypoints: create.waypoints, - points: driverRoute ? driverRoute.points : passengerRoute.points, + points: baseRoute.points, }; return new RouteEntity({ id: v4(), @@ -111,52 +111,3 @@ export class RouteEntity extends AggregateRoot { points, }); } - -// import { IGeodesic } from '../interfaces/geodesic.interface'; -// import { Point } from '../types/point.type'; -// import { SpacetimePoint } from './spacetime-point'; - -// export class Route { -// distance: number; -// duration: number; -// fwdAzimuth: number; -// backAzimuth: number; -// distanceAzimuth: number; -// points: Point[]; -// spacetimePoints: SpacetimePoint[]; -// private geodesic: IGeodesic; - -// constructor(geodesic: IGeodesic) { -// this.distance = undefined; -// this.duration = undefined; -// this.fwdAzimuth = undefined; -// this.backAzimuth = undefined; -// this.distanceAzimuth = undefined; -// this.points = []; -// this.spacetimePoints = []; -// this.geodesic = geodesic; -// } - -// setPoints = (points: Point[]): void => { -// this.points = points; -// this.setAzimuth(points); -// }; - -// setSpacetimePoints = (spacetimePoints: SpacetimePoint[]): void => { -// this.spacetimePoints = spacetimePoints; -// }; - -// protected setAzimuth = (points: Point[]): void => { -// const inverse = this.geodesic.inverse( -// points[0].lon, -// points[0].lat, -// points[points.length - 1].lon, -// points[points.length - 1].lat, -// ); -// this.fwdAzimuth = -// inverse.azimuth >= 0 ? inverse.azimuth : 360 - Math.abs(inverse.azimuth); -// this.backAzimuth = -// this.fwdAzimuth > 180 ? this.fwdAzimuth - 180 : this.fwdAzimuth + 180; -// this.distanceAzimuth = inverse.distance; -// }; -// } diff --git a/src/modules/geography/core/domain/route.types.ts b/src/modules/geography/core/domain/route.types.ts index 5860261..748c759 100644 --- a/src/modules/geography/core/domain/route.types.ts +++ b/src/modules/geography/core/domain/route.types.ts @@ -52,7 +52,7 @@ export type Waypoint = Coordinates & { export type SpacetimePoint = Coordinates & { duration: number; - distance: number; + distance?: number; }; export enum Role { diff --git a/src/modules/geography/infrastructure/geodesic.ts b/src/modules/geography/infrastructure/geodesic.ts index a0f1e76..f7ac3f1 100644 --- a/src/modules/geography/infrastructure/geodesic.ts +++ b/src/modules/geography/infrastructure/geodesic.ts @@ -22,6 +22,7 @@ export class Geodesic implements GeodesicPort { lat2, lon2, ); + if (!azimuth || !distance) throw new Error('Azimuth not found'); return { azimuth, distance }; }; } diff --git a/src/modules/geography/infrastructure/graphhopper-georouter.ts b/src/modules/geography/infrastructure/graphhopper-georouter.ts index ce67939..198fcf3 100644 --- a/src/modules/geography/infrastructure/graphhopper-georouter.ts +++ b/src/modules/geography/infrastructure/graphhopper-georouter.ts @@ -72,11 +72,12 @@ export class GraphhopperGeorouter implements GeorouterPort { .map((point) => [point.lat, point.lon].join('%2C')) .join('&point='), ].join(''); - const route = await lastValueFrom( + return await lastValueFrom( this.httpService.get(url).pipe( - map((res) => - res.data ? this.createRoute(res, path.type) : undefined, - ), + map((response) => { + if (response.data) return this.createRoute(response, path.type); + throw new Error(); + }), catchError((error: AxiosError) => { if (error.code == AxiosError.ERR_BAD_REQUEST) { throw new RouteNotFoundException( @@ -88,7 +89,6 @@ export class GraphhopperGeorouter implements GeorouterPort { }), ), ); - return route; }), ); return routes; @@ -156,12 +156,20 @@ export class GraphhopperGeorouter implements GeorouterPort { const indices = this.getIndices(points, snappedWaypoints); const times = this.getTimes(durations, indices); const distances = this.getDistances(instructions, indices); - return indices.map((index) => ({ - lon: points[index][1], - lat: points[index][0], - distance: distances.find((distance) => distance.index == index)?.distance, - duration: times.find((time) => time.index == index)?.duration, - })); + return indices.map((index) => { + const duration = times.find((time) => time.index == index); + if (!duration) + throw new Error(`Duration not found for waypoint #${index}`); + const distance = distances.find((distance) => distance.index == index); + if (!distance && instructions.length > 0) + throw new Error(`Distance not found for waypoint #${index}`); + return { + lon: points[index][1], + lat: points[index][0], + distance: distance?.distance, + duration: duration.duration, + }; + }); }; private getIndices = ( @@ -182,7 +190,7 @@ export class GraphhopperGeorouter implements GeorouterPort { index: number; originIndex: number; waypoint: number[]; - nearest: number; + nearest?: number; distance: number; } >{ @@ -209,7 +217,7 @@ export class GraphhopperGeorouter implements GeorouterPort { } } for (const missedWaypoint of missedWaypoints) { - indices[missedWaypoint.originIndex] = missedWaypoint.nearest; + indices[missedWaypoint.originIndex] = missedWaypoint.nearest as number; } return indices; }; diff --git a/src/modules/geography/route.mapper.ts b/src/modules/geography/route.mapper.ts index 4f5d63c..a714ff5 100644 --- a/src/modules/geography/route.mapper.ts +++ b/src/modules/geography/route.mapper.ts @@ -14,26 +14,20 @@ import { RouteResponseDto } from './interface/dtos/route.response.dto'; export class RouteMapper implements Mapper { - toPersistence = (): undefined => { - return undefined; - }; - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - toDomain = (): undefined => { - return undefined; - }; - - // eslint-disable-next-line @typescript-eslint/no-unused-vars toResponse = (entity: RouteEntity): RouteResponseDto => { const response = new RouteResponseDto(); - response.driverDistance = Math.round(entity.getProps().driverDistance); - response.driverDuration = Math.round(entity.getProps().driverDuration); - response.passengerDistance = Math.round( - entity.getProps().passengerDistance, - ); - response.passengerDuration = Math.round( - entity.getProps().passengerDuration, - ); + response.driverDistance = entity.getProps().driverDistance + ? Math.round(entity.getProps().driverDistance as number) + : undefined; + response.driverDuration = entity.getProps().driverDuration + ? Math.round(entity.getProps().driverDuration as number) + : undefined; + response.passengerDistance = entity.getProps().passengerDistance + ? Math.round(entity.getProps().passengerDistance as number) + : undefined; + response.passengerDuration = entity.getProps().passengerDuration + ? Math.round(entity.getProps().passengerDuration as number) + : undefined; response.fwdAzimuth = Math.round(entity.getProps().fwdAzimuth); response.backAzimuth = Math.round(entity.getProps().backAzimuth); response.distanceAzimuth = Math.round(entity.getProps().distanceAzimuth); diff --git a/src/modules/geography/tests/unit/infrastructure/geodesic.spec.ts b/src/modules/geography/tests/unit/infrastructure/geodesic.spec.ts index a71df2e..c094a0e 100644 --- a/src/modules/geography/tests/unit/infrastructure/geodesic.spec.ts +++ b/src/modules/geography/tests/unit/infrastructure/geodesic.spec.ts @@ -8,7 +8,7 @@ describe('Matcher geodesic', () => { it('should get inverse values', () => { const geodesic: Geodesic = new Geodesic(); const inv = geodesic.inverse(0, 0, 1, 1); - expect(Math.round(inv.azimuth)).toBe(45); - expect(Math.round(inv.distance)).toBe(156900); + expect(Math.round(inv.azimuth as number)).toBe(45); + expect(Math.round(inv.distance as number)).toBe(156900); }); }); diff --git a/src/modules/geography/tests/unit/route.mapper.spec.ts b/src/modules/geography/tests/unit/route.mapper.spec.ts index 0846a7b..7492a85 100644 --- a/src/modules/geography/tests/unit/route.mapper.spec.ts +++ b/src/modules/geography/tests/unit/route.mapper.spec.ts @@ -16,14 +16,6 @@ describe('Route Mapper', () => { expect(routeMapper).toBeDefined(); }); - it('should map domain entity to persistence data', async () => { - expect(routeMapper.toPersistence()).toBeUndefined(); - }); - - it('should map persisted data to domain entity', async () => { - expect(routeMapper.toDomain()).toBeUndefined(); - }); - it('should map domain entity to response', async () => { const now = new Date(); const routeEntity: RouteEntity = new RouteEntity({ diff --git a/src/modules/matcher/interface/grpc-controllers/dtos/validators/has-valid-position-indexes.validator.ts b/src/modules/matcher/interface/grpc-controllers/dtos/validators/has-valid-position-indexes.validator.ts index 04504a8..302de1c 100644 --- a/src/modules/matcher/interface/grpc-controllers/dtos/validators/has-valid-position-indexes.validator.ts +++ b/src/modules/matcher/interface/grpc-controllers/dtos/validators/has-valid-position-indexes.validator.ts @@ -1,17 +1,11 @@ import { WaypointDto } from '../waypoint.dto'; export const hasValidPositionIndexes = (waypoints: WaypointDto[]): boolean => { - if (!waypoints) return false; if (waypoints.length == 0) return false; - if (waypoints.every((waypoint) => waypoint.position === undefined)) - return false; - if (waypoints.every((waypoint) => typeof waypoint.position === 'number')) { - const positions = Array.from(waypoints, (waypoint) => waypoint.position); - positions.sort(); - for (let i = 1; i < positions.length; i++) - if (positions[i] != positions[i - 1] + 1) return false; + const positions = Array.from(waypoints, (waypoint) => waypoint.position); + positions.sort(); + for (let i = 1; i < positions.length; i++) + if (positions[i] != positions[i - 1] + 1) return false; - return true; - } - return false; + return true; }; diff --git a/src/modules/matcher/interface/grpc-controllers/dtos/waypoint.dto.ts b/src/modules/matcher/interface/grpc-controllers/dtos/waypoint.dto.ts index ded5386..1d6ebd6 100644 --- a/src/modules/matcher/interface/grpc-controllers/dtos/waypoint.dto.ts +++ b/src/modules/matcher/interface/grpc-controllers/dtos/waypoint.dto.ts @@ -1,8 +1,7 @@ -import { IsInt, IsOptional } from 'class-validator'; +import { IsInt } from 'class-validator'; import { AddressDto } from './address.dto'; export class WaypointDto extends AddressDto { - @IsOptional() @IsInt() - position?: number; + position: number; } diff --git a/src/modules/matcher/tests/unit/interface/has-valid-position-indexes.validator.spec.ts b/src/modules/matcher/tests/unit/interface/has-valid-position-indexes.validator.spec.ts index a0c0661..aed3a13 100644 --- a/src/modules/matcher/tests/unit/interface/has-valid-position-indexes.validator.spec.ts +++ b/src/modules/matcher/tests/unit/interface/has-valid-position-indexes.validator.spec.ts @@ -1,8 +1,9 @@ import { hasValidPositionIndexes } from '@modules/matcher/interface/grpc-controllers/dtos/validators/has-valid-position-indexes.validator'; import { WaypointDto } from '@modules/matcher/interface/grpc-controllers/dtos/waypoint.dto'; -describe('addresses position validator', () => { +describe('Waypoint position validator', () => { const mockAddress1: WaypointDto = { + position: 0, lon: 48.689445, lat: 6.17651, houseNumber: '5', @@ -12,6 +13,7 @@ describe('addresses position validator', () => { country: 'France', }; const mockAddress2: WaypointDto = { + position: 1, lon: 48.8566, lat: 2.3522, locality: 'Paris', @@ -19,45 +21,15 @@ describe('addresses position validator', () => { country: 'France', }; const mockAddress3: WaypointDto = { + position: 2, lon: 49.2628, lat: 4.0347, locality: 'Reims', - postalCode: '51454', + postalCode: '51000', country: 'France', }; - it('should not validate if no position is defined', () => { - expect( - hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), - ).toBeFalsy(); - }); - it('should not validate if only one position is defined', () => { - mockAddress1.position = 0; - expect( - hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), - ).toBeFalsy(); - }); - it('should not validate if positions are partially defined', () => { - mockAddress1.position = 0; - mockAddress2.position = null; - mockAddress3.position = undefined; - expect( - hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), - ).toBeFalsy(); - }); - - it('should not validate if multiple positions have same value', () => { - mockAddress1.position = 0; - mockAddress2.position = 1; - mockAddress3.position = 1; - expect( - hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), - ).toBeFalsy(); - }); - it('should validate if all positions are ordered', () => { - mockAddress1.position = 0; - mockAddress2.position = 1; - mockAddress3.position = 2; + it('should validate if positions are ordered', () => { expect( hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), ).toBeTruthy(); @@ -68,8 +40,23 @@ describe('addresses position validator', () => { hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), ).toBeTruthy(); }); + it('should not validate if positions are not valid', () => { + mockAddress1.position = 0; + mockAddress2.position = 2; + mockAddress3.position = 3; + expect( + hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), + ).toBeFalsy(); + }); + it('should not validate if multiple positions have same value', () => { + mockAddress1.position = 0; + mockAddress2.position = 1; + mockAddress3.position = 1; + expect( + hasValidPositionIndexes([mockAddress1, mockAddress2, mockAddress3]), + ).toBeFalsy(); + }); it('should not validate if no waypoints are defined', () => { - expect(hasValidPositionIndexes(undefined)).toBeFalsy(); expect(hasValidPositionIndexes([])).toBeFalsy(); }); }); diff --git a/tsconfig.json b/tsconfig.json index ed12947..e077152 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,7 +12,7 @@ "baseUrl": "./", "incremental": true, "skipLibCheck": true, - "strictNullChecks": false, + "strictNullChecks": true, "noImplicitAny": false, "strictBindCallApply": false, "forceConsistentCasingInFileNames": false,