From 388a74086eb0593f7bba3e1637389b8c7b796e96 Mon Sep 17 00:00:00 2001 From: ajnart Date: Fri, 31 Jul 2026 13:32:48 +0200 Subject: [PATCH] fix: address Copilot cache review --- packages/redis/src/lib/channel.spec.ts | 21 +++++++++++++++++++ packages/redis/src/lib/channel.ts | 10 +++++++-- .../src/test/update-checker-cache.spec.ts | 9 ++++++++ .../request-handler/src/update-checker.ts | 8 +++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 packages/redis/src/lib/channel.spec.ts diff --git a/packages/redis/src/lib/channel.spec.ts b/packages/redis/src/lib/channel.spec.ts new file mode 100644 index 000000000..0ec39a2a5 --- /dev/null +++ b/packages/redis/src/lib/channel.spec.ts @@ -0,0 +1,21 @@ +// @vitest-environment node + +import { describe, expect, test, vi } from "vitest"; + +vi.mock("./connection", () => ({ + createRedisConnection: () => null, +})); + +import { createLockChannel } from "./channel"; + +describe("createLockChannel", () => { + test("degrades gracefully when Redis connections are disabled", async () => { + const lock = createLockChannel("test-lock"); + + const token = await lock.acquireAsync(60); + + expect(token).toEqual(expect.any(String)); + if (!token) throw new Error("Expected the fallback lock to return a token"); + await expect(lock.releaseAsync(token)).resolves.toBeUndefined(); + }); +}); diff --git a/packages/redis/src/lib/channel.ts b/packages/redis/src/lib/channel.ts index 6f5771e83..f388fa094 100644 --- a/packages/redis/src/lib/channel.ts +++ b/packages/redis/src/lib/channel.ts @@ -139,11 +139,17 @@ export const createLockChannel = (name: string) => { return { acquireAsync: async (ttlSeconds: number) => { const token = createId(); - const result = await getSetClient.set(name, token, "EX", ttlSeconds, "NX"); + const client = getSetClient as typeof getSetClient | null; + if (!client) return token; + + const result = await client.set(name, token, "EX", ttlSeconds, "NX"); return result === "OK" ? token : null; }, releaseAsync: async (token: string) => { - await getSetClient.eval( + const client = getSetClient as typeof getSetClient | null; + if (!client) return; + + await client.eval( "if redis.call('get', KEYS[1]) == ARGV[1] then return redis.call('del', KEYS[1]) else return 0 end", 1, name, diff --git a/packages/request-handler/src/test/update-checker-cache.spec.ts b/packages/request-handler/src/test/update-checker-cache.spec.ts index 4144348ff..b88038f59 100644 --- a/packages/request-handler/src/test/update-checker-cache.spec.ts +++ b/packages/request-handler/src/test/update-checker-cache.spec.ts @@ -11,10 +11,12 @@ const mocks = vi.hoisted(() => { const values = new Map(); const locks = new Set(); const listReleases = vi.fn(); + const redisOperation = vi.fn(); return { env: { NO_EXTERNAL_CONNECTION: false }, listReleases, + redisOperation, values, locks, removeFreshResult() { @@ -28,6 +30,7 @@ vi.mock("@homarr/common/env", () => ({ env: mocks.env })); vi.mock("@homarr/redis", () => ({ createGetSetChannel: (name: string) => ({ getAsync: async () => { + mocks.redisOperation(); const stored = mocks.values.get(name); if (!stored) return null; if (stored.expiresAt !== null && Date.now() >= stored.expiresAt) { @@ -37,22 +40,26 @@ vi.mock("@homarr/redis", () => ({ return stored.value; }, setAsync: async (value: unknown, options?: { ttlSeconds?: number }) => { + mocks.redisOperation(); mocks.values.set(name, { value, expiresAt: options?.ttlSeconds ? Date.now() + options.ttlSeconds * 1_000 : null, }); }, removeAsync: async () => { + mocks.redisOperation(); mocks.values.delete(name); }, }), createLockChannel: (name: string) => ({ acquireAsync: async () => { + mocks.redisOperation(); if (mocks.locks.has(name)) return null; mocks.locks.add(name); return "test-lock-token"; }, releaseAsync: async () => { + mocks.redisOperation(); mocks.locks.delete(name); }, }), @@ -94,6 +101,7 @@ describe("persisted update checker cache", () => { vi.restoreAllMocks(); mocks.env.NO_EXTERNAL_CONNECTION = false; mocks.listReleases.mockReset(); + mocks.redisOperation.mockReset(); mocks.values.clear(); mocks.locks.clear(); }); @@ -202,5 +210,6 @@ describe("persisted update checker cache", () => { await expect(handler.getDataAsync()).resolves.toMatchObject({ data: { availableUpdates: [] } }); expect(mocks.listReleases).not.toHaveBeenCalled(); + expect(mocks.redisOperation).not.toHaveBeenCalled(); }); }); diff --git a/packages/request-handler/src/update-checker.ts b/packages/request-handler/src/update-checker.ts index c3f010eec..8ba5389f6 100644 --- a/packages/request-handler/src/update-checker.ts +++ b/packages/request-handler/src/update-checker.ts @@ -42,6 +42,14 @@ const waitForConcurrentUpdateCheckAsync = async () => { }; const getCachedAvailableUpdatesAsync = async (): Promise => { + if (env.NO_EXTERNAL_CONNECTION) { + return { + availableUpdates: [], + attemptedAt: Date.now(), + checkedAt: null, + }; + } + const cached = await freshUpdateCheckChannel.getAsync(); if (cached) return cached;