Start exploring a refactoring to components with scopes and resources

src/components/changes/mod.rs

@@ -0,0 +1,31 @@
+use diesel;
+use schema::article_revisions;
+
+mod query_parameters;
+mod resource;
+mod scope;
+
+pub use self::query_parameters::QueryParameters;
+pub use self::scope::Scope;
+pub use self::resource::Resource;
+
+fn apply_query_config<'a>(
+    mut query: article_revisions::BoxedQuery<'a, diesel::sqlite::Sqlite>,
+    article_id: Option<i32>,
+    author: Option<String>,
+    limit: i32,
+)
+    -> article_revisions::BoxedQuery<'a, diesel::sqlite::Sqlite>
+{
+    use diesel::prelude::*;
+
+    if let Some(article_id) = article_id {
+        query = query.filter(article_revisions::article_id.eq(article_id));
+    }
+
+    if let Some(author) = author {
+        query = query.filter(article_revisions::author.eq(author));
+    }
+
+    query.limit(limit as i64 + 1)
+}

src/components/changes/query_parameters.rs

@@ -0,0 +1,50 @@
+use serde_urlencoded;
+
+use pagination::Pagination;
+
+pub const DEFAULT_LIMIT: i32 = 30;
+
+#[derive(Serialize, Deserialize, Default)]
+pub struct QueryParameters {
+    pub after: Option<i32>,
+    pub before: Option<i32>,
+
+    pub article_id: Option<i32>,
+    pub author: Option<String>,
+
+    pub limit: Option<i32>,
+}
+
+impl QueryParameters {
+    pub fn pagination(self, pagination: Pagination<i32>) -> Self {
+        Self {
+            after: if let Pagination::After(x) = pagination { Some(x) } else { None },
+            before: if let Pagination::Before(x) = pagination { Some(x) } else { None },
+            ..self
+        }
+    }
+
+    pub fn article_id(self, article_id: Option<i32>) -> Self {
+        Self { article_id, ..self }
+    }
+
+    pub fn author(self, author: Option<String>) -> Self {
+        Self { author, ..self }
+    }
+
+    pub fn limit(self, limit: i32) -> Self {
+        Self {
+            limit: if limit != DEFAULT_LIMIT { Some(limit) } else { None },
+            ..self
+        }
+    }
+
+    pub fn into_link(self) -> String {
+        let args = serde_urlencoded::to_string(self).expect("Serializing to String cannot fail");
+        if args.len() > 0 {
+            format!("?{}", args)
+        } else {
+            "_changes".to_owned()
+        }
+    }
+}

src/components/changes/resource.rs

@@ -1,169 +1,20 @@
-use diesel;
 use futures::{self, Future};
-use futures::future::{done, finished};
 use hyper;
 use hyper::header::ContentType;
 use hyper::server::*;
-use serde_urlencoded;
 
 use mimes::*;
+use pagination::Pagination;
+use resources::DiffQueryParameters;
 use schema::article_revisions;
 use site::system_page;
 use state::State;
-use web::{Resource, ResponseFuture};
+use web;
 
-use super::diff_resource;
+use super::apply_query_config;
-use super::pagination::Pagination;
+use super::query_parameters;
-use super::TemporaryRedirectResource;
 
