diff --git a/app/services/osm.js b/app/services/osm.js index ff75538..9544534 100644 --- a/app/services/osm.js +++ b/app/services/osm.js @@ -457,11 +457,11 @@ out center; // request using comma-separated IDs (max ~50 per request). // // Note: the multi-fetch endpoint returns ways/relations WITHOUT their child - // nodes, so normalized data may lack lat/lon and geometry. We intentionally - // do NOT write these results to the general OSM cache — only the returned - // Map is used by the caller (the contributions service manages its own - // name cache). This prevents incomplete data from breaking place detail - // navigation, which needs the full single-object endpoint. + // nodes, so normalized data may lack lat/lon and geometry. + // We only write complete results (those with lat/lon) to the general OSM + // cache. Nodes from the batch endpoint always include coordinates, so they + // are cached. Ways/relations without child node coordinates are not cached, + // preserving the full single-object endpoint for place detail navigation. const MAX_IDS_PER_REQUEST = 50; for (let i = 0; i < group.length; i += MAX_IDS_PER_REQUEST) { @@ -488,6 +488,21 @@ out center; ); if (normalized) { result.set(item.cacheKey, normalized); + // Warm the OSM cache for complete results (nodes always have + // lat/lon from the batch endpoint; ways/relations only when + // child nodes are included in the response). + if ( + normalized.lat != null && + normalized.lon != null && + normalized.title + ) { + this._storeInMemoryAndLocalStorage( + item.cacheKey, + osmType, + item.osmId, + normalized + ); + } } } } catch (e) { diff --git a/tests/unit/services/osm-test.js b/tests/unit/services/osm-test.js index be60890..839c71e 100644 --- a/tests/unit/services/osm-test.js +++ b/tests/unit/services/osm-test.js @@ -329,7 +329,7 @@ module('Unit | Service | osm', function (hooks) { ); }); - test('fetchOsmObjectsBatch does not write to the general OSM cache', async function (assert) { + test('fetchOsmObjectsBatch writes complete results to the general OSM cache', async function (assert) { let service = this.owner.lookup('service:osm'); service.fetchWithRetry = async () => ({ @@ -343,13 +343,38 @@ module('Unit | Service | osm', function (hooks) { await service.fetchOsmObjectsBatch([{ osmType: 'node', osmId: '100' }]); - assert.false( + assert.true( service.cachedPlaces.has('node:100'), - 'Batch fetch does not write to the in-memory OSM cache' + 'Batch fetch writes complete results (with lat/lon) to the in-memory OSM cache' + ); + assert.ok( + await service.localForage.get('osm-cache', 'node:100'), + 'Batch fetch writes complete results to the persistent OSM cache' + ); + }); + + test('fetchOsmObjectsBatch does NOT cache incomplete results without lat/lon', async function (assert) { + let service = this.owner.lookup('service:osm'); + + // Way without child nodes in the response -> no lat/lon in normalized result + service.fetchWithRetry = async () => ({ + ok: true, + json: async () => ({ + elements: [ + { id: 200, type: 'way', nodes: [], tags: { name: 'Way 200' } }, + ], + }), + }); + + await service.fetchOsmObjectsBatch([{ osmType: 'way', osmId: '200' }]); + + assert.false( + service.cachedPlaces.has('way:200'), + 'Batch fetch does NOT cache incomplete results (ways without child nodes)' ); assert.notOk( - await service.localForage.get('osm-cache', 'node:100'), - 'Batch fetch does not write to the persistent OSM cache' + await service.localForage.get('osm-cache', 'way:200'), + 'Batch fetch does NOT write incomplete results to persistent cache' ); });