mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
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
This commit is contained in:
@@ -10,7 +10,7 @@ export interface SmartScope {
|
||||
isPublic: boolean;
|
||||
syncToKobo: boolean;
|
||||
displayOrder: number;
|
||||
bookCount?: number;
|
||||
bookCount?: number | null;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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}`);
|
||||
}
|
||||
|
||||
@@ -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)', () => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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<string, unknown> = { type: 'rule', field, operator };
|
||||
if (value !== undefined) rule.value = value;
|
||||
if (valueTo !== undefined) rule.valueTo = valueTo;
|
||||
|
||||
@@ -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<RuleField>(['addedAt', 'startedAt', 'finishedAt', 'publishedDate']);
|
||||
const DATE_VALUE_OPERATORS = new Set<RuleOperator>(['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<Rule> = z
|
||||
.object({
|
||||
type: z.literal('rule'),
|
||||
@@ -29,6 +54,14 @@ const ruleSchema: z.ZodType<Rule> = 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<Rule>;
|
||||
|
||||
const groupRuleSchema = (maxDepth: number): z.ZodType<GroupRule> =>
|
||||
|
||||
@@ -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' }] });
|
||||
|
||||
@@ -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 };
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user