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 f197a2b
Show file tree
Hide file tree
Showing 628 changed files with 93,409 additions and 25,029 deletions.
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
2 changes: 1 addition & 1 deletion k8s-openapi-codegen/src/swagger20/paths.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub struct Operation {
pub kubernetes_group_kind_version: Option<super::KubernetesGroupKindVersion>,
pub parameters: Vec<std::sync::Arc<Parameter>>,
pub path: Path,
pub responses: std::collections::BTreeMap<reqwest::StatusCode, Option<super::Schema>>,
pub responses: std::collections::BTreeMap<reqwest::StatusCode, super::Schema>,
pub tag: String,
}

Expand Down
Loading

0 comments on commit f197a2b

Please sign in to comment.