From 0903ec4f4d311287600ba17457dfb712adad255f Mon Sep 17 00:00:00 2001 From: Spyros Date: Fri, 20 Sep 2024 20:48:53 +0100 Subject: [PATCH] bugfix/552-existing-graph-contains-cycle-the-graph-changes --- application/database/db.py | 55 +++++++---------- application/database/inmemory_graph.py | 81 ++++++++++++++++++++++---- 2 files changed, 90 insertions(+), 46 deletions(-) diff --git a/application/database/db.py b/application/database/db.py index eb256194..035249d8 100644 --- a/application/database/db.py +++ b/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( diff --git a/application/database/inmemory_graph.py b/application/database/inmemory_graph.py index c8c4f1e8..92c0793d 100644 --- a/application/database/inmemory_graph.py +++ b/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