fix(server): single library exclusion matcher (#32081)

(cherry picked from commit c3dee6f98b7a231adc3d0716a287c4b47bc69d3e)
This commit is contained in:
Zack Pollard
2026-10-31 07:23:13 +03:00
committed by Codex
parent abfb6c0a0e
commit f5f154f2d8
8 changed files with 262 additions and 95 deletions
+14 -10
View File
@@ -511,18 +511,22 @@ limit
$4
-- AssetRepository.detectOfflineExternalAssets
update "asset"
set
"isOffline" = $1,
"deletedAt" = $2
begin
declare offline_check no scroll cursor for (
select
"asset"."id",
"asset"."originalPath"
from
"asset"
where
"isOffline" = $3
and "isExternal" = $4
and "libraryId" = $5::uuid
and (
not "originalPath" like $6
or "originalPath" ~* $7
"asset"."libraryId" = $1::uuid
and "asset"."isOffline" = $2
and "asset"."isExternal" = $3
)
fetch 10000
from
offline_check
commit
-- AssetRepository.filterNewExternalAssetPaths
select
+37 -24
View File
@@ -7,13 +7,13 @@ import {
type SelectQueryBuilder,
type Selectable,
type ShallowDehydrateObject,
UpdateResult,
type Updateable,
sql,
} from 'kysely';
import { jsonArrayFrom } from 'kysely/helpers/postgres';
import { isEmpty, isUndefined, omitBy } from 'lodash-es';
import { InjectKysely } from 'nestjs-kysely';
import { JOBS_LIBRARY_PAGINATION_SIZE } from 'src/constants.js';
import { LockableProperty, Stack } from 'src/database.js';
import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators.js';
import { type AuthDto, isMetadataHidden } from 'src/dtos/auth.dto.js';
@@ -54,7 +54,6 @@ import {
withTagId,
withTags,
} from 'src/utils/database.js';
import { globToPostgresRegex } from 'src/utils/misc.js';
export type AssetStats = Record<AssetType, number>;
@@ -1092,31 +1091,45 @@ export class AssetRepository {
.execute();
}
@GenerateSql({ params: [DummyValue.UUID, [DummyValue.STRING], [DummyValue.STRING]] })
async detectOfflineExternalAssets(
libraryId: string,
importPaths: string[],
exclusionPatterns: string[],
): Promise<UpdateResult> {
const paths = importPaths.map((importPath) => `${importPath}%`);
const exclusions = exclusionPatterns.map((pattern) => globToPostgresRegex(pattern));
@GenerateSql({ params: [DummyValue.UUID, () => true] })
detectOfflineExternalAssets(libraryId: string, isInLibrary: (originalPath: string) => boolean): Promise<number> {
// read and write on one connection, so concurrent library scans can't deadlock on the pool
return this.db.transaction().execute(async (trx) => {
const query = trx
.selectFrom('asset')
.select(['asset.id', 'asset.originalPath'])
.where('asset.libraryId', '=', asUuid(libraryId))
.where('asset.isOffline', '=', false)
.where('asset.isExternal', '=', true);
await sql`declare offline_check no scroll cursor for ${query}`.execute(trx);
return this.db
const fetchBatch = async () => {
const { rows } = await sql<{ id: string; originalPath: string }>`
fetch ${sql.lit(JOBS_LIBRARY_PAGINATION_SIZE)} from offline_check
`.execute(trx);
return rows;
};
const deletedAt = new Date();
let count = 0;
for (let rows = await fetchBatch(); rows.length > 0; rows = await fetchBatch()) {
const ids = rows.filter(({ originalPath }) => !isInLibrary(originalPath)).map(({ id }) => id);
if (ids.length === 0) {
continue;
}
// a concurrent check of the same library may have offlined these rows after the cursor was declared
const { numUpdatedRows } = await trx
.updateTable('asset')
.set({
isOffline: true,
deletedAt: new Date(),
})
.set({ isOffline: true, deletedAt })
.where('id', '=', anyUuid(ids))
.where('isOffline', '=', false)
.where('isExternal', '=', true)
.where('libraryId', '=', asUuid(libraryId))
.where((eb) =>
eb.or([
eb.not(eb.or(paths.map((path) => eb('originalPath', 'like', path)))),
eb.or(exclusions.map((pattern) => eb('originalPath', '~*', pattern))),
]),
)
.executeTakeFirstOrThrow();
.executeTakeFirst();
count += Number(numUpdatedRows);
}
return count;
});
}
@GenerateSql({ params: [DummyValue.UUID, [DummyValue.STRING]] })
+25 -20
View File
@@ -273,31 +273,42 @@ describe(LibraryService.name, () => {
});
describe('handleQueueSyncAssets', () => {
it('should call the offline check', async () => {
const library = factory.library();
it('should offline assets outside the import paths or covered by an exclusion pattern', async () => {
const library = factory.library({ importPaths: ['/import/'], exclusionPatterns: ['**/raw/**'] });
const asset = AssetFactory.create({ libraryId: library.id, originalPath: '/import/photo.jpg' });
mocks.library.get.mockResolvedValue(library);
mocks.asset.getLibraryAssetCount.mockResolvedValue(3);
mocks.asset.detectOfflineExternalAssets.mockResolvedValue(2);
mocks.library.streamAssetIds.mockReturnValue(makeStream([asset]));
await expect(sut.handleQueueSyncAssets({ id: library.id })).resolves.toBe(JobStatus.Success);
expect(mocks.asset.detectOfflineExternalAssets).toHaveBeenCalledWith(library.id, expect.any(Function));
const [, isInLibrary] = mocks.asset.detectOfflineExternalAssets.mock.calls[0];
expect(isInLibrary('/import/photo.jpg')).toBe(true);
expect(isInLibrary('/elsewhere/photo.jpg')).toBe(false);
expect(isInLibrary('/import/RAW/photo.jpg')).toBe(false);
});
it('should not queue asset sync when every asset was offlined', async () => {
const library = factory.library({ importPaths: ['/import/'] });
mocks.library.get.mockResolvedValue(library);
mocks.storage.walk.mockImplementation(async function* generator() {});
mocks.asset.getLibraryAssetCount.mockResolvedValue(1);
mocks.asset.detectOfflineExternalAssets.mockResolvedValue({ numUpdatedRows: 1n });
mocks.asset.detectOfflineExternalAssets.mockResolvedValue(1);
const response = await sut.handleQueueSyncAssets({ id: library.id });
await expect(sut.handleQueueSyncAssets({ id: library.id })).resolves.toBe(JobStatus.Success);
expect(response).toBe(JobStatus.Success);
expect(mocks.asset.detectOfflineExternalAssets).toHaveBeenCalledWith(
library.id,
library.importPaths,
library.exclusionPatterns,
);
expect(mocks.library.streamAssetIds).not.toHaveBeenCalled();
expect(mocks.job.queue).not.toHaveBeenCalled();
});
it('should skip an empty library', async () => {
const library = factory.library();
mocks.library.get.mockResolvedValue(library);
mocks.storage.walk.mockImplementation(async function* generator() {});
mocks.asset.getLibraryAssetCount.mockResolvedValue(0);
mocks.asset.detectOfflineExternalAssets.mockResolvedValue({ numUpdatedRows: 1n });
const response = await sut.handleQueueSyncAssets({ id: library.id });
@@ -310,10 +321,9 @@ describe(LibraryService.name, () => {
const asset = AssetFactory.create({ libraryId: library.id, isExternal: true });
mocks.library.get.mockResolvedValue(library);
mocks.storage.walk.mockImplementation(async function* generator() {});
mocks.asset.detectOfflineExternalAssets.mockResolvedValue(0);
mocks.library.streamAssetIds.mockReturnValue(makeStream([asset]));
mocks.asset.getLibraryAssetCount.mockResolvedValue(1);
mocks.asset.detectOfflineExternalAssets.mockResolvedValue({ numUpdatedRows: 0n });
const response = await sut.handleQueueSyncAssets({ id: library.id });
@@ -330,11 +340,6 @@ describe(LibraryService.name, () => {
});
expect(response).toBe(JobStatus.Success);
expect(mocks.asset.detectOfflineExternalAssets).toHaveBeenCalledWith(
library.id,
library.importPaths,
library.exclusionPatterns,
);
});
it("should fail if library can't be found", async () => {
+6 -22
View File
@@ -34,7 +34,7 @@ import { AssetSyncResult } from 'src/repositories/library.repository.js';
import { AssetTable } from 'src/schema/tables/asset.table.js';
import { BaseService } from 'src/services/base.service.js';
import { mimeTypes } from 'src/utils/mime-types.js';
import { batched, findOrFail, handlePromiseError } from 'src/utils/misc.js';
import { batched, createLibraryMatcher, findOrFail, handlePromiseError } from 'src/utils/misc.js';
@Injectable()
export class LibraryService extends BaseService {
@@ -483,6 +483,7 @@ export class LibraryService extends BaseService {
@OnJob({ name: JobName.LibrarySyncAssets, queue: QueueName.Library })
async handleSyncAssets(job: JobOf<JobName.LibrarySyncAssets>): Promise<JobStatus> {
const assets = await this.assetJobRepository.getForSyncAssets(job.assetIds);
const isInLibrary = createLibraryMatcher(job);
const assetIdsToOffline: string[] = [];
const trashedAssetIdsToOffline: string[] = [];
@@ -517,20 +518,13 @@ export class LibraryService extends BaseService {
break;
}
case AssetSyncResult.CHECK_OFFLINE: {
const isInImportPath = job.importPaths.some((path) => asset.originalPath.startsWith(path));
if (!isInImportPath) {
if (!isInLibrary(asset.originalPath)) {
this.logger.verbose(
`Offline asset ${asset.originalPath} is still not in any import path, keeping offline in library ${job.libraryId}`,
`Offline asset ${asset.originalPath} is still not in any import path or covered by an exclusion pattern, keeping offline in library ${job.libraryId}`,
);
break;
}
const isExcluded = job.exclusionPatterns.some((pattern) =>
picomatch.isMatch(asset.originalPath, pattern, { nocase: true }),
);
if (!isExcluded) {
this.logger.debug(`Offline asset ${asset.originalPath} is now online in library ${job.libraryId}`);
if (asset.status === AssetStatus.Trashed) {
trashedAssetIdsToOnline.push(asset.id);
@@ -539,13 +533,6 @@ export class LibraryService extends BaseService {
}
break;
}
this.logger.verbose(
`Offline asset ${asset.originalPath} is in an import path but still covered by exclusion pattern, keeping offline in library ${job.libraryId}`,
);
break;
}
}
}
@@ -721,14 +708,11 @@ export class LibraryService extends BaseService {
`Checking ${assetCount} asset(s) against import paths and exclusion patterns in library ${library.id}...`,
);
const offlineResult = await this.assetRepository.detectOfflineExternalAssets(
const affectedAssetCount = await this.assetRepository.detectOfflineExternalAssets(
library.id,
library.importPaths,
library.exclusionPatterns,
createLibraryMatcher(library),
);
const affectedAssetCount = Number(offlineResult.numUpdatedRows);
this.logger.log(
`${affectedAssetCount} asset(s) out of ${assetCount} were offlined due to import paths and/or exclusion pattern(s) in library ${library.id}`,
);
+26 -3
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest';
import { getKeysDeep, globToPostgresRegex, unsetDeep } from 'src/utils/misc.js';
import { createLibraryMatcher, getKeysDeep, unsetDeep } from 'src/utils/misc.js';
describe('getKeysDeep', () => {
it('should handle an empty object', () => {
@@ -52,9 +52,24 @@ describe('unsetDeep', () => {
});
});
const matches = (glob: string, value: string) => new RegExp(globToPostgresRegex(glob)).test(value);
const matches = (glob: string, value: string) =>
!createLibraryMatcher({ importPaths: ['/'], exclusionPatterns: [glob] })(value);
describe('createLibraryMatcher', () => {
it.each([
{ path: '/photos/a.jpg', importPaths: ['/photos/'], isInLibrary: true },
{ path: '/other/a.jpg', importPaths: ['/photos/'], isInLibrary: false },
{ path: '/other/a.jpg', importPaths: ['/photos/', '/other/'], isInLibrary: true },
// unlike SQL `LIKE`, `_` and `%` are literal
{ path: '/photosX2020/a.jpg', importPaths: ['/photos_2020/'], isInLibrary: false },
{ path: '/photos/2020/a.jpg', importPaths: ['/photos%/'], isInLibrary: false },
])(
'should report $path as in library: $isInLibrary, with import paths $importPaths',
({ path, importPaths, isInLibrary }) => {
expect(createLibraryMatcher({ importPaths, exclusionPatterns: [] })(path)).toBe(isInLibrary);
},
);
describe('globToPostgresRegex', () => {
const testCases: [string, string, boolean][] = [
['**/Raw/**', '/foo/Raw/bar.jpg', true],
['**/Raw/**', '/foo/bar.jpg', false],
@@ -69,6 +84,14 @@ describe('globToPostgresRegex', () => {
// a bare `*` must not cross a path separator, unlike SQL `LIKE`'s `%`
['/path/*.*', '/path/photo.jpg', true],
['/path/*.*', '/path/2020/photo.jpg', false],
// matching is case-insensitive
['**/raw/**', '/foo/RAW/bar.jpg', true],
['**/*.ARW', '/foo/bar.arw', true],
// Postgres regexes disagreed with picomatch on these, or rejected them
['**/İstanbul/**', '/foo/istanbul/a.jpg', false],
['**/ΚΎΠΡΟΣ/**', '/foo/Κύπρος/a.jpg', true],
[String.raw`**/a\y/**`, '/foo/a/b.jpg', false],
[String.raw`**/a\k/**`, '/foo/ak/b.jpg', true],
];
it.each(testCases)('should match %s against %s as %s', (glob, value, expected) => {
+11 -4
View File
@@ -372,10 +372,17 @@ export const useSwagger = (app: INestApplication, { write }: { write: boolean })
writeFileSync(outputPath, JSON.stringify(patchOpenAPI(openApiDoc), null, 2), { encoding: 'utf8' });
};
// Compiles a glob to the equivalent Postgres regex (Postgres's Advanced Regular Expression
// dialect is a superset of what picomatch emits, so the two stay in sync with `picomatch.isMatch`,
// including which paths a lone `*` may cross vs `/`).
export const globToPostgresRegex = (glob: string) => picomatch.makeRe(glob).source;
// Offline detection and the re-online check must share one matcher, or an asset flips state on every scan
export const createLibraryMatcher = ({
importPaths,
exclusionPatterns,
}: {
importPaths: string[];
exclusionPatterns: string[];
}) => {
const isExcluded = picomatch(exclusionPatterns, { nocase: true });
return (path: string) => importPaths.some((importPath) => path.startsWith(importPath)) && !isExcluded(path);
};
export function clamp(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, value));
@@ -1,11 +1,14 @@
import { Kysely } from 'kysely';
import { Insertable, Kysely } from 'kysely';
import { chunk } from 'lodash-es';
import { JOBS_LIBRARY_PAGINATION_SIZE } from 'src/constants.js';
import { AssetFileType, AssetOrder, AssetOrderBy, AssetType, AssetVisibility } from 'src/enum.js';
import { AssetRepository } from 'src/repositories/asset.repository.js';
import { LibraryRepository } from 'src/repositories/library.repository.js';
import { LoggingRepository } from 'src/repositories/logging.repository.js';
import { DB } from 'src/schema/index.js';
import { AssetTable } from 'src/schema/tables/asset.table.js';
import { BaseService } from 'src/services/base.service.js';
import { newMediumService } from 'test/medium.factory.js';
import { mediumFactory, newMediumService } from 'test/medium.factory.js';
import { factory } from 'test/small.factory.js';
import { getKyselyDB } from 'test/utils.js';
@@ -25,6 +28,13 @@ beforeAll(async () => {
defaultDatabase = await getKyselyDB();
});
const newExternalLibrary = async (ctx: ReturnType<typeof setup>['ctx']) => {
const { user } = await ctx.newUser();
return ctx
.get(LibraryRepository)
.create({ ownerId: user.id, name: 'library', importPaths: [], exclusionPatterns: [] });
};
// Metadata extraction is repeatable: probing improves, files get repaired,
// and a re-run has to be able to correct what an earlier run stored.
const audioRow = (assetId: string, n: number) => ({
@@ -625,4 +635,56 @@ describe(AssetRepository.name, () => {
await expect(sut.createAll([])).resolves.toStrictEqual([]);
});
});
describe('detectOfflineExternalAssets', () => {
it('should offline the assets the matcher rejects and return how many it offlined', async () => {
const { ctx, sut } = setup();
const [library, otherLibrary] = await Promise.all([newExternalLibrary(ctx), newExternalLibrary(ctx)]);
const deletedAt = new Date('2020-01-01T00:00:00.000Z');
const newExternalAsset = (originalPath: string, dto: Partial<Insertable<AssetTable>> = {}) =>
ctx.newAsset({ ownerId: library.ownerId, libraryId: library.id, isExternal: true, originalPath, ...dto });
const [{ asset: kept }, { asset: rejected }, { asset: alreadyOffline }, { asset: otherLibraryAsset }] =
await Promise.all([
newExternalAsset('/keep/a.jpg'),
newExternalAsset('/reject/b.jpg'),
newExternalAsset('/reject/c.jpg', { isOffline: true, deletedAt }),
ctx.newAsset({
ownerId: otherLibrary.ownerId,
libraryId: otherLibrary.id,
isExternal: true,
originalPath: '/reject/d.jpg',
}),
]);
await expect(sut.detectOfflineExternalAssets(library.id, (path) => path.startsWith('/keep/'))).resolves.toBe(1);
await expect(sut.getById(kept.id)).resolves.toMatchObject({ isOffline: false, deletedAt: null });
await expect(sut.getById(rejected.id)).resolves.toMatchObject({ isOffline: true, deletedAt: expect.any(Date) });
await expect(sut.getById(alreadyOffline.id)).resolves.toMatchObject({ isOffline: true, deletedAt });
await expect(sut.getById(otherLibraryAsset.id)).resolves.toMatchObject({ isOffline: false, deletedAt: null });
});
it('should offline each asset once when the same library is checked concurrently', async () => {
const { ctx, sut } = setup();
const library = await newExternalLibrary(ctx);
const assets = Array.from({ length: JOBS_LIBRARY_PAGINATION_SIZE * 2 }, (_, index) =>
mediumFactory.assetInsert({
ownerId: library.ownerId,
libraryId: library.id,
isExternal: true,
originalPath: `/reject/${index}.jpg`,
}),
);
for (const batch of chunk(assets, 1000)) {
await sut.createAll(batch);
}
const counts = await Promise.all([
sut.detectOfflineExternalAssets(library.id, () => false),
sut.detectOfflineExternalAssets(library.id, () => false),
]);
expect(counts[0] + counts[1]).toBe(assets.length);
}, 60_000);
});
});
@@ -1,9 +1,11 @@
import { Kysely } from 'kysely';
import { chunk } from 'lodash-es';
import { existsSync } from 'node:fs';
import { copyFile, mkdir, mkdtemp, rm, utimes, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { dirname, join, resolve } from 'node:path';
import type { SystemConfig } from 'src/dtos/config.dto.js';
import { JOBS_LIBRARY_PAGINATION_SIZE } from 'src/constants.js';
import { StorageCore } from 'src/cores/storage.core.js';
import { AssetStatus, JobName, JobStatus } from 'src/enum.js';
import { AssetJobRepository } from 'src/repositories/asset-job.repository.js';
@@ -20,7 +22,7 @@ import { WebsocketRepository } from 'src/repositories/websocket.repository.js';
import { DB } from 'src/schema/index.js';
import { LibraryService } from 'src/services/library.service.js';
import { systemConfigStub } from 'test/fixtures/system-config.stub.js';
import { MediumTestContext, testAssetsDir } from 'test/medium.factory.js';
import { MediumTestContext, mediumFactory, testAssetsDir } from 'test/medium.factory.js';
import { newUuid } from 'test/small.factory.js';
import { getKyselyDB } from 'test/utils.js';
@@ -574,6 +576,44 @@ describe(LibraryService.name, () => {
await ctx.scan(library.id);
await expect(ctx.getAssetPaths(library.id)).resolves.toEqual([asset1]);
});
// Postgres case folding disagreed with picomatch on these: `İ` folds to `i`, but `Σ` does not fold to `ς`
it.each([
{ pattern: '**/İstanbul/**', folder: 'istanbul', isExcluded: false },
{ pattern: '**/ΚΎΠΡΟΣ/**', folder: 'Κύπρος', isExcluded: true },
])('should apply $pattern to $folder the same way on every scan', async ({ pattern, folder, isExcluded }) => {
const { sut, ctx } = setup();
const asset = await createFile(join(importRoot, folder, 'asset.png'));
const library = await ctx.createLibrary({ importPaths: [importRoot] });
await ctx.scan(library.id);
await sut.update(library.id, { exclusionPatterns: [pattern] });
const expected = isExcluded ? [] : [asset];
await ctx.scan(library.id);
await expect(ctx.getAssetPaths(library.id)).resolves.toEqual(expected);
await ctx.scan(library.id);
await expect(ctx.getAssetPaths(library.id)).resolves.toEqual(expected);
});
it('should offline a missing asset when an exclusion pattern is not a valid Postgres regex', async () => {
const { ctx } = setup();
const kept = await createFile(join(importPath, 'kept.png'));
const missing = await createFile(join(importPath, 'missing.png'));
const library = await ctx.createLibrary({
importPaths: [importPath],
exclusionPatterns: [String.raw`**/a\k/**`],
});
await ctx.scan(library.id);
await expect(ctx.getAssetPaths(library.id)).resolves.toEqual([kept, missing]);
await rm(missing);
await ctx.scan(library.id);
await expect(ctx.getAssetPaths(library.id)).resolves.toEqual([kept]);
});
});
describe('watch', () => {
@@ -646,6 +686,35 @@ describe(LibraryService.name, () => {
expect(updated).toEqual(expect.objectContaining({ isOffline: true }));
expect(updated?.deletedAt).toBeInstanceOf(Date);
});
it('should not exhaust the database pool when more libraries are checked at once than it has connections', async () => {
const { sut, ctx } = setup();
const assetRepo = ctx.get(AssetRepository);
// the pool has 10 connections, and each library spans more than one batch so it offlines assets mid-read
const libraries = await Promise.all(
Array.from({ length: 10 }, () => ctx.createLibrary({ importPaths: [importPath] })),
);
for (const library of libraries) {
const assets = Array.from({ length: JOBS_LIBRARY_PAGINATION_SIZE + 1 }, (_, index) =>
mediumFactory.assetInsert({
ownerId: library.ownerId,
libraryId: library.id,
originalPath: join(outsidePath, `${index}.jpg`),
isExternal: true,
}),
);
for (const batch of chunk(assets, 1000)) {
await assetRepo.createAll(batch);
}
}
await expect(Promise.all(libraries.map(({ id }) => sut.handleQueueSyncAssets({ id })))).resolves.toEqual(
libraries.map(() => JobStatus.Success),
);
for (const { id } of libraries) {
await expect(ctx.getAssetPaths(id)).resolves.toEqual([]);
}
}, 60_000);
});
describe('handleSyncAssets', () => {