diff --git a/server/src/nest/memories/synology.service.ts b/server/src/nest/memories/synology.service.ts index 377cc57b0..87a2d6107 100644 --- a/server/src/nest/memories/synology.service.ts +++ b/server/src/nest/memories/synology.service.ts @@ -732,24 +732,47 @@ export class SynologyService { ): Promise> { const parsedId = this._splitPackedSynologyId(photoId); if (!parsedId) return fail('Invalid photo ID format', 400); - const infoParams: ApiCallParams = { - api: 'SYNO.Foto.Browse.Item', - method: 'get', - version: 5, - id: `[${Number(parsedId.id) + 1}]`, //for some reason synology wants id moved by one to get image info - additional: ['resolution', 'exif', 'gps', 'address', 'orientation', 'description'], - }; - if (passphrase) infoParams.passphrase = passphrase; - const result = await this._requestSynologyApi<{ list: SynologyPhotoItem[] }>(targetUserId, infoParams); - if (!result.success) return result as ServiceResult; + // The packed id carries the thumbnail's unit id, while Browse.Item `get` + // wants the item's own id. On most libraries the two are equal; on some the + // item is one further on, which is why this once always asked for unit + 1 + // and, on the others, stored the next photo's date and place (#3019). So + // both are asked in turn, and only an item whose own thumbnail is this unit + // counts: never a neighbour's capture time or GPS. + const unit = Number(parsedId.id); + let lastFailure: ServiceResult = fail('Photo not found', 404); + for (const itemId of [unit, unit + 1]) { + const infoParams: ApiCallParams = { + api: 'SYNO.Foto.Browse.Item', + method: 'get', + version: 5, + id: `[${itemId}]`, + additional: ['thumbnail', 'resolution', 'exif', 'gps', 'address', 'orientation', 'description'], + }; + if (passphrase) infoParams.passphrase = passphrase; + const result = await this._requestSynologyApi<{ list: SynologyPhotoItem[] }>(targetUserId, infoParams); + if (!result.success) { + lastFailure = result as ServiceResult; + continue; + } + const metadata = result.data.list?.find((item) => this._isItemOfUnit(item, parsedId.id)); + if (!metadata) continue; - const metadata = result.data.list?.[0]; - if (!metadata) return fail('Photo not found', 404); + const normalized = this._normalizeSynologyPhotoInfo(metadata); + normalized.id = photoId; + return success(normalized); + } + return lastFailure; + } - const normalized = this._normalizeSynologyPhotoInfo(metadata); - normalized.id = photoId; - return success(normalized); + /** + * Whether an item from Browse.Item is the one behind a unit id: its thumbnail's + * cache key starts with that unit. The key's second half is a timestamp that + * moves when the photo is edited, so the unit alone decides. + */ + private _isItemOfUnit(item: SynologyPhotoItem, unitId: string): boolean { + const key = String(item.additional?.thumbnail?.cache_key ?? ''); + return key.split('_')[0] === unitId; } async fetchSynologyThumbnailBytes( diff --git a/server/tests/unit/nest/synology.service.test.ts b/server/tests/unit/nest/synology.service.test.ts index 604fd226b..cdf12168d 100644 --- a/server/tests/unit/nest/synology.service.test.ts +++ b/server/tests/unit/nest/synology.service.test.ts @@ -531,6 +531,66 @@ describe('the search window', () => { }); }); +describe('getSynologyAssetInfo', () => { + /** A Browse.Item `get` answer holding one item, its thumbnail keyed to a unit. */ + const item = (cacheKey: string, time: number) => + api({ + list: [{ id: 1, time, additional: { thumbnail: { cache_key: cacheKey }, gps: { latitude: 1, longitude: 2 } } }], + }); + const requestedIds = () => + safeFetch.mock.calls.map((c) => new URLSearchParams(String((c[1] as { body: URLSearchParams }).body)).get('id')); + const takenAt = (r: unknown) => (r as { data: { takenAt: string } }).data.takenAt; + + it('SYNO-U090: reads the asset itself where the item id is its unit id, not the next photo (#3019)', async () => { + safeFetch.mockResolvedValueOnce(item('10314_1793757217', 1718236800)); + + const result = await svc.getSynologyAssetInfo(USER, '10314_1793757217', USER); + + expect(requestedIds()).toEqual(['[10314]']); + expect(takenAt(result)).toBe(new Date(1718236800 * 1000).toISOString()); + expect((result as { data: { id: string } }).data.id).toBe('10314_1793757217'); + }); + + it('SYNO-U091: where the item is one further on, takes that one and not the neighbour the unit id names', async () => { + safeFetch + .mockResolvedValueOnce(item('10313_1700000000', 1500000000)) + .mockResolvedValueOnce(item('10314_1793757217', 1718236800)); + + const result = await svc.getSynologyAssetInfo(USER, '10314_1793757217', USER); + + expect(requestedIds()).toEqual(['[10314]', '[10315]']); + expect(takenAt(result)).toBe(new Date(1718236800 * 1000).toISOString()); + }); + + it("SYNO-U092: answers not found rather than another photo's date and place", async () => { + safeFetch + .mockResolvedValueOnce(item('10313_1700000000', 1500000000)) + .mockResolvedValueOnce(item('10316_1700000000', 1500000000)); + + const result = await svc.getSynologyAssetInfo(USER, '10314_1793757217', USER); + + expect(result).toEqual({ success: false, error: { message: 'Photo not found', status: 404 } }); + }); + + it('SYNO-U093: matches an edited photo by its unit, whatever timestamp its key carries now', async () => { + safeFetch.mockResolvedValueOnce(item('10314_1800000000', 1718236800)); + + const result = await svc.getSynologyAssetInfo(USER, '10314_1793757217', USER); + + expect(result.success).toBe(true); + expect(requestedIds()).toEqual(['[10314]']); + }); + + it('SYNO-U094: asks for the thumbnail with the metadata, the only way to tell which photo answered', async () => { + safeFetch.mockResolvedValueOnce(item('10314_1793757217', 1718236800)); + + await svc.getSynologyAssetInfo(USER, '10314_1793757217', USER); + + const body = new URLSearchParams(String((safeFetch.mock.calls[0][1] as { body: URLSearchParams }).body)); + expect(body.get('additional')).toContain('thumbnail'); + }); +}); + describe('fetchSynologyThumbnailBytes', () => { it('SYNO-U080: fails without credentials rather than fetching', async () => { await seedUser(6, { synology_url: null });