Consolidate all list and watch operation optional parameters into a s…

…ingle type.

Fixes #36

README.md

@@ -53,7 +53,7 @@ This crate moves all optional parameters to separate structs, one for each API.
 list_namespaced_pod("kube-system", Default::default());
 
 // List all pods in the kube-system namespace with label foo=bar
-list_namespaced_pod("kube-system", ListNamespacedPodOptional { label_selector: Some("foo=bar"), ..Default::default());
+list_namespaced_pod("kube-system", ListOptional { label_selector: Some("foo=bar"), ..Default::default());
 ```
 
 The second example uses struct update syntax to explicitly set one field of the struct and `Default` the rest.

k8s-openapi-codegen/src/fixups.rs

@@ -393,9 +393,14 @@ pub(crate) fn remove_delete_options_body_parameter(spec: &mut crate::swagger20::
 ///
 /// This also helps solve the problem that the default list operation's response type is a list type, which would be incorrect if the user called the function
 /// with the `watch` parameter set. Thus it's applied even to those list operations which don't have corresponding deprecated watch or watchlist operations.
+///
+/// This fixup also synthesizes mod-root-level `ListOptional` and `WatchOptional` types which have the common parameters of all list and watch operations respectively.
 pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::Spec) -> Result<(), crate::Error> {
 	use std::fmt::Write;
 
+	let mut list_optional_parameters: Option<std::collections::HashSet<String>> = None;
+	let mut list_optional_definition: std::collections::BTreeMap<crate::swagger20::PropertyName, crate::swagger20::Schema> = Default::default();
+	let mut watch_optional_definition: std::collections::BTreeMap<crate::swagger20::PropertyName, crate::swagger20::Schema> = Default::default();
 	let mut list_operations = vec![];
 
 	for operation in &spec.operations {
@@ -404,27 +409,77 @@ pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::S
 		}
 
 		if !operation.id.starts_with("list") {
-			continue;
+			return Err(format!(r#"operation {} is a list operation but doesn't start with "list""#, operation.id).into());
 		}
 
-		let watch_index = match operation.parameters.iter().position(|p| p.name == "watch") {
-			Some(watch_index) => watch_index,
-			None => continue,
-		};
+		let list_optional_parameters =
+			&*list_optional_parameters.get_or_insert_with(|| operation.parameters.iter().map(|p| p.name.clone()).collect());
 
-		let continue_index = match operation.parameters.iter().position(|p| p.name == "continue") {
-			Some(continue_index) => continue_index,
-			None => return Err(format!("operation {} is a list operation with a watch parameter but doesn't have a continue parameter", operation.id).into()),
-		};
+		for expected_parameter_name in list_optional_parameters {
+			let expected_parameter =
+				if let Some(expected_parameter) = operation.parameters.iter().find(|p| p.name == *expected_parameter_name && !p.required) {
+					&**expected_parameter
+				}
+				else {
+					return Err(format!("operation {} is a list operation but doesn't have a {} parameter", operation.id, expected_parameter_name).into());
+				};
+
+			if expected_parameter_name != "watch" {
+				list_optional_definition
+					.entry(crate::swagger20::PropertyName(expected_parameter_name.to_owned()))
+					.or_insert_with(|| expected_parameter.clone().schema);
+			}
+
+			if expected_parameter_name != "continue" && expected_parameter_name != "limit" && expected_parameter_name != "watch" {
+				watch_optional_definition
+					.entry(crate::swagger20::PropertyName(expected_parameter_name.to_owned()))
+					.or_insert_with(|| expected_parameter.clone().schema);
+			}
+		}
+
+		for parameter in &operation.parameters {
+			if !parameter.required && !list_optional_parameters.contains(&*parameter.name) {
+				return Err(format!("operation {} contains unexpected optional parameter {}", operation.id, parameter.name).into());
+			}
+		}
+
+		let continue_index = operation.parameters.iter().position(|p| p.name == "continue").unwrap();
+		let limit_index = operation.parameters.iter().position(|p| p.name == "limit").unwrap();
+		let watch_index = operation.parameters.iter().position(|p| p.name == "watch").unwrap();
 
-		list_operations.push((operation.id.to_owned(), watch_index, continue_index));
+		list_operations.push((operation.id.to_owned(), continue_index, limit_index, watch_index));
 	}
 
 	if list_operations.is_empty() {
 		return Err("never found any list-watch operations".into());
 	}
 
-	for (list_operation_id, watch_index, continue_index) in list_operations {
+	spec.definitions.insert(crate::swagger20::DefinitionPath("io.k8s.ListOptional".to_string()), crate::swagger20::Schema {
+		description: Some("Common parameters for all list operations.".to_string()),
+		kind: crate::swagger20::SchemaKind::ListOptional(list_optional_definition),
+		kubernetes_group_kind_versions: None,
+	});
+
+	spec.definitions.insert(crate::swagger20::DefinitionPath("io.k8s.WatchOptional".to_string()), crate::swagger20::Schema {
+		description: Some("Common parameters for all watch operations.".to_string()),
+		kind: crate::swagger20::SchemaKind::WatchOptional(watch_optional_definition),
+		kubernetes_group_kind_versions: None,
+	});
+
+	let watch_parameter = std::sync::Arc::new(crate::swagger20::Parameter {
+		location: crate::swagger20::ParameterLocation::Query,
+		name: "watch".to_string(),
+		required: true,
+		schema: crate::swagger20::Schema {
+			description: None,
+			kind: crate::swagger20::SchemaKind::Watch,
+			kubernetes_group_kind_versions: None,
+		},
+	});
+
+	let mut converted_watch_operations: std::collections::HashSet<_> = Default::default();
+
+	for (list_operation_id, continue_index, limit_index, watch_index) in list_operations {
 		let watch_operation_id = list_operation_id.replacen("list", "watch", 1);
 		let watch_list_operation_id =
 			if watch_operation_id.ends_with("ForAllNamespaces") {
@@ -441,17 +496,6 @@ pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::S
 			spec.operations.swap_remove(watch_list_operation_index);
 		}
 
-		let watch_parameter = std::sync::Arc::new(crate::swagger20::Parameter {
-			location: crate::swagger20::ParameterLocation::Query,
-			name: "watch".to_string(),
-			required: true,
-			schema: crate::swagger20::Schema {
-				description: None,
-				kind: crate::swagger20::SchemaKind::Watch,
-				kubernetes_group_kind_versions: None,
-			},
-		});
-
 		let (original_list_operation_index, original_list_operation) = spec.operations.iter().enumerate().find(|(_, o)| o.id == list_operation_id).unwrap();
 
 		let mut base_description = original_list_operation.description.as_ref().map_or("", std::ops::Deref::deref).to_owned();
@@ -476,12 +520,13 @@ pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::S
 				writeln!(description, "This operation only supports watching one item, or a list of items, of this type for changes.")?;
 				description
 			}),
-			id: watch_operation_id,
-			kubernetes_action: Some(crate::swagger20::KubernetesAction::WatchList),
+			id: watch_operation_id.clone(),
+			kubernetes_action: Some(crate::swagger20::KubernetesAction::Watch),
 			..original_list_operation.clone()
 		};
 		watch_operation.parameters[watch_index] = watch_parameter.clone();
-		watch_operation.parameters.swap_remove(continue_index);
+		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 {
 			description: None,
 			kind: crate::swagger20::SchemaKind::Ref(crate::swagger20::RefPath("io.k8s.apimachinery.pkg.apis.meta.v1.WatchEvent".to_owned())),
@@ -490,6 +535,19 @@ pub(crate) fn separate_watch_from_list_operations(spec: &mut crate::swagger20::S
 
 		spec.operations[original_list_operation_index] = list_operation;
 		spec.operations.push(watch_operation);
+		converted_watch_operations.insert(watch_operation_id);
+	}
+
+	for operation in &spec.operations {
+		match operation.kubernetes_action {
+			Some(crate::swagger20::KubernetesAction::Watch) |
+			Some(crate::swagger20::KubernetesAction::WatchList) =>
+				if !converted_watch_operations.contains(&operation.id) {
+					return Err(format!("found a watch operation that wasn't synthesized from a list operation: {:?}", operation).into());
+				},
+
+			_ => (),
+		}
 	}
 
 	Ok(())

k8s-openapi-codegen/src/main.rs

@@ -731,6 +731,65 @@ fn run(supported_version: supported_version::SupportedVersion, out_dir_base: &st
 					num_generated_type_aliases += 1;
 				},
 
+				swagger20::SchemaKind::ListOptional(properties) |
+				swagger20::SchemaKind::WatchOptional(properties) => {
+					struct Property<'a> {
+						schema: &'a swagger20::Schema,
+						field_name: std::borrow::Cow<'static, str>,
+						field_type_name: String,
+					}
+
+					let properties = {
+						let mut result = Vec::with_capacity(properties.len());
+
+						for (name, schema) in properties {
+							let field_name = get_rust_ident(&name);
+
+							let type_name = get_rust_borrow_type(&schema.kind, &replace_namespaces, mod_root)?;
+
+							let field_type_name =
+								if type_name.starts_with('&') {
+									format!("Option<&'a {}>", &type_name[1..])
+								}
+								else {
+									format!("Option<{}>", type_name)
+								};
+
+							result.push(Property {
+								schema,
+								field_name,
+								field_type_name,
+							});
+						}
+
+						result
+					};
+
+					writeln!(file, "#[derive(Clone, Copy, Debug, Default, PartialEq)]")?;
+					writeln!(file, "pub struct {}<'a> {{", type_name)?;
+
+					for (i, Property { schema, field_name, field_type_name, .. }) in properties.iter().enumerate() {
+						if i > 0 {
+							writeln!(file)?;
+						}
+
+						if let Some(ref description) = schema.description {
+							for line in get_comment_text(description, "") {
+								writeln!(file, "    ///{}", line)?;
+							}
+						}
+
+						write!(file, "    pub {}: ", field_name)?;
+
+						write!(file, "{}", field_type_name)?;
+
+						writeln!(file, ",")?;
+					}
+					writeln!(file, "}}")?;
+
+					num_generated_structs += 1;
+				},
+
 				swagger20::SchemaKind::Watch => unreachable!(),
 			}
 
@@ -782,6 +841,7 @@ fn run(supported_version: supported_version::SupportedVersion, out_dir_base: &st
 }
 
 fn can_be_default(kind: &swagger20::SchemaKind, spec: &swagger20::Spec) -> Result<bool, Error> {
+	#[allow(clippy::match_same_arms)]
 	match kind {
 		swagger20::SchemaKind::Properties(properties) => {
 			for (schema, required) in properties.values() {
@@ -810,6 +870,9 @@ fn can_be_default(kind: &swagger20::SchemaKind, spec: &swagger20::Spec) -> Resul
 
 		swagger20::SchemaKind::Ty(_) => Ok(true),
 
+		swagger20::SchemaKind::ListOptional(_) |
+		swagger20::SchemaKind::WatchOptional(_) => Ok(true),
+
 		swagger20::SchemaKind::Watch => unreachable!(),
 	}
 }
@@ -1002,6 +1065,9 @@ fn get_rust_borrow_type(
 		swagger20::SchemaKind::Ty(swagger20::Type::JSONSchemaPropsOrBool) |
 		swagger20::SchemaKind::Ty(swagger20::Type::JSONSchemaPropsOrStringArray) => Err("JSON schema types not supported".into()),
 
+		swagger20::SchemaKind::ListOptional(_) => Err("ListOptional type not supported".into()),
+		swagger20::SchemaKind::WatchOptional(_) => Err("WatchOptional type not supported".into()),
+
 		swagger20::SchemaKind::Watch => Ok("".into()), // Value is unused since this parameter is implicit
 	}
 }
@@ -1040,6 +1106,9 @@ fn get_rust_type(
 		swagger20::SchemaKind::Ty(swagger20::Type::JSONSchemaPropsOrBool) |
 		swagger20::SchemaKind::Ty(swagger20::Type::JSONSchemaPropsOrStringArray) => Err("JSON schema types not supported".into()),
 
+		swagger20::SchemaKind::ListOptional(_) => Err("ListOptional type not supported".into()),
+		swagger20::SchemaKind::WatchOptional(_) => Err("WatchOptional type not supported".into()),
+
 		swagger20::SchemaKind::Watch => unreachable!(),
 	}
 }
@@ -1077,7 +1146,7 @@ fn write_operation(
 	writeln!(file, "// Generated from operation {}", operation.id)?;
 
 	let (operation_fn_name, operation_result_name, operation_optional_parameters_name) =
-		get_operation_names(operation, type_name_and_ref_path_and_parent_mod_rs.is_some())?;
+		get_operation_names(operation, type_name_and_ref_path_and_parent_mod_rs.is_some(), mod_root)?;
 
 	let operation_responses: Result<Vec<_>, _> =
 		operation.responses.iter()
@@ -1328,15 +1397,23 @@ fn write_operation(
 	}
 
 	if let Some((_, _, parent_mod_rs)) = type_name_and_ref_path_and_parent_mod_rs {
-		if optional_parameters.is_empty() {
+		if
+			optional_parameters.is_empty() ||
+			operation.kubernetes_action == Some(swagger20::KubernetesAction::List) ||
+			operation.kubernetes_action == Some(swagger20::KubernetesAction::Watch)
+		{
 			writeln!(parent_mod_rs, "    {},", operation_result_name)?;
 		}
 		else {
 			writeln!(parent_mod_rs, "    {}, {},", operation_optional_parameters_name, operation_result_name)?;
 		}
 	}
 
-	if !optional_parameters.is_empty() {
+	if
+		!optional_parameters.is_empty() &&
+		operation.kubernetes_action != Some(swagger20::KubernetesAction::List) &&
+		operation.kubernetes_action != Some(swagger20::KubernetesAction::Watch)
+	{
 		writeln!(file)?;
 
 		if let Some((type_name, _, _)) = type_name_and_ref_path_and_parent_mod_rs {
@@ -1510,6 +1587,7 @@ fn write_operation(
 fn get_operation_names(
 	operation: &swagger20::Operation,
 	strip_tag: bool,
+	mod_root: &str,
 ) -> Result<(std::borrow::Cow<'static, str>, String, String), Error> {
 	let operation_id =
 		if strip_tag {
@@ -1543,7 +1621,11 @@ fn get_operation_names(
 	let first_char = chars.next().ok_or_else(|| format!("operation has empty ID: {:?}", operation))?.to_uppercase();
 	let rest_chars = chars.as_str();
 	let operation_result_name = format!("{}{}Response", first_char, rest_chars);
-	let operation_optional_parameters_name = format!("{}{}Optional", first_char, rest_chars);
+	let operation_optional_parameters_name = match operation.kubernetes_action {
+		Some(swagger20::KubernetesAction::List) => format!("crate::{}::ListOptional", mod_root),
+		Some(swagger20::KubernetesAction::Watch) => format!("crate::{}::WatchOptional", mod_root),
+		_ => format!("{}{}Optional", first_char, rest_chars),
+	};
 
 	Ok((operation_fn_name, operation_result_name, operation_optional_parameters_name))
 }

k8s-openapi-codegen/src/swagger20/definitions.rs

@@ -161,6 +161,10 @@ pub enum SchemaKind {
 	Ref(RefPath),
 	Ty(Type),
 
+	/// Special types for common parameters of list and watch operations
+	ListOptional(std::collections::BTreeMap<PropertyName, Schema>),
+	WatchOptional(std::collections::BTreeMap<PropertyName, Schema>),
+
 	/// Special type for implicit watch parameter of a watch operation
 	Watch,
 }

k8s-openapi-tests/src/job.rs

@@ -153,7 +153,7 @@ fn create() {
 
 		// Delete all pods of the job using label selector
 		let (request, response_body) =
-			api::Pod::list_namespaced_pod("default", api::ListNamespacedPodOptional {
+			api::Pod::list_namespaced_pod("default", k8s_openapi::ListOptional {
 				label_selector: Some("job-name=k8s-openapi-tests-create-job"),
 				..Default::default()
 			})

src/lib.rs

@@ -67,10 +67,14 @@
 //! 1. The API function has required parameters and optional parameters. All optional parameters are taken as a single struct with optional fields.
 //!
 //!    Specifically for the [`api::core::v1::Pod::list_namespaced_pod`] operation, the `namespace` parameter is required and taken by the function itself,
-//!    while other optional parameters like `field_selector` are fields of the [`api::core::v1::ListNamespacedPodOptional`] struct. An instance of
+//!    while other optional parameters like `field_selector` are fields of the [`ListOptional`] struct. An instance of
 //!    this struct is taken as the last parameter of `Pod::list_namespaced_pod`. This struct impls [`Default`] so that you can just pass in `Default::default()`
 //!    if you don't want to specify values for any of the optional parameters.
 //!
+//!    All list API that take optional parameters do so using the [`ListOptional`] struct. Similarly, all watch API that take optional parameters do so using
+//!    the [`WatchOptional`] struct. Other API functions with unique parameters have their own `Optional` structs, such as [`api::core::v1::DeleteNamespacedPodOptional`]
+//!    for [`api::core::v1::Pod::delete_namespaced_pod`]
+//!
 //! 1. The function returns an [`http::Request`] value with the URL path, query string, and request body filled out according to the parameters
 //!    given to the function. The function does *not* execute this request. You can execute this `http::Request` using any HTTP client library you want to use.
 //!    It does not matter whether you use a synchronous client like `reqwest`, or an asynchronous client like `hyper`, or a mock client that returns bytes