From 2dee008a4a38bbe0806fb318bc986348e557d1bb Mon Sep 17 00:00:00 2001 From: Ralf Becher Date: Thu, 16 Jul 2026 19:08:02 +0200 Subject: [PATCH 1/3] osi converter: preserve N dimensions over one column across the round-trip (#222) OBML allows N dimensions over one column (grain variants, role-playing via `via`); OSI is field-centric (one dimension per field). The converter used to `break` after the first matching dimension on export and build one dimension per field on import, so a second dimension on the same column was dropped silently - both names lost, zero warnings. - Export (obml_to_osi): the first matching dimension stays the primary carried on the field; any extras are serialized into an `obml_extra_dimensions` list extension (name + resultType + timeGrain + format + description + owner + via), and a fidelity warning is emitted (warn, do not raise). - Import (osi_to_obml): rebuild each preserved descriptor as its own dimension. The collision-key logic is factored into `_insert_dimension` and reused for the primary and every extra. Descriptors are opaque foreign-modifiable data, so each is shape-guarded (must be a dict with a non-empty string name; string props only) - a malformed payload is skipped, never crashes on an unhashable key. Tests: TestMultipleDimensionsPerColumn - all dimensions survive with distinct grains, export warns (not silent), and malformed extra-dimension payloads never crash. Closes #222. Refs #201, #202, #220. --- .../src/osi_orionbelt/obml_to_osi.py | 29 ++++- .../src/osi_orionbelt/osi_to_obml.py | 71 ++++++++--- ...test_osi_converter_roundtrip_robustness.py | 120 ++++++++++++++++++ 3 files changed, 198 insertions(+), 22 deletions(-) diff --git a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py index b470c91d..fb6f9917 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py @@ -388,10 +388,18 @@ def _convert_column( ext_data["obml_comment"] = col_obj["comment"] if col_obj.get("owner"): ext_data["obml_owner"] = col_obj["owner"] - # Preserve OBML-only dimension properties (timeGrain, format, resultType, etc.) + # Preserve OBML-only dimension properties (timeGrain, format, resultType, etc.). + # OBML allows N dimensions over one column (grain variants, role-playing + # via ``via``); OSI is field-centric (one dimension per field). The first + # match is the primary dimension carried on the field; any extras are + # preserved as descriptors so the reverse trip can rebuild them instead of + # dropping them silently. matched_dim: dict[str, Any] | None = None + extra_dims: list[dict[str, Any]] = [] for _dim_name, dim_obj in obml_dimensions.items(): - if dim_obj.get("dataObject") == do_name and dim_obj.get("column") == col_name: + if dim_obj.get("dataObject") != do_name or dim_obj.get("column") != col_name: + continue + if matched_dim is None: matched_dim = dim_obj # Preserve the dimension's OBML name explicitly. The OSI field # name is the physical code, so without this the round-trip @@ -410,7 +418,22 @@ def _convert_column( ext_data["obml_dimension_owner"] = dim_obj["owner"] if dim_obj.get("via"): ext_data["obml_dimension_via"] = dim_obj["via"] - break + else: + descriptor: dict[str, Any] = {"name": _dim_name} + for prop in ("resultType", "timeGrain", "format", "description", "owner", "via"): + if dim_obj.get(prop): + descriptor[prop] = dim_obj[prop] + extra_dims.append(descriptor) + if extra_dims: + ext_data["obml_extra_dimensions"] = extra_dims + extra_names = ", ".join(d["name"] for d in extra_dims) + self.warnings.append( + f"Column '{do_name}.{col_name}' backs {len(extra_dims) + 1} OBML " + f"dimensions; OSI represents one dimension per field, so the " + f"{len(extra_dims)} additional dimension(s) ({extra_names}) are " + f"preserved via an OBSL extension for the reverse conversion but " + f"are not natively visible to other OSI tools." + ) field["custom_extensions"] = [ { "vendor_name": _VENDOR_OBML, diff --git a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py index b107704c..8a2ca104 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py @@ -566,6 +566,7 @@ def _extract_dimensions(self, datasets: list) -> dict: dim_def["synonyms"] = list(ai_ctx["synonyms"]) # Restore OBML-only dimension properties from custom_extensions restored_name: str | None = None + extra_descriptors: list[Any] = [] for ext in field.get("custom_extensions", []): if ext.get("vendor_name") in _OBML_VENDOR_READ: try: @@ -590,6 +591,11 @@ def _extract_dimensions(self, datasets: list) -> dict: dim_def["owner"] = ext_data["obml_dimension_owner"] if ext_data.get("obml_dimension_via"): dim_def["via"] = ext_data["obml_dimension_via"] + # Additional dimensions over the same column, preserved + # by the export because OSI has no slot for them. + _extras = ext_data.get("obml_extra_dimensions") + if isinstance(_extras, list): + extra_descriptors = _extras except (json.JSONDecodeError, TypeError): pass break @@ -603,27 +609,54 @@ def _extract_dimensions(self, datasets: list) -> dict: if not dim_def["synonyms"]: del dim_def["synonyms"] base_name = restored_name or field_name - # Dimension names must be unique across the model. When the same - # name occurs in more than one data object (e.g. foreign OSI where - # two datasets share a bare field name and no OBML-origin name was - # restored), qualify the later one with its data object instead of - # silently overwriting the earlier dimension. A restored OBML name - # is unique by construction, so this fallback is foreign-OSI only. - key = base_name - if key in dimensions and dimensions[key].get("dataObject") != ds_name: - key = f"{ds_name} {base_name}" - suffix = 2 - while key in dimensions: - key = f"{ds_name} {base_name} {suffix}" - suffix += 1 - self.warnings.append( - f"Dimension name '{base_name}' occurs in multiple data " - f"objects; emitted '{ds_name}.{base_name}' as dimension " - f"'{key}' to avoid a collision." - ) - dimensions[key] = dim_def + self._insert_dimension(dimensions, ds_name, base_name, dim_def) + # Rebuild any additional OBML dimensions the export preserved for + # this column (OSI is one-dimension-per-field). Each descriptor is + # opaque foreign-modifiable data, so guard its shape. + for desc in extra_descriptors: + if not isinstance(desc, dict): + continue + dname = desc.get("name") + if not (isinstance(dname, str) and dname): + continue + extra_def: dict[str, Any] = { + "dataObject": ds_name, + "column": field_name, + "resultType": desc.get("resultType") or abstract_type, + } + for prop in ("timeGrain", "format", "description", "owner", "via"): + value = desc.get(prop) + if isinstance(value, str) and value: + extra_def[prop] = value + self._insert_dimension(dimensions, ds_name, dname, extra_def) return dimensions + def _insert_dimension( + self, dimensions: dict[str, Any], ds_name: str, base_name: str, dim_def: dict[str, Any] + ) -> None: + """Insert ``dim_def`` under a unique key. + + Dimension names must be unique across the model. When ``base_name`` + already names a dimension on a *different* data object (foreign OSI where + two datasets share a bare field name and no OBML-origin name was + restored), qualify the later one with its data object and warn instead of + silently overwriting the earlier dimension. A restored OBML name is unique + by construction, so the qualification is foreign-OSI only. + """ + key = base_name + if key in dimensions and dimensions[key].get("dataObject") != ds_name: + key = f"{ds_name} {base_name}" + suffix = 2 + while key in dimensions: + key = f"{ds_name} {base_name} {suffix}" + suffix += 1 + self.warnings.append( + f"Dimension name '{base_name}' occurs in multiple data " + f"objects; emitted '{ds_name}.{base_name}' as dimension " + f"'{key}' to avoid a collision." + ) + dimensions[key] = dim_def + def _convert_metrics(self, osi_metrics: list, ds_map: dict) -> tuple[dict, dict]: """ Convert OSI metrics to OBML measures and metrics. diff --git a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py index 5714f0df..702dd3b6 100644 --- a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py +++ b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py @@ -371,6 +371,126 @@ def test_non_string_restored_name_is_ignored(self, bad: Any) -> None: assert list(obml["dimensions"]) == ["dt"] +# --------------------------------------------------------------------------- +# #222: N OBML dimensions over one column +# --------------------------------------------------------------------------- + + +class TestMultipleDimensionsPerColumn: + """OBML allows N dimensions over one column (grain variants, role-playing); + OSI is one-dimension-per-field. The export preserves the extras via an + extension and warns (not silent); the import rebuilds them all (#222).""" + + _OBML = { + "version": 1.0, + "dataObjects": { + "Orders": { + "code": "FACT", + "database": "WH", + "schema": "PUBLIC", + "columns": { + "date": {"code": "order_date", "abstractType": "date"}, + "Amount": {"code": "amt", "abstractType": "float"}, + }, + } + }, + "dimensions": { + "Order Day": { + "dataObject": "Orders", + "column": "date", + "resultType": "date", + "timeGrain": "day", + }, + "Order Month": { + "dataObject": "Orders", + "column": "date", + "resultType": "date", + "timeGrain": "month", + }, + "Order Quarter": { + "dataObject": "Orders", + "column": "date", + "resultType": "date", + "timeGrain": "quarter", + }, + }, + "measures": { + "Revenue": { + "columns": [{"dataObject": "Orders", "column": "Amount"}], + "aggregation": "sum", + "resultType": "float", + } + }, + } + + def _roundtrip(self) -> tuple[dict[str, Any], list[str]]: + exp = conv.OBMLtoOSI(self._OBML, model_name="s") + osi = exp.convert() + return conv.OSItoOBML(osi).convert(), exp.warnings + + def test_all_dimensions_survive_with_distinct_grains(self) -> None: + obml, _ = self._roundtrip() + dims = obml["dimensions"] + assert set(dims) == {"Order Day", "Order Month", "Order Quarter"} + assert {d["timeGrain"] for d in dims.values()} == {"day", "month", "quarter"} + # All three still point at the same physical column. + assert {d["column"] for d in dims.values()} == {"order_date"} + + def test_export_warns_not_silent(self) -> None: + _, warnings = self._roundtrip() + assert any("backs 3 OBML dimensions" in w for w in warnings), warnings + + @pytest.mark.parametrize( + "bad", + [ + "not-a-list", + [5], + [{"no_name": 1}], + [{"name": ["x"]}], + [{"name": ""}], + [{"name": "OK"}, 7], + ], + ) + def test_malformed_extra_dimensions_never_crash(self, bad: Any) -> None: + # obml_extra_dimensions is opaque to validate_osi. A malformed payload + # (not a list, or items that are not dicts / lack a string name) must be + # skipped, never crash the converter on an unhashable key. + osi = { + "version": "0.2.0.dev0", + "semantic_model": [ + { + "name": "s", + "datasets": [ + { + "name": "Orders", + "source": "WH.PUBLIC.orders", + "fields": [ + { + "name": "dt", + "expression": { + "dialects": [{"dialect": "ANSI_SQL", "expression": "dt"}] + }, + "dimension": {}, + "custom_extensions": [ + { + "vendor_name": "ORIONBELT", + "data": json.dumps({"obml_extra_dimensions": bad}), + } + ], + } + ], + } + ], + } + ], + } + obml = conv.OSItoOBML(osi).convert() + # The primary field-name dimension is always present; any well-formed + # extra (the {"name": "OK"} case) is rebuilt, malformed items skipped. + assert "dt" in obml["dimensions"] + assert all(isinstance(k, str) for k in obml["dimensions"]) + + # --------------------------------------------------------------------------- # P2: validate_osi robustness on malformed input # --------------------------------------------------------------------------- From fe0c286b793a49c9534c7309e9baca5ef4d6a79a Mon Sep 17 00:00:00 2001 From: Ralf Becher Date: Thu, 16 Jul 2026 19:14:52 +0200 Subject: [PATCH 2/3] Address review: preserve extra dimensions' synonyms + customExtensions The obml_extra_dimensions descriptor only carried the scalar props (resultType, timeGrain, format, description, owner, via), so a non-primary dimension's own synonyms and vendor customExtensions were dropped on the round-trip. Include both in the descriptor on export and restore them on import, with the same shape guards as the rest of the opaque extension data: synonyms kept only as non-empty strings, customExtensions only as dict entries. Malformed values are filtered, never crash. Tests: extra dimension's synonyms + customExtensions survive the round-trip; malformed synonyms/customExtensions payloads are filtered without crashing. Refs #222. --- .../src/osi_orionbelt/obml_to_osi.py | 6 +++ .../src/osi_orionbelt/osi_to_obml.py | 13 +++++++ ...test_osi_converter_roundtrip_robustness.py | 39 +++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py index fb6f9917..79c64a28 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py @@ -423,6 +423,12 @@ def _convert_column( for prop in ("resultType", "timeGrain", "format", "description", "owner", "via"): if dim_obj.get(prop): descriptor[prop] = dim_obj[prop] + # Carry the extra dimension's own synonyms and vendor extensions + # too, so it round-trips with the same fidelity as the primary. + if dim_obj.get("synonyms"): + descriptor["synonyms"] = dim_obj["synonyms"] + if dim_obj.get("customExtensions"): + descriptor["customExtensions"] = dim_obj["customExtensions"] extra_dims.append(descriptor) if extra_dims: ext_data["obml_extra_dimensions"] = extra_dims diff --git a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py index 8a2ca104..b4344bfb 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py @@ -628,6 +628,19 @@ def _extract_dimensions(self, datasets: list) -> dict: value = desc.get(prop) if isinstance(value, str) and value: extra_def[prop] = value + # Restore the extra dimension's own synonyms / vendor + # extensions. Opaque foreign data, so keep only well-shaped + # entries (string synonyms; dict extensions). + syns = desc.get("synonyms") + if isinstance(syns, list): + clean_syns = [s for s in syns if isinstance(s, str) and s] + if clean_syns: + extra_def["synonyms"] = clean_syns + exts = desc.get("customExtensions") + if isinstance(exts, list): + clean_exts = [e for e in exts if isinstance(e, dict)] + if clean_exts: + extra_def["customExtensions"] = clean_exts self._insert_dimension(dimensions, ds_name, dname, extra_def) return dimensions diff --git a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py index 702dd3b6..fdfed40f 100644 --- a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py +++ b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py @@ -440,6 +440,41 @@ def test_export_warns_not_silent(self) -> None: _, warnings = self._roundtrip() assert any("backs 3 OBML dimensions" in w for w in warnings), warnings + def test_extra_dimension_synonyms_and_extensions_preserved(self) -> None: + # An extra dimension's own synonyms and vendor extensions must survive, + # matching the primary's fidelity (not just the scalar props). + obml = { + "version": 1.0, + "dataObjects": { + "Orders": { + "code": "F", + "database": "W", + "schema": "P", + "columns": {"date": {"code": "dt", "abstractType": "date"}}, + } + }, + "dimensions": { + "Order Day": { + "dataObject": "Orders", + "column": "date", + "resultType": "date", + "timeGrain": "day", + }, + "Order Month": { + "dataObject": "Orders", + "column": "date", + "resultType": "date", + "timeGrain": "month", + "synonyms": ["monthly date"], + "customExtensions": [{"vendor": "ACME", "data": '{"y": 2}'}], + }, + }, + } + back = conv.OSItoOBML(conv.OBMLtoOSI(obml, model_name="s").convert()).convert() + month = back["dimensions"]["Order Month"] + assert month.get("synonyms") == ["monthly date"] + assert month.get("customExtensions") == [{"vendor": "ACME", "data": '{"y": 2}'}] + @pytest.mark.parametrize( "bad", [ @@ -449,6 +484,10 @@ def test_export_warns_not_silent(self) -> None: [{"name": ["x"]}], [{"name": ""}], [{"name": "OK"}, 7], + [{"name": "OK", "synonyms": "not-a-list"}], + [{"name": "OK", "synonyms": [1, {}]}], + [{"name": "OK", "customExtensions": "bad"}], + [{"name": "OK", "customExtensions": [7]}], ], ) def test_malformed_extra_dimensions_never_crash(self, bad: Any) -> None: From ee6bdb393782506636a869a73184a794221a39be Mon Sep 17 00:00:00 2001 From: Ralf Becher Date: Thu, 16 Jul 2026 19:21:31 +0200 Subject: [PATCH 3/3] Also preserve the primary dimension's synonyms + customExtensions The primary (single) dimension on a column lost its own synonyms entirely and its customExtensions moved to the column (OSI has no dimension entity, so they surfaced as field foreign extensions and re-imported onto the column). Now the extras preserved these but the primary did not - inconsistent. Stash the primary dimension's synonyms and customExtensions in authoritative obml_dimension_synonyms / obml_dimension_custom_extensions extension keys and restore them to the dimension on import, with the same shape guards (string synonyms, dict extensions). The existing foreign-extension emission stays, so the dimension's vendor extensions remain visible to other OSI tools on the field; that column-side surfacing is pre-existing and unchanged. Test: the primary dimension's synonyms + customExtensions survive the round trip. Full package suite green (the field-foreign-extension export test still passes). Refs #222. --- .../src/osi_orionbelt/obml_to_osi.py | 10 +++++++ .../src/osi_orionbelt/osi_to_obml.py | 13 +++++++++ ...test_osi_converter_roundtrip_robustness.py | 27 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py index 79c64a28..8153e9e7 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py @@ -418,6 +418,16 @@ def _convert_column( ext_data["obml_dimension_owner"] = dim_obj["owner"] if dim_obj.get("via"): ext_data["obml_dimension_via"] = dim_obj["via"] + # The dimension's own synonyms and vendor extensions have no + # native OSI slot (OSI has no dimension entity), so preserve them + # authoritatively here for the reverse trip. The customExtensions + # are also emitted as field foreign extensions below for OSI-tool + # visibility; that path lands them on the column on re-import, + # while this one restores them to the dimension. + if dim_obj.get("synonyms"): + ext_data["obml_dimension_synonyms"] = dim_obj["synonyms"] + if dim_obj.get("customExtensions"): + ext_data["obml_dimension_custom_extensions"] = dim_obj["customExtensions"] else: descriptor: dict[str, Any] = {"name": _dim_name} for prop in ("resultType", "timeGrain", "format", "description", "owner", "via"): diff --git a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py index b4344bfb..2fd770f1 100644 --- a/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py +++ b/packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py @@ -591,6 +591,19 @@ def _extract_dimensions(self, datasets: list) -> dict: dim_def["owner"] = ext_data["obml_dimension_owner"] if ext_data.get("obml_dimension_via"): dim_def["via"] = ext_data["obml_dimension_via"] + # The dimension's own synonyms / vendor extensions, + # restored authoritatively to the dimension. Opaque + # foreign data, so keep only well-shaped entries. + _syns = ext_data.get("obml_dimension_synonyms") + if isinstance(_syns, list): + clean_syns = [s for s in _syns if isinstance(s, str) and s] + if clean_syns: + dim_def["synonyms"] = clean_syns + _exts = ext_data.get("obml_dimension_custom_extensions") + if isinstance(_exts, list): + clean_exts = [e for e in _exts if isinstance(e, dict)] + if clean_exts: + dim_def["customExtensions"] = clean_exts # Additional dimensions over the same column, preserved # by the export because OSI has no slot for them. _extras = ext_data.get("obml_extra_dimensions") diff --git a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py index fdfed40f..54fdd04a 100644 --- a/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py +++ b/packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py @@ -333,6 +333,33 @@ def test_restored_name_not_left_as_a_synonym(self) -> None: for name, dim in obml["dimensions"].items(): assert name not in dim.get("synonyms", []) + def test_primary_dimension_synonyms_and_extensions_preserved(self) -> None: + # The primary (single) dimension on a column keeps its own synonyms and + # vendor extensions across the round-trip, not just the scalar props. + obml = { + "version": 1.0, + "dataObjects": { + "Orders": { + "code": "o", + "database": "W", + "schema": "P", + "columns": {"Status": {"code": "status", "abstractType": "string"}}, + } + }, + "dimensions": { + "Status": { + "dataObject": "Orders", + "column": "Status", + "synonyms": ["state", "condition"], + "customExtensions": [{"vendor": "TABLEAU", "data": '{"role": "dim"}'}], + } + }, + } + back = conv.OSItoOBML(conv.OBMLtoOSI(obml).convert()).convert() + dim = back["dimensions"]["Status"] + assert dim.get("synonyms") == ["state", "condition"] + assert dim.get("customExtensions") == [{"vendor": "TABLEAU", "data": '{"role": "dim"}'}] + @pytest.mark.parametrize("bad", [["x"], 5, "", {}, None]) def test_non_string_restored_name_is_ignored(self, bad: Any) -> None: # obml_dimension_name is opaque to validate_osi, so a foreign payload may