From 17acaa449cd677c3962b2ddcc1447135c436472e Mon Sep 17 00:00:00 2001 From: sbriat Date: Mon, 22 May 2023 11:25:09 +0200 Subject: [PATCH] tests and refactor for ad geography --- .../ad/domain/dtos/create-ad.request.ts | 8 +- src/modules/ad/domain/entities/geography.ts | 94 ++++++------ .../ad/domain/usecases/create-ad.usecase.ts | 25 ++-- .../ad/tests/unit/domain/geography.spec.ts | 138 ++++++++++++++++++ .../tests/unit/domain/time-converter.spec.ts | 10 +- .../secondaries/prisma-repository.abstract.ts | 1 - .../secondaries/postgres-direction-encoder.ts | 4 +- .../{coordinates.ts => coordinate.ts} | 2 +- .../domain/entities/spacetime-point.ts | 8 +- .../interfaces/direction-encoder.interface.ts | 4 +- .../geography/domain/types/point.type.ts | 4 +- .../geography/tests/unit/coordinate.spec.ts | 8 + .../unit/postgres-direction-encoder.spec.ts | 30 ++++ .../entities/ecosystem/spacetime-point.ts | 8 +- 14 files changed, 264 insertions(+), 80 deletions(-) create mode 100644 src/modules/ad/tests/unit/domain/geography.spec.ts rename src/modules/geography/domain/entities/{coordinates.ts => coordinate.ts} (92%) create mode 100644 src/modules/geography/tests/unit/coordinate.spec.ts create mode 100644 src/modules/geography/tests/unit/postgres-direction-encoder.spec.ts diff --git a/src/modules/ad/domain/dtos/create-ad.request.ts b/src/modules/ad/domain/dtos/create-ad.request.ts index be87f29..58a1bb0 100644 --- a/src/modules/ad/domain/dtos/create-ad.request.ts +++ b/src/modules/ad/domain/dtos/create-ad.request.ts @@ -12,7 +12,7 @@ import { IsString, } from 'class-validator'; import { Frequency } from '../types/frequency.enum'; -import { Coordinates } from '../../../geography/domain/entities/coordinates'; +import { Coordinate } from '../../../geography/domain/entities/coordinate'; import { Type } from 'class-transformer'; import { HasTruthyWith } from './has-truthy-with.validator'; @@ -115,11 +115,11 @@ export class CreateAdRequest { @AutoMap() sunMargin: number; - @Type(() => Coordinates) + @Type(() => Coordinate) @IsArray() @ArrayMinSize(2) - @AutoMap(() => [Coordinates]) - waypoints: Coordinates[]; + @AutoMap(() => [Coordinate]) + waypoints: Coordinate[]; @IsNumber() @AutoMap() diff --git a/src/modules/ad/domain/entities/geography.ts b/src/modules/ad/domain/entities/geography.ts index fb0d06f..b720399 100644 --- a/src/modules/ad/domain/entities/geography.ts +++ b/src/modules/ad/domain/entities/geography.ts @@ -1,24 +1,17 @@ -import { Coordinates } from '../../../geography/domain/entities/coordinates'; +import { Coordinate } from '../../../geography/domain/entities/coordinate'; import { Route } from '../../../geography/domain/entities/route'; -import { IFindTimezone } from '../../../geography/domain/interfaces/timezone-finder.interface'; import { Role } from '../types/role.enum'; import { IGeorouter } from '../../../geography/domain/interfaces/georouter.interface'; import { Path } from '../../../geography/domain/types/path.type'; -import { Timezoner } from '../../../geography/domain/types/timezoner'; import { GeorouterSettings } from '../../../geography/domain/types/georouter-settings.type'; export class Geography { - private points: Coordinates[]; - timezones: string[]; + private coordinates: Coordinate[]; driverRoute: Route; passengerRoute: Route; - timezoneFinder: IFindTimezone; - constructor(points: Coordinates[], timezoner: Timezoner) { - this.points = points; - this.timezones = [timezoner.timezone]; - this.timezoneFinder = timezoner.finder; - this.setTimezones(); + constructor(coordinates: Coordinate[]) { + this.coordinates = coordinates; } createRoutes = async ( @@ -26,39 +19,7 @@ export class Geography { georouter: IGeorouter, settings: GeorouterSettings, ): Promise => { - const paths: Path[] = []; - if (roles.includes(Role.DRIVER) && roles.includes(Role.PASSENGER)) { - if (this.points.length == 2) { - // 2 points => same route for driver and passenger - const commonPath: Path = { - key: RouteKey.COMMON, - points: this.points, - }; - paths.push(commonPath); - } else { - const driverPath: Path = { - key: RouteKey.DRIVER, - points: this.points, - }; - const passengerPath: Path = { - key: RouteKey.PASSENGER, - points: [this.points[0], this.points[this.points.length - 1]], - }; - paths.push(driverPath, passengerPath); - } - } else if (roles.includes(Role.DRIVER)) { - const driverPath: Path = { - key: RouteKey.DRIVER, - points: this.points, - }; - paths.push(driverPath); - } else if (roles.includes(Role.PASSENGER)) { - const passengerPath: Path = { - key: RouteKey.PASSENGER, - points: [this.points[0], this.points[this.points.length - 1]], - }; - paths.push(passengerPath); - } + const paths: Path[] = this.getPaths(roles); const routes = await georouter.route(paths, settings); if (routes.some((route) => route.key == RouteKey.COMMON)) { this.driverRoute = routes.find( @@ -81,11 +42,46 @@ export class Geography { } }; - private setTimezones = (): void => { - this.timezones = this.timezoneFinder.timezones( - this.points[0].lat, - this.points[0].lon, - ); + private getPaths = (roles: Role[]): Path[] => { + const paths: Path[] = []; + if (roles.includes(Role.DRIVER) && roles.includes(Role.PASSENGER)) { + if (this.coordinates.length == 2) { + // 2 points => same route for driver and passenger + const commonPath: Path = { + key: RouteKey.COMMON, + points: this.coordinates, + }; + paths.push(commonPath); + } else { + const driverPath: Path = this.createDriverPath(); + const passengerPath: Path = this.createPassengerPath(); + paths.push(driverPath, passengerPath); + } + } else if (roles.includes(Role.DRIVER)) { + const driverPath: Path = this.createDriverPath(); + paths.push(driverPath); + } else if (roles.includes(Role.PASSENGER)) { + const passengerPath: Path = this.createPassengerPath(); + paths.push(passengerPath); + } + return paths; + }; + + private createDriverPath = (): Path => { + return { + key: RouteKey.DRIVER, + points: this.coordinates, + }; + }; + + private createPassengerPath = (): Path => { + return { + key: RouteKey.PASSENGER, + points: [ + this.coordinates[0], + this.coordinates[this.coordinates.length - 1], + ], + }; }; } diff --git a/src/modules/ad/domain/usecases/create-ad.usecase.ts b/src/modules/ad/domain/usecases/create-ad.usecase.ts index 8621933..6fb9d85 100644 --- a/src/modules/ad/domain/usecases/create-ad.usecase.ts +++ b/src/modules/ad/domain/usecases/create-ad.usecase.ts @@ -15,6 +15,7 @@ import { Role } from '../types/role.enum'; import { Geography } from '../entities/geography'; import { IEncodeDirection } from '../../../geography/domain/interfaces/direction-encoder.interface'; import { TimeConverter } from '../entities/time-converter'; +import { Coordinate } from 'src/modules/geography/domain/entities/coordinate'; @CommandHandler(CreateAdCommand) export class CreateAdUseCase { @@ -38,7 +39,6 @@ export class CreateAdUseCase { private readonly directionEncoder: IEncodeDirection, ) { this.defaultParams = defaultParamsProvider.getParams(); - this.timezone = this.defaultParams.DEFAULT_TIMEZONE; this.georouter = georouterCreator.create( this.defaultParams.GEOROUTER_TYPE, this.defaultParams.GEOROUTER_URL, @@ -48,8 +48,9 @@ export class CreateAdUseCase { async execute(command: CreateAdCommand): Promise { try { this.ad = this.mapper.map(command.createAdRequest, CreateAdRequest, Ad); + this.setTimezone(command.createAdRequest.waypoints); + this.setGeography(command.createAdRequest.waypoints); this.setRoles(command.createAdRequest); - this.setGeography(command.createAdRequest); await this.geography.createRoutes(this.roles, this.georouter, { withDistance: false, withPoints: true, @@ -112,18 +113,24 @@ export class CreateAdUseCase { } } + private setTimezone = (coordinates: Coordinate[]): void => { + this.timezone = this.defaultParams.DEFAULT_TIMEZONE; + try { + const timezones = this.timezoneFinder.timezones( + coordinates[0].lat, + coordinates[0].lon, + ); + if (timezones.length > 0) this.timezone = timezones[0]; + } catch (e) {} + }; + private setRoles = (createAdRequest: CreateAdRequest): void => { this.roles = []; if (createAdRequest.driver) this.roles.push(Role.DRIVER); if (createAdRequest.passenger) this.roles.push(Role.PASSENGER); }; - private setGeography = (createAdRequest: CreateAdRequest): void => { - this.geography = new Geography(createAdRequest.waypoints, { - timezone: this.defaultParams.DEFAULT_TIMEZONE, - finder: this.timezoneFinder, - }); - if (this.geography.timezones.length > 0) - this.timezone = this.geography.timezones[0]; + private setGeography = (coordinates: Coordinate[]): void => { + this.geography = new Geography(coordinates); }; } diff --git a/src/modules/ad/tests/unit/domain/geography.spec.ts b/src/modules/ad/tests/unit/domain/geography.spec.ts new file mode 100644 index 0000000..bf3de17 --- /dev/null +++ b/src/modules/ad/tests/unit/domain/geography.spec.ts @@ -0,0 +1,138 @@ +import { Role } from '../../../domain/types/role.enum'; +import { Geography } from '../../../domain/entities/geography'; +import { Coordinate } from '../../../../geography/domain/entities/coordinate'; +import { IGeorouter } from '../../../../geography/domain/interfaces/georouter.interface'; +import { GeorouterSettings } from '../../../../geography/domain/types/georouter-settings.type'; +import { Route } from '../../../../geography/domain/entities/route'; +import { IGeodesic } from '../../../../geography/domain/interfaces/geodesic.interface'; + +const simpleCoordinates: Coordinate[] = [ + { + lon: 6, + lat: 47, + }, + { + lon: 6.1, + lat: 47.1, + }, +]; + +const complexCoordinates: Coordinate[] = [ + { + lon: 6, + lat: 47, + }, + { + lon: 6.1, + lat: 47.1, + }, + { + lon: 6.2, + lat: 47.2, + }, +]; + +const mockGeodesic: IGeodesic = { + inverse: jest.fn(), +}; + +const driverRoute: Route = new Route(mockGeodesic); +driverRoute.distance = 25000; + +const commonRoute: Route = new Route(mockGeodesic); +commonRoute.distance = 20000; + +const mockGeorouter: IGeorouter = { + route: jest + .fn() + .mockResolvedValueOnce([ + { + key: 'driver', + route: driverRoute, + }, + ]) + .mockResolvedValueOnce([ + { + key: 'passenger', + route: commonRoute, + }, + ]) + .mockResolvedValueOnce([ + { + key: 'common', + route: commonRoute, + }, + ]) + .mockResolvedValueOnce([ + { + key: 'driver', + route: driverRoute, + }, + { + key: 'passenger', + route: commonRoute, + }, + ]), +}; + +const georouterSettings: GeorouterSettings = { + withDistance: false, + withPoints: true, + withTime: false, +}; + +describe('Geography entity', () => { + it('should be defined', () => { + expect(new Geography(simpleCoordinates)).toBeDefined(); + }); + + it('should create a route as driver', async () => { + const geography = new Geography(complexCoordinates); + await geography.createRoutes( + [Role.DRIVER], + mockGeorouter, + georouterSettings, + ); + expect(geography.driverRoute).toBeDefined(); + expect(geography.passengerRoute).toBeUndefined(); + expect(geography.driverRoute.distance).toBe(25000); + }); + + it('should create a route as passenger', async () => { + const geography = new Geography(simpleCoordinates); + await geography.createRoutes( + [Role.PASSENGER], + mockGeorouter, + georouterSettings, + ); + expect(geography.driverRoute).toBeUndefined(); + expect(geography.passengerRoute).toBeDefined(); + expect(geography.passengerRoute.distance).toBe(20000); + }); + + it('should create routes as driver and passenger with simple coordinates', async () => { + const geography = new Geography(simpleCoordinates); + await geography.createRoutes( + [Role.DRIVER, Role.PASSENGER], + mockGeorouter, + georouterSettings, + ); + expect(geography.driverRoute).toBeDefined(); + expect(geography.passengerRoute).toBeDefined(); + expect(geography.driverRoute.distance).toBe(20000); + expect(geography.passengerRoute.distance).toBe(20000); + }); + + it('should create routes as driver and passenger with complex coordinates', async () => { + const geography = new Geography(complexCoordinates); + await geography.createRoutes( + [Role.DRIVER, Role.PASSENGER], + mockGeorouter, + georouterSettings, + ); + expect(geography.driverRoute).toBeDefined(); + expect(geography.passengerRoute).toBeDefined(); + expect(geography.driverRoute.distance).toBe(25000); + expect(geography.passengerRoute.distance).toBe(20000); + }); +}); diff --git a/src/modules/ad/tests/unit/domain/time-converter.spec.ts b/src/modules/ad/tests/unit/domain/time-converter.spec.ts index 00138e4..9c4113d 100644 --- a/src/modules/ad/tests/unit/domain/time-converter.spec.ts +++ b/src/modules/ad/tests/unit/domain/time-converter.spec.ts @@ -15,7 +15,7 @@ describe('TimeConverter', () => { ).toBe(6); }); - it('should return undefined when trying to convert a Europe/Paris datetime to utc datetime without a valid date, time or timezone', () => { + it('should return undefined when trying to convert a Europe/Paris datetime to utc datetime without a valid date', () => { expect( TimeConverter.toUtcDatetime(undefined, '07:00', 'Europe/Paris'), ).toBeUndefined(); @@ -26,6 +26,9 @@ describe('TimeConverter', () => { 'Europe/Paris', ), ).toBeUndefined(); + }); + + it('should return undefined when trying to convert a Europe/Paris datetime to utc datetime without a valid time', () => { expect( TimeConverter.toUtcDatetime( new Date('2023-05-01'), @@ -36,9 +39,12 @@ describe('TimeConverter', () => { expect( TimeConverter.toUtcDatetime(new Date('2023-05-01'), 'a', 'Europe/Paris'), ).toBeUndefined(); + }); + + it('should return undefined when trying to convert a datetime to utc datetime without a valid timezone', () => { expect( TimeConverter.toUtcDatetime( - new Date('2023-13-01'), + new Date('2023-12-01'), '07:00', 'OlympusMons/Mars', ), diff --git a/src/modules/database/adapters/secondaries/prisma-repository.abstract.ts b/src/modules/database/adapters/secondaries/prisma-repository.abstract.ts index be96d47..635e966 100644 --- a/src/modules/database/adapters/secondaries/prisma-repository.abstract.ts +++ b/src/modules/database/adapters/secondaries/prisma-repository.abstract.ts @@ -205,7 +205,6 @@ export abstract class PrismaRepository implements IRepository { const command = `INSERT INTO ${this.model} ("${Object.keys(fields).join( '","', )}") VALUES (${Object.values(fields).join(',')})`; - console.log(command); return await this._prisma.$executeRawUnsafe(command); } catch (e) { if (e instanceof Prisma.PrismaClientKnownRequestError) { diff --git a/src/modules/geography/adapters/secondaries/postgres-direction-encoder.ts b/src/modules/geography/adapters/secondaries/postgres-direction-encoder.ts index 8e249c0..3a85ace 100644 --- a/src/modules/geography/adapters/secondaries/postgres-direction-encoder.ts +++ b/src/modules/geography/adapters/secondaries/postgres-direction-encoder.ts @@ -1,8 +1,8 @@ -import { Coordinates } from '../../domain/entities/coordinates'; +import { Coordinate } from '../../domain/entities/coordinate'; import { IEncodeDirection } from '../../domain/interfaces/direction-encoder.interface'; export class PostgresDirectionEncoder implements IEncodeDirection { - encode = (coordinates: Coordinates[]): string => + encode = (coordinates: Coordinate[]): string => [ "'LINESTRING(", coordinates.map((point) => [point.lon, point.lat].join(' ')).join(), diff --git a/src/modules/geography/domain/entities/coordinates.ts b/src/modules/geography/domain/entities/coordinate.ts similarity index 92% rename from src/modules/geography/domain/entities/coordinates.ts rename to src/modules/geography/domain/entities/coordinate.ts index 05769c2..4dd416a 100644 --- a/src/modules/geography/domain/entities/coordinates.ts +++ b/src/modules/geography/domain/entities/coordinate.ts @@ -1,7 +1,7 @@ import { AutoMap } from '@automapper/classes'; import { IsLatitude, IsLongitude, IsNumber } from 'class-validator'; -export class Coordinates { +export class Coordinate { constructor(lon: number, lat: number) { this.lon = lon; this.lat = lat; diff --git a/src/modules/geography/domain/entities/spacetime-point.ts b/src/modules/geography/domain/entities/spacetime-point.ts index 97f130d..7d720e1 100644 --- a/src/modules/geography/domain/entities/spacetime-point.ts +++ b/src/modules/geography/domain/entities/spacetime-point.ts @@ -1,12 +1,12 @@ -import { Coordinates } from './coordinates'; +import { Coordinate } from './coordinate'; export class SpacetimePoint { - coordinates: Coordinates; + coordinate: Coordinate; duration: number; distance: number; - constructor(coordinates: Coordinates, duration: number, distance: number) { - this.coordinates = coordinates; + constructor(coordinate: Coordinate, duration: number, distance: number) { + this.coordinate = coordinate; this.duration = duration; this.distance = distance; } diff --git a/src/modules/geography/domain/interfaces/direction-encoder.interface.ts b/src/modules/geography/domain/interfaces/direction-encoder.interface.ts index 52a5ce1..0f38a49 100644 --- a/src/modules/geography/domain/interfaces/direction-encoder.interface.ts +++ b/src/modules/geography/domain/interfaces/direction-encoder.interface.ts @@ -1,5 +1,5 @@ -import { Coordinates } from '../entities/coordinates'; +import { Coordinate } from '../entities/coordinate'; export interface IEncodeDirection { - encode(coordinates: Coordinates[]): string; + encode(coordinates: Coordinate[]): string; } diff --git a/src/modules/geography/domain/types/point.type.ts b/src/modules/geography/domain/types/point.type.ts index b3733a8..37c49e6 100644 --- a/src/modules/geography/domain/types/point.type.ts +++ b/src/modules/geography/domain/types/point.type.ts @@ -1,6 +1,6 @@ import { PointType } from './point-type.enum'; -import { Coordinates } from '../entities/coordinates'; +import { Coordinate } from '../entities/coordinate'; -export type Point = Coordinates & { +export type Point = Coordinate & { type?: PointType; }; diff --git a/src/modules/geography/tests/unit/coordinate.spec.ts b/src/modules/geography/tests/unit/coordinate.spec.ts new file mode 100644 index 0000000..6bc92e1 --- /dev/null +++ b/src/modules/geography/tests/unit/coordinate.spec.ts @@ -0,0 +1,8 @@ +import { Coordinate } from '../../domain/entities/coordinate'; + +describe('Coordinate entity', () => { + it('should be defined', () => { + const coordinate: Coordinate = new Coordinate(6, 47); + expect(coordinate).toBeDefined(); + }); +}); diff --git a/src/modules/geography/tests/unit/postgres-direction-encoder.spec.ts b/src/modules/geography/tests/unit/postgres-direction-encoder.spec.ts new file mode 100644 index 0000000..71b8ea3 --- /dev/null +++ b/src/modules/geography/tests/unit/postgres-direction-encoder.spec.ts @@ -0,0 +1,30 @@ +import { PostgresDirectionEncoder } from '../../adapters/secondaries/postgres-direction-encoder'; +import { Coordinate } from '../../domain/entities/coordinate'; + +describe('Postgres direction encoder', () => { + it('should be defined', () => { + const postgresDirectionEncoder: PostgresDirectionEncoder = + new PostgresDirectionEncoder(); + expect(postgresDirectionEncoder).toBeDefined(); + }); + it('should encode coordinates to a postgres direction', () => { + const postgresDirectionEncoder: PostgresDirectionEncoder = + new PostgresDirectionEncoder(); + const coordinates: Coordinate[] = [ + { + lon: 6, + lat: 47, + }, + { + lon: 6.1, + lat: 47.1, + }, + { + lon: 6.2, + lat: 47.2, + }, + ]; + const direction = postgresDirectionEncoder.encode(coordinates); + expect(direction).toBe("'LINESTRING(6 47,6.1 47.1,6.2 47.2)'"); + }); +}); diff --git a/src/modules/matcher/domain/entities/ecosystem/spacetime-point.ts b/src/modules/matcher/domain/entities/ecosystem/spacetime-point.ts index 91dd7dc..8a45b80 100644 --- a/src/modules/matcher/domain/entities/ecosystem/spacetime-point.ts +++ b/src/modules/matcher/domain/entities/ecosystem/spacetime-point.ts @@ -1,12 +1,12 @@ -import { Coordinates } from '../../../../geography/domain/entities/coordinates'; +import { Coordinate } from '../../../../geography/domain/entities/coordinate'; export class SpacetimePoint { - coordinates: Coordinates; + coordinate: Coordinate; duration: number; distance: number; - constructor(coordinates: Coordinates, duration: number, distance: number) { - this.coordinates = coordinates; + constructor(coordinate: Coordinate, duration: number, distance: number) { + this.coordinate = coordinate; this.duration = duration; this.distance = distance; }