diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index ccfcda4cb..79f91cfc8 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -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 }; } } diff --git a/server/tests/unit/nest/maps.search-merge.test.ts b/server/tests/unit/nest/maps.search-merge.test.ts index 24a5aa2c9..3a9608029 100644 --- a/server/tests/unit/nest/maps.search-merge.test.ts +++ b/server/tests/unit/nest/maps.search-merge.test.ts @@ -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([])); diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index 4c43cd1e2..a67158511 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -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 => []), + mockSearch: vi.fn(async (): Promise => []), })); vi.mock('../../../src/nest/maps/trek-places.client', async (importOriginal) => ({ ...(await importOriginal()), trekPlacesNearby: mockNearby, + trekPlacesSearch: mockSearch, })); // Injected stub since the photo-cache fold (was a path mock of the module).