final review tidy up

Author: analogrelay

Cargo.lock

@@ -17,7 +17,6 @@ categories = ["api-bindings"]
 async-trait.workspace = true
 azure_core.workspace = true
 futures.workspace = true
-pin-project.workspace = true
 serde_json.workspace = true
 serde.workspace = true
 tracing.workspace = true

sdk/cosmos/azure_data_cosmos/Cargo.toml

@@ -95,7 +95,7 @@ impl ContainerMetadata {
 /// The cache can be cloned cheaply, and all clones share the same underlying cache data.
 #[derive(Clone)]
 pub struct ContainerMetadataCache {
-    /// Caches stable container metadata, mapping from container link and RID to metadata.
+    /// Caches stable container metadata, mapping from container link to metadata.
     container_properties_cache: Cache<ResourceLink, Arc<ContainerMetadata>>,
 }
 
@@ -133,15 +133,11 @@ impl ContainerMetadataCache {
     ) -> Result<Arc<ContainerMetadata>, CacheError> {
         // TODO: Background refresh. We can do background refresh by storing an expiry time in the cache entry.
         // Then, if the entry is stale, we can return the stale entry and spawn a background task to refresh it.
-        // There's a little trickiness here in that
+        // There's a little trickiness here in that we can't directly spawn a task because that depends on a specific Async Runtime (tokio, smol, etc).
+        // The core SDK has an AsyncRuntime abstraction that we can use to spawn the task.
         Ok(self
             .container_properties_cache
             .try_get_with_by_ref(key, async { init.await.map(Arc::new) })
             .await?)
     }
-
-    /// Clears the cached container metadata for the specified key, so that the next request will fetch fresh data.
-    pub async fn clear_container_metadata(&self, key: &ResourceLink) {
-        self.container_properties_cache.invalidate(key).await;
-    }
 }

sdk/cosmos/azure_data_cosmos/src/cache.rs

