Merge pull request #630 from the-luap/fix/lightroom-export-623
fix+feat: bundled bugfixes — Lightroom (#623), hero gap (#624), stale cache (#625), publish password (#627), low-memory OOM (#628), duplicate gallery (#626)
This commit is contained in:
@@ -0,0 +1,71 @@
|
||||
/**
|
||||
* exportAsTxt — issue #623 regression test.
|
||||
*
|
||||
* The admin UI labels the TXT export "for Lightroom search". Lightroom's
|
||||
* filename search wants ONE comma-separated line WITHOUT file extensions
|
||||
* (the gallery JPEGs may map to RAW files in the catalog). The frontend
|
||||
* now passes separator='comma' + include_extension=false for the TXT
|
||||
* format; this test pins the resulting shape so a future refactor can't
|
||||
* silently regress it back to the newline-separated form the bug reported.
|
||||
*
|
||||
* Also pins backward compatibility: a direct API caller passing no options
|
||||
* still gets the original newline-with-extension behaviour, so existing
|
||||
* integrations don't break.
|
||||
*/
|
||||
jest.mock('../../src/database/db', () => ({ db: jest.fn() }));
|
||||
jest.mock('../../src/services/xmpGenerator', () => ({ XmpGenerator: class {} }));
|
||||
|
||||
const { PhotoExportService } = require('../../src/services/photoExportService');
|
||||
const service = new PhotoExportService();
|
||||
|
||||
const PHOTOS = [
|
||||
{ original_filename: 'IMG_0001.jpg', filename: 'abc123.jpg' },
|
||||
{ original_filename: 'IMG_0002.JPEG', filename: 'def456.jpeg' },
|
||||
{ original_filename: 'shoot.final.tif', filename: 'ghi789.tif' },
|
||||
{ original_filename: null, filename: 'fallback.png' }, // null original → falls back to filename
|
||||
];
|
||||
|
||||
describe('exportAsTxt (issue #623)', () => {
|
||||
it('Lightroom mode: comma-joined, no extension, no space', () => {
|
||||
const result = service.exportAsTxt(PHOTOS, {
|
||||
separator: 'comma',
|
||||
include_extension: false,
|
||||
});
|
||||
expect(result.content).toBe('IMG_0001,IMG_0002,shoot.final,fallback');
|
||||
expect(result.contentType).toBe('text/plain');
|
||||
});
|
||||
|
||||
it('backward compatible: no options → newline-joined with extensions', () => {
|
||||
const result = service.exportAsTxt(PHOTOS);
|
||||
expect(result.content).toBe(
|
||||
'IMG_0001.jpg\nIMG_0002.JPEG\nshoot.final.tif\nfallback.png',
|
||||
);
|
||||
});
|
||||
|
||||
it('semicolon separator joins without a trailing space', () => {
|
||||
const result = service.exportAsTxt(PHOTOS, {
|
||||
separator: 'semicolon',
|
||||
include_extension: false,
|
||||
});
|
||||
expect(result.content).toBe('IMG_0001;IMG_0002;shoot.final;fallback');
|
||||
});
|
||||
|
||||
it('filename_format=picpeak uses photo.filename (hashed) instead of original', () => {
|
||||
const result = service.exportAsTxt(PHOTOS, {
|
||||
filename_format: 'picpeak',
|
||||
separator: 'comma',
|
||||
include_extension: false,
|
||||
});
|
||||
expect(result.content).toBe('abc123,def456,ghi789,fallback');
|
||||
});
|
||||
|
||||
it('extension stripping uses only the last segment ("a.b.c" → "a.b")', () => {
|
||||
// path.parse('shoot.final.tif').name === 'shoot.final' — Lightroom
|
||||
// catalogs that store basenames like "shoot.final" still match.
|
||||
const result = service.exportAsTxt(
|
||||
[{ original_filename: 'shoot.final.tif', filename: 'x.tif' }],
|
||||
{ separator: 'comma', include_extension: false },
|
||||
);
|
||||
expect(result.content).toBe('shoot.final');
|
||||
});
|
||||
});
|
||||
@@ -1062,9 +1062,25 @@ router.get('/:id', adminAuth, requirePermission('events.view'), async (req, res)
|
||||
});
|
||||
|
||||
// Publish a draft event (set is_draft=false and queue creation email)
|
||||
router.post('/:id/publish', adminAuth, requirePermission('events.edit'), requireEventOwnership, async (req, res) => {
|
||||
router.post('/:id/publish', adminAuth, requirePermission('events.edit'), requireEventOwnership, [
|
||||
// Optional password the admin re-types in the publish dialog so the
|
||||
// gallery_created email can carry the actual plaintext (#627). When the
|
||||
// event is password-protected and the body carries a password, picpeak
|
||||
// re-hashes + writes `password_hash` (the admin may have mistyped at
|
||||
// creation; this guarantees the email content matches the live login
|
||||
// password). When omitted, behaviour is the legacy sentinel for backward
|
||||
// compat with API-only consumers.
|
||||
body('password').optional().isString().isLength({ min: 6 })
|
||||
.withMessage('Password must be at least 6 characters long'),
|
||||
], async (req, res) => {
|
||||
try {
|
||||
const errors = validationResult(req);
|
||||
if (!errors.isEmpty()) {
|
||||
return res.status(400).json({ errors: errors.array() });
|
||||
}
|
||||
|
||||
const { id } = req.params;
|
||||
const { password } = req.body;
|
||||
const event = await db('events').where('id', id).first();
|
||||
|
||||
if (!event) {
|
||||
@@ -1075,8 +1091,15 @@ router.post('/:id/publish', adminAuth, requirePermission('events.edit'), require
|
||||
return res.status(400).json({ error: 'Event is already published' });
|
||||
}
|
||||
|
||||
// Set is_draft to false
|
||||
await db('events').where('id', id).update({ is_draft: formatBoolean(false) });
|
||||
const requirePassword = parseBooleanInput(event.require_password, true);
|
||||
const publishUpdates = { is_draft: formatBoolean(false) };
|
||||
if (requirePassword && password) {
|
||||
// Re-hash so the stored hash matches what the email carries — even if
|
||||
// the admin mistypes vs. what was set at draft creation, the gallery
|
||||
// password the customer receives is the one that actually works.
|
||||
publishUpdates.password_hash = await bcrypt.hash(password, getBcryptRounds());
|
||||
}
|
||||
await db('events').where('id', id).update(publishUpdates);
|
||||
|
||||
// Queue creation email
|
||||
const customerEmail = event.customer_email || event.host_email;
|
||||
@@ -1085,6 +1108,18 @@ router.post('/:id/publish', adminAuth, requirePermission('events.edit'), require
|
||||
const frontendBase = await getFrontendBaseUrl();
|
||||
const { shareUrl } = await buildShareLinkVariants({ slug: event.slug, shareToken: event.share_token });
|
||||
|
||||
let galleryPasswordForEmail;
|
||||
if (!requirePassword) {
|
||||
galleryPasswordForEmail = 'No password required';
|
||||
} else if (password) {
|
||||
// Admin re-typed the password in the publish dialog — put it straight
|
||||
// into the email so the customer can actually log in (#627).
|
||||
galleryPasswordForEmail = password;
|
||||
} else {
|
||||
// Legacy fallback for API-only publishes that don't carry the password.
|
||||
galleryPasswordForEmail = '(set at creation)';
|
||||
}
|
||||
|
||||
const emailData = {
|
||||
customer_name: customerName,
|
||||
customer_email: customerEmail,
|
||||
@@ -1092,7 +1127,7 @@ router.post('/:id/publish', adminAuth, requirePermission('events.edit'), require
|
||||
event_name: event.event_name,
|
||||
event_date: event.event_date,
|
||||
gallery_link: shareUrl || `${frontendBase}/gallery/${event.slug}`,
|
||||
gallery_password: parseBooleanInput(event.require_password, true) ? '(set at creation)' : 'No password required',
|
||||
gallery_password: galleryPasswordForEmail,
|
||||
expiry_date: event.expires_at ? new Date(event.expires_at).toISOString() : null,
|
||||
welcome_message: event.welcome_message || ''
|
||||
};
|
||||
@@ -1141,6 +1176,193 @@ router.post('/:id/publish', adminAuth, requirePermission('events.edit'), require
|
||||
}
|
||||
});
|
||||
|
||||
// Duplicate an event (#626). Creates a new DRAFT gallery that inherits the
|
||||
// source event's branding, behaviour, hero/header, feedback, and category
|
||||
// configuration — admin then fills in customer + publishes via the publish
|
||||
// dialog (#627), where the password is set. Photos, hero photo selection,
|
||||
// client-access secrets, customer assignments, archive/sent state are NOT
|
||||
// carried over.
|
||||
router.post('/:id/duplicate', adminAuth, requirePermission('events.create'), requireEventOwnership, [
|
||||
body('event_name').trim().notEmpty().withMessage('Event name is required'),
|
||||
body('event_date').optional({ values: 'falsy' }).isDate(),
|
||||
body('customer_name').optional().trim(),
|
||||
body('customer_email').optional({ values: 'falsy' }).isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL),
|
||||
], async (req, res) => {
|
||||
try {
|
||||
const errors = validationResult(req);
|
||||
if (!errors.isEmpty()) {
|
||||
return res.status(400).json({ errors: errors.array() });
|
||||
}
|
||||
|
||||
const { id } = req.params;
|
||||
const source = await db('events').where('id', id).first();
|
||||
if (!source) {
|
||||
return res.status(404).json({ error: 'Source event not found' });
|
||||
}
|
||||
|
||||
const { event_name, event_date, customer_name, customer_email } = req.body;
|
||||
|
||||
// Generate a fresh unique slug using the same shape as the create path.
|
||||
const slugify = require('../utils/slug').slugify;
|
||||
const processedEventName = slugify(event_name);
|
||||
const slugSuffix = event_date || crypto.randomBytes(3).toString('hex');
|
||||
const baseSlug = `${source.event_type}-${processedEventName}-${slugSuffix}`;
|
||||
let slug = baseSlug;
|
||||
let counter = 1;
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
while (await db('events').where({ slug }).first()) {
|
||||
slug = `${baseSlug}-${counter}`;
|
||||
counter += 1;
|
||||
}
|
||||
|
||||
// Recompute expires_at: preserve the source's expiration window (delta
|
||||
// between source.expires_at and source.event_date) so the duplicate keeps
|
||||
// the same "active for N days" feel. Falls back to 30 days if source had
|
||||
// no expiration set.
|
||||
let newExpiresAt = null;
|
||||
if (event_date) {
|
||||
let expirationDays = 30;
|
||||
if (source.expires_at && source.event_date) {
|
||||
const days = Math.round(
|
||||
(new Date(source.expires_at).getTime() - new Date(source.event_date).getTime())
|
||||
/ (24 * 60 * 60 * 1000),
|
||||
);
|
||||
if (days > 0) expirationDays = days;
|
||||
}
|
||||
const [year, month, day] = event_date.split('-').map((s) => parseInt(s, 10));
|
||||
const baseDate = new Date(year, month - 1, day);
|
||||
baseDate.setDate(baseDate.getDate() + expirationDays);
|
||||
newExpiresAt = baseDate;
|
||||
}
|
||||
|
||||
const shareToken = crypto.randomBytes(16).toString('hex');
|
||||
const { shareLinkToStore } = await buildShareLinkVariants({ slug, shareToken });
|
||||
|
||||
// Random-placeholder password hash. When the admin publishes via the
|
||||
// PublishGalleryDialog (#627), the dialog re-hashes whatever they type and
|
||||
// overwrites this. Pattern matches the create path at line ~606.
|
||||
const password_hash = await bcrypt.hash(crypto.randomBytes(32).toString('hex'), getBcryptRounds());
|
||||
|
||||
// Create the storage folder structure (same as create path).
|
||||
const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
|
||||
const eventPath = path.join(storagePath, 'events/active', slug);
|
||||
await fs.mkdir(path.join(eventPath, 'collages'), { recursive: true });
|
||||
await fs.mkdir(path.join(eventPath, 'individual'), { recursive: true });
|
||||
|
||||
const customerColumnsAvailable = await hasCustomerContactColumns();
|
||||
const calendarColumnsExist = await hasColumnCached('events', 'is_full_day');
|
||||
|
||||
// Build the insert row. Copy behaviour + branding fields from source;
|
||||
// leave per-gallery secrets / state / photos blank.
|
||||
const insertResult = await db('events').insert({
|
||||
slug,
|
||||
event_type: source.event_type,
|
||||
event_name,
|
||||
event_date: event_date || null,
|
||||
...(calendarColumnsExist ? {
|
||||
event_time_start: source.event_time_start,
|
||||
event_time_end: source.event_time_end,
|
||||
is_full_day: source.is_full_day,
|
||||
} : {}),
|
||||
...(customerColumnsAvailable ? {
|
||||
customer_name: customer_name || null,
|
||||
customer_email: customer_email || null,
|
||||
} : {}),
|
||||
host_name: customer_name || null,
|
||||
host_email: customer_email || null,
|
||||
admin_email: source.admin_email || null,
|
||||
password_hash,
|
||||
welcome_message: source.welcome_message || '',
|
||||
color_theme: source.color_theme,
|
||||
share_link: shareLinkToStore,
|
||||
share_token: shareToken,
|
||||
expires_at: newExpiresAt ? newExpiresAt.toISOString() : null,
|
||||
created_at: new Date().toISOString(),
|
||||
created_by: req.admin.id,
|
||||
allow_user_uploads: source.allow_user_uploads,
|
||||
upload_category_id: source.upload_category_id,
|
||||
allow_downloads: source.allow_downloads,
|
||||
disable_right_click: source.disable_right_click,
|
||||
enable_devtools_protection: source.enable_devtools_protection,
|
||||
watermark_downloads: source.watermark_downloads,
|
||||
watermark_text: source.watermark_text,
|
||||
allow_presigned_download: source.allow_presigned_download,
|
||||
require_password: source.require_password,
|
||||
css_template_id: source.css_template_id || null,
|
||||
hero_logo_visible: source.hero_logo_visible,
|
||||
hero_logo_size: source.hero_logo_size,
|
||||
hero_logo_position: source.hero_logo_position,
|
||||
header_style: source.header_style || 'standard',
|
||||
hero_divider_style: source.hero_divider_style || 'wave',
|
||||
hero_image_anchor: source.hero_image_anchor || 'center',
|
||||
photo_cap: source.photo_cap || null,
|
||||
is_draft: formatBoolean(true),
|
||||
default_photo_sort: source.default_photo_sort || 'upload_date_desc',
|
||||
// Client-access secrets and the OG-share opt-in deliberately do NOT
|
||||
// carry over — admin re-decides per gallery.
|
||||
client_access_enabled: formatBoolean(false),
|
||||
og_image_share_enabled: formatBoolean(false),
|
||||
}).returning('id');
|
||||
|
||||
const newEventId = insertResult[0]?.id || insertResult[0];
|
||||
|
||||
// Copy event_feedback_settings if the source had a row (only present when
|
||||
// feedback_enabled was true on the source event).
|
||||
const sourceFeedback = await db('event_feedback_settings').where({ event_id: id }).first();
|
||||
if (sourceFeedback) {
|
||||
await db('event_feedback_settings').insert({
|
||||
event_id: newEventId,
|
||||
feedback_enabled: sourceFeedback.feedback_enabled,
|
||||
allow_ratings: sourceFeedback.allow_ratings,
|
||||
allow_likes: sourceFeedback.allow_likes,
|
||||
allow_comments: sourceFeedback.allow_comments,
|
||||
allow_favorites: sourceFeedback.allow_favorites,
|
||||
require_name_email: sourceFeedback.require_name_email,
|
||||
moderate_comments: sourceFeedback.moderate_comments,
|
||||
show_feedback_to_guests: sourceFeedback.show_feedback_to_guests,
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
// Copy per-event photo categories (global categories are not duplicated —
|
||||
// they apply to every event already). Mapping by name; photo_categories
|
||||
// has no foreign key into photos here so we just clone the rows.
|
||||
if (await db.schema.hasTable('photo_categories')) {
|
||||
const sourceCategories = await db('photo_categories')
|
||||
.where({ event_id: id })
|
||||
.where(function () { this.whereNull('is_global').orWhere('is_global', formatBoolean(false)); })
|
||||
.select('name', 'slug', 'is_global');
|
||||
if (sourceCategories.length > 0) {
|
||||
await db('photo_categories').insert(
|
||||
sourceCategories.map((c) => ({
|
||||
event_id: newEventId,
|
||||
name: c.name,
|
||||
slug: c.slug,
|
||||
is_global: formatBoolean(false),
|
||||
})),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
await logActivity('event_duplicated',
|
||||
{ source_event_id: parseInt(id, 10), source_event_name: source.event_name },
|
||||
newEventId,
|
||||
{ type: 'admin', id: req.admin.id, name: req.admin.username },
|
||||
);
|
||||
|
||||
res.json({
|
||||
message: 'Event duplicated successfully',
|
||||
id: newEventId,
|
||||
slug,
|
||||
is_draft: true,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.error('Error duplicating event:', { error: error.message });
|
||||
res.status(500).json({ error: 'Failed to duplicate event' });
|
||||
}
|
||||
});
|
||||
|
||||
// Update event
|
||||
router.put('/:id', adminAuth, requirePermission('events.edit'), requireEventOwnership, [
|
||||
body('event_name').optional().trim().notEmpty(),
|
||||
|
||||
@@ -16,18 +16,52 @@
|
||||
* is enough for the rare two-process case during dev).
|
||||
*
|
||||
* Tunables (env, all optional):
|
||||
* UPLOAD_PROCESSOR_CONCURRENCY default 2
|
||||
* UPLOAD_PROCESSOR_CONCURRENCY default 2 on hosts with ≥3GB RAM,
|
||||
* 1 on smaller hosts (auto-detected
|
||||
* via os.totalmem() with one-shot
|
||||
* warning, #628). Always honoured
|
||||
* when set explicitly.
|
||||
* UPLOAD_PROCESSOR_POLL_MS default 1000
|
||||
* UPLOAD_PROCESSOR_STUCK_TIMEOUT_MS default 600000 (10 minutes)
|
||||
* UPLOAD_PROCESSOR_DISABLED default false (set 'true' to opt out, e.g. in CI)
|
||||
*/
|
||||
|
||||
const os = require('os');
|
||||
const { db } = require('../database/db');
|
||||
const logger = require('../utils/logger');
|
||||
const { processPhoto } = require('./photoProcessor');
|
||||
|
||||
const POLL_INTERVAL_MS = parseInt(process.env.UPLOAD_PROCESSOR_POLL_MS || '1000', 10);
|
||||
const CONCURRENCY = Math.max(1, parseInt(process.env.UPLOAD_PROCESSOR_CONCURRENCY || '2', 10));
|
||||
|
||||
// Soft default: two worker loops × sharp.concurrency(2) means up to four
|
||||
// libvips threads can decode full-resolution photos in parallel. Each decode
|
||||
// holds the full uncompressed frame in RAM — a 24MP photo is ~96MB before
|
||||
// resize. On a 2GB VPS (the documented but barely-viable minimum) one busy
|
||||
// batch is enough to OOM-kill the backend and surface as 503s on thumbnails
|
||||
// (#628). When the host reports < 3GB total memory AND the admin hasn't set
|
||||
// an explicit override, drop the default to 1 and log a one-shot warning
|
||||
// naming the override env var. Explicit env-var setters keep their value.
|
||||
//
|
||||
// os.totalmem() reports container memory under cgroup v2 (Docker / k8s) and
|
||||
// host memory on bare metal — accurate enough for this decision in either
|
||||
// deployment shape.
|
||||
function pickDefaultConcurrency() {
|
||||
if (process.env.UPLOAD_PROCESSOR_CONCURRENCY !== undefined) {
|
||||
return parseInt(process.env.UPLOAD_PROCESSOR_CONCURRENCY, 10);
|
||||
}
|
||||
const totalRamGB = os.totalmem() / (1024 ** 3);
|
||||
if (totalRamGB < 3) {
|
||||
logger.warn?.(
|
||||
`[backgroundProcessor] Detected ${totalRamGB.toFixed(1)}GB total RAM (< 3GB threshold). ` +
|
||||
'Defaulting UPLOAD_PROCESSOR_CONCURRENCY to 1 to avoid OOM on heavy upload batches. ' +
|
||||
'Set UPLOAD_PROCESSOR_CONCURRENCY=2 (or higher) explicitly to override.',
|
||||
);
|
||||
return 1;
|
||||
}
|
||||
return 2;
|
||||
}
|
||||
|
||||
const CONCURRENCY = Math.max(1, pickDefaultConcurrency());
|
||||
const STUCK_TIMEOUT_MS = parseInt(process.env.UPLOAD_PROCESSOR_STUCK_TIMEOUT_MS || '600000', 10);
|
||||
const JANITOR_INTERVAL_MS = 60 * 1000;
|
||||
|
||||
|
||||
@@ -81,21 +81,36 @@ class PhotoExportService {
|
||||
|
||||
/**
|
||||
* Export as plain text filename list
|
||||
*
|
||||
* include_extension defaults to true for backward compatibility with any
|
||||
* direct API consumer. The admin UI sets it to false for the Lightroom
|
||||
* search use case — the gallery JPEGs may correspond to RAW files in the
|
||||
* photographer's catalog, so the search has to match on the stem only.
|
||||
*
|
||||
* The comma separator joins without a space, the form Lightroom's filename
|
||||
* search expects (per issue #623).
|
||||
*/
|
||||
exportAsTxt(photos, options = {}) {
|
||||
const { filename_format = 'original', separator = 'newline' } = options;
|
||||
const {
|
||||
filename_format = 'original',
|
||||
separator = 'newline',
|
||||
include_extension = true,
|
||||
} = options;
|
||||
|
||||
const filenames = photos.map(photo =>
|
||||
filename_format === 'original' ? (photo.original_filename || photo.filename) : photo.filename
|
||||
);
|
||||
const filenames = photos.map(photo => {
|
||||
const name = filename_format === 'original'
|
||||
? (photo.original_filename || photo.filename)
|
||||
: photo.filename;
|
||||
return include_extension ? name : path.parse(name).name;
|
||||
});
|
||||
|
||||
let content;
|
||||
switch (separator) {
|
||||
case 'comma':
|
||||
content = filenames.join(', ');
|
||||
content = filenames.join(',');
|
||||
break;
|
||||
case 'semicolon':
|
||||
content = filenames.join('; ');
|
||||
content = filenames.join(';');
|
||||
break;
|
||||
default:
|
||||
content = filenames.join('\n');
|
||||
|
||||
Reference in New Issue
Block a user