diff --git a/.trunk/configs/.cspell.json b/.trunk/configs/.cspell.json index a0dfac46a..ccdbc4d40 100644 --- a/.trunk/configs/.cspell.json +++ b/.trunk/configs/.cspell.json @@ -122,6 +122,7 @@ "Doctest", "Doctests", "linenums", - "XLYOFNOQVPJJNP" + "XLYOFNOQVPJJNP", + "CRIPTUUID" ] } diff --git a/src/cript/nodes/core.py b/src/cript/nodes/core.py index 3f43fedd4..c8daff22d 100644 --- a/src/cript/nodes/core.py +++ b/src/cript/nodes/core.py @@ -179,20 +179,13 @@ def _from_json(cls, json_dict: dict): if field_name not in arguments: arguments[field_name] = getattr(default_dataclass, field_name) - # If a node with this UUID already exists, we don't create a new node. - # Instead we use the existing node from the cache and just update it. - from cript.nodes.uuid_base import UUIDBaseNode - - if "uuid" in json_dict and json_dict["uuid"] in UUIDBaseNode._uuid_cache: - node = UUIDBaseNode._uuid_cache[json_dict["uuid"]] - else: # Create a new node - try: - node = cls(**arguments) - # TODO we should not catch all exceptions if we are handling them, and instead let it fail - # to create a good error message that points to the correct place that it failed to make debugging easier - except Exception as exc: - print(cls, arguments) - raise exc + try: + node = cls(**arguments) + # TODO we should not catch all exceptions if we are handling them, and instead let it fail + # to create a good error message that points to the correct place that it failed to make debugging easier + except Exception as exc: + print(cls, arguments) + raise exc attrs = cls.JsonAttributes(**arguments) @@ -213,6 +206,8 @@ def _from_json(cls, json_dict: dict): return node def __deepcopy__(self, memo): + from cript.nodes.util.core import get_uuid_from_uid + # Ideally I would call `asdict`, but that is not allowed inside a deepcopy chain. # Making a manual transform into a dictionary here. arguments = {} @@ -224,6 +219,8 @@ def __deepcopy__(self, memo): # a new uid will prompt the creation of a new matching uuid. uid = get_new_uid() arguments["uid"] = uid + if "uuid" in arguments: + arguments["uuid"] = get_uuid_from_uid(uid) # Create node and init constructor attributes node = self.__class__(**arguments) diff --git a/src/cript/nodes/exceptions.py b/src/cript/nodes/exceptions.py index 0d1942388..dfda18352 100644 --- a/src/cript/nodes/exceptions.py +++ b/src/cript/nodes/exceptions.py @@ -4,6 +4,19 @@ from cript.exceptions import CRIPTException +class CRIPTUUIDException(CRIPTException): + def __init__(self, uuid: str, old_type: str, new_type: str): + self.uuid = uuid + self.old_type = old_type + self.new_type = new_type + + def __str__(self) -> str: + return_msg = f"UUID collision error. A new node with UUID {self.uuid} is created of type {self.new_type}," + return_msg += f" but a node with the same UUID exists already as type {self.old_type}." + return_msg += " Please report the error on https://github.com/C-Accel-CRIPT/Python-SDK/issues , thank you." + return return_msg + + class CRIPTNodeSchemaError(CRIPTException): """ ## Definition @@ -335,27 +348,6 @@ def __str__(self) -> str: return ret_string -def get_orphaned_experiment_exception(orphaned_node): - """ - Return the correct specific Exception based in the orphaned node type for nodes not correctly listed in experiment. - """ - from cript.nodes.primary_nodes.computation import Computation - from cript.nodes.primary_nodes.computation_process import ComputationProcess - from cript.nodes.primary_nodes.data import Data - from cript.nodes.primary_nodes.process import Process - - if isinstance(orphaned_node, Data): - return CRIPTOrphanedDataError(orphaned_node) - if isinstance(orphaned_node, Process): - return CRIPTOrphanedProcessError(orphaned_node) - if isinstance(orphaned_node, Computation): - return CRIPTOrphanedComputationError(orphaned_node) - if isinstance(orphaned_node, ComputationProcess): - return CRIPTOrphanedComputationalProcessError(orphaned_node) - # Base case raise the parent exception. TODO add bug warning. - return CRIPTOrphanedExperimentError(orphaned_node) - - class CRIPTOrphanedDataError(CRIPTOrphanedExperimentError): """ ## Definition diff --git a/src/cript/nodes/primary_nodes/project.py b/src/cript/nodes/primary_nodes/project.py index 48e182463..c644221bc 100644 --- a/src/cript/nodes/primary_nodes/project.py +++ b/src/cript/nodes/primary_nodes/project.py @@ -105,10 +105,8 @@ def __init__(self, name: str, collection: Optional[List[Collection]] = None, mat self._update_json_attrs_if_valid(new_json_attrs) def validate(self, api=None, is_patch=False, force_validation: bool = False): - from cript.nodes.exceptions import ( - CRIPTOrphanedMaterialError, - get_orphaned_experiment_exception, - ) + from cript.nodes.exceptions import CRIPTOrphanedMaterialError + from cript.nodes.util.core import get_orphaned_experiment_exception # First validate like other nodes super().validate(api=api, is_patch=is_patch, force_validation=force_validation) diff --git a/src/cript/nodes/util/__init__.py b/src/cript/nodes/util/__init__.py index 635151b72..4b7e58be9 100644 --- a/src/cript/nodes/util/__init__.py +++ b/src/cript/nodes/util/__init__.py @@ -1,48 +1,9 @@ -from cript.nodes.exceptions import ( - CRIPTOrphanedComputationalProcessError, - CRIPTOrphanedComputationError, - CRIPTOrphanedDataError, - CRIPTOrphanedMaterialError, - CRIPTOrphanedProcessError, -) -from cript.nodes.primary_nodes.experiment import Experiment -from cript.nodes.primary_nodes.project import Project - # trunk-ignore-begin(ruff/F401) +from .core import ( + add_orphaned_nodes_to_project, + get_orphaned_experiment_exception, + get_uuid_from_uid, +) from .json import NodeEncoder, load_nodes_from_json # trunk-ignore-end(ruff/F401) - - -def add_orphaned_nodes_to_project(project: Project, active_experiment: Experiment, max_iteration: int = -1): - """ - Helper function that adds all orphaned material nodes of the project graph to the - `project.materials` attribute. - Material additions only is permissible with `active_experiment is None`. - This function also adds all orphaned data, process, computation and computational process nodes - of the project graph to the `active_experiment`. - This functions call `project.validate` and might raise Exceptions from there. - """ - if active_experiment is not None and active_experiment not in project.find_children({"node": ["Experiment"]}): - raise RuntimeError(f"The provided active experiment {active_experiment} is not part of the project graph. Choose an active experiment that is part of a collection of this project.") - - counter = 0 - while True: - if counter > max_iteration >= 0: - break # Emergency stop - try: - project.validate() - except CRIPTOrphanedMaterialError as exc: - # because calling the setter calls `validate` we have to force add the material. - project._json_attrs.material.append(exc.orphaned_node) - except CRIPTOrphanedDataError as exc: - active_experiment.data += [exc.orphaned_node] - except CRIPTOrphanedProcessError as exc: - active_experiment.process += [exc.orphaned_node] - except CRIPTOrphanedComputationError as exc: - active_experiment.computation += [exc.orphaned_node] - except CRIPTOrphanedComputationalProcessError as exc: - active_experiment.computation_process += [exc.orphaned_node] - else: - break - counter += 1 diff --git a/src/cript/nodes/util/core.py b/src/cript/nodes/util/core.py new file mode 100644 index 000000000..938ec7201 --- /dev/null +++ b/src/cript/nodes/util/core.py @@ -0,0 +1,69 @@ +import uuid + +from cript.nodes.exceptions import ( + CRIPTOrphanedComputationalProcessError, + CRIPTOrphanedComputationError, + CRIPTOrphanedDataError, + CRIPTOrphanedExperimentError, + CRIPTOrphanedMaterialError, + CRIPTOrphanedProcessError, +) + + +def get_uuid_from_uid(uid): + return str(uuid.UUID(uid[2:])) + + +def add_orphaned_nodes_to_project(project, active_experiment, max_iteration: int = -1): + """ + Helper function that adds all orphaned material nodes of the project graph to the + `project.materials` attribute. + Material additions only is permissible with `active_experiment is None`. + This function also adds all orphaned data, process, computation and computational process nodes + of the project graph to the `active_experiment`. + This functions call `project.validate` and might raise Exceptions from there. + """ + if active_experiment is not None and active_experiment not in project.find_children({"node": ["Experiment"]}): + raise RuntimeError(f"The provided active experiment {active_experiment} is not part of the project graph. Choose an active experiment that is part of a collection of this project.") + + counter = 0 + while True: + if counter > max_iteration >= 0: + break # Emergency stop + try: + project.validate() + except CRIPTOrphanedMaterialError as exc: + # because calling the setter calls `validate` we have to force add the material. + project._json_attrs.material.append(exc.orphaned_node) + except CRIPTOrphanedDataError as exc: + active_experiment.data += [exc.orphaned_node] + except CRIPTOrphanedProcessError as exc: + active_experiment.process += [exc.orphaned_node] + except CRIPTOrphanedComputationError as exc: + active_experiment.computation += [exc.orphaned_node] + except CRIPTOrphanedComputationalProcessError as exc: + active_experiment.computation_process += [exc.orphaned_node] + else: + break + counter += 1 + + +def get_orphaned_experiment_exception(orphaned_node): + """ + Return the correct specific Exception based in the orphaned node type for nodes not correctly listed in experiment. + """ + from cript.nodes.primary_nodes.computation import Computation + from cript.nodes.primary_nodes.computation_process import ComputationProcess + from cript.nodes.primary_nodes.data import Data + from cript.nodes.primary_nodes.process import Process + + if isinstance(orphaned_node, Data): + return CRIPTOrphanedDataError(orphaned_node) + if isinstance(orphaned_node, Process): + return CRIPTOrphanedProcessError(orphaned_node) + if isinstance(orphaned_node, Computation): + return CRIPTOrphanedComputationError(orphaned_node) + if isinstance(orphaned_node, ComputationProcess): + return CRIPTOrphanedComputationalProcessError(orphaned_node) + # Base case raise the parent exception. TODO add bug warning. + return CRIPTOrphanedExperimentError(orphaned_node) diff --git a/src/cript/nodes/util/json.py b/src/cript/nodes/util/json.py index e61ff2307..274c1df1c 100644 --- a/src/cript/nodes/util/json.py +++ b/src/cript/nodes/util/json.py @@ -14,6 +14,7 @@ CRIPTJsonDeserializationError, CRIPTJsonNodeError, ) +from cript.nodes.uuid_base import UUIDBaseNode class NodeEncoder(json.JSONEncoder): @@ -294,7 +295,7 @@ def __call__(self, node_str: Union[Dict, str]) -> Dict: return node_dict -def load_nodes_from_json(nodes_json: Union[str, Dict]): +def load_nodes_from_json(nodes_json: Union[str, Dict], _use_uuid_cache: Optional[Dict] = None): """ User facing function, that return a node and all its children from a json string input. @@ -349,7 +350,22 @@ def load_nodes_from_json(nodes_json: Union[str, Dict]): # but catches a lot of odd cases. And at the moment performance is not bottle necked here if not isinstance(nodes_json, str): nodes_json = json.dumps(nodes_json) - return json.loads(nodes_json, object_hook=node_json_hook) + + # Store previous UUIDBaseNode Cache state + previous_uuid_cache = UUIDBaseNode._uuid_cache + + if _use_uuid_cache is not None: # If requested use a custom cache. + UUIDBaseNode._uuid_cache = _use_uuid_cache + + try: + loaded_nodes = json.loads(nodes_json, object_hook=node_json_hook) + finally: + # Definitively restore the old cache state + UUIDBaseNode._uuid_cache = previous_uuid_cache + + if _use_uuid_cache is not None: + return loaded_nodes, _use_uuid_cache + return loaded_nodes def _rename_field(serialize_dict: Dict, old_name: str, new_name: str) -> Dict: diff --git a/src/cript/nodes/uuid_base.py b/src/cript/nodes/uuid_base.py index 1a3aa6e33..87fac9043 100644 --- a/src/cript/nodes/uuid_base.py +++ b/src/cript/nodes/uuid_base.py @@ -1,13 +1,10 @@ import uuid from abc import ABC from dataclasses import dataclass, field, replace -from typing import Any, Dict +from typing import Any, Dict, Optional from cript.nodes.core import BaseNode - - -def get_uuid_from_uid(uid): - return str(uuid.UUID(uid[2:])) +from cript.nodes.exceptions import CRIPTUUIDException class UUIDBaseNode(BaseNode, ABC): @@ -32,16 +29,26 @@ class JsonAttributes(BaseNode.JsonAttributes): _json_attrs: JsonAttributes = JsonAttributes() + def __new__(cls, *args, **kwargs): + uuid: Optional[str] = kwargs.get("uuid") + if uuid and uuid in UUIDBaseNode._uuid_cache: + existing_node_to_overwrite = UUIDBaseNode._uuid_cache[uuid] + if type(existing_node_to_overwrite) is not cls: + raise CRIPTUUIDException(uuid, type(existing_node_to_overwrite), cls) + return existing_node_to_overwrite + new_uuid_node = super().__new__(cls) + return new_uuid_node + def __init__(self, **kwargs): + from cript.nodes.util.core import get_uuid_from_uid + # initialize Base class with node super().__init__(**kwargs) # Respect uuid if passed as argument, otherwise construct uuid from uid - uuid = kwargs.get("uuid", get_uuid_from_uid(self.uid)) + uuid: str = kwargs.get("uuid", get_uuid_from_uid(self.uid)) # replace name and notes within PrimaryBase self._json_attrs = replace(self._json_attrs, uuid=uuid) - - # Place successfully created node in the UUID cache - self._uuid_cache[uuid] = self + UUIDBaseNode._uuid_cache[uuid] = self @property def uuid(self) -> uuid.UUID: @@ -54,11 +61,6 @@ def url(self): api = _get_global_cached_api() return f"{api.host}/{api.api_prefix}/{api.api_version}/{self.uuid}" - def __deepcopy__(self, memo): - node = super().__deepcopy__(memo) - node._json_attrs = replace(node._json_attrs, uuid=get_uuid_from_uid(node.uid)) - return node - @property def updated_by(self): return self._json_attrs.updated_by diff --git a/tests/test_node_util.py b/tests/test_node_util.py index 3b03e93b1..806d4799b 100644 --- a/tests/test_node_util.py +++ b/tests/test_node_util.py @@ -344,3 +344,23 @@ def test_expanded_json(complex_project_node): # raise CRIPTJsonDeserializationError with pytest.raises(cript.nodes.exceptions.CRIPTJsonDeserializationError): cript.load_nodes_from_json(condensed_json) + + +def test_uuid_cache_override(complex_project_node): + normal_serial = complex_project_node.get_expanded_json() + reloaded_project = cript.load_nodes_from_json(normal_serial) + + # For a normal load, the reloaded node as to be the same as before. + assert reloaded_project is complex_project_node + + # Load with custom cache override + custom_project, cache = cript.load_nodes_from_json(normal_serial, _use_uuid_cache=dict()) + + assert custom_project is not reloaded_project + + # Make sure that the nodes in the different caches are different + for key in cache: + old_node = cript.nodes.uuid_base.UUIDBaseNode._uuid_cache[key] + new_node = cache[key] + assert old_node.uuid == new_node.uuid + assert old_node is not new_node