Fix bug when deleting a node that has a self-edge.

src/frontend/editable_graph.tsx

@@ -23,9 +23,13 @@ import {
 
 // The stuff that we store in each (visible) node:
 interface NodeObjType {
-  nodeCreation: NodeCreation; // The delta that created the node
-  outgoing: Map<string, [EdgeCreation|EdgeUpdate, NodeObjType]>; // Outgoing edges: Mapping from edge label to the most recent EdgeCreation or EdgeUpdate.
+  // The delta that created the node
+  nodeCreation: NodeCreation;
 
+  // Outgoing edges: Mapping from edge label to the most recent EdgeCreation or EdgeUpdate.
+  outgoing: Map<string, [EdgeCreation|EdgeUpdate, NodeObjType]>;
+
+  // For any edge that is or was incoming, we keep the delta that set, unset or deleted that edge. If this node is deleted, this deletion must depend on those deltas.
   incoming: Array<[EdgeCreation|EdgeUpdate|NodeDeletion, NodeObjType]>;
 }
 
@@ -133,14 +137,15 @@ export class EditableGraph extends React.Component<EditableGraphProps, EditableG
           // console.log("incoming:", node.obj.incoming);
           // console.log("outgoing:", node.obj.outgoing);
 
-          const deleteDependencies: any[] = [];
+          const outgoingEdgeDependencies: any[] = [];
+          const incomingEdgeDependencies: any[] = [];
           const compositeDeltas: any[] = [];
 
           for (const [incomingEdge,sourceObj] of node.obj.incoming) {
             // We also depend on incoming edges that were deleted (because their source node was deleted):
             if (incomingEdge instanceof NodeDeletion) {
               console.log("edge was already deleted");
-              deleteDependencies.push(incomingEdge);
+              incomingEdgeDependencies.push(incomingEdge);
             }
             else {
               if (incomingEdge.target.target === node.obj.nodeCreation) {
@@ -148,13 +153,13 @@ export class EditableGraph extends React.Component<EditableGraphProps, EditableG
                 // Must set the value of every incoming edge to 'null' (with an EdgeUpdate):
                 const edgeUnset = new EdgeUpdate(incomingEdge, null);
                 sourceObj.outgoing.set(incomingEdge.getCreation().label, [edgeUnset,node.obj]);
-                deleteDependencies.push(edgeUnset);
+                incomingEdgeDependencies.push(edgeUnset);
                 compositeDeltas.push(edgeUnset);
               }
               else {
                 console.log("edge already pointing somewhere else");
                 // Edge is already pointing somewhere else: just include the operation as a dependency for the deletion.
-                deleteDependencies.push(incomingEdge);
+                incomingEdgeDependencies.push(incomingEdge);
               }
             }
           }
@@ -162,14 +167,16 @@ export class EditableGraph extends React.Component<EditableGraphProps, EditableG
           for (const [outgoingEdge, targetObj] of node.obj.outgoing.values()) {
             targetObj.incoming.splice(targetObj.incoming.findIndex(([d,_])=>d===outgoingEdge), 1);
             targetIncomings.push(targetObj.incoming);
-            deleteDependencies.push(outgoingEdge);
+            outgoingEdgeDependencies.push(outgoingEdge);
           }
-          const nodeDeletion = new NodeDeletion(node.obj.nodeCreation, deleteDependencies);
+          const nodeDeletion = new NodeDeletion(node.obj.nodeCreation, outgoingEdgeDependencies, incomingEdgeDependencies);
           compositeDeltas.push(nodeDeletion);
           for (const targetIncoming of targetIncomings) {
             targetIncoming.push([nodeDeletion, node.obj]);
           }
           this.graphRef.current.deleteNode(node.id);
+          // console.log("deleteDependencies:", deleteDependencies);
+          // console.log("compositeDeltas:", compositeDeltas);
           const tx = this.compositeLvl.createComposite(compositeDeltas);
           this.currentVersion = this.versionRegistry.createVersion(this.currentVersion, tx);
           this.props.newDeltaHandler(tx);

src/onion/composite_delta.test.ts

@@ -26,7 +26,7 @@ describe("Composite delta", () => {
     assert(_.isEqual(composite1.getDependencies(), []), "expected composite1 to have no dependencies");
 
 
-    const nodeDeletion = new NodeDeletion(nodeCreation, []);
+    const nodeDeletion = new NodeDeletion(nodeCreation, [], []);
     const nodeCreation2 = new NodeCreation(getId());
     const composite2 = level.createComposite([nodeDeletion, nodeCreation2]);
     assert(_.isEqual(composite2.getDependencies(), [composite1]), "expected composite2 to depend on composite 1");
@@ -39,11 +39,11 @@ describe("Composite delta", () => {
     const nodeCreation = new NodeCreation(getId());
     const compositeCreate = level.createComposite([nodeCreation]);
 
-    const nodeDeletion1 = new NodeDeletion(nodeCreation, []);
+    const nodeDeletion1 = new NodeDeletion(nodeCreation, [], []);
     const compositeDelete1 = level.createComposite([nodeDeletion1]);
     assert(_.isEqual(compositeDelete1.getConflicts(), []), "there should not yet be a conflict")
 
-    const nodeDeletion2 = new NodeDeletion(nodeCreation, []);
+    const nodeDeletion2 = new NodeDeletion(nodeCreation, [], []);
     const compositeDelete2 = level.createComposite([nodeDeletion2]);
 
     assert(_.isEqual(compositeDelete1.getConflicts(), [compositeDelete2]), "expected compositeDelete1 to conflict with compositeDelete2");
@@ -56,10 +56,10 @@ describe("Composite delta", () => {
     const level = new CompositeLevel();
 
     const nodeCreation = new NodeCreation(getId());
-    const nodeDeletion = new NodeDeletion(nodeCreation, []);
+    const nodeDeletion = new NodeDeletion(nodeCreation, [], []);
 
     const composite = level.createComposite([nodeCreation, nodeDeletion]);
-    
+
     assert(_.isEqual(composite.getDependencies(), []), "expected composite to have no dependencies");
     assert(_.isEqual(composite.getConflicts(), []), "expected composite to have no conflicts");
   });

src/onion/primitive_delta.test.ts

@@ -23,10 +23,10 @@ describe("Delta", () => {
     const creation = new NodeCreation(getId());
     assert(creation.getConflicts().length === 0, "did not expect node creation to be conflicting with any other operation");
 
-    const deletion1 = new NodeDeletion(creation, []);
+    const deletion1 = new NodeDeletion(creation, [], []);
     assert(deletion1.getConflicts().length === 0, "did not expect first deletion alone to be conflicting with anything");
 
-    const deletion2 = new NodeDeletion(creation, []);
+    const deletion2 = new NodeDeletion(creation, [], []);
     assert(deletion1.getConflicts().length === 1, "expected second deletion to be conflicting with first deletion");
     assert(deletion2.getConflicts().length === 1, "expected second deletion to be conflicting with first deletion");
 
@@ -73,7 +73,7 @@ describe("Delta", () => {
     const sourceCreation = new NodeCreation(getId());
     const targetCreation = new NodeCreation(getId());
 
-    const sourceDeletion = new NodeDeletion(sourceCreation, []);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [], []);
     assert(sourceDeletion.getConflicts().length === 0, "expected no conflicts so far");
 
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
@@ -91,7 +91,7 @@ describe("Delta", () => {
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
     assert(edgeCreation.getConflicts().length === 0, "expected no conflicts so far");
 
-    const sourceDeletion = new NodeDeletion(sourceCreation, []);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [], []);
     assert(edgeCreation.getConflicts().length === 1, "expected require/delete conflict, because edge source is concurrently deleted");
     assert(sourceDeletion.getConflicts().length === 1, "expected require/delete conflict, because edge source is concurrently deleted");
   });
@@ -105,7 +105,7 @@ describe("Delta", () => {
 
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
 
-    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation]);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation], []);
     assert(sourceDeletion.getConflicts().length === 0, "Since deletion was aware of the edge creation, there should be no conflict");
   });
 
@@ -122,7 +122,7 @@ describe("Delta", () => {
 
     // no conflicts so far
 
-    const sourceDeletion = new NodeDeletion(sourceCreation, []);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [], []);
 
     assert(edgeCreation.getConflicts().length === 1, "expected require/delete conflict, because edge source is concurrently deleted");
     assert(sourceDeletion.getConflicts().length === 1, "expected require/delete conflict, because edge source is concurrently deleted");
@@ -137,7 +137,7 @@ describe("Delta", () => {
     const sourceCreation = new NodeCreation(getId());
     const targetCreation = new NodeCreation(getId());
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
-    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation]);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation], []);
 
     // no conflicts so far
 
@@ -153,7 +153,7 @@ describe("Delta", () => {
     const sourceCreation = new NodeCreation(getId());
     const targetCreation = new NodeCreation(getId());
 
-    const targetDeletion = new NodeDeletion(targetCreation, []);
+    const targetDeletion = new NodeDeletion(targetCreation, [], []);
 
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
     assert(edgeCreation.getConflicts().length === 1, "expected require/delete conflict");
@@ -168,7 +168,7 @@ describe("Delta", () => {
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
 
     // delete target of edge, unaware that edge exists:
-    const targetDeletion = new NodeDeletion(targetCreation, []);
+    const targetDeletion = new NodeDeletion(targetCreation, [], []);
 
     assert(edgeCreation.getConflicts().length === 1, "expected require/delete conflict");
     assert(targetDeletion.getConflicts().length === 1, "expected require/delete conflict");
@@ -183,28 +183,37 @@ describe("Delta", () => {
     const edgeUpdate = new EdgeUpdate(edgeCreation, sourceCreation); // turn edge into self-edge
 
     // because of the edgeUpdate, 'target' is no longer the target of the edge, and can be deleted:
-    const targetDeletion = new NodeDeletion(targetCreation, [edgeUpdate]);
+    const targetDeletion = new NodeDeletion(targetCreation, [], [edgeUpdate]);
 
     console.log(edgeCreation.getConflicts())
     assert(edgeCreation.getConflicts().length === 0, "expected no require/delete conflict");
     assert(targetDeletion.getConflicts().length === 0, "expected no require/delete conflict");
   });
 
-
-  // Not yet supported
   it("Delete source and target of edge (no conflict)", () => {
     const getId = mockUuid();
 
     const sourceCreation = new NodeCreation(getId());
     const targetCreation = new NodeCreation(getId());
     const edgeCreation = new EdgeCreation(sourceCreation, "label", targetCreation);
-    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation]);
+    const sourceDeletion = new NodeDeletion(sourceCreation, [edgeCreation], []);
 
     assert(sourceDeletion.getConflicts().length === 0, "expected no conflicts");
 
-    const targetDeletion = new NodeDeletion(targetCreation, [sourceDeletion]); // unsupported :(
+    const targetDeletion = new NodeDeletion(targetCreation, [], [sourceDeletion]);
 
     assert(targetDeletion.getConflicts().length === 0, "expected no conflicts");
-  })
+  });
 
+  it("Delete node with self-edge", () => {
+    const getId = mockUuid();
+
+    const nodeCreation = new NodeCreation(getId());
+    const edgeCreation = new EdgeCreation(nodeCreation, "label", nodeCreation);
+    const edgeUpdate = new EdgeUpdate(edgeCreation, null);
+    const nodeDeletion = new NodeDeletion(nodeCreation, [edgeUpdate], [edgeUpdate]);
+
+    console.log(nodeDeletion.getConflicts());
+    assert(nodeDeletion.getConflicts().length === 0, "expected no conflicts");
+  })
 });

