Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Scope enforcement to the response tree, never to "every model type": operation-u
ClickHouse Cloud OpenAPI spec: https://api.clickhouse.cloud/v1

- `.github/workflows/openapi-drift.yml` runs `scripts/check-openapi-drift.py` daily. Python owns fetching, issue rendering, and GitHub orchestration only; `python3 scripts/check-openapi-drift.py --dry-run` reproduces the rendered issue without creating one.
- `crates/clickhouse-openapi-analyzer` is the single implementation of parsing and comparison. `rust_inventory.rs` parses `client.rs`, `models.rs`, and `meta.rs` with `syn`; `openapi.rs` inventories the target spec and vendored snapshot; `compare.rs` maps them and emits typed findings; `config.rs` owns ClickHouse-specific policy; `report.rs` defines the stable JSON/text report; `main.rs` is the executable used by Python. Do not duplicate source parsing, exemptions, or comparison logic in tests or Python.
- `crates/clickhouse-openapi-analyzer` is the single implementation of parsing and comparison. `rust_inventory.rs` walks and parses the module trees rooted at `client.rs`, `models.rs`, and `meta.rs` with `syn`; module cfg evaluation uses the analyzer host target, excludes `test`, treats feature-gated API as enabled, and conservatively retains unknown custom cfgs. `openapi.rs` inventories the target spec and vendored snapshot; `compare.rs` maps them and emits typed findings; `config.rs` owns ClickHouse-specific policy; `report.rs` defines the stable JSON/text report; `main.rs` is the executable used by Python. Do not duplicate source parsing, exemptions, or comparison logic in tests or Python.
- The analyzer is private (`publish = false`) and a dev dependency of `clickhouse-cloud-api`. Parser/tooling dependencies such as `syn` must not enter either published crate's normal dependency graph.
- `crates/clickhouse-cloud-api/tests/spec_coverage_test.rs` analyzes the vendored snapshot; its ignored test analyzes the live spec. Both and the scheduled workflow call the same analyzer and must agree.

