From b685139d5b94406c063eb02bceb02ebb5c6fcc77 Mon Sep 17 00:00:00 2001 From: Magnus Hoff Date: Mon, 20 Nov 2017 16:07:33 +0100 Subject: [PATCH] Implement merge conflict handling for the noscript case #23 --- src/merge/mod.rs | 49 +++++++++++ src/models.rs | 2 +- src/resources/article_resource.rs | 85 +++++++++++++------- src/resources/new_article_resource.rs | 8 +- src/resources/temporary_redirect_resource.rs | 10 +-- src/state.rs | 19 ++++- templates/article.html | 2 +- 7 files changed, 134 insertions(+), 41 deletions(-) diff --git a/src/merge/mod.rs b/src/merge/mod.rs index d601919..a2f136f 100644 --- a/src/merge/mod.rs +++ b/src/merge/mod.rs @@ -29,6 +29,55 @@ impl<'a> MergeResult<&'a str> { } } +impl MergeResult { + pub fn flatten(self) -> String { + match self { + MergeResult::Clean(x) => x, + MergeResult::Conflicted(x) => { + x.into_iter() + .flat_map(|out| match out { + Output::Conflict(a, _o, b) => { + let mut x: Vec = vec![]; + x.push("<<<<<<< Your changes:\n".into()); + x.extend(a.into_iter().map(|x| format!("{}\n", x))); + x.push("======= Their changes:\n".into()); + x.extend(b.into_iter().map(|x| format!("{}\n", x))); + x.push(">>>>>>> Conflict ends here\n".into()); + x + }, + Output::Resolved(x) => + x.into_iter().map(|x| format!("{}\n", x)).collect(), + }) + .collect() + } + } + } +} + +impl MergeResult { + pub fn flatten(self) -> String { + match self { + MergeResult::Clean(x) => x, + MergeResult::Conflicted(x) => { + x.into_iter() + .flat_map(|out| match out { + Output::Conflict(a, _o, b) => { + let mut x: Vec = vec![]; + x.push('<'); + x.extend(a); + x.push('|'); + x.extend(b); + x.push('>'); + x + }, + Output::Resolved(x) => x, + }) + .collect() + } + } + } +} + pub fn merge_lines<'a>(a: &'a str, o: &'a str, b: &'a str) -> MergeResult<&'a str> { let oa = diff::lines(o, a); let ob = diff::lines(o, b); diff --git a/src/models.rs b/src/models.rs index 70e918a..9a2acf2 100644 --- a/src/models.rs +++ b/src/models.rs @@ -29,7 +29,7 @@ impl ArticleRevision { pub fn link(&self) -> &str { slug_link(&self.slug) } } -#[derive(Debug, Queryable)] +#[derive(Debug, PartialEq, Queryable)] pub struct ArticleRevisionStub { pub sequence_number: i32, diff --git a/src/resources/article_resource.rs b/src/resources/article_resource.rs index 4bbc0bb..464eb5d 100644 --- a/src/resources/article_resource.rs +++ b/src/resources/article_resource.rs @@ -10,16 +10,28 @@ use assets::ScriptJs; use mimes::*; use rendering::render_markdown; use site::Layout; -use state::State; +use state::{State, UpdateResult, RebaseConflict}; use web::{Resource, ResponseFuture}; use super::changes_resource::QueryParameters; -pub struct ArticleResource { - state: State, - article_id: i32, +#[derive(BartDisplay)] +#[template="templates/article.html"] +struct Template<'a> { revision: i32, + last_updated: Option<&'a str>, + edit: bool, + cancel_url: Option<&'a str>, + title: &'a str, + raw: &'a str, + rendered: String, +} + +impl<'a> Template<'a> { + fn script_js_checksum(&self) -> &'static str { + ScriptJs::checksum() + } } #[derive(Deserialize)] @@ -29,6 +41,13 @@ struct UpdateArticle { body: String, } +pub struct ArticleResource { + state: State, + article_id: i32, + revision: i32, + edit: bool, +} + impl ArticleResource { pub fn new(state: State, article_id: i32, revision: i32, edit: bool) -> Self { Self { state, article_id, revision, edit } @@ -73,21 +92,6 @@ impl Resource for ArticleResource { } fn get(self: Box) -> ResponseFuture { - #[derive(BartDisplay)] - #[template="templates/article.html"] - struct Template<'a> { - revision: i32, - last_updated: Option<&'a str>, - - edit: bool, - cancel_url: Option<&'a str>, - title: &'a str, - raw: &'a str, - rendered: String, - - script_js_checksum: &'a str, - } - let data = self.state.get_article_revision(self.article_id, self.revision) .map(|x| x.expect("Data model guarantees that this exists")); let head = self.head(); @@ -110,7 +114,6 @@ impl Resource for ArticleResource { title: &data.title, raw: &data.body, rendered: render_markdown(&data.body), - script_js_checksum: ScriptJs::checksum(), }, }.to_string())) })) @@ -187,13 +190,41 @@ impl Resource for ArticleResource { self.state.update_article(self.article_id, update.base_revision, update.title, update.body, identity) }) .and_then(|updated| { - let updated = updated.unwrap(); - futures::finished(Response::new() - .with_status(hyper::StatusCode::SeeOther) - .with_header(ContentType(TEXT_PLAIN.clone())) - .with_header(Location::new(updated.link().to_owned())) - .with_body("See other") - ) + match updated { + UpdateResult::Success(updated) => Ok(Response::new() + .with_status(hyper::StatusCode::SeeOther) + .with_header(ContentType(TEXT_PLAIN.clone())) + .with_header(Location::new(updated.link().to_owned())) + .with_body("See other") + ), + UpdateResult::RebaseConflict(RebaseConflict { + base_article, title, body + }) => { + let title = title.flatten(); + let body = body.flatten(); + Ok(Response::new() + .with_status(hyper::StatusCode::Ok) + .with_header(ContentType(TEXT_HTML.clone())) + .with_body(Layout { + base: None, + title: &title, + body: &Template { + revision: base_article.revision, + last_updated: Some(&last_updated( + base_article.article_id, + &Local.from_utc_datetime(&base_article.created), + base_article.author.as_ref().map(|x| &**x) + )), + edit: true, + cancel_url: Some(base_article.link()), + title: &title, + raw: &body, + rendered: render_markdown(&body), + }, + }.to_string()) + ) + } + } }) ) } diff --git a/src/resources/new_article_resource.rs b/src/resources/new_article_resource.rs index c619535..c17aca3 100644 --- a/src/resources/new_article_resource.rs +++ b/src/resources/new_article_resource.rs @@ -67,8 +67,11 @@ impl Resource for NewArticleResource { title: &'a str, raw: &'a str, rendered: &'a str, - - script_js_checksum: &'a str, + } + impl<'a> Template<'a> { + fn script_js_checksum(&self) -> &'static str { + ScriptJs::checksum() + } } let title = self.slug.as_ref() @@ -92,7 +95,6 @@ impl Resource for NewArticleResource { title: &title, raw: "", rendered: EMPTY_ARTICLE_MESSAGE, - script_js_checksum: ScriptJs::checksum(), }, }.to_string())) })) diff --git a/src/resources/temporary_redirect_resource.rs b/src/resources/temporary_redirect_resource.rs index 53b3417..aab660e 100644 --- a/src/resources/temporary_redirect_resource.rs +++ b/src/resources/temporary_redirect_resource.rs @@ -48,10 +48,10 @@ impl Resource for TemporaryRedirectResource { } 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))) - })) + self.get() + } + + fn post(self: Box, _body: hyper::Body, _identity: Option) -> ResponseFuture { + self.get() } } diff --git a/src/state.rs b/src/state.rs index d9adab5..d78f206 100644 --- a/src/state.rs +++ b/src/state.rs @@ -42,9 +42,9 @@ struct NewRevision<'a> { #[derive(Debug, PartialEq)] pub struct RebaseConflict { - base_revision: i32, - title: merge::MergeResult, - body: merge::MergeResult, + pub base_article: models::ArticleRevisionStub, + pub title: merge::MergeResult, + pub body: merge::MergeResult, } #[derive(Debug, PartialEq)] @@ -164,6 +164,17 @@ impl<'a> SyncState<'a> { ) } + fn get_article_revision_stub(&self, article_id: i32, revision: i32) -> Result, Error> { + use schema::article_revisions; + + Ok(self.query_article_revision_stubs(move |query| { + query + .filter(article_revisions::article_id.eq(article_id)) + .filter(article_revisions::revision.eq(revision)) + .limit(1) + })?.pop()) + } + pub fn lookup_slug(&self, slug: String) -> Result { #[derive(Queryable)] struct ArticleRevisionStub { @@ -233,7 +244,7 @@ impl<'a> SyncState<'a> { (Clean(title), Clean(body)) => (title, body), (title_merge, body_merge) => { return Ok(RebaseResult::Conflict(RebaseConflict { - base_revision: revision, + base_article: self.get_article_revision_stub(article_id, revision+1)?.expect("Application layer guarantee"), title: title_merge, body: body_merge.to_strings(), })); diff --git a/templates/article.html b/templates/article.html index 1b2c9d4..b9093af 100644 --- a/templates/article.html +++ b/templates/article.html @@ -1,4 +1,4 @@ - +