Skip to content

Commit

Permalink
Grace period for modern thumbnails on publish (#227)
Browse files Browse the repository at this point in the history
  • Loading branch information
lbesson authored Feb 22, 2023
1 parent 265e045 commit 2e55c1e
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 17 deletions.
4 changes: 2 additions & 2 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export const AUTO_ORIENT_ORIGINAL = yn(process.env['AUTO_ORIENT_ORIGINAL'], { de
export const GENERATE_AVIF = yn(process.env['GENERATE_AVIF'], { default: false });
export const GENERATE_WEBP = yn(process.env['GENERATE_WEBP'], { default: false });

export const S3_EXPIRE_HOURS = 2;

export const ALLOWED_ORIGINS = (process.env['ALLOWED_ORIGINS'] ?? '*').split(',');

export const PREFER_IDENTIFY_OVER_FILE = yn(process.env['PREFER_IDENTIFY_OVER_FILE'], { default: false });
export const THUMBNAILS_PUBLISH_DELAY = Number.parseInt(process.env['THUMBNAILS_PUBLISH_DELAY'] ?? '', 10) || 20000;
export const S3_EXPIRE_HOURS = 2;
55 changes: 46 additions & 9 deletions src/koa/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ import Router from '@koa/router';
import { koaBody } from 'koa-body';
import fs from 'node:fs';
import path from 'node:path';
import { AUTO_ORIENT_ORIGINAL } from '../config.js';
import { AUTO_ORIENT_ORIGINAL, THUMBNAILS_PUBLISH_DELAY } from '../config.js';
import { autoOrient } from '../image/autoorient.js';
import { getFileFormat } from '../image/filetype.js';
import { rotateImages } from '../image/rotate.js';
import { allThumbnailKeys, baseThumbnailKeys, createThumbnails, modernThumbnailKeys } from '../image/thumbnails.js';
import { log } from '../log.js';
import {
promDeletedImagesCounter,
promErrorsCounter,
promPublishedImagesCounter,
promPublishedThumbnailsErrorCounter,
promRotatedImagesCounter,
promUploadedImagesCounter
} from '../metrics/prometheus.js';
Expand Down Expand Up @@ -62,7 +64,12 @@ router.post('/upload', bodyParser, async ctx => {
log.debug(`${keyPrefix} - uploading original file`);

await uploadOriginalAndBaseThumbnails(originalKey);
allRendered.then(async () => uploadModernThumbnails(originalKey)).catch(error => log.error(error));
allRendered
.then(async () => uploadModernThumbnails(originalKey))
.catch(error => {
log.error(error);
promErrorsCounter.inc();
});

log.debug(`${keyPrefix} - returning response`);

Expand All @@ -86,13 +93,43 @@ router.post('/publish', bodyParser, apiOnly, async ctx => {
}

if (!published) {
await Promise.all(
allThumbnailKeys(key).map(async resizedKey => {
if (await incomingStorage.exists(resizedKey)) {
return incomingStorage.move(resizedKey, activeStorage);
}
})
);
const publishThumbnails = async (thumbnailKeys: string[]): Promise<string[]> => {
const unrenderedThumbnailKeys: string[] = [];
await Promise.all(
thumbnailKeys.map(async resizedKey => {
if (await incomingStorage.exists(resizedKey)) {
return incomingStorage.move(resizedKey, activeStorage);
} else {
unrenderedThumbnailKeys.push(resizedKey);
}
})
);
return unrenderedThumbnailKeys;
};

const missingThumbnailKeys = await publishThumbnails(allThumbnailKeys(key));

// In some cases, it could be that all "modern" thumbnails are not yet generated (they are generated asyncrhonously)
// In that case, we give it some more time to fullfill. This will be done asynchronously, and will not prevent the response to be sent.
if (missingThumbnailKeys.length) {
setTimeout(
async () =>
publishThumbnails(missingThumbnailKeys)
.then(({ length }) => {
if (!length) {
return;
}
// some thumbnails were not generated
log.error(`${length} thumbnails could not be published for image ${key}`);
promPublishedThumbnailsErrorCounter.labels({ key }).inc(length);
})
.catch(error => {
log.error(error);
promErrorsCounter.inc();
}),
THUMBNAILS_PUBLISH_DELAY
);
}

await incomingStorage.move(key, activeStorage);

Expand Down
8 changes: 7 additions & 1 deletion src/metrics/prometheus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ export const promUploadedImagesCounter = new Counter({
// published images
export const promPublishedImagesCounter = new Counter({
name: 'published_images',
help: 'Counter of images published to the active bucket'
help: 'Counter of images published to the active bucket',
labelNames: ['key'] as const
});

export const promPublishedThumbnailsErrorCounter = new Counter({
name: 'published_thumbnails_error',
help: 'Counter of thumbnails that could not be published to the active bucket'
});

// deleted images
Expand Down
1 change: 1 addition & 0 deletions test/environment/env-vars.options.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ process.env.GENERATE_AVIF = '1';
process.env.GENERATE_WEBP = '1';
process.env.AUTO_ORIENT_ORIGINAL = '1';
process.env.ALLOWED_ORIGINS = '*';
process.env.THUMBNAILS_PUBLISH_DELAY = '1000';
39 changes: 34 additions & 5 deletions test/unit/options/webp.avif.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import request from 'supertest';
import { THUMBNAILS_PUBLISH_DELAY } from '../../../src/config.js';
import { isAvifWriteSupported, isWebpWriteSupported } from '../../../src/exec/imagemagick.js';
import { baseThumbnailKeys, modernThumbnailKeys } from '../../../src/image/thumbnails.js';
import { koa } from '../../../src/koa/app.js';
import { generateUniqueKeyPrefix } from '../../../src/koa/utils.js';
import { activeStorage, incomingStorage } from '../../../src/storage/storage.js';

const MODERN_GENERATION_TIME = 10000;
const MAX_MODERN_GENERATION_TIME = 10000;

describe('Asynchronous generation of Webp and Avif enabled', () => {
test(
Expand All @@ -29,12 +30,12 @@ describe('Asynchronous generation of Webp and Avif enabled', () => {
// modules with jest and --experimental-vm-modules is not top
// for the moment.
// We expect the generations of modern thumbnails to take less than 10s
await new Promise(resolve => setTimeout(resolve, MODERN_GENERATION_TIME));
await new Promise(resolve => setTimeout(resolve, MAX_MODERN_GENERATION_TIME));
for (const resizedKey of modernThumbnailKeys(key)) {
expect(await incomingStorage.exists(resizedKey)).toBe(true);
}
},
MODERN_GENERATION_TIME * 2
MAX_MODERN_GENERATION_TIME * 2
);

test(
Expand Down Expand Up @@ -65,11 +66,39 @@ describe('Asynchronous generation of Webp and Avif enabled', () => {
// modules with jest and --experimental-vm-modules is not top
// for the moment.
// We expect the generations of modern thumbnails to take less than 10s
await new Promise(resolve => setTimeout(resolve, MODERN_GENERATION_TIME));
await new Promise(resolve => setTimeout(resolve, MAX_MODERN_GENERATION_TIME));
for (const fileKey of modernThumbnailKeys(filename)) {
expect(await activeStorage.exists(fileKey)).toBe(true);
}
},
MODERN_GENERATION_TIME * 2
MAX_MODERN_GENERATION_TIME * 2
);

test('POST /publish will give a grace period for modern thumbnails to be generated after upload', async () => {
const key = generateUniqueKeyPrefix();
// make it so that BI webp is not available in incoming folder
await incomingStorage.put(`${key}.png`, 'test/data/piano.png');
await incomingStorage.put(`${key}BI.png`, 'test/data/piano.png');
await incomingStorage.put(`${key}BI.avif`, 'test/data/piano.png'); // we don't really care about the actual format

const response = await request(koa.callback())
.post('/publish')
.send({ secret: 'my secret', filename: `${key}.png` });

expect(JSON.parse(response.text)).toEqual({ success: true });
expect(response.status).toBe(200);
expect(await incomingStorage.exists(`${key}.png`)).toBe(false);
expect(await incomingStorage.exists(`${key}BI.png`)).toBe(false);
expect(await incomingStorage.exists(`${key}BI.avif`)).toBe(false);
expect(await activeStorage.exists(`${key}.png`)).toBe(true);
expect(await activeStorage.exists(`${key}BI.png`)).toBe(true);
expect(await activeStorage.exists(`${key}BI.avif`)).toBe(true);
expect(await activeStorage.exists(`${key}MI.avif`)).toBe(false); // could not be published, doesn't exist in incoming

await incomingStorage.put(`${key}MI.avif`, 'test/data/piano.png');

await new Promise(resolve => setTimeout(resolve, THUMBNAILS_PUBLISH_DELAY + 1000));

expect(await activeStorage.exists(`${key}MI.avif`)).toBe(true); // should now be published
});
});

0 comments on commit 2e55c1e

Please sign in to comment.