From bf9716ccb8dfe3030f1f0d029b97576ffd5913d1 Mon Sep 17 00:00:00 2001 From: Magnus Hoff Date: Mon, 20 Nov 2017 15:08:34 +0100 Subject: [PATCH] Propagate rebase conflicts out from state object --- src/merge/mod.rs | 13 +++- src/merge/output.rs | 15 +++- src/resources/article_resource.rs | 2 + src/state.rs | 115 +++++++++++++++++++++++------- 4 files changed, 118 insertions(+), 27 deletions(-) diff --git a/src/merge/mod.rs b/src/merge/mod.rs index c0486fe..d601919 100644 --- a/src/merge/mod.rs +++ b/src/merge/mod.rs @@ -13,11 +13,22 @@ use self::output::Output::Resolved; pub use self::output::Output; #[derive(Debug, PartialEq)] -pub enum MergeResult { +pub enum MergeResult { Clean(String), Conflicted(Vec>), } +impl<'a> MergeResult<&'a str> { + pub fn to_strings(self) -> MergeResult { + match self { + MergeResult::Clean(x) => MergeResult::Clean(x), + MergeResult::Conflicted(x) => MergeResult::Conflicted( + x.into_iter().map(Output::to_strings).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/merge/output.rs b/src/merge/output.rs index 30e278b..5fb58b2 100644 --- a/src/merge/output.rs +++ b/src/merge/output.rs @@ -6,11 +6,24 @@ use diff::Result::*; use super::chunk::Chunk; #[derive(Debug, PartialEq)] -pub enum Output { +pub enum Output { Resolved(Vec), Conflict(Vec, Vec, Vec), } +impl<'a> Output<&'a str> { + pub fn to_strings(self) -> Output { + match self { + Output::Resolved(x) => Output::Resolved(x.into_iter().map(str::to_string).collect()), + Output::Conflict(a, o, b) => Output::Conflict( + a.into_iter().map(str::to_string).collect(), + o.into_iter().map(str::to_string).collect(), + b.into_iter().map(str::to_string).collect(), + ), + } + } +} + fn choose_left(operations: &[diff::Result]) -> Vec { operations .iter() diff --git a/src/resources/article_resource.rs b/src/resources/article_resource.rs index 192e383..4bbc0bb 100644 --- a/src/resources/article_resource.rs +++ b/src/resources/article_resource.rs @@ -148,6 +148,7 @@ 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::Ok) .with_header(ContentType(APPLICATION_JSON.clone())) @@ -186,6 +187,7 @@ 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())) diff --git a/src/state.rs b/src/state.rs index e63a2c6..d9adab5 100644 --- a/src/state.rs +++ b/src/state.rs @@ -40,6 +40,34 @@ struct NewRevision<'a> { latest: bool, } +#[derive(Debug, PartialEq)] +pub struct RebaseConflict { + base_revision: i32, + title: merge::MergeResult, + body: merge::MergeResult, +} + +#[derive(Debug, PartialEq)] +enum RebaseResult { + Clean { title: String, body: String }, + Conflict(RebaseConflict), +} + +pub enum UpdateResult { + Success(models::ArticleRevision), + RebaseConflict(RebaseConflict), +} + +impl UpdateResult { + // TODO Move to mod tests below + pub fn unwrap(self) -> models::ArticleRevision { + match self { + UpdateResult::Success(x) => x, + _ => panic!("Expected success") + } + } +} + fn decide_slug(conn: &SqliteConnection, article_id: i32, prev_title: &str, title: &str, prev_slug: Option<&str>) -> Result { let base_slug = ::slug::slugify(title); @@ -175,7 +203,7 @@ impl<'a> SyncState<'a> { } fn rebase_update(&self, article_id: i32, target_base_revision: i32, existing_base_revision: i32, title: String, body: String) - -> Result<(String, String), Error> + -> Result { let mut title_a = title; let mut body_a = body; @@ -195,22 +223,33 @@ impl<'a> SyncState<'a> { let (title_b, body_b) = stored.pop().expect("Application layer guarantee"); let (title_o, body_o) = stored.pop().expect("Application layer guarantee"); - title_a = match merge::merge_chars(&title_a, &title_o, &title_b) { - merge::MergeResult::Clean(merged) => merged, - _ => unimplemented!("Missing handling of merge conflicts"), + use merge::MergeResult::*; + + let update = { + let title_merge = merge::merge_chars(&title_a, &title_o, &title_b); + let body_merge = merge::merge_lines(&body_a, &body_o, &body_b); + + match (title_merge, body_merge) { + (Clean(title), Clean(body)) => (title, body), + (title_merge, body_merge) => { + return Ok(RebaseResult::Conflict(RebaseConflict { + base_revision: revision, + title: title_merge, + body: body_merge.to_strings(), + })); + }, + } }; - body_a = match merge::merge_lines(&body_a, &body_o, &body_b) { - merge::MergeResult::Clean(merged) => merged, - _ => unimplemented!("Missing handling of merge conflicts"), - }; + title_a = update.0; + body_a = update.1; } - Ok((title_a, body_a)) + Ok(RebaseResult::Clean { title: title_a, body: body_a }) } pub fn update_article(&self, article_id: i32, base_revision: i32, title: String, body: String, author: Option) - -> Result + -> Result { if title.is_empty() { Err("title cannot be empty")?; @@ -236,7 +275,12 @@ impl<'a> SyncState<'a> { Err("This edit is based on a future version of the article")?; } - let (title, body) = self.rebase_update(article_id, latest_revision, base_revision, title, body)?; + let rebase_result = self.rebase_update(article_id, latest_revision, base_revision, title, body)?; + + let (title, body) = match rebase_result { + RebaseResult::Clean { title, body } => (title, body), + RebaseResult::Conflict(x) => return Ok(UpdateResult::RebaseConflict(x)), + }; let new_revision = latest_revision + 1; @@ -262,11 +306,11 @@ impl<'a> SyncState<'a> { .into(article_revisions::table) .execute(self.db_connection)?; - Ok(article_revisions::table + Ok(UpdateResult::Success(article_revisions::table .filter(article_revisions::article_id.eq(article_id)) .filter(article_revisions::revision.eq(new_revision)) .first::(self.db_connection)? - ) + )) }) } @@ -409,7 +453,7 @@ impl State { } pub fn update_article(&self, article_id: i32, base_revision: i32, title: String, body: String, author: Option) - -> CpuFuture + -> CpuFuture { self.execute(move |state| state.update_article(article_id, base_revision, title, body, author)) } @@ -465,7 +509,7 @@ mod test { let article = state.create_article(None, "Title".into(), "Body".into(), None).unwrap(); - let new_revision = state.update_article(article.article_id, article.revision, article.title.clone(), "New body".into(), None).unwrap(); + let new_revision = state.update_article(article.article_id, article.revision, article.title.clone(), "New body".into(), None).unwrap().unwrap(); assert_eq!(article.article_id, new_revision.article_id); @@ -486,8 +530,8 @@ mod test { let article = state.create_article(None, "Title".into(), "Body".into(), None).unwrap(); - let first_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "New body".into(), None).unwrap(); - let second_edit = state.update_article(article.article_id, first_edit.revision, article.title.clone(), "Newer body".into(), None).unwrap(); + let first_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "New body".into(), None).unwrap().unwrap(); + let second_edit = state.update_article(article.article_id, first_edit.revision, article.title.clone(), "Newer body".into(), None).unwrap().unwrap(); assert_eq!("Newer body", second_edit.body); } @@ -498,8 +542,8 @@ mod test { let article = state.create_article(None, "Title".into(), "a\nb\nc\n".into(), None).unwrap(); - let first_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nx\nb\nc\n".into(), None).unwrap(); - let second_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nb\ny\nc\n".into(), None).unwrap(); + let first_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nx\nb\nc\n".into(), None).unwrap().unwrap(); + let second_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nb\ny\nc\n".into(), None).unwrap().unwrap(); assert!(article.revision < first_edit.revision); assert!(first_edit.revision < second_edit.revision); @@ -513,11 +557,11 @@ mod test { let article = state.create_article(None, "Title".into(), "a\nb\nc\n".into(), None).unwrap(); - let edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nx1\nb\nc\n".into(), None).unwrap(); - let edit = state.update_article(article.article_id, edit.revision, article.title.clone(), "a\nx1\nx2\nb\nc\n".into(), None).unwrap(); - let edit = state.update_article(article.article_id, edit.revision, article.title.clone(), "a\nx1\nx2\nx3\nb\nc\n".into(), None).unwrap(); + let edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nx1\nb\nc\n".into(), None).unwrap().unwrap(); + let edit = state.update_article(article.article_id, edit.revision, article.title.clone(), "a\nx1\nx2\nb\nc\n".into(), None).unwrap().unwrap(); + let edit = state.update_article(article.article_id, edit.revision, article.title.clone(), "a\nx1\nx2\nx3\nb\nc\n".into(), None).unwrap().unwrap(); - let rebase_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nb\ny\nc\n".into(), None).unwrap(); + let rebase_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "a\nb\ny\nc\n".into(), None).unwrap().unwrap(); assert!(article.revision < edit.revision); assert!(edit.revision < rebase_edit.revision); @@ -531,12 +575,33 @@ mod test { let article = state.create_article(None, "titlle".into(), "".into(), None).unwrap(); - let first_edit = state.update_article(article.article_id, article.revision, "Titlle".into(), article.body.clone(), None).unwrap(); - let second_edit = state.update_article(article.article_id, article.revision, "title".into(), article.body.clone(), None).unwrap(); + let first_edit = state.update_article(article.article_id, article.revision, "Titlle".into(), article.body.clone(), None).unwrap().unwrap(); + let second_edit = state.update_article(article.article_id, article.revision, "title".into(), article.body.clone(), None).unwrap().unwrap(); assert!(article.revision < first_edit.revision); assert!(first_edit.revision < second_edit.revision); assert_eq!("Title", second_edit.title); } + + #[test] + fn update_article_when_merge_conflict() { + init!(state); + + let article = state.create_article(None, "Title".into(), "a".into(), None).unwrap(); + + state.update_article(article.article_id, article.revision, article.title.clone(), "b".into(), None).unwrap().unwrap(); + let conflict_edit = state.update_article(article.article_id, article.revision, article.title.clone(), "c".into(), None).unwrap(); + + match conflict_edit { + UpdateResult::Success(..) => panic!("Expected conflict"), + UpdateResult::RebaseConflict(RebaseConflict { base_revision, title, body }) => { + assert_eq!(article.revision, base_revision); + assert_eq!(title, merge::MergeResult::Clean(article.title.clone())); + assert_eq!(body, merge::MergeResult::Conflicted(vec![ + merge::Output::Conflict(vec!["c"], vec!["a"], vec!["b"]), + ]).to_strings()); + } + }; + } }