Update storage::data_directory module

- add membership validation for the add_content()
- add StorageProviderHelper
- add runtime integration
- add some comments
- add signed storage provider check for accept_content() and reject_content().

runtime-modules/common/src/origin_validator.rs

@@ -1,5 +1,5 @@
 /// Abstract validator for the origin(account_id) and actor_id (eg.: thread author id).
 pub trait ActorOriginValidator<Origin, ActorId, AccountId> {
-    /// Check for valid combination of origin and actor_id
+    /// Check for valid combination of origin and actor_id.
     fn ensure_actor_origin(origin: Origin, actor_id: ActorId) -> Result<AccountId, &'static str>;
 }

runtime-modules/storage/src/data_directory.rs

@@ -2,18 +2,26 @@
 //#![warn(missing_docs)]
 
 use codec::{Codec, Decode, Encode};
-use roles::actors;
-use roles::traits::Roles;
 use rstd::prelude::*;
 use sr_primitives::traits::{MaybeSerialize, Member, SimpleArithmetic};
 use srml_support::{decl_event, decl_module, decl_storage, dispatch, ensure, Parameter};
-use system::{self, ensure_root, ensure_signed};
+use system::{self, ensure_root};
+
+use common::origin_validator::ActorOriginValidator;
 
 use crate::data_object_type_registry;
 use crate::data_object_type_registry::IsActiveDataObjectType;
+use crate::{StorageBureaucracy, StorageProviderId, MemberId};
+
+
+// TODO: create a StorageProviderHelper implementation
 
 pub trait Trait:
-    timestamp::Trait + system::Trait + data_object_type_registry::Trait + membership::members::Trait
+    timestamp::Trait
+    + system::Trait
+    + data_object_type_registry::Trait
+    + membership::members::Trait
+    + bureaucracy::Trait<bureaucracy::Instance2>
 {
     type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
 
@@ -28,16 +36,23 @@ pub trait Trait:
         + MaybeSerialize
         + PartialEq;
 
-    type Roles: Roles<Self>;
+    type StorageProviderHelper: StorageProviderHelper<Self>;
     type IsActiveDataObjectType: data_object_type_registry::IsActiveDataObjectType<Self>;
+
+    /// Validates member id and origin combination.
+    type MemberOriginValidator: ActorOriginValidator<
+        Self::Origin,
+        MemberId<Self>,
+        Self::AccountId,
+    >;
 }
 
 static MSG_CID_NOT_FOUND: &str = "Content with this ID not found.";
 static MSG_LIAISON_REQUIRED: &str = "Only the liaison for the content may modify its status.";
-static MSG_CREATOR_MUST_BE_MEMBER: &str = "Only active members may create content.";
 static MSG_DO_TYPE_MUST_BE_ACTIVE: &str =
     "Cannot create content for inactive or missing data object type.";
 
+// TODO consider to remove it
 #[derive(Clone, Encode, Decode, PartialEq, Debug)]
 pub struct BlockAndTime<T: Trait> {
     pub block: T::BlockNumber,
@@ -57,59 +72,63 @@ impl Default for LiaisonJudgement {
     }
 }
 
+/// Manages content ids, type and storage provider decision about it.
 #[derive(Clone, Encode, Decode, PartialEq, Debug)]
 pub struct DataObject<T: Trait> {
-    pub owner: T::AccountId,
+    pub owner: MemberId<T>,
     pub added_at: BlockAndTime<T>,
     pub type_id: <T as data_object_type_registry::Trait>::DataObjectTypeId,
     pub size: u64,
-    pub liaison: T::AccountId,
+    pub liaison: StorageProviderId<T>,
     pub liaison_judgement: LiaisonJudgement,
     pub ipfs_content_id: Vec<u8>,
-    // TODO add support for this field (Some if judgment == Rejected)
-    // pub rejection_reason: Option<Vec<u8>>,
 }
 
 #[derive(Clone, Encode, Decode, PartialEq, Debug)]
 pub enum ContentVisibility {
-    Draft, // TODO rename to Unlisted?
+    Draft,
     Public,
 }
 
 impl Default for ContentVisibility {
     fn default() -> Self {
-        ContentVisibility::Draft // TODO make Public by default?
+        ContentVisibility::Draft
     }
 }
 
 decl_storage! {
     trait Store for Module<T: Trait> as DataDirectory {
-
-        // TODO default_liaison = Joystream storage account id.
-
-        // TODO this list of ids should be moved off-chain once we have Content Indexer.
-        // TODO deprecated, moved tp storage relationship
         pub KnownContentIds get(known_content_ids): Vec<T::ContentId> = vec![];
 
         pub DataObjectByContentId get(data_object_by_content_id):
             map T::ContentId => Option<DataObject<T>>;
-
-        // Default storage provider account id, overrides all active storage providers as liaison if set
-        pub PrimaryLiaisonAccountId get(primary_liaison_account_id): Option<T::AccountId>;
     }
 }
 
 decl_event! {
+    /// _Data directory_ events
     pub enum Event<T> where
         <T as Trait>::ContentId,
-        <T as system::Trait>::AccountId
+        MemberId = MemberId<T>,
+        StorageProviderId = StorageProviderId<T>
     {
-        // The account is the one who uploaded the content.
-        ContentAdded(ContentId, AccountId),
-
-        // The account is the liaison - only they can reject or accept
-        ContentAccepted(ContentId, AccountId),
-        ContentRejected(ContentId, AccountId),
+        /// Emits on adding of the content.
+        /// Params:
+        /// - Id of the relationship.
+        /// - Id of the member.
+        ContentAdded(ContentId, MemberId),
+
+        /// Emits when the storage provider accepts a content.
+        /// Params:
+        /// - Id of the relationship.
+        /// - Id of the storage provider.
+        ContentAccepted(ContentId, StorageProviderId),
+
+        /// Emits when the storage provider rejects a content.
+        /// Params:
+        /// - Id of the relationship.
+        /// - Id of the storage provider.
+        ContentRejected(ContentId, StorageProviderId),
     }
 }
 
