From 25eac2f3c59ec6350a2b9a9de766049b4bf1fc37 Mon Sep 17 00:00:00 2001 From: Stig Ofstad Date: Tue, 3 Oct 2023 13:24:32 +0200 Subject: [PATCH] refactor: remove last traces of 'update_uncontained' --- src/features/document/document_feature.py | 6 ---- .../use_cases/add_document_use_case.py | 28 ++++++------------- .../use_cases/update_document_use_case.py | 6 +--- .../export/use_cases/export_use_case.py | 18 +++++++----- src/services/document_service.py | 21 ++------------ .../create_entity_with_user_as_owner.feature | 2 +- src/tests/bdd/document/add_file.feature | 16 +++++------ src/tests/bdd/document/remove.feature | 2 +- src/tests/bdd/document/remove_2.feature | 2 +- src/tests/bdd/steps/domain.py | 4 ++- .../test_data_source_and_repositories.py | 14 ++++++++-- .../services/document_service/test_save.py | 6 ++-- .../test_tree_node_update.py | 3 -- src/tests/unit/use_cases/test_reference.py | 1 - 14 files changed, 52 insertions(+), 77 deletions(-) diff --git a/src/features/document/document_feature.py b/src/features/document/document_feature.py index 09c01fd45..1dfd89ff3 100644 --- a/src/features/document/document_feature.py +++ b/src/features/document/document_feature.py @@ -56,7 +56,6 @@ def update( id_address: str, data: Json = Form(...), files: Optional[List[UploadFile]] = File(None), - update_uncontained: Optional[bool] = False, user: User = Depends(auth_w_jwt_or_pat), ): """Update an Existing Document in the Database. @@ -71,7 +70,6 @@ def update( - The PROTOCOL is optional, and the default is dmss. - document (dict): The document to replace the previous version. - files: Optional list of files to be stored as part of this document. - - update_uncontained (bool): Optional flag specifying whether to also update uncontained attributes in the document. - user (User): The authenticated user accessing the endpoint, automatically generated from provided bearer token or Access-Key. Returns: @@ -89,7 +87,6 @@ def update( address=Address.from_absolute(id_address), data=data, files=files, - update_uncontained=update_uncontained, document_service=document_service, ) @@ -100,7 +97,6 @@ def add_document( address: str, document: Json = Form(...), files: Optional[List[UploadFile]] = File(None), - update_uncontained: Optional[bool] = False, user: User = Depends(auth_w_jwt_or_pat), ): """Add a document to a package or a data source using an address. @@ -118,7 +114,6 @@ def add_document( - The PROTOCOL is optional, and the default is dmss. - document (dict): The document that is to be stored. - files: Optional list of files to be stored as part of this document. - - update_uncontained (bool): Optional flag specifying whether to also update uncontained attributes in the document. - user (User): The authenticated user accessing the endpoint, automatically generated from provided bearer token or Access-Key. Returns: @@ -137,7 +132,6 @@ def add_document( address=Address.from_absolute(address), document=document, files=files, - update_uncontained=update_uncontained, document_service=document_service, ) diff --git a/src/features/document/use_cases/add_document_use_case.py b/src/features/document/use_cases/add_document_use_case.py index 6940bbed3..64a31847c 100644 --- a/src/features/document/use_cases/add_document_use_case.py +++ b/src/features/document/use_cases/add_document_use_case.py @@ -18,15 +18,12 @@ from services.document_service import DocumentService -def _add_document_to_data_source( - data_source_id: str, document: dict, update_uncontained: Optional[bool], document_service: DocumentService -) -> dict: +def _add_document_to_data_source(data_source_id: str, document: dict, document_service: DocumentService) -> dict: """Add the document to an existing data source. Args: data_source_id: The data source ID document: The entity to be added - update_uncontained: Whether to update uncontained children (deprecated) document_service: The document service Returns: @@ -53,7 +50,7 @@ def _add_document_to_data_source( new_node.set_uid(new_node.generate_id()) - document_service.save(new_node, data_source_id, update_uncontained=update_uncontained) + document_service.save(new_node, data_source_id) return {"uid": new_node.node_id} @@ -62,7 +59,6 @@ def _add_document_to_entity_or_list( address: Address, document: dict, files: dict[str, BinaryIO] | None, - update_uncontained: Optional[bool], document_service: DocumentService, ) -> dict: """Add the document to an existing entity. @@ -71,7 +67,6 @@ def _add_document_to_entity_or_list( address: Reference to an existing entity or to an attribute (complex or list attribute) inside an entity. document: The entity to be added files: Dict with names and files of the files contained in the document - update_uncontained: Whether to update uncontained children (deprecated) Returns: A dict that contains the ID of the added document. @@ -131,16 +126,12 @@ def _add_document_to_entity_or_list( ) parent_node.add_child(list_node) list_node.add_child(new_node) - document_service.save( - parent_node, address.data_source, update_uncontained=update_uncontained, initial=True - ) + document_service.save(parent_node, address.data_source, initial=True) return {"uid": f"{list_node.node_id}"} else: parent_node.add_child(new_node) - document_service.save( - parent_node, address.data_source, update_uncontained=update_uncontained, initial=True - ) + document_service.save(parent_node, address.data_source, initial=True) return {"uid": f"{new_node.node_id}"} if target.type != SIMOS.PACKAGE.value and target.type != "object": @@ -180,13 +171,13 @@ def _add_document_to_entity_or_list( if new_node.should_have_id(): new_node.set_uid(new_node.generate_id()) target.add_child(new_node) - document_service.save(target.find_parent(), address.data_source, update_uncontained=False) + document_service.save(target.find_parent(), address.data_source) else: new_node.parent = target.parent target.parent.replace(new_node.node_id, new_node) - document_service.save(target.find_parent(), address.data_source, update_uncontained=False) + document_service.save(target.find_parent(), address.data_source) - document_service.save(new_node, address.data_source, update_uncontained=update_uncontained) + document_service.save(new_node, address.data_source) return {"uid": new_node.node_id} @@ -196,7 +187,6 @@ def add_document_use_case( address: Address, document_service: DocumentService, files: Optional[List[UploadFile]] = None, - update_uncontained: Optional[bool] = False, ) -> dict: """Add document to a data source or existing entity. Can also be used to add (complex) items to a list. @@ -205,7 +195,6 @@ def add_document_use_case( address: Reference to a package, attribute inside an entity (either a list or a complex attribute) or a data source document_service: The document service files: Dict with names and files of the files contained in the document - update_uncontained: Whether to update uncontained children (deprecated) Returns: A dict that contains the ID of the added document. @@ -213,12 +202,11 @@ def add_document_use_case( validate_entity_against_self(document, document_service.get_blueprint) if not address.path: - return _add_document_to_data_source(address.data_source, document, update_uncontained, document_service) + return _add_document_to_data_source(address.data_source, document, document_service) return _add_document_to_entity_or_list( address=address, document=document, files={f.filename: f.file for f in files} if files else None, - update_uncontained=update_uncontained, document_service=document_service, ) diff --git a/src/features/document/use_cases/update_document_use_case.py b/src/features/document/use_cases/update_document_use_case.py index e27d047f7..c31230fbf 100644 --- a/src/features/document/use_cases/update_document_use_case.py +++ b/src/features/document/use_cases/update_document_use_case.py @@ -18,7 +18,6 @@ def _update_document( data: Union[dict, list], document_service: DocumentService, files: dict[str, BinaryIO] | None, - update_uncontained: Optional[bool], ): """ Update a document. @@ -50,7 +49,7 @@ def _update_document( if files: merge_entity_and_files(node, files) - document_service.save(node, address.data_source, update_uncontained=update_uncontained, initial=True) + document_service.save(node, address.data_source, initial=True) logger.info(f"Updated entity '{address}'") return {"data": tree_node_to_dict(node)} @@ -60,7 +59,6 @@ def update_document_use_case( data: Union[dict, list], document_service: DocumentService, files: Optional[List[UploadFile]] = None, - update_uncontained: Optional[bool] = True, ): """Update document. @@ -69,7 +67,6 @@ def update_document_use_case( data: The data to be updated document_service: The document service files: Dict with names and files of the files contained in the document - update_uncontained: Whether to update uncontained children (deprecated) Returns: A dict that contains the updated document. """ @@ -79,7 +76,6 @@ def update_document_use_case( data=data, document_service=document_service, files={f.filename: f.file for f in files} if files else None, - update_uncontained=update_uncontained, ) # Do not invalidate the blueprint cache if it was not a blueprint that was changed if "type" in document["data"] and document["data"]["type"] == SIMOS.BLUEPRINT.value: diff --git a/src/features/export/use_cases/export_use_case.py b/src/features/export/use_cases/export_use_case.py index 1c06949a8..7414460cb 100644 --- a/src/features/export/use_cases/export_use_case.py +++ b/src/features/export/use_cases/export_use_case.py @@ -23,13 +23,17 @@ def save_node_to_zipfile( if document_meta: document_node.entity["_meta_"] = document_meta storage_client = ZipFileClient(zip_file) - document_service.save( - document_node, - data_source_id, - storage_client, - update_uncontained=True, - combined_document_meta=document_meta, - ) + path = "" # Path here is used to get the proper file structure in the zip file + for node in document_node.traverse(): + if not node.storage_contained and not node.is_array(): + path = f"{path}/{node.entity['name']}/" if path else f"{node.entity['name']}" + document_service.save( + node=node, + data_source_id=data_source_id, + path=path, + repository=storage_client, + combined_document_meta=document_meta, + ) def create_zip_export(document_service: DocumentService, address: Address, user: User) -> str: diff --git a/src/services/document_service.py b/src/services/document_service.py index ab338fe12..68ac211a5 100644 --- a/src/services/document_service.py +++ b/src/services/document_service.py @@ -109,14 +109,12 @@ def save( data_source_id: str, repository=None, path="", - update_uncontained: bool = False, combined_document_meta: dict | None = None, initial: bool = False, ) -> Dict: """ - Recursively saves a Node. - Digs down to the leaf children, and based on storageContained, - either saves the entity and returns the Reference, OR returns the entire entity. + Saves a Node. + Converting uncontained child entities to references. node: The Node to save path: A Filesystem equivalent path for this node. Used when writing zip-files. @@ -132,9 +130,7 @@ def save( """ if initial and node.storage_contained: - self.save( - node.parent, data_source_id, repository, path, update_uncontained, combined_document_meta, initial - ) + self.save(node.parent, data_source_id, repository, path, combined_document_meta, initial) if not node.entity: return {} # If not passed a custom repository to save into, use the DocumentService's storage @@ -144,7 +140,6 @@ def save( # If the node is a package, we build the path string to be used by filesystem like repositories. # Also, check for duplicate names in the package. if node.type == SIMOS.PACKAGE.value: - path = f"{path}/{node.entity['name']}/" if path else f"{node.entity['name']}" if len(node.children) > 0: packageContent = node.children[0] contentListNames = [] @@ -158,16 +153,6 @@ def save( contentListNames.append(child.entity["name"]) - if update_uncontained: # If flag is set, dig down and save uncontained documents - for child in node.children: - if child.is_array(): - [ - self.save(x, data_source_id, repository, path, update_uncontained, combined_document_meta) - for x in child.children - ] - else: - self.save(child, data_source_id, repository, path, update_uncontained, combined_document_meta) - if node.type == SIMOS.BLOB.value: node.entity = self.save_blob_data(node, repository) diff --git a/src/tests/bdd/authentication/create_entity_with_user_as_owner.feature b/src/tests/bdd/authentication/create_entity_with_user_as_owner.feature index 07143f908..6bc338351 100644 --- a/src/tests/bdd/authentication/create_entity_with_user_as_owner.feature +++ b/src/tests/bdd/authentication/create_entity_with_user_as_owner.feature @@ -19,7 +19,7 @@ Feature: Set logged in user as owner when creating an entity """ Given the logged in user is "johndoe" with roles "dmss-admin" Given authentication is enabled - Given i access the resource url "/api/documents/test-DS/$2.content?update_uncontained=False" + Given i access the resource url "/api/documents/test-DS/$2.content" When i make a form-data "POST" request """ { diff --git a/src/tests/bdd/document/add_file.feature b/src/tests/bdd/document/add_file.feature index 80f1c055b..424ef8289 100644 --- a/src/tests/bdd/document/add_file.feature +++ b/src/tests/bdd/document/add_file.feature @@ -339,7 +339,7 @@ Feature: Explorer - Add file """ Scenario: Add file - not contained - Given i access the resource url "/api/documents/test-DS/$1.content?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/$1.content" When i make a form-data "POST" request """ { @@ -502,7 +502,7 @@ Feature: Explorer - Add file Then the response status should be "OK" Scenario: Add Parent entity without a name attribute with -by-path endpoint - Given i access the resource url "/api/documents/test-DS/root_package?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/root_package" When i make a "POST" request with "1" files """ { @@ -557,7 +557,7 @@ Feature: Explorer - Add file And the response should have valid uid Scenario: Add Comment entity without a name attribute with -by-path endpoint - Given i access the resource url "/api/documents/test-DS/root_package?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/root_package" When i make a "POST" request with "1" files """ { @@ -572,7 +572,7 @@ Feature: Explorer - Add file Then the response status should be "OK" Scenario: Add blueprint without a name attribute with -by-path endpoint should fail - Given i access the resource url "/api/documents/test-DS/root_package?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/root_package" When i make a "POST" request with "1" files """ { @@ -596,7 +596,7 @@ Feature: Explorer - Add file """ Scenario: Add package without a name attribute with -by-path endpoint should fail - Given i access the resource url "/api/documents/test-DS/root_package?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/root_package" When i make a "POST" request with "1" files """ { @@ -620,7 +620,7 @@ Feature: Explorer - Add file """ Scenario: Add parent entity without a name attribute with add_by_parent_id endpoint - Given i access the resource url "/api/documents/test-DS/1.content?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/1.content" When i make a form-data "POST" request """ { @@ -643,7 +643,7 @@ Feature: Explorer - Add file """ Scenario: Add comment entity without a name attribute with add_by_parent_id endpoint - Given i access the resource url "/api/documents/test-DS/$1.content?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/$1.content" When i make a form-data "POST" request """ { @@ -703,7 +703,7 @@ Feature: Explorer - Add file """ Scenario: Add file with multiple PDFs - Given i access the resource url "/api/documents/test-DS/root_package?update_uncontained=True" + Given i access the resource url "/api/documents/test-DS/root_package" When i make a "POST" request with "4" files """ { diff --git a/src/tests/bdd/document/remove.feature b/src/tests/bdd/document/remove.feature index 16a144c06..d5d28bd64 100644 --- a/src/tests/bdd/document/remove.feature +++ b/src/tests/bdd/document/remove.feature @@ -19,7 +19,7 @@ Feature: Explorer - Remove | 4 | 1 | sub_package_2 | | dmss://system/SIMOS/Package | | 3 | 2 | document_1 | | dmss://system/SIMOS/Blueprint | - Scenario: Remove root package + Scenario: Remove root package by id Given i access the resource url "/api/documents/data-source-name/$1" When i make a "DELETE" request Then the response status should be "OK" diff --git a/src/tests/bdd/document/remove_2.feature b/src/tests/bdd/document/remove_2.feature index 9eac6c4af..5909f4a56 100644 --- a/src/tests/bdd/document/remove_2.feature +++ b/src/tests/bdd/document/remove_2.feature @@ -13,7 +13,7 @@ Feature: Explorer - Remove by path | 4 | 1 | sub_package_2 | | dmss://system/SIMOS/Package | | 3 | 2 | document_1 | | dmss://system/SIMOS/Blueprint | - Scenario: Remove root package + Scenario: Remove root package by name Given i access the resource url "/api/documents/data-source-name/blueprints" When i make a "DELETE" request Then the response status should be "OK" diff --git a/src/tests/bdd/steps/domain.py b/src/tests/bdd/steps/domain.py index 5c3a7e8c6..f0470e7ac 100644 --- a/src/tests/bdd/steps/domain.py +++ b/src/tests/bdd/steps/domain.py @@ -93,7 +93,9 @@ def step_impl_documents(context, data_source_id: str, collection: str): document_service = DocumentService(get_data_source, user=context.user) tree: Node = generate_tree(data_source_id, context.table, document_service) tree.show_tree() - document_service.save(node=tree, data_source_id=data_source_id, update_uncontained=True) + for node in tree.traverse(): + if not node.storage_contained and not node.is_array(): + document_service.save(node=node, data_source_id=data_source_id) @given('AccessControlList for document "{document_id}" in data-source "{data_source}" is') diff --git a/src/tests/unit/services/document_service/test_data_source_and_repositories.py b/src/tests/unit/services/document_service/test_data_source_and_repositories.py index da9789102..f85d907ce 100644 --- a/src/tests/unit/services/document_service/test_data_source_and_repositories.py +++ b/src/tests/unit/services/document_service/test_data_source_and_repositories.py @@ -38,7 +38,11 @@ def test_save_into_multiple_repositories(self): "name": "Parent", "description": "", "type": "uncontained_blueprint", - "uncontained_in_every_way": {"name": "a_reference", "type": "dmss://system/SIMOS/NamedEntity"}, + "uncontained_in_every_way": { + "name": "a_reference", + "type": "dmss://system/SIMOS/NamedEntity", + "_id": "$2", + }, } default_doc_storage = {} @@ -95,7 +99,9 @@ def mock_find_one(**kwargs): recipe_provider=self.recipe_provider, ) - self.mock_document_service.save(node, "testing", update_uncontained=True) + for sub_node in node.traverse(): + if not sub_node.storage_contained and not sub_node.is_array(): + self.mock_document_service.save(node=sub_node, data_source_id="testing") # Test that both repos gets written into assert blob_doc_storage and default_doc_storage @@ -220,7 +226,9 @@ def mock_find_one(**kwargs): blob_doc, self.mock_document_service.get_blueprint, uid="1", recipe_provider=self.recipe_provider ) - self.mock_document_service.save(node, "testing", update_uncontained=True) + for sub_node in node.traverse(): + if not sub_node.storage_contained and not sub_node.is_array(): + self.mock_document_service.save(node=sub_node, data_source_id="testing") # Test that both repos gets written into assert blob_doc_storage and default_doc_storage diff --git a/src/tests/unit/services/document_service/test_save.py b/src/tests/unit/services/document_service/test_save.py index 3c4a92ff3..431bdf737 100644 --- a/src/tests/unit/services/document_service/test_save.py +++ b/src/tests/unit/services/document_service/test_save.py @@ -72,7 +72,9 @@ def mock_update(entity: dict, *args, **kwargs): contained_node.update( {"_id": "4", "name": "ref2", "description": "TEST_MODIFY", "type": "dmss://system/SIMOS/NamedEntity"} ) - self.mock_document_service.save(node, "testing", update_uncontained=True) + for sub_node in node.traverse(): + if not sub_node.storage_contained and not sub_node.is_array(): + self.mock_document_service.save(node=sub_node, data_source_id="testing") assert doc_storage["4"] == { "_id": "4", @@ -108,7 +110,7 @@ def mock_update(entity: dict, *args, **kwargs): contained_node.update( {"name": "RENAMED", "description": "TEST_MODIFY", "type": "dmss://system/SIMOS/NamedEntity"} ) - self.mock_document_service.save(contained_node, "testing", update_uncontained=True, initial=True) + self.mock_document_service.save(contained_node, "testing", initial=True) assert doc_storage["1"]["containedPersonInfo"]["description"] == "TEST_MODIFY" diff --git a/src/tests/unit/tree_functionality/test_tree_node_update.py b/src/tests/unit/tree_functionality/test_tree_node_update.py index 5d3e5ba7c..d0ed9b135 100644 --- a/src/tests/unit/tree_functionality/test_tree_node_update.py +++ b/src/tests/unit/tree_functionality/test_tree_node_update.py @@ -104,7 +104,6 @@ def repository_provider(data_source_id, user: User): add_document_use_case( address=Address("$1.box", "testing"), document={"type": "Box", "name": "box", "description": "box"}, - update_uncontained=True, document_service=document_service, ) assert get_and_print_diff(doc_storage["1"], doc_1_after) == [] @@ -198,7 +197,6 @@ def repository_provider(data_source_id, user: User): add_document_use_case( address=Address("$1.chest.box", "testing"), document={"name": "box", "description": "box", "type": "Box"}, - update_uncontained=True, document_service=document_service, ) @@ -236,7 +234,6 @@ def mock_update(entity: dict, *args, **kwargs): add_document_use_case( address=Address("$1.box", "testing"), document={"type": "Box", "name": "duplicate", "description": "box"}, - update_uncontained=True, document_service=document_service, ) diff --git a/src/tests/unit/use_cases/test_reference.py b/src/tests/unit/use_cases/test_reference.py index b175bf600..08bb307e0 100644 --- a/src/tests/unit/use_cases/test_reference.py +++ b/src/tests/unit/use_cases/test_reference.py @@ -323,7 +323,6 @@ def mock_update(entity: dict, *args, **kwargs): add_document_use_case( address=Address("$1.uncontained_in_every_way", "testing"), document=reference, - update_uncontained=True, document_service=self.document_service, ) assert len(doc_storage["1"]["uncontained_in_every_way"]) == 2