From c0bd9ecf696de0751603890b82182e6858fbcff8 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:08:51 +0000 Subject: [PATCH] fix(smart-scope): handle invalid filters gracefully in count calculation Validate smart scope filter rules before constructing query clauses during count computation. Catch validation errors for broken rules so invalid filters return null for bookCount rather than triggering an internal server error when fetching all smart scopes. Closes #787 --- packages/types/src/smart-scope.ts | 2 +- .../src/common/utils/timezone.utils.test.ts | 23 ++++ server/src/common/utils/timezone.utils.ts | 64 +++++++-- .../book/book-query-builder.service.test.ts | 10 ++ .../book/pipes/book-query.pipe.test.ts | 11 ++ .../book/utils/group-rule.validator.test.ts | 127 ++++++++++++++++-- .../book/utils/group-rule.validator.ts | 33 +++++ .../smart-scope/smart-scope.service.test.ts | 67 +++++++++ .../smart-scope/smart-scope.service.ts | 17 ++- 9 files changed, 334 insertions(+), 20 deletions(-) diff --git a/packages/types/src/smart-scope.ts b/packages/types/src/smart-scope.ts index ba136950a..d088a457b 100644 --- a/packages/types/src/smart-scope.ts +++ b/packages/types/src/smart-scope.ts @@ -10,7 +10,7 @@ export interface SmartScope { isPublic: boolean; syncToKobo: boolean; displayOrder: number; - bookCount?: number; + bookCount?: number | null; createdAt: string; updatedAt: string; } diff --git a/server/src/common/utils/timezone.utils.test.ts b/server/src/common/utils/timezone.utils.test.ts index ae9b4e506..cf00819e5 100644 --- a/server/src/common/utils/timezone.utils.test.ts +++ b/server/src/common/utils/timezone.utils.test.ts @@ -18,6 +18,12 @@ describe('timezone utils', () => { }); it('validates date keys with strict calendar checks', () => { + expect(isDateKey('0000-01-01')).toBe(false); + expect(isDateKey('0001-01-01')).toBe(true); + expect(isDateKey('0021-12-31')).toBe(true); + expect(isDateKey('0099-01-01')).toBe(true); + expect(isDateKey('0100-01-01')).toBe(true); + expect(isDateKey('0999-12-31')).toBe(true); expect(isDateKey('2026-05-20')).toBe(true); expect(isDateKey('2026-02-29')).toBe(false); expect(isDateKey('2024-02-29')).toBe(true); @@ -32,6 +38,15 @@ describe('timezone utils', () => { expect(toDateKeyInTimeZone(instant, 'America/New_York')).toBe('2025-12-31'); }); + it.each(['0001-01-01', '0021-12-31', '0099-01-01', '0100-01-01', '0999-12-31'])('preserves a four-digit year when formatting %s', (dateKey) => { + expect(toDateKeyInTimeZone(new Date(`${dateKey}T12:00:00.000Z`), 'UTC')).toBe(dateKey); + }); + + it('rejects dates outside the supported date-key range', () => { + expect(() => toDateKeyInTimeZone(new Date('0000-01-01T12:00:00.000Z'), 'UTC')).toThrow(RangeError); + expect(() => toDateKeyInTimeZone(new Date('+010000-01-01T12:00:00.000Z'), 'UTC')).toThrow(RangeError); + }); + it('converts a date key to a UTC timestamp representing local midnight', () => { const utcMidnight = toTimeZoneStartOfDay('2026-05-20', 'UTC'); expect(utcMidnight.toISOString()).toBe('2026-05-20T00:00:00.000Z'); @@ -40,6 +55,14 @@ describe('timezone utils', () => { expect(toDateKeyInTimeZone(denverMidnight, 'America/Denver')).toBe('2026-05-20'); }); + it.each(['0001-01-01', '0021-12-31', '0099-01-01'])('converts low-year date key %s without applying a 1900 offset', (dateKey) => { + const utcMidnight = toTimeZoneStartOfDay(dateKey, 'UTC'); + expect(toDateKeyInTimeZone(utcMidnight, 'UTC')).toBe(dateKey); + + const denverMidnight = toTimeZoneStartOfDay(dateKey, 'America/Denver'); + expect(toDateKeyInTimeZone(denverMidnight, 'America/Denver')).toBe(dateKey); + }); + it('throws when date key is malformed', () => { expect(() => toTimeZoneStartOfDay('2026-02-30', 'UTC')).toThrow(RangeError); }); diff --git a/server/src/common/utils/timezone.utils.ts b/server/src/common/utils/timezone.utils.ts index 6bb0b3a87..c702afb68 100644 --- a/server/src/common/utils/timezone.utils.ts +++ b/server/src/common/utils/timezone.utils.ts @@ -1,13 +1,19 @@ -import { fromZonedTime } from 'date-fns-tz'; - const DATE_KEY_RE = /^(\d{4})-(\d{2})-(\d{2})$/; +type DateTimePartType = 'year' | 'month' | 'day' | 'hour' | 'minute' | 'second' | 'era'; -function getDatePart(parts: Intl.DateTimeFormatPart[], type: 'year' | 'month' | 'day'): string { +function getDatePart(parts: Intl.DateTimeFormatPart[], type: DateTimePartType): string { const part = parts.find((entry) => entry.type === type); if (!part) throw new RangeError(`Missing '${type}' in formatted date parts`); return part.value; } +function createUtcDate(year: number, month: number, day: number, hour = 0, minute = 0, second = 0): Date { + const date = new Date(0); + date.setUTCHours(hour, minute, second, 0); + date.setUTCFullYear(year, month - 1, day); + return date; +} + export function isValidTimeZone(timeZone: string): boolean { try { new Intl.DateTimeFormat('en-US', { timeZone }); @@ -30,7 +36,8 @@ export function isDateKey(value: string): boolean { const year = Number(match[1]); const month = Number(match[2]); const day = Number(match[3]); - const parsed = new Date(Date.UTC(year, month - 1, day)); + if (year === 0) return false; + const parsed = createUtcDate(year, month, day); return parsed.getUTCFullYear() === year && parsed.getUTCMonth() + 1 === month && parsed.getUTCDate() === day; } @@ -43,20 +50,57 @@ export function toDateKeyInTimeZone(date: Date, timeZone: string): string { year: 'numeric', month: '2-digit', day: '2-digit', + era: 'short', }).formatToParts(date); - const year = getDatePart(parts, 'year'); + const era = parts.find((entry) => entry.type === 'era')?.value; + const year = Number(getDatePart(parts, 'year')); const month = getDatePart(parts, 'month'); const day = getDatePart(parts, 'day'); - return `${year}-${month}-${day}`; + if (era !== 'AD' || !Number.isInteger(year) || year < 1 || year > 9999) { + throw new RangeError('Date is outside the supported date-key range'); + } + return `${String(year).padStart(4, '0')}-${month}-${day}`; } export function toTimeZoneStartOfDay(dateKey: string, timeZone: string): Date { if (!isDateKey(dateKey)) { throw new RangeError(`Invalid date key: ${dateKey}`); } - const parsed = fromZonedTime(`${dateKey}T00:00:00`, timeZone); - if (Number.isNaN(parsed.getTime())) { - throw new RangeError(`Invalid timezone conversion for date key: ${dateKey}`); + const match = DATE_KEY_RE.exec(dateKey)!; + const year = Number(match[1]); + const month = Number(match[2]); + const day = Number(match[3]); + const target = createUtcDate(year, month, day); + const formatter = new Intl.DateTimeFormat('en-US', { + timeZone, + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + hourCycle: 'h23', + era: 'short', + }); + let candidate = target; + + for (let attempt = 0; attempt < 4; attempt += 1) { + const parts = formatter.formatToParts(candidate); + const formattedYear = Number(getDatePart(parts, 'year')); + const era = getDatePart(parts, 'era'); + const astronomicalYear = era === 'BC' ? 1 - formattedYear : formattedYear; + const formattedAsUtc = createUtcDate( + astronomicalYear, + Number(getDatePart(parts, 'month')), + Number(getDatePart(parts, 'day')), + Number(getDatePart(parts, 'hour')), + Number(getDatePart(parts, 'minute')), + Number(getDatePart(parts, 'second')), + ); + const adjustmentMs = target.getTime() - formattedAsUtc.getTime(); + if (adjustmentMs === 0) return candidate; + candidate = new Date(candidate.getTime() + adjustmentMs); } - return parsed; + + throw new RangeError(`Invalid timezone conversion for date key: ${dateKey}`); } diff --git a/server/src/modules/book/book-query-builder.service.test.ts b/server/src/modules/book/book-query-builder.service.test.ts index b840b7da7..c3f14b29a 100644 --- a/server/src/modules/book/book-query-builder.service.test.ts +++ b/server/src/modules/book/book-query-builder.service.test.ts @@ -1219,6 +1219,16 @@ describe('read status date filters (startedAt / finishedAt)', () => { const rule = getRuleSql(where); expect(rule.values[1]).toBe('2025-12-31'); }); + + it('preserves a zero-padded year below 1000', () => { + const { builder } = makeBuilder(); + const where = builder.buildWhere( + wrapRule({ type: 'rule', field: 'finishedAt', operator: 'before', value: '0021-12-31' }) as never, + USER_CTX, + ) as any; + const rule = getRuleSql(where); + expect(rule.values[1]).toBe('0021-12-31'); + }); }); describe('statusRuleToSql (fileAvailability)', () => { diff --git a/server/src/modules/book/pipes/book-query.pipe.test.ts b/server/src/modules/book/pipes/book-query.pipe.test.ts index f1609c6c9..5bedf8697 100644 --- a/server/src/modules/book/pipes/book-query.pipe.test.ts +++ b/server/src/modules/book/pipes/book-query.pipe.test.ts @@ -50,6 +50,17 @@ describe('BookQueryPipe', () => { expect(result.filter).toBeDefined(); }); + it('accepts ISO timestamps supported by date filter rules', () => { + const result = pipe.transform({ + filter: { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field: 'startedAt', operator: 'before', value: '2026-01-01T02:30:00.000Z' }], + }, + }); + expect(result.filter).toBeDefined(); + }); + it('throws BadRequestException for invalid sort field', () => { expect(() => pipe.transform({ sort: [{ field: 'unknownField', dir: 'asc' }] })).toThrow(BadRequestException); }); diff --git a/server/src/modules/book/utils/group-rule.validator.test.ts b/server/src/modules/book/utils/group-rule.validator.test.ts index 21770e222..3e9eb3192 100644 --- a/server/src/modules/book/utils/group-rule.validator.test.ts +++ b/server/src/modules/book/utils/group-rule.validator.test.ts @@ -6,14 +6,17 @@ import { validateGroupRule, groupRuleSchema } from './group-rule.validator'; /** * Returns a minimal { value, valueTo } pair that satisfies the Zod schema for a - * given operator. The validator only checks operator-field compatibility and - * value type (string | number | string[] | number[]), not field-level semantics, - * so any structurally valid value is sufficient here. + * given field/operator pair. The validator checks operator-field compatibility, + * value type (string | number | string[] | number[]), and - for date fields with + * before/after/between - that the value(s) parse to a real date, so date fields + * need real date strings here while everything else can use structurally valid stubs. * * The exhaustive `never` default ensures TypeScript raises a compile error when a * new operator is added to RuleOperator but not handled in this helper. */ -function validRuleValue(operator: RuleOperator): { value?: unknown; valueTo?: unknown } { +const DATE_FIELDS: RuleField[] = ['addedAt', 'startedAt', 'finishedAt', 'publishedDate']; + +function validRuleValue(operator: RuleOperator, field: RuleField): { value?: unknown; valueTo?: unknown } { switch (operator) { case 'isEmpty': case 'isNotEmpty': @@ -26,14 +29,15 @@ function validRuleValue(operator: RuleOperator): { value?: unknown; valueTo?: un case 'isUnlocked': case 'isUpNext': return {}; + case 'before': + case 'after': + return { value: '2024-01-01' }; case 'contains': case 'notContains': case 'startsWith': case 'endsWith': case 'eq': case 'notEq': - case 'before': - case 'after': return { value: 'test' }; case 'gt': case 'gte': @@ -42,7 +46,7 @@ function validRuleValue(operator: RuleOperator): { value?: unknown; valueTo?: un case 'withinLast': return { value: 1 }; case 'between': - return { value: 1, valueTo: 2 }; + return DATE_FIELDS.includes(field) ? { value: '2024-01-01', valueTo: '2024-06-01' } : { value: 1, valueTo: 2 }; case 'includesAny': case 'includesAll': case 'excludesAll': @@ -293,6 +297,113 @@ describe('validateGroupRule', () => { }); }); +describe('date field value validation', () => { + const DATE_FIELDS_UNDER_TEST: RuleField[] = ['addedAt', 'startedAt', 'finishedAt', 'publishedDate']; + + it.each(DATE_FIELDS_UNDER_TEST)("rejects a 2-digit year for '%s' before/after/between (issue #787 regression)", (field) => { + const before = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'before', value: '21-12-31' }] }; + const after = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'after', value: '21-12-31' }] }; + const between = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field, operator: 'between', value: '2023-01-01', valueTo: '21-12-31' }], + }; + + expect(() => validateGroupRule(before)).toThrow(BadRequestException); + expect(() => validateGroupRule(after)).toThrow(BadRequestException); + expect(() => validateGroupRule(between)).toThrow(BadRequestException); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("rejects a non-date string for '%s' with before", (field) => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'before', value: 'not-a-date' }] }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("rejects a missing value for '%s' with after", (field) => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'after' }] }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("rejects a valid value with a malformed valueTo for '%s' between", (field) => { + const rule = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field, operator: 'between', value: '2023-01-01', valueTo: 'nope' }], + }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("accepts a well-formed YYYY-MM-DD value for '%s' before/after/between", (field) => { + const before = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'before', value: '2024-01-01' }] }; + const after = { type: 'group', join: 'AND', rules: [{ type: 'rule', field, operator: 'after', value: '2024-01-01' }] }; + const between = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field, operator: 'between', value: '2023-01-01', valueTo: '2024-01-01' }], + }; + + expect(() => validateGroupRule(before)).not.toThrow(); + expect(() => validateGroupRule(after)).not.toThrow(); + expect(() => validateGroupRule(between)).not.toThrow(); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("accepts a zero-padded year below 1000 for '%s'", (field) => { + const rule = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field, operator: 'before', value: '0021-12-31' }], + }; + expect(validateGroupRule(rule)).toBeDefined(); + }); + + it.each(DATE_FIELDS_UNDER_TEST)("accepts a valid ISO timestamp for '%s'", (field) => { + const rule = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field, operator: 'before', value: '2026-01-01T02:30:00.000Z' }], + }; + expect(validateGroupRule(rule)).toBeDefined(); + }); + + it('rejects a calendar-invalid date like 2024-02-30 even though the regex shape matches', () => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'finishedAt', operator: 'before', value: '2024-02-30' }] }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it('rejects an ISO timestamp whose calendar date is invalid', () => { + const rule = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field: 'finishedAt', operator: 'before', value: '2024-02-30T00:00:00.000Z' }], + }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it('rejects an ISO timestamp whose time is invalid', () => { + const rule = { + type: 'group', + join: 'AND', + rules: [{ type: 'rule', field: 'finishedAt', operator: 'before', value: '2024-02-29T24:00:00.000Z' }], + }; + expect(() => validateGroupRule(rule)).toThrow(BadRequestException); + }); + + it('does not require date-shaped values for withinLast (a day count, not a date)', () => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'finishedAt', operator: 'withinLast', value: 30 }] }; + expect(validateGroupRule(rule)).toBeDefined(); + }); + + it('does not apply date validation to non-date fields sharing the between operator', () => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'pageCount', operator: 'between', value: 100, valueTo: 300 }] }; + expect(validateGroupRule(rule)).toBeDefined(); + }); + + it('accepts a finite epoch value supported by the query builder', () => { + const rule = { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'addedAt', operator: 'before', value: Date.UTC(2024, 0, 1) }] }; + expect(validateGroupRule(rule)).toBeDefined(); + }); +}); + describe('groupRuleSchema depth enforcement', () => { it('groups at depth 5 are valid', () => { const schema = groupRuleSchema(5); @@ -324,7 +435,7 @@ describe('groupRuleSchema depth enforcement', () => { describe('field × operator exhaustive validation', () => { it.each(Object.entries(FIELD_OPERATORS) as [RuleField, RuleOperator[]][])('accepts all valid operators for field: %s', (field, operators) => { for (const operator of operators) { - const { value, valueTo } = validRuleValue(operator); + const { value, valueTo } = validRuleValue(operator, field); const rule: Record = { type: 'rule', field, operator }; if (value !== undefined) rule.value = value; if (valueTo !== undefined) rule.valueTo = valueTo; diff --git a/server/src/modules/book/utils/group-rule.validator.ts b/server/src/modules/book/utils/group-rule.validator.ts index d252a8167..18b20f976 100644 --- a/server/src/modules/book/utils/group-rule.validator.ts +++ b/server/src/modules/book/utils/group-rule.validator.ts @@ -9,10 +9,35 @@ import { type CommunityRatingProvider, type GroupRule, type Rule, + type RuleField, + type RuleOperator, } from '@bookorbit/types'; +import { isDateKey } from '../../../common/utils/timezone.utils'; const COMMUNITY_RATING_PROVIDER_VALUES = ['any', ...COMMUNITY_RATING_PROVIDER_KEYS] as const; +const DATE_VALUE_FIELDS = new Set(['addedAt', 'startedAt', 'finishedAt', 'publishedDate']); +const DATE_VALUE_OPERATORS = new Set(['before', 'after', 'between']); +const ISO_DATE_TIME_RE = /^(\d{4}-\d{2}-\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/; + +function isValidRuleDateValue(value: unknown): boolean { + if (typeof value === 'number') { + return Number.isFinite(value) && !Number.isNaN(new Date(value).getTime()); + } + if (typeof value !== 'string') return false; + + const normalized = value.trim(); + if (isDateKey(normalized)) return true; + + const isoMatch = ISO_DATE_TIME_RE.exec(normalized); + if (!isoMatch || !isDateKey(isoMatch[1]!)) return false; + + const hour = Number(isoMatch[2]); + const minute = Number(isoMatch[3]); + const second = Number(isoMatch[4]); + return hour <= 23 && minute <= 59 && second <= 59 && !Number.isNaN(new Date(normalized).getTime()); +} + const ruleSchema: z.ZodType = z .object({ type: z.literal('rule'), @@ -29,6 +54,14 @@ const ruleSchema: z.ZodType = z if (rule.provider !== undefined && rule.field !== 'communityRating') { ctx.addIssue({ code: 'custom', message: 'Provider is only valid for community rating rules', path: ['provider'] }); } + if (DATE_VALUE_FIELDS.has(rule.field as RuleField) && DATE_VALUE_OPERATORS.has(rule.operator as RuleOperator)) { + if (!isValidRuleDateValue(rule.value)) { + ctx.addIssue({ code: 'custom', message: `Operator '${rule.operator}' requires a valid date value`, path: ['value'] }); + } + if (rule.operator === 'between' && !isValidRuleDateValue(rule.valueTo)) { + ctx.addIssue({ code: 'custom', message: `Operator '${rule.operator}' requires a valid date valueTo`, path: ['valueTo'] }); + } + } }) as z.ZodType; const groupRuleSchema = (maxDepth: number): z.ZodType => diff --git a/server/src/modules/smart-scope/smart-scope.service.test.ts b/server/src/modules/smart-scope/smart-scope.service.test.ts index cbabca77e..70b6e879b 100644 --- a/server/src/modules/smart-scope/smart-scope.service.test.ts +++ b/server/src/modules/smart-scope/smart-scope.service.test.ts @@ -129,6 +129,73 @@ describe('SmartScopeService', () => { ]); }); + it('findAll marks a single broken scope count unavailable instead of failing the whole list (issue #787 regression)', async () => { + const { service, smartScopeRepo, libraryService, queryBuilder, bookReadService } = makeService(); + const logger = (service as unknown as { logger: Logger }).logger; + const errorSpy = vi.spyOn(logger, 'error').mockImplementation(() => undefined); + const user = makeUser({ id: 8 }); + const brokenScope = makeSmartScope({ + id: 1, + name: 'Broken Scope', + filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'finishedAt', operator: 'after', value: '21-12-31' }] }, + }); + const healthyScope = makeSmartScope({ + id: 2, + name: 'Healthy Scope', + filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'space' }] }, + }); + + smartScopeRepo.findAllForUser.mockResolvedValue([brokenScope, healthyScope]); + libraryService.findAccessibleLibraryIds.mockResolvedValue([2, 3]); + queryBuilder.buildWhere.mockReturnValueOnce('where-2'); + bookReadService.countWhere.mockResolvedValueOnce(7); + + const result = await service.findAll(user); + + expect(result).toEqual([ + { ...brokenScope, bookCount: null }, + { ...healthyScope, bookCount: 7 }, + ]); + expect(queryBuilder.buildWhere).toHaveBeenCalledTimes(1); + expect(queryBuilder.buildWhere).toHaveBeenCalledWith(healthyScope.filter, { accessibleLibraryIds: [2, 3], userId: 8, timeZone: 'UTC' }); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('[smart_scope.count] [fail] scopeId=1 userId=8')); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('durationMs=')); + }); + + it('findAll propagates unexpected count query failures', async () => { + const { service, smartScopeRepo, libraryService, queryBuilder, bookReadService } = makeService(); + const user = makeUser({ id: 8 }); + const brokenScope = makeSmartScope({ + id: 3, + filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'space' }] }, + }); + + smartScopeRepo.findAllForUser.mockResolvedValue([brokenScope]); + libraryService.findAccessibleLibraryIds.mockResolvedValue([2, 3]); + queryBuilder.buildWhere.mockReturnValueOnce('where-3'); + bookReadService.countWhere.mockRejectedValueOnce(new Error('date/time field value out of range: "21-12-31"')); + + await expect(service.findAll(user)).rejects.toThrow('date/time field value out of range'); + }); + + it('findAll propagates unexpected filter builder failures', async () => { + const { service, smartScopeRepo, libraryService, queryBuilder } = makeService(); + const user = makeUser({ id: 8 }); + const brokenScope = makeSmartScope({ + id: 3, + filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'space' }] }, + }); + + smartScopeRepo.findAllForUser.mockResolvedValue([brokenScope]); + libraryService.findAccessibleLibraryIds.mockResolvedValue([2, 3]); + queryBuilder.buildWhere.mockImplementationOnce(() => { + throw new TypeError('unexpected builder failure'); + }); + + await expect(service.findAll(user)).rejects.toThrow('unexpected builder failure'); + }); + it('create sets defaults and persists validated values', async () => { const { service, smartScopeRepo } = makeService(); const created = makeSmartScope({ id: 7, isPublic: false, defaultSort: [{ field: 'title', dir: 'asc' }] }); diff --git a/server/src/modules/smart-scope/smart-scope.service.ts b/server/src/modules/smart-scope/smart-scope.service.ts index 33db8bc36..6aa437604 100644 --- a/server/src/modules/smart-scope/smart-scope.service.ts +++ b/server/src/modules/smart-scope/smart-scope.service.ts @@ -4,6 +4,7 @@ import type { SQL } from 'drizzle-orm'; import type { BookQuery, BooksPage, GroupRule, JumpBucketsQuery, JumpBucketsResponse, SortSpec } from '@bookorbit/types'; import type { RequestUser } from '../../common/types/request-user'; import { normalizeIconValue } from '../../common/utils/icon-value.utils'; +import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { resolveTimeZone } from '../../common/utils/timezone.utils'; import type { SmartScope } from '../../db/schema/smart-scopes'; import { BookService } from '../book/book.service'; @@ -74,7 +75,21 @@ export class SmartScopeService { if (!smartScope.filter) { return { ...smartScope, bookCount: 0 }; } - const where = this.queryBuilder.buildWhere(smartScope.filter, { accessibleLibraryIds, userId: user.id, timeZone }); + const startedAt = Date.now(); + let where: SQL | undefined; + try { + const filter = validateGroupRule(smartScope.filter); + if (!filter) return { ...smartScope, bookCount: 0 }; + where = this.queryBuilder.buildWhere(filter, { accessibleLibraryIds, userId: user.id, timeZone }); + } catch (err) { + if (!(err instanceof BadRequestException)) throw err; + const errorClass = err.constructor.name; + const error = sanitizeLogValue(err.message); + this.logger.error( + `[smart_scope.count] [fail] scopeId=${smartScope.id} userId=${user.id} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${error}" - smart scope filter is invalid`, + ); + return { ...smartScope, bookCount: null }; + } const bookCount = await this.bookReadService.countWhere(where); return { ...smartScope, bookCount }; }),