Fix - Foam workspace update (live) (#497)

* improved delta logic in graph.js

fixes a bug that was due to using object.splice inside a forEach loop (sometimes stackoverflow is wrong - removing the element inside the loop will actually reduce the iterations and not all elements in the array will be visited!)

* various fixes to live update of workspace model + tests

* added tab size option in settings.json

.vscode/settings.json

@@ -21,5 +21,6 @@
   "editor.defaultFormatter": "esbenp.prettier-vscode",
   "prettier.requireConfig": true,
   "editor.formatOnSave": true,
+  "editor.tabSize": 2,
   "jest.debugCodeLens.showWhenTestStateIn": ["fail", "unknown", "pass"]
 }

packages/foam-core/src/model/workspace.ts

@@ -184,7 +184,7 @@ export class FoamWorkspace implements IDisposable {
     if (keepMonitoring) {
       workspace.disposables.push(
         workspace.onDidAdd(resource => {
-          FoamWorkspace.resolveResource(workspace, resource);
+          FoamWorkspace.updateLinksRelatedToAddedResource(workspace, resource);
         }),
         workspace.onDidUpdate(change => {
           FoamWorkspace.updateLinksForResource(
@@ -194,7 +194,10 @@ export class FoamWorkspace implements IDisposable {
           );
         }),
         workspace.onDidDelete(resource => {
-          FoamWorkspace.deleteLinksForResource(workspace, resource.uri);
+          FoamWorkspace.updateLinksRelatedToDeletedResource(
+            workspace,
+            resource
+          );
         })
       );
     }
@@ -325,16 +328,28 @@ export class FoamWorkspace implements IDisposable {
     const id = uriToResourceId(uri);
     const deleted = workspace.resources[id];
     delete workspace.resources[id];
+
+    const name = uriToResourceName(uri);
+    workspace.resourcesByName[name] = workspace.resourcesByName[name].filter(
+      resId => resId !== id
+    );
+    if (workspace.resourcesByName[name].length === 0) {
+      delete workspace.resourcesByName[name];
+    }
+
     isSome(deleted) && workspace.onDidDeleteEmitter.fire(deleted);
     return deleted ?? null;
   }
 
   public static resolveResource(workspace: FoamWorkspace, resource: Resource) {
-    // prettier-ignore
-    resource.type === 'note' && resource.links.forEach(link => {
-      const targetUri = FoamWorkspace.resolveLink(workspace, resource, link)
-      workspace = FoamWorkspace.connect(workspace, resource.uri, targetUri)
-    });
+    if (resource.type === 'note') {
+      delete workspace.links[resource.uri.path];
+      // prettier-ignore
+      resource.links.forEach(link => {
+        const targetUri = FoamWorkspace.resolveLink(workspace, resource, link);
+        workspace = FoamWorkspace.connect(workspace, resource.uri, targetUri);
+      });
+    }
     return workspace;
   }
 
@@ -354,32 +369,60 @@ export class FoamWorkspace implements IDisposable {
     }
     if (oldResource.type === 'note' && newResource.type === 'note') {
       const patch = diff(oldResource.links, newResource.links, isEqual);
-      workspace = patch.removed.reduce((g, link) => {
-        const target = workspace.resolveLink(oldResource, link);
-        return FoamWorkspace.disconnect(g, oldResource.uri, target);
+      workspace = patch.removed.reduce((ws, link) => {
+        const target = ws.resolveLink(oldResource, link);
+        return FoamWorkspace.disconnect(ws, oldResource.uri, target);
       }, workspace);
-      workspace = patch.added.reduce((g, link) => {
-        const target = workspace.resolveLink(newResource, link);
-        return FoamWorkspace.connect(g, newResource.uri, target);
+      workspace = patch.added.reduce((ws, link) => {
+        const target = ws.resolveLink(newResource, link);
+        return FoamWorkspace.connect(ws, newResource.uri, target);
       }, workspace);
     }
     return workspace;
   }
 
-  private static deleteLinksForResource(workspace: FoamWorkspace, uri: URI) {
-    delete workspace.links[uri.path];
-    // we rebuild the backlinks by resolving any link that was pointing to the deleted resource
-    const toCheck = workspace.backlinks[uri.path];
-    delete workspace.backlinks[uri.path];
+  private static updateLinksRelatedToAddedResource(
+    workspace: FoamWorkspace,
+    resource: Resource
+  ) {
+    // check if any existing connection can be filled by new resource
+    const name = uriToResourceName(resource.uri);
+    if (name in workspace.placeholders) {
+      const placeholder = workspace.placeholders[name];
+      delete workspace.placeholders[name];
+      const resourcesToUpdate = workspace.backlinks[placeholder.uri.path] ?? [];
+      workspace = resourcesToUpdate.reduce(
+        (ws, res) => FoamWorkspace.resolveResource(ws, ws.get(res.source)),
+        workspace
+      );
+    }
 
-    toCheck.forEach(link => {
-      const source = workspace.get(link.source);
-      source.type === 'note' &&
-        source.links.forEach(l => {
-          const targetUri = FoamWorkspace.resolveLink(workspace, source, l);
-          workspace = FoamWorkspace.connect(workspace, uri, targetUri);
-        });
-    });
+    // resolve the resource
+    workspace = FoamWorkspace.resolveResource(workspace, resource);
+  }
+
+  private static updateLinksRelatedToDeletedResource(
+    workspace: FoamWorkspace,
+    resource: Resource
+  ) {
+    const uri = resource.uri;
+
+    // remove forward links from old resource
+    const resourcesPointedByDeletedNote = workspace.links[uri.path] ?? [];
+    delete workspace.links[uri.path];
+    workspace = resourcesPointedByDeletedNote.reduce(
+      (ws, link) => FoamWorkspace.disconnect(ws, uri, link.target),
+      workspace
+    );
+
+    // recompute previous links to old resource
+    const notesPointingToDeletedResource = workspace.backlinks[uri.path] ?? [];
+    delete workspace.backlinks[uri.path];
+    workspace = notesPointingToDeletedResource.reduce(
+      (ws, link) => FoamWorkspace.resolveResource(ws, ws.get(link.source)),
+      workspace
+    );
+    return workspace;
   }
 
   private static connect(workspace: FoamWorkspace, source: URI, target: URI) {
@@ -402,11 +445,20 @@ export class FoamWorkspace implements IDisposable {
     target: URI
   ) {
     workspace.links[source.path] = workspace.links[source.path]?.filter(
-      c => c.source.path === source.path && c.target.path === target.path
+      c => c.source.path !== source.path || c.target.path !== target.path
     );
+    if (workspace.links[source.path].length === 0) {
+      delete workspace.links[source.path];
+    }
     workspace.backlinks[target.path] = workspace.backlinks[target.path]?.filter(
-      c => c.source.path === source.path && c.target.path === target.path
+      c => c.source.path !== source.path || c.target.path !== target.path
     );
+    if (workspace.backlinks[target.path].length === 0) {
+      delete workspace.backlinks[target.path];
+      if (isPlaceholder(target)) {
+        delete workspace.placeholders[uriToPlaceholderId(target)];
+      }
+    }
     return workspace;
   }
 }

packages/foam-core/test/core.test.ts

@@ -1,5 +1,4 @@
 import { NoteLinkDefinition, Note, Attachment } from '../src/model/note';
-import { uriToSlug } from '../src/utils';
 import { URI } from '../src/common/uri';
 import { Logger } from '../src/utils/log';
 

packages/foam-core/test/janitor/generateLinkReferences.test.ts

@@ -7,7 +7,6 @@ import { FileDataStore } from '../../src/services/datastore';
 import { Logger } from '../../src/utils/log';
 import { URI } from '../../src/common/uri';
 import { FoamWorkspace } from '../../src/model/workspace';
-import { Resource } from '../../src/model/note';
 import { getBasename } from '../../src/utils';
 
 Logger.setLevel('error');

packages/foam-core/test/markdown-provider.test.ts

@@ -199,7 +199,6 @@ date: 20-12-12
   });
 
   it('should parse empty frontmatter', () => {
-    const workspace = new FoamWorkspace();
     const note = createNoteFromMarkdown(
       '/page-f.md',
       `

packages/foam-core/test/plugin.test.ts

@@ -1,7 +1,6 @@
 import path from 'path';
 import { loadPlugins } from '../src/plugins';
 import { createMarkdownParser } from '../src/markdown-provider';
-import { createTestNote } from './core.test';
 import { FoamConfig, createConfigFromObject } from '../src/config';
 import { URI } from '../src/common/uri';
 import { Logger } from '../src/utils/log';

packages/foam-core/test/workspace.test.ts

@@ -422,4 +422,188 @@ describe('Updating workspace happy path', () => {
     expect(() => ws.get(placeholderUri('page-b'))).toThrow();
     expect(ws.get(noteB.uri).type).toEqual('note');
   });
+
+  it('removing link to placeholder should remove placeholder', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ to: '/path/to/another/page-b.md' }],
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA).resolveLinks();
+    expect(ws.get(placeholderUri('/path/to/another/page-b.md')).type).toEqual(
+      'placeholder'
+    );
+
+    // update the note
+    const noteABis = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [],
+    });
+    ws.set(noteABis).resolveLinks();
+
+    expect(() =>
+      ws.get(placeholderUri('/path/to/another/page-b.md'))
+    ).toThrow();
+  });
+});
+
+describe('Monitoring of workspace state', () => {
+  it('Update links when modifying note', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ slug: 'page-b' }],
+    });
+    const noteB = createTestNote({
+      uri: '/path/to/another/page-b.md',
+      links: [{ slug: 'page-c' }],
+    });
+    const noteC = createTestNote({
+      uri: '/path/to/more/page-c.md',
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA)
+      .set(noteB)
+      .set(noteC)
+      .resolveLinks(true);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([noteB.uri]);
+    expect(ws.getBacklinks(noteB.uri)).toEqual([noteA.uri]);
+    expect(ws.getBacklinks(noteC.uri)).toEqual([noteB.uri]);
+
+    // update the note
+    const noteABis = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ slug: 'page-c' }],
+    });
+    ws.set(noteABis);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([noteC.uri]);
+    expect(ws.getBacklinks(noteB.uri)).toEqual([]);
+    expect(
+      ws
+        .getBacklinks(noteC.uri)
+        .map(link => link.path)
+        .sort()
+    ).toEqual(['/path/to/another/page-b.md', '/path/to/page-a.md']);
+  });
+
+  it('Removing target note should produce placeholder for wikilinks', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ slug: 'page-b' }],
+    });
+    const noteB = createTestNote({
+      uri: '/path/to/another/page-b.md',
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA)
+      .set(noteB)
+      .resolveLinks(true);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([noteB.uri]);
+    expect(ws.getBacklinks(noteB.uri)).toEqual([noteA.uri]);
+    expect(ws.get(noteB.uri).type).toEqual('note');
+
+    // remove note-b
+    ws.delete(noteB.uri);
+
+    expect(() => ws.get(noteB.uri)).toThrow();
+    expect(ws.get(placeholderUri('page-b')).type).toEqual('placeholder');
+  });
+
+  it('Adding note should replace placeholder for wikilinks', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ slug: 'page-b' }],
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA).resolveLinks(true);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([placeholderUri('page-b')]);
+    expect(ws.get(placeholderUri('page-b')).type).toEqual('placeholder');
+
+    // add note-b
+    const noteB = createTestNote({
+      uri: '/path/to/another/page-b.md',
+    });
+
+    ws.set(noteB);
+
+    expect(() => ws.get(placeholderUri('page-b'))).toThrow();
+    expect(ws.get(noteB.uri).type).toEqual('note');
+  });
+
+  it('Removing target note should produce placeholder for direct links', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ to: '/path/to/another/page-b.md' }],
+    });
+    const noteB = createTestNote({
+      uri: '/path/to/another/page-b.md',
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA)
+      .set(noteB)
+      .resolveLinks(true);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([noteB.uri]);
+    expect(ws.getBacklinks(noteB.uri)).toEqual([noteA.uri]);
+    expect(ws.get(noteB.uri).type).toEqual('note');
+
+    // remove note-b
+    ws.delete(noteB.uri);
+
+    expect(() => ws.get(noteB.uri)).toThrow();
+    expect(ws.get(placeholderUri('/path/to/another/page-b.md')).type).toEqual(
+      'placeholder'
+    );
+  });
+
+  it('Adding note should replace placeholder for direct links', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ to: '/path/to/another/page-b.md' }],
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA).resolveLinks(true);
+
+    expect(ws.getLinks(noteA.uri)).toEqual([
+      placeholderUri('/path/to/another/page-b.md'),
+    ]);
+    expect(ws.get(placeholderUri('/path/to/another/page-b.md')).type).toEqual(
+      'placeholder'
+    );
+
+    // add note-b
+    const noteB = createTestNote({
+      uri: '/path/to/another/page-b.md',
+    });
+
+    ws.set(noteB);
+
+    expect(() => ws.get(placeholderUri('page-b'))).toThrow();
+    expect(ws.get(noteB.uri).type).toEqual('note');
+  });
+
+  it('removing link to placeholder should remove placeholder', () => {
+    const noteA = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [{ to: '/path/to/another/page-b.md' }],
+    });
+    const ws = new FoamWorkspace();
+    ws.set(noteA).resolveLinks(true);
+    expect(ws.get(placeholderUri('/path/to/another/page-b.md')).type).toEqual(
+      'placeholder'
+    );
+
+    // update the note
+    const noteABis = createTestNote({
+      uri: '/path/to/page-a.md',
+      links: [],
+    });
+    ws.set(noteABis);
+    expect(() =>
+      ws.get(placeholderUri('/path/to/another/page-b.md'))
+    ).toThrow();
+  });
 });

packages/foam-vscode/src/features/dataviz.ts

@@ -1,7 +1,7 @@
 import * as vscode from 'vscode';
 import * as path from 'path';
 import { FoamFeature } from '../types';
-import { Foam, Logger, FoamWorkspace } from 'foam-core';
+import { Foam, Logger } from 'foam-core';
 import { TextDecoder } from 'util';
 import { getGraphStyle, getTitleMaxLength } from '../settings';
 import { isSome } from '../utils';

packages/foam-vscode/static/graphs/default/graph.js

@@ -99,15 +99,20 @@ const Actions = {
       const links = graphInfo.links;
 
       // compute graph delta, for smooth transitions we need to mutate objects in-place
-      const remaining = new Set(Object.keys(m.nodeInfo));
-      m.data.nodes.forEach((node, index, object) => {
-        if (remaining.has(node.id)) {
-          remaining.delete(node.id);
+      const nodeIdsToAdd = new Set(Object.keys(m.nodeInfo));
+      const nodeIndexesToRemove = new Set();
+      m.data.nodes.forEach((node, index) => {
+        if (nodeIdsToAdd.has(node.id)) {
+          nodeIdsToAdd.delete(node.id);
         } else {
-          object.splice(index, 1); // delete the element
+          nodeIndexesToRemove.add(index);
         }
       });
-      remaining.forEach(nodeId => {
+      // apply the delta
+      nodeIndexesToRemove.forEach(index => {
+        m.data.nodes.splice(index, 1); // delete the element
+      });
+      nodeIdsToAdd.forEach(nodeId => {
         m.data.nodes.push({
           id: nodeId,
         });