From c6912c8e0a48a01bce08c96658921f20be9cb7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 15 Dec 2016 11:38:05 +0100 Subject: [PATCH] Removing on_done --- dapps/src/apps/fetcher.rs | 169 ++++++++++++++++++------------------ dapps/src/handlers/fetch.rs | 18 +--- 2 files changed, 90 insertions(+), 97 deletions(-) diff --git a/dapps/src/apps/fetcher.rs b/dapps/src/apps/fetcher.rs index 2430af035..e1fa4c066 100644 --- a/dapps/src/apps/fetcher.rs +++ b/dapps/src/apps/fetcher.rs @@ -120,7 +120,7 @@ impl ContentFetcher { // 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)) + (None, fetch_control.to_async_handler(path, control)) }, // We need to start fetching the content None => { @@ -129,11 +129,12 @@ impl ContentFetcher { let content = self.resolver.resolve(content_hex); let cache = self.cache.clone(); - let on_done = move |id: String, result: Option| { + let id = content_id.clone(); + let on_done = move |result: Option| { let mut cache = cache.lock(); match result { Some(endpoint) => { - cache.insert(id, ContentStatus::Ready(endpoint)); + cache.insert(id.clone(), ContentStatus::Ready(endpoint)); }, // In case of error None => { @@ -248,43 +249,45 @@ struct ContentInstaller { id: String, mime: String, content_path: PathBuf, - on_done: Box) + Send>, + on_done: Box) + Send>, } impl ContentValidator for ContentInstaller { type Error = ValidationError; fn validate_and_install(&self, path: PathBuf) -> Result<(String, LocalPageEndpoint), ValidationError> { - // Create dir - try!(fs::create_dir_all(&self.content_path)); + let validate = || { + // Create dir + try!(fs::create_dir_all(&self.content_path)); - // Validate hash - let mut file_reader = io::BufReader::new(try!(fs::File::open(&path))); - let hash = try!(sha3(&mut file_reader)); - let id = try!(self.id.as_str().parse().map_err(|_| ValidationError::InvalidContentId)); - if id != hash { - return Err(ValidationError::HashMismatch { - expected: id, - got: hash, - }); - } + // Validate hash + let mut file_reader = io::BufReader::new(try!(fs::File::open(&path))); + let hash = try!(sha3(&mut file_reader)); + let id = try!(self.id.as_str().parse().map_err(|_| ValidationError::InvalidContentId)); + if id != hash { + return Err(ValidationError::HashMismatch { + expected: id, + got: hash, + }); + } - // And prepare path for a file - let filename = path.file_name().expect("We always fetch a file."); - let mut content_path = self.content_path.clone(); - content_path.push(&filename); + // And prepare path for a file + let filename = path.file_name().expect("We always fetch a file."); + let mut content_path = self.content_path.clone(); + content_path.push(&filename); - if content_path.exists() { - try!(fs::remove_dir_all(&content_path)) - } + if content_path.exists() { + try!(fs::remove_dir_all(&content_path)) + } - try!(fs::copy(&path, &content_path)); + try!(fs::copy(&path, &content_path)); + Ok(LocalPageEndpoint::single_file(content_path, self.mime.clone(), PageCache::Enabled)) + }; - Ok((self.id.clone(), LocalPageEndpoint::single_file(content_path, self.mime.clone(), PageCache::Enabled))) - } - - fn done(&self, endpoint: Option) { - (self.on_done)(self.id.clone(), endpoint) + // Make sure to always call on_done (even in case of errors)! + let result = validate(); + (self.on_done)(result.as_ref().ok().cloned()); + result.map(|endpoint| (self.id.clone(), endpoint)) } } @@ -292,7 +295,7 @@ impl ContentValidator for ContentInstaller { struct DappInstaller { id: String, dapps_path: PathBuf, - on_done: Box) + Send>, + on_done: Box) + Send>, embeddable_on: Option<(String, u16)>, } @@ -333,67 +336,67 @@ impl ContentValidator for DappInstaller { fn validate_and_install(&self, app_path: PathBuf) -> Result<(String, LocalPageEndpoint), ValidationError> { trace!(target: "dapps", "Opening dapp bundle at {:?}", app_path); - let mut file_reader = io::BufReader::new(try!(fs::File::open(app_path))); - let hash = try!(sha3(&mut file_reader)); - let id = try!(self.id.as_str().parse().map_err(|_| ValidationError::InvalidContentId)); - if id != hash { - return Err(ValidationError::HashMismatch { - expected: id, - got: hash, - }); - } - let file = file_reader.into_inner(); - // Unpack archive - let mut zip = try!(zip::ZipArchive::new(file)); - // First find manifest file - let (mut manifest, manifest_dir) = try!(Self::find_manifest(&mut zip)); - // Overwrite id to match hash - manifest.id = self.id.clone(); + let validate = || { + let mut file_reader = io::BufReader::new(try!(fs::File::open(app_path))); + let hash = try!(sha3(&mut file_reader)); + let id = try!(self.id.as_str().parse().map_err(|_| ValidationError::InvalidContentId)); + if id != hash { + return Err(ValidationError::HashMismatch { + expected: id, + got: hash, + }); + } + let file = file_reader.into_inner(); + // Unpack archive + let mut zip = try!(zip::ZipArchive::new(file)); + // First find manifest file + let (mut manifest, manifest_dir) = try!(Self::find_manifest(&mut zip)); + // Overwrite id to match hash + manifest.id = self.id.clone(); - let target = self.dapp_target_path(&manifest); + let target = self.dapp_target_path(&manifest); - // Remove old directory - if target.exists() { - warn!(target: "dapps", "Overwriting existing dapp: {}", manifest.id); - try!(fs::remove_dir_all(target.clone())); - } + // Remove old directory + if target.exists() { + warn!(target: "dapps", "Overwriting existing dapp: {}", manifest.id); + try!(fs::remove_dir_all(target.clone())); + } - // Unpack zip - for i in 0..zip.len() { - let mut file = try!(zip.by_index(i)); - // TODO [todr] Check if it's consistent on windows. - let is_dir = file.name().chars().rev().next() == Some('/'); + // Unpack zip + for i in 0..zip.len() { + let mut file = try!(zip.by_index(i)); + // TODO [todr] Check if it's consistent on windows. + let is_dir = file.name().chars().rev().next() == Some('/'); - let file_path = PathBuf::from(file.name()); - let location_in_manifest_base = file_path.strip_prefix(&manifest_dir); - // Create files that are inside manifest directory - if let Ok(location_in_manifest_base) = location_in_manifest_base { - let p = target.join(location_in_manifest_base); - // Check if it's a directory - if is_dir { - try!(fs::create_dir_all(p)); - } else { - let mut target = try!(fs::File::create(p)); - try!(io::copy(&mut file, &mut target)); + let file_path = PathBuf::from(file.name()); + let location_in_manifest_base = file_path.strip_prefix(&manifest_dir); + // Create files that are inside manifest directory + if let Ok(location_in_manifest_base) = location_in_manifest_base { + let p = target.join(location_in_manifest_base); + // Check if it's a directory + if is_dir { + try!(fs::create_dir_all(p)); + } else { + let mut target = try!(fs::File::create(p)); + try!(io::copy(&mut file, &mut target)); + } } } - } - // Write manifest - let manifest_str = try!(serialize_manifest(&manifest).map_err(ValidationError::ManifestSerialization)); - let manifest_path = target.join(MANIFEST_FILENAME); - let mut manifest_file = try!(fs::File::create(manifest_path)); - try!(manifest_file.write_all(manifest_str.as_bytes())); + // Write manifest + let manifest_str = try!(serialize_manifest(&manifest).map_err(ValidationError::ManifestSerialization)); + let manifest_path = target.join(MANIFEST_FILENAME); + let mut manifest_file = try!(fs::File::create(manifest_path)); + try!(manifest_file.write_all(manifest_str.as_bytes())); + // Create endpoint + let endpoint = LocalPageEndpoint::new(target, manifest.clone().into(), PageCache::Enabled, self.embeddable_on.clone()); + // Return modified app manifest + Ok(endpoint) + }; - // Create endpoint - let app = LocalPageEndpoint::new(target, manifest.clone().into(), PageCache::Enabled, self.embeddable_on.clone()); - - // Return modified app manifest - Ok((manifest.id.clone(), app)) - } - - fn done(&self, endpoint: Option) { - (self.on_done)(self.id.clone(), endpoint) + let result = validate(); + (self.on_done)(result.as_ref().ok().cloned()); + result.map(|endpoint| (self.id.clone(), endpoint)) } } diff --git a/dapps/src/handlers/fetch.rs b/dapps/src/handlers/fetch.rs index 6fb524293..8e5f250ce 100644 --- a/dapps/src/handlers/fetch.rs +++ b/dapps/src/handlers/fetch.rs @@ -29,6 +29,7 @@ use hyper::{server, Decoder, Encoder, Next, Method, Control}; use hyper::net::HttpStream; use hyper::status::StatusCode; +use endpoint::EndpointPath; use handlers::{ContentHandler, Redirection, extract_url}; use page::LocalPageEndpoint; @@ -45,7 +46,6 @@ pub trait ContentValidator { type Error: fmt::Debug + fmt::Display; fn validate_and_install(&self, app: PathBuf) -> Result<(String, LocalPageEndpoint), Self::Error>; - fn done(&self, Option); } pub struct FetchControl { @@ -88,7 +88,9 @@ impl FetchControl { self.abort.store(true, Ordering::SeqCst); } - pub fn to_handler(&self, control: Control) -> Box + Send> { + pub fn to_async_handler(&self, path: EndpointPath, control: Control) -> Box + Send> { + // TODO [ToDr] We should be able to pass EndpointPath to handler as well + // (request may be coming from different domain, etc) let (tx, rx) = mpsc::channel(); self.listeners.lock().push((control, tx)); @@ -141,25 +143,13 @@ pub struct ContentFetcherHandler { embeddable_on: Option<(String, u16)>, } -impl Drop for ContentFetcherHandler { - fn drop(&mut self) { - let result = match self.status { - FetchState::Done(_, ref result, _) => Some(result.clone()), - _ => None, - }; - self.installer.done(result); - } -} - impl ContentFetcherHandler { - pub fn new( url: String, control: Control, handler: H, embeddable_on: Option<(String, u16)>, ) -> (Self, Arc) { - let fetch_control = Arc::new(FetchControl::default()); let client = Client::default(); let handler = ContentFetcherHandler {