From 619cd8f897abfd27bbb8ca6e8ffd80e3c78db5c0 Mon Sep 17 00:00:00 2001 From: Zzz <59008530+ordinary-s@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:52:55 +0800 Subject: [PATCH] fix(redis): honor configured value scan page size --- .../RedisKeyBrowser.infiniteScroll.spec.ts | 114 ++++++++++++------ .../src/components/redis/RedisKeyBrowser.vue | 8 +- 2 files changed, 84 insertions(+), 38 deletions(-) diff --git a/apps/desktop/src/components/redis/RedisKeyBrowser.infiniteScroll.spec.ts b/apps/desktop/src/components/redis/RedisKeyBrowser.infiniteScroll.spec.ts index 2c35127fef..6fa92edbad 100644 --- a/apps/desktop/src/components/redis/RedisKeyBrowser.infiniteScroll.spec.ts +++ b/apps/desktop/src/components/redis/RedisKeyBrowser.infiniteScroll.spec.ts @@ -5,6 +5,7 @@ import { createI18n } from "vue-i18n"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { RedisKeyInfo } from "@/lib/backend/api"; import { defaultRedisKeyGrouping, type RedisKeyGrouping } from "@/lib/redis/redisKeyGrouping"; +import { REDIS_SCAN_PAGE_SIZE_OPTIONS } from "@/lib/redis/redisKeyPattern"; const grouping = ref(); vi.mock("@/lib/redis/redisKeyViewScheduler", () => ({ createRedisKeyViewYield: () => () => new Promise((resolve) => requestAnimationFrame(() => resolve())) })); @@ -368,7 +369,7 @@ function resetApiMocks() { mocks.queryResultMaxRowsEnabled = true; mocks.queryResultMaxRows = 5000; mocks.redisScanKeysBatch.mockResolvedValue({ cursor: 0, keys: [], total_keys: 0 }); - mocks.redisScanValues.mockResolvedValue({ cursor: 0, keys: [], total_keys: 0 }); + mocks.redisScanValues.mockReset().mockResolvedValue({ cursor: 0, keys: [], total_keys: 0 }); } // A short, un-collapsed tree (a real DB has millions of keys folded into a @@ -673,25 +674,85 @@ function clickFetchAll(host: HTMLElement) { } describe("RedisKeyBrowser bounded value search (issue #7779)", () => { - it.each([ + describe.each([ ["value", false], ["all", true], - ] as const)("loads only one initial %s page and clamps COUNT", async (mode, searchBoth) => { - mocks.redisScanPageSize = 5000; - mocks.redisScanValues.mockResolvedValueOnce({ cursor: 41, keys: [], total_keys: 5_000_000 }).mockResolvedValue({ cursor: 0, keys: [keyInfo("unexpected")], total_keys: 0 }); + ] as const)("%s search", (mode, searchBoth) => { + it.each(REDIS_SCAN_PAGE_SIZE_OPTIONS)("uses configured COUNT %i and stops after each explicit action on empty pages", async (pageSize) => { + stubNonOverflowingViewport(); + mocks.redisScanPageSize = pageSize; + mocks.redisScanValues + .mockResolvedValueOnce({ cursor: 41, keys: [], total_keys: 5_000_000 }) + .mockResolvedValueOnce({ cursor: 73, keys: [], total_keys: 0 }) + .mockResolvedValue({ cursor: 0, keys: [keyInfo("unexpected")], total_keys: 0 }); - const host = mountBrowser(); - await settleThoroughly(); - await searchByValue(host, mode); + const host = mountBrowser(); + await settleThoroughly(); + await searchByValue(host, mode); + await settleThoroughly(10); - expect(mocks.redisScanValues).toHaveBeenCalledTimes(1); - expect(mocks.redisScanValues).toHaveBeenLastCalledWith("connection", 0, 0, "*", "needle", 100, searchBoth); - expect(host.textContent).toContain("redis.loadMoreKeys"); + expect(mocks.redisScanValues).toHaveBeenCalledTimes(1); + expect(mocks.redisScanValues).toHaveBeenLastCalledWith("connection", 0, 0, "*", "needle", pageSize, searchBoth); + expect(host.textContent).toContain("redis.loadMoreKeys"); + + clickLoadMore(host); + await settleThoroughly(10); + + expect(mocks.redisScanValues).toHaveBeenCalledTimes(2); + expect(mocks.redisScanValues).toHaveBeenLastCalledWith("connection", 0, 41, "*", "needle", pageSize, searchBoth); + expect(host.textContent).toContain("redis.loadMoreKeys"); + }); + + it.each(REDIS_SCAN_PAGE_SIZE_OPTIONS)("uses configured COUNT %i throughout explicit Fetch all", async (pageSize) => { + mocks.redisScanPageSize = pageSize; + mocks.redisScanValues.mockImplementation((_connectionId: string, _db: number, cursor: number) => { + if (cursor === 0) return Promise.resolve({ cursor: 11, keys: [], total_keys: 5_000_000 }); + if (cursor === 11) return Promise.resolve({ cursor: 12, keys: [], total_keys: 0 }); + return Promise.resolve({ cursor: 0, keys: [keyInfo("last")], total_keys: 0 }); + }); + + const host = mountBrowser(); + await settleThoroughly(); + await searchByValue(host, mode); + expect(mocks.redisScanValues).toHaveBeenCalledTimes(1); + clickFetchAll(host); + await settleThoroughly(); + + expect(mocks.redisScanValues.mock.calls).toEqual([0, 11, 12].map((cursor) => ["connection", 0, cursor, "*", "needle", pageSize, searchBoth])); + expect(host.textContent).toContain("last"); + }); + + it("stops Fetch all after the in-flight configured page and retains its prior cursor", async () => { + mocks.redisScanPageSize = 5000; + const pending = deferred<{ cursor: number; keys: RedisKeyInfo[]; total_keys: number }>(); + mocks.redisScanValues + .mockResolvedValueOnce({ cursor: 41, keys: [], total_keys: 5_000_000 }) + .mockReturnValueOnce(pending.promise) + .mockResolvedValue({ cursor: 0, keys: [keyInfo("resumed")], total_keys: 0 }); + + const host = mountBrowser(); + await settleThoroughly(); + await searchByValue(host, mode); + clickFetchAll(host); + await settleThoroughly(); + expect(mocks.redisScanValues).toHaveBeenCalledTimes(2); + const stop = Array.from(host.querySelectorAll("button")).find((button) => button.textContent?.includes("redis.stopFetchAll") && !button.disabled); + expect(stop, "stop fetch all button").toBeDefined(); + stop!.click(); + pending.resolve({ cursor: 73, keys: [], total_keys: 0 }); + await settleThoroughly(10); + + expect(mocks.redisScanValues).toHaveBeenCalledTimes(2); + clickLoadMore(host); + await settleThoroughly(); + expect(mocks.redisScanValues.mock.calls).toEqual([0, 41, 41].map((cursor) => ["connection", 0, cursor, "*", "needle", 5000, searchBoth])); + expect(host.textContent).toContain("resumed"); + }); }); - it("advances exactly one page per Load more or real scroll and preserves the opaque cursor", async () => { + it.each(["value", "all"] as const)("advances exactly one %s page per Load more or real scroll and preserves the opaque cursor", async (mode) => { stubNonOverflowingViewport(); - mocks.redisScanPageSize = 1000; + mocks.redisScanPageSize = 5000; mocks.redisScanValues.mockImplementation((_connectionId: string, _db: number, cursor: number) => { if (cursor === 0) return Promise.resolve({ cursor: 41, keys: [keyInfo("match")], total_keys: 5_000_000 }); if (cursor === 41) return Promise.resolve({ cursor: 73, keys: [keyInfo("match")], total_keys: 0 }); @@ -700,7 +761,7 @@ describe("RedisKeyBrowser bounded value search (issue #7779)", () => { const host = mountBrowser(); await settleThoroughly(); - await searchByValue(host); + await searchByValue(host, mode); expect(mocks.redisScanValues).toHaveBeenCalledTimes(1); host.querySelector(".redis-key-scroller")?.dispatchEvent(new Event("resize")); @@ -714,29 +775,13 @@ describe("RedisKeyBrowser bounded value search (issue #7779)", () => { host.querySelector(".redis-key-scroller")?.dispatchEvent(new Event("scroll")); await vi.waitFor(() => expect(mocks.redisScanValues).toHaveBeenCalledTimes(3)); + await settleThoroughly(10); + expect(mocks.redisScanValues).toHaveBeenCalledTimes(3); expect(mocks.redisScanValues.mock.calls[2]?.[2]).toBe(73); - expect(mocks.redisScanValues.mock.calls.every((call) => (call[5] as number) <= 100)).toBe(true); + expect(mocks.redisScanValues.mock.calls).toEqual([0, 41, 73].map((cursor) => ["connection", 0, cursor, "*", "needle", 5000, mode === "all"])); expect(host.textContent).toContain("sparse"); }); - it("keeps explicit Fetch all interruptible by clamping every value page to COUNT 100", async () => { - mocks.redisScanPageSize = 10_000; - mocks.redisScanValues.mockImplementation((_connectionId: string, _db: number, cursor: number) => { - if (cursor === 0) return Promise.resolve({ cursor: 11, keys: [], total_keys: 5_000_000 }); - if (cursor === 11) return Promise.resolve({ cursor: 12, keys: [], total_keys: 0 }); - return Promise.resolve({ cursor: 0, keys: [keyInfo("last")], total_keys: 0 }); - }); - - const host = mountBrowser(); - await settleThoroughly(); - await searchByValue(host); - clickFetchAll(host); - await settleThoroughly(); - - expect(mocks.redisScanValues.mock.calls.map((call) => call[2])).toEqual([0, 11, 12]); - expect(mocks.redisScanValues.mock.calls.every((call) => call[5] === 100)).toBe(true); - }); - it("does not retry a rejected value continuation", async () => { stubNonOverflowingViewport(); mocks.redisScanValues.mockResolvedValueOnce({ cursor: 31, keys: [keyInfo("match")], total_keys: 5_000_000 }).mockRejectedValue(new Error("value scan failed")); @@ -756,7 +801,7 @@ describe("RedisKeyBrowser bounded value search (issue #7779)", () => { describe("RedisKeyBrowser KeepAlive empty scan pages (issue #7779)", () => { it.each(["value", "all"] as const)("retains an empty %s scan cursor across tab switches", async (mode) => { - mocks.infiniteScroll = false; + mocks.redisScanPageSize = 5000; mocks.redisScanValues.mockImplementation((_connectionId: string, _db: number, cursor: number) => { if (cursor === 0) return Promise.resolve({ cursor: 41, keys: [], total_keys: 5_000_000 }); if (cursor === 41) return Promise.resolve({ cursor: 73, keys: [], total_keys: 0 }); @@ -776,6 +821,7 @@ describe("RedisKeyBrowser KeepAlive empty scan pages (issue #7779)", () => { clickLoadMore(browser.host); await settleThoroughly(); expect(mocks.redisScanValues.mock.calls.map((call) => call[2])).toEqual([0, 41, 73]); + expect(mocks.redisScanValues.mock.calls.every((call) => call[5] === 5000)).toBe(true); expect(browser.host.textContent).toContain("match"); }); diff --git a/apps/desktop/src/components/redis/RedisKeyBrowser.vue b/apps/desktop/src/components/redis/RedisKeyBrowser.vue index 1b4fda05a3..1cbc1c5b20 100644 --- a/apps/desktop/src/components/redis/RedisKeyBrowser.vue +++ b/apps/desktop/src/components/redis/RedisKeyBrowser.vue @@ -231,7 +231,6 @@ interface ScanIterationBudget { // per-call budget (see `fetchScanPage`); only the automatic follow-up check // they hand off to afterward is constrained by this shared budget. const AUTO_LOAD_TOTAL_SCAN_ITERATIONS = 50; -const REDIS_VALUE_SCAN_COUNT = 100; let autoLoadBudget: ScanIterationBudget = { remaining: AUTO_LOAD_TOTAL_SCAN_ITERATIONS }; let redisBrowserIsActive = true; let reloadKeysOnActivation = false; @@ -840,7 +839,7 @@ function isCurrentScanOperation(requestId: number, operationId?: number): boolea async function fetchScanPage(requestId = searchRequestId, operationId?: number, iterationBudget?: ScanIterationBudget): Promise { const pageSize = redisScanPageSize.value; if (isValueSearchMode.value) { - return api.redisScanValues(props.connectionId, props.db, scanCursor.value, "*", valueQuery.value, Math.min(pageSize, REDIS_VALUE_SCAN_COUNT), searchMode.value === "all"); + return api.redisScanValues(props.connectionId, props.db, scanCursor.value, "*", valueQuery.value, pageSize, searchMode.value === "all"); } // Keep each backend call small so a changed search can cancel between calls. @@ -886,11 +885,12 @@ async function fetchScanPage(requestId = searchRequestId, operationId?: number, /// Batch-scan variant that performs multiple SCAN iterations server-side. /// Dramatically reduces frontend↔backend roundtrips for bulk loading. async function fetchScanBatchPage(maxIterations: number, options: { count?: number; includeTypes?: boolean } = {}): Promise { - const pageSize = options.count ?? redisScanPageSize.value; // Value search cannot be batched because each key requires a GET. + // Keep its configured COUNT independent of the key-only bulk override. if (isValueSearchMode.value) { - return api.redisScanValues(props.connectionId, props.db, scanCursor.value, "*", valueQuery.value, Math.min(pageSize, REDIS_VALUE_SCAN_COUNT), searchMode.value === "all"); + return api.redisScanValues(props.connectionId, props.db, scanCursor.value, "*", valueQuery.value, redisScanPageSize.value, searchMode.value === "all"); } + const pageSize = options.count ?? redisScanPageSize.value; return api.redisScanKeysBatch(props.connectionId, props.db, scanCursor.value, effectivePattern.value, pageSize, maxIterations, options.includeTypes ?? false); }