From 210ef1a364862701da1d77699bb2d5eed26dd506 Mon Sep 17 00:00:00 2001 From: "a.krantz" Date: Thu, 25 Jun 2026 12:35:32 +0200 Subject: [PATCH] fix: bulk data hash generation is broken --- src/odsbox_diff/ods_diff_hierarchy/collect.py | 34 ++- tests/ods_diff_hierarchy/test_collect.py | 200 ++++++++++++++++++ 2 files changed, 225 insertions(+), 9 deletions(-) diff --git a/src/odsbox_diff/ods_diff_hierarchy/collect.py b/src/odsbox_diff/ods_diff_hierarchy/collect.py index 75adec6..978168c 100644 --- a/src/odsbox_diff/ods_diff_hierarchy/collect.py +++ b/src/odsbox_diff/ods_diff_hierarchy/collect.py @@ -97,21 +97,37 @@ def _collect_bulk_data( self._print_progress_bar(sub_matrix_index + 1, sub_matrices.shape[0], "Bulk:", fill="*") submatrix_id = submatrix_row.id try: + attribute_id = self._mc.attribute_no_throw(local_column_entity, "id") + attribute_values = self._mc.attribute_no_throw(local_column_entity, "values") + attribute_generation_parameters = self._mc.attribute_no_throw( + local_column_entity, "generation_parameters" + ) + attribute_flags = self._mc.attribute_no_throw(local_column_entity, "flags") + if attribute_id is None or attribute_values is None: + log.warning( + "AoLocalColumn attributes 'id' or 'values' not found. Skipping bulk hash for submatrix %s.", + submatrix_id, + ) + continue + + attributes_bulk: dict[str, int] = {} + attributes_bulk[attribute_id.name] = 1 + if attribute_generation_parameters is not None: + attributes_bulk[attribute_generation_parameters.name] = 1 + attributes_bulk[attribute_values.name] = 1 + if attribute_flags is not None: + attributes_bulk[attribute_flags.name] = 1 + bulk_data = self._query_data( { local_column_entity.name: {"submatrix": submatrix_id}, - "$attributes": { - "id": 1, - "generation_parameters": 1, - "values": 1, - "flags": 1, - }, + "$attributes": attributes_bulk, } ) - bulk_data.columns = pd.Index(["id", "generation_parameters", "values", "flags"]) + id_column_name = f"{local_column_entity.name}.{attribute_id.name}" for _, row in bulk_data.iterrows(): - hash_value = self._hash_pandas_row(row) - local_column_id = row.id + local_column_id = row[id_column_name] + hash_value = self._hash_pandas_row(row.drop(labels=[id_column_name], errors="ignore")) parent_dictionary = lookup.get((local_column_entity.name, local_column_id), None) if parent_dictionary is None: raise ValueError("parent wasn't added") diff --git a/tests/ods_diff_hierarchy/test_collect.py b/tests/ods_diff_hierarchy/test_collect.py index c535bc2..19ebdf6 100644 --- a/tests/ods_diff_hierarchy/test_collect.py +++ b/tests/ods_diff_hierarchy/test_collect.py @@ -5,9 +5,12 @@ import json import zipfile from pathlib import Path +from typing import Any, cast +from unittest.mock import MagicMock import pandas as pd import pytest +from requests import HTTPError from odsbox_diff.ods_diff_hierarchy.collect import ( Collector, @@ -43,6 +46,203 @@ def test_different_rows_differ(self) -> None: h2 = Collector._hash_pandas_row(pd.Series([1, 2, 4])) assert h1 != h2 + def test_bulk_hash_ignores_id_when_excluded(self) -> None: + row_a = pd.Series([101, "gp", "vals", "flags"], index=["id", "generation_parameters", "values", "flags"]) + row_b = pd.Series([202, "gp", "vals", "flags"], index=["id", "generation_parameters", "values", "flags"]) + + h1 = Collector._hash_pandas_row(row_a.drop(labels=["id"])) + h2 = Collector._hash_pandas_row(row_b.drop(labels=["id"])) + assert h1 == h2 + + +class TestCollectBulkData: + def _mock_collector(self) -> tuple[Collector, Any, Any]: + """Create a Collector with mocked ConI and ModelCache.""" + mock_con_i: Any = MagicMock() + mock_mc: Any = MagicMock() + mock_con_i.mc = mock_mc + collector = Collector(cast(Any, mock_con_i)) + return collector, mock_con_i, mock_mc + + def test_collect_bulk_data_success(self) -> None: + """Test successful bulk hash computation for LocalColumns.""" + collector, mock_con_i, mock_mc = self._mock_collector() + + sub_matrix_entity = MagicMock() + sub_matrix_entity.name = "SubMatrix" + local_column_entity = MagicMock() + local_column_entity.name = "LocalColumn" + mock_mc.entity_by_base_name.side_effect = lambda name: ( + sub_matrix_entity if name == "AoSubMatrix" else local_column_entity + ) + + attr_id = MagicMock() + attr_id.name = "Id" + attr_values = MagicMock() + attr_values.name = "Values" + attr_gp = MagicMock() + attr_gp.name = "GenerationParameters" + attr_flags = MagicMock() + attr_flags.name = "Flags" + mock_mc.attribute_no_throw.side_effect = lambda entity, name: { + "id": attr_id, + "values": attr_values, + "generation_parameters": attr_gp, + "flags": attr_flags, + }.get(name) + + submatrices_df = pd.DataFrame( + { + "id": [1], + "measurement": [10], + "number_of_rows": [2], + } + ) + bulk_data_df = pd.DataFrame( + { + "LocalColumn.Id": [101, 102], + "GenerationParameters": ["gp1", "gp2"], + "Values": [[1, 2, 3], [4, 5, 6]], + "Flags": [[0, 0, 0], [0, 1, 0]], + } + ) + mock_con_i.query_data.side_effect = [submatrices_df, bulk_data_df] + lookup: dict[tuple[str, int], dict[str, Any]] = { + ("LocalColumn", 101): {}, + ("LocalColumn", 102): {}, + } + + collector._collect_bulk_data(lookup, "measurement", 10, show_progress=False) + + assert "_BULK_HASH" in lookup[("LocalColumn", 101)] + assert "_BULK_HASH" in lookup[("LocalColumn", 102)] + assert lookup[("LocalColumn", 101)]["_BULK_HASH"] != lookup[("LocalColumn", 102)]["_BULK_HASH"] + + def test_collect_bulk_data_missing_required_attributes(self) -> None: + """Test that bulk hash is skipped when required attributes are missing.""" + collector, mock_con_i, mock_mc = self._mock_collector() + + sub_matrix_entity = MagicMock() + sub_matrix_entity.name = "SubMatrix" + local_column_entity = MagicMock() + local_column_entity.name = "LocalColumn" + mock_mc.entity_by_base_name.side_effect = lambda name: ( + sub_matrix_entity if name == "AoSubMatrix" else local_column_entity + ) + + mock_mc.attribute_no_throw.return_value = None + + submatrices_df = pd.DataFrame( + { + "id": [1], + "measurement": [10], + "number_of_rows": [1], + } + ) + mock_con_i.query_data.return_value = submatrices_df + lookup: dict[tuple[str, int], dict[str, Any]] = {} + collector._collect_bulk_data(lookup, "measurement", 10, show_progress=False) + assert mock_con_i.query_data.call_count == 1 + + def test_collect_bulk_data_http_error_handling(self) -> None: + """Test that HTTPError is caught and logged without crashing.""" + collector, mock_con_i, mock_mc = self._mock_collector() + + sub_matrix_entity = MagicMock() + sub_matrix_entity.name = "SubMatrix" + local_column_entity = MagicMock() + local_column_entity.name = "LocalColumn" + mock_mc.entity_by_base_name.side_effect = lambda name: ( + sub_matrix_entity if name == "AoSubMatrix" else local_column_entity + ) + + attr_id = MagicMock() + attr_id.name = "Id" + attr_values = MagicMock() + attr_values.name = "Values" + attr_gp = MagicMock() + attr_gp.name = "GenerationParameters" + attr_flags = MagicMock() + attr_flags.name = "Flags" + mock_mc.attribute_no_throw.side_effect = lambda entity, name: { + "id": attr_id, + "values": attr_values, + "generation_parameters": attr_gp, + "flags": attr_flags, + }.get(name) + + submatrices_df = pd.DataFrame( + { + "id": [1], + "measurement": [10], + "number_of_rows": [1], + } + ) + mock_con_i.query_data.side_effect = [submatrices_df, HTTPError("Connection failed")] + lookup: dict[tuple[str, int], dict[str, Any]] = { + ("SubMatrix", 1): {}, + } + collector._collect_bulk_data(lookup, "measurement", 10, show_progress=False) + assert "_BULK_HASH_CALCULATION_ERROR" in lookup[("SubMatrix", 1)] + assert "Unable to retrieve bulk for Submatrix 1" in lookup[("SubMatrix", 1)]["_BULK_HASH_CALCULATION_ERROR"] + + def test_collect_bulk_data_id_excluded_from_hash(self) -> None: + """Verify that changing only the id column does not change the bulk hash.""" + collector, mock_con_i, mock_mc = self._mock_collector() + + sub_matrix_entity = MagicMock() + sub_matrix_entity.name = "SubMatrix" + local_column_entity = MagicMock() + local_column_entity.name = "LocalColumn" + mock_mc.entity_by_base_name.side_effect = lambda name: ( + sub_matrix_entity if name == "AoSubMatrix" else local_column_entity + ) + + attr_id = MagicMock() + attr_id.name = "Id" + attr_values = MagicMock() + attr_values.name = "Values" + attr_gp = MagicMock() + attr_gp.name = "GenerationParameters" + attr_flags = MagicMock() + attr_flags.name = "Flags" + mock_mc.attribute_no_throw.side_effect = lambda entity, name: { + "id": attr_id, + "values": attr_values, + "generation_parameters": attr_gp, + "flags": attr_flags, + }.get(name) + + submatrices_df = pd.DataFrame( + { + "id": [1], + "measurement": [10], + "number_of_rows": [1], + } + ) + + bulk_data_df = pd.DataFrame( + { + "LocalColumn.Id": [101, 202], + "GenerationParameters": ["same_gp", "same_gp"], + "Values": ["same_vals", "same_vals"], + "Flags": ["same_flags", "same_flags"], + } + ) + + mock_con_i.query_data.side_effect = [submatrices_df, bulk_data_df] + + lookup: dict[tuple[str, int], dict[str, Any]] = { + ("LocalColumn", 101): {}, + ("LocalColumn", 202): {}, + } + + collector._collect_bulk_data(lookup, "measurement", 10, show_progress=False) + + hash1 = lookup[("LocalColumn", 101)]["_BULK_HASH"] + hash2 = lookup[("LocalColumn", 202)]["_BULK_HASH"] + assert hash1 == hash2 + class TestSaveLoadCollectResults: def test_json_roundtrip(self, tmp_path: Path) -> None: