Remove traits AsDataLakeClient and AsFileSystemClient (#491)

* Removes AsStorageClient trait in favor of into_storage_client function. Matches CosmosClient in that it doesn't use Arc anymore. Still uses Arc e.g. in StorageAccountClient.new_emulator which should probably be removed as well. e2e test passes.

* Format code.

* Removes AsDataLakeClient and AsFileSystemClient traits in favor of into_data_lake_client and into_file_system_client functions. Matches CosmosClient in that it doesn't use Arc and doesn't return Errors anymore. e2e test passes.

* Revert "Removes AsStorageClient trait in favor of into_storage_client function. Matches CosmosClient in that it doesn't use Arc anymore. Still uses Arc e.g. in StorageAccountClient.new_emulator which should probably be removed as well. e2e test passes."

This reverts commit d252f98

* Fix up problems stemming from reverting commit d252f98. StorageClient can no longer create a DataLakeClient and callers must use DataLakeClient.new() instead.

* PR feedback: Change from String to &str.

Author: thovoll

sdk/storage/examples/data_lake_00.rs

@@ -32,11 +32,17 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync>> {
     println!("token expires on {}", bearer_token.expires_on);
     println!();
 
-    let data_lake = storage_account_client
-        .as_storage_client()
-        .as_data_lake_client(account, bearer_token.token.secret().to_owned())?;
+    let storage_client = storage_account_client.as_storage_client();
+    let data_lake = DataLakeClient::new(
+        storage_client,
+        account,
+        bearer_token.token.secret().to_owned(),
+        None,
+    );
 
-    let file_system = data_lake.as_file_system_client(&file_system_name)?;
+    let file_system = data_lake
+        .clone()
+        .into_file_system_client(file_system_name.to_string());
 
     let mut fs_properties = Properties::new();
     fs_properties.insert("AddedVia", "Azure SDK for Rust");

sdk/storage/src/data_lake/clients/data_lake_client.rs

@@ -1,6 +1,7 @@
 use crate::core::prelude::*;
 use crate::data_lake::authorization_policy::AuthorizationPolicy;
 use crate::data_lake::authorization_policy::DataLakeContext;
+use crate::data_lake::clients::FileSystemClient;
 use crate::data_lake::requests::*;
 use azure_core::pipeline::Pipeline;
 use azure_core::prelude::*;
@@ -9,83 +10,16 @@ use bytes::Bytes;
 use http::method::Method;
 use http::request::{Builder, Request};
 use std::sync::Arc;
-use url::{ParseError, Url};
 
 const DEFAULT_DNS_SUFFIX: &str = "dfs.core.windows.net";
 
-pub trait AsDataLakeClient<A: Into<String>> {
-    fn as_data_lake_client(
-        &self,
-        account: A,
-        bearer_token: String,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError>;
-
-    #[cfg(feature = "mock_transport_framework")]
-    fn as_data_lake_client_with_transaction(
-        &self,
-        account: A,
-        bearer_token: String,
-        transaction_name: impl Into<String>,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError>;
-}
-
-pub trait AsCustomDataLakeClient<DS: Into<String>, A: Into<String>> {
-    fn as_data_lake_client_with_custom_dns_suffix(
-        &self,
-        account: A,
-        bearer_token: String,
-        dns_suffix: DS,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError>;
-}
-
-impl<A: Into<String>> AsDataLakeClient<A> for Arc<StorageClient> {
-    fn as_data_lake_client(
-        &self,
-        account: A,
-        bearer_token: String,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError> {
-        DataLakeClient::new(self.clone(), account.into(), bearer_token, None)
-    }
-
-    #[cfg(feature = "mock_transport_framework")]
-    fn as_data_lake_client_with_transaction(
-        &self,
-        account: A,
-        bearer_token: String,
-        transaction_name: impl Into<String>,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError> {
-        DataLakeClient::new_with_transaction(
-            self.clone(),
-            account.into(),
-            bearer_token,
-            transaction_name,
-        )
-    }
-}
-
-impl<DS: Into<String>, A: Into<String>> AsCustomDataLakeClient<DS, A> for Arc<StorageClient> {
-    fn as_data_lake_client_with_custom_dns_suffix(
-        &self,
-        account: A,
-        bearer_token: String,
-        dns_suffix: DS,
-    ) -> Result<Arc<DataLakeClient>, url::ParseError> {
-        DataLakeClient::new(
-            self.clone(),
-            account.into(),
-            bearer_token,
-            Some(dns_suffix.into()),
-        )
-    }
-}
-
 #[derive(Debug, Clone)]
 pub struct DataLakeClient {
     pipeline: Pipeline<DataLakeContext>,
     storage_client: Arc<StorageClient>,
     account: String,
     custom_dns_suffix: Option<String>,
-    url: Url, // TODO: Use CloudLocation similar to CosmosClient
+    url: String, // TODO: Use CloudLocation similar to CosmosClient
 }
 
 impl DataLakeClient {
@@ -95,20 +29,17 @@ impl DataLakeClient {
         bearer_token: String,
         custom_dns_suffix: Option<String>,
         options: ClientOptions<DataLakeContext>,
-    ) -> Result<Arc<Self>, url::ParseError> {
+    ) -> Self {
         // we precalculate the url once in the constructor
         // so we do not have to do it at every request.
-        // This means we have to account for possible
-        // malfolmed urls in the constructor, hence
-        // the Result<_, url::ParseError>.
-        let url = url::Url::parse(&format!(
+        let url = format!(
             "https://{}.{}",
             account,
             match custom_dns_suffix.as_ref() {
                 Some(custom_dns_suffix) => custom_dns_suffix,
                 None => DEFAULT_DNS_SUFFIX,
             }
-        ))?;
+        );
 
         let per_call_policies = Vec::new();
         let auth_policy: Arc<dyn azure_core::Policy<DataLakeContext>> =
@@ -127,21 +58,21 @@ impl DataLakeClient {
             per_retry_policies,
         );
 
-        Ok(Arc::new(Self {
+        Self {
             pipeline,
             storage_client,
             account,
             custom_dns_suffix,
             url,
-        }))
+        }
     }
 
     pub fn new(
         storage_client: Arc<StorageClient>,
         account: String,
         bearer_token: String,
         custom_dns_suffix: Option<String>,
-    ) -> Result<Arc<DataLakeClient>, ParseError> {
+    ) -> DataLakeClient {
         Self::new_with_options(
             storage_client,
             account,
@@ -157,7 +88,7 @@ impl DataLakeClient {
         account: String,
         bearer_token: String,
         transaction_name: impl Into<String>,
-    ) -> Result<Arc<DataLakeClient>, ParseError> {
+    ) -> DataLakeClient {
         Self::new_with_options(
             storage_client,
             account,
@@ -175,14 +106,18 @@ impl DataLakeClient {
         self.storage_client.storage_account_client().http_client()
     }
 
-    pub(crate) fn url(&self) -> &Url {
+    pub(crate) fn url(&self) -> &str {
         &self.url
     }
 
     pub fn list(&self) -> ListFileSystemsBuilder {
         ListFileSystemsBuilder::new(self)
     }
 
+    pub fn into_file_system_client(self, file_system_name: String) -> FileSystemClient {
+        FileSystemClient::new(self, file_system_name)
+    }
+
     pub(crate) fn prepare_request(
         &self,
         url: &str,

sdk/storage/src/data_lake/clients/file_system_client.rs

@@ -6,41 +6,29 @@ use azure_core::pipeline::Pipeline;
 use azure_core::{Context, HttpClient, PipelineContext};
 use bytes::Bytes;
 
-use std::sync::Arc;
 use url::Url;
 
-pub trait AsFileSystemClient<A: Into<String>> {
-    fn as_file_system_client(&self, name: A) -> Result<Arc<FileSystemClient>, url::ParseError>;
-}
-
-impl<A: Into<String>> AsFileSystemClient<A> for Arc<DataLakeClient> {
-    fn as_file_system_client(&self, name: A) -> Result<Arc<FileSystemClient>, url::ParseError> {
-        FileSystemClient::new(self.clone(), name.into())
-    }
-}
-
 #[derive(Debug, Clone)]
 pub struct FileSystemClient {
-    data_lake_client: Arc<DataLakeClient>,
+    data_lake_client: DataLakeClient,
     name: String,
     url: Url,
 }
 
 impl FileSystemClient {
-    pub(crate) fn new(
-        data_lake_client: Arc<DataLakeClient>,
-        name: String,
-    ) -> Result<Arc<Self>, url::ParseError> {
-        let mut url = data_lake_client.url().to_owned();
+    pub(crate) fn new(data_lake_client: DataLakeClient, name: String) -> Self {
+        let mut url = url::Url::parse(data_lake_client.url()).unwrap();
+
         url.path_segments_mut()
-            .map_err(|_| url::ParseError::SetHostOnCannotBeABaseUrl)?
+            .map_err(|_| url::ParseError::SetHostOnCannotBeABaseUrl)
+            .unwrap()
             .push(&name);
 
-        Ok(Arc::new(Self {
+        Self {
             data_lake_client,
             name,
             url,
-        }))
+        }
     }
 
     pub fn create(&self) -> CreateFileSystemBuilder {

sdk/storage/src/data_lake/clients/mod.rs

@@ -1,4 +1,4 @@
 mod data_lake_client;
-pub use data_lake_client::{AsDataLakeClient, DataLakeClient};
+pub use data_lake_client::DataLakeClient;
 mod file_system_client;
-pub use file_system_client::{AsFileSystemClient, FileSystemClient};
+pub use file_system_client::FileSystemClient;

sdk/storage/src/data_lake/requests/list_file_systems_builder.rs

@@ -40,9 +40,7 @@ impl<'a> ListFileSystemsBuilder<'a> {
     pub async fn execute(
         &self,
     ) -> Result<ListFileSystemsResponse, Box<dyn std::error::Error + Sync + Send>> {
-        // we clone this so we can add custom
-        // query parameters
-        let mut url = self.data_lake_client.url().clone();
+        let mut url = url::Url::parse(self.data_lake_client.url())?;
 
         url.query_pairs_mut().append_pair("resource", "account");
 

sdk/storage/tests/data_lake.rs

@@ -31,13 +31,17 @@ async fn test_data_lake_file_system_functions() -> Result<(), Box<dyn Error + Se
         .get_token(resource_id)
         .await?;
 
-    let data_lake_client = storage_account_client
-        .as_storage_client()
-        // This test won't work during replay in CI until all operations are converted to pipeline architecture
-        // .as_data_lake_client_with_transaction(account, bearer_token.token.secret().to_owned(), "test_data_lake_file_system_functions")?;
-        .as_data_lake_client(account, bearer_token.token.secret().to_owned())?;
+    let storage_client = storage_account_client.as_storage_client();
+    let data_lake_client = DataLakeClient::new(
+        storage_client,
+        account,
+        bearer_token.token.secret().to_owned(),
+        None,
+    );
 
-    let file_system_client = data_lake_client.as_file_system_client(&file_system_name)?;
+    let file_system_client = data_lake_client
+        .clone()
+        .into_file_system_client(file_system_name.to_string());
 
     let mut fs_properties = Properties::new();
     fs_properties.insert("AddedVia", "Azure SDK for Rust");