From eec3d103a55aab1370bbb344cf2abc4e763661c7 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 17 Feb 2023 00:51:45 +0200 Subject: [PATCH 1/5] Enable testing and benchmarking together --- pallets/roles/src/benchmarking.rs | 70 ++++++++++++++++++++----------- pallets/roles/src/lib.rs | 2 +- pallets/roles/src/mock.rs | 3 -- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/pallets/roles/src/benchmarking.rs b/pallets/roles/src/benchmarking.rs index f1032606..57c63dd2 100644 --- a/pallets/roles/src/benchmarking.rs +++ b/pallets/roles/src/benchmarking.rs @@ -4,19 +4,23 @@ // FIXME: refactor once SpacesInterface is added. -use super::*; use frame_benchmarking::{account, benchmarks}; use frame_support::dispatch::DispatchError; use frame_system::RawOrigin; -use pallet_permissions::SpacePermission as SP; -use pallet_spaces::types::Space; use sp_std::{prelude::Vec, vec}; -use subsocial_support::{Content, User}; -use subsocial_support::mock_functions::{valid_content_ipfs, another_valid_content_ipfs}; -fn create_dummy_space( +use pallet_permissions::SpacePermission as SP; +use subsocial_support::{ + mock_functions::{another_valid_content_ipfs, valid_content_ipfs}, + Content, User, +}; + +use super::*; + +#[cfg(not(test))] +fn get_dummy_space_id( origin: RawOrigin, -) -> Result, DispatchError> { +) -> Result { let space_id = pallet_spaces::NextSpaceId::::get(); pallet_spaces::Pallet::::create_space(origin.into(), Content::None, None)?; @@ -24,7 +28,25 @@ fn create_dummy_space( let space = pallet_spaces::SpaceById::::get(space_id) .ok_or(DispatchError::Other("Space not found"))?; - Ok(space) + Ok(space.id) +} + +#[cfg(test)] +fn get_dummy_space_id( + _origin: RawOrigin, +) -> Result { + Ok(crate::mock::SPACE1) +} + +#[cfg(not(test))] +fn get_caller_account() -> T::AccountId { + account::("Acc1", 1, 0) +} + +#[cfg(test)] +fn get_caller_account() -> T::AccountId { + let mut bytes: &[u8] = &crate::mock::ACCOUNT1.to_le_bytes(); + T::AccountId::decode(&mut bytes).expect("failed to get caller_account") } fn dummy_list_of_users(num_of_users: u32) -> Vec> { @@ -68,25 +90,25 @@ benchmarks! { where_clause { where T: pallet_spaces::Config } create_role { - let caller_origin = RawOrigin::Signed(account::("Acc1", 1, 0)); - let space = create_dummy_space::(caller_origin.clone())?; + let caller_origin = RawOrigin::Signed(get_caller_account::()); + let space_id = get_dummy_space_id::(caller_origin.clone())?; let time_to_live: Option = Some(100u32.into()); let content = valid_content_ipfs(); let perms = vec![SP::ManageRoles]; let role_id = NextRoleId::::get(); - }: _(caller_origin, space.id, time_to_live, content, perms) + }: _(caller_origin, space_id, time_to_live, content, perms) verify { let role = RoleById::::get(role_id).unwrap(); - let space_roles_ids = RoleIdsBySpaceId::::get(space.id); + let space_roles_ids = RoleIdsBySpaceId::::get(space_id); ensure!(role.id == role_id, "Role id doesn't match"); ensure!(space_roles_ids.contains(&role_id), "Role id not in space roles"); } update_role { - let caller_origin = RawOrigin::Signed(account::("Acc1", 1, 0)); - let space = create_dummy_space::(caller_origin.clone())?; - let (role, _) = create_dummy_role::(caller_origin.clone(), space.id, 10)?; + let caller_origin = RawOrigin::Signed(get_caller_account::()); + let space_id = get_dummy_space_id::(caller_origin.clone())?; + let (role, _) = create_dummy_role::(caller_origin.clone(), space_id, 10)?; ensure!(!role.disabled, "Role should be enabled"); @@ -103,9 +125,9 @@ benchmarks! { delete_role { let x in 0..T::MaxUsersToProcessPerDeleteRole::get().into(); - let caller_origin = RawOrigin::Signed(account::("Acc1", 1, 0)); - let space = create_dummy_space::(caller_origin.clone())?; - let (role, _) = create_dummy_role::(caller_origin.clone(), space.id, x)?; + let caller_origin = RawOrigin::Signed(get_caller_account::()); + let space_id = get_dummy_space_id::(caller_origin.clone())?; + let (role, _) = create_dummy_role::(caller_origin.clone(), space_id, x)?; }: _(caller_origin, role.id, x) verify { let deleted = RoleById::::get(role.id).is_none(); @@ -114,9 +136,9 @@ benchmarks! { grant_role { let x in 1..500; - let caller_origin = RawOrigin::Signed(account::("Acc1", 1, 0)); - let space = create_dummy_space::(caller_origin.clone())?; - let (role, _) = create_dummy_role::(caller_origin.clone(), space.id, 0)?; + let caller_origin = RawOrigin::Signed(get_caller_account::()); + let space_id = get_dummy_space_id::(caller_origin.clone())?; + let (role, _) = create_dummy_role::(caller_origin.clone(), space_id, 0)?; let users_to_grant = dummy_list_of_users::(x); }: _(caller_origin, role.id, users_to_grant.clone()) @@ -129,9 +151,9 @@ benchmarks! { revoke_role { let x in 1..500; - let caller_origin = RawOrigin::Signed(account::("Acc1", 1, 0)); - let space = create_dummy_space::(caller_origin.clone())?; - let (role, users_to_revoke) = create_dummy_role::(caller_origin.clone(), space.id, x)?; + let caller_origin = RawOrigin::Signed(get_caller_account::()); + let space_id = get_dummy_space_id::(caller_origin.clone())?; + let (role, users_to_revoke) = create_dummy_role::(caller_origin.clone(), space_id, x)?; }: _(caller_origin, role.id, users_to_revoke) verify { let granted_users = UsersByRoleId::::get(role.id); diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs index 1d96f7b6..0228dd06 100644 --- a/pallets/roles/src/lib.rs +++ b/pallets/roles/src/lib.rs @@ -38,7 +38,7 @@ mod mock; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; -#[cfg(all(test, not(feature = "runtime-benchmarks")))] +#[cfg(test)] mod tests; pub mod weights; diff --git a/pallets/roles/src/mock.rs b/pallets/roles/src/mock.rs index 0a8bc947..ebab49b2 100644 --- a/pallets/roles/src/mock.rs +++ b/pallets/roles/src/mock.rs @@ -116,9 +116,6 @@ parameter_types! { impl Config for Test { type Event = Event; type MaxUsersToProcessPerDeleteRole = MaxUsersToProcessPerDeleteRole; - #[cfg(feature = "runtime-benchmarks")] - type SpacePermissionsProvider = Spaces; - #[cfg(not(feature = "runtime-benchmarks"))] type SpacePermissionsProvider = Self; type SpaceFollows = Roles; type IsAccountBlocked = (); From a6b3919fa4a57416d07dc71ac0f72ed047c1c972 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 17 Feb 2023 08:59:46 +0200 Subject: [PATCH 2/5] Use cfg_if create to make it more readable --- Cargo.lock | 1 + pallets/roles/Cargo.toml | 1 + pallets/roles/src/benchmarking.rs | 41 ++++++++++++++++--------------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bb090e51..5b2698c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5898,6 +5898,7 @@ dependencies = [ name = "pallet-roles" version = "0.1.7" dependencies = [ + "cfg-if 1.0.0", "frame-benchmarking", "frame-support", "frame-system", diff --git a/pallets/roles/Cargo.toml b/pallets/roles/Cargo.toml index cd089cd1..92ed9ffb 100644 --- a/pallets/roles/Cargo.toml +++ b/pallets/roles/Cargo.toml @@ -34,6 +34,7 @@ try-runtime = ['frame-support/try-runtime'] [dependencies] codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } scale-info = { version = "2.1.2", default-features = false, features = ["derive"] } +cfg-if = "1.0.0" # Local dependencies pallet-permissions = { default-features = false, path = '../permissions' } diff --git a/pallets/roles/src/benchmarking.rs b/pallets/roles/src/benchmarking.rs index 57c63dd2..f6791d8b 100644 --- a/pallets/roles/src/benchmarking.rs +++ b/pallets/roles/src/benchmarking.rs @@ -4,6 +4,7 @@ // FIXME: refactor once SpacesInterface is added. +use cfg_if::cfg_if; use frame_benchmarking::{account, benchmarks}; use frame_support::dispatch::DispatchError; use frame_system::RawOrigin; @@ -17,37 +18,37 @@ use subsocial_support::{ use super::*; -#[cfg(not(test))] fn get_dummy_space_id( + #[allow(unused_variables)] origin: RawOrigin, ) -> Result { - let space_id = pallet_spaces::NextSpaceId::::get(); + cfg_if! { + if #[cfg(test)] { + Ok(crate::mock::SPACE1) + } else { + let space_id = pallet_spaces::NextSpaceId::::get(); - pallet_spaces::Pallet::::create_space(origin.into(), Content::None, None)?; + pallet_spaces::Pallet::::create_space(origin.into(), Content::None, None)?; - let space = pallet_spaces::SpaceById::::get(space_id) - .ok_or(DispatchError::Other("Space not found"))?; + let space = pallet_spaces::SpaceById::::get(space_id) + .ok_or(DispatchError::Other("Space not found"))?; - Ok(space.id) -} - -#[cfg(test)] -fn get_dummy_space_id( - _origin: RawOrigin, -) -> Result { - Ok(crate::mock::SPACE1) + Ok(space.id) + } + } } -#[cfg(not(test))] fn get_caller_account() -> T::AccountId { - account::("Acc1", 1, 0) + cfg_if! { + if #[cfg(test)] { + let mut bytes: &[u8] = &crate::mock::ACCOUNT1.to_le_bytes(); + T::AccountId::decode(&mut bytes).expect("failed to get caller_account") + } else { + account::("Acc1", 1, 0) + } + } } -#[cfg(test)] -fn get_caller_account() -> T::AccountId { - let mut bytes: &[u8] = &crate::mock::ACCOUNT1.to_le_bytes(); - T::AccountId::decode(&mut bytes).expect("failed to get caller_account") -} fn dummy_list_of_users(num_of_users: u32) -> Vec> { let mut users_to_grant = Vec::>::new(); From 450fbe366fdc81271f93843c199d97d5ca311c30 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 17 Feb 2023 09:01:26 +0200 Subject: [PATCH 3/5] formatting --- pallets/roles/src/benchmarking.rs | 4 +--- pallets/roles/src/lib.rs | 11 +++++++---- pallets/roles/src/mock.rs | 18 +++++++++--------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pallets/roles/src/benchmarking.rs b/pallets/roles/src/benchmarking.rs index f6791d8b..2a20a73e 100644 --- a/pallets/roles/src/benchmarking.rs +++ b/pallets/roles/src/benchmarking.rs @@ -19,8 +19,7 @@ use subsocial_support::{ use super::*; fn get_dummy_space_id( - #[allow(unused_variables)] - origin: RawOrigin, + #[allow(unused_variables)] origin: RawOrigin, ) -> Result { cfg_if! { if #[cfg(test)] { @@ -49,7 +48,6 @@ fn get_caller_account() -> T::AccountId { } } - fn dummy_list_of_users(num_of_users: u32) -> Vec> { let mut users_to_grant = Vec::>::new(); diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs index 0228dd06..5760932b 100644 --- a/pallets/roles/src/lib.rs +++ b/pallets/roles/src/lib.rs @@ -17,6 +17,7 @@ use scale_info::TypeInfo; use sp_runtime::RuntimeDebug; use sp_std::{collections::btree_set::BTreeSet, prelude::*}; +pub use pallet::*; use pallet_permissions::{ Pallet as Permissions, PermissionChecker, SpacePermission, SpacePermissionSet, }; @@ -25,12 +26,11 @@ use subsocial_support::{ traits::{IsAccountBlocked, IsContentBlocked, SpaceFollowsProvider, SpacePermissionsProvider}, Content, ModerationError, SpaceId, User, WhoAndWhenOf, }; +pub use types::*; -pub use pallet::*; pub mod functions; pub mod types; -pub use types::*; // pub mod rpc; #[cfg(test)] @@ -44,13 +44,16 @@ pub mod weights; #[frame_support::pallet] pub mod pallet { - use super::*; - use crate::weights::WeightInfo; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; + use pallet_permissions::SpacePermissionsInfoOf; use subsocial_support::{remove_from_vec, WhoAndWhen}; + use crate::weights::WeightInfo; + + use super::*; + #[pallet::config] pub trait Config: frame_system::Config + pallet_permissions::Config + pallet_timestamp::Config diff --git a/pallets/roles/src/mock.rs b/pallets/roles/src/mock.rs index ebab49b2..51b0638e 100644 --- a/pallets/roles/src/mock.rs +++ b/pallets/roles/src/mock.rs @@ -1,21 +1,21 @@ -use super::*; - -use sp_core::H256; -use sp_io::TestExternalities; -use sp_std::{collections::btree_set::BTreeSet, prelude::Vec}; - use frame_support::{ assert_ok, dispatch::{DispatchError, DispatchResult}, parameter_types, traits::{ConstU32, Everything}, }; +use sp_core::H256; +use sp_io::TestExternalities; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, }; +use sp_std::{collections::btree_set::BTreeSet, prelude::Vec}; -use pallet_permissions::{SpacePermission, SpacePermission as SP, SpacePermissions}; +use pallet_permissions::{ + default_permissions::DefaultSpacePermissions, SpacePermission, SpacePermission as SP, + SpacePermissions, +}; use subsocial_support::{ traits::{SpaceFollowsProvider, SpacePermissionsProvider as SpacePermissionsProviderT}, Content, SpaceId, SpacePermissionsInfo, User, @@ -23,6 +23,8 @@ use subsocial_support::{ use crate as roles; +use super::*; + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -103,8 +105,6 @@ impl pallet_balances::Config for Test { type ReserveIdentifier = (); } -use pallet_permissions::default_permissions::DefaultSpacePermissions; - impl pallet_permissions::Config for Test { type DefaultSpacePermissions = DefaultSpacePermissions; } From 684895d6c03f851535aaf61be2552e608a60e80c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 4 Jan 2024 15:57:51 +0000 Subject: [PATCH 4/5] Add missing license headers --- pallets/roles/src/mock.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pallets/roles/src/mock.rs b/pallets/roles/src/mock.rs index fc108d39..745a69a9 100644 --- a/pallets/roles/src/mock.rs +++ b/pallets/roles/src/mock.rs @@ -1,3 +1,9 @@ +// Copyright (C) DAPPFORCE PTE. LTD. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0. +// +// Full notice is available at https://github.com/dappforce/subsocial-parachain/blob/main/COPYRIGHT +// Full license is available at https://github.com/dappforce/subsocial-parachain/blob/main/LICENSE + use frame_support::{ assert_ok, dispatch::{DispatchError, DispatchResult}, From d3722fb6b67df4e1ae6eaeaf606b64eba47704ff Mon Sep 17 00:00:00 2001 From: Vlad Proshchavaiev <32250097+F3Joule@users.noreply.github.com> Date: Thu, 4 Jan 2024 18:15:07 +0200 Subject: [PATCH 5/5] Fix small warnings --- Cargo.lock | 2 +- pallets/roles/src/benchmarking.rs | 2 +- pallets/roles/src/lib.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30f06af9..5b1e6290 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6277,7 +6277,7 @@ dependencies = [ name = "pallet-roles" version = "0.2.0" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "frame-benchmarking", "frame-support", "frame-system", diff --git a/pallets/roles/src/benchmarking.rs b/pallets/roles/src/benchmarking.rs index 1a6290ec..76a6763e 100644 --- a/pallets/roles/src/benchmarking.rs +++ b/pallets/roles/src/benchmarking.rs @@ -12,7 +12,7 @@ use cfg_if::cfg_if; use frame_benchmarking::{account, benchmarks}; -use frame_support::dispatch::DispatchError; +use frame_support::{dispatch::DispatchError, traits::Get}; use frame_system::RawOrigin; use sp_std::{prelude::Vec, vec}; diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs index f939dc5f..ea054874 100644 --- a/pallets/roles/src/lib.rs +++ b/pallets/roles/src/lib.rs @@ -6,7 +6,7 @@ //! # Roles Module //! -//! This module allow you to create dynalic roles with an associated set of permissions +//! This module allow you to create dynamic roles with an associated set of permissions //! and grant them to users (accounts or space ids) within a given space. //! //! For example if you want to create a space that enables editors in a similar way to Medium, @@ -17,8 +17,8 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::{Decode, Encode}; -use frame_support::{dispatch::DispatchResult, ensure, traits::Get}; -use frame_system::{self as system, ensure_signed}; +use frame_support::{dispatch::DispatchResult, ensure}; +use frame_system::{self as system}; use scale_info::TypeInfo; use sp_runtime::RuntimeDebug; use sp_std::{collections::btree_set::BTreeSet, prelude::*};