From 82290d8b0baf61b1a5d0babcb02a31666b40a3fa Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Sat, 8 Jul 2023 15:53:40 +0200 Subject: [PATCH] Get rid of read_file_into, use streams to serve files from domain manager --- src/domain/manager.rs | 21 --------------- src/origin.rs | 19 -------------- src/origin/git.rs | 59 +++++-------------------------------------- src/origin/mux.rs | 11 -------- src/service/http.rs | 29 ++++++++------------- 5 files changed, 16 insertions(+), 123 deletions(-) diff --git a/src/domain/manager.rs b/src/domain/manager.rs index e13c704..e228c30 100644 --- a/src/domain/manager.rs +++ b/src/domain/manager.rs @@ -57,8 +57,6 @@ impl From for GetFileError { } } -pub type ReadFileIntoError = GetFileError; - #[derive(thiserror::Error, Debug)] pub enum SyncError { #[error("not found")] @@ -138,13 +136,6 @@ pub type GetAcmeHttp01ChallengeKeyError = acme::manager::GetHttp01ChallengeKeyEr pub trait Manager: Sync + Send + rustls::server::ResolvesServerCert { fn get_config(&self, domain: &domain::Name) -> Result; - fn read_file_into( - &self, - domain: &domain::Name, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), ReadFileIntoError>; - fn get_file<'store>( &'store self, domain: &domain::Name, @@ -237,18 +228,6 @@ impl Manager for ManagerImpl { Ok(self.domain_config_store.get(domain)?) } - fn read_file_into( - &self, - domain: &domain::Name, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), ReadFileIntoError> { - let config = self.domain_config_store.get(domain)?; - self.origin_store - .read_file_into(&config.origin_descr, path, into)?; - Ok(()) - } - fn get_file<'store>( &'store self, domain: &domain::Name, diff --git a/src/origin.rs b/src/origin.rs index f6d47f6..7a98736 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -41,8 +41,6 @@ pub enum GetFileError { Unexpected(#[from] unexpected::Error), } -pub type ReadFileIntoError = GetFileError; - #[mockall::automock] /// Describes a storage mechanism for Origins. Each Origin is uniquely identified by its Descr. pub trait Store { @@ -52,13 +50,6 @@ pub trait Store { fn all_descrs(&self) -> Result, AllDescrsError>; - fn read_file_into( - &self, - descr: &Descr, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), ReadFileIntoError>; - fn get_file(&self, descr: &Descr, path: &str) -> Result; } @@ -75,16 +66,6 @@ impl Store for sync::Arc> { self.lock().unwrap().all_descrs() } - /// Deprecated, use get_file - fn read_file_into( - &self, - descr: &Descr, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), ReadFileIntoError> { - self.lock().unwrap().read_file_into(descr, path, into) - } - fn get_file<'store>( &'store self, descr: &Descr, diff --git a/src/origin/git.rs b/src/origin/git.rs index 629f0ae..79b3fb4 100644 --- a/src/origin/git.rs +++ b/src/origin/git.rs @@ -290,50 +290,6 @@ impl super::Store for FSStore { ).try_collect() } - fn read_file_into( - &self, - descr: &origin::Descr, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), origin::ReadFileIntoError> { - let repo_snapshot = match self.get_repo_snapshot(descr) { - Ok(Some(repo_snapshot)) => repo_snapshot, - Ok(None) => return Err(origin::ReadFileIntoError::DescrNotSynced), - Err(e) => return Err(e.into()), - }; - - let mut clean_path = Path::new(path); - clean_path = clean_path.strip_prefix("/").unwrap_or(clean_path); - - let repo = repo_snapshot.repo.to_thread_local(); - - let file_object = repo - .find_object(repo_snapshot.tree_object_id) - .map_unexpected_while(|| { - format!("finding tree object {}", repo_snapshot.tree_object_id) - })? - .peel_to_tree() - .map_unexpected_while(|| { - format!("peeling tree object {}", repo_snapshot.tree_object_id) - })? - .lookup_entry_by_path(clean_path) - .map_unexpected_while(|| { - format!( - "looking up {} in tree object {}", - clean_path.display(), - repo_snapshot.tree_object_id - ) - })? - .ok_or(origin::ReadFileIntoError::FileNotFound)? - .object() - .or_unexpected()?; - - into.write_all(file_object.data.as_ref()) - .or_unexpected_while("copying out file")?; - - Ok(()) - } - fn get_file<'store>( &'store self, descr: &origin::Descr, @@ -405,17 +361,14 @@ mod tests { store.sync(&descr).expect("sync should succeed"); store.sync(&descr).expect("second sync should succeed"); - { - // RepoSnapshot doesn't exist - let mut into: Vec = vec![]; - assert!(matches!( - store.read_file_into(&other_descr, "DNE", &mut into), - Err::<_, origin::ReadFileIntoError>(origin::ReadFileIntoError::DescrNotSynced), - )); - } + // RepoSnapshot doesn't exist + match store.get_file(&other_descr, "DNE") { + Err(origin::GetFileError::DescrNotSynced) => (), + _ => assert!(false, "descr should have not been found"), + }; let assert_file_dne = |path: &str| match store.get_file(&descr, path) { - Err(origin::ReadFileIntoError::FileNotFound) => (), + Err(origin::GetFileError::FileNotFound) => (), _ => assert!(false, "file should have not been found"), }; diff --git a/src/origin/mux.rs b/src/origin/mux.rs index 649366b..2ae26b0 100644 --- a/src/origin/mux.rs +++ b/src/origin/mux.rs @@ -41,17 +41,6 @@ where Ok(res) } - fn read_file_into( - &self, - descr: &origin::Descr, - path: &str, - into: &mut dyn std::io::Write, - ) -> Result<(), origin::ReadFileIntoError> { - (self.mapping_fn)(descr) - .or_unexpected_while(format!("mapping {:?} to store", &descr))? - .read_file_into(descr, path, into) - } - fn get_file<'store>( &'store self, descr: &origin::Descr, diff --git a/src/service/http.rs b/src/service/http.rs index 0f9e1b3..8be9fe3 100644 --- a/src/service/http.rs +++ b/src/service/http.rs @@ -11,7 +11,7 @@ use std::{future, net, sync}; use crate::error::unexpected; use crate::{domain, service, util}; -type SvcResponse = Result, String>; +type SvcResponse = Result, String>; pub struct Service { domain_manager: sync::Arc, @@ -94,7 +94,7 @@ struct DomainSyncArgs { } impl<'svc> Service { - fn serve_string(&self, status_code: u16, path: &'_ str, body: Vec) -> SvcResponse { + fn serve(&self, status_code: u16, path: &'_ str, body: Body) -> SvcResponse { let content_type = mime_guess::from_path(path) .first_or_octet_stream() .to_string(); @@ -102,14 +102,13 @@ impl<'svc> Service { match Response::builder() .status(status_code) .header("Content-Type", content_type) - .body(body.into()) + .body(body) { Ok(res) => Ok(res), Err(err) => Err(format!("failed to build {}: {}", path, err)), } } - //// TODO make this use an io::Write, rather than SvcResponse fn render(&self, status_code: u16, name: &'_ str, value: T) -> SvcResponse where T: Serialize, @@ -125,7 +124,7 @@ impl<'svc> Service { } }; - self.serve_string(status_code, name, rendered.into()) + self.serve(status_code, name, rendered.into()) } fn render_error_page(&'svc self, status_code: u16, e: &'_ str) -> SvcResponse { @@ -170,16 +169,15 @@ impl<'svc> Service { false => path, }; - let mut buf = Vec::::new(); - match self.domain_manager.read_file_into(&domain, path, &mut buf) { - Ok(_) => self.serve_string(200, path, buf), - Err(domain::manager::ReadFileIntoError::DomainNotFound) => { + match self.domain_manager.get_file(&domain, path) { + Ok(f) => self.serve(200, path, Body::wrap_stream(f)), + Err(domain::manager::GetFileError::DomainNotFound) => { return self.render_error_page(404, "Domain not found") } - Err(domain::manager::ReadFileIntoError::FileNotFound) => { + Err(domain::manager::GetFileError::FileNotFound) => { self.render_error_page(404, "File not found") } - Err(domain::manager::ReadFileIntoError::Unexpected(e)) => { + Err(domain::manager::GetFileError::Unexpected(e)) => { self.render_error_page(500, format!("failed to fetch file {path}: {e}").as_str()) } } @@ -355,14 +353,7 @@ impl<'svc> Service { let token = path.trim_start_matches("/.well-known/acme-challenge/"); if let Ok(key) = self.domain_manager.get_acme_http01_challenge_key(token) { - let body: hyper::Body = key.into(); - return match Response::builder().status(200).body(body) { - Ok(res) => Ok(res), - Err(err) => Err(format!( - "failed to write acme http-01 challenge key: {}", - err - )), - }; + return self.serve(200, "token.txt", key.into()); } }