From 89f1444c516aef931f90279d0df399a2c5fe8d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 11:16:19 +0200 Subject: [PATCH] Displaying special page when syncing. 404 instead of redirection --- dapps/src/apps/fetcher.rs | 77 ++++++++++++++++++++++++---------- dapps/src/lib.rs | 26 ++++++++++-- dapps/src/router/mod.rs | 11 +++-- dapps/src/tests/fetch.rs | 38 +++++++++++++++++ dapps/src/tests/helpers.rs | 23 +++++++--- dapps/src/tests/mod.rs | 1 + dapps/src/tests/redirection.rs | 12 +++--- parity/dapps.rs | 11 +++-- parity/run.rs | 1 + 9 files changed, 157 insertions(+), 43 deletions(-) create mode 100644 dapps/src/tests/fetch.rs diff --git a/dapps/src/apps/fetcher.rs b/dapps/src/apps/fetcher.rs index 6223fb5e8..cb58af976 100644 --- a/dapps/src/apps/fetcher.rs +++ b/dapps/src/apps/fetcher.rs @@ -30,6 +30,7 @@ use hyper::Control; use hyper::status::StatusCode; use random_filename; +use SyncStatus; use util::{Mutex, H256}; use util::sha3::sha3; use page::LocalPageEndpoint; @@ -44,6 +45,7 @@ const MAX_CACHED_DAPPS: usize = 10; pub struct AppFetcher { dapps_path: PathBuf, resolver: R, + sync: Arc, dapps: Arc>, } @@ -56,13 +58,14 @@ impl Drop for AppFetcher { impl AppFetcher { - pub fn new(resolver: R) -> Self { + pub fn new(resolver: R, sync_status: Arc) -> Self { let mut dapps_path = env::temp_dir(); dapps_path.push(random_filename()); AppFetcher { dapps_path: dapps_path, resolver: resolver, + sync: sync_status, dapps: Arc::new(Mutex::new(ContentCache::default())), } } @@ -74,14 +77,20 @@ impl AppFetcher { pub fn contains(&self, app_id: &str) -> bool { let mut dapps = self.dapps.lock(); - match dapps.get(app_id) { - // Check if we already have the app - Some(_) => true, - // fallback to resolver - None => match app_id.from_hex() { - Ok(app_id) => self.resolver.resolve(app_id).is_some(), - _ => false, - }, + // Check if we already have the app + if dapps.get(app_id).is_some() { + return true; + } + // fallback to resolver + if let Ok(app_id) = app_id.from_hex() { + // if app_id is valid, but we are syncing always return true. + if self.sync.is_major_syncing() { + return true; + } + // else try to resolve the app_id + self.resolver.resolve(app_id).is_some() + } else { + false } } @@ -89,6 +98,15 @@ impl AppFetcher { let mut dapps = self.dapps.lock(); let app_id = path.app_id.clone(); + if self.sync.is_major_syncing() { + 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.", + None + )); + } + let (new_status, handler) = { let status = dapps.get(&app_id); match status { @@ -108,20 +126,32 @@ impl AppFetcher { // We need to start fetching app None => { let app_hex = app_id.from_hex().expect("to_handler is called only when `contains` returns true."); - let app = self.resolver.resolve(app_hex).expect("to_handler is called only when `contains` returns true."); - let abort = Arc::new(AtomicBool::new(false)); + let app = self.resolver.resolve(app_hex); - (Some(ContentStatus::Fetching(abort.clone())), Box::new(ContentFetcherHandler::new( - app, - abort, - control, - path.using_dapps_domains, - DappInstaller { - dapp_id: app_id.clone(), - dapps_path: self.dapps_path.clone(), - dapps: self.dapps.clone(), - } - )) as Box) + if let Some(app) = app { + let abort = Arc::new(AtomicBool::new(false)); + + (Some(ContentStatus::Fetching(abort.clone())), Box::new(ContentFetcherHandler::new( + app, + abort, + control, + path.using_dapps_domains, + DappInstaller { + dapp_id: app_id.clone(), + dapps_path: self.dapps_path.clone(), + dapps: self.dapps.clone(), + } + )) as Box) + } else { + // This may happen when sync status changes in between + // `contains` and `to_handler` + (None, Box::new(ContentHandler::error( + StatusCode::NotFound, + "Resource Not Found", + "Requested resource was not found.", + None + )) as Box) + } }, } }; @@ -274,6 +304,7 @@ impl ContentValidator for DappInstaller { #[cfg(test)] mod tests { use std::env; + use std::sync::Arc; use util::Bytes; use endpoint::EndpointInfo; use page::LocalPageEndpoint; @@ -292,7 +323,7 @@ mod tests { fn should_true_if_contains_the_app() { // given let path = env::temp_dir(); - let fetcher = AppFetcher::new(FakeResolver); + let fetcher = AppFetcher::new(FakeResolver, Arc::new(|| false)); let handler = LocalPageEndpoint::new(path, EndpointInfo { name: "fake".into(), description: "".into(), diff --git a/dapps/src/lib.rs b/dapps/src/lib.rs index e24def0a1..0a2297189 100644 --- a/dapps/src/lib.rs +++ b/dapps/src/lib.rs @@ -88,11 +88,22 @@ use ethcore_rpc::Extendable; static DAPPS_DOMAIN : &'static str = ".parity"; +/// Indicates sync status +pub trait SyncStatus: Send + Sync { + /// Returns true if there is a major sync happening. + fn is_major_syncing(&self) -> bool; +} + +impl SyncStatus for F where F: Fn() -> bool + Send + Sync { + fn is_major_syncing(&self) -> bool { self() } +} + /// Webapps HTTP+RPC server build. pub struct ServerBuilder { dapps_path: String, handler: Arc, registrar: Arc, + sync_status: Arc, } impl Extendable for ServerBuilder { @@ -108,9 +119,15 @@ impl ServerBuilder { dapps_path: dapps_path, handler: Arc::new(IoHandler::new()), registrar: registrar, + sync_status: Arc::new(|| false), } } + /// Change default sync status. + pub fn with_sync_status(&mut self, status: Arc) { + self.sync_status = status; + } + /// Asynchronously start server with no authentication, /// returns result with `Server` handle on success or an error. pub fn start_unsecured_http(&self, addr: &SocketAddr, hosts: Option>) -> Result { @@ -120,7 +137,8 @@ impl ServerBuilder { NoAuth, self.handler.clone(), self.dapps_path.clone(), - self.registrar.clone() + self.registrar.clone(), + self.sync_status.clone(), ) } @@ -133,7 +151,8 @@ impl ServerBuilder { HttpBasicAuth::single_user(username, password), self.handler.clone(), self.dapps_path.clone(), - self.registrar.clone() + self.registrar.clone(), + self.sync_status.clone(), ) } } @@ -167,10 +186,11 @@ impl Server { handler: Arc, dapps_path: String, registrar: Arc, + sync_status: Arc, ) -> Result { let panic_handler = Arc::new(Mutex::new(None)); let authorization = Arc::new(authorization); - let apps_fetcher = Arc::new(apps::fetcher::AppFetcher::new(apps::urlhint::URLHintContract::new(registrar))); + let apps_fetcher = Arc::new(apps::fetcher::AppFetcher::new(apps::urlhint::URLHintContract::new(registrar), sync_status)); let endpoints = Arc::new(apps::all_endpoints(dapps_path)); let special = Arc::new({ let mut special = HashMap::new(); diff --git a/dapps/src/router/mod.rs b/dapps/src/router/mod.rs index 9326917bf..5dc1018b0 100644 --- a/dapps/src/router/mod.rs +++ b/dapps/src/router/mod.rs @@ -24,12 +24,12 @@ use DAPPS_DOMAIN; use std::sync::Arc; use std::collections::HashMap; use url::{Url, Host}; -use hyper::{self, server, Next, Encoder, Decoder, Control}; +use hyper::{self, server, Next, Encoder, Decoder, Control, StatusCode}; use hyper::net::HttpStream; use apps; use apps::fetcher::AppFetcher; use endpoint::{Endpoint, Endpoints, EndpointPath}; -use handlers::{Redirection, extract_url}; +use handlers::{Redirection, extract_url, ContentHandler}; use self::auth::{Authorization, Authorized}; /// Special endpoints are accessible on every domain (every dapp) @@ -94,7 +94,12 @@ impl server::Handler for Router { // Redirection to main page (maybe 404 instead?) (Some(ref path), _) if *req.method() == hyper::method::Method::Get => { let address = apps::redirection_address(path.using_dapps_domains, self.main_page); - Redirection::new(address.as_str()) + Box::new(ContentHandler::error( + StatusCode::NotFound, + "404 Not Found", + "Requested content was not found on a server.", + Some(&format!("Go back to the Home Page.", address)) + )) }, // Redirect any GET request to home. _ if *req.method() == hyper::method::Method::Get => { diff --git a/dapps/src/tests/fetch.rs b/dapps/src/tests/fetch.rs new file mode 100644 index 000000000..6fd65c00f --- /dev/null +++ b/dapps/src/tests/fetch.rs @@ -0,0 +1,38 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use tests::helpers::{serve_with_registrar, request}; + +#[test] +fn should_resolve_dapp() { + // given + let (server, registrar) = serve_with_registrar(); + + // 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 404 Not Found".to_owned()); + assert_eq!(registrar.calls.lock().len(), 2); +} + diff --git a/dapps/src/tests/helpers.rs b/dapps/src/tests/helpers.rs index 1296162d2..4cd21520c 100644 --- a/dapps/src/tests/helpers.rs +++ b/dapps/src/tests/helpers.rs @@ -58,22 +58,35 @@ impl ContractClient for FakeRegistrar { } } -pub fn serve_hosts(hosts: Option>) -> Server { +pub fn init_server(hosts: Option>) -> (Server, Arc) { 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 builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar); - builder.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap() + let builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar.clone()); + ( + builder.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap(), + registrar, + ) } pub fn serve_with_auth(user: &str, pass: &str) -> Server { let registrar = Arc::new(FakeRegistrar::new()); - let builder = ServerBuilder::new(env::temp_dir().to_str().unwrap().into(), registrar); + let mut dapps_path = env::temp_dir(); + dapps_path.push("non-existent-dir-to-prevent-fs-files-from-loading"); + let builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar); builder.start_basic_auth_http(&"127.0.0.1:0".parse().unwrap(), None, user, pass).unwrap() } +pub fn serve_hosts(hosts: Option>) -> Server { + init_server(hosts).0 +} + +pub fn serve_with_registrar() -> (Server, Arc) { + init_server(None) +} + pub fn serve() -> Server { - serve_hosts(None) + init_server(None).0 } pub fn request(server: Server, request: &str) -> http_client::Response { diff --git a/dapps/src/tests/mod.rs b/dapps/src/tests/mod.rs index 8c5bf2283..cc0e693c9 100644 --- a/dapps/src/tests/mod.rs +++ b/dapps/src/tests/mod.rs @@ -20,6 +20,7 @@ mod helpers; mod api; mod authorization; +mod fetch; mod redirection; mod validation; diff --git a/dapps/src/tests/redirection.rs b/dapps/src/tests/redirection.rs index 53aa393e2..2fdecbd4d 100644 --- a/dapps/src/tests/redirection.rs +++ b/dapps/src/tests/redirection.rs @@ -57,7 +57,7 @@ fn should_redirect_to_home_when_trailing_slash_is_missing() { } #[test] -fn should_redirect_to_home_on_invalid_dapp() { +fn should_display_404_on_invalid_dapp() { // given let server = serve(); @@ -72,12 +72,12 @@ fn should_redirect_to_home_on_invalid_dapp() { ); // then - assert_eq!(response.status, "HTTP/1.1 302 Found".to_owned()); - assert_eq!(response.headers.get(0).unwrap(), "Location: /home/"); + assert_eq!(response.status, "HTTP/1.1 404 Not Found".to_owned()); + assert!(response.body.contains("href=\"/home/")); } #[test] -fn should_redirect_to_home_on_invalid_dapp_with_domain() { +fn should_display_404_on_invalid_dapp_with_domain() { // given let server = serve(); @@ -92,8 +92,8 @@ fn should_redirect_to_home_on_invalid_dapp_with_domain() { ); // then - assert_eq!(response.status, "HTTP/1.1 302 Found".to_owned()); - assert_eq!(response.headers.get(0).unwrap(), "Location: http://home.parity/"); + assert_eq!(response.status, "HTTP/1.1 404 Not Found".to_owned()); + assert!(response.body.contains("href=\"http://home.parity/")); } #[test] diff --git a/parity/dapps.rs b/parity/dapps.rs index 7f759ed3c..fae395b5a 100644 --- a/parity/dapps.rs +++ b/parity/dapps.rs @@ -18,6 +18,7 @@ use std::sync::Arc; use io::PanicHandler; use rpc_apis; use ethcore::client::Client; +use ethsync::SyncProvider; use helpers::replace_home; #[derive(Debug, PartialEq, Clone)] @@ -49,6 +50,7 @@ pub struct Dependencies { pub panic_handler: Arc, pub apis: Arc, pub client: Arc, + pub sync: Arc, } pub fn new(configuration: Configuration, deps: Dependencies) -> Result, String> { @@ -117,9 +119,12 @@ mod server { ) -> Result { use ethcore_dapps as dapps; - let server = dapps::ServerBuilder::new(dapps_path, Arc::new(Registrar { - client: deps.client.clone(), - })); + let mut server = dapps::ServerBuilder::new( + dapps_path, + Arc::new(Registrar { client: deps.client.clone() }) + ); + let sync = deps.sync.clone(); + server.with_sync_status(Arc::new(move || sync.status().is_major_syncing())); let server = rpc_apis::setup_rpc(server, deps.apis.clone(), rpc_apis::ApiSet::UnsafeContext); let start_result = match auth { None => { diff --git a/parity/run.rs b/parity/run.rs index 71995cd5f..8a68fe1af 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -224,6 +224,7 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { panic_handler: panic_handler.clone(), apis: deps_for_rpc_apis.clone(), client: client.clone(), + sync: sync_provider.clone(), }; // start dapps server