@@ -117,64 +136,86 @@ decl_module! {
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
         fn deposit_event() = default;
 
-        // TODO send file_name as param so we could create a Draft metadata in this fn
         pub fn add_content(
             origin,
-            content_id: T::ContentId,
+            member_id: MemberId<T>,
+            content_id: T::ContentId, // TODO generate content_id by runtime
             type_id: <T as data_object_type_registry::Trait>::DataObjectTypeId,
             size: u64,
             ipfs_content_id: Vec<u8>
         ) {
-            let who = ensure_signed(origin)?;
-            ensure!(<membership::members::Module<T>>::is_member_account(&who), MSG_CREATOR_MUST_BE_MEMBER);
+            T::MemberOriginValidator::ensure_actor_origin(
+                origin,
+                member_id,
+            )?;
 
             ensure!(T::IsActiveDataObjectType::is_active_data_object_type(&type_id),
                 MSG_DO_TYPE_MUST_BE_ACTIVE);
 
             ensure!(!<DataObjectByContentId<T>>::exists(content_id),
-                "Data object aready added under this content id");
-
-            let liaison = match Self::primary_liaison_account_id() {
-                // Select primary liaison if set
-                Some(primary_liaison) => primary_liaison,
+                "Data object already added under this content id");
 
-                // Select liaison from staked roles if available
-                _ => T::Roles::random_account_for_role(actors::Role::StorageProvider)?
-            };
+            let liaison = T::StorageProviderHelper::get_random_storage_provider()?;
 
             // Let's create the entry then
             let data: DataObject<T> = DataObject {
                 type_id,
                 size,
                 added_at: Self::current_block_and_time(),
-                owner: who.clone(),
+                owner: member_id,
                 liaison,
                 liaison_judgement: LiaisonJudgement::Pending,
                 ipfs_content_id,
             };
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <DataObjectByContentId<T>>::insert(&content_id, data);
-            Self::deposit_event(RawEvent::ContentAdded(content_id, who));
+            Self::deposit_event(RawEvent::ContentAdded(content_id, member_id));
         }
 