src/onion/primitive_delta.ts

@@ -82,48 +82,66 @@ export class NodeDeletion implements Delta {
   // Conflicts: Concurrent creation/update of an edge with as target the deleted node.
   edgeTargetConflicts: Array<EdgeCreation | EdgeUpdate> = [];
 
-  // afterOps: If the deleted node had any outgoing edges, then for every such edge, the most recent EdgeCreation/EdgeUpdate must be included as a dependency. If the deleted node had any incoming edges, then a follow-up EdgeUpdate that sets the target of that edge to another node (or null) must be included as a dependency.
-  constructor(creation: NodeCreation, afterOps: Array<EdgeCreation | EdgeUpdate | NodeDeletion>) {
+  // Parameters:
+  //   deletedOutgoingEdges: For every outgoing edge of this node being deleted, must explicitly specify the most recent EdgeCreation/EdgeUpdate on this edge, to make it explicit that this deletion happens AFTER the EdgeCreation/EdgeUpdate (instead of concurrently, which is a conflict).
+  //   afterIncomingEdges: For every edge that is or was (once) incoming to this node, must explicitly specify an EdgeUpdate/NodeDeletion that makes this edge point somewhere else (no longer to this node).
+  constructor(creation: NodeCreation, deletedOutgoingEdges: Array<EdgeCreation|EdgeUpdate>, afterIncomingEdges: Array<EdgeUpdate|NodeDeletion>) {
     this.creation = creation;
-    this.deletedOutgoingEdges = [];
-    this.afterIncomingEdges = [];
+    this.deletedOutgoingEdges = deletedOutgoingEdges;
+    this.afterIncomingEdges = afterIncomingEdges;
 
-    if (_.uniq(afterOps).length !== afterOps.length) {
-      throw new Error("Assertion failed: afterOps contains duplicates.");
+    // Check some assertions
+    if (_.uniq(deletedOutgoingEdges).length !== deletedOutgoingEdges.length) {
+      throw new Error("Assertion failed: deletedOutgoingEdges contains duplicates.");
     }
-
-    // hash will be calculated based on all EdgeCreations/EdgeUpdates that we depend on, by XOR (insensitive to array order).
-    const hash = createHash('sha256').update(this.creation.hash);
-    let union = Buffer.alloc(32); // all zeroes
-    for (const after of afterOps) {
-      union = bufferXOR(union, after.getHash());
+    if (_.uniq(afterIncomingEdges).length !== afterIncomingEdges.length) {
+      throw new Error("Assertion failed: deletedOutgoingEdges contains duplicates.");
     }
-    this.hash = hash.update(union).digest();
-
-    // partition 'afterOps' into outgoing and incoming edges, while checking some assertions:
-    for (const after of afterOps) {
-      if (after instanceof NodeDeletion) {
-        this.afterIncomingEdges.push(after);
+    for (const supposedlyOutgoingEdge of this.deletedOutgoingEdges) {
+      if (supposedlyOutgoingEdge.getCreation().source !== this.creation) {
+        throw new Error("Assertion failed: Every element of delOutgoings must be an EdgeCreation or EdgeUpdate of an outgoing edge of the deleted node.")
       }
-      else {
-        if (after.getCreation().source === this.creation) {
-          this.deletedOutgoingEdges.push(after);
-        }
-        else {
-          if (!(after instanceof EdgeUpdate)) {
-            throw new Error("Assertion failed: Expected EdgeUpdate here.");
-          }
-          if (after.target.target === creation) {
-            throw new Error("Assertion failed: NodeDeletion must not have after-dependency on EdgeUpdate that sets target to deleted node.");
-          }
-
-          if (! [...after.iterUpdates()].some(e => e.target.target === this.creation)) {
-            throw new Error("Assertion failed: None of the EdgeUpdates of after set the target to the deleted node.");
+    }
+    for (const supposedlyIncomingEdge of this.afterIncomingEdges) {
+      if (supposedlyIncomingEdge instanceof NodeDeletion) {
+        // should check if the NodeDeletion deletes an edge that was *once* 
+        let isReallyIncomingEdge = false;
+        for (const deletedEdge of supposedlyIncomingEdge.deletedOutgoingEdges) {
+          let current: EdgeCreation|EdgeUpdate|undefined = deletedEdge;
+          while (current !== undefined) {
+            if (current.target.target === this.creation) {
+              isReallyIncomingEdge = true;
+              break;
+            }
+            if (current instanceof EdgeUpdate) current = current.overwrites
+            else break;
           }
-          this.afterIncomingEdges.push(after);
+          if (isReallyIncomingEdge) break;
+        }
+        if (!isReallyIncomingEdge) {
+          throw new Error("Assertion failed: NodeDeletion in afterIncomingEdges does not delete an incoming edge.");
         }
       }
+      else {      
+        if (supposedlyIncomingEdge.target.target === this.creation) {
+          throw new Error("Assertion failed: Every element in afterIncomingEdges MUST set the target of the edge to SOME PLACE ELSE.");
+        }
+        if (![...supposedlyIncomingEdge.iterUpdates()].some(e => e.target.target === this.creation)) {
+          throw new Error("Assertion failed: None of the EdgeUpdates of supposedlyIncomingEdge set the target to the node being deleted.");
+        }
+      }
+    }
+
+    // hash will be calculated based on all EdgeCreations/EdgeUpdates/NodeDeletions that we depend on, by XOR (insensitive to array order).
+    const hash = createHash('sha256').update(this.creation.hash);
+    let union = Buffer.alloc(32); // all zeroes
+    for (const deletedOutgoing of deletedOutgoingEdges) {
+      union = bufferXOR(union, deletedOutgoing.getHash());
     }
+    for (const afterIncoming of afterIncomingEdges) {
+      union = bufferXOR(union, afterIncoming.getHash());
+    }
+    this.hash = hash.update(union).digest();
 
     // Detect conflicts
 

src/onion/version.test.ts

@@ -29,7 +29,7 @@ describe("Version", () => {
     const registry = new VersionRegistry();
 
     const nodeCreation = new NodeCreation(getId());
-    const nodeDeletion = new NodeDeletion(nodeCreation, []);
+    const nodeDeletion = new NodeDeletion(nodeCreation, [], []);
 
     const version1 = registry.createVersion(initialVersion, nodeCreation);
     const version2 = registry.createVersion(version1, nodeDeletion);
@@ -62,7 +62,7 @@ describe("Version", () => {
     const A = new NodeCreation(getId());
     const B = new NodeCreation(getId());
     const C = new NodeCreation(getId());
-    const D = new NodeDeletion(A, []);
+    const D = new NodeDeletion(A, [], []);
 
     const v1 = registry.quickVersion([D,B,A]);
     const v2 = registry.quickVersion([C,B]);
@@ -130,12 +130,12 @@ describe("Version", () => {
     const Y = new NodeCreation(getId());
     const Z = new NodeCreation(getId());
 
-    const A = new NodeDeletion(X, []);
+    const A = new NodeDeletion(X, [], []);
     const B = new EdgeCreation(X, "label", Y); // conflicts with A
     assert(_.isEqual(B.getConflicts(), [A]), "Expected B to conflict with A");
     const C = new EdgeCreation(Y, "label", Z);
     const BB = new EdgeUpdate(B, null); // unset edge B.
-    const D = new NodeDeletion(Y, [BB]); // conflicts with C
+    const D = new NodeDeletion(Y, [], [BB]); // conflicts with C
 
     assert(_.isEqual(D.getConflicts(), [C]), "Expected D to conflict with C");
 
@@ -202,7 +202,7 @@ describe("Version", () => {
 
     for (let i=0; i<HOW_MANY; i++) {
       const creation = new NodeCreation(getId());
-      const deletion = new NodeDeletion(creation, []);
+      const deletion = new NodeDeletion(creation, [], []);
       const edge = new EdgeCreation(creation, "l", creation); // conflicts with deletion0
 
       creations.push(creation);

webpack.config.js

@@ -2,7 +2,7 @@ const path = require('path');
 
 module.exports = {
   entry: path.resolve(__dirname, 'src', 'frontend', 'index.tsx'),
-  devtool: 'inline-source-map',
+  // devtool: 'inline-source-map',
   module: {
     rules: [
       {