From ee936d6e8846e1b321b6465643ebe8a57791d50b Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 13 Jul 2026 16:28:17 +0200 Subject: [PATCH 1/7] Fix hybrid content queries with multi-parent server-side index fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split `hybridQuery` content requests using `parentId: {$in: [...]}` into individual `parentId` equality queries pinned to `content-parentId-publishDate-index`, then merge, sort, and limit their results server-side. This avoids Mango’s full content-partition scan caused by combining `$in` with a `publishDate` sort. Add query-service coverage for the fan-out behavior and document why clients may use the above-cap multi-parent query path now that the API can execute it efficiently. --- api/src/endpoints/query.service.spec.ts | 119 +++++++++++++++++- api/src/endpoints/query.service.ts | 115 ++++++++++++++++- .../components/ExplorePage/PinnedTopics.vue | 6 +- .../components/HomePage/HomePagePinned.vue | 6 +- app/src/components/VideoPage/PinnedVideo.vue | 6 +- .../util/HybridQuery/queryIntrospection.ts | 8 +- 6 files changed, 243 insertions(+), 17 deletions(-) diff --git a/api/src/endpoints/query.service.spec.ts b/api/src/endpoints/query.service.spec.ts index 295f458810..9d6a12de61 100644 --- a/api/src/endpoints/query.service.spec.ts +++ b/api/src/endpoints/query.service.spec.ts @@ -12,7 +12,10 @@ import * as permissions from "../permissions/permissions.service"; describe("QueryService", () => { let service: QueryService; - let dbService: { executeFindQuery: jest.Mock; on: jest.Mock }; + let dbService: { + executeFindQuery: jest.Mock; + on: jest.Mock; + }; let logger: Logger; const mockUser = { @@ -515,9 +518,7 @@ describe("QueryService", () => { // Expiry date filter must NOT be present const hasExpiryFilter = sel.$and.some( - (c: any) => - c.$or && - c.$or.some((o: any) => o.expiryDate !== undefined), + (c: any) => c.$or && c.$or.some((o: any) => o.expiryDate !== undefined), ); expect(hasExpiryFilter).toBe(false); @@ -801,6 +802,116 @@ describe("QueryService", () => { }); }); + describe("parentId $in scatter-gather", () => { + function allowContent() { + const access = { + [DocType.Post]: ["gp1"], + [DocType.Tag]: ["gt1"], + [DocType.Language]: ["lang-g1"], + } as any; + (permissions.PermissionSystem.accessMapToGroups as jest.Mock).mockReturnValueOnce( + access, + ); + (service as any).languages = [{ _id: "lang-eng", memberOf: ["lang-g1"] }]; + } + + it("executes one indexed equality query per parent, then merges, sorts, and limits", async () => { + allowContent(); + dbService.executeFindQuery + .mockResolvedValueOnce({ + docs: [ + { _id: "c1", publishDate: 100, updatedTimeUtc: 1 }, + { _id: "c2", publishDate: 300, updatedTimeUtc: 3 }, + ], + execution_stats: { + total_keys_examined: 2, + total_docs_examined: 2, + execution_time_ms: 4, + }, + }) + .mockResolvedValueOnce({ + docs: [ + { _id: "c2", publishDate: 300, updatedTimeUtc: 3 }, + { _id: "c3", publishDate: 200, updatedTimeUtc: 2 }, + ], + execution_stats: { + total_keys_examined: 3, + total_docs_examined: 3, + execution_time_ms: 5, + }, + }); + + const query = makeQuery((s) => { + (s as any).type = DocType.Content; + (s as any).parentId = { $in: ["p2", "p1", "p2"] }; + }); + query.sort = [{ publishDate: "desc" }]; + query.limit = 2; + (query as any).use_index = "content-publishDate-index"; + + const result = await service.query(query, mockUser); + + expect(dbService.executeFindQuery).toHaveBeenCalledTimes(2); + const executed = dbService.executeFindQuery.mock.calls.map(([call]) => call); + expect(executed.map((call) => call.use_index)).toEqual([ + "content-parentId-publishDate-index", + "content-parentId-publishDate-index", + ]); + expect(executed.map((call) => call.selector.$and.find((c: any) => c.parentId)?.parentId)).toEqual([ + "p2", + "p1", + ]); + expect(executed.map((call) => call.sort)).toEqual([ + [{ publishDate: "desc" }], + [{ publishDate: "desc" }], + ]); + expect(executed.map((call) => call.limit)).toEqual([2, 2]); + expect(result.docs.map((doc) => doc._id)).toEqual(["c2", "c3"]); + expect(result.execution_stats).toEqual({ + total_keys_examined: 5, + total_docs_examined: 5, + execution_time_ms: 9, + results_returned: 2, + }); + expect(result.blockStart).toBe(3); + expect(result.blockEnd).toBe(2); + }); + + it("short-circuits an empty parentId list without touching CouchDB", async () => { + allowContent(); + const query = makeQuery((s) => { + (s as any).type = DocType.Content; + (s as any).parentId = { $in: [] }; + }); + + const result = await service.query(query, mockUser); + + expect(result.docs).toEqual([]); + expect(dbService.executeFindQuery).not.toHaveBeenCalled(); + }); + + it("passes a content query without parentId $in through unchanged", async () => { + allowContent(); + const query = makeQuery((s) => { + (s as any).type = DocType.Content; + }); + query.sort = [{ publishDate: "desc" }]; + query.limit = 2; + (query as any).use_index = "content-publishDate-index"; + + await service.query(query, mockUser); + + expect(dbService.executeFindQuery).toHaveBeenCalledTimes(1); + expect(dbService.executeFindQuery).toHaveBeenCalledWith( + expect.objectContaining({ + sort: [{ publishDate: "desc" }], + limit: 2, + use_index: "content-publishDate-index", + }), + ); + }); + }); + describe("language cache clearing on DB disconnect", () => { it("clears the languages cache on disconnect and refetches on reconnect", async () => { const { EventEmitter } = await import("node:events"); diff --git a/api/src/endpoints/query.service.ts b/api/src/endpoints/query.service.ts index d5ca178e58..8b6089cb9c 100644 --- a/api/src/endpoints/query.service.ts +++ b/api/src/endpoints/query.service.ts @@ -271,7 +271,7 @@ export class QueryService { // only knowable post-hoc). Set here, not in executeFindQuery, so the auth / // languages / search callers of that method are unaffected. (query as any).execution_stats = true; - const result = await this.db.executeFindQuery(query); + const result = await this.executeQuery(query, type as DocType); // Data minimization (covers both sync and HybridQuery — both POST /query): a non-CMS // response can only contain an expired Content doc via the app's includeExpired update-sync, @@ -285,6 +285,119 @@ export class QueryService { } return result; } + + /** + * A publishDate-led index must scan the Content partition before applying parentId, + * and no index can serve `parentId: { $in: [...] }` with a global publishDate sort. + * Fan out to per-parent index seeks and merge their results instead. + */ + private async executeQuery(query: MongoQueryDto, type: DocType): Promise { + if (type !== DocType.Content) return this.db.executeFindQuery(query); + + const parentIdCriteria = findParentIdIn(query.selector.$and || []); + if (!parentIdCriteria) return this.db.executeFindQuery(query); + + const rawIds = parentIdCriteria.$in; + if (!Array.isArray(rawIds)) return this.db.executeFindQuery(query); + if (rawIds.some((id) => typeof id !== "string")) { + throw new HttpException( + "'parentId.$in' values must be strings", + HttpStatus.BAD_REQUEST, + ); + } + + const parentIds = [...new Set(rawIds as string[])]; + if (parentIds.length === 0) return { docs: [], blockStart: 0, blockEnd: 0 }; + + // ponytail: unbounded fan-out; add a concurrency cap if a parent set ever gets large + const results = await Promise.all( + parentIds.map((id) => + this.db.executeFindQuery({ + ...query, + selector: { + $and: (query.selector.$and || []).map((condition) => + condition.parentId === parentIdCriteria ? { parentId: id } : condition, + ), + }, + use_index: "content-parentId-publishDate-index", + }), + ), + ); + + const seen = new Set(); + const docs = applySortAndLimit( + results + .flatMap((result) => result.docs || []) + .filter((doc) => !seen.has(doc._id) && (seen.add(doc._id), true)), + query.sort, + query.limit, + ); + const result: DbQueryResult = { + docs, + execution_stats: { + total_keys_examined: results.reduce( + (total, item) => total + (item.execution_stats?.total_keys_examined ?? 0), + 0, + ), + total_docs_examined: results.reduce( + (total, item) => total + (item.execution_stats?.total_docs_examined ?? 0), + 0, + ), + execution_time_ms: results.reduce( + (total, item) => total + (item.execution_stats?.execution_time_ms ?? 0), + 0, + ), + results_returned: docs.length, + }, + }; + setBlockRange(result); + return result; + } +} + +function findParentIdIn(and: MongoSelectorDto[]): MongoComparisonCriteria | undefined { + for (const condition of and) { + const value = condition.parentId; + if (value && typeof value === "object" && !Array.isArray(value) && "$in" in value) { + return value as MongoComparisonCriteria; + } + } + return undefined; +} + +function applySortAndLimit( + docs: any[], + sort: MongoQueryDto["sort"], + limit: number | undefined, +): any[] { + let result = docs; + if (sort?.length) { + const [field, direction] = Object.entries(sort[0] || {})[0] || []; + if (field) { + const desc = direction === "desc"; + result = docs.slice().sort((a, b) => { + const av = a?.[field]; + const bv = b?.[field]; + let cmp = 0; + if (av == null && bv != null) cmp = -1; + else if (av != null && bv == null) cmp = 1; + else if (av < bv) cmp = -1; + else if (av > bv) cmp = 1; + if (desc) cmp = -cmp; + if (cmp !== 0) return cmp; + return String(a?._id ?? "").localeCompare(String(b?._id ?? "")); + }); + } + } + return typeof limit === "number" ? result.slice(0, Math.max(0, limit)) : result; +} + +function setBlockRange(result: DbQueryResult): void { + const times = result.docs + .map((doc) => doc?.updatedTimeUtc) + .filter((value): value is number => typeof value === "number"); + result.blockStart = times.length ? Math.max(...times) : 0; + result.blockEnd = times.length ? Math.min(...times) : 0; } /** diff --git a/app/src/components/ExplorePage/PinnedTopics.vue b/app/src/components/ExplorePage/PinnedTopics.vue index 796c6dfec4..4f12fbbb66 100644 --- a/app/src/components/ExplorePage/PinnedTopics.vue +++ b/app/src/components/ExplorePage/PinnedTopics.vue @@ -28,9 +28,9 @@ const topics = useContentQuery( // This indexes the local Dexie read and lets the API older-tail supplement fan out to // per-parent index seeks when the combined parentId set is within the fan-out cap. // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); when the combined set exceeds the cap the - // supplement falls back to a content-partition scan. sort+limit bound the window; - // contentByTag re-sorts per category for display. + // it (it is not in the local window); above the client fan-out cap the API resolves the + // single request by fanning out to per-parent index seeks server-side. + // sort+limit bound the window; contentByTag re-sorts per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/app/src/components/HomePage/HomePagePinned.vue b/app/src/components/HomePage/HomePagePinned.vue index 4d0b71d749..85833aab2d 100644 --- a/app/src/components/HomePage/HomePagePinned.vue +++ b/app/src/components/HomePage/HomePagePinned.vue @@ -35,9 +35,9 @@ const pinnedCategoryContent = useContentQuery( // This indexes the local Dexie read and lets the API older-tail supplement fan out to // per-parent index seeks when the combined parentId set is within the fan-out cap. // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); when the combined set exceeds the cap the - // supplement falls back to a content-partition scan. sort+limit bound the window; - // contentByTag re-sorts per category for display. + // it (it is not in the local window); above the client fan-out cap the API resolves the + // single request by fanning out to per-parent index seeks server-side. + // sort+limit bound the window; contentByTag re-sorts per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/app/src/components/VideoPage/PinnedVideo.vue b/app/src/components/VideoPage/PinnedVideo.vue index 281c45560c..0f5da4372a 100644 --- a/app/src/components/VideoPage/PinnedVideo.vue +++ b/app/src/components/VideoPage/PinnedVideo.vue @@ -36,9 +36,9 @@ const pinnedCategoryContent = useContentQuery( // This indexes the local Dexie read and lets the API older-tail supplement fan out to // per-parent index seeks when the combined parentId set is within the fan-out cap. // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); when the combined set exceeds the cap the - // supplement falls back to a content-partition scan. sort+limit bound the window; - // contentByTag re-sorts per category for display. + // it (it is not in the local window); above the client fan-out cap the API resolves the + // single request by fanning out to per-parent index seeks server-side. + // sort+limit bound the window; contentByTag re-sorts per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/shared/src/util/HybridQuery/queryIntrospection.ts b/shared/src/util/HybridQuery/queryIntrospection.ts index af96c701b8..21cebd5a88 100644 --- a/shared/src/util/HybridQuery/queryIntrospection.ts +++ b/shared/src/util/HybridQuery/queryIntrospection.ts @@ -189,8 +189,9 @@ export function decideContentApiQuery( /** * Cap on parents to fan out. Beyond this, a burst of N concurrent POSTs (e.g. a - * large bookmark list) costs more than one scan query, so we fall back to a single - * query rather than flooding the API. + * large bookmark list) costs more than one request, so we fall back to a single + * request. Above the client fan-out cap the API resolves that request by fanning out + * to per-parent index seeks server-side; the cap still prevents the client from creating a burst. */ export const FANOUT_MAX_PARENTS = 25; @@ -246,7 +247,8 @@ function fanOut( * {@link fanOut}). The per-parent over-fetch is corrected when `HybridQuery` re-applies * sort+limit to the merged result via {@link applySortLimit}. Returns `[api]` unchanged * otherwise — non-Content, no `$in`, an empty `$in`, or more than - * {@link FANOUT_MAX_PARENTS} (the full-scan fallback avoids a request storm). + * {@link FANOUT_MAX_PARENTS}; above the client fan-out cap the API resolves the single + * request by fanning out to per-parent index seeks server-side. * * NOTE: there is intentionally no `parentTags` fan-out. A `parentTags $elemMatch:$in` * targets a multikey (array) index, and CouchDB cannot serve a `publishDate` sort from a From 4b003a212faed8b8f643b209936eb9906bb1e5dd Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 13 Jul 2026 16:43:49 +0200 Subject: [PATCH 2/7] Update query.service.ts --- api/src/endpoints/query.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/endpoints/query.service.ts b/api/src/endpoints/query.service.ts index 8b6089cb9c..ff13dcbc88 100644 --- a/api/src/endpoints/query.service.ts +++ b/api/src/endpoints/query.service.ts @@ -309,7 +309,7 @@ export class QueryService { const parentIds = [...new Set(rawIds as string[])]; if (parentIds.length === 0) return { docs: [], blockStart: 0, blockEnd: 0 }; - // ponytail: unbounded fan-out; add a concurrency cap if a parent set ever gets large + // Unbounded fan-out; add a concurrency cap if a parent set ever gets large. const results = await Promise.all( parentIds.map((id) => this.db.executeFindQuery({ From bedaa47bb6569245068b456361c6f2932db40d08 Mon Sep 17 00:00:00 2001 From: Dirk Date: Fri, 17 Jul 2026 12:57:05 +0200 Subject: [PATCH 3/7] tmp --- api/src/endpoints/query.controller.spec.ts | 91 +++++++++++++++++++ api/src/endpoints/query.controller.ts | 69 ++++++++++++++ shared/src/api/sync/liveSync.spec.ts | 22 +++++ shared/src/api/sync/liveSync.ts | 3 +- shared/src/api/sync/sync.spec.ts | 22 +++++ shared/src/api/sync/sync.ts | 17 ++-- shared/src/config.ts | 16 ++-- shared/src/db/retention.spec.ts | 22 ++++- shared/src/db/retention.ts | 9 +- .../HybridQuery/queryIntrospection.spec.ts | 14 +++ .../util/HybridQuery/queryIntrospection.ts | 23 ++++- 11 files changed, 286 insertions(+), 22 deletions(-) diff --git a/api/src/endpoints/query.controller.spec.ts b/api/src/endpoints/query.controller.spec.ts index ef71afdb1f..d3e0859aba 100644 --- a/api/src/endpoints/query.controller.spec.ts +++ b/api/src/endpoints/query.controller.spec.ts @@ -99,6 +99,40 @@ describe("QueryController", () => { expect(queryService.query).not.toHaveBeenCalled(); }); + it("rejects an updatedTimeUtc range with both bounds at 0 even in bypass mode", async () => { + configService.get.mockImplementation(configFor(true)); + + const body = { + identifier: "sync", + selector: { + type: "post", + updatedTimeUtc: { $lte: 0, $gte: 0 }, + }, + }; + + await expect( + controller.processPostReq(body, mockRequest(), mockReply()), + ).rejects.toThrow("updatedTimeUtc $lte and $gte must not both be 0"); + expect(queryService.query).not.toHaveBeenCalled(); + }); + + it("allows a sync history range whose lower bound alone is 0", async () => { + configService.get.mockImplementation(configFor(true)); + queryService.query.mockResolvedValue({ docs: [] }); + + const body = { + identifier: "sync", + selector: { + type: "post", + updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 }, + }, + }; + + await controller.processPostReq(body, mockRequest(), mockReply()); + + expect(queryService.query).toHaveBeenCalledTimes(1); + }); + it("removes identifier from the body before passing to the service", async () => { configService.get.mockImplementation(configFor(true)); queryService.query.mockResolvedValue({ docs: [] }); @@ -145,6 +179,63 @@ describe("QueryController", () => { expect(result.execution_stats).toBeUndefined(); }); + it("logs the pre-injection sync dimensions for an expensive sync query", async () => { + configService.get.mockImplementation(configFor(true)); + queryService.query.mockResolvedValue({ + docs: [], + execution_stats: { total_docs_examined: 2419, execution_time_ms: 600 }, + }); + + const body = { + identifier: "sync", + selector: { + type: "content", + updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 }, + parentType: "post", + memberOf: { $elemMatch: { $in: ["group-a", "group-b"] } }, + $or: [ + { language: { $in: ["lang-eng", "lang-fra"] } }, + { + $and: [ + { + $not: { + availableTranslations: { $elemMatch: { $eq: "lang-eng" } }, + }, + }, + ], + }, + ], + publishDate: { $gte: 1234 }, + }, + limit: 100, + sort: [{ updatedTimeUtc: "desc" }], + use_index: "sync-content-index", + cms: false, + includeExpired: false, + }; + + await controller.processPostReq(body, mockRequest(), mockReply()); + + expect(logger.warn).toHaveBeenCalledWith( + "Expensive /query", + expect.objectContaining({ + sync_context: { + parentType: "post", + updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 }, + publishDate: { $gte: 1234 }, + requestedMemberOf: ["group-a", "group-b"], + requestedMemberOfCount: 2, + requestedLanguages: ["lang-eng", "lang-fra"], + requestedLanguageCount: 2, + cms: false, + includeExpired: false, + limit: 100, + use_index: "sync-content-index", + }, + }), + ); + }); + it("keys an anonymous identity by ip when there is no userId", async () => { configService.get.mockImplementation(configFor(true)); queryService.query.mockResolvedValue({ diff --git a/api/src/endpoints/query.controller.ts b/api/src/endpoints/query.controller.ts index bc36f62e72..60b1da226d 100644 --- a/api/src/endpoints/query.controller.ts +++ b/api/src/endpoints/query.controller.ts @@ -58,6 +58,17 @@ export class QueryController { ); } + // A sync cursor collapsed to the epoch cannot match a valid updatedTimeUtc value, but + // CouchDB may still walk the selected index before returning no documents. Keep this + // invariant outside the optional validation bypass so the impossible query never reaches + // QueryService/CouchDB. + const updatedTimeUtc = body?.selector?.updatedTimeUtc; + if (updatedTimeUtc?.$lte === 0 && updatedTimeUtc?.$gte === 0) { + throw new BadRequestException( + "Invalid query: updatedTimeUtc $lte and $gte must not both be 0", + ); + } + const bypassValidation = this.configService.get("validation.bypassTemplateValidation") || false; @@ -73,6 +84,10 @@ export class QueryController { // `identifier` is an observability label only; strip it before the query runs. const identifier = typeof body?.identifier === "string" ? body.identifier : "unknown"; + // Capture the client-created sync dimensions before QueryService expands/mutates the + // selector and injects permission/publication filters. Temporary diagnostic context for + // identifying which sync column is producing an expensive CouchDB scan. + const syncContext = identifier === "sync" ? buildSyncContext(body) : undefined; delete body.identifier; const result = await this.queryService.query(body as MongoQueryDto, request.user); @@ -99,6 +114,7 @@ export class QueryController { results_returned: result?.docs?.length ?? 0, execution_time_ms: result?.execution_stats?.execution_time_ms, use_index: body?.use_index, + ...(syncContext ? { sync_context: syncContext } : {}), // Computed lazily on the post-injection query — reflects what CouchDB // actually executed, which is what you want when deciding on an index. fingerprint: selectorFingerprint(body), @@ -113,3 +129,56 @@ export class QueryController { return result; } } + +function buildSyncContext(body: any) { + const selector = body?.selector ?? {}; + const requestedMemberOf = Array.isArray(selector?.memberOf?.$elemMatch?.$in) + ? selector.memberOf.$elemMatch.$in + : []; + const requestedLanguages = collectIncludedLanguages(selector); + + return { + parentType: selector.parentType, + updatedTimeUtc: selector.updatedTimeUtc, + publishDate: selector.publishDate, + requestedMemberOf, + requestedMemberOfCount: requestedMemberOf.length, + requestedLanguages, + requestedLanguageCount: requestedLanguages.length, + cms: body?.cms === true, + includeExpired: body?.includeExpired === true, + limit: body?.limit, + use_index: body?.use_index, + }; +} + +function collectIncludedLanguages(selector: any): string[] { + const languages = new Set(); + + function visit(node: any): void { + if (!node || typeof node !== "object") return; + if (Array.isArray(node)) { + node.forEach(visit); + return; + } + + for (const [key, value] of Object.entries(node)) { + if (key === "language") { + if (typeof value === "string") languages.add(value); + else if (value && typeof value === "object") { + const criterion = value as { $eq?: unknown; $in?: unknown }; + if (typeof criterion.$eq === "string") languages.add(criterion.$eq); + if (Array.isArray(criterion.$in)) { + criterion.$in.forEach((language) => { + if (typeof language === "string") languages.add(language); + }); + } + } + } + visit(value); + } + } + + visit(selector); + return [...languages]; +} diff --git a/shared/src/api/sync/liveSync.spec.ts b/shared/src/api/sync/liveSync.spec.ts index f25f3882ad..76bf473ffd 100644 --- a/shared/src/api/sync/liveSync.spec.ts +++ b/shared/src/api/sync/liveSync.spec.ts @@ -177,4 +177,26 @@ describe("liveSync.applyLiveData (sync live persister)", () => { const ids = (await db.docs.toArray()).map((d) => d._id); expect(ids).toContain("always-offline"); }); + + it("persists below-cutoff Tag content without a retention row", async () => { + const CUTOFF = 1_000_000; + config.contentPublishDateCutoff = CUTOFF; + syncList.value = [ + entry({ chunkType: `${DocType.Content}:${DocType.Tag}`, languages: ["en"] }), + ]; + + await applyLiveData({ + docs: [ + { + type: DocType.Content, + _id: "old-tag-content", + parentType: DocType.Tag, + language: "en", + publishDate: CUTOFF - 1000, + }, + ] as any, + }); + + expect(await db.docs.get("old-tag-content")).toBeDefined(); + }); }); diff --git a/shared/src/api/sync/liveSync.ts b/shared/src/api/sync/liveSync.ts index a011100ce3..d6f7a520ae 100644 --- a/shared/src/api/sync/liveSync.ts +++ b/shared/src/api/sync/liveSync.ts @@ -39,7 +39,7 @@ function roomDocTypesFromSyncList(): DocType[] { * `isSyncableDoc` (the sync-`syncList`-derived gate), and the result is written * via `db.bulkPut` (which resolves `DeleteCmd`s with its own stale-delete guard). * - * Below-cutoff Content is written through ONLY if we're already keeping it offline + * Below-cutoff Post content is written through ONLY if we're already keeping it offline * (a `retention` row exists) — so a live edit to an offline-cached older article * stays fresh, while a below-cutoff doc we aren't caching is not persisted (it * would otherwise be written here and evicted on the next sync). DeleteCmds and @@ -56,6 +56,7 @@ export async function applyLiveData(data: ApiDataResponseDto): Promise { const isBelowCutoffContent = (d: BaseDocumentDto): boolean => { if (d.type !== DocType.Content) return false; const content = d as ContentDto; + if (content.parentType !== DocType.Post) return false; if (content.parentAlwaysOffline === true) return false; const pd = content.publishDate; return pd !== undefined && pd < cutoff; diff --git a/shared/src/api/sync/sync.spec.ts b/shared/src/api/sync/sync.spec.ts index 0d23e38d88..4d1b972c04 100644 --- a/shared/src/api/sync/sync.spec.ts +++ b/shared/src/api/sync/sync.spec.ts @@ -1890,6 +1890,28 @@ describe("sync module", () => { ); }); + it("tag content sync uses the open publishDate range without a companion run", async () => { + await sync({ + type: DocType.Content, + subType: DocType.Tag, + memberOf: ["group1"], + languages: ["en"], + limit: 100, + includeDeleteCmds: false, + }); + + expect(syncBatch).toHaveBeenCalledTimes(1); + expect(syncBatch).toHaveBeenCalledWith( + expect.objectContaining({ + type: DocType.Content, + subType: DocType.Tag, + publishDateMin: OPEN_MIN, + publishDateMax: OPEN_MAX, + }), + ); + expect(vi.mocked(syncBatch).mock.calls[0][0].alwaysOffline).toBeUndefined(); + }); + it("sync() also triggers a companion always-offline run when a cutoff is configured", async () => { await sync({ type: DocType.Content, diff --git a/shared/src/api/sync/sync.ts b/shared/src/api/sync/sync.ts index b30efbb5f0..7df67d4aa6 100644 --- a/shared/src/api/sync/sync.ts +++ b/shared/src/api/sync/sync.ts @@ -268,12 +268,13 @@ export async function sync(options: SyncRunnerOptions): Promise { try { await _runSync(options); - // Content callers get an automatic companion run that syncs always-offline + // Post-content callers get an automatic companion run that syncs always-offline // docs (parentAlwaysOffline === true) regardless of the publishDate cutoff. // Centralized here so callers don't need to know about `alwaysOffline` or // issue a second sync() call themselves. if ( options.type === DocType.Content && + options.subType === DocType.Post && !options.alwaysOffline && hasContentPublishDateCutoff() ) { @@ -285,14 +286,14 @@ export async function sync(options: SyncRunnerOptions): Promise { } async function _runSync(options: SyncRunnerOptions): Promise { - // publishDate is a Content-only sync dimension. For Content callers, an unspecified - // floor falls back to the configured cutoff so sync never pulls content older than the - // app/HybridQuery treat as "remote-only". Non-Content callers (Language, Redirect, - // Storage, AuthProvider, Group, …) leave the bounds undefined — every downstream code - // path that does something with publishDate is wrapped in `if (type === Content)` and - // every comparison goes through `resolveRange` which treats undefined as OPEN_MIN/MAX. + // publishDate is a Content-only sync dimension. Post content uses the configured cutoff + // so sync does not pull ordinary posts older than the app/HybridQuery treats as + // "remote-only". Tag content must remain open-ended: tags can be long-lived navigation + // parents, so applying the rolling Post window can exclude every matching document. + // Non-Content callers (Language, Redirect, Storage, AuthProvider, Group, …) leave the + // bounds undefined; downstream comparisons resolve those as OPEN_MIN/MAX. if (options.type === DocType.Content) { - if (options.alwaysOffline) { + if (options.subType !== DocType.Post || options.alwaysOffline) { options.publishDateMin = OPEN_MIN; options.publishDateMax = OPEN_MAX; } else { diff --git a/shared/src/config.ts b/shared/src/config.ts index 31fd242d97..46300508d4 100644 --- a/shared/src/config.ts +++ b/shared/src/config.ts @@ -39,14 +39,14 @@ export type SharedConfig = { */ appLanguageIdsAsRef?: Ref; /** - * publishDate floor for content. Content older than this is NOT synced and is - * fetched on demand from the API by `HybridQuery`. Omit (or pass `OPEN_MIN`) for - * no cutoff (full sync). Apps typically pass a rolling + * publishDate floor for Post content. Posts older than this are NOT synced and are + * fetched on demand from the API by `HybridQuery`; Tag content remains fully synced. + * Omit (or pass `OPEN_MIN`) for no cutoff (full sync). Apps typically pass a rolling * `Date.now() - CONTENT_SYNC_WINDOW_MS`; CMS leaves it unset. */ contentPublishDateCutoff?: number; /** - * How long (ms) a below-cutoff Content document is retained in IndexedDB after it + * How long (ms) a below-cutoff Post-content document is retained in IndexedDB after it * was last viewed / featured / persisted offline, before `evictStaleBelowCutoff` * removes it. Bounds the offline document store as the sync window slides. * Defaults to 30 days. Only meaningful when `contentPublishDateCutoff` is set. @@ -64,10 +64,10 @@ export function initConfig(newConfig: SharedConfig) { } /** - * Single source of truth for the content publishDate cutoff. Read by sync - * (which floors content `publishDateMin` to this value) and by `HybridQuery` - * (which fetches `publishDate <= cutoff` from the API for the older tail). - * Defaults to `OPEN_MIN` when unset — i.e. no cutoff, full content sync, + * Single source of truth for the Post-content publishDate cutoff. Read by sync + * (which floors Post-content `publishDateMin` to this value) and by `HybridQuery` + * (which fetches older Post content from the API on demand). Tag content is fully synced. + * Defaults to `OPEN_MIN` when unset — i.e. no cutoff, full Post-content sync, * no older-tail API fetch. */ export function getContentPublishDateCutoff(): number { diff --git a/shared/src/db/retention.spec.ts b/shared/src/db/retention.spec.ts index 8285b062d7..e7d41bb589 100644 --- a/shared/src/db/retention.spec.ts +++ b/shared/src/db/retention.spec.ts @@ -23,7 +23,14 @@ import { DocType, type BaseDocumentDto } from "../types"; const CUTOFF = 1000; const content = (_id: string, publishDate: number): BaseDocumentDto => - ({ _id, type: DocType.Content, publishDate, updatedTimeUtc: 1, memberOf: [] }) as unknown as BaseDocumentDto; + ({ + _id, + type: DocType.Content, + parentType: DocType.Post, + publishDate, + updatedTimeUtc: 1, + memberOf: [], + }) as unknown as BaseDocumentDto; describe("retention", () => { beforeAll(async () => { @@ -216,6 +223,19 @@ describe("retention", () => { expect(await db.docs.get("always-offline")).toBeDefined(); }); + it("does not evict Tag content below the Post cutoff", async () => { + await db.docs.bulkPut([ + { + ...(content("tag-content", 400) as object), + parentType: DocType.Tag, + } as BaseDocumentDto, + ]); + + await evictStaleBelowCutoff(); + + expect(await db.docs.get("tag-content")).toBeDefined(); + }); + it("leaves a Content doc that has no publishDate (absent from the publishDate index)", async () => { // A Content doc with no publishDate isn't in the publishDate index, so the // below-cutoff range query never sees it → never evicted (documented: only diff --git a/shared/src/db/retention.ts b/shared/src/db/retention.ts index 3c4af5558b..c6667b0c61 100644 --- a/shared/src/db/retention.ts +++ b/shared/src/db/retention.ts @@ -99,8 +99,8 @@ export async function flushRetention(): Promise { } /** - * Delete below-cutoff Content whose retention deadline has passed (or was never set). - * Covers both supplement-persisted docs and content that slid out of the sync window. + * Delete below-cutoff Post content whose retention deadline has passed (or was never set). + * Covers both supplement-persisted docs and Post content that slid out of the sync window. * Inert in CMS / when no cutoff is configured. Call only while online (it runs after a * content sync) so evicted-but-still-wanted docs can be re-fetched. */ @@ -124,7 +124,10 @@ export async function evictStaleBelowCutoff(): Promise { .below(cutoff) .and((d) => { if (d.type !== DocType.Content) return false; - return (d as ContentDto).parentAlwaysOffline !== true; + const content = d as ContentDto; + return ( + content.parentType === DocType.Post && content.parentAlwaysOffline !== true + ); }) .primaryKeys()) as string[]; if (!ids.length) return; diff --git a/shared/src/util/HybridQuery/queryIntrospection.spec.ts b/shared/src/util/HybridQuery/queryIntrospection.spec.ts index 93b9fe7fcd..9ce4db2195 100644 --- a/shared/src/util/HybridQuery/queryIntrospection.spec.ts +++ b/shared/src/util/HybridQuery/queryIntrospection.spec.ts @@ -188,4 +188,18 @@ describe("decideContentApiQuery — older-tail supplement", () => { expect(decideContentApiQuery(feed(), [])).toBeUndefined(); config.contentPublishDateCutoff = 1000; }); + + it("returns undefined for Tag content because Tag sync covers the full corpus", () => { + const tagFeed = feed({ + selector: { + $and: [ + { type: "content" }, + { parentType: "tag" }, + { status: "published" }, + ], + }, + }); + + expect(decideContentApiQuery(tagFeed, [])).toBeUndefined(); + }); }); diff --git a/shared/src/util/HybridQuery/queryIntrospection.ts b/shared/src/util/HybridQuery/queryIntrospection.ts index 21cebd5a88..0e502e7e65 100644 --- a/shared/src/util/HybridQuery/queryIntrospection.ts +++ b/shared/src/util/HybridQuery/queryIntrospection.ts @@ -29,6 +29,23 @@ export function readType(query: MangoQuery): DocType | undefined { return undefined; } +/** Read an explicit top-level Content `parentType` equality, if present. */ +function readParentType(query: MangoQuery): DocType | undefined { + if (!query?.selector || typeof query.selector !== "object") return undefined; + const conditions = expandMangoSelector(query.selector).$and ?? []; + for (const cond of conditions) { + if (cond.$or || cond.$nor || cond.$not || cond.$and) continue; + if (!("parentType" in cond)) continue; + const criteria = (cond as Record).parentType; + if (typeof criteria === "string") return criteria as DocType; + if (criteria && typeof criteria === "object" && !Array.isArray(criteria)) { + const eq = (criteria as Record).$eq; + if (typeof eq === "string") return eq as DocType; + } + } + return undefined; +} + /** * True iff at least one syncList entry currently tracks the given doc type * (regardless of subType / memberOf / language). Used by the non-content branch: @@ -137,7 +154,8 @@ export function withPublishDate(selector: MangoSelector, cutoff: number): MangoS * for the older tail). * * Cutoff comes from `getContentPublishDateCutoff()` (`SharedConfig`), so this - * function reflects the same global value sync uses to floor content sync depth. + * function reflects the same global value sync uses to floor Post-content sync depth. + * Tag content is fully synced and therefore never needs an older-tail supplement. * When there is no cutoff (`OPEN_MIN` — full-corpus sync) the API has nothing to * supply, so this returns `undefined` and the read is Dexie-only. */ @@ -145,6 +163,9 @@ export function decideContentApiQuery( query: MangoQuery, localDocs: readonly T[], ): MangoQuery | undefined { + // Tag content is synced across the open publishDate range, so Dexie is authoritative. + if (readParentType(query) === DocType.Tag) return undefined; + const cutoff = getContentPublishDateCutoff(); // No cutoff ⇒ sync has all synced-language content, so the API has nothing to supply — skip. From 2ef41369f50bd14ccbda267df4d205cefa21a123 Mon Sep 17 00:00:00 2001 From: Dirk Date: Tue, 21 Jul 2026 14:36:46 +0200 Subject: [PATCH 4/7] Cap and rate-limit content query parentId fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QueryService's per-parent fan-out (parentId.$in) had no upper bound, so a caller could force one CouchDB request per id with no cap — worse than the full-scan it replaced. Add a hard maxFanoutParents cap, bound concurrent CouchDB requests per fan-out, and feed the existing per-identity rate limiter immediately for oversized-but-allowed fan-outs rather than waiting on post-hoc query-cost stats. Co-Authored-By: Claude Sonnet 5 --- api/src/configuration.spec.ts | 17 ++++++ api/src/configuration.ts | 21 ++++++++ api/src/endpoints/query.controller.spec.ts | 53 ++++++++++++++++++ api/src/endpoints/query.controller.ts | 33 ++++++++++++ api/src/endpoints/query.service.spec.ts | 52 ++++++++++++++++++ api/src/endpoints/query.service.ts | 62 +++++++++++++++++----- 6 files changed, 225 insertions(+), 13 deletions(-) diff --git a/api/src/configuration.spec.ts b/api/src/configuration.spec.ts index c844556638..4be79c238e 100644 --- a/api/src/configuration.spec.ts +++ b/api/src/configuration.spec.ts @@ -92,6 +92,9 @@ describe("configuration", () => { delete process.env.QUERY_MAX_LIMIT; delete process.env.QUERY_EXPENSIVE_DOCS_EXAMINED; delete process.env.QUERY_EXPENSIVE_EXAMINED_RATIO; + delete process.env.QUERY_MAX_FANOUT_PARENTS; + delete process.env.QUERY_FANOUT_CONCURRENCY; + delete process.env.QUERY_FANOUT_STRIKE_THRESHOLD; delete process.env.QUERY_RATE_LIMIT_ENABLED; delete process.env.QUERY_RATE_LIMIT_FREE_STRIKES; delete process.env.QUERY_RATE_LIMIT_BASE_BACKOFF_MS; @@ -102,6 +105,9 @@ describe("configuration", () => { expect(config.query.maxLimit).toBe(500); expect(config.query.expensiveDocsExamined).toBe(1000); expect(config.query.expensiveExaminedRatio).toBe(10); + expect(config.query.maxFanoutParents).toBe(200); + expect(config.query.fanoutConcurrency).toBe(20); + expect(config.query.fanoutStrikeThreshold).toBe(25); expect(config.query.rateLimit).toEqual({ enabled: false, freeStrikes: 3, @@ -121,4 +127,15 @@ describe("configuration", () => { expect(config.query.rateLimit.freeStrikes).toBe(5); expect(config.query.expensiveDocsExamined).toBe(2000); }); + + it("should read the fan-out cap/concurrency/strike config from env vars", () => { + process.env.QUERY_MAX_FANOUT_PARENTS = "50"; + process.env.QUERY_FANOUT_CONCURRENCY = "5"; + process.env.QUERY_FANOUT_STRIKE_THRESHOLD = "10"; + + const config = configuration(); + expect(config.query.maxFanoutParents).toBe(50); + expect(config.query.fanoutConcurrency).toBe(5); + expect(config.query.fanoutStrikeThreshold).toBe(10); + }); }); diff --git a/api/src/configuration.ts b/api/src/configuration.ts index 6e7cc3ed53..a6c4b803f5 100644 --- a/api/src/configuration.ts +++ b/api/src/configuration.ts @@ -52,6 +52,24 @@ export type QueryConfig = { * QUERY_EXPENSIVE_EXAMINED_RATIO (default 10). */ expensiveExaminedRatio: number; + /** + * Maximum distinct `parentId` values a `parentId.$in` fan-out query may request. + * Guards against a caller forcing one CouchDB request per id with no upper bound. + * Environment variable: QUERY_MAX_FANOUT_PARENTS (default 200). + */ + maxFanoutParents: number; + /** + * Maximum concurrent CouchDB requests for one parentId fan-out. Bounds load even + * when the fan-out is large but under the cap above. + * Environment variable: QUERY_FANOUT_CONCURRENCY (default 20). + */ + fanoutConcurrency: number; + /** + * parentId fan-out size above which a rate-limiter strike is recorded immediately, + * rather than waiting on post-hoc query-cost stats. Environment variable: + * QUERY_FANOUT_STRIKE_THRESHOLD (default 25). + */ + fanoutStrikeThreshold: number; /** Per-identity expensive-query rate limiter (default off). */ rateLimit: QueryRateLimitConfig; }; @@ -121,6 +139,9 @@ export default () => maxLanguages: parseInt(process.env.QUERY_MAX_LANGUAGES, 10) || 4, expensiveDocsExamined: parseInt(process.env.QUERY_EXPENSIVE_DOCS_EXAMINED, 10) || 1000, expensiveExaminedRatio: parseInt(process.env.QUERY_EXPENSIVE_EXAMINED_RATIO, 10) || 10, + maxFanoutParents: parseInt(process.env.QUERY_MAX_FANOUT_PARENTS, 10) || 200, + fanoutConcurrency: parseInt(process.env.QUERY_FANOUT_CONCURRENCY, 10) || 20, + fanoutStrikeThreshold: parseInt(process.env.QUERY_FANOUT_STRIKE_THRESHOLD, 10) || 25, rateLimit: { enabled: process.env.QUERY_RATE_LIMIT_ENABLED === "true", freeStrikes: parseInt(process.env.QUERY_RATE_LIMIT_FREE_STRIKES, 10) || 3, diff --git a/api/src/endpoints/query.controller.spec.ts b/api/src/endpoints/query.controller.spec.ts index d3e0859aba..5c78c14628 100644 --- a/api/src/endpoints/query.controller.spec.ts +++ b/api/src/endpoints/query.controller.spec.ts @@ -37,6 +37,10 @@ describe("QueryController", () => { return 1000; case "query.expensiveExaminedRatio": return 10; + case "query.maxFanoutParents": + return 200; + case "query.fanoutStrikeThreshold": + return 25; default: return undefined; } @@ -133,6 +137,55 @@ describe("QueryController", () => { expect(queryService.query).toHaveBeenCalledTimes(1); }); + it("rejects a parentId fan-out above the configured max", async () => { + configService.get.mockImplementation(configFor(true)); + + const body = { + selector: { + type: "content", + parentId: { $in: Array.from({ length: 201 }, (_, i) => `p${i}`) }, + }, + }; + + await expect( + controller.processPostReq(body, mockRequest(), mockReply()), + ).rejects.toThrow("'parentId.$in' exceeds the maximum fan-out size (200)"); + expect(queryService.query).not.toHaveBeenCalled(); + }); + + it("records an immediate strike for an oversized-but-under-cap parentId fan-out", async () => { + configService.get.mockImplementation(configFor(true)); + queryService.query.mockResolvedValue({ docs: [] }); + + const body = { + selector: { + type: "content", + parentId: { $in: Array.from({ length: 26 }, (_, i) => `p${i}`) }, + }, + }; + + await controller.processPostReq(body, mockRequest(), mockReply()); + + expect(rateLimiter.recordStrike).toHaveBeenCalledWith("mock-user"); + expect(queryService.query).toHaveBeenCalledTimes(1); + }); + + it("does not strike for a parentId fan-out at or below the strike threshold", async () => { + configService.get.mockImplementation(configFor(true)); + queryService.query.mockResolvedValue({ docs: [] }); + + const body = { + selector: { + type: "content", + parentId: { $in: Array.from({ length: 25 }, (_, i) => `p${i}`) }, + }, + }; + + await controller.processPostReq(body, mockRequest(), mockReply()); + + expect(rateLimiter.recordStrike).not.toHaveBeenCalled(); + }); + it("removes identifier from the body before passing to the service", async () => { configService.get.mockImplementation(configFor(true)); queryService.query.mockResolvedValue({ docs: [] }); diff --git a/api/src/endpoints/query.controller.ts b/api/src/endpoints/query.controller.ts index 60b1da226d..c556c0c2fc 100644 --- a/api/src/endpoints/query.controller.ts +++ b/api/src/endpoints/query.controller.ts @@ -22,6 +22,7 @@ import { FastifyReply, FastifyRequest } from "fastify"; import { classifyQueryCost } from "./queryStats"; import { selectorFingerprint } from "../util/selectorFingerprint"; import { QueryRateLimiterService } from "../ratelimit/queryRateLimiter.service"; +import { expandMangoSelector } from "../util/expandMangoQuery"; /** Endpoint supporting MongoDB like queries (Mango Query) */ @Controller("query") @@ -69,6 +70,22 @@ export class QueryController { ); } + // Reject an oversized parentId fan-out, and strike large-but-allowed ones early — + // the fan-out size is known before the query runs, so abuse doesn't need to wait + // for post-hoc execution_stats. + const maxFanoutParents = this.configService.get("query.maxFanoutParents") ?? 200; + const fanoutStrikeThreshold = + this.configService.get("query.fanoutStrikeThreshold") ?? 25; + const fanoutSize = countParentIdFanout(body?.selector); + if (fanoutSize > maxFanoutParents) { + throw new BadRequestException( + `Invalid query: 'parentId.$in' exceeds the maximum fan-out size (${maxFanoutParents})`, + ); + } + if (fanoutSize > fanoutStrikeThreshold) { + this.rateLimiter.recordStrike(identityKey); + } + const bypassValidation = this.configService.get("validation.bypassTemplateValidation") || false; @@ -130,6 +147,22 @@ export class QueryController { } } +/** + * Size of the selector's `parentId.$in` array, or 0 if absent. Selector is expanded + * first since `parentId` may sit at the top level or nested under `$and`. + */ +function countParentIdFanout(selector: any): number { + if (!selector || typeof selector !== "object") return 0; + const conditions = expandMangoSelector(selector).$and ?? []; + for (const condition of conditions) { + const value = (condition as any)?.parentId; + if (value && typeof value === "object" && Array.isArray(value.$in)) { + return value.$in.length; + } + } + return 0; +} + function buildSyncContext(body: any) { const selector = body?.selector ?? {}; const requestedMemberOf = Array.isArray(selector?.memberOf?.$elemMatch?.$in) diff --git a/api/src/endpoints/query.service.spec.ts b/api/src/endpoints/query.service.spec.ts index 9d6a12de61..02d0d8fbb1 100644 --- a/api/src/endpoints/query.service.spec.ts +++ b/api/src/endpoints/query.service.spec.ts @@ -3,6 +3,7 @@ import { Test } from "@nestjs/testing"; import { HttpException, HttpStatus } from "@nestjs/common"; import { QueryService } from "./query.service"; import { DbService } from "../db/db.service"; +import { ConfigService } from "@nestjs/config"; import { WINSTON_MODULE_PROVIDER } from "nest-winston"; import type { Logger } from "winston"; import { AclPermission, DocType, PublishStatus } from "../enums"; @@ -16,6 +17,7 @@ describe("QueryService", () => { executeFindQuery: jest.Mock; on: jest.Mock; }; + let configService: { get: jest.Mock }; let logger: Logger; const mockUser = { @@ -28,6 +30,7 @@ describe("QueryService", () => { on: jest.fn(), } as any; logger = { info: jest.fn(), error: jest.fn() } as unknown as Logger; + configService = { get: jest.fn().mockReturnValue(undefined) }; jest.spyOn(permissions.PermissionSystem, "accessMapToGroups").mockReturnValue({} as any); @@ -36,6 +39,7 @@ describe("QueryService", () => { QueryService, { provide: DbService, useValue: dbService }, { provide: WINSTON_MODULE_PROVIDER, useValue: logger }, + { provide: ConfigService, useValue: configService }, ], }).compile(); @@ -890,6 +894,53 @@ describe("QueryService", () => { expect(dbService.executeFindQuery).not.toHaveBeenCalled(); }); + it("rejects a parentId fan-out above the configured maximum", async () => { + allowContent(); + configService.get.mockImplementation((key: string) => + key === "query.maxFanoutParents" ? 2 : undefined, + ); + + const query = makeQuery((s) => { + (s as any).type = DocType.Content; + (s as any).parentId = { $in: ["p1", "p2", "p3"] }; + }); + + await expect(service.query(query, mockUser)).rejects.toEqual( + new HttpException( + "'parentId.$in' exceeds the maximum fan-out size (2)", + HttpStatus.BAD_REQUEST, + ), + ); + expect(dbService.executeFindQuery).not.toHaveBeenCalled(); + }); + + it("bounds concurrent CouchDB requests to the configured fan-out concurrency", async () => { + allowContent(); + configService.get.mockImplementation((key: string) => + key === "query.fanoutConcurrency" ? 2 : undefined, + ); + + let inFlight = 0; + let maxInFlight = 0; + dbService.executeFindQuery.mockImplementation(async () => { + inFlight++; + maxInFlight = Math.max(maxInFlight, inFlight); + await new Promise((resolve) => setTimeout(resolve, 5)); + inFlight--; + return { docs: [] }; + }); + + const query = makeQuery((s) => { + (s as any).type = DocType.Content; + (s as any).parentId = { $in: ["p1", "p2", "p3", "p4", "p5"] }; + }); + + await service.query(query, mockUser); + + expect(dbService.executeFindQuery).toHaveBeenCalledTimes(5); + expect(maxInFlight).toBe(2); + }); + it("passes a content query without parentId $in through unchanged", async () => { allowContent(); const query = makeQuery((s) => { @@ -936,6 +987,7 @@ describe("QueryService", () => { QueryService, { provide: DbService, useValue: dbMock }, { provide: WINSTON_MODULE_PROVIDER, useValue: logger }, + { provide: ConfigService, useValue: configService }, ], }).compile(); diff --git a/api/src/endpoints/query.service.ts b/api/src/endpoints/query.service.ts index ff13dcbc88..ab8ea11b4f 100644 --- a/api/src/endpoints/query.service.ts +++ b/api/src/endpoints/query.service.ts @@ -1,4 +1,5 @@ import { Injectable, Inject, HttpException, HttpStatus } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; import { DbQueryResult, DbService } from "../db/db.service"; import { AclPermission, DocType, PublishStatus, Uuid } from "../enums"; import { PermissionSystem } from "../permissions/permissions.service"; @@ -20,6 +21,7 @@ export class QueryService { @Inject(WINSTON_MODULE_PROVIDER) private readonly logger: Logger, private db: DbService, + private readonly configService: ConfigService, ) { // Get list of languages from the database for content doc filtering by user accessible language. // This list is kept updated in memory to reduce database load @@ -309,19 +311,29 @@ export class QueryService { const parentIds = [...new Set(rawIds as string[])]; if (parentIds.length === 0) return { docs: [], blockStart: 0, blockEnd: 0 }; - // Unbounded fan-out; add a concurrency cap if a parent set ever gets large. - const results = await Promise.all( - parentIds.map((id) => - this.db.executeFindQuery({ - ...query, - selector: { - $and: (query.selector.$and || []).map((condition) => - condition.parentId === parentIdCriteria ? { parentId: id } : condition, - ), - }, - use_index: "content-parentId-publishDate-index", - }), - ), + // Reject an oversized parentId fan-out. The client's own fan-out cap is + // client-side only, so this is the authoritative backstop. + const maxFanoutParents = this.configService.get("query.maxFanoutParents") ?? 200; + if (parentIds.length > maxFanoutParents) { + throw new HttpException( + `'parentId.$in' exceeds the maximum fan-out size (${maxFanoutParents})`, + HttpStatus.BAD_REQUEST, + ); + } + + // Cap concurrent CouchDB requests for the fan-out, so one request can't open + // one connection per parent at once. + const fanoutConcurrency = this.configService.get("query.fanoutConcurrency") ?? 20; + const results = await mapWithConcurrency(parentIds, fanoutConcurrency, (id) => + this.db.executeFindQuery({ + ...query, + selector: { + $and: (query.selector.$and || []).map((condition) => + condition.parentId === parentIdCriteria ? { parentId: id } : condition, + ), + }, + use_index: "content-parentId-publishDate-index", + }), ); const seen = new Set(); @@ -355,6 +367,30 @@ export class QueryService { } } +/** + * Run `fn` over `items` with at most `concurrency` calls in flight at once, preserving + * result order. Used to bound CouchDB load during a parentId fan-out. + */ +async function mapWithConcurrency( + items: T[], + concurrency: number, + fn: (item: T) => Promise, +): Promise { + const results: R[] = new Array(items.length); + let next = 0; + + async function worker(): Promise { + while (next < items.length) { + const i = next++; + results[i] = await fn(items[i]); + } + } + + const workerCount = Math.max(1, Math.min(concurrency, items.length)); + await Promise.all(Array.from({ length: workerCount }, worker)); + return results; +} + function findParentIdIn(and: MongoSelectorDto[]): MongoComparisonCriteria | undefined { for (const condition of and) { const value = condition.parentId; From 0012dfa952050e3388752cf4b346d74e51c3a79b Mon Sep 17 00:00:00 2001 From: Dirk Date: Wed, 22 Jul 2026 15:05:27 +0200 Subject: [PATCH 5/7] Address PR #1818 review comments - Extract query.service.ts's free-standing helpers into api/src/util/queryFanout.ts (parentId fan-out + result merging) and querySelector.ts (selector field extraction), shrinking the service file and matching the existing util/ layout. - Trim the duplicated fan-out comments in PinnedTopics/HomePagePinned/PinnedVideo that restated queryIntrospection.ts's internals instead of pointing to it. - Centralize the Post-vs-Tag publishDate-windowing check into a single isWindowedContentSubType() predicate in shared/config.ts, replacing four independent `=== DocType.Post` comparisons in sync.ts, liveSync.ts, and retention.ts so the policy can't drift between call sites. No behavior change. Co-Authored-By: Claude Sonnet 5 --- api/src/endpoints/query.service.ts | 155 +----------------- api/src/util/queryFanout.ts | 72 ++++++++ api/src/util/querySelector.ts | 85 ++++++++++ .../components/ExplorePage/PinnedTopics.vue | 10 +- .../components/HomePage/HomePagePinned.vue | 10 +- app/src/components/VideoPage/PinnedVideo.vue | 10 +- shared/src/api/sync/liveSync.ts | 19 ++- shared/src/api/sync/sync.ts | 30 ++-- shared/src/config.ts | 15 +- shared/src/db/retention.ts | 15 +- 10 files changed, 224 insertions(+), 197 deletions(-) create mode 100644 api/src/util/queryFanout.ts create mode 100644 api/src/util/querySelector.ts diff --git a/api/src/endpoints/query.service.ts b/api/src/endpoints/query.service.ts index ab8ea11b4f..da6e2bae05 100644 --- a/api/src/endpoints/query.service.ts +++ b/api/src/endpoints/query.service.ts @@ -7,10 +7,12 @@ import { WINSTON_MODULE_PROVIDER } from "nest-winston"; import { Logger } from "winston"; import { JwtUserDetails } from "../auth/authIdentity.service"; import { MongoQueryDto } from "../dto/MongoQueryDto"; -import { MongoComparisonCriteria, MongoSelectorDto } from "../dto/MongoSelectorDto"; +import { MongoSelectorDto } from "../dto/MongoSelectorDto"; import { LanguageDto } from "../dto/LanguageDto"; import { expandMangoSelector } from "../util/expandMangoQuery"; import { isExpiredContent, stripExpiredContent } from "../util/stripExpiredContent"; +import { applySortAndLimit, findParentIdIn, mapWithConcurrency, setBlockRange } from "../util/queryFanout"; +import { extractFieldFromAnd, extractMemberOf, removeMemberOf } from "../util/querySelector"; @Injectable() export class QueryService { @@ -366,154 +368,3 @@ export class QueryService { return result; } } - -/** - * Run `fn` over `items` with at most `concurrency` calls in flight at once, preserving - * result order. Used to bound CouchDB load during a parentId fan-out. - */ -async function mapWithConcurrency( - items: T[], - concurrency: number, - fn: (item: T) => Promise, -): Promise { - const results: R[] = new Array(items.length); - let next = 0; - - async function worker(): Promise { - while (next < items.length) { - const i = next++; - results[i] = await fn(items[i]); - } - } - - const workerCount = Math.max(1, Math.min(concurrency, items.length)); - await Promise.all(Array.from({ length: workerCount }, worker)); - return results; -} - -function findParentIdIn(and: MongoSelectorDto[]): MongoComparisonCriteria | undefined { - for (const condition of and) { - const value = condition.parentId; - if (value && typeof value === "object" && !Array.isArray(value) && "$in" in value) { - return value as MongoComparisonCriteria; - } - } - return undefined; -} - -function applySortAndLimit( - docs: any[], - sort: MongoQueryDto["sort"], - limit: number | undefined, -): any[] { - let result = docs; - if (sort?.length) { - const [field, direction] = Object.entries(sort[0] || {})[0] || []; - if (field) { - const desc = direction === "desc"; - result = docs.slice().sort((a, b) => { - const av = a?.[field]; - const bv = b?.[field]; - let cmp = 0; - if (av == null && bv != null) cmp = -1; - else if (av != null && bv == null) cmp = 1; - else if (av < bv) cmp = -1; - else if (av > bv) cmp = 1; - if (desc) cmp = -cmp; - if (cmp !== 0) return cmp; - return String(a?._id ?? "").localeCompare(String(b?._id ?? "")); - }); - } - } - return typeof limit === "number" ? result.slice(0, Math.max(0, limit)) : result; -} - -function setBlockRange(result: DbQueryResult): void { - const times = result.docs - .map((doc) => doc?.updatedTimeUtc) - .filter((value): value is number => typeof value === "number"); - result.blockStart = times.length ? Math.max(...times) : 0; - result.blockEnd = times.length ? Math.min(...times) : 0; -} - -/** - * Extract memberOf groups from the top-level $and array. - * (After expansion, memberOf will always be a condition in the $and array. - */ -function extractMemberOf(selector: MongoSelectorDto): Uuid[] { - for (const condition of selector.$and || []) { - const memberOf = (condition as MongoSelectorDto).memberOf; - if (!memberOf) continue; - - if (typeof memberOf === "string") { - return [memberOf]; - } - - if (Array.isArray((memberOf as MongoComparisonCriteria).$in)) { - return (memberOf as MongoComparisonCriteria).$in as string[]; - } - - if (Array.isArray((memberOf as MongoComparisonCriteria).$elemMatch?.$in)) { - return (memberOf as MongoComparisonCriteria).$elemMatch.$in as string[]; - } - - throw new HttpException("Invalid memberOf field in selector", HttpStatus.BAD_REQUEST); - } - - return []; -} - -/** - * Remove memberOf from conditions in the top-level $and array. - * (After expansion, memberOf will always be a condition in the $and array.) - */ -function removeMemberOf(selector: MongoSelectorDto): void { - if (!selector.$and) return; - - for (const condition of selector.$and) { - if ((condition as any).memberOf !== undefined) { - delete (condition as any).memberOf; - } - } - - // Remove any conditions that are now empty after memberOf deletion - selector.$and = selector.$and.filter((condition) => Object.keys(condition).length > 0); -} - -/** - * Extract a field value from the $and array. - * Returns the first matching value found, or undefined if not present. - * Throws if multiple different values are found for the same field. - */ -function extractFieldFromAnd(andArray: MongoSelectorDto[], fieldName: string): T | undefined { - let foundValue: T | undefined; - - for (const condition of andArray) { - if (fieldName in condition) { - const value = condition[fieldName] as T; - - // Only accept simple equality values (string, number, boolean) - if ( - typeof value !== "string" && - typeof value !== "number" && - typeof value !== "boolean" - ) { - throw new HttpException( - `'${fieldName}' field must be a simple equality value`, - HttpStatus.BAD_REQUEST, - ); - } - - if (foundValue !== undefined && foundValue !== value) { - throw new HttpException( - `Multiple different '${fieldName}' values found in selector`, - HttpStatus.BAD_REQUEST, - ); - } - - foundValue = value; - } - } - - return foundValue; -} diff --git a/api/src/util/queryFanout.ts b/api/src/util/queryFanout.ts new file mode 100644 index 0000000000..9358e3263e --- /dev/null +++ b/api/src/util/queryFanout.ts @@ -0,0 +1,72 @@ +import { DbQueryResult } from "../db/db.service"; +import { MongoComparisonCriteria, MongoSelectorDto } from "../dto/MongoSelectorDto"; +import { MongoQueryDto } from "../dto/MongoQueryDto"; + +/** + * Run `fn` over `items` with at most `concurrency` calls in flight at once, preserving + * result order. Used to bound CouchDB load during a parentId fan-out. + */ +export async function mapWithConcurrency( + items: T[], + concurrency: number, + fn: (item: T) => Promise, +): Promise { + const results: R[] = new Array(items.length); + let next = 0; + + async function worker(): Promise { + while (next < items.length) { + const i = next++; + results[i] = await fn(items[i]); + } + } + + const workerCount = Math.max(1, Math.min(concurrency, items.length)); + await Promise.all(Array.from({ length: workerCount }, worker)); + return results; +} + +export function findParentIdIn(and: MongoSelectorDto[]): MongoComparisonCriteria | undefined { + for (const condition of and) { + const value = condition.parentId; + if (value && typeof value === "object" && !Array.isArray(value) && "$in" in value) { + return value as MongoComparisonCriteria; + } + } + return undefined; +} + +export function applySortAndLimit( + docs: any[], + sort: MongoQueryDto["sort"], + limit: number | undefined, +): any[] { + let result = docs; + if (sort?.length) { + const [field, direction] = Object.entries(sort[0] || {})[0] || []; + if (field) { + const desc = direction === "desc"; + result = docs.slice().sort((a, b) => { + const av = a?.[field]; + const bv = b?.[field]; + let cmp = 0; + if (av == null && bv != null) cmp = -1; + else if (av != null && bv == null) cmp = 1; + else if (av < bv) cmp = -1; + else if (av > bv) cmp = 1; + if (desc) cmp = -cmp; + if (cmp !== 0) return cmp; + return String(a?._id ?? "").localeCompare(String(b?._id ?? "")); + }); + } + } + return typeof limit === "number" ? result.slice(0, Math.max(0, limit)) : result; +} + +export function setBlockRange(result: DbQueryResult): void { + const times = result.docs + .map((doc) => doc?.updatedTimeUtc) + .filter((value): value is number => typeof value === "number"); + result.blockStart = times.length ? Math.max(...times) : 0; + result.blockEnd = times.length ? Math.min(...times) : 0; +} diff --git a/api/src/util/querySelector.ts b/api/src/util/querySelector.ts new file mode 100644 index 0000000000..ee7b593f13 --- /dev/null +++ b/api/src/util/querySelector.ts @@ -0,0 +1,85 @@ +import { HttpException, HttpStatus } from "@nestjs/common"; +import { Uuid } from "../enums"; +import { MongoComparisonCriteria, MongoSelectorDto } from "../dto/MongoSelectorDto"; + +/** + * Extract memberOf groups from the top-level $and array. + * (After expansion, memberOf will always be a condition in the $and array. + */ +export function extractMemberOf(selector: MongoSelectorDto): Uuid[] { + for (const condition of selector.$and || []) { + const memberOf = (condition as MongoSelectorDto).memberOf; + if (!memberOf) continue; + + if (typeof memberOf === "string") { + return [memberOf]; + } + + if (Array.isArray((memberOf as MongoComparisonCriteria).$in)) { + return (memberOf as MongoComparisonCriteria).$in as string[]; + } + + if (Array.isArray((memberOf as MongoComparisonCriteria).$elemMatch?.$in)) { + return (memberOf as MongoComparisonCriteria).$elemMatch.$in as string[]; + } + + throw new HttpException("Invalid memberOf field in selector", HttpStatus.BAD_REQUEST); + } + + return []; +} + +/** + * Remove memberOf from conditions in the top-level $and array. + * (After expansion, memberOf will always be a condition in the $and array.) + */ +export function removeMemberOf(selector: MongoSelectorDto): void { + if (!selector.$and) return; + + for (const condition of selector.$and) { + if ((condition as any).memberOf !== undefined) { + delete (condition as any).memberOf; + } + } + + // Remove any conditions that are now empty after memberOf deletion + selector.$and = selector.$and.filter((condition) => Object.keys(condition).length > 0); +} + +/** + * Extract a field value from the $and array. + * Returns the first matching value found, or undefined if not present. + * Throws if multiple different values are found for the same field. + */ +export function extractFieldFromAnd(andArray: MongoSelectorDto[], fieldName: string): T | undefined { + let foundValue: T | undefined; + + for (const condition of andArray) { + if (fieldName in condition) { + const value = condition[fieldName] as T; + + // Only accept simple equality values (string, number, boolean) + if ( + typeof value !== "string" && + typeof value !== "number" && + typeof value !== "boolean" + ) { + throw new HttpException( + `'${fieldName}' field must be a simple equality value`, + HttpStatus.BAD_REQUEST, + ); + } + + if (foundValue !== undefined && foundValue !== value) { + throw new HttpException( + `Multiple different '${fieldName}' values found in selector`, + HttpStatus.BAD_REQUEST, + ); + } + + foundValue = value; + } + } + + return foundValue; +} diff --git a/app/src/components/ExplorePage/PinnedTopics.vue b/app/src/components/ExplorePage/PinnedTopics.vue index 4f12fbbb66..32cc829a53 100644 --- a/app/src/components/ExplorePage/PinnedTopics.vue +++ b/app/src/components/ExplorePage/PinnedTopics.vue @@ -25,12 +25,10 @@ const topics = useContentQuery( ], // Resolve each pinned category to the post ids tagged with it (parentTaggedDocs — the // server-mirrored copy of the tag's taggedDocs) and seek child content by parentId. - // This indexes the local Dexie read and lets the API older-tail supplement fan out to - // per-parent index seeks when the combined parentId set is within the fan-out cap. - // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); above the client fan-out cap the API resolves the - // single request by fanning out to per-parent index seeks server-side. - // sort+limit bound the window; contentByTag re-sorts per category for display. + // Featured content can predate the sync cutoff, so the API older-tail supplement is + // REQUIRED to surface it (it is not in the local window) — see queryIntrospection.ts + // for how that supplement is built. sort+limit bound the window; contentByTag re-sorts + // per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/app/src/components/HomePage/HomePagePinned.vue b/app/src/components/HomePage/HomePagePinned.vue index 85833aab2d..04f72d2db2 100644 --- a/app/src/components/HomePage/HomePagePinned.vue +++ b/app/src/components/HomePage/HomePagePinned.vue @@ -32,12 +32,10 @@ const pinnedCategoryContent = useContentQuery( ], // Resolve each pinned category to the post ids tagged with it (parentTaggedDocs — the // server-mirrored copy of the tag's taggedDocs) and seek child content by parentId. - // This indexes the local Dexie read and lets the API older-tail supplement fan out to - // per-parent index seeks when the combined parentId set is within the fan-out cap. - // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); above the client fan-out cap the API resolves the - // single request by fanning out to per-parent index seeks server-side. - // sort+limit bound the window; contentByTag re-sorts per category for display. + // Featured content can predate the sync cutoff, so the API older-tail supplement is + // REQUIRED to surface it (it is not in the local window) — see queryIntrospection.ts + // for how that supplement is built. sort+limit bound the window; contentByTag re-sorts + // per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/app/src/components/VideoPage/PinnedVideo.vue b/app/src/components/VideoPage/PinnedVideo.vue index 0f5da4372a..7f5e26381d 100644 --- a/app/src/components/VideoPage/PinnedVideo.vue +++ b/app/src/components/VideoPage/PinnedVideo.vue @@ -33,12 +33,10 @@ const pinnedCategoryContent = useContentQuery( ], // Resolve each pinned category to the post ids tagged with it (parentTaggedDocs — the // server-mirrored copy of the tag's taggedDocs) and seek child content by parentId. - // This indexes the local Dexie read and lets the API older-tail supplement fan out to - // per-parent index seeks when the combined parentId set is within the fan-out cap. - // Featured content can predate the sync cutoff, so the supplement is REQUIRED to surface - // it (it is not in the local window); above the client fan-out cap the API resolves the - // single request by fanning out to per-parent index seeks server-side. - // sort+limit bound the window; contentByTag re-sorts per category for display. + // Featured content can predate the sync cutoff, so the API older-tail supplement is + // REQUIRED to surface it (it is not in the local window) — see queryIntrospection.ts + // for how that supplement is built. sort+limit bound the window; contentByTag re-sorts + // per category for display. { cache: true, limit: 50, sort: [{ publishDate: "desc" }] }, ); diff --git a/shared/src/api/sync/liveSync.ts b/shared/src/api/sync/liveSync.ts index d6f7a520ae..f070f482d8 100644 --- a/shared/src/api/sync/liveSync.ts +++ b/shared/src/api/sync/liveSync.ts @@ -4,7 +4,7 @@ import { db } from "../../db/database"; import { isSyncableDoc } from "../../db/isSyncable"; import { getSocket } from "../../socket/socketio"; import { setBaseRooms } from "../../socket/roomSubscriptions"; -import { getContentPublishDateCutoff } from "../../config"; +import { getContentPublishDateCutoff, isWindowedContentSubType } from "../../config"; import { syncList } from "./state"; import { splitChunkTypeString } from "./utils"; @@ -39,13 +39,14 @@ function roomDocTypesFromSyncList(): DocType[] { * `isSyncableDoc` (the sync-`syncList`-derived gate), and the result is written * via `db.bulkPut` (which resolves `DeleteCmd`s with its own stale-delete guard). * - * Below-cutoff Post content is written through ONLY if we're already keeping it offline - * (a `retention` row exists) — so a live edit to an offline-cached older article - * stays fresh, while a below-cutoff doc we aren't caching is not persisted (it - * would otherwise be written here and evicted on the next sync). DeleteCmds and - * above-cutoff / non-Content docs are unaffected; the gate is inert when no cutoff - * is configured (CMS). Exported so the persistence decision can be unit-tested - * without a live socket. + * Below-cutoff content of a windowed subtype (currently: Post — see + * isWindowedContentSubType) is written through ONLY if we're already keeping it + * offline (a `retention` row exists) — so a live edit to an offline-cached older + * article stays fresh, while a below-cutoff doc we aren't caching is not persisted + * (it would otherwise be written here and evicted on the next sync). DeleteCmds, + * non-windowed subtypes (Tag), and above-cutoff / non-Content docs are unaffected; + * the gate is inert when no cutoff is configured (CMS). Exported so the persistence + * decision can be unit-tested without a live socket. */ export async function applyLiveData(data: ApiDataResponseDto): Promise { // Docs the client is allowed to store in IndexedDB (shared gate with @@ -56,7 +57,7 @@ export async function applyLiveData(data: ApiDataResponseDto): Promise { const isBelowCutoffContent = (d: BaseDocumentDto): boolean => { if (d.type !== DocType.Content) return false; const content = d as ContentDto; - if (content.parentType !== DocType.Post) return false; + if (!isWindowedContentSubType(content.parentType)) return false; if (content.parentAlwaysOffline === true) return false; const pd = content.publishDate; return pd !== undefined && pd < cutoff; diff --git a/shared/src/api/sync/sync.ts b/shared/src/api/sync/sync.ts index 7df67d4aa6..df2ea57302 100644 --- a/shared/src/api/sync/sync.ts +++ b/shared/src/api/sync/sync.ts @@ -21,7 +21,11 @@ import { import { trim } from "./trim"; import { syncActive, syncList, syncTolerance } from "./state"; import { merge } from "./merge"; -import { getContentPublishDateCutoff, hasContentPublishDateCutoff } from "../../config"; +import { + getContentPublishDateCutoff, + hasContentPublishDateCutoff, + isWindowedContentSubType, +} from "../../config"; import { evictStaleBelowCutoff } from "../../db/retention"; let _httpService: HttpReq; @@ -268,13 +272,13 @@ export async function sync(options: SyncRunnerOptions): Promise { try { await _runSync(options); - // Post-content callers get an automatic companion run that syncs always-offline - // docs (parentAlwaysOffline === true) regardless of the publishDate cutoff. - // Centralized here so callers don't need to know about `alwaysOffline` or - // issue a second sync() call themselves. + // Windowed content callers (currently: Post — see isWindowedContentSubType) get an + // automatic companion run that syncs always-offline docs (parentAlwaysOffline === + // true) regardless of the publishDate cutoff. Centralized here so callers don't + // need to know about `alwaysOffline` or issue a second sync() call themselves. if ( options.type === DocType.Content && - options.subType === DocType.Post && + isWindowedContentSubType(options.subType) && !options.alwaysOffline && hasContentPublishDateCutoff() ) { @@ -286,14 +290,14 @@ export async function sync(options: SyncRunnerOptions): Promise { } async function _runSync(options: SyncRunnerOptions): Promise { - // publishDate is a Content-only sync dimension. Post content uses the configured cutoff - // so sync does not pull ordinary posts older than the app/HybridQuery treats as - // "remote-only". Tag content must remain open-ended: tags can be long-lived navigation - // parents, so applying the rolling Post window can exclude every matching document. - // Non-Content callers (Language, Redirect, Storage, AuthProvider, Group, …) leave the - // bounds undefined; downstream comparisons resolve those as OPEN_MIN/MAX. + // publishDate is a Content-only sync dimension. Windowed subtypes (currently: Post — + // see isWindowedContentSubType) use the configured cutoff so sync does not pull + // ordinary posts older than the app/HybridQuery treats as "remote-only". Non-windowed + // subtypes (Tag) remain open-ended. Non-Content callers (Language, Redirect, Storage, + // AuthProvider, Group, …) leave the bounds undefined; downstream comparisons resolve + // those as OPEN_MIN/MAX. if (options.type === DocType.Content) { - if (options.subType !== DocType.Post || options.alwaysOffline) { + if (!isWindowedContentSubType(options.subType) || options.alwaysOffline) { options.publishDateMin = OPEN_MIN; options.publishDateMax = OPEN_MAX; } else { diff --git a/shared/src/config.ts b/shared/src/config.ts index 46300508d4..a0388b48da 100644 --- a/shared/src/config.ts +++ b/shared/src/config.ts @@ -1,5 +1,5 @@ import { ref, Ref } from "vue"; -import { Uuid } from "./types"; +import { DocType, Uuid } from "./types"; import { OPEN_MIN } from "./api/sync/utils"; export const changeReqWarnings = ref([]); @@ -83,6 +83,19 @@ export function hasContentPublishDateCutoff(): boolean { return getContentPublishDateCutoff() !== OPEN_MIN; } +/** + * Single canonical definition of which Content `subType` (equivalently, a + * `ContentDto.parentType`) is windowed by the publishDate cutoff. Post is the only + * windowed subtype: Tag content stays fully synced regardless of the configured + * cutoff, since tags can be long-lived navigation parents that a rolling window + * would otherwise exclude wholesale. Callers should ask "is this windowed?" via + * this function rather than re-deriving the answer with an inline `=== DocType.Post` + * comparison, so the policy can't drift between call sites. + */ +export function isWindowedContentSubType(subType: DocType | undefined): boolean { + return subType === DocType.Post; +} + /** * TTL (ms) for offline-persisted below-cutoff Content. Read by the retention buffer * when stamping a doc's keep-alive deadline. Defaults to 30 days when unset. diff --git a/shared/src/db/retention.ts b/shared/src/db/retention.ts index c6667b0c61..15167888b5 100644 --- a/shared/src/db/retention.ts +++ b/shared/src/db/retention.ts @@ -25,7 +25,12 @@ import { DateTime } from "luxon"; import { db, type RetentionEntry } from "./database"; -import { config, getContentPublishDateCutoff, getOfflineRetentionTtl } from "../config"; +import { + config, + getContentPublishDateCutoff, + getOfflineRetentionTtl, + isWindowedContentSubType, +} from "../config"; import { DocType, type ContentDto } from "../types"; import { OPEN_MIN } from "../api/sync/utils"; import { scheduleCorpusStatsRecompute } from "../fts/ftsIndexer"; @@ -99,8 +104,9 @@ export async function flushRetention(): Promise { } /** - * Delete below-cutoff Post content whose retention deadline has passed (or was never set). - * Covers both supplement-persisted docs and Post content that slid out of the sync window. + * Delete below-cutoff windowed content (currently: Post — see isWindowedContentSubType) + * whose retention deadline has passed (or was never set). Covers both + * supplement-persisted docs and windowed content that slid out of the sync window. * Inert in CMS / when no cutoff is configured. Call only while online (it runs after a * content sync) so evicted-but-still-wanted docs can be re-fetched. */ @@ -126,7 +132,8 @@ export async function evictStaleBelowCutoff(): Promise { if (d.type !== DocType.Content) return false; const content = d as ContentDto; return ( - content.parentType === DocType.Post && content.parentAlwaysOffline !== true + isWindowedContentSubType(content.parentType) && + content.parentAlwaysOffline !== true ); }) .primaryKeys()) as string[]; From 20a59fe06167924cb5729bb2c19947081058f0f8 Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 27 Jul 2026 15:07:44 +0200 Subject: [PATCH 6/7] fix(api): remove temporary sync-context diagnostics from /query buildSyncContext/collectIncludedLanguages were added as temporary debug logging alongside the real updatedTimeUtc epoch-cursor fix and never cleaned up. Drop them and document the endpoint's actual request-time guards instead. Co-Authored-By: Claude Sonnet 5 --- api/src/endpoints/README.md | 22 ++++++++ api/src/endpoints/query.controller.spec.ts | 57 --------------------- api/src/endpoints/query.controller.ts | 58 ---------------------- 3 files changed, 22 insertions(+), 115 deletions(-) create mode 100644 api/src/endpoints/README.md diff --git a/api/src/endpoints/README.md b/api/src/endpoints/README.md new file mode 100644 index 0000000000..bab517ece3 --- /dev/null +++ b/api/src/endpoints/README.md @@ -0,0 +1,22 @@ +# Endpoints + +REST endpoints exposed by the API: `changeRequest`, `query`, `ftsSearch`, `storageStatus`. All are gated by `AuthGuard`. See `api/CLAUDE.md` for the full architecture of each; this file documents the `/query` request-time guards in `query.controller.ts`, since they're easy to lose track of. + +## `/query` request-time guards + +`processPostReq` runs a fixed sequence of cheap, pre-execution checks before a Mango query ever reaches CouchDB. Each one exists to stop a specific failure mode; none of them are diagnostic-only — every check here either rejects a request or feeds the rate limiter. + +1. **Rate-limit gate** (`rateLimiter.check`) — an identity already in backoff from prior expensive queries is rejected outright with `429` + `Retry-After`. No-op when `QueryRateLimiterService` is disabled. +2. **Epoch-cursor rejection** — `selector.updatedTimeUtc` with both `$lte` and `$gte` at `0` can never match a real document (timestamps are always `> 0`), but CouchDB still has to walk the chosen index before returning empty. A sync client with a corrupted/reset cursor could otherwise generate a full-index scan on every poll. This check runs even when `BYPASS_TEMPLATE_VALIDATION=true`, because it isn't schema validation — it's an invariant about what a valid query can ever match. +3. **`parentId` fan-out cap** (`countParentIdFanout`) — `QueryService` issues one CouchDB request per id in `selector.parentId.$in`, so an unbounded array is worse than the full scan it replaced. Sizes above `query.maxFanoutParents` (default 200) are rejected; sizes above `query.fanoutStrikeThreshold` (default 25) are allowed but immediately strike the rate limiter, since the cost is known before the query runs — no need to wait on post-hoc `execution_stats`. +4. **Schema/operator validation** (`validateQuery`) — the universal selector validator (shape, `limit` cap, `use_index` registry membership, operator allowlist, per-request language cap for non-CMS queries). Skippable via `BYPASS_TEMPLATE_VALIDATION` (dev/test only). + +After the query executes, `classifyQueryCost` inspects `execution_stats` to decide if the query was expensive (docs-examined threshold or examined:returned ratio). An expensive query logs a `warn("Expensive /query", …)` line — `identifier`, `identity`, `reason`, the execution stats, `use_index`, and a `selectorFingerprint(body)` of the post-injection selector — and strikes the rate limiter (enforcement bites the *next* request from that identity, not this one). `execution_stats` itself is always stripped from the client response. + +`body.identifier` is not used for dispatch — it's only a caller-supplied label carried into this log line and into rate-limit context (e.g. `"sync"` for app/CMS sync calls). + +### Adding a new pre-execution guard + +If you find another selector shape that's cheap to reject up front (known before `QueryService` runs), follow the pattern above: check it before `validateQuery`, throw `BadRequestException` for the reject case, and call `rateLimiter.recordStrike(identityKey)` for an allowed-but-costly case rather than waiting on `execution_stats`. Keep the check itself in `query.controller.ts` as a small pure function (see `countParentIdFanout`) so it's unit-testable without a DB. + +Avoid adding fields here purely for logging/debugging an investigation — that kind of temporary diagnostic instrumentation tends to outlive the investigation it was added for. If you need to correlate an expensive-query spike with request shape, prefer extending `selectorFingerprint` (already computed on every expensive-query log line) over introducing a new ad-hoc context object. diff --git a/api/src/endpoints/query.controller.spec.ts b/api/src/endpoints/query.controller.spec.ts index 5c78c14628..c6ad0642d9 100644 --- a/api/src/endpoints/query.controller.spec.ts +++ b/api/src/endpoints/query.controller.spec.ts @@ -232,63 +232,6 @@ describe("QueryController", () => { expect(result.execution_stats).toBeUndefined(); }); - it("logs the pre-injection sync dimensions for an expensive sync query", async () => { - configService.get.mockImplementation(configFor(true)); - queryService.query.mockResolvedValue({ - docs: [], - execution_stats: { total_docs_examined: 2419, execution_time_ms: 600 }, - }); - - const body = { - identifier: "sync", - selector: { - type: "content", - updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 }, - parentType: "post", - memberOf: { $elemMatch: { $in: ["group-a", "group-b"] } }, - $or: [ - { language: { $in: ["lang-eng", "lang-fra"] } }, - { - $and: [ - { - $not: { - availableTranslations: { $elemMatch: { $eq: "lang-eng" } }, - }, - }, - ], - }, - ], - publishDate: { $gte: 1234 }, - }, - limit: 100, - sort: [{ updatedTimeUtc: "desc" }], - use_index: "sync-content-index", - cms: false, - includeExpired: false, - }; - - await controller.processPostReq(body, mockRequest(), mockReply()); - - expect(logger.warn).toHaveBeenCalledWith( - "Expensive /query", - expect.objectContaining({ - sync_context: { - parentType: "post", - updatedTimeUtc: { $lte: Number.MAX_SAFE_INTEGER, $gte: 0 }, - publishDate: { $gte: 1234 }, - requestedMemberOf: ["group-a", "group-b"], - requestedMemberOfCount: 2, - requestedLanguages: ["lang-eng", "lang-fra"], - requestedLanguageCount: 2, - cms: false, - includeExpired: false, - limit: 100, - use_index: "sync-content-index", - }, - }), - ); - }); - it("keys an anonymous identity by ip when there is no userId", async () => { configService.get.mockImplementation(configFor(true)); queryService.query.mockResolvedValue({ diff --git a/api/src/endpoints/query.controller.ts b/api/src/endpoints/query.controller.ts index c556c0c2fc..6dd0e99b53 100644 --- a/api/src/endpoints/query.controller.ts +++ b/api/src/endpoints/query.controller.ts @@ -101,10 +101,6 @@ export class QueryController { // `identifier` is an observability label only; strip it before the query runs. const identifier = typeof body?.identifier === "string" ? body.identifier : "unknown"; - // Capture the client-created sync dimensions before QueryService expands/mutates the - // selector and injects permission/publication filters. Temporary diagnostic context for - // identifying which sync column is producing an expensive CouchDB scan. - const syncContext = identifier === "sync" ? buildSyncContext(body) : undefined; delete body.identifier; const result = await this.queryService.query(body as MongoQueryDto, request.user); @@ -131,7 +127,6 @@ export class QueryController { results_returned: result?.docs?.length ?? 0, execution_time_ms: result?.execution_stats?.execution_time_ms, use_index: body?.use_index, - ...(syncContext ? { sync_context: syncContext } : {}), // Computed lazily on the post-injection query — reflects what CouchDB // actually executed, which is what you want when deciding on an index. fingerprint: selectorFingerprint(body), @@ -162,56 +157,3 @@ function countParentIdFanout(selector: any): number { } return 0; } - -function buildSyncContext(body: any) { - const selector = body?.selector ?? {}; - const requestedMemberOf = Array.isArray(selector?.memberOf?.$elemMatch?.$in) - ? selector.memberOf.$elemMatch.$in - : []; - const requestedLanguages = collectIncludedLanguages(selector); - - return { - parentType: selector.parentType, - updatedTimeUtc: selector.updatedTimeUtc, - publishDate: selector.publishDate, - requestedMemberOf, - requestedMemberOfCount: requestedMemberOf.length, - requestedLanguages, - requestedLanguageCount: requestedLanguages.length, - cms: body?.cms === true, - includeExpired: body?.includeExpired === true, - limit: body?.limit, - use_index: body?.use_index, - }; -} - -function collectIncludedLanguages(selector: any): string[] { - const languages = new Set(); - - function visit(node: any): void { - if (!node || typeof node !== "object") return; - if (Array.isArray(node)) { - node.forEach(visit); - return; - } - - for (const [key, value] of Object.entries(node)) { - if (key === "language") { - if (typeof value === "string") languages.add(value); - else if (value && typeof value === "object") { - const criterion = value as { $eq?: unknown; $in?: unknown }; - if (typeof criterion.$eq === "string") languages.add(criterion.$eq); - if (Array.isArray(criterion.$in)) { - criterion.$in.forEach((language) => { - if (typeof language === "string") languages.add(language); - }); - } - } - } - visit(value); - } - } - - visit(selector); - return [...languages]; -} From 6d8fb8db077f46033b1abeb186847b32474542b0 Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 27 Jul 2026 15:37:56 +0200 Subject: [PATCH 7/7] fix(shared): skip degenerate both-zero floor query in syncBatch (#1815) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit calcChunk returns {blockStart: 0, blockEnd: 0} on the initialSync:false continuation when the column's only stored chunk has blockEnd === 0 (already at the epoch floor). The existing sub-tolerance early-return only catches strictly inverted ranges (blockStart < blockEnd), so the equal both-zero range fell through to the /query POST — producing the updatedTimeUtc $lte:0,$gte:0 selector that walked the whole CouchDB index returning no docs (#1815). Add a narrow early-return guard that seals the column eof and skips the /query call when blockStart === 0 && blockEnd === 0. A healthy eof column's catch-up poll (blockStart = MAX_SAFE_INTEGER, blockEnd = frontier.blockStart - syncTolerance) is left intact so new-doc detection still works. The API's both-zero 400 guard (query.controller.ts) remains the server-side backstop. Co-Authored-By: Claude --- shared/src/api/sync/syncBatch.spec.ts | 67 ++++++++++++++++++++++++++- shared/src/api/sync/syncBatch.ts | 15 ++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/shared/src/api/sync/syncBatch.spec.ts b/shared/src/api/sync/syncBatch.spec.ts index bd109845fa..e934ded2f5 100644 --- a/shared/src/api/sync/syncBatch.spec.ts +++ b/shared/src/api/sync/syncBatch.spec.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { syncBatch } from "./syncBatch"; import { setCancelSync } from "./sync"; -import { syncList } from "./state"; +import { syncList, syncTolerance } from "./state"; import { DocType, BaseDocumentDto } from "../../types"; import { OPEN_MAX, OPEN_MIN } from "./utils"; @@ -1229,4 +1229,69 @@ describe("syncBatch", () => { expect(syncList.value[0].chunkType).toBe("content:post:alwaysOffline"); }); }); + + describe("degenerate floor column (both-zero range — #1815)", () => { + // calcChunk returns {blockStart: 0, blockEnd: 0} when a column's only stored chunk has + // blockEnd === 0 (already at the epoch floor) on the initialSync:false continuation path. + // That yields the degenerate updatedTimeUtc $lte:0,$gte:0 selector that walked the whole + // CouchDB index returning no docs. + + it("skips /query and seals the column eof for an already-floored column", async () => { + syncList.value = [ + { + chunkType: "redirect", + memberOf: ["g1"], + blockStart: 5000, + blockEnd: 0, + eof: false, + } as any, + ]; + const http = { post: vi.fn(async () => ({ docs: [] })) }; + const result = await syncBatch({ + type: DocType.Redirect, + memberOf: ["g1"], + limit: 100, + initialSync: false, + httpService: http as any, + }); + expect(http.post).not.toHaveBeenCalled(); + expect(result?.eof).toBe(true); + expect(syncList.value[0].eof).toBe(true); + }); + + it("still polls /query for a healthy eof column's catch-up window (initialSync:true)", async () => { + // A healthy eof column must still be polled for new docs: initialSync:true gives + // blockStart = MAX_SAFE_INTEGER, blockEnd = frontier.blockStart - syncTolerance — a + // bounded catch-up window, NOT the both-zero floor range. The new guard must not + // suppress it. + syncList.value = [ + { + chunkType: "redirect", + memberOf: ["g1"], + blockStart: 5000, + blockEnd: 0, + eof: true, + } as any, + ]; + const capturedBodies: any[] = []; + const http = { + post: vi.fn(async (_path: string, body: any) => { + capturedBodies.push(body); + return { docs: [] }; + }), + }; + await syncBatch({ + type: DocType.Redirect, + memberOf: ["g1"], + limit: 100, + initialSync: true, + httpService: http as any, + }); + expect(http.post).toHaveBeenCalledTimes(1); + expect(capturedBodies[0].selector.updatedTimeUtc.$lte).toBe( + Number.MAX_SAFE_INTEGER, + ); + expect(capturedBodies[0].selector.updatedTimeUtc.$gte).toBe(5000 - syncTolerance); + }); + }); }); diff --git a/shared/src/api/sync/syncBatch.ts b/shared/src/api/sync/syncBatch.ts index 768d420796..264890cce3 100644 --- a/shared/src/api/sync/syncBatch.ts +++ b/shared/src/api/sync/syncBatch.ts @@ -42,6 +42,21 @@ export async function syncBatch(options: SyncOptions) { // that this is the first sync for this type and memberOf groups. const firstSync = chunk.blockEnd === 0; + // A column whose frontier is already at the epoch floor (blockStart === 0 && blockEnd === 0) + // has no valid updatedTimeUtc window to query — $lte:0,$gte:0 can only match docs at exactly + // the epoch, which real content never carries, yet CouchDB still walks the chosen index before + // returning empty (the #1815 full-table scan). Seal the column eof and skip the /query call + // entirely; the API's both-zero 400 guard (query.controller.ts) is the server-side backstop for + // any cursor corruption that still reaches it. A healthy eof column's catch-up poll + // (blockStart = MAX_SAFE_INTEGER, blockEnd = frontier.blockStart - syncTolerance) is left intact + // so new-doc detection still works. + if (chunk.blockStart === 0 && chunk.blockEnd === 0) { + const mergeResult = merge(options); + mergeResult.eof = true; + markColumnEof(options); + return { ...mergeResult, firstSync }; + } + // If the calculated range is inverted (blockStart < blockEnd) AND the inversion is within // `syncTolerance`, treat it as the intended sub-tolerance boundary gap — stop iteration and // seal the frontier to avoid infinite recursion on identical chunks.