feat(places): opening hours from the index, with OSM still first

The index can now carry opening hours, from two sources: an
OpenStreetMap layer the service keeps as a separate database file, and
the schema.org data a place publishes on its own site, read in the same
page fetch the description comes from.

OSM stays the first choice where it has them. Measured across seven
countries it covers 27.5 percent of gastronomy, and an OSM entry
describes this exact object and gets corrected by people who walked past
it, where a chain's website often carries one set of hours for every
branch. The rest is what the operator publishes, which measured 8.3
percent on the same kind of place.

Both go through the same expansion into weekday lines, so a card reading
`opening_hours` gets one shape and not two.

Five maps.service cases went red when search started answering from the
index ahead of Google; they are about the shape of the Google request,
so they now switch the index off first.

server/src/nest/maps/maps.service.ts

@@ -1833,11 +1833,10 @@ export class MapsService {
       const found = await trekPlacesById(placeId.slice('gers:'.length)).catch(() => null);
       if (!found) return { place: null };
 
-      // What the index does not have, the free sources still do. The index
-      // carries no opening hours yet and never carries cuisine, wheelchair
-      // access or a menu link; OpenStreetMap has all of them for the same
-      // building. Without this the hours a user saw while adding the place
-      // disappeared from its card afterwards, which reads like data loss.
+      // What the index does not have, the free sources still do: cuisine,
+      // wheelchair access, a menu link. OpenStreetMap has all of them for the
+      // same building, and without this the details a user saw while adding the
+      // place disappeared from its card afterwards, which reads like data loss.
       //
       // Matched by name and coordinate, with the same two gates
       // resolveOsmIdentity applies everywhere: within range, and sharing a
@@ -1849,16 +1848,42 @@ export class MapsService {
       }).catch(() => null);
 
       const record = toPlaceRecord(found);
-      if (!osm) return { place: record };
+      // Hours the index read off the operator's own site, run through the same
+      // expansion OSM's go through — the client reads a list of weekday lines,
+      // not the raw syntax, and handing it two shapes for one field would be a
+      // bug on every card that shows it.
+      //
+      // Measured across seven countries, OpenStreetMap has hours for 27.5
+      // percent of gastronomy; this covers part of the rest. It is the
+      // fallback, not the first choice: an OSM entry describes this exact
+      // object and gets corrected by people who walked past, where a chain's
+      // website often carries one set of hours for every branch.
+      const fromSite =
+        typeof found.hours?.osm === 'string' ? buildOsmDetails({ opening_hours: found.hours.osm }, '', '') : null;
+      if (!osm) {
+        return {
+          place: fromSite?.opening_hours
+            ? {
+                ...record,
+                opening_hours: fromSite.opening_hours,
+                open_now: fromSite.open_now,
+                opening_periods: fromSite.opening_periods,
+              }
+            : record,
+        };
+      }
 
       const osmDetails = buildOsmDetails(osm.tags, '', '');
