Simplified Client interface. (#408)

Currently, the Client trait has numerous small traits underneath it, but
only the umbrella Client trait is ever used. This PR simplifies this
interface by dropping the unneeded fine-grained traits.

It also consolidates the multiple routes for the global dedup query into
a single function that returns an in-memory shard to further reduce the
complexity.

There should be no functionality change, just code moving around and
trait simplification.

Author: hoytak

cas_client/src/interface.rs

@@ -1,167 +1,83 @@
+use std::collections::HashMap;
+use std::sync::Arc;
+
+use bytes::Bytes;
+use cas_object::SerializedCasObject;
+use cas_types::FileRange;
+use mdb_shard::file_structs::MDBFileInfo;
+use merklehash::MerkleHash;
+use progress_tracking::item_tracking::SingleItemProgressUpdater;
+use progress_tracking::upload_tracking::CompletionTracker;
+
+use crate::error::Result;
 #[cfg(not(target_family = "wasm"))]
-use mdb_shard::shard_file_reconstructor::FileReconstructor;
-
-#[cfg(not(target_family = "wasm"))]
-use crate::CasClientError;
+use crate::OutputProvider;
 
 /// A Client to the Shard service. The shard service
 /// provides for
 /// 1. upload shard to the shard service
 /// 2. querying of file->reconstruction information
 /// 3. querying of chunk->shard information
-#[cfg(not(target_family = "wasm"))]
-pub trait ShardClientInterface:
-    RegistrationClient + FileReconstructor<CasClientError> + ShardDedupProber + Send + Sync
-{
-}
-// In webassembly environment ShardClientInterface does not include FileReconstructor
-// and does not require Send/Sync'ness
-#[cfg(target_family = "wasm")]
-pub trait ShardClientInterface: RegistrationClient + ShardDedupProber {}
-
-#[cfg(not(target_family = "wasm"))]
-pub trait Client: UploadClient + ReconstructionClient + ShardClientInterface {}
-// in webassembly environment the general "Client" interface does not support the
-// ReconstructionClient download interface.
-#[cfg(target_family = "wasm")]
-pub trait Client: UploadClient + ShardClientInterface {}
-
-// interfaces used on the download path, primarily relating to fetching and using file reconstructions
-// to download file data.
-// not enabled in webassembly
-#[cfg(not(target_family = "wasm"))]
-mod download {
-    use std::collections::HashMap;
-    use std::sync::Arc;
-
-    use cas_types::{FileRange, QueryReconstructionResponse};
-    use merklehash::MerkleHash;
-    use progress_tracking::item_tracking::SingleItemProgressUpdater;
-
-    use crate::error::Result;
-    use crate::output_provider::OutputProvider;
-
-    /// A Client to the CAS (Content Addressed Storage) service to allow reconstructing a
-    /// pointer file based on FileID (MerkleHash).
+#[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
+#[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
+pub trait Client {
+    /// Get an entire file by file hash with an optional bytes range.
     ///
-    /// To simplify this crate, it is intentional that the client does not create its own http_client or
-    /// spawn its own threads. Instead, it is expected to be given the parallelism harness/threadpool/queue
-    /// on which it is expected to run. This allows the caller to better optimize overall system utilization
-    /// by controlling the number of concurrent requests.
-    #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-    #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
-    pub trait ReconstructionClient {
-        /// Get an entire file by file hash with an optional bytes range.
-        ///
-        /// The http_client passed in is a non-authenticated client. This is used to directly communicate
-        /// with the backing store (S3) to retrieve xorbs.
-        async fn get_file(
-            &self,
-            hash: &MerkleHash,
-            byte_range: Option<FileRange>,
-            output_provider: &OutputProvider,
-            progress_updater: Option<Arc<SingleItemProgressUpdater>>,
-        ) -> Result<u64>;
-
-        async fn batch_get_file(&self, files: HashMap<MerkleHash, &OutputProvider>) -> Result<u64> {
-            let mut n_bytes = 0;
-            // Provide the basic naive implementation as a default.
-            for (h, w) in files {
-                n_bytes += self.get_file(&h, None, w, None).await?;
-            }
-            Ok(n_bytes)
+    /// The http_client passed in is a non-authenticated client. This is used to directly communicate
+    /// with the backing store (S3) to retrieve xorbs.
+    #[cfg(not(target_family = "wasm"))]
+    async fn get_file(
+        &self,
+        hash: &MerkleHash,
+        byte_range: Option<FileRange>,
+        output_provider: &OutputProvider,
+        progress_updater: Option<Arc<SingleItemProgressUpdater>>,
+    ) -> Result<u64>;
+
+    #[cfg(not(target_family = "wasm"))]
+    async fn batch_get_file(&self, files: HashMap<MerkleHash, &OutputProvider>) -> Result<u64> {
+        let mut n_bytes = 0;
+        // Provide the basic naive implementation as a default.
+        for (h, w) in files {
+            n_bytes += self.get_file(&h, None, w, None).await?;
         }
+        Ok(n_bytes)
     }
 
-    /// A Client to the CAS (Content Addressed Storage) service that is able to obtain
-    /// the reconstruction info of a file by FileID (MerkleHash). Return
-    /// - Ok(Some(response)) if the query succeeded,
-    /// - Ok(None) if the specified range can't be satisfied,
-    /// - Err(e) for other errors.
-    #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-    #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
-    pub trait Reconstruct {
-        async fn get_reconstruction(
-            &self,
-            hash: &MerkleHash,
-            byte_range: Option<FileRange>,
-        ) -> Result<Option<QueryReconstructionResponse>>;
-    }
-}
-
-// upload interfaces for operations required for the upload path
-// including global deduplication
-// enabled in standard build environments and web assembly
-mod upload {
-    use std::sync::Arc;
-
-    use cas_object::SerializedCasObject;
-    use merklehash::MerkleHash;
-    use progress_tracking::upload_tracking::CompletionTracker;
-
-    use crate::error::Result;
-
-    #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-    #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
-    pub trait RegistrationClient {
-        async fn upload_shard(
-            &self,
-            prefix: &str,
-            hash: &MerkleHash,
-            force_sync: bool,
-            shard_data: bytes::Bytes,
-            salt: &[u8; 32],
-        ) -> Result<bool>;
-    }
-
-    /// Probes for shards that provide dedup information for a chunk, and, if
-    /// any are found, writes them to disk and returns the path.
-    #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-    #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
-    pub trait ShardDedupProber {
-        #[cfg(not(target_family = "wasm"))]
-        async fn query_for_global_dedup_shard(
-            &self,
-            prefix: &str,
-            chunk_hash: &MerkleHash,
-            salt: &[u8; 32],
-        ) -> Result<Option<std::path::PathBuf>>;
-
-        async fn query_for_global_dedup_shard_in_memory(
-            &self,
-            prefix: &str,
-            chunk_hash: &MerkleHash,
-            salt: &[u8; 32],
-        ) -> Result<Option<Vec<u8>>>;
-    }
-
-    /// A Client to the CAS (Content Addressed Storage) service to allow storage and
-    /// management of XORBs (Xet Object Remote Block). A XORB represents a collection
-    /// of arbitrary bytes. These bytes are hashed according to a Xet Merkle Hash
-    /// producing a Merkle Tree. XORBs in the CAS are identified by a combination of
-    /// a prefix namespacing the XORB and the hash at the root of the Merkle Tree.
-    #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-    #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
-    pub trait UploadClient {
-        /// Insert a serialized XORB into the CAS, returning the number of bytes read.
-        async fn upload_xorb(
-            &self,
-            prefix: &str,
-            serialized_cas_object: SerializedCasObject,
-            upload_tracker: Option<Arc<CompletionTracker>>,
-        ) -> Result<u64>;
-
-        /// Check if a XORB already exists.
-        async fn exists(&self, prefix: &str, hash: &MerkleHash) -> Result<bool>;
-
-        /// Indicates if the serialized cas object should have a written footer.
-        /// This should only be true for testing with LocalClient.
-        fn use_xorb_footer(&self) -> bool;
-    }
+    async fn get_file_reconstruction_info(
+        &self,
+        file_hash: &MerkleHash,
+    ) -> Result<Option<(MDBFileInfo, Option<MerkleHash>)>>;
+
+    async fn query_for_global_dedup_shard(
+        &self,
+        prefix: &str,
+        chunk_hash: &MerkleHash,
+        salt: &[u8; 32],
+    ) -> Result<Option<Bytes>>;
+
+    /// Upload a new shard.
+    async fn upload_shard(
+        &self,
+        prefix: &str,
+        hash: &MerkleHash,
+        force_sync: bool,
+        shard_data: bytes::Bytes,
+        salt: &[u8; 32],
+    ) -> Result<bool>;
+
+    /// Upload a new xorb.
+    async fn upload_xorb(
+        &self,
+        prefix: &str,
+        serialized_cas_object: SerializedCasObject,
+        upload_tracker: Option<Arc<CompletionTracker>>,
+    ) -> Result<u64>;
+
+    /// Check if a XORB already exists.
+    async fn exists(&self, prefix: &str, hash: &MerkleHash) -> Result<bool>;
+
+    /// Indicates if the serialized cas object should have a written footer.
+    /// This should only be true for testing with LocalClient.
+    fn use_xorb_footer(&self) -> bool;
 }
-
-// export out interfaces to be referred to directly out of the interface sub-crate
-// users of cas_client interface are unaware of the module level separation between download/upload.
-#[cfg(not(target_family = "wasm"))]
-pub use download::*;
-pub use upload::*;

cas_client/src/lib.rs

@@ -2,18 +2,14 @@
 
 pub use chunk_cache::{CacheConfig, CHUNK_CACHE_SIZE_BYTES};
 pub use http_client::{build_auth_http_client, build_http_client, Api, RetryConfig};
-use interface::RegistrationClient;
-pub use interface::{Client, UploadClient};
-#[cfg(not(target_family = "wasm"))]
-pub use interface::{Reconstruct, ReconstructionClient};
+pub use interface::Client;
 #[cfg(not(target_family = "wasm"))]
 pub use local_client::LocalClient;
 #[cfg(not(target_family = "wasm"))]
 pub use output_provider::{FileProvider, OutputProvider};
 pub use remote_client::RemoteClient;
 
 pub use crate::error::CasClientError;
-pub use crate::interface::ShardClientInterface;
 
 mod constants;
 #[cfg(not(target_family = "wasm"))]