util `fake-fetch` (#8363)

* start

* hash-fetch

* rpc

* revert price-info

* remove used file

* adapt to changes in fetch

* make fake-fetch generic over

* fix tests to comply with the new `fake-fetch`

Cargo.lock

@@ -1025,6 +1025,15 @@ dependencies = [
  "vm 0.1.0",
 ]
 
+[[package]]
+name = "fake-fetch"
+version = "0.0.1"
+dependencies = [
+ "fetch 0.1.0",
+ "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "hyper 0.11.24 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "fdlimit"
 version = "0.1.1"
@@ -1940,6 +1949,7 @@ dependencies = [
  "ethcore-transaction 0.1.0",
  "ethereum-types 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethkey 0.3.0",
+ "fake-fetch 0.0.1",
  "fdlimit 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2063,6 +2073,7 @@ dependencies = [
  "ethabi-derive 5.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethcore-bytes 0.1.0",
  "ethereum-types 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "fake-fetch 0.0.1",
  "fetch 0.1.0",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2148,6 +2159,7 @@ dependencies = [
  "ethjson 0.1.0",
  "ethkey 0.3.0",
  "ethstore 0.2.0",
+ "fake-fetch 0.0.1",
  "fetch 0.1.0",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2465,6 +2477,7 @@ dependencies = [
 name = "price-info"
 version = "1.11.0"
 dependencies = [
+ "fake-fetch 0.0.1",
  "fetch 0.1.0",
  "futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",

Cargo.toml

@@ -82,6 +82,7 @@ rustc_version = "0.2"
 pretty_assertions = "0.1"
 ipnetwork = "0.12.6"
 tempdir = "0.3"
+fake-fetch = { path = "util/fake-fetch" }
 
 [target.'cfg(windows)'.dependencies]
 winapi = "0.2"

hash-fetch/Cargo.toml

@@ -28,3 +28,4 @@ ethabi-contract = "5.0"
 [dev-dependencies]
 hyper = "0.11"
 parking_lot = "0.5"
+fake-fetch = { path = "../util/fake-fetch" }

hash-fetch/src/client.rs

@@ -193,53 +193,14 @@ fn random_temp_path() -> PathBuf {
 
 #[cfg(test)]
 mod tests {
-	extern crate hyper;
+	use fake_fetch::FakeFetch;
 	use rustc_hex::FromHex;
 	use std::sync::{Arc, mpsc};
 	use parking_lot::Mutex;
-	use futures::future;
 	use futures_cpupool::CpuPool;
-	use fetch::{self, Fetch, Url, Request};
 	use parity_reactor::Remote;
 	use urlhint::tests::{FakeRegistrar, URLHINT};
 	use super::{Error, Client, HashFetch, random_temp_path};
-	use self::hyper::StatusCode;
-
-
-	#[derive(Clone)]
-	struct FakeFetch {
-		return_success: bool
-	}
-
-	impl Fetch for FakeFetch {
-		type Result = future::Ok<fetch::Response, fetch::Error>;
-
-		fn fetch(&self, request: Request, abort: fetch::Abort) -> Self::Result {
-			assert_eq!(request.url().as_str(), "https://parity.io/assets/images/ethcore-black-horizontal.png");
-			let u = request.url().clone();
-			future::ok(if self.return_success {
-				fetch::client::Response::new(u, hyper::Response::new().with_body(&b"result"[..]), abort)
-			} else {
-				fetch::client::Response::new(u, hyper::Response::new().with_status(StatusCode::NotFound), abort)
-			})
-		}
-
-		fn get(&self, url: &str, abort: fetch::Abort) -> Self::Result {
-			let url: Url = match url.parse() {
-				Ok(u) => u,
-				Err(e) => return future::err(e.into())
-			};
-			self.fetch(Request::get(url), abort)
-		}
-
-		fn post(&self, url: &str, abort: fetch::Abort) -> Self::Result {
-			let url: Url = match url.parse() {
-				Ok(u) => u,
-				Err(e) => return future::err(e.into())
-			};
-			self.fetch(Request::post(url), abort)
-		}
-	}
 
 	fn registrar() -> FakeRegistrar {
 		let mut registrar = FakeRegistrar::new();
@@ -254,7 +215,7 @@ mod tests {
 	fn should_return_error_if_hash_not_found() {
 		// given
 		let contract = Arc::new(FakeRegistrar::new());
-		let fetch = FakeFetch { return_success: false };
+		let fetch = FakeFetch::new(None::<usize>);
 		let client = Client::with_fetch(contract.clone(), CpuPool::new(1), fetch, Remote::new_sync());
 
 		// when
@@ -272,7 +233,7 @@ mod tests {
 	fn should_return_error_if_response_is_not_successful() {
 		// given
 		let registrar = Arc::new(registrar());
-		let fetch = FakeFetch { return_success: false };
+		let fetch = FakeFetch::new(None::<usize>);
 		let client = Client::with_fetch(registrar.clone(), CpuPool::new(1), fetch, Remote::new_sync());
 
 		// when
@@ -290,7 +251,7 @@ mod tests {
 	fn should_return_hash_mismatch() {
 		// given
 		let registrar = Arc::new(registrar());
-		let fetch = FakeFetch { return_success: true };
+		let fetch = FakeFetch::new(Some(1));
 		let mut client = Client::with_fetch(registrar.clone(), CpuPool::new(1), fetch, Remote::new_sync());
 		let path = random_temp_path();
 		let path2 = path.clone();
@@ -304,7 +265,7 @@ mod tests {
 
 		// then
 		let result = rx.recv().unwrap();
-		let hash = "0x06b0a4f426f6713234b2d4b2468640bc4e0bb72657a920ad24c5087153c593c8".into();
+		let hash = "0x2be00befcf008bc0e7d9cdefc194db9c75352e8632f48498b5a6bfce9f02c88e".into();
 		assert_eq!(result.unwrap_err(), Error::HashMismatch { expected: 2.into(), got: hash });
 		assert!(!path.exists(), "Temporary file should be removed.");
 	}
@@ -313,12 +274,12 @@ mod tests {
 	fn should_return_path_if_hash_matches() {
 		// given
 		let registrar = Arc::new(registrar());
-		let fetch = FakeFetch { return_success: true };
+		let fetch = FakeFetch::new(Some(1));
 		let client = Client::with_fetch(registrar.clone(), CpuPool::new(1), fetch, Remote::new_sync());
 
 		// when
 		let (tx, rx) = mpsc::channel();
-		client.fetch("0x06b0a4f426f6713234b2d4b2468640bc4e0bb72657a920ad24c5087153c593c8".into(),
+		client.fetch("0x2be00befcf008bc0e7d9cdefc194db9c75352e8632f48498b5a6bfce9f02c88e".into(),
 			Default::default(),
 			Box::new(move |result| { tx.send(result).unwrap(); }));
 

hash-fetch/src/lib.rs

@@ -42,6 +42,10 @@ extern crate ethabi_derive;
 extern crate ethabi_contract;
 #[cfg(test)]
 extern crate parking_lot;
+#[cfg(test)]
+extern crate hyper;
+#[cfg(test)]
+extern crate fake_fetch;
 
 mod client;
 

price-info/Cargo.toml

@@ -16,3 +16,4 @@ serde_json = "1.0"
 [dev-dependencies]
 hyper = "0.11"
 parking_lot = "0.5"
+fake-fetch = { path = "../util/fake-fetch" }

price-info/src/lib.rs

@@ -25,6 +25,9 @@ extern crate serde_json;
 #[macro_use]
 extern crate log;
 
+#[cfg(test)]
+extern crate fake_fetch;
+
 pub extern crate fetch;
 
 use std::cmp;
@@ -133,68 +136,18 @@ impl<F: Fetch> Client<F> {
 
 #[cfg(test)]
 mod test {
-	extern crate hyper;
-	extern crate parking_lot;
-
-	use self::parking_lot::Mutex;
 	use std::sync::Arc;
-	use std::sync::atomic::{AtomicBool, Ordering};
-	use fetch;
-	use fetch::{Fetch, Url, Request};
 	use futures_cpupool::CpuPool;
-	use futures::future::{self, FutureResult};
 	use Client;
-	use self::hyper::StatusCode;
+	use std::sync::atomic::{AtomicBool, Ordering};
+	use fake_fetch::FakeFetch;
 
-	#[derive(Clone)]
-	struct FakeFetch(Option<String>, Arc<Mutex<u64>>);
-
-	impl FakeFetch {
-		fn new() -> Result<Self, fetch::Error> {
-			Ok(FakeFetch(None, Default::default()))
-		}
+	fn price_info_ok(response: &str) -> Client<FakeFetch<String>> {
+		Client::new(FakeFetch::new(Some(response.to_owned())), CpuPool::new(1))
 	}
 
-	impl Fetch for FakeFetch {
-		type Result = FutureResult<fetch::Response, fetch::Error>;
-
-		fn fetch(&self, request: Request, abort: fetch::Abort) -> Self::Result {
-			assert_eq!(request.url().as_str(), "https://api.etherscan.io/api?module=stats&action=ethprice");
-			let u = request.url().clone();
-			let mut val = self.1.lock();
-			*val = *val + 1;
-			if let Some(ref response) = self.0 {
-				let r = hyper::Response::new().with_body(response.clone());
-				future::ok(fetch::client::Response::new(u, r, abort))
-			} else {
-				let r = hyper::Response::new().with_status(StatusCode::NotFound);
-				future::ok(fetch::client::Response::new(u, r, abort))
-			}
-		}
-
-		fn get(&self, url: &str, abort: fetch::Abort) -> Self::Result {
-			let url: Url = match url.parse() {
-				Ok(u) => u,
-				Err(e) => return future::err(e.into())
-			};
-			self.fetch(Request::get(url), abort)
-		}
-
-		fn post(&self, url: &str, abort: fetch::Abort) -> Self::Result {
-			let url: Url = match url.parse() {
-				Ok(u) => u,
-				Err(e) => return future::err(e.into())
-			};
-			self.fetch(Request::post(url), abort)
-		}
-	}
-
-	fn price_info_ok(response: &str) -> Client<FakeFetch> {
-		Client::new(FakeFetch(Some(response.to_owned()), Default::default()), CpuPool::new(1))
-	}
-
-	fn price_info_not_found() -> Client<FakeFetch> {
-		Client::new(FakeFetch::new().unwrap(), CpuPool::new(1))
+	fn price_info_not_found() -> Client<FakeFetch<String>> {
+		Client::new(FakeFetch::new(None::<String>), CpuPool::new(1))
 	}
 
 	#[test]

rpc/Cargo.toml

@@ -70,3 +70,4 @@ pretty_assertions = "0.1"
 macros = { path = "../util/macros" }
 ethcore-network = { path = "../util/network" }
 kvdb-memorydb = { path = "../util/kvdb-memorydb" }
+fake-fetch = { path = "../util/fake-fetch" }

rpc/src/lib.rs

@@ -91,6 +91,9 @@ extern crate macros;
 #[cfg(test)]
 extern crate kvdb_memorydb;
 
+#[cfg(test)]
+extern crate fake_fetch;
+
 extern crate tempdir;
 
 pub extern crate jsonrpc_ws_server as ws;

rpc/src/v1/tests/helpers/mod.rs

@@ -17,14 +17,12 @@
 //! Test rpc services.
 
 mod dapps;
-mod fetch;
 mod miner_service;
 mod snapshot_service;
 mod sync_provider;
 mod update_service;
 
 pub use self::dapps::TestDappsService;
-pub use self::fetch::TestFetch;
 pub use self::miner_service::TestMinerService;
 pub use self::snapshot_service::TestSnapshotService;
 pub use self::sync_provider::{Config, TestSyncProvider};

rpc/src/v1/tests/mocked/parity_set.rs

@@ -26,9 +26,11 @@ use futures_cpupool::CpuPool;
 
 use jsonrpc_core::IoHandler;
 use v1::{ParitySet, ParitySetClient};
-use v1::tests::helpers::{TestMinerService, TestFetch, TestUpdater, TestDappsService};
+use v1::tests::helpers::{TestMinerService, TestUpdater, TestDappsService};
 use super::manage_network::TestManageNetwork;
 
+use fake_fetch::FakeFetch;
+
 fn miner_service() -> Arc<TestMinerService> {
 	Arc::new(TestMinerService::default())
 }
@@ -45,7 +47,7 @@ fn updater_service() -> Arc<TestUpdater> {
 	Arc::new(TestUpdater::default())
 }
 
-pub type TestParitySetClient = ParitySetClient<TestBlockChainClient, TestMinerService, TestUpdater, TestFetch>;
+pub type TestParitySetClient = ParitySetClient<TestBlockChainClient, TestMinerService, TestUpdater, FakeFetch<usize>>;
 
 fn parity_set_client(
 	client: &Arc<TestBlockChainClient>,
@@ -55,7 +57,7 @@ fn parity_set_client(
 ) -> TestParitySetClient {
 	let dapps_service = Arc::new(TestDappsService);
 	let pool = CpuPool::new(1);
-	ParitySetClient::new(client, miner, updater, &(net.clone() as Arc<ManageNetwork>), Some(dapps_service), TestFetch::default(), pool)
+	ParitySetClient::new(client, miner, updater, &(net.clone() as Arc<ManageNetwork>), Some(dapps_service), FakeFetch::new(Some(1)), pool)
 }
 
 #[test]

util/fake-fetch/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+description = "Mock fetcher for testing"
+name = "fake-fetch"
+version = "0.0.1"
+license = "GPL-3.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+
+[dependencies]
+fetch = { path = "../fetch" }
+futures = "0.1"
+hyper = "0.11"

util/fake-fetch/src/lib.rs

@@ -1,4 +1,4 @@
-// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// Copyright 2015-2018 Parity Technologies (UK) Ltd.
 // This file is part of Parity.
 
 // Parity is free software: you can redistribute it and/or modify
@@ -14,37 +14,42 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-//! Test implementation of fetch client.
+extern crate fetch;
+extern crate hyper;
+extern crate futures;
 
-use std::thread;
-use std::boxed::Box;
-use jsonrpc_core::futures::{self, Future};
-use fetch::{self, Fetch, Url, Request};
-use futures::future;
-use hyper;
+use hyper::StatusCode;
+use futures::{future, future::FutureResult};
+use fetch::{Fetch, Url, Request};
 
-/// Test implementation of fetcher. Will always return the same file.
-#[derive(Default, Clone)]
-pub struct TestFetch;
+#[derive(Clone, Default)]
+pub struct FakeFetch<T> where T: Clone + Send + Sync {
+	val: Option<T>,
+}
 
-impl Fetch for TestFetch {
-	type Result = Box<Future<Item = fetch::Response, Error = fetch::Error> + Send + 'static>;
+impl<T> FakeFetch<T> where T: Clone + Send + Sync {
+	pub fn new(t: Option<T>) -> Self {
+		FakeFetch { val : t }
+	}
+}
+
+impl<T: 'static> Fetch for FakeFetch<T> where T: Clone + Send+ Sync {
+	type Result = FutureResult<fetch::Response, fetch::Error>;
 
 	fn fetch(&self, request: Request, abort: fetch::Abort) -> Self::Result {
 		let u = request.url().clone();
-		let (tx, rx) = futures::oneshot();
-		thread::spawn(move || {
+		future::ok(if self.val.is_some() {
 			let r = hyper::Response::new().with_body(&b"Some content"[..]);
-			tx.send(fetch::Response::new(u, r, abort)).unwrap();
-		});
-
-		Box::new(rx.map_err(|_| fetch::Error::Aborted))
+			fetch::client::Response::new(u, r, abort)
+		} else {
+			fetch::client::Response::new(u, hyper::Response::new().with_status(StatusCode::NotFound), abort)
+		})
 	}
 
 	fn get(&self, url: &str, abort: fetch::Abort) -> Self::Result {
 		let url: Url = match url.parse() {
 			Ok(u) => u,
-			Err(e) => return Box::new(future::err(e.into()))
+			Err(e) => return future::err(e.into())
 		};
 		self.fetch(Request::get(url), abort)
 	}
@@ -52,7 +57,7 @@ impl Fetch for TestFetch {
 	fn post(&self, url: &str, abort: fetch::Abort) -> Self::Result {
 		let url: Url = match url.parse() {
 			Ok(u) => u,
-			Err(e) => return Box::new(future::err(e.into()))
+			Err(e) => return future::err(e.into())
 		};
 		self.fetch(Request::post(url), abort)
 	}