fix(@angular-devkit/schematics): support VirtualTree/HostTree interop

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();

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))

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<number>();
 
   private _dirCache = new Map<Path, HostDirEntry>();
 
@@ -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);
+      }
+    });
+  }
+}

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<Action>({ kind: 'c', path: normalize('/file2') }),
+        jasmine.objectContaining<Action>({ kind: 'c', path: normalize('/file3') }),
+    ] as any);
+  });
+});

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));
         };
     }
 

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();
       })