diff --git a/CHANGELOG.md b/CHANGELOG.md index fee58b76a..63b81669f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- `datacontract breaking` command and `POST /breaking` endpoint for breaking change detection (#1016) - `datacontract test` checks the ODCS array options `minItems`, `maxItems` and `uniqueItems` (#1514) - `datacontract export odcs` defaults `status` to `draft` when the source DCS contract has no `info.status` diff --git a/README.md b/README.md index 7ab4eb087..62469bf9b 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,9 @@ $ datacontract lint odcs.yaml # show a changelog between two data contracts $ datacontract changelog v1.odcs.yaml v2.odcs.yaml +# fail when a contract change is backward-incompatible +$ datacontract breaking v1.odcs.yaml v2.odcs.yaml + # execute schema and quality checks (define credentials as environment variables) $ datacontract test odcs.yaml diff --git a/datacontract/api.py b/datacontract/api.py index f9ffe3aec..910c2a965 100644 --- a/datacontract/api.py +++ b/datacontract/api.py @@ -15,6 +15,7 @@ from datacontract.config import Config, known_env_names from datacontract.data_contract import DataContract, ExportFormat +from datacontract.model.breaking import BreakingChangeEntry from datacontract.model.changelog import ChangelogEntry from datacontract.model.exceptions import DataContractException, DefinitionResolutionError from datacontract.model.run import Check, ResultEnum, Run @@ -323,6 +324,14 @@ def _cli_version() -> str: "url": "https://docs.datacontract.com/commands/changelog", }, }, + { + "name": "breaking", + "description": "Compare two versions of a data contract and grade each change by compatibility impact.", + "externalDocs": { + "description": "Documentation", + "url": "https://docs.datacontract.com/commands/breaking", + }, + }, ], ) @@ -881,6 +890,21 @@ class ChangelogResponse(BaseModel): ) +class BreakingChangesResponse(BaseModel): + """Every change between two versions of a data contract, each graded `info`, `warning` or `error`.""" + + summary: list[BreakingChangeEntry] = Field( + description="One entry per changed element, graded by the most severe change. A property whose type and requiredness " + "both changed appears once.", + ) + entries: list[BreakingChangeEntry] = Field( + description="Every single change with its grade and the old and new value.", + ) + is_breaking: bool = Field( + description="Whether any detected change is classified as an error-level breaking change.", + ) + + @app.post( "/changelog", tags=["changelog"], @@ -935,6 +959,54 @@ async def changelog_endpoint( os.unlink(v2_path) +@app.post( + "/breaking", + tags=["breaking"], + operation_id="breakingChangesDataContracts", + summary="Show compatibility impact between two data contracts.", + description=""" +Compare two versions of an ODCS data contract and detect backward-incompatible changes. + +`POST` a JSON body with `v1` (before) and `v2` (after) as YAML strings. A contract that cannot be +parsed is answered with `422`. + """, + response_description="The breaking changes detected between the two data contracts.", + responses={ + **AUTHENTICATION_RESPONSES, + 422: { + "description": "One of the two data contracts is not valid YAML or not a valid data contract.", + "model": UnprocessableEntityResponse, + "content": {"application/json": {"example": {"detail": "Invalid YAML: while parsing a block mapping"}}}, + }, + }, +) +def breaking_endpoint( + body: ChangelogRequest, + api_key: Annotated[str | None, Depends(api_key_header)] = None, +) -> BreakingChangesResponse: + check_api_key(api_key) + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f1: + f1.write(body.v1) + v1_path = f1.name + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f2: + f2.write(body.v2) + v2_path = f2.name + + try: + result = DataContract(data_contract_file=v1_path).breaking(DataContract(data_contract_file=v2_path)) + return BreakingChangesResponse(summary=result.summary, entries=result.entries, is_breaking=result.is_breaking) + except yaml.YAMLError as e: + raise HTTPException(status_code=422, detail=f"Invalid YAML: {e}") + except pydantic.ValidationError as e: + raise HTTPException(status_code=422, detail=f"Invalid data contract: {e}") + except DataContractException as e: + raise HTTPException(status_code=422, detail=f"Data Contract Validation Failure: {e}") + finally: + os.unlink(v1_path) + os.unlink(v2_path) + + @app.post( "/export", tags=["export"], diff --git a/datacontract/breaking/__init__.py b/datacontract/breaking/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/datacontract/breaking/detector.py b/datacontract/breaking/detector.py new file mode 100644 index 000000000..777c28848 --- /dev/null +++ b/datacontract/breaking/detector.py @@ -0,0 +1,74 @@ +from collections import defaultdict +from collections.abc import Iterable + +from datacontract.breaking.rules import DEFAULT_RULES, BreakingChangeRule +from datacontract.model.breaking import BreakingChangeEntry, BreakingChangeLevel, BreakingChangeResult +from datacontract.model.changelog import ChangelogEntry, ChangelogResult, ChangelogType + +_LEVEL_ORDER = { + BreakingChangeLevel.INFO: 0, + BreakingChangeLevel.WARNING: 1, + BreakingChangeLevel.ERROR: 2, +} + + +class BreakingChangeDetector: + """Classify detailed changelog entries using an ordered rule set.""" + + def __init__(self, rules: Iterable[BreakingChangeRule] = DEFAULT_RULES): + self._rules = tuple(rules) + if not self._rules: + raise ValueError("At least one breaking-change rule is required") + + def detect(self, changelog: ChangelogResult) -> BreakingChangeResult: + entries = [self._classify(entry) for entry in changelog.entries] + # Nothing inside a newly added element can break a consumer: it did not exist before. + added = {entry.path for entry in entries if entry.change_type == ChangelogType.added} + for entry in entries: + segments = entry.path.split(".") + if any(".".join(segments[:i]) in added for i in range(1, len(segments))): + entry.level = BreakingChangeLevel.INFO + entries_by_prefix = defaultdict(list) + for entry in entries: + prefix = "" + for segment in entry.path.split("."): + prefix = f"{prefix}.{segment}" if prefix else segment + entries_by_prefix[prefix].append(entry) + summary = [self._summarize(entry, entries_by_prefix.get(entry.path, [])) for entry in changelog.summary] + return BreakingChangeResult(v1=changelog.v1, v2=changelog.v2, summary=summary, entries=entries) + + def _classify(self, entry: ChangelogEntry) -> BreakingChangeEntry: + for rule in self._rules: + evaluation = rule.evaluate(entry) + if evaluation is not None: + return BreakingChangeEntry( + path=entry.path, + change_type=entry.type, + level=evaluation.level, + message=evaluation.message, + rule_id=evaluation.rule_id, + old_value=entry.old_value, + new_value=entry.new_value, + ) + raise ValueError(f"No breaking-change rule classified {entry.path}") + + @staticmethod + def _summarize(summary_entry: ChangelogEntry, entries: list[BreakingChangeEntry]) -> BreakingChangeEntry: + if not entries: + return BreakingChangeEntry( + path=summary_entry.path, + change_type=summary_entry.type, + level=BreakingChangeLevel.INFO, + message=f"Changed contract at {summary_entry.path}", + rule_id="summary-fallback", + ) + highest = max(entries, key=lambda entry: _LEVEL_ORDER[entry.level]) + return BreakingChangeEntry( + path=summary_entry.path, + change_type=summary_entry.type, + level=highest.level, + message=f"{highest.level.value.capitalize()} impact at {summary_entry.path}", + rule_id=f"summary:{highest.rule_id}", + old_value=highest.old_value, + new_value=highest.new_value, + ) diff --git a/datacontract/breaking/rules.py b/datacontract/breaking/rules.py new file mode 100644 index 000000000..b2f6526a5 --- /dev/null +++ b/datacontract/breaking/rules.py @@ -0,0 +1,196 @@ +from abc import ABC, abstractmethod +from dataclasses import dataclass +from datetime import date + +from datacontract.model.breaking import BreakingChangeLevel +from datacontract.model.changelog import ChangelogEntry, ChangelogType + + +@dataclass(frozen=True) +class RuleEvaluation: + rule_id: str + level: BreakingChangeLevel + message: str + + +class BreakingChangeRule(ABC): + rule_id: str + + @abstractmethod + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + """Return a classification when this rule applies to ``entry``.""" + + +class SchemaRemovedRule(BreakingChangeRule): + rule_id = "schema-removed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + segments = entry.path.split(".") + if entry.type == ChangelogType.removed and len(segments) == 2 and segments[0] == "schema": + return RuleEvaluation(self.rule_id, BreakingChangeLevel.ERROR, f"Removed schema {segments[1]}") + return None + + +class FieldRemovedRule(BreakingChangeRule): + rule_id = "field-removed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + segments = entry.path.split(".") + if entry.type != ChangelogType.removed or not _is_schema_property_path(segments): + return None + property_name = segments[-1] + return RuleEvaluation(self.rule_id, BreakingChangeLevel.ERROR, f"Removed property {property_name}") + + +class RequiredChangedRule(BreakingChangeRule): + rule_id = "required-changed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + if not entry.path.endswith(".required"): + return None + old = _parse_bool(entry.old_value) + new = _parse_bool(entry.new_value) + if entry.type == ChangelogType.added and new is True: + level = BreakingChangeLevel.ERROR + elif entry.type == ChangelogType.removed or new is False: + # Dropping the requirement, or writing out the optional default, only loosens the contract. + level = BreakingChangeLevel.INFO + elif entry.type == ChangelogType.updated and old is False and new is True: + level = BreakingChangeLevel.ERROR + elif entry.type in (ChangelogType.added, ChangelogType.updated): + level = BreakingChangeLevel.WARNING + else: + return None + return RuleEvaluation(self.rule_id, level, _change_message("requiredness", entry)) + + +class TypeChangedRule(BreakingChangeRule): + rule_id = "type-changed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + if not entry.path.endswith((".logicalType", ".physicalType")): + return None + if entry.type == ChangelogType.updated: + level = BreakingChangeLevel.ERROR + elif entry.type == ChangelogType.added: + level = BreakingChangeLevel.INFO + elif entry.type == ChangelogType.removed: + level = BreakingChangeLevel.WARNING + else: + return None + return RuleEvaluation(self.rule_id, level, _change_message("type", entry)) + + +class UniqueConstraintRule(BreakingChangeRule): + rule_id = "unique-constraint-changed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + if not entry.path.endswith(".unique"): + return None + old = _parse_bool(entry.old_value) + new = _parse_bool(entry.new_value) + if new is True and old is not True: + level = BreakingChangeLevel.ERROR + elif old is True and new is not True: + level = BreakingChangeLevel.INFO + else: + level = BreakingChangeLevel.WARNING + return RuleEvaluation(self.rule_id, level, _change_message("uniqueness", entry)) + + +class KeyConstraintRule(BreakingChangeRule): + rule_id = "key-constraint-changed" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + if not entry.path.endswith((".primaryKey", ".primary_key")): + return None + return RuleEvaluation(self.rule_id, BreakingChangeLevel.WARNING, _change_message("key constraint", entry)) + + +class ValidationConstraintRule(BreakingChangeRule): + rule_id = "validation-constraint-changed" + _suffixes = ( + ".pattern", + ".minLength", + ".maxLength", + ".minimum", + ".maximum", + ".exclusiveMinimum", + ".exclusiveMaximum", + ) + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation | None: + if not entry.path.endswith(self._suffixes): + return None + old = _parse_number(entry.old_value) + new = _parse_number(entry.new_value) + if old is None or new is None: + level = BreakingChangeLevel.WARNING + elif _is_tightening(entry.path, old, new): + level = BreakingChangeLevel.ERROR + else: + level = BreakingChangeLevel.INFO + return RuleEvaluation(self.rule_id, level, _change_message("validation constraint", entry)) + + +class MetadataFallbackRule(BreakingChangeRule): + rule_id = "metadata-or-unknown-change" + + def evaluate(self, entry: ChangelogEntry) -> RuleEvaluation: + return RuleEvaluation(self.rule_id, BreakingChangeLevel.INFO, _change_message("contract", entry)) + + +def _is_schema_property_path(segments: list[str]) -> bool: + return len(segments) >= 3 and segments[0] == "schema" and segments[-2] == "properties" + + +def _parse_bool(value: str | None) -> bool | None: + if value is None: + return None + normalized = value.strip().lower() + if normalized in {"true", "1"}: + return True + if normalized in {"false", "0"}: + return False + return None + + +def _parse_number(value: str | None) -> float | None: + if value is None: + return None + normalized = value.strip() + try: + return float(normalized) + except ValueError: + try: + return float(date.fromisoformat(normalized).toordinal()) + except ValueError: + return None + + +def _is_tightening(path: str, old: float, new: float) -> bool: + if path.endswith((".minLength", ".minimum", ".exclusiveMinimum")): + return new > old + if path.endswith((".maxLength", ".maximum", ".exclusiveMaximum")): + return new < old + return True + + +def _change_message(subject: str, entry: ChangelogEntry) -> str: + if entry.type == ChangelogType.added: + return f"Added {subject} at {entry.path}" + if entry.type == ChangelogType.removed: + return f"Removed {subject} at {entry.path}" + return f"Changed {subject} at {entry.path} from {entry.old_value!r} to {entry.new_value!r}" + + +DEFAULT_RULES: tuple[BreakingChangeRule, ...] = ( + RequiredChangedRule(), + SchemaRemovedRule(), + FieldRemovedRule(), + TypeChangedRule(), + UniqueConstraintRule(), + KeyConstraintRule(), + ValidationConstraintRule(), + MetadataFallbackRule(), +) diff --git a/datacontract/cli.py b/datacontract/cli.py index 4f94b797a..bad06d8ce 100644 --- a/datacontract/cli.py +++ b/datacontract/cli.py @@ -28,6 +28,7 @@ "edit", "lint", "changelog", + "breaking", "sync", # `dbt sync` subcommand; no top-level `sync`, so this only orders the dbt group "test", "ci", @@ -263,6 +264,7 @@ def _print_publish_failure(run, out=None): # Display order for `--help` is controlled by COMMAND_ORDER above, not by import order. from datacontract import ( # noqa: E402, F401 command_api, + command_breaking, command_catalog, command_changelog, command_ci, diff --git a/datacontract/command_breaking.py b/datacontract/command_breaking.py new file mode 100644 index 000000000..02c447e53 --- /dev/null +++ b/datacontract/command_breaking.py @@ -0,0 +1,39 @@ +import typer +from typing_extensions import Annotated + +from datacontract.cli import app, console, debug_option, enable_debug_logging +from datacontract.config import cli_config +from datacontract.data_contract import DataContract +from datacontract.output.text_breaking_results import write_text_breaking_results + + +@app.command( + name="breaking", + epilog="Example: datacontract breaking datacontract-v1.yaml datacontract-v2.yaml", +) +def breaking( + v1: Annotated[ + str, + typer.Argument(help="The location (url, s3 url, or local path) of the source (before) data contract YAML."), + ], + v2: Annotated[ + str, + typer.Argument(help="The location (url, s3 url, or local path) of the target (after) data contract YAML."), + ], + inline_references: Annotated[ + bool, + typer.Option( + help="Resolve external references (currently: authoritativeDefinitions\\[type in {definition, semantics}]) " + "in the contract and inline the fetched content from the configured entropy-data host." + ), + ] = True, + debug: debug_option = None, +): + """Show compatibility impact between two data contracts.""" + enable_debug_logging(debug) + result = DataContract(config=cli_config(), data_contract_file=v1, inline_references=inline_references).breaking( + DataContract(config=cli_config(), data_contract_file=v2, inline_references=inline_references) + ) + write_text_breaking_results(result, console) + if result.is_breaking: + raise typer.Exit(code=1) diff --git a/datacontract/data_contract.py b/datacontract/data_contract.py index ced201d07..798e96bab 100644 --- a/datacontract/data_contract.py +++ b/datacontract/data_contract.py @@ -9,6 +9,7 @@ from duckdb.duckdb import DuckDBPyConnection from pyspark.sql import SparkSession +from datacontract.breaking.detector import BreakingChangeDetector from datacontract.config import Config from datacontract.engines.checks.dimensions import default_dimension from datacontract.engines.data_contract_test import execute_data_contract_test @@ -18,6 +19,7 @@ from datacontract.init.init_template import get_init_template from datacontract.integration.entropy_data import publish_test_results_to_entropy_data from datacontract.lint import resolve +from datacontract.model.breaking import BreakingChangeResult from datacontract.model.changelog import ChangelogEntry, ChangelogResult, ChangelogType from datacontract.model.exceptions import DataContractException, DataContractValidationErrors from datacontract.model.run import Check, ResultEnum, Run @@ -280,6 +282,11 @@ def changelog(self, other: "DataContract") -> ChangelogResult: ) return result + def breaking(self, other: "DataContract", detector: BreakingChangeDetector | None = None) -> BreakingChangeResult: + """Classify the changelog between this contract and another for compatibility impact.""" + changelog = self.changelog(other) + return (detector or BreakingChangeDetector()).detect(changelog) + @classmethod def import_from_source( cls, diff --git a/datacontract/model/breaking.py b/datacontract/model/breaking.py new file mode 100644 index 000000000..157f1f1b6 --- /dev/null +++ b/datacontract/model/breaking.py @@ -0,0 +1,33 @@ +from enum import Enum + +from pydantic import BaseModel, Field, computed_field + +from datacontract.model.changelog import ChangelogType + + +class BreakingChangeLevel(str, Enum): + INFO = "info" + WARNING = "warning" + ERROR = "error" + + +class BreakingChangeEntry(BaseModel): + path: str + change_type: ChangelogType + level: BreakingChangeLevel + message: str + rule_id: str + old_value: str | None = None + new_value: str | None = None + + +class BreakingChangeResult(BaseModel): + v1: str + v2: str + summary: list[BreakingChangeEntry] = Field(default_factory=list) + entries: list[BreakingChangeEntry] = Field(default_factory=list) + + @computed_field + @property + def is_breaking(self) -> bool: + return any(entry.level == BreakingChangeLevel.ERROR for entry in self.entries) diff --git a/datacontract/output/text_breaking_results.py b/datacontract/output/text_breaking_results.py new file mode 100644 index 000000000..8fd2fc674 --- /dev/null +++ b/datacontract/output/text_breaking_results.py @@ -0,0 +1,81 @@ +import io +from collections import Counter + +from rich import box +from rich.console import Console +from rich.table import Table + +from datacontract.model.breaking import BreakingChangeEntry, BreakingChangeLevel, BreakingChangeResult +from datacontract.output.text_changelog_results import _wrap + +_VAL_W = 30 +_LEVEL_ORDER = [BreakingChangeLevel.ERROR, BreakingChangeLevel.WARNING, BreakingChangeLevel.INFO] +_LEVEL_COLOR = { + BreakingChangeLevel.ERROR: "red", + BreakingChangeLevel.WARNING: "yellow", + BreakingChangeLevel.INFO: "green", +} + + +def write_text_breaking_results(result: BreakingChangeResult, console: Console): + _print_summary(result, console) + _print_table(result, console) + + +def _badges(entries: list[BreakingChangeEntry]) -> str: + counts = Counter(entry.level for entry in entries) + parts = [] + for level in _LEVEL_ORDER: + count = counts[level] + if count: + color = _LEVEL_COLOR[level] + parts.append(f"[ [{color}]{count} {level.value.capitalize()}[/{color}] ]") + return " ".join(parts) + + +def _print_summary(result: BreakingChangeResult, console: Console): + if not result.summary: + return + console.print("Summary") + console.print(_badges(result.summary)) + table = Table(box=box.ROUNDED, show_header=True) + table.add_column("Severity", no_wrap=True) + table.add_column("Change", no_wrap=True) + table.add_column("Field", no_wrap=True) + for entry in result.summary: + table.add_row(_severity_markup(entry.level), entry.change_type.value.capitalize(), entry.path) + _print_wide(table, console) + + +def _print_table(result: BreakingChangeResult, console: Console): + console.print("Details") + table = Table(box=box.ROUNDED) + table.add_column("Severity", no_wrap=True) + table.add_column("Change", no_wrap=True) + table.add_column("Path", no_wrap=True) + table.add_column("Old Value", max_width=_VAL_W, no_wrap=True) + table.add_column("New Value", max_width=_VAL_W, no_wrap=True) + table.add_column("Message", max_width=_VAL_W, no_wrap=True) + for entry in result.entries: + table.add_row( + _severity_markup(entry.level), + entry.change_type.value.capitalize(), + entry.path, + _wrap(entry.old_value or "", _VAL_W), + _wrap(entry.new_value or "", _VAL_W), + _wrap(entry.message, _VAL_W), + ) + _print_wide(table, console) + + +def _severity_markup(level: BreakingChangeLevel) -> str: + color = _LEVEL_COLOR[level] + return f"[{color}]{level.value.upper()}[/{color}]" + + +def _print_wide(table: Table, console: Console): + buf = io.StringIO() + wide = Console(file=buf, width=300, highlight=False, force_terminal=console.is_terminal, no_color=console.no_color) + wide.print(table) + print(buf.getvalue(), end="") + print("") diff --git a/docs/docs/api.md b/docs/docs/api.md index 437d03a6d..b22362045 100644 --- a/docs/docs/api.md +++ b/docs/docs/api.md @@ -66,9 +66,11 @@ curl -X POST "http://localhost:4242/export?format=sql" \ --data-binary @datacontract.yaml ``` -## Changelog between two contracts +## Comparing two contracts -POST a JSON body with `v1` (before) and `v2` (after) as YAML strings. The response is a JSON object with `summary` and `entries`: +Both comparison endpoints take the same JSON body: `v1` (before) and `v2` (after) as YAML strings. + +`POST /changelog` lists what changed. The response is a JSON object with `summary` (one entry per changed field) and `entries` (one per atomic change): ```bash curl -X POST "http://localhost:4242/changelog" \ @@ -79,6 +81,37 @@ curl -X POST "http://localhost:4242/changelog" \ }' ``` +`POST /breaking` grades those same changes for compatibility impact. It adds a `level` (`info`, `warning` or `error`) and a `rule_id` to every entry, plus a top-level `is_breaking` flag that is `true` when any entry is an `error`: + +```bash +curl -X POST "http://localhost:4242/breaking" \ + -H "Content-Type: application/json" \ + -d '{ + "v1": "'"$(cat v1.odcs.yaml)"'", + "v2": "'"$(cat v2.odcs.yaml)"'" + }' +``` + +```json +{ + "summary": [...], + "entries": [ + { + "path": "schema.orders.properties.order_id.logicalTypeOptions.pattern", + "change_type": "updated", + "level": "warning", + "message": "Changed validation constraint at schema.orders.properties.order_id.logicalTypeOptions.pattern from '^ORD-[0-9]+$' to '^ORD-[0-9]{4}$'", + "rule_id": "validation-constraint-changed", + "old_value": "^ORD-[0-9]+$", + "new_value": "^ORD-[0-9]{4}$" + } + ], + "is_breaking": false +} +``` + +See [Compare contract versions](./compare-contract-versions.md) for the severity levels and the rules behind them. + ## Configure server credentials To connect to a data source, set the required credentials as environment variables **before starting the API** (see [Configuration](./configuration.md)). For example, for Snowflake: diff --git a/docs/docs/compare-contract-versions.md b/docs/docs/compare-contract-versions.md new file mode 100644 index 000000000..13f687a3b --- /dev/null +++ b/docs/docs/compare-contract-versions.md @@ -0,0 +1,56 @@ +--- +sidebar_position: 8 +title: "Compare contract versions" +description: "Describe differences and classify backward-compatibility impact between two ODCS data contracts." +--- + +# Compare contract versions + +Use `datacontract changelog` to describe the differences between two versions of a contract, or use `datacontract breaking` to classify their backward-compatibility impact. + +## Changelog + +Use `datacontract changelog` to compare the source contract (`v1`) with the target contract (`v2`) and report the changes between them: + +```bash +datacontract changelog v1.odcs.yaml v2.odcs.yaml +``` + +See the generated [changelog command reference](./commands/changelog.md) for all options. + +## Breaking changes + +Use `datacontract breaking` when a contract change must be checked for backward compatibility. The command compares the source contract (`v1`) with the target contract (`v2`) and preserves the detailed changelog while adding a severity classification. + +```bash +datacontract breaking v1.odcs.yaml v2.odcs.yaml +``` + +See the generated [breaking command reference](./commands/breaking.md) for all options. + +### Severity levels + +- **ERROR** - a backward-incompatible change. The command exits with status `1`. +- **WARNING** - a potentially incompatible change that requires review. The command exits with status `0`. +- **INFO** - informational or currently unclassified metadata. The command exits with status `0`. + +The result is breaking only when at least one detailed entry has severity `ERROR`. + +### Initial compatibility rules + +The first ODCS implementation treats schema and property removals, requiredness tightening, type changes, and uniqueness tightening as errors. Primary-key changes and changes to validation constraints whose direction cannot be proven are warnings. Additions, relaxed constraints, descriptions, tags, business names, custom properties, and unrecognized changes are informational unless a more specific rule applies. + +Every detailed changelog entry receives exactly one classification. Unknown fields use the informational fallback so that introducing a new ODCS field does not make detection fail. + +### API + +The same result is available from `POST /breaking`, using the same JSON request shape as `POST /changelog`: + +```json +{ + "v1": "", + "v2": "" +} +``` + +The response includes `summary`, `entries`, and an `is_breaking` boolean. The endpoint returns HTTP `200` for a valid comparison even when `is_breaking` is `true`; invalid YAML or contracts return HTTP `422`. \ No newline at end of file diff --git a/docs/docs/python-library.md b/docs/docs/python-library.md index a8970c628..936d0fd4c 100644 --- a/docs/docs/python-library.md +++ b/docs/docs/python-library.md @@ -121,7 +121,9 @@ print(data_contract.export("odcs")) See [Imports](./imports/index.md) for the full list of formats. -## Compare two contracts (changelog) +## Compare two contracts + +`changelog()` lists what changed between two versions: ```python from datacontract.data_contract import DataContract @@ -133,6 +135,17 @@ result = v1.changelog(v2) print(result) ``` +`breaking()` grades those same changes for compatibility impact. Every entry carries a `level` (`info`, `warning` or `error`), and `is_breaking` is `True` when any entry is an error: + +```python +result = v1.breaking(v2) + +for entry in result.entries: + print(entry.level.value, entry.path, entry.message) +``` + +See [Compare contract versions](./compare-contract-versions.md) for the severity levels and the rules behind them. + ## Spark DataFrames and Databricks Pass a `SparkSession` to test in-memory DataFrames (registered as temporary views) or to run inside a Databricks notebook: diff --git a/docs/docusaurus.config.ts b/docs/docusaurus.config.ts index 6c9ff95d2..90cf6cb97 100644 --- a/docs/docusaurus.config.ts +++ b/docs/docusaurus.config.ts @@ -1,5 +1,5 @@ -import {themes as prismThemes} from 'prism-react-renderer'; -import type {Config} from '@docusaurus/types'; +import { themes as prismThemes } from 'prism-react-renderer'; +import type { Config } from '@docusaurus/types'; import type * as Preset from '@docusaurus/preset-classic'; // This runs in Node.js - Don't use client-side code here (browser APIs, JSX...) @@ -21,7 +21,7 @@ function withAccessibleTokenColors( const color = entry.style?.color; const replacement = color && replacements[color]; return replacement - ? {...entry, style: {...entry.style, color: replacement}} + ? { ...entry, style: { ...entry.style, color: replacement } } : entry; }), }; @@ -113,7 +113,7 @@ const config: Config = { // Raise the priority of the main hub pages so crawlers can // distinguish them from the long tail of import/export reference pages. createSitemapItems: async (params) => { - const {defaultCreateSitemapItems, ...rest} = params; + const { defaultCreateSitemapItems, ...rest } = params; const items = await defaultCreateSitemapItems(rest); const hubs: Record = { 'https://docs.datacontract.com/': 1.0, @@ -122,7 +122,7 @@ const config: Config = { }; return items.map((item) => { const priority = hubs[item.url]; - return priority ? {...item, priority} : item; + return priority ? { ...item, priority } : item; }); }, }, @@ -160,6 +160,7 @@ const config: Config = { 'testing/index.md', 'testing/*.md', 'schema.md', + 'compare-contract-versions.md', 'quality-rules/index.md', 'quality-rules/*.md', 'service-levels.md', @@ -212,8 +213,8 @@ const config: Config = { themeConfig: { image: 'img/datacontractcli.png', metadata: [ - {name: 'og:type', content: 'website'}, - {name: 'og:site_name', content: 'Data Contract CLI'}, + { name: 'og:type', content: 'website' }, + { name: 'og:site_name', content: 'Data Contract CLI' }, { name: 'keywords', content: @@ -261,16 +262,16 @@ const config: Config = { { title: 'Docs', items: [ - {label: 'What is Data Contract CLI?', to: '/'}, - {label: 'Quickstart', to: '/quickstart'}, - {label: 'Commands', to: '/commands/'}, - {label: 'Release Notes', to: '/release-notes'}, + { label: 'What is Data Contract CLI?', to: '/' }, + { label: 'Quickstart', to: '/quickstart' }, + { label: 'Commands', to: '/commands/' }, + { label: 'Release Notes', to: '/release-notes' }, ], }, { title: 'Community', items: [ - {label: 'Slack', href: 'https://datacontract.com/slack'}, + { label: 'Slack', href: 'https://datacontract.com/slack' }, { label: 'GitHub', href: 'https://github.com/datacontract/datacontract-cli', @@ -284,16 +285,16 @@ const config: Config = { { title: 'More', items: [ - {label: 'datacontract.com', href: 'https://datacontract.com'}, - {label: 'Data Contract Editor', href: 'https://editor.datacontract.com'}, - {label: 'PyPI', href: 'https://pypi.org/project/datacontract-cli/'}, + { label: 'datacontract.com', href: 'https://datacontract.com' }, + { label: 'Data Contract Editor', href: 'https://editor.datacontract.com' }, + { label: 'PyPI', href: 'https://pypi.org/project/datacontract-cli/' }, ], }, { title: 'Legal', items: [ - {label: 'Legal Notice', href: 'https://entropy-data.com/legal-notice'}, - {label: 'Privacy Policy', href: 'https://entropy-data.com/privacy-policy'}, + { label: 'Legal Notice', href: 'https://entropy-data.com/legal-notice' }, + { label: 'Privacy Policy', href: 'https://entropy-data.com/privacy-policy' }, ], }, ], diff --git a/tests/fixtures/changelog/breaking/warning_only_v1.yaml b/tests/fixtures/changelog/breaking/warning_only_v1.yaml new file mode 100644 index 000000000..1c71138f9 --- /dev/null +++ b/tests/fixtures/changelog/breaking/warning_only_v1.yaml @@ -0,0 +1,14 @@ +apiVersion: v3.0.2 +kind: DataContract +id: orders-contract-001 +status: active +version: 1.0.0 +schema: + - name: orders + physicalName: orders_tbl + properties: + - name: order_id + logicalType: string + required: true + logicalTypeOptions: + pattern: "^ORD-[0-9]+$" diff --git a/tests/fixtures/changelog/breaking/warning_only_v2.yaml b/tests/fixtures/changelog/breaking/warning_only_v2.yaml new file mode 100644 index 000000000..c4d686d51 --- /dev/null +++ b/tests/fixtures/changelog/breaking/warning_only_v2.yaml @@ -0,0 +1,17 @@ +apiVersion: v3.0.2 +kind: DataContract +id: orders-contract-001 +status: active +version: 2.0.0 +schema: + - name: orders + physicalName: orders_tbl + properties: + - name: order_id + logicalType: string + required: true + logicalTypeOptions: + pattern: "^ORD-[0-9]{4}$" + - name: region + logicalType: string + required: false diff --git a/tests/fixtures/changelog/golden_breaking_text.txt b/tests/fixtures/changelog/golden_breaking_text.txt new file mode 100644 index 000000000..b59936018 --- /dev/null +++ b/tests/fixtures/changelog/golden_breaking_text.txt @@ -0,0 +1,112 @@ +Summary +[ 3 Error ] [ 5 Info ] +╭──────────┬─────────┬───────────────────────────────────────╮ +│ Severity │ Change │ Field │ +├──────────┼─────────┼───────────────────────────────────────┤ +│ INFO │ Added │ schema.customers │ +│ ERROR │ Removed │ schema.orders.properties.customer_id │ +│ ERROR │ Updated │ schema.orders.properties.order_date │ +│ INFO │ Updated │ schema.orders.properties.order_id │ +│ INFO │ Added │ schema.orders.properties.region │ +│ ERROR │ Updated │ schema.orders.properties.total_amount │ +│ INFO │ Updated │ slaProperties.availability │ +│ INFO │ Updated │ version │ +╰──────────┴─────────┴───────────────────────────────────────╯ + +Details +╭──────────┬─────────┬──────────────────────────────────────────────────────────┬────────────────────────────────┬───────────────────────────────┬────────────────────────────────╮ +│ Severity │ Change │ Path │ Old Value │ New Value │ Message │ +├──────────┼─────────┼──────────────────────────────────────────────────────────┼────────────────────────────────┼───────────────────────────────┼────────────────────────────────┤ +│ INFO │ Added │ schema.customers │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers │ +│ INFO │ Added │ schema.customers.physicalName │ │ customers_tbl │ Added contract at │ +│ │ │ │ │ │ schema.customers.physicalName │ +│ INFO │ Added │ schema.customers.properties │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties │ +│ INFO │ Added │ schema.customers.properties.country │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.country.logicalType │ │ string │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.country.partitionKeyPosition │ │ 1 │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.country.partitioned │ │ True │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.country.required │ │ False │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.created_at │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.created_at.description │ │ Record creation timestamp │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.created_at.logicalType │ │ timestamp │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.created_at.required │ │ True │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.customer_id │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.customer_id.description │ │ Unique order ID │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.customer_id.logicalType │ │ string │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.customer_id.primaryKey │ │ True │ Added key constraint at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.customer_id.required │ │ True │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.c… │ +│ INFO │ Added │ schema.customers.properties.date_of_birth │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.d… │ +│ INFO │ Added │ schema.customers.properties.date_of_birth.classification │ │ restricted │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.d… │ +│ INFO │ Added │ schema.customers.properties.date_of_birth.logicalType │ │ date │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.d… │ +│ INFO │ Added │ schema.customers.properties.date_of_birth.required │ │ False │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.d… │ +│ INFO │ Added │ schema.customers.properties.email │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.e… │ +│ INFO │ Added │ schema.customers.properties.email.classification │ │ confidential │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.e… │ +│ INFO │ Added │ schema.customers.properties.email.encryptedName │ │ email_encrypt │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.e… │ +│ INFO │ Added │ schema.customers.properties.email.logicalType │ │ string │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.e… │ +│ INFO │ Added │ schema.customers.properties.email.required │ │ True │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.e… │ +│ INFO │ Added │ schema.customers.properties.full_name │ │ │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.f… │ +│ INFO │ Added │ schema.customers.properties.full_name.businessName │ │ Customer Full Name │ Added contract at │ +│ │ │ │ │ │ schema.customers.properties.f… │ +│ INFO │ Added │ schema.customers.properties.full_name.logicalType │ │ string │ Added type at │ +│ │ │ │ │ │ schema.customers.properties.f… │ +│ INFO │ Added │ schema.customers.properties.full_name.required │ │ True │ Added requiredness at │ +│ │ │ │ │ │ schema.customers.properties.f… │ +│ ERROR │ Removed │ schema.orders.properties.customer_id │ │ │ Removed property customer_id │ +│ WARNING │ Removed │ schema.orders.properties.customer_id.logicalType │ string │ │ Removed type at │ +│ │ │ │ │ │ schema.orders.properties.cust… │ +│ INFO │ Removed │ schema.orders.properties.customer_id.required │ True │ │ Removed requiredness at │ +│ │ │ │ │ │ schema.orders.properties.cust… │ +│ ERROR │ Updated │ schema.orders.properties.order_date.logicalType │ string │ date │ Changed type at │ +│ │ │ │ │ │ schema.orders.properties.orde… │ +│ │ │ │ │ │ from 'string' to 'date' │ +│ INFO │ Updated │ schema.orders.properties.order_id.description │ Unique order ID and a rather │ Unique order ID and another │ Changed contract at │ +│ │ │ │ lenghty description that │ rather lenghty description │ schema.orders.properties.orde… │ +│ │ │ │ should be wrapped in the table │ that should be wrapped in the │ from 'Unique order ID and a │ +│ │ │ │ │ table │ rather lenghty description │ +│ │ │ │ │ │ that should be wrapped in the │ +│ │ │ │ │ │ table' to 'Unique order ID and │ +│ │ │ │ │ │ another rather lenghty │ +│ │ │ │ │ │ description that should be │ +│ │ │ │ │ │ wrapped in the table' │ +│ INFO │ Added │ schema.orders.properties.region │ │ │ Added contract at │ +│ │ │ │ │ │ schema.orders.properties.regi… │ +│ INFO │ Added │ schema.orders.properties.region.logicalType │ │ string │ Added type at │ +│ │ │ │ │ │ schema.orders.properties.regi… │ +│ INFO │ Added │ schema.orders.properties.region.required │ │ False │ Added requiredness at │ +│ │ │ │ │ │ schema.orders.properties.regi… │ +│ ERROR │ Updated │ schema.orders.properties.total_amount.required │ False │ True │ Changed requiredness at │ +│ │ │ │ │ │ schema.orders.properties.tota… │ +│ │ │ │ │ │ from 'False' to 'True' │ +│ INFO │ Updated │ slaProperties.availability.value │ 99.9% │ 99.5% │ Changed contract at │ +│ │ │ │ │ │ slaProperties.availability.va… │ +│ │ │ │ │ │ from '99.9%' to '99.5%' │ +│ INFO │ Updated │ version │ 1.0.0 │ 2.0.0 │ Changed contract at version │ +│ │ │ │ │ │ from '1.0.0' to '2.0.0' │ +╰──────────┴─────────┴──────────────────────────────────────────────────────────┴────────────────────────────────┴───────────────────────────────┴────────────────────────────────╯ + diff --git a/tests/fixtures/changelog/helper/generate_golden.py b/tests/fixtures/changelog/helper/generate_golden.py index 18ff2036f..503804bcc 100644 --- a/tests/fixtures/changelog/helper/generate_golden.py +++ b/tests/fixtures/changelog/helper/generate_golden.py @@ -9,6 +9,7 @@ Golden files written: tests/fixtures/changelog/golden_changelog_text.txt + tests/fixtures/changelog/golden_breaking_text.txt After running, review the diff with git and commit if the changes are expected: git diff tests/fixtures/changelog/ @@ -27,26 +28,39 @@ V2 = os.path.normpath(os.path.join(REPO_ROOT, "tests/fixtures/changelog/integration/changelog_integration_v2.yaml")) -def generate(): - # Import here so the script can be run from the repo root with venv activated - from datacontract.data_contract import DataContract - from datacontract.output.text_changelog_results import write_text_changelog_results - - result = DataContract(data_contract_file=V1).changelog(DataContract(data_contract_file=V2)) - +def _render(write, result) -> str: buf = io.StringIO() con = Console(file=buf, width=300, highlight=False, no_color=True) old_stdout = sys.stdout sys.stdout = buf try: - write_text_changelog_results(result, con) + write(result, con) finally: sys.stdout = old_stdout + return buf.getvalue() + + +def generate(): + # Import here so the script can be run from the repo root with venv activated + from datacontract.data_contract import DataContract + from datacontract.output.text_breaking_results import write_text_breaking_results + from datacontract.output.text_changelog_results import write_text_changelog_results + + v1 = DataContract(data_contract_file=V1) + outputs = { + "golden_changelog_text.txt": _render( + write_text_changelog_results, v1.changelog(DataContract(data_contract_file=V2)) + ), + "golden_breaking_text.txt": _render( + write_text_breaking_results, v1.breaking(DataContract(data_contract_file=V2)) + ), + } - text_path = os.path.normpath(os.path.join(FIXTURE_DIR, "golden_changelog_text.txt")) - with open(text_path, "w", encoding="utf-8") as f: - f.write(buf.getvalue()) - print(f"Written: {text_path}") + for name, text in outputs.items(): + text_path = os.path.normpath(os.path.join(FIXTURE_DIR, name)) + with open(text_path, "w", encoding="utf-8") as f: + f.write(text) + print(f"Written: {text_path}") print("\nDone. Review changes with: git diff tests/fixtures/changelog/") diff --git a/tests/test_api.py b/tests/test_api.py index 6858f9c50..08d12ff67 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -69,6 +69,28 @@ def test_changelog_invalid_yaml(): assert "Cannot parse YAML" in detail +def test_breaking(): + with open("fixtures/changelog/integration/changelog_integration_v1.yaml", "r") as f: + v1 = f.read() + with open("fixtures/changelog/integration/changelog_integration_v2.yaml", "r") as f: + v2 = f.read() + response = client.post(url="/breaking", json={"v1": v1, "v2": v2}) + assert response.status_code == 200 + data = response.json() + assert "v1" not in data + assert "v2" not in data + assert data["is_breaking"] is True + assert "summary" in data + assert "entries" in data + assert data["entries"][0]["level"] in ("info", "warning", "error") + assert "message" in data["entries"][0] + + +def test_breaking_invalid_yaml(): + response = client.post(url="/breaking", json={"v1": "invalid: yaml: [", "v2": "valid: yaml"}) + assert response.status_code == 422 + + def test_changelog_invalid_data_contract(): invalid_contract = """ apiVersion: '1.0' diff --git a/tests/test_breaking.py b/tests/test_breaking.py new file mode 100644 index 000000000..f43b4955c --- /dev/null +++ b/tests/test_breaking.py @@ -0,0 +1,337 @@ +import io +import sys +from pathlib import Path + +from rich.console import Console +from typer.testing import CliRunner + +from datacontract.breaking.detector import BreakingChangeDetector +from datacontract.breaking.rules import ( + BreakingChangeRule, + FieldRemovedRule, + KeyConstraintRule, + MetadataFallbackRule, + RequiredChangedRule, + RuleEvaluation, + SchemaRemovedRule, + TypeChangedRule, + UniqueConstraintRule, + ValidationConstraintRule, +) +from datacontract.cli import app +from datacontract.data_contract import DataContract +from datacontract.model.breaking import BreakingChangeLevel +from datacontract.model.changelog import ChangelogEntry, ChangelogResult, ChangelogType +from datacontract.output.text_breaking_results import write_text_breaking_results + +V1 = "fixtures/changelog/integration/changelog_integration_v1.yaml" +V2 = "fixtures/changelog/integration/changelog_integration_v2.yaml" +WARNING_ONLY_V1 = "fixtures/changelog/breaking/warning_only_v1.yaml" +WARNING_ONLY_V2 = "fixtures/changelog/breaking/warning_only_v2.yaml" + +GOLDEN_TEXT = Path(__file__).parent / "fixtures/changelog/golden_breaking_text.txt" + +runner = CliRunner() + + +def _entry(path, change_type, old_value=None, new_value=None): + return ChangelogEntry(path=path, type=change_type, old_value=old_value, new_value=new_value) + + +def test_required_change_to_true_is_error(): + result = RequiredChangedRule().evaluate( + _entry("schema.orders.properties.customer_id.required", ChangelogType.updated, "False", "True") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_required_change_to_false_is_info(): + result = RequiredChangedRule().evaluate( + _entry("schema.orders.properties.customer_id.required", ChangelogType.updated, "true", "false") + ) + assert result is not None + assert result.level == BreakingChangeLevel.INFO + + +def test_adding_required_false_is_info(): + result = RequiredChangedRule().evaluate( + _entry("schema.orders.properties.region.required", ChangelogType.added, None, "False") + ) + assert result is not None + assert result.level == BreakingChangeLevel.INFO + + +def test_adding_a_type_is_info(): + result = TypeChangedRule().evaluate( + _entry("schema.orders.properties.region.logicalType", ChangelogType.added, None, "string") + ) + assert result is not None + assert result.level == BreakingChangeLevel.INFO + + +def test_removing_a_type_is_warning(): + result = TypeChangedRule().evaluate( + _entry("schema.orders.properties.region.logicalType", ChangelogType.removed, "string") + ) + assert result is not None + assert result.level == BreakingChangeLevel.WARNING + + +def test_type_change_is_error(): + result = TypeChangedRule().evaluate( + _entry("schema.orders.properties.order_id.logicalType", ChangelogType.updated, "string", "integer") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_removing_schema_is_breaking(): + result = SchemaRemovedRule().evaluate(_entry("schema.orders", ChangelogType.removed)) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + assert result.message == "Removed schema orders" + + +def test_removing_property_metadata_is_not_property_removal(): + result = FieldRemovedRule().evaluate( + _entry("schema.orders.properties.customer_id.description", ChangelogType.removed, "old description") + ) + assert result is None + + +def test_removing_property_is_breaking(): + result = FieldRemovedRule().evaluate( + _entry("schema.orders.properties.customer_id", ChangelogType.removed, "string") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + assert result.message == "Removed property customer_id" + + +def test_removing_nested_property_is_breaking(): + result = FieldRemovedRule().evaluate( + _entry("schema.orders.properties.customer.properties.email", ChangelogType.removed, "string") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + assert result.message == "Removed property email" + + +def test_removing_nested_property_metadata_is_not_property_removal(): + result = FieldRemovedRule().evaluate( + _entry("schema.orders.properties.customer.properties.email.description", ChangelogType.removed, "old") + ) + assert result is None + + +def test_tightening_uniqueness_is_breaking(): + result = UniqueConstraintRule().evaluate( + _entry("schema.orders.properties.order_id.unique", ChangelogType.updated, "False", "True") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_changing_primary_key_is_warning(): + result = KeyConstraintRule().evaluate( + _entry("schema.orders.properties.order_id.primaryKey", ChangelogType.updated, "True", "False") + ) + assert result is not None + assert result.level == BreakingChangeLevel.WARNING + + +def test_tightening_validation_constraint_is_breaking(): + result = ValidationConstraintRule().evaluate( + _entry("schema.orders.properties.customer_id.minLength", ChangelogType.updated, "5", "10") + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_tightening_date_validation_constraint_is_breaking(): + result = ValidationConstraintRule().evaluate( + _entry( + "schema.orders.properties.order_date.logicalTypeOptions.minimum", + ChangelogType.updated, + "2024-01-01", + "2024-02-01", + ) + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_relaxing_date_minimum_constraint_is_info(): + result = ValidationConstraintRule().evaluate( + _entry( + "schema.orders.properties.order_date.logicalTypeOptions.minimum", + ChangelogType.updated, + "2024-02-01", + "2024-01-01", + ) + ) + assert result is not None + assert result.level == BreakingChangeLevel.INFO + + +def test_tightening_date_maximum_constraint_is_breaking(): + result = ValidationConstraintRule().evaluate( + _entry( + "schema.orders.properties.order_date.logicalTypeOptions.maximum", + ChangelogType.updated, + "2024-02-01", + "2024-01-01", + ) + ) + assert result is not None + assert result.level == BreakingChangeLevel.ERROR + + +def test_relaxing_date_validation_constraint_is_info(): + result = ValidationConstraintRule().evaluate( + _entry( + "schema.orders.properties.order_date.logicalTypeOptions.maximum", + ChangelogType.updated, + "2024-01-01", + "2024-02-01", + ) + ) + assert result is not None + assert result.level == BreakingChangeLevel.INFO + + +def test_invalid_date_validation_constraint_is_warning(): + result = ValidationConstraintRule().evaluate( + _entry( + "schema.orders.properties.order_date.logicalTypeOptions.minimum", + ChangelogType.updated, + "not-a-date", + "2024-01-01", + ) + ) + assert result is not None + assert result.level == BreakingChangeLevel.WARNING + + +def test_unknown_change_is_info(): + result = MetadataFallbackRule().evaluate(_entry("description.purpose", ChangelogType.updated, "old", "new")) + assert result.level == BreakingChangeLevel.INFO + assert result.message == "Changed contract at description.purpose from 'old' to 'new'" + + +def test_unmatched_entry_uses_info_fallback(): + changelog = ChangelogResult( + v1="v1", + v2="v2", + entries=[_entry("description.purpose", ChangelogType.updated, "old", "new")], + ) + result = BreakingChangeDetector().detect(changelog) + assert result.entries[0].level == BreakingChangeLevel.INFO + assert result.entries[0].rule_id == "metadata-or-unknown-change" + assert not result.is_breaking + + +def test_required_inside_an_added_schema_is_not_breaking(): + changelog = ChangelogResult( + v1="v1", + v2="v2", + entries=[ + _entry("schema.customers", ChangelogType.added), + _entry("schema.customers.properties.customer_id", ChangelogType.added), + _entry("schema.customers.properties.customer_id.required", ChangelogType.added, None, "True"), + ], + ) + result = BreakingChangeDetector().detect(changelog) + assert all(entry.level == BreakingChangeLevel.INFO for entry in result.entries) + assert not result.is_breaking + + +def test_required_added_to_an_existing_property_is_breaking(): + changelog = ChangelogResult( + v1="v1", + v2="v2", + entries=[_entry("schema.orders.properties.order_id.required", ChangelogType.added, None, "True")], + ) + result = BreakingChangeDetector().detect(changelog) + assert result.is_breaking + + +def test_summary_uses_highest_detail_severity(): + changelog = ChangelogResult( + v1="v1", + v2="v2", + summary=[_entry("schema.orders", ChangelogType.updated)], + entries=[ + _entry("schema.orders.description", ChangelogType.updated, "old", "new"), + _entry("schema.orders.properties.id", ChangelogType.removed, "string"), + ], + ) + result = BreakingChangeDetector().detect(changelog) + assert result.summary[0].level == BreakingChangeLevel.ERROR + + +def test_golden_output(): + result = DataContract(data_contract_file=V1).breaking(DataContract(data_contract_file=V2)) + assert len(result.entries) == len( + DataContract(data_contract_file=V1).changelog(DataContract(data_contract_file=V2)).entries + ) + buf = io.StringIO() + con = Console(file=buf, width=300, highlight=False, no_color=True) + old_stdout = sys.stdout + sys.stdout = buf + try: + write_text_breaking_results(result, con) + finally: + sys.stdout = old_stdout + assert buf.getvalue() == GOLDEN_TEXT.read_text(encoding="utf-8"), ( + "Breaking-change text output has changed. If intentional, regenerate " + "golden_breaking_text.txt (see tests/fixtures/changelog/helper/generate_golden.py)." + ) + + +def test_data_contract_breaking_reuses_changelog(): + result = DataContract(data_contract_file=V1).breaking(DataContract(data_contract_file=V2)) + assert result.v1 == V1 + assert result.v2 == V2 + assert result.is_breaking + + +def test_cli_warning_only_change_exits_zero_but_shows_warning(): + result = runner.invoke(app, ["breaking", WARNING_ONLY_V1, WARNING_ONLY_V2]) + assert result.exit_code == 0 + assert "[ 1 Warning ] [ 2 Info ]" in result.output + + +def test_cli_missing_file_exits_nonzero(): + result = runner.invoke(app, ["breaking", "unknown.yaml", "unknown.yaml"]) + assert result.exit_code == 1 + assert result.exception is not None + assert "The file 'unknown.yaml' does not exist." in str(result.exception) + + +def test_cli_exits_nonzero_and_badges_errors_for_mixed_changes(): + result = runner.invoke(app, ["breaking", V1, V2]) + assert result.exit_code == 1 + assert "[ 3 Error ] [ 5 Info ]" in result.output + + +def test_detector_uses_first_matching_rule(): + class FirstRule(BreakingChangeRule): + rule_id = "first" + + def evaluate(self, entry): + return RuleEvaluation(self.rule_id, BreakingChangeLevel.INFO, "first") + + class SecondRule(BreakingChangeRule): + rule_id = "second" + + def evaluate(self, entry): + return RuleEvaluation(self.rule_id, BreakingChangeLevel.WARNING, "second") + + result = BreakingChangeDetector((FirstRule(), SecondRule())).detect( + ChangelogResult(v1="v1", v2="v2", entries=[_entry("field", ChangelogType.updated)]) + ) + + assert result.entries[0].rule_id == "first" + assert result.entries[0].level == BreakingChangeLevel.INFO diff --git a/tests/test_cli.py b/tests/test_cli.py index 155e3526e..4c386aa1c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -130,6 +130,18 @@ def test_changelog_with_changes(): assert "Added" in result.output +def test_breaking_help(): + result = runner.invoke(app, ["breaking", "--help"]) + assert result.exit_code == 0 + + +def test_breaking_without_changes_exits_zero(): + fixture = "fixtures/changelog/integration/changelog_integration_v1.yaml" + result = runner.invoke(app, ["breaking", fixture, fixture]) + assert result.exit_code == 0 + assert "Details" in result.output + + def test_error_message_keeps_bracketed_text(monkeypatch, capsys): """Rich markup must not eat hints like `pip install "botocore[crt]"`.""" from datacontract import cli diff --git a/update_command_docs.py b/update_command_docs.py index ac0472b85..ef72c99c2 100644 --- a/update_command_docs.py +++ b/update_command_docs.py @@ -38,6 +38,7 @@ "edit", "lint", "changelog", + "breaking", "test", "dbt", "ci", @@ -59,6 +60,8 @@ "api": ("Run as a web server", "../api.md"), "publish": ("Publish to Entropy Data", "../entropy-data.md"), "lint": ("Open Data Contract Standard", "../open-data-contract-standard.md"), + "changelog": ("Compare contract versions", "../compare-contract-versions.md"), + "breaking": ("Compare contract versions", "../compare-contract-versions.md"), "import": ("Imports", "../imports/index.md"), "export": ("Exports", "../exports/index.md"), }