From 65a04f7d83099cbd0e96f29a54be2b8fb43756eb Mon Sep 17 00:00:00 2001 From: Rhys Date: Tue, 23 Mar 2021 09:42:59 +0000 Subject: [PATCH 01/11] feat(content-item): dependency tree printing command (tree) --- src/commands/content-item/tree.ts | 185 ++++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 src/commands/content-item/tree.ts diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts new file mode 100644 index 00000000..f2858476 --- /dev/null +++ b/src/commands/content-item/tree.ts @@ -0,0 +1,185 @@ +import { getDefaultLogPath } from '../../common/log-helpers'; +import { Argv, Arguments } from 'yargs'; +import { join, extname, resolve } from 'path'; +import { ConfigurationParameters } from '../configure'; +import { lstat, readdir, readFile } from 'fs'; +import { promisify } from 'util'; + +import { ContentItem, ContentRepository } from 'dc-management-sdk-js'; +import { + ContentDependancyTree, + ItemContentDependancies, + RepositoryContentItem +} from '../../common/content-item/content-dependancy-tree'; +import { ContentMapping } from '../../common/content-item/content-mapping'; + +export function getTempFolder(name: string, platform: string = process.platform): string { + return join(process.env[platform == 'win32' ? 'USERPROFILE' : 'HOME'] || __dirname, '.amplience', `copy-${name}/`); +} + +export const command = 'tree '; + +export const desc = 'Print a content dependency tree from content in the given folder.'; + +export const LOG_FILENAME = (platform: string = process.platform): string => + getDefaultLogPath('item', 'tree', platform); + +export const builder = (yargs: Argv): void => { + yargs.positional('dir', { + type: 'string', + describe: 'Path to the content items to build a tree from.. Should be in the same format as an export.' + }); +}; + +interface TreeOptions { + dir: string; +} + +const traverseRecursive = async (path: string, action: (path: string) => Promise): Promise => { + const dir = await promisify(readdir)(path); + + await Promise.all( + dir.map(async (contained: string) => { + contained = join(path, contained); + const stat = await promisify(lstat)(contained); + return await (stat.isDirectory() ? traverseRecursive(contained, action) : action(contained)); + }) + ); +}; + +const prepareContentForTree = async ( + repos: { basePath: string; repo: ContentRepository }[], + argv: Arguments +): Promise => { + const contentItems: RepositoryContentItem[] = []; + const schemaNames = new Set(); + + for (let i = 0; i < repos.length; i++) { + const repo = repos[i].repo; + + await traverseRecursive(resolve(repos[i].basePath), async path => { + // Is this valid content? Must have extension .json to be considered, for a start. + if (extname(path) !== '.json') { + return; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let contentJSON: any; + try { + const contentText = await promisify(readFile)(path, { encoding: 'utf8' }); + contentJSON = JSON.parse(contentText); + } catch (e) { + console.error(`Couldn't read content item at '${path}': ${e.toString()}`); + return; + } + + // Get the folder id via the mapping. + + // Only filter relevant information - for example status and previous content repo are not useful. + const filteredContent = { + id: contentJSON.id, + label: contentJSON.label, + locale: contentJSON.locale, + body: contentJSON.body, + deliveryId: contentJSON.deliveryId == contentJSON.Id || argv.excludeKeys ? undefined : contentJSON.deliveryId, + folderId: null, + publish: contentJSON.lastPublishedVersion != null + }; + + if (argv.excludeKeys) { + delete filteredContent.body._meta.deliveryKey; + } + + schemaNames.add(contentJSON.body._meta.schema); + + contentItems.push({ repo: repo, content: new ContentItem(filteredContent) }); + }); + } + + return new ContentDependancyTree(contentItems, new ContentMapping()); +}; + +export const handler = async (argv: Arguments): Promise => { + const dir = argv.dir; + + const baseDirContents = await promisify(readdir)(dir); + const importRepos: { basePath: string; repo: ContentRepository }[] = []; + for (let i = 0; i < baseDirContents.length; i++) { + const name = baseDirContents[i]; + const path = join(dir, name); + const status = await promisify(lstat)(path); + if (status.isDirectory()) { + importRepos.push({ basePath: path, repo: new ContentRepository() }); + } + } + + const tree = await prepareContentForTree(importRepos, argv); + + // Print the items in the tree. + // Keep a set of all items that have already been printed. + // Starting at the highest level, print all dependencies on the tree. + + if (tree == null) return; + + const evaluated = new Set(); + const firstSecondThird = (index: number, total: number): number => { + return index === 0 ? 0 : index == total - 1 ? 2 : 1; + }; + + const fstPipes = ['├', '├', '└']; + + const printDependency = ( + item: ItemContentDependancies, + depth: number, + evalThis: ItemContentDependancies[], + fst: number, + prefix: string + ): boolean => { + const pipe = depth < 0 ? '' : fstPipes[fst] + '─ '; + + if (evalThis.indexOf(item) !== -1) { + console.log(`${prefix}${pipe}*** (${item.owner.content.label})`); + return false; + } else if (evaluated.has(item)) { + if (depth > 0) { + console.log(`${prefix}${pipe}(${item.owner.content.label})`); + } + return false; + } else { + console.log(`${prefix}${pipe}${item.owner.content.label}`); + } + + evalThis.push(item); + evaluated.add(item); + + item.dependancies.forEach((dep, index) => { + if (dep.resolved) { + const subFst = firstSecondThird(index, item.dependancies.length); + const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; + printDependency(dep.resolved, depth + 1, [...evalThis], subFst, prefix + subPrefix); + } + }); + return true; + }; + + for (let i = tree.levels.length - 1; i >= 0; i--) { + const level = tree.levels[i]; + console.log(`=== LEVEL ${i + 1} (${level.items.length}) ===`); + + level.items.forEach(item => { + printDependency(item, -1, [], 0, ''); + console.log(''); + }); + } + + console.log(`=== CIRCULAR (${tree.circularLinks.length}) ===`); + let topLevelPrints = 0; + tree.circularLinks.forEach(item => { + if (printDependency(item, -1, [], 0, '')) { + topLevelPrints++; + console.log(''); + } + }); + + console.log(`Finished. Circular Dependencies printed: ${topLevelPrints}`); +}; From c395cb9dff7bc608b1ba5970b916b78b28e47111 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 31 Mar 2021 09:15:48 +0100 Subject: [PATCH 02/11] feat(content-item): improve circular import, tree printing --- src/commands/content-item/import.ts | 21 ++- src/commands/content-item/tree.ts | 170 +++++++++++++----- .../content-item/content-dependancy-tree.ts | 136 +++++++++++++- 3 files changed, 270 insertions(+), 57 deletions(-) diff --git a/src/commands/content-item/import.ts b/src/commands/content-item/import.ts index ba8dbdda..acb23b09 100644 --- a/src/commands/content-item/import.ts +++ b/src/commands/content-item/import.ts @@ -675,12 +675,22 @@ const prepareContentForImport = async ( return tree; }; -const rewriteDependancy = (dep: ContentDependancyInfo, mapping: ContentMapping): void => { - const id = mapping.getContentItem(dep.dependancy.id) || dep.dependancy.id; +const rewriteDependancy = (dep: ContentDependancyInfo, mapping: ContentMapping, allowNull: boolean): void => { + let id = mapping.getContentItem(dep.dependancy.id); + + if (id == null && !allowNull) { + id = dep.dependancy.id; + } + if (dep.dependancy._meta.schema === '_hierarchy') { dep.owner.content.body._meta.hierarchy.parentId = id; } else { - dep.dependancy.id = id; + if (id == null) { + delete dep.parent[dep.index]; + } else { + dep.parent[dep.index] = dep.dependancy; + dep.dependancy.id = id; + } } }; @@ -706,7 +716,7 @@ const importTree = async ( // Replace any dependancies with the existing mapping. item.dependancies.forEach(dep => { - rewriteDependancy(dep, mapping); + rewriteDependancy(dep, mapping, false); }); const originalId = content.id; @@ -781,7 +791,7 @@ const importTree = async ( const content = item.owner.content; item.dependancies.forEach(dep => { - rewriteDependancy(dep, mapping); + rewriteDependancy(dep, mapping, true); }); const originalId = content.id; @@ -815,6 +825,7 @@ const importTree = async ( newDependants[i] = newItem; mapping.registerContentItem(originalId as string, newItem.id as string); + mapping.registerContentItem(newItem.id as string, newItem.id as string); } else { if (itemShouldPublish(content) && (newItem.version != oldVersion || argv.republish)) { publishable.push({ item: newItem, node: item }); diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index f2858476..a3b006f1 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -99,6 +99,131 @@ const prepareContentForTree = async ( return new ContentDependancyTree(contentItems, new ContentMapping()); }; +type CircularLink = [number, number]; +interface ParentReference { + item: ItemContentDependancies; + line: number; +} + +const firstSecondThird = (index: number, total: number): number => { + return index === 0 ? 0 : index == total - 1 ? 2 : 1; +}; + +const fstPipes = ['├', '├', '└']; +const circularPipes = ['╗', '║', '╝']; +const circularLine = '═'; + +const printDependency = ( + item: ItemContentDependancies, + evaluated: Set, + lines: string[], + circularLinks: CircularLink[], + evalThis: ParentReference[], + fst: number, + prefix: string +): boolean => { + const depth = evalThis.length - 1; + const pipe = depth < 0 ? '' : fstPipes[fst] + '─ '; + + const circularMatch = evalThis.find(parent => parent.item == item); + if (circularMatch) { + lines.push(`${prefix}${pipe}*** (${item.owner.content.label})`); + circularLinks.push([circularMatch.line, lines.length - 1]); + return false; + } else if (evaluated.has(item)) { + if (depth > -1) { + lines.push(`${prefix}${pipe}(${item.owner.content.label})`); + } + return false; + } else { + lines.push(`${prefix}${pipe}${item.owner.content.label}`); + } + + evalThis.push({ item, line: lines.length - 1 }); + evaluated.add(item); + + const filteredItems = item.dependancies.filter(dep => dep.resolved); + filteredItems.forEach((dep, index) => { + if (dep.resolved) { + const subFst = firstSecondThird(index, filteredItems.length); + const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; + printDependency(dep.resolved, evaluated, lines, circularLinks, [...evalThis], subFst, prefix + subPrefix); + } + }); + return true; +}; + +const fillWhitespace = (original: string, current: string, char: string, targetLength: number): string => { + if (current.length < original.length + 1) { + current += ' '; + } + + let position = original.length + 1; + let repeats = targetLength - (original.length + 1); + + // Replace existing whitespace characters + while (position < current.length && repeats > 0) { + if (current[position] != char && current[position] == ' ') { + current = current.slice(0, position) + char + current.slice(position + 1); + } + + position++; + repeats--; + } + + if (repeats > 0) { + current += char.repeat(repeats); + } + + return current; +}; + +const printTree = (item: ItemContentDependancies, evaluated: Set): boolean => { + const lines: string[] = []; + const circularLinks: CircularLink[] = []; + + const result = printDependency(item, evaluated, lines, circularLinks, [], 0, ''); + + if (!result) return false; + + const modifiedLines = [...lines]; + + // Render circular references. + // These are drawn as pipes on the right hand side, from a start line to an end line. + + const maxWidth = Math.max(...lines.map(x => x.length)); + + for (let i = 0; i < circularLinks.length; i++) { + const link = circularLinks[i]; + let linkDist = maxWidth + 2; + + // Find overlapping circular links. Push the link out further if a previously drawn line is there. + for (let j = 0; j < i; j++) { + const link2 = circularLinks[j]; + if (link[0] <= link2[1] && link[1] >= link2[0]) { + linkDist += 2; + } + } + + // Write the circular dependency lines into the tree. + + for (let ln = link[0]; ln <= link[1]; ln++) { + const end = ln == link[0] || ln == link[1]; + const original = lines[ln]; + let current = modifiedLines[ln]; + + current = fillWhitespace(original, current, end ? circularLine : ' ', linkDist); + current += circularPipes[firstSecondThird(ln - link[0], link[1] - link[0] + 1)]; + + modifiedLines[ln] = current; + } + } + + modifiedLines.forEach(line => console.log(line)); + console.log(''); + return true; +}; + export const handler = async (argv: Arguments): Promise => { const dir = argv.dir; @@ -122,62 +247,21 @@ export const handler = async (argv: Arguments(); - const firstSecondThird = (index: number, total: number): number => { - return index === 0 ? 0 : index == total - 1 ? 2 : 1; - }; - - const fstPipes = ['├', '├', '└']; - - const printDependency = ( - item: ItemContentDependancies, - depth: number, - evalThis: ItemContentDependancies[], - fst: number, - prefix: string - ): boolean => { - const pipe = depth < 0 ? '' : fstPipes[fst] + '─ '; - - if (evalThis.indexOf(item) !== -1) { - console.log(`${prefix}${pipe}*** (${item.owner.content.label})`); - return false; - } else if (evaluated.has(item)) { - if (depth > 0) { - console.log(`${prefix}${pipe}(${item.owner.content.label})`); - } - return false; - } else { - console.log(`${prefix}${pipe}${item.owner.content.label}`); - } - - evalThis.push(item); - evaluated.add(item); - - item.dependancies.forEach((dep, index) => { - if (dep.resolved) { - const subFst = firstSecondThird(index, item.dependancies.length); - const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; - printDependency(dep.resolved, depth + 1, [...evalThis], subFst, prefix + subPrefix); - } - }); - return true; - }; for (let i = tree.levels.length - 1; i >= 0; i--) { const level = tree.levels[i]; console.log(`=== LEVEL ${i + 1} (${level.items.length}) ===`); level.items.forEach(item => { - printDependency(item, -1, [], 0, ''); - console.log(''); + printTree(item, evaluated); }); } console.log(`=== CIRCULAR (${tree.circularLinks.length}) ===`); let topLevelPrints = 0; tree.circularLinks.forEach(item => { - if (printDependency(item, -1, [], 0, '')) { + if (printTree(item, evaluated)) { topLevelPrints++; - console.log(''); } }); diff --git a/src/common/content-item/content-dependancy-tree.ts b/src/common/content-item/content-dependancy-tree.ts index 62ee583b..0f7afac2 100644 --- a/src/common/content-item/content-dependancy-tree.ts +++ b/src/common/content-item/content-dependancy-tree.ts @@ -22,6 +22,10 @@ export interface ContentDependancyInfo { resolved?: ItemContentDependancies; dependancy: ContentDependancy; owner: RepositoryContentItem; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + parent: any; + index: string | number; } export interface ItemContentDependancies { @@ -39,6 +43,12 @@ export const referenceTypes = [ 'http://bigcontent.io/cms/schema/v1/core#/definitions/content-reference' ]; +enum CircularDependencyStage { + Standalone = 0, + Intertwined, + Parent +} + type RecursiveSearchStep = Body | ContentDependancy | Array; export class ContentDependancyTree { @@ -96,8 +106,40 @@ export class ContentDependancyTree { // Remaining items in the info array are connected to circular dependancies, so must be resolved via rewriting. + // Create dependency layers for circular dependencies + + const circularStages: ItemContentDependancies[][] = []; + while (unresolvedCount > 0) { + const stage: ItemContentDependancies[] = []; + + // To be in this stage, the circular dependency must contain no other circular dependencies (before self-loop). + // The circular dependencies that appear before self loop are + const lastUnresolvedCount = unresolvedCount; + const circularLevels = info.map(item => this.topLevelCircular(item, info)); + + const chosenLevel = Math.min(...circularLevels) as CircularDependencyStage; + + for (let i = 0; i < info.length; i++) { + const item = info[i]; + if (circularLevels[i] === chosenLevel) { + stage.push(item); + circularLevels.splice(i, 1); + info.splice(i--, 1); + } + } + + unresolvedCount = info.length; + if (unresolvedCount === lastUnresolvedCount) { + break; + } + + circularStages.push(stage); + } + this.levels = stages; - this.circularLinks = info; + this.circularLinks = []; + circularStages.forEach(stage => this.circularLinks.push(...stage)); + this.all = allInfo; this.byId = new Map(allInfo.map(info => [info.owner.content.id as string, info])); this.requiredSchema = Array.from(requiredSchema); @@ -106,11 +148,13 @@ export class ContentDependancyTree { private searchObjectForContentDependancies( item: RepositoryContentItem, body: RecursiveSearchStep, - result: ContentDependancyInfo[] + result: ContentDependancyInfo[], + parent: RecursiveSearchStep | null, + index: string | number ): void { if (Array.isArray(body)) { - body.forEach(contained => { - this.searchObjectForContentDependancies(item, contained, result); + body.forEach((contained, index) => { + this.searchObjectForContentDependancies(item, contained, result, body, index); }); } else if (body != null) { const allPropertyNames = Object.getOwnPropertyNames(body); @@ -121,14 +165,14 @@ export class ContentDependancyTree { typeof body.contentType === 'string' && typeof body.id === 'string' ) { - result.push({ dependancy: body as ContentDependancy, owner: item }); + result.push({ dependancy: body as ContentDependancy, owner: item, parent, index }); return; } allPropertyNames.forEach(propName => { const prop = (body as Body)[propName]; if (typeof prop === 'object') { - this.searchObjectForContentDependancies(item, prop, result); + this.searchObjectForContentDependancies(item, prop, result, body, propName); } }); } @@ -161,10 +205,76 @@ export class ContentDependancyTree { } } + private topLevelCircular( + top: ItemContentDependancies, + unresolved: ItemContentDependancies[] + ): CircularDependencyStage { + let selfLoop = false; + let intertwinedLoop = false; + const seenBefore = new Set(); + + const traverse = ( + top: ItemContentDependancies, + item: ItemContentDependancies | undefined, + depth: number, + unresolved: ItemContentDependancies[], + seenBefore: Set, + intertwined: boolean + ): boolean => { + let hasCircular = false; + + if (item == null) { + return false; + } else if (top === item && depth > 0) { + selfLoop = true; + return false; + } else if (top !== item && unresolved.indexOf(item) !== -1) { + // Contains a circular dependency. + + if (!intertwined) { + // Does it loop back to the parent? + const storedSelfLoop = selfLoop; + intertwinedLoop = traverse(item, item, 0, [top], new Set(), true); + selfLoop = storedSelfLoop; + } + + hasCircular = true; + } + + if (seenBefore.has(item)) { + return false; + } + + seenBefore.add(item); + + item.dependancies.forEach(dep => { + hasCircular = traverse(top, dep.resolved, depth++, unresolved, seenBefore, intertwined) || hasCircular; + }); + + return hasCircular; + }; + + const hasCircular = traverse(top, top, 0, unresolved, seenBefore, false); + + if (hasCircular) { + if (intertwinedLoop) { + if (selfLoop) { + return CircularDependencyStage.Intertwined; + } else { + return CircularDependencyStage.Parent; + } + } else { + return CircularDependencyStage.Parent; + } + } else { + return CircularDependencyStage.Standalone; + } + } + private identifyContentDependancies(items: RepositoryContentItem[]): ItemContentDependancies[] { return items.map(item => { const result: ContentDependancyInfo[] = []; - this.searchObjectForContentDependancies(item, item.content.body, result); + this.searchObjectForContentDependancies(item, item.content.body, result, null, 0); // Hierarchy parent is also a dependancy. if (item.content.body._meta.hierarchy && item.content.body._meta.hierarchy.parentId) { @@ -176,7 +286,9 @@ export class ContentDependancyTree { id: item.content.body._meta.hierarchy.parentId, contentType: '' }, - owner: item + owner: item, + parent: null, + index: 0 }); } @@ -198,7 +310,13 @@ export class ContentDependancyTree { const target = idMap.get(dep.dependancy.id as string); dep.resolved = target; if (target) { - target.dependants.push({ owner: target.owner, resolved: item, dependancy: dep.dependancy }); + target.dependants.push({ + owner: target.owner, + resolved: item, + dependancy: dep.dependancy, + parent: dep.parent, + index: dep.index + }); resolve(target); } }); From a54f7f257dc6819218a5d02810fe3eb47262dd8b Mon Sep 17 00:00:00 2001 From: Rhys Date: Thu, 8 Apr 2021 23:42:48 +0100 Subject: [PATCH 03/11] test(content-item): add tree command tests, works on any folder, fix circular bugs --- .../__snapshots__/tree.spec.ts.snap | 90 +++++ src/commands/content-item/tree.spec.ts | 309 ++++++++++++++++++ src/commands/content-item/tree.ts | 139 ++++---- .../content-item/content-dependancy-tree.ts | 22 +- 4 files changed, 473 insertions(+), 87 deletions(-) create mode 100644 src/commands/content-item/__snapshots__/tree.spec.ts.snap create mode 100644 src/commands/content-item/tree.spec.ts diff --git a/src/commands/content-item/__snapshots__/tree.spec.ts.snap b/src/commands/content-item/__snapshots__/tree.spec.ts.snap new file mode 100644 index 00000000..eec692aa --- /dev/null +++ b/src/commands/content-item/__snapshots__/tree.spec.ts.snap @@ -0,0 +1,90 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`content-item tree command handler tests should detect and print circular dependencies with a double line indicator 1`] = ` +"=== LEVEL 2 (1) === +item6 +└─ item5 + +=== LEVEL 1 (3) === +item3 + +item7 + +=== CIRCULAR (3) === +item1 ═════════════════╗ +├─ item2 ║ +│ └─ item4 ║ +│ └─ *** (item1) ══╝ +└─ (item3) + +Finished. Circular Dependencies printed: 1" +`; + +exports[`content-item tree command handler tests should detect intertwined circular dependencies with multiple lines with different position 1`] = ` +"test +test +=== CIRCULAR (6) === +item5 ══════════════╗ +└─ item6 ║ + └─ *** (item5) ══╝ + +item1 ══════════════════════╗ +└─ item2 ═════════════════╗ ║ + └─ item3 ║ ║ + ├─ *** (item2) ═════╝ ║ + └─ item4 ║ + ├─ *** (item1) ════╝ + └─ (item5) + +Finished. Circular Dependencies printed: 2" +`; + +exports[`content-item tree command handler tests should print a single content item by itself 1`] = ` +"=== LEVEL 1 (1) === +item1 + +Finished. Circular Dependencies printed: 0" +`; + +exports[`content-item tree command handler tests should print a tree of content items 1`] = ` +"=== LEVEL 4 (1) === +item1 +├─ item2 +│ ├─ item4 +│ └─ item6 +│ └─ item5 +└─ item3 + +=== LEVEL 3 (1) === +=== LEVEL 2 (1) === +=== LEVEL 1 (3) === +Finished. Circular Dependencies printed: 0" +`; + +exports[`content-item tree command handler tests should print an error when invalid json is found 1`] = ` +"=== LEVEL 1 (1) === +item1 + +Finished. Circular Dependencies printed: 0" +`; + +exports[`content-item tree command handler tests should print an error when invalid json is found 2`] = `"Couldn't read content item at '/Users/rhys/Documents/amplience/dc-cli/temp/tree/invalud/repo1/badfile.json': SyntaxError: Unexpected token o in JSON at position 1"`; + +exports[`content-item tree command handler tests should print multiple disjoint trees of content items 1`] = ` +"=== LEVEL 3 (1) === +item1 +├─ item2 +│ └─ item4 +└─ item3 + +=== LEVEL 2 (2) === +item6 +└─ item5 + +=== LEVEL 1 (4) === +item7 + +Finished. Circular Dependencies printed: 0" +`; + +exports[`content-item tree command handler tests should print nothing if no content is present 1`] = `"Finished. Circular Dependencies printed: 0"`; diff --git a/src/commands/content-item/tree.spec.ts b/src/commands/content-item/tree.spec.ts new file mode 100644 index 00000000..62e85e8d --- /dev/null +++ b/src/commands/content-item/tree.spec.ts @@ -0,0 +1,309 @@ +// Copy tests are rather simple since they most of the work is done by import/export. +// Unique features are revert, throwing when parameters are wrong/missing, +// and forwarding input parameters to both import and export. + +import { builder, command, handler, firstSecondThird, fillWhitespace, LOG_FILENAME } from './tree'; +import Yargs from 'yargs/yargs'; + +import { writeFile } from 'fs'; +import { join } from 'path'; +import { promisify } from 'util'; + +import { ensureDirectoryExists } from '../../common/import/directory-utils'; +import rmdir from 'rimraf'; +import { getDefaultLogPath } from '../../common/log-helpers'; + +import { ItemTemplate } from '../../common/dc-management-sdk-js/mock-content'; +import { dependsOn } from '../../commands/content-item/__mocks__/dependant-content-helper'; +import { ContentItem, Status } from 'dc-management-sdk-js'; + +jest.mock('../../services/dynamic-content-client-factory'); +jest.mock('../../common/log-helpers'); + +const consoleLogSpy = jest.spyOn(console, 'log'); +const consoleErrorSpy = jest.spyOn(console, 'error'); + +function rimraf(dir: string): Promise { + return new Promise((resolve): void => { + rmdir(dir, resolve); + }); +} + +describe('content-item tree command', () => { + afterEach((): void => { + jest.resetAllMocks(); + }); + + it('should command should defined', function() { + expect(command).toEqual('tree '); + }); + + describe('builder tests', function() { + it('should configure yargs', function() { + const argv = Yargs(process.argv.slice(2)); + const spyPositional = jest.spyOn(argv, 'positional').mockReturnThis(); + + builder(argv); + + expect(spyPositional).toHaveBeenCalledWith('dir', { + type: 'string', + describe: 'Path to the content items to build a tree from. Should be in the same format as an export.' + }); + }); + }); + + describe('firstSecondThird tests', function() { + it('should return 0 for the first item in a list, above size 1', () => { + expect(firstSecondThird(0, 2)).toEqual(0); + expect(firstSecondThird(0, 3)).toEqual(0); + expect(firstSecondThird(0, 4)).toEqual(0); + }); + + it('should return 2 for the last item in a list', () => { + expect(firstSecondThird(0, 1)).toEqual(2); + expect(firstSecondThird(1, 2)).toEqual(2); + expect(firstSecondThird(2, 3)).toEqual(2); + expect(firstSecondThird(3, 4)).toEqual(2); + }); + + it('should return 1 for any middle item in a list, above size 2', () => { + expect(firstSecondThird(1, 3)).toEqual(1); + expect(firstSecondThird(1, 4)).toEqual(1); + expect(firstSecondThird(2, 4)).toEqual(1); + }); + }); + + describe('fillWhitespace tests', function() { + it('should fill space characters only after the original string with the given character up to the length', () => { + expect(fillWhitespace(' ', ' ', '-', 4)).toEqual(' '); + expect(fillWhitespace(' ', ' ', '-', 8)).toEqual(' ----'); + }); + + it('should inherit non-space characters from the current string', () => { + expect(fillWhitespace(' ', ' char', '-', 4)).toEqual(' char'); + expect(fillWhitespace(' ', ' char', '-', 8)).toEqual(' char'); + expect(fillWhitespace(' ', ' c a ', '-', 8)).toEqual(' c-a-'); + expect(fillWhitespace(' ', ' h r', '-', 8)).toEqual(' -h-r'); + expect(fillWhitespace(' ', ' ', '-', 8)).toEqual(' ----'); + }); + }); + + describe('handler tests', function() { + const yargArgs = { + $0: 'test', + _: ['test'], + json: true + }; + const config = { + clientId: 'client-id', + clientSecret: 'client-id', + hubId: 'hub-id' + }; + + beforeAll(async () => { + await rimraf('temp/tree/'); + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + const itemFromTemplate = (template: ItemTemplate): ContentItem => { + const item = new ContentItem({ + label: template.label, + status: template.status || Status.ACTIVE, + id: template.id || template.label, + folderId: null, + version: template.version, + lastPublishedVersion: template.lastPublishedVersion, + locale: template.locale, + body: { + ...template.body, + _meta: { + schema: template.typeSchemaUri + } + }, + + // Not meant to be here, but used later for sorting by repository + repoId: template.repoId + }); + + return item; + }; + + const createContent = async (basePath: string, template: ItemTemplate): Promise => { + await promisify(writeFile)( + join(basePath, template.label + '.json'), + JSON.stringify(itemFromTemplate(template).toJSON()) + ); + }; + + it('should use getDefaultLogPath for LOG_FILENAME with process.platform as default', function() { + LOG_FILENAME(); + + expect(getDefaultLogPath).toHaveBeenCalledWith('item', 'tree', process.platform); + }); + + it('should print nothing if no content is present', async () => { + await ensureDirectoryExists('temp/tree/empty'); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/empty' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should print a single content item by itself', async () => { + const basePath = 'temp/tree/single/repo1'; + await ensureDirectoryExists(basePath); + + await promisify(writeFile)(join(basePath, 'dummyFile.txt'), 'ignored'); + + await createContent(basePath, { + label: 'item1', + id: 'id1', + repoId: 'repo1', + body: {}, + typeSchemaUri: 'http://type.com' + }); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/single' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should print a tree of content items', async () => { + const basePath = 'temp/tree/multiple/repo1'; + await ensureDirectoryExists(basePath); + + const shared = { typeSchemaUri: 'http://type.com', repoId: 'repo1' }; + + await createContent(basePath, { label: 'item1', id: 'id1', body: dependsOn(['id2', 'id3']), ...shared }); + await createContent(basePath, { label: 'item2', id: 'id2', body: dependsOn(['id4', 'id6']), ...shared }); + await createContent(basePath, { label: 'item3', id: 'id3', body: {}, ...shared }); + await createContent(basePath, { label: 'item4', id: 'id4', body: {}, ...shared }); + await createContent(basePath, { label: 'item5', id: 'id5', body: {}, ...shared }); + await createContent(basePath, { label: 'item6', id: 'id6', body: dependsOn(['id5']), ...shared }); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/multiple' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should print multiple disjoint trees of content items', async () => { + const basePath = 'temp/tree/disjoint/repo1'; + await ensureDirectoryExists(basePath); + + const shared = { typeSchemaUri: 'http://type.com', repoId: 'repo1' }; + + await createContent(basePath, { label: 'item1', id: 'id1', body: dependsOn(['id2', 'id3']), ...shared }); + await createContent(basePath, { label: 'item2', id: 'id2', body: dependsOn(['id4']), ...shared }); + await createContent(basePath, { label: 'item3', id: 'id3', body: {}, ...shared }); + await createContent(basePath, { label: 'item4', id: 'id4', body: {}, ...shared }); + + await createContent(basePath, { label: 'item5', id: 'id5', body: {}, ...shared }); + await createContent(basePath, { label: 'item6', id: 'id6', body: dependsOn(['id5']), ...shared }); + + await createContent(basePath, { label: 'item7', id: 'id7', body: {}, ...shared }); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/disjoint' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should detect and print circular dependencies with a double line indicator', async () => { + const basePath = 'temp/tree/disjoint/repo1'; + await ensureDirectoryExists(basePath); + + const shared = { typeSchemaUri: 'http://type.com', repoId: 'repo1' }; + + await createContent(basePath, { label: 'item1', id: 'id1', body: dependsOn(['id2', 'id3']), ...shared }); + await createContent(basePath, { label: 'item2', id: 'id2', body: dependsOn(['id4']), ...shared }); + await createContent(basePath, { label: 'item3', id: 'id3', body: {}, ...shared }); + await createContent(basePath, { label: 'item4', id: 'id4', body: dependsOn(['id1']), ...shared }); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/disjoint' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should detect intertwined circular dependencies with multiple lines with different position', async () => { + const basePath = 'temp/tree/intertwine/repo1'; + await ensureDirectoryExists(basePath); + + const shared = { typeSchemaUri: 'http://type.com', repoId: 'repo1' }; + + await createContent(basePath, { label: 'item1', id: 'id1', body: dependsOn(['id2']), ...shared }); + await createContent(basePath, { label: 'item2', id: 'id2', body: dependsOn(['id3']), ...shared }); + await createContent(basePath, { label: 'item3', id: 'id3', body: dependsOn(['id2', 'id4']), ...shared }); + await createContent(basePath, { label: 'item4', id: 'id4', body: dependsOn(['id1', 'id5']), ...shared }); + + await createContent(basePath, { label: 'item5', id: 'id5', body: dependsOn(['id6']), ...shared }); + await createContent(basePath, { label: 'item6', id: 'id6', body: dependsOn(['id5']), ...shared }); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/intertwine' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + + it('should print an error when invalid json is found', async () => { + const basePath = 'temp/tree/invalud/repo1'; + await ensureDirectoryExists(basePath); + + await createContent(basePath, { + label: 'item1', + id: 'id1', + repoId: 'repo1', + body: {}, + typeSchemaUri: 'http://type.com' + }); + await promisify(writeFile)(join(basePath, 'badfile.json'), 'not json'); + + const argv = { + ...yargArgs, + ...config, + dir: 'temp/tree/invalud' + }; + + await handler(argv); + + expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + expect(consoleErrorSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + }); + }); +}); diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index a3b006f1..85bad30a 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -13,10 +13,6 @@ import { } from '../../common/content-item/content-dependancy-tree'; import { ContentMapping } from '../../common/content-item/content-mapping'; -export function getTempFolder(name: string, platform: string = process.platform): string { - return join(process.env[platform == 'win32' ? 'USERPROFILE' : 'HOME'] || __dirname, '.amplience', `copy-${name}/`); -} - export const command = 'tree '; export const desc = 'Print a content dependency tree from content in the given folder.'; @@ -27,7 +23,7 @@ export const LOG_FILENAME = (platform: string = process.platform): string => export const builder = (yargs: Argv): void => { yargs.positional('dir', { type: 'string', - describe: 'Path to the content items to build a tree from.. Should be in the same format as an export.' + describe: 'Path to the content items to build a tree from. Should be in the same format as an export.' }); }; @@ -35,9 +31,11 @@ interface TreeOptions { dir: string; } -const traverseRecursive = async (path: string, action: (path: string) => Promise): Promise => { +export const traverseRecursive = async (path: string, action: (path: string) => Promise): Promise => { const dir = await promisify(readdir)(path); + dir.sort(); + await Promise.all( dir.map(async (contained: string) => { contained = join(path, contained); @@ -47,54 +45,33 @@ const traverseRecursive = async (path: string, action: (path: string) => Promise ); }; -const prepareContentForTree = async ( - repos: { basePath: string; repo: ContentRepository }[], +export const prepareContentForTree = async ( + repo: { basePath: string; repo: ContentRepository }, argv: Arguments -): Promise => { +): Promise => { const contentItems: RepositoryContentItem[] = []; const schemaNames = new Set(); - for (let i = 0; i < repos.length; i++) { - const repo = repos[i].repo; - - await traverseRecursive(resolve(repos[i].basePath), async path => { - // Is this valid content? Must have extension .json to be considered, for a start. - if (extname(path) !== '.json') { - return; - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let contentJSON: any; - try { - const contentText = await promisify(readFile)(path, { encoding: 'utf8' }); - contentJSON = JSON.parse(contentText); - } catch (e) { - console.error(`Couldn't read content item at '${path}': ${e.toString()}`); - return; - } + await traverseRecursive(resolve(repo.basePath), async path => { + // Is this valid content? Must have extension .json to be considered, for a start. + if (extname(path) !== '.json') { + return; + } - // Get the folder id via the mapping. - - // Only filter relevant information - for example status and previous content repo are not useful. - const filteredContent = { - id: contentJSON.id, - label: contentJSON.label, - locale: contentJSON.locale, - body: contentJSON.body, - deliveryId: contentJSON.deliveryId == contentJSON.Id || argv.excludeKeys ? undefined : contentJSON.deliveryId, - folderId: null, - publish: contentJSON.lastPublishedVersion != null - }; - - if (argv.excludeKeys) { - delete filteredContent.body._meta.deliveryKey; - } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let contentJSON: any; + try { + const contentText = await promisify(readFile)(path, { encoding: 'utf8' }); + contentJSON = JSON.parse(contentText); + } catch (e) { + console.error(`Couldn't read content item at '${path}': ${e.toString()}`); + return; + } - schemaNames.add(contentJSON.body._meta.schema); + schemaNames.add(contentJSON.body._meta.schema); - contentItems.push({ repo: repo, content: new ContentItem(filteredContent) }); - }); - } + contentItems.push({ repo: repo.repo, content: new ContentItem(contentJSON) }); + }); return new ContentDependancyTree(contentItems, new ContentMapping()); }; @@ -105,15 +82,15 @@ interface ParentReference { line: number; } -const firstSecondThird = (index: number, total: number): number => { - return index === 0 ? 0 : index == total - 1 ? 2 : 1; +export const firstSecondThird = (index: number, total: number): number => { + return index == total - 1 ? 2 : index === 0 ? 0 : 1; }; const fstPipes = ['├', '├', '└']; const circularPipes = ['╗', '║', '╝']; const circularLine = '═'; -const printDependency = ( +export const printDependency = ( item: ItemContentDependancies, evaluated: Set, lines: string[], @@ -144,22 +121,24 @@ const printDependency = ( const filteredItems = item.dependancies.filter(dep => dep.resolved); filteredItems.forEach((dep, index) => { - if (dep.resolved) { - const subFst = firstSecondThird(index, filteredItems.length); - const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; - printDependency(dep.resolved, evaluated, lines, circularLinks, [...evalThis], subFst, prefix + subPrefix); - } + const subFst = firstSecondThird(index, filteredItems.length); + const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; + printDependency( + dep.resolved as ItemContentDependancies, + evaluated, + lines, + circularLinks, + [...evalThis], + subFst, + prefix + subPrefix + ); }); return true; }; -const fillWhitespace = (original: string, current: string, char: string, targetLength: number): string => { - if (current.length < original.length + 1) { - current += ' '; - } - - let position = original.length + 1; - let repeats = targetLength - (original.length + 1); +export const fillWhitespace = (original: string, current: string, char: string, targetLength: number): string => { + let position = original.length; + let repeats = targetLength - original.length; // Replace existing whitespace characters while (position < current.length && repeats > 0) { @@ -178,14 +157,15 @@ const fillWhitespace = (original: string, current: string, char: string, targetL return current; }; -const printTree = (item: ItemContentDependancies, evaluated: Set): boolean => { - const lines: string[] = []; +export const printTree = (item: ItemContentDependancies, evaluated: Set): boolean => { + let lines: string[] = []; const circularLinks: CircularLink[] = []; const result = printDependency(item, evaluated, lines, circularLinks, [], 0, ''); if (!result) return false; + lines = lines.map(line => line + ' '); const modifiedLines = [...lines]; // Render circular references. @@ -227,25 +207,12 @@ const printTree = (item: ItemContentDependancies, evaluated: Set): Promise => { const dir = argv.dir; - const baseDirContents = await promisify(readdir)(dir); - const importRepos: { basePath: string; repo: ContentRepository }[] = []; - for (let i = 0; i < baseDirContents.length; i++) { - const name = baseDirContents[i]; - const path = join(dir, name); - const status = await promisify(lstat)(path); - if (status.isDirectory()) { - importRepos.push({ basePath: path, repo: new ContentRepository() }); - } - } - - const tree = await prepareContentForTree(importRepos, argv); + const tree = await prepareContentForTree({ basePath: dir, repo: new ContentRepository() }, argv); // Print the items in the tree. // Keep a set of all items that have already been printed. // Starting at the highest level, print all dependencies on the tree. - if (tree == null) return; - const evaluated = new Set(); for (let i = tree.levels.length - 1; i >= 0; i--) { @@ -257,13 +224,17 @@ export const handler = async (argv: Arguments { - if (printTree(item, evaluated)) { - topLevelPrints++; - } - }); + + if (tree.circularLinks.length > 0) { + console.log(`=== CIRCULAR (${tree.circularLinks.length}) ===`); + + tree.circularLinks.forEach(item => { + if (printTree(item, evaluated)) { + topLevelPrints++; + } + }); + } console.log(`Finished. Circular Dependencies printed: ${topLevelPrints}`); }; diff --git a/src/common/content-item/content-dependancy-tree.ts b/src/common/content-item/content-dependancy-tree.ts index 0f7afac2..fda52e3d 100644 --- a/src/common/content-item/content-dependancy-tree.ts +++ b/src/common/content-item/content-dependancy-tree.ts @@ -211,8 +211,13 @@ export class ContentDependancyTree { ): CircularDependencyStage { let selfLoop = false; let intertwinedLoop = false; + let isParent = false; const seenBefore = new Set(); + if (top.owner.content.label == 'item5') { + console.log('test'); + } + const traverse = ( top: ItemContentDependancies, item: ItemContentDependancies | undefined, @@ -234,8 +239,15 @@ export class ContentDependancyTree { if (!intertwined) { // Does it loop back to the parent? const storedSelfLoop = selfLoop; - intertwinedLoop = traverse(item, item, 0, [top], new Set(), true); + const childIntertwined = traverse(item, item, 0, [top], new Set(), true); selfLoop = storedSelfLoop; + + if (childIntertwined) { + intertwinedLoop = true; + } else { + // We're the parent of a non-intertwined circular loop. + isParent = true; + } } hasCircular = true; @@ -248,7 +260,7 @@ export class ContentDependancyTree { seenBefore.add(item); item.dependancies.forEach(dep => { - hasCircular = traverse(top, dep.resolved, depth++, unresolved, seenBefore, intertwined) || hasCircular; + hasCircular = traverse(top, dep.resolved, depth + 1, unresolved, seenBefore, intertwined) || hasCircular; }); return hasCircular; @@ -256,9 +268,13 @@ export class ContentDependancyTree { const hasCircular = traverse(top, top, 0, unresolved, seenBefore, false); + if (top.owner.content.label == 'item5') { + console.log('test'); + } + if (hasCircular) { if (intertwinedLoop) { - if (selfLoop) { + if (selfLoop && !isParent) { return CircularDependencyStage.Intertwined; } else { return CircularDependencyStage.Parent; From 05cad094104b0d335f0d68172f69f20a86490bf4 Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 9 Apr 2021 11:49:23 +0100 Subject: [PATCH 04/11] fix(content-item): fix load order for tree --- src/commands/content-item/tree.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index 85bad30a..1fcc4a1d 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -36,13 +36,16 @@ export const traverseRecursive = async (path: string, action: (path: string) => dir.sort(); - await Promise.all( - dir.map(async (contained: string) => { - contained = join(path, contained); - const stat = await promisify(lstat)(contained); - return await (stat.isDirectory() ? traverseRecursive(contained, action) : action(contained)); - }) - ); + for (let i = 0; i < dir.length; i++) { + let contained = dir[i]; + contained = join(path, contained); + const stat = await promisify(lstat)(contained); + if (stat.isDirectory()) { + await traverseRecursive(contained, action); + } else { + await action(contained); + } + } }; export const prepareContentForTree = async ( From fbf1584141ae55c6dee356b5a96dd56987b1793a Mon Sep 17 00:00:00 2001 From: Rhys Date: Fri, 9 Apr 2021 16:12:05 +0100 Subject: [PATCH 05/11] fix(content-item): remove testing logs --- src/commands/content-item/__snapshots__/tree.spec.ts.snap | 4 +--- src/common/content-item/content-dependancy-tree.ts | 8 -------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/commands/content-item/__snapshots__/tree.spec.ts.snap b/src/commands/content-item/__snapshots__/tree.spec.ts.snap index eec692aa..302c916c 100644 --- a/src/commands/content-item/__snapshots__/tree.spec.ts.snap +++ b/src/commands/content-item/__snapshots__/tree.spec.ts.snap @@ -21,9 +21,7 @@ Finished. Circular Dependencies printed: 1" `; exports[`content-item tree command handler tests should detect intertwined circular dependencies with multiple lines with different position 1`] = ` -"test -test -=== CIRCULAR (6) === +"=== CIRCULAR (6) === item5 ══════════════╗ └─ item6 ║ └─ *** (item5) ══╝ diff --git a/src/common/content-item/content-dependancy-tree.ts b/src/common/content-item/content-dependancy-tree.ts index fda52e3d..73a90e74 100644 --- a/src/common/content-item/content-dependancy-tree.ts +++ b/src/common/content-item/content-dependancy-tree.ts @@ -214,10 +214,6 @@ export class ContentDependancyTree { let isParent = false; const seenBefore = new Set(); - if (top.owner.content.label == 'item5') { - console.log('test'); - } - const traverse = ( top: ItemContentDependancies, item: ItemContentDependancies | undefined, @@ -268,10 +264,6 @@ export class ContentDependancyTree { const hasCircular = traverse(top, top, 0, unresolved, seenBefore, false); - if (top.owner.content.label == 'item5') { - console.log('test'); - } - if (hasCircular) { if (intertwinedLoop) { if (selfLoop && !isParent) { From b31e54ca4303ffec6b1a05eb63c69add5b3e68e8 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 12 Apr 2021 02:18:25 +0100 Subject: [PATCH 06/11] feat(config): allow using configure command with a custom file --- src/commands/configure.spec.ts | 28 +++++++++++++++++++++++----- src/commands/configure.ts | 10 +++++++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/commands/configure.spec.ts b/src/commands/configure.spec.ts index 2591c163..ffac620d 100644 --- a/src/commands/configure.spec.ts +++ b/src/commands/configure.spec.ts @@ -30,7 +30,7 @@ describe('configure command', function() { jest.spyOn(fs, 'mkdirSync').mockReturnValueOnce(undefined); jest.spyOn(fs, 'writeFileSync').mockReturnValueOnce(undefined); - handler({ ...yargArgs, ...configFixture }); + handler({ ...yargArgs, ...configFixture, config: CONFIG_FILENAME() }); expect(fs.existsSync).toHaveBeenCalledWith(expect.stringMatching(/\.amplience$/)); expect(fs.mkdirSync).toHaveBeenCalledWith(expect.stringMatching(/\.amplience$/), { recursive: true }); @@ -48,7 +48,7 @@ describe('configure command', function() { jest.spyOn(fs, 'mkdirSync'); jest.spyOn(fs, 'writeFileSync').mockReturnValueOnce(undefined); - handler({ ...yargArgs, ...configFixture }); + handler({ ...yargArgs, ...configFixture, config: CONFIG_FILENAME() }); expect(fs.existsSync).toHaveBeenCalledWith(expect.stringMatching(/\.amplience$/)); expect(fs.mkdirSync).not.toHaveBeenCalled(); @@ -58,6 +58,24 @@ describe('configure command', function() { ); }); + it('should write a config file and use the specified file', () => { + jest + .spyOn(fs, 'existsSync') + .mockReturnValueOnce(false) + .mockReturnValueOnce(false); + jest.spyOn(fs, 'mkdirSync').mockReturnValueOnce(undefined); + jest.spyOn(fs, 'writeFileSync').mockReturnValueOnce(undefined); + + handler({ ...yargArgs, ...configFixture, config: 'subdirectory/custom-config.json' }); + + expect(fs.existsSync).toHaveBeenCalledWith(expect.stringMatching(/subdirectory$/)); + expect(fs.mkdirSync).toHaveBeenCalledWith(expect.stringMatching(/subdirectory$/), { recursive: true }); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.stringMatching(new RegExp('subdirectory/custom-config.json$')), + JSON.stringify(configFixture) + ); + }); + it('should report an error if its not possible to create the .amplience dir', () => { jest .spyOn(fs, 'existsSync') @@ -69,7 +87,7 @@ describe('configure command', function() { jest.spyOn(fs, 'writeFileSync').mockReturnValueOnce(undefined); expect(() => { - handler({ ...yargArgs, ...configFixture }); + handler({ ...yargArgs, ...configFixture, config: CONFIG_FILENAME() }); }).toThrowError(/^Unable to create dir ".*". Reason: .*/); expect(fs.existsSync).toHaveBeenCalledWith(expect.stringMatching(/\.amplience$/)); @@ -88,7 +106,7 @@ describe('configure command', function() { }); expect(() => { - handler({ ...yargArgs, ...configFixture }); + handler({ ...yargArgs, ...configFixture, config: CONFIG_FILENAME() }); }).toThrowError(/^Unable to write config file ".*". Reason: .*/); expect(fs.existsSync).toHaveBeenCalledWith(expect.stringMatching(/\.amplience$/)); @@ -104,7 +122,7 @@ describe('configure command', function() { jest.spyOn(fs, 'readFileSync').mockReturnValueOnce(JSON.stringify(configFixture)); jest.spyOn(fs, 'writeFileSync'); - handler({ ...yargArgs, ...configFixture }); + handler({ ...yargArgs, ...configFixture, config: CONFIG_FILENAME() }); expect(fs.writeFileSync).not.toHaveBeenCalled(); }); diff --git a/src/commands/configure.ts b/src/commands/configure.ts index f8546ba8..b59749aa 100644 --- a/src/commands/configure.ts +++ b/src/commands/configure.ts @@ -17,6 +17,10 @@ export type ConfigurationParameters = { hubId: string; }; +type ConfigArgument = { + config: string; +}; + export const configureCommandOptions: CommandOptions = { clientId: { type: 'string', demandOption: true }, clientSecret: { type: 'string', demandOption: true }, @@ -43,14 +47,14 @@ const writeConfigFile = (configFile: string, parameters: ConfigurationParameters export const readConfigFile = (configFile: string): object => fs.existsSync(configFile) ? JSON.parse(fs.readFileSync(configFile, 'utf-8')) : {}; -export const handler = (argv: Arguments): void => { +export const handler = (argv: Arguments): void => { const { clientId, clientSecret, hubId } = argv; - const storedConfig = readConfigFile(CONFIG_FILENAME()); + const storedConfig = readConfigFile(argv.config); if (isEqual(storedConfig, { clientId, clientSecret, hubId })) { console.log('Config file up-to-date. Please use `--help` for command usage.'); return; } - writeConfigFile(CONFIG_FILENAME(), { clientId, clientSecret, hubId }); + writeConfigFile(argv.config, { clientId, clientSecret, hubId }); console.log('Config file updated.'); }; From 81d12e0da376a5c790d5e658ce00c295c0c0cbe7 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 21 Apr 2021 15:18:12 +0100 Subject: [PATCH 07/11] fix: fix references setting null when already imported --- src/commands/content-item/import.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/content-item/import.ts b/src/commands/content-item/import.ts index acb23b09..5bfdf193 100644 --- a/src/commands/content-item/import.ts +++ b/src/commands/content-item/import.ts @@ -791,7 +791,7 @@ const importTree = async ( const content = item.owner.content; item.dependancies.forEach(dep => { - rewriteDependancy(dep, mapping, true); + rewriteDependancy(dep, mapping, pass === 0); }); const originalId = content.id; From 5d4f519c6a1e4b9d9dd6fec005a19660e35cdc53 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 2 Jun 2021 00:12:59 +0100 Subject: [PATCH 08/11] refactor: address feedback 1 --- src/commands/content-item/import.ts | 8 +++++--- src/commands/content-item/tree.ts | 11 +++++------ src/common/content-item/content-dependancy-tree.ts | 14 +++++++------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/commands/content-item/import.ts b/src/commands/content-item/import.ts index 5bfdf193..6e37d4da 100644 --- a/src/commands/content-item/import.ts +++ b/src/commands/content-item/import.ts @@ -26,6 +26,7 @@ import { ItemContentDependancies, ContentDependancyInfo } from '../../common/content-item/content-dependancy-tree'; +import { Body } from '../../common/content-item/body'; import { AmplienceSchemaValidator, defaultSchemaLookup } from '../../common/content-item/amplience-schema-validator'; import { createLog, getDefaultLogPath } from '../../common/log-helpers'; @@ -684,11 +685,12 @@ const rewriteDependancy = (dep: ContentDependancyInfo, mapping: ContentMapping, if (dep.dependancy._meta.schema === '_hierarchy') { dep.owner.content.body._meta.hierarchy.parentId = id; - } else { + } else if (dep.parent) { + const parent = dep.parent as Body; if (id == null) { - delete dep.parent[dep.index]; + delete parent[dep.index]; } else { - dep.parent[dep.index] = dep.dependancy; + parent[dep.index] = dep.dependancy; dep.dependancy.id = id; } } diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index 1fcc4a1d..ef6f5a47 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -49,8 +49,7 @@ export const traverseRecursive = async (path: string, action: (path: string) => }; export const prepareContentForTree = async ( - repo: { basePath: string; repo: ContentRepository }, - argv: Arguments + repo: { basePath: string; repo: ContentRepository } ): Promise => { const contentItems: RepositoryContentItem[] = []; const schemaNames = new Set(); @@ -93,7 +92,7 @@ const fstPipes = ['├', '├', '└']; const circularPipes = ['╗', '║', '╝']; const circularLine = '═'; -export const printDependency = ( +export const addDependency = ( item: ItemContentDependancies, evaluated: Set, lines: string[], @@ -126,7 +125,7 @@ export const printDependency = ( filteredItems.forEach((dep, index) => { const subFst = firstSecondThird(index, filteredItems.length); const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; - printDependency( + addDependency( dep.resolved as ItemContentDependancies, evaluated, lines, @@ -164,7 +163,7 @@ export const printTree = (item: ItemContentDependancies, evaluated: Set): Promise => { const dir = argv.dir; - const tree = await prepareContentForTree({ basePath: dir, repo: new ContentRepository() }, argv); + const tree = await prepareContentForTree({ basePath: dir, repo: new ContentRepository() }); // Print the items in the tree. // Keep a set of all items that have already been printed. diff --git a/src/common/content-item/content-dependancy-tree.ts b/src/common/content-item/content-dependancy-tree.ts index 73a90e74..4ddb825a 100644 --- a/src/common/content-item/content-dependancy-tree.ts +++ b/src/common/content-item/content-dependancy-tree.ts @@ -13,7 +13,7 @@ export interface RepositoryContentItem { } export interface ContentDependancy { - _meta: { schema: DependancyContentTypeSchema }; + _meta: { schema: DependancyContentTypeSchema; name: string }; contentType: string; id: string | undefined; } @@ -23,8 +23,7 @@ export interface ContentDependancyInfo { dependancy: ContentDependancy; owner: RepositoryContentItem; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - parent: any; + parent?: RecursiveSearchStep; index: string | number; } @@ -149,7 +148,7 @@ export class ContentDependancyTree { item: RepositoryContentItem, body: RecursiveSearchStep, result: ContentDependancyInfo[], - parent: RecursiveSearchStep | null, + parent: RecursiveSearchStep | undefined, index: string | number ): void { if (Array.isArray(body)) { @@ -282,20 +281,21 @@ export class ContentDependancyTree { private identifyContentDependancies(items: RepositoryContentItem[]): ItemContentDependancies[] { return items.map(item => { const result: ContentDependancyInfo[] = []; - this.searchObjectForContentDependancies(item, item.content.body, result, null, 0); + this.searchObjectForContentDependancies(item, item.content.body, result, undefined, 0); // Hierarchy parent is also a dependancy. if (item.content.body._meta.hierarchy && item.content.body._meta.hierarchy.parentId) { result.push({ dependancy: { _meta: { - schema: '_hierarchy' + schema: '_hierarchy', + name: '_hierarchy' }, id: item.content.body._meta.hierarchy.parentId, contentType: '' }, owner: item, - parent: null, + parent: undefined, index: 0 }); } From bffd2bf795c9d27bb4506552956af326cd5b0df3 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 2 Jun 2021 09:56:04 +0100 Subject: [PATCH 09/11] style: use tree builder class for managing state --- .../__mocks__/dependant-content-helper.ts | 3 +- src/commands/content-item/tree.ts | 92 +++++++++---------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/commands/content-item/__mocks__/dependant-content-helper.ts b/src/commands/content-item/__mocks__/dependant-content-helper.ts index 23b1979e..c1dd3277 100644 --- a/src/commands/content-item/__mocks__/dependant-content-helper.ts +++ b/src/commands/content-item/__mocks__/dependant-content-helper.ts @@ -3,7 +3,8 @@ import { ContentDependancy } from '../../../common/content-item/content-dependan function dependancy(id: string): ContentDependancy { return { _meta: { - schema: 'http://bigcontent.io/cms/schema/v1/core#/definitions/content-link' + schema: 'http://bigcontent.io/cms/schema/v1/core#/definitions/content-link', + name: 'content-link' }, contentType: 'https://dev-solutions.s3.amazonaws.com/DynamicContentTypes/Accelerators/blog.json', id: id diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index ef6f5a47..a252a97d 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -48,9 +48,10 @@ export const traverseRecursive = async (path: string, action: (path: string) => } }; -export const prepareContentForTree = async ( - repo: { basePath: string; repo: ContentRepository } -): Promise => { +export const prepareContentForTree = async (repo: { + basePath: string; + repo: ContentRepository; +}): Promise => { const contentItems: RepositoryContentItem[] = []; const schemaNames = new Set(); @@ -92,51 +93,42 @@ const fstPipes = ['├', '├', '└']; const circularPipes = ['╗', '║', '╝']; const circularLine = '═'; -export const addDependency = ( - item: ItemContentDependancies, - evaluated: Set, - lines: string[], - circularLinks: CircularLink[], - evalThis: ParentReference[], - fst: number, - prefix: string -): boolean => { - const depth = evalThis.length - 1; - const pipe = depth < 0 ? '' : fstPipes[fst] + '─ '; - - const circularMatch = evalThis.find(parent => parent.item == item); - if (circularMatch) { - lines.push(`${prefix}${pipe}*** (${item.owner.content.label})`); - circularLinks.push([circularMatch.line, lines.length - 1]); - return false; - } else if (evaluated.has(item)) { - if (depth > -1) { - lines.push(`${prefix}${pipe}(${item.owner.content.label})`); +export class TreeBuilder { + lines: string[] = []; + circularLinks: CircularLink[] = []; + + constructor(public evaluated: Set) {} + + addDependency(item: ItemContentDependancies, evalThis: ParentReference[], fst: number, prefix: string): boolean { + const depth = evalThis.length - 1; + const pipe = depth < 0 ? '' : fstPipes[fst] + '─ '; + + const circularMatch = evalThis.find(parent => parent.item == item); + if (circularMatch) { + this.lines.push(`${prefix}${pipe}*** (${item.owner.content.label})`); + this.circularLinks.push([circularMatch.line, this.lines.length - 1]); + return false; + } else if (this.evaluated.has(item)) { + if (depth > -1) { + this.lines.push(`${prefix}${pipe}(${item.owner.content.label})`); + } + return false; + } else { + this.lines.push(`${prefix}${pipe}${item.owner.content.label}`); } - return false; - } else { - lines.push(`${prefix}${pipe}${item.owner.content.label}`); - } - evalThis.push({ item, line: lines.length - 1 }); - evaluated.add(item); - - const filteredItems = item.dependancies.filter(dep => dep.resolved); - filteredItems.forEach((dep, index) => { - const subFst = firstSecondThird(index, filteredItems.length); - const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; - addDependency( - dep.resolved as ItemContentDependancies, - evaluated, - lines, - circularLinks, - [...evalThis], - subFst, - prefix + subPrefix - ); - }); - return true; -}; + evalThis.push({ item, line: this.lines.length - 1 }); + this.evaluated.add(item); + + const filteredItems = item.dependancies.filter(dep => dep.resolved); + filteredItems.forEach((dep, index) => { + const subFst = firstSecondThird(index, filteredItems.length); + const subPrefix = depth == -1 ? '' : fst === 2 ? ' ' : '│ '; + this.addDependency(dep.resolved as ItemContentDependancies, [...evalThis], subFst, prefix + subPrefix); + }); + return true; + } +} export const fillWhitespace = (original: string, current: string, char: string, targetLength: number): string => { let position = original.length; @@ -160,14 +152,14 @@ export const fillWhitespace = (original: string, current: string, char: string, }; export const printTree = (item: ItemContentDependancies, evaluated: Set): boolean => { - let lines: string[] = []; - const circularLinks: CircularLink[] = []; + const builder = new TreeBuilder(evaluated); - const result = addDependency(item, evaluated, lines, circularLinks, [], 0, ''); + const result = builder.addDependency(item, [], 0, ''); if (!result) return false; - lines = lines.map(line => line + ' '); + const circularLinks = builder.circularLinks; + const lines = builder.lines.map(line => line + ' '); const modifiedLines = [...lines]; // Render circular references. From 60f96162149058b061644c3a19d592b628d35884 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 16 Jun 2021 16:27:19 +0100 Subject: [PATCH 10/11] refactor: improve circular link type definition --- src/commands/content-item/tree.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/commands/content-item/tree.ts b/src/commands/content-item/tree.ts index a252a97d..79380bc9 100644 --- a/src/commands/content-item/tree.ts +++ b/src/commands/content-item/tree.ts @@ -79,7 +79,9 @@ export const prepareContentForTree = async (repo: { return new ContentDependancyTree(contentItems, new ContentMapping()); }; -type CircularLink = [number, number]; +type LineIndexFrom = number; +type LineIndexTo = number; +type CircularLink = [LineIndexFrom, LineIndexTo]; interface ParentReference { item: ItemContentDependancies; line: number; From 48720d78b229d1144f4d68efd2bce379fb528f03 Mon Sep 17 00:00:00 2001 From: Rhys Date: Thu, 17 Jun 2021 12:25:00 +0100 Subject: [PATCH 11/11] test: fix broken snapshots in tree tests --- src/commands/content-item/__snapshots__/tree.spec.ts.snap | 2 -- src/commands/content-item/tree.spec.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/commands/content-item/__snapshots__/tree.spec.ts.snap b/src/commands/content-item/__snapshots__/tree.spec.ts.snap index 302c916c..0357a9bc 100644 --- a/src/commands/content-item/__snapshots__/tree.spec.ts.snap +++ b/src/commands/content-item/__snapshots__/tree.spec.ts.snap @@ -66,8 +66,6 @@ item1 Finished. Circular Dependencies printed: 0" `; -exports[`content-item tree command handler tests should print an error when invalid json is found 2`] = `"Couldn't read content item at '/Users/rhys/Documents/amplience/dc-cli/temp/tree/invalud/repo1/badfile.json': SyntaxError: Unexpected token o in JSON at position 1"`; - exports[`content-item tree command handler tests should print multiple disjoint trees of content items 1`] = ` "=== LEVEL 3 (1) === item1 diff --git a/src/commands/content-item/tree.spec.ts b/src/commands/content-item/tree.spec.ts index 62e85e8d..3afccf61 100644 --- a/src/commands/content-item/tree.spec.ts +++ b/src/commands/content-item/tree.spec.ts @@ -303,7 +303,7 @@ describe('content-item tree command', () => { await handler(argv); expect(consoleLogSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); - expect(consoleErrorSpy.mock.calls.map(args => args[0]).join('\n')).toMatchSnapshot(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); }); }); });