mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(books): read back user-supplied dates Postgres stored
Decode added dates and manual reading dates with an explicit parser instead of new Date(), which reads a zero-padded year below 100 as a two-digit year: years 0013 to 0031 became an Invalid Date and made every listing holding the book answer 500, and years 0001 to 0099 shifted silently into 19xx or 20xx. The column type is unchanged on the wire, so no migration is needed. Closes #1143
This commit is contained in:
@@ -0,0 +1,77 @@
|
||||
import { parsePgTimestamptz } from './pg-timestamp.utils';
|
||||
|
||||
function iso(value: string): string {
|
||||
return parsePgTimestamptz(value).toISOString();
|
||||
}
|
||||
|
||||
describe('parsePgTimestamptz', () => {
|
||||
it('parses the text form the driver returns for a modern timestamp', () => {
|
||||
expect(iso('2026-08-25 07:42:00+00')).toBe('2026-08-25T07:42:00.000Z');
|
||||
});
|
||||
|
||||
it('keeps millisecond precision and truncates the microseconds Date cannot hold', () => {
|
||||
expect(iso('2026-08-25 07:42:00.643+00')).toBe('2026-08-25T07:42:00.643Z');
|
||||
expect(iso('2026-08-25 07:42:00.643127+00')).toBe('2026-08-25T07:42:00.643Z');
|
||||
expect(iso('2026-08-25 07:42:00.999999+00')).toBe('2026-08-25T07:42:00.999Z');
|
||||
});
|
||||
|
||||
it('applies the offset the server rendered the value in', () => {
|
||||
expect(iso('2026-08-25 07:42:00-05')).toBe('2026-08-25T12:42:00.000Z');
|
||||
expect(iso('2026-08-25 07:42:00+05:30')).toBe('2026-08-25T02:12:00.000Z');
|
||||
expect(iso('2026-08-25 07:42:00Z')).toBe('2026-08-25T07:42:00.000Z');
|
||||
});
|
||||
|
||||
it('applies a local-mean-time offset carrying seconds', () => {
|
||||
// Named zones render pre-1900 instants at their LMT offset, which has a seconds part.
|
||||
expect(iso('1850-06-15 00:53:28+00:53:28')).toBe('1850-06-15T00:00:00.000Z');
|
||||
});
|
||||
|
||||
it('reads a year below 100 as itself rather than as a two-digit year', () => {
|
||||
// The defect: `new Date()` reads these as 1950 and 2001.
|
||||
expect(iso('0050-06-15 00:00:00+00')).toBe('0050-06-15T00:00:00.000Z');
|
||||
expect(iso('0001-01-01 00:00:00+00')).toBe('0001-01-01T00:00:00.000Z');
|
||||
expect(iso('0099-12-31 00:00:00+00')).toBe('0099-12-31T00:00:00.000Z');
|
||||
});
|
||||
|
||||
it('parses the year band that made `new Date()` fail outright', () => {
|
||||
// Issue #1143: years 0013 to 0031 produced an Invalid Date, so serializing the row threw.
|
||||
expect(iso('0013-01-01 00:00:00+00')).toBe('0013-01-01T00:00:00.000Z');
|
||||
expect(iso('0025-08-25 00:00:00+00')).toBe('0025-08-25T00:00:00.000Z');
|
||||
expect(iso('0031-12-31 00:00:00+00')).toBe('0031-12-31T00:00:00.000Z');
|
||||
});
|
||||
|
||||
it('round-trips the year of every value Postgres can render in four digits', () => {
|
||||
for (let year = 1; year <= 9999; year++) {
|
||||
const rendered = `${String(year).padStart(4, '0')}-06-15 00:00:00+00`;
|
||||
const parsed = parsePgTimestamptz(rendered);
|
||||
expect(Number.isNaN(parsed.getTime())).toBe(false);
|
||||
expect(parsed.getUTCFullYear()).toBe(year);
|
||||
expect(parsed.getUTCMonth()).toBe(5);
|
||||
expect(parsed.getUTCDate()).toBe(15);
|
||||
}
|
||||
});
|
||||
|
||||
it('parses years past four digits', () => {
|
||||
expect(parsePgTimestamptz('10000-01-01 00:00:00+00').getUTCFullYear()).toBe(10000);
|
||||
});
|
||||
|
||||
it('maps a BC year onto its astronomical year', () => {
|
||||
// Postgres numbers BC years from 1, so 1 BC is astronomical year 0.
|
||||
expect(parsePgTimestamptz('0001-01-01 00:00:00+00 BC').getUTCFullYear()).toBe(0);
|
||||
expect(parsePgTimestamptz('4713-01-01 00:00:00+00 BC').getUTCFullYear()).toBe(-4712);
|
||||
});
|
||||
|
||||
it('returns an invalid date for a value outside the range Date can hold', () => {
|
||||
expect(Number.isNaN(parsePgTimestamptz('294276-12-31 00:00:00+00').getTime())).toBe(true);
|
||||
});
|
||||
|
||||
it('passes a Date through untouched', () => {
|
||||
const date = new Date('2026-08-25T07:42:00.000Z');
|
||||
expect(parsePgTimestamptz(date)).toBe(date);
|
||||
});
|
||||
|
||||
it('falls back to Date parsing for anything it does not recognize', () => {
|
||||
expect(iso('2026-08-25T07:42:00.000Z')).toBe('2026-08-25T07:42:00.000Z');
|
||||
expect(Number.isNaN(parsePgTimestamptz('not a timestamp').getTime())).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* PostgreSQL emits `timestamp with time zone` as text, and the driver hands that text
|
||||
* straight through. `new Date()` parses the space-separated form with V8's legacy parser,
|
||||
* which reads a zero-padded year below 100 as a two-digit year: `0050-06-15` comes back as
|
||||
* 1950, and years 0013 to 0031 fail to parse entirely. Any column that can hold a
|
||||
* user-supplied date must be decoded here instead.
|
||||
*/
|
||||
const PG_TIMESTAMPTZ_RE =
|
||||
/^(?<year>\d+)-(?<month>\d{2})-(?<day>\d{2})[ T](?<hour>\d{2}):(?<minute>\d{2}):(?<second>\d{2})(?:\.(?<fraction>\d+))?(?:Z|(?<sign>[+-])(?<offsetHours>\d{2})(?::?(?<offsetMinutes>\d{2}))?(?::?(?<offsetSeconds>\d{2}))?)?(?<era> BC)?$/;
|
||||
|
||||
export function parsePgTimestamptz(value: string | Date): Date {
|
||||
if (value instanceof Date) return value;
|
||||
|
||||
const groups = PG_TIMESTAMPTZ_RE.exec(value)?.groups;
|
||||
if (!groups) return new Date(value);
|
||||
|
||||
// Postgres numbers BC years from 1, so 1 BC is astronomical year 0.
|
||||
const calendarYear = Number(groups.year);
|
||||
const year = groups.era ? 1 - calendarYear : calendarYear;
|
||||
// Postgres keeps microseconds; Date only holds milliseconds, so drop the excess rather
|
||||
// than rounding a value like .9996 up into the next second.
|
||||
const milliseconds = groups.fraction ? Math.trunc(Number(`0.${groups.fraction}`) * 1000) : 0;
|
||||
|
||||
// Assigning the parts separately avoids Date.UTC(), which maps years 0 to 99 into 1900.
|
||||
const date = new Date(0);
|
||||
date.setUTCHours(Number(groups.hour), Number(groups.minute), Number(groups.second), milliseconds);
|
||||
date.setUTCFullYear(year, Number(groups.month) - 1, Number(groups.day));
|
||||
|
||||
if (!groups.sign) return date;
|
||||
const offsetSeconds = Number(groups.offsetHours) * 3600 + Number(groups.offsetMinutes ?? 0) * 60 + Number(groups.offsetSeconds ?? 0);
|
||||
const offsetMs = (groups.sign === '-' ? -1 : 1) * offsetSeconds * 1000;
|
||||
return offsetMs === 0 ? date : new Date(date.getTime() - offsetMs);
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
varchar,
|
||||
} from 'drizzle-orm/pg-core';
|
||||
|
||||
import { timestamptz } from './columns';
|
||||
import { libraryFolders, libraries } from './libraries';
|
||||
|
||||
export const books = pgTable(
|
||||
@@ -31,7 +32,9 @@ export const books = pgTable(
|
||||
primaryAuthorSortName: varchar('primary_author_sort_name', { length: 500 }),
|
||||
folderPath: varchar('folder_path', { length: 4096 }).notNull(),
|
||||
status: varchar('status', { length: 20 }).notNull().default('present'),
|
||||
addedAt: timestamp('added_at', { withTimezone: true }).defaultNow().notNull(),
|
||||
addedAt: timestamptz('added_at')
|
||||
.default(sql`now()`)
|
||||
.notNull(),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true })
|
||||
.defaultNow()
|
||||
.notNull()
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
import { customType } from 'drizzle-orm/pg-core';
|
||||
|
||||
import { parsePgTimestamptz } from '../../common/utils/pg-timestamp.utils';
|
||||
|
||||
/**
|
||||
* `timestamp with time zone` for columns that can hold a user-supplied date.
|
||||
*
|
||||
* Identical to `timestamp(name, { withTimezone: true })` on the wire; it only replaces the
|
||||
* decoder, because drizzle's own one misreads years below 100 and throws on 0013 to 0031.
|
||||
* See `parsePgTimestamptz`. Columns the application stamps itself (created/updated marks,
|
||||
* job timestamps) are always near the present, so plain `timestamp()` remains fine there.
|
||||
*/
|
||||
export const timestamptz = customType<{ data: Date; driverData: string }>({
|
||||
dataType: () => 'timestamp with time zone',
|
||||
toDriver: (value) => value.toISOString(),
|
||||
fromDriver: (value) => parsePgTimestamptz(value),
|
||||
});
|
||||
@@ -3,6 +3,7 @@ import { check, date, index, integer, jsonb, pgTable, primaryKey, real, serial,
|
||||
import type { ReadStatus, ReadStatusSource, ReadingAttemptOrigin, ReadingAttemptOutcome, ReadingSessionSource } from '@bookorbit/types';
|
||||
|
||||
import { bookFiles, books } from './books';
|
||||
import { timestamptz } from './columns';
|
||||
import { libraries } from './libraries';
|
||||
import { users } from './auth';
|
||||
|
||||
@@ -19,8 +20,8 @@ export const userBookStatus = pgTable(
|
||||
status: varchar('status', { length: 20 }).$type<ReadStatus>().notNull().default('unread'),
|
||||
// 'auto' (derived from progress) | 'manual' (user-set/imported; protected from progress updates except want_to_read)
|
||||
source: varchar('source', { length: 10 }).$type<ReadStatusSource>().notNull().default('auto'),
|
||||
startedAt: timestamp('started_at', { withTimezone: true }),
|
||||
finishedAt: timestamp('finished_at', { withTimezone: true }),
|
||||
startedAt: timestamptz('started_at'),
|
||||
finishedAt: timestamptz('finished_at'),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true })
|
||||
.notNull()
|
||||
.defaultNow()
|
||||
|
||||
@@ -12,7 +12,7 @@ import { emailPreferences } from './email-preferences';
|
||||
import { smartScopes } from './smart-scopes';
|
||||
import { koboSyncSettings, koboLibrarySnapshots, koboReadingStates } from './kobo';
|
||||
import { emailRecipients, emailRecipientGroups } from './email-recipients';
|
||||
import { readerDefaultPreferences, readerPreferences, readingProgress, annotations, userReadingDailyStats } from './reader';
|
||||
import { readerDefaultPreferences, readerPreferences, readingProgress, annotations, userBookStatus, userReadingDailyStats } from './reader';
|
||||
import { migrationSources, migrationProfiles, migrationPlanArtifacts, migrationRuns, migrationRunMetrics } from './migration';
|
||||
import { getTableConfig } from 'drizzle-orm/pg-core';
|
||||
import { FILE_ROLES } from '../../modules/scanner/lib/classify';
|
||||
@@ -63,6 +63,33 @@ describe('Database Schema Logic', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('user-supplied date columns', () => {
|
||||
// Issue #1143: these three hold dates the user types, so they can legitimately land in
|
||||
// the year range drizzle's own timestamp decoder gets wrong. Reverting any of them to
|
||||
// `timestamp(name, { withTimezone: true })` brings back a 500 on every book listing.
|
||||
const columns = [
|
||||
['books.addedAt', books.addedAt],
|
||||
['userBookStatus.startedAt', userBookStatus.startedAt],
|
||||
['userBookStatus.finishedAt', userBookStatus.finishedAt],
|
||||
] as const;
|
||||
|
||||
it.each(columns)('%s is still a timestamp with time zone on the wire', (_name, column) => {
|
||||
expect(column.getSQLType()).toBe('timestamp with time zone');
|
||||
});
|
||||
|
||||
it.each(columns)('%s decodes a year below 100 as itself', (_name, column) => {
|
||||
expect(column.mapFromDriverValue('0050-06-15 00:00:00+00').toISOString()).toBe('0050-06-15T00:00:00.000Z');
|
||||
});
|
||||
|
||||
it.each(columns)('%s decodes the year band that used to throw', (_name, column) => {
|
||||
expect(column.mapFromDriverValue('0025-08-25 00:00:00+00').toISOString()).toBe('0025-08-25T00:00:00.000Z');
|
||||
});
|
||||
|
||||
it.each(columns)('%s still encodes a Date as an ISO string', (_name, column) => {
|
||||
expect(column.mapToDriverValue(new Date('2026-08-25T07:42:00.000Z'))).toBe('2026-08-25T07:42:00.000Z');
|
||||
});
|
||||
});
|
||||
|
||||
describe('onUpdateFn presence', () => {
|
||||
const hasOnUpdateFn = (column: any) => {
|
||||
return typeof column.onUpdateFn === 'function';
|
||||
|
||||
@@ -11,6 +11,7 @@ import type {
|
||||
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
|
||||
import { parsePgTimestamptz } from '../../common/utils/pg-timestamp.utils';
|
||||
import { LibraryService } from '../library/library.service';
|
||||
import { canonicalizeIsbn, mediaFamilyForFormat } from './book-duplicate-normalize';
|
||||
import { BookDuplicatesRepository } from './book-duplicates.repository';
|
||||
@@ -97,8 +98,8 @@ export class BookDuplicatesService implements OnApplicationBootstrap {
|
||||
: null,
|
||||
readingProgress: row.reading_progress === null ? null : Number(row.reading_progress),
|
||||
collections: row.collections,
|
||||
addedAt: new Date(row.added_at).toISOString(),
|
||||
updatedAt: row.updated_at ? new Date(row.updated_at).toISOString() : null,
|
||||
addedAt: parsePgTimestamptz(row.added_at).toISOString(),
|
||||
updatedAt: row.updated_at ? parsePgTimestamptz(row.updated_at).toISOString() : null,
|
||||
hasCover: row.has_cover,
|
||||
};
|
||||
const current = previewsByGroup.get(row.group_id);
|
||||
|
||||
@@ -18,6 +18,7 @@ import { isAudioFormat, isComicFormat, normalizeCoverAspectRatio } from '@bookor
|
||||
import { buildContentFilterClauses } from '../../common/utils/content-filter-sql.utils';
|
||||
import { accentInsensitiveIlike } from '../../common/utils/accent-insensitive-search.utils';
|
||||
import { advanceIsoTimestamp } from '../../common/utils/iso-timestamp.utils';
|
||||
import { parsePgTimestamptz } from '../../common/utils/pg-timestamp.utils';
|
||||
import { seriesIndexSortKeySql } from '../../common/utils/series-index-sql.utils';
|
||||
import { SeriesIdentityService } from '../../common/services/series-identity.service';
|
||||
import { SeriesMembershipService } from '../../common/services/series-membership.service';
|
||||
@@ -1055,8 +1056,8 @@ export class BookRepository {
|
||||
coverAspectRatio: r.cover_aspect_ratio,
|
||||
primaryFileId: r.primary_file_id,
|
||||
folderPath: r.folder_path,
|
||||
addedAt: new Date(r.added_at),
|
||||
updatedAt: new Date(r.updated_at),
|
||||
addedAt: parsePgTimestamptz(r.added_at),
|
||||
updatedAt: parsePgTimestamptz(r.updated_at),
|
||||
title: r.title,
|
||||
seriesId: r.series_id,
|
||||
seriesName: r.series_name,
|
||||
@@ -1078,7 +1079,7 @@ export class BookRepository {
|
||||
readCount: r.read_count !== null ? Number(r.read_count) : null,
|
||||
coverBookIds: r.cover_book_ids,
|
||||
coverUpdatedAtByBookId: parseDateByBookId(r.cover_updated_at_by_book_id),
|
||||
seriesLatestAddedAt: r.sort_added_at ? new Date(r.sort_added_at) : null,
|
||||
seriesLatestAddedAt: r.sort_added_at ? parsePgTimestamptz(r.sort_added_at) : null,
|
||||
firstVolumeBookId: r.first_volume_book_id ?? null,
|
||||
latestVolumeBookId: r.latest_volume_book_id ?? null,
|
||||
firstUnreadBookId: r.first_unread_book_id ?? null,
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { books } from '../../../db/schema/books';
|
||||
|
||||
import { assembleBookCards, assembleCollapsedBookCards, collapseBookCards } from './assemble-book-cards';
|
||||
|
||||
function makeBookRow(id: number, overrides?: Partial<Parameters<typeof assembleBookCards>[0][number]>) {
|
||||
@@ -69,6 +71,19 @@ describe('assembleBookCards', () => {
|
||||
expect(card.files).toEqual([{ id: 10, format: 'epub', role: 'primary', sizeBytes: 4096 }]);
|
||||
});
|
||||
|
||||
// Issue #1143: a user-typed added date such as year 0025 decoded to an Invalid Date, and
|
||||
// serializing the card threw RangeError, so every listing that contained the book 500'd.
|
||||
it.each(['0025-08-25 00:00:00+00', '0013-01-01 00:00:00+00', '0050-06-15 00:00:00+00', '0001-01-01 00:00:00+00'])(
|
||||
'serializes a book whose added date came back from Postgres as %s',
|
||||
(stored) => {
|
||||
const rows = [makeBookRow(1, { addedAt: books.addedAt.mapFromDriverValue(stored) })];
|
||||
|
||||
const [card] = assembleBookCards(rows, [], [], [], []);
|
||||
|
||||
expect(card.addedAt).toBe(`${stored.slice(0, 10)}T00:00:00.000Z`);
|
||||
},
|
||||
);
|
||||
|
||||
it('passes through Hardcover book and edition identifiers', () => {
|
||||
const rows = [makeBookRow(1, { hardcoverId: 'new-orleans-rush', hardcoverEditionId: '8941973' })];
|
||||
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
import { randomUUID } from 'crypto';
|
||||
|
||||
import { eq } from 'drizzle-orm';
|
||||
import { rm } from 'fs/promises';
|
||||
import { join } from 'path';
|
||||
|
||||
import * as unzipper from 'unzipper';
|
||||
import { Permission } from '@bookorbit/types';
|
||||
|
||||
import { books } from '../src/db/schema/books';
|
||||
|
||||
import {
|
||||
authHeader,
|
||||
closeMetadataWriteE2EContext,
|
||||
@@ -710,4 +714,43 @@ describe('Book API contract (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () => {
|
||||
expectError(missingThumbnail, 404, `No thumbnail for book ${visibleEpub.bookId}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('user-supplied added dates', () => {
|
||||
// Issue #1143: an added date the user typed, such as year 0025, came back from Postgres
|
||||
// as an Invalid Date, so serializing the card threw and every listing holding the book
|
||||
// answered 500. These are the year bands the driver's own decoder gets wrong.
|
||||
it.each(['0025-08-25', '0013-01-01', '0031-12-31', '0050-06-15', '0001-01-01'])(
|
||||
'round-trips an added date of %s through storage and back into a listing',
|
||||
async (addedAt) => {
|
||||
const bookId = hiddenEpub.bookId;
|
||||
const [original] = await ctx.db.select({ addedAt: books.addedAt }).from(books).where(eq(books.id, bookId));
|
||||
const expected = `${addedAt}T00:00:00.000Z`;
|
||||
|
||||
try {
|
||||
const update = await ctx.app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/v1/books/${bookId}/added-at`,
|
||||
headers: authHeader(ctx.adminToken),
|
||||
payload: { addedAt },
|
||||
});
|
||||
|
||||
expect(update.statusCode).toBe(200);
|
||||
expect((update.json() as { addedAt: string | null }).addedAt).toBe(expected);
|
||||
|
||||
const listing = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/books/query',
|
||||
headers: authHeader(ctx.adminToken),
|
||||
payload: { pagination: { page: 0, size: 200 } },
|
||||
});
|
||||
|
||||
expect(listing.statusCode).toBe(201);
|
||||
const items = (listing.json() as { items: { id: number; addedAt: string }[] }).items;
|
||||
expect(items.find((item) => item.id === bookId)?.addedAt).toBe(expected);
|
||||
} finally {
|
||||
await ctx.db.update(books).set({ addedAt: original!.addedAt }).where(eq(books.id, bookId));
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user