Revert "Have get_file accept and return structs, which will be easier to extend going forward"

This reverts commit a86020eedf.

Turns out that even with this change proxying won't work properly via
origin::Store, it'll just have to be a special case. I'm keeping the
commit around in case we want this for a later case.

src/domain/manager.rs

@@ -24,9 +24,6 @@ impl From<store::GetError> for GetSettingsError {
     }
 }
 
-pub type GetFileRequest<'req> = origin::GetFileRequest<'req>;
-pub type GetFileResponse = origin::GetFileResponse;
-
 #[derive(thiserror::Error, Debug)]
 pub enum GetFileError {
     #[error("domain not found")]
@@ -145,11 +142,11 @@ pub type GetAcmeHttp01ChallengeKeyError = acme::manager::GetHttp01ChallengeKeyEr
 pub trait Manager: Sync + Send + rustls::server::ResolvesServerCert {
     fn get_settings(&self, domain: &domain::Name) -> Result<domain::Settings, GetSettingsError>;
 
-    fn get_file<'req>(
+    fn get_file<'store>(
-        &self,
+        &'store self,
-        domain: &'req domain::Name,
+        domain: &domain::Name,
-        req: GetFileRequest<'req>,
+        path: &str,
-    ) -> Result<GetFileResponse, GetFileError>;
+    ) -> Result<util::BoxByteStream, GetFileError>;
 
     fn sync_cert<'mgr>(
         &'mgr self,
@@ -242,14 +239,14 @@ impl Manager for ManagerImpl {
         Ok(self.domain_store.get(domain)?)
     }
 
-    fn get_file<'req>(
+    fn get_file<'store>(
-        &self,
+        &'store self,
-        domain: &'req domain::Name,
+        domain: &domain::Name,
-        req: GetFileRequest<'req>,
+        path: &str,
-    ) -> Result<GetFileResponse, GetFileError> {
+    ) -> Result<util::BoxByteStream, GetFileError> {
-        let settings = self.domain_store.get(domain)?;
+        let config = self.domain_store.get(domain)?;
-        let res = self.origin_store.get_file(&settings.origin_descr, req)?;
+        let f = self.origin_store.get_file(&config.origin_descr, path)?;
-        Ok(res)
+        Ok(f)
     }
 
     fn sync_cert<'mgr>(

src/origin.rs

@@ -9,7 +9,7 @@ pub use descr::Descr;
 
 use crate::error::unexpected;
 use crate::util;
-use std::{net, sync};
+use std::sync;
 
 #[derive(thiserror::Error, Clone, Debug, PartialEq)]
 pub enum SyncError {
@@ -32,15 +32,6 @@ pub enum AllDescrsError {
     Unexpected(#[from] unexpected::Error),
 }
 
-pub struct GetFileRequest<'a> {
-    pub path: &'a str,
-    pub client_ip: &'a net::IpAddr,
-}
-
-pub struct GetFileResponse {
-    pub body: util::BoxByteStream,
-}
-
 #[derive(thiserror::Error, Debug)]
 pub enum GetFileError {
     #[error("descr not synced")]
@@ -62,11 +53,7 @@ pub trait Store {
 
     fn all_descrs(&self) -> Result<Vec<Descr>, AllDescrsError>;
 
-    fn get_file<'req>(
+    fn get_file(&self, descr: &Descr, path: &str) -> Result<util::BoxByteStream, GetFileError>;
-        &self,
-        descr: &'req Descr,
-        req: GetFileRequest<'req>,
-    ) -> Result<GetFileResponse, GetFileError>;
 }
 
 pub fn new_mock() -> sync::Arc<sync::Mutex<MockStore>> {
@@ -82,11 +69,11 @@ impl Store for sync::Arc<sync::Mutex<MockStore>> {
         self.lock().unwrap().all_descrs()
     }
 
-    fn get_file<'req>(
+    fn get_file<'store>(
-        &self,
+        &'store self,
-        descr: &'req Descr,
+        descr: &Descr,
-        req: GetFileRequest<'req>,
+        path: &str,
-    ) -> Result<GetFileResponse, GetFileError> {
+    ) -> Result<util::BoxByteStream, GetFileError> {
-        self.lock().unwrap().get_file(descr, req)
+        self.lock().unwrap().get_file(descr, path)
     }
 }

