Simplify ClientOptions (#3016)

Resolves #2956 by making nested options non-`Option`. Only leaf fields
should be optional. An `Option<T>` allocates as much space as `T`
regardless, so why deal with all the hassle - both for callers and
callees - of dealing with `Option<T>` for intermediate members?

Author: heaths

sdk/core/azure_core/CHANGELOG.md

@@ -6,7 +6,10 @@
 
 ### Breaking Changes
 
+- Changed `ClientOptions::retry` from `Option<RetryOptions>` to `RetryOptions`.
 - Removed several unreferenced HTTP headers and accessor structures for those headers.
+- Renamed `TransportOptions` to `Transport`.
+- Renamed `TransportOptions::new_custom_policy()` to `Transport::with_policy()`.
 
 ### Bugs Fixed
 

sdk/core/azure_core/README.md

@@ -436,7 +436,7 @@ In many cases with `reqwest`, importing features may be enough. See their [docum
 If you do need to write code to customize the `reqwest::Client`, you can pass it in `ClientOptions` to our client libraries:
 
 ```rust no_run
-use azure_core::http::{ClientOptions, TransportOptions};
+use azure_core::http::{ClientOptions, Transport};
 use azure_identity::DeveloperToolsCredential;
 use azure_security_keyvault_secrets::{SecretClient, SecretClientOptions};
 use std::sync::Arc;
@@ -445,7 +445,7 @@ let http_client = Arc::new(reqwest::ClientBuilder::new().gzip(true).build().unwr
 
 let options = SecretClientOptions {
     client_options: ClientOptions {
-        transport: Some(TransportOptions::new(http_client)),
+        transport: Some(Transport::new(http_client)),
         ..Default::default()
     },
     ..Default::default()
@@ -611,7 +611,7 @@ and set that as the transport in any `ClientOptions` used to configure your Azur
 
 ```rust no_run
 use std::sync::Arc;
-use azure_core::http::{HttpClient, ClientOptions, TransportOptions};
+use azure_core::http::{HttpClient, ClientOptions, Transport};
 use azure_security_keyvault_secrets::SecretClientOptions;
 
 let client = Arc::new(
@@ -624,7 +624,7 @@ let client = Arc::new(
 
 let options = SecretClientOptions {
     client_options: ClientOptions {
-        transport: Some(TransportOptions::new(client.clone())),
+        transport: Some(Transport::new(client.clone())),
         ..Default::default()
     },
     ..Default::default()

sdk/core/azure_core/benches/http_transport_benchmarks.rs

@@ -6,7 +6,7 @@ use azure_core::{
     fmt::SafeDebug,
     http::{
         BufResponse, ClientMethodOptions, ClientOptions, HttpClient, Method, Pipeline, Request,
-        TransportOptions, Url,
+        Transport, Url,
     },
     Result,
 };
@@ -168,7 +168,7 @@ pub fn disable_pooling_http_transport_test(c: &mut Criterion) {
         let transport = new_reqwest_client_disable_connection_pool();
         let options = TestServiceClientOptions {
             client_options: ClientOptions {
-                transport: Some(TransportOptions::new(transport)),
+                transport: Some(Transport::new(transport)),
                 ..Default::default()
             },
             ..Default::default()

sdk/core/azure_core/examples/core_pager.rs

@@ -3,7 +3,7 @@
 
 use azure_core::{
     credentials::TokenCredential,
-    http::{headers::Headers, BufResponse, HttpClient, Method, StatusCode, TransportOptions},
+    http::{headers::Headers, BufResponse, HttpClient, Method, StatusCode, Transport},
 };
 use azure_core_test::{credentials::MockCredential, http::MockHttpClient};
 use azure_security_keyvault_secrets::{ResourceExt, SecretClient, SecretClientOptions};
@@ -21,7 +21,7 @@ async fn test_pager() -> Result<(), Box<dyn std::error::Error>> {
     // You normally would create credentials from `azure_identity` and
     // use the default transport in production.
     let (credential, transport) = setup()?;
-    options.client_options.transport = Some(TransportOptions::new(transport));
+    options.client_options.transport = Some(Transport::new(transport));
 
     let client = SecretClient::new(
         "https://my-vault.vault.azure.net",

sdk/core/azure_core/examples/core_poller.rs

@@ -5,7 +5,7 @@ use azure_core::{
     credentials::TokenCredential,
     http::{
         headers::{Headers, RETRY_AFTER},
-        BufResponse, HttpClient, Method, StatusCode, TransportOptions,
+        BufResponse, HttpClient, Method, StatusCode, Transport,
     },
 };
 use azure_core_test::{credentials::MockCredential, http::MockHttpClient};
@@ -26,7 +26,7 @@ async fn test_poller() -> Result<(), Box<dyn std::error::Error>> {
     // You normally would create credentials from `azure_identity` and
     // use the default transport in production.
     let (credential, transport) = setup()?;
-    options.client_options.transport = Some(TransportOptions::new(transport));
+    options.client_options.transport = Some(Transport::new(transport));
 
     let client = CertificateClient::new(
         "https://my-vault.vault.azure.net",

sdk/core/azure_core/examples/core_remove_user_agent.rs

@@ -7,7 +7,7 @@ use azure_core::{
     http::{
         headers::Headers,
         policies::{Policy, PolicyResult},
-        BufResponse, Context, HttpClient, Method, Request, StatusCode, TransportOptions,
+        BufResponse, Context, HttpClient, Method, Request, StatusCode, Transport,
     },
 };
 use azure_core_test::{credentials::MockCredential, http::MockHttpClient};
@@ -52,7 +52,7 @@ async fn test_remove_user_agent() -> Result<(), Box<dyn std::error::Error>> {
     // You normally would create credentials from `azure_identity` and
     // use the default transport in production.
     let (credential, transport) = setup()?;
-    options.client_options.transport = Some(TransportOptions::new(transport));
+    options.client_options.transport = Some(Transport::new(transport));
 
     // Construct the client with these options and a shared credential.
     let client = SecretClient::new(

sdk/core/azure_core/examples/core_ureq_client.rs

@@ -4,7 +4,7 @@
 use async_trait::async_trait;
 use azure_core::{
     error::ErrorKind,
-    http::{headers::Headers, BufResponse, ClientOptions, HttpClient, Request, TransportOptions},
+    http::{headers::Headers, BufResponse, ClientOptions, HttpClient, Request, Transport},
 };
 use azure_identity::DeveloperToolsCredential;
 use azure_security_keyvault_secrets::{ResourceExt as _, SecretClient, SecretClientOptions};
@@ -56,7 +56,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let agent = Arc::new(Agent::default());
     let options = SecretClientOptions {
         client_options: ClientOptions {
-            transport: Some(TransportOptions::new(agent)),
+            transport: Some(Transport::new(agent)),
             ..Default::default()
         },
         ..Default::default()

sdk/core/azure_core/src/http/options/mod.rs

@@ -9,7 +9,7 @@ use std::sync::Arc;
 use typespec_client_core::http::policies::Policy;
 pub use typespec_client_core::http::{
     ClientMethodOptions, ExponentialRetryOptions, FixedRetryOptions, LoggingOptions,
-    PipelineOptions, RetryOptions, TransportOptions,
+    PipelineOptions, RetryOptions, Transport,
 };
 pub use user_agent::*;
 
@@ -23,19 +23,19 @@ pub struct ClientOptions {
     pub per_try_policies: Vec<Arc<dyn Policy>>,
 
     /// Retry options.
-    pub retry: Option<RetryOptions>,
+    pub retry: RetryOptions,
 
     /// Transport options.
-    pub transport: Option<TransportOptions>,
+    pub transport: Option<Transport>,
 
     /// User-Agent telemetry options.
-    pub user_agent: Option<UserAgentOptions>,
+    pub user_agent: UserAgentOptions,
 
     /// Options for request instrumentation, such as distributed tracing.
     ///
     /// If not specified, defaults to no instrumentation.
     ///
-    pub instrumentation: Option<InstrumentationOptions>,
+    pub instrumentation: InstrumentationOptions,
 
     /// Logging options
     ///
@@ -65,8 +65,8 @@ impl ClientOptions {
 
         (
             CoreClientOptions {
-                user_agent: self.user_agent.unwrap_or_default(),
-                instrumentation: self.instrumentation.unwrap_or_default(),
+                user_agent: self.user_agent,
+                instrumentation: self.instrumentation,
             },
             options,
         )

sdk/core/azure_core/src/http/pipeline.rs

@@ -141,7 +141,7 @@ mod tests {
             headers::{self, HeaderName, Headers},
             policies::Policy,
             request::options::ClientRequestId,
-            BufResponse, ClientOptions, Context, Method, Request, StatusCode, TransportOptions,
+            BufResponse, ClientOptions, Context, Method, Request, StatusCode, Transport,
             UserAgentOptions,
         },
         Bytes,
@@ -160,7 +160,7 @@ mod tests {
         let mut ctx = Context::new();
         ctx.insert(ClientRequestId::new(CLIENT_REQUEST_ID.to_string()));
 
-        let transport = TransportOptions::new(Arc::new(MockHttpClient::new(|req| {
+        let transport = Transport::new(Arc::new(MockHttpClient::new(|req| {
             async {
                 // Assert
                 let header_value = req
@@ -218,7 +218,7 @@ mod tests {
         let mut ctx = Context::new();
         ctx.insert(ClientRequestId::new(CLIENT_REQUEST_ID.to_string()));
 
-        let transport = TransportOptions::new(Arc::new(MockHttpClient::new(|req| {
+        let transport = Transport::new(Arc::new(MockHttpClient::new(|req| {
             async {
                 // Assert
                 let header_value = req
@@ -269,7 +269,7 @@ mod tests {
         // Arrange
         let ctx = Context::new();
 
-        let transport = TransportOptions::new(Arc::new(MockHttpClient::new(|req| {
+        let transport = Transport::new(Arc::new(MockHttpClient::new(|req| {
             async {
                 // Assert
                 let user_agent = req
@@ -324,7 +324,7 @@ mod tests {
         const CUSTOM_APPLICATION_ID: &str = "my-custom-app/2.1.0";
         let ctx = Context::new();
 
-        let transport = TransportOptions::new(Arc::new(MockHttpClient::new(|req| {
+        let transport = Transport::new(Arc::new(MockHttpClient::new(|req| {
             async {
                 // Assert
                 let user_agent = req
@@ -354,7 +354,7 @@ mod tests {
 
         let options = ClientOptions {
             transport: Some(transport),
-            user_agent: Some(user_agent_options),
+            user_agent: user_agent_options,
             ..Default::default()
         };
 

sdk/core/azure_core/src/http/policies/bearer_token_policy.rs

@@ -156,7 +156,7 @@ mod tests {
         Arc,
     };
     use typespec_client_core::{
-        http::{policies::TransportPolicy, BufResponse, Method, TransportOptions},
+        http::{policies::TransportPolicy, BufResponse, Method, Transport},
         time::Duration,
     };
 
@@ -211,9 +211,7 @@ mod tests {
         let credential = MockCredential::new(&[]);
         let policy = BearerTokenCredentialPolicy::new(Arc::new(credential), ["scope"]);
         let client = MockHttpClient::new(|_| panic!("expected an error from get_token"));
-        let transport = Arc::new(TransportPolicy::new(TransportOptions::new(Arc::new(
-            client,
-        ))));
+        let transport = Arc::new(TransportPolicy::new(Transport::new(Arc::new(client))));
         let mut req = Request::new("https://localhost".parse().unwrap(), Method::Get);
 
         let err = policy
@@ -249,7 +247,7 @@ mod tests {
             }
             .boxed()
         }));
-        let transport = Arc::new(TransportPolicy::new(TransportOptions::new(client)));
+        let transport = Arc::new(TransportPolicy::new(Transport::new(client)));
 
         let mut handles = vec![];
         for _ in 0..4 {