diff --git a/src/gtfs/__tests__/transfers.test.ts b/src/gtfs/__tests__/transfers.test.ts index ac0e88a..2997242 100644 --- a/src/gtfs/__tests__/transfers.test.ts +++ b/src/gtfs/__tests__/transfers.test.ts @@ -8,9 +8,13 @@ import { Timetable, TransferTypes } from '../../timetable/timetable.js'; import { encode } from '../../timetable/tripStopId.js'; import { GtfsStopsMap } from '../stops.js'; import { + addGeneratedTransfers, + addMissingSiblingTransfers, buildTripTransfers, + ForbiddenTransfersMap, GtfsTripTransfer, parseTransfers, + TransfersMap, } from '../transfers.js'; import { TripsMapping } from '../trips.js'; @@ -96,13 +100,15 @@ describe('GTFS transfers parser', () => { assert.deepEqual(result.tripContinuations, []); }); - it('should ignore impossible transfer types (3 and 5)', async () => { + it('should only retain unscoped impossible transfers as forbidden', async () => { const mockedStream = new Readable(); mockedStream.push( - 'from_stop_id,to_stop_id,transfer_type,min_transfer_time\n', + 'from_stop_id,to_stop_id,from_trip_id,to_trip_id,from_route_id,to_route_id,transfer_type,min_transfer_time\n', ); - mockedStream.push('"1100084","8014440:0:1","3","180"\n'); - mockedStream.push('"1100097","8014447","5","240"\n'); + mockedStream.push('"1100084","8014440:0:1","","","","","3","180"\n'); + mockedStream.push('"8014440:0:1","1100084","trip-a","","","","3","180"\n'); + mockedStream.push('"1100097","8014447","","","","route-b","3","240"\n'); + mockedStream.push('"1100097","8014447","","","","","5","240"\n'); mockedStream.push(null); const stopsMap: GtfsStopsMap = new Map([ @@ -151,6 +157,7 @@ describe('GTFS transfers parser', () => { const result = await parseTransfers(mockedStream, stopsMap, new Set()); assert.deepEqual(result.transfers, new Map()); + assert.deepEqual(result.forbiddenTransfers, new Map([[0, new Set([1])]])); assert.deepEqual(result.tripContinuations, []); }); @@ -881,6 +888,159 @@ describe('GTFS transfers parser', () => { }); }); +describe('generated transfers', () => { + const stopsMap: GtfsStopsMap = new Map([ + [ + 'station', + { + id: 0, + sourceStopId: 'station', + name: 'Interchange', + children: [1, 2], + locationType: 'STATION', + }, + ], + [ + 'platform-a', + { + id: 1, + sourceStopId: 'platform-a', + name: 'Interchange', + parent: 0, + children: [], + locationType: 'SIMPLE_STOP_OR_PLATFORM', + }, + ], + [ + 'platform-b', + { + id: 2, + sourceStopId: 'platform-b', + name: 'Interchange', + parent: 0, + children: [], + locationType: 'SIMPLE_STOP_OR_PLATFORM', + }, + ], + ]); + + it('adds directed fallback transfers between active sibling platforms', () => { + const transfers: TransfersMap = new Map(); + + const added = addMissingSiblingTransfers( + stopsMap, + new Set([1, 2]), + transfers, + ); + + assert.strictEqual(added, 2); + assert.deepStrictEqual( + transfers, + new Map([ + [ + 1, + [ + { + destination: 2, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + }, + ], + ], + [ + 2, + [ + { + destination: 1, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + }, + ], + ], + ]), + ); + }); + + it('preserves explicit transfers and excludes forbidden sibling directions', () => { + const transfers: TransfersMap = new Map([ + [ + 1, + [ + { + destination: 2, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + minTransferTime: 7, + }, + ], + ], + ]); + const forbiddenTransfers: ForbiddenTransfersMap = new Map([ + [2, new Set([1])], + ]); + + const added = addMissingSiblingTransfers( + stopsMap, + new Set([1, 2]), + transfers, + forbiddenTransfers, + ); + + assert.strictEqual(added, 0); + assert.deepStrictEqual(transfers.get(1), [ + { + destination: 2, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + minTransferTime: 7, + }, + ]); + assert.strictEqual(transfers.has(2), false); + }); + + it('excludes forbidden directions from generated transfers', () => { + const transfers: TransfersMap = new Map(); + const generatedTransfers: TransfersMap = new Map([ + [ + 1, + [ + { + destination: 2, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + minTransferTime: 3, + }, + ], + ], + [ + 2, + [ + { + destination: 1, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + minTransferTime: 3, + }, + ], + ], + ]); + const forbiddenTransfers: ForbiddenTransfersMap = new Map([ + [1, new Set([2])], + ]); + + const added = addGeneratedTransfers( + generatedTransfers, + new Set([1, 2]), + transfers, + forbiddenTransfers, + ); + + assert.strictEqual(added, 1); + assert.strictEqual(transfers.has(1), false); + assert.deepStrictEqual(transfers.get(2), [ + { + destination: 1, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + minTransferTime: 3, + }, + ]); + }); +}); + describe('buildTripTransfers', () => { it('should build trip transfers for valid data', () => { const tripsMapping: TripsMapping = new Map([ diff --git a/src/gtfs/parser.ts b/src/gtfs/parser.ts index 95d530b..095378c 100644 --- a/src/gtfs/parser.ts +++ b/src/gtfs/parser.ts @@ -6,14 +6,16 @@ import { StopId } from '../stops/stops.js'; import { StopsIndex } from '../stops/stopsIndex.js'; import { RouteType, Timetable } from '../timetable/timetable.js'; import { TransferGenerator } from '../transfers/generator.js'; -import { getOrInsert } from '../utils/map.js'; import { FrequenciesMap, parseFrequencies } from './frequencies.js'; import { standardGtfsProfile } from './profiles/standard.js'; import { indexRoutes, parseRoutes } from './routes.js'; import { parseCalendar, parseCalendarDates, ServiceIds } from './services.js'; import { parseStops } from './stops.js'; import { + addGeneratedTransfers, + addMissingSiblingTransfers, buildTripTransfers, + ForbiddenTransfersMap, GtfsTripTransfer, parseTransfers, TransfersMap, @@ -36,6 +38,12 @@ const TRANSFERS_FILE = 'transfers.txt'; export type GtfsProfile = { routeTypeParser: (routeType: number) => Maybe; + /** + * Derive fallback transfers between active stops that share a parent station. + * + * @default true + */ + deriveSiblingTransfers?: boolean; }; export class GtfsParser { @@ -123,6 +131,7 @@ export class GtfsParser { ); let transfers: TransfersMap = new Map(); + let forbiddenTransfers: ForbiddenTransfersMap = new Map(); let tripContinuationsList: GtfsTripTransfer[] = []; let guaranteedTripTransfersList: GtfsTripTransfer[] = []; if (entries[TRANSFERS_FILE]) { @@ -131,10 +140,12 @@ export class GtfsParser { const transfersStream = await zip.stream(TRANSFERS_FILE); const { transfers: parsedTransfers, + forbiddenTransfers: parsedForbiddenTransfers, tripContinuations: parsedTripContinuations, guaranteedTripTransfers: parsedGuaranteedTripTransfers, } = await parseTransfers(transfersStream, parsedStops, activeServiceIds); transfers = parsedTransfers; + forbiddenTransfers = parsedForbiddenTransfers; tripContinuationsList = parsedTripContinuations; guaranteedTripTransfersList = parsedGuaranteedTripTransfers; const transfersEnd = performance.now(); @@ -174,6 +185,20 @@ export class GtfsParser { `${routes.length} valid unique routes. (${(stopTimesEnd - stopTimesStart).toFixed(2)}ms)`, ); + if (this.profile.deriveSiblingTransfers !== false) { + const siblingTransfersStart = performance.now(); + const siblingTransfersAdded = addMissingSiblingTransfers( + parsedStops, + activeStopIds, + transfers, + forbiddenTransfers, + ); + const siblingTransfersEnd = performance.now(); + log.info( + `${siblingTransfersAdded} sibling transfers added. (${(siblingTransfersEnd - siblingTransfersStart).toFixed(2)}ms)`, + ); + } + if (this.transferGenerator) { log.info('Generating virtual transfers'); const virtualTransfersStart = performance.now(); @@ -190,24 +215,12 @@ export class GtfsParser { originStops, stopsIndex, ); - let addedTransfers = 0; - for (const [fromStop, newTransfers] of generatedTransfers) { - const existing = getOrInsert(transfers, fromStop, []); - // Deduplicate per directed pair against existing (feed) transfers, and - // only keep transfers into stops a route actually calls at. - const connected = new Set(existing.map((t) => t.destination)); - for (const transfer of newTransfers) { - if ( - !activeStopIds.has(transfer.destination) || - connected.has(transfer.destination) - ) { - continue; - } - connected.add(transfer.destination); - existing.push(transfer); - addedTransfers += 1; - } - } + const addedTransfers = addGeneratedTransfers( + generatedTransfers, + activeStopIds, + transfers, + forbiddenTransfers, + ); const virtualTransfersEnd = performance.now(); log.info( `${addedTransfers} virtual transfers added. (${(virtualTransfersEnd - virtualTransfersStart).toFixed(2)}ms)`, diff --git a/src/gtfs/profiles/__tests__/extended.test.ts b/src/gtfs/profiles/__tests__/extended.test.ts index ae89509..e04e1eb 100644 --- a/src/gtfs/profiles/__tests__/extended.test.ts +++ b/src/gtfs/profiles/__tests__/extended.test.ts @@ -5,6 +5,10 @@ import { RouteTypes } from '../../../timetable/timetable.js'; import { extendedGtfsProfile } from '../extended.js'; describe('The extended GTFS feed parser', () => { + it('derives sibling transfers by default', () => { + assert.strictEqual(extendedGtfsProfile.deriveSiblingTransfers, true); + }); + it('should convert the extended route type to GTFS route type', () => { assert.ok(extendedGtfsProfile.routeTypeParser); assert.equal(extendedGtfsProfile.routeTypeParser(106), RouteTypes.RAIL); diff --git a/src/gtfs/profiles/__tests__/standard.test.ts b/src/gtfs/profiles/__tests__/standard.test.ts new file mode 100644 index 0000000..d0bb1f7 --- /dev/null +++ b/src/gtfs/profiles/__tests__/standard.test.ts @@ -0,0 +1,10 @@ +import assert from 'node:assert'; +import { describe, it } from 'node:test'; + +import { standardGtfsProfile } from '../standard.js'; + +describe('The standard GTFS feed parser', () => { + it('derives sibling transfers by default', () => { + assert.strictEqual(standardGtfsProfile.deriveSiblingTransfers, true); + }); +}); diff --git a/src/gtfs/profiles/extended.ts b/src/gtfs/profiles/extended.ts index 39b16bf..e82cc58 100644 --- a/src/gtfs/profiles/extended.ts +++ b/src/gtfs/profiles/extended.ts @@ -142,5 +142,6 @@ const routeTypeParser = (routeType: number): Maybe => { }; export const extendedGtfsProfile: GtfsProfile = { + deriveSiblingTransfers: true, routeTypeParser, }; diff --git a/src/gtfs/profiles/standard.ts b/src/gtfs/profiles/standard.ts index e0dec2c..6a3b583 100644 --- a/src/gtfs/profiles/standard.ts +++ b/src/gtfs/profiles/standard.ts @@ -2,6 +2,7 @@ import { RouteTypes } from '../../timetable/timetable.js'; import { GtfsProfile } from '../parser.js'; export const standardGtfsProfile: GtfsProfile = { + deriveSiblingTransfers: true, routeTypeParser: (routeType: number) => { switch (routeType) { case 0: diff --git a/src/gtfs/transfers.ts b/src/gtfs/transfers.ts index 56217fe..4745903 100644 --- a/src/gtfs/transfers.ts +++ b/src/gtfs/transfers.ts @@ -28,6 +28,15 @@ export type GtfsTransferType = export type TransfersMap = Map; +/** + * Directed stop pairs for which the feed explicitly disallows transfers. + * + * These constraints are retained during parsing so generated transfers do not + * accidentally re-enable a connection declared with GTFS transfer_type=3. + * They are not serialized into the routing timetable. + */ +export type ForbiddenTransfersMap = Map>; + export type GtfsTripTransfer = { fromStop: StopId; fromTrip: GtfsTripId; @@ -193,10 +202,12 @@ export const parseTransfers = async ( activeServiceIds: ServiceIds, ): Promise<{ transfers: TransfersMap; + forbiddenTransfers: ForbiddenTransfersMap; tripContinuations: GtfsTripTransfer[]; guaranteedTripTransfers: GtfsTripTransfer[]; }> => { const transfers: TransfersMap = new Map(); + const forbiddenTransfers: ForbiddenTransfersMap = new Map(); const tripContinuations: GtfsTripTransfer[] = []; const guaranteedTripTransfers: GtfsTripTransfer[] = []; @@ -213,10 +224,9 @@ export const parseTransfers = async ( continue; } - if ( - transferEntry.transfer_type === 3 || - transferEntry.transfer_type === 5 - ) { + // Type 5 only prevents remaining seated between two specific trips. It + // does not prohibit an ordinary stop-to-stop transfer. + if (transferEntry.transfer_type === 5) { continue; } @@ -267,6 +277,24 @@ export const parseTransfers = async ( ); } break; + case 3: { + if ( + transferEntry.from_trip_id || + transferEntry.to_trip_id || + transferEntry.from_route_id || + transferEntry.to_route_id + ) { + log.warn( + `Unsupported transfer of type 3 with trip or route constraints: from_trip_id=${transferEntry.from_trip_id}, to_trip_id=${transferEntry.to_trip_id}, from_route_id=${transferEntry.from_route_id}, to_route_id=${transferEntry.to_route_id}.`, + ); + break; + } + const forbiddenDestinations = + forbiddenTransfers.get(fromStop.id) ?? new Set(); + forbiddenDestinations.add(toStop.id); + forbiddenTransfers.set(fromStop.id, forbiddenDestinations); + break; + } case 0: // Recommended transfer case 2: // Requires minimal time default: @@ -282,11 +310,113 @@ export const parseTransfers = async ( return { transfers, + forbiddenTransfers, tripContinuations, guaranteedTripTransfers, }; }; +/** + * Adds missing directed transfers between route-served child stops belonging + * to the same parent station. + * + * GTFS feeds commonly use parent_station to group platforms while omitting the + * corresponding transfers.txt rows. The generated transfers intentionally do + * not carry a fixed duration: routing applies the query's fallback minimum + * transfer time. Explicit and forbidden feed entries always take precedence. + * + * @returns The number of directed sibling transfers added. + */ +export const addMissingSiblingTransfers = ( + stopsMap: GtfsStopsMap, + activeStops: ReadonlySet, + transfers: TransfersMap, + forbiddenTransfers: ForbiddenTransfersMap = new Map(), +): number => { + let addedTransfers = 0; + + for (const parent of stopsMap.values()) { + const activeChildren = parent.children.filter((child) => + activeStops.has(child), + ); + if (activeChildren.length < 2) continue; + + for (const fromStop of activeChildren) { + const existing = transfers.get(fromStop) ?? []; + const connected = new Set( + existing.map((transfer) => transfer.destination), + ); + const forbidden = forbiddenTransfers.get(fromStop); + + for (const toStop of activeChildren) { + if ( + toStop === fromStop || + connected.has(toStop) || + forbidden?.has(toStop) + ) { + continue; + } + existing.push({ + destination: toStop, + type: TransferTypes.REQUIRES_MINIMAL_TIME, + }); + connected.add(toStop); + addedTransfers += 1; + } + + if (existing.length > 0) { + transfers.set(fromStop, existing); + } + } + } + + return addedTransfers; +}; + +/** + * Merges generated transfers into parsed feed transfers. + * + * Only transfers between active stops are kept. Explicit feed transfers and + * forbidden directed pairs take precedence over generated candidates. + * + * @returns The number of generated transfers added. + */ +export const addGeneratedTransfers = ( + generatedTransfers: ReadonlyMap, + activeStops: ReadonlySet, + transfers: TransfersMap, + forbiddenTransfers: ForbiddenTransfersMap = new Map(), +): number => { + let addedTransfers = 0; + + for (const [fromStop, candidates] of generatedTransfers) { + if (!activeStops.has(fromStop)) continue; + + const existing = transfers.get(fromStop) ?? []; + const connected = new Set(existing.map((transfer) => transfer.destination)); + const forbidden = forbiddenTransfers.get(fromStop); + + for (const transfer of candidates) { + if ( + !activeStops.has(transfer.destination) || + connected.has(transfer.destination) || + forbidden?.has(transfer.destination) + ) { + continue; + } + connected.add(transfer.destination); + existing.push(transfer); + addedTransfers += 1; + } + + if (existing.length > 0) { + transfers.set(fromStop, existing); + } + } + + return addedTransfers; +}; + /** * Disambiguates stops involved in a transfer. *