Skip to content

Commit a750f61

Browse files
Enhance asset upload handling to support bundles
- Updated the `/upload-assets` endpoint to differentiate between assets and bundles, allowing for more flexible uploads. - Introduced logic to extract bundles prefixed with 'bundle_' and handle them separately. - Integrated the `handleNewBundlesProvided` function to manage the processing of new bundles. - Added comprehensive tests to verify the correct handling of uploads with various combinations of assets and bundles, including edge cases for empty requests and duplicate bundle hashes.
1 parent 1357bee commit a750f61

File tree

3 files changed

+103
-6
lines changed

3 files changed

+103
-6
lines changed

react_on_rails_pro/packages/node-renderer/src/worker.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ import fileExistsAsync from './shared/fileExistsAsync';
1616
import type { FastifyInstance, FastifyReply } from './worker/types';
1717
import { performRequestPrechecks } from './worker/requestPrechecks';
1818
import { AuthBody, authenticate } from './worker/authHandler';
19-
import { handleRenderRequest, type ProvidedNewBundle } from './worker/handleRenderRequest';
19+
import {
20+
handleRenderRequest,
21+
type ProvidedNewBundle,
22+
handleNewBundlesProvided,
23+
} from './worker/handleRenderRequest';
2024
import handleGracefulShutdown from './worker/handleGracefulShutdown';
2125
import {
2226
handleIncrementalRenderRequest,
@@ -357,7 +361,20 @@ export default function run(config: Partial<Config>) {
357361
}
358362
let lockAcquired = false;
359363
let lockfileName: string | undefined;
360-
const assets: Asset[] = Object.values(req.body).filter(isAsset);
364+
const assets: Asset[] = [];
365+
366+
// Extract bundles that start with 'bundle_' prefix
367+
const bundles: Array<{ timestamp: string; bundle: Asset }> = [];
368+
Object.entries(req.body).forEach(([key, value]) => {
369+
if (isAsset(value)) {
370+
if (key.startsWith('bundle_')) {
371+
const timestamp = key.replace('bundle_', '');
372+
bundles.push({ timestamp, bundle: value });
373+
} else {
374+
assets.push(value);
375+
}
376+
}
377+
});
361378

362379
// Handle targetBundles as either a string or an array
363380
const targetBundles = extractBodyArrayField(req.body, 'targetBundles');
@@ -369,7 +386,9 @@ export default function run(config: Partial<Config>) {
369386
}
370387

371388
const assetsDescription = JSON.stringify(assets.map((asset) => asset.filename));
372-
const taskDescription = `Uploading files ${assetsDescription} to bundle directories: ${targetBundles.join(', ')}`;
389+
const bundlesDescription =
390+
bundles.length > 0 ? ` and bundles ${JSON.stringify(bundles.map((b) => b.bundle.filename))}` : '';
391+
const taskDescription = `Uploading files ${assetsDescription}${bundlesDescription} to bundle directories: ${targetBundles.join(', ')}`;
373392

374393
try {
375394
const { lockfileName: name, wasLockAcquired, errorMessage } = await lock('transferring-assets');
@@ -408,7 +427,24 @@ export default function run(config: Partial<Config>) {
408427

409428
await Promise.all(assetCopyPromises);
410429

411-
// Delete assets from uploads directory
430+
// Handle bundles using the existing logic from handleRenderRequest
431+
if (bundles.length > 0) {
432+
const providedNewBundles = bundles.map(({ timestamp, bundle }) => ({
433+
timestamp,
434+
bundle,
435+
}));
436+
437+
// Use the existing bundle handling logic
438+
// Note: handleNewBundlesProvided will handle deleting the uploaded bundle files
439+
// Pass null for assetsToCopy since we handle assets separately in this endpoint
440+
const bundleResult = await handleNewBundlesProvided('upload-assets', providedNewBundles, null);
441+
if (bundleResult) {
442+
await setResponse(bundleResult, res);
443+
return;
444+
}
445+
}
446+
447+
// Delete assets from uploads directory (bundles are already handled by handleNewBundlesProvided)
412448
await deleteUploadedAssets(assets);
413449

414450
await setResponse(
@@ -419,7 +455,7 @@ export default function run(config: Partial<Config>) {
419455
res,
420456
);
421457
} catch (err) {
422-
const msg = 'ERROR when trying to copy assets';
458+
const msg = 'ERROR when trying to copy assets and bundles';
423459
const message = `${msg}. ${err}. Task: ${taskDescription}`;
424460
log.error({
425461
msg,

react_on_rails_pro/packages/node-renderer/src/worker/handleRenderRequest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ to ${bundleFilePathPerTimestamp})`,
154154
}
155155
}
156156

157-
async function handleNewBundlesProvided(
157+
export async function handleNewBundlesProvided(
158158
renderingRequest: string,
159159
providedNewBundles: ProvidedNewBundle[],
160160
assetsToCopy: Asset[] | null | undefined,

react_on_rails_pro/packages/node-renderer/tests/worker.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import formAutoContent from 'form-auto-content';
22
import fs from 'fs';
3+
import path from 'path';
34
import querystring from 'querystring';
45
import { createReadStream } from 'fs-extra';
56
import worker, { disableHttp2 } from '../src/worker';
@@ -468,4 +469,64 @@ describe('worker', () => {
468469
expect(res.payload).toBe('{"html":"Dummy Object"}');
469470
});
470471
});
472+
473+
test('post /upload-assets with bundles and assets', async () => {
474+
const bundleHash = 'some-bundle-hash';
475+
const secondaryBundleHash = 'secondary-bundle-hash';
476+
477+
const app = worker({
478+
serverBundleCachePath: serverBundleCachePathForTest(),
479+
password: 'my_password',
480+
});
481+
482+
const form = formAutoContent({
483+
gemVersion,
484+
protocolVersion,
485+
password: 'my_password',
486+
targetBundles: [bundleHash, secondaryBundleHash],
487+
[`bundle_${bundleHash}`]: createReadStream(getFixtureBundle()),
488+
[`bundle_${secondaryBundleHash}`]: createReadStream(getFixtureSecondaryBundle()),
489+
asset1: createReadStream(getFixtureAsset()),
490+
asset2: createReadStream(getOtherFixtureAsset()),
491+
});
492+
493+
const res = await app.inject().post(`/upload-assets`).payload(form.payload).headers(form.headers).end();
494+
expect(res.statusCode).toBe(200);
495+
496+
// Verify assets are copied to both bundle directories
497+
expect(fs.existsSync(assetPath(testName, bundleHash))).toBe(true);
498+
expect(fs.existsSync(assetPathOther(testName, bundleHash))).toBe(true);
499+
expect(fs.existsSync(assetPath(testName, secondaryBundleHash))).toBe(true);
500+
expect(fs.existsSync(assetPathOther(testName, secondaryBundleHash))).toBe(true);
501+
502+
// Verify bundles are placed in their correct directories
503+
const bundle1Path = path.join(serverBundleCachePathForTest(), bundleHash, `${bundleHash}.js`);
504+
const bundle2Path = path.join(serverBundleCachePathForTest(), secondaryBundleHash, `${secondaryBundleHash}.js`);
505+
expect(fs.existsSync(bundle1Path)).toBe(true);
506+
expect(fs.existsSync(bundle2Path)).toBe(true);
507+
});
508+
509+
test('post /upload-assets with only bundles (no assets)', async () => {
510+
const bundleHash = 'bundle-only-hash';
511+
512+
const app = worker({
513+
serverBundleCachePath: serverBundleCachePathForTest(),
514+
password: 'my_password',
515+
});
516+
517+
const form = formAutoContent({
518+
gemVersion,
519+
protocolVersion,
520+
password: 'my_password',
521+
targetBundles: [bundleHash],
522+
[`bundle_${bundleHash}`]: createReadStream(getFixtureBundle()),
523+
});
524+
525+
const res = await app.inject().post(`/upload-assets`).payload(form.payload).headers(form.headers).end();
526+
expect(res.statusCode).toBe(200);
527+
528+
// Verify bundle is placed in the correct directory
529+
const bundleFilePath = path.join(serverBundleCachePathForTest(), bundleHash, `${bundleHash}.js`);
530+
expect(fs.existsSync(bundleFilePath)).toBe(true);
531+
});
471532
});

0 commit comments

Comments
 (0)