From c4d407d2cb1f01415df2e9fcc7e0c3b925b0382f Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 16:05:35 +0100 Subject: [PATCH 01/12] fix telemetry --- .../strapi/lib/commands/transfer/export.js | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 130a9466d5..7772fff7b0 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -74,6 +74,7 @@ module.exports = async (opts) => { }, }); + let transferExitCode; try { logger.log(`Starting export...`); @@ -88,16 +89,18 @@ module.exports = async (opts) => { }; }; - progress.on('transfer::start', (payload) => { - strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); + progress.on('transfer::start', async (payload) => { + await strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); }); - progress.on('transfer::finish', (payload) => { - strapi.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); + progress.on('transfer::finish', async (payload) => { + await strapi.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); + transferExitCode = 0; }); - progress.on('transfer::error', (payload) => { - strapi.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); + progress.on('transfer::error', async (payload) => { + await strapi.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); + transferExitCode = 1; }); const results = await engine.transfer(); @@ -113,11 +116,37 @@ module.exports = async (opts) => { logger.log(`${chalk.bold('Export process has been completed successfully!')}`); logger.log(`Export archive is in ${chalk.green(outFile)}`); - process.exit(0); } catch (e) { logger.error('Export process failed unexpectedly:', e.toString()); process.exit(1); } + + /* + * We need to wait for the telemetry to finish before exiting the process. + * The order of execution for this function is: + * - create providers and engine + * - create progress callbacks + * - await the engine transfer + * - having async calls inside, it allows the transfer::start to process + * - the code block including the table printing executes + * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: + * - the progress callbacks execute + * + * Because of that, we can't exit the process in the progress callbacks and instead have to wait for them to tell us it's safe to exit + */ + const waitForExitCode = async (maxWait) => { + const startTime = Date.now(); + while (Date.now() - startTime < maxWait) { + if (transferExitCode !== undefined) { + process.exit(transferExitCode); + } + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + } + process.exit(0); + }; + waitForExitCode(5000); }; /** From 7ee5030bd60f005ccb0778e2ace143e6055af0e4 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 16:13:45 +0100 Subject: [PATCH 02/12] fix import and cleanup --- .../strapi/lib/commands/transfer/export.js | 42 +++++------ .../strapi/lib/commands/transfer/import.js | 69 +++++++++++++------ 2 files changed, 70 insertions(+), 41 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 7772fff7b0..2bf323475c 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -75,34 +75,34 @@ module.exports = async (opts) => { }); let transferExitCode; - try { - logger.log(`Starting export...`); + logger.log(`Starting export...`); - const progress = engine.progress.stream; + const progress = engine.progress.stream; - const telemetryPayload = (/* payload */) => { - return { - eventProperties: { - source: engine.sourceProvider.name, - destination: engine.destinationProvider.name, - }, - }; + const telemetryPayload = (/* payload */) => { + return { + eventProperties: { + source: engine.sourceProvider.name, + destination: engine.destinationProvider.name, + }, }; + }; - progress.on('transfer::start', async (payload) => { - await strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); - }); + progress.on('transfer::start', async (payload) => { + await strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); + }); - progress.on('transfer::finish', async (payload) => { - await strapi.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); - transferExitCode = 0; - }); + progress.on('transfer::finish', async (payload) => { + await strapi.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); + transferExitCode = 0; + }); - progress.on('transfer::error', async (payload) => { - await strapi.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); - transferExitCode = 1; - }); + progress.on('transfer::error', async (payload) => { + await strapi.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); + transferExitCode = 1; + }); + try { const results = await engine.transfer(); const outFile = results.destination.file.path; diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index df8506b3b8..21b635b223 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -79,42 +79,71 @@ module.exports = async (opts) => { }; const engine = createTransferEngine(source, destination, engineOptions); - try { - logger.info('Starting import...'); + let transferExitCode; + logger.info('Starting import...'); - const progress = engine.progress.stream; - const telemetryPayload = (/* payload */) => { - return { - eventProperties: { - source: engine.sourceProvider.name, - destination: engine.destinationProvider.name, - }, - }; + const progress = engine.progress.stream; + const telemetryPayload = (/* payload */) => { + return { + eventProperties: { + source: engine.sourceProvider.name, + destination: engine.destinationProvider.name, + }, }; + }; - progress.on('transfer::start', (payload) => { - strapiInstance.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); - }); + progress.on('transfer::start', async (payload) => { + await strapiInstance.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); + }); - progress.on('transfer::finish', (payload) => { - strapiInstance.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); - }); + progress.on('transfer::finish', async (payload) => { + await strapiInstance.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); + transferExitCode = 0; + }); - progress.on('transfer::error', (payload) => { - strapiInstance.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); - }); + progress.on('transfer::error', async (payload) => { + await strapiInstance.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); + transferExitCode = 1; + }); + try { const results = await engine.transfer(); const table = buildTransferTable(results.engine); logger.info(table.toString()); logger.info('Import process has been completed successfully!'); - process.exit(0); } catch (e) { logger.error('Import process failed unexpectedly:'); logger.error(e); process.exit(1); } + + /* + * We need to wait for the telemetry to finish before exiting the process. + * The order of execution for this function is: + * - create providers and engine + * - create progress callbacks + * - await the engine transfer + * - having async calls inside, it allows the transfer::start to process + * - the code block including the table printing executes + * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: + * - the progress callbacks execute + * + * Because of that, we can't exit the process in the progress callbacks and instead have to wait for them to tell us it's safe to exit + */ + const waitForExitCode = async (maxWait) => { + const startTime = Date.now(); + while (Date.now() - startTime < maxWait) { + if (transferExitCode !== undefined) { + process.exit(transferExitCode); + } + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + } + process.exit(0); + }; + waitForExitCode(5000); }; /** From b59da2bca89658e2277006c0c3bb60dea84627ee Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 16:50:31 +0100 Subject: [PATCH 03/12] clarify comment --- packages/core/strapi/lib/commands/transfer/export.js | 2 +- packages/core/strapi/lib/commands/transfer/import.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 2bf323475c..912ed97711 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -123,7 +123,7 @@ module.exports = async (opts) => { /* * We need to wait for the telemetry to finish before exiting the process. - * The order of execution for this function is: + * The order of execution for the overall export function is: * - create providers and engine * - create progress callbacks * - await the engine transfer diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 21b635b223..9c382aadb0 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -120,7 +120,7 @@ module.exports = async (opts) => { /* * We need to wait for the telemetry to finish before exiting the process. - * The order of execution for this function is: + * The order of execution for the overall import function is: * - create providers and engine * - create progress callbacks * - await the engine transfer From adb5073f00cc16f458e256bd5d9a7d83d17f6116 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 16:52:57 +0100 Subject: [PATCH 04/12] clarify comment --- packages/core/strapi/lib/commands/transfer/export.js | 3 ++- packages/core/strapi/lib/commands/transfer/import.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 912ed97711..055710afd3 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -132,7 +132,8 @@ module.exports = async (opts) => { * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: * - the progress callbacks execute * - * Because of that, we can't exit the process in the progress callbacks and instead have to wait for them to tell us it's safe to exit + * Because of that, we have to wait until the progress callbacks have executed, but can't allow them to exit by themselves. + * Instead we have to wait for them to tell us it's safe to exit */ const waitForExitCode = async (maxWait) => { const startTime = Date.now(); diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 9c382aadb0..00f8498665 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -129,7 +129,8 @@ module.exports = async (opts) => { * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: * - the progress callbacks execute * - * Because of that, we can't exit the process in the progress callbacks and instead have to wait for them to tell us it's safe to exit + * Because of that, we have to wait until the progress callbacks have executed, but can't allow them to exit by themselves. + * Instead we have to wait for them to tell us it's safe to exit */ const waitForExitCode = async (maxWait) => { const startTime = Date.now(); From d75ae711140366beb5da78379358c890dc444361 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 18:02:34 +0100 Subject: [PATCH 05/12] fix tests --- .../lib/commands/__tests__/export.test.js | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/core/strapi/lib/commands/__tests__/export.test.js b/packages/core/strapi/lib/commands/__tests__/export.test.js index bf30ebeeda..58da3110da 100644 --- a/packages/core/strapi/lib/commands/__tests__/export.test.js +++ b/packages/core/strapi/lib/commands/__tests__/export.test.js @@ -1,13 +1,20 @@ 'use strict'; +const { Readable } = require('stream'); const utils = require('../transfer/utils'); const mockDataTransfer = { createLocalFileDestinationProvider: jest.fn(), createLocalStrapiSourceProvider: jest.fn(), - createTransferEngine: jest.fn().mockReturnValue({ - transfer: jest.fn().mockReturnValue(Promise.resolve({})), - }), + createTransferEngine() { + return { + transfer: jest.fn().mockReturnValue(Promise.resolve({})), + progress: { + on: jest.fn(), + stream: Readable.from([1, 2, 3]), + }, + }; + }, }; jest.mock( @@ -85,12 +92,17 @@ describe('export', () => { expect(exit).toHaveBeenCalled(); }); - it('compresses the output file if specified', async () => { - const compress = true; - await exportCommand({ compress }); + it('uses compress option', async () => { + await exportCommand({ compress: false }); expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ - compression: { enabled: compress }, + compression: { enabled: false }, + }) + ); + await exportCommand({ compress: true }); + expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( + expect.objectContaining({ + compression: { enabled: true }, }) ); expect(exit).toHaveBeenCalled(); From e24fc80b79177482d00c780f3c880bd1a6b66812 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 22 Dec 2022 18:16:55 +0100 Subject: [PATCH 06/12] fix stream --- packages/core/strapi/lib/commands/__tests__/export.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/strapi/lib/commands/__tests__/export.test.js b/packages/core/strapi/lib/commands/__tests__/export.test.js index 58da3110da..fefced0dc5 100644 --- a/packages/core/strapi/lib/commands/__tests__/export.test.js +++ b/packages/core/strapi/lib/commands/__tests__/export.test.js @@ -1,6 +1,5 @@ 'use strict'; -const { Readable } = require('stream'); const utils = require('../transfer/utils'); const mockDataTransfer = { @@ -11,7 +10,9 @@ const mockDataTransfer = { transfer: jest.fn().mockReturnValue(Promise.resolve({})), progress: { on: jest.fn(), - stream: Readable.from([1, 2, 3]), + stream: { + on: jest.fn(), + }, }, }; }, From 8a62757911354932d0a859e19beb51305ff77499 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 28 Dec 2022 09:14:56 +0100 Subject: [PATCH 07/12] fix events --- .../strapi/lib/commands/transfer/export.js | 48 +++---------------- .../strapi/lib/commands/transfer/import.js | 48 +++---------------- 2 files changed, 14 insertions(+), 82 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 055710afd3..e0c0de6d79 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -74,12 +74,11 @@ module.exports = async (opts) => { }, }); - let transferExitCode; logger.log(`Starting export...`); const progress = engine.progress.stream; - const telemetryPayload = (/* payload */) => { + const getTelemetryPayload = (/* payload */) => { return { eventProperties: { source: engine.sourceProvider.name, @@ -88,18 +87,8 @@ module.exports = async (opts) => { }; }; - progress.on('transfer::start', async (payload) => { - await strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); - }); - - progress.on('transfer::finish', async (payload) => { - await strapi.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); - transferExitCode = 0; - }); - - progress.on('transfer::error', async (payload) => { - await strapi.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); - transferExitCode = 1; + progress.on('transfer::start', async () => { + await strapi.telemetry.send('didDEITSProcessStart', getTelemetryPayload()); }); try { @@ -117,37 +106,14 @@ module.exports = async (opts) => { logger.log(`${chalk.bold('Export process has been completed successfully!')}`); logger.log(`Export archive is in ${chalk.green(outFile)}`); } catch (e) { + await strapi.telemetry.send('didDEITSProcessFail', getTelemetryPayload()); logger.error('Export process failed unexpectedly:', e.toString()); process.exit(1); } - /* - * We need to wait for the telemetry to finish before exiting the process. - * The order of execution for the overall export function is: - * - create providers and engine - * - create progress callbacks - * - await the engine transfer - * - having async calls inside, it allows the transfer::start to process - * - the code block including the table printing executes - * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: - * - the progress callbacks execute - * - * Because of that, we have to wait until the progress callbacks have executed, but can't allow them to exit by themselves. - * Instead we have to wait for them to tell us it's safe to exit - */ - const waitForExitCode = async (maxWait) => { - const startTime = Date.now(); - while (Date.now() - startTime < maxWait) { - if (transferExitCode !== undefined) { - process.exit(transferExitCode); - } - await new Promise((resolve) => { - setTimeout(resolve, 50); - }); - } - process.exit(0); - }; - waitForExitCode(5000); + // Note: Telemetry can't be sent in a finish event, because it runs async after this block but we can't await it, so if process.exit is used it won't send + await strapi.telemetry.send('didDEITSProcessFinish', getTelemetryPayload()); + process.exit(0); }; /** diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 00f8498665..2140f56ece 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -79,11 +79,10 @@ module.exports = async (opts) => { }; const engine = createTransferEngine(source, destination, engineOptions); - let transferExitCode; logger.info('Starting import...'); const progress = engine.progress.stream; - const telemetryPayload = (/* payload */) => { + const getTelemetryPayload = () => { return { eventProperties: { source: engine.sourceProvider.name, @@ -92,18 +91,8 @@ module.exports = async (opts) => { }; }; - progress.on('transfer::start', async (payload) => { - await strapiInstance.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); - }); - - progress.on('transfer::finish', async (payload) => { - await strapiInstance.telemetry.send('didDEITSProcessFinish', telemetryPayload(payload)); - transferExitCode = 0; - }); - - progress.on('transfer::error', async (payload) => { - await strapiInstance.telemetry.send('didDEITSProcessFail', telemetryPayload(payload)); - transferExitCode = 1; + progress.on('transfer::start', async () => { + await strapiInstance.telemetry.send('didDEITSProcessStart', getTelemetryPayload()); }); try { @@ -113,38 +102,15 @@ module.exports = async (opts) => { logger.info('Import process has been completed successfully!'); } catch (e) { + await strapiInstance.telemetry.send('didDEITSProcessFail', getTelemetryPayload()); logger.error('Import process failed unexpectedly:'); logger.error(e); process.exit(1); } - /* - * We need to wait for the telemetry to finish before exiting the process. - * The order of execution for the overall import function is: - * - create providers and engine - * - create progress callbacks - * - await the engine transfer - * - having async calls inside, it allows the transfer::start to process - * - the code block including the table printing executes - * - *** any async code (for example, the fs.pathExists) after engine.transfer will execute next tick, therefore: - * - the progress callbacks execute - * - * Because of that, we have to wait until the progress callbacks have executed, but can't allow them to exit by themselves. - * Instead we have to wait for them to tell us it's safe to exit - */ - const waitForExitCode = async (maxWait) => { - const startTime = Date.now(); - while (Date.now() - startTime < maxWait) { - if (transferExitCode !== undefined) { - process.exit(transferExitCode); - } - await new Promise((resolve) => { - setTimeout(resolve, 50); - }); - } - process.exit(0); - }; - waitForExitCode(5000); + // Note: Telemetry can't be sent in a finish event, because it runs async after this block but we can't await it, so if process.exit is used it won't send + await strapi.telemetry.send('didDEITSProcessFinish', getTelemetryPayload()); + process.exit(0); }; /** From 2a1335717bd9712081674853d656fd5faf24e41b Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 28 Dec 2022 11:00:52 +0100 Subject: [PATCH 08/12] fix tests --- .../lib/commands/__tests__/export.test.js | 145 +++++++++++------- 1 file changed, 90 insertions(+), 55 deletions(-) diff --git a/packages/core/strapi/lib/commands/__tests__/export.test.js b/packages/core/strapi/lib/commands/__tests__/export.test.js index fefced0dc5..6a57e78533 100644 --- a/packages/core/strapi/lib/commands/__tests__/export.test.js +++ b/packages/core/strapi/lib/commands/__tests__/export.test.js @@ -1,111 +1,146 @@ 'use strict'; -const utils = require('../transfer/utils'); - -const mockDataTransfer = { - createLocalFileDestinationProvider: jest.fn(), - createLocalStrapiSourceProvider: jest.fn(), - createTransferEngine() { - return { - transfer: jest.fn().mockReturnValue(Promise.resolve({})), - progress: { - on: jest.fn(), - stream: { - on: jest.fn(), - }, - }, - }; - }, -}; - -jest.mock( - '@strapi/data-transfer', - () => { - return mockDataTransfer; - }, - { virtual: true } -); - -const exportCommand = require('../transfer/export'); - -const exit = jest.spyOn(process, 'exit').mockImplementation(() => {}); -jest.spyOn(console, 'error').mockImplementation(() => {}); - -jest.mock('../transfer/utils'); - -const defaultFileName = 'defaultFilename'; - describe('export', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); + const defaultFileName = 'defaultFilename'; + + // mock @strapi/data-transfer + const mockDataTransfer = { + createLocalFileDestinationProvider: jest.fn().mockReturnValue({ name: 'testDest' }), + createLocalStrapiSourceProvider: jest.fn().mockReturnValue({ name: 'testSource' }), + createTransferEngine() { + return { + transfer: jest.fn().mockReturnValue(Promise.resolve({})), + progress: { + on: jest.fn(), + stream: { + on: jest.fn(), + }, + }, + sourceProvider: { name: 'testSource' }, + destinationProvider: { name: 'testDestination' }, + }; + }, + }; + jest.mock( + '@strapi/data-transfer', + () => { + return mockDataTransfer; + }, + { virtual: true } + ); + + // mock utils + const mockUtils = { + createStrapiInstance() { + return { + telemetry: { + send: jest.fn(), + }, + }; + }, + getDefaultExportName: jest.fn(() => defaultFileName), + }; + jest.mock( + '../transfer/utils', + () => { + return mockUtils; + }, + { virtual: true } + ); + + // other spies= + jest.spyOn(console, 'log').mockImplementation(() => {}); + jest.spyOn(console, 'warn').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => {}); + + // Now that everything is mocked, import export command + const exportCommand = require('../transfer/export'); + + const expectExit = async (code, fn) => { + const exit = jest.spyOn(process, 'exit').mockImplementation((number) => { + throw new Error(`process.exit: ${number}`); + }); + await expect(async () => { + await fn(); + }).rejects.toThrow(); + expect(exit).toHaveBeenCalledWith(code); + exit.mockRestore(); + }; + + beforeEach(() => {}); it('uses path provided by user', async () => { - const filename = 'testfile'; + const filename = 'test'; - await exportCommand({ file: filename }); + await expectExit(1, async () => { + await exportCommand({ file: filename }); + }); expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ file: { path: filename }, }) ); - expect(utils.getDefaultExportName).not.toHaveBeenCalled(); - expect(exit).toHaveBeenCalled(); + expect(mockUtils.getDefaultExportName).not.toHaveBeenCalled(); }); it('uses default path if not provided by user', async () => { - utils.getDefaultExportName.mockReturnValue(defaultFileName); - - await exportCommand({}); + await expectExit(1, async () => { + await exportCommand({}); + }); + expect(mockUtils.getDefaultExportName).toHaveBeenCalledTimes(1); expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ file: { path: defaultFileName }, }) ); - - expect(utils.getDefaultExportName).toHaveBeenCalled(); - expect(exit).toHaveBeenCalled(); }); it('encrypts the output file if specified', async () => { const encrypt = true; - await exportCommand({ encrypt }); + await expectExit(1, async () => { + await exportCommand({ encrypt }); + }); + expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ encryption: { enabled: encrypt }, }) ); - expect(exit).toHaveBeenCalled(); }); it('encrypts the output file with the given key', async () => { const key = 'secret-key'; const encrypt = true; + await expectExit(1, async () => { + await exportCommand({ encrypt, key }); + }); - await exportCommand({ encrypt, key }); expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ encryption: { enabled: encrypt, key }, }) ); - expect(exit).toHaveBeenCalled(); }); it('uses compress option', async () => { - await exportCommand({ compress: false }); + await expectExit(1, async () => { + await exportCommand({ compress: false }); + }); + expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ compression: { enabled: false }, }) ); - await exportCommand({ compress: true }); + await expectExit(1, async () => { + await exportCommand({ compress: true }); + }); expect(mockDataTransfer.createLocalFileDestinationProvider).toHaveBeenCalledWith( expect.objectContaining({ compression: { enabled: true }, }) ); - expect(exit).toHaveBeenCalled(); }); }); From 1090e2244c7b895bb9fd19594e0ec7b8097c571e Mon Sep 17 00:00:00 2001 From: Convly Date: Wed, 28 Dec 2022 15:58:53 +0100 Subject: [PATCH 09/12] Run lerna watch commands in parallel + prevent output clear from tsc -w --- package.json | 2 +- packages/core/data-transfer/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index a6f2655350..77306117f0 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "prepare": "husky install", "setup": "yarn && yarn clean && yarn build", "clean": "lerna run --stream clean --no-private", - "watch": "lerna run --stream watch --no-private", + "watch": "lerna run --stream watch --no-private --parallel", "build": "lerna run --stream build --no-private", "generate": "plop --plopfile ./packages/generators/admin/plopfile.js", "lint": "npm-run-all -p lint:code lint:css", diff --git a/packages/core/data-transfer/package.json b/packages/core/data-transfer/package.json index 0c4f3e093e..3838a1543b 100644 --- a/packages/core/data-transfer/package.json +++ b/packages/core/data-transfer/package.json @@ -30,7 +30,7 @@ "build": "tsc -p tsconfig.json", "clean": "rimraf ./dist", "build:clean": "yarn clean && yarn build", - "watch": "yarn build -w", + "watch": "yarn build -w --preserveWatchOutput", "test:unit": "jest --verbose" }, "directories": { From fadcfac5ccc7c253acf4281e2e00a99f1f373d46 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 28 Dec 2022 16:08:08 +0100 Subject: [PATCH 10/12] throw errors before restoring --- packages/core/data-transfer/lib/engine/index.ts | 7 ++++--- .../providers/local-file-source-provider/index.ts | 15 +++++++++++---- .../core/strapi/lib/commands/transfer/export.js | 3 +-- .../core/strapi/lib/commands/transfer/import.js | 3 +-- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/core/data-transfer/lib/engine/index.ts b/packages/core/data-transfer/lib/engine/index.ts index f40c7b61fb..a8b4e4d1de 100644 --- a/packages/core/data-transfer/lib/engine/index.ts +++ b/packages/core/data-transfer/lib/engine/index.ts @@ -159,7 +159,7 @@ class TransferEngine< }); } - #emitTransferUpdate(type: 'start' | 'finish' | 'error', payload?: object) { + #emitTransferUpdate(type: 'init' | 'start' | 'finish' | 'error', payload?: object) { this.progress.stream.emit(`transfer::${type}`, payload); } @@ -336,9 +336,8 @@ class TransferEngine< // reset data between transfers this.progress.data = {}; - this.#emitTransferUpdate('start'); - try { + this.#emitTransferUpdate('init'); await this.bootstrap(); await this.init(); @@ -351,6 +350,8 @@ class TransferEngine< ); } + this.#emitTransferUpdate('start'); + await this.beforeTransfer(); // Run the transfer stages diff --git a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts index 8904817982..fb828e41db 100644 --- a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts +++ b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts @@ -49,6 +49,8 @@ class LocalFileSourceProvider implements ISourceProvider { options: ILocalFileSourceProviderOptions; + #metadata?: IMetadata; + constructor(options: ILocalFileSourceProviderOptions) { this.options = options; @@ -60,15 +62,20 @@ class LocalFileSourceProvider implements ISourceProvider { } /** - * Pre flight checks regarding the provided options (making sure that the provided path is correct, etc...) + * Pre flight checks regarding the provided options, making sure that the file can be opened (decrypted, decompressed), etc. */ async bootstrap() { const { path: filePath } = this.options.file; + try { - // This is only to show a nicer error, it doesn't ensure the file will still exist when we try to open it later - await fs.access(filePath, fs.constants.R_OK); + // Read the metadata to ensure the file can be parsed + this.#metadata = await this.getMetadata(); } catch (e) { - throw new Error(`Can't access file "${filePath}".`); + throw new Error(`Can't read file "${filePath}".`); + } + + if (!this.#metadata) { + throw new Error('Can\'t read file "metadata.json"'); } } diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 130a9466d5..cfc23bc26c 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -75,8 +75,6 @@ module.exports = async (opts) => { }); try { - logger.log(`Starting export...`); - const progress = engine.progress.stream; const telemetryPayload = (/* payload */) => { @@ -89,6 +87,7 @@ module.exports = async (opts) => { }; progress.on('transfer::start', (payload) => { + logger.log(`Starting export...`); strapi.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); }); diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index df8506b3b8..d19f8ac51d 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -80,8 +80,6 @@ module.exports = async (opts) => { const engine = createTransferEngine(source, destination, engineOptions); try { - logger.info('Starting import...'); - const progress = engine.progress.stream; const telemetryPayload = (/* payload */) => { return { @@ -93,6 +91,7 @@ module.exports = async (opts) => { }; progress.on('transfer::start', (payload) => { + logger.info('Starting import...'); strapiInstance.telemetry.send('didDEITSProcessStart', telemetryPayload(payload)); }); From 8677a4ac6c68ec9b6431dfd49a121fcf3cd991d5 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 28 Dec 2022 16:56:27 +0100 Subject: [PATCH 11/12] Update packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts --- .../lib/providers/local-file-source-provider/index.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts index fb828e41db..647cf3111e 100644 --- a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts +++ b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts @@ -74,9 +74,6 @@ class LocalFileSourceProvider implements ISourceProvider { throw new Error(`Can't read file "${filePath}".`); } - if (!this.#metadata) { - throw new Error('Can\'t read file "metadata.json"'); - } } getMetadata() { From b566c0d44c6555a2d6174bfcecdbada638ceaf35 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 28 Dec 2022 16:56:57 +0100 Subject: [PATCH 12/12] Update packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts --- .../lib/providers/local-file-source-provider/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts index 647cf3111e..ff078043dd 100644 --- a/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts +++ b/packages/core/data-transfer/lib/providers/local-file-source-provider/index.ts @@ -73,7 +73,6 @@ class LocalFileSourceProvider implements ISourceProvider { } catch (e) { throw new Error(`Can't read file "${filePath}".`); } - } getMetadata() {