From 3c65582d8eaa60f915a849c8ff01a50b231fa247 Mon Sep 17 00:00:00 2001 From: Romain Thouvenin Date: Thu, 18 Apr 2024 17:54:40 +0200 Subject: [PATCH] Improve support for trips that span more than one date - Remove the where clause on schedule times, as it is not possible to compute a sensible driver date within the SQL query. - Simplify the date clause comparisons - Replace the compared date range with one adjusted with the driver duration, to catch ads that depart on a different day --- .../selector/passenger-oriented.selector.ts | 201 ++++++------------ .../ad/core/domain/candidate.entity.ts | 2 +- .../unit/core/match.query-handler.spec.ts | 27 ++- 3 files changed, 84 insertions(+), 146 deletions(-) diff --git a/src/modules/ad/core/application/queries/match/selector/passenger-oriented.selector.ts b/src/modules/ad/core/application/queries/match/selector/passenger-oriented.selector.ts index 226a639..e1caae6 100644 --- a/src/modules/ad/core/application/queries/match/selector/passenger-oriented.selector.ts +++ b/src/modules/ad/core/application/queries/match/selector/passenger-oriented.selector.ts @@ -1,16 +1,19 @@ import { AdEntity } from '@modules/ad/core/domain/ad.entity'; import { Frequency, Role } from '@modules/ad/core/domain/ad.types'; import { CandidateEntity } from '@modules/ad/core/domain/candidate.entity'; -import { ScheduleItem } from '@modules/ad/core/domain/value-objects/schedule-item.value-object'; import { Point } from '../../../types/point.type'; import { Waypoint } from '../../../types/waypoint.type'; import { Selector } from '../algorithm.abstract'; +import { ScheduleItem } from '../match.query'; /** * This class complements the AdRepository prisma service by turning a match query object into a SQL query, - * with the assumption that the query is passenger-oriented (i.e. it is up to the driver to go out of his way to pick up the passenger) - * TODO: Converting the query object into a SQL query is a job for the prisma service / repository implementation, - * any logic related to being passenger-oriented should be in the domain layer + * with the assumption that the query is passenger-oriented (i.e. it is up to the driver to go out of his way to pick up the passenger). + * The idea is to make a rough filter of the ads in DB to limit the number of ads to be processed more precisely by the application code. + * TODO: Converting the query object into a SQL query is a job for the repository implementation + * (or anything behind the repository interface), + * any logic related to being passenger-oriented should be in the domain layer. + * (though it might be difficult to describe generically the search criteria with a query object) */ export class PassengerOrientedSelector extends Selector { select = async (): Promise => { @@ -25,6 +28,7 @@ export class PassengerOrientedSelector extends Selector { query: this._createQueryString(Role.PASSENGER), role: Role.PASSENGER, }); + return ( await Promise.all( queryStringRoles.map>( @@ -140,8 +144,7 @@ export class PassengerOrientedSelector extends Selector { [ this._whereRole(role), this._whereStrict(), - this._whereDate(), - this._whereSchedule(role), + this._whereDate(role), this._whereExcludedAd(), this._whereAzimuth(), this._whereProportion(role), @@ -160,110 +163,71 @@ export class PassengerOrientedSelector extends Selector { : `frequency='${Frequency.RECURRENT}'` : ''; - private _whereDate = (): string => - this.query.frequency == Frequency.PUNCTUAL - ? `("fromDate" <= '${this.query.fromDate}' AND "toDate" >= '${this.query.fromDate}')` - : `(\ - (\ - "fromDate" <= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\ - "toDate" >= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\ - ) OR (\ - "fromDate" >= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\ - "toDate" <= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\ - ) OR (\ - "fromDate" <= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\ - "toDate" <= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\ - ) OR (\ - "fromDate" >= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\ - "toDate" >= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\ - )\ - )`; + /** + * Generates the WHERE clause checking that the date range of the query intersects with the range of the ad. + * Note that driver dates might not be comparable with passenger dates when the trip is by night or very long. + * For this reason, the pickup date is adjusted with the driver duration, + * so as to compare with the maximum / minimum driver date that could make sense for the passenger. + * This may return more ads than necessary, but they will be filtered out in further processing. + */ + private _whereDate = (role: Role): string => { + const maxFromDate = this._maxFromDate(role); + const minToDate = this._minToDate(role); + return `("fromDate" <= ${maxFromDate} AND "toDate" >= ${minToDate})`; + }; - private _whereSchedule = (role: Role): string => { - // no schedule filtering if schedule is not set - if (this.query.schedule === undefined) return ''; - const schedule: string[] = []; - // we need full dates to compare times, because margins can lead to compare on previous or next day - // - first we establish a base calendar (up to a week) - const scheduleDates: Date[] = this._datesBetweenBoundaries( - this.query.fromDate, - this.query.toDate, - ); - // - then we compare each resulting day of the schedule with each day of calendar, - // adding / removing margin depending on the role - scheduleDates.map((date: Date) => { - (this.query.schedule as ScheduleItem[]) - .filter( - (scheduleItem: ScheduleItem) => date.getUTCDay() == scheduleItem.day, - ) - .map((scheduleItem: ScheduleItem) => { - switch (role) { - case Role.PASSENGER: - schedule.push(this._wherePassengerSchedule(date, scheduleItem)); - break; - case Role.DRIVER: - schedule.push(this._whereDriverSchedule(date, scheduleItem)); - break; - } - }); - }); - if (schedule.length > 0) { - return ['(', schedule.join(' OR '), ')'].join(''); + private _maxFromDate = (role: Role): string => { + if (role == Role.DRIVER) { + const querySchedule = this.query.schedule; + // When there is no schedule (search whole day), we consider the driver accepts to depart until 23:59 + const maxScheduleTime = + querySchedule === undefined + ? '23:59' + : querySchedule.reduce( + (max, s) => (s.time > max ? s.time : max), + '00:00', + ); + const [h, m] = maxScheduleTime.split(':'); + const maxFromDate = new Date(this.query.toDate); + maxFromDate.setHours(parseInt(h)); + maxFromDate.setMinutes(parseInt(m)); + maxFromDate.setSeconds(this.query.driverRoute!.duration); + return `'${maxFromDate.getUTCFullYear()}-${maxFromDate.getUTCMonth() + 1}-${maxFromDate.getUTCDate()}'`; + } else { + return `'${this.query.toDate}'`; + } + }; + + private _minToDate = (role: Role): string => { + if (role == Role.PASSENGER) { + const querySchedule = this.query.schedule; + // When there is no schedule (search whole day), we consider the passenger accepts to depart from 00:00 + const minScheduleTime = + querySchedule === undefined + ? '00:00' + : querySchedule.reduce( + (min, s) => (s.time < min ? s.time : min), + '23:59', + ); + const [h, m] = minScheduleTime.split(':'); + const minToDate = new Date(this.query.fromDate); + minToDate.setHours(parseInt(h)); + minToDate.setMinutes(parseInt(m)); + return `(make_timestamp(\ + ${minToDate.getUTCFullYear()},\ + ${minToDate.getUTCMonth() + 1},\ + ${minToDate.getUTCDate()},\ + ${minToDate.getUTCHours()},\ + ${minToDate.getUTCMinutes()},0)\ + - concat("driverDuration", ' second')::interval)::date`; + } else { + return `'${this.query.fromDate}'`; } - return ''; }; private _whereExcludedAd = (): string => this.query.excludedAdId ? `ad.uuid <> '${this.query.excludedAdId}'` : ''; - private _wherePassengerSchedule = ( - date: Date, - scheduleItem: ScheduleItem, - ): string => { - let maxDepartureDatetime: Date = new Date(date); - maxDepartureDatetime.setHours(parseInt(scheduleItem.time.split(':')[0])); - maxDepartureDatetime.setMinutes(parseInt(scheduleItem.time.split(':')[1])); - maxDepartureDatetime = this._addMargin( - maxDepartureDatetime, - scheduleItem.margin as number, - ); - // we want the min departure time of the driver to be before the max departure time of the passenger - return `make_timestamp(\ - ${maxDepartureDatetime.getUTCFullYear()},\ - ${maxDepartureDatetime.getUTCMonth() + 1},\ - ${maxDepartureDatetime.getUTCDate()},\ - CAST(EXTRACT(hour from time) as integer),\ - CAST(EXTRACT(minute from time) as integer),0) - interval '1 second' * margin <=\ - make_timestamp(\ - ${maxDepartureDatetime.getUTCFullYear()},\ - ${maxDepartureDatetime.getUTCMonth() + 1},\ - ${maxDepartureDatetime.getUTCDate()},${maxDepartureDatetime.getUTCHours()},${maxDepartureDatetime.getUTCMinutes()},0)`; - }; - - private _whereDriverSchedule = ( - date: Date, - scheduleItem: ScheduleItem, - ): string => { - let minDepartureDatetime: Date = new Date(date); - minDepartureDatetime.setHours(parseInt(scheduleItem.time.split(':')[0])); - minDepartureDatetime.setMinutes(parseInt(scheduleItem.time.split(':')[1])); - minDepartureDatetime = this._addMargin( - minDepartureDatetime, - -(scheduleItem.margin as number), - ); - // we want the max departure time of the passenger to be after the min departure time of the driver - return `make_timestamp(\ - ${minDepartureDatetime.getUTCFullYear()}, - ${minDepartureDatetime.getUTCMonth() + 1}, - ${minDepartureDatetime.getUTCDate()},\ - CAST(EXTRACT(hour from time) as integer),\ - CAST(EXTRACT(minute from time) as integer),0) + interval '1 second' * margin >=\ - make_timestamp(\ - ${minDepartureDatetime.getUTCFullYear()}, - ${minDepartureDatetime.getUTCMonth() + 1}, - ${minDepartureDatetime.getUTCDate()},${minDepartureDatetime.getUTCHours()},${minDepartureDatetime.getUTCMinutes()},0)`; - }; - private _whereAzimuth = (): string => { if (!this.query.useAzimuth) return ''; const { minAzimuth, maxAzimuth } = this._azimuthRange( @@ -323,37 +287,6 @@ export class PassengerOrientedSelector extends Selector { } }; - /** - * Returns an array of dates containing all the dates (limited to 7 by default) between 2 boundary dates. - * - * The array length can be limited to a _max_ number of dates (default: 7) - */ - private _datesBetweenBoundaries = ( - firstDate: string, - lastDate: string, - max = 7, - ): Date[] => { - const fromDate: Date = new Date(firstDate); - const toDate: Date = new Date(lastDate); - const dates: Date[] = []; - let count = 0; - for ( - let date = fromDate; - date <= toDate; - date.setUTCDate(date.getUTCDate() + 1) - ) { - dates.push(new Date(date)); - count++; - if (count == max) break; - } - return dates; - }; - - private _addMargin = (date: Date, marginInSeconds: number): Date => { - date.setUTCSeconds(marginInSeconds); - return date; - }; - private _azimuthRange = ( azimuth: number, margin: number, diff --git a/src/modules/ad/core/domain/candidate.entity.ts b/src/modules/ad/core/domain/candidate.entity.ts index 26bd990..052ba61 100644 --- a/src/modules/ad/core/domain/candidate.entity.ts +++ b/src/modules/ad/core/domain/candidate.entity.ts @@ -353,7 +353,7 @@ class Schedule extends ValueObject<{ duration, ); acc.push({ - day: itemDate.getUTCDay(), + day: driverStartDatetime.getUTCDay(), margin: scheduleItemProps.margin, time: this._formatTime(driverStartDatetime), }); diff --git a/src/modules/ad/tests/unit/core/match.query-handler.spec.ts b/src/modules/ad/tests/unit/core/match.query-handler.spec.ts index ee60fa3..e6e9eea 100644 --- a/src/modules/ad/tests/unit/core/match.query-handler.spec.ts +++ b/src/modules/ad/tests/unit/core/match.query-handler.spec.ts @@ -1,8 +1,8 @@ import { - Domain, - KeyType, Configurator, + Domain, GetConfigurationRepositoryPort, + KeyType, } from '@mobicoop/configuration-module'; import { CARPOOL_CONFIG_DEPARTURE_TIME_MARGIN, @@ -16,8 +16,9 @@ import { AD_REPOSITORY, INPUT_DATETIME_TRANSFORMER, MATCHING_REPOSITORY, + TIMEZONE_FINDER, + TIME_CONVERTER, } from '@modules/ad/ad.di-tokens'; -import { DateTimeTransformerPort } from '@modules/ad/core/application/ports/datetime-transformer.port'; import { MatchingRepositoryPort } from '@modules/ad/core/application/ports/matching.repository.port'; import { MatchQuery } from '@modules/ad/core/application/queries/match/match.query'; import { @@ -30,6 +31,9 @@ import { Frequency, Role } from '@modules/ad/core/domain/ad.types'; import { Target } from '@modules/ad/core/domain/candidate.types'; import { MatchEntity } from '@modules/ad/core/domain/match.entity'; import { MatchingEntity } from '@modules/ad/core/domain/matching.entity'; +import { InputDateTimeTransformer } from '@modules/ad/infrastructure/input-datetime-transformer'; +import { TimeConverter } from '@modules/ad/infrastructure/time-converter'; +import { TimezoneFinder } from '@modules/ad/infrastructure/timezone-finder'; import { MATCH_CONFIG_ALGORITHM, MATCH_CONFIG_AZIMUTH_MARGIN, @@ -344,13 +348,6 @@ const mockConfigurationRepository: GetConfigurationRepositoryPort = { ), }; -const mockInputDateTimeTransformer: DateTimeTransformerPort = { - fromDate: jest.fn(), - toDate: jest.fn(), - day: jest.fn(), - time: jest.fn(), -}; - const mockRouteProvider = simpleMockGeorouter; describe('Match Query Handler', () => { @@ -372,9 +369,17 @@ describe('Match Query Handler', () => { provide: AD_CONFIGURATION_REPOSITORY, useValue: mockConfigurationRepository, }, + { + provide: TIMEZONE_FINDER, + useClass: TimezoneFinder, + }, + { + provide: TIME_CONVERTER, + useClass: TimeConverter, + }, { provide: INPUT_DATETIME_TRANSFORMER, - useValue: mockInputDateTimeTransformer, + useClass: InputDateTimeTransformer, }, ], }).compile();