+      const hoursFrom = osmDetails.opening_hours ? osmDetails : fromSite;
       return {
         place: {
           // Index first: its name, coordinate and contact details are the ones
           // the user picked. OSM only fills what is still missing.
           ...osmDetails,
           ...record,
-          opening_hours: osmDetails.opening_hours ?? null,
+          opening_hours: hoursFrom?.opening_hours ?? null,
+          open_now: hoursFrom?.open_now ?? null,
+          opening_periods: hoursFrom?.opening_periods ?? null,
           website: record.website ?? osmDetails.website ?? null,
           phone: record.phone ?? osmDetails.phone ?? null,
           osm_id: placeId,

server/src/nest/maps/trek-places.client.ts

@@ -49,7 +49,15 @@ export interface TrekPlace {
   };
   brand: { name: string | null; wikidata: string | null } | null;
   source: string;
-  hours: unknown | null;
+  /**
+   * Opening hours in OSM syntax, plus where they came from. The index carries
+   * them for a small share of places; the rest are read from the schema.org
+   * data the operator publishes on their own site, in the same page fetch the
+   * description comes from.
+   */
+  hours: { osm: string; source: string | null; sourceUrl: string | null } | null;
+  /** Quoted from the place's own site, when it publishes one for machines. */
+  description?: { text: string; source: string | null; sourceUrl: string | null } | null;
   score?: number;
 }
 

server/tests/unit/nest/maps.gers-details.test.ts

@@ -0,0 +1,100 @@
+/**
+ * MAPS-GERS-001..007 — the details of a place that came from the index.
+ *
+ * The case that started this: L'Osteria Steinstrasse showed opening hours while
+ * it was being added and none on its card afterwards. The index is not the only
+ * source for a place it holds, and this is where the rest gets filled in.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const { mockById } = vi.hoisted(() => ({
+  mockById: vi.fn(async (_gers: string): Promise<unknown> => null),
+}));
+vi.mock('../../../src/nest/maps/trek-places.client', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('../../../src/nest/maps/trek-places.client')>()),
+  trekPlacesById: mockById,
+}));
+
+vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: '0'.repeat(64) }));
+
+import { MapsService } from '../../../src/nest/maps/maps.service';
+import type { DatabaseService } from '../../../src/nest/database/database.service';
+import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
+
+const PLACE = {
+  gers: 'abc-123',
+  name: "L'Osteria",
+  lat: 54.0879,
+  lng: 12.1408,
+  category: 'restaurant',
+  categoryPath: 'eat_and_drink>restaurant',
+  confidence: 1,
+  address: { freeform: 'Steinstrasse 9', locality: 'Rostock', postcode: '18055', region: null, country: 'DE' },
+  contact: { website: 'https://losteria.net/', phone: null, email: null, socials: null },
+  brand: null,
+  source: 'overture',
+  hours: null as { osm: string } | null,
+};
+
+function make(osmTags: Record<string, string> | null) {
+  const database = { get: vi.fn(() => undefined) } as unknown as DatabaseService;
+  const svc = new MapsService(database, {} as PlacePhotoCacheService);
+  vi.spyOn(svc, 'resolveOsmIdentity').mockResolvedValue(
+    osmTags ? { tags: osmTags, osmUrl: 'https://www.openstreetmap.org/node/1', matchedName: "L'Osteria" } : null,
+  );
+  return svc;
+}
+
+beforeEach(() => {
+  mockById.mockReset();
+  mockById.mockResolvedValue({ ...PLACE });
+  vi.spyOn(console, 'warn').mockImplementation(() => {});
+});
+
+describe('MapsService.getPlaceDetails for a gers: id', () => {
+  it('MAPS-GERS-001: prefers the hours OpenStreetMap has for the same building', async () => {
+    // OSM describes this exact object and gets corrected by people who walked
+    // past. A chain's website often carries one set of hours for every branch.
+    mockById.mockResolvedValue({ ...PLACE, hours: { osm: 'Mo-Su 12:00-22:00' } });
+    const out = await make({ opening_hours: 'Mo-Su 11:30-23:00' }).getPlaceDetails(1, 'gers:abc-123');
+    // Expanded to weekday lines, which is the shape the card reads.
+    expect(out.place?.opening_hours).toEqual(expect.arrayContaining(['Monday: 11:30-23:00']));
+  });
+
+  it('MAPS-GERS-002: falls back to the hours the operator publishes', async () => {
+    // OpenStreetMap has hours for 27.5 percent of gastronomy, measured across
+    // seven countries. This is what covers part of the rest.
+    mockById.mockResolvedValue({ ...PLACE, hours: { osm: 'Mo-Su 12:00-22:00' } });
+    const out = await make({ cuisine: 'italian' }).getPlaceDetails(1, 'gers:abc-123');
+    expect(out.place?.opening_hours).toEqual(expect.arrayContaining(['Monday: 12:00-22:00']));
+  });
+
+  it('MAPS-GERS-003: still answers when OpenStreetMap knows nothing about it', async () => {
+    mockById.mockResolvedValue({ ...PLACE, hours: { osm: 'Mo-Su 12:00-22:00' } });
+    const out = await make(null).getPlaceDetails(1, 'gers:abc-123');
+    expect(out.place?.name).toBe("L'Osteria");
+    // Same shape whether the hours came from OSM or from the operator's site.
+    expect(out.place?.opening_hours).toEqual(expect.arrayContaining(['Monday: 12:00-22:00']));
+  });
+
+  it('MAPS-GERS-004: a place nobody has hours for shows none, not an empty string', async () => {
+    const out = await make(null).getPlaceDetails(1, 'gers:abc-123');
+    expect(out.place?.opening_hours ?? null).toBeNull();
+  });
+
+  it('MAPS-GERS-005: the index wins on the fields the user actually picked', async () => {
+    const out = await make({ name: 'Etwas anderes', opening_hours: 'Mo-Fr 09:00-17:00' })
+      .getPlaceDetails(1, 'gers:abc-123');
+    expect(out.place).toMatchObject({ name: "L'Osteria", osm_id: 'gers:abc-123', source: 'trek-places' });
+  });
+
+  it('MAPS-GERS-006: an unknown id is null rather than a thrown request', async () => {
+    mockById.mockResolvedValue(null);
+    expect(await make(null).getPlaceDetails(1, 'gers:nope')).toEqual({ place: null });
+  });
+
+  it('MAPS-GERS-007: a failing lookup is a null, not a 500', async () => {
+    mockById.mockRejectedValue(new Error('places api down'));
+    expect(await make(null).getPlaceDetails(1, 'gers:abc-123')).toEqual({ place: null });
+  });
+});

server/tests/unit/nest/maps.service.test.ts

@@ -144,7 +144,27 @@ import type { SsrfResult } from '../../../src/utils/ssrfGuard';
 // flowing exactly as they did for the legacy module.
 const svc = new MapsService(new DatabaseService(db as never), photoCacheStub);
 
+/**
+ * Switch the TREK Places index off for one case.
+ *
+ * The index now answers search and autocomplete before Google is asked, which
+ * is the point of it. A case that is about the shape of the GOOGLE request has
+ * to take the index out of the way first, or it asserts against the index's URL
+ * and fails for the right reason.
+ *
+ * Spied on the method rather than driven through app_settings, because
+ * trekPlacesEnabled writes its key straight into the SQL and the db stub only
+ * sees bound parameters. The default and the 'false' path are covered in
+ * maps.area.test.ts, which does go through the setting.
+ */
+let indexSpy: { mockRestore: () => void } | null = null;
+function indexOff(): void {
+  indexSpy = vi.spyOn(svc, 'trekPlacesEnabled').mockReturnValue(false);
+}
+
 afterEach(() => {
+  indexSpy?.mockRestore();
+  indexSpy = null;
   vi.unstubAllGlobals();
   mockDbGet.mockReset();
   mockDbGet.mockReturnValue(undefined);
@@ -1196,6 +1216,7 @@ describe('searchPlaces (fetch stubbed)', () => {
   // on its own. The body field and the query parameter are what Google's
   // reference specifies for each half.
   it('MAPS-039f: sends the session token in the autocomplete body', async () => {
+    indexOff();
     mockDbGet.mockReturnValueOnce({ maps_api_key: 'ENCRYPTED' }).mockReturnValueOnce(null);
     const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ suggestions: [] }) });
     vi.stubGlobal('fetch', fetchMock);
@@ -1208,6 +1229,7 @@ describe('searchPlaces (fetch stubbed)', () => {
   });
 
   it('MAPS-039g: omits the field entirely when there is no session token', async () => {
+    indexOff();
     mockDbGet.mockReturnValueOnce({ maps_api_key: 'ENCRYPTED' }).mockReturnValueOnce(null);
     const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ suggestions: [] }) });
     vi.stubGlobal('fetch', fetchMock);
@@ -1335,6 +1357,7 @@ describe('searchPlaces (fetch stubbed)', () => {
   });
 
   it('MAPS-183: drops permanently closed Google results and keeps the rest (#1341)', async () => {
+    indexOff();
     mockDbGet.mockReturnValueOnce({ maps_api_key: 'some-key' });
     const fetchMock = vi.fn().mockResolvedValue({
       ok: true,
@@ -1530,6 +1553,7 @@ describe('autocompletePlaces (fetch stubbed)', () => {
   });
 
   it('MAPS-088: includes locationBias in Google request when provided', async () => {
+    indexOff();
     mockDbGet.mockReturnValueOnce({ maps_api_key: 'test-key' });
     const fetchMock = vi.fn().mockResolvedValue({
       ok: true,
@@ -1549,6 +1573,7 @@ describe('autocompletePlaces (fetch stubbed)', () => {
   });
 
   it('MAPS-089: omits locationBias from Google request when not provided', async () => {
+    indexOff();
     mockDbGet.mockReturnValueOnce({ maps_api_key: 'test-key' });
     const fetchMock = vi.fn().mockResolvedValue({
       ok: true,