The existence check matched on filename OR path. replacePhoto regenerates both — a fresh generated filename and a fresh managed path — so a watched-folder photo that had its file replaced stopped matching either arm. The original is still sitting in the watched folder, so the next sweep imported it again and the gallery ended up holding the delivered edit AND the untouched original: the same duplicate shape external_relpath prevents for reference galleries. source_filename is now a third arm. It is the stable key here — written once at ingest by this same path and preserved across a replace by design. Rows predating migration 193 are covered by its backfill: COALESCE(original_filename, filename), and this path never wrote original_filename, so for watcher rows that resolves to the basename being compared. The query is lifted into an exported findExistingPhoto() so the test drives it rather than a copy — the thing under test IS the query, so a query-builder mock would only assert that knex was called the way the test expects. Predates the Lightroom round-trip and applies to the admin replace path too; it became reachable when #1165 brought watcher galleries into round-trip scope. Six tests against a real SQLite database. The load-bearing one fails without the change, verified by removing the arm and re-running; the other five pin what must not move — filename and path matching, the pre-193 backfill shape, a genuinely new file, and event scoping. Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
064b1bcb14
commit
6ca8baab23
@@ -0,0 +1,171 @@
|
|||||||
|
/**
|
||||||
|
* The watcher must not re-import a photo whose file was replaced (#1226).
|
||||||
|
*
|
||||||
|
* `replacePhoto` generates a fresh `filename` AND a fresh managed `path`, so a
|
||||||
|
* watched-folder photo that has been replaced — through the Lightroom
|
||||||
|
* round-trip (#745) or the admin replace — matched neither of the two arms the
|
||||||
|
* existence check used to have. The original file is still sitting in the
|
||||||
|
* watched folder, so the next sweep imported it again and the gallery ended up
|
||||||
|
* holding the delivered edit AND the untouched original.
|
||||||
|
*
|
||||||
|
* `source_filename` is the stable key: written once at ingest and preserved
|
||||||
|
* across a replace by design (migration 193).
|
||||||
|
*
|
||||||
|
* Runs against a real SQLite database rather than a query-builder mock — the
|
||||||
|
* thing under test IS the query, so a mock would only assert that knex was
|
||||||
|
* called the way the test expects.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const path = require('path');
|
||||||
|
const fs = require('fs');
|
||||||
|
const os = require('os');
|
||||||
|
|
||||||
|
process.env.NODE_ENV = 'test';
|
||||||
|
process.env.TEST_DATABASE_PATH = path.join(
|
||||||
|
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-watcher-dedupe-')), 'db.sqlite',
|
||||||
|
);
|
||||||
|
process.env.JWT_SECRET = process.env.JWT_SECRET || 'watcher-dedupe-test-secret';
|
||||||
|
|
||||||
|
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
|
||||||
|
const { findExistingPhoto } = require('../../src/services/fileWatcher');
|
||||||
|
|
||||||
|
const EVENT_SLUG = 'watcher-dedupe-event';
|
||||||
|
const WATCHED_BASENAME = 'IMG_1234.JPG';
|
||||||
|
const WATCHED_RELPATH = `${EVENT_SLUG}/individual/IMG_1234.JPG`;
|
||||||
|
|
||||||
|
let db;
|
||||||
|
let cleanup;
|
||||||
|
let eventId;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
({ db, cleanup } = await bootCrmDb());
|
||||||
|
await seedMinimal(db);
|
||||||
|
const inserted = await db('events').insert({
|
||||||
|
slug: EVENT_SLUG,
|
||||||
|
event_type: 'wedding',
|
||||||
|
event_name: 'Watcher Dedupe',
|
||||||
|
event_date: '2026-08-29',
|
||||||
|
host_email: '[email protected]',
|
||||||
|
admin_email: '[email protected]',
|
||||||
|
password_hash: 'x',
|
||||||
|
share_link: `/gallery/${EVENT_SLUG}/share`,
|
||||||
|
share_token: 'watcher-dedupe-share',
|
||||||
|
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
|
||||||
|
is_active: 1,
|
||||||
|
is_archived: 0,
|
||||||
|
is_draft: 0,
|
||||||
|
created_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
eventId = inserted[0]?.id ?? inserted[0];
|
||||||
|
}, 120000);
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await db('photos').where({ event_id: eventId }).del();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
if (cleanup) await cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
|
async function addPhoto(row) {
|
||||||
|
const inserted = await db('photos').insert({
|
||||||
|
event_id: eventId,
|
||||||
|
type: 'individual',
|
||||||
|
uploaded_at: new Date().toISOString(),
|
||||||
|
...row,
|
||||||
|
}).returning('id');
|
||||||
|
return inserted[0]?.id ?? inserted[0];
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('fileWatcher existence check (#1226)', () => {
|
||||||
|
it('recognises a photo whose file was replaced, so it is not imported twice', async () => {
|
||||||
|
// Exactly the post-replace row: filename and path both regenerated by
|
||||||
|
// replacePhoto, source_filename still holding what the watcher imported.
|
||||||
|
const id = await addPhoto({
|
||||||
|
filename: 'wedding_individual_1755892345.jpg',
|
||||||
|
source_filename: WATCHED_BASENAME,
|
||||||
|
path: 'events/active/wedding/individual/wedding_individual_1755892345.jpg',
|
||||||
|
});
|
||||||
|
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found).toBeTruthy();
|
||||||
|
expect(found.id).toBe(id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still recognises an ordinary watcher import by filename', async () => {
|
||||||
|
const id = await addPhoto({
|
||||||
|
filename: WATCHED_BASENAME,
|
||||||
|
source_filename: WATCHED_BASENAME,
|
||||||
|
path: WATCHED_RELPATH,
|
||||||
|
});
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found.id).toBe(id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still recognises a row by path when the name differs', async () => {
|
||||||
|
const id = await addPhoto({
|
||||||
|
filename: 'something-else.jpg',
|
||||||
|
source_filename: 'something-else.jpg',
|
||||||
|
path: WATCHED_RELPATH,
|
||||||
|
});
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found.id).toBe(id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('covers rows predating migration 193, whose backfill set source_filename', async () => {
|
||||||
|
// The watcher never wrote original_filename, so COALESCE resolved to
|
||||||
|
// filename — the same basename this compares against.
|
||||||
|
const id = await addPhoto({
|
||||||
|
filename: WATCHED_BASENAME,
|
||||||
|
original_filename: null,
|
||||||
|
source_filename: WATCHED_BASENAME,
|
||||||
|
path: 'events/active/wedding/individual/old-layout.jpg',
|
||||||
|
});
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found.id).toBe(id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not match a genuinely new file', async () => {
|
||||||
|
await addPhoto({
|
||||||
|
filename: 'IMG_9999.JPG',
|
||||||
|
source_filename: 'IMG_9999.JPG',
|
||||||
|
path: `${EVENT_SLUG}/individual/IMG_9999.JPG`,
|
||||||
|
});
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found).toBeFalsy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is scoped to the event — the same basename elsewhere is not a match', async () => {
|
||||||
|
const other = await db('events').insert({
|
||||||
|
slug: 'other-watcher-event',
|
||||||
|
event_type: 'wedding',
|
||||||
|
event_name: 'Other',
|
||||||
|
event_date: '2026-08-29',
|
||||||
|
host_email: '[email protected]',
|
||||||
|
admin_email: '[email protected]',
|
||||||
|
password_hash: 'x',
|
||||||
|
share_link: '/gallery/other-watcher-event/share',
|
||||||
|
share_token: 'other-watcher-share',
|
||||||
|
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
|
||||||
|
is_active: 1,
|
||||||
|
is_archived: 0,
|
||||||
|
is_draft: 0,
|
||||||
|
created_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
const otherId = other[0]?.id ?? other[0];
|
||||||
|
await db('photos').insert({
|
||||||
|
event_id: otherId,
|
||||||
|
type: 'individual',
|
||||||
|
uploaded_at: new Date().toISOString(),
|
||||||
|
filename: WATCHED_BASENAME,
|
||||||
|
source_filename: WATCHED_BASENAME,
|
||||||
|
path: `other-watcher-event/individual/${WATCHED_BASENAME}`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const found = await findExistingPhoto(eventId, WATCHED_BASENAME, WATCHED_RELPATH);
|
||||||
|
expect(found).toBeFalsy();
|
||||||
|
|
||||||
|
await db('photos').where({ event_id: otherId }).del();
|
||||||
|
await db('events').where({ id: otherId }).del();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -63,6 +63,27 @@ function startFileWatcher() {
|
|||||||
logger.info('File watcher started');
|
logger.info('File watcher started');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Has this watched file already been imported into this event?
|
||||||
|
*
|
||||||
|
* Exported so the regression test drives this query rather than a copy of it.
|
||||||
|
* See the caller for why source_filename is one of the arms.
|
||||||
|
*
|
||||||
|
* @param {number} eventId
|
||||||
|
* @param {string} basename the file's basename on disk
|
||||||
|
* @param {string} relativePath the path the import would store
|
||||||
|
*/
|
||||||
|
async function findExistingPhoto(eventId, basename, relativePath) {
|
||||||
|
return db('photos')
|
||||||
|
.where({ event_id: eventId })
|
||||||
|
.where(function() {
|
||||||
|
this.where('filename', basename)
|
||||||
|
.orWhere('source_filename', basename)
|
||||||
|
.orWhere('path', relativePath);
|
||||||
|
})
|
||||||
|
.first();
|
||||||
|
}
|
||||||
|
|
||||||
async function processNewPhoto(filePath) {
|
async function processNewPhoto(filePath) {
|
||||||
const relativePath = path.relative(WATCH_PATH(), filePath);
|
const relativePath = path.relative(WATCH_PATH(), filePath);
|
||||||
const pathParts = relativePath.split(path.sep);
|
const pathParts = relativePath.split(path.sep);
|
||||||
@@ -122,14 +143,23 @@ async function processNewPhoto(filePath) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if photo already exists (by filename or path, to handle replacements)
|
// Check if photo already exists.
|
||||||
const existingPhoto = await db('photos')
|
//
|
||||||
.where({ event_id: event.id })
|
// `source_filename` is in here, not just filename/path, because a REPLACEMENT
|
||||||
.where(function() {
|
// changes both of those (#1226). replacePhoto generates a fresh filename and
|
||||||
this.where('filename', path.basename(filePath))
|
// a fresh managed path, so a watched-folder photo that had its file replaced
|
||||||
.orWhere('path', relativePath);
|
// — through the Lightroom round-trip (#745) or the admin replace — stopped
|
||||||
})
|
// matching either arm, and the next sweep imported the untouched original a
|
||||||
.first();
|
// second time. The gallery then held the edit AND the original: the same
|
||||||
|
// duplicate shape external_relpath prevents for reference galleries.
|
||||||
|
//
|
||||||
|
// source_filename is the stable key: written once at ingest (below) and
|
||||||
|
// preserved across a replace by design. Rows predating migration 193 are
|
||||||
|
// covered too — its backfill sets source_filename from
|
||||||
|
// COALESCE(original_filename, filename), and this path never wrote
|
||||||
|
// original_filename, so for watcher rows that resolves to the basename this
|
||||||
|
// compares against.
|
||||||
|
const existingPhoto = await findExistingPhoto(event.id, path.basename(filePath), relativePath);
|
||||||
|
|
||||||
if (!existingPhoto) {
|
if (!existingPhoto) {
|
||||||
// Add to database
|
// Add to database
|
||||||
@@ -192,4 +222,4 @@ async function removePhoto(filePath) {
|
|||||||
logger.info(`Removed photo: ${relativePath}`);
|
logger.info(`Removed photo: ${relativePath}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { startFileWatcher };
|
module.exports = { startFileWatcher, findExistingPhoto };
|
||||||
|
|||||||
Reference in New Issue
Block a user