Browse Source

content-directory: asset deletion

Mokhtar Naamani 4 years ago
parent
commit
19cc757026

+ 1 - 11
runtime-modules/common/src/storage.rs

@@ -24,13 +24,7 @@ pub enum StorageObjectOwner<MemberId, ChannelId, DAOId> {
     WorkingGroup(WorkingGroup), // acts through new extrinsic in working group
 }
 
-// To be implemented by current storage data_directory runtime module.
-// Defined in 'common' package
 pub trait StorageSystem<T: crate::StorageOwnership + crate::MembershipTypes> {
-    // Should hook into call on storage system,
-    // but requires rich error (with reasons)  types.
-    // caller already knows the `ContentId`s as they are part of
-    // the ContentUploadParameters
     fn atomically_add_content(
         owner: StorageObjectOwner<T::MemberId, T::ChannelId, T::DAOId>,
         content_parameters: Vec<ContentParameters<T::ContentId, T::DataObjectTypeId>>,
@@ -42,16 +36,12 @@ pub trait StorageSystem<T: crate::StorageOwnership + crate::MembershipTypes> {
         content_parameters: Vec<ContentParameters<T::ContentId, T::DataObjectTypeId>>,
     ) -> DispatchResult;
 
-    // Should hook into call on storage system,
-    // but requires rich error (with reasons)  types.
-    // caller already knows the `ContentId`s as they are part of
-    // the ContentUploadParameters
     fn atomically_remove_content(
         owner: &StorageObjectOwner<T::MemberId, T::ChannelId, T::DAOId>,
         content_ids: &[T::ContentId],
     ) -> DispatchResult;
 
-    // Checks if given owner can remove content under givencontent ids from the storage system
+    // Checks if given owner can remove content under given content ids from the storage system
     fn can_remove_content(
         owner: &StorageObjectOwner<T::MemberId, T::ChannelId, T::DAOId>,
         content_ids: &[T::ContentId],

+ 4 - 1
runtime-modules/content/src/errors.rs

@@ -55,9 +55,12 @@ decl_error! {
         /// A Channel or Video Category does not exist.
         CategoryDoesNotExist,
 
-        /// Channel coes not exist
+        /// Channel does not exist
         ChannelDoesNotExist,
 
+        /// Channel must have been deleted
+        ChannelMustNotExist,
+
         /// Curators can only censor non-curator group owned channels
         CannotCensoreCuratorGroupOwnedChannels
     }

+ 51 - 11
runtime-modules/content/src/lib.rs

@@ -803,17 +803,6 @@ decl_module! {
                 &channel.owner,
             )?;
 
-            // Important Note: Since channel ownership information is lost after a channel is deleted, if there are
-            // any lingering data objects in storage owned by the channel there is no way to authenticate an actor
-            // if they wish to delete those objects. So the risk of relying on user to delete data objects prior
-            // to deleting a channel is that there will be unrecoverable capacity on storage nodes. Quota/Limits is not
-            // a problem because the channel is removed. So storage lead would have to occasionally dispatch and extrinsic
-            // to cleanup storage data objects.. Or we can delete them all here.
-            // TOOD: Delete all data objects owned by the channel from storage.
-            // If we don't track the objects in content directory, how will storage iterate? IterableStorageMap?
-            // let object_owner = StorageObjectOwner::<T>::Channel(channel_id);
-            // T::StorageSystem::delete_objects_owned_by(object_owner);
-
             channel.videos.iter().for_each(|id| {
                 VideoById::<T>::remove(id);
                 Self::deposit_event(RawEvent::VideoDeleted(*id));
@@ -841,6 +830,57 @@ decl_module! {
             Self::deposit_event(RawEvent::ChannelDeleted(channel_id));
         }
 
+        /// Remove assets of a channel from storage
+        #[weight = 10_000_000] // TODO: adjust weight
+        pub fn remove_channel_assets(
+            origin,
+            actor: ContentActor<T::CuratorGroupId, T::CuratorId, T::MemberId>,
+            channel_id: T::ChannelId,
+            assets: Vec<ContentId<T>>,
+        ) {
+            // check that channel exists
+            let channel = Self::ensure_channel_exists(&channel_id)?;
+
+            ensure_actor_authorized_to_update_or_delete_channel::<T>(
+                origin,
+                &actor,
+                &channel.owner,
+            )?;
+
+            let object_owner = StorageObjectOwner::<T>::Channel(channel_id);
+
+            T::StorageSystem::atomically_remove_content(&object_owner, &assets)?;
+        }
+
+        // The content directory doesn't track individual content ids of assets uploaded for a channel.
+        // A channel owner can manage their storage usage with remove_channel_assets(). However when they choose
+        // to delete their channel they may leave behind some assets if not explicitly removed.
+        // So the 'lead' can and should occasionally dispatch this call to cleanup and recover storage capacity of
+        // deleted channels.
+        /// Lead utility to remove assets of a deleted channel from storage
+        #[weight = 10_000_000] // TODO: adjust weight
+        pub fn remove_deleted_channel_assets(
+            origin,
+            actor: ContentActor<T::CuratorGroupId, T::CuratorId, T::MemberId>,
+            channel_id: T::ChannelId,
+            assets: Vec<ContentId<T>>,
+        ) {
+            ensure_actor_authorized_to_delete_stale_assets::<T>(
+                origin,
+                &actor
+            )?;
+
+            // Ensure channel does not exist
+            ensure!(
+                !ChannelById::<T>::contains_key(channel_id),
+                Error::<T>::ChannelMustNotExist
+            );
+
+            let object_owner = StorageObjectOwner::<T>::Channel(channel_id);
+
+            T::StorageSystem::atomically_remove_content(&object_owner, &assets)?;
+        }
+
         #[weight = 10_000_000] // TODO: adjust weight
         pub fn censor_channel(
             origin,

+ 19 - 0
runtime-modules/content/src/permissions/mod.rs

@@ -12,6 +12,7 @@ use frame_support::{ensure, Parameter};
 pub use serde::{Deserialize, Serialize};
 use sp_arithmetic::traits::BaseArithmetic;
 use sp_runtime::traits::{MaybeSerializeDeserialize, Member};
+use system::ensure_root;
 
 /// Model of authentication manager.
 pub trait ContentActorAuthenticator: system::Trait + MembershipTypes {
@@ -272,6 +273,24 @@ pub fn ensure_actor_authorized_to_manage_categories<T: Trait>(
     }
 }
 
+pub fn ensure_actor_authorized_to_delete_stale_assets<T: Trait>(
+    origin: T::Origin,
+    actor: &ContentActor<T::CuratorGroupId, T::CuratorId, T::MemberId>,
+) -> DispatchResult {
+    // Only Lead and (sudo) can delete assets no longer associated with a channel or person.
+    match actor {
+        ContentActor::Lead => {
+            let sender = ensure_signed(origin)?;
+            ensure_lead_auth_success::<T>(&sender)?;
+            Ok(())
+        }
+        _ => {
+            ensure_root(origin)?;
+            Ok(())
+        }
+    }
+}
+
 /// Enum, representing all possible `Actor`s
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Eq, PartialEq, Clone, Copy, Debug)]

+ 15 - 1
runtime-modules/content/src/tests/mock.rs

@@ -165,7 +165,7 @@ impl ContentActorAuthenticator for Test {
 
 pub struct MockStorageSystem {}
 
-// Anyone can upload without restriction
+// Anyone can upload and delete without restriction
 impl StorageSystem<Test> for MockStorageSystem {
     fn atomically_add_content(
         _owner: StorageObjectOwner<Test>,
@@ -180,6 +180,20 @@ impl StorageSystem<Test> for MockStorageSystem {
     ) -> DispatchResult {
         Ok(())
     }
+
+    fn atomically_remove_content(
+        _owner: &StorageObjectOwner<Test>,
+        _content_ids: &[u64],
+    ) -> DispatchResult {
+        Ok(())
+    }
+
+    fn can_remove_content(
+        _owner: &StorageObjectOwner<Test>,
+        _content_ids: &[u64],
+    ) -> DispatchResult {
+        Ok(())
+    }
 }
 
 parameter_types! {