bugfix/552-existing-graph-contains-cycle-the-graph-changes

application/database/db.py

@@ -673,7 +673,7 @@ def parse_node_no_links(node: NeoDocument) -> cre_defs.Document:
 
 
 class Node_collection:
-    graph: nx.Graph = None
+    graph: inmemory_graph.CRE_Graph = None
     neo_db: NEO_DB = None
     session = sqla.session
 
@@ -749,34 +749,6 @@ def __get_all_nodes_and_cres(self) -> List[cre_defs.Document]:
             result.append(self.get_cre_by_db_id(cid[0]))
         return result
 
-    def __introduces_cycle(self, node_from: str, node_to: str) -> Any:
-        if not self.graph:
-            logger.error("graph is null")
-            raise ValueError(
-                "internal CRE graph is None while importing, cannot detect cycles, this is unrecoverable"
-            )
-        try:
-            existing_cycle = nx.find_cycle(self.graph.graph)
-            if existing_cycle:
-                logger.fatal(
-                    "Existing graph contains cycle,"
-                    "this not a recoverable error,"
-                    f" manual database actions are required {existing_cycle}"
-                )
-                raise ValueError(
-                    "Existing graph contains cycle,"
-                    "this not a recoverable error,"
-                    f" manual database actions are required {existing_cycle}"
-                )
-        except nx.exception.NetworkXNoCycle:
-            pass  # happy path, we don't want cycles
-        new_graph = self.graph.graph.copy()
-        new_graph.add_edge(node_from, node_to)
-        try:
-            return nx.find_cycle(new_graph)
-        except nx.NetworkXNoCycle:
-            return False
-
     @classmethod
     def object_select(cls, node: Node, skip_attributes: List = []) -> List[Node]:
         if not node:
@@ -1616,15 +1588,27 @@ def add_internal_link(
             f" {higher.external_id}:{higher.name}"
             f" -> {lower.external_id}:{lower.name} of type {type.value},adding"
         )
-        cycle = self.__introduces_cycle(f"CRE: {higher.id}", f"CRE: {lower.id}")
+        if not self.graph:
+            logger.error("graph is null")
+            raise ValueError(
+                "internal CRE graph is None while importing, cannot detect cycles, this is unrecoverable"
+            )
+
+        higher_cre = CREfromDB(higher)
+        lower_cre = CREfromDB(higher)
+        link_to = cre_defs.Link(document=lower_cre, ltype=type)
+        cycle = self.graph.adds_cycle(doc_from=higher_cre, link_to=link_to)
+
         if not cycle:
             self.session.add(
                 InternalLinks(type=type.value, cre=lower.id, group=higher.id)
             )
             self.session.commit()
             if self.graph:
