fix(places): keep two different tracks of the same name apart on import

Two GPX tracks a watch named alike (two "Afternoon Hike"s) were taken for one:
the import compared a track only by its name, so the second was skipped as
already in the trip. A track now matches a track of that name only when it
also starts, ends and reaches as far as that one, to about a hundred metres,
so the same file imported again is still skipped. The GPX, KML and tour
imports and the tour preview all read the same rule.

Closes #2870

server/src/nest/places/places.helpers.ts

@@ -86,15 +86,58 @@ export async function reclaimPhotoCache(
 
 export interface DedupSet {
   names: Set<string>;
-  /** The names of the places with a line: a track to import matches only these (#2810). Absent where none is read. */
-  trackNames?: Set<string>;
+  /**
+   * The places with a line, keyed by name and course (`trackKey`): a track to import
+   * matches only these (#2810), and only one that runs the same way (#2870). Absent
+   * where none is read.
+   */
+  tracks?: Set<string>;
   coords: Array<{ lat: number; lng: number }>;
   /** Provider ids (google_place_id, google_ftid, osm_id, amap_poi_id) of the places already in the trip. */
   externalIds: Set<string>;
 }
 
-/** A candidate for the dedup set; `track` when it is a line (a GPX track or route, a KML path). */
-export type DedupCandidate = PlaceMatchCandidate & { track?: boolean };
+/**
+ * A candidate for the dedup set; `track` when it is a line (a GPX track or route, a
+ * KML path), with that line in `routeGeometry`.
+ */
+export type DedupCandidate = PlaceMatchCandidate & { track?: boolean; routeGeometry?: string | null };
+
+/**
+ * Where a line starts, ends and how far it reaches, to about a hundred metres: two
+ * hikes a watch names alike ("Afternoon Hike") differ here, while the same file read
+ * again gives the same answer however its points were rounded or thinned. Empty for
+ * a line that does not parse.
+ */
+function lineCourse(routeGeometry: string | null | undefined): string {
+  let points: unknown;
+  try {
+    points = JSON.parse(routeGeometry ?? '');
+  } catch {
+    return '';
+  }
+  if (!Array.isArray(points)) return '';
+  const line = points.filter(
+    (p): p is [number, number] => Array.isArray(p) && Number.isFinite(p[0]) && Number.isFinite(p[1]),
+  );
+  if (line.length === 0) return '';
+  // A loop rather than Math.min(...): a recorded day holds more points than a call takes arguments.
+  let [south, west] = line[0];
+  let [north, east] = line[0];
+  for (const [lat, lng] of line) {
+    south = Math.min(south, lat);
+    north = Math.max(north, lat);
+    west = Math.min(west, lng);
+    east = Math.max(east, lng);
+  }
+  const [first, last] = [line[0], line[line.length - 1]];
+  return [first[0], first[1], last[0], last[1], south, west, north, east].map((v) => v.toFixed(3)).join(',');
+}
+
+/** A track's entry in `DedupSet.tracks`: its normalized name and its course. */
+function trackKey(name: string, routeGeometry: string | null | undefined): string {
+  return `${name}\u0000${lineCourse(routeGeometry)}`;
+}
 
 /**
  * Returns true if a candidate place is already represented in the dedup set.
@@ -114,9 +157,14 @@ export function isPlaceDuplicate(candidate: DedupCandidate, dedup: DedupSet): bo
     if (strategy.by === 'externalId') {
       if (dedup.externalIds.has(strategy.id)) return true;
     } else if (strategy.by === 'name') {
-      // A track is the same as a track of that name, never as a sight of that name:
-      // a watch names a hike after the place it went to (#2810).
-      if ((candidate.track ? dedup.trackNames : dedup.names)?.has(strategy.name)) return true;
+      // A track is the same as a track of that name that runs the same way, never as a
+      // sight of that name: a watch names a hike after the place it went to (#2810), and
+      // two hikes of one day alike (#2870).
+      if (candidate.track) {
+        if (dedup.tracks?.has(trackKey(strategy.name, candidate.routeGeometry))) return true;
+      } else if (dedup.names.has(strategy.name)) {
+        return true;
+      }
     } else if (
       dedup.coords.some(
         (c) =>
@@ -143,19 +191,19 @@ export function dedupSetOf(
   >,
 ): DedupSet {
   const names = new Set<string>();
-  const trackNames = new Set<string>();
+  const tracks = new Set<string>();
   const coords: Array<{ lat: number; lng: number }> = [];
   const externalIds = new Set<string>();
   for (const row of rows) {
     for (const id of externalIdsOf(row)) externalIds.add(id);
     if (row.name) {
       names.add(row.name.trim().toLowerCase());
-      if (row.route_geometry) trackNames.add(row.name.trim().toLowerCase());
+      if (row.route_geometry) tracks.add(trackKey(row.name.trim().toLowerCase(), row.route_geometry));
     } else if (row.lat != null && row.lng != null) {
       coords.push({ lat: row.lat, lng: row.lng });
     }
   }
-  return { names, trackNames, coords, externalIds };
+  return { names, tracks, coords, externalIds };
 }
 
 /** Record a newly inserted place so subsequent candidates in the same batch are checked against it. */
@@ -164,7 +212,7 @@ export function trackInsertedInDedupSet(place: DedupCandidate, dedup: DedupSet):
   const normalizedName = normalizePlaceName(place.name);
   if (normalizedName) {
     dedup.names.add(normalizedName);
-    if (place.track) (dedup.trackNames ??= new Set()).add(normalizedName);
+    if (place.track) (dedup.tracks ??= new Set()).add(trackKey(normalizedName, place.routeGeometry));
   } else if (place.lat != null && place.lng != null) {
     dedup.coords.push({ lat: place.lat, lng: place.lng });
   }

server/src/nest/places/places.service.ts

@@ -758,7 +758,7 @@ export class PlacesService {
   async gpxDuplicates(tripId: string, rows: PreparedGpxPlace[]): Promise<boolean[]> {
     const dedup = await this.buildDedupSet(tripId);
     return rows.map((wp) => {
-      const candidate = { name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry };
+      const candidate = { name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry, routeGeometry: wp.routeGeometry };
       if (isPlaceDuplicate(candidate, dedup)) return true;
       trackInsertedInDedupSet(candidate, dedup);
       return false;
@@ -925,7 +925,7 @@ export class PlacesService {
     // `findWithTagsAndRatings` INSIDE the caller's transaction, so the read
     // sees the still-uncommitted row.
     for (const wp of waypoints) {
-      if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup)) {
+      if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry, routeGeometry: wp.routeGeometry }, dedup)) {
         skipped++;
         continue;
       }
@@ -960,7 +960,7 @@ export class PlacesService {
       });
       const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
       created.push(place);
-      trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry }, dedup);
+      trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng, track: !!wp.routeGeometry, routeGeometry: wp.routeGeometry }, dedup);
     }
 
     return { places: created, count: created.length, skipped };
