From 61d2181f85b0eb7e30a0639cb0839a7259f3ea95 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Sat, 16 Jun 2018 19:24:22 -0400 Subject: [PATCH] fix(@angular-devkit/schematics): support VirtualTree/HostTree interop --- .../schematics/src/sink/dryrun_spec.ts | 16 +- .../schematics/src/sink/host_spec.ts | 27 +-- .../schematics/src/tree/host-tree.ts | 156 ++++++++---------- .../schematics/src/tree/host-tree_spec.ts | 38 +++++ .../tools/file-system-engine-host-base.ts | 4 +- .../tools/file-system-engine-host_spec.ts | 4 +- 6 files changed, 141 insertions(+), 104 deletions(-) create mode 100644 packages/angular_devkit/schematics/src/tree/host-tree_spec.ts diff --git a/packages/angular_devkit/schematics/src/sink/dryrun_spec.ts b/packages/angular_devkit/schematics/src/sink/dryrun_spec.ts index 724ea68914..dbfb2f1bc4 100644 --- a/packages/angular_devkit/schematics/src/sink/dryrun_spec.ts +++ b/packages/angular_devkit/schematics/src/sink/dryrun_spec.ts @@ -6,9 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ // tslint:disable:no-implicit-dependencies -import { normalize, virtualFs } from '@angular-devkit/core'; +import { Path, normalize, virtualFs } from '@angular-devkit/core'; import { toArray } from 'rxjs/operators'; -import { FileSystemCreateTree, FileSystemTree } from '../tree/filesystem'; +import { HostCreateTree, HostTree } from '../tree/host-tree'; import { optimize } from '../tree/static'; import { DryRunSink } from './dryrun'; @@ -22,7 +22,7 @@ const host = new virtualFs.test.TestHost({ describe('DryRunSink', () => { it('works when creating everything', done => { - const tree = new FileSystemCreateTree(host); + const tree = new HostCreateTree(host); tree.create('/test', 'testing 1 2'); const recorder = tree.beginUpdate('/test'); @@ -31,7 +31,9 @@ describe('DryRunSink', () => { tree.overwrite('/hello', 'world'); const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test']; - expect(tree.files.sort()).toEqual(files.map(normalize)); + const treeFiles: Path[] = []; + tree.visit(path => treeFiles.push(path)); + expect(treeFiles.sort()).toEqual(files.map(normalize)); const sink = new DryRunSink(new virtualFs.SimpleMemoryHost()); sink.reporter.pipe(toArray()) @@ -49,7 +51,7 @@ describe('DryRunSink', () => { }); it('works with root', done => { - const tree = new FileSystemTree(host); + const tree = new HostTree(host); tree.create('/test', 'testing 1 2'); const recorder = tree.beginUpdate('/test'); @@ -58,7 +60,9 @@ describe('DryRunSink', () => { tree.overwrite('/hello', 'world'); const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test']; - expect(tree.files.sort()).toEqual(files.map(normalize)); + const treeFiles: Path[] = []; + tree.visit(path => treeFiles.push(path)); + expect(treeFiles.sort()).toEqual(files.map(normalize)); // Need to create this file on the filesystem, otherwise the commit phase will fail. const outputHost = new virtualFs.SimpleMemoryHost(); diff --git a/packages/angular_devkit/schematics/src/sink/host_spec.ts b/packages/angular_devkit/schematics/src/sink/host_spec.ts index f8ee665968..03e08dd0f9 100644 --- a/packages/angular_devkit/schematics/src/sink/host_spec.ts +++ b/packages/angular_devkit/schematics/src/sink/host_spec.ts @@ -7,9 +7,9 @@ */ // tslint:disable:no-implicit-dependencies import { normalize, virtualFs } from '@angular-devkit/core'; -import { FileSystemTree, HostSink } from '@angular-devkit/schematics'; +import { HostSink } from '@angular-devkit/schematics'; import { fileBufferToString } from '../../../core/src/virtual-fs/host'; -import { FileSystemCreateTree } from '../tree/filesystem'; +import { HostCreateTree, HostTree } from '../tree/host-tree'; import { optimize } from '../tree/static'; @@ -20,7 +20,7 @@ describe('FileSystemSink', () => { '/sub/directory/file2': '', '/sub/file1': '', }); - const tree = new FileSystemCreateTree(host); + const tree = new HostCreateTree(host); tree.create('/test', 'testing 1 2'); const recorder = tree.beginUpdate('/test'); @@ -28,14 +28,16 @@ describe('FileSystemSink', () => { tree.commitUpdate(recorder); const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test']; - expect(tree.files).toEqual(files.map(normalize)); + const treeFiles: string[] = []; + tree.visit(path => treeFiles.push(path)); + expect(treeFiles.sort()).toEqual(files); const outputHost = new virtualFs.test.TestHost(); const sink = new HostSink(outputHost); sink.commit(optimize(tree)) .toPromise() .then(() => { - const tmpFiles = outputHost.files; + const tmpFiles = outputHost.files.sort(); expect(tmpFiles as string[]).toEqual(files); expect(outputHost.sync.read(normalize('/test')).toString()) .toBe('testing testing 1 2'); @@ -51,7 +53,7 @@ describe('FileSystemSink', () => { '/sub/directory/file2': '/sub/directory/file2', '/sub/file1': '/sub/file1', }); - const tree = new FileSystemCreateTree(host); + const tree = new HostCreateTree(host); const outputHost = new virtualFs.test.TestHost(); const sink = new HostSink(outputHost); @@ -64,7 +66,7 @@ describe('FileSystemSink', () => { const host = new virtualFs.test.TestHost({ '/file0': '/file0', }); - const tree = new FileSystemTree(host); + const tree = new HostTree(host); tree.rename('/file0', '/file1'); const sink = new HostSink(host); @@ -82,7 +84,7 @@ describe('FileSystemSink', () => { const host = new virtualFs.test.TestHost({ '/sub/directory/file2': '', }); - const tree = new FileSystemTree(host); + const tree = new HostTree(host); tree.rename('/sub/directory/file2', '/another-directory/file2'); const sink = new HostSink(host); @@ -99,7 +101,7 @@ describe('FileSystemSink', () => { const host = new virtualFs.test.TestHost({ '/file0': 'world', }); - const tree = new FileSystemTree(host); + const tree = new HostTree(host); tree.delete('/file0'); tree.create('/file0', 'hello'); @@ -116,9 +118,14 @@ describe('FileSystemSink', () => { const host = new virtualFs.test.TestHost({ '/file0': 'world', }); - const tree = new FileSystemTree(host); + const tree = new HostTree(host); + tree.rename('/file0', '/file1'); + expect(tree.exists('/file0')).toBeFalsy(); + expect(tree.exists('/file1')).toBeTruthy(); + tree.create('/file0', 'hello'); + expect(tree.exists('/file0')).toBeTruthy(); const sink = new HostSink(host); sink.commit(optimize(tree)) diff --git a/packages/angular_devkit/schematics/src/tree/host-tree.ts b/packages/angular_devkit/schematics/src/tree/host-tree.ts index 44c1e4a508..c70a2cbc02 100644 --- a/packages/angular_devkit/schematics/src/tree/host-tree.ts +++ b/packages/angular_devkit/schematics/src/tree/host-tree.ts @@ -16,8 +16,6 @@ import { normalize, virtualFs, } from '@angular-devkit/core'; -import { ReadonlyHost } from '../../../core/src/virtual-fs/host'; -import { CordHostRecord } from '../../../core/src/virtual-fs/host/record'; import { ContentHasMutatedException, FileAlreadyExistException, @@ -95,9 +93,10 @@ export class HostDirEntry implements DirEntry { export class HostTree implements Tree { - private _id = _uniqueId++; + private readonly _id = --_uniqueId; private _record: virtualFs.CordHost; private _recordSync: virtualFs.SyncDelegateHost; + private _ancestry = new Set(); private _dirCache = new Map(); @@ -116,79 +115,28 @@ export class HostTree implements Tree { } protected _willCreate(path: Path) { - let current: ReadonlyHost = this._record; - while (current && current != this._backend) { - if (!(current instanceof virtualFs.CordHost)) { - break; - } - - if (current.willCreate(path)) { - return true; - } - - current = current.backend; - } - - return false; + return this._record.willCreate(path); } + protected _willOverwrite(path: Path) { - let current: ReadonlyHost = this._record; - while (current && current != this._backend) { - if (!(current instanceof virtualFs.CordHost)) { - break; - } - - if (current.willOverwrite(path)) { - return true; - } - - current = current.backend; - } - - return false; + return this._record.willOverwrite(path); } + protected _willDelete(path: Path) { - let current: ReadonlyHost = this._record; - while (current && current != this._backend) { - if (!(current instanceof virtualFs.CordHost)) { - break; - } - - if (current.willDelete(path)) { - return true; - } - - current = current.backend; - } - - return false; + return this._record.willDelete(path); } + protected _willRename(path: Path) { - let current: ReadonlyHost = this._record; - while (current && current != this._backend) { - if (!(current instanceof virtualFs.CordHost)) { - break; - } - - if (current.willRename(path)) { - return true; - } - - current = current.backend; - } - - return false; + return this._record.willRename(path); } - branch(): Tree { - // Freeze our own records, and swap. This is so the branch and this Tree don't share the same - // history anymore. - const record = this._record; - this._record = new virtualFs.CordHost(record); - this._recordSync = new virtualFs.SyncDelegateHost(this._record); + const branchedTree = new HostTree(this._backend); + branchedTree._record = this._record.clone(); + branchedTree._recordSync = new virtualFs.SyncDelegateHost(branchedTree._record); + branchedTree._ancestry = new Set(this._ancestry).add(this._id); - return new HostTree(record); + return branchedTree; } merge(other: Tree, strategy: MergeStrategy = MergeStrategy.Default): void { @@ -197,6 +145,12 @@ export class HostTree implements Tree { return; } + if (other instanceof HostTree && other._ancestry.has(this._id)) { + // Workaround for merging a branch back into one of its ancestors + // More complete branch point tracking is required to avoid + strategy |= MergeStrategy.Overwrite; + } + const creationConflictAllowed = (strategy & MergeStrategy.AllowCreationConflict) == MergeStrategy.AllowCreationConflict; const overwriteConflictAllowed = @@ -205,15 +159,17 @@ export class HostTree implements Tree { (strategy & MergeStrategy.AllowOverwriteConflict) == MergeStrategy.AllowDeleteConflict; other.actions.forEach(action => { - if (action.id === this._id) { - return; - } - switch (action.kind) { case 'c': { const { path, content } = action; if ((this._willCreate(path) || this._willOverwrite(path))) { + const existingContent = this.read(path); + if (existingContent && content.equals(existingContent)) { + // Identical outcome; no action required + return; + } + if (!creationConflictAllowed) { throw new MergeConflictException(path); } @@ -228,21 +184,41 @@ export class HostTree implements Tree { case 'o': { const { path, content } = action; + if (this._willDelete(path) && !overwriteConflictAllowed) { + throw new MergeConflictException(path); + } // Ignore if content is the same (considered the same change). - if (this._willOverwrite(path) && !overwriteConflictAllowed) { - throw new MergeConflictException(path); + if (this._willOverwrite(path)) { + const existingContent = this.read(path); + if (existingContent && content.equals(existingContent)) { + // Identical outcome; no action required + return; + } + + if (!overwriteConflictAllowed) { + throw new MergeConflictException(path); + } } // We use write here as merge validation has already been done, and we want to let // the CordHost do its job. - this._record.overwrite(path, content as {} as virtualFs.FileBuffer).subscribe(); + this._record.write(path, content as {} as virtualFs.FileBuffer).subscribe(); return; } case 'r': { const { path, to } = action; + if (this._willDelete(path)) { + throw new MergeConflictException(path); + } + if (this._willRename(path)) { + if (this._record.willRenameTo(path, to)) { + // Identical outcome; no action required + return; + } + // No override possible for renaming. throw new MergeConflictException(path); } @@ -253,9 +229,16 @@ export class HostTree implements Tree { case 'd': { const { path } = action; - if (this._willDelete(path) && !deleteConflictAllowed) { + if (this._willDelete(path)) { + // TODO: This should technically check the content (e.g., hash on delete) + // Identical outcome; no action required + return; + } + + if (!this.exists(path) && !deleteConflictAllowed) { throw new MergeConflictException(path); } + this._recordSync.delete(path); return; @@ -372,16 +355,7 @@ export class HostTree implements Tree { get actions(): Action[] { // Create a list of all records until we hit our original backend. This is to support branches // that diverge from each others. - const allRecords: CordHostRecord[] = [...this._record.records()]; - let current = this._record.backend; - while (current != this._backend) { - if (!(current instanceof virtualFs.CordHost)) { - break; - } - - allRecords.unshift(...current.records()); - current = current.backend; - } + const allRecords = [...this._record.records()]; return clean( allRecords @@ -426,3 +400,17 @@ export class HostTree implements Tree { ); } } + +export class HostCreateTree extends HostTree { + constructor(host: virtualFs.ReadonlyHost) { + super(); + + const tempHost = new HostTree(host); + tempHost.visit(path => { + const content = tempHost.read(path); + if (content) { + this.create(path, content); + } + }); + } +} diff --git a/packages/angular_devkit/schematics/src/tree/host-tree_spec.ts b/packages/angular_devkit/schematics/src/tree/host-tree_spec.ts new file mode 100644 index 0000000000..1fd141033e --- /dev/null +++ b/packages/angular_devkit/schematics/src/tree/host-tree_spec.ts @@ -0,0 +1,38 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +// tslint:disable:no-any +import { normalize, virtualFs } from '@angular-devkit/core'; +import { Action } from './action'; +import { HostTree } from './host-tree'; +import { VirtualTree } from './virtual'; + +describe('HostTree', () => { + it('is backward compatible with VirtualTree', () => { + + const fs = new virtualFs.test.TestHost({ + '/file1': '', + }); + + const tree = new HostTree(fs); + const vTree = new VirtualTree(); + + tree.create('/file2', ''); + vTree.create('/file3', ''); + + // This is the behaviour of 6.0.x merge (returning the branch). + // We need to be compatible with it. + const tree2 = tree.branch(); + tree2.merge(vTree); + + const actions = tree2.actions; + expect(actions).toEqual([ + jasmine.objectContaining({ kind: 'c', path: normalize('/file2') }), + jasmine.objectContaining({ kind: 'c', path: normalize('/file3') }), + ] as any); + }); +}); diff --git a/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts b/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts index 33e63e0dde..c8e2dc09cd 100644 --- a/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts +++ b/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts @@ -19,7 +19,7 @@ import { mergeMap } from 'rxjs/operators'; import { Url } from 'url'; import { EngineHost, - FileSystemCreateTree, + HostCreateTree, RuleFactory, Source, TaskExecutor, @@ -252,7 +252,7 @@ export abstract class FileSystemEngineHostBase implements resolve(dirname(context.schematic.description.path), url.path || ''), ); - return new FileSystemCreateTree(new virtualFs.ScopedHost(new NodeJsSyncHost(), root)); + return new HostCreateTree(new virtualFs.ScopedHost(new NodeJsSyncHost(), root)); }; } diff --git a/packages/angular_devkit/schematics/tools/file-system-engine-host_spec.ts b/packages/angular_devkit/schematics/tools/file-system-engine-host_spec.ts index d07f05ffec..50e2033aa6 100644 --- a/packages/angular_devkit/schematics/tools/file-system-engine-host_spec.ts +++ b/packages/angular_devkit/schematics/tools/file-system-engine-host_spec.ts @@ -8,7 +8,7 @@ // tslint:disable:no-any // tslint:disable:no-implicit-dependencies import { normalize, virtualFs } from '@angular-devkit/core'; -import { FileSystemTree, HostSink, SchematicEngine } from '@angular-devkit/schematics'; +import { HostSink, HostTree, SchematicEngine } from '@angular-devkit/schematics'; import { FileSystemEngineHost } from '@angular-devkit/schematics/tools'; import * as path from 'path'; import { of as observableOf } from 'rxjs'; @@ -292,7 +292,7 @@ describe('FileSystemEngineHost', () => { const collection = engine.createCollection('extra-properties'); const schematic = collection.createSchematic('schematic1'); - schematic.call({}, observableOf(new FileSystemTree(host))).toPromise() + schematic.call({}, observableOf(new HostTree(host))).toPromise() .then(tree => { return new HostSink(host).commit(tree).toPromise(); })