feat!: eliminate identifier and version concatenation, DeepDiff test

schivmeister · schivmeister · commit 3374147ecab1 · 2026-03-12T22:43:57.000+06:00
A package identifier should be interpretable standalone and should not
be a composite. The version of the mapping should not enable the
existence of multiple packages as that is a potentially problematic
situation to be in.

Also, update a DeepDiff-based test to do a more basic and simple
field-based comparison while we transition fully to MSSDK (the
filesystem-based functionality will have to be deprecated/replaced,
otherwise the models will not match).
diff --git a/src/ted_sws/core/model/transform.py b/src/ted_sws/core/model/transform.py
@@ -369,8 +369,3 @@ def _populate_legacy_from_mssdk(self) -> None:
                 resources=resources,
                 rml_mapping_rules=rml_rules
             )
-
-    # TODO check this out and remove if not needed (see if any production package ID does not come with version)
-    def get_mongodb_id(self) -> str:
-        """Get MongoDB _id for this package."""
-        return f"{self.id}_v{self.version}"
diff --git a/src/ted_sws/notice_metadata_processor/services/notice_eligibility.py b/src/ted_sws/notice_metadata_processor/services/notice_eligibility.py
@@ -89,11 +89,11 @@ def notice_eligibility_checker(notice: Notice, mapping_package_repository: Mappi
 
     if possible_mapping_packages:
         best_version = possible_mapping_packages[0].version
-        mapping_package_identifier_with_version = possible_mapping_packages[0].get_mongodb_id()
+        mapping_package_identifier_with_version = possible_mapping_packages[0].identifier
         for mapping_package in possible_mapping_packages[1:]:
             if semantic_version.Version(mapping_package.version) > semantic_version.Version(best_version):
                 best_version = mapping_package.version
-                mapping_package_identifier_with_version = mapping_package.get_mongodb_id()
+                mapping_package_identifier_with_version = mapping_package.identifier
 
         notice.set_is_eligible_for_transformation(eligibility=True)
         return notice.ted_id, mapping_package_identifier_with_version
diff --git a/src/ted_sws/notice_transformer/services/notice_transformer.py b/src/ted_sws/notice_transformer/services/notice_transformer.py
@@ -41,7 +41,7 @@ def transform_notice(notice: Notice, mapping_package: MappingPackage, rml_mapper
             file.write(notice.xml_manifestation.object_data)
         rdf_result = rml_mapper.execute(package_path=package_path)
         notice.set_rdf_manifestation(
-            rdf_manifestation=RDFManifestation(mapping_package_id=mapping_package.get_mongodb_id(),
+            rdf_manifestation=RDFManifestation(mapping_package_id=mapping_package.identifier,
                                                object_data=rdf_result))
     return notice
 
diff --git a/src/ted_sws/notice_validator/adapters/xpath_coverage_runner.py b/src/ted_sws/notice_validator/adapters/xpath_coverage_runner.py
@@ -30,7 +30,7 @@ class CoverageRunner:
     def __init__(self, mapping_package: MappingPackage):
         """"""
         self.mapping_package = mapping_package
-        self.mapping_package_id = mapping_package.get_mongodb_id()
+        self.mapping_package_id = mapping_package.identifier
         self.init_xpath_data(mapping_package=mapping_package)
 
     def init_xpath_data(self, mapping_package: MappingPackage):
diff --git a/src/ted_sws/notice_validator/services/shacl_test_suite_runner.py b/src/ted_sws/notice_validator/services/shacl_test_suite_runner.py
@@ -59,7 +59,7 @@ def execute_test_suite(self) -> SHACLTestSuiteValidationReport:
         except Exception as e:
             shacl_shape_validation_result.error = str(e)[:100]
 
-        return SHACLTestSuiteValidationReport(mapping_package_identifier=self.mapping_package.get_mongodb_id(),
+        return SHACLTestSuiteValidationReport(mapping_package_identifier=self.mapping_package.identifier,
                                               test_suite_identifier=self.shacl_test_suite.identifier,
                                               validation_results=shacl_shape_validation_result,
                                               object_data="SHACLTestSuiteExecution")
diff --git a/src/ted_sws/notice_validator/services/sparql_test_suite_runner.py b/src/ted_sws/notice_validator/services/sparql_test_suite_runner.py
@@ -113,7 +113,7 @@ def execute_test_suite(self) -> SPARQLTestSuiteValidationReport:
         """
         sparql_runner = SPARQLRunner(self.rdf_manifestation.object_data)
         test_suite_executions = SPARQLTestSuiteValidationReport(
-            mapping_package_identifier=self.mapping_package.get_mongodb_id(),
+            mapping_package_identifier=self.mapping_package.identifier,
             test_suite_identifier=self.sparql_test_suite.identifier,
             validation_results=[],
             object_data="SPARQLTestSuiteExecution")
diff --git a/test/unit/data_manager/conftest.py b/test/unit/data_manager/conftest.py
@@ -55,5 +55,5 @@ def daily_supra_notice():
 
 
 @pytest.fixture
-def fake_mapping_package_identifier_with_version(fake_mapping_package):
-    return fake_mapping_package.get_mongodb_id()
+def fake_mapping_package_identifier(fake_mapping_package):
+    return fake_mapping_package.identifier
diff --git a/test/unit/data_manager/test_mapping_package_repository.py b/test/unit/data_manager/test_mapping_package_repository.py
@@ -1,19 +1,17 @@
-from deepdiff import DeepDiff
-
 from src.ted_sws.data_manager.adapters.mapping_package_repository import MappingPackageRepositoryMongoDB, \
     MappingPackageRepositoryInFileSystem
 
 
 def test_mapping_package_repository_mongodb(mongodb_client, fake_mapping_package,
-                                          fake_mapping_package_identifier_with_version, aggregates_database_name):
+                                          fake_mapping_package_identifier, aggregates_database_name):
     mapping_package_repository = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
     mapping_package_repository.add(mapping_package=fake_mapping_package)
-    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier_with_version)
+    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier)
     assert result_mapping_package
     assert result_mapping_package.identifier == fake_mapping_package.identifier
     result_mapping_package.title = "updated_title"
     mapping_package_repository.update(mapping_package=result_mapping_package)
-    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier_with_version)
+    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier)
     assert result_mapping_package.shacl_test_suites[0].identifier == "fake_shacl_test_suite"
     assert result_mapping_package.title == "updated_title"
     result_mapping_packages = list(mapping_package_repository.list())
@@ -22,17 +20,24 @@ def test_mapping_package_repository_mongodb(mongodb_client, fake_mapping_package
 
 
 def test_mapping_package_repository_mongodb_update_invalid_id(mongodb_client, fake_mapping_package,
-                                                            fake_mapping_package_identifier_with_version,
+                                                            fake_mapping_package_identifier,
                                                             aggregates_database_name):
     mapping_package_repository = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
     mapping_package_repository.add(mapping_package=fake_mapping_package)
-    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier_with_version)
+    result_mapping_package = mapping_package_repository.get(reference=fake_mapping_package_identifier)
     assert result_mapping_package
     assert result_mapping_package.identifier == fake_mapping_package.identifier
     result_mapping_package.identifier = "updated_id"
     result_mapping_package.title = "updated_title"
     mapping_package_repository.update(mapping_package=result_mapping_package)
-    result_mapping_package = mapping_package_repository.get(reference=result_mapping_package.identifier)
+    try:
+        result_mapping_package = mapping_package_repository.get(reference=result_mapping_package.identifier)
+    except Exception as e:
+        from mapping_suite_sdk.core.adapters.repository import ModelNotFoundError
+        if isinstance(e, ModelNotFoundError):
+            result_mapping_package = None
+        else:
+            raise
     assert result_mapping_package is None
     mongodb_client.drop_database(aggregates_database_name)
 
@@ -74,18 +79,32 @@ def test_mapping_package_repository_in_file_system(file_system_repository_path,
     assert len(result_mapping_packages) == 2
     mapping_package_repository.clear_repository()
 
-
+# FIXME: during the transition to MSSDK the models for writing will be different, so do a very minimal test on specific fields
 def test_inter_transactions_mapping_package_repositories(mongodb_client, file_system_repository_path, fake_mapping_package,
-                                                       fake_mapping_package_identifier_with_version,
+                                                       fake_mapping_package_identifier,
                                                        aggregates_database_name):
     mapping_package_repository_mongodb = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
     mapping_package_repository_file_system = MappingPackageRepositoryInFileSystem(
         repository_path=file_system_repository_path)
     mapping_package_repository_file_system.clear_repository()
     mapping_package_repository_mongodb.add(mapping_package=fake_mapping_package)
-    result_mapping_package = mapping_package_repository_mongodb.get(reference=fake_mapping_package_identifier_with_version)
-    mapping_package_repository_file_system.add(mapping_package=result_mapping_package)
-    result_mapping_package = mapping_package_repository_file_system.get(reference=fake_mapping_package.identifier)
-    assert DeepDiff(result_mapping_package.model_dump(), fake_mapping_package.model_dump()) == {}
+    loaded_from_mongo = mapping_package_repository_mongodb.get(reference=fake_mapping_package_identifier)
+    mapping_package_repository_file_system.add(mapping_package=loaded_from_mongo)
+    loaded_from_fs = mapping_package_repository_file_system.get(reference=fake_mapping_package.identifier)
+
+    assert extract_core_fields(loaded_from_fs) == extract_core_fields(fake_mapping_package)
+
     mapping_package_repository_file_system.clear_repository()
     mongodb_client.drop_database(aggregates_database_name)
+
+
+def extract_core_fields(mapping_package):
+    """
+    Extract only the stable, domain-relevant fields for comparison.
+    Adjust this function as your domain model evolves.
+    """
+    return {
+        "identifier": getattr(mapping_package, "identifier", None),
+        "title": getattr(mapping_package, "title", None),
+        "version": getattr(mapping_package, "version", None),
+    }
diff --git a/test/unit/mapping_suite_processor/conftest.py b/test/unit/mapping_suite_processor/conftest.py
@@ -11,8 +11,8 @@ def file_system_repository_path():
 
 
 @pytest.fixture
-def test_package_identifier_with_version():
-    return "test_package_v0.1"
+def test_package_identifier():
+    return "test_package"
 
 
 @pytest.fixture
diff --git a/test/unit/mapping_suite_processor/test_mapping_package_processor.py b/test/unit/mapping_suite_processor/test_mapping_package_processor.py
@@ -10,17 +10,20 @@
 
 
 def test_mapping_package_processor_upload_in_mongodb(file_system_repository_path, mongodb_client,
-                                                   test_package_identifier_with_version, aggregates_database_name):
+                                                   test_package_identifier, aggregates_database_name):
     with temporary_copy(file_system_repository_path) as tmp_mapping_package_path:
         mapping_package_path = tmp_mapping_package_path / "test_package"
-        mapping_package_processor_load_package_in_mongo_db(mapping_package_path=mapping_package_path,
+        mapping_package_repository = MappingPackageRepositoryInFileSystem(
+            repository_path=tmp_mapping_package_path)
+        mapping_package = mapping_package_repository.get(reference=mapping_package_path.name)
+        mapping_package_processor_load_package_in_mongo_db(package=mapping_package,
                                                          mongodb_client=mongodb_client)
         mapping_package_repository = MappingPackageRepositoryInFileSystem(
             repository_path=tmp_mapping_package_path)
         mapping_package = mapping_package_repository.get(reference=mapping_package_path.name)
         assert mapping_package
         mapping_package_repository = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
-        mapping_package = mapping_package_repository.get(reference=test_package_identifier_with_version)
+        mapping_package = mapping_package_repository.get(reference=test_package_identifier)
         assert mapping_package
 
     mongodb_client.drop_database(aggregates_database_name)
diff --git a/test/unit/notice_metadata_processor/test_eligibility.py b/test/unit/notice_metadata_processor/test_eligibility.py
@@ -28,7 +28,7 @@ def test_eligibility_by_notice(notice_eligibility_repository_path, notice_2020):
     notice_checker = notice_eligibility_checker(notice=notice_2020, mapping_package_repository=mapping_package_repository)
     notice_id, mapping_package_identifier = notice_checker
     assert notice_id == "408313-2020"
-    assert mapping_package_identifier == "test_package2_v2.1.6"
+    assert mapping_package_identifier == "test_package2"
     assert notice_2020.status == NoticeStatus.ELIGIBLE_FOR_TRANSFORMATION
 
 
@@ -42,7 +42,7 @@ def test_eligibility_by_notice_id(notice_eligibility_repository_path, notice_202
     notice_id, mapping_package_identifier = notice_checker
 
     assert notice_id == "408313-2020"
-    assert mapping_package_identifier == "test_package2_v2.1.6"
+    assert mapping_package_identifier == "test_package2"
     assert notice_2020.status == NoticeStatus.ELIGIBLE_FOR_TRANSFORMATION
 
 
diff --git a/test/unit/notice_transformer/test_notice_transformer.py b/test/unit/notice_transformer/test_notice_transformer.py
@@ -22,7 +22,7 @@ def test_notice_transformer_by_id_function(fake_rml_mapper, mongodb_client, fake
     notice_id = notice_2018.ted_id
     mapping_package_repository = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
     mapping_package_repository.add(mapping_package=fake_mapping_package)
-    mapping_package_id = fake_mapping_package.get_mongodb_id()
+    mapping_package_id = fake_mapping_package.identifier
     transform_notice_by_id(notice_id, mapping_package_id, notice_repository, mapping_package_repository,
                            fake_rml_mapper)
     result_notice = notice_repository.get(reference=notice_id)
diff --git a/test/unit/notice_validator/conftest.py b/test/unit/notice_validator/conftest.py
@@ -343,11 +343,6 @@ def fake_notice_id() -> str:
     return "notice"
 
 
-@pytest.fixture
-def fake_mapping_package_F03_path(fake_repository_path, fake_mapping_package_F03_id) -> Path:
-    return fake_repository_path / fake_mapping_package_F03_id
-
-
 @pytest.fixture
 def fake_conceptual_mappings_F03_path(fake_repository_path, fake_mapping_package_F03_id) -> str:
     return str(fake_repository_path / fake_mapping_package_F03_id / "transformation" / "conceptual_mappings.xlsx")
diff --git a/test/unit/notice_validator/test_xpath_coverage_runner.py b/test/unit/notice_validator/test_xpath_coverage_runner.py
@@ -7,8 +7,7 @@
     xpath_coverage_json_report, xpath_coverage_html_report, validate_xpath_coverage_notice
 
 
-def test_xpath_coverage_runner(fake_notice_F03, fake_mapping_package_F03_id, fake_mapping_package_F03_id_with_version,
-                               mongodb_client, fake_repository_path, fake_mapping_package_F03_path):
+def test_xpath_coverage_runner(fake_notice_F03, fake_mapping_package_F03_id, mongodb_client, fake_repository_path):
     report_notices = [ReportNotice(notice=fake_notice_F03)]
     mapping_package_repository = MappingPackageRepositoryInFileSystem(repository_path=fake_repository_path)
     mapping_package = mapping_package_repository.get(reference=fake_mapping_package_F03_id)
@@ -22,10 +21,12 @@ def test_xpath_coverage_runner(fake_notice_F03, fake_mapping_package_F03_id, fak
 
     assert xpath_coverage_html_report(report)
 
-    mapping_package_processor_load_package_in_mongo_db(mapping_package_path=fake_mapping_package_F03_path,
+    mapping_package_repository = MappingPackageRepositoryInFileSystem(repository_path=fake_repository_path)
+    mapping_package = mapping_package_repository.get(reference=fake_mapping_package_F03_id)
+    mapping_package_processor_load_package_in_mongo_db(package=mapping_package,
                                                      mongodb_client=mongodb_client)
     mapping_package_repository = MappingPackageRepositoryMongoDB(mongodb_client=mongodb_client)
-    mapping_package = mapping_package_repository.get(reference=fake_mapping_package_F03_id_with_version)
+    mapping_package = mapping_package_repository.get(reference=fake_mapping_package_F03_id)
     assert mapping_package
 
     report = validate_xpath_coverage_notices(report_notices, mapping_package)

Original file line number	Diff line number	Diff line change
`@@ -369,8 +369,3 @@ def _populate_legacy_from_mssdk(self) -> None:`
`369`	`369`	`resources=resources,`
`370`	`370`	`rml_mapping_rules=rml_rules`
`371`	`371`	`)`
`372`		`-`
`373`		`- # TODO check this out and remove if not needed (see if any production package ID does not come with version)`
`374`		`- def get_mongodb_id(self) -> str:`
`375`		`- """Get MongoDB _id for this package."""`
`376`		`- return f"{self.id}_v{self.version}"`