feat(relay): add governed SDMX read binding - #702
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>
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>
2b54ade to
7a97a71
Compare
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
7a97a71 to
8460923
Compare
![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: 8460923274
ℹ️ 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".
| if matches!(access, CompiledAccess::Public) { | ||
| for (name, visibility) in [ | ||
| ("resources", self.contract.metadata_visibility.resources), | ||
| ("semantics", self.contract.metadata_visibility.semantics), | ||
| ] { | ||
| if visibility != Visibility::Public { |
There was a problem hiding this comment.
When a statistical dataset is protected, this public-only guard allows metadataVisibility.resources or metadataVisibility.semantics to be operator-only, even though an authorized data caller receives the dataset name, description, components, concepts, and codelist identities from structure_response and the embedded SDMX data structure. Such a contract therefore compiles while mounting metadata explicitly configured to remain operator-only; apply the same operator-only rejection used by validate_metadata_closure for protected consultation operations.
Useful? React with 👍 / 👎.
| fn codelist_urn(dataflow: &CompiledStatisticalDataset, component_id: &str) -> String { | ||
| format!( | ||
| "urn:sdmx:org.sdmx.infomodel.codelist.Codelist={}:CL_{}({})", | ||
| dataflow.sdmx.agency_id, component_id, dataflow.sdmx.version | ||
| ) |
There was a problem hiding this comment.
If two datasets use the same agency/version and share a common component ID such as REF_AREA, this generates the identical CL_REF_AREA URN even when the components reference different governed vocabulary files. The compiler only checks dataflow and DSD endpoint tuples, so this ordinary multi-dataset configuration compiles and publishes two incompatible vocabularies under one SDMX identity, causing consumers to interpret one DSD using the other's codes; include a dataset-specific identity component or reject conflicting generated codelist identities.
Useful? React with 👍 / 👎.
| if (is_range && text.contains(',')) || (!is_range && text.contains('+')) { | ||
| return Err(ProblemCode::StatisticalFeatureUnsupported); | ||
| } | ||
| let separator = if is_range { '+' } else { ',' }; | ||
| for term in text.split(separator) { |
There was a problem hiding this comment.
A reviewed code or string value may contain , or + because governed codelist validation only requires non-empty unique strings, but this parser always interprets commas as value separators and rejects plus signs outside time ranges. URL-encoding cannot escape them because decoding happens before this split, so values such as A,B are silently queried as two other values and A+B is unqueryable; either reject reserved delimiters during compilation or introduce an escaping-aware parser.
Useful? React with 👍 / 👎.
| .map(|(position, component)| { | ||
| json!({ | ||
| "id": component.id, | ||
| "position": position, |
There was a problem hiding this comment.
SDMX data-structure dimension positions are one-based, but enumerate() is emitted directly here, producing ordinary positions 0, 1, ... and a time position of dimensions.len(). Every generated DSD with an ordinary dimension therefore contains an invalid zero position and shifts the declared key order by one, so validating SDMX consumers may reject it or interpret the dimension order incorrectly; emit position + 1 and place the time dimension at dimensions.len() + 1.
Useful? React with 👍 / 👎.
| let id = to_authoring_id(&column.name); | ||
| if attribute_columns.contains(&column.name) { | ||
| attributes.push(( | ||
| id.clone(), |
There was a problem hiding this comment.
Distinct SQLite columns can normalize to the same authoring ID—for example, ref_area and ref-area both become refArea—but both entries are appended without a uniqueness check. OrderedMap::from_entries then serializes duplicate YAML mapping keys, while the strict contract parser explicitly rejects duplicate keys, so relayctl inspect --statistical-view can generate a starter that cannot be consumed by the authoring workflow; report the collision or derive unique deterministic IDs before writing the file.
Useful? React with 👍 / 👎.
| "application/vnd.sdmx.data+json" if version.is_none_or(|value| value == "2.1.0") => { | ||
| Some(DataRepresentation::Json) | ||
| } | ||
| "application/vnd.sdmx.data+csv" if version.is_none_or(|value| value == "2.1.0") => { | ||
| Some(DataRepresentation::Csv) | ||
| } | ||
| "application/json" | "*/*" if version.is_none() => Some(DataRepresentation::Json), | ||
| "text/csv" if version.is_none() => Some(DataRepresentation::Csv), |
There was a problem hiding this comment.
A client sending the valid media range Accept: application/* should match either vendor SDMX application representation, but this negotiation recognizes only exact media types, application/json, and */*, returning 406 instead. The structure negotiator has the same omission, so generic HTTP clients that advertise an application wildcard cannot consume either new SDMX surface; treat application/* as a supported range while retaining the existing quality and version selection.
Useful? React with 👍 / 👎.
| "operationId": format!("{}.sdmx.{}", dataflow.id, kind.replace('/', ".")), | ||
| "x-registry-family": "aggregate-data", | ||
| "x-registry-capability-operation": dataflow.operation_identifier(), | ||
| "security": security, |
There was a problem hiding this comment.
For a protected dataset, the runtime authorizes both structure routes with the dataset's exact compiled scope, but these generated OpenAPI operations only declare generic bearer security. The corresponding data operation emits x-registry-required-scope, so the full deployment contract currently omits the only machine-readable indication of which scope a client needs for dataflow and DSD reads; add the same required-scope extension to each protected structure operation.
Useful? React with 👍 / 👎.
| fn value_document(value: &SqlValue) -> Value { | ||
| let value = csv_value(Some(value)).unwrap_or_default(); | ||
| json!({"id": value, "name": value}) |
There was a problem hiding this comment.
This renders every dimension and coded-attribute value as an SDMX id, but supported decimal and free-string dimensions can contain values such as 1.5 or North East that do not satisfy the SDMX identifier pattern. Those contracts and source rows pass compilation and runtime validation, yet the resulting SDMX-JSON fails its advertised 2.1 schema; emit the typed value form for non-identifier component values and validate any values that are deliberately emitted as code IDs.
Useful? React with 👍 / 👎.
| for (suffix, operator) in [("lower", ">="), ("upper", "<=")] { | ||
| let bound_present = format!("dimension_{time_index}_{suffix}_present"); | ||
| let bound = format!("dimension_{time_index}_{suffix}"); | ||
| parameters.push(parameter(&bound_present)); | ||
| parameters.push(parameter(&bound)); | ||
| predicates.push(format!( | ||
| "(:{bound_present} = 0 OR {} {operator} :{bound})", | ||
| quote_identifier(&dataflow.time.source_column) |
There was a problem hiding this comment.
The accepted time grammar permits annual, quarterly, monthly, and daily periods in the same dataset, but these predicates compare their raw text in SQLite. Mixed-granularity ranges therefore return incorrect observations—for example, 2024-10 sorts before 2024-Q2 even though October is later—while the request is reported as successful; normalize periods to comparable temporal bounds or require and enforce one homogeneous period format before using lexical comparisons.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a governed SDMX REST read binding to Relay V2 as a stacked change on #699.
/sdmx/v2relayctl inspect --statistical-viewscaffolding that requires explicit view, time, and measure columns and never samples values or guesses codelistsThis does not add dynamic aggregation, arbitrary operators, schema or availability behavior, maintenance/history APIs, streaming, a generic storage abstraction, or
registryctlcompatibility work. Schema and availability paths are reserved with a value-free501. This is standards-alignment work, not a conformance or certification claim.Security and correctness
no-store, and public OpenAPI excludes protected datasetsA fresh clean-context staff/security review completed with GO and no P0/P1 findings.
Verification
cargo fmt --all -- --checkcargo check --locked -p registry-relay-v2 --all-targets --features toolingcargo check --locked -p registry-relayctl --all-targetscargo clippy --locked -p registry-relay-v2 --all-targets --features tooling -- -D warningscargo clippy --locked -p registry-relayctl --all-targets -- -D warningscargo test --locked -p registry-relay-v2 --features toolingcargo test --locked -p registry-relayctlproducts/relay-v2/scripts/check-generated.shproducts/relay-v2/scripts/check-contracts.shproducts/relay-v2/scripts/test-http.shpython3 .github/scripts/test_ci_changes.pyPost-rebase verification reran all eight real-router SDMX tests and the full Relay V2 product/adopter contract gate.