Expand Down
10 changes: 5 additions & 5 deletions crates/clickhouse-cloud-api/tests/models_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ where
/// declare `["webhook", "email"]`), so a variant defaulting its discriminator to
/// another variant's value would silently retype the value on the next
/// deserialize. The covered list is enforced structurally, not by convention:
/// it must equal the set of hand-written `impl Default for` blocks in
/// `models.rs` (via the analyzer's `model_types_with_manual_default_impl`), so
/// a new union gaining a `Default` without a list entry fails this test.
/// it must equal the set of hand-written `impl Default for` blocks in the model
/// module tree (via the analyzer's `model_types_with_manual_default_impl`), so a
/// new union gaining a `Default` without a list entry fails this test.
#[test]
fn discriminated_union_defaults_round_trip_to_the_same_variant() {
let mut covered: Vec<&str> = Vec::new();
Expand Down Expand Up @@ -73,7 +73,7 @@ fn discriminated_union_defaults_round_trip_to_the_same_variant() {

covered.sort_unstable();
let manual_default_impls = clickhouse_openapi_analyzer::model_types_with_manual_default_impl(
include_str!("../src/models.rs"),
std::path::Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/src")),
)
.unwrap();
assert_eq!(
Expand All @@ -82,7 +82,7 @@ fn discriminated_union_defaults_round_trip_to_the_same_variant() {
.iter()
.map(String::as_str)
.collect::<Vec<_>>(),
"the covered list must equal the manual `impl Default for` blocks in models.rs"
"the covered list must equal the manual `impl Default for` blocks in the model tree"
);
}

Expand Down
36 changes: 19 additions & 17 deletions crates/clickhouse-cloud-api/tests/spec_coverage_test.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use std::collections::BTreeSet;
use std::path::Path;

use clickhouse_openapi_analyzer::config::clickhouse_cloud_config;
use clickhouse_openapi_analyzer::{
AnalysisInput, analyze, integer_model_fields_typed_as_float, model_fields_with_serde_default,
response_tree,
model_types, response_tree,
};

const SPEC_JSON: &str = include_str!("../clickhouse_cloud_openapi.json");
const CLIENT_RS: &str = include_str!("../src/client.rs");
const MODELS_RS: &str = include_str!("../src/models.rs");
const META_RS: &str = include_str!("../src/meta.rs");
const RUST_SOURCE_ROOT: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src");
const LIVE_SPEC_URL: &str = "https://api.clickhouse.cloud/v1";

#[test]
Expand Down Expand Up @@ -52,7 +51,7 @@ const ALL_OPTION_EXCEPTIONS: &[(&str, &str)] = &[];
/// would itself create `FieldOptionalityMismatch` drift.
#[test]
fn every_response_tree_field_is_option() {
let tree = response_tree(CLIENT_RS, MODELS_RS).unwrap();
let tree = response_tree(Path::new(RUST_SOURCE_ROOT)).unwrap();
assert!(
tree.types.len() >= 300,
"vacuous test: the response tree collapsed to {} types — did client.rs \
Expand All @@ -79,7 +78,7 @@ fn every_response_tree_field_is_option() {
/// `#[serde(skip_serializing_if = "Option::is_none")]`.
#[test]
fn every_response_tree_option_field_omits_none_when_serialized() {
let tree = response_tree(CLIENT_RS, MODELS_RS).unwrap();
let tree = response_tree(Path::new(RUST_SOURCE_ROOT)).unwrap();
assert!(
!tree.types.is_empty(),
"vacuous test: the response tree is empty"
Expand All @@ -100,8 +99,7 @@ fn every_response_tree_option_field_omits_none_when_serialized() {
fn integer_schema_fields_are_not_typed_as_float() {
let offenders = integer_model_fields_typed_as_float(
SPEC_JSON,
CLIENT_RS,
MODELS_RS,
Path::new(RUST_SOURCE_ROOT),
&clickhouse_cloud_config(),
)
.unwrap();
Expand All @@ -111,15 +109,15 @@ fn integer_schema_fields_are_not_typed_as_float() {
);
}

/// `#[serde(default)]` is banned in `models.rs`. On a required request field it
/// fabricates a value (`""`/`0`/`false`) indistinguishable from a genuine
/// `#[serde(default)]` is banned in the model module tree. On a required request
/// field it fabricates a value (`""`/`0`/`false`) indistinguishable from a genuine
/// server-sent one — a consumer doing get → tweak → post would silently persist
/// it (the write-back hazard that sank the superseded issue-312 policy). On
/// response fields it is dead weight: every response-tree field is `Option<T>`
/// (enforced above), where a missing key already deserializes to `None`.
#[test]
fn models_carry_no_serde_default() {
let offenders = model_fields_with_serde_default(MODELS_RS).unwrap();
let offenders = model_fields_with_serde_default(Path::new(RUST_SOURCE_ROOT)).unwrap();
assert!(
offenders.is_empty(),
"remove #[serde(default)] from: {offenders:?}"
Expand All @@ -136,11 +134,17 @@ fn models_carry_no_serde_default() {
/// they return into all-`Option` `{Name}Response` variants before wiring them up.
#[test]
fn scim_models_are_outside_the_response_tree() {
let model_types = model_types(Path::new(RUST_SOURCE_ROOT)).unwrap();
let scim_model_types = model_types
.iter()
.filter(|name| name.starts_with("Scim"))
.collect::<Vec<_>>();
assert!(
MODELS_RS.matches("\npub struct Scim").count() >= 30,
"vacuous test: the SCIM model family is no longer named `Scim*`"
scim_model_types.len() >= 40,
"vacuous test: the SCIM model family collapsed to {} types",
scim_model_types.len()
);
let tree = response_tree(CLIENT_RS, MODELS_RS).unwrap();
let tree = response_tree(Path::new(RUST_SOURCE_ROOT)).unwrap();
let scim_response_types = tree
.types
.iter()
Expand Down Expand Up @@ -179,9 +183,7 @@ fn analyze_spec(
AnalysisInput {
spec_json,
snapshot_json: SPEC_JSON,
client_rs: CLIENT_RS,
models_rs: MODELS_RS,
meta_rs: META_RS,
rust_source_root: Path::new(RUST_SOURCE_ROOT),
},
config,
)
Expand Down
12 changes: 12 additions & 0 deletions crates/clickhouse-openapi-analyzer/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
fn main() {
for (cargo_var, rust_var) in [
("CARGO_CFG_TARGET_ENV", "ANALYZER_TARGET_ENV"),
("CARGO_CFG_TARGET_VENDOR", "ANALYZER_TARGET_VENDOR"),
] {
println!("cargo::rerun-if-env-changed={cargo_var}");
println!(
"cargo::rustc-env={rust_var}={}",
std::env::var(cargo_var).expect("Cargo must provide target cfg values")
);
}
}
79 changes: 52 additions & 27 deletions crates/clickhouse-openapi-analyzer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub mod config;
pub mod report;

use std::collections::BTreeSet;
use std::path::Path;

use config::AnalyzerConfig;
use openapi::OpenApiInventory;
Expand All @@ -37,9 +38,9 @@ use thiserror::Error;
pub struct AnalysisInput<'a> {
pub spec_json: &'a str,
pub snapshot_json: &'a str,
pub client_rs: &'a str,
pub models_rs: &'a str,
pub meta_rs: &'a str,
/// Directory containing the `client`, `models`, and `meta` root modules.
/// Both `<name>.rs` and `<name>/mod.rs` facade layouts are supported.
pub rust_source_root: &'a Path,
}

#[derive(Debug, Error)]
Expand All @@ -48,11 +49,10 @@ pub enum AnalyzeError {
SpecJson(#[source] serde_json::Error),
#[error("failed to parse snapshot OpenAPI JSON: {0}")]
SnapshotJson(#[source] serde_json::Error),
// Covers both syn parse failures and analyzer policy rejections (e.g. a banned
// `rename_all`), which both surface as a `syn::Error`; the source message is
// self-explanatory, so the wrapper text stays neutral rather than claiming a parse failure.
#[error("invalid Rust source: {0}")]
RustSource(#[source] syn::Error),
// Covers module loading, syn parse failures, and source-policy rejections
// such as a banned `rename_all`.
#[error("invalid Rust source tree: {0}")]
RustSource(String),
#[error("invalid target OpenAPI document: {0}")]
SpecInventory(String),
#[error("invalid snapshot OpenAPI document: {0}")]
Expand All @@ -65,8 +65,7 @@ pub fn analyze(
) -> Result<DriftReport, AnalyzeError> {
let spec = serde_json::from_str(input.spec_json).map_err(AnalyzeError::SpecJson)?;
let snapshot = serde_json::from_str(input.snapshot_json).map_err(AnalyzeError::SnapshotJson)?;
let rust = RustInventory::parse(input.client_rs, input.models_rs, input.meta_rs)
.map_err(AnalyzeError::RustSource)?;
let rust = load_rust_inventory(input.rust_source_root)?;
let spec = OpenApiInventory::build(&spec, config).map_err(AnalyzeError::SpecInventory)?;
let snapshot =
OpenApiInventory::build(&snapshot, config).map_err(AnalyzeError::SnapshotInventory)?;
Expand All @@ -93,10 +92,10 @@ pub struct ResponseTree {
pub option_fields_missing_skip_serializing_if: BTreeSet<(String, String)>,
}

/// Computes response-tree membership from the library's `client.rs` and
/// `models.rs` sources.
pub fn response_tree(client_rs: &str, models_rs: &str) -> Result<ResponseTree, AnalyzeError> {
let rust = RustInventory::parse(client_rs, models_rs, "").map_err(AnalyzeError::RustSource)?;
/// Computes response-tree membership from the library's client and model module
/// trees.
pub fn response_tree(rust_source_root: &Path) -> Result<ResponseTree, AnalyzeError> {
let rust = load_rust_inventory(rust_source_root)?;
let types = rust.response_reachable_types();
let mut non_option_fields = BTreeSet::new();
let mut option_fields_missing_skip_serializing_if = BTreeSet::new();
Expand Down Expand Up @@ -128,13 +127,12 @@ pub fn response_tree(client_rs: &str, models_rs: &str) -> Result<ResponseTree, A
/// Rust response tree, so unused schemas do not expand the policy surface.
pub fn integer_model_fields_typed_as_float(
spec_json: &str,
client_rs: &str,
models_rs: &str,
rust_source_root: &Path,
config: &AnalyzerConfig,
) -> Result<BTreeSet<(String, String)>, AnalyzeError> {
let spec = serde_json::from_str(spec_json).map_err(AnalyzeError::SpecJson)?;
let spec = OpenApiInventory::build(&spec, config).map_err(AnalyzeError::SpecInventory)?;
let rust = RustInventory::parse(client_rs, models_rs, "").map_err(AnalyzeError::RustSource)?;
let rust = load_rust_inventory(rust_source_root)?;
let response_types = rust.response_reachable_types();
let mut offenders = BTreeSet::new();

Expand Down Expand Up @@ -162,7 +160,7 @@ pub fn integer_model_fields_typed_as_float(
Ok(offenders)
}

/// Lists every public model struct field in `models_rs` that carries a
/// Lists every public model struct field in the model module tree that carries a
/// field-level `#[serde(default)]` (a container-level one reports every field
/// of its struct), as `StructName.rust_field_name`.
///
Expand All @@ -176,12 +174,19 @@ pub fn integer_model_fields_typed_as_float(
/// because it compares Rust source against a repository policy rather than
/// against the OpenAPI spec. The parsing stays behind this narrow function so
/// `syn` never enters the `clickhouse-cloud-api` dependency graph.
pub fn model_fields_with_serde_default(models_rs: &str) -> Result<Vec<String>, AnalyzeError> {
rust_inventory::model_fields_with_serde_default(models_rs).map_err(AnalyzeError::RustSource)
pub fn model_fields_with_serde_default(
rust_source_root: &Path,
) -> Result<Vec<String>, AnalyzeError> {
Ok(load_rust_inventory(rust_source_root)?.model_fields_with_serde_default())
}

/// Lists every model type in `models_rs` with a hand-written `impl Default
/// for` block, sorted by name.
/// Lists every public struct, enum, and type alias in the model module tree.
pub fn model_types(rust_source_root: &Path) -> Result<BTreeSet<String>, AnalyzeError> {
Ok(load_rust_inventory(rust_source_root)?.model_types)
}

/// Lists every model type in the model module tree with a hand-written `impl
/// Default for` block, sorted by name.
///
/// Backs the completeness half of
/// `discriminated_union_defaults_round_trip_to_the_same_variant` in
Expand All @@ -193,14 +198,32 @@ pub fn model_fields_with_serde_default(models_rs: &str) -> Result<Vec<String>, A
/// [`model_fields_with_serde_default`], this is a repository-policy check
/// rather than a drift `FindingKind`, and it keeps `syn` out of the published
/// crate's dependency graph.
pub fn model_types_with_manual_default_impl(models_rs: &str) -> Result<Vec<String>, AnalyzeError> {
rust_inventory::model_types_with_manual_default_impl(models_rs)
.map_err(AnalyzeError::RustSource)
pub fn model_types_with_manual_default_impl(
rust_source_root: &Path,
) -> Result<Vec<String>, AnalyzeError> {
Ok(load_rust_inventory(rust_source_root)?
.manual_default_impls
.into_iter()
.collect())
}

fn load_rust_inventory(rust_source_root: &Path) -> Result<RustInventory, AnalyzeError> {
RustInventory::load(rust_source_root)
.map_err(|error| AnalyzeError::RustSource(error.to_string()))
}

#[cfg(test)]
mod tests {
use super::*;
use std::fs;

fn source_tree(client: &str, models: &str) -> tempfile::TempDir {
let directory = tempfile::tempdir().unwrap();
fs::write(directory.path().join("client.rs"), client).unwrap();
fs::write(directory.path().join("models.rs"), models).unwrap();
fs::write(directory.path().join("meta.rs"), "").unwrap();
directory
}

#[test]
fn response_tree_reports_membership_and_non_option_fields() {
Expand All @@ -221,7 +244,8 @@ mod tests {
pub struct WidgetLeaf { pub value: Option<String> }
pub struct WidgetPostRequest { pub name: String, pub note: Option<String> }
"#;
let tree = response_tree(client, models).unwrap();
let source = source_tree(client, models);
let tree = response_tree(source.path()).unwrap();
assert_eq!(
tree.types,
BTreeSet::from(["Widget".to_string(), "WidgetLeaf".to_string()])
Expand Down Expand Up @@ -305,8 +329,9 @@ mod tests {
}
"#;

let source = source_tree(client, models);
assert_eq!(
integer_model_fields_typed_as_float(spec, client, models, &AnalyzerConfig::default())
integer_model_fields_typed_as_float(spec, source.path(), &AnalyzerConfig::default())
.unwrap(),
BTreeSet::from([
("Widget".to_string(), "count".to_string()),
Expand Down
13 changes: 2 additions & 11 deletions crates/clickhouse-openapi-analyzer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ struct Args {
#[arg(long)]
snapshot: PathBuf,
#[arg(long)]
client: PathBuf,
#[arg(long)]
models: PathBuf,
#[arg(long)]
meta: PathBuf,
source_root: PathBuf,
}

fn main() {
Expand All @@ -31,16 +27,11 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
let args = Args::parse();
let spec = fs::read_to_string(&args.spec)?;
let snapshot = fs::read_to_string(&args.snapshot)?;
let client = fs::read_to_string(&args.client)?;
let models = fs::read_to_string(&args.models)?;
let meta = fs::read_to_string(&args.meta)?;
let report = analyze(
AnalysisInput {
spec_json: &spec,
snapshot_json: &snapshot,
client_rs: &client,
models_rs: &models,
meta_rs: &meta,
rust_source_root: &args.source_root,
},
&clickhouse_cloud_config(),
)?;
Expand Down
Loading
Loading