From 2e5b549ea8dc93a42881a73619f4a07457eeea7f Mon Sep 17 00:00:00 2001 From: Magnus Hoff Date: Mon, 6 Nov 2017 15:09:21 +0100 Subject: [PATCH] Restructure the diff resource so it is only possible to see diffs between revisions of the same article --- src/resources/changes_resource.rs | 13 +-- src/resources/diff_resource.rs | 131 +++++------------------------- src/wiki_lookup.rs | 20 ++++- 3 files changed, 45 insertions(+), 119 deletions(-) diff --git a/src/resources/changes_resource.rs b/src/resources/changes_resource.rs index 5926e5f..8515bd0 100644 --- a/src/resources/changes_resource.rs +++ b/src/resources/changes_resource.rs @@ -12,7 +12,7 @@ use site::Layout; use state::State; use web::{Resource, ResponseFuture}; -use super::diff_resource::{self, ArticleRevisionReference}; +use super::diff_resource; use super::pagination::Pagination; use super::TemporaryRedirectResource; @@ -337,10 +337,13 @@ impl Resource for ChangesResource { _slug: x.slug, title: x.title, _latest: x.latest, - diff_link: diff_resource::QueryParameters::new( - ArticleRevisionReference::Some { article_id: x.article_id as u32, revision: x.revision as u32 - 1 }, - ArticleRevisionReference::Some { article_id: x.article_id as u32, revision: x.revision as u32 }, - ).into_link() + diff_link: format!("_diff/{}?{}", + x.article_id, + diff_resource::QueryParameters::new( + x.revision as u32 - 1, + x.revision as u32, + ) + ), } }).collect::>(); diff --git a/src/resources/diff_resource.rs b/src/resources/diff_resource.rs index 1385f09..b2ffb1e 100644 --- a/src/resources/diff_resource.rs +++ b/src/resources/diff_resource.rs @@ -1,108 +1,21 @@ -use std; -use std::str::FromStr; +use std::{self, fmt}; use diff; use futures::{self, Future}; -use futures::future::{done, finished}; +use futures::future::done; use hyper; use hyper::header::ContentType; use hyper::server::*; use serde_urlencoded; use mimes::*; -use models::ArticleRevision; use site::Layout; use state::State; use web::{Resource, ResponseFuture}; -const NONE: &str = "none"; - type BoxResource = Box; type Error = Box; -#[derive(Clone)] -pub enum ArticleRevisionReference { - None, - Some { - article_id: u32, - revision: u32, - } -} - -use std::num::ParseIntError; - -pub enum ArticleRevisionReferenceParseError { - SplitError, - ParseIntError(ParseIntError), -} - -use std::fmt; -impl fmt::Display for ArticleRevisionReferenceParseError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use self::ArticleRevisionReferenceParseError::*; - match self { - &SplitError => write!(f, "invalid format, must contain one @"), - &ParseIntError(ref r) => r.fmt(f) - } - } -} - -impl From for ArticleRevisionReferenceParseError { - fn from(x: ParseIntError) -> ArticleRevisionReferenceParseError { - ArticleRevisionReferenceParseError::ParseIntError(x) - } -} - -impl FromStr for ArticleRevisionReference { - type Err = ArticleRevisionReferenceParseError; - - fn from_str(s: &str) -> Result { - if s == NONE { - return Ok(ArticleRevisionReference::None); - } - - let items: Vec<&str> = s.split("@").collect(); - if items.len() != 2 { - return Err(ArticleRevisionReferenceParseError::SplitError) - } - - let article_id = items[0].parse::()?; - let revision = items[1].parse::()?; - - Ok(ArticleRevisionReference::Some { article_id, revision }) - } -} - -impl fmt::Display for ArticleRevisionReference { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - &ArticleRevisionReference::None => write!(f, "{}", NONE), - &ArticleRevisionReference::Some { article_id, revision } => { - write!(f, "{}@{}", article_id, revision) - } - } - } -} - -use serde::{de, Deserialize, Deserializer}; -impl<'de> Deserialize<'de> for ArticleRevisionReference { - fn deserialize(deserializer: D) -> Result - where D: Deserializer<'de> - { - let s = String::deserialize(deserializer)?; - FromStr::from_str(&s).map_err(de::Error::custom) - } -} - -use serde::{Serialize, Serializer}; -impl Serialize for ArticleRevisionReference { - fn serialize(&self, serializer: S) -> Result - where S: Serializer - { - serializer.serialize_str(&self.to_string()) - } -} - #[derive(Clone)] pub struct DiffLookup { state: State, @@ -110,17 +23,19 @@ pub struct DiffLookup { #[derive(Serialize, Deserialize)] pub struct QueryParameters { - from: ArticleRevisionReference, - to: ArticleRevisionReference, + from: u32, + to: u32, } impl QueryParameters { - pub fn new(from: ArticleRevisionReference, to: ArticleRevisionReference) -> QueryParameters { + pub fn new(from: u32, to: u32) -> QueryParameters { QueryParameters { from, to } } +} - pub fn into_link(self) -> String { - format!("_diff?{}", serde_urlencoded::to_string(self).expect("Serializing to String cannot fail")) +impl fmt::Display for QueryParameters { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(&serde_urlencoded::to_string(self).expect("Serializing to String cannot fail")) } } @@ -129,26 +44,27 @@ impl DiffLookup { Self { state } } - pub fn lookup(&self, query: Option<&str>) -> Box, Error=::web::Error>> { + pub fn lookup(&self, article_id: u32, query: Option<&str>) -> Box, Error=::web::Error>> { let state = self.state.clone(); Box::new(done((|| -> Result, ::web::Error> { let params: QueryParameters = serde_urlencoded::from_str(query.unwrap_or(""))?; - Ok(Some(Box::new(DiffResource::new(state, params.from, params.to)))) + Ok(Some(Box::new(DiffResource::new(state, article_id, params.from, params.to)))) }()))) } } pub struct DiffResource { state: State, - from: ArticleRevisionReference, - to: ArticleRevisionReference, + article_id: u32, + from: u32, + to: u32, } impl DiffResource { - pub fn new(state: State, from: ArticleRevisionReference, to: ArticleRevisionReference) -> Self { - Self { state, from, to } + pub fn new(state: State, article_id: u32, from: u32, to: u32) -> Self { + Self { state, article_id, from, to } } fn query_args(&self) -> QueryParameters { @@ -157,15 +73,6 @@ impl DiffResource { to: self.to.clone(), } } - - fn get_article_revision(&self, r: &ArticleRevisionReference) -> Box, Error = Error>> { - match r { - &ArticleRevisionReference::None => Box::new(finished(None)), - &ArticleRevisionReference::Some { article_id, revision } => Box::new( - self.state.get_article_revision(article_id as i32, revision as i32) - ), - } - } } impl Resource for DiffResource { @@ -196,8 +103,8 @@ impl Resource for DiffResource { added: Option, } - let from = self.get_article_revision(&self.from); - let to = self.get_article_revision(&self.to); + let from = self.state.get_article_revision(self.article_id as i32, self.from as i32); + let to = self.state.get_article_revision(self.article_id as i32, self.to as i32); let head = self.head(); @@ -205,7 +112,7 @@ impl Resource for DiffResource { .and_then(move |(head, from, to)| { Ok(head .with_body(Layout { - base: None, // Hmm, should perhaps accept `base` as argument + base: Some("../"), // Hmm, should perhaps accept `base` as argument title: "Difference", body: &Template { title: &diff::chars( diff --git a/src/wiki_lookup.rs b/src/wiki_lookup.rs index f448d82..a66a520 100644 --- a/src/wiki_lookup.rs +++ b/src/wiki_lookup.rs @@ -137,6 +137,22 @@ impl WikiLookup { ) } + fn diff_lookup_f(&self, path: &str, query: Option<&str>) -> ::Future { + let article_id: u32 = match (|| -> Result<_, ::Error> { + let (article_id, tail) = split_one(path)?; + if tail.is_some() { + return Err("Not found".into()); + } + + Ok(article_id.parse()?) + })() { + Ok(x) => x, + Err(_) => return Box::new(finished(None)), + }; + + Box::new(self.diff_lookup.lookup(article_id, query)) + } + fn reserved_lookup(&self, path: &str, query: Option<&str>) -> ::Future { let (head, tail) = match split_one(path) { Ok(x) => x, @@ -154,8 +170,8 @@ impl WikiLookup { self.by_id_lookup(tail, query), ("_changes", None) => Box::new(self.changes_lookup.lookup(query)), - ("_diff", None) => - Box::new(self.diff_lookup.lookup(query)), + ("_diff", Some(tail)) => + self.diff_lookup_f(tail, query), ("_new", None) => Box::new(finished(Some(Box::new(NewArticleResource::new(self.state.clone(), None)) as BoxResource))), ("_revisions", Some(tail)) =>