@@ -1033,7 +1033,7 @@ export class PlacesService {
         const fallbackName = `Placemark ${fallbackIndex}`;
         const name = parsedPlacemark.name || fallbackName;
 
-        if (isPlaceDuplicate({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup)) {
+        if (isPlaceDuplicate({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath, routeGeometry: parsedPlacemark.routeGeometry }, dedup)) {
           summary.skippedCount += 1;
           dupCount++;
           fallbackIndex += 1;
@@ -1079,7 +1079,7 @@ export class PlacesService {
 
         const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
         created.push(place);
-        trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath }, dedup);
+        trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng, track: isPath, routeGeometry: parsedPlacemark.routeGeometry }, dedup);
         summary.createdCount += 1;
         fallbackIndex += 1;
       }

server/tests/e2e/tours.e2e.test.ts

@@ -535,6 +535,22 @@ describe('Tours e2e (real guards + temp SQLite)', () => {
       expect(await countRows(orm, Tours)).toBe(1);
     });
 
+    it('imports two different tracks of the same name, and still skips either one the second time (#2870)', async () => {
+      // A second hike the watch called "Alpine loop" too, somewhere else entirely.
+      const other = TRACK_GPX.replace(/lat="49(\.\d+)?" lon="12/g, (m) => m.replace('49', '47').replace('12', '11'));
+      expect((await importGpx(OWNER, TRACK_GPX)).body).toMatchObject({ skipped: 0 });
+      expect((await previewGpx(OWNER, other)).body.tracks[0]).toMatchObject({ duplicate: false });
+      const second = await importGpx(OWNER, other);
+      expect(second.status).toBe(201);
+      expect(second.body).toMatchObject({ skipped: 0 });
+      expect(second.body.tours).toHaveLength(1);
+      expect(await countRows(orm, Tours)).toBe(2);
+
+      expect((await importGpx(OWNER, TRACK_GPX)).body).toEqual({ tours: [], caution: false, skipped: 1 });
+      expect((await importGpx(OWNER, other)).body).toEqual({ tours: [], caution: false, skipped: 1 });
+      expect(await countRows(orm, Tours)).toBe(2);
+    });
+
     it('takes over a track from before Tours with the same name instead of skipping it (#2810)', async () => {
       // 4.x stored a GPX track as a plain place: a line, no tours row.
       const line = '[[49,12,100],[49.01,12.01,150],[49.02,12.02,120]]';

server/tests/unit/nest/places.helpers.test.ts

@@ -33,7 +33,9 @@ describe('isPlaceDuplicate / trackInsertedInDedupSet', () => {
       { name: ' 008 ', lat: 1, lng: 2, route_geometry: '[[1,2],[3,4]]' },
     ]);
     expect(isPlaceDuplicate({ name: 'hokitika gorge', lat: -42.9, lng: 171.0, track: true }, dedup)).toBe(false);
-    expect(isPlaceDuplicate({ name: '008', lat: 1, lng: 2, track: true }, dedup)).toBe(true);
+    expect(isPlaceDuplicate({ name: '008', lat: 1, lng: 2, track: true, routeGeometry: '[[1,2],[3,4]]' }, dedup)).toBe(
+      true,
+    );
     // A pin still matches every place of its name, a track included.
     expect(isPlaceDuplicate({ name: 'Hokitika Gorge', lat: null, lng: null }, dedup)).toBe(true);
     expect(isPlaceDuplicate({ name: '008', lat: null, lng: null }, dedup)).toBe(true);
@@ -46,7 +48,37 @@ describe('isPlaceDuplicate / trackInsertedInDedupSet', () => {
     trackInsertedInDedupSet({ name: 'Ridge', lat: 1, lng: 2 }, bare);
     expect(isPlaceDuplicate({ name: 'Ridge', lat: 1, lng: 2, track: true }, bare)).toBe(false);
     trackInsertedInDedupSet({ name: 'Lake', lat: 1, lng: 2, track: true }, bare);
-    expect([...(bare.trackNames ?? [])]).toEqual(['lake']);
+    expect([...(bare.tracks ?? [])]).toEqual(['lake\u0000']);
+  });
+
+  it('keeps two tracks of one name apart when they run differently (#2870)', () => {
+    // Two watch hikes, both called "Afternoon Hike": one up the valley, one along the coast.
+    const valley = '[[46.5,8.0,1200],[46.52,8.03,1500],[46.55,8.06,1900]]';
+    const coast = '[[43.7,7.25],[43.72,7.3],[43.74,7.35]]';
+    const dedup = dedupSetOf([{ name: 'Afternoon Hike', lat: 46.5, lng: 8.0, route_geometry: valley }]);
+    expect(
+      isPlaceDuplicate({ name: 'afternoon hike', lat: 43.7, lng: 7.25, track: true, routeGeometry: coast }, dedup),
+    ).toBe(false);
+    // The same file again is still the same track, its points rounded or thinned as they may be.
+    const valleyAgain = '[[46.50001,8.00002],[46.55,8.06]]';
+    expect(
+      isPlaceDuplicate({ name: 'Afternoon Hike', lat: 46.5, lng: 8.0, track: true, routeGeometry: valleyAgain }, dedup),
+    ).toBe(true);
+
+    // Within one file: the first of the two goes in, the second is not taken for it.
+    const batch = emptyDedup();
+    const first = { name: 'Day 1', lat: 46.5, lng: 8.0, track: true, routeGeometry: valley };
+    const second = { name: 'Day 1', lat: 43.7, lng: 7.25, track: true, routeGeometry: coast };
+    expect(isPlaceDuplicate(first, batch)).toBe(false);
+    trackInsertedInDedupSet(first, batch);
+    expect(isPlaceDuplicate(second, batch)).toBe(false);
+    trackInsertedInDedupSet(second, batch);
+    expect(isPlaceDuplicate({ ...second }, batch)).toBe(true);
+
+    // A line that does not parse falls back to the name alone, on both sides.
+    const broken = dedupSetOf([{ name: 'Loop', lat: 1, lng: 2, route_geometry: 'not json' }]);
+    expect(isPlaceDuplicate({ name: 'Loop', lat: 1, lng: 2, track: true, routeGeometry: '{}' }, broken)).toBe(true);
+    expect(isPlaceDuplicate({ name: 'Loop', lat: 1, lng: 2, track: true, routeGeometry: coast }, broken)).toBe(false);
   });
 
   it('matches a named place case- and whitespace-insensitively', () => {