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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixed

- Generated clients keep a deprecated field required in the response copy of a type split by direction. When a type's request and response shapes differ, consolidation splits it into `input::X` and `output::X`, and 0.24.0-alpha.1 relaxed `output::X` as a response-only type. Code that sends back a value it read, such as an `output::X` passed where `input::X` is expected, then failed to type-check in TypeScript, because the field was optional in one and required in the other. Such a type is also used in requests, so it now keeps deprecated fields required, like any other type used in both directions.

## 0.24.0-alpha.2

### Fixed
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 46 additions & 1 deletion reflectapi/src/codegen/schema/deprecation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) -> HashSet<R
.output_types
.types()
.map(|t| t.name().to_string())
.filter(|name| !schema.input_types.has_type(name) && !tried.contains(name))
.filter(|name| {
!schema.input_types.has_type(name)
&& !has_input_twin(schema, name)
&& !tried.contains(name)
})
.collect();
let mut relaxed = HashSet::new();
let mut relax = |type_name: &str, variant: Option<&str>, field: &mut Field| {
Expand Down Expand Up @@ -67,6 +71,22 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) -> HashSet<R
relaxed
}

/// Whether `name` is the response copy of a type that consolidation split
/// because its request and response shapes differ (`Schema::consolidate_types`
/// renames them `a::input::X` and `a::output::X`). Such a type is also sent
/// in requests, and clients often send back what they read, so it keeps its
/// deprecated fields required like any other type used in both directions.
fn has_input_twin(schema: &Schema, name: &str) -> bool {
let mut parts: Vec<&str> = name.split("::").collect();
match parts.len().checked_sub(2) {
Some(module) if parts[module] == "output" => {
parts[module] = "input";
schema.input_types.has_type(&parts.join("::"))
}
_ => false,
}
}

/// Types a client decodes by trying them in turn, and every type these
/// reach. A first-match decoder takes the first variant that matches, and a
/// flattened `Option` is `None` only when its type fails to decode, so an
Expand Down Expand Up @@ -351,4 +371,29 @@ mod tests {
[("old".into(), true)]
);
}

#[test]
fn the_response_copy_of_a_split_type_keeps_deprecated_fields() {
let old = serde_json::json!({"name": "old", "required": true, "type": {"name": "u32"}, "deprecation_note": "gone soon"});
let mut schema: Schema = serde_json::from_value(serde_json::json!({
"name": "split",
"input_types": {"types": [
{"kind": "primitive", "name": "u32"},
{"kind": "struct", "name": "rules::input::Split", "fields": {"named": [old.clone()]}}
]},
"output_types": {"types": [
{"kind": "primitive", "name": "u32"},
{"kind": "struct", "name": "rules::output::Split", "fields": {"named": [old.clone()]}},
{"kind": "struct", "name": "replies::output::Lone", "fields": {"named": [old]}}
]}
}))
.unwrap();
let relaxed = super::relax_deprecated_response_fields(&mut schema);
// A client may send back the `output` copy it read as the `input`
// twin; an `output` module without a twin is an ordinary response.
assert_eq!(
relaxed.into_iter().map(|f| f.type_name).collect::<Vec<_>>(),
["replies::output::Lone"]
);
}
}
Loading