feat(relay): clarify access profiles and add named spatial search - #700
feat(relay): clarify access profiles and add named spatial search#700jeremi wants to merge 24 commits into
Conversation
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee04e649e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [dev-dependencies] | ||
| async-trait.workspace = true | ||
| futures.workspace = true | ||
| jsonschema.workspace = true |
There was a problem hiding this comment.
The reviewed commit has no Signed-off-by: trailer, so it does not satisfy the repository's mandatory DCO policy and will be rejected by DCO enforcement; recreate or squash the commit with git commit -s.
AGENTS.md reference: AGENTS.md:L266-L266
Useful? React with 👍 / 👎.
| } else if geojson && supports_representation(operation, ContractRepresentation::Geojson) { | ||
| Ok(WireRepresentation::GeoJson(GeoJsonProfile::Rfc7946)) | ||
| } else if json && supports_representation(operation, ContractRepresentation::Json) { | ||
| Ok(WireRepresentation::Json) |
There was a problem hiding this comment.
When a spatial client sends a weighted fallback such as Accept: application/json;q=1, application/geo+json;q=0.1, negotiation records only booleans and this fixed ordering selects GeoJSON despite JSON having the higher quality. That unexpectedly changes the response envelope for clients that explicitly prefer JSON; rank the operation's acceptable representations by their parsed quality and header order instead.
Useful? React with 👍 / 👎.
| json!(operation | ||
| .schema_reference | ||
| .strip_suffix(".schema.json") | ||
| .map(|base| format!("{base}.geojson.schema.json")) | ||
| .unwrap_or_else(|| format!("{}.geojson", operation.schema_reference))), |
There was a problem hiding this comment.
For generated operation references such as /v2/artifacts/registered-premises--read-schema, the suffix test fails and assigns the GeoJSON schema an $id ending in -schema.geojson. The artifact is actually served under the identifier registered-premises--read-geojson-schema, so consumers resolving the schema's declared identity receive a nonexistent artifact; derive $id from the generated GeoJSON artifact identifier instead.
Useful? React with 👍 / 👎.
| let longitude_span = bbox.east - bbox.west; | ||
| let latitude_span = bbox.north - bbox.south; | ||
| if longitude_span > f64::from(spatial.maximum_longitude_span_degrees) | ||
| || latitude_span > f64::from(spatial.maximum_latitude_span_degrees) |
There was a problem hiding this comment.
When a publisher sets a one-degree maximum, a mathematically exact boundary box such as bbox=-128.8,0,-127.8,1 is rejected because the parsed f64 subtraction produces 1.0000000000000142. This makes valid requests at the documented inclusive span limit fail depending on their coordinate values; validate the decimal span without binary-rounding error, and keep the mirrored PointBbox::is_within check consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 368d3295be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let Some(value) = headers.get(ACCEPT) else { | ||
| return Ok(Representation::Json); | ||
| return supports_representation(operation, ContractRepresentation::Json) | ||
| .then_some(WireRepresentation::Json) | ||
| .ok_or(ProblemCode::UnsupportedRepresentation); |
There was a problem hiding this comment.
When an operation explicitly declares only representations: [geojson] or [json-ld], which compile_representations permits, a request without an Accept header should accept any available representation, but this branch tries only JSON and returns 406. Generic clients sending Accept: */* or application/* fail similarly because those wildcards only enable JSON; choose among the operation's declared formats for absent or wildcard Accept values.
Useful? React with 👍 / 👎.
| - Relay V2 bounded Point collection responses | ||
| evidence_docs: | ||
| - label: Relay V2 spatial response acceptance tests | ||
| url: https://github.com/registrystack/registry-stack/blob/ee04e649e78c78b17ea9460f49c4f0e2906b39b9/crates/registry-relay-v2/tests/acceptance_http.rs |
There was a problem hiding this comment.
The four new GeoJSON/JSON-FG evidence URLs pin commit ee04e649..., but ancestry checks against both pushed main and reviewed commit ba0edfd1... fail; that SHA exists only in the local review history. After the change is squashed or merged these public documentation links can return 404, including their generated JSON copies, so replace the source YAML URLs with stable, main-reachable evidence and regenerate the data.
AGENTS.md reference: AGENTS.md:L269-L271
Useful? React with 👍 / 👎.
| if let Some(spatial_query) = &list.spatial_query { | ||
| let Some(geometry) = primary_geometry else { | ||
| self.error( | ||
| "list.spatial_query_without_geometry", | ||
| &format!("{location}.spatialQuery"), |
There was a problem hiding this comment.
For a list declaring spatialQuery.bbox with only JSON/JSON-LD representations, this branch checks that a non-personal primary geometry exists but never checks that the selected disclosure profile contains it; the only such membership check is conditional on opting into GeoJSON. The compiler therefore accepts a contract that exposes record membership through bbox tests over a geometry excluded from the operation's stated disclosure maximum, contrary to the documented requirement in docs/site/src/content/docs/configure/relay.mdx; validate disclosure membership whenever bbox is enabled, independently of representation choice.
AGENTS.md reference: AGENTS.md:L64-L65
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
368d329 to
bc73b88
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc73b883ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Err(ProblemCode::FieldsInvalid); | ||
| } | ||
| } | ||
| "profile" => { |
There was a problem hiding this comment.
When an authored exact filter is named profile or bbox, compilation still accepts it because RESERVED_PARAMETERS contains neither name, but these new branches intercept the parameter before declared filters are processed. Such a contract therefore generates an unreachable filter (and may also emit duplicate OpenAPI parameters); reject both names during compilation as the Relay V2 product contract requires.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
| "semanticModelReference": representation.semantic_model_reference, | ||
| "contextReference": representation.context_reference, | ||
| "formats": response_format_documents(resource, representation), | ||
| "spatialQuery": operation.query.spatial_bbox.as_ref().map(|spatial| json!({ |
There was a problem hiding this comment.
For a list with spatial_bbox, the compiler sets operation.pattern to Search, and runtime metadata plus OpenAPI advertise consultation.search; however, capability_inventory still derives pattern from OperationKind::List, so every generated public, full, or operation-bound capability artifact emits pattern: list beside this new spatialQuery. Use the compiled pattern here so generated discovery agrees with the bounded-point-search contract.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
| an access or row boundary. | ||
| Registry Core remains present. | ||
|
|
||
| ## Add a bounded Point profile |
There was a problem hiding this comment.
This commit combines runtime changes under crates/, product-contract changes under products/, and public-site changes under docs/site/, despite the repository requiring each change to remain within one owning area. Split these into separately scoped commits so each area's review and verification boundary remains enforceable.
AGENTS.md reference: AGENTS.md:L272-L274
Useful? React with 👍 / 👎.
| parameters.push(json!({ | ||
| "name": "bbox", | ||
| "in": "query", | ||
| "required": false, |
There was a problem hiding this comment.
For a spatial list with no equality filters and allowUnfiltered: false—including the new registered-premises acceptance contract—the runtime rejects every request that omits bbox, but the generated OpenAPI declares this parameter optional. Generated clients and readers can therefore issue requests that the published contract says are valid but Relay always rejects; set required when bbox is the only reachable query constraint, or otherwise encode that requirement in the operation contract.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
jeremi
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15ba97b1e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ), | ||
| }, | ||
| properties, | ||
| primary_geometry, |
There was a problem hiding this comment.
When a contract uses the new primaryGeometry and its semanticTerm or sourceRequired value changes after classification review, this compiled field is absent from classification_inventory_digest's ResourceInventory, while classification_inventory_report likewise enumerates only resource.properties. Because the carrier columns and disclosure profile can remain unchanged, the inventory digest does not change and a stale ClassificationReview can still pass production compilation even though the published geometry meaning or requiredness changed; include the geometry's governed binding in both canonical inventory paths.
Useful? React with 👍 / 👎.
| "the primary geometry name must be URL-safe camelCase", | ||
| ); | ||
| } | ||
| if property_names.contains(geometry.name.as_str()) { |
There was a problem hiding this comment.
When primaryGeometry.name is a valid camel-case transport name such as registryIdentifier, recordIdentifier, domainData, items, or meta, this check accepts it because it only tests scalar-property collisions. json_ld_context later inserts the geometry term into the same context map as those Registry Core and envelope terms, so one insertion overwrites the other and JSON-LD consumers interpret either the core value or geometry under the wrong semantic mapping; reject geometry names that collide with the generated JSON-LD vocabulary.
Useful? React with 👍 / 👎.
| if matches!(operation.kind, OperationKind::List) | ||
| && representation.disclosure_handling >= Handling::Confidential | ||
| && access_profile.disclosure_handling >= Handling::Confidential |
There was a problem hiding this comment.
When a protected named Point-bbox search discloses confidential data, compilation permits it, but this contextual-review condition only matches OperationKind::List. The generated review findings therefore omit the nonpublic collection-disclosure prompt for the newly added search operation even though the corresponding restricted-data compiler checks explicitly treat lists and searches alike; include OperationKind::Search here or emit an equivalent search-specific finding.
Useful? React with 👍 / 👎.
1 participant
What changed
This refines Relay V2 around four distinct concepts and adds a deliberately bounded spatial capability:
fieldscan only reduce the selected access profile.AcceptandformatProfileselect serialization only.The business-registry example now exposes a separately authorized named Point-bbox search at
/v2/resources/{resource}/searches/{search}. It supports governed JSON, JSON-LD, RFC 7946 GeoJSON, and the bounded JSON-FG profile over the same Registry Records. List and search access remain independent.relayctl check --production --explainnow returns a deterministic, value-free explanation of each operation, access profile, query capability, transform, handling floor, cache posture, and wire format.relayctl generatewrites the same canonicaloperation-explanation.jsonbytes.Product boundary
The spatial cut stays intentionally narrow: one reviewed Point per resource, CRS84, one fixed inclusive bounded bbox predicate, named Consultation search, and read-only SQLite comparisons. It does not add OGC API Features, CQL2, EDR, tiles, spatial joins, reprojection, GeoPackage decoding, SpatiaLite, extension loading, generic geometry, dynamic policies, or media-type-specific access rights.
Spatial output inherits the selected access profile's public or protected posture, exact OAuth scope, optional purpose, optional principal or verified-claim row binding, disclosure profile, field minimization, audit, quota, and cache rules.
Contract and security notes
representation,defaultRepresentation, single-profile, and listspatialQueryforms are rejected without aliases.bboxis reserved for named spatial search and cannot compile as an unusable ordinary list filter.no-store.Generated and public documentation
bboxblock cursor-only requests.inclusive-point-within-bboxpredicate identifier.Verification
cargo fmt --checkcargo test --locked -p registry-relay-v2 --all-featurescargo test --locked -p registry-relayctlcargo clippy --locked -p registry-relay-v2 -p registry-relayctl --all-targets --all-features -- -D warningsproducts/relay-v2/scripts/check-contracts.shproducts/relay-v2/scripts/check-generated.shcargo deny checkcd docs/site && npm run checkClean-context tutorial, security, and staff-engineering reviews were run after the final remediation.