-                self.graph.add_edge(
-                    f"CRE: {higher.id}", f"CRE: {lower.id}", ltype=type.value
+                self.graph = self.graph.add_graph_edge(
+                    doc_from=higher_cre,
+                    link_to=link_to,
+                    graph=self.graph.graph,
                 )
         else:
             for item in cycle:
@@ -1677,10 +1661,13 @@ def add_link(
             )
             self.session.add(Links(type=type.value, cre=cre.id, node=node.id))
             if self.graph:
-                self.graph.add_edge(
-                    f"CRE: {cre.id}", f"Node: {str(node.id)}", ltype=type.value
+                self.graph.add_graph_edge(
+                    doc_from=CREfromDB(cre),
+                    link_to=cre_defs.Link(document=nodeFromDB(node),ltype=type.value),
+                    graph=self.graph.graph
                 )
 
+
         self.session.commit()
 
     def find_path_between_nodes(

application/database/inmemory_graph.py

@@ -1,7 +1,6 @@
 import sys
 import networkx as nx
 from typing import List, Tuple
-from pprint import pprint
 from application.defs import cre_defs as defs
 
 
@@ -21,12 +20,31 @@ def instance(cls, documents: List[defs.Document] = None) -> "CRE_Graph":
     def __init__(sel):
         raise ValueError("CRE_Graph is a singleton, please call instance() instead")
 
-    def add_edge(self, *args, **kwargs):
-        return self.graph.add_edge(*args, **kwargs)
-
     def add_node(self, *args, **kwargs):
         return self.graph.add_node(*args, **kwargs)
 
+    def adds_cycle(self, doc_from: defs.Document, link_to: defs.Link):
+        try:
+            existing_cycle = nx.find_cycle(self.graph)
+            if existing_cycle:
+                raise ValueError(
+                    "Existing graph contains cycle,"
+                    "this not a recoverable error,"
+                    f" manual database actions are required {existing_cycle}"
+                )
+        except nx.exception.NetworkXNoCycle:
+            pass  # happy path, we don't want cycles
+        new_graph = self.graph.copy()
+
+        # this needs our special add_edge but with the copied graph
+        new_graph = self.add_graph_edge(
+            doc_from=doc_from, link_to=link_to, graph=new_graph
+        )
+        try:
+            return nx.find_cycle(new_graph)
+        except nx.NetworkXNoCycle:
+            return False
+
     def get_hierarchy(self, rootIDs: List[str], creID: str):
         if creID in rootIDs:
             return 0
@@ -104,6 +122,52 @@ def add_dbnode(cls, dbnode: defs.Node, graph: nx.DiGraph) -> nx.DiGraph:
             logger.error("Called with dbnode being none")
         return graph
 
+    @classmethod
+    def add_graph_edge(
+        cls,
+        doc_from: defs.Document,
+        link_to: defs.Link,
+        graph: nx.DiGraph,
+    ) -> nx.digraph:
+
+        to_doctype = defs.Credoctypes.CRE.value
+        if link_to.document.doctype != defs.Credoctypes.CRE.value:
+            to_doctype = "Node"
+
+        if doc_from.doctype == defs.Credoctypes.CRE:
+            if link_to.ltype == defs.LinkTypes.Contains:
+                graph.add_edge(
+                    f"{doc_from.doctype}: {doc_from.id}",
+                    f"{to_doctype}: {link_to.document.id}",
+                    ltype=link_to.ltype,
+                )
+            elif link_to.ltype == defs.LinkTypes.PartOf:
+                graph.add_edge(
+                    f"{to_doctype}: {link_to.document.id}",
+                    f"{doc_from.doctype}: {doc_from.id}",
+                    ltype=defs.LinkTypes.Contains,
+                )
+            elif link_to.ltype == defs.LinkTypes.Related:
+                # do nothing if the opposite already exists in the graph, otherwise we introduce a cycle
+                if graph.has_edge(
+                    f"{to_doctype}: {link_to.document.id}",
+                    f"{doc_from.doctype}: {doc_from.id}",
+                ):
+                    return graph
+
+                graph.add_edge(
+                    f"{doc_from.doctype}: {doc_from.id}",
+                    f"{to_doctype}: {link_to.document.id}",
+                    ltype=defs.LinkTypes.Related,
+                )
+        else:
+            graph.add_edge(
+                f"{doc_from.doctype}: {doc_from.id}",
+                f"{to_doctype}: {link_to.document.id}",
+                ltype=link_to.ltype,
+            )
+        return graph
+
     @classmethod
     def __load_cre_graph(cls, documents: List[defs.Document]) -> nx.Graph:
         graph = cls.graph
@@ -119,17 +183,10 @@ def __load_cre_graph(cls, documents: List[defs.Document]) -> nx.Graph:
                 graph = cls.add_dbnode(dbnode=doc, graph=graph)
                 from_doctype = doc.doctype
             for link in doc.links:
-                to_doctype = None
                 if link.document.doctype == defs.Credoctypes.CRE:
                     graph = cls.add_cre(dbcre=link.document, graph=graph)
-                    to_doctype = defs.Credoctypes.CRE
                 else:
                     graph = cls.add_dbnode(dbnode=link.document, graph=graph)
-                    to_doctype = "Node"
-                graph.add_edge(
-                    f"{from_doctype}: {doc.id}",
-                    f"{to_doctype}: {link.document.id}",
-                    ltype=link.ltype,
-                )
+            graph = cls.add_graph_edge(doc_from=doc, link_to=link, graph=graph)
         cls.graph = graph
         return graph