Skip to content

Commit

Permalink
Don't generate response variants for status codes that don't have an …
Browse files Browse the repository at this point in the history 
…associated type.

The spec does not record any possible HTTP status codes other than
HTTP 200, 201, 202 and 401. But in common operation, many APIs can fail with
HTTP 403, GETs on non-existent resources can fail with HTTP 404, and PUTs on
resources with the wrong resource version can fail with HTTP 409.

This commit ignores all variants that don't have an associated type in the spec.

This commit also changes the `Other` variant to have a
`Result<Option<serde_json::Value>, serde_json::Error>` value. This allows
the variant to handle the common case where the response contains a JSON value,
but still allow the case of non-JSON content that the user should parse from
the response body manually.

Fixes #40
Fixes #41
  • Loading branch information
Arnavion committed Apr 22, 2019
1 parent 2cb747b commit 22d6a71
Show file tree
Hide file tree
Showing 636 changed files with 93,458 additions and 25,071 deletions.
19 changes: 12 additions & 7 deletions build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,20 @@ fn main() -> Result<(), Box<std::error::Error>> {
writeln!(f, "/// let response: CreateCustomResourceDefinitionResponse = unimplemented!();")?;
writeln!(f, "/// let status_code: http::StatusCode = unimplemented!();")?;
writeln!(f, "///")?;
writeln!(f, "/// let custom_resource_definition: CustomResourceDefinition = k8s_match!(response, {{")?;
writeln!(f, "/// k8s_if_1_8!(CreateCustomResourceDefinitionResponse::Other if status_code == http::StatusCode::CREATED => {{")?;
writeln!(f, "/// // Parse response body into a CustomResourceDefinition")?;
writeln!(f, "/// Ok(unimplemented!())")?;
writeln!(f, "/// }}),")?;
writeln!(f, "/// k8s_if_ge_1_9!(CreateCustomResourceDefinitionResponse::Created(custom_resource_definition) => {{")?;
writeln!(f, "/// let custom_resource_definition: CustomResourceDefinition = k8s_match!((response, status_code), {{")?;
writeln!(f, "/// k8s_if_1_8!((CreateCustomResourceDefinitionResponse::Other(value), http::StatusCode::CREATED) => {{")?;
writeln!(f, "/// let value = match value? {{")?;
writeln!(f, "/// Some(value) => value,")?;
writeln!(f, "/// None => unimplemented!(), // Append more response data and try again.")?;
writeln!(f, "/// }};")?;
writeln!(f, "/// let custom_resource_definition = serde::Deserialize::deserialize(value)?;")?;
writeln!(f, "/// Ok(custom_resource_definition)")?;
writeln!(f, "/// }}),")?;
writeln!(f, r#"/// other => Err(format!("unexpected response {{}} {{:?}}", status_code, other)),"#)?;
writeln!(f, "///")?;
writeln!(f, "/// k8s_if_ge_1_9!((CreateCustomResourceDefinitionResponse::Created(custom_resource_definition), _) =>")?;
writeln!(f, "/// Ok(custom_resource_definition)),")?;
writeln!(f, "///")?;
writeln!(f, r#"/// (other, status_code) => Err(format!("unexpected response {{}} {{:?}}", status_code, other)),"#)?;
writeln!(f, "/// }})?;")?;
writeln!(f, "/// #")?;
writeln!(f, "/// # Ok(())")?;
Expand Down
6 changes: 3 additions & 3 deletions k8s-openapi-codegen/src/fixups.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) fn deployment_rollback_create_response_type(spec: &mut crate::swagger

if let Some(operation) = spec.operations.iter_mut().find(|o| o.id == "createAppsV1beta1NamespacedDeploymentRollback") {
for response in operation.responses.values_mut() {
if let Some(crate::swagger20::Schema { kind: crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath(ref_path)), .. }) = response {
if let crate::swagger20::Schema { kind: crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath(ref_path)), .. } = response {
if ref_path == "io.k8s.api.apps.v1beta1.DeploymentRollback" {
std::mem::replace(ref_path, "io.k8s.apimachinery.pkg.apis.meta.v1.Status".to_string());
found = true;
Expand Down Expand Up @@ -527,11 +527,11 @@ pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::S
watch_operation.parameters[watch_index] = watch_parameter.clone();
watch_operation.parameters.swap_remove(std::cmp::max(continue_index, limit_index));
watch_operation.parameters.swap_remove(std::cmp::min(continue_index, limit_index));
watch_operation.responses.insert(reqwest::StatusCode::OK, Some(crate::swagger20::Schema {
watch_operation.responses.insert(reqwest::StatusCode::OK, crate::swagger20::Schema {
description: None,
kind: crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath("io.k8s.apimachinery.pkg.apis.meta.v1.WatchEvent".to_owned())),
kubernetes_group_kind_versions: None,
}));
});

spec.operations[original_list_operation_index] = list_operation;
spec.operations.push(watch_operation);
Expand Down
230 changes: 109 additions & 121 deletions k8s-openapi-codegen/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1391,20 +1391,13 @@ fn write_operation(
_ => return Err(format!("unrecognized status code {}", status_code)),
};

let schema = schema.as_ref();
let is_delete_ok_status = match &schema.kind {
swagger20::SchemaKind::Ref(ref_path) if
&**ref_path == "io.k8s.apimachinery.pkg.apis.meta.v1.Status" &&
operation.method == swagger20::Method::Delete &&
status_code == reqwest::StatusCode::OK => true,

let is_delete_ok_status = if let Some(schema) = schema {
match &schema.kind {
swagger20::SchemaKind::Ref(ref_path) if
&**ref_path == "io.k8s.apimachinery.pkg.apis.meta.v1.Status" &&
operation.method == swagger20::Method::Delete &&
status_code == reqwest::StatusCode::OK => true,

_ => false,
}
}
else {
false
_ => false,
};

Ok((http_status_code, variant_name, schema, is_delete_ok_status))
Expand Down Expand Up @@ -1687,53 +1680,41 @@ fn write_operation(
writeln!(file, "pub enum {} {{", operation_result_name)?;

for &(_, variant_name, schema, is_delete_ok_status) in &operation_responses {
if let Some(schema) = schema {
if is_delete_ok_status {
// DELETE operations that return metav1.Status for HTTP 200 can also return the object itself instead.
//
// Ref https://github.com/kubernetes/kubernetes/issues/59501
writeln!(file, " {}Status({}),", variant_name, get_rust_type(&schema.kind, replace_namespaces, mod_root)?)?;
writeln!(file, " {}Value({}),", variant_name, get_fully_qualified_type_name(
type_name_and_ref_path_and_parent_mod_rs.as_ref()
.map(|(_, type_ref_path, _)| type_ref_path)
.ok_or_else(|| "DELETE-Ok-Status that isn't associated with a type")?,
&replace_namespaces,
mod_root)?)?;
}
else {
match &schema.kind {
crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath(ref_path)) if ref_path == "io.k8s.apimachinery.pkg.apis.meta.v1.WatchEvent" =>
writeln!(
file,
" {}({}<{}>),",
variant_name,
get_rust_type(&schema.kind, replace_namespaces, mod_root)?,
type_name_and_ref_path_and_parent_mod_rs.as_ref()
.map(|(type_name, _, _)| type_name)
.ok_or_else(|| "WatchEvent operation that isn't associated with a type")?)?,

_ => writeln!(file, " {}({}),", variant_name, get_rust_type(&schema.kind, replace_namespaces, mod_root)?)?,
}
}
if is_delete_ok_status {
// DELETE operations that return metav1.Status for HTTP 200 can also return the object itself instead.
//
// Ref https://github.com/kubernetes/kubernetes/issues/59501
writeln!(file, " {}Status({}),", variant_name, get_rust_type(&schema.kind, replace_namespaces, mod_root)?)?;
writeln!(file, " {}Value({}),", variant_name, get_fully_qualified_type_name(
type_name_and_ref_path_and_parent_mod_rs.as_ref()
.map(|(_, type_ref_path, _)| type_ref_path)
.ok_or_else(|| "DELETE-Ok-Status that isn't associated with a type")?,
&replace_namespaces,
mod_root)?)?;
}
else {
writeln!(file, " {},", variant_name)?;
match &schema.kind {
crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath(ref_path)) if ref_path == "io.k8s.apimachinery.pkg.apis.meta.v1.WatchEvent" =>
writeln!(
file,
" {}({}<{}>),",
variant_name,
get_rust_type(&schema.kind, replace_namespaces, mod_root)?,
type_name_and_ref_path_and_parent_mod_rs.as_ref()
.map(|(type_name, _, _)| type_name)
.ok_or_else(|| "WatchEvent operation that isn't associated with a type")?)?,

_ => writeln!(file, " {}({}),", variant_name, get_rust_type(&schema.kind, replace_namespaces, mod_root)?)?,
}
}
}
writeln!(file, " Other,")?;

writeln!(file, " Other(Result<Option<serde_json::Value>, serde_json::Error>),")?;
writeln!(file, "}}")?;
writeln!(file)?;

writeln!(file, "impl crate::Response for {} {{", operation_result_name)?;

let uses_buf = operation_responses.iter().any(|&(_, _, schema, _)| schema.is_some());

if uses_buf {
writeln!(file, " fn try_from_parts(status_code: http::StatusCode, buf: &[u8]) -> Result<(Self, usize), crate::ResponseError> {{")?;
}
else {
writeln!(file, " fn try_from_parts(status_code: http::StatusCode, _: &[u8]) -> Result<(Self, usize), crate::ResponseError> {{")?;
}
writeln!(file, " fn try_from_parts(status_code: http::StatusCode, buf: &[u8]) -> Result<(Self, usize), crate::ResponseError> {{")?;

let is_watch = match operation.kubernetes_action {
Some(swagger20::KubernetesAction::Watch) | Some(swagger20::KubernetesAction::WatchList) => true,
Expand All @@ -1742,80 +1723,87 @@ fn write_operation(

writeln!(file, " match status_code {{")?;
for &(http_status_code, variant_name, schema, is_delete_ok_status) in &operation_responses {
write!(file, " http::StatusCode::{} => ", http_status_code)?;
if let Some(schema) = schema {
writeln!(file, "{{")?;

match &schema.kind {
swagger20::SchemaKind::Ty(swagger20::Type::String { .. }) => {
writeln!(file, " if buf.is_empty() {{")?;
writeln!(file, " return Err(crate::ResponseError::NeedMoreData);")?;
writeln!(file, " }}")?;
writeln!(file)?;
writeln!(file, " let (result, len) = match std::str::from_utf8(buf) {{")?;
writeln!(file, " Ok(s) => (s, buf.len()),")?;
writeln!(file, " Err(err) => match (err.valid_up_to(), err.error_len()) {{")?;
writeln!(file, " (0, Some(_)) => return Err(crate::ResponseError::Utf8(err)),")?;
writeln!(file, " (0, None) => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " (valid_up_to, _) => (")?;
writeln!(file, " unsafe {{ std::str::from_utf8_unchecked(buf.get_unchecked(..valid_up_to)) }},")?;
writeln!(file, " valid_up_to,")?;
writeln!(file, " ),")?;
writeln!(file, " }},")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result.to_string()), len))", operation_result_name, variant_name)?;
},

swagger20::SchemaKind::Ref(_) => if is_watch {
writeln!(file, " let mut deserializer = serde_json::Deserializer::from_slice(buf).into_iter();")?;
writeln!(file, " let (result, byte_offset) = match deserializer.next() {{")?;
writeln!(file, " Some(Ok(value)) => (value, deserializer.byte_offset()),")?;
writeln!(file, " Some(Err(ref err)) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Some(Err(err)) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " None => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result), byte_offset))", operation_result_name, variant_name)?;
}
else if is_delete_ok_status {
writeln!(file, " let result: serde_json::Map<String, serde_json::Value> = match serde_json::from_slice(buf) {{")?;
writeln!(file, " Ok(value) => value,")?;
writeln!(file, " Err(ref err) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Err(err) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " }};")?;
writeln!(file, r#" let is_status = match result.get("kind") {{"#)?;
writeln!(file, r#" Some(serde_json::Value::String(s)) if s == "Status" => true,"#)?;
writeln!(file, " _ => false,")?;
writeln!(file, " }};")?;
writeln!(file, " if is_status {{")?;
writeln!(file, " let result = serde::Deserialize::deserialize(serde_json::Value::Object(result));")?;
writeln!(file, " let result = result.map_err(crate::ResponseError::Json)?;")?;
writeln!(file, " Ok(({}::{}Status(result), buf.len()))", operation_result_name, variant_name)?;
writeln!(file, " }}")?;
writeln!(file, " else {{")?;
writeln!(file, " let result = serde::Deserialize::deserialize(serde_json::Value::Object(result));")?;
writeln!(file, " let result = result.map_err(crate::ResponseError::Json)?;")?;
writeln!(file, " Ok(({}::{}Value(result), buf.len()))", operation_result_name, variant_name)?;
writeln!(file, " }}")?;
}
else {
writeln!(file, " let result = match serde_json::from_slice(buf) {{")?;
writeln!(file, " Ok(value) => value,")?;
writeln!(file, " Err(ref err) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Err(err) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result), buf.len()))", operation_result_name, variant_name)?;
},
writeln!(file, " http::StatusCode::{} => {{", http_status_code)?;

match &schema.kind {
swagger20::SchemaKind::Ty(swagger20::Type::String { .. }) => {
writeln!(file, " if buf.is_empty() {{")?;
writeln!(file, " return Err(crate::ResponseError::NeedMoreData);")?;
writeln!(file, " }}")?;
writeln!(file)?;
writeln!(file, " let (result, len) = match std::str::from_utf8(buf) {{")?;
writeln!(file, " Ok(s) => (s, buf.len()),")?;
writeln!(file, " Err(err) => match (err.valid_up_to(), err.error_len()) {{")?;
writeln!(file, " (0, Some(_)) => return Err(crate::ResponseError::Utf8(err)),")?;
writeln!(file, " (0, None) => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " (valid_up_to, _) => (")?;
writeln!(file, " unsafe {{ std::str::from_utf8_unchecked(buf.get_unchecked(..valid_up_to)) }},")?;
writeln!(file, " valid_up_to,")?;
writeln!(file, " ),")?;
writeln!(file, " }},")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result.to_string()), len))", operation_result_name, variant_name)?;
},

other => return Err(format!("operation {} has unrecognized type for response of variant {}: {:?}", operation.id, variant_name, other).into()),
swagger20::SchemaKind::Ref(_) => if is_watch {
writeln!(file, " let mut deserializer = serde_json::Deserializer::from_slice(buf).into_iter();")?;
writeln!(file, " let (result, byte_offset) = match deserializer.next() {{")?;
writeln!(file, " Some(Ok(value)) => (value, deserializer.byte_offset()),")?;
writeln!(file, " Some(Err(ref err)) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Some(Err(err)) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " None => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result), byte_offset))", operation_result_name, variant_name)?;
}
else if is_delete_ok_status {
writeln!(file, " let result: serde_json::Map<String, serde_json::Value> = match serde_json::from_slice(buf) {{")?;
writeln!(file, " Ok(value) => value,")?;
writeln!(file, " Err(ref err) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Err(err) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " }};")?;
writeln!(file, r#" let is_status = match result.get("kind") {{"#)?;
writeln!(file, r#" Some(serde_json::Value::String(s)) if s == "Status" => true,"#)?;
writeln!(file, " _ => false,")?;
writeln!(file, " }};")?;
writeln!(file, " if is_status {{")?;
writeln!(file, " let result = serde::Deserialize::deserialize(serde_json::Value::Object(result));")?;
writeln!(file, " let result = result.map_err(crate::ResponseError::Json)?;")?;
writeln!(file, " Ok(({}::{}Status(result), buf.len()))", operation_result_name, variant_name)?;
writeln!(file, " }}")?;
writeln!(file, " else {{")?;
writeln!(file, " let result = serde::Deserialize::deserialize(serde_json::Value::Object(result));")?;
writeln!(file, " let result = result.map_err(crate::ResponseError::Json)?;")?;
writeln!(file, " Ok(({}::{}Value(result), buf.len()))", operation_result_name, variant_name)?;
writeln!(file, " }}")?;
}
else {
writeln!(file, " let result = match serde_json::from_slice(buf) {{")?;
writeln!(file, " Ok(value) => value,")?;
writeln!(file, " Err(ref err) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Err(err) => return Err(crate::ResponseError::Json(err)),")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::{}(result), buf.len()))", operation_result_name, variant_name)?;
},

writeln!(file, " }},")?;
}
else {
writeln!(file, "Ok(({}::{}, 0)),", operation_result_name, variant_name)?;
other => return Err(format!("operation {} has unrecognized type for response of variant {}: {:?}", operation.id, variant_name, other).into()),
}

writeln!(file, " }},")?;
}
writeln!(file, " _ => Ok(({}::Other, 0)),", operation_result_name)?;
writeln!(file, " _ => {{")?;
writeln!(file, " let (result, read) =")?;
writeln!(file, " if buf.is_empty() {{")?;
writeln!(file, " (Ok(None), 0)")?;
writeln!(file, " }}")?;
writeln!(file, " else {{")?;
writeln!(file, " match serde_json::from_slice(buf) {{")?;
writeln!(file, " Ok(value) => (Ok(Some(value)), buf.len()),")?;
writeln!(file, " Err(ref err) if err.is_eof() => return Err(crate::ResponseError::NeedMoreData),")?;
writeln!(file, " Err(err) => (Err(err), 0),")?;
writeln!(file, " }}")?;
writeln!(file, " }};")?;
writeln!(file, " Ok(({}::Other(result), read))", operation_result_name)?;
writeln!(file, " }},")?;
writeln!(file, " }}")?;
writeln!(file, " }}")?;
writeln!(file, "}}")?;
Expand Down
13 changes: 9 additions & 4 deletions k8s-openapi-codegen/src/swagger20/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,15 @@ impl<'de> serde::Deserialize<'de> for Spec {
) -> Result<Operation, D::Error> where D: serde::Deserializer<'de> {
let responses: Result<_, _> =
value.responses.into_iter()
.map(|(status_code_str, response)| {
let status_code = status_code_str.parse().map_err(|_|
serde::de::Error::invalid_value(serde::de::Unexpected::Str(&status_code_str), &"string representation of an HTTP status code"))?;
Ok((status_code, response.schema))
.filter_map(|(status_code_str, response)| {
let status_code = match status_code_str.parse() {
Ok(status_code) => status_code,
Err(_) => return Some(Err(serde::de::Error::invalid_value(
serde::de::Unexpected::Str(&status_code_str),
&"string representation of an HTTP status code"))),
};
let schema = response.schema?;
Some(Ok((status_code, schema)))
})
.collect();

Expand Down
Loading

0 comments on commit 22d6a71

Please sign in to comment.