From 77da80cc5e4451082721d1aa009914b9c174aa5b Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 19 Mar 2021 01:55:32 +0000 Subject: [PATCH 01/12] feat(hub): clean hub command --- src/commands/content-item/archive.spec.ts | 204 +++++++--------- src/commands/content-item/archive.ts | 20 +- src/commands/content-item/unarchive.spec.ts | 199 ++++++--------- src/commands/content-item/unarchive.ts | 44 +++- src/commands/content-type-schema/archive.ts | 5 +- src/commands/content-type/archive.ts | 4 +- src/commands/event/archive.ts | 7 +- src/commands/hub/clean.spec.ts | 227 ++++++++++++++++++ src/commands/hub/clean.ts | 73 ++++++ src/commands/hub/model/clean-hub-step.ts | 8 + .../hub/steps/content-clean-step.spec.ts | 77 ++++++ src/commands/hub/steps/content-clean-step.ts | 25 ++ .../hub/steps/schema-clean-step.spec.ts | 77 ++++++ src/commands/hub/steps/schema-clean-step.ts | 25 ++ .../hub/steps/type-clean-step.spec.ts | 77 ++++++ src/commands/hub/steps/type-clean-step.ts | 25 ++ src/common/archive/archive-options.ts | 4 +- .../content-item/content-dependancy-tree.ts | 2 +- src/interfaces/clean-hub-builder-options.ts | 7 + 19 files changed, 842 insertions(+), 268 deletions(-) create mode 100644 src/commands/hub/clean.spec.ts create mode 100644 src/commands/hub/clean.ts create mode 100644 src/commands/hub/model/clean-hub-step.ts create mode 100644 src/commands/hub/steps/content-clean-step.spec.ts create mode 100644 src/commands/hub/steps/content-clean-step.ts create mode 100644 src/commands/hub/steps/schema-clean-step.spec.ts create mode 100644 src/commands/hub/steps/schema-clean-step.ts create mode 100644 src/commands/hub/steps/type-clean-step.spec.ts create mode 100644 src/commands/hub/steps/type-clean-step.ts create mode 100644 src/interfaces/clean-hub-builder-options.ts diff --git a/src/commands/content-item/archive.spec.ts b/src/commands/content-item/archive.spec.ts index 5d0ba813..661ea91d 100644 --- a/src/commands/content-item/archive.spec.ts +++ b/src/commands/content-item/archive.spec.ts @@ -26,7 +26,6 @@ describe('content-item archive command', () => { clientSecret: 'client-id', hubId: 'hub-id' }; - const archiveMockFunc = jest.fn(); const mockValues = ( archiveError = false @@ -36,17 +35,70 @@ describe('content-item archive command', () => { mockItemsList: () => void; mockArchive: () => void; mockItemGetById: () => void; + mockItemUpdate: () => void; mockRepoGet: () => void; mockFolderGet: () => void; + contentItems: ContentItem[]; } => { const mockGet = jest.fn(); const mockGetList = jest.fn(); const mockItemsList = jest.fn(); const mockArchive = jest.fn(); const mockItemGetById = jest.fn(); + const mockItemUpdate = jest.fn(); const mockRepoGet = jest.fn(); const mockFolderGet = jest.fn(); + const contentItems = [ + new ContentItem({ + id: '1', + label: 'item1', + repoId: 'repo1', + folderId: 'folder1', + status: 'ACTIVE', + body: { + _meta: { + schema: 'http://test.com' + } + }, + client: { + performActionThatReturnsResource: mockArchive, + updateResource: mockItemUpdate + }, + _links: { + archive: { + href: 'https://api.amplience.net/v2/content/content-items/1/archive' + } + } + }), + new ContentItem({ + id: '2', + label: 'item2', + repoId: 'repo1', + folderId: 'folder1', + status: 'ACTIVE', + body: { + _meta: { + schema: 'http://test1.com' + } + }, + client: { + performActionThatReturnsResource: mockArchive, + updateResource: mockItemUpdate + }, + _links: { + archive: { + href: 'https://api.amplience.net/v2/content/content-items/2/archive' + } + } + }) + ]; + + contentItems[0].related.archive = mockArchive; + contentItems[0].related.update = mockItemUpdate; + contentItems[1].related.archive = mockArchive; + contentItems[1].related.update = mockItemUpdate; + (dynamicContentClientFactory as jest.Mock).mockReturnValue({ hubs: { get: mockGet @@ -137,76 +189,13 @@ describe('content-item archive command', () => { }) ); - mockItemGetById.mockResolvedValue( - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { archive: mockArchive }, - client: { - performActionThatReturnsResource: mockArchive - }, - _links: { - archive: { - href: 'https://api.amplience.net/v2/content/content-items/1/archive' - } - } - }) - ); + mockItemGetById.mockResolvedValue(contentItems[0]); - mockItemsList.mockResolvedValue( - new MockPage(ContentItem, [ - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { archive: mockArchive }, - client: { - performActionThatReturnsResource: mockArchive - }, - _links: { - archive: { - href: 'https://api.amplience.net/v2/content/content-items/1/archive' - } - } - }), - new ContentItem({ - id: '2', - label: 'item2', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test1.com' - } - }, - client: { - performActionThatReturnsResource: mockArchive - }, - _links: { - archive: { - href: 'https://api.amplience.net/v2/content/content-items/2/archive' - } - }, - related: { archive: mockArchive } - }) - ]) - ); + mockItemUpdate.mockResolvedValue(contentItems[0]); + + mockArchive.mockResolvedValue(contentItems[0]); + + mockItemsList.mockResolvedValue(new MockPage(ContentItem, contentItems)); if (archiveError) { mockArchive.mockRejectedValue(new Error('Error')); @@ -220,60 +209,13 @@ describe('content-item archive command', () => { mockItemsList, mockArchive, mockItemGetById, + mockItemUpdate, mockRepoGet, - mockFolderGet + mockFolderGet, + contentItems }; }; - const contentItems = [ - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { archive: archiveMockFunc }, - client: { - performActionThatReturnsResource: archiveMockFunc - }, - _links: { - archive: { - href: 'https://api.amplience.net/v2/content/content-items/1/archive' - } - } - }), - new ContentItem({ - id: '2', - label: 'item2', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test1.com' - } - }, - client: { - performActionThatReturnsResource: archiveMockFunc - }, - _links: { - archive: { - href: 'https://api.amplience.net/v2/content/content-items/2/archive' - } - }, - related: { archive: archiveMockFunc } - }) - ]; - - archiveMockFunc.mockResolvedValue({ - message: 'Success' - }); - it('should command should defined', function() { expect(command).toEqual('archive [id]'); }); @@ -775,7 +717,9 @@ describe('content-item archive command', () => { await promisify(unlink)('temp/content-item-archive.log'); } - const { mockItemGetById, mockArchive } = mockValues(); + const { mockItemGetById, mockArchive, contentItems } = mockValues(); + + contentItems[0].body._meta.deliveryKey = 'delivery-key'; const argv = { ...yargArgs, @@ -797,13 +741,19 @@ describe('content-item archive command', () => { const logLines = log.split('\n'); let total = 0; + let deliveryKeys = 0; logLines.forEach(line => { if (line.indexOf('ARCHIVE') !== -1) { total++; } + + if (line.indexOf('delivery-key') !== -1) { + deliveryKeys++; + } }); expect(total).toEqual(1); + expect(deliveryKeys).toEqual(1); await promisify(unlink)('temp/content-item-archive.log'); }); @@ -874,6 +824,8 @@ describe('content-item archive command', () => { describe('filterContentItems tests', () => { it('should filter content items', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems }); @@ -885,6 +837,8 @@ describe('content-item archive command', () => { }); it('should filter content items by content type', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, contentType: '/test.com/' @@ -897,6 +851,8 @@ describe('content-item archive command', () => { }); it('should filter content items by content types', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, contentType: ['/test.com/', '/test1.com/'] @@ -909,6 +865,8 @@ describe('content-item archive command', () => { }); it('should filter content items by name', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, name: ['/item1/'] @@ -924,6 +882,8 @@ describe('content-item archive command', () => { describe('processItems tests', () => { it('should archive content items', async () => { + const { contentItems, mockArchive } = mockValues(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any (readline as any).setResponses(['y']); @@ -934,7 +894,7 @@ describe('content-item archive command', () => { logFile: './logFile.log' }); - expect(archiveMockFunc).toBeCalledTimes(2); + expect(mockArchive).toBeCalledTimes(2); if (await promisify(exists)('./logFile.log')) { await promisify(unlink)('./logFile.log'); diff --git a/src/commands/content-item/archive.ts b/src/commands/content-item/archive.ts index 148454e8..b9392668 100644 --- a/src/commands/content-item/archive.ts +++ b/src/commands/content-item/archive.ts @@ -8,6 +8,7 @@ import ArchiveOptions from '../../common/archive/archive-options'; import { ContentItem, DynamicContent } from 'dc-management-sdk-js'; import { equalsOrRegex } from '../../common/filter/filter'; import { getDefaultLogPath } from '../../common/log-helpers'; +import { FileLog } from '../../common/file-log'; export const command = 'archive [id]'; @@ -226,7 +227,7 @@ export const processItems = async ({ contentItems: ContentItem[]; force?: boolean; silent?: boolean; - logFile?: string; + logFile?: string | FileLog; allContent: boolean; missingContent: boolean; ignoreError?: boolean; @@ -250,15 +251,24 @@ export const processItems = async ({ } const timestamp = Date.now().toString(); - const log = new ArchiveLog(`Content Items Archive Log - ${timestamp}\n`); + const log = typeof logFile === 'object' ? logFile : new ArchiveLog(`Content Items Archive Log - ${timestamp}\n`); let successCount = 0; for (let i = 0; i < contentItems.length; i++) { try { + const deliveryKey = contentItems[i].body._meta.deliveryKey; + let args = contentItems[i].id; + if (deliveryKey) { + contentItems[i].body._meta.deliveryKey = null; + + contentItems[i] = await contentItems[i].related.update(contentItems[i]); + + args += ` ${deliveryKey}`; + } await contentItems[i].related.archive(); - log.addAction('ARCHIVE', `${contentItems[i].id}\n`); + log.addAction('ARCHIVE', `${args}\n`); successCount++; } catch (e) { log.addComment(`ARCHIVE FAILED: ${contentItems[i].id}`); @@ -273,7 +283,7 @@ export const processItems = async ({ } } - if (!silent && logFile) { + if (!silent && typeof logFile === 'string') { await log.writeToFile(logFile.replace('', timestamp)); } @@ -284,7 +294,7 @@ export const handler = async (argv: Arguments { clientSecret: 'client-id', hubId: 'hub-id' }; - const unarchiveMockFunc = jest.fn(); const mockValues = ( unarchiveError = false @@ -43,18 +42,71 @@ describe('content-item unarchive command', () => { mockGetList: () => void; mockItemsList: () => void; mockUnarchive: () => void; + mockItemUpdate: () => void; mockItemGetById: () => void; mockRepoGet: () => void; mockFolderGet: () => void; + contentItems: ContentItem[]; } => { const mockGet = jest.fn(); const mockGetList = jest.fn(); const mockItemsList = jest.fn(); const mockUnarchive = jest.fn(); + const mockItemUpdate = jest.fn(); const mockItemGetById = jest.fn(); const mockRepoGet = jest.fn(); const mockFolderGet = jest.fn(); + const item = new ContentItem({ + id: '1', + label: 'item1', + repoId: 'repo1', + folderId: 'folder1', + status: 'ARCHIVED', + body: { + _meta: { + schema: 'http://test.com' + } + }, + client: { + performActionThatReturnsResource: mockUnarchive + }, + _links: { + unarchive: { + href: 'https://api.amplience.net/v2/content/content-items/1/unarchive' + } + } + }); + + const contentItems = [ + item, + new ContentItem({ + id: '2', + label: 'item2', + repoId: 'repo1', + folderId: 'folder1', + status: 'ARCHIVED', + body: { + _meta: { + schema: 'http://test1.com' + } + }, + client: { + performActionThatReturnsResource: mockUnarchive + }, + _links: { + unarchive: { + href: 'https://api.amplience.net/v2/content/content-items/2/unarchive' + } + } + }) + ]; + + contentItems[0].related.unarchive = mockUnarchive; + contentItems[0].related.update = mockItemUpdate; + contentItems[1].related.unarchive = mockUnarchive; + contentItems[1].related.update = mockItemUpdate; + (dynamicContentClientFactory as jest.Mock).mockReturnValue({ hubs: { get: mockGet @@ -145,76 +197,11 @@ describe('content-item unarchive command', () => { }) ); - mockItemGetById.mockResolvedValue( - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { unarchive: mockUnarchive }, - client: { - performActionThatReturnsResource: mockUnarchive - }, - _links: { - unarchive: { - href: 'https://api.amplience.net/v2/content/content-items/1/unarchive' - } - } - }) - ); + mockItemGetById.mockResolvedValue(item); + mockUnarchive.mockResolvedValue(item); + mockItemUpdate.mockResolvedValue(item); - mockItemsList.mockResolvedValue( - new MockPage(ContentItem, [ - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { unarchive: mockUnarchive }, - client: { - performActionThatReturnsResource: mockUnarchive - }, - _links: { - unarchive: { - href: 'https://api.amplience.net/v2/content/content-items/1/unarchive' - } - } - }), - new ContentItem({ - id: '2', - label: 'item2', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test1.com' - } - }, - client: { - performActionThatReturnsResource: mockUnarchive - }, - _links: { - unarchive: { - href: 'https://api.amplience.net/v2/content/content-items/2/unarchive' - } - }, - related: { unarchive: mockUnarchive } - }) - ]) - ); + mockItemsList.mockResolvedValue(new MockPage(ContentItem, contentItems)); if (unarchiveError) { mockUnarchive.mockRejectedValue(new Error('Error')); @@ -227,61 +214,14 @@ describe('content-item unarchive command', () => { mockGetList, mockItemsList, mockUnarchive, + mockItemUpdate, mockItemGetById, mockRepoGet, - mockFolderGet + mockFolderGet, + contentItems }; }; - const contentItems = [ - new ContentItem({ - id: '1', - label: 'item1', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test.com' - } - }, - related: { unarchive: unarchiveMockFunc }, - client: { - performActionThatReturnsResource: unarchiveMockFunc - }, - _links: { - unarchive: { - href: 'https://api.amplience.net/v2/content/content-items/1/unarchive' - } - } - }), - new ContentItem({ - id: '2', - label: 'item2', - repoId: 'repo1', - folderId: 'folder1', - status: 'ACTIVE', - body: { - _meta: { - schema: 'http://test1.com' - } - }, - client: { - performActionThatReturnsResource: unarchiveMockFunc - }, - _links: { - unarchive: { - href: 'https://api.amplience.net/v2/content/content-items/2/unarchive' - } - }, - related: { unarchive: unarchiveMockFunc } - }) - ]; - - unarchiveMockFunc.mockResolvedValue({ - message: 'Success' - }); - it('should command should defined', function() { expect(command).toEqual('unarchive [id]'); }); @@ -696,7 +636,7 @@ describe('content-item unarchive command', () => { (readline as any).setResponses(['y']); const logFileName = 'temp/content-item-unarchive.log'; - const log = '// Type log test file\n' + 'ARCHIVE 1\n' + 'ARCHIVE 2\n' + 'ARCHIVE idMissing'; + const log = '// Type log test file\n' + 'ARCHIVE 1\n' + 'ARCHIVE 2 delivery-key\n' + 'ARCHIVE idMissing'; const dir = dirname(logFileName); if (!(await promisify(exists)(dir))) { @@ -704,7 +644,7 @@ describe('content-item unarchive command', () => { } await promisify(writeFile)(logFileName, log); - const { mockGet, mockGetList, mockUnarchive, mockItemsList } = mockValues(); + const { mockGet, mockGetList, mockUnarchive, mockItemsList, mockItemUpdate } = mockValues(); const argv = { ...yargArgs, @@ -719,6 +659,9 @@ describe('content-item unarchive command', () => { expect(mockGet).toHaveBeenCalled(); expect(mockGetList).toHaveBeenCalled(); expect(mockItemsList).toHaveBeenCalled(); + expect(mockItemUpdate).toHaveBeenCalled(); + const updateItem: ContentItem = (mockItemUpdate as jest.Mock).mock.calls[0][0]; + expect(updateItem.body._meta.deliveryKey).toEqual('delivery-key'); expect(mockUnarchive).toBeCalledTimes(2); }); @@ -882,6 +825,8 @@ describe('content-item unarchive command', () => { describe('filterContentItems tests', () => { it('should filter content items', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems }); @@ -893,6 +838,8 @@ describe('content-item unarchive command', () => { }); it('should filter content items by content type', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, contentType: '/test.com/' @@ -905,6 +852,8 @@ describe('content-item unarchive command', () => { }); it('should filter content items by content types', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, contentType: ['/test.com/', '/test1.com/'] @@ -917,6 +866,8 @@ describe('content-item unarchive command', () => { }); it('should filter content items by name', async () => { + const { contentItems } = mockValues(); + const result = await filterContentItems({ contentItems, name: ['/item1/'] @@ -932,6 +883,8 @@ describe('content-item unarchive command', () => { describe('processItems tests', () => { it('should unarchive content items', async () => { + const { contentItems, mockUnarchive } = mockValues(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any (readline as any).setResponses(['y']); @@ -942,7 +895,7 @@ describe('content-item unarchive command', () => { logFile: './logFile.log' }); - expect(unarchiveMockFunc).toBeCalledTimes(2); + expect(mockUnarchive).toBeCalledTimes(2); if (await promisify(exists)('./logFile.log')) { await promisify(unlink)('./logFile.log'); diff --git a/src/commands/content-item/unarchive.ts b/src/commands/content-item/unarchive.ts index 28d1a3e1..215fb766 100644 --- a/src/commands/content-item/unarchive.ts +++ b/src/commands/content-item/unarchive.ts @@ -89,18 +89,36 @@ export const filterContentItems = async ({ if (revertLog != null) { const log = await new ArchiveLog().loadFromFile(revertLog); - const ids = log.getData('ARCHIVE'); - const contentItemsFiltered = contentItems.filter(contentItem => ids.indexOf(contentItem.id || '') != -1); - if (contentItems.length != ids.length) { + const archived = log.getData('ARCHIVE'); + const items = archived.map(args => args.split(' ')); + + // The archive actions may include delivery keys that were removed. + // Add these back to the content item, which will cause the unarchive to assign them later. + const contentItemsFiltered = contentItems + .map(contentItem => { + const entry = items.find(item => item[0] === contentItem.id || ''); + if (entry) { + contentItem.body._meta.deliveryKey = entry[1]; + return contentItem; + } else { + return null; + } + }) + .filter(contentItem => !!contentItem); + + if (contentItemsFiltered.length != archived.length) { missingContent = true; } return { - contentItems: contentItemsFiltered, + contentItems: contentItemsFiltered as ContentItem[], missingContent }; } + // Delete the delivery keys, as the unarchive will attempt to reassign them if present. + contentItems.forEach(item => delete item.body._meta.deliveryKey); + if (name != null) { const itemsArray: string[] = Array.isArray(name) ? name : [name]; const contentItemsFiltered = contentItems.filter( @@ -181,14 +199,15 @@ export const getContentItems = async ({ folderId != null ? await Promise.all( folders.map(async source => { - const items = await paginator(source.related.contentItems.list); + const items = await paginator(source.related.contentItems.list, { status: 'ARCHIVED' }); - contentItems.push(...items.filter(item => item.status == 'ACTIVE')); + contentItems.push(...items.filter(item => item.status !== 'ACTIVE')); }) ) : await Promise.all( contentRepositories.map(async source => { - const items = await paginator(source.related.contentItems.list, { status: 'ACTIVE' }); + const items = await paginator(source.related.contentItems.list, { status: 'ARCHIVED' }); + contentItems.push(...items); }) ); @@ -256,7 +275,14 @@ export const processItems = async ({ for (let i = 0; i < contentItems.length; i++) { try { - await contentItems[i].related.unarchive(); + const deliveryKey = contentItems[i].body._meta.deliveryKey; + contentItems[i] = await contentItems[i].related.unarchive(); + + if (contentItems[i].body._meta.deliveryKey != deliveryKey) { + // Restore the delivery key if present. (only on ARCHIVE revert) + contentItems[i].body._meta.deliveryKey = deliveryKey; + await contentItems[i].related.update(contentItems[i]); + } log.addAction('UNARCHIVE', `${contentItems[i].id}\n`); successCount++; @@ -284,7 +310,7 @@ export const handler = async (argv: Arguments', timestamp)); } diff --git a/src/commands/content-type/archive.ts b/src/commands/content-type/archive.ts index 1248b62f..558551f7 100644 --- a/src/commands/content-type/archive.ts +++ b/src/commands/content-type/archive.ts @@ -135,7 +135,7 @@ export const handler = async (argv: Arguments', timestamp)); } diff --git a/src/commands/event/archive.ts b/src/commands/event/archive.ts index a6acb427..dafd8fa0 100644 --- a/src/commands/event/archive.ts +++ b/src/commands/event/archive.ts @@ -8,6 +8,7 @@ import ArchiveOptions from '../../common/archive/archive-options'; import { Edition, Event, DynamicContent } from 'dc-management-sdk-js'; import { equalsOrRegex } from '../../common/filter/filter'; import { getDefaultLogPath } from '../../common/log-helpers'; +import { FileLog } from '../../common/file-log'; const maxAttempts = 30; export const command = 'archive [id]'; @@ -157,7 +158,7 @@ export const processItems = async ({ }[]; force?: boolean; silent?: boolean; - logFile?: string; + logFile?: string | FileLog; missingContent: boolean; ignoreError?: boolean; }): Promise => { @@ -204,7 +205,7 @@ export const processItems = async ({ } const timestamp = Date.now().toString(); - const log = new ArchiveLog(`Events Archive Log - ${timestamp}\n`); + const log = typeof logFile === 'object' ? logFile : new ArchiveLog(`Events Archive Log - ${timestamp}\n`); let successCount = 0; @@ -245,7 +246,7 @@ export const processItems = async ({ } } - if (!silent && logFile) { + if (!silent && typeof logFile === 'string') { await log.writeToFile(logFile.replace('', timestamp)); } diff --git a/src/commands/hub/clean.spec.ts b/src/commands/hub/clean.spec.ts new file mode 100644 index 00000000..d1ecf1d4 --- /dev/null +++ b/src/commands/hub/clean.spec.ts @@ -0,0 +1,227 @@ +import { builder, command, handler, LOG_FILENAME } from './clean'; +import { getDefaultLogPath } from '../../common/log-helpers'; +import Yargs from 'yargs/yargs'; + +import * as content from './steps/content-clean-step'; +import * as schema from './steps/schema-clean-step'; +import * as type from './steps/type-clean-step'; + +import rmdir from 'rimraf'; +import { ConfigurationParameters } from '../configure'; +import { Arguments } from 'yargs'; +import { FileLog } from '../../common/file-log'; +import { CleanHubBuilderOptions } from '../../interfaces/clean-hub-builder-options'; + +jest.mock('readline'); + +jest.mock('../../services/dynamic-content-client-factory'); + +let success = [true, true, true, true]; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function succeedOrFail(mock: any, succeed: () => boolean): jest.Mock { + mock.mockImplementation(() => Promise.resolve(succeed())); + return mock; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function mockStep(name: string, success: () => boolean): any { + return jest.fn().mockImplementation(() => ({ + run: succeedOrFail(jest.fn(), success), + revert: succeedOrFail(jest.fn(), success), + getName: jest.fn().mockReturnValue(name) + })); +} + +jest.mock('./steps/content-clean-step', () => ({ ContentCleanStep: mockStep('Clean Content', () => success[0]) })); +jest.mock('./steps/type-clean-step', () => ({ TypeCleanStep: mockStep('Clean Content Types', () => success[1]) })); +jest.mock('./steps/schema-clean-step', () => ({ + SchemaCleanStep: mockStep('Clean Content Type Schemas', () => success[2]) +})); + +jest.mock('../../common/log-helpers', () => ({ + ...jest.requireActual('../../common/log-helpers'), + getDefaultLogPath: jest.fn() +})); + +function rimraf(dir: string): Promise { + return new Promise((resolve): void => { + rmdir(dir, resolve); + }); +} + +function getMocks(): jest.Mock[] { + return [content.ContentCleanStep as jest.Mock, type.TypeCleanStep as jest.Mock, schema.SchemaCleanStep as jest.Mock]; +} + +function clearMocks(): void { + const mocks = getMocks(); + + mocks.forEach(mock => { + mock.mock.results.forEach(obj => { + const instance = obj.value; + (instance.run as jest.Mock).mockClear(); + (instance.revert as jest.Mock).mockClear(); + }); + }); +} + +describe('hub clean command', () => { + afterEach((): void => { + jest.restoreAllMocks(); + }); + + it('should command should defined', function() { + expect(command).toEqual('clean'); + }); + + it('should use getDefaultLogPath for LOG_FILENAME with process.platform as default', function() { + LOG_FILENAME(); + + expect(getDefaultLogPath).toHaveBeenCalledWith('hub', 'clean', process.platform); + }); + + describe('builder tests', function() { + it('should configure yargs', function() { + const argv = Yargs(process.argv.slice(2)); + const spyPositional = jest.spyOn(argv, 'positional').mockReturnThis(); + const spyOption = jest.spyOn(argv, 'option').mockReturnThis(); + + builder(argv); + + expect(spyPositional).not.toHaveBeenCalled(); + + expect(spyOption).toHaveBeenCalledWith('f', { + type: 'boolean', + boolean: true, + describe: + 'Overwrite content, create and assign content types, and ignore content with missing types/references without asking.' + }); + + expect(spyOption).toHaveBeenCalledWith('logFile', { + type: 'string', + default: LOG_FILENAME, + describe: 'Path to a log file to write to.' + }); + + expect(spyOption).toHaveBeenCalledWith('step', { + type: 'number', + describe: 'Start at a numbered step. 0: Schema, 1: Type, 2: Content' + }); + }); + }); + + describe('handler tests', function() { + const yargArgs = { + $0: 'test', + _: ['test'] + }; + + const config = { + clientId: 'client-id', + clientSecret: 'client-id', + hubId: 'hub-id' + }; + + beforeAll(async () => { + await rimraf('temp/clean/'); + }); + + afterAll(async () => { + await rimraf('temp/clean/'); + }); + + it('should call all steps in order with given parameters', async () => { + clearMocks(); + success = [true, true, true, true]; + + const argv: Arguments = { + ...yargArgs, + ...config, + + logFile: 'temp/clean/steps/all.log', + force: true + }; + + await handler(argv); + + argv.logFile = expect.any(FileLog); + + const mocks = getMocks(); + + mocks.forEach(mock => { + const instance = mock.mock.results[0].value; + + expect(instance.run).toHaveBeenCalledWith(argv); + }); + + const loadLog = new FileLog(); + await loadLog.loadFromFile('temp/clean/steps/all.log'); + }); + + it('should handle false returns from each of the steps by stopping the process', async () => { + for (let i = 0; i < 3; i++) { + clearMocks(); + success = [i != 0, i != 1, i != 2]; + + const argv: Arguments = { + ...yargArgs, + ...config, + + logFile: 'temp/clean/steps/fail' + i + '.log', + force: true + }; + + await handler(argv); + + const mocks = getMocks(); + + mocks.forEach((mock, index) => { + const instance = mock.mock.results[0].value; + + if (index > i) { + expect(instance.run).not.toHaveBeenCalled(); + } else { + expect(instance.run).toHaveBeenCalledWith(argv); + } + }); + + const loadLog = new FileLog(); + await loadLog.loadFromFile('temp/clean/steps/fail' + i + '.log'); + } + }); + + it('should start from the step given as a parameter', async () => { + for (let i = 0; i < 3; i++) { + clearMocks(); + success = [true, true, true]; + + const argv: Arguments = { + ...yargArgs, + ...config, + + step: i, + logFile: 'temp/clean/steps/step' + i + '.log', + force: true + }; + + await handler(argv); + + const mocks = getMocks(); + + mocks.forEach((mock, index) => { + const instance = mock.mock.results[0].value; + + if (index < i) { + expect(instance.run).not.toHaveBeenCalled(); + } else { + expect(instance.run).toHaveBeenCalledWith(argv); + } + }); + + const loadLog = new FileLog(); + await loadLog.loadFromFile('temp/clean/steps/step' + i + '.log'); + } + }); + }); +}); diff --git a/src/commands/hub/clean.ts b/src/commands/hub/clean.ts new file mode 100644 index 00000000..b84ae1d0 --- /dev/null +++ b/src/commands/hub/clean.ts @@ -0,0 +1,73 @@ +import { getDefaultLogPath } from '../../common/log-helpers'; +import { Argv, Arguments } from 'yargs'; +import { ConfigurationParameters } from '../configure'; + +import { FileLog } from '../../common/file-log'; + +import { CleanHubBuilderOptions } from '../../interfaces/clean-hub-builder-options'; +import { SchemaCleanStep } from './steps/schema-clean-step'; +import { TypeCleanStep } from './steps/type-clean-step'; +import { ContentCleanStep } from './steps/content-clean-step'; + +export const command = 'clean'; + +export const desc = + 'Cleans an entire hub. This will archive all content items, types and schemas. Note that these are not fully deleted, but they can be resurrected by a future import.'; + +export const LOG_FILENAME = (platform: string = process.platform): string => + getDefaultLogPath('hub', 'clean', platform); + +export const builder = (yargs: Argv): void => { + yargs + .alias('f', 'force') + .option('f', { + type: 'boolean', + boolean: true, + describe: + 'Overwrite content, create and assign content types, and ignore content with missing types/references without asking.' + }) + + .option('logFile', { + type: 'string', + default: LOG_FILENAME, + describe: 'Path to a log file to write to.' + }) + + .option('step', { + type: 'number', + describe: 'Start at a numbered step. 0: Schema, 1: Type, 2: Content' + }); +}; + +const steps = [new ContentCleanStep(), new TypeCleanStep(), new SchemaCleanStep()]; + +export const handler = async (argv: Arguments): Promise => { + const logFile = argv.logFile; + const log = typeof logFile === 'string' || logFile == null ? new FileLog(logFile) : logFile; + + argv.logFile = log; + + // Steps system: Each step performs another part of the clean command. + // If a step fails, we can return to that step on a future attempt. + + for (let i = argv.step || 0; i < steps.length; i++) { + const step = steps[i]; + + log.appendLine(`=== Running Step ${i} - ${step.getName()} ===`); + + const success = await step.run(argv); + + if (!success) { + log.appendLine(`Step ${i} (${step.getName()}) Failed. Terminating.`); + log.appendLine(''); + log.appendLine('To continue the clean from this point, use the option:'); + log.appendLine(`--step ${i}`); + + break; + } + } + + if (typeof logFile !== 'object') { + await log.close(); + } +}; diff --git a/src/commands/hub/model/clean-hub-step.ts b/src/commands/hub/model/clean-hub-step.ts new file mode 100644 index 00000000..649099f2 --- /dev/null +++ b/src/commands/hub/model/clean-hub-step.ts @@ -0,0 +1,8 @@ +import { Arguments } from 'yargs'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; + +export interface CleanHubStep { + getName(): string; + run(argv: Arguments): Promise; +} diff --git a/src/commands/hub/steps/content-clean-step.spec.ts b/src/commands/hub/steps/content-clean-step.spec.ts new file mode 100644 index 00000000..a0788176 --- /dev/null +++ b/src/commands/hub/steps/content-clean-step.spec.ts @@ -0,0 +1,77 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { join } from 'path'; + +import * as archive from '../../content-item/archive'; + +import { ContentCleanStep } from './content-clean-step'; + +jest.mock('../../../services/dynamic-content-client-factory'); +jest.mock('../../content-item/archive'); + +describe('content clean step', () => { + const yargArgs = { + $0: 'test', + _: ['test'] + }; + + const config = { + clientId: 'client-id', + clientSecret: 'client-id', + hubId: 'hub-id' + }; + + function reset(): void { + jest.resetAllMocks(); + } + + beforeEach(async () => { + reset(); + }); + + function generateState( + directory: string, + logName: string + ): Arguments { + return { + ...yargArgs, + ...config, + + logFile: new FileLog(join(directory, logName + '.log')) + }; + } + + it('should have the name "Clean Content"', () => { + const step = new ContentCleanStep(); + expect(step.getName()).toEqual('Clean Content'); + }); + + it('should call the copy command with arguments from the state', async () => { + const argv = generateState('temp/clean-content/run/', 'run'); + + (archive.handler as jest.Mock).mockResolvedValue(true); + + const step = new ContentCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalledWith({ + ...argv + }); + + expect(result).toBeTruthy(); + }); + + it('should return false when the copy command fails', async () => { + const argv = generateState('temp/clean-content/fail/', 'fail'); + + (archive.handler as jest.Mock).mockRejectedValue(false); + + const step = new ContentCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalled(); + expect(result).toBeFalsy(); + }); +}); diff --git a/src/commands/hub/steps/content-clean-step.ts b/src/commands/hub/steps/content-clean-step.ts new file mode 100644 index 00000000..30bc7bb6 --- /dev/null +++ b/src/commands/hub/steps/content-clean-step.ts @@ -0,0 +1,25 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { handler } from '../../content-item/archive'; +import { CleanHubStep } from '../model/clean-hub-step'; + +export class ContentCleanStep implements CleanHubStep { + getName(): string { + return 'Clean Content'; + } + + async run(argv: Arguments): Promise { + try { + await handler({ + ...argv + }); + } catch (e) { + (argv.logFile as FileLog).appendLine(`ERROR: Could not archive content. \n${e}`); + return false; + } + + return true; + } +} diff --git a/src/commands/hub/steps/schema-clean-step.spec.ts b/src/commands/hub/steps/schema-clean-step.spec.ts new file mode 100644 index 00000000..1766a6f7 --- /dev/null +++ b/src/commands/hub/steps/schema-clean-step.spec.ts @@ -0,0 +1,77 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { join } from 'path'; + +import * as archive from '../../content-type-schema/archive'; + +import { SchemaCleanStep } from './schema-clean-step'; + +jest.mock('../../../services/dynamic-content-client-factory'); +jest.mock('../../content-type-schema/archive'); + +describe('schema clean step', () => { + const yargArgs = { + $0: 'test', + _: ['test'] + }; + + const config = { + clientId: 'client-id', + clientSecret: 'client-id', + hubId: 'hub-id' + }; + + function reset(): void { + jest.resetAllMocks(); + } + + beforeEach(async () => { + reset(); + }); + + function generateState( + directory: string, + logName: string + ): Arguments { + return { + ...yargArgs, + ...config, + + logFile: new FileLog(join(directory, logName + '.log')) + }; + } + + it('should have the name "Clean Content Type Schemas"', () => { + const step = new SchemaCleanStep(); + expect(step.getName()).toEqual('Clean Content Type Schemas'); + }); + + it('should call the copy command with arguments from the state', async () => { + const argv = generateState('temp/clean-schema/run/', 'run'); + + (archive.handler as jest.Mock).mockResolvedValue(true); + + const step = new SchemaCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalledWith({ + ...argv + }); + + expect(result).toBeTruthy(); + }); + + it('should return false when the copy command fails', async () => { + const argv = generateState('temp/clean-schema/fail/', 'fail'); + + (archive.handler as jest.Mock).mockRejectedValue(false); + + const step = new SchemaCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalled(); + expect(result).toBeFalsy(); + }); +}); diff --git a/src/commands/hub/steps/schema-clean-step.ts b/src/commands/hub/steps/schema-clean-step.ts new file mode 100644 index 00000000..bf341026 --- /dev/null +++ b/src/commands/hub/steps/schema-clean-step.ts @@ -0,0 +1,25 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { handler } from '../../content-type-schema/archive'; +import { CleanHubStep } from '../model/clean-hub-step'; + +export class SchemaCleanStep implements CleanHubStep { + getName(): string { + return 'Clean Content Type Schemas'; + } + + async run(argv: Arguments): Promise { + try { + await handler({ + ...argv + }); + } catch (e) { + (argv.logFile as FileLog).appendLine(`ERROR: Could not archive schemas. \n${e}`); + return false; + } + + return true; + } +} diff --git a/src/commands/hub/steps/type-clean-step.spec.ts b/src/commands/hub/steps/type-clean-step.spec.ts new file mode 100644 index 00000000..07c4e5a6 --- /dev/null +++ b/src/commands/hub/steps/type-clean-step.spec.ts @@ -0,0 +1,77 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { join } from 'path'; + +import * as archive from '../../content-type/archive'; + +import { TypeCleanStep } from './type-clean-step'; + +jest.mock('../../../services/dynamic-content-client-factory'); +jest.mock('../../content-type/archive'); + +describe('type clean step', () => { + const yargArgs = { + $0: 'test', + _: ['test'] + }; + + const config = { + clientId: 'client-id', + clientSecret: 'client-id', + hubId: 'hub-id' + }; + + function reset(): void { + jest.resetAllMocks(); + } + + beforeEach(async () => { + reset(); + }); + + function generateState( + directory: string, + logName: string + ): Arguments { + return { + ...yargArgs, + ...config, + + logFile: new FileLog(join(directory, logName + '.log')) + }; + } + + it('should have the name "Clean Content Types"', () => { + const step = new TypeCleanStep(); + expect(step.getName()).toEqual('Clean Content Types'); + }); + + it('should call the copy command with arguments from the state', async () => { + const argv = generateState('temp/clean-schema/run/', 'run'); + + (archive.handler as jest.Mock).mockResolvedValue(true); + + const step = new TypeCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalledWith({ + ...argv + }); + + expect(result).toBeTruthy(); + }); + + it('should return false when the copy command fails', async () => { + const argv = generateState('temp/clean-schema/fail/', 'fail'); + + (archive.handler as jest.Mock).mockRejectedValue(false); + + const step = new TypeCleanStep(); + const result = await step.run(argv); + + expect(archive.handler).toHaveBeenCalled(); + expect(result).toBeFalsy(); + }); +}); diff --git a/src/commands/hub/steps/type-clean-step.ts b/src/commands/hub/steps/type-clean-step.ts new file mode 100644 index 00000000..54aeb692 --- /dev/null +++ b/src/commands/hub/steps/type-clean-step.ts @@ -0,0 +1,25 @@ +import { Arguments } from 'yargs'; +import { FileLog } from '../../../common/file-log'; +import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; +import { ConfigurationParameters } from '../../configure'; +import { handler } from '../../content-type/archive'; +import { CleanHubStep } from '../model/clean-hub-step'; + +export class TypeCleanStep implements CleanHubStep { + getName(): string { + return 'Clean Content Types'; + } + + async run(argv: Arguments): Promise { + try { + await handler({ + ...argv + }); + } catch (e) { + (argv.logFile as FileLog).appendLine(`ERROR: Could not archive types. \n${e}`); + return false; + } + + return true; + } +} diff --git a/src/common/archive/archive-options.ts b/src/common/archive/archive-options.ts index 03e119fc..f1a4d0e5 100644 --- a/src/common/archive/archive-options.ts +++ b/src/common/archive/archive-options.ts @@ -1,3 +1,5 @@ +import { FileLog } from '../file-log'; + export default interface ArchiveOptions { id?: string; schemaId?: string | string[]; @@ -6,7 +8,7 @@ export default interface ArchiveOptions { folderId?: string | string[]; name?: string | string[]; contentType?: string | string[]; - logFile?: string; + logFile?: string | FileLog; force?: boolean; silent?: boolean; ignoreError?: boolean; diff --git a/src/common/content-item/content-dependancy-tree.ts b/src/common/content-item/content-dependancy-tree.ts index b0210021..62ee583b 100644 --- a/src/common/content-item/content-dependancy-tree.ts +++ b/src/common/content-item/content-dependancy-tree.ts @@ -112,7 +112,7 @@ export class ContentDependancyTree { body.forEach(contained => { this.searchObjectForContentDependancies(item, contained, result); }); - } else { + } else if (body != null) { const allPropertyNames = Object.getOwnPropertyNames(body); // Does this object match the pattern expected for a content item or reference? if ( diff --git a/src/interfaces/clean-hub-builder-options.ts b/src/interfaces/clean-hub-builder-options.ts new file mode 100644 index 00000000..cd75c0e6 --- /dev/null +++ b/src/interfaces/clean-hub-builder-options.ts @@ -0,0 +1,7 @@ +import { FileLog } from '../common/file-log'; + +export interface CleanHubBuilderOptions { + logFile?: string | FileLog; + force?: boolean; + step?: number; +} From cf8c704c1cbd0cdba621a4a2a4bf09b70df25fa7 Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 19 Mar 2021 09:42:16 +0000 Subject: [PATCH 02/12] feat(archive-log): add log group changes from clone --- src/commands/hub/clean.ts | 1 + src/common/archive/archive-log.ts | 59 ++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/commands/hub/clean.ts b/src/commands/hub/clean.ts index b84ae1d0..d4d71643 100644 --- a/src/commands/hub/clean.ts +++ b/src/commands/hub/clean.ts @@ -53,6 +53,7 @@ export const handler = async (argv: Arguments = new Map([['_default', []]]); - constructor(public title?: string) {} + public accessGroup: ArchiveLogItem[]; + + constructor(public title?: string) { + this.accessGroup = this.items.get('_default') as ArchiveLogItem[]; + } async loadFromFile(path: string): Promise { const log = await promisify(readFile)(path, 'utf8'); const logLines = log.split('\n'); - this.items = []; + + this.switchGroup('_default'); logLines.forEach((line, index) => { if (line.startsWith('//')) { // The first comment is the title, all ones after it should be recorded as comment items. @@ -35,6 +40,10 @@ export class ArchiveLog { this.addComment(message); } return; + } else if (line.startsWith('> ')) { + // Group start. End the active group and start building another. + this.switchGroup(line.substring(2)); + return; } if (index === logLines.length - 1) { @@ -46,6 +55,8 @@ export class ArchiveLog { } } }); + + this.switchGroup('_default'); return this; } @@ -74,12 +85,18 @@ export class ArchiveLog { async writeToFile(path: string): Promise { try { let log = `// ${this.title}\n`; - this.items.forEach(item => { - if (item.comment) { - log += `// ${item.data}\n`; - } else { - log += `${item.action} ${item.data}\n`; + this.items.forEach((group, groupName) => { + if (groupName !== '_default') { + log += `> ${groupName}\n`; } + + group.forEach(item => { + if (item.comment) { + log += `// ${item.data}\n`; + } else { + log += `${item.action} ${item.data}\n`; + } + }); }); log += this.getResultCode(); @@ -123,18 +140,36 @@ export class ArchiveLog { this.addError(LogErrorLevel.ERROR, message, error); } + switchGroup(group: string): void { + let targetGroup = this.items.get(group); + + if (!targetGroup) { + targetGroup = []; + + this.items.set(group, targetGroup); + } + + this.accessGroup = targetGroup; + } + addComment(comment: string): void { const lines = comment.split('\n'); lines.forEach(line => { - this.items.push({ comment: true, data: line }); + this.accessGroup.push({ comment: true, data: line }); }); } addAction(action: string, data: string): void { - this.items.push({ comment: false, action: action, data: data }); + this.accessGroup.push({ comment: false, action: action, data: data }); } - getData(action: string): string[] { - return this.items.filter(item => !item.comment && item.action === action).map(item => item.data); + getData(action: string, group = '_default'): string[] { + const items = this.items.get(group); + + if (!items) { + throw new Error(`Group ${group} was missing from the log file.`); + } + + return items.filter(item => !item.comment && item.action === action).map(item => item.data); } } From 4bf31ddde962d07725299e0b3eddc110bdc2203d Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 19 Mar 2021 11:52:42 +0000 Subject: [PATCH 03/12] fix(hub): add missing hub command --- src/__snapshots__/cli.spec.ts.snap | 1 + src/commands/hub.ts | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 src/commands/hub.ts diff --git a/src/__snapshots__/cli.spec.ts.snap b/src/__snapshots__/cli.spec.ts.snap index e41e9ace..3001fdf2 100644 --- a/src/__snapshots__/cli.spec.ts.snap +++ b/src/__snapshots__/cli.spec.ts.snap @@ -10,6 +10,7 @@ Commands: dc-cli content-type-schema Content Type Schema dc-cli content-type Content Type dc-cli event Event + dc-cli hub Hub dc-cli settings Settings Options: diff --git a/src/commands/hub.ts b/src/commands/hub.ts new file mode 100644 index 00000000..3a0f8272 --- /dev/null +++ b/src/commands/hub.ts @@ -0,0 +1,12 @@ +import { Argv } from 'yargs'; +import YargsCommandBuilderOptions from '../common/yargs/yargs-command-builder-options'; + +export const command = 'hub'; + +export const desc = 'Hub'; + +export const builder = (yargs: Argv): Argv => + yargs + .commandDir('hub', YargsCommandBuilderOptions) + .demandCommand() + .help(); From 6b95a9a731815a8aa85b3dde546c21971032b3e5 Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 19 Mar 2021 12:43:21 +0000 Subject: [PATCH 04/12] fix(archive): remove unnecessary new lines after ARCHIVE actions --- src/commands/content-item/archive.ts | 2 +- src/commands/content-type-schema/archive.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/content-item/archive.ts b/src/commands/content-item/archive.ts index b9392668..390113f0 100644 --- a/src/commands/content-item/archive.ts +++ b/src/commands/content-item/archive.ts @@ -268,7 +268,7 @@ export const processItems = async ({ } await contentItems[i].related.archive(); - log.addAction('ARCHIVE', `${args}\n`); + log.addAction('ARCHIVE', `${args}`); successCount++; } catch (e) { log.addComment(`ARCHIVE FAILED: ${contentItems[i].id}`); diff --git a/src/commands/content-type-schema/archive.ts b/src/commands/content-type-schema/archive.ts index 699720d6..c8974bd0 100644 --- a/src/commands/content-type-schema/archive.ts +++ b/src/commands/content-type-schema/archive.ts @@ -141,7 +141,7 @@ export const handler = async (argv: Arguments Date: Wed, 14 Apr 2021 12:59:02 +0100 Subject: [PATCH 05/12] refactor: address feedback --- src/commands/content-item/archive.ts | 2 +- src/commands/content-item/unarchive.ts | 2 +- src/commands/content-type-schema/archive.ts | 4 ++-- src/commands/content-type/archive.ts | 3 ++- src/commands/event/archive.ts | 2 +- src/commands/hub/clean.ts | 4 ++-- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/commands/content-item/archive.ts b/src/commands/content-item/archive.ts index 390113f0..3f389686 100644 --- a/src/commands/content-item/archive.ts +++ b/src/commands/content-item/archive.ts @@ -251,7 +251,7 @@ export const processItems = async ({ } const timestamp = Date.now().toString(); - const log = typeof logFile === 'object' ? logFile : new ArchiveLog(`Content Items Archive Log - ${timestamp}\n`); + const log = logFile instanceof FileLog ? logFile : new ArchiveLog(`Content Items Archive Log - ${timestamp}\n`); let successCount = 0; diff --git a/src/commands/content-item/unarchive.ts b/src/commands/content-item/unarchive.ts index 215fb766..aacf23ce 100644 --- a/src/commands/content-item/unarchive.ts +++ b/src/commands/content-item/unarchive.ts @@ -201,7 +201,7 @@ export const getContentItems = async ({ folders.map(async source => { const items = await paginator(source.related.contentItems.list, { status: 'ARCHIVED' }); - contentItems.push(...items.filter(item => item.status !== 'ACTIVE')); + contentItems.push(...items); }) ) : await Promise.all( diff --git a/src/commands/content-type-schema/archive.ts b/src/commands/content-type-schema/archive.ts index c8974bd0..aac28731 100644 --- a/src/commands/content-type-schema/archive.ts +++ b/src/commands/content-type-schema/archive.ts @@ -8,6 +8,7 @@ import { equalsOrRegex } from '../../common/filter/filter'; import { confirmArchive } from '../../common/archive/archive-helpers'; import ArchiveOptions from '../../common/archive/archive-options'; import { getDefaultLogPath } from '../../common/log-helpers'; +import { FileLog } from '../../common/file-log'; export const command = 'archive [id]'; @@ -132,8 +133,7 @@ export const handler = async (argv: Arguments): Promise => { const logFile = argv.logFile; - const log = typeof logFile === 'string' || logFile == null ? new FileLog(logFile) : logFile; + const log = logFile instanceof FileLog ? logFile : new FileLog(logFile); argv.logFile = log; @@ -68,7 +68,7 @@ export const handler = async (argv: Arguments Date: Wed, 12 May 2021 12:23:22 +0100 Subject: [PATCH 06/12] test(content-item): add test for content dependency tree null property bug fix --- .../content-dependancy-tree.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/common/content-item/content-dependancy-tree.spec.ts b/src/common/content-item/content-dependancy-tree.spec.ts index b6e4792c..2be3371b 100644 --- a/src/common/content-item/content-dependancy-tree.spec.ts +++ b/src/common/content-item/content-dependancy-tree.spec.ts @@ -221,6 +221,25 @@ describe('content-dependancy-tree', () => { } }); + it('should correctly handle content item bodies with null properties or array items', () => { + const bodyWithNullLeaves = dependsOn(['id1']); + bodyWithNullLeaves.links.push(null); + bodyWithNullLeaves.nullProperty = null; + + const items = createItems([ + { label: 'id1', body: dependsOn([]), repoId: 'repo1', typeSchemaUri: 'http://type.com' }, + { label: 'id2', body: bodyWithNullLeaves, repoId: 'repo1', typeSchemaUri: 'http://type.com' } + ]); + + const tree = new ContentDependancyTree(items, new ContentMapping()); + + expect(tree.levels.length).toEqual(2); + expect(tree.circularLinks.length).toEqual(0); + expect(tree.all.length).toEqual(2); + expect(tree.byId.size).toEqual(2); + expect(tree.requiredSchema.length).toEqual(1); + }); + it('should successfully remove content dependancies', () => { const items = defaultDependantItems(); From 17f1ff78f690a531e02b2fa290b97cbf4021b0f3 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 19 May 2021 16:34:42 +0100 Subject: [PATCH 07/12] refactor: create FileLog from yargs, remove union type in arguments --- src/commands/content-item/archive.spec.ts | 46 +++++++++++--- src/commands/content-item/archive.ts | 16 ++--- src/commands/content-item/copy.spec.ts | 12 ++-- src/commands/content-item/copy.ts | 13 ++-- src/commands/content-item/move.spec.ts | 24 ++++---- src/commands/content-item/move.ts | 9 +-- .../content-type-schema/archive.spec.ts | 60 +++++++++++++------ src/commands/content-type-schema/archive.ts | 14 ++--- src/commands/content-type/archive.spec.ts | 47 +++++++++------ src/commands/content-type/archive.ts | 15 +++-- src/commands/event/archive.spec.ts | 20 +++++-- src/commands/event/archive.ts | 17 +++--- src/commands/hub/clean.spec.ts | 11 ++-- src/commands/hub/clean.ts | 14 ++--- src/common/archive/archive-options.ts | 2 +- src/common/file-log.spec.ts | 32 +++++++++- src/common/file-log.ts | 19 ++++-- src/common/log-helpers.ts | 13 ++++ src/interfaces/clean-hub-builder-options.ts | 2 +- .../copy-item-builder-options.interface.ts | 2 +- .../export-item-builder-options.interface.ts | 2 +- .../import-item-builder-options.interface.ts | 2 +- 22 files changed, 254 insertions(+), 138 deletions(-) diff --git a/src/commands/content-item/archive.spec.ts b/src/commands/content-item/archive.spec.ts index 661ea91d..5cf876ab 100644 --- a/src/commands/content-item/archive.spec.ts +++ b/src/commands/content-item/archive.spec.ts @@ -1,4 +1,13 @@ -import { builder, command, handler, LOG_FILENAME, filterContentItems, getContentItems, processItems } from './archive'; +import { + builder, + command, + handler, + LOG_FILENAME, + filterContentItems, + getContentItems, + processItems, + coerceLog +} from './archive'; import dynamicContentClientFactory from '../../services/dynamic-content-client-factory'; import { ContentRepository, ContentItem, Folder } from 'dc-management-sdk-js'; import Yargs from 'yargs/yargs'; @@ -7,11 +16,18 @@ import MockPage from '../../common/dc-management-sdk-js/mock-page'; import { dirname } from 'path'; import { promisify } from 'util'; import { exists, readFile, unlink, mkdir, writeFile } from 'fs'; +import { FileLog } from '../../common/file-log'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; jest.mock('readline'); jest.mock('../../services/dynamic-content-client-factory'); +jest.mock('../../common/log-helpers', () => ({ + ...jest.requireActual('../../common/log-helpers'), + getDefaultLogPath: jest.fn() +})); + describe('content-item archive command', () => { afterEach((): void => { jest.restoreAllMocks(); @@ -24,7 +40,8 @@ describe('content-item archive command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; const mockValues = ( @@ -286,12 +303,26 @@ describe('content-item archive command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }); }); describe('handler tests', function() { + it('should use getDefaultLogPath for LOG_FILENAME with process.platform as default', function() { + LOG_FILENAME(); + + expect(getDefaultLogPath).toHaveBeenCalledWith('content-item', 'archive', process.platform); + }); + + it('should generate a log with coerceLog with the appropriate title', function() { + const logFile = coerceLog('filename.log'); + + expect(logFile).toEqual(expect.any(FileLog)); + expect(logFile.title).toMatch(/^Content Items Archive Log \- ./); + }); + it('should archive all content', async () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any (readline as any).setResponses(['y']); @@ -643,7 +674,6 @@ describe('content-item archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), silent: true, force: true, revertLog: logFileName @@ -696,7 +726,6 @@ describe('content-item archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), silent: true, force: true, revertLog: 'wrongFileName.log' @@ -724,7 +753,7 @@ describe('content-item archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/content-item-archive.log', + logFile: createLog('temp/content-item-archive.log'), id: '1' }; @@ -891,7 +920,7 @@ describe('content-item archive command', () => { contentItems, allContent: true, missingContent: false, - logFile: './logFile.log' + logFile: createLog('./logFile.log') }); expect(mockArchive).toBeCalledTimes(2); @@ -907,7 +936,8 @@ describe('content-item archive command', () => { await processItems({ contentItems: [], allContent: true, - missingContent: false + missingContent: false, + logFile: new FileLog() }); expect(console.log).toBeCalled(); diff --git a/src/commands/content-item/archive.ts b/src/commands/content-item/archive.ts index 3f389686..20432e8c 100644 --- a/src/commands/content-item/archive.ts +++ b/src/commands/content-item/archive.ts @@ -7,7 +7,7 @@ import { confirmArchive } from '../../common/archive/archive-helpers'; import ArchiveOptions from '../../common/archive/archive-options'; import { ContentItem, DynamicContent } from 'dc-management-sdk-js'; import { equalsOrRegex } from '../../common/filter/filter'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { getDefaultLogPath, createLog } from '../../common/log-helpers'; import { FileLog } from '../../common/file-log'; export const command = 'archive [id]'; @@ -17,6 +17,8 @@ export const desc = 'Archive Content Items'; export const LOG_FILENAME = (platform: string = process.platform): string => getDefaultLogPath('content-item', 'archive', platform); +export const coerceLog = (logFile: string): FileLog => createLog(logFile, 'Content Items Archive Log'); + export const builder = (yargs: Argv): void => { yargs .positional('id', { @@ -70,7 +72,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }; @@ -227,7 +230,7 @@ export const processItems = async ({ contentItems: ContentItem[]; force?: boolean; silent?: boolean; - logFile?: string | FileLog; + logFile: FileLog; allContent: boolean; missingContent: boolean; ignoreError?: boolean; @@ -250,8 +253,7 @@ export const processItems = async ({ } } - const timestamp = Date.now().toString(); - const log = logFile instanceof FileLog ? logFile : new ArchiveLog(`Content Items Archive Log - ${timestamp}\n`); + const log = logFile.open(); let successCount = 0; @@ -283,9 +285,7 @@ export const processItems = async ({ } } - if (!silent && typeof logFile === 'string') { - await log.writeToFile(logFile.replace('', timestamp)); - } + await log.close(!silent); console.log(`Archived ${successCount} content items.`); }; diff --git a/src/commands/content-item/copy.spec.ts b/src/commands/content-item/copy.spec.ts index 40530386..85b85b0e 100644 --- a/src/commands/content-item/copy.spec.ts +++ b/src/commands/content-item/copy.spec.ts @@ -13,7 +13,7 @@ import { Arguments } from 'yargs'; import { ExportItemBuilderOptions } from '../../interfaces/export-item-builder-options.interface'; import { ConfigurationParameters } from '../configure'; import { ImportItemBuilderOptions } from '../../interfaces/import-item-builder-options.interface'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import * as copyConfig from '../../common/content-item/copy-config'; import { FileLog } from '../../common/file-log'; @@ -157,7 +157,8 @@ describe('content-item copy command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }); }); }); @@ -171,7 +172,8 @@ describe('content-item copy command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; beforeAll(async () => { @@ -431,7 +433,7 @@ describe('content-item copy command', () => { expect(importCalls[0].skipIncomplete).toEqual(argv.skipIncomplete); }); - it('should not close the log if provided as part of the arguments', async () => { + it('should not close the log if previously opened', async () => { const copyConfig = { srcHubId: 'hub2-id', srcClientId: 'acc2-id', @@ -442,7 +444,7 @@ describe('content-item copy command', () => { dstSecret: 'acc2-secret' }; - const log = new FileLog(); + const log = new FileLog().open(); const argv = { ...yargArgs, ...config, diff --git a/src/commands/content-item/copy.ts b/src/commands/content-item/copy.ts index f798432d..5b967546 100644 --- a/src/commands/content-item/copy.ts +++ b/src/commands/content-item/copy.ts @@ -1,4 +1,4 @@ -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { Argv, Arguments } from 'yargs'; import { join } from 'path'; import { CopyItemBuilderOptions } from '../../interfaces/copy-item-builder-options.interface'; @@ -8,7 +8,6 @@ import rmdir from 'rimraf'; import { handler as exporter } from './export'; import { handler as importer } from './import'; import { ensureDirectoryExists } from '../../common/import/directory-utils'; -import { FileLog } from '../../common/file-log'; import { revert } from './import-revert'; import { loadCopyConfig } from '../../common/content-item/copy-config'; @@ -137,7 +136,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }); }; @@ -148,8 +148,7 @@ function rimraf(dir: string): Promise { } export const handler = async (argv: Arguments): Promise => { - const logFile = argv.logFile; - const log = typeof logFile === 'string' || logFile == null ? new FileLog(logFile) : logFile; + const log = argv.logFile.open(); const tempFolder = getTempFolder(Date.now().toString()); const yargArgs = { @@ -241,9 +240,7 @@ export const handler = async (argv: Arguments ({ + ...jest.requireActual('../../common/log-helpers'), + getDefaultLogPath: jest.fn() +})); // eslint-disable-next-line @typescript-eslint/no-explicit-any const copierAny = copier as any; @@ -138,7 +142,8 @@ describe('content-item move command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }); }); }); @@ -152,7 +157,8 @@ describe('content-item move command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; beforeAll(async () => { @@ -417,9 +423,7 @@ describe('content-item move command', () => { dstHubId: 'hub2-id', dstClientId: 'acc2-id', - dstSecret: 'acc2-secret', - - logFile: LOG_FILENAME() + dstSecret: 'acc2-secret' }; await handler(argv); @@ -466,7 +470,7 @@ describe('content-item move command', () => { validate: false, skipIncomplete: false, - logFile: 'temp/move/failLog.txt' + logFile: createFileLog('temp/move/failLog.txt') }; await handler(argv); @@ -489,9 +493,7 @@ describe('content-item move command', () => { dstHubId: 'hub2-id', dstClientId: 'acc2-id', - dstSecret: 'acc2-secret', - - logFile: LOG_FILENAME() + dstSecret: 'acc2-secret' }; await handler(argv); diff --git a/src/commands/content-item/move.ts b/src/commands/content-item/move.ts index 5e73884e..8616ba26 100644 --- a/src/commands/content-item/move.ts +++ b/src/commands/content-item/move.ts @@ -1,4 +1,4 @@ -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { Argv, Arguments } from 'yargs'; import { CopyItemBuilderOptions } from '../../interfaces/copy-item-builder-options.interface'; import { ConfigurationParameters } from '../configure'; @@ -132,7 +132,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }); }; @@ -204,7 +205,7 @@ export const handler = async (argv: Arguments ({ + ...jest.requireActual('../../common/log-helpers'), + getDefaultLogPath: jest.fn() +})); + describe('content-item-schema archive command', () => { afterEach((): void => { jest.restoreAllMocks(); @@ -69,9 +76,23 @@ describe('content-item-schema archive command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }); + + it('should use getDefaultLogPath for LOG_FILENAME with process.platform as default', function() { + LOG_FILENAME(); + + expect(getDefaultLogPath).toHaveBeenCalledWith('schema', 'archive', process.platform); + }); + + it('should generate a log with coerceLog with the appropriate title', function() { + const logFile = coerceLog('filename.log'); + + expect(logFile).toEqual(expect.any(FileLog)); + expect(logFile.title).toMatch(/^Content Type Schema Archive Log \- ./); + }); }); describe('handler tests', function() { @@ -83,7 +104,8 @@ describe('content-item-schema archive command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; function generateMockSchemaList( @@ -150,7 +172,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + schemaId: 'http://schemas.com/schema2', silent: true }; @@ -182,7 +204,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + schemaId: 'http://schemas.com/schema2', silent: true }; @@ -214,7 +236,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + schemaId: 'http://schemas.com/schema2', silent: true, force: true @@ -254,7 +276,7 @@ describe('content-item-schema archive command', () => { ...yargArgs, id: 'content-type-schema-id', ...config, - logFile: LOG_FILENAME(), + force: true, silent: true }; @@ -279,7 +301,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + schemaId: 'http://schemas.com/schema2', force: true, silent: true @@ -314,7 +336,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + schemaId: ['/schemaMatch/'], // Pass as an array to cover that case too. force: true, silent: true @@ -344,7 +366,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true }; @@ -390,7 +412,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + slient: true, force: true, revertLog: logFileName @@ -429,7 +451,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/schema-archive-test.log', + logFile: createLog('temp/schema-archive-test.log'), schemaId: '/schemaMatch/', force: true }; @@ -487,7 +509,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/schema-archive-failed.log', + logFile: createLog('temp/schema-archive-failed.log'), schemaId: '/schemaMatch/', force: true }; @@ -541,7 +563,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/schema-archive-skip.log', + logFile: createLog('temp/schema-archive-skip.log'), schemaId: '/schemaMatch/', ignoreError: true, force: true @@ -575,7 +597,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true }; @@ -586,7 +608,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true, revertLog: 'doesntExist.txt' @@ -612,7 +634,7 @@ describe('content-item-schema archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true }; @@ -622,7 +644,7 @@ describe('content-item-schema archive command', () => { const argv2 = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true, id: 'test' @@ -633,7 +655,7 @@ describe('content-item-schema archive command', () => { const argv3 = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), + force: true, silent: true, id: 'test', diff --git a/src/commands/content-type-schema/archive.ts b/src/commands/content-type-schema/archive.ts index aac28731..0583b2ab 100644 --- a/src/commands/content-type-schema/archive.ts +++ b/src/commands/content-type-schema/archive.ts @@ -7,7 +7,7 @@ import paginator from '../../common/dc-management-sdk-js/paginator'; import { equalsOrRegex } from '../../common/filter/filter'; import { confirmArchive } from '../../common/archive/archive-helpers'; import ArchiveOptions from '../../common/archive/archive-options'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { FileLog } from '../../common/file-log'; export const command = 'archive [id]'; @@ -17,6 +17,8 @@ export const desc = 'Archive Content Type Schemas'; export const LOG_FILENAME = (platform: string = process.platform): string => getDefaultLogPath('schema', 'archive', platform); +export const coerceLog = (logFile: string): FileLog => createLog(logFile, 'Content Type Schema Archive Log'); + export const builder = (yargs: Argv): void => { yargs .positional('id', { @@ -55,7 +57,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }; @@ -132,8 +135,7 @@ export const handler = async (argv: Arguments', timestamp)); - } + await log.close(!silent); console.log(`Archived ${successCount} content type schemas.`); }; diff --git a/src/commands/content-type/archive.spec.ts b/src/commands/content-type/archive.spec.ts index 5014a2a0..aecb6893 100644 --- a/src/commands/content-type/archive.spec.ts +++ b/src/commands/content-type/archive.spec.ts @@ -1,4 +1,4 @@ -import { builder, command, handler, LOG_FILENAME } from './archive'; +import { builder, coerceLog, command, handler, LOG_FILENAME } from './archive'; import dynamicContentClientFactory from '../../services/dynamic-content-client-factory'; import { ContentType, Hub } from 'dc-management-sdk-js'; import Yargs from 'yargs/yargs'; @@ -7,11 +7,18 @@ import { exists, readFile, unlink, mkdir, writeFile } from 'fs'; import { dirname } from 'path'; import { promisify } from 'util'; import readline from 'readline'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; +import { FileLog } from '../../common/file-log'; jest.mock('readline'); jest.mock('../../services/dynamic-content-client-factory'); +jest.mock('../../common/log-helpers', () => ({ + ...jest.requireActual('../../common/log-helpers'), + getDefaultLogPath: jest.fn() +})); + describe('content-type archive command', () => { afterEach((): void => { jest.restoreAllMocks(); @@ -69,9 +76,23 @@ describe('content-type archive command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }); + + it('should use getDefaultLogPath for LOG_FILENAME with process.platform as default', function() { + LOG_FILENAME(); + + expect(getDefaultLogPath).toHaveBeenCalledWith('type', 'archive', process.platform); + }); + + it('should generate a log with coerceLog with the appropriate title', function() { + const logFile = coerceLog('filename.log'); + + expect(logFile).toEqual(expect.any(FileLog)); + expect(logFile.title).toMatch(/^Content Type Archive Log \- ./); + }); }); describe('handler tests', function() { @@ -83,7 +104,8 @@ describe('content-type archive command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; function generateMockTypeList( @@ -161,7 +183,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), schemaId: 'http://schemas.com/schema2', silent: true }; @@ -199,7 +220,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), schemaId: 'http://schemas.com/schema2', silent: true }; @@ -237,7 +257,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), schemaId: 'http://schemas.com/schema2', silent: true, force: true @@ -277,7 +296,6 @@ describe('content-type archive command', () => { ...yargArgs, id: 'content-type-id', ...config, - logFile: LOG_FILENAME(), force: true, silent: true }; @@ -308,7 +326,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), schemaId: 'http://schemas.com/schema2', force: true, silent: true @@ -343,7 +360,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), schemaId: ['/schemaMatch/'], // Pass as an array to cover that case too. force: true, silent: true @@ -373,7 +389,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true }; @@ -415,7 +430,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), slient: true, force: true, revertLog: logFileName @@ -454,7 +468,7 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/type-archive-test.log', + logFile: createLog('temp/type-archive-test.log'), schemaId: '/schemaMatch/', force: true }; @@ -512,7 +526,7 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/type-archive-failed.log', + logFile: createLog('temp/type-archive-failed.log'), schemaId: '/schemaMatch/', force: true }; @@ -566,7 +580,7 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: 'temp/type-archive-skip.log', + logFile: createLog('temp/type-archive-skip.log'), schemaId: '/schemaMatch/', ignoreError: true, force: true @@ -600,7 +614,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true }; @@ -611,7 +624,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true, revertLog: 'doesntExist.txt' @@ -637,7 +649,6 @@ describe('content-type archive command', () => { const argv = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true }; @@ -647,7 +658,6 @@ describe('content-type archive command', () => { const argv2 = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true, id: 'test' @@ -658,7 +668,6 @@ describe('content-type archive command', () => { const argv3 = { ...yargArgs, ...config, - logFile: LOG_FILENAME(), force: true, silent: true, id: 'test', diff --git a/src/commands/content-type/archive.ts b/src/commands/content-type/archive.ts index e4142fad..a180bdb8 100644 --- a/src/commands/content-type/archive.ts +++ b/src/commands/content-type/archive.ts @@ -8,7 +8,7 @@ import { equalsOrRegex } from '../../common/filter/filter'; import { confirmArchive } from '../../common/archive/archive-helpers'; import ArchiveOptions from '../../common/archive/archive-options'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { FileLog } from '../../common/file-log'; export const command = 'archive [id]'; @@ -18,6 +18,8 @@ export const desc = 'Archive Content Types'; export const LOG_FILENAME = (platform: string = process.platform): string => getDefaultLogPath('type', 'archive', platform); +export const coerceLog = (logFile: string): FileLog => createLog(logFile, 'Content Type Archive Log'); + export const builder = (yargs: Argv): void => { yargs .positional('id', { @@ -56,7 +58,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }; @@ -134,9 +137,7 @@ export const handler = async (argv: Arguments', timestamp)); - } + await log.close(!silent); console.log(`Archived ${successCount} content types.`); }; diff --git a/src/commands/event/archive.spec.ts b/src/commands/event/archive.spec.ts index 0bd0197d..4fe36d59 100644 --- a/src/commands/event/archive.spec.ts +++ b/src/commands/event/archive.spec.ts @@ -1,4 +1,4 @@ -import { builder, command, handler, LOG_FILENAME, getEvents } from './archive'; +import { builder, command, handler, LOG_FILENAME, getEvents, coerceLog } from './archive'; import dynamicContentClientFactory from '../../services/dynamic-content-client-factory'; import { Event, Edition, Hub } from 'dc-management-sdk-js'; import Yargs from 'yargs/yargs'; @@ -6,6 +6,7 @@ import readline from 'readline'; import MockPage from '../../common/dc-management-sdk-js/mock-page'; import { promisify } from 'util'; import { exists, readFile, unlink } from 'fs'; +import { FileLog } from '../../common/file-log'; jest.mock('readline'); @@ -24,7 +25,8 @@ describe('event archive command', () => { const config = { clientId: 'client-id', clientSecret: 'client-id', - hubId: 'hub-id' + hubId: 'hub-id', + logFile: new FileLog() }; it('should command should defined', function() { @@ -65,9 +67,17 @@ describe('event archive command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }); + + it('should generate a log with coerceLog with the appropriate title', function() { + const logFile = coerceLog('filename.log'); + + expect(logFile).toEqual(expect.any(FileLog)); + expect(logFile.title).toMatch(/^Events Archive Log \- ./); + }); }); const mockValues = ({ @@ -554,7 +564,7 @@ describe('event archive command', () => { const argv = { ...yargArgs, ...config, - logFile, + logFile: new FileLog(logFile), silent: false, id: '1' }; @@ -637,7 +647,7 @@ describe('event archive command', () => { ...yargArgs, ...config, silent: false, - logFile: 'temp/event-archive.log', + logFile: new FileLog('temp/event-archive.log'), id: '1' }; diff --git a/src/commands/event/archive.ts b/src/commands/event/archive.ts index 0350fa3d..6cfe63d5 100644 --- a/src/commands/event/archive.ts +++ b/src/commands/event/archive.ts @@ -1,13 +1,12 @@ import { Arguments, Argv } from 'yargs'; import { ConfigurationParameters } from '../configure'; import dynamicContentClientFactory from '../../services/dynamic-content-client-factory'; -import { ArchiveLog } from '../../common/archive/archive-log'; import paginator from '../../common/dc-management-sdk-js/paginator'; import { confirmArchive } from '../../common/archive/archive-helpers'; import ArchiveOptions from '../../common/archive/archive-options'; import { Edition, Event, DynamicContent } from 'dc-management-sdk-js'; import { equalsOrRegex } from '../../common/filter/filter'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { FileLog } from '../../common/file-log'; const maxAttempts = 30; @@ -18,6 +17,8 @@ export const desc = 'Archive Events'; export const LOG_FILENAME = (platform: string = process.platform): string => getDefaultLogPath('event', 'archive', platform); +export const coerceLog = (logFile: string): FileLog => createLog(logFile, 'Events Archive Log'); + export const builder = (yargs: Argv): void => { yargs .positional('id', { @@ -44,7 +45,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: coerceLog }); }; @@ -158,7 +160,7 @@ export const processItems = async ({ }[]; force?: boolean; silent?: boolean; - logFile?: string | FileLog; + logFile: FileLog; missingContent: boolean; ignoreError?: boolean; }): Promise => { @@ -204,8 +206,7 @@ export const processItems = async ({ } } - const timestamp = Date.now().toString(); - const log = logFile instanceof FileLog ? logFile : new ArchiveLog(`Events Archive Log - ${timestamp}\n`); + const log = logFile.open(); let successCount = 0; @@ -246,9 +247,7 @@ export const processItems = async ({ } } - if (!silent && typeof logFile === 'string') { - await log.writeToFile(logFile.replace('', timestamp)); - } + await log.close(!silent); return console.log(`Processed ${successCount} events.`); } catch (e) { diff --git a/src/commands/hub/clean.spec.ts b/src/commands/hub/clean.spec.ts index d1ecf1d4..511178fa 100644 --- a/src/commands/hub/clean.spec.ts +++ b/src/commands/hub/clean.spec.ts @@ -1,5 +1,5 @@ import { builder, command, handler, LOG_FILENAME } from './clean'; -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import Yargs from 'yargs/yargs'; import * as content from './steps/content-clean-step'; @@ -101,7 +101,8 @@ describe('hub clean command', () => { expect(spyOption).toHaveBeenCalledWith('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }); expect(spyOption).toHaveBeenCalledWith('step', { @@ -139,7 +140,7 @@ describe('hub clean command', () => { ...yargArgs, ...config, - logFile: 'temp/clean/steps/all.log', + logFile: createLog('temp/clean/steps/all.log'), force: true }; @@ -168,7 +169,7 @@ describe('hub clean command', () => { ...yargArgs, ...config, - logFile: 'temp/clean/steps/fail' + i + '.log', + logFile: createLog('temp/clean/steps/fail' + i + '.log'), force: true }; @@ -201,7 +202,7 @@ describe('hub clean command', () => { ...config, step: i, - logFile: 'temp/clean/steps/step' + i + '.log', + logFile: createLog('temp/clean/steps/step' + i + '.log'), force: true }; diff --git a/src/commands/hub/clean.ts b/src/commands/hub/clean.ts index 31686ad3..8c7081cf 100644 --- a/src/commands/hub/clean.ts +++ b/src/commands/hub/clean.ts @@ -1,9 +1,7 @@ -import { getDefaultLogPath } from '../../common/log-helpers'; +import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import { Argv, Arguments } from 'yargs'; import { ConfigurationParameters } from '../configure'; -import { FileLog } from '../../common/file-log'; - import { CleanHubBuilderOptions } from '../../interfaces/clean-hub-builder-options'; import { SchemaCleanStep } from './steps/schema-clean-step'; import { TypeCleanStep } from './steps/type-clean-step'; @@ -30,7 +28,8 @@ export const builder = (yargs: Argv): void => { .option('logFile', { type: 'string', default: LOG_FILENAME, - describe: 'Path to a log file to write to.' + describe: 'Path to a log file to write to.', + coerce: createLog }) .option('step', { @@ -42,8 +41,7 @@ export const builder = (yargs: Argv): void => { const steps = [new ContentCleanStep(), new TypeCleanStep(), new SchemaCleanStep()]; export const handler = async (argv: Arguments): Promise => { - const logFile = argv.logFile; - const log = logFile instanceof FileLog ? logFile : new FileLog(logFile); + const log = argv.logFile.open(); argv.logFile = log; @@ -68,7 +66,5 @@ export const handler = async (argv: Arguments { describe('file-log tests', () => { it('should create a log file when filename is specified, and closed', async () => { - const log = new FileLog('file.log'); + const log = new FileLog('file.log').open(); log.appendLine('Test Message'); const writeSpy = jest.spyOn(log, 'writeToFile').mockImplementation(() => Promise.resolve(true)); await log.close(); @@ -15,7 +15,7 @@ describe('file-log', () => { }); it('should not create a log file when filename is null, and closed', async () => { - const log = new FileLog(); + const log = new FileLog().open(); log.appendLine('Test Message'); const writeSpy = jest.spyOn(log, 'writeToFile').mockImplementation(() => Promise.resolve(true)); await log.close(); @@ -27,7 +27,7 @@ describe('file-log', () => { jest.spyOn(Date, 'now').mockImplementation(() => 1234); await ensureDirectoryExists('temp/'); - const log = new FileLog('temp/FileWithDate-.log'); + const log = new FileLog('temp/FileWithDate-.log').open(); log.appendLine('Test Message'); await log.close(); @@ -40,5 +40,31 @@ describe('file-log', () => { await promisify(unlink)('temp/FileWithDate-1234.log'); }); + + it('should only save the log after it has been closed as many times as it was opened', async () => { + const log = new FileLog('notYet.log').open(); + + // Add a nested open. + log.open(); + + log.appendLine('Test Message'); + const writeSpy = jest.spyOn(log, 'writeToFile').mockImplementation(() => Promise.resolve(true)); + await log.close(); + + expect(writeSpy).not.toBeCalled(); // There is still a user, shouldn't save yet. + + await log.close(); + + expect(writeSpy).toBeCalled(); + }); + + it('should not save a log file if false is provided to the close method, and it is the last close', async () => { + const log = new FileLog('noWrite.log').open(); + log.appendLine('Test Message'); + const writeSpy = jest.spyOn(log, 'writeToFile').mockImplementation(() => Promise.resolve(true)); + await log.close(false); + + expect(writeSpy).not.toBeCalled(); + }); }); }); diff --git a/src/common/file-log.ts b/src/common/file-log.ts index 476d3564..a93f0e91 100644 --- a/src/common/file-log.ts +++ b/src/common/file-log.ts @@ -1,6 +1,7 @@ import { ArchiveLog } from './archive/archive-log'; export class FileLog extends ArchiveLog { + private openedCount = 0; closed: boolean; constructor(private filename?: string) { @@ -18,11 +19,19 @@ export class FileLog extends ArchiveLog { this.addComment(text as string); } - public async close(): Promise { - if (this.filename != null) { - await this.writeToFile(this.filename); - } + public open(): FileLog { + this.openedCount++; + + return this; + } + + public async close(writeIfClosed = true): Promise { + if (--this.openedCount <= 0) { + if (this.filename != null && writeIfClosed) { + await this.writeToFile(this.filename); + } - this.closed = true; + this.closed = true; + } } } diff --git a/src/common/log-helpers.ts b/src/common/log-helpers.ts index 3a6eca12..971fc4e1 100644 --- a/src/common/log-helpers.ts +++ b/src/common/log-helpers.ts @@ -1,4 +1,5 @@ import { join } from 'path'; +import { FileLog } from './file-log'; export function getDefaultLogPath(type: string, action: string, platform: string = process.platform): string { return join( @@ -7,3 +8,15 @@ export function getDefaultLogPath(type: string, action: string, platform: string `logs/${type}-${action}-.log` ); } + +export function createLog(logFile: string, title?: string): FileLog { + const log = new FileLog(logFile); + + if (title !== undefined) { + const timestamp = Date.now().toString(); + + log.title = `${title} - ${timestamp}\n`; + } + + return log; +} diff --git a/src/interfaces/clean-hub-builder-options.ts b/src/interfaces/clean-hub-builder-options.ts index cd75c0e6..1a32429c 100644 --- a/src/interfaces/clean-hub-builder-options.ts +++ b/src/interfaces/clean-hub-builder-options.ts @@ -1,7 +1,7 @@ import { FileLog } from '../common/file-log'; export interface CleanHubBuilderOptions { - logFile?: string | FileLog; + logFile: FileLog; force?: boolean; step?: number; } diff --git a/src/interfaces/copy-item-builder-options.interface.ts b/src/interfaces/copy-item-builder-options.interface.ts index 8c2ad947..6cbcb975 100644 --- a/src/interfaces/copy-item-builder-options.interface.ts +++ b/src/interfaces/copy-item-builder-options.interface.ts @@ -20,7 +20,7 @@ export interface CopyItemBuilderOptions { validate?: boolean; skipIncomplete?: boolean; media?: boolean; - logFile?: string | FileLog; + logFile: FileLog; copyConfig?: string | CopyConfig; revertLog?: string; diff --git a/src/interfaces/export-item-builder-options.interface.ts b/src/interfaces/export-item-builder-options.interface.ts index 32ad6e9d..084fdb28 100644 --- a/src/interfaces/export-item-builder-options.interface.ts +++ b/src/interfaces/export-item-builder-options.interface.ts @@ -6,7 +6,7 @@ export interface ExportItemBuilderOptions { repoId?: string[] | string; schemaId?: string[] | string; name?: string[] | string; - logFile?: string | FileLog; + logFile?: FileLog; publish?: boolean; exportedIds?: string[]; diff --git a/src/interfaces/import-item-builder-options.interface.ts b/src/interfaces/import-item-builder-options.interface.ts index bbbf35e7..afbd49dc 100644 --- a/src/interfaces/import-item-builder-options.interface.ts +++ b/src/interfaces/import-item-builder-options.interface.ts @@ -12,7 +12,7 @@ export interface ImportItemBuilderOptions { skipIncomplete?: boolean; excludeKeys?: boolean; media?: boolean; - logFile?: string | FileLog; + logFile?: FileLog; revertLog?: string; } From e8486c807219f045369039382bdff6589646b712 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 19 May 2021 18:53:26 +0100 Subject: [PATCH 08/12] fix: fix copy config errors --- src/common/content-item/copy-config.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/common/content-item/copy-config.spec.ts b/src/common/content-item/copy-config.spec.ts index f1521ec9..23ce188d 100644 --- a/src/common/content-item/copy-config.spec.ts +++ b/src/common/content-item/copy-config.spec.ts @@ -12,7 +12,8 @@ jest.mock('fs'); const yargArgs = { $0: 'test', _: ['test'], - json: true + json: true, + logFile: new FileLog() }; describe('copy-config', () => { @@ -90,7 +91,7 @@ describe('copy-config', () => { it('should return a config object based on the arguments when no config file argument is given', async () => { const log = new FileLog(); - const argv: Arguments = { + const argv: Arguments = { ...yargArgs, hubId: 'test', clientId: 'test2', From 8db837fd44a0e635bb8b86d0f272d88a61d4fa3b Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 24 May 2021 09:46:21 +0100 Subject: [PATCH 09/12] test: add log-helpers tests --- src/common/log-helpers.spec.ts | 58 ++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 src/common/log-helpers.spec.ts diff --git a/src/common/log-helpers.spec.ts b/src/common/log-helpers.spec.ts new file mode 100644 index 00000000..fa9f4fee --- /dev/null +++ b/src/common/log-helpers.spec.ts @@ -0,0 +1,58 @@ +import { createLog, getDefaultLogPath } from './log-helpers'; +import { join } from 'path'; + +describe('log-helpers', () => { + describe('getDefaultLogPath tests', () => { + it('should build a log path out of the given type, action and platform', async () => { + process.env['USERPROFILE'] = '/win32path'; + process.env['HOME'] = '/unixpath'; + + const pathWin = getDefaultLogPath('example1', 'action1', 'win32'); + expect(pathWin).toMatchInlineSnapshot(`"/win32path/.amplience/logs/example1-action1-.log"`); + + const pathUnix = getDefaultLogPath('example2', 'action2', 'unix'); + expect(pathUnix).toMatchInlineSnapshot(`"/unixpath/.amplience/logs/example2-action2-.log"`); + + const pathAuto = getDefaultLogPath('example3', 'action3'); + if (process.platform === 'win32') { + expect(pathAuto).toMatchInlineSnapshot(`"/win32path/.amplience/logs/example3-action3-.log"`); + } else { + expect(pathAuto).toMatchInlineSnapshot(`"/unixpath/.amplience/logs/example3-action3-.log"`); + } + + delete process.env['HOME']; + + const pathUndefined = getDefaultLogPath('example2', 'action2', 'unix'); + expect(pathUndefined).toEqual(join(__dirname, '/.amplience/logs/example2-action2-.log')); + }); + }); + + describe('createLog tests', () => { + it('should create a log instance with the given filename, without opening it', async () => { + const log = createLog('exampleFilename.txt'); + + // Undefined title matches the filename. + expect(log.title).toEqual('exampleFilename.txt'); + expect(log['filename']).toEqual('exampleFilename.txt'); + expect(log['openedCount']).toEqual(0); + }); + + it('should create a log instance with the given filename, without opening it', async () => { + const log = createLog('exampleFilename.txt'); + + // Undefined title matches the filename. + expect(log.title).toEqual('exampleFilename.txt'); + expect(log['filename']).toEqual('exampleFilename.txt'); + expect(log['openedCount']).toEqual(0); + }); + + it('should create a log instance with the given title, adding a timestamp afterwards', async () => { + const log = createLog('exampleFilename2.txt', 'title with timestamp'); + + // Followed by timestamp. + expect(log.title).toMatch(/^title with timestamp \- ./); + expect(log['filename']).toEqual('exampleFilename2.txt'); + expect(log['openedCount']).toEqual(0); + }); + }); +}); From 8c72b0418c5e2aa16a79c543885c46c5456ad04c Mon Sep 17 00:00:00 2001 From: Rhys Date: Thu, 17 Jun 2021 10:46:25 +0100 Subject: [PATCH 10/12] refactor: use a string enum rather than a step index for --step --- src/commands/hub/clean.spec.ts | 39 +++++++++++++++----- src/commands/hub/clean.ts | 19 ++++++---- src/commands/hub/model/clean-hub-step-id.ts | 0 src/commands/hub/model/clean-hub-step.ts | 7 ++++ src/commands/hub/steps/content-clean-step.ts | 6 ++- src/commands/hub/steps/schema-clean-step.ts | 6 ++- src/commands/hub/steps/type-clean-step.ts | 6 ++- src/interfaces/clean-hub-builder-options.ts | 3 +- 8 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 src/commands/hub/model/clean-hub-step-id.ts diff --git a/src/commands/hub/clean.spec.ts b/src/commands/hub/clean.spec.ts index 511178fa..490a904c 100644 --- a/src/commands/hub/clean.spec.ts +++ b/src/commands/hub/clean.spec.ts @@ -1,4 +1,5 @@ -import { builder, command, handler, LOG_FILENAME } from './clean'; +import { builder, command, handler, LOG_FILENAME, steps } from './clean'; +import { CleanHubStepId } from './model/clean-hub-step'; import { createLog, getDefaultLogPath } from '../../common/log-helpers'; import Yargs from 'yargs/yargs'; @@ -25,18 +26,25 @@ function succeedOrFail(mock: any, succeed: () => boolean): jest.Mock { } // eslint-disable-next-line @typescript-eslint/no-explicit-any -function mockStep(name: string, success: () => boolean): any { +function mockStep(name: string, id: string, success: () => boolean): any { return jest.fn().mockImplementation(() => ({ run: succeedOrFail(jest.fn(), success), revert: succeedOrFail(jest.fn(), success), - getName: jest.fn().mockReturnValue(name) + getName: jest.fn().mockReturnValue(name), + getId: jest.fn().mockReturnValue(id) })); } -jest.mock('./steps/content-clean-step', () => ({ ContentCleanStep: mockStep('Clean Content', () => success[0]) })); -jest.mock('./steps/type-clean-step', () => ({ TypeCleanStep: mockStep('Clean Content Types', () => success[1]) })); +jest.mock('./steps/content-clean-step', () => ({ + ContentCleanStep: mockStep('Clean Content', 'content', () => success[0]) +})); + +jest.mock('./steps/type-clean-step', () => ({ + TypeCleanStep: mockStep('Clean Content Types', 'type', () => success[1]) +})); + jest.mock('./steps/schema-clean-step', () => ({ - SchemaCleanStep: mockStep('Clean Content Type Schemas', () => success[2]) + SchemaCleanStep: mockStep('Clean Content Type Schemas', 'schema', () => success[2]) })); jest.mock('../../common/log-helpers', () => ({ @@ -106,8 +114,9 @@ describe('hub clean command', () => { }); expect(spyOption).toHaveBeenCalledWith('step', { - type: 'number', - describe: 'Start at a numbered step. 0: Schema, 1: Type, 2: Content' + type: 'string', + describe: 'Start at a specific step. Steps after the one you specify will also run.', + choices: steps.map(step => step.getId()) }); }); }); @@ -201,7 +210,7 @@ describe('hub clean command', () => { ...yargArgs, ...config, - step: i, + step: Object.values(CleanHubStepId)[i], logFile: createLog('temp/clean/steps/step' + i + '.log'), force: true }; @@ -224,5 +233,17 @@ describe('hub clean command', () => { await loadLog.loadFromFile('temp/clean/steps/step' + i + '.log'); } }); + + it('should only have one of each type of step', () => { + const stepsSoFar = new Set(); + + for (const step of steps) { + const id = step.getId(); + + expect(stepsSoFar.has(id)).toBeFalsy(); + + stepsSoFar.add(id); + } + }); }); }); diff --git a/src/commands/hub/clean.ts b/src/commands/hub/clean.ts index 8c7081cf..790cb9d9 100644 --- a/src/commands/hub/clean.ts +++ b/src/commands/hub/clean.ts @@ -1,5 +1,5 @@ import { createLog, getDefaultLogPath } from '../../common/log-helpers'; -import { Argv, Arguments } from 'yargs'; +import { Argv, Arguments, choices } from 'yargs'; import { ConfigurationParameters } from '../configure'; import { CleanHubBuilderOptions } from '../../interfaces/clean-hub-builder-options'; @@ -15,6 +15,8 @@ export const desc = export const LOG_FILENAME = (platform: string = process.platform): string => getDefaultLogPath('hub', 'clean', platform); +export const steps = [new ContentCleanStep(), new TypeCleanStep(), new SchemaCleanStep()]; + export const builder = (yargs: Argv): void => { yargs .alias('f', 'force') @@ -33,13 +35,12 @@ export const builder = (yargs: Argv): void => { }) .option('step', { - type: 'number', - describe: 'Start at a numbered step. 0: Schema, 1: Type, 2: Content' + type: 'string', + describe: 'Start at a specific step. Steps after the one you specify will also run.', + choices: steps.map(step => step.getId()) }); }; -const steps = [new ContentCleanStep(), new TypeCleanStep(), new SchemaCleanStep()]; - export const handler = async (argv: Arguments): Promise => { const log = argv.logFile.open(); @@ -48,7 +49,9 @@ export const handler = async (argv: Arguments step.getId() === argv.step)); + + for (let i = stepIndex; i < steps.length; i++) { const step = steps[i]; log.switchGroup(step.getName()); @@ -57,10 +60,10 @@ export const handler = async (argv: Arguments): Promise; } diff --git a/src/commands/hub/steps/content-clean-step.ts b/src/commands/hub/steps/content-clean-step.ts index 30bc7bb6..e5a8badf 100644 --- a/src/commands/hub/steps/content-clean-step.ts +++ b/src/commands/hub/steps/content-clean-step.ts @@ -3,9 +3,13 @@ import { FileLog } from '../../../common/file-log'; import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; import { ConfigurationParameters } from '../../configure'; import { handler } from '../../content-item/archive'; -import { CleanHubStep } from '../model/clean-hub-step'; +import { CleanHubStep, CleanHubStepId } from '../model/clean-hub-step'; export class ContentCleanStep implements CleanHubStep { + getId(): CleanHubStepId { + return CleanHubStepId.Content; + } + getName(): string { return 'Clean Content'; } diff --git a/src/commands/hub/steps/schema-clean-step.ts b/src/commands/hub/steps/schema-clean-step.ts index bf341026..7be0e230 100644 --- a/src/commands/hub/steps/schema-clean-step.ts +++ b/src/commands/hub/steps/schema-clean-step.ts @@ -3,9 +3,13 @@ import { FileLog } from '../../../common/file-log'; import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; import { ConfigurationParameters } from '../../configure'; import { handler } from '../../content-type-schema/archive'; -import { CleanHubStep } from '../model/clean-hub-step'; +import { CleanHubStep, CleanHubStepId } from '../model/clean-hub-step'; export class SchemaCleanStep implements CleanHubStep { + getId(): CleanHubStepId { + return CleanHubStepId.Schema; + } + getName(): string { return 'Clean Content Type Schemas'; } diff --git a/src/commands/hub/steps/type-clean-step.ts b/src/commands/hub/steps/type-clean-step.ts index 54aeb692..018e6f99 100644 --- a/src/commands/hub/steps/type-clean-step.ts +++ b/src/commands/hub/steps/type-clean-step.ts @@ -3,9 +3,13 @@ import { FileLog } from '../../../common/file-log'; import { CleanHubBuilderOptions } from '../../../interfaces/clean-hub-builder-options'; import { ConfigurationParameters } from '../../configure'; import { handler } from '../../content-type/archive'; -import { CleanHubStep } from '../model/clean-hub-step'; +import { CleanHubStep, CleanHubStepId } from '../model/clean-hub-step'; export class TypeCleanStep implements CleanHubStep { + getId(): CleanHubStepId { + return CleanHubStepId.Type; + } + getName(): string { return 'Clean Content Types'; } diff --git a/src/interfaces/clean-hub-builder-options.ts b/src/interfaces/clean-hub-builder-options.ts index 1a32429c..8944bc62 100644 --- a/src/interfaces/clean-hub-builder-options.ts +++ b/src/interfaces/clean-hub-builder-options.ts @@ -1,7 +1,8 @@ +import { CleanHubStepId } from '../commands/hub/model/clean-hub-step'; import { FileLog } from '../common/file-log'; export interface CleanHubBuilderOptions { logFile: FileLog; force?: boolean; - step?: number; + step?: CleanHubStepId; } From db61cbd67b56640a96be8ac4d600703ce2a3c419 Mon Sep 17 00:00:00 2001 From: Rhys Date: Thu, 17 Jun 2021 10:50:10 +0100 Subject: [PATCH 11/12] refactor: remove unused import --- src/commands/hub/clean.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/hub/clean.ts b/src/commands/hub/clean.ts index 790cb9d9..5d67e0d6 100644 --- a/src/commands/hub/clean.ts +++ b/src/commands/hub/clean.ts @@ -1,5 +1,5 @@ import { createLog, getDefaultLogPath } from '../../common/log-helpers'; -import { Argv, Arguments, choices } from 'yargs'; +import { Argv, Arguments } from 'yargs'; import { ConfigurationParameters } from '../configure'; import { CleanHubBuilderOptions } from '../../interfaces/clean-hub-builder-options'; From f86f2a82b7f238ccb5a3fb7c6f77f1aa7a9ec1df Mon Sep 17 00:00:00 2001 From: Rhys Date: Thu, 17 Jun 2021 11:37:54 +0100 Subject: [PATCH 12/12] test: add tests to ensure the ids are correct --- src/commands/hub/steps/content-clean-step.spec.ts | 6 ++++++ src/commands/hub/steps/schema-clean-step.spec.ts | 6 ++++++ src/commands/hub/steps/type-clean-step.spec.ts | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/src/commands/hub/steps/content-clean-step.spec.ts b/src/commands/hub/steps/content-clean-step.spec.ts index a0788176..6ae62c0d 100644 --- a/src/commands/hub/steps/content-clean-step.spec.ts +++ b/src/commands/hub/steps/content-clean-step.spec.ts @@ -7,6 +7,7 @@ import { join } from 'path'; import * as archive from '../../content-item/archive'; import { ContentCleanStep } from './content-clean-step'; +import { CleanHubStepId } from '../model/clean-hub-step'; jest.mock('../../../services/dynamic-content-client-factory'); jest.mock('../../content-item/archive'); @@ -43,6 +44,11 @@ describe('content clean step', () => { }; } + it('should have the id "content"', () => { + const step = new ContentCleanStep(); + expect(step.getId()).toEqual(CleanHubStepId.Content); + }); + it('should have the name "Clean Content"', () => { const step = new ContentCleanStep(); expect(step.getName()).toEqual('Clean Content'); diff --git a/src/commands/hub/steps/schema-clean-step.spec.ts b/src/commands/hub/steps/schema-clean-step.spec.ts index 1766a6f7..a5221ab1 100644 --- a/src/commands/hub/steps/schema-clean-step.spec.ts +++ b/src/commands/hub/steps/schema-clean-step.spec.ts @@ -7,6 +7,7 @@ import { join } from 'path'; import * as archive from '../../content-type-schema/archive'; import { SchemaCleanStep } from './schema-clean-step'; +import { CleanHubStepId } from '../model/clean-hub-step'; jest.mock('../../../services/dynamic-content-client-factory'); jest.mock('../../content-type-schema/archive'); @@ -43,6 +44,11 @@ describe('schema clean step', () => { }; } + it('should have the id "schema"', () => { + const step = new SchemaCleanStep(); + expect(step.getId()).toEqual(CleanHubStepId.Schema); + }); + it('should have the name "Clean Content Type Schemas"', () => { const step = new SchemaCleanStep(); expect(step.getName()).toEqual('Clean Content Type Schemas'); diff --git a/src/commands/hub/steps/type-clean-step.spec.ts b/src/commands/hub/steps/type-clean-step.spec.ts index 07c4e5a6..23d3075a 100644 --- a/src/commands/hub/steps/type-clean-step.spec.ts +++ b/src/commands/hub/steps/type-clean-step.spec.ts @@ -7,6 +7,7 @@ import { join } from 'path'; import * as archive from '../../content-type/archive'; import { TypeCleanStep } from './type-clean-step'; +import { CleanHubStepId } from '../model/clean-hub-step'; jest.mock('../../../services/dynamic-content-client-factory'); jest.mock('../../content-type/archive'); @@ -43,6 +44,11 @@ describe('type clean step', () => { }; } + it('should have the id "type"', () => { + const step = new TypeCleanStep(); + expect(step.getId()).toEqual(CleanHubStepId.Type); + }); + it('should have the name "Clean Content Types"', () => { const step = new TypeCleanStep(); expect(step.getName()).toEqual('Clean Content Types');