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

Author: clydin

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,22 +20,24 @@ 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');
     recorder.insertLeft(8, 'testing ');
     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;
+  protected _willOverwrite(path: Path) {
+    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;
+  protected _willDelete(path: Path) {
+    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;
+  protected _willRename(path: Path) {
+    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);
+      }
+    });
+  }
+}