-        // The LiaisonJudgement can be updated, but only by the liaison.
-        pub(crate) fn accept_content(origin, content_id: T::ContentId) {
-            let who = ensure_signed(origin)?;
-            Self::update_content_judgement(&who, content_id, LiaisonJudgement::Accepted)?;
+        /// Storage provider accepts a content. Requires signed storage provider account and its id.
+        /// The LiaisonJudgement can be updated, but only by the liaison.
+        pub(crate) fn accept_content(
+            origin,
+            storage_provider_id: StorageProviderId<T>,
+            content_id: T::ContentId
+        ) {
+            <StorageBureaucracy<T>>::ensure_worker_signed(origin, &storage_provider_id)?;
+
+            // == MUTATION SAFE ==
+
+            Self::update_content_judgement(&storage_provider_id, content_id, LiaisonJudgement::Accepted)?;
+
             <KnownContentIds<T>>::mutate(|ids| ids.push(content_id));
-            Self::deposit_event(RawEvent::ContentAccepted(content_id, who));
+
+            Self::deposit_event(RawEvent::ContentAccepted(content_id, storage_provider_id));
         }
 
-        pub(crate) fn reject_content(origin, content_id: T::ContentId) {
-            let who = ensure_signed(origin)?;
-            Self::update_content_judgement(&who, content_id, LiaisonJudgement::Rejected)?;
-            Self::deposit_event(RawEvent::ContentRejected(content_id, who));
+        /// Storage provider rejects a content. Requires signed storage provider account and its id.
+        /// The LiaisonJudgement can be updated, but only by the liaison.
+        pub(crate) fn reject_content(
+            origin,
+            storage_provider_id: StorageProviderId<T>,
+            content_id: T::ContentId
+        ) {
+            <StorageBureaucracy<T>>::ensure_worker_signed(origin, &storage_provider_id)?;
+
+            // == MUTATION SAFE ==
+
+            Self::update_content_judgement(&storage_provider_id, content_id, LiaisonJudgement::Rejected)?;
+            Self::deposit_event(RawEvent::ContentRejected(content_id, storage_provider_id));
         }
 
         // Sudo methods
 
         fn remove_known_content_id(origin, content_id: T::ContentId) {
             ensure_root(origin)?;
+
+            // == MUTATION SAFE ==
+
             let upd_content_ids: Vec<T::ContentId> = Self::known_content_ids()
                 .into_iter()
                 .filter(|&id| id != content_id)
@@ -184,6 +225,9 @@ decl_module! {
 
         fn set_known_content_id(origin, content_ids: Vec<T::ContentId>) {
             ensure_root(origin)?;
+
+            // == MUTATION SAFE ==
+
             <KnownContentIds<T>>::put(content_ids);
         }
     }
@@ -198,14 +242,14 @@ impl<T: Trait> Module<T> {
     }
 
     fn update_content_judgement(
-        who: &T::AccountId,
+        storage_provider_id: &StorageProviderId<T>,
         content_id: T::ContentId,
         judgement: LiaisonJudgement,
     ) -> dispatch::Result {
         let mut data = Self::data_object_by_content_id(&content_id).ok_or(MSG_CID_NOT_FOUND)?;
 
         // Make sure the liaison matches
-        ensure!(data.liaison == *who, MSG_LIAISON_REQUIRED);
+        ensure!(data.liaison == *storage_provider_id, MSG_LIAISON_REQUIRED);
 
         data.liaison_judgement = judgement;
         <DataObjectByContentId<T>>::insert(content_id, data);
@@ -214,10 +258,20 @@ impl<T: Trait> Module<T> {
     }
 }
 
+
+/// Provides random storage provider id. We use it when assign the content to the storage provider.
+pub trait StorageProviderHelper<T: Trait> {
+    /// Provides random storage provider id.
+    fn get_random_storage_provider() -> Result<StorageProviderId<T>, &'static str>;
+}
+
+/// Content access helper.
 pub trait ContentIdExists<T: Trait> {
-    fn has_content(_which: &T::ContentId) -> bool;
+    /// Verifies the content existence.
+    fn has_content(id: &T::ContentId) -> bool;
 
-    fn get_data_object(_which: &T::ContentId) -> Result<DataObject<T>, &'static str>;
+    /// Returns the data object for the provided content id.
+    fn get_data_object(id: &T::ContentId) -> Result<DataObject<T>, &'static str>;
 }
 
 impl<T: Trait> ContentIdExists<T> for Module<T> {

runtime-modules/storage/src/data_object_storage_registry.rs

@@ -29,13 +29,10 @@ use sr_primitives::traits::{MaybeSerialize, Member, SimpleArithmetic};
 use srml_support::{decl_error, decl_event, decl_module, decl_storage, ensure, Parameter};
 
 use crate::data_directory::{self, ContentIdExists};
-use crate::StorageBureaucracy;
+use crate::{StorageBureaucracy, StorageProviderId};
 
 const DEFAULT_FIRST_RELATIONSHIP_ID: u32 = 1;
 
-/// Storage provider is a worker from the bureuacracy module.
-pub type StorageProviderId<T> = bureaucracy::WorkerId<T>;
-
 /// The _Data object storage registry_ main _Trait_
 pub trait Trait:
     timestamp::Trait
@@ -175,6 +172,10 @@ decl_module! {
                 ready: false,
             };
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <Relationships<T>>::insert(new_id, dosr);
             <NextRelationshipId<T>>::mutate(|n| {
                 *n += T::DataObjectStorageRelationshipId::from(1);

runtime-modules/storage/src/data_object_type_registry.rs

@@ -86,10 +86,6 @@ pub struct DataObjectType {
 
     /// Active/Disabled flag.
     pub active: bool,
-    // TODO in future releases
-    // - maximum size
-    // - replication factor
-    // - storage tranches (empty is ok)
 }
 
 impl Default for DataObjectType {
@@ -103,9 +99,6 @@ impl Default for DataObjectType {
 
 decl_storage! {
     trait Store for Module<T: Trait> as DataObjectTypeRegistry {
-
-        // TODO hardcode data object type for ID 1
-
         /// Data object type ids should start at this value.
         pub FirstDataObjectTypeId get(first_data_object_type_id) config(first_data_object_type_id):
             T::DataObjectTypeId = T::DataObjectTypeId::from(DEFAULT_FIRST_DATA_OBJECT_TYPE_ID);
@@ -165,6 +158,10 @@ decl_module! {
                 active: data_object_type.active,
             };
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <DataObjectTypes<T>>::insert(new_do_type_id, do_type);
             <NextDataObjectTypeId<T>>::mutate(|n| { *n += T::DataObjectTypeId::from(1); });
 
@@ -180,6 +177,10 @@ decl_module! {
             do_type.description = data_object_type.description.clone();
             do_type.active = data_object_type.active;
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <DataObjectTypes<T>>::insert(id, do_type);
 
             Self::deposit_event(RawEvent::DataObjectTypeUpdated(id));
@@ -193,6 +194,10 @@ decl_module! {
 
             do_type.active = true;
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <DataObjectTypes<T>>::insert(id, do_type);
 
             Self::deposit_event(RawEvent::DataObjectTypeUpdated(id));
@@ -206,6 +211,10 @@ decl_module! {
 
             do_type.active = false;
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             <DataObjectTypes<T>>::insert(id, do_type);
 
             Self::deposit_event(RawEvent::DataObjectTypeUpdated(id));

runtime-modules/storage/src/lib.rs

@@ -9,3 +9,10 @@ mod tests;
 
 // Alias for storage working group bureaucracy
 pub(crate) type StorageBureaucracy<T> = bureaucracy::Module<T, bureaucracy::Instance2>;
+
+// Alias for the member id.
+pub(crate) type MemberId<T> = <T as membership::members::Trait>::MemberId;
+
+
+/// Storage provider is a worker from the bureaucracy module.
+pub type StorageProviderId<T> = bureaucracy::WorkerId<T>;

runtime/src/integration/mod.rs

@@ -1 +1,2 @@
 pub mod proposals;
+pub mod storage;

runtime/src/integration/storage.rs

@@ -0,0 +1,9 @@
+use crate::{Runtime, ActorId};
+
+pub struct StorageProviderHelper;
+
+impl storage::data_directory::StorageProviderHelper<Runtime> for StorageProviderHelper {
+	fn get_random_storage_provider() -> Result<ActorId, &'static str> {
+		unimplemented!()
+	}
+}

runtime/src/lib.rs

@@ -110,6 +110,10 @@ pub type ThreadId = u64;
 /// See the Note about ThreadId
 pub type PostId = u64;
 
+/// Represent an actor in membership group, which is the same in the working groups.
+pub type ActorId = u64;
+
+
 /// Opaque types. These are used by the CLI to instantiate machinery that don't need to know
 /// the specifics of the runtime. They can then be made to be agnostic over specific formats
 /// of data like extrinsics, allowing for them to continue syncing the network through upgrades
@@ -534,20 +538,6 @@ impl versioned_store_permissions::CreateClassPermissionsChecker<Runtime>
     }
 }
 
-// Impl this in the permissions module - can't be done here because
-// neither CreateClassPermissionsChecker or (X, Y) are local types?
-// impl<
-//         T: versioned_store_permissions::Trait,
-//         X: versioned_store_permissions::CreateClassPermissionsChecker<T>,
-//         Y: versioned_store_permissions::CreateClassPermissionsChecker<T>,
-//     > versioned_store_permissions::CreateClassPermissionsChecker<T> for (X, Y)
-// {
-//     fn account_can_create_class_permissions(account: &T::AccountId) -> bool {
-//         X::account_can_create_class_permissions(account)
-//             || Y::account_can_create_class_permissions(account)
-//     }
-// }
-
 pub struct ContentLeadOrSudoKeyCanCreateClasses {}
 impl versioned_store_permissions::CreateClassPermissionsChecker<Runtime>
     for ContentLeadOrSudoKeyCanCreateClasses
@@ -706,8 +696,9 @@ impl storage::data_directory::Trait for Runtime {
     type Event = Event;
     type ContentId = ContentId;
     type SchemaId = u64;
-    type Roles = LookupRoles;
+    type StorageProviderHelper = integration::storage::StorageProviderHelper;
     type IsActiveDataObjectType = DataObjectTypeRegistry;
+    type MemberOriginValidator = MembershipOriginValidator<Self>;
 }
 
 impl storage::data_object_storage_registry::Trait for Runtime {
@@ -762,7 +753,7 @@ impl members::Trait for Runtime {
     type MemberId = u64;
     type PaidTermId = u64;
     type SubscriptionId = u64;
-    type ActorId = u64;
+    type ActorId = ActorId;
     type InitialMembersBalance = InitialMembersBalance;
 }