@@ -26,7 +26,6 @@ use serde::{de::DeserializeOwned, Serialize};
 /// You can get a `Container` by calling [`DatabaseClient::container_client()`](crate::clients::DatabaseClient::container_client()).
 #[derive(Clone)]
 pub struct ContainerClient {
-    id: String,
     link: ResourceLink,
     items_link: ResourceLink,
     connection: CosmosConnection,
@@ -44,7 +43,6 @@ impl ContainerClient {
         let items_link = link.feed(ResourceType::Items);
 
         Self {
-            id: container_id.to_string(),
             link,
             items_link,
             connection,
@@ -77,7 +75,10 @@ impl ContainerClient {
         // TODO: Replace with `response.body().json()` when that becomes borrowing.
         let properties = serde_json::from_slice::<ContainerProperties>(response.body())?;
         let metadata = ContainerMetadata::from_properties(&properties, self.link.clone())?;
-        self.connection.cache.set_container_metadata(metadata).await;
+        self.connection
+            .cache()
+            .set_container_metadata(metadata)
+            .await;
 
         Ok(response.into())
     }
@@ -705,7 +706,7 @@ impl ContainerClient {
     async fn metadata(&self) -> azure_core::Result<Arc<ContainerMetadata>> {
         Ok(self
             .connection
-            .cache
+            .cache()
             .get_container_metadata(&self.link, async {
                 let properties = self.read_properties(None).await?.into_body()?;
                 ContainerMetadata::from_properties(&properties, self.link.clone())

sdk/cosmos/azure_data_cosmos/src/clients/container_client.rs

@@ -139,7 +139,7 @@ impl CosmosClient {
 
     /// Gets the endpoint of the database account this client is connected to.
     pub fn endpoint(&self) -> &Url {
-        &self.connection.endpoint
+        self.connection.endpoint()
     }
 
     /// Executes a query against databases in the account.

sdk/cosmos/azure_data_cosmos/src/clients/cosmos_client.rs

@@ -31,8 +31,8 @@ use crate::{
 /// A connection is cheap to clone, and all clones share the same underlying HTTP pipeline and metadata cache.
 #[derive(Clone)]
 pub struct CosmosConnection {
-    pub endpoint: Url,
-    pub cache: ContainerMetadataCache,
+    endpoint: Url,
+    cache: ContainerMetadataCache,
     pipeline: azure_core::http::Pipeline,
 }
 
@@ -56,6 +56,14 @@ impl CosmosConnection {
         }
     }
 
+    pub fn endpoint(&self) -> &Url {
+        &self.endpoint
+    }
+
+    pub fn cache(&self) -> &ContainerMetadataCache {
+        &self.cache
+    }
+
     /// Creates a [`Url`] out of the provided [`ResourceLink`]
     ///
     /// This is a little backwards, ideally we'd accept [`ResourceLink`] in the [`CosmosPipeline::send`] method,

sdk/cosmos/azure_data_cosmos/src/connection/mod.rs

@@ -1,16 +1,19 @@
 //! Internal module to define several newtypes used in the SDK.
 
 macro_rules! string_newtype {
-    ($name:ident) => {
+    ($(#[$attr:meta])* $name:ident) => {
+        $(#[$attr])*
         #[derive(serde::Deserialize, serde::Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
         #[serde(transparent)]
         pub struct $name(String);
 
         impl $name {
+            #[doc = concat!("Creates a new `", stringify!($name), "` from a `String`.")]
             pub fn new(value: String) -> Self {
                 Self(value)
             }
 
+            #[doc = concat!("Returns a reference to the inner `str` of the `", stringify!($name), "`.")]
             pub fn value(&self) -> &str {
                 &self.0
             }
@@ -36,4 +39,9 @@ macro_rules! string_newtype {
     };
 }
 
-string_newtype!(ResourceId);
+string_newtype!(
+    /// Represents a Resource ID, which is a unique identifier for a resource within a Cosmos DB account.
+    ///
+    /// In most cases, you don't need to use this type directly, as the SDK will handle resource IDs for you.
+    ResourceId
+);

sdk/cosmos/azure_data_cosmos/src/types.rs

@@ -241,8 +241,6 @@ pub async fn container_crud_hierarchical_pk(context: TestContext) -> Result<(),
 
 #[recorded::test]
 pub async fn container_read_throughput_twice(context: TestContext) -> Result<(), Box<dyn Error>> {
-    use azure_core::http::StatusCode;
-
     let recorder = Arc::new(LocalRecorder::new());
     let account = TestAccount::from_env(
         context,
@@ -256,7 +254,6 @@ pub async fn container_read_throughput_twice(context: TestContext) -> Result<(),
     let cosmos_client = account.connect_with_key(None)?;
     let db_client = test_data::create_database(&account, &cosmos_client).await?;
 
-    // Create the container with manual throughput
     let properties = ContainerProperties {
         id: "ThroughputTestContainer".into(),
         partition_key: "/id".into(),

sdk/cosmos/azure_data_cosmos/tests/cosmos_containers.rs

@@ -2,7 +2,7 @@ use std::sync::Arc;
 
 use azure_core::http::{
     policies::{Policy, PolicyResult},
-    BufResponse, Context, Method, RawResponse, Request, StatusCode,
+    BufResponse, Context, RawResponse, Request,
 };
 
 #[derive(Debug, Clone)]
@@ -23,21 +23,7 @@ impl LocalRecorder {
         }
     }
 
-    pub async fn to_transaction_summary(&self) -> Vec<(Method, String, Option<StatusCode>)> {
-        self.transactions
-            .read()
-            .await
-            .iter()
-            .map(|t| {
-                (
-                    t.request.method(),
-                    t.request.url().to_string(),
-                    t.response.as_ref().map(|r| r.status()),
-                )
-            })
-            .collect()
-    }
-
+    /// Returns a copy of all recorded transactions
     pub async fn to_transactions(&self) -> Vec<Transaction> {
         self.transactions.write().await.clone()
     }