Probed the new guard against a partially-broken derivation rather than only the total-failure case the floors were written for: deleting the transitive step from `_request_reachable`, so only schemas named directly on a request body resolve. Both floors stay satisfied and `test_the_derivation_reached_a_real_population` stays GREEN. What reddens is the planted-member test — its plant is reached through a nested `$ref` exactly so that it can — and the disposition test, since the nested rows vanish and report as PHANTOM. So the floors are the crude backstop against a parse that reached nothing at all, and the planted-member test is what actually holds the walk honest. Recorded in the docstring, because a floor described as anti-vacuity reads as coverage it does not have, and the next reader deciding which of these to keep working should know which one is load-bearing.
292 lines
15 KiB
Python
292 lines
15 KiB
Python
"""#807 guard: every schema that can SILENTLY DROP a member on a SPA write has a stated disposition.
|
|
|
|
WHAT THIS BLOCKS. A request-body property that is absent from its schema's `required` array emits
|
|
into `web/src/api/generated/v1.d.ts` as an OPTIONAL member (`"weight"?: number`). A SPA builder may
|
|
then omit it, `tsc` says nothing — optional means omittable, by design — and on a FULL-REPLACE write
|
|
the server stores the field's default. That is #754's mechanism and it is what #807 found live in
|
|
`MultiCollectionItemRequest.weight` and `UpdateFFmpegProfileRequest.qsvPreferNativeDecoder`.
|
|
|
|
WHY IT IS A DERIVED GUARD AND NOT A TABLE IN A DOC. #807 shipped the disposition list by hand
|
|
TWICE and got it wrong BOTH times, each time by sorting a schema on its NAME rather than on what its
|
|
endpoint does:
|
|
|
|
round 1 a prose sentence exempted "create/update" — `updateMultiCollection` and
|
|
`updateFFmpegProfile` are full replaces, and both were live silent drops.
|
|
round 2 a hand-written table replaced that sentence and omitted `ArtworkContentTypeModel`,
|
|
because `…Model` reads as a response model. It is reachable from `PUT /channels/{id}`.
|
|
|
|
Two misses from one mechanism, so the mechanism goes rather than the list getting a third patch.
|
|
`testing.guard-derives-population-from-source` is explicit that a hand-written list is "a filter
|
|
frozen at authoring time, correct on the day it was written and unable to report the day it stopped
|
|
being" — and unlike #820's population (sites in code, which needs compiler-API tooling), THIS
|
|
population has an authoritative machine-readable source: the OpenAPI document.
|
|
|
|
SCOPE vs POPULATION, per that same record. The POPULATION — which schemas can drop a member on a
|
|
write — is DERIVED here, every run, from `ErsatzTV/wwwroot/openapi/v1.json`. The DISPOSITIONS below
|
|
are the SCOPE: a reviewed policy choice per schema, legitimately hand-written, and each one is
|
|
FORCED to exist by the set-equality assertion. A new optional member anywhere in the request graph
|
|
fails this test until someone writes down what should happen about it.
|
|
|
|
Set equality is asserted in BOTH directions and reported separately, because they are opposite
|
|
defects: `missing` is a schema the API can drop and nobody has ruled on (the #807 defect), `phantom`
|
|
is a disposition for a schema that no longer has an optional member reachable from a request body
|
|
(the registry claiming coverage of something gone).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
OPENAPI = REPO_ROOT / "ErsatzTV" / "wwwroot" / "openapi" / "v1.json"
|
|
|
|
# --- the closed disposition vocabulary -------------------------------------------------------
|
|
#
|
|
# COVERED the SPA builds this body and the builder is annotated `Complete<T>`, so omitting a
|
|
# member is a typecheck error (`web/src/api/completeRequest.ts`).
|
|
# CREATE a POST that creates a new entity, where an omitted member correctly means "use the
|
|
# default". Annotating it would be a BUG, not coverage — see the from-lineup note.
|
|
# TRIGGER the body parameterises an ACTION and replaces no stored entity, so there is nothing to
|
|
# drop.
|
|
# COMPUTED the optional members are get-only computed properties on the C# record. System.Text.Json
|
|
# never deserializes them, so the client cannot drop a stored value by omitting them —
|
|
# and `Complete<T>` must NOT be applied here, because it would force a caller to invent
|
|
# server-computed values in an outbound request.
|
|
COVERED = "COVERED"
|
|
CREATE = "CREATE"
|
|
TRIGGER = "TRIGGER"
|
|
COMPUTED = "COMPUTED"
|
|
|
|
DISPOSITIONS: dict[str, tuple[str, str]] = {
|
|
"UpdateFFmpegProfileRequest": (
|
|
COVERED,
|
|
"PUT /ffmpeg/profiles/{id} is a full replace. `qsvPreferNativeDecoder` is a defaulted ctor "
|
|
"param so ASP.NET drops it from `required`. Was a LIVE silent drop before #807.",
|
|
),
|
|
"CreateFFmpegProfileRequest": (
|
|
COVERED,
|
|
"FFmpegProfilesScreen's `Draft` feeds BOTH the POST and the full-replace PUT, so the draft "
|
|
"type itself is `Complete<…>` rather than only the update wrapper.",
|
|
),
|
|
"MultiCollectionItemRequest": (
|
|
COVERED,
|
|
"Nested in the full-replace PUT /multi-collections/{id}. `weight` is a defaulted ctor param "
|
|
"(`CreateMultiCollectionRequest.cs`). Was a LIVE silent drop before #807: the screen carried "
|
|
"a prose comment warning that dropping it resets every weight to 1, and a comment is not a "
|
|
"check.",
|
|
),
|
|
"ArtworkContentTypeModel": (
|
|
COMPUTED,
|
|
"Reachable from the full-replace PUT /channels/{id} (via `UpdateChannelRequest.logo`), so "
|
|
"the endpoint test alone would put it in COVERED. It is not: `IsExternalUrl`, "
|
|
"`HasContentType` and `UrlWithContentType` are computed get-only properties on the record "
|
|
"`ArtworkContentTypeModel(string Path, string ContentType)`, never deserialized, so a "
|
|
"client omitting them drops nothing. Annotating the SPA site `Complete<…>` would force it "
|
|
"to fabricate server-computed values. This row exists because #807's hand-written table "
|
|
"omitted this schema on the strength of its `…Model` name.",
|
|
),
|
|
"AutoTunedChannelRequest": (
|
|
CREATE,
|
|
"POST /channels/auto-tune. `CreateChannelFromLineupHandler` does `Channels.Add(...)` and "
|
|
"rejects a duplicate number; it never overwrites an existing channel.",
|
|
),
|
|
"CreateChannelFromLineupAdvancedOptionsRequest": (
|
|
CREATE,
|
|
"POST /channels/from-lineup. Omission is LOAD-BEARING here: `CreateChannelFromLineupClearField` "
|
|
"documents that for the template-inheritable fields a null/omitted override means INHERIT the "
|
|
"template value, with a separate explicit `clear` list to force NONE. `Complete<T>` would "
|
|
"collapse that third state into explicit-null.",
|
|
),
|
|
"AutoTuneSourceWeightRequest": (
|
|
CREATE,
|
|
"Nested in the auto-tune POST body; same create-time semantics as its parent.",
|
|
),
|
|
"ScanShowRequest": (
|
|
TRIGGER,
|
|
"POST /libraries/{id}/scan-show starts a scan. `deepScan` parameterises the action; no "
|
|
"entity is replaced.",
|
|
),
|
|
}
|
|
|
|
# Anti-vacuity floors. A completeness check whose population came back empty must not report that it
|
|
# proved everything (`testing.guard-ships-with-mutation-proof`). These are LOWER bounds on the
|
|
# derived intermediates, deliberately well under today's values (141 reachable, 13 with optional
|
|
# members) so ordinary schema churn does not trip them — they exist to catch a broken parse or a
|
|
# `$ref` walk that reached nothing, not to pin the corpus.
|
|
MIN_REQUEST_REACHABLE_SCHEMAS = 40
|
|
MIN_SCHEMAS_WITH_OPTIONAL_MEMBERS = 5
|
|
|
|
|
|
def _load() -> dict:
|
|
if not OPENAPI.is_file():
|
|
pytest.fail(f"{OPENAPI.relative_to(REPO_ROOT)} is missing — run scripts/update-openapi.sh")
|
|
return json.loads(OPENAPI.read_text())
|
|
|
|
|
|
def _schema_refs(node: object, out: set[str]) -> None:
|
|
"""Collect every `#/components/schemas/X` name anywhere under `node`."""
|
|
if isinstance(node, dict):
|
|
ref = node.get("$ref")
|
|
if isinstance(ref, str) and ref.startswith("#/components/schemas/"):
|
|
out.add(ref.rsplit("/", 1)[1])
|
|
for value in node.values():
|
|
_schema_refs(value, out)
|
|
elif isinstance(node, list):
|
|
for value in node:
|
|
_schema_refs(value, out)
|
|
|
|
|
|
def _request_reachable(doc: dict) -> set[str]:
|
|
"""Every schema reachable from ANY operation's request body, transitively.
|
|
|
|
Two deliberate non-restrictions, both because this population has now been drawn by hand wrongly
|
|
twice and every hand-drawn edge is a place to be wrong again:
|
|
|
|
NO VERB ALLOW-LIST. An earlier draft scanned POST/PUT/PATCH, which reads as obviously right and
|
|
already had an exception: `DELETE /api/v1/media-items` carries a request body. Rather than argue
|
|
that a DELETE body cannot cause a full-replace drop — probably true, and exactly the kind of
|
|
"probably" that produced this record's two live misses — every operation carrying a request body
|
|
seeds the walk, and anything it surfaces must acquire a stated disposition.
|
|
|
|
TRANSITIVE, and that is load-bearing rather than thorough: `MultiCollectionItemRequest` and
|
|
`ArtworkContentTypeModel` are both NESTED, so a check reading only top-level request bodies
|
|
would have missed both of the schemas this guard exists because of. `_schema_refs` walks every
|
|
value of every dict and every list element, so `oneOf` (23 occurrences today), `items`,
|
|
`additionalProperties` and any future composition keyword are covered without naming them.
|
|
"""
|
|
schemas = doc["components"]["schemas"]
|
|
seeds: set[str] = set()
|
|
for operations in doc["paths"].values():
|
|
for operation in operations.values():
|
|
if isinstance(operation, dict) and operation.get("requestBody"):
|
|
_schema_refs(operation["requestBody"], seeds)
|
|
|
|
seen: set[str] = set()
|
|
stack = list(seeds)
|
|
while stack:
|
|
name = stack.pop()
|
|
if name in seen or name not in schemas:
|
|
continue
|
|
seen.add(name)
|
|
nested: set[str] = set()
|
|
_schema_refs(schemas[name], nested)
|
|
stack.extend(nested - seen)
|
|
return seen
|
|
|
|
|
|
def _optional_members(doc: dict) -> dict[str, list[str]]:
|
|
"""Schema -> its properties that sit OUTSIDE `required`, i.e. the ones that emit `?:`."""
|
|
out: dict[str, list[str]] = {}
|
|
for name, schema in doc["components"]["schemas"].items():
|
|
properties = schema.get("properties") or {}
|
|
if not properties:
|
|
continue
|
|
optional = sorted(set(properties) - set(schema.get("required") or []))
|
|
if optional:
|
|
out[name] = optional
|
|
return out
|
|
|
|
|
|
def _droppable(doc: dict) -> dict[str, list[str]]:
|
|
reachable = _request_reachable(doc)
|
|
return {n: m for n, m in _optional_members(doc).items() if n in reachable}
|
|
|
|
|
|
def test_the_derivation_reached_a_real_population() -> None:
|
|
"""Anti-vacuity: a broken `$ref` walk or parse must not read as 'nothing to rule on'.
|
|
|
|
What this test does NOT do, measured 2026-08-22 rather than assumed: it does not catch a
|
|
PARTIALLY broken walk. Deleting the transitive step from `_request_reachable` — so only the
|
|
schemas named directly on a request body resolve — leaves both floors satisfied and this test
|
|
GREEN. What reddens is `test_MUTATION_a_planted_optional_member_is_reported_as_MISSING`, whose
|
|
planted schema is reached through a nested `$ref` precisely so that it can, plus the disposition
|
|
test (the nested rows vanish and report as PHANTOM). So the floors below are the crude backstop
|
|
against a parse that reached nothing at all; the planted-member test is what actually holds the
|
|
walk honest, and it should be the one kept working if these two ever conflict.
|
|
"""
|
|
doc = _load()
|
|
reachable = _request_reachable(doc)
|
|
optional = _optional_members(doc)
|
|
assert len(reachable) >= MIN_REQUEST_REACHABLE_SCHEMAS, (
|
|
f"only {len(reachable)} schemas reachable from a request body — the $ref walk is broken, "
|
|
"not the API"
|
|
)
|
|
assert len(optional) >= MIN_SCHEMAS_WITH_OPTIONAL_MEMBERS, (
|
|
f"only {len(optional)} schemas have a property outside `required` — suspect the parse"
|
|
)
|
|
|
|
|
|
def test_every_droppable_request_schema_has_a_stated_disposition() -> None:
|
|
"""Set equality, both directions, accumulated into ONE message.
|
|
|
|
Failing fast on the first mismatch hands back one schema at a time and invites fixing them one
|
|
at a time, which is how #754's twin stayed hidden.
|
|
"""
|
|
droppable = _droppable(_load())
|
|
|
|
unruled = sorted(set(droppable) - set(DISPOSITIONS))
|
|
phantom = sorted(set(DISPOSITIONS) - set(droppable))
|
|
|
|
problems: list[str] = []
|
|
if unruled:
|
|
problems.append(
|
|
"MISSING — reachable from a POST/PUT/PATCH body with a member outside `required`, and "
|
|
"no disposition written down. Decide what happens to each and add a row:\n"
|
|
+ "\n".join(f" {n}: optional members {droppable[n]}" for n in unruled)
|
|
)
|
|
if phantom:
|
|
problems.append(
|
|
"PHANTOM — a disposition for a schema that is no longer request-reachable with an "
|
|
"optional member. Delete the row rather than leaving it claiming coverage:\n"
|
|
+ "\n".join(f" {n}" for n in phantom)
|
|
)
|
|
assert not problems, "\n\n".join(problems)
|
|
|
|
|
|
def test_every_disposition_uses_the_closed_vocabulary() -> None:
|
|
"""A free-text disposition would let a row read as considered while saying nothing."""
|
|
allowed = {COVERED, CREATE, TRIGGER, COMPUTED}
|
|
for name, (disposition, why) in DISPOSITIONS.items():
|
|
assert disposition in allowed, f"{name}: {disposition!r} is not one of {sorted(allowed)}"
|
|
assert len(why.strip()) >= 40, f"{name}: the reason is too thin to be a decision"
|
|
|
|
|
|
def test_MUTATION_a_planted_optional_member_is_reported_as_MISSING() -> None:
|
|
"""The checker-guard mutation proof (`testing.guard-ships-with-mutation-proof`).
|
|
|
|
Disarming a checker makes it ABSENT rather than red, so the mutation goes into the guarded
|
|
ARTIFACT: plant a request-reachable schema carrying a member outside `required` and require the
|
|
derivation to surface it. Mutating the checker's own population instead would be the trap that
|
|
record names — a shrunken population makes every real row report PHANTOM, so the proof would go
|
|
red on a false positive while saying nothing about the MISSING detection this row claims.
|
|
|
|
The planted schema is reached through a real request body, so this also exercises the
|
|
transitive `$ref` walk that the hand-written list twice failed to do by eye.
|
|
"""
|
|
doc = _load()
|
|
schemas = doc["components"]["schemas"]
|
|
|
|
host = next(
|
|
(n for n in _request_reachable(doc) if isinstance(schemas.get(n), dict) and schemas[n].get("properties")),
|
|
None,
|
|
)
|
|
assert host is not None, "no request-reachable schema with properties — the walk is broken"
|
|
|
|
schemas["PlantedDroppableRequest"] = {
|
|
"type": "object",
|
|
"properties": {"plantedRequired": {"type": "string"}, "plantedOptional": {"type": "string"}},
|
|
"required": ["plantedRequired"],
|
|
}
|
|
schemas[host]["properties"]["plantedLink"] = {"$ref": "#/components/schemas/PlantedDroppableRequest"}
|
|
|
|
droppable = _droppable(doc)
|
|
assert "PlantedDroppableRequest" in droppable, (
|
|
"the planted schema was NOT surfaced — the derivation cannot see a droppable member, so a "
|
|
"green run of this file proves nothing"
|
|
)
|
|
assert droppable["PlantedDroppableRequest"] == ["plantedOptional"]
|
|
assert "PlantedDroppableRequest" not in DISPOSITIONS
|