From f03011e0ecd34e46e2e7046e7bf64ce4cb4f0876 Mon Sep 17 00:00:00 2001 From: Ralksta <16941050+ralksta@users.noreply.github.com> Date: Wed, 5 Aug 2026 00:27:06 +0200 Subject: [PATCH] fix: gallery.yaml album order on homepage + stale settings cache (#391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: honour gallery.yaml order for homepage albums getStandaloneAlbums() filtered the Immich API album list but kept the API's own (creation/update) order, so reordering the albums list in gallery.yaml or the admin panel had no effect on the homepage. Sort by the config list's index instead. Fixes #352 Co-Authored-By: Claude Fable 5 * fix: serve fresh settings after admin save without restart getConfig() short-circuited on a per-worker in-memory cache in production. Admin saves invalidate that cache only in the worker that handles the API request; when Next.js renders pages in a different worker/process, its copy stayed stale until a container restart. Drop the in-memory config cache entirely and rebuild the config per call. Freshness comes from the existing mtime-checked YAML cache in loadYaml() — one statSync per file, negligible next to Immich API calls, and the same behaviour dev mode has always had. Fixes #351 Co-Authored-By: Claude Fable 5 --------- Co-authored-by: ralksta Co-authored-by: Claude Fable 5 --- lib/__tests__/config-cache.test.ts | 62 ++++++++++++++++++++++++++++++ lib/__tests__/immich.test.ts | 20 +++++++++- lib/config/index.ts | 21 ++++------ lib/immich.ts | 7 +++- 4 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 lib/__tests__/config-cache.test.ts diff --git a/lib/__tests__/config-cache.test.ts b/lib/__tests__/config-cache.test.ts new file mode 100644 index 0000000..2b0c553 --- /dev/null +++ b/lib/__tests__/config-cache.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +// Mock env.ts so importing config.ts doesn't trigger Zod validation +vi.mock('@/lib/env', () => ({ + env: { + IMMICH_API_URL: 'http://localhost:2283', + IMMICH_API_KEY: 'test-key', + SITE_TITLE: 'Test Gallery', + SITE_SUBTITLE: '', + CACHE_TTL: 300, + RATE_LIMIT_RPM: 120, + }, +})); + +vi.mock('@/lib/secret', () => ({ + resolveAuthSecret: () => 'test-auth-secret-32-chars-long-min', +})); + +// Replace the filesystem-backed YAML layer. The real loadYaml() already +// revalidates by mtime, so "the file changed on disk" is modelled by the +// mock returning different content. +vi.mock('@/lib/config/parser', () => ({ + loadYaml: vi.fn(), + clearYamlCache: vi.fn(), + validateUuid: (id: string) => id, +})); + +import { getConfig, invalidateConfigCache } from '@/lib/config'; +import { loadYaml } from '@/lib/config/parser'; + +const ALBUM_ID = '11111111-1111-1111-1111-111111111111'; + +function mockYamlFiles(settings: Record) { + vi.mocked(loadYaml).mockImplementation((filename: string) => { + if (filename === 'gallery.yaml') return { albums: [ALBUM_ID] }; + if (filename === 'settings.yaml') return settings; + return null; + }); +} + +describe('getConfig() in production', () => { + beforeEach(() => { + vi.stubEnv('NODE_ENV', 'production'); + invalidateConfigCache(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('picks up settings.yaml changes without invalidateConfigCache()', () => { + // An admin save may run in a different worker/process than page + // rendering, so this worker never sees invalidateConfigCache(). + // getConfig() must delegate freshness to the mtime-checked YAML layer + // instead of short-circuiting on a stale in-memory copy. + mockYamlFiles({ title: 'Old Title' }); + expect(getConfig().siteTitle).toBe('Old Title'); + + mockYamlFiles({ title: 'New Title' }); + expect(getConfig().siteTitle).toBe('New Title'); + }); +}); diff --git a/lib/__tests__/immich.test.ts b/lib/__tests__/immich.test.ts index a59be5e..e36844e 100644 --- a/lib/__tests__/immich.test.ts +++ b/lib/__tests__/immich.test.ts @@ -12,7 +12,7 @@ vi.mock('../config', async () => { immich: { apiUrl: 'http://immich.test/api', apiKey: 'test-key' }, authSecret: 'test-auth-secret-32-chars-long-min', albums: ['album-1', 'album-2'], - standaloneAlbums: ['album-1'], + standaloneAlbums: ['album-2', 'album-1'], subpages: [], albumOverrides: { 'album-1': 'Override Name' }, albumDescriptions: {}, @@ -78,6 +78,24 @@ describe('ImmichClient', () => { }); }); + describe('getStandaloneAlbums()', () => { + it('orders albums by gallery.yaml order, not Immich API order', async () => { + // The Immich API returns albums in its own (creation/update) order; + // the config lists album-2 first, so that must win on the homepage. + mockFetch.mockResolvedValueOnce({ + ok: true, + headers: { get: () => 'application/json' }, + json: async () => [ + { id: 'album-1', albumName: 'First in API', assetCount: 1 }, + { id: 'album-2', albumName: 'Second in API', assetCount: 1 }, + ], + }); + + const albums = await immich.getStandaloneAlbums(); + expect(albums.map((a) => a.id)).toEqual(['album-2', 'album-1']); + }); + }); + describe('getAlbum()', () => { it('returns null for unallowed album IDs', async () => { const album = await immich.getAlbum('album-3'); diff --git a/lib/config/index.ts b/lib/config/index.ts index 0b8d326..bd47dd7 100644 --- a/lib/config/index.ts +++ b/lib/config/index.ts @@ -16,11 +16,8 @@ import { export * from './schema'; export * from './theme'; -let _config: AppConfig | null = null; - -/** Invalidate the cached config so the next getConfig() call re-reads YAML files. */ +/** Invalidate the cached YAML so the next getConfig() call re-parses the files. */ export function invalidateConfigCache(): void { - _config = null; clearYamlCache(); } @@ -49,12 +46,10 @@ export function buildSubpageGrid(raw?: { } export function getConfig(): AppConfig { - // _config is a per-worker in-memory cache. - // invalidateConfigCache() clears it + the underlying YAML mtime cache, - // so after an admin save every worker re-parses on next request. - // In dev we always re-parse so hot-reload works. - if (_config && process.env.NODE_ENV === 'production') return _config; - + // No in-memory config cache: admin saves can land in a different + // worker/process than page rendering, so a per-worker cache goes stale + // until restart. Freshness comes from the mtime-checked YAML cache in + // loadYaml() — a statSync per file, negligible next to Immich API calls. const apiUrl = env.IMMICH_API_URL; const apiKey = env.IMMICH_API_KEY; const authSecret = resolveAuthSecret(); @@ -64,7 +59,7 @@ export function getConfig(): AppConfig { if (!gallery || !apiKey || !apiUrl) { // Return dummy config if gallery.yaml is missing - _config = { + return { immich: { apiUrl: `${apiUrl}/api`, apiKey }, authSecret, albums: [], @@ -96,7 +91,6 @@ export function getConfig(): AppConfig { trustedProxyHops: env.TRUSTED_PROXY_HOPS, needsSetup: true, }; - return _config; } const theme = resolveTheme(settings.theme); @@ -215,7 +209,7 @@ export function getConfig(): AppConfig { const subpageAlbumIds = new Set(subpages.flatMap((sp) => sp.albumIds)); const allAlbumIds = [...new Set([...standaloneAlbumIds, ...subpageAlbumIds])]; - _config = { + return { immich: { apiUrl: `${apiUrl}/api`, apiKey }, authSecret, albums: allAlbumIds, @@ -280,5 +274,4 @@ export function getConfig(): AppConfig { trustedProxyHops: env.TRUSTED_PROXY_HOPS, needsSetup: false, }; - return _config; } diff --git a/lib/immich.ts b/lib/immich.ts index 8cff3a2..e6051b5 100644 --- a/lib/immich.ts +++ b/lib/immich.ts @@ -276,8 +276,11 @@ class ImmichClient { */ async getStandaloneAlbums(): Promise { const albums = await this.getAlbums(); - const standaloneIds = new Set(this.config.standaloneAlbums); - return albums.filter((a) => standaloneIds.has(a.id)); + // The API returns albums in its own order; gallery.yaml order wins. + const orderIdx = new Map(this.config.standaloneAlbums.map((id, i) => [id, i])); + return albums + .filter((a) => orderIdx.has(a.id)) + .sort((a, b) => orderIdx.get(a.id)! - orderIdx.get(b.id)!); } /**