Add API changelog tooling: semantic OpenAPI diff with severity ranking (#82688)

* Add API changelog tooling: semantic OpenAPI diff with severity ranking

Answering "has the API changelog been updated?" currently means reading a
10k-line JSON diff by hand, so it falls to whoever is asked in Slack. This adds
the mechanical part of that job as two mage tasks.

`./bin/mage openapi-diff OLD.json NEW.json [--severity breaking]`

resources/openapi/openapi.json is generated from the Malli endpoint schemas, so
the spec at any git ref is what the API was at that ref. The task parses two
specs into {METHOD /path -> shape} maps, inlines $refs (component schemas change
too), and compares params, request body, and response schemas.

Findings are classified BREAKING / ADDITIVE / DOC_ONLY and sorted breaking-first.
A change is breaking when it requires MORE from the caller or provides LESS to
the caller; anything else is just a change. That rule is directional and it
inverts between request and response, so severity is decided from both schemas
rather than from how a change happens to render:

  request   breaking: newly required, type/enum narrowed, schema closed, field removed
            additive: new OPTIONAL field or param, type/enum widened, made nullable
  response  breaking: field removed, field may now be null
            additive: new field returned

Adding an optional parameter is not breaking; returning extra data is not
breaking. A public diff of two Stripe specs found 663 of 679 changes additive,
which is why filtering matters more than summarizing.

`./bin/mage openapi-staleness`

The self-healing CI job runs only on PRs labelled `openapi-self-healing`, so the
committed spec drifts; it is currently ~38 API commits behind. A stale spec makes
the diff under-report, so this reports drift and names the commits rather than
letting a clean diff read as "no changes". The label gate is deliberate (#67854
introduced it to stage the rollout after #67294 removed the spec entirely over
merge conflicts), so it is left alone here.

Response coverage is partial: ~199 response entries declare a schema and can be
compared, ~1,700 are description-only 2XX/4XX/5XX stubs and cannot be.

Tests cover both directions of every request and response rule, $ref resolution
including the SCIM schemas whose property is literally named "$ref", a guard
against the failure mode in mozilla-ai/otari#1315 (breaking markers dropped at
render time), and a signal-to-noise case asserting additive bulk cannot drown
the breaking few.

* Generate specs from git refs, so the diff is not bound to the committed blob

`./bin/mage openapi-diff --refs <old-ref> <new-ref>` checks each ref out into a
throwaway worktree, runs that ref's own `generate-openapi-spec`, and diffs the
two results. Nothing is committed and no checkout is touched.

This removes the staleness problem rather than reporting it. The committed
`openapi.json` only updates on PRs labelled `openapi-self-healing`, so it lags
source badly: `--committed origin/release-x.63.x origin/master` reports 0
findings, because both refs carry the same stale blob. Generating from source
reports 670 -> 714 operations and 10 removed endpoints for the same pair.

It needs no support from the ref being diffed beyond `generate-openapi-spec`
already existing, so it works on historical refs. `--committed` keeps the fast
path and now prints a warning that its result under-reports, since an empty diff
from a stale spec is indistinguishable from an API that did not change.

Validated against the real v63 -> master delta. The top removals are the three
`GET .../execute` endpoints that the hand-written 0.64 changelog documents as
replaced by `POST .../execute/values`, and it independently surfaces a query
param rename (`pinned_state` -> `pinned-state`) that has no entry yet.

That run also shows a precision limit worth stating: it reports 239 breaking
findings, but 217 are the identical `additionalProperties: false` line from one
PR (#82447, closing mu/defn argument schemas). The classification is right --
closing a schema does require more of the caller -- but as changelog input that
is one entry, not 217. The skill now tells the drafter to collapse findings by
text and treat a repeated finding as a single systematic change.

* Group identical findings so a systematic change reads as one entry

`--grouped` collapses findings with identical text across endpoints and sorts by
blast radius, because the per-endpoint view is the wrong shape for drafting a
changelog.

On the real v63 -> master delta it turns 239 breaking findings into 113 distinct
changes. The largest spans 217 endpoints and is a single upstream PR (#82447,
closing mu/defn argument schemas): one changelog entry, reported once with its
endpoints, instead of 217 identical lines to scroll past.

Grouping is a separate render mode rather than a transformation of the default.
The per-endpoint view answers "what changed on this route?", the grouped view
answers "what distinct changes happened?", and collapsing the former into the
latter would lose the ability to check a specific endpoint.

The long tail is where the individually-interesting entries live: the same run
surfaces a `pinned_state` -> `pinned-state` query param rename that no changelog
entry covers yet.

* Fix review findings: two defects let breaking changes pass silently

Eric's review found 13 issues. The two that mattered both caused false
negatives - the tool reporting nothing where a caller would break.

The depth guard counted nesting levels, not `$ref` hops. `seen` already breaks
cycles, so the counter was doing a different job than its name claimed: past 12
levels both specs collapsed to the literal "<deep>" and compared equal, hiding
every change in 52 of 637 operations. It now advances only on a `$ref` hop, and
52 falls to 17.

`response-lines` compared only top-level properties and delegated anything deeper
to `(widening? new old)`. Swapping those arguments inverts value-set semantics -
which is why the nullable case read correctly - but not object-property
semantics, so a removed nested response field read as additive. It now recurses
the way `body-lines` does.

Chasing the second one surfaced a third of the same shape: `widening?` never
compared array `items`, so a narrowing inside an array was invisible. Found by a
test written for the SCIM `$ref`-named-property case.

Also fixed:

- `committed-spec!` built a shell command by interpolation, so a ref containing
  `$`, a quote, or a space produced a wrong file. Values now ride as positional
  arguments and the shell never parses them.
- The `--grouped` header counted severity-filtered-out changes, inflating the
  count above the body printed beneath it.
- The removed and added endpoint lists were severity-gated, so `--severity
  breaking` hid the added half of an endpoint move - at exactly the severity the
  skill tells a drafter to start with. Both lists now print at every severity.
- The staleness check watched 66 of the 158 files that contain `defendpoint`,
  missing `src/metabase/*/api/*.clj` and all of `enterprise/backend/src`, so it
  could report `OK: spec is current` with dozens of newer endpoint files on disk.
  It also compared `%cI` strings across a history carrying both `+08:00` and `Z`,
  and listed commits from a narrower pathspec than the date came from.
- `DOC_ONLY` was unreachable: a description-only change produced no findings and
  was dropped. Descriptions are what the skill reads to spot changes it cannot
  explain, so a reworded docstring now surfaces as doc-only.
- A misspelled `--severity` fell through to nil and silently printed everything.
  It now exits. `--committed` without `--refs` was accepted and ignored; it now
  exits too.
- Each `--refs` run left two multi-megabyte specs in /tmp.
- `brief` truncated its non-map branch without the "..." its map branch adds.
- The first commit message claimed a test covered the SCIM `$ref`-named property.
  No such test existed; one exists now, along with tests for the depth guard, the
  nested response removal, and the doc-only path.

One correction to that commit message stands: `babashka.json` would have avoided
the `:discouraged-namespace` ratchet entirely, since `mage/doctor.clj` already
uses it unsuppressed. Keeping cheshire for consistency with the three other mage
namespaces that use it; the budget moves 68 -> 69 against current master.

The `de-slop` skill the review could not find does exist, at
~/.claude/skills/de-slop - it is a personal skill, not in the repo.

* Fix second review: four more directions where a finding was inverted or absent

All nine findings addressed. The reviewer supplied a failing test for each of the
first five; all five reproduced on the previous head, and all now pass.

Three of the four major findings were the same mistake in different places: a
schema shape that the output path handed to `widening?` with swapped arguments,
which inverts value-set semantics but not object-property semantics.

- A response array of objects went to the leaf branch, so a removed element field
  read as additive and an added one read as breaking. 24 of the 199 comparable
  response schemas in the committed spec are top-level arrays. Arrays now recurse
  through `items` in the output direction.
- `schema-props` unwraps a nullable `oneOf`, so a nullable OBJECT reached the
  object branch, where only keys and requiredness were compared and the type
  change was invisible. The committed spec has 726 nullable object properties.
  Both `body-lines` and `response-lines` now compare type sets there, with the
  severity test pointing the correct way for each direction.
- `response-lines` discarded the required sets, so a response field the API may
  now omit produced nothing - the same loss of guarantee as one that may now be
  null, which was already breaking. 174 of 637 operations declare `required` in a
  response schema.

The fourth major finding was a shortcut in `widening?`: any empty old schema was
treated as "no constraint", so a field going from Malli `:any` to a concrete type
read as additive. `operations` stored an absent body as `{}`, which made "no body"
and "any value" indistinguishable; an absent body is now nil and `{}` falls
through to the type and enum checks. After ref resolution the committed spec has
1,182 property schemas that are `{}`.

The depth limit is gone rather than re-tuned. `seen` already terminates every
path against a finite component set, so the limit only ever replaced deep nodes
with the same literal on both sides - which compares equal and hides the change.
The MBQL clause schemas form chains long enough to hit it: 17 operations carried
`<deep>`, and now none do while 18 terminate on genuine cycles via `<recursive>`.
A self-diff of the committed spec costs 2.9s instead of 1.4s and still reports 0
findings.

Also fixed:

- `format-epoch` shelled out to `date -r`, which only BSD and macOS read as epoch
  seconds; on GNU coreutils it means "the mtime of this file" and printed nothing,
  silently, because the call was `:quiet?`. Now `java.time`.
- `openapi-staleness` claimed the spec was "current" from endpoint files alone.
  661 of 763 component schemas live outside `.api` namespaces, so a shared-schema
  change moves the generated spec invisibly. The OK message now says what it
  actually checked and points at `--refs`.
- A `--refs` run that could neither generate nor read the second ref leaked the
  first spec: materialization happened in the `let` bindings, outside the `try`
  whose `finally` removes the directory.
- The skill told an agent that two identical committed specs prove the API did not
  change, and that the commit touching `openapi.json` is the PR that changed the
  API. Both are wrong for the same reason - the committed spec only moves on
  `openapi-self-healing` PRs. It now says to re-run without `--committed` before
  reporting no changes, and to search endpoint source for attribution.

* Drop the ratchet bump: babashka.json instead of cheshire, and fix stale docs

The `:discouraged-namespace` increase was avoidable and the rationale I gave for
it was backwards. `babashka.json` is not in the discouraged list, and 6 mage
namespaces already use it unsuppressed against 5 on cheshire - so the
"consistency" argument pointed the other way. Swapping to it removes both the
68 -> 69 bump and the `:clj-kondo/ignore`, and this PR no longer touches
.clj-kondo/ratchets.edn at all. `read-str` keywordizes by default, so the two
spec reads pass `{:key-fn identity}`; this namespace is string-key throughout.
Verified unchanged behavior: self-diff still 0 findings, the real 0.64 change
still 1 breaking.

Three docstring counts were wrong. The `api-source-pathspecs` docstring claimed
238 `/api/ee/` paths (actual 106 - it looks like 106 paths plus 132 operations,
double-counted), 66 files matching `src/metabase/**/api.clj` (actual 65), and 158
files containing `defendpoint` (actual 160). The argument the docstring makes is
unaffected - the narrow pathspec still misses most endpoint files - but the
numbers are now the measured ones.

Also removed documentation that described code this branch already deleted:

- A comment in `resolve-refs` said "Only a $ref hop advances depth" while the
  docstring 17 lines above correctly says there is no depth limit. There is no
  depth counter left; the comment described the previous commit's version.
- Two test comments and a test name referenced a `<deep>` placeholder guard that
  no longer exists, and cited a "52 of 637" measurement of the mechanism that was
  removed. The tests themselves are valid - they assert deep and long-chain
  changes are detected - so only the prose changed.
- SKILL.md quoted "`--committed` reports 0 findings" for v63 vs master as a fixed
  figure. That was true when written and is not now: the blobs have since
  diverged and it reports 1. The underlying point stands, so it is stated
  qualitatively rather than as a number that drifts.
- SKILL.md said `--refs` "works on any ref". The code only attempts that and has
  a documented fallback for refs that will not build with the current toolchain;
  the text now says so.

* Fix third review: object unions and nullable arrays hid breaking changes

- A union of two or more object variants was compared by its first
  variant only, so removing or editing any other variant produced no
  finding. Such unions now compare whole: widening only when every old
  variant survives unchanged.
- Nullable arrays (`oneOf [array, null]`) skipped the array branch, so a
  change to their items was ranked additive. Both array branches now
  unwrap the null first.
- The render test never rendered. It now asserts breaking operations
  print under the breaking headings of both renderers, and a new test
  covers the --severity filter.

* Rank changes to unmodeled schema keywords breaking, and compare bounds, allOf, and tuples

The old widening check returned "safe" for any keyword it did not
recognize, so each review round found another way for a breaking change
to read as additive: allOf, minimum/minLength, map-of value schemas,
prefixItems, nullable enums, closed nullable objects.

`compatible?` replaces it. It takes a direction (:request or :response),
compares unions variant by variant, compares bounds, pattern, and format,
and compares allOf and prefixItems by position. A change to any keyword
it does not model ranks breaking. A false breaking finding costs one
look; a missed one ships undocumented.

Documentation keys (description, title, examples, default) are stripped
before comparison, so a reworded field description is doc-only instead
of an additive schema change.

Also:
- When two summaries are identical, the finding names where the schemas
  differ (e.g. `items.enum: +"measure"`), so distinct changes no longer
  merge in the grouped view.
- Generation deletes the checked-out spec before running, so a generator
  that logs and swallows an error no longer passes the committed spec off
  as fresh. A timeout falls back to the committed spec instead of
  throwing.
- An empty or non-spec input file exits 1 instead of reporting 0 BREAKING.
- openapi-staleness lists commits after the spec commit, not including it.

* Close the gaps an independent check found in the compatible? rewrite

- Only a nullable union unwraps to its object. `string | object` and
  similar unions are compared whole, so a change to the non-object
  variant is no longer hidden.
- Keywords beside an object's properties (allOf, a map-of
  additionalProperties, minProperties) are compared.
