add an operationId strategy to reject or ignore missing operations

Author: arlyon

progenitor-impl/src/cli.rs

@@ -24,7 +24,7 @@ struct CliOperation {
 impl Generator {
     /// Generate a `clap`-based CLI.
     pub fn cli(&mut self, spec: &OpenAPI, crate_name: &str) -> Result<TokenStream> {
-        validate_openapi(spec)?;
+        validate_openapi(spec, self.settings.operation_id_strategy)?;
 
         // Convert our components dictionary to schemars
         let schemas = spec.components.iter().flat_map(|components| {
@@ -53,6 +53,7 @@ impl Generator {
 
         let methods = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| self.cli_method(method))
             .collect::<Vec<_>>();
 
@@ -62,15 +63,18 @@ impl Generator {
 
         let cli_fns = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| format_ident!("cli_{}", sanitize(&method.operation_id, Case::Snake)))
             .collect::<Vec<_>>();
         let execute_fns = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| format_ident!("execute_{}", sanitize(&method.operation_id, Case::Snake)))
             .collect::<Vec<_>>();
 
         let cli_variants = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| format_ident!("{}", sanitize(&method.operation_id, Case::Pascal)))
             .collect::<Vec<_>>();
 

progenitor-impl/src/httpmock.rs

@@ -30,7 +30,7 @@ impl Generator {
     /// the SDK. This can include `::` and instances of `-` in the crate name
     /// should be converted to `_`.
     pub fn httpmock(&mut self, spec: &OpenAPI, crate_path: &str) -> Result<TokenStream> {
-        validate_openapi(spec)?;
+        validate_openapi(spec, self.settings.operation_id_strategy)?;
 
         // Convert our components dictionary to schemars
         let schemas = spec.components.iter().flat_map(|components| {
@@ -59,11 +59,13 @@ impl Generator {
 
         let methods = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| self.httpmock_method(method))
             .collect::<Vec<_>>();
 
         let op = raw_methods
             .iter()
+            .filter_map(|method| method.as_ref())
             .map(|method| format_ident!("{}", &method.operation_id))
             .collect::<Vec<_>>();
         let when = methods.iter().map(|op| &op.when).collect::<Vec<_>>();

progenitor-impl/src/lib.rs

@@ -67,6 +67,8 @@ pub struct GenerationSettings {
     post_hook_async: Option<TokenStream>,
     extra_derives: Vec<String>,
 
+    operation_id_strategy: OperationIdStrategy,
+
     map_type: Option<String>,
     unknown_crates: UnknownPolicy,
     crates: BTreeMap<String, CrateSpec>,
@@ -112,6 +114,23 @@ impl Default for TagStyle {
     }
 }
 
+/// Style for handing operations that do not have an operation ID.
+#[derive(Copy, Clone)]
+pub enum OperationIdStrategy {
+    /// The default behaviour. Reject when any operation on the resulting
+    /// client does not have an operation ID.
+    RejectMissing,
+    /// Omit any operation on the resulting client that does not have an
+    /// operation ID.
+    OmitMissing,
+}
+
+impl Default for OperationIdStrategy {
+    fn default() -> Self {
+        Self::RejectMissing
+    }
+}
+
 impl GenerationSettings {
     /// Create new generator settings with default values.
     pub fn new() -> Self {
@@ -241,6 +260,16 @@ impl GenerationSettings {
         self.map_type = Some(map_type.to_string());
         self
     }
+
+    /// Set the strategy to be used when encountering operations that do not
+    /// have an operation ID.
+    pub fn with_operation_id_strategy(
+        &mut self,
+        operation_id_strategy: OperationIdStrategy,
+    ) -> &mut Self {
+        self.operation_id_strategy = operation_id_strategy;
+        self
+    }
 }
 
 impl Default for Generator {
@@ -306,7 +335,7 @@ impl Generator {
 
     /// Emit a [TokenStream] containing the generated client code.
     pub fn generate_tokens(&mut self, spec: &OpenAPI) -> Result<TokenStream> {
-        validate_openapi(spec)?;
+        validate_openapi(spec, self.settings.operation_id_strategy)?;
 
         // Convert our components dictionary to schemars
         let schemas = spec.components.iter().flat_map(|components| {
@@ -328,8 +357,9 @@ impl Generator {
                     (path.as_str(), method, operation, &item.parameters)
                 })
             })
-            .map(|(path, method, operation, path_parameters)| {
+            .filter_map(|(path, method, operation, path_parameters)| {
                 self.process_operation(operation, &spec.components, path, method, path_parameters)
+                    .transpose()
             })
             .collect::<Result<Vec<_>>>()?;
 
@@ -674,7 +704,7 @@ fn validate_openapi_spec_version(spec_version: &str) -> Result<()> {
 }
 
 /// Do some very basic checks of the OpenAPI documents.
-pub fn validate_openapi(spec: &OpenAPI) -> Result<()> {
+pub fn validate_openapi(spec: &OpenAPI, operation_id_strategy: OperationIdStrategy) -> Result<()> {
     validate_openapi_spec_version(spec.openapi.as_str())?;
 
     let mut opids = HashSet::new();
@@ -695,10 +725,15 @@ pub fn validate_openapi(spec: &OpenAPI) -> Result<()> {
                             )));
                         }
                     } else {
-                        return Err(Error::UnexpectedFormat(format!(
-                            "path {} is missing operation ID",
-                            p.0,
-                        )));
+                        match operation_id_strategy {
+                            OperationIdStrategy::RejectMissing => {
+                                return Err(Error::UnexpectedFormat(format!(
+                                    "path {} is missing operation ID",
+                                    p.0,
+                                )));
+                            }
+                            OperationIdStrategy::OmitMissing => {}
+                        }
                     }
                     Ok(())
                 })

progenitor-impl/src/method.rs

@@ -290,8 +290,10 @@ impl Generator {
         path: &str,
         method: &str,
         path_parameters: &[ReferenceOr<Parameter>],
-    ) -> Result<OperationMethod> {
-        let operation_id = operation.operation_id.as_ref().unwrap();
+    ) -> Result<Option<OperationMethod>> {
+        let Some(operation_id) = operation.operation_id.as_ref() else {
+            return Ok(None);
+        };
 
         let mut combined_path_parameters = parameter_map(path_parameters, components)?;
         for operation_param in items(&operation.parameters, components) {
@@ -534,7 +536,7 @@ impl Generator {
             )));
         }
 
-        Ok(OperationMethod {
+        Ok(Some(OperationMethod {
             operation_id: sanitize(operation_id, Case::Snake),
             tags: operation.tags.clone(),
             method: HttpMethod::from_str(method)?,
@@ -545,7 +547,7 @@ impl Generator {
             responses,
             dropshot_paginated,
             dropshot_websocket,
-        })
+        }))
     }
 
     pub(crate) fn positional_method(

progenitor-impl/tests/output/src/missing_operation_id_builder.rs

@@ -0,0 +1,117 @@
+#[allow(unused_imports)]
+use progenitor_client::{encode_path, ClientHooks, OperationInfo, RequestBuilderExt};
+#[allow(unused_imports)]
+pub use progenitor_client::{ByteStream, ClientInfo, Error, ResponseValue};
+/// Types used as operation parameters and responses.
+#[allow(clippy::all)]
+pub mod types {
+    /// Error types.
+    pub mod error {
+        /// Error from a `TryFrom` or `FromStr` implementation.
+        pub struct ConversionError(::std::borrow::Cow<'static, str>);
+        impl ::std::error::Error for ConversionError {}
+        impl ::std::fmt::Display for ConversionError {
+            fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> {
+                ::std::fmt::Display::fmt(&self.0, f)
+            }
+        }
+
+        impl ::std::fmt::Debug for ConversionError {
+            fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> {
+                ::std::fmt::Debug::fmt(&self.0, f)
+            }
+        }
+
+        impl From<&'static str> for ConversionError {
+            fn from(value: &'static str) -> Self {
+                Self(value.into())
+            }
+        }
+
+        impl From<String> for ConversionError {
+            fn from(value: String) -> Self {
+                Self(value.into())
+            }
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+///Client for Parameter name collision test
+///
+///Minimal API for testing collision between parameter names and generated code
+///
+///Version: v1
+pub struct Client {
+    pub(crate) baseurl: String,
+    pub(crate) client: reqwest::Client,
+}
+
+impl Client {
+    /// Create a new client.
+    ///
+    /// `baseurl` is the base URL provided to the internal
+    /// `reqwest::Client`, and should include a scheme and hostname,
+    /// as well as port and a path stem if applicable.
+    pub fn new(baseurl: &str) -> Self {
+        #[cfg(not(target_arch = "wasm32"))]
+        let client = {
+            let dur = std::time::Duration::from_secs(15);
+            reqwest::ClientBuilder::new()
+                .connect_timeout(dur)
+                .timeout(dur)
+        };
+        #[cfg(target_arch = "wasm32")]
+        let client = reqwest::ClientBuilder::new();
+        Self::new_with_client(baseurl, client.build().unwrap())
+    }
+
+    /// Construct a new client with an existing `reqwest::Client`,
+    /// allowing more control over its configuration.
+    ///
+    /// `baseurl` is the base URL provided to the internal
+    /// `reqwest::Client`, and should include a scheme and hostname,
+    /// as well as port and a path stem if applicable.
+    pub fn new_with_client(baseurl: &str, client: reqwest::Client) -> Self {
+        Self {
+            baseurl: baseurl.to_string(),
+            client,
+        }
+    }
+}
+
+impl ClientInfo<()> for Client {
+    fn api_version() -> &'static str {
+        "v1"
+    }
+
+    fn baseurl(&self) -> &str {
+        self.baseurl.as_str()
+    }
+
+    fn client(&self) -> &reqwest::Client {
+        &self.client
+    }
+
+    fn inner(&self) -> &() {
+        &()
+    }
+}
+
+impl ClientHooks<()> for &Client {}
+impl Client {}
+/// Types for composing operation parameters.
+#[allow(clippy::all)]
+pub mod builder {
+    use super::types;
+    #[allow(unused_imports)]
+    use super::{
+        encode_path, ByteStream, ClientHooks, ClientInfo, Error, OperationInfo, RequestBuilderExt,
+        ResponseValue,
+    };
+}
+
+/// Items consumers will typically use such as the Client.
+pub mod prelude {
+    pub use self::super::Client;
+}