From 1de276fcc87e50e98a161df14cf07814cd7a2af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 19 Aug 2026 11:01:42 -0600 Subject: [PATCH] Fix OSM data refreshes not immediately rendering --- app/routes/place.js | 43 ++++++++++++++++++-- app/services/osm.js | 16 ++++++-- app/services/storage.js | 3 +- tests/unit/services/osm-test.js | 61 +++++++++++++++++++++++++++++ tests/unit/services/storage-test.js | 8 +++- 5 files changed, 121 insertions(+), 10 deletions(-) diff --git a/app/routes/place.js b/app/routes/place.js index 619a6c0..eaa82d4 100644 --- a/app/routes/place.js +++ b/app/routes/place.js @@ -1,6 +1,23 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; +// Detects whether the fresh OSM data differs from the currently-shown model +// by more than ~1m of coordinate drift, or in any tag. Mirrors the relevant +// subset of `storage.refreshPlace`'s diff for the non-bookmark path. +function hasOsmChanges(place, fresh) { + const latDiff = Math.abs((place.lat ?? 0) - (fresh.lat ?? 0)); + const lonDiff = Math.abs((place.lon ?? 0) - (fresh.lon ?? 0)); + if (latDiff > 0.00001 || lonDiff > 0.00001) return true; + + const oldTags = place.osmTags || {}; + const newTags = fresh.osmTags || {}; + const allKeys = new Set([...Object.keys(oldTags), ...Object.keys(newTags)]); + for (const key of allKeys) { + if (oldTags[key] !== newTags[key]) return true; + } + return false; +} + export default class PlaceRoute extends Route { @service storage; @service osm; @@ -131,14 +148,32 @@ export default class PlaceRoute extends Route { } async checkUpdates(place) { - // Only check for updates if it's a saved place (has ID) and is an OSM object - if (place && place.id && place.osmId && place.osmType) { + if (!place || !place.osmId || !place.osmType) return; + + // Bookmarked place — refresh via storage, which persists the update. + if (place.id) { const updatedPlace = await this.storage.refreshPlace(place); if (updatedPlace) { - // If an update occurred, refresh the map UI selection without moving the camera - // This ensures the sidebar shows the new data + // If an update occurred, refresh the map UI selection without moving + // the camera. This ensures the sidebar shows the new data. this.mapUi.selectPlace(updatedPlace, { preventZoom: true }); } + return; + } + + // Non-bookmarked explicit OSM place — fetch fresh and update the model + // in-place if anything changed, so the sidebar reflects the latest OSM + // data without requiring a re-open. + try { + const fresh = await this.osm.fetchOsmObject(place.osmId, place.osmType, { + forceFresh: true, + }); + if (fresh && hasOsmChanges(place, fresh)) { + Object.assign(place, fresh); + this.mapUi.selectPlace(place, { preventZoom: true }); + } + } catch (e) { + console.debug('[place] Fresh fetch failed for', place.osmId, e); } } diff --git a/app/services/osm.js b/app/services/osm.js index 68eaa1a..ff75538 100644 --- a/app/services/osm.js +++ b/app/services/osm.js @@ -328,19 +328,27 @@ out center; return this.normalizePoi(data.elements[0]); } - async fetchOsmObject(osmId, osmType) { + async fetchOsmObject(osmId, osmType, { forceFresh = false } = {}) { if (!osmId || !osmType) return null; const cacheKey = this._buildCacheKey(osmType, osmId); + + // Force a fresh fetch from the API, bypassing the cache. Used by callers + // that need genuinely current data (e.g. storage.refreshPlace, which + // diffs the bookmark against freshly-fetched OSM data). + if (forceFresh) { + return this._fetchAndCacheOsmObject(osmId, osmType, cacheKey); + } + + // Cached-first path: return the in-memory entry if it's still warm. const cached = this.cachedPlaces.get(cacheKey); if (cached && Date.now() - cached.timestamp < OsmService.IN_MEMORY_TTL_MS) { console.debug(`Using in-memory cached OSM object for ${cacheKey}`); return cached.data; } - // If we have a persistent (IndexedDB) cache entry, return it immediately and - // kick off a background refresh. This keeps the UI snappy while still ensuring - // the cache is updated with the latest OSM data. + // Otherwise return the persistent IndexedDB entry and refresh in the + // background so the next visit is fresh. const localCached = await this._readLocalCache(osmType, osmId); if (localCached) { console.debug(`Using IndexedDB cached OSM object for ${cacheKey}`); diff --git a/app/services/storage.js b/app/services/storage.js index 80a9737..9637c63 100644 --- a/app/services/storage.js +++ b/app/services/storage.js @@ -426,7 +426,8 @@ export default class StorageService extends Service { console.debug(`Checking for updates for ${place.title} (${place.osmId})`); const freshData = await this.osm.fetchOsmObject( place.osmId, - place.osmType + place.osmType, + { forceFresh: true } ); if (!freshData) { diff --git a/tests/unit/services/osm-test.js b/tests/unit/services/osm-test.js index c30d59b..be60890 100644 --- a/tests/unit/services/osm-test.js +++ b/tests/unit/services/osm-test.js @@ -432,4 +432,65 @@ module('Unit | Service | osm', function (hooks) { 'Uses relations.json endpoint' ); }); + + test('fetchOsmObject with forceFresh bypasses the cache and returns fresh data', async function (assert) { + let service = this.owner.lookup('service:osm'); + + // Seed both caches so we can prove forceFresh skips them. + service.cachedPlaces.set('node:5', { + data: { title: 'Stale In-Memory', lat: 1, lon: 1 }, + timestamp: Date.now(), + }); + await service.localForage.set('osm-cache', 'node:5', { + data: { title: 'Stale IndexedDB', lat: 1, lon: 1 }, + timestamp: Date.now(), + }); + + service.fetchWithRetry = async () => ({ + ok: true, + json: async () => ({ + elements: [ + { + id: 5, + type: 'node', + lat: 2, + lon: 3, + tags: { name: 'Fresh From API' }, + }, + ], + }), + }); + + const result = await service.fetchOsmObject('5', 'node', { + forceFresh: true, + }); + + assert.strictEqual( + result.title, + 'Fresh From API', + 'Returns fresh API data, not cached data' + ); + assert.strictEqual(result.lat, 2); + assert.strictEqual(result.lon, 3); + }); + + test('fetchOsmObject without forceFresh returns the in-memory cache entry without hitting the API', async function (assert) { + let service = this.owner.lookup('service:osm'); + + service.cachedPlaces.set('node:6', { + data: { title: 'Warm In-Memory', lat: 4, lon: 5 }, + timestamp: Date.now(), + }); + + let fetchCalled = 0; + service.fetchWithRetry = async () => { + fetchCalled++; + return { ok: true, json: async () => ({ elements: [] }) }; + }; + + const result = await service.fetchOsmObject('6', 'node'); + + assert.strictEqual(fetchCalled, 0, 'API was not hit'); + assert.strictEqual(result.title, 'Warm In-Memory'); + }); }); diff --git a/tests/unit/services/storage-test.js b/tests/unit/services/storage-test.js index 60df0d6..eef4bba 100644 --- a/tests/unit/services/storage-test.js +++ b/tests/unit/services/storage-test.js @@ -15,8 +15,10 @@ module('Unit | Service | storage', function (hooks) { let service = this.owner.lookup('service:storage'); // Stub OSM Service + let capturedOptions; class OsmStub extends Service { - async fetchOsmObject(id, type) { + async fetchOsmObject(id, type, options) { + capturedOptions = options; return { osmId: id, osmType: type, @@ -49,6 +51,10 @@ module('Unit | Service | storage', function (hooks) { assert.ok(updatePlaceCalled, 'updatePlace should be called'); assert.strictEqual(result.lat, 52.5201, 'Latitude updated'); + assert.ok( + capturedOptions?.forceFresh, + 'refreshPlace fetches fresh OSM data (forceFresh: true)' + ); }); test('refreshPlace ignores tiny coordinate drift', async function (assert) {