-const DEFAULT_LIMIT: i32 = 30;
+pub struct Resource {
-
-type BoxResource = Box<Resource + Sync + Send>;
-
-#[derive(Clone)]
-pub struct ChangesLookup {
-    state: State,
-    show_authors: bool,
-}
-
-#[derive(Serialize, Deserialize, Default)]
-pub struct QueryParameters {
-    after: Option<i32>,
-    before: Option<i32>,
-
-    article_id: Option<i32>,
-    author: Option<String>,
-
-    limit: Option<i32>,
-}
-
-impl QueryParameters {
-    pub fn pagination(self, pagination: Pagination<i32>) -> Self {
-        Self {
-            after: if let Pagination::After(x) = pagination { Some(x) } else { None },
-            before: if let Pagination::Before(x) = pagination { Some(x) } else { None },
-            ..self
-        }
-    }
-
-    pub fn article_id(self, article_id: Option<i32>) -> Self {
-        Self { article_id, ..self }
-    }
-
-    pub fn author(self, author: Option<String>) -> Self {
-        Self { author, ..self }
-    }
-
-    pub fn limit(self, limit: i32) -> Self {
-        Self {
-            limit: if limit != DEFAULT_LIMIT { Some(limit) } else { None },
-            ..self
-        }
-    }
-
-    pub fn into_link(self) -> String {
-        let args = serde_urlencoded::to_string(self).expect("Serializing to String cannot fail");
-        if args.len() > 0 {
-            format!("?{}", args)
-        } else {
-            "_changes".to_owned()
-        }
-    }
-}
-
-fn apply_query_config<'a>(
-    mut query: article_revisions::BoxedQuery<'a, diesel::sqlite::Sqlite>,
-    article_id: Option<i32>,
-    author: Option<String>,
-    limit: i32,
-)
-    -> article_revisions::BoxedQuery<'a, diesel::sqlite::Sqlite>
-{
-    use diesel::prelude::*;
-
-    if let Some(article_id) = article_id {
-        query = query.filter(article_revisions::article_id.eq(article_id));
-    }
-
-    if let Some(author) = author {
-        query = query.filter(article_revisions::author.eq(author));
-    }
-
-    query.limit(limit as i64 + 1)
-}
-
-impl ChangesLookup {
-    pub fn new(state: State, show_authors: bool) -> ChangesLookup {
-        Self { state, show_authors }
-    }
-
-    pub fn lookup(&self, query: Option<&str>) -> Box<Future<Item=Option<BoxResource>, Error=::web::Error>> {
-        use super::pagination;
-
-        let state = self.state.clone();
-        let show_authors = self.show_authors;
-
-        Box::new(
-            done((|| {
-                let params: QueryParameters = serde_urlencoded::from_str(query.unwrap_or(""))?;
-
-                let pagination = pagination::from_fields(params.after, params.before)?;
-
-                let limit = match params.limit {
-                    None => Ok(DEFAULT_LIMIT),
-                    Some(x) if 1 <= x && x <= 100 => Ok(x),
-                    _ => Err("`limit` argument must be in range [1, 100]"),
-                }?;
-
-                Ok((pagination, params.article_id, params.author, limit))
-            })())
-            .and_then(move |(pagination, article_id, author, limit)| match pagination {
-                Pagination::After(x) => {
-                    let author2 = author.clone();
-
-                    Box::new(state.query_article_revision_stubs(move |query| {
-                        use diesel::prelude::*;
-
-                        apply_query_config(query, article_id, author2, limit)
-                            .filter(article_revisions::sequence_number.gt(x))
-                            .order(article_revisions::sequence_number.asc())
-                    }).and_then(move |mut data| {
-                        let extra_element = if data.len() > limit as usize {
-                            data.pop()
-                        } else {
-                            None
-                        };
-
-                        let args =
-                            QueryParameters {
-                                after: None,
-                                before: None,
-                                article_id,
-                                author,
-                                limit: None,
-                            }
-                            .limit(limit);
-
-                        Ok(Some(match extra_element {
-                            Some(x) => Box::new(TemporaryRedirectResource::new(
-                                args
-                                    .pagination(Pagination::Before(x.sequence_number))
-                                    .into_link()
-                            )) as BoxResource,
-                            None => Box::new(TemporaryRedirectResource::new(
-                                args.into_link()
-                            )) as BoxResource,
-                        }))
-                    })) as Box<Future<Item=Option<BoxResource>, Error=::web::Error>>
-                },
-                Pagination::Before(x) => Box::new(finished(Some(Box::new(ChangesResource::new(state, show_authors, Some(x), article_id, author, limit)) as BoxResource))),
-                Pagination::None => Box::new(finished(Some(Box::new(ChangesResource::new(state, show_authors, None, article_id, author, limit)) as BoxResource))),
-            })
-        )
-    }
-}
-
-pub struct ChangesResource {
     state: State,
     show_authors: bool,
     before: Option<i32>,
@@ -172,41 +23,41 @@ pub struct ChangesResource {
     limit: i32,
 }
 
-impl ChangesResource {
+impl Resource {
     pub fn new(state: State, show_authors: bool, before: Option<i32>, article_id: Option<i32>, author: Option<String>, limit: i32) -> Self {
-        Self { state, show_authors, before, article_id, author, limit }
+        Resource { state, show_authors, before, article_id, author, limit }
     }
 
-    fn query_args(&self) -> QueryParameters {
+    fn query_args(&self) -> query_parameters::QueryParameters {
-        QueryParameters {
+        query_parameters::QueryParameters {
             after: None,
             before: self.before,
             article_id: self.article_id,
             author: self.author.clone(),
-            ..QueryParameters::default()
+            ..query_parameters::QueryParameters::default()
         }
         .limit(self.limit)
     }
 }
 
-impl Resource for ChangesResource {
+impl web::Resource for Resource {
     fn allow(&self) -> Vec<hyper::Method> {
         use hyper::Method::*;
         vec![Options, Head, Get]
     }
 
-    fn head(&self) -> ResponseFuture {
+    fn head(&self) -> web::ResponseFuture {
         Box::new(futures::finished(Response::new()
             .with_status(hyper::StatusCode::Ok)
             .with_header(ContentType(TEXT_HTML.clone()))
         ))
     }
 
-    fn get(self: Box<Self>) -> ResponseFuture {
+    fn get(self: Box<Self>) -> web::ResponseFuture {
         use chrono::{TimeZone, Local};
 
         struct Row<'a> {
-            resource: &'a ChangesResource,
+            resource: &'a Resource,
             sequence_number: i32,
 
             article_id: i32,
@@ -239,7 +90,7 @@ impl Resource for ChangesResource {
         #[derive(BartDisplay)]
         #[template="templates/changes.html"]
         struct Template<'a> {
-            resource: &'a ChangesResource,
+            resource: &'a Resource,
 
             show_authors: bool,
             newer: Option<NavLinks>,
@@ -341,7 +192,7 @@ impl Resource for ChangesResource {
                             if x.revision > 1 {
                                 Some(format!("_diff/{}?{}",
                                     x.article_id,
-                                    diff_resource::QueryParameters::new(
+                                    DiffQueryParameters::new(
                                         x.revision as u32 - 1,
                                         x.revision as u32,
                                     )

src/components/changes/scope.rs

@@ -0,0 +1,90 @@
+use futures::Future;
+use futures::future::{done, finished};
+use serde_urlencoded;
+
+use pagination::{self, Pagination};
+use resources::TemporaryRedirectResource;
+use schema::article_revisions;
+use state::State;
+use web;
+
+use super::apply_query_config;
+use super::query_parameters;
+use super::Resource;
+
+type BoxResource = Box<web::Resource + Sync + Send>;
+
+#[derive(Clone)]
+pub struct Scope {
+    state: State,
+    show_authors: bool,
+}
+
+impl Scope {
+    pub fn new(state: State, show_authors: bool) -> Scope {
+        Self { state, show_authors }
+    }
+
+    pub fn lookup(&self, query: Option<&str>) -> Box<Future<Item=Option<BoxResource>, Error=::web::Error>> {
+        let state = self.state.clone();
+        let show_authors = self.show_authors;
+
+        Box::new(
+            done((|| {
+                let params: query_parameters::QueryParameters = serde_urlencoded::from_str(query.unwrap_or(""))?;
+
+                let pagination = pagination::from_fields(params.after, params.before)?;
+
+                let limit = match params.limit {
+                    None => Ok(query_parameters::DEFAULT_LIMIT),
+                    Some(x) if 1 <= x && x <= 100 => Ok(x),
+                    _ => Err("`limit` argument must be in range [1, 100]"),
+                }?;
+
+                Ok((pagination, params.article_id, params.author, limit))
+            })())
+            .and_then(move |(pagination, article_id, author, limit)| match pagination {
+                Pagination::After(x) => {
+                    let author2 = author.clone();
+
+                    Box::new(state.query_article_revision_stubs(move |query| {
+                        use diesel::prelude::*;
+
+                        apply_query_config(query, article_id, author2, limit)
+                            .filter(article_revisions::sequence_number.gt(x))
+                            .order(article_revisions::sequence_number.asc())
+                    }).and_then(move |mut data| {
+                        let extra_element = if data.len() > limit as usize {
+                            data.pop()
+                        } else {
+                            None
+                        };
+
+                        let args =
+                            query_parameters::QueryParameters {
+                                after: None,
+                                before: None,
+                                article_id,
+                                author,
+                                limit: None,
+                            }
+                            .limit(limit);
+
+                        Ok(Some(match extra_element {
+                            Some(x) => Box::new(TemporaryRedirectResource::new(
+                                args
+                                    .pagination(Pagination::Before(x.sequence_number))
+                                    .into_link()
+                            )) as BoxResource,
+                            None => Box::new(TemporaryRedirectResource::new(
+                                args.into_link()
+                            )) as BoxResource,
+                        }))
+                    })) as Box<Future<Item=Option<BoxResource>, Error=::web::Error>>
+                },
+                Pagination::Before(x) => Box::new(finished(Some(Box::new(Resource::new(state, show_authors, Some(x), article_id, author, limit)) as BoxResource))),
+                Pagination::None => Box::new(finished(Some(Box::new(Resource::new(state, show_authors, None, article_id, author, limit)) as BoxResource))),
+            })
+        )
+    }
+}

src/components/mod.rs

@@ -0,0 +1 @@
+pub mod changes;

src/lib.rs

@@ -32,10 +32,12 @@ use std::net::{IpAddr, SocketAddr};
 
 mod assets;
 mod build_config;
+mod components;
 mod db;
 mod merge;
 mod mimes;
 mod models;
+mod pagination;
 mod rendering;
 mod resources;
 mod schema;

src/resources/article_resource.rs

@@ -13,7 +13,7 @@ use site::Layout;
 use state::{State, UpdateResult, RebaseConflict};
 use web::{Resource, ResponseFuture};
 
-use super::changes_resource::QueryParameters;
+use components::changes::QueryParameters;
 
 #[derive(BartDisplay)]
 #[template="templates/article.html"]

src/resources/article_revision_resource.rs

@@ -4,15 +4,15 @@ use hyper;
 use hyper::header::ContentType;
 use hyper::server::*;
 
+use components::changes::QueryParameters;
 use mimes::*;
 use models;
+use pagination::Pagination;
 use rendering::render_markdown;
 use site::system_page;
 use web::{Resource, ResponseFuture};
 
-use super::changes_resource::QueryParameters;
 use super::diff_resource;
-use super::pagination::Pagination;
 
 pub struct ArticleRevisionResource {
     data: models::ArticleRevision,

src/resources/diff_resource.rs

@@ -8,15 +8,14 @@ use hyper::header::ContentType;
 use hyper::server::*;
 use serde_urlencoded;
 
+use components::changes;
 use mimes::*;
 use models::ArticleRevision;
+use pagination::Pagination;
 use site::Layout;
 use state::State;
 use web::{Resource, ResponseFuture};
 
-use super::changes_resource;
-use super::pagination::Pagination;
-
 type BoxResource = Box<Resource + Sync + Send>;
 
 #[derive(Clone)]
@@ -126,7 +125,7 @@ impl Resource for DiffResource {
                             consecutive: self.to.revision - self.from.revision == 1,
                             article_id: self.from.article_id as u32,
                             article_history_link: &format!("_changes{}",
-                                changes_resource::QueryParameters::default()
+                                changes::QueryParameters::default()
                                     .article_id(Some(self.from.article_id))
                                     .pagination(Pagination::After(self.from.revision))
                                     .into_link()

src/resources/mod.rs

@@ -1,9 +1,6 @@
-pub mod pagination;
-
 mod about_resource;
 mod article_revision_resource;
 mod article_resource;
-mod changes_resource;
 mod diff_resource;
 mod html_resource;
 mod new_article_resource;
@@ -15,8 +12,8 @@ mod temporary_redirect_resource;
 pub use self::about_resource::AboutResource;
 pub use self::article_revision_resource::ArticleRevisionResource;
 pub use self::article_resource::ArticleResource;
-pub use self::changes_resource::{ChangesLookup, ChangesResource};
 pub use self::diff_resource::{DiffLookup, DiffResource};
+pub use self::diff_resource::QueryParameters as DiffQueryParameters;
 pub use self::html_resource::HtmlResource;
 pub use self::new_article_resource::NewArticleResource;
 pub use self::read_only_resource::ReadOnlyResource;

src/site.rs

@@ -11,7 +11,7 @@ use hyper;
 
 use assets::{ThemesCss, StyleCss, SearchJs};
 use build_config;
-use web::Lookup;
+use web::Scope;
 use wiki_lookup::WikiLookup;
 
 const THEMES: [&str; 19] = ["red", "pink", "purple", "deep-purple", "indigo",
@@ -115,12 +115,14 @@ impl Site {
 }
 
 fn root_base_from_request_uri(path: &str) -> Option<String> {
+    use std::iter::repeat;
+
     assert!(path.starts_with("/"));
     let slashes = path[1..].matches('/').count();
 
     match slashes {
         0 => None,
-        n => Some(::std::iter::repeat("../").take(n).collect())
+        n => Some(repeat("../").take(n).collect())
     }
 }
 
@@ -145,7 +147,7 @@ impl Service for Site {
         let base = root_base_from_request_uri(uri.path());
         let base2 = base.clone(); // Bah, stupid clone
 
-        Box::new(self.root.lookup(uri.path(), uri.query())
+        Box::new(self.root.scope_lookup(uri.path(), uri.query())
             .and_then(move |resource| match resource {
                 Some(mut resource) => {
                     use hyper::Method::*;

src/web/mod.rs

@@ -1,5 +1,5 @@
 mod resource;
-mod lookup;
+mod scope;
 
 pub use self::resource::*;
-pub use self::lookup::*;
+pub use self::scope::*;

src/web/scope.rs

@@ -1,9 +1,9 @@
 use futures;
 
-pub trait Lookup {
+pub trait Scope {
     type Resource;
     type Error;
     type Future: futures::Future<Item=Option<Self::Resource>, Error=Self::Error>;
 
-    fn lookup(&self, path: &str, query: Option<&str>) -> Self::Future;
+    fn scope_lookup(&self, path: &str, query: Option<&str>) -> Self::Future;
 }

src/wiki_lookup.rs

@@ -8,8 +8,9 @@ use percent_encoding::percent_decode;
 use slug::slugify;
 
 use resources::*;
+use components::*;
 use state::State;
-use web::{Lookup, Resource};
+use web::{Scope, Resource};
 
 #[allow(unused)]
 use assets::*;
@@ -40,7 +41,7 @@ lazy_static! {
 #[derive(Clone)]
 pub struct WikiLookup {
     state: State,
-    changes_lookup: ChangesLookup,
+    changes_lookup: changes::Scope,
     diff_lookup: DiffLookup,
     search_lookup: SearchLookup,
 }
@@ -104,15 +105,15 @@ fn fs_lookup(root: &str, path: &str) ->
 
 impl WikiLookup {
     pub fn new(state: State, show_authors: bool) -> WikiLookup {
-        let changes_lookup = ChangesLookup::new(state.clone(), show_authors);
+        let changes_lookup = changes::Scope::new(state.clone(), show_authors);
         let diff_lookup = DiffLookup::new(state.clone());
         let search_lookup = SearchLookup::new(state.clone());
 
         WikiLookup { state, changes_lookup, diff_lookup, search_lookup }
     }
 
-    fn revisions_lookup(&self, path: &str, _query: Option<&str>) -> <Self as Lookup>::Future {
+    fn revisions_lookup(&self, path: &str, _query: Option<&str>) -> <Self as Scope>::Future {
-        let (article_id, revision): (i32, i32) = match (|| -> Result<_, <Self as Lookup>::Error> {
+        let (article_id, revision): (i32, i32) = match (|| -> Result<_, <Self as Scope>::Error> {
             let (article_id, tail) = split_one(path)?;
             let (revision, tail) = split_one(tail.ok_or("Not found")?)?;
             if tail.is_some() {
@@ -135,8 +136,8 @@ impl WikiLookup {
         )
     }
 
-    fn by_id_lookup(&self, path: &str, _query: Option<&str>) -> <Self as Lookup>::Future {
+    fn by_id_lookup(&self, path: &str, _query: Option<&str>) -> <Self as Scope>::Future {
-        let article_id: i32 = match (|| -> Result<_, <Self as Lookup>::Error> {
+        let article_id: i32 = match (|| -> Result<_, <Self as Scope>::Error> {
             let (article_id, tail) = split_one(path)?;
             if tail.is_some() {
                 return Err("Not found".into());
@@ -158,8 +159,8 @@ impl WikiLookup {
         )
     }
 
-    fn diff_lookup_f(&self, path: &str, query: Option<&str>) -> <Self as Lookup>::Future {
+    fn diff_lookup_f(&self, path: &str, query: Option<&str>) -> <Self as Scope>::Future {
-        let article_id: u32 = match (|| -> Result<_, <Self as Lookup>::Error> {
+        let article_id: u32 = match (|| -> Result<_, <Self as Scope>::Error> {
             let (article_id, tail) = split_one(path)?;
             if tail.is_some() {
                 return Err("Not found".into());
@@ -174,7 +175,7 @@ impl WikiLookup {
         Box::new(self.diff_lookup.lookup(article_id, query))
     }
 
-    fn reserved_lookup(&self, path: &str, query: Option<&str>) -> <Self as Lookup>::Future {
+    fn reserved_lookup(&self, path: &str, query: Option<&str>) -> <Self as Scope>::Future {
         let (head, tail) = match split_one(path) {
             Ok(x) => x,
             Err(x) => return Box::new(failed(x.into())),
@@ -209,7 +210,7 @@ impl WikiLookup {
         }
     }
 
-    fn article_lookup(&self, path: &str, query: Option<&str>) -> <Self as Lookup>::Future {
+    fn article_lookup(&self, path: &str, query: Option<&str>) -> <Self as Scope>::Future {
         let (slug, tail) = match split_one(path) {
             Ok(x) => x,
             Err(x) => return Box::new(failed(x.into())),
@@ -246,12 +247,12 @@ impl WikiLookup {
     }
 }
 
-impl Lookup for WikiLookup {
+impl Scope for WikiLookup {
     type Resource = BoxResource;
     type Error = Box<::std::error::Error + Send + Sync>;
     type Future = Box<Future<Item = Option<Self::Resource>, Error = Self::Error>>;
 
-    fn lookup(&self, path: &str, query: Option<&str>) -> Self::Future {
+    fn scope_lookup(&self, path: &str, query: Option<&str>) -> Self::Future {
         assert!(path.starts_with("/"));
         let path = &path[1..];