From 19fcac83a4319d472b0328fd25c119dbb6160a89 Mon Sep 17 00:00:00 2001 From: Ramu Kamath Date: Tue, 13 Oct 2026 13:59:12 +0530 Subject: [PATCH 1/6] Reject duplicate YAML keys in data contracts --- CHANGELOG.md | 3 ++ datacontract/lint/resolve.py | 23 ++++++++++++- tests/test_resolve_duplicate_keys.py | 49 ++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/test_resolve_duplicate_keys.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e64949e..8da0345b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Reject duplicate YAML mapping keys instead of silently overwriting contract fields. + ## [1.2.5] - 2026-10-12 ### Changed diff --git a/datacontract/lint/resolve.py b/datacontract/lint/resolve.py index 884f18628..5f8a20eac 100644 --- a/datacontract/lint/resolve.py +++ b/datacontract/lint/resolve.py @@ -54,7 +54,28 @@ class _LaxOpenDataContractStandard(OpenDataContractStandard): class _SafeLoaderNoTimestamp(yaml.SafeLoader): """SafeLoader that keeps dates/timestamps as strings instead of converting to datetime objects.""" - pass + def construct_mapping(self, node, deep=False): + # Check only explicit keys before SafeLoader expands YAML merge keys. + # An explicit override of an inherited anchor value is valid YAML. + seen = set() + for key_node, _ in node.value: + if key_node.tag == "tag:yaml.org,2002:merge": + continue + key = self.construct_object(key_node, deep=deep) + try: + duplicate = key in seen + seen.add(key) + except TypeError: + # Let SafeLoader report its usual unhashable-key error. + return super().construct_mapping(node, deep=deep) + if duplicate: + raise yaml.constructor.ConstructorError( + "while constructing a mapping", + node.start_mark, + f"found duplicate key {key!r}", + key_node.start_mark, + ) + return super().construct_mapping(node, deep=deep) # Remove the timestamp implicit resolver so dates like 2022-01-15 stay as strings diff --git a/tests/test_resolve_duplicate_keys.py b/tests/test_resolve_duplicate_keys.py new file mode 100644 index 000000000..e1e4edeb9 --- /dev/null +++ b/tests/test_resolve_duplicate_keys.py @@ -0,0 +1,49 @@ +"""Duplicate contract keys must not silently overwrite schema or quality rules.""" + +import pytest + +from datacontract.data_contract import DataContract +from datacontract.lint.resolve import _to_yaml +from datacontract.model.exceptions import DataContractException + + +@pytest.mark.parametrize( + "document", + [ + "status: active\nstatus: draft\n", + "schema:\n- name: orders\n properties:\n - name: id\n required: true\n required: false\n", + "schema:\n- name: orders\n quality: {type: sql, query: SELECT 1, query: SELECT 2}\n", + ], +) +def test_duplicate_mapping_keys_are_rejected(document): + with pytest.raises(DataContractException, match="duplicate key"): + _to_yaml(document) + + +def test_duplicate_key_error_identifies_source_and_line(): + with pytest.raises(DataContractException) as exc: + _to_yaml("version: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") + assert "contracts/orders.yaml" in str(exc.value) + assert "line 2" in str(exc.value) + + +def test_yaml_merge_allows_explicit_override(): + parsed = _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true}\n") + assert parsed["column"]["required"] is True + + +def test_yaml_merge_does_not_hide_duplicate_explicit_keys(): + with pytest.raises(DataContractException, match="duplicate key"): + _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true, required: false}\n") + + +def test_date_strings_and_distinct_mapping_keys_are_unchanged(): + parsed = _to_yaml("date: 2026-10-13\nfirst: {name: id}\nsecond: {name: id}\n") + assert parsed["date"] == "2026-10-13" + assert parsed["first"] == parsed["second"] == {"name": "id"} + + +def test_lint_reports_duplicate_keys_as_a_failed_check(): + run = DataContract(data_contract_str="apiVersion: v3.0.2\napiVersion: v3.2.0\n").lint() + assert not run.has_passed() + assert any("duplicate key" in (check.reason or "") for check in run.checks) From e864bd6972f95a118e60941d24d1dcf9e0dfe935 Mon Sep 17 00:00:00 2001 From: Ramu Kamath Date: Tue, 13 Oct 2026 14:07:14 +0530 Subject: [PATCH 2/6] Annotate duplicate-key validation and regression tests --- datacontract/lint/resolve.py | 2 +- tests/test_resolve_duplicate_keys.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/datacontract/lint/resolve.py b/datacontract/lint/resolve.py index 5f8a20eac..b2b00894b 100644 --- a/datacontract/lint/resolve.py +++ b/datacontract/lint/resolve.py @@ -54,7 +54,7 @@ class _LaxOpenDataContractStandard(OpenDataContractStandard): class _SafeLoaderNoTimestamp(yaml.SafeLoader): """SafeLoader that keeps dates/timestamps as strings instead of converting to datetime objects.""" - def construct_mapping(self, node, deep=False): + def construct_mapping(self, node: yaml.MappingNode, deep: bool = False) -> dict[object, object]: # Check only explicit keys before SafeLoader expands YAML merge keys. # An explicit override of an inherited anchor value is valid YAML. seen = set() diff --git a/tests/test_resolve_duplicate_keys.py b/tests/test_resolve_duplicate_keys.py index e1e4edeb9..ab49d3284 100644 --- a/tests/test_resolve_duplicate_keys.py +++ b/tests/test_resolve_duplicate_keys.py @@ -15,35 +15,35 @@ "schema:\n- name: orders\n quality: {type: sql, query: SELECT 1, query: SELECT 2}\n", ], ) -def test_duplicate_mapping_keys_are_rejected(document): +def test_duplicate_mapping_keys_are_rejected(document: str) -> None: with pytest.raises(DataContractException, match="duplicate key"): _to_yaml(document) -def test_duplicate_key_error_identifies_source_and_line(): +def test_duplicate_key_error_identifies_source_and_line() -> None: with pytest.raises(DataContractException) as exc: _to_yaml("version: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") assert "contracts/orders.yaml" in str(exc.value) assert "line 2" in str(exc.value) -def test_yaml_merge_allows_explicit_override(): +def test_yaml_merge_allows_explicit_override() -> None: parsed = _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true}\n") assert parsed["column"]["required"] is True -def test_yaml_merge_does_not_hide_duplicate_explicit_keys(): +def test_yaml_merge_does_not_hide_duplicate_explicit_keys() -> None: with pytest.raises(DataContractException, match="duplicate key"): _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true, required: false}\n") -def test_date_strings_and_distinct_mapping_keys_are_unchanged(): +def test_date_strings_and_distinct_mapping_keys_are_unchanged() -> None: parsed = _to_yaml("date: 2026-10-13\nfirst: {name: id}\nsecond: {name: id}\n") assert parsed["date"] == "2026-10-13" assert parsed["first"] == parsed["second"] == {"name": "id"} -def test_lint_reports_duplicate_keys_as_a_failed_check(): +def test_lint_reports_duplicate_keys_as_a_failed_check() -> None: run = DataContract(data_contract_str="apiVersion: v3.0.2\napiVersion: v3.2.0\n").lint() assert not run.has_passed() assert any("duplicate key" in (check.reason or "") for check in run.checks) From 9c5d8c68367b605e4a82b07fcb1665f091932a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20Sch=C3=B6dl?= Date: Mon, 19 Oct 2026 15:15:34 +0200 Subject: [PATCH 3/6] Move the duplicate-key tests into test_resolve.py --- tests/test_resolve.py | 48 ++++++++++++++++++++++++++- tests/test_resolve_duplicate_keys.py | 49 ---------------------------- 2 files changed, 47 insertions(+), 50 deletions(-) delete mode 100644 tests/test_resolve_duplicate_keys.py diff --git a/tests/test_resolve.py b/tests/test_resolve.py index ab75d134d..dffcb80bf 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -1,6 +1,10 @@ import tempfile -from datacontract.lint.resolve import resolve_data_contract +import pytest + +from datacontract.data_contract import DataContract +from datacontract.lint.resolve import _to_yaml, resolve_data_contract +from datacontract.model.exceptions import DataContractException from datacontract.model.server import get_server_type # logging.basicConfig(level=logging.INFO, force=True) @@ -313,3 +317,45 @@ def test_resolve_dcs_status_preserved_when_set(): """, ) assert odcs.status == "active" + + +@pytest.mark.parametrize( + "document", + [ + "status: active\nstatus: draft\n", + "schema:\n- name: orders\n properties:\n - name: id\n required: true\n required: false\n", + "schema:\n- name: orders\n quality: {type: sql, query: SELECT 1, query: SELECT 2}\n", + ], +) +def test_duplicate_mapping_keys_are_rejected(document: str) -> None: + with pytest.raises(DataContractException, match="duplicate key"): + _to_yaml(document) + + +def test_duplicate_key_error_identifies_source_and_line() -> None: + with pytest.raises(DataContractException) as exc: + _to_yaml("version: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") + assert "contracts/orders.yaml" in str(exc.value) + assert "line 2" in str(exc.value) + + +def test_yaml_merge_allows_explicit_override() -> None: + parsed = _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true}\n") + assert parsed["column"]["required"] is True + + +def test_yaml_merge_does_not_hide_duplicate_explicit_keys() -> None: + with pytest.raises(DataContractException, match="duplicate key"): + _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true, required: false}\n") + + +def test_date_strings_and_distinct_mapping_keys_are_unchanged() -> None: + parsed = _to_yaml("date: 2026-10-13\nfirst: {name: id}\nsecond: {name: id}\n") + assert parsed["date"] == "2026-10-13" + assert parsed["first"] == parsed["second"] == {"name": "id"} + + +def test_lint_reports_duplicate_keys_as_a_failed_check() -> None: + run = DataContract(data_contract_str="apiVersion: v3.0.2\napiVersion: v3.2.0\n").lint() + assert not run.has_passed() + assert any("duplicate key" in (check.reason or "") for check in run.checks) diff --git a/tests/test_resolve_duplicate_keys.py b/tests/test_resolve_duplicate_keys.py deleted file mode 100644 index ab49d3284..000000000 --- a/tests/test_resolve_duplicate_keys.py +++ /dev/null @@ -1,49 +0,0 @@ -"""Duplicate contract keys must not silently overwrite schema or quality rules.""" - -import pytest - -from datacontract.data_contract import DataContract -from datacontract.lint.resolve import _to_yaml -from datacontract.model.exceptions import DataContractException - - -@pytest.mark.parametrize( - "document", - [ - "status: active\nstatus: draft\n", - "schema:\n- name: orders\n properties:\n - name: id\n required: true\n required: false\n", - "schema:\n- name: orders\n quality: {type: sql, query: SELECT 1, query: SELECT 2}\n", - ], -) -def test_duplicate_mapping_keys_are_rejected(document: str) -> None: - with pytest.raises(DataContractException, match="duplicate key"): - _to_yaml(document) - - -def test_duplicate_key_error_identifies_source_and_line() -> None: - with pytest.raises(DataContractException) as exc: - _to_yaml("version: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") - assert "contracts/orders.yaml" in str(exc.value) - assert "line 2" in str(exc.value) - - -def test_yaml_merge_allows_explicit_override() -> None: - parsed = _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true}\n") - assert parsed["column"]["required"] is True - - -def test_yaml_merge_does_not_hide_duplicate_explicit_keys() -> None: - with pytest.raises(DataContractException, match="duplicate key"): - _to_yaml("defaults: &defaults {required: false}\ncolumn: {<<: *defaults, required: true, required: false}\n") - - -def test_date_strings_and_distinct_mapping_keys_are_unchanged() -> None: - parsed = _to_yaml("date: 2026-10-13\nfirst: {name: id}\nsecond: {name: id}\n") - assert parsed["date"] == "2026-10-13" - assert parsed["first"] == parsed["second"] == {"name": "id"} - - -def test_lint_reports_duplicate_keys_as_a_failed_check() -> None: - run = DataContract(data_contract_str="apiVersion: v3.0.2\napiVersion: v3.2.0\n").lint() - assert not run.has_passed() - assert any("duplicate key" in (check.reason or "") for check in run.checks) From 1ff96cb6a046b2e07a016073fde60547b857de6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20Sch=C3=B6dl?= Date: Mon, 19 Oct 2026 15:15:41 +0200 Subject: [PATCH 4/6] Point the duplicate-key error at the key's first occurrence The error's first location used to be the start of the enclosing mapping, often line 1 of the contract. It now names the key and the line where it first appears, followed by the duplicate. --- datacontract/lint/resolve.py | 12 ++++++------ tests/test_resolve.py | 4 +++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/datacontract/lint/resolve.py b/datacontract/lint/resolve.py index b2b00894b..b855e8864 100644 --- a/datacontract/lint/resolve.py +++ b/datacontract/lint/resolve.py @@ -57,24 +57,24 @@ class _SafeLoaderNoTimestamp(yaml.SafeLoader): def construct_mapping(self, node: yaml.MappingNode, deep: bool = False) -> dict[object, object]: # Check only explicit keys before SafeLoader expands YAML merge keys. # An explicit override of an inherited anchor value is valid YAML. - seen = set() + first_marks = {} for key_node, _ in node.value: if key_node.tag == "tag:yaml.org,2002:merge": continue key = self.construct_object(key_node, deep=deep) try: - duplicate = key in seen - seen.add(key) + first_mark = first_marks.get(key) except TypeError: # Let SafeLoader report its usual unhashable-key error. return super().construct_mapping(node, deep=deep) - if duplicate: + if first_mark is not None: raise yaml.constructor.ConstructorError( - "while constructing a mapping", - node.start_mark, + f"found key {key!r}", + first_mark, f"found duplicate key {key!r}", key_node.start_mark, ) + first_marks[key] = key_node.start_mark return super().construct_mapping(node, deep=deep) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index dffcb80bf..a81ebe659 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -334,9 +334,11 @@ def test_duplicate_mapping_keys_are_rejected(document: str) -> None: def test_duplicate_key_error_identifies_source_and_line() -> None: with pytest.raises(DataContractException) as exc: - _to_yaml("version: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") + _to_yaml("apiVersion: v3.2.0\nversion: 1.0.0\nversion: 2.0.0\n", "contracts/orders.yaml") assert "contracts/orders.yaml" in str(exc.value) assert "line 2" in str(exc.value) + assert "line 3" in str(exc.value) + assert "line 1" not in str(exc.value) def test_yaml_merge_allows_explicit_override() -> None: From 2b18021e829a48b46b70be39c0b1c62fb65b417c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20Sch=C3=B6dl?= Date: Mon, 19 Oct 2026 15:15:46 +0200 Subject: [PATCH 5/6] Trim the duplicate-key loader comments --- datacontract/lint/resolve.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/datacontract/lint/resolve.py b/datacontract/lint/resolve.py index b855e8864..121df3795 100644 --- a/datacontract/lint/resolve.py +++ b/datacontract/lint/resolve.py @@ -55,8 +55,7 @@ class _SafeLoaderNoTimestamp(yaml.SafeLoader): """SafeLoader that keeps dates/timestamps as strings instead of converting to datetime objects.""" def construct_mapping(self, node: yaml.MappingNode, deep: bool = False) -> dict[object, object]: - # Check only explicit keys before SafeLoader expands YAML merge keys. - # An explicit override of an inherited anchor value is valid YAML. + # Skip merge keys: overriding a value inherited from an anchor is valid YAML. first_marks = {} for key_node, _ in node.value: if key_node.tag == "tag:yaml.org,2002:merge": @@ -65,7 +64,6 @@ def construct_mapping(self, node: yaml.MappingNode, deep: bool = False) -> dict[ try: first_mark = first_marks.get(key) except TypeError: - # Let SafeLoader report its usual unhashable-key error. return super().construct_mapping(node, deep=deep) if first_mark is not None: raise yaml.constructor.ConstructorError( From ca8df4af845b7516997555d5d8b72f1788f17103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20Sch=C3=B6dl?= Date: Mon, 19 Oct 2026 15:15:46 +0200 Subject: [PATCH 6/6] Add CHANGELOG entry --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 472a9e955..b86bd3fc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The bundled Data Contract Editor (`datacontract edit`) is updated to 0.1.16 +- `datacontract lint` and `datacontract test`: reject duplicate keys in a data contract instead of silently keeping the last value (#1741 @kamathramu87) ### Fixed -- `datacontract export sql`: apostrophes and backslashes in descriptions no longer break ClickHouse and Snowflake `COMMENT` clauses (#1729) -- Reject duplicate YAML mapping keys instead of silently overwriting contract fields. +- `datacontract export sql`: apostrophes and backslashes in descriptions no longer break ClickHouse and Snowflake `COMMENT` clauses (#1729,#1731 @banshee @ezno12) ## [1.2.7] - 2026-10-16