fix: address PR review findings
- Strip integration secrets from server cache keys (security) - Add max size + TTL eviction to in-process cache Map (memory) - Scope widget refresh + cache age to current widget kind (UX) - Remove double caching in update-checker (simplicity) - Simplify timetable router (verbosity) - Fix update-checker import path
This commit is contained in:
6 files changed
+33
-34
No files matched your search
+1
-1
@@ -77,4 +77,4 @@ skills-lock.json
|
||||
# docusaurus
|
||||
apps/docs/.docusaurus
|
||||
apps/docs/build
|
||||
.pnpm-store
|
||||
.pnpm-store
|
||||
@@ -49,9 +49,10 @@ export const WidgetContextMenu = ({ item, widgetStateRef, children }: WidgetCont
|
||||
const { gridstack } = useSectionContext().refs;
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
const widgetQueryKey = useMemo(() => [["widget", item.kind]], [item.kind]);
|
||||
const handleRefetch = useCallback(() => {
|
||||
void queryClient.invalidateQueries({ queryKey: [["widget"]] });
|
||||
}, [queryClient]);
|
||||
void queryClient.invalidateQueries({ queryKey: widgetQueryKey });
|
||||
}, [queryClient, widgetQueryKey]);
|
||||
|
||||
const options = useMemo(
|
||||
() => reduceWidgetOptionsWithDefaultValues(item.kind, settings, item.options) as Record<string, unknown>,
|
||||
@@ -226,7 +227,7 @@ export const WidgetContextMenu = ({ item, widgetStateRef, children }: WidgetCont
|
||||
<Group justify="space-between" wrap="nowrap">
|
||||
{tMenu("refresh")}
|
||||
<Text size="xs" c="dimmed">
|
||||
<WidgetCacheAge queryClient={queryClient} />
|
||||
<WidgetCacheAge queryClient={queryClient} queryKey={widgetQueryKey} />
|
||||
</Text>
|
||||
</Group>
|
||||
</Menu.Item>
|
||||
@@ -264,7 +265,7 @@ export const WidgetContextMenu = ({ item, widgetStateRef, children }: WidgetCont
|
||||
);
|
||||
};
|
||||
|
||||
const WidgetCacheAge = ({ queryClient }: { queryClient: QueryClient }) => {
|
||||
const WidgetCacheAge = ({ queryClient, queryKey }: { queryClient: QueryClient; queryKey: unknown[] }) => {
|
||||
const [, setTick] = useState(0);
|
||||
useEffect(() => {
|
||||
const id = setInterval(() => setTick((n) => n + 1), 1000);
|
||||
@@ -273,12 +274,13 @@ const WidgetCacheAge = ({ queryClient }: { queryClient: QueryClient }) => {
|
||||
|
||||
const timestamps = queryClient
|
||||
.getQueryCache()
|
||||
.findAll({ queryKey: [["widget"]] })
|
||||
.findAll({ queryKey })
|
||||
.map((q) => q.state.dataUpdatedAt)
|
||||
.filter(Boolean);
|
||||
if (timestamps.length === 0) return null;
|
||||
|
||||
const seconds = Math.floor((Date.now() - Math.max(...timestamps)) / 1000);
|
||||
const oldest = Math.min(...timestamps);
|
||||
const seconds = Math.floor((Date.now() - oldest) / 1000);
|
||||
if (seconds < 5) return "just now";
|
||||
if (seconds < 60) return `${seconds}s ago`;
|
||||
return `${Math.floor(seconds / 60)}m ago`;
|
||||
|
||||
@@ -19,14 +19,7 @@ export const timetableRouter = createTRPCRouter({
|
||||
}),
|
||||
)
|
||||
.query(async ({ input }) => {
|
||||
const innerHandler = timetableGetTimetableRequestHandler.handler({
|
||||
baseUrl: input.baseUrl,
|
||||
stationId: input.stationId,
|
||||
limit: input.limit,
|
||||
});
|
||||
|
||||
const { data } = await innerHandler.getDataAsync();
|
||||
|
||||
const { data } = await timetableGetTimetableRequestHandler.handler(input).getDataAsync();
|
||||
return data;
|
||||
}),
|
||||
searchStations: publicProcedure
|
||||
@@ -37,13 +30,7 @@ export const timetableRouter = createTRPCRouter({
|
||||
}),
|
||||
)
|
||||
.query(async ({ input }) => {
|
||||
const innerHandler = timetableSearchStationsRequestHandler.handler({
|
||||
baseUrl: input.baseUrl,
|
||||
query: input.query,
|
||||
});
|
||||
|
||||
const { data } = await innerHandler.getDataAsync();
|
||||
|
||||
const { data } = await timetableSearchStationsRequestHandler.handler(input).getDataAsync();
|
||||
return data;
|
||||
}),
|
||||
});
|
||||
@@ -21,8 +21,10 @@ export const createIntegrationRequestHandler = <
|
||||
>(
|
||||
options: Options<TData, TKind, TInput>,
|
||||
) => ({
|
||||
handler: (integration: IntegrationOfKind<TKind>, itemOptions: TInput) =>
|
||||
createRequestHandler<TData, { options: TInput; integration: IntegrationOfKind<TKind> }>({
|
||||
requestAsync: async (input) => options.requestAsync(input.integration, input.options),
|
||||
}).handler({ options: itemOptions, integration }),
|
||||
handler: (integration: IntegrationOfKind<TKind>, itemOptions: TInput) => {
|
||||
const inner = createRequestHandler<TData, { integrationId: string; options: TInput }>({
|
||||
requestAsync: async (input) => options.requestAsync(integration, input.options),
|
||||
});
|
||||
return inner.handler({ integrationId: integration.id, options: itemOptions });
|
||||
},
|
||||
});
|
||||
@@ -5,11 +5,19 @@ interface Options<TData, TInput extends Record<string, unknown>> {
|
||||
|
||||
type CacheEntry<TData> = { data: TData; timestamp: Date; expiresAt: number };
|
||||
|
||||
const MAX_CACHE_SIZE = 1000;
|
||||
const cache = new Map<string, CacheEntry<unknown>>();
|
||||
const inflight = new Map<string, Promise<CacheEntry<unknown>>>();
|
||||
|
||||
const DEFAULT_TTL_MS = 10_000;
|
||||
|
||||
const evictExpired = () => {
|
||||
const now = Date.now();
|
||||
for (const [key, entry] of cache) {
|
||||
if (now >= entry.expiresAt) cache.delete(key);
|
||||
}
|
||||
};
|
||||
|
||||
export const createRequestHandler = <TData, TInput extends Record<string, unknown>>(
|
||||
options: Options<TData, TInput>,
|
||||
) => ({
|
||||
@@ -22,6 +30,7 @@ export const createRequestHandler = <TData, TInput extends Record<string, unknow
|
||||
if (cached && Date.now() < cached.expiresAt) {
|
||||
return { data: cached.data, timestamp: cached.timestamp };
|
||||
}
|
||||
if (cached) cache.delete(key);
|
||||
|
||||
const existing = inflight.get(key);
|
||||
if (existing) return existing as Promise<CacheEntry<TData>>;
|
||||
@@ -29,6 +38,11 @@ export const createRequestHandler = <TData, TInput extends Record<string, unknow
|
||||
const promise = options
|
||||
.requestAsync(input)
|
||||
.then((data) => {
|
||||
if (cache.size >= MAX_CACHE_SIZE) evictExpired();
|
||||
if (cache.size >= MAX_CACHE_SIZE) {
|
||||
const oldest = cache.keys().next().value;
|
||||
if (oldest) cache.delete(oldest);
|
||||
}
|
||||
const entry: CacheEntry<TData> = { data, timestamp: new Date(), expiresAt: Date.now() + ttl };
|
||||
cache.set(key, entry as CacheEntry<unknown>);
|
||||
inflight.delete(key);
|
||||
|
||||
@@ -5,24 +5,18 @@ import { env } from "@homarr/common/env";
|
||||
import { fetchWithTrustedCertificatesAsync } from "@homarr/core/infrastructure/http";
|
||||
import { createLogger } from "@homarr/core/infrastructure/logs";
|
||||
import { ErrorWithMetadata } from "@homarr/core/infrastructure/logs/error";
|
||||
import { createRequestHandler } from "@homarr/request-handler/lib/request-handler";
|
||||
import { createRequestHandler } from "./lib/request-handler";
|
||||
|
||||
import packageJson from "../../../package.json";
|
||||
|
||||
const logger = createLogger({ module: "updateCheckerRequestHandler" });
|
||||
|
||||
const CACHE_TTL_MS = 30 * 60 * 1000;
|
||||
let cached: { data: Update[]; expiresAt: number } | null = null;
|
||||
|
||||
export const updateCheckerRequestHandler = createRequestHandler({
|
||||
async requestAsync(_) {
|
||||
if (cached && Date.now() < cached.expiresAt) {
|
||||
return { availableUpdates: cached.data };
|
||||
}
|
||||
const availableUpdates = await getAvailableUpdatesAsync(packageJson.version);
|
||||
cached = { data: availableUpdates, expiresAt: Date.now() + CACHE_TTL_MS };
|
||||
return { availableUpdates };
|
||||
},
|
||||
cacheTtlMs: 30 * 60 * 1000,
|
||||
});
|
||||
|
||||
interface Update {
|
||||
|
||||
Reference in new issue
Block a user