RHL-022 refactor(storage): clean up comments and improve directory listing filters

lib/storage.js

@@ -1,28 +1,5 @@
-// ---------------------------------------------------------------------------
-// Ordner: lib
-// Datei: storage.js
-// Relativer Pfad: lib/storage.js
-// ---------------------------------------------------------------------------
-// lib/storage.js
-// -----------------------------------------------------------------------------
-// Central abstraction layer for reading files and directories from the NAS
-// share mounted at `NAS_ROOT_PATH` (e.g. `/mnt/niederlassungen`).
-//
-// All access to the branch/year/month/day/PDF structure should go through
-// these functions instead of using `fs` directly in route handlers.
-//
-// - Read-only: no write/delete operations here.
-// - Async only: uses `fs/promises` + async/await to avoid blocking the event loop.
-//
-// RHL-006 (Caching / Freshness):
-// - We add a small process-local TTL micro-cache for directory listings.
-// - Goal: reduce filesystem load while keeping freshness predictable.
-// - Security note: RBAC is enforced in API routes BEFORE calling storage helpers,
-//   therefore caching here does not bypass auth/permissions.
-// -----------------------------------------------------------------------------
-
-import fs from "node:fs/promises"; // Promise-based filesystem API
-import path from "node:path"; // Safe path utilities (handles separators)
+import fs from "node:fs/promises";
+import path from "node:path";
 
 // Root directory of the NAS share, injected via environment variable.
 // On the Linux app server, this is typically `/mnt/niederlassungen`.
