Jak/stapi v0.2.0 - #129
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the workspace to STAPI v0.2.0 across the Pydantic models, FastAPI server, and Python client, and adds a new pystapi-schema-generator package to export a clean, generic OpenAPI document.
Changes:
- Bump STAPI versions and align server/client behavior and conformance URI handling for v0.2.0.
- Restructure request/response models (notably
SearchParameters,OrderRequest, async opportunity search records/status collections) and tighten JSON-schema serialization requirements. - Add a standalone schema generator package + script to export a deterministic OpenAPI YAML with cleaned schema names/operationIds.
Reviewed changes
Copilot reviewed 47 out of 49 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds new workspace package and dependency updates (incl. pyyaml/types). |
| stapi-pydantic/tests/test_shared.py | Adds schema/serialization tests for shared models (Link/RootResponse/Conformance). |
| stapi-pydantic/tests/test_search_parameters.py | Adds tests for open-ended datetime intervals and SearchParameters behavior. |
| stapi-pydantic/tests/test_product.py | Adds/updates tests for v0.2.0 product models and required fields. |
| stapi-pydantic/tests/test_order.py | Expands tests for order models (status code extensibility, bbox computation, stored requests). |
| stapi-pydantic/tests/test_opportunity.py | Expands tests for opportunity request/record/status models and bbox/schema requirements. |
| stapi-pydantic/tests/test_filter.py | Adds tests for CQL2 property-name extraction helper. |
| stapi-pydantic/src/stapi_pydantic/shared.py | Introduces reusable NumberMatched annotated field and updates Link serialization behavior. |
| stapi-pydantic/src/stapi_pydantic/search_parameters.py | Adds SearchParameters model shared by Order/Opportunity requests. |
| stapi-pydantic/src/stapi_pydantic/root.py | Forces defaults to be required in serialization schema for RootResponse. |
| stapi-pydantic/src/stapi_pydantic/product.py | Updates Product/ProductsCollection fields to v0.2.0 shape (stapi_type/version, required description, numberMatched). |
| stapi-pydantic/src/stapi_pydantic/order.py | Refactors order request/storage shapes, adds bbox computation, introduces status collections and generic status code handling. |
| stapi-pydantic/src/stapi_pydantic/opportunity.py | Refactors opportunity request/search record/status models; adds bbox computation and collections. |
| stapi-pydantic/src/stapi_pydantic/geometry.py | Adds bbox computation helper used by Order/Opportunity models. |
| stapi-pydantic/src/stapi_pydantic/filter.py | Adds cql2_property_names traversal utility. |
| stapi-pydantic/src/stapi_pydantic/datetime_interval.py | Adds singly-open datetime interval parsing/serialization for SearchParameters. |
| stapi-pydantic/src/stapi_pydantic/constants.py | Bumps STAPI_VERSION to 0.2.0. |
| stapi-pydantic/src/stapi_pydantic/conformance.py | Adjusts conformance model schema requirements for serialization. |
| stapi-pydantic/src/stapi_pydantic/init.py | Re-exports new/renamed models and utilities for v0.2.0. |
| stapi-pydantic/pyproject.toml | Bumps package version and adds typing-extensions dependency. |
| stapi-fastapi/tests/test_product.py | Updates server response assertions to new product collection shape. |
| stapi-fastapi/tests/test_order.py | Updates tests for new order request shape, required-filter enforcement, and status collection responses. |
| stapi-fastapi/tests/test_opportunity.py | Adds test ensuring required queryable predicates are enforced for opportunity search. |
| stapi-fastapi/tests/test_opportunity_async.py | Adds/updates async opportunity search tests (monitor links, statuses collection, conformance behavior, Prefer handling). |
| stapi-fastapi/tests/shared.py | Updates test products’ conformsTo behavior to rely on router-derived conformances. |
| stapi-fastapi/tests/conftest.py | Adjusts fixtures to new opportunity search body shape and stops force-overriding product conformsTo. |
| stapi-fastapi/tests/backends.py | Updates mock backends for new request/record shapes and fixes opportunity geometry reflection. |
| stapi-fastapi/tests/application.py | Wires the async search-record-statuses backend in the test application. |
| stapi-fastapi/src/stapi_fastapi/routers/root_router.py | Renames/extends async-search links, returns collections for statuses/records, and gates statuses endpoint correctly. |
| stapi-fastapi/src/stapi_fastapi/routers/product_router.py | Updates payload models, adds required-queryables validation, and improves OpenAPI response metadata. |
| stapi-fastapi/src/stapi_fastapi/errors.py | Changes QueryablesError to HTTP 400. |
| stapi-fastapi/src/stapi_fastapi/backends/product_backend.py | Updates backend type aliases for new payload/request models. |
| stapi-fastapi/pyproject.toml | Bumps stapi-fastapi version to 0.9.0. |
| scripts/run-tests.sh | Includes pystapi-schema-generator in the test runner loop. |
| scripts/export-openapi | Adds shim script to export OpenAPI YAML via the new generator package. |
| pystapi-schema-generator/tests/test_application.py | Adds snapshot/invariant tests for exported OpenAPI (paths, schemas, determinism, cleanliness). |
| pystapi-schema-generator/src/pystapi_schema_generator/py.typed | Marks package as typed. |
| pystapi-schema-generator/src/pystapi_schema_generator/application.py | Implements reference app + OpenAPI post-processing (clean ids/names, dedup, templated product paths, examples). |
| pystapi-schema-generator/src/pystapi_schema_generator/init.py | Exposes generator entrypoints. |
| pystapi-schema-generator/README.md | Documents CLI usage. |
| pystapi-schema-generator/pyproject.toml | Defines the new package, dependencies, and console script. |
| pystapi-client/tests/test_client.py | Adds tests for updated conformance URI patterns and product-scoped opportunity capability checks. |
| pystapi-client/tests/fixtures/products.json | Updates fixtures for v0.2.0 product shapes and per-product conformsTo inventory. |
| pystapi-client/tests/fixtures/landing_page.json | Updates root conformance URIs to v0.2.0 API-level classes only. |
| pystapi-client/tests/conftest.py | Mocks per-product GET endpoints for product conformance capability checks. |
| pystapi-client/src/pystapi_client/conformance.py | Updates conformance class inventory and tightens URI regex patterns. |
| pystapi-client/src/pystapi_client/client.py | Switches opportunity support checks to product-scoped conformsTo and updates request models. |
| pystapi-client/pyproject.toml | Bumps pystapi-client version to 0.0.2. |
| pyproject.toml | Adds new workspace member/package and updates dev dependencies and mypy file set. |
Comments suppressed due to low confidence (1)
stapi-pydantic/src/stapi_pydantic/order.py:97
- OrderStatus.new currently instantiates OrderStatus directly, which bypasses the calling class's generic parameterization/subclassing (e.g. OrderStatus[NarrowCodes]) and can allow values that should be rejected. Use cls(...) so constraints are applied consistently.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| async def create_order(self, payload: OrderPayload, request: Request, response: Response) -> Order: # type: ignore | ||
| def validate_required_queryables(self, search_parameters: SearchParameters) -> None: | ||
| required = set(self.product.queryables.model_json_schema().get("required", [])) |
There was a problem hiding this comment.
Seems the required fields list should be cached? And if no fields are required this method should short circuit, no?
There was a problem hiding this comment.
Maybe this should be a method on the product or its queryables rather than this router?
| DatetimeInterval = Annotated[ | ||
| tuple[AwareDatetime, AwareDatetime], | ||
| BeforeValidator(validate_before), | ||
| AfterValidator(validate_after), | ||
| WrapSerializer(serialize, return_type=str), | ||
| WithJsonSchema({"type": "string"}), | ||
| ] | ||
|
|
||
| # Interval that may be open (via ``..`` or an empty string) on at most one | ||
| # end, per the Search Parameters Object datetime definition. | ||
| OpenDatetimeInterval = Annotated[ | ||
| tuple[AwareDatetime | None, AwareDatetime | None], | ||
| BeforeValidator(validate_open_before), | ||
| AfterValidator(validate_open_after), | ||
| WrapSerializer(serialize_open, return_type=str), | ||
| WithJsonSchema({"type": "string"}), | ||
| ] |
There was a problem hiding this comment.
Why two types here? Shouldn't this just be DatetimeInterval, which should support open intervals?
There was a problem hiding this comment.
Seems like collapsing these types would also allow collapsing the validation functions to some degree.
| # Per spec, whenever the client sent a Prefer header the server must | ||
| # respond with Preference-Applied indicating the mode actually applied. | ||
| # This branch always applies the sync ("wait") mode. |
|
|
||
| async def create_order(self, payload: OrderPayload, request: Request, response: Response) -> Order: # type: ignore | ||
| def validate_required_queryables(self, search_parameters: SearchParameters) -> None: | ||
| required = set(self.product.queryables.model_json_schema().get("required", [])) |
There was a problem hiding this comment.
Seems the required fields list should be cached? And if no fields are required this method should short circuit, no?
|
|
||
| async def create_order(self, payload: OrderPayload, request: Request, response: Response) -> Order: # type: ignore | ||
| def validate_required_queryables(self, search_parameters: SearchParameters) -> None: | ||
| required = set(self.product.queryables.model_json_schema().get("required", [])) |
There was a problem hiding this comment.
Maybe this should be a method on the product or its queryables rather than this router?
| def _build_client(base_url: str = "http://stapiserver", **root_router_kwargs: Any) -> TestClient: | ||
| """Build a test client with an app whose root router is configured explicitly.""" | ||
|
|
||
| @asynccontextmanager | ||
| async def lifespan(app: FastAPI) -> AsyncIterator[dict[str, Any]]: | ||
| yield { | ||
| "_orders_db": InMemoryOrderDB(), | ||
| "_opportunities_db": InMemoryOpportunityDB(), | ||
| "_opportunities": [create_mock_opportunity()], | ||
| } | ||
|
|
||
| root_router = RootRouter( | ||
| get_orders=mock_get_orders, | ||
| get_order=mock_get_order, | ||
| get_order_statuses=mock_get_order_statuses, | ||
| conformances=[API.core], | ||
| **root_router_kwargs, | ||
| ) | ||
| root_router.add_product(product_test_spotlight_async_opportunity) | ||
|
|
||
| app = FastAPI(lifespan=lifespan) | ||
| app.include_router(root_router, prefix="") | ||
| return TestClient(app, base_url=base_url) | ||
|
|
||
|
|
||
| def _build_async_client(with_statuses: bool, base_url: str = "http://stapiserver") -> TestClient: | ||
| """Async-capable client, optionally with the statuses backend wired.""" | ||
|
|
||
| @asynccontextmanager | ||
| async def lifespan(app: FastAPI) -> AsyncIterator[dict[str, Any]]: | ||
| yield { | ||
| "_orders_db": InMemoryOrderDB(), | ||
| "_opportunities_db": InMemoryOpportunityDB(), | ||
| "_opportunities": [create_mock_opportunity()], | ||
| } | ||
|
|
||
| kwargs: dict[str, Any] = {} | ||
| if with_statuses: | ||
| kwargs["get_opportunity_search_record_statuses"] = mock_get_opportunity_search_record_statuses | ||
|
|
||
| root_router = RootRouter( | ||
| get_orders=mock_get_orders, | ||
| get_order=mock_get_order, | ||
| get_order_statuses=mock_get_order_statuses, | ||
| get_opportunity_search_records=mock_get_opportunity_search_records, | ||
| get_opportunity_search_record=mock_get_opportunity_search_record, | ||
| conformances=[API.core], | ||
| **kwargs, | ||
| ) | ||
| root_router.add_product(product_test_spotlight_async_opportunity) | ||
|
|
||
| app = FastAPI(lifespan=lifespan) | ||
| app.include_router(root_router, prefix="") | ||
| return TestClient(app, base_url=base_url) |
There was a problem hiding this comment.
Don't these duplicate functionality already present elsewhere? Couldn't the config of the app be configured using some pytest marks or something, providing products/root router behaviors in a more composible/reusable manner?
| raise ValueError("cannot compute bbox: geometry has no coordinates") | ||
| lons = [c[0] for c in coords] | ||
| lats = [c[1] for c in coords] | ||
| if all(len(c) >= 3 for c in coords): |
There was a problem hiding this comment.
Does this work with M coordinates, or could it accidentally conflate M coords as Z?
| stapi_version: str = STAPI_VERSION | ||
| # geojson-pydantic excludes bbox-when-None via a custom serializer schema | ||
| # gen can't see; override with a schema-visible exclude_if. | ||
| bbox: BBox | None = Field(default=None, exclude_if=lambda v: v is None) |
There was a problem hiding this comment.
Should this be computed from the opportunity bboxes? Seems like this would also apply to other collection models (OrderCollection? SearchRecordCollection?)
| # bbox is spec-REQUIRED; non-nullable annotation makes the schema | ||
| # non-nullable in both modes, the config makes it serialization-required, | ||
| # and the compute_bbox after-validator fills the computed default. | ||
| bbox: BBox = cast(BBox, None) |
There was a problem hiding this comment.
I'm not sure this makes sense, and it leads to a type error (Variable is mutable so its type is invariant). Perhaps the spec should simply require bbox, and here we just go with the BBox | None annotation anyway? That might make the openapi docs subtly misaligned, however...hmm, I'm not sure about this one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RhnGNojViYvYDevhtz999a
…tion stapi fields
…yables enforcement
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes the OrderPayload, OpportunityPayload, OrderSearchParameters, OrderStatuses, and OpportunitySearchRecords aliases that scaffolded the v0.2.0 migration. Only the spec-aligned names remain: OrderRequest, OpportunityRequest, SearchParameters, OrderStatusCollection, and OpportunitySearchRecordCollection. Breaking change, accepted for this release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Creates the package at its final location in one step. The module was previously introduced as scripts/openapi_app.py, moved into stapi-fastapi as reference_app.py, then extracted here, so a reviewer read the same ~200 lines three times and watched stapi-fastapi's pyproject gain and lose an export extra.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spec's Search Parameters Object datetime allows open ends via '..' or an empty string, with only singly-open intervals permitted. Adds OpenDatetimeInterval; OpportunityProperties keeps the closed DatetimeInterval since an opportunity window is concrete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nable Spec frames statuses as extensible: providers may support additional statuses through extensions. status_code is now enum-or-string (known codes still validate to the enum), and OrderStatus / OpportunitySearchStatus are generic so implementations can constrain the accepted set with their own StrEnum, e.g. OrderStatus[MyCodes]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- bbox non-nullable and serialization-required on Order/Opportunity - spec-REQUIRED defaulted fields (type, stapi_type, stapi_version, links, conformsTo, ...) marked required in serialization schemas - Link schema no longer degrades to a bare object in serialization - stored order requests and search parameters round-trip unknown fields - Opportunity id is string-only; collections omit null id - Product.description required per spec - numberMatched available on all collections - clear error for bbox computation on empty geometries Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- products advertise sync/async opportunity classes per actual capability - search records carry rel=monitor links when statuses endpoint exists - landing page uses spec rel search-records - Preference-Applied always sent when a preference was specified - statuses endpoint and conformance gated on async search support - Location headers and correct media types documented in OpenAPI Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- opportunity capability checks consult the product's conformsTo per spec, not the root document - conformance URI patterns anchor the version as a single path segment - enum gains API-level extension classes (order-statuses, searches-opportunity, searches-opportunity-statuses) - fixtures model a spec-compliant server Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the pieces the model and router changes build on: - `bbox_from_geometry_input` computes a bbox from a geometry in model, mapping, or `__geo_interface__` form, so a before-validator can fill bbox before the geometry field itself has been parsed. - `union_bboxes` unions member bboxes for collections, degrading to 2D when the members are mixed since elevation is unknown for the 2D ones. - `Queryables.required_property_names` reads the required set from the published queryables JSON Schema, cached per subclass. Consulted on every search and order request, so deriving it per call was wasteful. Also tightens the 3D check in `compute_geometry_bbox` from `>= 3` to `== 3`: GeoJSON positions are 2D or 3D only (RFC 7946 3.1.1), so a third element is always elevation and never a measure. Co-Authored-By: Claude <noreply@anthropic.com>
…erval names Four related model corrections, all surfacing in the exported schema. Status objects disagreed with the spec in opposite directions. `OpportunitySearchStatus` marked `reason_code`/`reason_text` required because `json_schema_serialization_defaults_required` promotes any defaulted field, while the spec marks both optional; they now carry the `exclude_if` the rest of the codebase already uses, so they are omitted rather than serialized as null. `OrderStatus` had the opposite problem — without that config its `links` was optional though the spec requires it. `OrderStatusCollection` emitted a second, unconstrained `OrderStatus-2` schema whose `status_code` had no schema at all, because its TypeVar fell back to the `OrderStatus[Any]` bound. Giving the TypeVar a default collapses it to a single `OrderStatus`, which also retires three now-stale `type: ignore[type-arg]` comments and the client TODO that tracked them. bbox is now filled by a before-validator rather than an after-validator. Assigning `self.bbox` after the fact was what produced the "variable is mutable so its type is invariant" error; computing it before field validation avoids the assignment entirely while keeping the field non-nullable and required in the schema. Collections gain a computed bbox too, but stay spec-OPTIONAL so other implementations need not derive one. BREAKING CHANGE: `DatetimeInterval` now denotes the general interval that may be open on one end, and the both-ends-bounded form is `BoundedDatetimeInterval` (was `DatetimeInterval`); `OpenDatetimeInterval` is removed. The name now qualifies the special case rather than the general one. Co-Authored-By: Claude <noreply@anthropic.com>
… the product The spec documents path parameters in camelCase and the exported document used snake_case for everything except `productId`, so the two disagreed wherever a reader copied a path. Routes now use `orderId`, `searchRecordId`, and `opportunityCollectionId`. `Path(alias=...)` keeps the Python parameters snake_case, with explicit titles because FastAPI would otherwise derive "Orderid" from the alias. Path parameter names are documentation only — they do not appear in request URLs — so this changes no wire format, only the published document. `validate_required_queryables` moves off the router and onto the product, which is what owns the queryables. It also no longer rebuilds the queryables JSON Schema on every search and order request: the required set is derived once and cached, and an empty set short-circuits before walking the filter. BREAKING CHANGE: path parameters in the OpenAPI document are camelCase. Generated clients that bind by parameter name will need regenerating. Co-Authored-By: Claude <noreply@anthropic.com>
Adds `test_no_required_property_is_nullable`, which asserts no schema property is both required and permitted to be null. That combination is always a bug here: `json_schema_serialization_defaults_required` promotes every defaulted field, so an optional-and-nullable field must carry `exclude_if` to stay out of the required set. Forgetting it is silent, which is exactly how the `OpportunitySearchStatus` reason fields became required. Verified to fail with those two fields when the bug is reintroduced. `test_opportunity_async` built its own apps through two local helpers that duplicated the conftest client fixtures. The fixtures now take per-test overrides via a `root_router_kwargs` marker, passing None to withhold a backend, so the helpers are gone and capability-gating tests express only what differs. `MyProductQueryables` derived from `BaseModel` rather than `Queryables`, violating the `type[Queryables]` annotation on `Product.__init__`. Nothing caught it because mypy's `files` covers `src/` only. Also updates the path and schema inventory snapshots, drops a comment describing a prior code state, and stops the determinism test replacing the whole environment when it only means to set PYTHONHASHSEED. Co-Authored-By: Claude <noreply@anthropic.com>
The validator's only entrypoint crashed on import, so the tool did not run at all. Three independent defects, none of which CI could see: the module is named `validate_api.py`, so pytest never collects it, and it was not type-checked. - `schemathesis` was declared `>=3.37.0` with no upper bound, so re-locking crossed into 4.x, where the checks moved out of `schemathesis.checks` and `experimental`/`from_uri` were removed. Capped below 4; migrating is a separate decision. - `SCHEMA_URL` pointed at `openapi.yaml` in the spec repo root, which 404s — the document lives at `spec/openapi.yaml`. - The checks were called directly as `(response, case)`, but their signature is `(ctx, response, case)` and only schemathesis can build a `CheckContext`; they are now handed to `call_and_validate`, which also drops the duplicate `validate_response` call. `schema.validate()` returns None, so asserting on it could never pass. The 4.x jump alone did not break this — the URL and check-call defects predate it, so the tool was already broken before the cap was crossed. Verified the module imports and loads the schema. Running it end to end still needs a STAPI server on localhost:8000. Co-Authored-By: Claude <noreply@anthropic.com>
mypy's `files` covered `src/` only, so no test file had ever been checked. Two real bugs were sitting in that blind spot: a product fixture deriving from `BaseModel` instead of `Queryables`, violating the `type[Queryables]` annotation on `Product.__init__`, and a pagination mock measuring against an already-sliced list. Each package carries its own `tests` package, so a single mypy run over all of them collides on the module name. scripts/run-mypy.sh checks one package at a time, mirroring run-tests.sh, and the hook now calls it. The stapi-fastapi annotations are the bulk of this: `-> None` on tests and fixtures, `pytest.FixtureRequest` on fixture params, parameterized generics for `Opportunity`/`OpportunityCollection`/`OrderRequest`, explicit declarations where inferring `OrderStatus[OrderStatusCode]` narrowed past the `OrderStatus[OrderStatusCode | str]` default, and an `AssertLink` Protocol for the `assert_link` fixture that was typed as a bare `Callable`. Four escape hatches remain, each commented: two casts where a fixture's concrete type is not visible through Starlette's `TestClient.app` or the GeoJSON geometry union, and two ignores for pydantic/dataclass_transform gaps no annotation can close. Also corrects `setup_orders_pagination`, annotated `-> list[Order]` while returning raw JSON dicts — invisible while `res.json()` was `Any`. `respx` ships no stubs, so it gets an `ignore_missing_imports` override alongside the existing `pygeofilter.parsers.*` one. Co-Authored-By: Claude <noreply@anthropic.com>
The order-statuses gate tested `self.get_order_statuses`, which is the handler method rather than the backend, so it was always truthy. A server that withheld the backend still advertised the conformance class, published the route, and emitted a monitor link on every order -- then 500ed when a client followed it. Every sibling gate was audited; this was the only one wrong. Product conformance seeded itself from the product's declared classes and only ever added, so an async-only product mounted on a sync root advertised opportunities classes whose routes were never registered. The advertised set is now derived from what is actually served. Conformance lists were built from a set, so their order changed between processes; they are now sorted. Pagination parameters were re-declared ad hoc on four handlers and the 100-item cap was applied to only one of them. `limit=0` dead-ended paging, `limit=-2` silently truncated the result set with no next link, and page two's self link pointed back at page one. Shared annotated query params now reject out-of-range values with a 422 and publish their bounds, and self links carry the current query parameters. Also adds the six missing operation summaries (FastAPI was deriving `Root:List-Orders` from route names), files create-order under Orders and the opportunity routes under Opportunities rather than Products, renames the status-collection route to `list-` to match its sibling, drops a duplicate route-name constant, and removes runtime dependencies the library never imports. BREAKING CHANGE: out-of-range `limit` values are now rejected with 422 rather than silently clamped or truncated. Co-Authored-By: Claude <noreply@anthropic.com>
Six defects that all passed the existing tests, mypy strict and ruff. numberMatched was write-only: it declared a serialization_alias but no validation alias, so a server's `numberMatched` parsed to None and was then dropped by exclude_if. It now round-trips under the wire name. Serialization aliases did not fire on nested items. Conformance and Product declare aliases but did not set serialize_by_alias, and model config is not inherited by nested models, so `conformsTo` and `type` dumped as `conforms_to` and `type_` -- contradicting their own published schema. A single shared config constant now applies to every response model, replacing six scattered copies. bbox is expressed as two annotated types rather than duplicated field declarations. The schema modes describe different things: validation describes what a caller may supply, where omitting bbox is legal because a validator derives it; serialization describes what is always emitted, where bbox is spec-REQUIRED. Neither may carry a null default, and a collection bbox may not carry a null branch, because BBox is non-nullable and exclude_if means null is never produced -- an empty collection omits bbox rather than reporting it as null. Collection set_bbox recursed unboundedly on empty features, because union_bboxes returns None for an empty sequence and that was assigned back into the field it had just tested. OrderStatus.new ignored cls, so a parameterized OrderStatus[MyCodes] returned a bare OrderStatus and accepted codes outside its enum. Also drops the dead Props/Geom/OPP TypeVars, types CQL2Filter as dict[str, Any], and replaces queryables' hand-rolled per-subclass cache with functools.cache. The client names its generic parameters instead of suppressing the type-arg check, which the CQL2Filter change retires. BREAKING CHANGE: models that declare aliases now serialize by alias, so model_dump() emits conformsTo and type rather than conforms_to and type_. Co-Authored-By: Claude <noreply@anthropic.com>
Three interdependent strands: the backend contract could not be unified
without touching every handler, the handlers could not declare their error
responses without one registration path, and narrowing the geometry union
spans all three packages plus the exported document.
**One page contract.** The four list backends returned four incompatible
shapes -- items with a token and a count, items with a token, a nested
Maybe of both, and one with no pagination at all. That last shape is why
GET /searches/opportunities/{searchRecordId}/statuses published no next or
limit: the handler had nothing to paginate. Every list backend now returns
a Page, so both previously unpaginated endpoints gain pagination by
construction, numberMatched is populated consistently rather than only on
OrderCollection, and GET /products publishes the count it always knew.
Page carries backend-supplied links because a stored opportunity
collection's create-order and search-record links are ones no handler can
reconstruct.
**One route registration path.** Thirteen hand-filled add_api_route calls
became declarative Route descriptors on which summary is required, with
tags derived from the route family rather than the owning router.
add_api_route is overridden too, so the guarantees hold for anything
reaching the router: it rejects a missing summary and merges the shared
error responses into every route. That closes three gaps at once -- 404
was raised at nine sites and declared nowhere, 400 likewise, and
Preference-Applied was a spec MUST that the server emitted but never
declared.
**Six geometry types.** The spec enumerates exactly six geometry
conformance classes and requires a Product to advertise at least one, so a
GeometryCollection was a value no implementation could declare support
for. stapi_pydantic.Geometry is now that six-member union and is exported;
geojson_pydantic's seven-member union is no longer referenced outside it.
Also reorders the generator's schema passes: deduplication ran first and
collapsed a now-identical Input/Output pair onto the alphabetically-first
name, publishing the mode suffix as if it meant something. Merging runs
first, and deduplication now sorts suffixed names last so it can never
elect one as the survivor.
Published document: 40 schemas become 37 (GeometryCollection and two
divergent parameterizations gone, no Input/Output variants remain), every
operation declares 400 and 404, Preference-Applied is declared on both
responses of the opportunity search, and the two unpaginated endpoints
gain next and limit.
BREAKING CHANGE: list backends must return Page; Geometry no longer
includes GeometryCollection.
Co-Authored-By: Claude <noreply@anthropic.com>
The CI step existed but could not report a problem. It ran `test 0`, a no-op standing in for the validation it was named after, and then checked `if [ $result ]` -- a non-empty-string test, true for every exit code -- so it printed "Validated OK!" unconditionally. The suite it was meant to run was never collected either, the module being named validate_api.py rather than test_validate_api.py. The validator also pinned its schema to the spec repo's main branch, so had it ever run it would have checked a server against whatever that branch happened to contain. It now validates the document this working tree produces, exported through pystapi-schema-generator, which keeps the check self-contained rather than depending on the spec repo being checked out alongside. These tests drive a live server, so they skip at module level unless the harness supplies a schema and base URL, keeping them out of the ordinary unit run without pretending they passed. The two POST endpoints trip hypothesis's filter_too_much health check -- it generates no valid bodies for a discriminated geometry union nested in a datetime-interval string. That is a generator limitation rather than a server/spec disagreement, and it was masking the contract checks, so it is suppressed explicitly. Verified both directions: the gate passes against the current document (17 passed), and removing 404 from the exported document fails it (14 failed). Before this branch's error-response work it reported 12 failures, all of them undocumented 404s. Co-Authored-By: Claude <noreply@anthropic.com>
The class was the only place in the project using the plural. Its own stapi_type is "ProductCollection", the spec's Product Collection section requires that value, and all six example documents use it -- so the published schema name contradicted the entity it described, and the exported document was the only artifact a reader would see the plural in. Product itself is singular, so the plural was internally inconsistent too. v0.2.0 is the breaking window this was waiting for. ProductsCollection remains as a deprecated alias bound to the same class, to be removed in the next breaking release. The collection discovery in the shared tests now deduplicates by identity rather than by exported name, since an alias exports one class under two names and would otherwise be parametrized twice. BREAKING CHANGE: the exported schema is named ProductCollection; the Python name ProductsCollection is deprecated. Co-Authored-By: Claude <noreply@anthropic.com>
…roduce
Registration had grown behaviour of its own. A shared set of error responses
was merged into every route inside an overridden add_api_route, where it
could be overridden but never narrowed -- so the landing page and the
conformance declaration, which take no input and call no backend, both
published a 404. Centralising something that varies per endpoint produced a
document that lies.
Errors are now a required field on Route, composed from BAD_REQUEST,
NOT_FOUND and SERVER_ERROR, and each route states the set its handler can
actually raise. What is shared is the wording, which really is identical;
the set is not, because it genuinely differs. Two routes state `{}`.
The add_api_route override is gone with it. Its only other job was to
reject a route without a summary, which the required `summary` field
already does at construction: overriding a standard FastAPI method to
restate what the type says bought nothing and made the router's behaviour
something a reader had to go and check. Route.to_api_route returns the
keyword arguments instead, and register_route is one line handing them to
FastAPI.
Tags move onto Route as an enum, replacing a sixteen-entry table keyed by
route family and a lookup that raised at runtime for a missing key. The
type checker now confirms a tag is a real one.
Also adds a Responses alias for the unwieldy response-declaration mapping,
and declares 500, which is raised deliberately at six sites when a backend
reports failure and so is part of the contract rather than a crash.
Finally, a collection's `self` and `next` links now carry the media type
their target actually serves. `next` was built by a helper that hard-coded
application/json, so every geo+json collection published a next link
contradicting both its own response and its own self link.
BREAKING CHANGE: Route requires `errors` and `tag`; the `register` method
is now `register_route` and takes a single Route; `add_api_route` is no
longer overridden and no longer injects error responses.
Co-Authored-By: Claude <noreply@anthropic.com>
Both packages declared a new version in pyproject while their changelogs stopped at the previous one, so every change on this branch was unrecorded -- including several that break consumers. That is the gap a changelog exists to close, and it is the one that matters most here: a reviewer had already reverted a commit for removing public API without it being surfaced anywhere. Entries are derived from the commits, with every breaking item stating the old behaviour and what to do about it.
0.9.0 needs the 0.2.0 models -- Page carries them, Geometry is the narrowed six-member union, and ProductCollection is the renamed class -- but the bound still admitted 0.1.0, so a resolver was free to install a version this release cannot work with.
The client imports Geometry and ProductCollection, which exist only in 0.2.0, but declared no lower bound at all -- so a resolver could install 0.1.0 and the package would fail on import.
… empty Two related ways the published schema was wider than the API. Seventeen properties advertised a `null` branch for a value the server never emits. Each carried `exclude_if=lambda v: v is None`, so it is omitted from the response -- but `X | None` is also how "unset" is spelled in Python, and that reached the document. A generated client modelled `numberMatched: int | None` and a strict validator accepted a null the server cannot produce. The rule and its fix already existed in shared.py but were applied to one field. `omitted_when_none` now pairs the two halves that have to agree, so they cannot be added apart. Product, Provider and RootResponse also over-required against the prose, which is normative. Product demanded conformsTo, title, keywords and providers; Provider demanded roles and url; RootResponse demanded title -- none of which the markdown marks REQUIRED. A Product response valid per the spec failed the published schema. Those fields defaulted to `[]` or `""` and so were always emitted, which `json_schema_serialization_defaults_required` then correctly marked required; they are now omitted when unset, which is what "optional" means in the prose. Requiredness now matches the markdown for all 21 spec entities. The eight remaining null branches are honest: six are geojson-pydantic's own geometry bbox, and `OpportunityRequest.next` and `SearchParameters.filter` are request fields that really do serialize as null. BREAKING CHANGE: Provider.roles and Provider.url are no longer required, and unset optional fields are omitted from responses rather than serialized as empty or null. Co-Authored-By: Claude <noreply@anthropic.com>
The generator polished the reference export while every real stapi-fastapi server kept publishing FastAPI defaults. A two-product deployment advertised operation ids like `root_test_satellite_provider_get_order_parameters_products_test_satellite_provider_order_parameters_get` and a 204-character component name. None of the generator work reached it, because none of it runs outside the reference export. Operation ids now come from the route name a router already knows, prefixed per product so they stay unique on a multi-product server -- the bare route name is not enough, since every product registers `get_product`. Twenty-one of twenty-one are unique and are valid identifiers. Generic component names keep pydantic bracket form but spell each parameter readably, so `OpportunityCollection_Annotated_Union_Point__…_` becomes `OpportunityCollection_Geometry__MyOpportunityProperties_`, 204 characters down to 56. Parameterizations stay distinct, so a multi-product server does not silently lose a schema to a name collision. Co-Authored-By: Claude <noreply@anthropic.com>
The gate exported the OpenAPI document from the very application it then validated, which can only show that the export matches the app -- never that either conforms to the spec. It now requires the document as an argument, and is no longer run in CI. scripts/export-openapi is dropped: it was a shim over the pystapi-schema-generator console script. The pystapi-validator entrypoint still pointed at tests/validate_api.py, which this branch renamed away, so the console script exited 4 without running anything. It is now a CLI taking a schema path and --base-url, resolving the suite relative to itself rather than to the working directory, and failing loudly rather than skipping when unconfigured. Also fixes a latent bug in the script: with no children, `kill $(pgrep -P ...)` runs argument-less and fails, which under `set -e` reported failure for a run that passed.
Squashed deliberately: these changes are interlocked (a rename in stapi-pydantic and its call sites in stapi-fastapi cannot type-check apart), and will be curated into coherent commits when the branch is rebuilt. stapi-pydantic: - conformsTo now round-trips. It carried only a serialization_alias, so Conformance could not parse its own output and silently dropped the value; RootResponse had the mirror bug via a camelCase field name. One shared ConformsTo definition is now used by Conformance, RootResponse and Product. - Link.model_validate no longer raises TypeError on a missing href. A custom __init__ makes pydantic route validation through it, so a required positional died in argument binding instead of reaching validation. - A malformed CQL2 filter is a ValidationError, not a 500: cql2 raises its own exception types, which pydantic does not convert. - JsonSchemaModel is removed. The queryables endpoints return the schema directly, which drops a PlainValidator/PlainSerializer pair and the orphan BaseModel component every deployment published. - OpportunityRequest.limit is int | None. The default page size is implementation-defined, so the model must not assert one the client never named. - The four copied bbox validators are two mixins; Order derives from Feature and OrderCollection from FeatureCollection rather than re-implementing them. - OrderCollection's __iter__/__len__/__getitem__ are gone in favour of the inherited iter()/length, matching OpportunityCollection. Overriding pydantic's reserved __iter__ broke dict(collection). - The ProductsCollection alias is dropped. stapi-fastapi: - A query parameter named `self` no longer 500s every collection endpoint. The raw query params were splatted into URL.include_query_params as Python keywords, colliding with its own `self`; repeated params were also collapsed. - The root router no longer advertises conformance classes whose routes it never registered, mirroring what build_conformances already did per product. - add_product rejects a duplicate id. include_router appends, so re-adding left the old routes serving every request while product_routers pointed at the new one. - A bad pagination token on POST opportunity search is a 404, as on every other paginated endpoint, and the body's limit is bounded like the GET query param. - An async-only product no longer documents a 200 OpportunityCollection it can never return. - Every url_for goes through route_name(), which register_route also uses, so one place knows how a prefixed route name is spelled. - get_products builds a Page and uses page_links like every other collection. - Deleted models/root.py, a dead duplicate RootResponse carrying the same conformsTo bug. tests: cover the gaps mutation testing exposed -- malformed CQL2, an unsupported Prefer value, a product declaring no geojson conformance, 3D and mixed-dimension bbox unions, and the self_link regressions. Two weak assertions replaced with ones that can fail. Commentary added by this branch is trimmed throughout.
… carried it OpportunitySearchRecord.request (an OpportunityRequest) becomes search_parameters (a SearchParameters), matching stapi-spec v0.2.0. A record describes what was searched for. Holding the whole request meant every record echoed back whatever limit/next the client happened to page with, so two records describing an identical search differed if the clients paged differently. The field is renamed as well as retyped, matching `search_parameters` on both request objects. Also corrects the OpportunityRequest docstring, which claimed an Opportunity Request could be submitted unmodified as an Order Request. That has been untrue since `limit`/`next` were added -- OrderRequest has neither. What is true holds at the field level: both carry the same SearchParameters.
The changelogs said what changed and marked every break, but a reader had to reconstruct the upgrade from twenty-odd bullets scattered across Added, Changed and Removed. Each version section now opens with the steps, ordered by what a reader does rather than by which section the change belongs to, with the backend protocol changes as before/after signature tables. Also corrects a stale entry: `StapiFastapiBaseRouter.add_api_route` was described as raising ValueError without a summary and merging shared error responses into every route. Neither is true -- `add_api_route` is not overridden, and the shared-error merge was the thing `Route.errors` replaced.
A reference STAPI application and the passes that turn FastAPI's raw OpenAPI output into the document published for the spec: generic operation ids, readable component names, and identical-schema deduplication. Deliberately last in the stack and self-contained. Nothing in the other packages imports it, so this commit can be dropped without touching them -- only the workspace member, the uv source, and the two script package lists come with it.
584e335 to
35d498f
Compare
| - name: Test | ||
| run: ./scripts/run-tests.sh | ||
| - name: Validate test server | ||
| run: uv run scripts/validate-stapi-fastapi |
There was a problem hiding this comment.
Rather than short-circuiting this check in the script so that the script is a noop, I figured it would be better to make the script functional and remove this from CI.
What I'm changing
How I did it
Checklist
./scripts/run-tests.shuv run pre-commit run --all-files