src/origin/git.rs

@@ -1,5 +1,5 @@
 use crate::error::unexpected::{self, Intoable, Mappable};
-use crate::origin;
+use crate::{origin, util};
 
 use std::path::{Path, PathBuf};
 use std::{collections, fs, io, sync};
@@ -297,18 +297,18 @@ impl super::Store for FSStore {
             ).try_collect()
     }
 
-    fn get_file<'req>(
+    fn get_file<'store>(
-        &self,
+        &'store self,
-        descr: &'req origin::Descr,
+        descr: &origin::Descr,
-        req: origin::GetFileRequest<'req>,
+        path: &str,
-    ) -> Result<origin::GetFileResponse, origin::GetFileError> {
+    ) -> Result<util::BoxByteStream, origin::GetFileError> {
         let repo_snapshot = match self.get_repo_snapshot(descr) {
             Ok(Some(repo_snapshot)) => repo_snapshot,
             Ok(None) => return Err(origin::GetFileError::DescrNotSynced),
             Err(e) => return Err(e.into()),
         };
 
-        let mut clean_path = Path::new(req.path);
+        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();
@@ -337,9 +337,7 @@ impl super::Store for FSStore {
         // TODO this is very not ideal, the whole file is first read totally into memory, and then
         // that is cloned.
         let data = file_object.data.clone();
-        Ok(origin::GetFileResponse {
+        Ok(Box::pin(stream::once(async move { Ok(data) })))
-            body: Box::pin(stream::once(async move { Ok(data) })),
-        })
     }
 }
 
