Pin-13 schema amendment 2: an inherited provider must actually name something
Appends to d76287c, which is left untouched.
That amendment justified IntegrationOwnership::Inherited's provider field
as a claim "a reader can check against the dependency graph rather than
take on trust" -- and then accepted "" and " " for it, in Rust and
through serde alike. An unnamed provider is not a weaker claim of
inheritance. It is the same claim with its evidence removed, and it renders
as Inherited in every table it reaches.
Fixed the way this crate already fixed the identical hole in CheckOutcome's
reason strings: a checked constructor, plus a hand-written Deserialize over
a private wire shadow so the JSON path runs the same check. A hand-edited
report is exactly where an unnamed provider would arrive from, so guarding
only the Rust constructor would have guarded the path nobody takes.
Seven tests. Six fail when the check is reverted to `if false`, verified by
doing it and restoring: the constructor on empty and on whitespace-only,
the deserialize path on both, the same empty provider nested inside a whole
adapter row (the shape a real report carries it in, not just the enum
alone), and the accepting case for a real provider. CandidateOwned keeps
deserializing unchanged.
No candidate report changes: both candidates already name real providers,
so this closes a hole rather than correcting a value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RSX4zSLgKvtiXaPjnMqLGz
This commit is contained in:
parent
d76287cd68
commit
775d7baced
|
|
@ -215,7 +215,7 @@ pub struct DependencyDelta {
|
|||
/// would have survived only as prose in `notes` — which is exactly how the
|
||||
/// two candidates diverged in the first place. So it is typed, required on
|
||||
/// every `Implemented` row, and therefore impossible to omit.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub enum IntegrationOwnership {
|
||||
/// The integration came with a dependency the candidate adopted; the
|
||||
|
|
@ -233,6 +233,66 @@ pub enum IntegrationOwnership {
|
|||
CandidateOwned,
|
||||
}
|
||||
|
||||
impl IntegrationOwnership {
|
||||
/// Checked constructor: rejects an empty-or-whitespace-only provider.
|
||||
///
|
||||
/// The amendment that introduced [`IntegrationOwnership::Inherited`]
|
||||
/// justified it as a claim "a reader can check against the dependency
|
||||
/// graph rather than take on trust" — and then let the field be `""`,
|
||||
/// which is checkable against nothing. An unnamed provider is not a
|
||||
/// weaker claim of inheritance; it is the same claim with its evidence
|
||||
/// removed, and it reads as `Inherited` in every table it appears in.
|
||||
pub fn inherited(provider: impl Into<String>) -> Result<Self, String> {
|
||||
let provider = provider.into();
|
||||
if provider.trim().is_empty() {
|
||||
return Err(
|
||||
"IntegrationOwnership::inherited: provider must not be empty or \
|
||||
whitespace-only — an inherited integration whose provider is unnamed cannot be \
|
||||
checked against the dependency graph, which is the only reason this variant \
|
||||
carries a provider at all"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
Ok(IntegrationOwnership::Inherited { provider })
|
||||
}
|
||||
}
|
||||
|
||||
/// Deserialization shadow for [`IntegrationOwnership`], so the JSON path
|
||||
/// runs the same provider check the constructor does. Without it a
|
||||
/// hand-edited or differently-generated report could carry
|
||||
/// `{"Inherited": {"provider": ""}}` straight past a guard that only ever
|
||||
/// existed in Rust — the same hole, and the same fix, as
|
||||
/// [`crate::outcome::CheckOutcome`]'s reason strings.
|
||||
#[derive(Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
enum IntegrationOwnershipWire {
|
||||
Inherited { provider: String },
|
||||
CandidateOwned,
|
||||
}
|
||||
|
||||
impl TryFrom<IntegrationOwnershipWire> for IntegrationOwnership {
|
||||
type Error = String;
|
||||
|
||||
fn try_from(wire: IntegrationOwnershipWire) -> Result<Self, String> {
|
||||
match wire {
|
||||
IntegrationOwnershipWire::Inherited { provider } => {
|
||||
IntegrationOwnership::inherited(provider)
|
||||
}
|
||||
IntegrationOwnershipWire::CandidateOwned => Ok(IntegrationOwnership::CandidateOwned),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for IntegrationOwnership {
|
||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
let wire = IntegrationOwnershipWire::deserialize(deserializer)?;
|
||||
IntegrationOwnership::try_from(wire).map_err(serde::de::Error::custom)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub enum AdapterStatus {
|
||||
|
|
@ -553,6 +613,78 @@ mod tests {
|
|||
assert!(err.contains("integration_ownership"), "{err}");
|
||||
}
|
||||
|
||||
/// Mutation: the checked constructor refuses an empty provider. If this
|
||||
/// passes, `Inherited`'s "checkable against the dependency graph" claim
|
||||
/// is decorative.
|
||||
#[test]
|
||||
fn the_inherited_constructor_rejects_an_empty_provider() {
|
||||
let err = IntegrationOwnership::inherited("").unwrap_err();
|
||||
assert!(err.contains("provider"), "{err}");
|
||||
assert!(err.contains("dependency graph"), "{err}");
|
||||
}
|
||||
|
||||
/// Whitespace-only is the same hole wearing a space — a provider of
|
||||
/// `" "` renders as `Inherited` in a table and names nothing.
|
||||
#[test]
|
||||
fn the_inherited_constructor_rejects_a_whitespace_only_provider() {
|
||||
assert!(IntegrationOwnership::inherited(" \t \n ").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_inherited_constructor_accepts_a_real_provider() {
|
||||
let own = IntegrationOwnership::inherited("eframe 0.35").unwrap();
|
||||
assert_eq!(
|
||||
own,
|
||||
IntegrationOwnership::Inherited {
|
||||
provider: "eframe 0.35".to_string()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
/// The JSON path must run the same check — a hand-edited report is
|
||||
/// exactly where an unnamed provider would arrive from.
|
||||
#[test]
|
||||
fn deserializing_an_empty_provider_is_refused() {
|
||||
let bad = serde_json::json!({ "Inherited": { "provider": "" } });
|
||||
let err = serde_json::from_value::<IntegrationOwnership>(bad)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(err.contains("provider"), "{err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializing_a_whitespace_only_provider_is_refused() {
|
||||
let bad = serde_json::json!({ "Inherited": { "provider": " " } });
|
||||
assert!(serde_json::from_value::<IntegrationOwnership>(bad).is_err());
|
||||
}
|
||||
|
||||
/// An empty provider must be refused when it arrives nested inside a
|
||||
/// whole adapter row, not only when deserialized on its own — that is
|
||||
/// the shape a real report carries it in.
|
||||
#[test]
|
||||
fn an_adapter_row_with_an_empty_provider_is_refused() {
|
||||
let bad = serde_json::json!({
|
||||
"Implemented": {
|
||||
"platform": "at-spi2",
|
||||
"notes": "n",
|
||||
"integration_ownership": { "Inherited": { "provider": "" } }
|
||||
}
|
||||
});
|
||||
let err = serde_json::from_value::<AdapterStatus>(bad)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(err.contains("provider"), "{err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializing_candidate_owned_still_works() {
|
||||
let ok = serde_json::json!("CandidateOwned");
|
||||
assert_eq!(
|
||||
serde_json::from_value::<IntegrationOwnership>(ok).unwrap(),
|
||||
IntegrationOwnership::CandidateOwned
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn adapter_rows_round_trip_through_json_both_ways() {
|
||||
for a in [
|
||||
|
|
|
|||
Loading…
Reference in New Issue