From dadc1f78e91a784c72692c875b3e0ea054ba8c7c Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Wed, 23 Aug 2023 14:18:57 +0200 Subject: [PATCH] fix tests --- .../data-transfer/src/__tests__/test-utils.ts | 2 +- .../__tests__/assets.test.ts | 66 ++++++++++++++----- .../local-destination/__tests__/index.test.ts | 30 ++++++++- .../providers/local-destination/index.ts | 43 +++++++----- 4 files changed, 104 insertions(+), 37 deletions(-) diff --git a/packages/core/data-transfer/src/__tests__/test-utils.ts b/packages/core/data-transfer/src/__tests__/test-utils.ts index c6ac8eed38..a95fd34f3f 100644 --- a/packages/core/data-transfer/src/__tests__/test-utils.ts +++ b/packages/core/data-transfer/src/__tests__/test-utils.ts @@ -29,7 +29,7 @@ export const getStrapiFactory = >( properties?: T ) => - (additionalProperties?: T) => { + (additionalProperties?: Partial) => { return { ...properties, ...additionalProperties } as Strapi.Strapi; }; diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/assets.test.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/assets.test.ts index 75095f14a1..0c20765c95 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/assets.test.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/assets.test.ts @@ -24,6 +24,25 @@ const transaction = jest.fn(async (cb) => { await cb({ trx, rollback }); }); +const strapiFactory = getStrapiFactory({ + dirs: { + static: { + public: 'static/public/assets', + }, + }, + db: { transaction }, + config: { + get(service) { + if (service === 'plugin.upload') { + return { + provider: 'local', + }; + } + return {}; + }, + }, +}); + describe('Local Strapi Destination Provider - Get Assets Stream', () => { test('Throws an error if the Strapi instance is not provided', async () => { /* @ts-ignore: disable-next-line */ @@ -35,17 +54,14 @@ describe('Local Strapi Destination Provider - Get Assets Stream', () => { 'Not able to stream Assets. Strapi instance not found' ); }); - test('Returns a stream', async () => { + + test('Returns a stream when assets restore is true', async () => { const provider = createLocalStrapiDestinationProvider({ - getStrapi: getStrapiFactory({ - dirs: { - static: { - public: 'static/public/assets', - }, - }, - db: { transaction }, - }), + getStrapi: () => strapiFactory(), strategy: 'restore', + restore: { + assets: true, + }, }); await provider.bootstrap(); @@ -54,6 +70,21 @@ describe('Local Strapi Destination Provider - Get Assets Stream', () => { expect(stream instanceof Writable).toBeTruthy(); }); + test('Throw an error if attempting to create stream while restore assets is false', async () => { + const provider = createLocalStrapiDestinationProvider({ + getStrapi: () => strapiFactory(), + strategy: 'restore', + restore: { + assets: false, + }, + }); + await provider.bootstrap(); + + expect(async () => provider.createAssetsWriteStream()).rejects.toThrow( + 'Attempting to transfer assets when they are not included' + ); + }); + test('Writes on the strapi assets path', async () => { (fse.createWriteStream as jest.Mock).mockImplementationOnce(createWriteStreamMock); const assetsDirectory = 'static/public/assets'; @@ -64,15 +95,18 @@ describe('Local Strapi Destination Provider - Get Assets Stream', () => { stream: Readable.from(['test', 'test-2']), }; const provider = createLocalStrapiDestinationProvider({ - getStrapi: getStrapiFactory({ - dirs: { - static: { - public: assetsDirectory, + getStrapi: () => + strapiFactory({ + dirs: { + static: { + public: assetsDirectory, + }, }, - }, - db: { transaction }, - }), + }), strategy: 'restore', + restore: { + assets: true, + }, }); await provider.bootstrap(); diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/index.test.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/index.test.ts index 257ee025f8..3c5f417a05 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/index.test.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/__tests__/index.test.ts @@ -19,7 +19,11 @@ jest.mock('../strategies/restore', () => { const strapiCommonProperties = { config: { - get: jest.fn().mockReturnValue({ provider: 'aws-s3' }), + get(service) { + if (service === 'plugin.upload') { + return { provider: 'local' }; + } + }, }, dirs: { static: { @@ -44,6 +48,11 @@ describe('Local Strapi Source Destination', () => { ...strapiCommonProperties, }), strategy: 'restore', + restore: { + entities: { + exclude: [], + }, + }, }); expect(provider.strapi).not.toBeDefined(); @@ -56,6 +65,11 @@ describe('Local Strapi Source Destination', () => { ...strapiCommonProperties, }), strategy: 'restore', + restore: { + entities: { + exclude: [], + }, + }, }); await provider.bootstrap(); @@ -71,6 +85,11 @@ describe('Local Strapi Source Destination', () => { ...strapiCommonProperties, }), strategy: 'restore', + restore: { + entities: { + exclude: [], + }, + }, }); await restoreProvider.bootstrap(); expect(restoreProvider.strapi).toBeDefined(); @@ -89,7 +108,9 @@ describe('Local Strapi Source Destination', () => { ).rejects.toThrow(); }); - test('Should delete all entities if it is a restore', async () => { + test.todo('Should not delete entities that are not included'); + + test('Should delete all entities if it is a restore with only exclude property', async () => { const entities = [ { entity: { id: 1, title: 'My first foo' }, @@ -161,6 +182,11 @@ describe('Local Strapi Source Destination', () => { const provider = createLocalStrapiDestinationProvider({ getStrapi: () => strapi, strategy: 'restore', + restore: { + entities: { + exclude: [], + }, + }, }); const deleteAllSpy = jest.spyOn(restoreApi, 'deleteRecords'); await provider.bootstrap(); diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts index a39dc3f903..6815f755af 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts @@ -13,7 +13,11 @@ import type { import { restore } from './strategies'; import * as utils from '../../../utils'; -import { ProviderTransferError, ProviderValidationError } from '../../../errors/providers'; +import { + ProviderInitializationError, + ProviderTransferError, + ProviderValidationError, +} from '../../../errors/providers'; import { assertValidStrapi } from '../../../utils/providers'; export const VALID_CONFLICT_STRATEGIES = ['restore']; @@ -54,6 +58,9 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { async bootstrap(): Promise { this.#validateOptions(); this.strapi = await this.options.getStrapi(); + if (!this.strapi) { + throw new ProviderInitializationError('Could not access local strapi'); + } this.transaction = utils.transaction.createTransaction(this.strapi); } @@ -99,7 +106,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } if ( - strapi.config.get('plugin.upload').provider !== 'local' && + this.strapi?.config.get('plugin.upload').provider !== 'local' && this.#areAssetsIncluded() && !this.#isContentTypeIncluded('plugin::upload.file') ) { @@ -126,7 +133,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { return; } - const stream: Readable = strapi.db + const stream: Readable = this.strapi.db // Create a query builder instance (default type is 'select') .queryBuilder('plugin::upload.file') // Fetch all columns @@ -138,10 +145,10 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { // TODO use bulk delete when exists in providers for await (const file of stream) { - await strapi.plugin('upload').provider.delete(file); + await this.strapi.plugin('upload').provider.delete(file); if (file.formats) { for (const fileFormat of Object.values(file.formats)) { - await strapi.plugin('upload').provider.delete(fileFormat); + await this.strapi.plugin('upload').provider.delete(fileFormat); } } } @@ -171,7 +178,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } getMetadata(): IMetadata { - const strapiVersion = strapi.config.get('info.strapi'); + const strapiVersion = this.strapi?.config.get('info.strapi'); const createdAt = new Date().toISOString(); return { @@ -185,8 +192,8 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { getSchemas() { assertValidStrapi(this.strapi, 'Not able to get Schemas'); const schemas = { - ...this.strapi.contentTypes, - ...this.strapi.components, + ...this.strapi?.contentTypes, + ...this.strapi?.components, }; return utils.schema.mapSchemasValues(schemas); @@ -227,7 +234,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { return; } - if (strapi.config.get('plugin.upload').provider === 'local') { + if (this.strapi.config.get('plugin.upload').provider === 'local') { const assetsDirectory = path.join(this.strapi.dirs.static.public, 'uploads'); const backupDirectory = path.join( this.strapi.dirs.static.public, @@ -267,7 +274,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } // TODO: this should catch all thrown errors and bubble it up to engine so it can be reported as a non-fatal diagnostic message telling the user they may need to manually delete assets - if (strapi.config.get('plugin.upload').provider === 'local') { + if (this.strapi?.config.get('plugin.upload').provider === 'local') { assertValidStrapi(this.strapi); const backupDirectory = path.join( this.strapi.dirs.static.public, @@ -286,7 +293,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } const removeAssetsBackup = this.#removeAssetsBackup.bind(this); - const strapi = this.strapi; + const thisStrapi = this.strapi; const transaction = this.transaction; const backupDirectory = this.uploadsBackupDirectoryName; @@ -304,7 +311,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { // TODO: Remove this logic in V5 if (!chunk.metadata) { // If metadata does not exist is because it is an old backup file - const assetsDirectory = path.join(strapi.dirs.static.public, 'uploads'); + const assetsDirectory = path.join(thisStrapi.dirs.static.public, 'uploads'); const entryPath = path.join(assetsDirectory, chunk.filename); const writableStream = fse.createWriteStream(entryPath); chunk.stream @@ -342,10 +349,10 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { buffer: chunk?.buffer, }; - const provider = strapi.config.get('plugin.upload').provider; + const provider = thisStrapi.config.get('plugin.upload').provider; try { - await strapi.plugin('upload').provider.uploadStream(uploadData); + await thisStrapi.plugin('upload').provider.uploadStream(uploadData); // if we're not supposed to transfer the associated entities, stop here if (!restoreMediaEntitiesContent) { @@ -354,14 +361,14 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { // Files formats are stored within the parent file entity if (uploadData?.type) { - const entry: IFile = await strapi.db.query('plugin::upload.file').findOne({ + const entry: IFile = await thisStrapi.db.query('plugin::upload.file').findOne({ where: { hash: uploadData.mainHash }, }); const specificFormat = entry?.formats?.[uploadData.type]; if (specificFormat) { specificFormat.url = uploadData.url; } - await strapi.db.query('plugin::upload.file').update({ + await thisStrapi.db.query('plugin::upload.file').update({ where: { hash: uploadData.mainHash }, data: { formats: entry.formats, @@ -370,11 +377,11 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { }); return callback(); } - const entry: IFile = await strapi.db.query('plugin::upload.file').findOne({ + const entry: IFile = await thisStrapi.db.query('plugin::upload.file').findOne({ where: { hash: uploadData.hash }, }); entry.url = uploadData.url; - await strapi.db.query('plugin::upload.file').update({ + await thisStrapi.db.query('plugin::upload.file').update({ where: { hash: uploadData.hash }, data: { url: entry.url,