- If the field walk finds nothing but the stripped schemas differ, the
  whole schema is ranked instead of falling through to doc-only.
- Inclusive and exclusive numeric bounds compare as one bound.
- A response allOf may gain members (narrowing output); a request allOf
  may lose them.
- An enum or const without a type widening to its type is additive.
- requestBody.required and request content types are compared, and a
  response that declares a schema for the first time is reported as
  additive.
- A missing input file says so.

* Match operations across path-variable renames; count only visible severities in the grouped header

- Operations and path params are keyed by position, not variable name.
  Renaming `{id}` to `{card-id}` leaves the URL a caller sends unchanged,
  so it is now a doc-only "path variable renamed" finding rather than a
  removed endpoint plus an added one. Routes that would merge under
  position-keying (`/api/database/{id}/schemas` and
  `/api/database/{virtual-db}/schemas`) keep their names. Reports still
  print the real spec paths.
- At --severity breaking, the grouped header no longer counts the
  new-endpoint list, which is additive. The list still prints.
- The skill's attribution section no longer opens with the
  openapi.json git log that it then says names the wrong author.

* Rank a changed or dropped request default as breaking

`default` was stripped with the documentation keys, so a default that
flipped (say a query param from false to true) ranked doc-only and
--severity breaking hid it. The schema still validates the same
requests, but a caller who omits the field now gets different
behavior.

Request defaults are now compared: changed or dropped is breaking,
added where there was none is additive. Response defaults are ignored.

* State one definition of breaking that covers every rule

A change is breaking when an existing caller, sending exactly what it
sent before, can now fail or get a different result. The old wording
(requires more / provides less) did not cover changed defaults, and it
presented two conservative rules - removed request fields and changes
to unmodeled keywords - as if they followed from it. Those are now
stated as what the tool does when it cannot tell.

* Skill: point to docs-write, and attribute with all six API source pathspecs

de-slop is a personal skill the repo does not ship; docs-write carries
the Metabase style guide. The attribution git log used three of the six
pathspecs in api-source-pathspecs and missed enterprise api/ dirs and
routes.clj files (35 tracked files, e.g. sso/api/saml.clj).
This commit is contained in:
bryan
2026-10-05 15:33:14 -06:00
committed by GitHub
parent 515ab1961f
commit 6cb8e9e904
5 changed files with 1703 additions and 0 deletions

View File

@@ -0,0 +1,184 @@
---
name: api-changelog
description: Draft or audit the REST API changelog (docs/developers-guide/api-changelog.md) by semantically diffing the checked-in OpenAPI spec between two git refs. Use when asked "has the API changelog been updated", "what are the breaking API changes in vNN", when cutting a release branch, or when reviewing whether a PR needs a changelog entry.
---
# API changelog
Answers "what changed in the REST API between two versions, and is the changelog
honest about it?"
## Ground truth
`resources/openapi/openapi.json` is generated from the Malli endpoint schemas, so
the spec at any git ref is what the API actually was at that ref. Diff the spec,
do not read endpoint source or boot a server.
Response coverage is partial: about 199 response entries declare a real schema
and are compared, but roughly 1,700 are description-only `2XX/4XX/5XX` stubs. For
an endpoint with no declared response schema, a response-shape change is
invisible here. Say so rather than implying the diff is complete.
### Check the spec is fresh first - it usually is not
The self-healing CI job (`.github/workflows/openapi-check.yml`) only runs on PRs
labelled `openapi-self-healing`, so the committed spec drifts behind master by
default. Verify before trusting a diff:
```bash
./bin/mage openapi-staleness
```
If the source is newer, regenerate before diffing the new ref:
```bash
bun run generate-openapi # rewrites resources/openapi/openapi.json in place
```
This staleness check only matters when you are diffing the committed spec.
`--refs` (below) generates each ref's spec from its own source, so it is not
subject to this drift at all - prefer it.
When you do read a committed spec, a change present in source but absent from
that spec is a **false negative**: the diff will not report it, and an empty
result is indistinguishable from an API that did not change. Confirm suspected
gaps with `grep -rn "defendpoint" src/.../api.clj`.
## Steps
1. Pick the refs. Default old ref = the previous release branch
(`origin/release-x.63.x`), new ref = the one being released
(`origin/release-x.64.x`) or `origin/master`. Confirm with the user if
ambiguous. `git fetch origin` first.
2. Extract and diff:
```bash
# Preferred: generate both specs from source. ~2 min (two JVM boots), no drift.
./bin/mage openapi-diff --refs origin/release-x.63.x origin/release-x.64.x --severity breaking
# Fast, but reads the committed spec, which lags source. Warns when it does.
./bin/mage openapi-diff --refs --committed origin/release-x.63.x origin/release-x.64.x
# Two spec files directly.
./bin/mage openapi-diff /tmp/old.json /tmp/new.json --severity breaking
```
`--refs` checks each ref out into a throwaway worktree and runs that ref's own
`generate-openapi-spec`, so it is not limited to refs that carry a committed spec
and never touches your checkout. Very old refs may not build with the current
toolchain; it falls back to the committed spec and says so when that happens.
Prefer it. `--committed` compares whatever blob each ref happens to carry, and
when a release branch and master share a stale one it reports far fewer findings
than the API actually changed - an artifact of the spec, not of the API.
### Start from the grouped view
A single upstream change can produce hundreds of findings. `--grouped` collapses
identical findings and sorts by blast radius, which is the shape you draft from:
```bash
./bin/mage openapi-diff --refs <old> <new> --severity breaking --grouped
```
On the real v63 -> master delta that turns 239 breaking findings into 113
distinct changes. The top entry spans 217 endpoints and is one PR (#82447,
closing `mu/defn` argument schemas) - **one changelog entry, not 217**.
Drop `--grouped` when you need the per-endpoint view to check a specific route.
A change spanning many endpoints is usually one upstream PR: write it once, name
the cause, and say which endpoints it spans. The long tail of single-endpoint
changes is where the individually-interesting entries are - the v63 -> master run
surfaced a `pinned_state` -> `pinned-state` query param rename there.
Findings are classified and sorted breaking-first.
**A change is breaking when an existing caller, sending exactly what it sent
before, can now fail or get a different result.** That covers three cases: the
API requires more of the request, provides less in the response, or behaves
differently for the same request. The first two invert between request and
response:
| | Breaking | Not breaking |
|---|---|---|
| **Request** | field or param becomes required; type, enum, or bound (`minimum`, `minLength`, `pattern`, ...) narrowed; `additionalProperties: false` added; default changed or dropped | new *optional* field or param; type, enum, or bound widened; field made nullable; schema opened; default added |
| **Response** | field removed or no longer always returned; field may now be `null`; new enum value | new field returned; field that was nullable never is |
| **Endpoint** | removed | added |
When the tool cannot tell, it ranks the change breaking: a false alarm costs
you one look, a miss ships undocumented. Two rules follow from that, not from
the definition, so check them before drafting an entry:
- **A removed request field** is ranked breaking because the spec does not say
whether the server rejects or acts on keys it does not declare. If the
endpoint ignores unknown keys, the removal is not breaking.
- **A change to a schema keyword the tool does not model** is ranked breaking
so that it is never hidden. Read the finding to decide.
A reworded description, title, or example is DOC_ONLY.
Adding an optional parameter is not breaking. Returning extra data is not
breaking. Existing callers keep working in both cases.
Start from `--severity breaking`. The long tail of API change is additive
(a public Stripe-spec diff found 663 of 679 changes additive); reading the
full diff to find the breaking few is how entries get missed.
**Identical or near-identical committed specs do not mean the API did not
change.** Two refs often carry the same stale blob, so `--committed` can report
zero findings for a pair that generating from source shows has removed endpoints.
Before reporting "no API surface changes", re-run without `--committed`. Only a
clean generated diff supports that conclusion.
3. Review the classification. The tool decides severity structurally, but two
cases still need your judgement:
- **A removed + added pair is often one endpoint moving** (method change, or a
param moving between query/path/body). The tool reports two findings; read
the docstrings to pair them and write ONE changelog entry describing the move.
- **An `ADDITIVE` finding can still be worth an entry** when it is a new
endpoint clients should know about. Additive means "will not break existing
callers", not "not worth mentioning".
4. For every breaking change, decide whether you can state **what it does for a
client** from the spec's `description` alone. If you cannot, do not guess and
do not paper over it - list it under "Needs author input" with the endpoint
name and what is unclear. Thin endpoint docstrings are the actual finding;
report them loudly.
5. Cross-check `docs/developers-guide/api-changelog.md` for the target version.
Report three buckets:
- **Undocumented**: breaking changes with no changelog entry -> draft entries
- **Stale**: changelog entries with no matching spec change -> verify by hand
(may be a response-shape or behavior change the spec cannot see - do not
delete without asking)
- **Covered**: matched, no action
6. Draft entries in the existing house style: `## Metabase 0.64.0` heading,
one `-` bullet per change, endpoint as `` `POST /api/foo/:id` `` with
**`:id` colon-style params, not OpenAPI `{id}` braces**. Say what changed, what
clients must do, and whether there was a deprecation period. Apply the
`docs-write` skill. Show the diff before writing to the file.
## Attribution
Find who to ask about an unclear change. Search the endpoint SOURCE, not `openapi.json`: the committed spec only changes on
PRs carrying the `openapi-self-healing` label, which is usually a later, unrelated
PR, so its history names the wrong author.
```bash
git log <old-ref>..<new-ref> --oneline -- \
'src/metabase/**/api.clj' 'src/metabase/**/api/*.clj' 'src/metabase/**/routes.clj' \
'enterprise/backend/src/**/api.clj' 'enterprise/backend/src/**/api/*.clj' 'enterprise/backend/src/**/routes.clj'
```
Attribution by route search is unreliable in both directions - `git log -S` reports
where a string's count *changed*, which for a long-lived route is its creation years
ago rather than its recent removal. Treat any commit you find as a candidate to
verify, never as the established cause.
## Backports
A change landing in both `0.64.0` and older lines gets a full entry under the
newest version and a one-line pointer under each backport version
("See the 0.64.0 entry."). Match the existing `POST /api/slack/bug-report` pattern.

24
bb.edn
View File

