From b3ffcb916ba06f8a355fbd1e91e867e3b7c605cc Mon Sep 17 00:00:00 2001 From: Magnus Hoff Date: Sun, 22 Oct 2017 13:38:13 +0200 Subject: [PATCH] Redirect after-pagination queries to the corresponding before-queries This normalizes the URLs, avoiding duplicate URLs for a sublist --- src/resources/changes_resource.rs | 15 ++++--- src/resources/mod.rs | 2 + src/resources/temporary_redirect_resource.rs | 46 ++++++++++++++++++++ src/wiki_lookup.rs | 2 +- 4 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 src/resources/temporary_redirect_resource.rs diff --git a/src/resources/changes_resource.rs b/src/resources/changes_resource.rs index 7b30609..2e3dbfc 100644 --- a/src/resources/changes_resource.rs +++ b/src/resources/changes_resource.rs @@ -11,16 +11,19 @@ use state::State; use web::{Resource, ResponseFuture}; use super::pagination::Pagination; +use super::TemporaryRedirectResource; const PAGE_SIZE: i32 = 30; +type BoxResource = Box; + pub struct ChangesResource { state: State, before: Option, } impl ChangesResource { - pub fn new(state: State, pagination: Pagination) -> Box> { + pub fn new(state: State, pagination: Pagination) -> Box> { match pagination { Pagination::After(x) => Box::new( state.query_article_revision_stubs(move |query| { @@ -38,14 +41,14 @@ impl ChangesResource { None }; - Ok(Self { - state, - before: extra_element.map(|x| x.sequence_number), + Ok(match extra_element { + Some(x) => Box::new(TemporaryRedirectResource::new(format!("?before={}", x.sequence_number))) as BoxResource, + None => Box::new(TemporaryRedirectResource::new(format!("_changes"))) as BoxResource, }) }) ), - Pagination::Before(x) => Box::new(finished(Self { state, before: Some(x) })), - Pagination::None => Box::new(finished(Self { state, before: None })), + Pagination::Before(x) => Box::new(finished(Box::new(Self { state, before: Some(x) }) as BoxResource)), + Pagination::None => Box::new(finished(Box::new(Self { state, before: None }) as BoxResource)), } } } diff --git a/src/resources/mod.rs b/src/resources/mod.rs index 097b909..6a66aa1 100644 --- a/src/resources/mod.rs +++ b/src/resources/mod.rs @@ -5,9 +5,11 @@ mod article_resource; mod changes_resource; mod new_article_resource; mod sitemap_resource; +mod temporary_redirect_resource; pub use self::article_redirect_resource::ArticleRedirectResource; pub use self::article_resource::ArticleResource; pub use self::changes_resource::ChangesResource; pub use self::new_article_resource::NewArticleResource; pub use self::sitemap_resource::SitemapResource; +pub use self::temporary_redirect_resource::TemporaryRedirectResource; diff --git a/src/resources/temporary_redirect_resource.rs b/src/resources/temporary_redirect_resource.rs new file mode 100644 index 0000000..085072e --- /dev/null +++ b/src/resources/temporary_redirect_resource.rs @@ -0,0 +1,46 @@ +use futures::{self, Future}; +use hyper; +use hyper::header::Location; +use hyper::server::*; + +use web::{Resource, ResponseFuture}; + +pub struct TemporaryRedirectResource { + location: String, +} + +impl TemporaryRedirectResource { + pub fn new(location: String) -> Self { + Self { location } + } +} + +impl Resource for TemporaryRedirectResource { + fn allow(&self) -> Vec { + use hyper::Method::*; + vec![Options, Head, Get, Put, Post] + } + + fn head(&self) -> ResponseFuture { + Box::new(futures::finished(Response::new() + .with_status(hyper::StatusCode::TemporaryRedirect) + .with_header(Location::new(self.location.clone())) + )) + } + + fn get(self: Box) -> ResponseFuture { + Box::new(self.head() + .and_then(move |head| { + Ok(head + .with_body(format!("Moved to {}", self.location))) + })) + } + + fn put(self: Box, _body: hyper::Body, _identity: Option) -> ResponseFuture { + Box::new(self.head() + .and_then(move |head| { + Ok(head + .with_body(format!("Moved to {}", self.location))) + })) + } +} diff --git a/src/wiki_lookup.rs b/src/wiki_lookup.rs index 44ffc65..9574918 100644 --- a/src/wiki_lookup.rs +++ b/src/wiki_lookup.rs @@ -87,7 +87,7 @@ impl WikiLookup { Box::new( done(pagination::from_str(query.unwrap_or("")).map_err(Into::into)) .and_then(move |pagination| ChangesResource::new(state, pagination)) - .and_then(|changes_resource| Ok(Some(Box::new(changes_resource) as BoxResource))) + .and_then(|resource| Ok(Some(resource))) ) }, ("_new", None) =>