From d6416c9be1f202a6156e4203c6c393689f569b05 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Mon, 7 Nov 2022 16:28:02 +0000 Subject: [PATCH] radicle-cob: remove Resource on Store The `Store` trait had a paramter for a `Resource`. This is too restrictive for implementations, particularly `heartwood`. Instead, where a unique identifier is needed for the `object::Storage`, i.e. in `create` and `update`, the identifier becomes a parameter. This allows the removal of passing the resource in `list`, `get` and `changegraph`. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-cob/src/identity.rs | 3 -- radicle-cob/src/lib.rs | 21 +++++------ .../src/object/collaboration/create.rs | 11 ++++-- radicle-cob/src/object/collaboration/get.rs | 20 +++++------ radicle-cob/src/object/collaboration/info.rs | 11 +++--- radicle-cob/src/object/collaboration/list.rs | 15 ++++---- .../src/object/collaboration/update.rs | 15 +++++--- radicle-cob/src/test/identity/person.rs | 7 ---- radicle-cob/src/test/identity/project.rs | 35 +++++-------------- radicle-cob/src/test/storage.rs | 4 +-- radicle-cob/src/tests.rs | 19 ++++++---- 11 files changed, 74 insertions(+), 87 deletions(-) diff --git a/radicle-cob/src/identity.rs b/radicle-cob/src/identity.rs index 359d8805..9a280b18 100644 --- a/radicle-cob/src/identity.rs +++ b/radicle-cob/src/identity.rs @@ -23,7 +23,4 @@ pub trait Identity { /// expected to be the latest address for the identity at the time /// of use. fn content_id(&self) -> Oid; - - /// The unique, stable identifier for the identity. - fn identifier(&self) -> Self::Identifier; } diff --git a/radicle-cob/src/lib.rs b/radicle-cob/src/lib.rs index 36b9340d..8483b907 100644 --- a/radicle-cob/src/lib.rs +++ b/radicle-cob/src/lib.rs @@ -40,12 +40,15 @@ //! * [`change::Storage`] -- **Note**: there is already an //! implementation for this for [`git2::Repository`] for convenience. //! -//! The `Store` also takes a generic parameter to indicate the type of -//! resource the collaborative object is living inside, for example a -//! software project. This `Resource` must also implement -//! [`identity::Identity`] to allow the internal logic to reference -//! the resource's content-address in `git` as well as the stable -//! identifier used for the resource. +//! ## Resource +//! +//! The [`create`] and [`update`] functions take a `Resource`. It +//! represents the type of resource the collaborative objects are +//! relating to, for example a software project. +//! +//! This `Resource` must implement [`identity::Identity`] to allow the +//! internal logic to reference the resource's content-address in +//! `git` as well as the stable identifier used for the resource. //! //! ## History Traversal //! @@ -111,16 +114,14 @@ mod tests; /// for the specific `git` storage: /// /// * [`object::Storage`] -/// * [`identity::Identity`] for `Resource` /// /// **Note**: [`change::Storage`] is already implemented for /// [`git2::Repository`]. It is expected that the underlying storage /// for `object::Storage` will also be `git2::Repository`, but if not /// please open an issue to change the definition of `Store` :) -pub trait Store +pub trait Store where - Resource: identity::Identity, - Self: object::Storage + Self: object::Storage + change::Storage< CreateError = git::change::error::Create, LoadError = git::change::error::Load, diff --git a/radicle-cob/src/object/collaboration/create.rs b/radicle-cob/src/object/collaboration/create.rs index 937fd751..ad623b5d 100644 --- a/radicle-cob/src/object/collaboration/create.rs +++ b/radicle-cob/src/object/collaboration/create.rs @@ -40,7 +40,11 @@ impl Create { /// ensures that the objects origin is cryptographically verifiable. /// /// The `resource` is the parent of this object, for example a -/// software project. +/// software project. Its content-address is stored in the object's +/// history. +/// +/// The `identifier` is a unqiue id that is passed through to the +/// [`crate::object::Storage`]. /// /// The `args` are the metadata for this [`CollaborativeObject`]. See /// [`Create`] for further information. @@ -48,10 +52,11 @@ pub fn create( storage: &S, signer: Signer, resource: &Resource, + identifier: &S::Identifier, args: Create, ) -> Result where - S: Store, + S: Store, Author: Identity, Author::Identifier: Clone + PartialEq, Resource: Identity, @@ -88,7 +93,7 @@ where let object_id = init_change.id().into(); storage - .update(&resource.identifier(), typename, &object_id, &init_change) + .update(identifier, typename, &object_id, &init_change) .map_err(|err| error::Create::Refs { err: Box::new(err) })?; Ok(CollaborativeObject { diff --git a/radicle-cob/src/object/collaboration/get.rs b/radicle-cob/src/object/collaboration/get.rs index d88ba7e6..2c248b29 100644 --- a/radicle-cob/src/object/collaboration/get.rs +++ b/radicle-cob/src/object/collaboration/get.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::{change_graph::ChangeGraph, identity, CollaborativeObject, ObjectId, Store, TypeName}; +use crate::{change_graph::ChangeGraph, CollaborativeObject, ObjectId, Store, TypeName}; use super::error; @@ -13,23 +13,23 @@ use super::error; /// [`crate::Change`]s at content-addressable locations. Please see /// [`Store`] for further information. /// -/// The `resource` is the parent of this object, for example a -/// software project. +/// The `identifier` is a unqiue id that is passed through to the +/// [`crate::object::Storage`]. /// -/// The `typename` is the type of object to be found, while the `oid` -/// is the identifier for the particular object under that type. -pub fn get( +/// The `typename` is the type of object to be found, while the +/// `object_id` is the identifier for the particular object under that +/// type. +pub fn get( storage: &S, - resource: &Resource, + identifier: &S::Identifier, typename: &TypeName, oid: &ObjectId, ) -> Result, error::Retrieve> where - S: Store, - Resource: identity::Identity, + S: Store, { let tip_refs = storage - .objects(&resource.identifier(), typename, oid) + .objects(identifier, typename, oid) .map_err(|err| error::Retrieve::Refs { err: Box::new(err) })?; Ok(ChangeGraph::load(storage, tip_refs.iter(), typename, oid).map(|graph| graph.evaluate())) } diff --git a/radicle-cob/src/object/collaboration/info.rs b/radicle-cob/src/object/collaboration/info.rs index 55783473..7305acea 100644 --- a/radicle-cob/src/object/collaboration/info.rs +++ b/radicle-cob/src/object/collaboration/info.rs @@ -14,7 +14,7 @@ use std::collections::BTreeSet; use git_ext::Oid; -use crate::{change_graph::ChangeGraph, identity::Identity, ObjectId, Store, TypeName}; +use crate::{change_graph::ChangeGraph, ObjectId, Store, TypeName}; use super::error; @@ -43,18 +43,17 @@ pub struct ChangeGraphInfo { /// /// The `typename` is the type of object to be found, while the `oid` /// is the identifier for the particular object under that type. -pub fn changegraph( +pub fn changegraph( storage: &S, - resource: &Resource, + identifier: &S::Identifier, typename: &TypeName, oid: &ObjectId, ) -> Result, error::Retrieve> where - S: Store, - Resource: Identity, + S: Store, { let tip_refs = storage - .objects(&resource.identifier(), typename, oid) + .objects(identifier, typename, oid) .map_err(|err| error::Retrieve::Refs { err: Box::new(err) })?; Ok( ChangeGraph::load(storage, tip_refs.iter(), typename, oid).map(|graph| ChangeGraphInfo { diff --git a/radicle-cob/src/object/collaboration/list.rs b/radicle-cob/src/object/collaboration/list.rs index ff988111..fb02dc44 100644 --- a/radicle-cob/src/object/collaboration/list.rs +++ b/radicle-cob/src/object/collaboration/list.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::{change_graph::ChangeGraph, identity::Identity, CollaborativeObject, Store, TypeName}; +use crate::{change_graph::ChangeGraph, CollaborativeObject, Store, TypeName}; use super::error; @@ -13,21 +13,20 @@ use super::error; /// [`crate::Change`]s at content-addressable locations. Please see /// [`Store`] for further information. /// -/// The `resource` is the parent of this object, for example a -/// software project. +/// The `identifier` is a unqiue id that is passed through to the +/// [`crate::object::Storage`]. /// /// The `typename` is the type of objects to listed. -pub fn list( +pub fn list( storage: &S, - resource: &Resource, + identifier: &S::Identifier, typename: &TypeName, ) -> Result, error::Retrieve> where - S: Store, - Resource: Identity, + S: Store, { let references = storage - .types(&resource.identifier(), typename) + .types(identifier, typename) .map_err(|err| error::Retrieve::Refs { err: Box::new(err) })?; log::trace!("loaded {} references", references.len()); let mut result = Vec::new(); diff --git a/radicle-cob/src/object/collaboration/update.rs b/radicle-cob/src/object/collaboration/update.rs index 634762b4..d9f463df 100644 --- a/radicle-cob/src/object/collaboration/update.rs +++ b/radicle-cob/src/object/collaboration/update.rs @@ -12,7 +12,7 @@ use super::error; /// The data required to update an object pub struct Update { - /// The identity of the author for this object's first change. + /// The identity of the author for the update of this object. pub author: Option, /// The CRDT changes to add to the object. pub changes: Contents, @@ -34,7 +34,11 @@ pub struct Update { /// ensures that the objects origin is cryptographically verifiable. /// /// The `resource` is the parent of this object, for example a -/// software project. +/// software project. Its content-address is stored in the +/// object's history. +/// +/// The `identifier` is a unqiue id that is passed through to the +/// [`crate::object::Storage`]. /// /// The `args` are the metadata for this [`CollaborativeObject`] /// udpate. See [`Update`] for further information. @@ -42,10 +46,11 @@ pub fn update( storage: &S, signer: Signer, resource: &Resource, + identifier: &S::Identifier, args: Update, ) -> Result where - S: Store, + S: Store, Author: Identity, Author::Identifier: Clone + PartialEq, Resource: Identity, @@ -71,7 +76,7 @@ where }; let existing_refs = storage - .objects(&resource.identifier(), typename, &object_id) + .objects(identifier, typename, &object_id) .map_err(|err| error::Update::Refs { err: Box::new(err) })?; let mut object = ChangeGraph::load(storage, existing_refs.iter(), typename, &object_id) @@ -94,7 +99,7 @@ where .history .extend(change.id, content, change.resource, changes); storage - .update(&resource.identifier(), typename, &object_id, &change) + .update(identifier, typename, &object_id, &change) .map_err(|err| error::Update::Refs { err: Box::new(err) })?; Ok(object) diff --git a/radicle-cob/src/test/identity/person.rs b/radicle-cob/src/test/identity/person.rs index d1ab4bef..e71812ed 100644 --- a/radicle-cob/src/test/identity/person.rs +++ b/radicle-cob/src/test/identity/person.rs @@ -102,11 +102,4 @@ impl Identity for Person { fn content_id(&self) -> Oid { self.content_id } - - fn identifier(&self) -> Self::Identifier { - Urn { - name: self.name().clone(), - remote: None, - } - } } diff --git a/radicle-cob/src/test/identity/project.rs b/radicle-cob/src/test/identity/project.rs index ded22c65..326f8b58 100644 --- a/radicle-cob/src/test/identity/project.rs +++ b/radicle-cob/src/test/identity/project.rs @@ -14,6 +14,15 @@ pub struct RemoteProject { pub person: test::Person, } +impl RemoteProject { + pub fn identifier(&self) -> Urn { + Urn { + name: self.project.name().clone(), + remote: Some(self.person.name().clone()), + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Project { pub payload: Payload, @@ -104,25 +113,6 @@ fn from_commit( })) } -impl Identity for Project { - type Identifier = Urn; - - fn is_delegate(&self, delegation: &crypto::PublicKey) -> bool { - self.delegates().contains(delegation) - } - - fn content_id(&self) -> Oid { - self.content_id - } - - fn identifier(&self) -> Self::Identifier { - Urn { - name: self.name().clone(), - remote: None, - } - } -} - impl Identity for RemoteProject { type Identifier = Urn; @@ -133,11 +123,4 @@ impl Identity for RemoteProject { fn content_id(&self) -> Oid { self.project.content_id } - - fn identifier(&self) -> Self::Identifier { - Urn { - name: self.project.name().clone(), - remote: Some(self.person.name().clone()), - } - } } diff --git a/radicle-cob/src/test/storage.rs b/radicle-cob/src/test/storage.rs index 142e25b1..969701e0 100644 --- a/radicle-cob/src/test/storage.rs +++ b/radicle-cob/src/test/storage.rs @@ -8,7 +8,7 @@ use crate::{ ObjectId, Store, }; -use super::identity::{RemoteProject, Urn}; +use super::identity::Urn; pub mod error { use thiserror::Error; @@ -60,7 +60,7 @@ impl Storage { } } -impl Store for Storage {} +impl Store for Storage {} impl change::Storage for Storage { type CreateError = ::CreateError; diff --git a/radicle-cob/src/tests.rs b/radicle-cob/src/tests.rs index 5d70512e..56cbf261 100644 --- a/radicle-cob/src/tests.rs +++ b/radicle-cob/src/tests.rs @@ -4,9 +4,7 @@ use crypto::test::signer::MockSigner; use quickcheck::Arbitrary; use radicle_crypto::Signer; -use crate::{ - create, get, history, identity::Identity, list, update, Create, ObjectId, TypeName, Update, -}; +use crate::{create, get, history, list, update, Create, ObjectId, TypeName, Update}; use super::test; @@ -26,6 +24,7 @@ fn roundtrip() { &storage, signer, &proj, + &proj.identifier(), Create { author: Some(terry), contents, @@ -35,7 +34,7 @@ fn roundtrip() { ) .unwrap(); - let expected = get(&storage, &proj, &typename, cob.id()) + let expected = get(&storage, &proj.identifier(), &typename, cob.id()) .unwrap() .expect("BUG: cob was missing"); @@ -57,6 +56,7 @@ fn list_cobs() { &storage, signer.clone(), &proj, + &proj.identifier(), Create { author: Some(terry.clone()), contents: history::Contents::Automerge(b"issue 1".to_vec()), @@ -70,6 +70,7 @@ fn list_cobs() { &storage, signer, &proj, + &proj.identifier(), Create { author: Some(terry), contents: history::Contents::Automerge(b"issue 2".to_vec()), @@ -79,7 +80,7 @@ fn list_cobs() { ) .unwrap(); - let mut expected = list(&storage, &proj, &typename).unwrap(); + let mut expected = list(&storage, &proj.identifier(), &typename).unwrap(); expected.sort_by(|x, y| x.id().cmp(y.id())); let mut actual = vec![issue_1, issue_2]; @@ -104,6 +105,7 @@ fn update_cob() { &storage, signer.clone(), &proj, + &proj.identifier(), Create { author: Some(terry.clone()), contents, @@ -113,7 +115,7 @@ fn update_cob() { ) .unwrap(); - let not_expected = get(&storage, &proj, &typename, cob.id()) + let not_expected = get(&storage, &proj.identifier(), &typename, cob.id()) .unwrap() .expect("BUG: cob was missing"); @@ -121,6 +123,7 @@ fn update_cob() { &storage, signer, &proj, + &proj.identifier(), Update { author: Some(terry), changes: history::Contents::Automerge(b"issue 1".to_vec()), @@ -131,7 +134,7 @@ fn update_cob() { ) .unwrap(); - let expected = get(&storage, &proj, &typename, updated.id()) + let expected = get(&storage, &proj.identifier(), &typename, updated.id()) .unwrap() .expect("BUG: cob was missing"); @@ -160,6 +163,7 @@ fn traverse_cobs() { &storage, terry_signer, &terry_proj, + &terry_proj.identifier(), Create { author: Some(terry), contents: history::Contents::Automerge(b"issue 1".to_vec()), @@ -181,6 +185,7 @@ fn traverse_cobs() { &storage, neil_signer, &neil_proj, + &neil_proj.identifier(), Update { author: Some(neil), changes: history::Contents::Automerge(b"issue 2".to_vec()),