@@ -61,22 +38,7 @@ function sortNumericStrings(a, b) {
 // -----------------------------------------------------------------------------
 // RHL-006: Storage micro-cache (process-local TTL cache)
 // -----------------------------------------------------------------------------
-//
-// Why a cache here (and not Next route caching)?
-// - We want to avoid any risk of shared caching across users/sessions.
-// - Next route caching / ISR-style caching is powerful but easy to misuse with auth.
-// - A micro-cache AFTER RBAC checks is safe and predictable.
-//
-// Important constraints:
-// - Process-local only: if we ever run multiple instances, caches are not shared.
-// - Short TTL only: we accept a small window where newly scanned PDFs might not
-//   appear immediately, but they will appear after TTL expires.
-// - Failure-safe: if a filesystem read throws, we do NOT keep a "poisoned" cache entry.
-// -----------------------------------------------------------------------------
 
-// TTLs chosen in the design (accepted by you):
-// - Branches/Years change rarely -> 60 seconds
-// - Months/Days/Files can change frequently -> 15 seconds
 const TTL_BRANCHES_MS = 60_000;
 const TTL_YEARS_MS = 60_000;
 const TTL_MONTHS_MS = 15_000;
@@ -126,8 +88,6 @@ async function withTtlCache(key, ttlMs, loader) {
 	const existing = __storageCache.get(key);
 
 	// 1) Collapsing concurrent calls:
-	// If another request already triggered the same filesystem read,
-	// we reuse the same promise to avoid redundant fs operations.
 	if (existing?.promise) {
 		return existing.promise;
 	}
@@ -142,7 +102,6 @@ async function withTtlCache(key, ttlMs, loader) {
 		try {
 			const value = await loader();
 
-			// Store resolved value with a fresh expiry timestamp.
 			__storageCache.set(key, {
 				value,
 				expiresAt: Date.now() + ttlMs,
@@ -150,13 +109,11 @@ async function withTtlCache(key, ttlMs, loader) {
 
 			return value;
 		} catch (err) {
-			// Do not keep failed results in cache.
 			__storageCache.delete(key);
 			throw err;
 		}
 	})();
 
-	// Store in-flight promise immediately so concurrent calls reuse it.
 	__storageCache.set(key, {
 		promise,
 		expiresAt: now + ttlMs,
@@ -167,12 +124,6 @@ async function withTtlCache(key, ttlMs, loader) {
 
 /**
  * TEST-ONLY helper: clear the micro-cache.
- *
- * Why this exists:
- * - Unit tests often mutate the filesystem fixture after calling list*() once.
- * - Without a cache reset, tests could observe stale values.
- *
- * We intentionally export this with a loud name to discourage production usage.
  */
 export function __clearStorageCacheForTests() {
 	__storageCache.clear();
@@ -180,41 +131,29 @@ export function __clearStorageCacheForTests() {
 
 // -----------------------------------------------------------------------------
 // 1. Branches (NL01, NL02, ...)
-// Path pattern: `${ROOT}/NLxx`
 // -----------------------------------------------------------------------------
 
 export async function listBranches() {
-	// RHL-006: cache directory listing for 60 seconds (branches change rarely).
 	return withTtlCache(buildCacheKey("branches"), TTL_BRANCHES_MS, async () => {
-		// Read the root directory of the NAS share.
-		// `withFileTypes: true` returns `Dirent` objects so we can call `isDirectory()`
-		// without extra stat() calls, which is more efficient.
 		const entries = await fs.readdir(fullPath(), { withFileTypes: true });
 
-		return (
-			entries
-				.filter(
-					(entry) =>
-						entry.isDirectory() && // only directories
-						entry.name !== "@Recently-Snapshot" && // ignore QNAP snapshot folder
-						/^NL\d+$/i.test(entry.name) // keep only names like "NL01", "NL02", ...
-				)
-				.map((entry) => entry.name)
-				// Sort by numeric branch number: NL1, NL2, ..., NL10
-				.sort((a, b) =>
-					sortNumericStrings(a.replace("NL", ""), b.replace("NL", ""))
-				)
-		);
+		return entries
+			.filter(
+				(entry) =>
+					entry.isDirectory() &&
+					entry.name !== "@Recently-Snapshot" &&
+					/^NL\d+$/.test(entry.name) // strict: UI expects "NL" uppercase, Linux FS is case-sensitive
+			)
+			.map((entry) => entry.name)
+			.sort((a, b) => sortNumericStrings(a.slice(2), b.slice(2)));
 	});
 }
 
 // -----------------------------------------------------------------------------
 // 2. Years (2023, 2024, ...)
-// Path pattern: `${ROOT}/${branch}/${year}`
 // -----------------------------------------------------------------------------
 
 export async function listYears(branch) {
-	// RHL-006: cache directory listing for 60 seconds (years change rarely).
 	return withTtlCache(
 		buildCacheKey("years", branch),
 		TTL_YEARS_MS,
@@ -223,9 +162,7 @@ export async function listYears(branch) {
 			const entries = await fs.readdir(dir, { withFileTypes: true });
 
 			return entries
-				.filter(
-					(entry) => entry.isDirectory() && /^\d{4}$/.test(entry.name) // exactly 4 digits → year folders like "2024"
-				)
+				.filter((entry) => entry.isDirectory() && /^\d{4}$/.test(entry.name))
 				.map((entry) => entry.name)
 				.sort(sortNumericStrings);
 		}
@@ -234,11 +171,9 @@ export async function listYears(branch) {
 
 // -----------------------------------------------------------------------------
 // 3. Months (01–12)
-// Path pattern: `${ROOT}/${branch}/${year}/${month}`
 // -----------------------------------------------------------------------------
 
 export async function listMonths(branch, year) {
-	// RHL-006: cache directory listing for 15 seconds (months can change occasionally).
 	return withTtlCache(
 		buildCacheKey("months", branch, year),
 		TTL_MONTHS_MS,
@@ -246,26 +181,26 @@ export async function listMonths(branch, year) {
 			const dir = fullPath(branch, year);
 			const entries = await fs.readdir(dir, { withFileTypes: true });
 
-			return (
-				entries
-					.filter(
-						(entry) => entry.isDirectory() && /^\d{1,2}$/.test(entry.name) // supports "1" or "10", we normalize below
-					)
-					// Normalize to two digits so the UI shows "01", "02", ..., "12"
-					.map((entry) => entry.name.trim().padStart(2, "0"))
-					.sort(sortNumericStrings)
-			);
+			// Important: filter to valid calendar months (1–12) so the UI never gets
+			// impossible values that would be rejected by route param validation.
+			return entries
+				.filter((entry) => entry.isDirectory() && /^\d{1,2}$/.test(entry.name))
+				.map((entry) => entry.name.trim())
+				.filter((raw) => {
+					const n = parseInt(raw, 10);
+					return Number.isInteger(n) && n >= 1 && n <= 12;
+				})
+				.map((raw) => raw.padStart(2, "0"))
+				.sort(sortNumericStrings);
 		}
 	);
 }
 
 // -----------------------------------------------------------------------------
 // 4. Days (01–31)
-// Path pattern: `${ROOT}/${branch}/${year}/${month}/${day}`
 // -----------------------------------------------------------------------------
 
 export async function listDays(branch, year, month) {
-	// RHL-006: cache directory listing for 15 seconds (days change frequently with new scans).
 	return withTtlCache(
 		buildCacheKey("days", branch, year, month),
 		TTL_DAYS_MS,
@@ -273,11 +208,15 @@ export async function listDays(branch, year, month) {
 			const dir = fullPath(branch, year, month);
 			const entries = await fs.readdir(dir, { withFileTypes: true });
 
+			// Important: filter to valid calendar days (1–31) to align with UI param validation.
 			return entries
-				.filter(
-					(entry) => entry.isDirectory() && /^\d{1,2}$/.test(entry.name) // supports "1" or "23"
-				)
-				.map((entry) => entry.name.trim().padStart(2, "0"))
+				.filter((entry) => entry.isDirectory() && /^\d{1,2}$/.test(entry.name))
+				.map((entry) => entry.name.trim())
+				.filter((raw) => {
+					const n = parseInt(raw, 10);
+					return Number.isInteger(n) && n >= 1 && n <= 31;
+				})
+				.map((raw) => raw.padStart(2, "0"))
 				.sort(sortNumericStrings);
 		}
 	);
@@ -285,11 +224,9 @@ export async function listDays(branch, year, month) {
 
 // -----------------------------------------------------------------------------
 // 5. Files (PDFs) for a given day
-// Path pattern: `${ROOT}/${branch}/${year}/${month}/${day}/<file>.pdf`
 // -----------------------------------------------------------------------------
 
 export async function listFiles(branch, year, month, day) {
-	// RHL-006: cache file listing for 15 seconds (new PDFs can appear at any time).
 	return withTtlCache(
 		buildCacheKey("files", branch, year, month, day),
 		TTL_FILES_MS,
@@ -297,20 +234,13 @@ export async function listFiles(branch, year, month, day) {
 			const dir = fullPath(branch, year, month, day);
 			const entries = await fs.readdir(dir);
 
-			return (
-				entries
-					// We only care about PDF files at the moment
-					.filter((name) => name.toLowerCase().endsWith(".pdf"))
-					.sort((a, b) => a.localeCompare(b, "en"))
-					.map((name) => ({
-						// Just the file name, e.g. "Stapel-1_Seiten-1_Zeit-1048.pdf"
-						name,
-						// Relative path from the NAS root, used for download URLs etc.
-						// Example: "NL01/2024/10/23/Stapel-1_Seiten-1_Zeit-1048.pdf"
-						relativePath: `${branch}/${year}/${month}/${day}/${name}`,
-					}))
-			);
+			return entries
+				.filter((name) => name.toLowerCase().endsWith(".pdf"))
+				.sort((a, b) => a.localeCompare(b, "en"))
+				.map((name) => ({
+					name,
+					relativePath: `${branch}/${year}/${month}/${day}/${name}`,
+				}));
 		}
 	);
 }
-// ---------------------------------------------------------------------------