@@ -347,13 +345,10 @@ impl super::Store for FSStore {
 mod tests {
     use crate::origin::{self, Config, Store};
     use futures::StreamExt;
-    use std::{net, str::FromStr};
     use tempdir::TempDir;
 
     #[tokio::test]
     async fn basic() {
-        let client_ip = net::IpAddr::from_str("127.0.0.1").unwrap();
-
         let tmp_dir = TempDir::new("origin_store_git").unwrap();
         let config = Config {
             store_dir_path: tmp_dir.path().to_path_buf(),
@@ -377,41 +372,20 @@ mod tests {
         store.sync(&descr).expect("second sync should succeed");
 
         // RepoSnapshot doesn't exist
-        match store.get_file(
+        match store.get_file(&other_descr, "DNE") {
-            &other_descr,
-            origin::GetFileRequest {
-                path: "DNE",
-                client_ip: &client_ip,
-            },
-        ) {
             Err(origin::GetFileError::DescrNotSynced) => (),
             _ => assert!(false, "descr should have not been found"),
         };
 
-        let assert_file_dne = |path: &str| match store.get_file(
+        let assert_file_dne = |path: &str| match store.get_file(&descr, path) {
-            &descr,
-            origin::GetFileRequest {
-                path,
-                client_ip: &client_ip,
-            },
-        ) {
             Err(origin::GetFileError::FileNotFound) => (),
             _ => assert!(false, "file should have not been found"),
         };
 
         let assert_file_not_empty = |path: &str| {
-            let origin::GetFileResponse { body } = store
+            let f = store.get_file(&descr, path).expect("file not retrieved");
-                .get_file(
-                    &descr,
-                    origin::GetFileRequest {
-                        path,
-                        client_ip: &client_ip,
-                    },
-                )
-                .expect("file not retrieved");
-
             async move {
-                let body = body.map(|r| r.unwrap()).concat().await;
+                let body = f.map(|r| r.unwrap()).concat().await;
                 assert!(body.len() > 0);
             }
         };

src/origin/mux.rs

@@ -1,5 +1,5 @@
 use crate::error::unexpected::Mappable;
-use crate::origin;
+use crate::{origin, util};
 
 pub struct Store<F, S>
 where
@@ -41,14 +41,14 @@ where
         Ok(res)
     }
 
-    fn get_file<'req>(
+    fn get_file<'store>(
-        &self,
+        &'store self,
-        descr: &'req origin::Descr,
+        descr: &origin::Descr,
-        req: origin::GetFileRequest<'req>,
+        path: &str,
-    ) -> Result<origin::GetFileResponse, origin::GetFileError> {
+    ) -> Result<util::BoxByteStream, origin::GetFileError> {
         (self.mapping_fn)(descr)
-            .or_unexpected_while(format!("mapping {:?} to store", descr))?
+            .or_unexpected_while(format!("mapping {:?} to store", &descr))?
-            .get_file(descr, req)
+            .get_file(descr, path)
     }
 }
 

src/service/http.rs

@@ -9,7 +9,7 @@ use hyper::{Body, Method, Request, Response};
 use serde::{Deserialize, Serialize};
 
 use std::str::FromStr;
-use std::{future, net, sync};
+use std::{future, sync};
 
 use crate::error::unexpected;
 use crate::{domain, service, util};
@@ -158,12 +158,7 @@ impl<'svc> Service {
         )
     }
 
-    fn serve_origin(
+    fn serve_origin(&self, domain: domain::Name, path: &str) -> Response<Body> {
-        &self,
-        domain: &domain::Name,
-        path: &str,
-        client_ip: &net::IpAddr,
-    ) -> Response<Body> {
         let mut path_owned;
 
         let path = match path.ends_with('/') {
@@ -175,12 +170,8 @@ impl<'svc> Service {
             false => path,
         };
 
-        let req = domain::manager::GetFileRequest { path, client_ip };
+        match self.domain_manager.get_file(&domain, path) {
-
+            Ok(f) => self.serve(200, path, Body::wrap_stream(f)),
-        match self.domain_manager.get_file(&domain, req) {
-            Ok(domain::manager::GetFileResponse { body }) => {
-                self.serve(200, path, Body::wrap_stream(body))
-            }
             Err(domain::manager::GetFileError::DomainNotFound) => {
                 return self.render_error_page(404, "Domain not found")
             }
@@ -375,7 +366,7 @@ impl<'svc> Service {
         self.render_page("/domains.html", Response { domains })
     }
 
-    async fn handle_request(&self, client_ip: net::IpAddr, req: Request<Body>) -> Response<Body> {
+    async fn handle_request(&self, req: Request<Body>) -> Response<Body> {
         let (req, body) = req.into_parts();
 
         let maybe_host = match (
@@ -424,7 +415,7 @@ impl<'svc> Service {
 
         // If a managed domain was given then serve that from its origin
         if let Some(domain) = maybe_host {
-            return self.serve_origin(&domain, req.uri.path(), &client_ip);
+            return self.serve_origin(domain, req.uri.path());
         }
 
         // Serve main domani site

src/service/http/tasks.rs

@@ -4,8 +4,6 @@ use crate::service;
 use std::{convert, future, sync};
 
 use futures::StreamExt;
-use hyper::server::conn::AddrStream;
-use tokio_rustls::server::TlsStream;
 use tokio_util::sync::CancellationToken;
 
 pub async fn listen_http(
@@ -15,14 +13,13 @@ pub async fn listen_http(
     let addr = service.config.http.http_addr.clone();
     let primary_domain = service.config.primary_domain.clone();
 
-    let make_service = hyper::service::make_service_fn(move |conn: &AddrStream| {
+    let make_service = hyper::service::make_service_fn(move |_| {
         let service = service.clone();
-        let client_ip = conn.remote_addr().ip();
 
         // Create a `Service` for responding to the request.
         let hyper_service = hyper::service::service_fn(move |req| {
             let service = service.clone();
-            async move { Ok::<_, convert::Infallible>(service.handle_request(client_ip, req).await) }
+            async move { Ok::<_, convert::Infallible>(service.handle_request(req).await) }
         });
 
         // Return the service to hyper.
@@ -51,14 +48,13 @@ pub async fn listen_https(
     let addr = service.config.http.https_addr.unwrap().clone();
     let primary_domain = service.config.primary_domain.clone();
 
-    let make_service = hyper::service::make_service_fn(move |conn: &TlsStream<AddrStream>| {
+    let make_service = hyper::service::make_service_fn(move |_| {
         let service = service.clone();
-        let client_ip = conn.get_ref().0.remote_addr().ip();
 
         // Create a `Service` for responding to the request.
         let hyper_service = hyper::service::service_fn(move |req| {
             let service = service.clone();
-            async move { Ok::<_, convert::Infallible>(service.handle_request(client_ip, req).await) }
+            async move { Ok::<_, convert::Infallible>(service.handle_request(req).await) }
         });
 
         // Return the service to hyper.