move StorageCredentials out of CloudLocation (#1423)

demoray · web-flow · commit d607009ef3bd · 2023-09-27T13:03:05.000-04:00
This is a needed step towards allowing the storage credentials to be updated without recreating all of the clients, such as what is needed to address #1415.
diff --git a/sdk/data_tables/src/clients/table_service_client.rs b/sdk/data_tables/src/clients/table_service_client.rs
@@ -12,46 +12,61 @@ use super::TableClient;
 pub struct TableServiceClientBuilder {
     cloud_location: CloudLocation,
     options: ClientOptions,
+    credentials: StorageCredentials,
 }
 
 impl TableServiceClientBuilder {
-    /// Create a new instance of `BlobServiceClientBuilder`.
+    /// Create a new instance of `TableServiceClientBuilder`.
     #[must_use]
-    pub fn new(account: impl Into<String>, credentials: impl Into<StorageCredentials>) -> Self {
-        Self::with_location(CloudLocation::Public {
-            account: account.into(),
-            credentials: credentials.into(),
-        })
+    pub fn new<A, C>(account: A, credentials: C) -> Self
+    where
+        A: Into<String>,
+        C: Into<StorageCredentials>,
+    {
+        Self::with_location(
+            CloudLocation::Public {
+                account: account.into(),
+            },
+            credentials,
+        )
     }
 
     /// Create a new instance of `BlobServiceClientBuilder` with a cloud location.
     #[must_use]
-    pub fn with_location(cloud_location: CloudLocation) -> Self {
+    pub fn with_location<C>(cloud_location: CloudLocation, credentials: C) -> Self
+    where
+        C: Into<StorageCredentials>,
+    {
         Self {
             options: ClientOptions::default(),
             cloud_location,
+            credentials: credentials.into(),
         }
     }
 
     /// Use the emulator with default settings
     #[must_use]
     pub fn emulator() -> Self {
-        Self::with_location(CloudLocation::Emulator {
-            address: "127.0.0.1".to_owned(),
-            port: 10002,
-        })
+        Self::with_location(
+            CloudLocation::Emulator {
+                address: "127.0.0.1".to_owned(),
+                port: 10002,
+            },
+            StorageCredentials::emulator(),
+        )
     }
 
     /// Convert the builder into a `TableServiceClient` instance.
     #[must_use]
     pub fn build(self) -> TableServiceClient {
-        let credentials = self.cloud_location.credentials();
+        let Self {
+            cloud_location,
+            options,
+            credentials,
+        } = self;
         TableServiceClient {
-            pipeline: azure_storage::clients::new_pipeline_from_options(
-                self.options,
-                credentials.clone(),
-            ),
-            cloud_location: self.cloud_location,
+            pipeline: azure_storage::clients::new_pipeline_from_options(options, credentials),
+            cloud_location,
         }
     }
 
diff --git a/sdk/storage/Cargo.toml b/sdk/storage/Cargo.toml
@@ -25,7 +25,6 @@ url = "2.2"
 uuid = { version = "1.0", features = ["v4"] }
 bytes = "1.0"
 RustyXML = "0.3"
-once_cell = "1.7"
 hmac = "0.12"
 sha2 = "0.10"
 
diff --git a/sdk/storage/src/authorization/mod.rs b/sdk/storage/src/authorization/mod.rs
@@ -5,9 +5,12 @@ use azure_core::{
     error::{ErrorKind, ResultExt},
 };
 use std::sync::Arc;
+use url::Url;
 
 pub(crate) use authorization_policy::AuthorizationPolicy;
 
+use crate::clients::{EMULATOR_ACCOUNT, EMULATOR_ACCOUNT_KEY};
+
 /// Credentials for accessing a storage account.
 ///
 /// # Example
@@ -112,6 +115,11 @@ impl StorageCredentials {
     pub fn anonymous() -> Self {
         Self::Anonymous
     }
+
+    /// Create an Access Key credential for use with the Azure Storage emulator
+    pub fn emulator() -> Self {
+        Self::access_key(EMULATOR_ACCOUNT, EMULATOR_ACCOUNT_KEY)
+    }
 }
 
 impl std::fmt::Debug for StorageCredentials {
@@ -147,6 +155,16 @@ impl From<Arc<dyn TokenCredential>> for StorageCredentials {
     }
 }
 
+impl TryFrom<&Url> for StorageCredentials {
+    type Error = azure_core::Error;
+    fn try_from(value: &Url) -> Result<Self, Self::Error> {
+        match value.query() {
+            Some(query) => Self::sas_token(query),
+            None => Ok(Self::Anonymous),
+        }
+    }
+}
+
 fn get_sas_token_parms(sas_token: &str) -> azure_core::Result<Vec<(String, String)>> {
     // Any base url will do: we just need to parse the SAS token
     // to get its query pairs.
diff --git a/sdk/storage/src/cloud_location.rs b/sdk/storage/src/cloud_location.rs
@@ -1,5 +1,4 @@
-use crate::{clients::ServiceType, StorageCredentials};
-use once_cell::sync::Lazy;
+use crate::clients::ServiceType;
 use std::convert::TryFrom;
 use url::Url;
 
@@ -8,22 +7,13 @@ use url::Url;
 #[derive(Debug, Clone)]
 pub enum CloudLocation {
     /// Azure public cloud
-    Public {
-        account: String,
-        credentials: StorageCredentials,
-    },
+    Public { account: String },
     /// Azure China cloud
-    China {
-        account: String,
-        credentials: StorageCredentials,
-    },
+    China { account: String },
     /// Use the well-known emulator
     Emulator { address: String, port: u16 },
     /// A custom base URL
-    Custom {
-        uri: String,
-        credentials: StorageCredentials,
-    },
+    Custom { uri: String },
 }
 
 impl CloudLocation {
@@ -51,15 +41,6 @@ impl CloudLocation {
         };
         Ok(url::Url::parse(&url)?)
     }
-
-    pub fn credentials(&self) -> &StorageCredentials {
-        match self {
-            CloudLocation::Public { credentials, .. }
-            | CloudLocation::China { credentials, .. }
-            | CloudLocation::Custom { credentials, .. } => credentials,
-            CloudLocation::Emulator { .. } => &EMULATOR_CREDENTIALS,
-        }
-    }
 }
 
 impl TryFrom<&Url> for CloudLocation {
@@ -68,11 +49,6 @@ impl TryFrom<&Url> for CloudLocation {
     // TODO: This only works for Public and China clouds.
     // ref: https://github.com/Azure/azure-sdk-for-rust/issues/502
     fn try_from(url: &Url) -> azure_core::Result<Self> {
-        let credentials = match url.query() {
-            Some(token) => StorageCredentials::sas_token(token)?,
-            None => StorageCredentials::Anonymous,
-        };
-
         let host = url.host_str().ok_or_else(|| {
             azure_core::Error::with_message(azure_core::error::ErrorKind::DataConversion, || {
                 "unable to find the target host in the URL"
@@ -92,14 +68,8 @@ impl TryFrom<&Url> for CloudLocation {
         let rest = domain.join(".");
 
         match rest.as_str() {
-            "core.windows.net" => Ok(CloudLocation::Public {
-                account,
-                credentials,
-            }),
-            "core.chinacloudapi.cn" => Ok(CloudLocation::China {
-                account,
-                credentials,
-            }),
+            "core.windows.net" => Ok(CloudLocation::Public { account }),
+            "core.chinacloudapi.cn" => Ok(CloudLocation::China { account }),
             _ => Err(azure_core::Error::with_message(
                 azure_core::error::ErrorKind::DataConversion,
                 || format!("URL refers to a domain that is not a Public or China domain: {host}"),
@@ -108,10 +78,6 @@ impl TryFrom<&Url> for CloudLocation {
     }
 }
 
-pub static EMULATOR_CREDENTIALS: Lazy<StorageCredentials> = Lazy::new(|| {
-    StorageCredentials::Key(EMULATOR_ACCOUNT.to_owned(), EMULATOR_ACCOUNT_KEY.to_owned())
-});
-
 /// The well-known account used by Azurite and the legacy Azure Storage Emulator.
 /// <https://docs.microsoft.com/azure/storage/common/storage-use-azurite#well-known-storage-account-and-key>
 pub const EMULATOR_ACCOUNT: &str = "devstoreaccount1";
@@ -133,9 +99,6 @@ mod tests {
         let cloud_location: CloudLocation = (&public_with_token).try_into()?;
         assert_eq!(public_without_token, cloud_location.url(ServiceType::Blob)?);
 
-        let creds = cloud_location.credentials();
-        assert!(matches!(creds, &StorageCredentials::SASToken(_)));
-
         let file_url = Url::parse("file://tmp/test.txt")?;
         let result: azure_core::Result<CloudLocation> = (&file_url).try_into();
         assert!(result.is_err());
diff --git a/sdk/storage_blobs/src/clients/blob_service_client.rs b/sdk/storage_blobs/src/clients/blob_service_client.rs
@@ -18,43 +18,63 @@ use super::{BlobClient, BlobLeaseClient, ContainerClient, ContainerLeaseClient};
 pub struct ClientBuilder {
     cloud_location: CloudLocation,
     options: ClientOptions,
+    credentials: StorageCredentials,
 }
 
 impl ClientBuilder {
     /// Create a new instance of `ClientBuilder`.
     #[must_use]
-    pub fn new(account: impl Into<String>, credentials: impl Into<StorageCredentials>) -> Self {
-        Self::with_location(CloudLocation::Public {
-            account: account.into(),
-            credentials: credentials.into(),
-        })
+    pub fn new<A, C>(account: A, credentials: C) -> Self
+    where
+        A: Into<String>,
+        C: Into<StorageCredentials>,
+    {
+        Self::with_location(
+            CloudLocation::Public {
+                account: account.into(),
+            },
+            credentials,
+        )
     }
 
     /// Create a new instance of `ClientBuilder` with a cloud location.
     #[must_use]
-    pub fn with_location(cloud_location: CloudLocation) -> Self {
+    pub fn with_location<C>(cloud_location: CloudLocation, credentials: C) -> Self
+    where
+        C: Into<StorageCredentials>,
+    {
         Self {
             options: ClientOptions::default(),
             cloud_location,
+            credentials: credentials.into(),
         }
     }
 
     /// Use the emulator with default settings
     #[must_use]
     pub fn emulator() -> Self {
-        Self::with_location(CloudLocation::Emulator {
-            address: "127.0.0.1".to_owned(),
-            port: 10000,
-        })
+        Self::with_location(
+            CloudLocation::Emulator {
+                address: "127.0.0.1".to_owned(),
+                port: 10000,
+            },
+            StorageCredentials::emulator(),
+        )
     }
 
     /// Convert the builder into a `BlobServiceClient` instance.
     #[must_use]
     pub fn blob_service_client(self) -> BlobServiceClient {
-        let credentials = self.cloud_location.credentials();
+        let Self {
+            cloud_location,
+            options,
+            credentials,
+        } = self;
+
         BlobServiceClient {
-            pipeline: new_pipeline_from_options(self.options, credentials.clone()),
-            cloud_location: self.cloud_location,
+            pipeline: new_pipeline_from_options(options, credentials.clone()),
+            cloud_location,
+            credentials,
         }
     }
 
@@ -138,6 +158,7 @@ impl ClientBuilder {
 pub struct BlobServiceClient {
     pipeline: Pipeline,
     cloud_location: CloudLocation,
+    credentials: StorageCredentials,
 }
 
 impl BlobServiceClient {
@@ -203,7 +224,7 @@ impl BlobServiceClient {
     }
 
     pub(crate) fn credentials(&self) -> &StorageCredentials {
-        self.cloud_location.credentials()
+        &self.credentials
     }
 
     pub(crate) fn finalize_request(
diff --git a/sdk/storage_blobs/src/clients/container_client.rs b/sdk/storage_blobs/src/clients/container_client.rs
@@ -32,14 +32,16 @@ impl ContainerClient {
 
     pub fn from_sas_url(url: &Url) -> azure_core::Result<Self> {
         let cloud_location: CloudLocation = url.try_into()?;
+        let credentials: StorageCredentials = url.try_into()?;
 
         let container = url.path().split_terminator('/').nth(1).ok_or_else(|| {
             azure_core::Error::with_message(azure_core::error::ErrorKind::DataConversion, || {
                 "unable to find storage container from url"
             })
         })?;
 
-        let client = ClientBuilder::with_location(cloud_location).container_client(container);
+        let client =
+            ClientBuilder::with_location(cloud_location, credentials).container_client(container);
         Ok(client)
     }
 
diff --git a/sdk/storage_datalake/src/clients/data_lake_client.rs b/sdk/storage_datalake/src/clients/data_lake_client.rs
@@ -10,34 +10,49 @@ use azure_storage::CloudLocation;
 pub struct DataLakeClientBuilder {
     cloud_location: CloudLocation,
     options: ClientOptions,
+    credentials: StorageCredentials,
 }
 
 impl DataLakeClientBuilder {
     /// Create a new instance of `DataLakeClientBuilder`.
     #[must_use]
-    pub fn new(account: impl Into<String>, credentials: StorageCredentials) -> Self {
-        Self::with_location(CloudLocation::Public {
-            account: account.into(),
+    pub fn new<A, C>(account: A, credentials: C) -> Self
+    where
+        A: Into<String>,
+        C: Into<StorageCredentials>,
+    {
+        Self::with_location(
+            CloudLocation::Public {
+                account: account.into(),
+            },
             credentials,
-        })
+        )
     }
 
     /// Create a new instance of `DataLakeClientBuilder` with a cloud location.
     #[must_use]
-    pub fn with_location(cloud_location: CloudLocation) -> Self {
+    pub fn with_location<C>(cloud_location: CloudLocation, credentials: C) -> Self
+    where
+        C: Into<StorageCredentials>,
+    {
         Self {
             options: ClientOptions::default(),
             cloud_location,
+            credentials: credentials.into(),
         }
     }
 
     /// Convert the builder into a `DataLakeClient` instance.
     #[must_use]
     pub fn build(self) -> DataLakeClient {
-        let credentials = self.cloud_location.credentials();
+        let Self {
+            credentials,
+            cloud_location,
+            options,
+        } = self;
         DataLakeClient {
-            pipeline: new_pipeline_from_options(self.options, credentials.clone()),
-            cloud_location: self.cloud_location,
+            pipeline: new_pipeline_from_options(options, credentials),
+            cloud_location,
         }
     }
 
diff --git a/sdk/storage_queues/src/clients/queue_service_client.rs b/sdk/storage_queues/src/clients/queue_service_client.rs
