Merge pull request #4 from ValuedMammal/review/103

Review 103

Author: praveenperera

src/async.rs

@@ -11,9 +11,11 @@
 
 //! Esplora by way of `reqwest` HTTP client.
 
+use async_trait::async_trait;
 use std::collections::HashMap;
 use std::marker::PhantomData;
 use std::str::FromStr;
+use std::time::Duration;
 
 use bitcoin::consensus::{deserialize, serialize, Decodable, Encodable};
 use bitcoin::hashes::{sha256, Hash};
@@ -28,8 +30,8 @@ use log::{debug, error, info, trace};
 use reqwest::{header, Client, Response};
 
 use crate::{
-    BlockStatus, BlockSummary, Builder, DefaultSleeper, Error, MerkleProof, OutputStatus, Tx,
-    TxStatus, BASE_BACKOFF_MILLIS, RETRYABLE_ERROR_CODES,
+    BlockStatus, BlockSummary, Builder, Error, MerkleProof, OutputStatus, Tx, TxStatus,
+    BASE_BACKOFF_MILLIS, RETRYABLE_ERROR_CODES,
 };
 
 #[derive(Debug, Clone)]
@@ -41,12 +43,12 @@ pub struct AsyncClient<S = DefaultSleeper> {
     /// Number of times to retry a request
     max_retries: usize,
 
-    runtime: PhantomData<S>,
+    marker: PhantomData<S>,
 }
 
 impl<S: Sleeper> AsyncClient<S> {
     /// Build an async client from a builder
-    pub fn from_builder(builder: Builder<S>) -> Result<Self, Error> {
+    pub fn from_builder(builder: Builder) -> Result<Self, Error> {
         let mut client_builder = Client::builder();
 
         #[cfg(not(target_arch = "wasm32"))]
@@ -75,7 +77,7 @@ impl<S: Sleeper> AsyncClient<S> {
             url: builder.base_url,
             client: client_builder.build()?,
             max_retries: builder.max_retries,
-            runtime: PhantomData,
+            marker: PhantomData,
         })
     }
 
@@ -84,7 +86,7 @@ impl<S: Sleeper> AsyncClient<S> {
             url,
             client,
             max_retries: crate::DEFAULT_MAX_RETRIES,
-            runtime: PhantomData,
+            marker: PhantomData,
         }
     }
 
@@ -451,15 +453,22 @@ fn is_status_retryable(status: reqwest::StatusCode) -> bool {
     RETRYABLE_ERROR_CODES.contains(&status.as_u16())
 }
 
-#[async_trait::async_trait]
+/// Trait that defines the ability to sleep within an async runtime
+#[async_trait]
 pub trait Sleeper {
-    async fn sleep(duration: std::time::Duration);
+    /// Wait until the specified `duration` has elapsed
+    async fn sleep(duration: Duration);
 }
 
