WIP: Graphchanges are not commutative (?)

graph/src/main/scala/Graph.scala

@@ -106,34 +106,30 @@ final case class Graph(nodes: Array[Node], edges: Array[Edge]) {
   @deprecated("Be aware that you are constructing a new graph here.", "")
   def filterNot(p: Node => Boolean): Graph = filter(node => !p(node))
 
-  def applyChangesWithUser(user: Node.User, c: GraphChanges): Graph = changeGraphInternal(addNodes = c.addNodes ++ Set(user), addEdges = c.addEdges, deleteEdges = c.delEdges)
-  def applyChanges(c: GraphChanges): Graph = changeGraphInternal(addNodes = c.addNodes, addEdges = c.addEdges, deleteEdges = c.delEdges)
+  def applyChangesWithUser(user: Node.User, c: GraphChanges): Graph = changeGraphInternal(c.copy(addNodes = c.addNodes + user))
+  def applyChanges(c: GraphChanges): Graph = changeGraphInternal(c)
+
+  private def changeGraphInternal(c:GraphChanges): Graph = {
 
-  private def changeGraphInternal(addNodes: collection.Set[Node], addEdges: collection.Set[Edge], deleteEdges: collection.Set[Edge] = Set.empty): Graph = {
     val nodesBuilder = mutable.ArrayBuilder.make[Node]()
     val edgesBuilder = mutable.ArrayBuilder.make[Edge]()
-    // nodesBuilder.sizeHint(nodes.length + addNodes.size)
-    // edgesBuilder.sizeHint(edges.length + addEdges.size)
-
-    val addNodeIds: Set[NodeId] = addNodes.map(_.id)(breakOut)
-    val addEdgeIds: Set[(NodeId, String, NodeId)] = addEdges.collect {
-      // we filter out edges without a unique constraint.
-      // this needs to correspond how it is defined in the database.
-      case e if !e.isInstanceOf[Edge.Author] => (e.sourceId, e.data.tpe, e.targetId)
-    }(breakOut)
-    val deleteEdgeIds: Set[(NodeId, String, NodeId)] = deleteEdges.map { e => (e.sourceId, e.data.tpe, e.targetId) }(breakOut)
-    val updatedEdgeIds = addEdgeIds ++ deleteEdgeIds
+
+    val addNodeIds: Set[NodeId] = c.addNodes.map(_.id)(breakOut)
+    val deleteEdgeTriples: Set[(NodeId, String, NodeId)] = c.delEdges.map { e => (e.sourceId, e.data.tpe, e.targetId) }(breakOut)
+    val updatedEdgeTriples = c.addEdgeTriples ++ deleteEdgeTriples
 
     nodes.foreach { node =>
       if(!addNodeIds(node.id)) nodesBuilder += node
     }
-    addNodes.foreach { node =>
+    c.addNodes.foreach { node =>
       nodesBuilder += node
     }
     edges.foreach { edge =>
-      if(!updatedEdgeIds((edge.sourceId, edge.data.tpe, edge.targetId))) edgesBuilder += edge
+      // keep all edges in the graph which will not be updated
+      @inline def edgeWillNotbeUpdated = !updatedEdgeTriples((edge.sourceId, edge.data.tpe, edge.targetId))
+      if(edgeWillNotbeUpdated) edgesBuilder += edge
     }
-    addEdges.foreach { edge =>
+    c.addEdges.foreach { edge =>
       edgesBuilder += edge
     }
 

graph/src/main/scala/GraphChanges.scala

@@ -8,8 +8,9 @@ import scala.collection.breakOut
 case class GraphChanges(
   addNodes: collection.Set[Node] = Set.empty,
   addEdges: collection.Set[Edge] = Set.empty,
-  // we do not really need a connection for deleting (ConnectionId instead), but we want to revert it again.
   delEdges: collection.Set[Edge] = Set.empty
+  // we do not really need a connection for deleting (a ConnectionId would be enough),
+  // but it allows us to calculate a reverse change
 ) {
   def withAuthor(userId: UserId, timestamp: EpochMilli = EpochMilli.now): GraphChanges =
     copy(
@@ -22,7 +23,6 @@ case class GraphChanges(
       addNodes = addNodes ++ other.addNodes,
       addEdges = addEdges ++ other.addEdges,
       delEdges = delEdges -- other.addEdges ++ other.delEdges
-      //FIXME: why was i here? inconsistent changes? .filter(c => !otherAddNodeIds(c.sourceId) && !otherAddNodeIds(c.targetId))
     )
   }
 
@@ -45,6 +45,14 @@ case class GraphChanges(
 
   lazy val consistent: GraphChanges = copy(addEdges = addEdges -- delEdges)
 
+
+  def addEdgeTriples: Set[(NodeId, String, NodeId)] = addEdges.collect {
+    // we filter out edges without a unique constraint.
+    // this needs to correspond how it is defined in the database.
+    case e if !e.isInstanceOf[Edge.Author] => (e.sourceId, e.data.tpe, e.targetId)
+  }(breakOut)
+
+
   def involvedNodeIds: collection.Set[NodeId] =
     addNodes.map(_.id) ++
       addEdges.flatMap(e => e.sourceId :: e.targetId :: Nil) ++

graph/src/test/scala/GraphSpec.scala

@@ -128,6 +128,20 @@ class GraphSpec extends FreeSpec with MustMatchers {
         assert(newGraph.edges.head.asInstanceOf[Edge.Parent].data.ordering.get.toInt == 7)
       }
 
+      "replace parent edge (with ordering) in graph by empty parent edge" in {
+        val graph = Graph(
+          nodes = List(1, 2),
+          edges = Set()
+        )
+
+        val oldParent = Edge.Parent(2: NodeId, new EdgeData.Parent(deletedAt = None, ordering = Some(5)), 1: NodeId)
+        val newParent = Edge.Parent(2: NodeId, new EdgeData.Parent(deletedAt = None, ordering = None), 1: NodeId)
+        val newGraph = graph.applyChanges(GraphChanges(addEdges = Set(oldParent,newParent)))
+
+        assert(newGraph.edges.size == 1)
+        assert(newGraph.edges.head.asInstanceOf[Edge.Parent].data.ordering == None)
+      }
+
       "replace parent edge in graph multiple parents" in {
         val graph = Graph(
           nodes = List(1, 2, 3),