@@ -253,6 +253,30 @@
:requires [[mage.quick-test-runner :as qtr]] :requires [[mage.quick-test-runner :as qtr]]
:task (task! (qtr/go parsed))} :task (task! (qtr/go parsed))}
openapi-diff
{:doc "Semantic diff of two OpenAPI specs, classified breaking-first, for the API changelog"
:examples [["./bin/mage openapi-diff old.json new.json" "Full diff, breaking changes first"]
["./bin/mage openapi-diff old.json new.json --severity breaking" "Only what needs a changelog entry"]
["./bin/mage openapi-diff --refs release-x.63.x release-x.64.x --severity breaking" "Generate both specs from source, then diff"]
["./bin/mage openapi-diff --refs --committed release-x.63.x master" "Use each ref's committed spec (fast, may be stale)"]
["./bin/mage openapi-diff --refs --grouped release-x.63.x master --severity breaking" "One line per distinct change, widest blast radius first"]]
:arg-schema [:tuple
[:string {:name "old" :description "Path to the older openapi.json, or a git ref with --refs"}]
[:string {:name "new" :description "Path to the newer openapi.json, or a git ref with --refs"}]]
:options [["-s" "--severity LEVEL" "Only show findings at or above LEVEL: breaking or additive"]
["-r" "--refs" "Treat the arguments as git refs and generate each spec from its source"]
["-c" "--committed" "With --refs, read each ref's committed spec instead of generating (fast, may be stale)"]
["-g" "--grouped" "Collapse identical findings across endpoints: one line per distinct change"]]
:requires [[mage.openapi-diff]]
:task (task! (mage.openapi-diff/cli-diff parsed))}
openapi-staleness
{:doc "Is the committed OpenAPI spec current relative to the API source? (a stale spec under-reports)"
:examples [["./bin/mage openapi-staleness"]]
:arg-schema [:tuple]
:requires [[mage.openapi-diff]]
:task (task! (mage.openapi-diff/cli-staleness parsed))}
affected-modules affected-modules
{:doc "Which backend modules have changes?" {:doc "Which backend modules have changes?"
:examples [["./bin/mage affected-modules master" "prints changes compared to `master`"] :examples [["./bin/mage affected-modules master" "prints changes compared to `master`"]

View File

@@ -0,0 +1,913 @@
(ns mage.openapi-diff
"Semantic diff of two OpenAPI specs, for drafting the REST API changelog.
`resources/openapi/openapi.json` is generated from the Malli endpoint schemas, so the spec at any
git ref is what the API was at that ref. Diffing two specs answers \"what changed for a client?\"
without reading endpoint source or booting a server.
A change is BREAKING when an existing caller, sending exactly what it sent before, can now fail
or get a different result. That covers three cases: the API requires more of the request, provides
less in the response, or behaves differently for the same request. The first two invert between
request and response, so severity is decided from both schemas rather than from how a change
happens to render:
request breaking: newly required, type/enum/bound narrowed, schema closed,
default changed or dropped (same request, different behavior)
additive: new OPTIONAL field or param, type/enum/bound widened, made nullable,
default added
response breaking: field removed or no longer always returned, may now be null, new enum value
additive: new field returned, values narrowed
When the comparison cannot tell, it ranks the change breaking. A false alarm costs a reviewer
one look; a miss ships undocumented. Two rules follow from that, not from the definition:
- a removed request field is breaking, because the spec does not say whether the server rejects
or acts on keys it does not declare
- a change to a schema keyword the comparison does not model is breaking
Descriptions, titles, and examples are compared separately and rank doc-only.
Response coverage is partial: only endpoints that declare a response schema can be compared. Most
emit description-only 2XX/4XX/5XX stubs."
(:require
;; mage runs under babashka; metabase.util.json isn't on its classpath
[babashka.json :as json]
[clojure.java.io :as io]
[clojure.set :as set]
[clojure.string :as str]
[mage.shell :as shell]
[mage.util :as u]))
(set! *warn-on-reflection* true)
(defn- resolve-refs
"Inline `$ref`s in `node` so two specs compare structurally rather than by ref name. Component
schemas change too, so a ref that looks identical can point at a different shape.
There is no depth limit: `seen` holds every ref on the current path, so a repeat becomes
`<recursive ...>` and every path terminates against a finite component set. A hop limit would
instead replace deep nodes with the same literal on both sides, which compares equal and hides
the change - the MBQL clause schemas form chains long enough to hit it."
([node spec] (resolve-refs node spec #{}))
([node spec seen]
(cond
(map? node)
;; A $ref node holds a STRING pointer. SCIM schemas have a property literally named "$ref"
;; (`{"properties": {"$ref": {...}}}`), which is data, not a reference - hence the string check.
(if-let [ref (let [r (get node "$ref")] (when (string? r) r))]
(if (contains? seen ref)
(str "<recursive " ref ">")
(let [path (str/split (subs ref 2) #"/")
target (get-in spec path {})
rest' (dissoc node "$ref")
merged (if (seq rest') (merge target rest') target)]
(resolve-refs merged spec (conj seen ref))))
(update-vals node #(resolve-refs % spec seen)))
(sequential? node) (mapv #(resolve-refs % spec seen) node)
:else node)))
(defn- non-null
"The single real variant of a nullable union such as `oneOf [<array> {type: null}]`, else `node`."
[node]
(let [variants (when (map? node) (concat (get node "oneOf") (get node "anyOf")))
others (remove #(= {"type" "null"} %) variants)]
(if (and (= 1 (count others)) (< 1 (count variants))) (first others) node)))
(defn- schema-props
"`[properties required-set]` for an object schema, else `[nil #{}]`. Unwraps the nullable
`oneOf [<object> {type: null}]` pattern, and only that: any other union, such as
`string | object` or two object variants, has no single property set, so callers compare it
whole. Taking one variant's properties would hide every change to the others."
[node]
(let [n (non-null node)]
(if-let [props (and (map? n) (get n "properties"))]
[props (set (get n "required"))]
[nil #{}])))
(defn- type-set
"Set of JSON types a schema accepts, or nil when unconstrained."
[node]
(when (and (map? node) (seq node))
(if-let [variants (seq (concat (get node "oneOf") (get node "anyOf")))]
(let [parts (map type-set variants)]
(when-not (some nil? parts)
(reduce set/union #{} parts)))
(when-let [t (get node "type")]
(if (string? t) #{t} (set t))))))
(defn- enum-set
"Set of literal values a schema accepts, or nil when it is not an enum."
[node]
(when (map? node)
(cond
(contains? node "const") #{(json/write-str (get node "const"))}
(sequential? (get node "enum")) (set (map json/write-str (get node "enum")))
:else nil)))
(def ^:private doc-keys
"Schema keys that document a value without constraining it."
["description" "title" "example" "examples" "deprecated"])
(defn- strip-docs
"`node` without [[doc-keys]] at any depth, so a reworded description is not a schema change.
Property NAMES are data, so a field literally called `description` survives, and literal values
under `enum`/`const`/`default` are not walked."
[node]
(cond
(map? node) (into {} (for [[k v] (apply dissoc node doc-keys)]
[k (cond
(and (= "properties" k) (map? v)) (update-vals v strip-docs)
(#{"enum" "const" "default"} k) v
:else (strip-docs v))]))
(sequential? node) (mapv strip-docs node)
:else node))
(def ^:private lower-bounds ["minLength" "minItems"])
(def ^:private upper-bounds ["maxLength" "maxItems"])
(def ^:private modeled-keys
"Keywords [[compatible?]] compares. A change to any other keyword is ranked incompatible."
(into #{"type" "enum" "const" "default" "oneOf" "anyOf" "allOf" "prefixItems" "items" "properties"
"required" "additionalProperties" "pattern" "format" "uniqueItems"}
(concat lower-bounds upper-bounds ["minimum" "exclusiveMinimum" "maximum" "exclusiveMaximum"])))
(defn- union? [node]
(and (map? node) (seq (concat (get node "oneOf") (get node "anyOf")))))
(defn- variants
"A union's variants, or the schema itself as its only variant."
[node]
(if (union? node) (concat (get node "oneOf") (get node "anyOf")) [node]))
(defn- numeric-bound
"A schema's effective numeric bound as `[value strictness]`, comparable with `compare`, or nil.
`inclusive`/`exclusive` name the keywords; exclusive is stricter at the same value, so it sorts
inward: above an inclusive lower bound, below an inclusive upper one."
[s inclusive exclusive inward]
(let [i (get s inclusive), x (get s exclusive)
candidates (cond-> []
(number? i) (conj [i 0])
(number? x) (conj [x inward]))]
(when (seq candidates)
(reduce #(if (pos? (* inward (compare %2 %1))) %2 %1) (first candidates) (rest candidates)))))
(defn- literal-types
"JSON types of an enum's or const's values, or nil when the schema is neither."
[node]
(when-let [vs (cond (contains? node "const") [(get node "const")]
(sequential? (get node "enum")) (get node "enum"))]
(set (map #(cond (string? %) "string" (integer? %) "integer" (number? %) "number"
(boolean? %) "boolean" (nil? %) "null" (map? %) "object" :else "array")
vs))))
(defn- accepts-all-values?
"True when the type, enum, bound, and pattern constraints of `wide` admit every value that
`narrow`'s admit. Object and array structure is [[compatible?]]'s job."
[wide narrow]
(let [tw (type-set wide), tn (or (type-set narrow) (literal-types narrow))
ew (enum-set wide), en (enum-set narrow)
bound-ok? (fn [k cmp] (let [w (get wide k), n (get narrow k)]
(or (not (number? w)) (and (number? n) (cmp n w)))))
numeric-ok? (fn [inclusive exclusive inward]
(let [w (numeric-bound wide inclusive exclusive inward)
n (numeric-bound narrow inclusive exclusive inward)]
(or (nil? w) (and n (not (neg? (* inward (compare n w))))))))]
(boolean
(and (or (nil? tw) (and tn (set/subset? tn (cond-> tw (contains? tw "number") (conj "integer")))))
(or (nil? ew) (and en (set/subset? en ew)))
(every? #(bound-ok? % >=) lower-bounds)
(every? #(bound-ok? % <=) upper-bounds)
(numeric-ok? "minimum" "exclusiveMinimum" 1)
(numeric-ok? "maximum" "exclusiveMaximum" -1)
;; Two regexes cannot be compared, so any change to a pattern or format narrows.
(every? #(or (not (contains? wide %)) (= (get wide %) (get narrow %))) ["pattern" "format"])
(or (not (true? (get wide "uniqueItems"))) (true? (get narrow "uniqueItems")))))))
(defn- compatible?
"True when changing a schema from `old` to `new` keeps existing callers working.
`dir` is `:request` or `:response`. Value constraints (type, enum, bounds) invert between the
two: a request must still ACCEPT every value it did, a response must only RETURN values it could.
Object fields do not invert: in both directions a removed field breaks, and so does a newly
required request field or a response field that is no longer always returned.
A keyword this function does not model ranks as incompatible when it changes. A false breaking
finding costs a reviewer one look; a missed one ships undocumented."
[dir old new]
(let [req? (= dir :request)
same? #(compatible? dir %1 %2)]
(cond
(= old new) true
;; An ABSENT old schema constrained nothing, so anything that does not newly demand something
;; is safe. `{}` is not absent - it accepts any value, and narrowing it requires more.
(nil? old)
(or (not req?)
(not (or (seq (get new "required")) (false? (get new "additionalProperties")))))
(not (and (map? old) (map? new))) false
;; Unions compare variant by variant: a request must still accept every old variant, and a
;; response may only return variants it already could.
(or (union? old) (union? new))
(and (if req?
(every? (fn [o] (some #(same? o %) (variants new))) (variants old))
(every? (fn [n] (some #(same? % n) (variants old))) (variants new)))
(or (not (and (union? old) (union? new)))
(= (dissoc old "oneOf" "anyOf") (dissoc new "oneOf" "anyOf"))))
:else
(let [[wide narrow] (if req? [new old] [old new])
;; Malli `:fn` predicates emit `{}` into an `allOf`. A member that constrains nothing
;; must not shift the positions of the members that do.
members (fn [s k] (cond->> (get s k)
(= k "allOf") (remove #(contains? #{{} {"allOf" []}} %))))
;; Members pair by position. `allOf` members are conjuncts, so the narrower side may carry
;; extra trailing ones: the old side for a request, the new side for a response.
positional? (fn [k] (let [o (members old k), n (members new k)]
(or (= o n)
(and (if (= k "allOf")
(if req? (<= (count n) (count o)) (<= (count o) (count n)))
(= (count o) (count n)))
(every? true? (map same? o n))))))
open-values (fn [s] (let [a (get s "additionalProperties")] (if (map? a) a {})))
closed? (fn [s] (false? (get s "additionalProperties")))
old-props (get old "properties" {}), new-props (get new "properties" {})
old-req (set (get old "required")), new-req (set (get new "required"))]
(boolean
(and (accepts-all-values? wide narrow)
;; A caller who omits a request field gets its default, so changing or dropping one
;; changes behavior without changing what validates. Adding one where there was none
;; does not: that field was already optional. Response defaults describe nothing.
(or (not req?) (not (contains? old "default")) (= (get old "default") (get new "default")))
(positional? "allOf")
(positional? "prefixItems")
(same? (get old "items" {}) (get new "items" {}))
;; A map-of value schema: `additionalProperties` holding a schema.
(or (closed? old) (closed? new) (same? (open-values old) (open-values new)))
(not (and req? (closed? new) (not (closed? old))))
(empty? (set/difference (set (keys old-props)) (set (keys new-props))))
(empty? (if req? (set/difference new-req old-req) (set/difference old-req new-req)))
(every? #(same? (get old-props %) (get new-props %))
(set/intersection (set (keys old-props)) (set (keys new-props))))
(= (apply dissoc old modeled-keys) (apply dissoc new modeled-keys))))))))
(defn- operations
"`{\"POST /api/card\" {:params .. :body .. :responses .. :description .. :docs ..}}` for every
operation. Schemas are ref-resolved and stripped of [[doc-keys]]; `:docs` keeps the whole
resolved operation, so a documentation-only change can still be reported."
[spec]
(let [entries
(for [[path methods] (get spec "paths")
[method raw-op] methods
:when (map? raw-op)
:let [op (resolve-refs raw-op spec)
;; A caller never sends a path variable's NAME, so renaming `{id}` to
;; `{card-id}` is not a change. Key operations and path params by position.
slot (zipmap (map second (re-seq #"\{([^}]+)\}" path)) (range))]]
[(str (str/upper-case method) " " (str/replace path #"\{[^}]+\}" "{}"))
{:display (str (str/upper-case method) " " path)
:params (into {}
(for [p (get op "parameters")]
[(if (= "path" (get p "in"))
(str "path:" (slot (get p "name")))
(str (get p "in") ":" (get p "name")))
{:label (str (get p "in") ":" (get p "name"))
:required (boolean (get p "required"))
:schema (strip-docs (get p "schema" {}))}]))
;; nil when there is no body at all. An empty MAP means "any value" (Malli `:any`) and
;; must still be compared: narrowing it to a type requires more of the caller.
:body-required (boolean (get-in op ["requestBody" "required"]))
:body-types (set (keys (get-in op ["requestBody" "content"])))
:body (let [content (get-in op ["requestBody" "content"])]
(some-> (or (get-in content ["application/json" "schema"])
(some #(get % "schema") (vals content)))
strip-docs))
:responses (into {}
(for [[code resp] (get op "responses")
:when (map? resp)
:let [schema (get-in resp ["content" "application/json" "schema"])]
:when schema]
[code (strip-docs schema)]))
:description (str/trim (or (get op "description") ""))
:docs op}])
;; `/api/database/{id}/schemas` and `/api/database/{virtual-db}/schemas` are distinct routes,
;; told apart by parameter patterns. Where position-keying would merge two, keep the names.
collides? (set (for [[k n] (frequencies (map first entries)) :when (> n 1)] k))]
(into {} (for [[k v] entries] [(if (collides? k) (:display v) k) v]))))
(defn- brief
"One-line schema summary: type/enum/const rather than a wall of JSON."
([value] (brief value 200))
([value limit]
(if (map? value)
(let [truncate #(cond-> % (> (count %) limit) (-> (subs 0 limit) (str "...")))]
(cond
(contains? value "const") (str "const=" (truncate (json/write-str (get value "const"))))
(contains? value "enum") (str "enum=" (truncate (json/write-str (get value "enum"))))
(seq (concat (get value "oneOf") (get value "anyOf")))
(str/join " | " (map #(brief % 60) (take 4 (concat (get value "oneOf") (get value "anyOf")))))
(and (= "object" (get value "type")) (map? (get value "properties")))
(str "object{" (truncate (str/join "," (sort (keys (get value "properties"))))) "}")
(= "array" (get value "type")) (str "array<" (brief (get value "items" {}) 60) ">")
(get value "type") (str (get value "type"))
:else (truncate (json/write-str value))))
(let [s (json/write-str value)]
(cond-> s (> (count s) limit) (-> (subs 0 limit) (str "...")))))))
(defn- deltas
"Paths at which `o` and `n` differ. Lists of scalars, such as enums, show added/removed values."
[path o n]
(let [show #(if (nil? %) "(none)" (brief % 40))]
(cond
(= o n) []
(and (map? o) (map? n))
(mapcat #(deltas (conj path %) (get o %) (get n %)) (sort (set/union (set (keys o)) (set (keys n)))))
(and (sequential? o) (sequential? n) (= (count o) (count n)))
(mapcat #(deltas (conj path %1) %2 %3) (range) o n)
:else
[(str (if (seq path) (str/join "." path) "value") ": "
(if (and (sequential? o) (sequential? n) (not-any? coll? (concat o n)))
(str/join " " (concat (map #(str "+" (json/write-str %)) (remove (set o) n))
(map #(str "-" (json/write-str %)) (remove (set n) o))))
(str (show o) " -> " (show n))))])))
(defn- brief-change
"`old -> new` summary. When the one-line briefs match, names where the schemas differ instead, so
a change neither reads as `X -> X` nor merges with a different change in the grouped view."
[old new]
(let [o (brief old), n (brief new)]
(if (not= o n)
(str o " -> " n)
(let [ds (deltas [] old new)]
(str o " (" (str/join "; " (take 3 ds)) (when (> (count ds) 3) "; ...") ")")))))
(def ^:private breaking :breaking)
(def ^:private additive :additive)
(def ^:private doc-only :doc-only)
(def ^:private severity-order {breaking 0, additive 1, doc-only 2})
(defn- worst
"Most severe of `severities`, for ranking an endpoint by its findings."
[severities]
(or (first (sort-by severity-order severities)) doc-only))
(defn- object-shell
"An object schema without its properties, and without the null of a nullable union: the
keywords (`allOf`, a map-of `additionalProperties`, `minProperties`, ...) that the property walk
does not see. A boolean `additionalProperties` is reported separately."
[node]
(let [n (non-null node)]
(cond-> (dissoc n "properties" "required")
(boolean? (get n "additionalProperties")) (dissoc "additionalProperties"))))
(defn- shell-lines
"A finding for the keywords beside an object's properties, when they changed."
[dir pad label old-schema new-schema]
(let [o (object-shell old-schema), n (object-shell new-schema)]
(when (not= o n)
[[(if (compatible? dir o n) additive breaking)
(str pad "~ " label ": " (brief-change o n))]])))
(defn- body-lines
"Recursively compare request-body schemas, reporting leaf-level changes as `[severity text]`."
([label old-schema new-schema] (body-lines label old-schema new-schema 0))
([label old-schema new-schema depth]
(let [pad (str " " (str/join (repeat depth " ")))
[old-props old-req] (schema-props old-schema)
[new-props new-req] (schema-props new-schema)]
(cond
(> depth 4)
[[(if (compatible? :request old-schema new-schema) additive breaking)
(str pad "~ " label ": " (brief-change old-schema new-schema))]]
(or (nil? old-props) (nil? new-props))
(let [closed? (and (false? (get new-schema "additionalProperties"))
(not (false? (get old-schema "additionalProperties"))))]
(cond-> [[(if (compatible? :request old-schema new-schema) additive breaking)
(str pad "~ " label ": " (brief-change old-schema new-schema))]]
closed? (conj [breaking (str pad "! " label " now rejects undeclared keys (additionalProperties: false)")])))
:else
(let [old-keys (set (keys old-props))
new-keys (set (keys new-props))
old-types (type-set old-schema)
new-types (type-set new-schema)]
(concat
;; `schema-props` unwraps a nullable `oneOf`, so both sides reach this branch and the
;; property comparison below never sees the type change. A field that no longer accepts
;; null requires more of the caller.
(when (not= old-types new-types)
[[(if (or (nil? new-types) (and old-types (set/subset? old-types new-types)))
additive breaking)
(str pad "~ " label " type: " (sort old-types) " -> " (sort new-types))]])
(shell-lines :request pad label old-schema new-schema)
(when (and (false? (get (non-null new-schema) "additionalProperties"))
(not (false? (get (non-null old-schema) "additionalProperties"))))
[[breaking (str pad "! " label " now rejects undeclared keys (additionalProperties: false)")]])
;; New fields: required ones demand more of the caller, optional ones do not.
(for [k (sort (set/difference new-keys old-keys))]
(if (contains? new-req k)
[breaking (str pad "+ " label "." k " (REQUIRED - breaking): " (brief (get new-props k)))]
[additive (str pad "+ " label "." k ": " (brief (get new-props k)))]))
(for [k (sort (set/difference old-keys new-keys))]
[breaking (str pad "- " label "." k " REMOVED (was "
(if (contains? old-req k) "required" "optional") "): " (brief (get old-props k)))])
(mapcat (fn [k]
(let [o (get old-props k), n (get new-props k)]
(concat
(when-not (= o n) (body-lines (str label "." k) o n (inc depth)))
(cond
(and (contains? new-req k) (not (contains? old-req k)))
[[breaking (str pad "! " label "." k " is now REQUIRED (breaking)")]]
(and (contains? old-req k) (not (contains? new-req k)))
[[additive (str pad "! " label "." k " is no longer required")]]))))
(sort (set/intersection old-keys new-keys)))))))))
(defn- response-lines
"Compare one response schema, recursively. The rule inverts for output: a caller breaks when the
API PROVIDES LESS. Returning extra data is additive - clients ignore unknown fields.
Recurses through objects and arrays so each finding names the field it is about; leaves are
ranked by [[compatible?]] in the `:response` direction."
([code old-schema new-schema] (response-lines code old-schema new-schema 0))
([code old-schema new-schema depth]
(let [label (str "response " code)
pad (str " " (str/join (repeat depth " ")))
[old-props old-req] (schema-props old-schema)
[new-props new-req] (schema-props new-schema)]
(cond
(> depth 6)
[[(if (compatible? :response old-schema new-schema) additive breaking)
(str pad "~ " label ": " (brief-change old-schema new-schema))]]
;; An array's element schema carries the output contract, so it recurses in the output
;; direction. Falling through to the leaf branch would compare items as INPUT and invert
;; every finding inside a response array. Nullable arrays unwrap, but only when nullability is
;; unchanged: otherwise the leaf branch must see, and report, the null.
(let [o (non-null old-schema), n (non-null new-schema)]
(and (= "array" (get o "type")) (= "array" (get n "type"))
(= (type-set old-schema) (type-set new-schema))
(not= (get o "items") (get n "items"))))
(response-lines (str code "[]") (get (non-null old-schema) "items" {})
(get (non-null new-schema) "items" {}) (inc depth))
(or (nil? old-props) (nil? new-props))
;; Leaf/non-object: providing a narrower set of values is safe, a wider one (a new null, a
;; new variant) can break a parsing client.
[[(if (compatible? :response old-schema new-schema) additive breaking)
(str pad "~ " label ": " (brief-change old-schema new-schema))]]
:else
(let [old-keys (set (keys old-props))
new-keys (set (keys new-props))
old-types (type-set old-schema)
new-types (type-set new-schema)]
(concat
;; `schema-props` unwraps a nullable `oneOf`, so both sides reach this branch and the
;; property comparison below never sees the type change. A response that may now be null
;; provides less.
(when (not= old-types new-types)
[[(if (or (nil? old-types) (and new-types (set/subset? new-types old-types)))
additive breaking)
(str pad "~ " label " type: " (sort old-types) " -> " (sort new-types))]])
(shell-lines :response pad label old-schema new-schema)
(for [k (sort (set/difference old-keys new-keys))]
[breaking (str pad "- " label "." k " REMOVED (provides less): " (brief (get old-props k)))])
(for [k (sort (set/difference new-keys old-keys))]
[additive (str pad "+ " label "." k ": " (brief (get new-props k)))])
;; A field the API may now omit provides less, exactly like one that may now be null.
(for [k (sort (set/intersection old-keys new-keys))
:when (and (contains? old-req k) (not (contains? new-req k)))]
[breaking (str pad "! " label "." k " is no longer always returned (provides less)")])
(mapcat (fn [k]
(let [o (get old-props k), n (get new-props k)]
(when (not= o n)
(response-lines (str code "." k) o n (inc depth)))))
(sort (set/intersection old-keys new-keys)))))))))
(defn- changed-operation-lines
"All findings for one surviving operation, as `[severity text]` pairs."
[old-op new-op]
(let [old-params (:params old-op), new-params (:params new-op)
old-keys (set (keys old-params)), new-keys (set (keys new-params))
lbl #(:label (or (get new-params %) (get old-params %)))]
(concat
(for [p (sort (set/difference new-keys old-keys))]
(if (:required (get new-params p))
[breaking (str " + param " (lbl p) " (REQUIRED - breaking): " (brief (:schema (get new-params p))))]
[additive (str " + param " (lbl p) ": " (brief (:schema (get new-params p))))]))
(for [p (sort (set/difference old-keys new-keys))]
[breaking (str " - param " (lbl p) " removed")])
(mapcat (fn [p]
(let [po (get old-params p), pn (get new-params p)]
(concat
(when (not= (:required po) (:required pn))
;; Becoming optional requires LESS of the caller: additive.
[[(if (:required pn) breaking additive)
(str " ! param " (lbl p) " required: " (:required po) " -> " (:required pn))]])
(when (not= (:schema po) (:schema pn))
[[(if (compatible? :request (:schema po) (:schema pn)) additive breaking)
(str " ~ param " (lbl p) " schema: " (brief-change (:schema po) (:schema pn)))]]))))
(sort (set/intersection old-keys new-keys)))
(when (and (:body-required new-op) (not (:body-required old-op)))
[[breaking " ! body is now REQUIRED (breaking)"]])
(when (and (:body-required old-op) (not (:body-required new-op)))
[[additive " ! body is no longer required"]])
;; A caller sends one content type; dropping it fails that caller even if the schema is the same.
(when-let [dropped (seq (sort (set/difference (:body-types old-op) (:body-types new-op))))]
[[breaking (str " - body content type no longer accepted: " (str/join ", " dropped))]])
(when (not= (:body old-op) (:body new-op))
(body-lines "body" (:body old-op) (:body new-op)))
(let [old-resp (:responses old-op), new-resp (:responses new-op)]
(concat
(mapcat (fn [code] (response-lines code (get old-resp code) (get new-resp code)))
(->> (set/intersection (set (keys old-resp)) (set (keys new-resp)))
(filter #(not= (get old-resp %) (get new-resp %)))
sort))
(for [code (sort (set/difference (set (keys old-resp)) (set (keys new-resp))))]
[breaking (str " - response " code " schema removed (provides less)")])
(for [code (sort (set/difference (set (keys new-resp)) (set (keys old-resp))))]
[additive (str " + response " code " schema now declared: " (brief (get new-resp code)))]))))))
(defn diff
"Structured diff of two parsed specs.
Returns `{:removed :added :changed :old :new :counts}`, with `:changed` sorted breaking-first.
Operations are named by their spec path (`GET /api/card/{id}`). `:old` and `:new` map those
names to the parsed operations, which the renderers read for descriptions."
[old-spec new-spec]
(let [old (operations old-spec)
new (operations new-spec)
old-keys (set (keys old)), new-keys (set (keys new))
display (fn [ops ks] (sort (map #(:display (get ops %)) ks)))
schemas (fn [op] (-> (dissoc op :description :docs :display)
(update :params update-vals #(dissoc % :label))))
changed (->> (sort (set/intersection old-keys new-keys))
(keep (fn [k]
(let [o (get old k)
n (get new k)
findings (changed-operation-lines o n)
;; A description-only change carries no schema finding, so
;; without this the DOC_ONLY severity is unreachable. The skill
;; reads descriptions to spot changes it cannot explain, so a
;; reworded docstring is worth surfacing.
findings (cond
(seq findings) findings
;; The walk found nothing, yet the stripped schemas
;; differ: rank the whole schema rather than let a
;; real change fall through to doc-only below.
(not= (schemas o) (schemas n))
[[(if (and (compatible? :request (:body o) (:body n))
(every? #(compatible? :response (get-in o [:responses %]) (get-in n [:responses %]))
(keys (:responses o))))
additive breaking)
" ~ schema changed in a shape the field-by-field comparison does not cover"]]
(not= (:description o) (:description n))
[[doc-only " ~ description changed"]]
(not= (:display o) (:display n))
[[doc-only (str " ~ path variable renamed (callers send the same URL): "
(:display o) " -> " (:display n))]]
;; Schemas are compared without their doc keys, so a
;; reworded field description or a new default lands here.
(not= (:docs o) (:docs n))
[[doc-only " ~ field documentation changed"]])]
(when (seq findings)
{:operation (:display n)
:findings findings
:severity (worst (map first findings))
:old-description (:description o)
:new-description (:description n)}))))
(sort-by (juxt (comp severity-order :severity) :operation)))]
{:removed (display old (set/difference old-keys new-keys))
:added (display new (set/difference new-keys old-keys))
:changed changed
:old (update-keys old #(:display (get old %)))
:new (update-keys new #(:display (get new %)))
:counts {:operations-before (count old)
:operations-after (count new)
:breaking (+ (count (set/difference old-keys new-keys))
(count (filter #(= breaking (:severity %)) changed)))}}))
(defn- print-operation [{:keys [operation findings old-description new-description]} show-docs?]
(println (str " ~ " operation))
(doseq [[_ text] findings] (println text))
(when (and show-docs? (not= old-description new-description))
(println (str " doc was: " (if (str/blank? old-description) "(none)" (subs old-description 0 (min 300 (count old-description))))))
(println (str " doc now: " (if (str/blank? new-description) "(none)" (subs new-description 0 (min 300 (count new-description))))))))
(defn- grouped-findings
"Findings across all changed operations, collapsed by identical text.
A systematic change (one PR closing every `mu/defn` schema, say) produces the same finding on
hundreds of endpoints. That is one changelog entry, so report it once with its endpoints rather
than hundreds of times."
[changed]
(->> (for [{:keys [operation findings]} changed
[severity text] findings]
{:severity severity :text (str/trim text) :operation operation})
(group-by (juxt :severity :text))
(map (fn [[[severity text] occurrences]]
{:severity severity
:text text
:operations (sort (distinct (map :operation occurrences)))}))
(sort-by (juxt (comp severity-order :severity)
(comp - count :operations)
:text))))
(defn print-grouped
"Render `diff` collapsed by distinct change, widest blast radius first."
[{:keys [removed added changed counts]} min-severity]
(let [visible? (fn [sev] (or (nil? min-severity)
(<= (severity-order sev) (severity-order min-severity))))
groups (filter (comp visible? :severity) (grouped-findings changed))
;; Count only what is printed. `groups` is severity-filtered, so counting `changed` or
;; every endpoint inflates the header above the body beneath it.
;; The new-endpoint list prints at every severity (see below) but is additive, so it only
;; counts when additive changes are shown.
added-counted (when (visible? additive) added)
shown-ops (into (set (mapcat :operations groups)) (concat removed added-counted))]
(println (format "# %d distinct changes across %d endpoints (%d -> %d operations)"
(+ (count groups) (if (seq removed) 1 0) (if (seq added-counted) 1 0))
(count shown-ops)
(:operations-before counts) (:operations-after counts)))
(println)
;; Endpoint lists print at every severity: a removed + added pair is usually one endpoint
;; moving, and --severity breaking is exactly where a drafter needs to see both halves.
(when (seq removed)
(println (format "## BREAKING: %d REMOVED ENDPOINTS" (count removed)))
(doseq [k removed] (println (str " - " k)))
(println))
(when (seq added)
(println (format "## ADDITIVE: %d NEW ENDPOINTS" (count added)))
(doseq [k added] (println (str " + " k)))
(println))
(doseq [{:keys [severity text operations]} groups]
(println (format "## %s - %d endpoint%s"
(str/upper-case (name severity))
(count operations)
(if (= 1 (count operations)) "" "s")))
(println (str " " text))
(doseq [op (take 5 operations)] (println (str " " op)))
(when (> (count operations) 5)
(println (str " ... and " (- (count operations) 5) " more")))
(println))
(println "# Grouped by identical finding. A change spanning many endpoints is usually one")
(println "# upstream PR, and belongs in the changelog as one entry naming its cause.")))
(defn print-diff
"Render `diff` to stdout, breaking-first. `min-severity` of `:breaking` or `:additive` hides
less-severe sections."
[{:keys [removed added changed old new counts]} min-severity]
(let [visible? (fn [sev] (or (nil? min-severity)
(<= (severity-order sev) (severity-order min-severity))))
total (+ (count removed) (count added) (count changed))]
(println (format "# %d findings, %d BREAKING (%d removed, %d added, %d changed; %d -> %d operations)"
total (:breaking counts) (count removed) (count added) (count changed)
(:operations-before counts) (:operations-after counts)))
(println)
(when (visible? breaking)
(println (format "## BREAKING: REMOVED ENDPOINTS (%d)" (count removed)))
(doseq [k removed]
(println (str " - " k))
(let [d (:description (get old k))]
(when-not (str/blank? d) (println (str " doc: " (subs d 0 (min 400 (count d))))))))
(let [bc (filter #(= breaking (:severity %)) changed)]
(println)
(println (format "## BREAKING: CHANGED ENDPOINTS (%d)" (count bc)))
(doseq [c bc] (print-operation c true))))
;; The added-endpoint list prints at every severity: a removed + added pair is usually one
;; endpoint moving, and hiding the added half at --severity breaking makes that unpairable.
(when (seq added)
(println)
(println (format "## ADDITIVE: NEW ENDPOINTS (%d) - check if any replaces a removed one" (count added)))
(doseq [k added]
(println (str " + " k))
(let [d (:description (get new k))]
(when-not (str/blank? d) (println (str " doc: " (subs d 0 (min 400 (count d))))))))
nil)
(when (visible? additive)
(let [ac (filter #(= additive (:severity %)) changed)]
(println)
(println (format "## ADDITIVE: CHANGED ENDPOINTS (%d)" (count ac)))
(doseq [c ac] (print-operation c false))))
(when (visible? doc-only)
(let [dc (filter #(= doc-only (:severity %)) changed)]
(println)
(println (format "## DOC_ONLY: CHANGED ENDPOINTS (%d)" (count dc)))
(doseq [c dc] (print-operation c true))))
(println)
(println "# Legend: + added - removed ~ modified ! requiredness changed")
(println "# Breaking = an existing caller, sending what it sent before, can now fail or get a")
(println "# different result. When the tool cannot tell, it ranks breaking.")))
(def ^:private spec-path "resources/openapi/openapi.json")
(defn- spec-blob-exists?
"Whether `ref` has a committed openapi.json."
[ref]
(zero? (:exit (shell/sh* {:quiet? true} "git" "rev-parse" "--verify" "--quiet"
(str ref ":" spec-path)))))
(defn- committed-spec!
"Write `ref`'s committed spec to `out-path`. Returns out-path.
Shells out with output redirected rather than capturing: the spec is megabytes of JSON, and
line-splitting then rejoining it is both wasteful and lossy on trailing whitespace."
[ref out-path]
(let [{:keys [exit]} (shell/sh* {:quiet? true}
;; Values ride as positional args so the shell never parses them:
;; interpolating a ref into the command string lets a ref
;; containing `$`, a quote, or a space produce a wrong file.
"sh" "-c" "git show \"$1\" > \"$2\"" "sh"
(str ref ":" spec-path) out-path)]
(when-not (zero? exit)
(u/exit (str "Could not read " spec-path " at " ref
". Try a fully-qualified ref such as origin/" ref ".") 1))
out-path))
(defn- generated-spec!
"Generate `ref`'s spec from source in a throwaway worktree and copy it to `out-path`.
Generating beats reading the committed blob because the committed spec only updates on PRs
labelled `openapi-self-healing`, so it lags the source it claims to describe. Costs a JVM boot.
Runs the ref's OWN `generate-openapi-spec` command, which writes to the worktree's copy of
`resources/openapi/openapi.json`; the worktree is then discarded, so no checkout is modified and
no support is needed from the ref beyond that command already existing. Returns nil when the ref
cannot be built."
[ref out-path]
(let [worktree (str "/tmp/openapi-diff-" (str/replace ref #"[^A-Za-z0-9]" "_") "-" (System/currentTimeMillis))]
(println (str " generating " ref " from source (JVM boot, ~2 min)..."))
(try
(let [{:keys [exit err]} (shell/sh* {:quiet? true} "git" "worktree" "add" "--detach" worktree ref)]
(when-not (zero? exit)
(u/exit (str "Could not create a worktree for " ref ":\n" (str/join "\n" err)) 1)))
;; The checkout includes the committed spec. Delete it so `.exists` below proves the command
;; wrote a file: the generator logs and swallows its own errors, then exits 0.
(io/delete-file (io/file worktree spec-path) true)
(let [{:keys [exit err]} (try
(shell/sh* {:quiet? true :dir worktree :timeout-ms 900000}
"clojure" "-M:run:ee" "generate-openapi-spec")
(catch clojure.lang.ExceptionInfo e
{:exit 1 :err [(ex-message e)]}))
generated (io/file worktree spec-path)]
(if (and (zero? exit) (.exists generated))
(do (io/copy generated (io/file out-path)) out-path)
(do (println (str " Could not generate a spec at " ref
" (old refs may not build with the current toolchain)."))
(doseq [line (take-last 3 (remove #(re-find #"(?i)reflection warning" %) err))]
(println (str " " line)))
nil)))
(finally
(shell/sh* {:quiet? true} "git" "worktree" "remove" "--force" worktree)))))
(defn- spec-for-ref!
"Materialize `ref`'s spec at `out-path`. Returns `[path stale?]`.
Generates from source unless `committed?`. A generation failure falls back to the committed blob,
which is reported as stale so the caller can say so rather than presenting a possibly-empty diff
as authoritative."
[ref out-path committed?]
(if committed?
[(committed-spec! ref out-path) true]
(if-let [generated (generated-spec! ref out-path)]
[generated false]
(if (spec-blob-exists? ref)
(do (println (str " Falling back to " ref "'s committed spec."))
[(committed-spec! ref out-path) true])
(u/exit (str "Could not generate a spec at " ref ", and it has no committed spec.") 1)))))
(def ^:private api-source-pathspecs
"Everything that can declare an endpoint.
`src/metabase/**/api.clj` alone matches 65 files while 160 contain `defendpoint`: endpoints also
live under `src/metabase/*/api/*.clj` and throughout `enterprise/backend/src`, and the spec
carries 106 `/api/ee/` paths. Watching only the narrow pathspec lets the check report
`OK: spec is current` while dozens of newer endpoint files sit on disk, which is the exact
failure it exists to prevent."
["src/metabase/**/api.clj"
"src/metabase/**/api/*.clj"
"src/metabase/**/routes.clj"
"enterprise/backend/src/**/api.clj"
"enterprise/backend/src/**/api/*.clj"
"enterprise/backend/src/**/routes.clj"])
(defn- last-commit-epoch
"Unix timestamp of the last commit touching `paths`, or nil when none.
Epoch seconds rather than `%cI`: this history carries both `+08:00` and `Z` offsets, so two
same-day commits in different zones sort wrong when ISO strings are compared lexically."
[paths]
(let [{:keys [out]} (apply shell/sh* {:quiet? true} "git" "log" "-1" "--format=%ct" "--" paths)]
(some-> (not-empty (str/trim (str/join out))) parse-long)))
(defn- format-epoch
"`epoch` seconds as a local `YYYY-MM-DD` date.
`java.time` rather than `date -r`: only BSD/macOS reads `-r` as epoch seconds, so on GNU
coreutils it means \"the mtime of this file\" and silently prints nothing."
[epoch]
(str (java.time.LocalDate/ofInstant (java.time.Instant/ofEpochSecond epoch)
(java.time.ZoneId/systemDefault))))
(defn cli-staleness
"Entry point for `./bin/mage openapi-staleness`.
The self-healing CI job only runs on PRs labelled `openapi-self-healing`, so the committed spec
drifts behind master by default. A stale spec makes a diff produce FALSE NEGATIVES: changes that
landed in source but were never regenerated are simply not reported."
[_]
(let [spec-epoch (last-commit-epoch [spec-path])
src-epoch (last-commit-epoch api-source-pathspecs)]
(println (str "spec last changed: " (if spec-epoch (format-epoch spec-epoch) "never")))
(println (str "API source last changed: " (if src-epoch (format-epoch src-epoch) "never")))
(cond
(not (and spec-epoch src-epoch))
(u/exit "\nCould not determine both dates; verify by hand." 1)
(< spec-epoch src-epoch)
(do
(println "\nSTALE: the committed OpenAPI spec predates the newest API source change.")
(println "A diff against this spec under-reports. Either regenerate it for the current")
(println "working tree with `bun run generate-openapi`, or confirm suspected gaps directly")
(println "in source with `grep -rn defendpoint src/metabase/<module>/api.clj`.")
(println "\nCommits that touched API source since the spec was last updated:")
;; A commit range, not `--since`: the commit that last updated the spec often touched API
;; source too, and `--since` includes it.
(let [spec-sha (first (:out (shell/sh* {:quiet? true} "git" "log" "-1" "--format=%H" "--" spec-path)))
{:keys [out]} (apply shell/sh* {:quiet? true}
"git" "log" "--format= %h %ad %an | %s" "--date=short"
(str spec-sha "..HEAD") "--"
api-source-pathspecs)]
(doseq [line (take 20 out)] (println line)))
(u/exit 1))
:else
(do (println "\nOK: no endpoint file is newer than the committed spec.")
;; Deliberately narrower than "the spec is current": 661 of the 763 component schemas
;; live outside `.api` namespaces (`metabase.lib.schema.*` and friends, often `.cljc`),
;; so a shared-schema change moves the generated spec without touching any endpoint file.
(println "This reads endpoint files only - a change to a shared schema such as")
(println "metabase.lib.schema.* will not show up here. `openapi-diff --refs` generates")
(println "both specs from source and is the reliable path.")))))
(defn cli-diff
"Entry point for `./bin/mage openapi-diff`.
Takes either two spec files, or two git refs with `--refs`. With `--refs` each ref's spec is
generated from its source in a throwaway worktree, which is slower than reading the committed
blob but is not subject to its drift."
[{:keys [arguments options]}]
(let [[old-arg new-arg] arguments
severity-arg (some-> (:severity options) str/lower-case)
;; An unrecognised value must not silently disable the filter: a typo would otherwise
;; print the full diff and read as "there were no other changes".
min-severity (case severity-arg
"breaking" breaking
"additive" additive
nil nil
(u/exit (str "Unknown --severity " (pr-str severity-arg)
". Use `breaking` or `additive`.") 1))
_ (when (and (:committed options) (not (:refs options)))
(u/exit "--committed only applies with --refs." 1))
tmp-dir (when (:refs options)
(str "/tmp/openapi-diff-specs-" (System/currentTimeMillis)))]
(try
;; Spec materialization sits INSIDE the try: `spec-for-ref!` exits when a ref can neither be
;; generated nor read, and doing this in the `let` bindings leaked the first spec.
(let [[old-path new-path stale-refs]
(if (:refs options)
(let [committed? (boolean (:committed options))]
(.mkdirs (io/file tmp-dir))
(let [[op ostale] (spec-for-ref! old-arg (str tmp-dir "/old.json") committed?)
[np nstale] (spec-for-ref! new-arg (str tmp-dir "/new.json") committed?)]
[op np (cond-> [] ostale (conj old-arg) nstale (conj new-arg))]))
[old-arg new-arg []])]
(when (:refs options) (println))
(let [read-spec (fn [path]
(when-not (.exists (io/file path))
(u/exit (str path " does not exist.") 1))
(let [s (try (json/read-str (slurp path) {:key-fn identity})
(catch Exception _ nil))]
;; An empty or non-spec file would otherwise diff as zero operations
;; and report "0 BREAKING".
(if (map? (get s "paths"))
s
(u/exit (str path " is not an OpenAPI spec (no \"paths\" object).") 1))))
d (diff (read-spec old-path) (read-spec new-path))]
(if (:grouped options)
(print-grouped d min-severity)
(print-diff d min-severity)))
(when (seq stale-refs)
(println)
(println (str "# WARNING: used the committed spec for " (str/join " and " stale-refs) "."))
(println "# The committed spec only updates on PRs labelled `openapi-self-healing`, so it lags")
(println "# source. This diff UNDER-REPORTS: changes never regenerated into the spec are absent,")
(println "# and an empty result does not mean there were no changes.")))
(finally
;; Generated specs are megabytes each; leaving them behind fills /tmp over a few runs.
(when tmp-dir
(doseq [f (reverse (file-seq (io/file tmp-dir)))]
(io/delete-file f true)))))))

View File

@@ -21,6 +21,7 @@
[mage.merge-kondo-ratchets-test] [mage.merge-kondo-ratchets-test]
[mage.merge-yaml-migrations-test] [mage.merge-yaml-migrations-test]
[mage.modules-test] [mage.modules-test]
[mage.openapi-diff-test]
[mage.project-tests-test] [mage.project-tests-test]
[mage.quick-test-runner-test] [mage.quick-test-runner-test]
[mage.shell-test] [mage.shell-test]

View File

@@ -0,0 +1,581 @@
(ns mage.openapi-diff-test
(:require
[clojure.test :refer [deftest is testing]]
[mage.openapi-diff :as openapi-diff]))
;; Referenced by core_test.clj to ensure namespace is loaded
(def keep-me :loaded)
(defn- spec
([paths] (spec paths {}))
([paths components] {"paths" paths "components" components}))
(defn- op
"An operation with an optional JSON request body, response schema, and parameters."
[& {:keys [body params response description]}]
(cond-> {"description" (or description "") "parameters" (or params [])}
body (assoc "requestBody" {"content" {"application/json" {"schema" body}}})
response (assoc "responses" {"2XX" {"description" "ok"
"content" {"application/json" {"schema" response}}}})))
(defn- obj
([props] {"type" "object" "properties" props})
([props required] (assoc (obj props) "required" required)))
(defn- qparam [required?]
[{"in" "query" "name" "q" "required" required? "schema" {"type" "string"}}])
(defn- breaking-count [old-spec new-spec]
(get-in (openapi-diff/diff old-spec new-spec) [:counts :breaking]))
(defn- breaking? [old-spec new-spec]
(pos? (breaking-count old-spec new-spec)))
(defn- body-change-breaking?
"Whether changing an endpoint's request body from `old-body` to `new-body` is classified breaking."
[old-body new-body]
(breaking? (spec {"/api/x" {"post" (op :body old-body)}})
(spec {"/api/x" {"post" (op :body new-body)}})))
(defn- findings-text [old-spec new-spec]
(->> (:changed (openapi-diff/diff old-spec new-spec))
(mapcat :findings)
(map second)))
(deftest endpoint-lifecycle-test
(testing "a removed endpoint is always breaking"
(let [d (openapi-diff/diff (spec {"/api/x" {"post" (op)}}) (spec {}))]
(is (= ["POST /api/x"] (:removed d)))
(is (= 1 (get-in d [:counts :breaking])))))
(testing "a new endpoint is additive"
(let [d (openapi-diff/diff (spec {}) (spec {"/api/x" {"get" (op)}}))]
(is (= ["GET /api/x"] (:added d)))
(is (zero? (get-in d [:counts :breaking])))))
(testing "identical specs produce no findings"
(let [s (spec {"/api/x" {"post" (op :body (obj {"a" {"type" "string"}}))}})
d (openapi-diff/diff s s)]
(is (empty? (:changed d)))
(is (zero? (get-in d [:counts :breaking])))))
(testing "identical specs WITH response schemas produce no findings"
;; Regression: unchanged responses were compared anyway, emitting `~ response 2XX: null -> null`
;; for every endpoint that declared one.
(let [s (spec {"/api/x" {"delete" (op :response {"type" "null"})
"post" (op :body (obj {"a" {"type" "string"}})
:response (obj {"id" {"type" "integer"}}))}})]
(is (empty? (:changed (openapi-diff/diff s s)))))))
(deftest request-requires-more-is-breaking-test
(testing "breaking = requires MORE from the caller"
(doseq [[label old-body new-body]
[["new required field"
(obj {"a" {"type" "string"}})
(obj {"a" {"type" "string"} "req" {"type" "string"}} ["req"])]
["existing field becomes required"
(obj {"a" {"type" "string"}})
(obj {"a" {"type" "string"}} ["a"])]
["type narrowed"
(obj {"f" {"oneOf" [{"type" "boolean"} {"type" "object"}]}})
(obj {"f" {"type" "boolean"}})]
["enum narrowed"
(obj {"f" {"enum" ["a" "b" "c"]}})
(obj {"f" {"enum" ["a" "b"]}})]
["schema closed to undeclared keys"
(obj {"a" {"type" "string"}})
(assoc (obj {"a" {"type" "string"}}) "additionalProperties" false)]
["field removed"
(obj {"a" {"type" "string"} "gone" {"type" "string"}})
(obj {"a" {"type" "string"}})]]]
(is (body-change-breaking? old-body new-body) label))))
(deftest request-requires-less-is-additive-test
(testing "not breaking = requires the same or LESS from the caller"
(doseq [[label old-body new-body]
[["new optional field"
(obj {"a" {"type" "string"}})
(obj {"a" {"type" "string"} "opt" {"type" "string"}})]
["type widened"
(obj {"f" {"type" "boolean"}})
(obj {"f" {"oneOf" [{"type" "boolean"} {"type" "object"}]}})]
["enum widened"
(obj {"f" {"enum" ["a" "b"]}})
(obj {"f" {"enum" ["a" "b" "c"]}})]
["field made nullable"
(obj {"a" {"type" "string"}})
(obj {"a" {"oneOf" [{"type" "string"} {"type" "null"}]}})]
["schema opened to undeclared keys"
(assoc (obj {"a" {"type" "string"}}) "additionalProperties" false)
(obj {"a" {"type" "string"}})]
["field no longer required"
(obj {"a" {"type" "string"}} ["a"])
(obj {"a" {"type" "string"}})]]]
(is (not (body-change-breaking? old-body new-body)) label))))
(deftest param-requiredness-is-directional-test
(testing "a param becoming required demands more: breaking"
(is (breaking? (spec {"/api/x" {"get" (op :params (qparam false))}})
(spec {"/api/x" {"get" (op :params (qparam true))}}))))
(testing "a param becoming optional demands less: additive"
(is (not (breaking? (spec {"/api/x" {"get" (op :params (qparam true))}})
(spec {"/api/x" {"get" (op :params (qparam false))}})))))
(testing "a new optional param is additive, a new required one is breaking"
(let [none (op :params [])
optional (op :params [{"in" "query" "name" "new" "required" false "schema" {"type" "string"}}])
required (op :params [{"in" "query" "name" "new" "required" true "schema" {"type" "string"}}])]
(is (not (breaking? (spec {"/api/x" {"get" none}}) (spec {"/api/x" {"get" optional}}))))
(is (breaking? (spec {"/api/x" {"get" none}}) (spec {"/api/x" {"get" required}})))))
(testing "a removed param is breaking"
(is (breaking? (spec {"/api/x" {"get" (op :params (qparam false))}})
(spec {"/api/x" {"get" (op :params [])}})))))
(deftest response-provides-less-is-breaking-test
(testing "the rule inverts for output: providing LESS breaks the caller"
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"} "gone" {"type" "string"}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}}))
"a removed response field provides less")
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"oneOf" [{"type" "string"} {"type" "null"}]}}))}}))
"a response field that may now be null provides less than a guaranteed string"))
(testing "providing MORE is additive: clients ignore unknown fields"
(is (not (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"} "extra" {"type" "string"}}))}})))
"returning extra data is not a breaking change")
(is (not (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"oneOf" [{"type" "string"} {"type" "null"}]}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}})))
"a field that is never null now is a stronger guarantee")))
(deftest nested-response-removal-is-breaking-test
(testing "a removed NESTED response field provides less, so it is breaking"
;; Regression: response-lines compared only top-level properties and delegated deeper schemas
;; to (widening? new old). Swapping those arguments inverts value-set semantics but not
;; object-property semantics, so a dropped nested field read as additive.
(let [resp (fn [props] (op :response (obj {"a" (obj props)})))
old-spec (spec {"/api/x" {"get" (resp {"x" {"type" "string"} "y" {"type" "string"}})}})
new-spec (spec {"/api/x" {"get" (resp {"x" {"type" "string"}})}})]
(is (breaking? old-spec new-spec))))
(testing "a new nested response field provides more, so it is additive"
(let [resp (fn [props] (op :response (obj {"a" (obj props)})))
old-spec (spec {"/api/x" {"get" (resp {"x" {"type" "string"}})}})
new-spec (spec {"/api/x" {"get" (resp {"x" {"type" "string"} "extra" {"type" "string"}})}})]
(is (not (breaking? old-spec new-spec))))))
(deftest description-change-is-doc-only-test
(testing "a description-only change surfaces as DOC_ONLY rather than vanishing"
(let [d (openapi-diff/diff (spec {"/api/x" {"get" (op :description "Old.")}})
(spec {"/api/x" {"get" (op :description "New.")}}))]
(is (= 1 (count (:changed d))))
(is (= :doc-only (:severity (first (:changed d)))))
(is (zero? (get-in d [:counts :breaking]))))))
(deftest nested-and-ref-test
(testing "nested body properties are compared recursively"
(let [nested (fn [reporter] (obj {"d" (obj {"reporter" reporter})}))
text (findings-text
(spec {"/api/x" {"post" (op :body (nested {"oneOf" [{"type" "object"} {"type" "boolean"}]}))}})
(spec {"/api/x" {"post" (op :body (nested {"type" "boolean"}))}}))]
(is (some #(re-find #"body\.d\.reporter" %) text))))
(testing "$refs are resolved, so a component-schema change surfaces as an endpoint change"
(let [ref {"$ref" "#/components/schemas/S"}
old-spec (spec {"/api/x" {"post" (op :body (obj {"a" ref}))}} {"schemas" {"S" {"type" "string"}}})
new-spec (spec {"/api/x" {"post" (op :body (obj {"a" ref}))}} {"schemas" {"S" {"type" "integer"}}})]
(is (seq (:changed (openapi-diff/diff old-spec new-spec))))
(is (breaking? old-spec new-spec) "string -> integer narrows what the caller may send")))
(testing "a property literally named $ref is data, not a reference"
;; SCIM schemas declare `{"properties": {"$ref": {...}}}`, where `$ref` is a real field name.
;; Treating it as a pointer crashed ref resolution against the committed spec.
(let [scim (fn [t] (obj {"members" {"type" "array"
"items" (obj {"$ref" {"type" t} "value" {"type" "string"}})}}))
old-spec (spec {"/api/x" {"put" (op :body (scim "string"))}})
new-spec (spec {"/api/x" {"put" (op :body (scim "integer"))}})]
(is (map? (openapi-diff/diff old-spec old-spec)) "resolves without throwing")
(is (breaking? old-spec new-spec) "string -> integer under a $ref-named property still narrows")))
(testing "a deeply nested change is still compared, not collapsed"
;; Deep nesting is ordinary and must be compared field by field. A hop or depth limit would
;; replace deep nodes with the same placeholder on both sides, which compares equal and reports
;; nothing; `resolve-refs` terminates via `seen` instead, so no limit is needed.
(let [nest (fn [leaf] (obj {"a" (obj {"b" (obj {"c" (obj {"d" (obj {"e" (obj {"f" leaf})})})})})}))
old-spec (spec {"/api/x" {"post" (op :body (nest {"enum" ["a" "b" "c"]}))}})
new-spec (spec {"/api/x" {"post" (op :body (nest {"enum" ["a"]}))}})]
(is (breaking? old-spec new-spec) "a narrowed enum six levels down is breaking, not invisible")))
(testing "a self-referential $ref terminates instead of blowing the stack"
(let [s (spec {"/api/x" {"post" (op :body {"$ref" "#/components/schemas/R"})}}
{"schemas" {"R" {"type" "object" "properties" {"self" {"$ref" "#/components/schemas/R"}}}}})]
(is (map? (openapi-diff/diff s s))))))
(deftest signal-to-noise-test
(testing "additive bulk does not drown the breaking few"
;; Mirrors the shape of a real API diff, where the long tail of change is additive.
(let [paths (into {} (for [i (range 20)]
[(str "/api/n" i) {"post" (op :body (obj {"a" {"type" "string"}}))}]))
new-paths (into {} (for [i (range 20)]
[(str "/api/n" i)
{"post" (op :body (obj {"a" {"type" "string"} "added" {"type" "string"}}))}]))
old-spec (spec (merge paths {"/api/gone" {"post" (op)}
"/api/narrow" {"post" (op :body (obj {"f" {"oneOf" [{"type" "object"} {"type" "boolean"}]}}))}}))
new-spec (spec (merge new-paths {"/api/narrow" {"post" (op :body (obj {"f" {"type" "boolean"}}))}}))
d (openapi-diff/diff old-spec new-spec)]
(is (= 2 (get-in d [:counts :breaking])) "1 removed endpoint + 1 narrowing, not 22")
(is (= 20 (count (filter #(= :additive (:severity %)) (:changed d)))))))
(testing "breaking findings sort above additive ones"
(let [d (openapi-diff/diff
(spec {"/api/aaa" {"post" (op :body (obj {"a" {"type" "string"}}))}
"/api/zzz" {"post" (op :body (obj {"b" {"type" "string"}}))}})
(spec {"/api/aaa" {"post" (op :body (obj {"a" {"type" "string"} "new" {"type" "string"}}))}
"/api/zzz" {"post" (op :body (obj {}))}}))]
(is (= "POST /api/zzz" (:operation (first (:changed d))))))))
(deftest grouping-collapses-systematic-changes-test
;; One upstream PR can close every endpoint's schema at once. That is one changelog entry, so the
;; grouped view must report it once with its blast radius, not once per endpoint.
(let [closed (assoc (obj {"a" {"type" "string"}}) "additionalProperties" false)
old-spec (spec (into {} (for [i (range 8)]
[(str "/api/n" i) {"post" (op :body (obj {"a" {"type" "string"}}))}])))
new-spec (spec (into {} (for [i (range 8)]
[(str "/api/n" i) {"post" (op :body closed)}])))
d (openapi-diff/diff old-spec new-spec)
groups (#'openapi-diff/grouped-findings (:changed d))]
(testing "8 endpoints with the same change collapse to one group"
(is (= 8 (count (:changed d))) "ungrouped still reports per-endpoint")
(is (= 1 (count groups)))
(is (= 8 (count (:operations (first groups))))))
(testing "the group keeps its severity and lists its endpoints"
(is (= :breaking (:severity (first groups))))
(is (= "POST /api/n0" (first (:operations (first groups))))
"operations are sorted so output is stable"))))
(deftest grouping-orders-by-blast-radius-test
(testing "the widest-reaching change sorts first, and distinct changes stay distinct"
(let [old-spec (spec (merge (into {} (for [i (range 5)]
[(str "/api/wide" i) {"post" (op :body (obj {"a" {"type" "string"}}))}]))
{"/api/narrow" {"post" (op :body (obj {"b" {"type" "string"}}))}}))
new-spec (spec (merge (into {} (for [i (range 5)]
[(str "/api/wide" i)
{"post" (op :body (assoc (obj {"a" {"type" "string"}})
"additionalProperties" false))}]))
{"/api/narrow" {"post" (op :body (obj {}))}}))
groups (#'openapi-diff/grouped-findings (:changed (openapi-diff/diff old-spec new-spec)))]
(is (= 2 (count groups)) "two distinct changes")
(is (= 5 (count (:operations (first groups)))) "the 5-endpoint change leads")
(is (= 1 (count (:operations (second groups))))))))
(def ^:private breaking-request-changes
[["removed field" (obj {"a" {"type" "string"}}) (obj {})]
["now required" (obj {"a" {"type" "string"}}) (obj {"a" {"type" "string"}} ["a"])]
["narrowed type"
(obj {"a" {"oneOf" [{"type" "object"} {"type" "boolean"}]}})
(obj {"a" {"type" "boolean"}})]
["closed schema"
(obj {"a" {"type" "string"}})
(assoc (obj {"a" {"type" "string"}}) "additionalProperties" false)]])
(deftest breaking-changes-are-classified-breaking-test
(testing "every breaking request change is classified breaking, never additive"
(doseq [[label old-body new-body] breaking-request-changes]
(let [d (openapi-diff/diff (spec {"/api/x" {"post" (op :body old-body)}})
(spec {"/api/x" {"post" (op :body new-body)}}))]
(is (pos? (get-in d [:counts :breaking])) (str label " must be breaking"))
(is (empty? (filter #(= :additive (:severity %)) (:changed d)))
(str label " must not leak into additive"))))))
(deftest breaking-markers-survive-rendering-test
;; Guards the failure mode in mozilla-ai/otari#1315, where a generator dropped breaking-change
;; markers at render time and a breaking commit read like any other entry.
(doseq [[label old-body new-body] breaking-request-changes]
(let [d (openapi-diff/diff (spec {"/api/x" {"post" (op :body old-body)}})
(spec {"/api/x" {"post" (op :body new-body)}}))]
(testing (str label " prints under the breaking heading of the flat report")
(is (re-find #"## BREAKING: CHANGED ENDPOINTS \(1\)\n ~ POST /api/x"
(with-out-str (openapi-diff/print-diff d :breaking)))))
(testing (str label " prints in a breaking group of the grouped report")
(is (re-find #"## BREAKING - 1 endpoint\n.*\n POST /api/x"
(with-out-str (openapi-diff/print-grouped d :breaking))))))))
(deftest severity-filter-hides-less-severe-findings-test
(let [d (openapi-diff/diff (spec {"/api/x" {"post" (op :body (obj {"a" {"type" "string"}}))}})
(spec {"/api/x" {"post" (op :body (obj {"a" {"type" "string"}
"b" {"type" "string"}}))}}))]
(testing "an additive change is hidden at --severity breaking"
(is (not (re-find #"POST /api/x" (with-out-str (openapi-diff/print-diff d :breaking)))))
(is (not (re-find #"POST /api/x" (with-out-str (openapi-diff/print-grouped d :breaking))))))
(testing "and shown at --severity additive"
(is (re-find #"POST /api/x" (with-out-str (openapi-diff/print-diff d :additive))))
(is (re-find #"POST /api/x" (with-out-str (openapi-diff/print-grouped d :additive)))))))
;; ---- direction: arrays, nullability, requiredness, ref chains ----
(deftest response-array-items-are-compared-as-output-test
(let [resp (fn [props] (op :response {"type" "array" "items" (obj props)}))
two (spec {"/api/x" {"get" (resp {"x" {"type" "string"} "y" {"type" "string"}})}})
one (spec {"/api/x" {"get" (resp {"x" {"type" "string"}})}})]
(testing "a field removed from the objects in a response array provides less"
(is (breaking? two one)))
(testing "a field added to the objects in a response array provides more"
(is (not (breaking? one two))))))
(deftest object-nullability-is-compared-test
(let [o (obj {"x" {"type" "string"}})
nullable {"oneOf" [o {"type" "null"}]}]
(testing "a request object field that no longer accepts null requires more"
(is (body-change-breaking? (obj {"a" nullable}) (obj {"a" o}))))
(testing "a response object field that may now be null provides less"
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" o}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" nullable}))}}))))))
(deftest unconstrained-field-gaining-a-type-is-breaking-test
(testing "a request field that accepted any value and now accepts only strings requires more"
(is (body-change-breaking? (obj {"a" {}}) (obj {"a" {"type" "string"}})))))
(deftest response-field-becoming-optional-is-breaking-test
(testing "a response field that the API may now omit provides less"
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}} ["a"]))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}})))))
(deftest long-ref-chain-is-compared-test
(testing "a change 13 $ref hops down is reported, not hidden behind <deep> on both sides"
(let [schemas (fn [leaf]
(into {"S13" leaf}
(for [i (range 13)]
[(str "S" i) (obj {"f" {"$ref" (str "#/components/schemas/S" (inc i))}})])))
body {"$ref" "#/components/schemas/S0"}]
(is (breaking? (spec {"/api/x" {"post" (op :body body)}} {"schemas" (schemas {"enum" ["a" "b"]})})
(spec {"/api/x" {"post" (op :body body)}} {"schemas" (schemas {"enum" ["a"]})}))))))
;; ---- unions and nullable arrays ----
(deftest removed-union-variant-is-breaking-test
(testing "a request body that stops accepting one variant of a oneOf requires more"
(let [a (obj {"kind" {"const" "a"} "x" {"type" "string"}} ["kind"])
b (obj {"kind" {"const" "b"} "y" {"type" "integer"}} ["kind"])]
(is (body-change-breaking? {"oneOf" [a b]} {"oneOf" [a]})))))
(deftest nullable-response-array-items-are-compared-test
(testing "a field removed from the objects in a nullable response array provides less"
(let [resp (fn [props] (op :response (obj {"a" {"oneOf" [{"type" "array" "items" (obj props)}
{"type" "null"}]}})))]
(is (breaking? (spec {"/api/x" {"get" (resp {"x" {"type" "string"} "y" {"type" "string"}})}})
(spec {"/api/x" {"get" (resp {"x" {"type" "string"}})}}))))))
;; ---- keywords beyond type, enum, and properties ----
(deftest tightened-bound-is-breaking-test
(testing "a request field that gains a bound or a narrower map value requires more"
(doseq [[label old-field new-field]
[[":int -> ms/PositiveInt" {"type" "integer"} {"type" "integer" "minimum" 1}]
[":string -> ms/NonBlankString" {"type" "string"} {"type" "string" "minLength" 1}]
["map-of values narrowed"
{"type" "object" "additionalProperties" {"type" "string"}}
{"type" "object" "additionalProperties" {"type" "integer"}}]]]
(is (body-change-breaking? (obj {"a" old-field}) (obj {"a" new-field})) label))))
(deftest loosened-bound-is-additive-test
(testing "the reverse of each tightening requires less of the caller"
(doseq [[label old-field new-field]
[["minimum dropped" {"type" "integer" "minimum" 1} {"type" "integer"}]
["maxLength raised" {"type" "string" "maxLength" 10} {"type" "string" "maxLength" 20}]
["pattern dropped" {"type" "string" "pattern" "^a"} {"type" "string"}]]]
(is (not (body-change-breaking? (obj {"a" old-field}) (obj {"a" new-field}))) label)))
(testing "a response bound that tightens provides a narrower set of values"
(is (not (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "integer"}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "integer" "minimum" 1}}))}}))))))
(deftest nullable-enum-is-compared-test
(let [nullable-enum (fn [vs] (obj {"f" {"oneOf" [{"type" "string" "enum" vs} {"type" "null"}]}}))]
(testing "a nullable request enum that loses a value requires more"
(is (body-change-breaking? (nullable-enum ["a" "b"]) (nullable-enum ["a"]))))
(testing "a nullable request enum that gains a value requires less"
(is (not (body-change-breaking? (nullable-enum ["a"]) (nullable-enum ["a" "b"])))))
(testing "a nullable response enum that gains a value can break a client's switch"
(is (breaking? (spec {"/api/x" {"get" (op :response (nullable-enum ["a"]))}})
(spec {"/api/x" {"get" (op :response (nullable-enum ["a" "b"]))}}))))))
(deftest response-map-of-values-test
(let [resp-spec (fn [r] (spec {"/api/x" {"get" (op :response r)}}))
setting (fn [props req] {"type" "object" "additionalProperties" (obj props req)})
old (obj {"settings" (setting {"enabled" {"type" "boolean"}
"type" {"enum" ["error" "warning"] "type" "string"}}
["enabled" "type"])} ["settings"])]
(testing "map-of value drops a required field"
(let [new (obj {"settings" (setting {"type" {"enum" ["error" "warning"] "type" "string"}} ["type"])} ["settings"])]
(is (breaking? (resp-spec old) (resp-spec new)))))
(testing "map-of value enum widened"
(let [new (obj {"settings" (setting {"enabled" {"type" "boolean"}
"type" {"enum" ["error" "warning" "info"] "type" "string"}}
["enabled" "type"])} ["settings"])]
(is (breaking? (resp-spec old) (resp-spec new)))))
(testing "nullable map-of value type string -> integer"
(let [o (obj {"attributes" {"oneOf" [{"additionalProperties" {"type" "string"} "type" "object"} {"type" "null"}]}} ["attributes"])
n (obj {"attributes" {"oneOf" [{"additionalProperties" {"type" "integer"} "type" "object"} {"type" "null"}]}} ["attributes"])]
(is (breaking? (resp-spec o) (resp-spec n)))))))
(deftest allof-is-compared-test
(testing "a request body wrapped in allOf (Malli :and) that gains a required field requires more"
(is (body-change-breaking? {"allOf" [(obj {"a" {"type" "string"}})]}
{"allOf" [(obj {"a" {"type" "string"} "req" {"type" "string"}} ["req"])]})))
(testing "a request field under allOf that changes string -> integer requires more"
(is (body-change-breaking? (obj {"f" {"allOf" [{"type" "string"}]}})
(obj {"f" {"allOf" [{"type" "integer"}]}}))))
(testing "a request body under allOf that gains an optional field requires less"
(is (not (body-change-breaking? {"allOf" [(obj {"a" {"type" "string"}})]}
{"allOf" [(obj {"a" {"type" "string"} "b" {"type" "string"}})]}))))
(testing "a response under allOf that loses a field provides less"
(is (breaking? (spec {"/api/x" {"get" (op :response {"allOf" [(obj {"a" {"type" "string"} "b" {"type" "string"}})]})}})
(spec {"/api/x" {"get" (op :response {"allOf" [(obj {"a" {"type" "string"}})]})}})))))
(deftest tuple-element-narrowed-is-breaking-test
(is (body-change-breaking? (obj {"t" {"type" "array" "prefixItems" [{"const" "x"} {"type" "string"}]}})
(obj {"t" {"type" "array" "prefixItems" [{"const" "x"} {"type" "integer"}]}}))))
(deftest unmodeled-keyword-change-is-breaking-test
(testing "a change to a keyword the tool does not model is ranked breaking, not hidden"
(is (body-change-breaking? (obj {"a" {"type" "array" "contains" {"type" "string"}}})
(obj {"a" {"type" "array" "contains" {"type" "integer"}}})))))
(deftest union-variant-edits-are-directional-test
(let [a (obj {"kind" {"const" "a"} "x" {"type" "string"}} ["kind"])
a+ (obj {"kind" {"const" "a"} "x" {"type" "string"} "z" {"type" "string"}} ["kind"])
b (obj {"kind" {"const" "b"} "y" {"type" "integer"}} ["kind"])]
(testing "a request variant that gains an optional field requires less"
(is (not (body-change-breaking? {"oneOf" [a b]} {"oneOf" [a+ b]}))))
(testing "a request variant that loses a field requires more"
(is (body-change-breaking? {"oneOf" [a+ b]} {"oneOf" [a b]})))))
;; ---- response structure ----
(deftest response-nullable-array-plus-items-test
(let [resp-spec (fn [r] (spec {"/api/x" {"get" (op :response r)}}))]
(testing "nullable array becomes non-null AND items drop a field"
(let [o {"oneOf" [{"type" "array" "items" (obj {"id" {"type" "integer"} "name" {"type" "string"}} ["id"])} {"type" "null"}]}
n {"type" "array" "items" (obj {"id" {"type" "integer"}} ["id"])}]
(is (breaking? (resp-spec o) (resp-spec n)))))
(testing "field removed at depth 8"
(let [nest (fn [leaf] (reduce (fn [acc k] (obj {k acc} [k])) leaf (map #(str "l" %) (range 8))))
o (nest (obj {"a" {"type" "string"} "b" {"type" "string"}}))
n (nest (obj {"a" {"type" "string"}}))]
(is (breaking? (resp-spec o) (resp-spec n)))))))
(deftest response-object-becomes-opaque-test
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"id" {"type" "integer"}} ["id"]))}})
(spec {"/api/x" {"get" (op :response {"type" "object"})}}))))
(deftest response-schema-removed-is-breaking-test
(is (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string"}}))}})
(spec {"/api/x" {"get" (op)}}))))
(deftest nullable-object-field-closed-is-breaking-test
(testing "a nullable request object field that starts rejecting undeclared keys requires more"
(let [o (obj {"x" {"type" "string"}})]
(is (body-change-breaking? (obj {"a" {"oneOf" [o {"type" "null"}]}})
(obj {"a" {"oneOf" [(assoc o "additionalProperties" false) {"type" "null"}]}}))))))
(deftest param-schema-severity-test
(let [p (fn [schema] (op :params [{"in" "query" "name" "q" "required" false "schema" schema}]))]
(testing "a query param enum narrowed requires more"
(is (breaking? (spec {"/api/x" {"get" (p {"enum" ["a" "b" "c"]})}})
(spec {"/api/x" {"get" (p {"enum" ["a"]})}}))))
(testing "a query param enum widened requires less"
(is (not (breaking? (spec {"/api/x" {"get" (p {"enum" ["a"]})}})
(spec {"/api/x" {"get" (p {"enum" ["a" "b"]})}})))))
(testing "a free string param becoming an enum requires more"
(is (breaking? (spec {"/api/x" {"get" (p {"type" "string"})}})
(spec {"/api/x" {"get" (p {"enum" ["a"]})}}))))))
;; ---- documentation and rendering ----
(deftest nested-description-change-is-doc-only-test
(testing "rewording a nested field's description is not a schema change"
(let [d (openapi-diff/diff
(spec {"/api/x" {"post" (op :body (obj {"a" {"type" "string" "description" "Old."}}))}})
(spec {"/api/x" {"post" (op :body (obj {"a" {"type" "string" "description" "New."}}))}}))]
(is (= [:doc-only] (map :severity (:changed d))))))
(testing "a field literally named description is still compared"
(is (body-change-breaking? (obj {"description" {"type" "string"}}) (obj {})))))
(deftest distinct-leaf-changes-stay-distinct-when-grouped-test
(testing "two different changes to the same field on two endpoints are two groups"
(let [body (fn [field] (op :body (obj {"a" field})))
old-spec (spec {"/api/p" {"post" (body {"type" "integer" "minimum" 1})}
"/api/q" {"post" (body {"type" "integer" "maximum" 10})}})
new-spec (spec {"/api/p" {"post" (body {"type" "integer"})}
"/api/q" {"post" (body {"type" "integer"})}})
groups (#'openapi-diff/grouped-findings (:changed (openapi-diff/diff old-spec new-spec)))]
(is (= 2 (count groups))))))
(deftest truncated-brief-names-the-change-test
(let [models (fn [vs] [{"in" "query" "name" "models" "required" false
"schema" {"oneOf" [{"type" "array" "items" {"type" "string" "enum" vs}} {"type" "null"}]}}])
old ["dashboard" "table" "dataset" "no_models" "timeline" "snippet" "collection" "transform" "document" "pulse" "metric" "card"]
new ["dashboard" "table" "dataset" "no_models" "timeline" "snippet" "collection" "measure" "transform" "document" "pulse" "metric" "card"]
lines (findings-text (spec {"/api/x" {"get" (op :params (models old))}})
(spec {"/api/x" {"get" (op :params (models new))}}))]
(is (some #(re-find #"\+\"measure\"" %) lines) "the added enum value is visible in the finding")))
(deftest unconstrained-allof-member-is-ignored-test
(testing "a Malli :fn predicate adds an empty allOf member, which the spec cannot compare"
(is (not (body-change-breaking? {"allOf" [(obj {"a" {"type" "string"}}) {}]}
{"allOf" [(obj {"a" {"type" "string"}}) {} {}]})))))
;; ---- shapes the verifier found falling through to doc-only ----
(deftest union-with-one-object-variant-is-compared-test
(testing "a string | object request field whose string enum narrows requires more"
(is (body-change-breaking? (obj {"x" {"oneOf" [{"type" "string" "enum" ["a" "b"]} (obj {"k" {"type" "string"}})]}})
(obj {"x" {"oneOf" [{"type" "string" "enum" ["a"]} (obj {"k" {"type" "string"}})]}})))))
(deftest keywords-beside-properties-are-compared-test
(testing "an allOf beside an object's properties that gains a required field requires more"
(is (body-change-breaking? (assoc (obj {"z" {"type" "string"}}) "allOf" [(obj {"a" {"type" "string"}})])
(assoc (obj {"z" {"type" "string"}}) "allOf" [(obj {"a" {"type" "string"} "c" {"type" "string"}} ["c"])]))))
(testing "minProperties added beside properties requires more"
(is (body-change-breaking? (obj {"a" {"type" "string"}}) (assoc (obj {"a" {"type" "string"}}) "minProperties" 1)))))
(deftest exclusive-and-inclusive-bounds-compare-as-one-test
(testing "exclusiveMinimum 0 -> minimum 0 accepts 0 too, so it requires less"
(is (not (body-change-breaking? (obj {"x" {"type" "integer" "exclusiveMinimum" 0}})
(obj {"x" {"type" "integer" "minimum" 0}})))))
(testing "minimum 0 -> exclusiveMinimum 0 rejects 0, so it requires more"
(is (body-change-breaking? (obj {"x" {"type" "integer" "minimum" 0}})
(obj {"x" {"type" "integer" "exclusiveMinimum" 0}})))))
(deftest request-body-envelope-is-compared-test
(let [with-body (fn [content required?] {"description" "" "parameters" []
"requestBody" {"required" required? "content" content}})
json {"application/json" {"schema" (obj {"a" {"type" "string"}})}}]
(testing "a body that becomes required requires more"
(is (breaking? (spec {"/api/x" {"post" (with-body json false)}})
(spec {"/api/x" {"post" (with-body json true)}}))))
(testing "dropping a content type fails callers that send it"
(is (breaking? (spec {"/api/x" {"post" (with-body json false)}})
(spec {"/api/x" {"post" (with-body {"multipart/form-data" (get json "application/json")} false)}}))))))
(deftest path-variable-rename-is-not-a-removal-test
(testing "renaming {id} to {card-id} leaves the URL that a caller sends unchanged"
(let [get-by (fn [n] (op :params [{"in" "path" "name" n "required" true "schema" {"type" "integer"}}]))
d (openapi-diff/diff (spec {"/api/card/{id}" {"get" (get-by "id")}})
(spec {"/api/card/{card-id}" {"get" (get-by "card-id")}}))]
(is (empty? (:removed d)))
(is (zero? (get-in d [:counts :breaking])))
(testing "and the report names the real paths"
(is (= ["GET /api/card/{card-id}"] (map :operation (:changed d))))
(is (= [:doc-only] (map :severity (:changed d))))))))
(deftest grouped-header-counts-only-visible-severities-test
(testing "at --severity breaking, a new endpoint is not counted in the header"
(let [d (openapi-diff/diff (spec {}) (spec {"/api/x" {"get" (op)}}))]
(is (re-find #"^# 0 distinct changes across 0 endpoints"
(with-out-str (openapi-diff/print-grouped d :breaking)))))))
(deftest routes-that-differ-only-by-variable-name-stay-distinct-test
(testing "two routes that normalize to the same path are both kept"
(let [s (spec {"/api/db/{id}/schemas" {"get" (op)} "/api/db/{virtual-db}/schemas" {"get" (op)}})]
(is (= 2 (get-in (openapi-diff/diff s s) [:counts :operations-before]))))))
(deftest request-default-change-is-breaking-test
(let [p (fn [schema] (spec {"/api/x" {"get" (op :params [{"in" "query" "name" "q" "required" false "schema" schema}])}}))]
(testing "a query param whose default flips changes behavior for callers who omit it"
(is (breaking? (p {"type" "boolean" "default" false}) (p {"type" "boolean" "default" true}))))
(testing "a dropped request default is breaking"
(is (body-change-breaking? (obj {"a" {"type" "string" "default" "x"}}) (obj {"a" {"type" "string"}}))))
(testing "a default added where none existed is not"
(is (not (breaking? (p {"type" "boolean"}) (p {"type" "boolean" "default" false})))))
(testing "a response default is not part of the contract"
(is (not (breaking? (spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string" "default" "x"}}))}})
(spec {"/api/x" {"get" (op :response (obj {"a" {"type" "string" "default" "y"}}))}})))))))