Return old-ish content even when syncing (#2757)

dapps/src/apps/fetcher.rs

@@ -69,6 +69,15 @@ impl<R: URLHint> ContentFetcher<R> {
 		}
 	}
 
+	fn still_syncing() -> Box<Handler> {
+		Box::new(ContentHandler::error(
+			StatusCode::ServiceUnavailable,
+			"Sync In Progress",
+			"Your node is still syncing. We cannot resolve any content before it's fully synced.",
+			Some("<a href=\"javascript:window.location.reload()\">Refresh</a>")
+		))
+	}
+
 	#[cfg(test)]
 	fn set_status(&self, content_id: &str, status: ContentStatus) {
 		self.cache.lock().insert(content_id.to_owned(), status);
@@ -84,12 +93,10 @@ impl<R: URLHint> ContentFetcher<R> {
 		}
 		// fallback to resolver
 		if let Ok(content_id) = content_id.from_hex() {
-			// if app_id is valid, but we are syncing always return true.
-			if self.sync.is_major_importing() {
-				return true;
-			}
 			// else try to resolve the app_id
-			self.resolver.resolve(content_id).is_some()
+			let has_content = self.resolver.resolve(content_id).is_some();
+			// if there is content or we are syncing return true
+			has_content || self.sync.is_major_importing()
 		} else {
 			false
 		}
@@ -99,28 +106,19 @@ impl<R: URLHint> ContentFetcher<R> {
 		let mut cache = self.cache.lock();
 		let content_id = path.app_id.clone();
 
-		if self.sync.is_major_importing() {
-			return Box::new(ContentHandler::error(
-				StatusCode::ServiceUnavailable,
-				"Sync In Progress",
-				"Your node is still syncing. We cannot resolve any content before it's fully synced.",
-				Some("<a href=\"javascript:window.location.reload()\">Refresh</a>")
-			));
-		}
-
 		let (new_status, handler) = {
 			let status = cache.get(&content_id);
 			match status {
-				// Just server dapp
+				// Just serve the content
 				Some(&mut ContentStatus::Ready(ref endpoint)) => {
 					(None, endpoint.to_async_handler(path, control))
 				},
-				// App is already being fetched
+				// Content is already being fetched
 				Some(&mut ContentStatus::Fetching(ref fetch_control)) => {
 					trace!(target: "dapps", "Content fetching in progress. Waiting...");
 					(None, fetch_control.to_handler(control))
 				},
-				// We need to start fetching app
+				// We need to start fetching the content
 				None => {
 					trace!(target: "dapps", "Content unavailable. Fetching... {:?}", content_id);
 					let content_hex = content_id.from_hex().expect("to_handler is called only when `contains` returns true.");
@@ -141,6 +139,10 @@ impl<R: URLHint> ContentFetcher<R> {
 					};
 
 					match content {
+						// Don't serve dapps if we are still syncing (but serve content)
+						Some(URLHintResult::Dapp(_)) if self.sync.is_major_importing() => {
+							(None, Self::still_syncing())
+						},
 						Some(URLHintResult::Dapp(dapp)) => {
 							let (handler, fetch_control) = ContentFetcherHandler::new(
 								dapp.url(),
@@ -170,6 +172,9 @@ impl<R: URLHint> ContentFetcher<R> {
 
 							(Some(ContentStatus::Fetching(fetch_control)), Box::new(handler) as Box<Handler>)
 						},
+						None if self.sync.is_major_importing() => {
+							(None, Self::still_syncing())
+						},
 						None => {
 							// This may happen when sync status changes in between
 							// `contains` and `to_handler`

dapps/src/tests/fetch.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use tests::helpers::{serve_with_registrar, request, assert_security_headers};
+use tests::helpers::{serve_with_registrar, serve_with_registrar_and_sync, request, assert_security_headers};
 
 #[test]
 fn should_resolve_dapp() {
@@ -37,3 +37,31 @@ fn should_resolve_dapp() {
 	assert_security_headers(&response.headers);
 }
 
+#[test]
+fn should_return_503_when_syncing_but_should_make_the_calls() {
+	// given
+	let (server, registrar) = serve_with_registrar_and_sync();
+	{
+		let mut responses = registrar.responses.lock();
+		let res1 = responses.get(0).unwrap().clone();
+		let res2 = responses.get(1).unwrap().clone();
+		// Registrar will be called twice - fill up the responses.
+		responses.push(res1);
+		responses.push(res2);
+	}
+
+	// when
+	let response = request(server,
+		"\
+			GET / HTTP/1.1\r\n\
+			Host: 1472a9e190620cdf6b31f383373e45efcfe869a820c91f9ccd7eb9fb45e4985d.parity\r\n\
+			Connection: close\r\n\
+			\r\n\
+		"
+	);
+
+	// then
+	assert_eq!(response.status, "HTTP/1.1 503 Service Unavailable".to_owned());
+	assert_eq!(registrar.calls.lock().len(), 4);
+	assert_security_headers(&response.headers);
+}

dapps/src/tests/helpers.rs

@@ -69,12 +69,13 @@ fn init_logger() {
 	}
 }
 
-pub fn init_server(hosts: Option<Vec<String>>) -> (Server, Arc<FakeRegistrar>) {
+pub fn init_server(hosts: Option<Vec<String>>, is_syncing: bool) -> (Server, Arc<FakeRegistrar>) {
 	init_logger();
 	let registrar = Arc::new(FakeRegistrar::new());
 	let mut dapps_path = env::temp_dir();
 	dapps_path.push("non-existent-dir-to-prevent-fs-files-from-loading");
 	let mut builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar.clone());
+	builder.with_sync_status(Arc::new(move || is_syncing));
 	builder.with_signer_port(Some(SIGNER_PORT));
 	(
 		builder.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap(),
@@ -93,15 +94,19 @@ pub fn serve_with_auth(user: &str, pass: &str) -> Server {
 }
 
 pub fn serve_hosts(hosts: Option<Vec<String>>) -> Server {
-	init_server(hosts).0
+	init_server(hosts, false).0
 }
 
 pub fn serve_with_registrar() -> (Server, Arc<FakeRegistrar>) {
-	init_server(None)
+	init_server(None, false)
+}
+
+pub fn serve_with_registrar_and_sync() -> (Server, Arc<FakeRegistrar>) {
+	init_server(None, true)
 }
 
 pub fn serve() -> Server {
-	init_server(None).0
+	init_server(None, false).0
 }
 
 pub fn request(server: Server, request: &str) -> http_client::Response {