From 06ad6a722f7af930d6cf2940dc51b71519dd1228 Mon Sep 17 00:00:00 2001 From: Kevin Gatera Date: Mon, 2 Mar 2026 22:08:15 -0500 Subject: [PATCH] add regression tests for postgres query edge cases --- test/server/models/MediaProgress.test.js | 70 +++++++++ .../scripts/migrateSqliteToPostgres.test.js | 15 ++ .../queries/libraryItemsBookFilters.test.js | 133 ++++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 test/server/models/MediaProgress.test.js create mode 100644 test/server/utils/queries/libraryItemsBookFilters.test.js diff --git a/test/server/models/MediaProgress.test.js b/test/server/models/MediaProgress.test.js new file mode 100644 index 000000000..a7c490306 --- /dev/null +++ b/test/server/models/MediaProgress.test.js @@ -0,0 +1,70 @@ +const { expect } = require('chai') +const sinon = require('sinon') + +const Logger = require('../../../server/Logger') +const MediaProgress = require('../../../server/models/MediaProgress') + +function createProgressInstance() { + const progress = Object.create(MediaProgress.prototype) + + Object.defineProperties(progress, { + id: { value: 'progress-1', writable: true, configurable: true }, + mediaItemId: { value: 'media-1', writable: true, configurable: true }, + duration: { value: 3600, writable: true, configurable: true }, + currentTime: { value: 120, writable: true, configurable: true }, + isFinished: { value: false, writable: true, configurable: true }, + hideFromContinueListening: { value: false, writable: true, configurable: true }, + extraData: { value: {}, writable: true, configurable: true } + }) + + progress.changed = sinon.stub().returns(false) + progress.set = sinon.stub().callsFake((payload) => Object.assign(progress, payload)) + progress.save = sinon.stub().resolves() + progress.reload = sinon.stub().resolves() + progress.constructor = { + update: sinon.stub().resolves(), + sequelize: { + escape: (value) => `'${value.toISOString ? value.toISOString() : value}'` + } + } + + return progress +} + +describe('MediaProgress', () => { + afterEach(() => { + sinon.restore() + }) + + describe('applyProgressUpdate', () => { + it('should update updatedAt via model update for valid lastUpdate', async () => { + const progress = createProgressInstance() + const infoStub = sinon.stub(Logger, 'info') + const lastUpdate = '2026-03-03T01:00:00.000Z' + + await progress.applyProgressUpdate({ currentTime: 130, lastUpdate }) + + expect(progress.save.calledOnce).to.equal(true) + expect(progress.constructor.update.calledOnce).to.equal(true) + expect(progress.constructor.update.firstCall.args[0].updatedAt.toISOString()).to.equal(new Date(lastUpdate).toISOString()) + expect(progress.constructor.update.firstCall.args[1]).to.deep.equal({ + where: { id: 'progress-1' }, + silent: true + }) + expect(progress.reload.calledOnce).to.equal(true) + expect(infoStub.calledWithMatch('[MediaProgress] Manually setting updatedAt')).to.equal(true) + }) + + it('should skip manual updatedAt update when lastUpdate is invalid', async () => { + const progress = createProgressInstance() + const warnStub = sinon.stub(Logger, 'warn') + + await progress.applyProgressUpdate({ currentTime: 130, lastUpdate: 'invalid-date' }) + + expect(progress.save.calledOnce).to.equal(true) + expect(progress.constructor.update.called).to.equal(false) + expect(progress.reload.called).to.equal(false) + expect(warnStub.calledWithMatch('[MediaProgress] Invalid date provided for lastUpdate')).to.equal(true) + }) + }) +}) diff --git a/test/server/scripts/migrateSqliteToPostgres.test.js b/test/server/scripts/migrateSqliteToPostgres.test.js index 9b218079e..d0bb3d592 100644 --- a/test/server/scripts/migrateSqliteToPostgres.test.js +++ b/test/server/scripts/migrateSqliteToPostgres.test.js @@ -4,6 +4,7 @@ const sqlite3 = require('sqlite3') const { normalizeJson, isIntegerCompatible, + convertValue, findOverlongVarcharValues, findIntegerTypeIssues } = require('../../../server/scripts/migrateSqliteToPostgres') @@ -128,4 +129,18 @@ describe('migrateSqliteToPostgres script helpers', () => { expect(isIntegerCompatible(10.5)).to.equal(false) expect(isIntegerCompatible('10.5')).to.equal(false) }) + + it('should coerce integer-like strings for postgres integer columns', () => { + expect(convertValue('42', { data_type: 'integer' })).to.equal(42) + expect(convertValue('-7', { data_type: 'bigint' })).to.equal(-7) + expect(convertValue('4.2', { data_type: 'integer' })).to.equal('4.2') + }) + + it('should always return valid json text for postgres json columns', () => { + const convertedObject = convertValue({ a: 1 }, { data_type: 'jsonb', udt_name: 'jsonb' }) + const convertedMalformed = convertValue('"{"x",1}"', { data_type: 'jsonb', udt_name: 'jsonb' }) + + expect(JSON.parse(convertedObject)).to.deep.equal({ a: 1 }) + expect(JSON.parse(convertedMalformed)).to.equal('"{"x",1}"') + }) }) diff --git a/test/server/utils/queries/libraryItemsBookFilters.test.js b/test/server/utils/queries/libraryItemsBookFilters.test.js new file mode 100644 index 000000000..b014c1399 --- /dev/null +++ b/test/server/utils/queries/libraryItemsBookFilters.test.js @@ -0,0 +1,133 @@ +const { expect } = require('chai') +const sinon = require('sinon') + +const Database = require('../../../../server/Database') +const Logger = require('../../../../server/Logger') +const libraryItemsBookFilters = require('../../../../server/utils/queries/libraryItemsBookFilters') + +function createSequelizeStub(dialect, models) { + return { + getDialect: () => dialect, + escape: (value) => `'${String(value).replace(/'/g, "''")}'`, + random: () => ({ fn: 'random' }), + models + } +} + +function createModelStubs() { + return { + book: { + findAndCountAll: sinon.stub(), + findAll: sinon.stub(), + count: sinon.stub() + }, + libraryItem: {}, + feed: {}, + bookSeries: {}, + series: { + findAll: sinon.stub() + }, + bookAuthor: {}, + author: {}, + mediaProgress: {}, + mediaItemShare: {} + } +} + +describe('libraryItemsBookFilters postgres query safety', () => { + let originalSequelize + let originalServerSettings + let modelStubs + + beforeEach(() => { + originalSequelize = Database.sequelize + originalServerSettings = global.ServerSettings + modelStubs = createModelStubs() + global.ServerSettings = { + sortingIgnorePrefix: false + } + }) + + afterEach(() => { + Database.sequelize = originalSequelize + global.ServerSettings = originalServerSettings + sinon.restore() + }) + + it('should avoid generating IN () when collapse-series has no include ids', async () => { + Database.sequelize = createSequelizeStub('postgres', modelStubs) + modelStubs.book.findAndCountAll.resolves({ rows: [], count: 0 }) + + sinon.stub(libraryItemsBookFilters, 'getCollapseSeriesBooksToExclude').resolves({ + booksToExclude: [], + bookSeriesToInclude: [] + }) + const warnStub = sinon.stub(Logger, 'warn') + + await libraryItemsBookFilters.getFilteredLibraryItems('library-1', { canAccessExplicitContent: true }, 'authors', 'author-1', 'media.metadata.publishedYear', true, true, [], 20, 0) + + const findOptions = modelStubs.book.findAndCountAll.firstCall.args[0] + const displayTitleExpression = findOptions.attributes.include[0][0].val + + expect(displayTitleExpression).to.include('COALESCE(NULL, libraryItem.title)') + expect(displayTitleExpression).to.not.include('IN ()') + expect(warnStub.calledOnce).to.equal(true) + }) + + it('should escape collapse-series ids safely for postgres subquery', async () => { + Database.sequelize = createSequelizeStub('postgres', modelStubs) + modelStubs.book.findAndCountAll.resolves({ rows: [], count: 0 }) + + sinon.stub(libraryItemsBookFilters, 'getCollapseSeriesBooksToExclude').resolves({ + booksToExclude: [], + bookSeriesToInclude: [{ id: "series-id-'quoted'", numBooks: 2, libraryItemIds: [] }] + }) + global.ServerSettings.sortingIgnorePrefix = true + + await libraryItemsBookFilters.getFilteredLibraryItems('library-1', { canAccessExplicitContent: true }, 'authors', 'author-1', 'media.metadata.title', false, true, [], 20, 0) + + const findOptions = modelStubs.book.findAndCountAll.firstCall.args[0] + const displayTitleExpression = findOptions.attributes.include[0][0].val + + expect(displayTitleExpression).to.include("bs.id IN ('series-id-''quoted''')") + expect(displayTitleExpression).to.include('libraryItem.titleIgnorePrefix') + }) + + it('should use postgres-safe join alias for sequence sorting', () => { + Database.sequelize = createSequelizeStub('postgres', modelStubs) + + const order = libraryItemsBookFilters.getOrder('sequence', false, false) + const expression = order[0][0].val + + expect(expression).to.include('"series->bookSeries"."sequence"') + expect(expression).to.include('CASE WHEN BTRIM') + }) + + it('should log generated query when findAndCountAll fails', async () => { + Database.sequelize = createSequelizeStub('postgres', modelStubs) + modelStubs.book.findAndCountAll.rejects(new Error('boom')) + const errorStub = sinon.stub(Logger, 'error') + + try { + await libraryItemsBookFilters.getFilteredLibraryItems('library-1', { canAccessExplicitContent: true }, 'authors', 'author-1', 'media.metadata.publishedYear', true, false, [], 20, 0) + expect.fail('Expected getFilteredLibraryItems to throw') + } catch (error) { + expect(error.message).to.equal('boom') + } + + expect(errorStub.calledWithMatch('[LibraryItemsBookFilters] findAndCountAll failed: boom')).to.equal(true) + expect(errorStub.calledWithMatch('[LibraryItemsBookFilters] findAndCountAll query:')).to.equal(true) + }) + + it('should use postgres-safe books->bookSeries alias in collapse-series selector query', async () => { + Database.sequelize = createSequelizeStub('postgres', modelStubs) + modelStubs.series.findAll.resolves([]) + + await libraryItemsBookFilters.getCollapseSeriesBooksToExclude({ where: {}, include: [] }, null) + + const findAllOptions = modelStubs.series.findAll.firstCall.args[0] + const orderExpression = findAllOptions.order[0].val + + expect(orderExpression).to.include('"books->bookSeries"."sequence"') + }) +})