mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(maps): say which source answered, and stop the stubs driving the index
Two halves of one mistake, both found by the suite. Letting each side of the merged search catch its own failure meant the index could contribute nothing while the answer still called itself "trek-places+openstreetmap". That label is not decoration: the search log writes it into the corpus a candidate index gets scored against later, so a list that is entirely OpenStreetMap must not be recorded as though the index had a hand in it. The source now names what actually contributed. And maps.service.test.ts drove the index for real. Its fetch stubs answer with json() and no text(), which is not what the index client reads, so the branch was entered and then failed on the stub — "uses Nominatim when user has no API key" was really testing that a stub is incomplete. With the new circuit breaker those failures also accumulated, and the breaker is process state, so it leaked into whatever file ran next. trekPlacesSearch is stubbed there now, beside the nearby stub that was already there for the same reason. The index paths keep their own files.
This commit is contained in:
@@ -1697,10 +1697,16 @@ export class MapsService {
|
||||
osmAnswer = osm;
|
||||
const places = mergeSearchResults(found.map(toPlaceRecord), osm);
|
||||
if (places.length > 0) {
|
||||
return {
|
||||
places,
|
||||
source: osm.length > 0 ? 'trek-places+openstreetmap' : 'trek-places',
|
||||
};
|
||||
// Names the sources that actually contributed, not the ones that were
|
||||
// asked. Either side can come back empty — the index turns down a common
|
||||
// single word without coordinates, and OpenStreetMap can be down — and
|
||||
// the search log writes this into the corpus a candidate index is later
|
||||
// scored against, so a list that is entirely OpenStreetMap must not be
|
||||
// recorded as though the index had a hand in it.
|
||||
const source = found.length > 0
|
||||
? (osm.length > 0 ? 'trek-places+openstreetmap' : 'trek-places')
|
||||
: 'openstreetmap';
|
||||
return { places, source };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -134,6 +134,24 @@ describe('MapsService.searchPlaces — the merged path', () => {
|
||||
expect(mockNominatim).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('MAPS-SEARCH-009: names the sources that answered, not the ones that were asked', async () => {
|
||||
// Either side can come back empty in ordinary use: the index turns down a
|
||||
// common single word without coordinates, and OpenStreetMap can be down. The
|
||||
// search log writes this field into the corpus a candidate index is scored
|
||||
// against later, so a list that is entirely OpenStreetMap must not be
|
||||
// recorded as though the index had a hand in it.
|
||||
mockSearch.mockRejectedValue(new Error('refused'));
|
||||
mockNominatim.mockResolvedValue(osmAnswer([osmRow('Museum')]));
|
||||
expect((await make().searchPlaces(1, 'museum')).source).toBe('openstreetmap');
|
||||
|
||||
mockSearch.mockResolvedValue([indexHit("L'Osteria")]);
|
||||
mockNominatim.mockResolvedValue(osmAnswer([]));
|
||||
expect((await make().searchPlaces(1, "L'Osteria")).source).toBe('trek-places');
|
||||
|
||||
mockNominatim.mockResolvedValue(osmAnswer([osmRow('Steinstrasse', 36.5, 12.2)]));
|
||||
expect((await make().searchPlaces(1, "L'Osteria")).source).toBe('trek-places+openstreetmap');
|
||||
});
|
||||
|
||||
it('MAPS-SEARCH-005: both empty asks nobody a second time', async () => {
|
||||
mockSearch.mockResolvedValue([]);
|
||||
mockNominatim.mockResolvedValue(osmAnswer([]));
|
||||
|
||||
@@ -120,16 +120,26 @@ vi.mock('../../../src/config', () => ({
|
||||
ENCRYPTION_KEY: '0'.repeat(64),
|
||||
}));
|
||||
|
||||
// pois() asks the index before Overpass and trekPlacesEnabled fails open, so a
|
||||
// case that reaches it with no fetch stub in place leaves the runner for
|
||||
// places.liketrek.com. Only the one export pois() calls is replaced; the rest of
|
||||
// the client stays real. The index path itself is covered in maps.pois.test.ts.
|
||||
const { mockNearby } = vi.hoisted(() => ({
|
||||
// pois(), searchPlaces() and autocompletePlaces() all ask the index before the
|
||||
// old path, and trekPlacesEnabled fails open — so a case that reaches one of
|
||||
// them with no fetch stub in place leaves the runner for places.liketrek.com.
|
||||
//
|
||||
// Search is stubbed for a second reason. The fetch stubs in this file answer
|
||||
// with `json()` and no `text()`, which is not what the index client reads, so
|
||||
// the index branch was entered and then failed on the stub. That made the cases
|
||||
// pass for the wrong reason — 'uses Nominatim when user has no API key' was
|
||||
// really testing that a stub is incomplete — and it left the client's circuit
|
||||
// breaker counting real consecutive failures, which is process state and leaks
|
||||
// into whatever file runs next. The index paths themselves are covered in
|
||||
// maps.pois.test.ts, maps.autocomplete.test.ts and maps.search-merge.test.ts.
|
||||
const { mockNearby, mockSearch } = vi.hoisted(() => ({
|
||||
mockNearby: vi.fn(async (): Promise<unknown[]> => []),
|
||||
mockSearch: vi.fn(async (): Promise<unknown[]> => []),
|
||||
}));
|
||||
vi.mock('../../../src/nest/maps/trek-places.client', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../../../src/nest/maps/trek-places.client')>()),
|
||||
trekPlacesNearby: mockNearby,
|
||||
trekPlacesSearch: mockSearch,
|
||||
}));
|
||||
|
||||
// Injected stub since the photo-cache fold (was a path mock of the module).
|
||||
|
||||
Reference in New Issue
Block a user