From 5526eceea020140a3c6604bb7e8e6a75e156d35e Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 24 Nov 2022 11:48:53 +0100 Subject: [PATCH] cob: Add new history type, simplify `Content` Keep the history type in the manifest, but no need to specify it for all individual changes. Signed-off-by: Alexis Sellier --- radicle-cob/src/backend/git/change.rs | 7 +++-- radicle-cob/src/change/store.rs | 1 + radicle-cob/src/history.rs | 9 ++++-- radicle-cob/src/history/entry.rs | 31 ++----------------- .../src/object/collaboration/create.rs | 5 ++- .../src/object/collaboration/update.rs | 8 +++-- radicle-cob/src/tests.rs | 29 ++++++++++------- radicle/src/cob.rs | 2 +- radicle/src/cob/issue.rs | 29 ++++++++--------- radicle/src/cob/label.rs | 19 +++++------- radicle/src/cob/patch.rs | 29 ++++++++--------- radicle/src/cob/store.rs | 10 +++--- 12 files changed, 80 insertions(+), 99 deletions(-) diff --git a/radicle-cob/src/backend/git/change.rs b/radicle-cob/src/backend/git/change.rs index 980944bd..9789aa3d 100644 --- a/radicle-cob/src/backend/git/change.rs +++ b/radicle-cob/src/backend/git/change.rs @@ -96,13 +96,14 @@ impl change::Storage for git2::Repository { { let change::Create { typename, + history_type, tips, message, contents, } = spec; let manifest = store::Manifest { typename, - history_type: (&contents).into(), + history_type, }; let revision = write_manifest(self, &manifest, &contents)?; @@ -200,7 +201,7 @@ fn load_contents( manifest: &store::Manifest, ) -> Result { Ok(match manifest.history_type { - HistoryType::Automerge => { + HistoryType::Default | HistoryType::Automerge => { let contents_tree_entry = tree .get_name(CHANGE_BLOB_NAME) .ok_or_else(|| error::Load::NoChange(tree.id().into()))?; @@ -208,7 +209,7 @@ fn load_contents( let contents_blob = contents_object .as_blob() .ok_or_else(|| error::Load::ChangeNotBlob(tree.id().into()))?; - entry::Contents::automerge(contents_blob.content()) + contents_blob.content().to_owned() } }) } diff --git a/radicle-cob/src/change/store.rs b/radicle-cob/src/change/store.rs index 6bf2e426..0ae9daf6 100644 --- a/radicle-cob/src/change/store.rs +++ b/radicle-cob/src/change/store.rs @@ -47,6 +47,7 @@ pub trait Storage { pub struct Create { pub typename: TypeName, + pub history_type: HistoryType, pub tips: Vec, pub message: String, pub contents: Contents, diff --git a/radicle-cob/src/history.rs b/radicle-cob/src/history.rs index 08015881..40795846 100644 --- a/radicle-cob/src/history.rs +++ b/radicle-cob/src/history.rs @@ -16,14 +16,17 @@ use crate::pruning_fold; pub mod entry; pub use entry::{Contents, Entry, EntryId}; -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] +#[derive( + Clone, Copy, Debug, Default, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize, +)] +#[serde(rename_all = "lowercase")] pub enum HistoryType { + #[default] + Default, Automerge, } /// The DAG of changes making up the history of a collaborative object. -/// -/// The `Author` represents the content address for the author of the change entry. #[derive(Clone, Debug)] pub struct History { graph: petgraph::Graph, diff --git a/radicle-cob/src/history/entry.rs b/radicle-cob/src/history/entry.rs index 04bfe8a7..427ffa5a 100644 --- a/radicle-cob/src/history/entry.rs +++ b/radicle-cob/src/history/entry.rs @@ -7,34 +7,9 @@ use git_ext::Oid; use crate::pruning_fold; -use super::HistoryType; - -#[derive(Clone, Debug, PartialEq, Hash, Eq)] -pub enum Contents { - Automerge(Vec), -} - -impl Contents { - pub fn automerge(bytes: &[u8]) -> Self { - Self::Automerge(bytes.to_vec()) - } -} - -impl From<&Contents> for HistoryType { - fn from(c: &Contents) -> Self { - match c { - Contents::Automerge(..) => HistoryType::Automerge, - } - } -} - -impl AsRef<[u8]> for Contents { - fn as_ref(&self) -> &[u8] { - match self { - Self::Automerge(bytes) => bytes, - } - } -} +/// Entry contents. +/// This is the change payload. +pub type Contents = Vec; /// A unique identifier for a history entry. #[derive(Clone, Copy, Debug, PartialEq, Hash, Eq, PartialOrd, Ord)] diff --git a/radicle-cob/src/object/collaboration/create.rs b/radicle-cob/src/object/collaboration/create.rs index 08caf974..cabacfcc 100644 --- a/radicle-cob/src/object/collaboration/create.rs +++ b/radicle-cob/src/object/collaboration/create.rs @@ -3,7 +3,7 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. -use crate::Store; +use crate::{HistoryType, Store}; use super::*; @@ -11,6 +11,8 @@ use super::*; pub struct Create { /// The identity of the author for this object's first change. pub author: Option, + /// The type of history that will be used for this object. + pub history_type: HistoryType, /// The CRDT history to initialize this object with. pub contents: Contents, /// The typename for this object. @@ -23,6 +25,7 @@ impl Create { fn create_spec(&self) -> change::Create { change::Create { typename: self.typename.clone(), + history_type: self.history_type, tips: Vec::new(), message: self.message.clone(), contents: self.contents.clone(), diff --git a/radicle-cob/src/object/collaboration/update.rs b/radicle-cob/src/object/collaboration/update.rs index 250cdc2c..5c098378 100644 --- a/radicle-cob/src/object/collaboration/update.rs +++ b/radicle-cob/src/object/collaboration/update.rs @@ -4,8 +4,8 @@ // Linking Exception. For full terms see the included LICENSE file. use crate::{ - change, change_graph::ChangeGraph, identity::Identity, CollaborativeObject, Contents, ObjectId, - Store, TypeName, + change, change_graph::ChangeGraph, identity::Identity, CollaborativeObject, Contents, + HistoryType, ObjectId, Store, TypeName, }; use super::error; @@ -14,6 +14,8 @@ use super::error; pub struct Update { /// The identity of the author for the update of this object. pub author: Option, + /// The type of history that will be used for this object. + pub history_type: HistoryType, /// The CRDT changes to add to the object. pub changes: Contents, /// The object ID of the object to be updated. @@ -60,6 +62,7 @@ where author, ref typename, object_id, + history_type, changes, message, } = args; @@ -89,6 +92,7 @@ where signer, change::Create { tips: object.tips().iter().cloned().collect(), + history_type, contents: changes.clone(), typename: typename.clone(), message, diff --git a/radicle-cob/src/tests.rs b/radicle-cob/src/tests.rs index 10299cf4..b3b78fdb 100644 --- a/radicle-cob/src/tests.rs +++ b/radicle-cob/src/tests.rs @@ -6,7 +6,7 @@ use quickcheck::Arbitrary; use radicle_crypto::Signer; use crate::{ - create, get, history, list, object, test::arbitrary::Invalid, update, Create, ObjectId, + create, get, list, object, test::arbitrary::Invalid, update, Create, HistoryType, ObjectId, TypeName, Update, }; @@ -22,7 +22,6 @@ fn roundtrip() { project: proj, person: terry.clone(), }; - let contents = history::Contents::Automerge(Vec::new()); let typename = "xyz.rad.issue".parse::().unwrap(); let cob = create( &storage, @@ -31,7 +30,8 @@ fn roundtrip() { &proj.identifier(), Create { author: Some(terry), - contents, + history_type: HistoryType::Automerge, + contents: Vec::new(), typename: typename.clone(), message: "creating xyz.rad.issue".to_string(), }, @@ -63,7 +63,8 @@ fn list_cobs() { &proj.identifier(), Create { author: Some(terry.clone()), - contents: history::Contents::Automerge(b"issue 1".to_vec()), + history_type: HistoryType::Automerge, + contents: b"issue 1".to_vec(), typename: typename.clone(), message: "creating xyz.rad.issue".to_string(), }, @@ -77,7 +78,8 @@ fn list_cobs() { &proj.identifier(), Create { author: Some(terry), - contents: history::Contents::Automerge(b"issue 2".to_vec()), + history_type: HistoryType::Automerge, + contents: b"issue 2".to_vec(), typename: typename.clone(), message: "commenting xyz.rad.issue".to_string(), }, @@ -103,7 +105,6 @@ fn update_cob() { project: proj, person: terry.clone(), }; - let contents = history::Contents::Automerge(Vec::new()); let typename = "xyz.rad.issue".parse::().unwrap(); let cob = create( &storage, @@ -112,7 +113,8 @@ fn update_cob() { &proj.identifier(), Create { author: Some(terry.clone()), - contents, + history_type: HistoryType::Automerge, + contents: Vec::new(), typename: typename.clone(), message: "creating xyz.rad.issue".to_string(), }, @@ -130,7 +132,8 @@ fn update_cob() { &proj.identifier(), Update { author: Some(terry), - changes: history::Contents::Automerge(b"issue 1".to_vec()), + changes: b"issue 1".to_vec(), + history_type: HistoryType::Automerge, object_id: *cob.id(), typename: typename.clone(), message: "commenting xyz.rad.issue".to_string(), @@ -170,7 +173,8 @@ fn traverse_cobs() { &terry_proj.identifier(), Create { author: Some(terry), - contents: history::Contents::Automerge(b"issue 1".to_vec()), + contents: b"issue 1".to_vec(), + history_type: HistoryType::Automerge, typename: typename.clone(), message: "creating xyz.rad.issue".to_string(), }, @@ -192,7 +196,8 @@ fn traverse_cobs() { &neil_proj.identifier(), Update { author: Some(neil), - changes: history::Contents::Automerge(b"issue 2".to_vec()), + changes: b"issue 2".to_vec(), + history_type: HistoryType::Automerge, object_id: *cob.id(), typename, message: "commenting on xyz.rad.issue".to_string(), @@ -210,7 +215,7 @@ fn traverse_cobs() { if let (Some(author), Some(project)) = (author, project) { if project.delegate_check(&author) { - acc.push(entry.contents().as_ref().to_vec()) + acc.push(entry.contents().to_vec()) } } ControlFlow::Continue(acc) @@ -220,7 +225,7 @@ fn traverse_cobs() { // traverse over the history and filter by changes that were only authorized by neil let contents = updated.history().traverse(Vec::new(), |mut acc, entry| { - acc.push(entry.contents().as_ref().to_vec()); + acc.push(entry.contents().to_vec()); ControlFlow::Continue(acc) }); diff --git a/radicle/src/cob.rs b/radicle/src/cob.rs index 1ede2f76..27291e6d 100644 --- a/radicle/src/cob.rs +++ b/radicle/src/cob.rs @@ -9,7 +9,7 @@ pub mod value; pub use cob::{ identity, object::collaboration::error, CollaborativeObject, Contents, Create, Entry, History, - ObjectId, TypeName, Update, + HistoryType, ObjectId, TypeName, Update, }; pub use shared::Timestamp; pub use store::Store; diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index ebe3688c..3dc1f874 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -135,16 +135,13 @@ impl FromHistory for Issue { fn from_history(history: &History) -> Result { let doc = history.traverse(Automerge::new(), |mut doc, entry| { - match entry.contents() { - Contents::Automerge(bytes) => { - match automerge::Change::from_bytes(bytes.clone()) { - Ok(change) => { - doc.apply_changes([change]).ok(); - } - Err(_err) => { - // Ignore - } - } + let bytes = entry.contents(); + match automerge::Change::from_bytes(bytes.clone()) { + Ok(change) => { + doc.apply_changes([change]).ok(); + } + Err(_err) => { + // Ignore } } ControlFlow::Continue(doc) @@ -379,7 +376,7 @@ mod events { .map_err(|failure| failure.error)? .result; - Ok(Contents::Automerge(doc.save_incremental())) + Ok(doc.save_incremental()) } pub fn comment( @@ -414,7 +411,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = issue.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn lifecycle( @@ -439,7 +436,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = issue.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn label( @@ -466,7 +463,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = issue.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn reply( @@ -503,7 +500,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = issue.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn react( @@ -546,7 +543,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = issue.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } } diff --git a/radicle/src/cob/label.rs b/radicle/src/cob/label.rs index 653c06f5..15642f8d 100644 --- a/radicle/src/cob/label.rs +++ b/radicle/src/cob/label.rs @@ -43,16 +43,13 @@ impl TryFrom<&History> for Label { fn try_from(history: &History) -> Result { let doc = history.traverse(Automerge::new(), |mut doc, entry| { - match entry.contents() { - Contents::Automerge(bytes) => { - match automerge::Change::from_bytes(bytes.clone()) { - Ok(change) => { - doc.apply_changes([change]).ok(); - } - Err(_err) => { - // Ignore - } - } + let bytes = entry.contents(); + match automerge::Change::from_bytes(bytes.clone()) { + Ok(change) => { + doc.apply_changes([change]).ok(); + } + Err(_err) => { + // Ignore } } ControlFlow::Continue(doc) @@ -143,7 +140,7 @@ mod events { ) .map_err(|failure| failure.error)?; - Ok(Contents::Automerge(doc.save_incremental())) + Ok(doc.save_incremental()) } } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 14720e54..40c5ea7a 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -161,16 +161,13 @@ impl TryFrom<&History> for Patch { fn try_from(history: &History) -> Result { let doc = history.traverse(Automerge::new(), |mut doc, entry| { - match entry.contents() { - Contents::Automerge(bytes) => { - match automerge::Change::from_bytes(bytes.clone()) { - Ok(change) => { - doc.apply_changes([change]).ok(); - } - Err(_err) => { - // Ignore - } - } + let bytes = entry.contents(); + match automerge::Change::from_bytes(bytes.clone()) { + Ok(change) => { + doc.apply_changes([change]).ok(); + } + Err(_err) => { + // Ignore } } ControlFlow::Continue(doc) @@ -750,7 +747,7 @@ mod events { .map_err(|failure| failure.error)? .result; - Ok(Contents::Automerge(doc.save_incremental())) + Ok(doc.save_incremental()) } pub fn comment( @@ -788,7 +785,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = patch.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn update( @@ -817,7 +814,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = patch.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok((revision_ix, Contents::Automerge(change))) + Ok((revision_ix, change)) } pub fn reply( @@ -857,7 +854,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = patch.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } pub fn review( @@ -888,7 +885,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = patch.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(((), Contents::Automerge(change))) + Ok(((), change)) } pub fn merge( @@ -921,7 +918,7 @@ mod events { #[allow(clippy::unwrap_used)] let change = patch.get_last_local_change().unwrap().raw_bytes().to_vec(); - Ok(Contents::Automerge(change)) + Ok(change) } } diff --git a/radicle/src/cob/store.rs b/radicle/src/cob/store.rs index 40fd20ef..cf3f7fa7 100644 --- a/radicle/src/cob/store.rs +++ b/radicle/src/cob/store.rs @@ -11,7 +11,7 @@ use crate::cob::shared::Author; use crate::cob::transaction::TransactionError; use crate::cob::CollaborativeObject; use crate::cob::{issue, label, patch}; -use crate::cob::{Contents, Create, History, ObjectId, TypeName, Update}; +use crate::cob::{Contents, Create, History, HistoryType, ObjectId, TypeName, Update}; use crate::crypto::PublicKey; use crate::git; use crate::identity::project; @@ -134,6 +134,7 @@ impl<'a, T: FromHistory> Store<'a, T> { Update { author: Some(cob::Author::from(*signer.public_key())), object_id, + history_type: HistoryType::Automerge, typename: T::type_name().clone(), message: message.to_owned(), changes, @@ -154,6 +155,7 @@ impl<'a, T: FromHistory> Store<'a, T> { &self.project, Create { author: Some(cob::Author::from(*signer.public_key())), + history_type: HistoryType::Automerge, typename: T::type_name().clone(), message: message.to_owned(), contents, @@ -182,11 +184,7 @@ impl<'a, T: FromHistory> Store<'a, T> { }; let doc = cob.history().traverse(Vec::new(), |mut doc, entry| { - match entry.contents() { - Contents::Automerge(bytes) => { - doc.extend(bytes); - } - } + doc.extend(entry.contents()); ControlFlow::Continue(doc) });