-#[cfg(feature = "tokio")]
-#[async_trait::async_trait]
-impl Sleeper for crate::DefaultSleeper {
-    async fn sleep(duration: std::time::Duration) {
+/// Default sleeper. Note this may only be used as a [`Sleeper`] implementation
+/// if the "tokio" feature is enabled.
+#[derive(Debug, Clone, Copy)]
+pub struct DefaultSleeper;
+
+#[cfg(any(test, feature = "tokio"))]
+#[async_trait]
+impl Sleeper for DefaultSleeper {
+    async fn sleep(duration: Duration) {
         tokio::time::sleep(duration).await;
     }
 }

src/lib.rs

@@ -66,13 +66,15 @@
 
 #![allow(clippy::result_large_err)]
 
+use std::collections::HashMap;
 use std::fmt;
 use std::num::TryFromIntError;
 use std::time::Duration;
-use std::{collections::HashMap, marker::PhantomData};
 
-pub mod api;
+#[cfg(feature = "async")]
+use r#async::Sleeper;
 
+pub mod api;
 #[cfg(feature = "async")]
 pub mod r#async;
 #[cfg(feature = "blocking")]
@@ -111,7 +113,7 @@ pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Option<f
 }
 
 #[derive(Debug, Clone)]
-pub struct Builder<S = DefaultSleeper> {
+pub struct Builder {
     /// The URL of the Esplora server.
     pub base_url: String,
     /// Optional URL of the proxy to use to make requests to the Esplora server
@@ -133,13 +135,8 @@ pub struct Builder<S = DefaultSleeper> {
     pub headers: HashMap<String, String>,
     /// Max retries
     pub max_retries: usize,
-    /// Async runtime, trait must implement `sleep` function, default is `tokio`
-    marker: PhantomData<S>,
 }
 
-#[derive(Debug, Clone, Copy)]
-pub struct DefaultSleeper;
-
 impl Builder {
     /// Instantiate a new builder
     pub fn new(base_url: &str) -> Self {
@@ -149,38 +146,9 @@ impl Builder {
             timeout: None,
             headers: HashMap::new(),
             max_retries: DEFAULT_MAX_RETRIES,
-            marker: PhantomData,
-        }
-    }
-
-    /// Build a blocking client from builder
-    #[cfg(feature = "blocking")]
-    pub fn build_blocking(self) -> BlockingClient {
-        BlockingClient::from_builder(self)
-    }
-}
-
-#[cfg(feature = "async")]
-impl<S: r#async::Sleeper> Builder<S> {
-    /// Instantiate a new builder, with a custom runtime
-    pub fn new_custom_runtime(base_url: &str) -> Self {
-        Builder {
-            base_url: base_url.to_string(),
-            proxy: None,
-            timeout: None,
-            headers: HashMap::new(),
-            max_retries: DEFAULT_MAX_RETRIES,
-            marker: PhantomData,
         }
     }
 
-    // Build an asynchronous client from builder
-    pub fn build_async(self) -> Result<AsyncClient<S>, Error> {
-        AsyncClient::from_builder(self)
-    }
-}
-
-impl<S> Builder<S> {
     /// Set the proxy of the builder
     pub fn proxy(mut self, proxy: &str) -> Self {
         self.proxy = Some(proxy.to_string());
@@ -205,6 +173,25 @@ impl<S> Builder<S> {
         self.max_retries = count;
         self
     }
+
+    /// Build a blocking client from builder
+    #[cfg(feature = "blocking")]
+    pub fn build_blocking(self) -> BlockingClient {
+        BlockingClient::from_builder(self)
+    }
+
+    /// Build an asynchronous client from builder
+    #[cfg(feature = "tokio")]
+    pub fn build_async(self) -> Result<AsyncClient, Error> {
+        AsyncClient::from_builder(self)
+    }
+
+    /// Build an asynchronous client from builder where the returned client uses a
+    /// user-defined [`Sleeper`].
+    #[cfg(feature = "async")]
+    pub fn build_async_with_sleeper<S: Sleeper>(self) -> Result<AsyncClient<S>, Error> {
+        AsyncClient::from_builder(self)
+    }
 }
 
 /// Errors that can happen during a request to `Esplora` servers.
@@ -284,6 +271,7 @@ mod test {
             bitcoind::bitcoincore_rpc::json::AddressType, bitcoind::bitcoincore_rpc::RpcApi,
             electrum_client::ElectrumApi,
         },
+        r#async::DefaultSleeper,
         std::time::Duration,
         tokio::sync::OnceCell,
     };
@@ -342,7 +330,9 @@ mod test {
         let blocking_client = builder.build_blocking();
 
         let builder_async = Builder::new(&format!("http://{}", esplora_url));
-        let async_client = builder_async.build_async().unwrap();
+        let async_client = builder_async
+            .build_async_with_sleeper::<DefaultSleeper>()
+            .unwrap();
 
         (blocking_client, async_client)
     }
@@ -1015,35 +1005,10 @@ mod test {
         assert_eq!(tx, tx_async);
     }
 
-    #[cfg(all(feature = "async", feature = "tokio"))]
+    #[cfg(feature = "tokio")]
     #[test]
     fn use_builder_with_tokio_as_normal() {
         let builder = Builder::new("https://blockstream.info/testnet/api");
-        let client = builder.build_async();
-        assert!(client.is_ok());
-    }
-
-    #[cfg(all(feature = "async", not(feature = "tokio")))]
-    mod custom_async_runtime {
-        use super::*;
-        use crate::r#async::Sleeper;
-
-        struct TestRuntime;
-
-        #[async_trait::async_trait]
-        impl Sleeper for TestRuntime {
-            async fn sleep(duration: Duration) {
-                tokio::time::sleep(duration).await;
-            }
-        }
-
-        #[test]
-        fn use_with_custom_runtime() {
-            let builder =
-                Builder::<TestRuntime>::new_custom_runtime("https://blockstream.info/testnet/api");
-
-            let client = builder.build_async();
-            assert!(client.is_ok());
-        }
+        let _client = builder.build_async().unwrap();
     }
 }