refactor(server): inject the transit base URL and the kitinerary paths as config tokens

The ownership rule says a boot-stable value that only Nest providers read
belongs to a registerAs token, yet TransitService froze TRANSIT_API_URL in a
module-top readEnv() const and KitineraryExtractorService read
KITINERARY_EXTRACTOR_PATH and PATH through readEnv() in its probe. Both now
come from tokens: transitConfig and kitineraryConfig, derived in
boot-derive.ts and dropped from deriveIntegrations, so readEnv() no longer
reads those variables at all. TransitModule and KitineraryExtractorModule
import AppConfigModule, which keeps them buildable on their own in tests.

Behaviour is unchanged: the same defaults, the same trailing-slash strip and
PATH split, now frozen per built app instead of at import. The app-config
README names each boot-stable variable that stays on readEnv() together with
the reader the container cannot inject into.
This commit is contained in:
Maurice
2026-10-08 17:11:38 +02:00
parent a6f635f3e0
commit aa8d9b4037
15 changed files with 159 additions and 67 deletions
+2 -1
View File
@@ -51,6 +51,7 @@ import { TodoService } from '../../src/nest/todo/todo.service';
import { TransitMcp } from '../../src/nest/transit/transit.mcp';
import { GoogleTransitProvider } from '../../src/nest/transit/google-transit.provider';
import { TransitService } from '../../src/nest/transit/transit.service';
import { transitConfig } from '../../src/nest/app-config/tokens';
import { FilesService } from '../../src/nest/files/files.service';
import { FilesMcp } from '../../src/nest/files/files.mcp';
import { TripsMcp } from '../../src/nest/trips/trips.mcp';
@@ -491,7 +492,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
// (`findUsernameEmail`, UM11's precedent) added in its place.
usersRepo, authService, addonsService,
),
new TransitMcp(new TransitService(new GoogleTransitProvider(appSettings, usersRepo, noGoogleQuota)), daysService, reservationsService, await createTestTripsRepo(db), authService, guards),
new TransitMcp(new TransitService(new GoogleTransitProvider(appSettings, usersRepo, noGoogleQuota), transitConfig()), daysService, reservationsService, await createTestTripsRepo(db), authService, guards),
new AtlasMcp(new AtlasService(
await createTestBucketListRepo(db), await createTestHiddenCountriesRepo(db),
await createTestHiddenRegionsRepo(db), await createTestVisitedCountriesRepo(db),
+24 -4
View File
@@ -21,7 +21,7 @@ import {
derivePush,
deriveAll,
} from '../../../src/app-config/derive';
import { deriveHttpBoot, deriveStorage } from '../../../src/app-config/boot-derive';
import { deriveHttpBoot, deriveKitinerary, deriveStorage, deriveTransit } from '../../../src/app-config/boot-derive';
// These tests PIN the exact legacy coercions each derived field replaced.
// If one fails after an edit, the edit changed runtime behavior — fix the
@@ -229,10 +229,8 @@ describe('derivePlugins', () => {
});
describe('deriveIntegrations', () => {
it('pins unsplash trim, transit base strip + default, nominatim trim + default, overpass timeout', () => {
it('pins unsplash trim, nominatim trim + default, overpass timeout', () => {
expect(deriveIntegrations({ UNSPLASH_ACCESS_KEY: ' key ' }).unsplashAccessKey).toBe('key');
expect(deriveIntegrations({}).transitApiBase).toBe('https://api.transitous.org');
expect(deriveIntegrations({ TRANSIT_API_URL: 'https://t.example//' }).transitApiBase).toBe('https://t.example');
expect(deriveIntegrations({}).nominatimUrl).toBe('https://nominatim.openstreetmap.org');
// Padded and whitespace-only both come back from a compose file or a ConfigMap,
// and the schema already called the second one unset.
@@ -253,6 +251,28 @@ describe('deriveIntegrations', () => {
});
});
describe('deriveTransit (transitConfig token)', () => {
it('TRANSIT_API_URL: trailing slashes stripped, Transitous unless set', () => {
expect(deriveTransit({}).apiBase).toBe('https://api.transitous.org');
expect(deriveTransit({ TRANSIT_API_URL: 'https://t.example//' }).apiBase).toBe('https://t.example');
});
});
describe('deriveKitinerary (kitineraryConfig token)', () => {
const sep = process.platform === 'win32' ? ';' : ':';
it('KITINERARY_EXTRACTOR_PATH passes through as given, unset stays unset', () => {
expect(deriveKitinerary({}).extractorPath).toBeUndefined();
expect(deriveKitinerary({ KITINERARY_EXTRACTOR_PATH: '/opt/ki' }).extractorPath).toBe('/opt/ki');
});
it('splits PATH (or the Windows Path) on the platform delimiter and drops blank entries', () => {
expect(deriveKitinerary({ PATH: ['/a', ' /b ', '', '/c'].join(sep) }).searchPath).toEqual(['/a', '/b', '/c']);
expect(deriveKitinerary({ Path: ['/w1', '/w2'].join(sep) }).searchPath).toEqual(['/w1', '/w2']);
expect(deriveKitinerary({}).searchPath).toEqual([]);
});
});
describe('deriveFiles (#1364)', () => {
it('defaults the upload limit to 50 MB and takes FILE_UPLOAD_LIMIT_MB', () => {
expect(deriveFiles({}).uploadLimitMb).toBe(50);
@@ -8,11 +8,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
* at all, and every branch that resolves it is a filesystem lookup that behaves
* differently on the three platforms TREK ships to.
*/
const { existsSync, readdirSync, readEnv, execFileSync, execFile, logDebug } = vi.hoisted(() => ({
const { existsSync, readdirSync, execFileSync, execFile, logDebug } = vi.hoisted(() => ({
logDebug: vi.fn(),
existsSync: vi.fn(),
readdirSync: vi.fn(),
readEnv: vi.fn(),
execFileSync: vi.fn(),
// A plain function, because the service promisifies it at module load.
execFile: vi.fn(),
@@ -29,7 +28,6 @@ vi.mock('node:fs', () => ({
// green-or-red depending on whether the developer happens to have KItinerary
// installed.
vi.mock('node:child_process', () => ({ execFileSync, execFile }));
vi.mock('../../../../src/app-config', () => ({ readEnv }));
// The logger reads readEnv().app.logLevel while its module is evaluated, so it
// has to be mocked rather than imported for real.
vi.mock('../../../../src/nest/audit/audit-log.logger', () => ({
@@ -44,9 +42,10 @@ import { KitineraryExtractorService } from '../../../../src/nest/booking-import/
// string would never match.
const onPath = (dir: string) => join(dir, 'kitinerary-extractor');
function boot(env: { kitineraryExtractorPath?: string; searchPath?: string[] } = {}) {
readEnv.mockReturnValue({ integrations: { searchPath: [], ...env } });
const svc = new KitineraryExtractorService();
// What the kitineraryConfig token hands the service: KITINERARY_EXTRACTOR_PATH
// and the split PATH, frozen when the app is built.
function boot(env: { extractorPath?: string; searchPath?: string[] } = {}) {
const svc = new KitineraryExtractorService({ extractorPath: undefined, searchPath: [], ...env });
svc.onModuleInit();
return svc;
}
@@ -61,14 +60,14 @@ beforeEach(() => {
describe('KitineraryExtractorService binary probe', () => {
it('KIT-EXT-001: takes the configured path when it exists', () => {
existsSync.mockImplementation((p: string) => p === '/opt/kitinerary-extractor');
expect(boot({ kitineraryExtractorPath: '/opt/kitinerary-extractor' }).isAvailable()).toBe(true);
expect(boot({ extractorPath: '/opt/kitinerary-extractor' }).isAvailable()).toBe(true);
});
it('KIT-EXT-002: an explicitly configured path that is missing disables the feature outright', () => {
// It deliberately does NOT fall through to the search: somebody who set the
// variable meant that binary, and silently using another one would hide the
// typo behind a working feature.
const svc = boot({ kitineraryExtractorPath: '/nope/kitinerary-extractor' });
const svc = boot({ extractorPath: '/nope/kitinerary-extractor' });
expect(svc.isAvailable()).toBe(false);
expect(readdirSync).not.toHaveBeenCalled();
});
@@ -138,7 +137,7 @@ describe('KitineraryExtractorService diagnostics', () => {
existsSync.mockImplementation((p: string) => p === '/opt/ki');
execFileSync.mockReturnValue(Buffer.from('kitinerary-extractor 6.3.3\n'));
expect(boot({ kitineraryExtractorPath: '/opt/ki' }).describe()).toEqual({
expect(boot({ extractorPath: '/opt/ki' }).describe()).toEqual({
available: true, path: '/opt/ki', version: '6.3.3', configuredPath: '/opt/ki',
});
});
@@ -147,7 +146,7 @@ describe('KitineraryExtractorService diagnostics', () => {
existsSync.mockImplementation((p: string) => p === '/opt/ki');
execFileSync.mockImplementation(() => { throw new Error('nope'); });
const described = boot({ kitineraryExtractorPath: '/opt/ki' }).describe();
const described = boot({ extractorPath: '/opt/ki' }).describe();
expect(described.available).toBe(true);
expect(described.version).toBeNull();
});
@@ -155,7 +154,7 @@ describe('KitineraryExtractorService diagnostics', () => {
it('KIT-EXT-012: a configured path that does not exist is reported as such', () => {
existsSync.mockReturnValue(false);
expect(boot({ kitineraryExtractorPath: '/nope/ki' }).describe()).toEqual({
expect(boot({ extractorPath: '/nope/ki' }).describe()).toEqual({
available: false, path: null, version: null, configuredPath: '/nope/ki',
});
});
@@ -184,7 +183,7 @@ describe('KitineraryExtractorService stderr handling', () => {
execFile.mockImplementation((_bin: string, _args: string[], _opts: unknown, cb: (e: unknown, r: unknown) => void) => {
cb(null, { stdout: '[]', stderr });
});
await boot({ kitineraryExtractorPath: '/opt/ki' }).extract(Buffer.from(''), 'booking.eml');
await boot({ extractorPath: '/opt/ki' }).extract(Buffer.from(''), 'booking.eml');
}
it('KIT-EXT-015: passes every raw line to the debug log, script errors included', async () => {
@@ -16,6 +16,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { GoogleTransitProvider, clearGoogleTransitCache } from '../../../src/nest/transit/google-transit.provider';
import { decodePolyline, encodePolyline } from '../../../src/nest/transit/transit.helpers';
import { TransitService } from '../../../src/nest/transit/transit.service';
import { transitConfig } from '../../../src/nest/app-config/tokens';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
import { noGoogleQuota } from '../../helpers/google-quota';
@@ -193,13 +194,13 @@ describe('activation', () => {
it('GTRANSIT-005: TransitService routes to Google only when it is active', async () => {
fetchMock.mockResolvedValue(okJson(subwayRoute()));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig());
await service.plan({ from: FROM, to: TO });
expect(fetchMock.mock.calls[0][0]).toBe('https://routes.googleapis.com/directions/v2:computeRoutes');
});
it('GTRANSIT-006: TransitService still validates the request before dispatching', async () => {
const service = new TransitService(makeProvider(GOOGLE_SETTINGS));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig());
await expect(service.plan({ from: 'nowhere', to: TO })).rejects.toThrow('from must be "lat,lng"');
await expect(service.plan({ from: FROM, to: TO, modes: 'ROCKET' })).rejects.toThrow('unsupported transit mode');
expect(fetchMock).not.toHaveBeenCalled();
@@ -477,14 +478,14 @@ describe('geocode', () => {
it('GTRANSIT-020: repeat lookups of the same station are answered from cache', async () => {
fetchMock.mockResolvedValue(okJson(places));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig());
await service.geocode('Kyobashi', 'en', undefined, 1);
await service.geocode('Kyobashi', 'en', undefined, 1);
expect(fetchMock).toHaveBeenCalledTimes(1);
});
it('GTRANSIT-021: a short query never reaches Google', async () => {
const service = new TransitService(makeProvider(GOOGLE_SETTINGS));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig());
// The provider is still named: the answer comes from the length guard, not
// from a backend that declined.
expect(await service.geocode('a', 'en', undefined, 1)).toEqual({ results: [], provider: 'google' });
@@ -684,7 +685,7 @@ describe('walk coalescing', () => {
describe('provider reporting', () => {
it('GTRANSIT-036: names Google when Google answered', async () => {
fetchMock.mockResolvedValue(okJson(subwayRoute()));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS));
const service = new TransitService(makeProvider(GOOGLE_SETTINGS), transitConfig());
const planned = await service.plan({ from: FROM, to: TO }, 'en', 1);
expect(planned.provider).toBe('google');
@@ -696,7 +697,7 @@ describe('provider reporting', () => {
fetchMock.mockResolvedValue(okJson({ itineraries: [] }));
// Google selected, but no key resolves — the request goes to Transitous and
// says so, instead of leaving an empty result to be blamed on Google.
const service = new TransitService(makeProvider({ transit_provider: 'google' }));
const service = new TransitService(makeProvider({ transit_provider: 'google' }), transitConfig());
const planned = await service.plan({ from: '48.8583,2.3470', to: '48.8809,2.3553' }, 'en', 1);
expect(planned.provider).toBe('transitous');
expect(planned.itineraries).toEqual([]);
@@ -705,7 +706,7 @@ describe('provider reporting', () => {
it('GTRANSIT-038: a cached answer still names its backend', async () => {
fetchMock.mockResolvedValue(okJson({ itineraries: [] }));
const service = new TransitService(makeProvider({}));
const service = new TransitService(makeProvider({}), transitConfig());
const first = await service.plan({ from: '48.1,2.1', to: '48.2,2.2' }, 'en', 1);
const second = await service.plan({ from: '48.1,2.1', to: '48.2,2.2' }, 'en', 1);
expect(fetchMock).toHaveBeenCalledTimes(1);
+19 -1
View File
@@ -12,6 +12,7 @@
import { deriveTransitStats, type TransitLeg } from '../../../src/nest/transit/transit.helpers';
import { GoogleTransitProvider } from '../../../src/nest/transit/google-transit.provider';
import { TransitService } from '../../../src/nest/transit/transit.service';
import { transitConfig } from '../../../src/nest/app-config/tokens';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
@@ -29,7 +30,7 @@ const fetchMock = vi.fn();
// exercising the MOTIS path — the Google branch has its own suite.
const noAppSettings = { getValue: async () => null } as unknown as AppSettingsRepository;
const noUsers = { getApiKeyColumn: async () => null } as unknown as UsersRepository;
const svc = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota));
const svc = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota), transitConfig());
beforeEach(() => {
vi.stubGlobal('fetch', fetchMock);
@@ -272,3 +273,20 @@ describe('quirk repairs (DI fold fix pass)', () => {
expect(fetchMock.mock.calls.length).toBe(fetchesBeforeTouch + 1);
});
});
describe('upstream base URL', () => {
it('TRANSIT-SVC-016: calls the instance the transitConfig token names (TRANSIT_API_URL)', async () => {
const own = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers, noGoogleQuota), {
apiBase: 'https://motis.internal.example',
});
fetchMock.mockResolvedValueOnce(okJson([]));
await own.geocode('own-motis-station');
expect(String(fetchMock.mock.calls[0][0])).toMatch(/^https:\/\/motis\.internal\.example\/api\/v1\/geocode\?/);
});
it('TRANSIT-SVC-017: defaults to Transitous when TRANSIT_API_URL is unset', async () => {
fetchMock.mockResolvedValueOnce(okJson([]));
await svc.geocode('default-base-station');
expect(String(fetchMock.mock.calls[0][0])).toMatch(/^https:\/\/api\.transitous\.org\/api\/v1\/geocode\?/);
});
});