From d9a51a834616e307e95227c7fe531cef68dd8475 Mon Sep 17 00:00:00 2001 From: iorveth Date: Sat, 25 Jul 2020 09:47:13 +0000 Subject: [PATCH 01/17] New uniqueness feature core logic implemented --- .../content-directory/src/entity.rs | 2 +- .../content-directory/src/errors.rs | 2 +- .../content-directory/src/helpers.rs | 22 ++ runtime-modules/content-directory/src/lib.rs | 299 +++++++++++++----- runtime-modules/content-directory/src/mock.rs | 1 + 5 files changed, 243 insertions(+), 83 deletions(-) diff --git a/runtime-modules/content-directory/src/entity.rs b/runtime-modules/content-directory/src/entity.rs index 15b5055aa8..cabcad3cce 100644 --- a/runtime-modules/content-directory/src/entity.rs +++ b/runtime-modules/content-directory/src/entity.rs @@ -53,7 +53,7 @@ impl Entity { } } - /// Get class id of this Entity + /// Get `class_id` of this `Entity` pub fn get_class_id(&self) -> T::ClassId { self.class_id } diff --git a/runtime-modules/content-directory/src/errors.rs b/runtime-modules/content-directory/src/errors.rs index 71275ddacb..6545b43f8b 100644 --- a/runtime-modules/content-directory/src/errors.rs +++ b/runtime-modules/content-directory/src/errors.rs @@ -123,4 +123,4 @@ pub const ERROR_CURATOR_GROUP_IS_NOT_ACTIVE: &str = "Curator group is not active pub const ERROR_ORIGIN_CANNOT_BE_MADE_INTO_RAW_ORIGIN: &str = "Origin cannot be made into raw origin"; pub const ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE: &str = - "Provided property value should be unique across all entity property values"; + "Property value should be unique across all Entities of this Class"; diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 3aa485f340..53eb9ab002 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -144,6 +144,28 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { } Ok(values_for_existing_properties) } + + /// Used to retrieve `OutputPropertyValue`s, which respective `Properties` have `unique` flag set + /// (skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required`) + pub fn compute_unique(&self) -> BTreeMap> { + self.iter() + .filter(|(property_id, _)| { + match self + .get(property_id) + .map(|value_for_property| value_for_property.unzip()) + { + Some((property, property_value)) if property.unique => { + // skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required` + property.required || *property_value != OutputPropertyValue::::default() + } + _ => false, + } + }) + .map(|(property_id, property_value)| { + (*property_id, property_value.get_value().to_owned()) + }) + .collect() + } } /// Length constraint for input validation diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index a283894cd2..cb788bf002 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -153,6 +153,9 @@ decl_storage! { /// Map, representing CuratorGroupId -> CuratorGroup relation pub CuratorGroupById get(curator_group_by_id) config(): map T::CuratorGroupId => CuratorGroup; + /// Used to enforce uniqueness of a property value across all Entities that have this property in a given Class. + pub UniquePropertyValues get(unique_property_values) config(): double_map hasher(blake2_128) (T::ClassId, PropertyId), blake2_128(OutputPropertyValue) => (); + /// Next runtime storage values used to maintain next id value, used on creation of respective curator groups, classes and entities pub NextClassId get(next_class_id) config(): T::ClassId; @@ -698,6 +701,8 @@ decl_module! { let class_properties = class.get_properties(); + let class_id = entity.get_class_id(); + let entity_property_values = entity.get_values(); // Create wrapper structure from provided entity_property_values and their corresponding Class properties @@ -731,14 +736,32 @@ decl_module! { // Ensure all provided `new_property_value_references_with_same_owner_flag_set` are valid Self::ensure_are_valid_references_with_same_owner_flag_set( - &new_values_for_existing_properties, &new_controller + new_values_for_existing_properties, &new_controller )?; + let new_output_property_value_references_with_same_owner_flag_set = Self::make_output_property_values(new_property_value_references_with_same_owner_flag_set); + + let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_value_references_with_same_owner_flag_set)?; + + // Retrieve OutputPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_unique_property_values = new_output_values_for_existing_properties.compute_unique(); + + // Ensure all provided Properties with unique flag set are unique on Class level + Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + + // + // == MUTATION SAFE == + // + + // Add property values, that should be unique on Class level + Self::add_unique_property_values(class_id, new_unique_property_values); + // Make updated entity_property_values from parameters provided let entity_property_values_updated = Self::make_updated_property_value_references_with_same_owner_flag_set( unused_property_id_references_with_same_owner_flag_set, &entity_property_values, - &new_property_value_references_with_same_owner_flag_set, + &new_output_property_value_references_with_same_owner_flag_set, ); // Transfer entity ownership @@ -748,13 +771,9 @@ decl_module! { // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::get_updated_inbound_rcs_delta( - class_properties, entity_property_values, new_property_value_references_with_same_owner_flag_set + class_properties, entity_property_values, new_output_property_value_references_with_same_owner_flag_set )?; - // - // == MUTATION SAFE == - // - // Update InboundReferenceCounter, based on previously calculated ReferenceCounterSideEffects, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -881,28 +900,39 @@ decl_module! { ) -> dispatch::Result { // Retrieve Entity and EntityAccessLevel for the actor, attemting to perform operation - let (_, entity, access_level) = Self::ensure_class_entity_and_access_level(origin, entity_id, &actor)?; - + let (class, entity, access_level) = Self::ensure_class_entity_and_access_level(origin, entity_id, &actor)?; + // Ensure actor with given EntityAccessLevel can remove entity EntityPermissions::::ensure_group_can_remove_entity(access_level)?; // Ensure any inbound InputPropertyValue::Reference points to the given Entity entity.ensure_rc_is_zero()?; + let class_properties = class.get_properties(); + + let class_id = entity.get_class_id(); + + let entity_values = entity.get_values(); + + let entity_unique_property_values = OutputValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique(); + // // == MUTATION SAFE == // + // Remove property value entries, that should be unique on Class level + Self::remove_unique_property_values(class_id, entity_unique_property_values); + // Remove entity >::remove(entity_id); // Decrement class entities counter - >::mutate(entity.get_class_id(), |class| class.decrement_entities_count()); + >::mutate(class_id, |class| class.decrement_entities_count()); let entity_controller = EntityController::::from_actor(&actor); // Decrement entity_creation_voucher after entity removal perfomed - >::mutate(entity.get_class_id(), entity_controller, |entity_creation_voucher| { + >::mutate(class_id, entity_controller, |entity_creation_voucher| { entity_creation_voucher.decrement_created_entities_count(); }); @@ -956,6 +986,8 @@ decl_module! { // against the type of its Property and check any additional constraints Self::ensure_property_values_are_valid(&entity_controller, &new_values_for_existing_properties)?; + let class_id = entity.get_class_id(); + let entity_property_values = entity.get_values(); let new_output_property_values = Self::make_output_property_values(new_property_values); @@ -965,11 +997,21 @@ decl_module! { schema, entity_property_values, &new_output_property_values ); + let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; + + // Retrieve OutputPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_unique_property_values = new_output_values_for_existing_properties.compute_unique(); + + // Ensure all provided Properties with unique flag set are unique on Class level + Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + // // == MUTATION SAFE == // - let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; + // Add property values, that should be unique on Class level + Self::add_unique_property_values(class_id, new_unique_property_values); // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::calculate_entities_inbound_rcs_delta( @@ -1033,30 +1075,42 @@ decl_module! { // against the type of its Property and check any additional constraints Self::ensure_property_values_are_valid(&entity_controller, &new_values_for_existing_properties)?; - // Get current property values of an entity, - // so we can update them if new values provided present in new_property_values. + let class_id = entity.get_class_id(); + + // Get current property values of an Entity let entity_property_values = entity.get_values(); - // Make updated entity_property_values from current entity_property_values and new_property_values provided + let new_output_property_values = Self::make_output_property_values(new_property_values); + + // Compute OutputPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_unique_property_values = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?.compute_unique(); + + // Ensure all provided Properties with unique flag set are unique on Class level + Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + + // + // == MUTATION SAFE == + // + + // Update property values, that should be unique on Class level + Self::add_unique_property_values(class_id, new_unique_property_values); + + // Make updated entity_property_values from current entity_property_values and new_output_property_values provided let entity_property_values_updated = - Self::make_updated_property_values(&entity_property_values, &new_property_values); + Self::make_updated_property_values(&entity_property_values, &new_output_property_values); // If property values should be updated if let Some(entity_property_values_updated) = entity_property_values_updated { - // Calculate entities reference counter side effects for current operation + // Calculate entities reference counter side effects for current operation (should always be safe) let entities_inbound_rcs_delta = - Self::get_updated_inbound_rcs_delta(class_properties, entity_property_values, new_property_values)?; - - // - // == MUTATION SAFE == - // + Self::get_updated_inbound_rcs_delta(class_properties, entity_property_values, new_output_property_values)?; // Update InboundReferenceCounter, based on previously calculated entities_inbound_rcs_delta, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); - // Update entity property values >::mutate(entity_id, |entity| { entity.set_values(entity_property_values_updated); @@ -1091,19 +1145,24 @@ decl_module! { let property_value_vector = entity.ensure_property_value_is_vec(in_class_schema_property_id)?; - // - // == MUTATION SAFE == - // + let class_id = entity.get_class_id(); // Calculate side effects for clear_property_vector operation, based on property_value_vector provided and its respective property. let entities_inbound_rcs_delta = Self::make_side_effects_for_clear_property_vector_operation(&property_value_vector, property); - // Decrease reference counters of involved entities (if some) - Self::update_entities_rcs(&entities_inbound_rcs_delta); - // Clear property_value_vector. let empty_property_value_vector = Self::clear_property_vector(property_value_vector); + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &empty_property_value_vector)?; + + // + // == MUTATION SAFE == + // + + // Decrease reference counters of involved entities (if some) + Self::update_entities_rcs(&entities_inbound_rcs_delta); + // Insert empty_property_value_vector into entity_property_values mapping at in_class_schema_property_id. // Retrieve updated entity_property_values let entity_values_updated = Self::insert_at_in_class_schema_property_id( @@ -1158,15 +1217,31 @@ decl_module! { property_value_vector .ensure_index_in_property_vector_is_valid(index_in_property_vector)?; - // - // == MUTATION SAFE == - // - let involved_entity_id = property_value_vector .get_vec_value() .get_involved_entities() .and_then(|involved_entities| involved_entities.get(index_in_property_vector as usize).copied()); + // Remove value at in_class_schema_property_id in property value vector + // Get VecInputPropertyValue wrapped in InputPropertyValue + let property_value_vector_updated = Self::remove_at_index_in_property_vector( + property_value_vector, index_in_property_vector + ); + + let class_id = entity.get_class_id(); + + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated)?; + + // + // == MUTATION SAFE == + // + + // Insert updated propery value into entity_property_values mapping at in_class_schema_property_id. + let entity_values_updated = Self::insert_at_in_class_schema_property_id( + entity.get_values(), in_class_schema_property_id, property_value_vector_updated + ); + let involved_entity_and_side_effect = if let Some(involved_entity_id) = involved_entity_id { // Decrease reference counter of involved entity (if some) let same_controller_status = property.property_type.same_controller_status(); @@ -1179,16 +1254,7 @@ decl_module! { None }; - // Remove value at in_class_schema_property_id in property value vector - // Get VecInputPropertyValue wrapped in InputPropertyValue - let property_value_vector_updated = Self::remove_at_index_in_property_vector( - property_value_vector, index_in_property_vector - ); - - // Insert updated propery value into entity_property_values mapping at in_class_schema_property_id. - let entity_values_updated = Self::insert_at_in_class_schema_property_id( - entity.get_values(), in_class_schema_property_id, property_value_vector_updated - ); + // Update entity property values >::mutate(entity_id, |entity| { @@ -1246,12 +1312,31 @@ decl_module! { entity_controller, )?; + let involved_entity = value.get_involved_entity(); + + // Insert SingleInputPropertyValue at in_class_schema_property_id into property value vector + // Get VecInputPropertyValue wrapped in InputPropertyValue + let property_value_vector_updated = Self::insert_at_index_in_property_vector( + property_value_vector, index_in_property_vector, value + ); + + let class_id = entity.get_class_id(); + + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated)?; + // // == MUTATION SAFE == // + // Insert updated property value into entity_property_values mapping at in_class_schema_property_id. + // Retrieve updated entity_property_values + let entity_values_updated = Self::insert_at_in_class_schema_property_id( + entity.get_values(), in_class_schema_property_id, property_value_vector_updated + ); + // Increase reference counter of involved entity (if some) - let involved_entity_and_side_effect = if let Some(entity_rc_to_increment) = value.get_involved_entity() { + let involved_entity_and_side_effect = if let Some(entity_rc_to_increment) = involved_entity { let same_controller_status = class_property.property_type.same_controller_status(); let rc_delta = EntityReferenceCounterSideEffect::atomic(same_controller_status, DeltaMode::Increment); @@ -1262,18 +1347,6 @@ decl_module! { None }; - // Insert SingleInputPropertyValue at in_class_schema_property_id into property value vector - // Get VecInputPropertyValue wrapped in InputPropertyValue - let property_value_vector_updated = Self::insert_at_index_in_property_vector( - property_value_vector, index_in_property_vector, value - ); - - // Insert updated property value into entity_property_values mapping at in_class_schema_property_id. - // Retrieve updated entity_property_values - let entity_values_updated = Self::insert_at_in_class_schema_property_id( - entity.get_values(), in_class_schema_property_id, property_value_vector_updated - ); - // Update entity property values >::mutate(entity_id, |entity| { entity.set_values(entity_values_updated); @@ -1347,15 +1420,6 @@ decl_module! { } impl Module { - pub fn make_output_property_values( - input_property_values: BTreeMap>, - ) -> BTreeMap> { - input_property_values - .into_iter() - .map(|(property_id, property_value)| (property_id, property_value.into())) - .collect() - } - /// Updates corresponding `Entity` `reference_counter` by `reference_counter_delta`. fn update_entity_rc( entity_id: T::EntityId, @@ -1371,10 +1435,56 @@ impl Module { }) } - /// Returns the stored `Class` if exist, error otherwise. - fn ensure_class_exists(class_id: T::ClassId) -> Result, &'static str> { - ensure!(>::exists(class_id), ERROR_CLASS_NOT_FOUND); - Ok(Self::class_by_id(class_id)) + /// Add/update property value, that should be unique on `Class` level + pub fn add_unique_property_value( + class_id: T::ClassId, + property_id: PropertyId, + property_value: OutputPropertyValue, + ) { + >::insert((class_id, property_id), property_value, ()); + } + + /// Remove property value, that should be unique on `Class` level + pub fn remove_unique_property_value( + class_id: T::ClassId, + property_id: PropertyId, + property_value: OutputPropertyValue, + ) { + >::remove((class_id, property_id), property_value); + } + + /// Add property values, that should be unique on `Class` level + pub fn add_unique_property_values( + class_id: T::ClassId, + new_output_values_for_existing_properties: BTreeMap>, + ) { + new_output_values_for_existing_properties + .into_iter() + .for_each(|(property_id, property_value)| { + Self::add_unique_property_value(class_id, property_id, property_value); + }); + } + + /// Remove property values, that should be unique on `Class` level + pub fn remove_unique_property_values( + class_id: T::ClassId, + new_output_values_for_existing_properties: BTreeMap>, + ) { + new_output_values_for_existing_properties + .into_iter() + .for_each(|(property_id, property_value)| { + Self::remove_unique_property_value(class_id, property_id, property_value); + }); + } + + /// Convert all provided `InputPropertyValue`'s into `OutputPropertyValue`'s + pub fn make_output_property_values( + input_property_values: BTreeMap>, + ) -> BTreeMap> { + input_property_values + .into_iter() + .map(|(property_id, property_value)| (property_id, property_value.into())) + .collect() } /// Update `entity_property_values` with `property_values` @@ -1508,13 +1618,13 @@ impl Module { pub fn get_updated_inbound_rcs_delta( class_properties: Vec>, entity_property_values: BTreeMap>, - new_property_values: BTreeMap>, + new_output_property_values: BTreeMap>, ) -> Result>, &'static str> { // Filter entity_property_values to get only those, which will be substituted with new_property_values let entity_property_values_to_update: BTreeMap> = entity_property_values .into_iter() - .filter(|(entity_id, _)| new_property_values.contains_key(entity_id)) + .filter(|(entity_id, _)| new_output_property_values.contains_key(entity_id)) .collect(); // Calculate entities reference counter side effects for update operation @@ -1529,14 +1639,12 @@ impl Module { DeltaMode::Decrement, ); - let new_property_values_output = Self::make_output_property_values(new_property_values); - // Calculate entities inbound reference counter delta with Increment DeltaMode for new_property_values, // as involved InputPropertyValue References will substitute the old ones let incremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( OutputValuesForExistingProperties::from( &class_properties, - &new_property_values_output, + &new_output_property_values, )?, DeltaMode::Increment, ); @@ -1643,6 +1751,36 @@ impl Module { } } + /// Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + pub fn ensure_property_unique_option_satisfied( + class_id: T::ClassId, + property_id: PropertyId, + property_value: &OutputPropertyValue, + ) -> Result<(), &'static str> { + ensure!( + !>::exists((class_id, property_id), property_value), + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE + ); + Ok(()) + } + + /// Ensure all provided `Properties` with `unique` flag set are `unique` on `Class` level + pub fn ensure_properties_unique_option_satisfied( + class_id: T::ClassId, + unique_new_property_values: &BTreeMap>, + ) -> Result<(), &'static str> { + for (&property_id, property_value) in unique_new_property_values { + Self::ensure_property_unique_option_satisfied(class_id, property_id, property_value)?; + } + Ok(()) + } + + /// Returns the stored `Class` if exist, error otherwise. + fn ensure_class_exists(class_id: T::ClassId) -> Result, &'static str> { + ensure!(>::exists(class_id), ERROR_CLASS_NOT_FOUND); + Ok(Self::class_by_id(class_id)) + } + /// Returns `Class` and `Entity` under given id, if exists, and `EntityAccessLevel` corresponding to `origin`, if permitted fn ensure_class_entity_and_access_level( origin: T::Origin, @@ -1729,7 +1867,7 @@ impl Module { /// Ensure all provided `new_property_value_references_with_same_owner_flag_set` are valid fn ensure_are_valid_references_with_same_owner_flag_set( - new_property_value_references_with_same_owner_flag_set: &InputValuesForExistingProperties< + new_property_value_references_with_same_owner_flag_set: InputValuesForExistingProperties< T, >, new_controller: &EntityController, @@ -1752,7 +1890,7 @@ impl Module { entity_property_values: &BTreeMap>, new_property_value_references_with_same_owner_flag_set: &BTreeMap< PropertyId, - InputPropertyValue, + OutputPropertyValue, >, ) -> Option>> { // Used to check if update performed @@ -1766,7 +1904,6 @@ impl Module { *property_id, new_property_value_reference_with_same_owner_flag_set .to_owned() - .into(), ); } @@ -1894,16 +2031,16 @@ impl Module { /// if update performed, returns updated entity property values pub fn make_updated_property_values( entity_property_values: &BTreeMap>, - new_property_values: &BTreeMap>, + new_output_property_values: &BTreeMap>, ) -> Option>> { // Used to check if updated performed let mut entity_property_values_updated = entity_property_values.to_owned(); - new_property_values - .iter() + new_output_property_values + .into_iter() .for_each(|(id, new_property_value)| { if let Some(entity_property_value) = entity_property_values_updated.get_mut(&id) { - entity_property_value.update(new_property_value.to_owned().into()); + entity_property_value.update(new_property_value.to_owned()); } }); diff --git a/runtime-modules/content-directory/src/mock.rs b/runtime-modules/content-directory/src/mock.rs index 05dfcc1128..6d8a330270 100644 --- a/runtime-modules/content-directory/src/mock.rs +++ b/runtime-modules/content-directory/src/mock.rs @@ -402,6 +402,7 @@ fn default_content_directory_genesis_config() -> GenesisConfig { class_by_id: vec![], entity_by_id: vec![], curator_group_by_id: vec![], + unique_property_values: vec![], next_class_id: 1, next_entity_id: 1, next_curator_group_id: 1, From 88a6660d454839b99f49843cf5d5c4258d5c0368 Mon Sep 17 00:00:00 2001 From: iorveth Date: Sat, 25 Jul 2020 16:13:37 +0000 Subject: [PATCH 02/17] cargo fmt --- runtime-modules/content-directory/src/lib.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index cb788bf002..ccb1e78bd8 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -702,7 +702,7 @@ decl_module! { let class_properties = class.get_properties(); let class_id = entity.get_class_id(); - + let entity_property_values = entity.get_values(); // Create wrapper structure from provided entity_property_values and their corresponding Class properties @@ -901,7 +901,7 @@ decl_module! { // Retrieve Entity and EntityAccessLevel for the actor, attemting to perform operation let (class, entity, access_level) = Self::ensure_class_entity_and_access_level(origin, entity_id, &actor)?; - + // Ensure actor with given EntityAccessLevel can remove entity EntityPermissions::::ensure_group_can_remove_entity(access_level)?; @@ -1254,7 +1254,7 @@ decl_module! { None }; - + // Update entity property values >::mutate(entity_id, |entity| { @@ -1867,9 +1867,7 @@ impl Module { /// Ensure all provided `new_property_value_references_with_same_owner_flag_set` are valid fn ensure_are_valid_references_with_same_owner_flag_set( - new_property_value_references_with_same_owner_flag_set: InputValuesForExistingProperties< - T, - >, + new_property_value_references_with_same_owner_flag_set: InputValuesForExistingProperties, new_controller: &EntityController, ) -> dispatch::Result { for updated_value_for_existing_property in @@ -1902,8 +1900,7 @@ impl Module { // Update entity_property_values map at property_id with new_property_value_reference_with_same_owner_flag_set entity_property_values_updated.insert( *property_id, - new_property_value_reference_with_same_owner_flag_set - .to_owned() + new_property_value_reference_with_same_owner_flag_set.to_owned(), ); } From 84d77828b4b39498437b3df9bd2f11b6c0a302aa Mon Sep 17 00:00:00 2001 From: iorveth Date: Sun, 26 Jul 2020 16:28:47 +0000 Subject: [PATCH 03/17] Update existing uniqueness test coverage, fix inbound reference counting logic --- runtime-modules/content-directory/src/lib.rs | 37 ++- .../content-directory/src/tests.rs | 11 +- .../src/tests/add_schema_support_to_entity.rs | 162 +++++------ .../src/tests/clear_entity_property_vector.rs | 18 +- .../tests/insert_at_entity_property_vector.rs | 30 +- .../tests/remove_at_entity_property_vector.rs | 22 +- .../src/tests/transfer_entity_ownership.rs | 275 +++++++++--------- .../tests/update_entity_property_values.rs | 175 +++++------ 8 files changed, 355 insertions(+), 375 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index ccb1e78bd8..abe015df7e 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -771,7 +771,7 @@ decl_module! { // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::get_updated_inbound_rcs_delta( - class_properties, entity_property_values, new_output_property_value_references_with_same_owner_flag_set + entity_id, class_properties, entity_property_values, new_output_property_value_references_with_same_owner_flag_set )?; // Update InboundReferenceCounter, based on previously calculated ReferenceCounterSideEffects, for each Entity involved @@ -1015,7 +1015,7 @@ decl_module! { // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::calculate_entities_inbound_rcs_delta( - new_output_values_for_existing_properties, DeltaMode::Increment + entity_id, new_output_values_for_existing_properties, DeltaMode::Increment ); // Update InboundReferenceCounter, based on previously calculated entities_inbound_rcs_delta, for each Entity involved @@ -1106,7 +1106,7 @@ decl_module! { // Calculate entities reference counter side effects for current operation (should always be safe) let entities_inbound_rcs_delta = - Self::get_updated_inbound_rcs_delta(class_properties, entity_property_values, new_output_property_values)?; + Self::get_updated_inbound_rcs_delta(entity_id, class_properties, entity_property_values, new_output_property_values)?; // Update InboundReferenceCounter, based on previously calculated entities_inbound_rcs_delta, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -1576,9 +1576,21 @@ impl Module { inbound_rcs_delta } + /// Filter references, pointing to the same `Entity` + fn filter_references_to_the_same_entity( + current_entity_id: T::EntityId, + involved_entity_ids: Vec, + ) -> Vec { + involved_entity_ids + .into_iter() + .filter(|involved_entity_id| current_entity_id != *involved_entity_id) + .collect() + } + /// Calculate `ReferenceCounterSideEffects`, based on `values_for_existing_properties` provided and chosen `DeltaMode` /// Returns calculated `ReferenceCounterSideEffects` fn calculate_entities_inbound_rcs_delta( + current_entity_id: T::EntityId, values_for_existing_properties: OutputValuesForExistingProperties, delta_mode: DeltaMode, ) -> Option> { @@ -1586,12 +1598,20 @@ impl Module { .values() .map(|value_for_existing_property| value_for_existing_property.unzip()) .filter_map(|(property, value)| { - value.get_involved_entities().map(|involved_entity_ids| { - ( + let involved_entity_ids = + value.get_involved_entities().map(|involved_entity_ids| { + Self::filter_references_to_the_same_entity( + current_entity_id, + involved_entity_ids, + ) + }); + match involved_entity_ids { + Some(involved_entity_ids) if !involved_entity_ids.is_empty() => Some(( involved_entity_ids, property.property_type.same_controller_status(), - ) - }) + )), + _ => None, + } }) // Aggeregate all sideffects on a single entity together into one side effect map .fold( @@ -1616,6 +1636,7 @@ impl Module { /// Compute `ReferenceCounterSideEffects`, based on `InputPropertyValue` `Reference`'s involved into update process. /// Returns updated `ReferenceCounterSideEffects` pub fn get_updated_inbound_rcs_delta( + current_entity_id: T::EntityId, class_properties: Vec>, entity_property_values: BTreeMap>, new_output_property_values: BTreeMap>, @@ -1632,6 +1653,7 @@ impl Module { // Calculate entities inbound reference counter delta with Decrement DeltaMode for entity_property_values_to_update, // as involved InputPropertyValue References will be substituted with new ones let decremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( + current_entity_id, OutputValuesForExistingProperties::from( &class_properties, &entity_property_values_to_update, @@ -1642,6 +1664,7 @@ impl Module { // Calculate entities inbound reference counter delta with Increment DeltaMode for new_property_values, // as involved InputPropertyValue References will substitute the old ones let incremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( + current_entity_id, OutputValuesForExistingProperties::from( &class_properties, &new_output_property_values, diff --git a/runtime-modules/content-directory/src/tests.rs b/runtime-modules/content-directory/src/tests.rs index 083fc19dac..9e44347662 100644 --- a/runtime-modules/content-directory/src/tests.rs +++ b/runtime-modules/content-directory/src/tests.rs @@ -301,7 +301,7 @@ pub fn emulate_entity_access_state_for_failure_case( } } -pub fn add_class_reference_schema() { +pub fn add_unique_class_reference_schema() { // Create property let property_type = PropertyType::::vec_reference(FIRST_CLASS_ID, true, VecMaxLengthConstraint::get()); @@ -310,7 +310,7 @@ pub fn add_class_reference_schema() { (PropertyNameLengthConstraint::get().max() - 1) as usize, property_type, true, - false, + true, ); // Add Schema to the Class @@ -322,8 +322,11 @@ pub fn add_class_reference_schema() { )); } -pub fn add_class_reference_schema_and_entity_schema_support(actor: &Actor, origin: u64) { - add_class_reference_schema(); +pub fn add_unique_class_reference_schema_and_entity_schema_support( + actor: &Actor, + origin: u64, +) { + add_unique_class_reference_schema(); let schema_property_value = InputPropertyValue::::vec_reference(vec![FIRST_ENTITY_ID, FIRST_ENTITY_ID]); diff --git a/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs b/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs index 5839835cfe..1194697201 100644 --- a/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs +++ b/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs @@ -1292,93 +1292,75 @@ fn add_schema_support_vec_property_is_too_long() { }) } -// #[test] -// fn add_schema_support_property_should_be_unique() { -// with_test_externalities(|| { -// // Create class with default permissions -// assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - -// let actor = Actor::Lead; - -// // Create entity -// assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); - -// let property_type = PropertyType::::single_text(TextMaxLengthConstraint::get()); - -// // Create first text property - -// let first_property = Property::::with_name_and_type( -// PropertyNameLengthConstraint::get().max() as usize, -// property_type.clone(), -// true, -// true, -// ); - -// // Create second text property - -// let second_property = Property::::with_name_and_type( -// PropertyNameLengthConstraint::get().max() as usize - 1, -// property_type, -// true, -// true, -// ); - -// // Add first Schema to the Class -// assert_ok!(add_class_schema( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// BTreeSet::new(), -// vec![first_property] -// )); - -// // Add second Schema to the Class -// assert_ok!(add_class_schema( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// BTreeSet::new(), -// vec![second_property] -// )); - -// let mut first_schema_property_values = BTreeMap::new(); - -// let schema_property_value = -// InputPropertyValue::::single_text(TextMaxLengthConstraint::get()); - -// first_schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value.clone()); - -// // Add Entity Schema support -// assert_ok!(add_schema_support_to_entity( -// LEAD_ORIGIN, -// actor.to_owned(), -// FIRST_ENTITY_ID, -// FIRST_SCHEMA_ID, -// first_schema_property_values, -// )); - -// // Runtime state before tested call - -// // Events number before tested call -// let number_of_events_before_call = System::events().len(); - -// let mut second_schema_property_values = BTreeMap::new(); - -// second_schema_property_values.insert(SECOND_PROPERTY_ID, schema_property_value); - -// // Make an attempt to add schema support to the entity, providing property value(s), which have duplicates or identical to thouse, -// // are already added to The Entity, though should be unique on Class Property level -// let add_schema_support_to_entity_result = add_schema_support_to_entity( -// LEAD_ORIGIN, -// actor, -// FIRST_ENTITY_ID, -// SECOND_SCHEMA_ID, -// second_schema_property_values, -// ); - -// // Failure checked -// assert_failure( -// add_schema_support_to_entity_result, -// ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, -// number_of_events_before_call, -// ); -// }) -// } +#[test] +fn add_schema_support_property_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); + + // Create second entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); + + let property_type = PropertyType::::single_text(TextMaxLengthConstraint::get()); + + // Create text property + + let property = Property::::with_name_and_type( + PropertyNameLengthConstraint::get().max() as usize, + property_type, + true, + true, + ); + + // Add Schema to the Class + assert_ok!(add_class_schema( + LEAD_ORIGIN, + FIRST_CLASS_ID, + BTreeSet::new(), + vec![property] + )); + + let mut schema_property_values = BTreeMap::new(); + + let schema_property_value = + InputPropertyValue::::single_text(TextMaxLengthConstraint::get()); + + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + // Add Entity Schema support to the first Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + FIRST_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values.clone(), + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + // Make an attempt to add schema support to the Entity, providing property values, which respective Class properties have + // unique flag set and same property values under same property_ids were already added to any Entity of this Class + let add_schema_support_to_entity_result = add_schema_support_to_entity( + LEAD_ORIGIN, + actor, + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values, + ); + + // Failure checked + assert_failure( + add_schema_support_to_entity_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} diff --git a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs index 3e65afbb53..ee8a69d9ee 100644 --- a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs @@ -68,7 +68,7 @@ fn clear_entity_property_vector_entity_not_found() { ); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); // Runtime state before tested call @@ -97,7 +97,7 @@ fn clear_entity_property_vector_lead_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -130,7 +130,7 @@ fn clear_entity_property_vector_member_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_MEMBER_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_MEMBER_ORIGIN); // Runtime state before tested call @@ -163,7 +163,7 @@ fn clear_entity_property_vector_curator_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); // Runtime state before tested call @@ -192,7 +192,7 @@ fn clear_entity_property_vector_curator_group_is_not_active() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); // Make curator group inactive to block it from any entity operations assert_ok!(set_curator_group_status( @@ -232,7 +232,7 @@ fn clear_entity_property_vector_curator_not_found_in_curator_group() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -268,7 +268,7 @@ fn clear_entity_property_vector_entity_access_denied() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -301,7 +301,7 @@ fn clear_entity_property_vector_values_locked_on_class_level() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -424,7 +424,7 @@ fn clear_entity_property_vector_unknown_entity_property_id() { assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let actor = Actor::Lead; diff --git a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs index 887fec1aa9..501a8b6205 100644 --- a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs @@ -79,7 +79,7 @@ fn insert_at_entity_property_vector_entity_not_found() { ); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); // Runtime state before tested call @@ -119,7 +119,7 @@ fn insert_at_entity_property_vector_lead_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -159,7 +159,7 @@ fn insert_at_entity_property_vector_member_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -199,7 +199,7 @@ fn insert_at_entity_property_vector_curator_group_is_not_active() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); // Make curator group inactive to block it from any entity operations assert_ok!(set_curator_group_status( @@ -246,7 +246,7 @@ fn insert_at_entity_property_vector_curator_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -289,7 +289,7 @@ fn insert_at_entity_property_vector_curator_not_found_in_curator_group() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -333,7 +333,7 @@ fn insert_at_entity_property_vector_access_denied() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -374,7 +374,7 @@ fn insert_at_entity_property_vector_values_locked_on_class_level() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -534,7 +534,7 @@ fn insert_at_entity_property_vector_unknown_entity_property_id() { assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let actor = Actor::Lead; @@ -651,7 +651,7 @@ fn insert_at_entity_property_vector_nonces_does_not_match() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -696,7 +696,7 @@ fn insert_at_entity_property_vector_index_is_out_of_range() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let entity_ids = vec![FIRST_ENTITY_ID, FIRST_ENTITY_ID]; let schema_property_value = @@ -757,7 +757,7 @@ fn insert_at_entity_property_vector_is_too_long() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let entity_ids = vec![FIRST_ENTITY_ID; VecMaxLengthConstraint::get() as usize]; let schema_property_value = @@ -1054,7 +1054,7 @@ fn insert_at_entity_property_vector_referenced_entity_not_found() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -1099,7 +1099,7 @@ fn insert_at_entity_property_vector_entity_can_not_be_referenced() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the first Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Create second Entity of first Class assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); @@ -1165,7 +1165,7 @@ fn insert_at_entity_property_vector_same_controller_constraint_violation() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the first Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Create second Entity assert_ok!(create_entity( diff --git a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs index e3bf93b746..414b985a8a 100644 --- a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs @@ -74,7 +74,7 @@ fn remove_at_entity_property_vector_entity_not_found() { ); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); // Runtime state before tested call @@ -112,7 +112,7 @@ fn remove_at_entity_property_vector_lead_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -150,7 +150,7 @@ fn remove_at_entity_property_vector_member_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -188,7 +188,7 @@ fn remove_at_entity_property_vector_curator_group_is_not_active() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); // Make curator group inactive to block it from any entity operations assert_ok!(set_curator_group_status( @@ -233,7 +233,7 @@ fn remove_at_entity_property_vector_curator_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, FIRST_CURATOR_ORIGIN); // Runtime state before tested call @@ -271,7 +271,7 @@ fn remove_at_entity_property_vector_curator_not_found_in_curator_group() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -313,7 +313,7 @@ fn remove_at_entity_property_vector_access_denied() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -352,7 +352,7 @@ fn remove_at_entity_property_vector_values_locked_on_class_level() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -506,7 +506,7 @@ fn remove_at_entity_property_vector_unknown_entity_property_id() { assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let actor = Actor::Lead; @@ -619,7 +619,7 @@ fn remove_at_entity_property_vector_nonces_does_not_match() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -662,7 +662,7 @@ fn remove_at_entity_property_vector_index_is_out_of_range() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); let entity_ids = vec![FIRST_ENTITY_ID, FIRST_ENTITY_ID]; let schema_property_value = diff --git a/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs b/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs index 3e6d360b86..06d73dc113 100644 --- a/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs +++ b/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs @@ -167,7 +167,7 @@ fn transfer_entity_ownership_inbound_same_owner_rc_does_not_equal_to_zero() { // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); let actor = Actor::Lead; @@ -225,7 +225,7 @@ fn transfer_entity_ownership_provided_property_value_ids_must_be_references_with // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); // Update class permissions to force any member be available to create Entities assert_ok!(update_class_permissions( @@ -308,7 +308,7 @@ fn transfer_entity_ownership_provided_new_property_value_referencing_entity_that // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); // Update class permissions to force any member be available to create Entities assert_ok!(update_class_permissions( @@ -398,7 +398,7 @@ fn transfer_entity_ownership_provided_new_property_value_referencing_non_existen // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); let actor = Actor::Lead; @@ -464,7 +464,7 @@ fn transfer_entity_ownership_provided_new_property_value_referencing_entity_cont // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); // Update class permissions to force any member be available to create Entities assert_ok!(update_class_permissions( @@ -542,7 +542,7 @@ fn transfer_entity_ownership_required_property_was_not_provided() { // Create class with default permissions assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - add_class_reference_schema(); + add_unique_class_reference_schema(); // Update class permissions to force any member be available to create Entities assert_ok!(update_class_permissions( @@ -609,139 +609,130 @@ fn transfer_entity_ownership_required_property_was_not_provided() { }) } -// #[test] -// fn transfer_entity_ownership_unique_constraint_violation() { -// with_test_externalities(|| { -// // Create class with default permissions -// assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - -// // Create first unique reference property with same_controller flag set -// let property_type = PropertyType::::vec_reference( -// FIRST_CLASS_ID, -// true, -// VecMaxLengthConstraint::get(), -// ); - -// let first_property = Property::::with_name_and_type( -// (PropertyNameLengthConstraint::get().max() - 1) as usize, -// property_type.clone(), -// true, -// true, -// ); - -// // Add first Schema to the Class -// assert_ok!(add_class_schema( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// BTreeSet::new(), -// vec![first_property] -// )); - -// // Create second reference property with same_controller flag set -// let second_property = Property::::with_name_and_type( -// PropertyNameLengthConstraint::get().max() as usize, -// property_type, -// true, -// false, -// ); - -// // Add second Schema to the Class -// assert_ok!(add_class_schema( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// BTreeSet::new(), -// vec![second_property] -// )); - -// // Update class permissions to force any member be available to create Entities -// assert_ok!(update_class_permissions( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// Some(true), -// None, -// None, -// None -// )); - -// let actor = Actor::Lead; - -// // Create first entity -// assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); - -// // Create second entity -// assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); - -// let first_schema_property_value = -// InputPropertyValue::::vec_reference(vec![SECOND_ENTITY_ID, SECOND_ENTITY_ID]); - -// let mut schema_property_values = BTreeMap::new(); -// schema_property_values.insert(FIRST_PROPERTY_ID, first_schema_property_value); - -// // Add first schema support to the first Entity -// assert_ok!(add_schema_support_to_entity( -// LEAD_ORIGIN, -// actor.to_owned(), -// FIRST_ENTITY_ID, -// FIRST_SCHEMA_ID, -// schema_property_values -// )); - -// let second_schema_property_value = InputPropertyValue::::vec_reference(vec![ -// SECOND_ENTITY_ID, -// SECOND_ENTITY_ID, -// SECOND_ENTITY_ID, -// ]); - -// let mut schema_property_values = BTreeMap::new(); -// schema_property_values.insert(SECOND_PROPERTY_ID, second_schema_property_value); - -// // Add second schema support to the first Entity -// assert_ok!(add_schema_support_to_entity( -// LEAD_ORIGIN, -// actor, -// FIRST_ENTITY_ID, -// SECOND_SCHEMA_ID, -// schema_property_values -// )); - -// // Create third entity -// assert_ok!(create_entity( -// FIRST_MEMBER_ORIGIN, -// FIRST_CLASS_ID, -// Actor::Member(FIRST_MEMBER_ID) -// )); - -// let new_controller = EntityController::Member(FIRST_MEMBER_ID); - -// let schema_property_value = InputPropertyValue::::vec_reference(vec![ -// THIRD_ENTITY_ID, -// THIRD_ENTITY_ID, -// THIRD_ENTITY_ID, -// ]); - -// let mut schema_property_values = BTreeMap::new(); -// schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value.clone()); -// schema_property_values.insert(SECOND_PROPERTY_ID, schema_property_value); - -// // Runtime state before tested call - -// // Events number before tested call -// let number_of_events_before_call = System::events().len(); - -// // Make an attempt to transfer ownership of Entity, providing new property value reference(s) with same owner flag set, -// // which have duplicates or identical to thouse, are already added to The Entity, though should be unique on Class Property level -// let transfer_entity_ownership_result = transfer_entity_ownership( -// LEAD_ORIGIN, -// FIRST_ENTITY_ID, -// new_controller, -// schema_property_values, -// ); - -// // Failure checked -// assert_failure( -// transfer_entity_ownership_result, -// ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, -// number_of_events_before_call, -// ); -// }) -// } +#[test] +fn transfer_entity_ownership_unique_constraint_violation() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + // Create unique reference property with same_controller flag set + let property_type = PropertyType::::vec_reference( + FIRST_CLASS_ID, + true, + VecMaxLengthConstraint::get(), + ); + + let property = Property::::with_name_and_type( + PropertyNameLengthConstraint::get().max() as usize, + property_type, + true, + true, + ); + + // Add Schema to the Class + assert_ok!(add_class_schema( + LEAD_ORIGIN, + FIRST_CLASS_ID, + BTreeSet::new(), + vec![property] + )); + + // Update class permissions to force any member be available to create Entities + assert_ok!(update_class_permissions( + LEAD_ORIGIN, + FIRST_CLASS_ID, + Some(true), + None, + None, + None + )); + + let actor = Actor::Lead; + + // Create first entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create second entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + let first_schema_property_value = + InputPropertyValue::::vec_reference(vec![SECOND_ENTITY_ID, SECOND_ENTITY_ID]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, first_schema_property_value); + + // Add schema support to the first Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + FIRST_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + let second_schema_property_value = InputPropertyValue::::vec_reference(vec![ + SECOND_ENTITY_ID, + SECOND_ENTITY_ID, + SECOND_ENTITY_ID, + ]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, second_schema_property_value); + + // Add schema support to the second Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor, + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + // Create third entity + assert_ok!(create_entity( + FIRST_MEMBER_ORIGIN, + FIRST_CLASS_ID, + Actor::Member(FIRST_MEMBER_ID) + )); + + let new_controller = EntityController::Member(FIRST_MEMBER_ID); + + let schema_property_value = InputPropertyValue::::vec_reference(vec![ + THIRD_ENTITY_ID, + THIRD_ENTITY_ID, + THIRD_ENTITY_ID, + ]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + assert_ok!(transfer_entity_ownership( + LEAD_ORIGIN, + FIRST_ENTITY_ID, + new_controller.clone(), + schema_property_values.clone(), + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + // Make an attempt to transfer ownership of Entity, providing new property value reference(s) + // with same owner flag set, which are identical to thouse, are already added to the another Entity of this Class, + // though should be unique on Class Property level + let transfer_entity_ownership_result = transfer_entity_ownership( + LEAD_ORIGIN, + SECOND_ENTITY_ID, + new_controller, + schema_property_values, + ); + + // Failure checked + assert_failure( + transfer_entity_ownership_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} diff --git a/runtime-modules/content-directory/src/tests/update_entity_property_values.rs b/runtime-modules/content-directory/src/tests/update_entity_property_values.rs index 065aee3961..a7c158d987 100644 --- a/runtime-modules/content-directory/src/tests/update_entity_property_values.rs +++ b/runtime-modules/content-directory/src/tests/update_entity_property_values.rs @@ -70,7 +70,7 @@ fn update_entity_property_values_entity_not_found() { ); // Create class reference schema - add_class_reference_schema(); + add_unique_class_reference_schema(); // Runtime state before tested call @@ -108,7 +108,7 @@ fn update_entity_property_values_lead_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -146,7 +146,7 @@ fn update_entity_property_values_member_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -184,7 +184,7 @@ fn update_entity_property_values_curator_group_is_not_active() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -232,7 +232,7 @@ fn update_entity_property_values_curator_auth_failed() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -273,7 +273,7 @@ fn update_entity_property_values_curator_not_found_in_curator_group() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support( + add_unique_class_reference_schema_and_entity_schema_support( &Actor::Curator(FIRST_CURATOR_GROUP_ID, FIRST_CURATOR_ID), FIRST_CURATOR_ORIGIN, ); @@ -314,7 +314,7 @@ fn update_entity_property_values_entity_access_denied() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -353,7 +353,7 @@ fn update_entity_property_values_locked_on_class_level() { ); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&Actor::Lead, LEAD_ORIGIN); // Runtime state before tested call @@ -475,7 +475,7 @@ fn update_entity_property_values_unknown_entity_property_id() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -518,7 +518,7 @@ fn update_entity_property_values_prop_value_do_not_match_type() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -560,7 +560,7 @@ fn update_entity_property_values_vec_prop_is_too_long() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -758,7 +758,7 @@ fn update_entity_property_values_referenced_entity_not_found() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Runtime state before tested call @@ -806,7 +806,7 @@ fn update_entity_property_values_referenced_entity_does_not_match_its_class() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the first Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Create second entity assert_ok!(create_entity(LEAD_ORIGIN, SECOND_CLASS_ID, actor.clone())); @@ -854,7 +854,7 @@ fn update_entity_property_values_entity_can_not_be_referenced() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the first Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Create second Entity of first Class assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); @@ -920,7 +920,7 @@ fn update_entity_property_values_same_controller_constraint_violation() { assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); // Create class reference schema and add corresponding schema support to the first Entity - add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); // Create second Entity assert_ok!(create_entity( @@ -960,85 +960,66 @@ fn update_entity_property_values_same_controller_constraint_violation() { }) } -// #[test] -// fn update_entity_property_values_property_should_be_unique() { -// with_test_externalities(|| { -// // Create class with default permissions -// assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); - -// let actor = Actor::Lead; - -// // Create first Entity -// assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); - -// // Create class reference schema and add corresponding schema support to the Entity -// add_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); - -// // Create second property with unique constraint -// let property_type = PropertyType::::vec_reference( -// FIRST_CLASS_ID, -// true, -// VecMaxLengthConstraint::get(), -// ); - -// let property = Property::::with_name_and_type( -// PropertyNameLengthConstraint::get().max() as usize, -// property_type, -// true, -// true, -// ); - -// // Add second Schema to the Class -// assert_ok!(add_class_schema( -// LEAD_ORIGIN, -// FIRST_CLASS_ID, -// BTreeSet::new(), -// vec![property] -// )); - -// let schema_property_value = InputPropertyValue::::vec_reference(vec![ -// FIRST_ENTITY_ID, -// FIRST_ENTITY_ID, -// FIRST_ENTITY_ID, -// ]); - -// let mut schema_property_values = BTreeMap::new(); -// schema_property_values.insert(SECOND_PROPERTY_ID, schema_property_value); - -// // Add schema support to the entity -// assert_ok!(add_schema_support_to_entity( -// LEAD_ORIGIN, -// actor.to_owned(), -// FIRST_ENTITY_ID, -// SECOND_SCHEMA_ID, -// schema_property_values -// )); - -// // Runtime state before tested call - -// // Events number before tested call -// let number_of_events_before_call = System::events().len(); - -// let mut schema_new_property_values = BTreeMap::new(); -// let schema_new_property_value = -// InputPropertyValue::::vec_reference(vec![FIRST_ENTITY_ID, FIRST_ENTITY_ID]); - -// schema_new_property_values.insert(SECOND_PROPERTY_ID, schema_new_property_value); - -// // Make an attempt to update entity property values, providing property value(s), which are identical to thouse, -// // are already added to The Entity, though should be unique on Class Property level -// let update_entity_property_values_result = update_entity_property_values( -// LEAD_ORIGIN, -// actor, -// FIRST_ENTITY_ID, -// schema_new_property_values, -// ); - -// // Failure checked -// assert_failure( -// update_entity_property_values_result, -// ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, -// number_of_events_before_call, -// ); -// }) -// } +#[test] +fn update_entity_property_values_property_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create Class reference schema and add corresponding schema support to the first Entity + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + + // Create second Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + let schema_property_value = InputPropertyValue::::vec_reference(vec![ + FIRST_ENTITY_ID, + FIRST_ENTITY_ID, + FIRST_ENTITY_ID, + ]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + // Add schema support to the entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + let mut schema_new_property_values = BTreeMap::new(); + let schema_new_property_value = + InputPropertyValue::::vec_reference(vec![FIRST_ENTITY_ID, FIRST_ENTITY_ID]); + + schema_new_property_values.insert(FIRST_PROPERTY_ID, schema_new_property_value); + + // Make an attempt to update entity property values, providing property value(s), which are identical to thouse, + // are already added to the another Entity of this Class, though should be unique on Class Property level + let update_entity_property_values_result = update_entity_property_values( + LEAD_ORIGIN, + actor, + SECOND_ENTITY_ID, + schema_new_property_values, + ); + + // Failure checked + assert_failure( + update_entity_property_values_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} From b54cd574853010ecde859144a2b5ffb004fad295 Mon Sep 17 00:00:00 2001 From: iorveth Date: Mon, 27 Jul 2020 18:30:27 +0000 Subject: [PATCH 04/17] Tests: vector level operations uniqueness feature test coverage, introduce SimplifiedOutputPropertyValue to store property values unique on Class level --- .../content-directory/src/helpers.rs | 4 +- runtime-modules/content-directory/src/lib.rs | 32 +++++---- .../content-directory/src/schema.rs | 2 +- .../content-directory/src/schema/convert.rs | 13 ++++ .../content-directory/src/schema/output.rs | 24 ++++++- .../content-directory/src/tests.rs | 2 + .../src/tests/clear_entity_property_vector.rs | 51 ++++++++++++++ .../tests/insert_at_entity_property_vector.rs | 65 ++++++++++++++++++ .../tests/remove_at_entity_property_vector.rs | 66 +++++++++++++++++++ 9 files changed, 240 insertions(+), 19 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 53eb9ab002..43f158860e 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -147,7 +147,7 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { /// Used to retrieve `OutputPropertyValue`s, which respective `Properties` have `unique` flag set /// (skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required`) - pub fn compute_unique(&self) -> BTreeMap> { + pub fn compute_unique(&self) -> BTreeMap> { self.iter() .filter(|(property_id, _)| { match self @@ -162,7 +162,7 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { } }) .map(|(property_id, property_value)| { - (*property_id, property_value.get_value().to_owned()) + (*property_id, property_value.get_value().to_owned().into()) }) .collect() } diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index abe015df7e..d89bcb1dd9 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -154,7 +154,7 @@ decl_storage! { pub CuratorGroupById get(curator_group_by_id) config(): map T::CuratorGroupId => CuratorGroup; /// Used to enforce uniqueness of a property value across all Entities that have this property in a given Class. - pub UniquePropertyValues get(unique_property_values) config(): double_map hasher(blake2_128) (T::ClassId, PropertyId), blake2_128(OutputPropertyValue) => (); + pub UniquePropertyValues get(unique_property_values) config(): double_map hasher(blake2_128) (T::ClassId, PropertyId), blake2_128(SimplifiedOutputPropertyValue) => (); /// Next runtime storage values used to maintain next id value, used on creation of respective curator groups, classes and entities @@ -1154,7 +1154,7 @@ decl_module! { let empty_property_value_vector = Self::clear_property_vector(property_value_vector); // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &empty_property_value_vector)?; + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &empty_property_value_vector.simplify())?; // // == MUTATION SAFE == @@ -1218,7 +1218,7 @@ decl_module! { .ensure_index_in_property_vector_is_valid(index_in_property_vector)?; let involved_entity_id = property_value_vector - .get_vec_value() + .get_vec_value_ref() .get_involved_entities() .and_then(|involved_entities| involved_entities.get(index_in_property_vector as usize).copied()); @@ -1231,7 +1231,7 @@ decl_module! { let class_id = entity.get_class_id(); // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated)?; + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated.simplify())?; // // == MUTATION SAFE == @@ -1323,7 +1323,7 @@ decl_module! { let class_id = entity.get_class_id(); // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated)?; + Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated.simplify())?; // // == MUTATION SAFE == @@ -1439,7 +1439,7 @@ impl Module { pub fn add_unique_property_value( class_id: T::ClassId, property_id: PropertyId, - property_value: OutputPropertyValue, + property_value: SimplifiedOutputPropertyValue, ) { >::insert((class_id, property_id), property_value, ()); } @@ -1448,7 +1448,7 @@ impl Module { pub fn remove_unique_property_value( class_id: T::ClassId, property_id: PropertyId, - property_value: OutputPropertyValue, + property_value: SimplifiedOutputPropertyValue, ) { >::remove((class_id, property_id), property_value); } @@ -1456,7 +1456,10 @@ impl Module { /// Add property values, that should be unique on `Class` level pub fn add_unique_property_values( class_id: T::ClassId, - new_output_values_for_existing_properties: BTreeMap>, + new_output_values_for_existing_properties: BTreeMap< + PropertyId, + SimplifiedOutputPropertyValue, + >, ) { new_output_values_for_existing_properties .into_iter() @@ -1468,7 +1471,10 @@ impl Module { /// Remove property values, that should be unique on `Class` level pub fn remove_unique_property_values( class_id: T::ClassId, - new_output_values_for_existing_properties: BTreeMap>, + new_output_values_for_existing_properties: BTreeMap< + PropertyId, + SimplifiedOutputPropertyValue, + >, ) { new_output_values_for_existing_properties .into_iter() @@ -1528,7 +1534,7 @@ impl Module { property: Property, ) -> Option> { let entity_ids_to_decrease_rc = property_value_vector - .get_vec_value() + .get_vec_value_ref() .get_involved_entities(); if let Some(entity_ids_to_decrease_rcs) = entity_ids_to_decrease_rc { @@ -1778,7 +1784,7 @@ impl Module { pub fn ensure_property_unique_option_satisfied( class_id: T::ClassId, property_id: PropertyId, - property_value: &OutputPropertyValue, + property_value: &SimplifiedOutputPropertyValue, ) -> Result<(), &'static str> { ensure!( !>::exists((class_id, property_id), property_value), @@ -1790,7 +1796,7 @@ impl Module { /// Ensure all provided `Properties` with `unique` flag set are `unique` on `Class` level pub fn ensure_properties_unique_option_satisfied( class_id: T::ClassId, - unique_new_property_values: &BTreeMap>, + unique_new_property_values: &BTreeMap>, ) -> Result<(), &'static str> { for (&property_id, property_value) in unique_new_property_values { Self::ensure_property_unique_option_satisfied(class_id, property_id, property_value)?; @@ -2057,7 +2063,7 @@ impl Module { let mut entity_property_values_updated = entity_property_values.to_owned(); new_output_property_values - .into_iter() + .iter() .for_each(|(id, new_property_value)| { if let Some(entity_property_value) = entity_property_values_updated.get_mut(&id) { entity_property_value.update(new_property_value.to_owned()); diff --git a/runtime-modules/content-directory/src/schema.rs b/runtime-modules/content-directory/src/schema.rs index fa72cba745..c444e30dde 100644 --- a/runtime-modules/content-directory/src/schema.rs +++ b/runtime-modules/content-directory/src/schema.rs @@ -384,7 +384,7 @@ impl Property { match ( single_value, - vec_value.get_vec_value(), + vec_value.get_vec_value_ref(), property_type_vec.get_vec_type(), ) { // Single values diff --git a/runtime-modules/content-directory/src/schema/convert.rs b/runtime-modules/content-directory/src/schema/convert.rs index 2eb578565c..b65d85c314 100644 --- a/runtime-modules/content-directory/src/schema/convert.rs +++ b/runtime-modules/content-directory/src/schema/convert.rs @@ -60,3 +60,16 @@ impl From> for VecOutputValue { } } } + +impl From> for SimplifiedOutputPropertyValue { + fn from(output_property_value: OutputPropertyValue) -> Self { + match output_property_value { + OutputPropertyValue::Single(output_value) => { + SimplifiedOutputPropertyValue::Single(output_value) + } + OutputPropertyValue::Vector(vector_output_value) => { + SimplifiedOutputPropertyValue::Vector(vector_output_value.get_vec_value()) + } + } + } +} diff --git a/runtime-modules/content-directory/src/schema/output.rs b/runtime-modules/content-directory/src/schema/output.rs index 105fbdb4f6..6607396ac4 100644 --- a/runtime-modules/content-directory/src/schema/output.rs +++ b/runtime-modules/content-directory/src/schema/output.rs @@ -1,5 +1,14 @@ use super::*; +/// Enum, representing either `OutputValue` or `VecOutputValue` +/// Simplified version, without nonces +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] +pub enum SimplifiedOutputPropertyValue { + Single(OutputValue), + Vector(VecOutputValue), +} + /// Enum, representing either `OutputValue` or `VecOutputPropertyValue` #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] @@ -55,10 +64,14 @@ impl OutputPropertyValue { } } OutputPropertyValue::Vector(vector_property_value) => vector_property_value - .get_vec_value() + .get_vec_value_ref() .get_involved_entities(), } } + + pub fn simplify(&self) -> SimplifiedOutputPropertyValue { + self.clone().into() + } } impl Default for OutputPropertyValue { @@ -102,7 +115,7 @@ impl OutputValue { /// Consists of `VecOutputPropertyValue` enum representation and `nonce`, used to avoid vector data race update conditions #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -#[derive(Encode, Decode, Default, Clone, PartialEq, Eq, Debug)] +#[derive(Encode, Decode, Default, Clone, Debug, PartialEq, Eq)] pub struct VecOutputPropertyValue { vec_value: VecOutputValue, nonce: T::Nonce, @@ -119,8 +132,13 @@ impl VecOutputPropertyValue { Self { vec_value, nonce } } + /// Retrieve `VecOutputValue` + pub fn get_vec_value(self) -> VecOutputValue { + self.vec_value + } + /// Retrieve `VecOutputValue` by reference - pub fn get_vec_value(&self) -> &VecOutputValue { + pub fn get_vec_value_ref(&self) -> &VecOutputValue { &self.vec_value } diff --git a/runtime-modules/content-directory/src/tests.rs b/runtime-modules/content-directory/src/tests.rs index 9e44347662..66512461e9 100644 --- a/runtime-modules/content-directory/src/tests.rs +++ b/runtime-modules/content-directory/src/tests.rs @@ -301,6 +301,7 @@ pub fn emulate_entity_access_state_for_failure_case( } } +/// Create class reference schema pub fn add_unique_class_reference_schema() { // Create property let property_type = @@ -322,6 +323,7 @@ pub fn add_unique_class_reference_schema() { )); } +/// Create class reference schema and add corresponding schema support to the Entity pub fn add_unique_class_reference_schema_and_entity_schema_support( actor: &Actor, origin: u64, diff --git a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs index ee8a69d9ee..f03bbaabca 100644 --- a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs @@ -504,3 +504,54 @@ fn clear_entity_property_vector_value_under_given_index_is_not_a_vector() { ); }) } + +#[test] +fn clear_entity_property_vector_property_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create unique class reference schema and add corresponding schema support to the first Entity + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + + // Create second Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create blank vec reference + let schema_property_value = InputPropertyValue::::vec_reference(vec![]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + // Add schema support to the second Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + // Make an attempt to clear property_vector under given `entity_id` & `in_class_schema_property_id` + // in case, when the same blank required & unique property value vector already added to another Entity of this Class. + let clear_entity_property_vector_result = + clear_entity_property_vector(LEAD_ORIGIN, actor, FIRST_ENTITY_ID, FIRST_PROPERTY_ID); + + // Failure checked + assert_failure( + clear_entity_property_vector_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} diff --git a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs index 501a8b6205..dd2a2716cc 100644 --- a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs @@ -1204,3 +1204,68 @@ fn insert_at_entity_property_vector_same_controller_constraint_violation() { ); }) } + +#[test] +fn insert_at_entity_property_vector_property_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create unique class reference schema and add corresponding schema support to the first Entity + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + + // Create second Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create vec reference + let schema_property_value = + InputPropertyValue::::vec_reference(vec![FIRST_ENTITY_ID]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + // Add schema support to the second Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + // Make an attempt to insert value at given `index_in_property_vector` + // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // when in result we`ll get required & unique property value vector, + // which is already added to another Entity of this Class. + let nonce = 0; + let index_in_property_vector = 0; + let input_value = InputValue::Reference(FIRST_ENTITY_ID); + + let insert_at_entity_property_vector_result = insert_at_entity_property_vector( + LEAD_ORIGIN, + actor, + SECOND_ENTITY_ID, + FIRST_PROPERTY_ID, + index_in_property_vector, + input_value, + nonce, + ); + + // Failure checked + assert_failure( + insert_at_entity_property_vector_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} diff --git a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs index 414b985a8a..85e93d1bef 100644 --- a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs @@ -708,3 +708,69 @@ fn remove_at_entity_property_vector_index_is_out_of_range() { ); }) } + +#[test] +fn remove_at_entity_property_vector_property_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create unique class reference schema and add corresponding schema support to the first Entity + add_unique_class_reference_schema_and_entity_schema_support(&actor, LEAD_ORIGIN); + + // Create second Entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.clone())); + + // Create vec reference + let schema_property_value = InputPropertyValue::::vec_reference(vec![ + FIRST_ENTITY_ID, + FIRST_ENTITY_ID, + FIRST_ENTITY_ID, + ]); + + let mut schema_property_values = BTreeMap::new(); + schema_property_values.insert(FIRST_PROPERTY_ID, schema_property_value); + + // Add schema support to the second Entity + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + schema_property_values + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + let nonce = 0; + let index_in_property_vector = 0; + + // Make an attempt to remove value at given `index_in_property_vector` + // from `PropertyValueVec` under `in_class_schema_property_id` in case, + // when in result we`ll get required & unique property value vector, + // which is already added to another Entity of this Class. + let remove_at_entity_property_vector_result = remove_at_entity_property_vector( + LEAD_ORIGIN, + actor, + SECOND_ENTITY_ID, + FIRST_PROPERTY_ID, + index_in_property_vector, + nonce, + ); + + // Failure checked + assert_failure( + remove_at_entity_property_vector_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} From e03886bc5786e8f75218251abe11152eb1ca7190 Mon Sep 17 00:00:00 2001 From: iorveth Date: Mon, 10 Aug 2020 17:15:45 +0000 Subject: [PATCH 05/17] Uniqueness feature - store only property value hashes instead of actual values --- .../content-directory/src/helpers.rs | 11 +- runtime-modules/content-directory/src/lib.rs | 113 ++++++++++-------- runtime-modules/content-directory/src/mock.rs | 1 - .../content-directory/src/schema/convert.rs | 12 -- .../content-directory/src/schema/output.rs | 29 +++-- 5 files changed, 84 insertions(+), 82 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 43f158860e..d8dfbf1924 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -145,9 +145,9 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { Ok(values_for_existing_properties) } - /// Used to retrieve `OutputPropertyValue`s, which respective `Properties` have `unique` flag set + /// Used to compute hashes from `OutputPropertyValue`s and their respective property ids, which respective `Properties` have `unique` flag set /// (skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required`) - pub fn compute_unique(&self) -> BTreeMap> { + pub fn compute_unique_hashes(&self) -> BTreeMap { self.iter() .filter(|(property_id, _)| { match self @@ -161,8 +161,11 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { _ => false, } }) - .map(|(property_id, property_value)| { - (*property_id, property_value.get_value().to_owned().into()) + .map(|(&property_id, property_value)| { + ( + property_id, + property_value.get_value().compute_unique_hash(property_id), + ) }) .collect() } diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index d89bcb1dd9..dcfa97e0e1 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -42,6 +42,8 @@ pub use serde::{Deserialize, Serialize}; /// Type, used in diffrent numeric constraints representations type MaxNumber = u32; +//type PropertyHash = T::Hash; + pub trait Trait: system::Trait + ActorAuthenticator + Debug + Clone { /// The overarching event type. type Event: From> + Into<::Event>; @@ -71,6 +73,7 @@ pub trait Trait: system::Trait + ActorAuthenticator + Debug + Clone { + Copy + Clone + One + + Hash + Zero + MaybeSerializeDeserialize + Eq @@ -93,6 +96,8 @@ pub trait Trait: system::Trait + ActorAuthenticator + Debug + Clone { + PartialEq + Ord; + //type SimplifiedPropertyHash: From + EncodeLike + Hash + Default + PartialEq + Eq + Ord + Codec + MaybeSerializeDeserialize; + /// Security/configuration constraints /// Type, representing min & max property name length constraints @@ -154,7 +159,9 @@ decl_storage! { pub CuratorGroupById get(curator_group_by_id) config(): map T::CuratorGroupId => CuratorGroup; /// Used to enforce uniqueness of a property value across all Entities that have this property in a given Class. - pub UniquePropertyValues get(unique_property_values) config(): double_map hasher(blake2_128) (T::ClassId, PropertyId), blake2_128(SimplifiedOutputPropertyValue) => (); + + /// Mapping of class id and its property id to the respective entity id and property value hash. + pub UniquePropertyValueHashes get(unique_property_value_hashes): double_map hasher(blake2_128) (T::ClassId, PropertyId), blake2_128(T::Hash) => (); /// Next runtime storage values used to maintain next id value, used on creation of respective curator groups, classes and entities @@ -745,17 +752,17 @@ decl_module! { // Retrieve OutputPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_unique_property_values = new_output_values_for_existing_properties.compute_unique(); + let new_unique_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_hashes)?; // // == MUTATION SAFE == // // Add property values, that should be unique on Class level - Self::add_unique_property_values(class_id, new_unique_property_values); + Self::add_unique_property_value_hashes(class_id, new_unique_hashes); // Make updated entity_property_values from parameters provided let entity_property_values_updated = @@ -914,14 +921,14 @@ decl_module! { let entity_values = entity.get_values(); - let entity_unique_property_values = OutputValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique(); + let unique_property_value_hashes = OutputValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique_hashes(); // // == MUTATION SAFE == // // Remove property value entries, that should be unique on Class level - Self::remove_unique_property_values(class_id, entity_unique_property_values); + Self::remove_unique_property_value_hashes(class_id, unique_property_value_hashes); // Remove entity >::remove(entity_id); @@ -1001,17 +1008,17 @@ decl_module! { // Retrieve OutputPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_unique_property_values = new_output_values_for_existing_properties.compute_unique(); + let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; // // == MUTATION SAFE == // // Add property values, that should be unique on Class level - Self::add_unique_property_values(class_id, new_unique_property_values); + Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::calculate_entities_inbound_rcs_delta( @@ -1085,17 +1092,17 @@ decl_module! { // Compute OutputPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_unique_property_values = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?.compute_unique(); + let new_unique_property_value_hashes = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?.compute_unique_hashes(); // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_properties_unique_option_satisfied(class_id, &new_unique_property_values)?; + Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; // // == MUTATION SAFE == // // Update property values, that should be unique on Class level - Self::add_unique_property_values(class_id, new_unique_property_values); + Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); // Make updated entity_property_values from current entity_property_values and new_output_property_values provided let entity_property_values_updated = @@ -1153,8 +1160,10 @@ decl_module! { // Clear property_value_vector. let empty_property_value_vector = Self::clear_property_vector(property_value_vector); + let property_value_hash = empty_property_value_vector.compute_unique_hash(in_class_schema_property_id); + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &empty_property_value_vector.simplify())?; + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; // // == MUTATION SAFE == @@ -1229,9 +1238,12 @@ decl_module! { ); let class_id = entity.get_class_id(); + + // Compute hash from unique property value and its respective property id + let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated.simplify())?; + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; // // == MUTATION SAFE == @@ -1322,8 +1334,11 @@ decl_module! { let class_id = entity.get_class_id(); - // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_vector_updated.simplify())?; + // Compute hash from unique property value and its respective property id + let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); + + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; // // == MUTATION SAFE == @@ -1435,51 +1450,45 @@ impl Module { }) } - /// Add/update property value, that should be unique on `Class` level - pub fn add_unique_property_value( + /// Add/update property value hash, that should be unique on `Class` level + pub fn add_unique_property_value_hash( class_id: T::ClassId, property_id: PropertyId, - property_value: SimplifiedOutputPropertyValue, + hash: T::Hash, ) { - >::insert((class_id, property_id), property_value, ()); + >::insert((class_id, property_id), hash, ()); } - /// Remove property value, that should be unique on `Class` level - pub fn remove_unique_property_value( + /// Remove property value hash, that should be unique on `Class` level + pub fn remove_unique_property_value_hash( class_id: T::ClassId, property_id: PropertyId, - property_value: SimplifiedOutputPropertyValue, + hash: T::Hash, ) { - >::remove((class_id, property_id), property_value); + >::remove((class_id, property_id), hash); } - /// Add property values, that should be unique on `Class` level - pub fn add_unique_property_values( + /// Add property value hashes, that should be unique on `Class` level + pub fn add_unique_property_value_hashes( class_id: T::ClassId, - new_output_values_for_existing_properties: BTreeMap< - PropertyId, - SimplifiedOutputPropertyValue, - >, + unique_property_value_hashes: BTreeMap, ) { - new_output_values_for_existing_properties + unique_property_value_hashes .into_iter() - .for_each(|(property_id, property_value)| { - Self::add_unique_property_value(class_id, property_id, property_value); + .for_each(|(property_id, hash)| { + Self::add_unique_property_value_hash(class_id, property_id, hash); }); } - /// Remove property values, that should be unique on `Class` level - pub fn remove_unique_property_values( + /// Remove property value hashes, that should be unique on `Class` level + pub fn remove_unique_property_value_hashes( class_id: T::ClassId, - new_output_values_for_existing_properties: BTreeMap< - PropertyId, - SimplifiedOutputPropertyValue, - >, + unique_property_value_hashes: BTreeMap, ) { - new_output_values_for_existing_properties + unique_property_value_hashes .into_iter() - .for_each(|(property_id, property_value)| { - Self::remove_unique_property_value(class_id, property_id, property_value); + .for_each(|(property_id, hash)| { + Self::remove_unique_property_value_hash(class_id, property_id, hash); }); } @@ -1780,26 +1789,26 @@ impl Module { } } - /// Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - pub fn ensure_property_unique_option_satisfied( + /// Ensure `Property` with `unique` flag set is `unique` on `Class` level + pub fn ensure_property_hash_unique_option_satisfied( class_id: T::ClassId, property_id: PropertyId, - property_value: &SimplifiedOutputPropertyValue, + unique_property_value_hash: &T::Hash, ) -> Result<(), &'static str> { ensure!( - !>::exists((class_id, property_id), property_value), + !>::exists((class_id, property_id), unique_property_value_hash), ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE ); Ok(()) } - /// Ensure all provided `Properties` with `unique` flag set are `unique` on `Class` level - pub fn ensure_properties_unique_option_satisfied( + /// Ensure all `Properties` with `unique` flag set are `unique` on `Class` level + pub fn ensure_property_hashes_unique_option_satisfied( class_id: T::ClassId, - unique_new_property_values: &BTreeMap>, + unique_property_value_hashes: &BTreeMap, ) -> Result<(), &'static str> { - for (&property_id, property_value) in unique_new_property_values { - Self::ensure_property_unique_option_satisfied(class_id, property_id, property_value)?; + for (&property_id, unique_property_value_hash) in unique_property_value_hashes { + Self::ensure_property_hash_unique_option_satisfied(class_id, property_id, unique_property_value_hash)?; } Ok(()) } diff --git a/runtime-modules/content-directory/src/mock.rs b/runtime-modules/content-directory/src/mock.rs index 6d8a330270..05dfcc1128 100644 --- a/runtime-modules/content-directory/src/mock.rs +++ b/runtime-modules/content-directory/src/mock.rs @@ -402,7 +402,6 @@ fn default_content_directory_genesis_config() -> GenesisConfig { class_by_id: vec![], entity_by_id: vec![], curator_group_by_id: vec![], - unique_property_values: vec![], next_class_id: 1, next_entity_id: 1, next_curator_group_id: 1, diff --git a/runtime-modules/content-directory/src/schema/convert.rs b/runtime-modules/content-directory/src/schema/convert.rs index b65d85c314..8ebd0d9571 100644 --- a/runtime-modules/content-directory/src/schema/convert.rs +++ b/runtime-modules/content-directory/src/schema/convert.rs @@ -61,15 +61,3 @@ impl From> for VecOutputValue { } } -impl From> for SimplifiedOutputPropertyValue { - fn from(output_property_value: OutputPropertyValue) -> Self { - match output_property_value { - OutputPropertyValue::Single(output_value) => { - SimplifiedOutputPropertyValue::Single(output_value) - } - OutputPropertyValue::Vector(vector_output_value) => { - SimplifiedOutputPropertyValue::Vector(vector_output_value.get_vec_value()) - } - } - } -} diff --git a/runtime-modules/content-directory/src/schema/output.rs b/runtime-modules/content-directory/src/schema/output.rs index 6607396ac4..ca2517808f 100644 --- a/runtime-modules/content-directory/src/schema/output.rs +++ b/runtime-modules/content-directory/src/schema/output.rs @@ -1,13 +1,5 @@ use super::*; - -/// Enum, representing either `OutputValue` or `VecOutputValue` -/// Simplified version, without nonces -#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -#[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] -pub enum SimplifiedOutputPropertyValue { - Single(OutputValue), - Vector(VecOutputValue), -} +use runtime_primitives::traits::Hash; /// Enum, representing either `OutputValue` or `VecOutputPropertyValue` #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] @@ -69,8 +61,19 @@ impl OutputPropertyValue { } } - pub fn simplify(&self) -> SimplifiedOutputPropertyValue { - self.clone().into() + /// Compute hash from unique property value and its respective property_id + pub fn compute_unique_hash(&self, property_id: PropertyId) -> T::Hash { + match self { + OutputPropertyValue::Single(output_value) => { + (property_id, output_value).using_encoded(::Hashing::hash) + } + OutputPropertyValue::Vector(vector_output_value) => { + // Do not hash nonce + let vector_output_value = vector_output_value.get_vec_value_ref(); + + (property_id, vector_output_value).using_encoded(::Hashing::hash) + } + } } } @@ -82,7 +85,7 @@ impl Default for OutputPropertyValue { /// OutputValue enum representation, related to corresponding `SingleOutputPropertyValue` structure #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -#[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] +#[derive(Encode, Decode, Hash, Clone, PartialEq, PartialOrd, Ord, Eq, Debug)] pub enum OutputValue { Bool(bool), Uint16(u16), @@ -267,7 +270,7 @@ impl VecOutputPropertyValue { /// Vector value enum representation, related to corresponding `VecOutputPropertyValue` structure #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -#[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] +#[derive(Encode, Decode, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug)] pub enum VecOutputValue { Bool(Vec), Uint16(Vec), From fb19f628aa35f195d4bb94eb8d226129a59a1f5f Mon Sep 17 00:00:00 2001 From: iorveth Date: Mon, 10 Aug 2020 19:47:47 +0000 Subject: [PATCH 06/17] Tests: Additional failure case for the uniqueness feature --- .../src/tests/add_schema_support_to_entity.rs | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs b/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs index 1194697201..4018067731 100644 --- a/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs +++ b/runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs @@ -1364,3 +1364,95 @@ fn add_schema_support_property_should_be_unique() { ); }) } + +#[test] +fn add_schema_support_properties_should_be_unique() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let actor = Actor::Lead; + + // Create first entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); + + // Create second entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); + + // Create third entity + assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned())); + + let property_type = PropertyType::::single_text(TextMaxLengthConstraint::get()); + + // Create text property + + let property = Property::::with_name_and_type( + PropertyNameLengthConstraint::get().max() as usize, + property_type, + true, + true, + ); + + // Add Schema to the Class + assert_ok!(add_class_schema( + LEAD_ORIGIN, + FIRST_CLASS_ID, + BTreeSet::new(), + vec![property] + )); + + let mut first_schema_property_values = BTreeMap::new(); + + let first_schema_property_value = + InputPropertyValue::::single_text(TextMaxLengthConstraint::get()); + + first_schema_property_values.insert(FIRST_PROPERTY_ID, first_schema_property_value); + + // Add Entity Schema support to the first Entity (property unique on the Class level added) + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + FIRST_ENTITY_ID, + FIRST_SCHEMA_ID, + first_schema_property_values.clone(), + )); + + let mut second_schema_property_values = BTreeMap::new(); + + let second_schema_property_value = + InputPropertyValue::::single_text(TextMaxLengthConstraint::get() - 1); + + second_schema_property_values.insert(FIRST_PROPERTY_ID, second_schema_property_value); + + // Add Entity Schema support to the second Entity (property unique on the Class level added) + assert_ok!(add_schema_support_to_entity( + LEAD_ORIGIN, + actor.to_owned(), + SECOND_ENTITY_ID, + FIRST_SCHEMA_ID, + second_schema_property_values, + )); + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + // Make an attempt to add schema support to the Entity, providing property values, which respective Class properties have + // unique flag set and same property values under same property_ids were already added to any Entity of this Class + let add_schema_support_to_entity_result = add_schema_support_to_entity( + LEAD_ORIGIN, + actor, + THIRD_ENTITY_ID, + FIRST_SCHEMA_ID, + first_schema_property_values, + ); + + // Failure checked + assert_failure( + add_schema_support_to_entity_result, + ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE, + number_of_events_before_call, + ); + }) +} From 44fa3b7cf3a37f167b56b5ee0958e111d066b24c Mon Sep 17 00:00:00 2001 From: iorveth Date: Mon, 10 Aug 2020 19:50:06 +0000 Subject: [PATCH 07/17] apply cargo fmt --- runtime-modules/content-directory/src/lib.rs | 17 ++++++++++++----- .../content-directory/src/schema/convert.rs | 1 - .../content-directory/src/schema/output.rs | 5 +++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index dcfa97e0e1..ea8994b062 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -1238,8 +1238,8 @@ decl_module! { ); let class_id = entity.get_class_id(); - - // Compute hash from unique property value and its respective property id + + // Compute hash from unique property value and its respective property id let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); // Ensure `Property` with `unique` flag set is `unique` on `Class` level @@ -1334,7 +1334,7 @@ decl_module! { let class_id = entity.get_class_id(); - // Compute hash from unique property value and its respective property id + // Compute hash from unique property value and its respective property id let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); // Ensure `Property` with `unique` flag set is `unique` on `Class` level @@ -1796,7 +1796,10 @@ impl Module { unique_property_value_hash: &T::Hash, ) -> Result<(), &'static str> { ensure!( - !>::exists((class_id, property_id), unique_property_value_hash), + !>::exists( + (class_id, property_id), + unique_property_value_hash + ), ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE ); Ok(()) @@ -1808,7 +1811,11 @@ impl Module { unique_property_value_hashes: &BTreeMap, ) -> Result<(), &'static str> { for (&property_id, unique_property_value_hash) in unique_property_value_hashes { - Self::ensure_property_hash_unique_option_satisfied(class_id, property_id, unique_property_value_hash)?; + Self::ensure_property_hash_unique_option_satisfied( + class_id, + property_id, + unique_property_value_hash, + )?; } Ok(()) } diff --git a/runtime-modules/content-directory/src/schema/convert.rs b/runtime-modules/content-directory/src/schema/convert.rs index 8ebd0d9571..2eb578565c 100644 --- a/runtime-modules/content-directory/src/schema/convert.rs +++ b/runtime-modules/content-directory/src/schema/convert.rs @@ -60,4 +60,3 @@ impl From> for VecOutputValue { } } } - diff --git a/runtime-modules/content-directory/src/schema/output.rs b/runtime-modules/content-directory/src/schema/output.rs index ca2517808f..a341beb8b7 100644 --- a/runtime-modules/content-directory/src/schema/output.rs +++ b/runtime-modules/content-directory/src/schema/output.rs @@ -61,7 +61,7 @@ impl OutputPropertyValue { } } - /// Compute hash from unique property value and its respective property_id + /// Compute hash from unique property value and its respective property_id pub fn compute_unique_hash(&self, property_id: PropertyId) -> T::Hash { match self { OutputPropertyValue::Single(output_value) => { @@ -71,7 +71,8 @@ impl OutputPropertyValue { // Do not hash nonce let vector_output_value = vector_output_value.get_vec_value_ref(); - (property_id, vector_output_value).using_encoded(::Hashing::hash) + (property_id, vector_output_value) + .using_encoded(::Hashing::hash) } } } From 0a2d3f8af6159e956fadac3bacf4a4583190e9f0 Mon Sep 17 00:00:00 2001 From: iorveth Date: Mon, 10 Aug 2020 20:37:25 +0000 Subject: [PATCH 08/17] Improve compute_unique_hashes implementation --- .../content-directory/src/helpers.rs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index d8dfbf1924..200a106111 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -98,6 +98,12 @@ impl<'a, T: Trait> OutputValueForExistingProperty<'a, T> { pub fn unzip(&self) -> (&Property, &OutputPropertyValue) { (self.0, self.1) } + + /// Check if Property is default and non `required` + pub fn is_default(&self) -> bool { + let (property, property_value) = self.unzip(); + !property.required && *property_value == OutputPropertyValue::::default() + } } /// Mapping, used to represent `PropertyId` relation to its respective `OutputValuesForExistingProperties` structure @@ -149,16 +155,12 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { /// (skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required`) pub fn compute_unique_hashes(&self) -> BTreeMap { self.iter() - .filter(|(property_id, _)| { - match self - .get(property_id) - .map(|value_for_property| value_for_property.unzip()) - { - Some((property, property_value)) if property.unique => { - // skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required` - property.required || *property_value != OutputPropertyValue::::default() - } - _ => false, + .filter(|(_, value_for_property)| { + if value_for_property.get_property().unique { + // skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required` + !value_for_property.is_default() + } else { + false } }) .map(|(&property_id, property_value)| { From 883863befbbdad929dd1bb0db11714ec96c555be Mon Sep 17 00:00:00 2001 From: iorveth Date: Tue, 11 Aug 2020 10:24:37 +0000 Subject: [PATCH 09/17] Uniqueness feature: fix potential issue with default non required values --- .../content-directory/src/helpers.rs | 21 ++++-- runtime-modules/content-directory/src/lib.rs | 70 +++++++++++++++---- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 200a106111..1632dc674d 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -80,7 +80,7 @@ pub struct OutputValueForExistingProperty<'a, T: Trait>( impl<'a, T: Trait> OutputValueForExistingProperty<'a, T> { /// Create single instance of `OutputValueForExistingProperty` from provided `property` and `value` - fn new(property: &'a Property, value: &'a OutputPropertyValue) -> Self { + pub fn new(property: &'a Property, value: &'a OutputPropertyValue) -> Self { Self(property, value) } @@ -156,12 +156,8 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { pub fn compute_unique_hashes(&self) -> BTreeMap { self.iter() .filter(|(_, value_for_property)| { - if value_for_property.get_property().unique { - // skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required` - !value_for_property.is_default() - } else { - false - } + // skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required` + value_for_property.get_property().unique && !value_for_property.is_default() }) .map(|(&property_id, property_value)| { ( @@ -171,6 +167,17 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { }) .collect() } + + /// Compute ids of property values, that are unique and default. + pub fn compute_unique_default_non_required_ids(&self) -> BTreeSet { + self.iter() + .filter(|(_, value_for_property)| { + value_for_property.get_property().unique && value_for_property.is_default() + }) + .map(|(property_id, _)| property_id) + .copied() + .collect() + } } /// Length constraint for input validation diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index ea8994b062..c10422487c 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -757,12 +757,18 @@ decl_module! { // Ensure all provided Properties with unique flag set are unique on Class level Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_hashes)?; + // Used to remove unique values, that were substituted with default and non required ones (if some). + let deafault_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + // // == MUTATION SAFE == // - // Add property values, that should be unique on Class level - Self::add_unique_property_value_hashes(class_id, new_unique_hashes); + // Add/update property values, that should be unique on Class level + Self::update_unique_property_value_hashes(class_id, new_unique_hashes); + + // Remove unique values, that were substituted with default and non required ones (if some). + Self::remove_unique_property_value_hashes(class_id, deafault_non_required_hashes); // Make updated entity_property_values from parameters provided let entity_property_values_updated = @@ -1018,7 +1024,7 @@ decl_module! { // // Add property values, that should be unique on Class level - Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + Self::update_unique_property_value_hashes(class_id, new_unique_property_value_hashes); // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::calculate_entities_inbound_rcs_delta( @@ -1092,17 +1098,26 @@ decl_module! { // Compute OutputPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_unique_property_value_hashes = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?.compute_unique_hashes(); + let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; + + let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); // Ensure all provided Properties with unique flag set are unique on Class level Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; + + // Used to remove unique values, that were substituted with default and non required ones (if some). + let deafault_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + // // == MUTATION SAFE == // // Update property values, that should be unique on Class level - Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + Self::update_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + + // Remove unique values, that were substituted with default and non required ones (if some). + Self::remove_unique_property_value_hashes(class_id, deafault_non_required_hashes); // Make updated entity_property_values from current entity_property_values and new_output_property_values provided let entity_property_values_updated = @@ -1155,15 +1170,19 @@ decl_module! { let class_id = entity.get_class_id(); // Calculate side effects for clear_property_vector operation, based on property_value_vector provided and its respective property. - let entities_inbound_rcs_delta = Self::make_side_effects_for_clear_property_vector_operation(&property_value_vector, property); + let entities_inbound_rcs_delta = Self::make_side_effects_for_clear_property_vector_operation(&property_value_vector, &property); // Clear property_value_vector. let empty_property_value_vector = Self::clear_property_vector(property_value_vector); let property_value_hash = empty_property_value_vector.compute_unique_hash(in_class_schema_property_id); - // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; + if OutputValueForExistingProperty::new(&property, &empty_property_value_vector).is_default() { + Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, property_value_hash) + } else { + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; + } // // == MUTATION SAFE == @@ -1242,8 +1261,12 @@ decl_module! { // Compute hash from unique property value and its respective property id let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - // Ensure `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; + if OutputValueForExistingProperty::new(&property, &property_value_vector_updated).is_default() { + Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, property_value_hash) + } else { + // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; + } // // == MUTATION SAFE == @@ -1451,7 +1474,7 @@ impl Module { } /// Add/update property value hash, that should be unique on `Class` level - pub fn add_unique_property_value_hash( + pub fn update_unique_property_value_hash( class_id: T::ClassId, property_id: PropertyId, hash: T::Hash, @@ -1468,15 +1491,15 @@ impl Module { >::remove((class_id, property_id), hash); } - /// Add property value hashes, that should be unique on `Class` level - pub fn add_unique_property_value_hashes( + /// Add/update property value hashes, that should be unique on `Class` level + pub fn update_unique_property_value_hashes( class_id: T::ClassId, unique_property_value_hashes: BTreeMap, ) { unique_property_value_hashes .into_iter() .for_each(|(property_id, hash)| { - Self::add_unique_property_value_hash(class_id, property_id, hash); + Self::update_unique_property_value_hash(class_id, property_id, hash); }); } @@ -1540,7 +1563,7 @@ impl Module { /// Returns calculated `ReferenceCounterSideEffects` pub fn make_side_effects_for_clear_property_vector_operation( property_value_vector: &VecOutputPropertyValue, - property: Property, + property: &Property, ) -> Option> { let entity_ids_to_decrease_rc = property_value_vector .get_vec_value_ref() @@ -1990,6 +2013,23 @@ impl Module { .collect() } + /// Used to remove unique values, that were substituted with default and non required ones (if some). + pub fn compute_default_non_required_hashes( + new_output_values_for_existing_properties: OutputValuesForExistingProperties, + entity_values: &BTreeMap>, + ) -> BTreeMap { + let unique_default_non_required_ids = + new_output_values_for_existing_properties.compute_unique_default_non_required_ids(); + + entity_values + .iter() + .filter(|(property_id, _)| unique_default_non_required_ids.contains(property_id)) + .map(|(&property_id, property_value)| { + (property_id, property_value.compute_unique_hash(property_id)) + }) + .collect() + } + /// Perform checks to ensure all required `property_values` under provided `unused_schema_property_ids` provided pub fn ensure_all_required_properties_provided( class_properties: &[Property], From 5623fa851c6c13fca68660193ade9c72db8f676c Mon Sep 17 00:00:00 2001 From: iorveth Date: Tue, 11 Aug 2020 10:44:35 +0000 Subject: [PATCH 10/17] Typo fixed --- runtime-modules/content-directory/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index c10422487c..00f2bd188f 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -758,7 +758,7 @@ decl_module! { Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_hashes)?; // Used to remove unique values, that were substituted with default and non required ones (if some). - let deafault_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + let default_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); // // == MUTATION SAFE == @@ -768,7 +768,7 @@ decl_module! { Self::update_unique_property_value_hashes(class_id, new_unique_hashes); // Remove unique values, that were substituted with default and non required ones (if some). - Self::remove_unique_property_value_hashes(class_id, deafault_non_required_hashes); + Self::remove_unique_property_value_hashes(class_id, default_non_required_hashes); // Make updated entity_property_values from parameters provided let entity_property_values_updated = @@ -1107,7 +1107,7 @@ decl_module! { // Used to remove unique values, that were substituted with default and non required ones (if some). - let deafault_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + let default_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); // // == MUTATION SAFE == @@ -1117,7 +1117,7 @@ decl_module! { Self::update_unique_property_value_hashes(class_id, new_unique_property_value_hashes); // Remove unique values, that were substituted with default and non required ones (if some). - Self::remove_unique_property_value_hashes(class_id, deafault_non_required_hashes); + Self::remove_unique_property_value_hashes(class_id, default_non_required_hashes); // Make updated entity_property_values from current entity_property_values and new_output_property_values provided let entity_property_values_updated = @@ -1180,7 +1180,7 @@ decl_module! { if OutputValueForExistingProperty::new(&property, &empty_property_value_vector).is_default() { Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, property_value_hash) } else { - // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level + // Ensure there is no another empty vector for this, possible, unique property Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; } From 9815057e582b9782c178c20f53ba7c605b8f309a Mon Sep 17 00:00:00 2001 From: iorveth Date: Wed, 12 Aug 2020 11:33:36 +0000 Subject: [PATCH 11/17] Fix uniqueness feature implementation issue --- .../content-directory/src/helpers.rs | 11 -- runtime-modules/content-directory/src/lib.rs | 151 ++++++++++-------- .../content-directory/src/schema/output.rs | 12 +- 3 files changed, 94 insertions(+), 80 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 1632dc674d..868fbaa393 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -167,17 +167,6 @@ impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { }) .collect() } - - /// Compute ids of property values, that are unique and default. - pub fn compute_unique_default_non_required_ids(&self) -> BTreeSet { - self.iter() - .filter(|(_, value_for_property)| { - value_for_property.get_property().unique && value_for_property.is_default() - }) - .map(|(property_id, _)| property_id) - .copied() - .collect() - } } /// Length constraint for input validation diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index 00f2bd188f..665234e2b5 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -757,18 +757,18 @@ decl_module! { // Ensure all provided Properties with unique flag set are unique on Class level Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_hashes)?; - // Used to remove unique values, that were substituted with default and non required ones (if some). - let default_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + // Used to remove unique hashes, that were substituted with new ones. + let old_unique_hashes = Self::compute_old_unique_hashes(&new_output_property_value_references_with_same_owner_flag_set, &entity_property_values); // // == MUTATION SAFE == // // Add/update property values, that should be unique on Class level - Self::update_unique_property_value_hashes(class_id, new_unique_hashes); + Self::add_unique_property_value_hashes(class_id, new_unique_hashes); - // Remove unique values, that were substituted with default and non required ones (if some). - Self::remove_unique_property_value_hashes(class_id, default_non_required_hashes); + // Remove unique hashes, that were substituted with new ones. + Self::remove_unique_property_value_hashes(class_id, old_unique_hashes); // Make updated entity_property_values from parameters provided let entity_property_values_updated = @@ -1023,8 +1023,8 @@ decl_module! { // == MUTATION SAFE == // - // Add property values, that should be unique on Class level - Self::update_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + // Add property value hashes, that should be unique on Class level + Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); // Calculate entities reference counter side effects for current operation let entities_inbound_rcs_delta = Self::calculate_entities_inbound_rcs_delta( @@ -1106,18 +1106,18 @@ decl_module! { Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; - // Used to remove unique values, that were substituted with default and non required ones (if some). - let default_non_required_hashes = Self::compute_default_non_required_hashes(new_output_values_for_existing_properties, &entity_property_values); + // Used to compute old unique hashes, that should be substituted with new ones. + let old_unique_hashes = Self::compute_old_unique_hashes(&new_output_property_values, &entity_property_values); // // == MUTATION SAFE == // - // Update property values, that should be unique on Class level - Self::update_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + // Update property value hashes, that should be unique on Class level + Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); - // Remove unique values, that were substituted with default and non required ones (if some). - Self::remove_unique_property_value_hashes(class_id, default_non_required_hashes); + // Remove unique hashes, that were substituted with new ones. (if some). + Self::remove_unique_property_value_hashes(class_id, old_unique_hashes); // Make updated entity_property_values from current entity_property_values and new_output_property_values provided let entity_property_values_updated = @@ -1167,26 +1167,35 @@ decl_module! { let property_value_vector = entity.ensure_property_value_is_vec(in_class_schema_property_id)?; - let class_id = entity.get_class_id(); - // Calculate side effects for clear_property_vector operation, based on property_value_vector provided and its respective property. let entities_inbound_rcs_delta = Self::make_side_effects_for_clear_property_vector_operation(&property_value_vector, &property); // Clear property_value_vector. - let empty_property_value_vector = Self::clear_property_vector(property_value_vector); + let empty_property_value_vector = Self::clear_property_vector(property_value_vector.clone()); - let property_value_hash = empty_property_value_vector.compute_unique_hash(in_class_schema_property_id); + if property.unique { + let class_id = entity.get_class_id(); - if OutputValueForExistingProperty::new(&property, &empty_property_value_vector).is_default() { - Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, property_value_hash) - } else { - // Ensure there is no another empty vector for this, possible, unique property - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; - } + // Compute new hash from unique property value and its respective property id + let new_property_value_hash = empty_property_value_vector.compute_unique_hash(in_class_schema_property_id); + + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; + + // Compute old hash from the old unique property value and its respective property id + let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); - // - // == MUTATION SAFE == - // + + // + // == MUTATION SAFE == + // + + // Add new property value hash, that should be unique on `Class` level + Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); + + // Remove old property value hash, that should be unique on `Class` level + Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, old_property_value_hash); + } // Decrease reference counters of involved entities (if some) Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -1253,24 +1262,31 @@ decl_module! { // Remove value at in_class_schema_property_id in property value vector // Get VecInputPropertyValue wrapped in InputPropertyValue let property_value_vector_updated = Self::remove_at_index_in_property_vector( - property_value_vector, index_in_property_vector + property_value_vector.clone(), index_in_property_vector ); - let class_id = entity.get_class_id(); + if property.unique { + let class_id = entity.get_class_id(); - // Compute hash from unique property value and its respective property id - let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); + // Compute new hash from unique property value and its respective property id + let new_property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - if OutputValueForExistingProperty::new(&property, &property_value_vector_updated).is_default() { - Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, property_value_hash) - } else { - // Ensure provided `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; - } + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; - // - // == MUTATION SAFE == - // + // Compute old hash from the old unique property value and its respective property id + let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); + + // + // == MUTATION SAFE == + // + + // Add new property value hash, that should be unique on `Class` level + Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); + + // Remove old property value hash, that should be unique on `Class` level + Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, old_property_value_hash); + } // Insert updated propery value into entity_property_values mapping at in_class_schema_property_id. let entity_values_updated = Self::insert_at_in_class_schema_property_id( @@ -1289,8 +1305,6 @@ decl_module! { None }; - - // Update entity property values >::mutate(entity_id, |entity| { entity.set_values(entity_values_updated); @@ -1324,7 +1338,7 @@ decl_module! { // Ensure Property under given PropertyId is unlocked from actor with given EntityAccessLevel // Retrieve corresponding Property by value - let class_property = class.ensure_class_property_type_unlocked_from( + let property = class.ensure_class_property_type_unlocked_from( in_class_schema_property_id, access_level, )?; @@ -1340,7 +1354,7 @@ decl_module! { let entity_controller = entity.get_permissions_ref().get_controller(); // Ensure property_value type is equal to the property_value_vector type and check all constraints - class_property.ensure_property_value_can_be_inserted_at_property_vector( + property.ensure_property_value_can_be_inserted_at_property_vector( &value, &property_value_vector, index_in_property_vector, @@ -1352,20 +1366,31 @@ decl_module! { // Insert SingleInputPropertyValue at in_class_schema_property_id into property value vector // Get VecInputPropertyValue wrapped in InputPropertyValue let property_value_vector_updated = Self::insert_at_index_in_property_vector( - property_value_vector, index_in_property_vector, value + property_value_vector.clone(), index_in_property_vector, value ); - let class_id = entity.get_class_id(); + if property.unique { + let class_id = entity.get_class_id(); - // Compute hash from unique property value and its respective property id - let property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); + // Compute new hash from unique property value and its respective property id + let new_property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - // Ensure `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &property_value_hash)?; + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; - // - // == MUTATION SAFE == - // + // Compute old hash from the old unique property value and its respective property id + let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); + + // + // == MUTATION SAFE == + // + + // Add property value hash, that should be unique on `Class` level + Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); + + // Remove property value hash, that should be unique on `Class` level + Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, old_property_value_hash); + } // Insert updated property value into entity_property_values mapping at in_class_schema_property_id. // Retrieve updated entity_property_values @@ -1375,7 +1400,7 @@ decl_module! { // Increase reference counter of involved entity (if some) let involved_entity_and_side_effect = if let Some(entity_rc_to_increment) = involved_entity { - let same_controller_status = class_property.property_type.same_controller_status(); + let same_controller_status = property.property_type.same_controller_status(); let rc_delta = EntityReferenceCounterSideEffect::atomic(same_controller_status, DeltaMode::Increment); // Update InboundReferenceCounter of involved entity, based on previously calculated ReferenceCounterSideEffect @@ -1473,8 +1498,8 @@ impl Module { }) } - /// Add/update property value hash, that should be unique on `Class` level - pub fn update_unique_property_value_hash( + /// Add property value hash, that should be unique on `Class` level + pub fn add_unique_property_value_hash( class_id: T::ClassId, property_id: PropertyId, hash: T::Hash, @@ -1491,15 +1516,15 @@ impl Module { >::remove((class_id, property_id), hash); } - /// Add/update property value hashes, that should be unique on `Class` level - pub fn update_unique_property_value_hashes( + /// Add property value hashes, that should be unique on `Class` level + pub fn add_unique_property_value_hashes( class_id: T::ClassId, unique_property_value_hashes: BTreeMap, ) { unique_property_value_hashes .into_iter() .for_each(|(property_id, hash)| { - Self::update_unique_property_value_hash(class_id, property_id, hash); + Self::add_unique_property_value_hash(class_id, property_id, hash); }); } @@ -2013,17 +2038,15 @@ impl Module { .collect() } - /// Used to remove unique values, that were substituted with default and non required ones (if some). - pub fn compute_default_non_required_hashes( - new_output_values_for_existing_properties: OutputValuesForExistingProperties, + /// Used to compute old unique hashes, that should be substituted with new ones. + pub fn compute_old_unique_hashes( + new_output_property_values: &BTreeMap>, entity_values: &BTreeMap>, ) -> BTreeMap { - let unique_default_non_required_ids = - new_output_values_for_existing_properties.compute_unique_default_non_required_ids(); entity_values .iter() - .filter(|(property_id, _)| unique_default_non_required_ids.contains(property_id)) + .filter(|(property_id, _)| new_output_property_values.contains_key(property_id)) .map(|(&property_id, property_value)| { (property_id, property_value.compute_unique_hash(property_id)) }) diff --git a/runtime-modules/content-directory/src/schema/output.rs b/runtime-modules/content-directory/src/schema/output.rs index a341beb8b7..3ea699a23f 100644 --- a/runtime-modules/content-directory/src/schema/output.rs +++ b/runtime-modules/content-directory/src/schema/output.rs @@ -68,11 +68,7 @@ impl OutputPropertyValue { (property_id, output_value).using_encoded(::Hashing::hash) } OutputPropertyValue::Vector(vector_output_value) => { - // Do not hash nonce - let vector_output_value = vector_output_value.get_vec_value_ref(); - - (property_id, vector_output_value) - .using_encoded(::Hashing::hash) + vector_output_value.compute_unique_hash(property_id) } } } @@ -126,6 +122,12 @@ pub struct VecOutputPropertyValue { } impl VecOutputPropertyValue { + /// Compute hash from unique vec property value and its respective property_id + pub fn compute_unique_hash(&self, property_id: PropertyId) -> T::Hash { + // Do not hash nonce + (property_id, &self.vec_value).using_encoded(::Hashing::hash) + } + /// Increase nonce by 1 fn increment_nonce(&mut self) -> T::Nonce { self.nonce += T::Nonce::one(); From 1da9b94d1778d2d35b6c40aa3c1fa900dcd786b5 Mon Sep 17 00:00:00 2001 From: iorveth Date: Wed, 12 Aug 2020 13:03:23 +0000 Subject: [PATCH 12/17] Refactoring: uniqueness feature --- runtime-modules/content-directory/src/lib.rs | 205 ++++++++++++------- 1 file changed, 127 insertions(+), 78 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index 665234e2b5..d272e832e4 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -42,7 +42,11 @@ pub use serde::{Deserialize, Serialize}; /// Type, used in diffrent numeric constraints representations type MaxNumber = u32; -//type PropertyHash = T::Hash; +/// Type representing a map for both new and old property values hashes +type PropertyValuesHashes = ( + BTreeMap::Hash>, + BTreeMap::Hash>, +); pub trait Trait: system::Trait + ActorAuthenticator + Debug + Clone { /// The overarching event type. @@ -748,23 +752,17 @@ decl_module! { let new_output_property_value_references_with_same_owner_flag_set = Self::make_output_property_values(new_property_value_references_with_same_owner_flag_set); - let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_value_references_with_same_owner_flag_set)?; - - // Retrieve OutputPropertyValues, which respective Properties have unique flag set - // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_unique_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); - - // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_hashes)?; - - // Used to remove unique hashes, that were substituted with new ones. - let old_unique_hashes = Self::compute_old_unique_hashes(&new_output_property_value_references_with_same_owner_flag_set, &entity_property_values); + // Compute old and new unique property value hashes. + // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level + let (new_unique_hashes, old_unique_hashes) = Self::ensure_property_values_hashes( + class_id, &class_properties, &new_output_property_value_references_with_same_owner_flag_set, &entity_property_values + )?; // // == MUTATION SAFE == // - // Add/update property values, that should be unique on Class level + // Add property values, that should be unique on Class level Self::add_unique_property_value_hashes(class_id, new_unique_hashes); // Remove unique hashes, that were substituted with new ones. @@ -1017,7 +1015,7 @@ decl_module! { let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; + Self::ensure_property_value_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; // // == MUTATION SAFE == @@ -1096,25 +1094,18 @@ decl_module! { let new_output_property_values = Self::make_output_property_values(new_property_values); - // Compute OutputPropertyValues, which respective Properties have unique flag set - // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; - - let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); - - // Ensure all provided Properties with unique flag set are unique on Class level - Self::ensure_property_hashes_unique_option_satisfied(class_id, &new_unique_property_value_hashes)?; - - - // Used to compute old unique hashes, that should be substituted with new ones. - let old_unique_hashes = Self::compute_old_unique_hashes(&new_output_property_values, &entity_property_values); + // Compute old and new unique property value hashes. + // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level + let (new_unique_hashes, old_unique_hashes) = Self::ensure_property_values_hashes( + class_id, &class_properties, &new_output_property_values, &entity_property_values + )?; // // == MUTATION SAFE == // - // Update property value hashes, that should be unique on Class level - Self::add_unique_property_value_hashes(class_id, new_unique_property_value_hashes); + // Add property value hashes, that should be unique on Class level + Self::add_unique_property_value_hashes(class_id, new_unique_hashes); // Remove unique hashes, that were substituted with new ones. (if some). Self::remove_unique_property_value_hashes(class_id, old_unique_hashes); @@ -1173,27 +1164,27 @@ decl_module! { // Clear property_value_vector. let empty_property_value_vector = Self::clear_property_vector(property_value_vector.clone()); - if property.unique { - let class_id = entity.get_class_id(); - - // Compute new hash from unique property value and its respective property id - let new_property_value_hash = empty_property_value_vector.compute_unique_hash(in_class_schema_property_id); - - // Ensure `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; - - // Compute old hash from the old unique property value and its respective property id - let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); + let class_id = entity.get_class_id(); + // Compute old and new vec unique property value hash. + // Ensure new property value hash with `unique` flag set is `unique` on `Class` level + let vec_property_value_hashes = if property.unique { + Some( + Self::ensure_vec_property_value_hashes(class_id, in_class_schema_property_id, &empty_property_value_vector, property_value_vector)? + ) + } else { + None + }; - // - // == MUTATION SAFE == - // + // + // == MUTATION SAFE == + // - // Add new property value hash, that should be unique on `Class` level + if let Some((new_property_value_hash, old_property_value_hash)) = vec_property_value_hashes { + // Add property value hash, that should be unique on `Class` level Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); - // Remove old property value hash, that should be unique on `Class` level + // Remove property value hash, that should be unique on `Class` level Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, old_property_value_hash); } @@ -1265,26 +1256,27 @@ decl_module! { property_value_vector.clone(), index_in_property_vector ); - if property.unique { - let class_id = entity.get_class_id(); - - // Compute new hash from unique property value and its respective property id - let new_property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - - // Ensure `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; + let class_id = entity.get_class_id(); - // Compute old hash from the old unique property value and its respective property id - let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); + // Compute old and new vec unique property value hash. + // Ensure new property value hash with `unique` flag set is `unique` on `Class` level + let vec_property_value_hashes = if property.unique { + Some( + Self::ensure_vec_property_value_hashes(class_id, in_class_schema_property_id, &property_value_vector_updated, property_value_vector)? + ) + } else { + None + }; - // - // == MUTATION SAFE == - // + // + // == MUTATION SAFE == + // - // Add new property value hash, that should be unique on `Class` level + if let Some((new_property_value_hash, old_property_value_hash)) = vec_property_value_hashes { + // Add property value hash, that should be unique on `Class` level Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); - // Remove old property value hash, that should be unique on `Class` level + // Remove property value hash, that should be unique on `Class` level Self::remove_unique_property_value_hash(class_id, in_class_schema_property_id, old_property_value_hash); } @@ -1369,22 +1361,23 @@ decl_module! { property_value_vector.clone(), index_in_property_vector, value ); - if property.unique { - let class_id = entity.get_class_id(); - - // Compute new hash from unique property value and its respective property id - let new_property_value_hash = property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); - - // Ensure `Property` with `unique` flag set is `unique` on `Class` level - Self::ensure_property_hash_unique_option_satisfied(class_id, in_class_schema_property_id, &new_property_value_hash)?; + let class_id = entity.get_class_id(); - // Compute old hash from the old unique property value and its respective property id - let old_property_value_hash = property_value_vector.compute_unique_hash(in_class_schema_property_id); + // Compute old and new vec unique property value hash. + // Ensure new property value hash with `unique` flag set is `unique` on `Class` level + let vec_property_value_hashes = if property.unique { + Some( + Self::ensure_vec_property_value_hashes(class_id, in_class_schema_property_id, &property_value_vector_updated, property_value_vector)? + ) + } else { + None + }; - // - // == MUTATION SAFE == - // + // + // == MUTATION SAFE == + // + if let Some((new_property_value_hash, old_property_value_hash)) = vec_property_value_hashes { // Add property value hash, that should be unique on `Class` level Self::add_unique_property_value_hash(class_id, in_class_schema_property_id, new_property_value_hash); @@ -1837,8 +1830,8 @@ impl Module { } } - /// Ensure `Property` with `unique` flag set is `unique` on `Class` level - pub fn ensure_property_hash_unique_option_satisfied( + /// Ensure property value hash with `unique` flag set is `unique` on `Class` level + pub fn ensure_property_value_hash_unique_option_satisfied( class_id: T::ClassId, property_id: PropertyId, unique_property_value_hash: &T::Hash, @@ -1853,13 +1846,13 @@ impl Module { Ok(()) } - /// Ensure all `Properties` with `unique` flag set are `unique` on `Class` level - pub fn ensure_property_hashes_unique_option_satisfied( + /// Ensure all property value hashes with `unique` flag set are `unique` on `Class` level + pub fn ensure_property_value_hashes_unique_option_satisfied( class_id: T::ClassId, unique_property_value_hashes: &BTreeMap, ) -> Result<(), &'static str> { for (&property_id, unique_property_value_hash) in unique_property_value_hashes { - Self::ensure_property_hash_unique_option_satisfied( + Self::ensure_property_value_hash_unique_option_satisfied( class_id, property_id, unique_property_value_hash, @@ -1868,6 +1861,63 @@ impl Module { Ok(()) } + /// Compute old and new vec unique property value hash. + /// Ensure new property value hash with `unique` flag set is `unique` on `Class` level + pub fn ensure_vec_property_value_hashes( + class_id: T::ClassId, + in_class_schema_property_id: PropertyId, + property_value_vector_updated: &OutputPropertyValue, + property_value_vector: VecOutputPropertyValue, + ) -> Result<(T::Hash, T::Hash), &'static str> { + // Compute new hash from unique property value and its respective property id + let new_property_value_hash = + property_value_vector_updated.compute_unique_hash(in_class_schema_property_id); + + // Ensure `Property` with `unique` flag set is `unique` on `Class` level + Self::ensure_property_value_hash_unique_option_satisfied( + class_id, + in_class_schema_property_id, + &new_property_value_hash, + )?; + + // Compute old hash from the old unique property value and its respective property id + let old_property_value_hash = + property_value_vector.compute_unique_hash(in_class_schema_property_id); + + Ok((new_property_value_hash, old_property_value_hash)) + } + + /// Compute old and new unique property value hashes. + /// Ensure new property value hashes with `unique` flag set are `unique` on `Class` level + pub fn ensure_property_values_hashes( + class_id: T::ClassId, + class_properties: &[Property], + new_output_property_values: &BTreeMap>, + entity_property_values: &BTreeMap>, + ) -> Result, &'static str> { + // Compute OutputPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from( + &class_properties, + &new_output_property_values, + )?; + + let new_unique_property_value_hashes = + new_output_values_for_existing_properties.compute_unique_hashes(); + + // Ensure all provided Properties with unique flag set are unique on Class level + Self::ensure_property_value_hashes_unique_option_satisfied( + class_id, + &new_unique_property_value_hashes, + )?; + + // Used to compute old unique hashes, that should be substituted with new ones. + let old_unique_hashes = + Self::compute_old_unique_hashes(&new_output_property_values, entity_property_values); + + Ok((new_unique_property_value_hashes, old_unique_hashes)) + } + /// Returns the stored `Class` if exist, error otherwise. fn ensure_class_exists(class_id: T::ClassId) -> Result, &'static str> { ensure!(>::exists(class_id), ERROR_CLASS_NOT_FOUND); @@ -2043,7 +2093,6 @@ impl Module { new_output_property_values: &BTreeMap>, entity_values: &BTreeMap>, ) -> BTreeMap { - entity_values .iter() .filter(|(property_id, _)| new_output_property_values.contains_key(property_id)) From 2afe21228074aa7ee8d8ee53d86442ca1fedda4f Mon Sep 17 00:00:00 2001 From: iorveth Date: Fri, 14 Aug 2020 09:47:26 +0000 Subject: [PATCH 13/17] Renaming: Output -> Stored --- .../content-directory/src/entity.rs | 14 +- .../content-directory/src/helpers.rs | 48 ++--- runtime-modules/content-directory/src/lib.rs | 108 ++++++------ .../content-directory/src/schema.rs | 22 +-- .../content-directory/src/schema/convert.rs | 52 +++--- .../content-directory/src/schema/output.rs | 166 +++++++++--------- .../tests/insert_at_entity_property_vector.rs | 48 ++--- 7 files changed, 229 insertions(+), 229 deletions(-) diff --git a/runtime-modules/content-directory/src/entity.rs b/runtime-modules/content-directory/src/entity.rs index cabcad3cce..9bf5f3d723 100644 --- a/runtime-modules/content-directory/src/entity.rs +++ b/runtime-modules/content-directory/src/entity.rs @@ -18,7 +18,7 @@ pub struct Entity { /// Values for properties on class that are used by some schema used by this entity /// Length is no more than Class.properties. - values: BTreeMap>, + values: BTreeMap>, /// Number of property values referencing current entity reference_counter: InboundReferenceCounter, @@ -42,7 +42,7 @@ impl Entity { controller: EntityController, class_id: T::ClassId, supported_schemas: BTreeSet, - values: BTreeMap>, + values: BTreeMap>, ) -> Self { Self { entity_permissions: EntityPermissions::::default_with_controller(controller), @@ -64,22 +64,22 @@ impl Entity { } /// Get `Entity` values by value - pub fn get_values(self) -> BTreeMap> { + pub fn get_values(self) -> BTreeMap> { self.values } /// Get `Entity` values by reference - pub fn get_values_ref(&self) -> &BTreeMap> { + pub fn get_values_ref(&self) -> &BTreeMap> { &self.values } /// Get `Entity` values by mutable reference - pub fn get_values_mut(&mut self) -> &mut BTreeMap> { + pub fn get_values_mut(&mut self) -> &mut BTreeMap> { &mut self.values } /// Get mutable reference to `Entity` values - pub fn set_values(&mut self, new_values: BTreeMap>) { + pub fn set_values(&mut self, new_values: BTreeMap>) { self.values = new_values; } @@ -128,7 +128,7 @@ impl Entity { pub fn ensure_property_value_is_vec( &self, in_class_schema_property_id: PropertyId, - ) -> Result, &'static str> { + ) -> Result, &'static str> { self.values .get(&in_class_schema_property_id) // Throw an error if a property was not found on entity diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 868fbaa393..a310608917 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -72,15 +72,15 @@ impl<'a, T: Trait> InputValuesForExistingProperties<'a, T> { } } -/// Wrapper for existing `OutputPropertyValue` and its respective `Class` `Property` -pub struct OutputValueForExistingProperty<'a, T: Trait>( +/// Wrapper for existing `StoredPropertyValue` and its respective `Class` `Property` +pub struct StoredValueForExistingProperty<'a, T: Trait>( &'a Property, - &'a OutputPropertyValue, + &'a StoredPropertyValue, ); -impl<'a, T: Trait> OutputValueForExistingProperty<'a, T> { - /// Create single instance of `OutputValueForExistingProperty` from provided `property` and `value` - pub fn new(property: &'a Property, value: &'a OutputPropertyValue) -> Self { +impl<'a, T: Trait> StoredValueForExistingProperty<'a, T> { + /// Create single instance of `StoredValueForExistingProperty` from provided `property` and `value` + pub fn new(property: &'a Property, value: &'a StoredPropertyValue) -> Self { Self(property, value) } @@ -89,69 +89,69 @@ impl<'a, T: Trait> OutputValueForExistingProperty<'a, T> { self.0 } - /// Retrieve `OutputPropertyValue` reference - pub fn get_value(&self) -> &OutputPropertyValue { + /// Retrieve `StoredPropertyValue` reference + pub fn get_value(&self) -> &StoredPropertyValue { self.1 } - /// Retrieve `Property` and `OutputPropertyValue` references - pub fn unzip(&self) -> (&Property, &OutputPropertyValue) { + /// Retrieve `Property` and `StoredPropertyValue` references + pub fn unzip(&self) -> (&Property, &StoredPropertyValue) { (self.0, self.1) } /// Check if Property is default and non `required` pub fn is_default(&self) -> bool { let (property, property_value) = self.unzip(); - !property.required && *property_value == OutputPropertyValue::::default() + !property.required && *property_value == StoredPropertyValue::::default() } } -/// Mapping, used to represent `PropertyId` relation to its respective `OutputValuesForExistingProperties` structure -pub struct OutputValuesForExistingProperties<'a, T: Trait>( - BTreeMap>, +/// Mapping, used to represent `PropertyId` relation to its respective `StoredValuesForExistingProperties` structure +pub struct StoredValuesForExistingProperties<'a, T: Trait>( + BTreeMap>, ); -impl<'a, T: Trait> Default for OutputValuesForExistingProperties<'a, T> { +impl<'a, T: Trait> Default for StoredValuesForExistingProperties<'a, T> { fn default() -> Self { Self(BTreeMap::default()) } } -impl<'a, T: Trait> Deref for OutputValuesForExistingProperties<'a, T> { - type Target = BTreeMap>; +impl<'a, T: Trait> Deref for StoredValuesForExistingProperties<'a, T> { + type Target = BTreeMap>; fn deref(&self) -> &Self::Target { &self.0 } } -impl<'a, T: Trait> DerefMut for OutputValuesForExistingProperties<'a, T> { +impl<'a, T: Trait> DerefMut for StoredValuesForExistingProperties<'a, T> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } -impl<'a, T: Trait> OutputValuesForExistingProperties<'a, T> { - /// Create `OutputValuesForExistingProperties` helper structure from provided `property_values` and their corresponding `Class` properties. +impl<'a, T: Trait> StoredValuesForExistingProperties<'a, T> { + /// Create `StoredValuesForExistingProperties` helper structure from provided `property_values` and their corresponding `Class` properties. /// Throws an error, when `Class` `Property` under `property_id`, corresponding to provided `property_value` not found pub fn from( properties: &'a [Property], - property_values: &'a BTreeMap>, + property_values: &'a BTreeMap>, ) -> Result { - let mut values_for_existing_properties = OutputValuesForExistingProperties::::default(); + let mut values_for_existing_properties = StoredValuesForExistingProperties::::default(); for (&property_id, property_value) in property_values { let property = properties .get(property_id as usize) .ok_or(ERROR_CLASS_PROP_NOT_FOUND)?; values_for_existing_properties.insert( property_id, - OutputValueForExistingProperty::new(property, property_value), + StoredValueForExistingProperty::new(property, property_value), ); } Ok(values_for_existing_properties) } - /// Used to compute hashes from `OutputPropertyValue`s and their respective property ids, which respective `Properties` have `unique` flag set + /// Used to compute hashes from `StoredPropertyValue`s and their respective property ids, which respective `Properties` have `unique` flag set /// (skip `PropertyId`s, which respective `property values` under this `Entity` are default and non `required`) pub fn compute_unique_hashes(&self) -> BTreeMap { self.iter() diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index d272e832e4..f1a53fc171 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -717,7 +717,7 @@ decl_module! { let entity_property_values = entity.get_values(); // Create wrapper structure from provided entity_property_values and their corresponding Class properties - let values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &entity_property_values)?; + let values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &entity_property_values)?; // Filter provided values_for_existing_properties, leaving only `Reference`'s with `SameOwner` flag set // Retrieve the set of corresponding property ids @@ -925,7 +925,7 @@ decl_module! { let entity_values = entity.get_values(); - let unique_property_value_hashes = OutputValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique_hashes(); + let unique_property_value_hashes = StoredValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique_hashes(); // // == MUTATION SAFE == @@ -1008,9 +1008,9 @@ decl_module! { schema, entity_property_values, &new_output_property_values ); - let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; + let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; - // Retrieve OutputPropertyValues, which respective Properties have unique flag set + // Retrieve StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); @@ -1533,10 +1533,10 @@ impl Module { }); } - /// Convert all provided `InputPropertyValue`'s into `OutputPropertyValue`'s + /// Convert all provided `InputPropertyValue`'s into `StoredPropertyValue`'s pub fn make_output_property_values( input_property_values: BTreeMap>, - ) -> BTreeMap> { + ) -> BTreeMap> { input_property_values .into_iter() .map(|(property_id, property_value)| (property_id, property_value.into())) @@ -1547,23 +1547,23 @@ impl Module { /// Returns updated `entity_property_values` fn make_updated_entity_property_values( schema: Schema, - entity_property_values: BTreeMap>, - output_property_values: &BTreeMap>, - ) -> BTreeMap> { + entity_property_values: BTreeMap>, + output_property_values: &BTreeMap>, + ) -> BTreeMap> { // Concatenate existing `entity_property_values` with `property_values`, provided, when adding `Schema` support. - let updated_entity_property_values: BTreeMap> = + let updated_entity_property_values: BTreeMap> = entity_property_values .into_iter() .chain(output_property_values.to_owned().into_iter()) .collect(); // Write all missing non required `Schema` `property_values` as `InputPropertyValue::default()` - let non_required_property_values: BTreeMap> = schema + let non_required_property_values: BTreeMap> = schema .get_properties() .iter() .filter_map(|property_id| { if !updated_entity_property_values.contains_key(property_id) { - Some((*property_id, OutputPropertyValue::default())) + Some((*property_id, StoredPropertyValue::default())) } else { None } @@ -1580,7 +1580,7 @@ impl Module { /// Calculate side effects for clear_property_vector operation, based on `property_value_vector` provided and its respective `property`. /// Returns calculated `ReferenceCounterSideEffects` pub fn make_side_effects_for_clear_property_vector_operation( - property_value_vector: &VecOutputPropertyValue, + property_value_vector: &VecStoredPropertyValue, property: &Property, ) -> Option> { let entity_ids_to_decrease_rc = property_value_vector @@ -1647,7 +1647,7 @@ impl Module { /// Returns calculated `ReferenceCounterSideEffects` fn calculate_entities_inbound_rcs_delta( current_entity_id: T::EntityId, - values_for_existing_properties: OutputValuesForExistingProperties, + values_for_existing_properties: StoredValuesForExistingProperties, delta_mode: DeltaMode, ) -> Option> { let entities_inbound_rcs_delta = values_for_existing_properties @@ -1694,11 +1694,11 @@ impl Module { pub fn get_updated_inbound_rcs_delta( current_entity_id: T::EntityId, class_properties: Vec>, - entity_property_values: BTreeMap>, - new_output_property_values: BTreeMap>, + entity_property_values: BTreeMap>, + new_output_property_values: BTreeMap>, ) -> Result>, &'static str> { // Filter entity_property_values to get only those, which will be substituted with new_property_values - let entity_property_values_to_update: BTreeMap> = + let entity_property_values_to_update: BTreeMap> = entity_property_values .into_iter() .filter(|(entity_id, _)| new_output_property_values.contains_key(entity_id)) @@ -1710,7 +1710,7 @@ impl Module { // as involved InputPropertyValue References will be substituted with new ones let decremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( current_entity_id, - OutputValuesForExistingProperties::from( + StoredValuesForExistingProperties::from( &class_properties, &entity_property_values_to_update, )?, @@ -1721,7 +1721,7 @@ impl Module { // as involved InputPropertyValue References will substitute the old ones let incremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( current_entity_id, - OutputValuesForExistingProperties::from( + StoredValuesForExistingProperties::from( &class_properties, &new_output_property_values, )?, @@ -1866,8 +1866,8 @@ impl Module { pub fn ensure_vec_property_value_hashes( class_id: T::ClassId, in_class_schema_property_id: PropertyId, - property_value_vector_updated: &OutputPropertyValue, - property_value_vector: VecOutputPropertyValue, + property_value_vector_updated: &StoredPropertyValue, + property_value_vector: VecStoredPropertyValue, ) -> Result<(T::Hash, T::Hash), &'static str> { // Compute new hash from unique property value and its respective property id let new_property_value_hash = @@ -1892,12 +1892,12 @@ impl Module { pub fn ensure_property_values_hashes( class_id: T::ClassId, class_properties: &[Property], - new_output_property_values: &BTreeMap>, - entity_property_values: &BTreeMap>, + new_output_property_values: &BTreeMap>, + entity_property_values: &BTreeMap>, ) -> Result, &'static str> { - // Compute OutputPropertyValues, which respective Properties have unique flag set + // Compute StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_output_values_for_existing_properties = OutputValuesForExistingProperties::from( + let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from( &class_properties, &new_output_property_values, )?; @@ -1963,7 +1963,7 @@ impl Module { /// Filter `provided values_for_existing_properties`, leaving only `Reference`'s with `SameOwner` flag set /// Returns the set of corresponding property ids pub fn get_property_id_references_with_same_owner_flag_set( - values_for_existing_properties: OutputValuesForExistingProperties, + values_for_existing_properties: StoredValuesForExistingProperties, ) -> BTreeSet { values_for_existing_properties // Iterate over the PropertyId's @@ -2028,12 +2028,12 @@ impl Module { /// Returns updated `entity_property_values`, if update performed pub fn make_updated_property_value_references_with_same_owner_flag_set( unused_property_id_references_with_same_owner_flag_set: BTreeSet, - entity_property_values: &BTreeMap>, + entity_property_values: &BTreeMap>, new_property_value_references_with_same_owner_flag_set: &BTreeMap< PropertyId, - OutputPropertyValue, + StoredPropertyValue, >, - ) -> Option>> { + ) -> Option>> { // Used to check if update performed let mut entity_property_values_updated = entity_property_values.clone(); @@ -2054,7 +2054,7 @@ impl Module { { entity_property_values_updated.insert( unused_property_id_reference_with_same_owner_flag_set, - OutputPropertyValue::default(), + StoredPropertyValue::default(), ); } @@ -2090,8 +2090,8 @@ impl Module { /// Used to compute old unique hashes, that should be substituted with new ones. pub fn compute_old_unique_hashes( - new_output_property_values: &BTreeMap>, - entity_values: &BTreeMap>, + new_output_property_values: &BTreeMap>, + entity_values: &BTreeMap>, ) -> BTreeMap { entity_values .iter() @@ -2136,7 +2136,7 @@ impl Module { /// Ensure all provided `new_property_values` are already exist in `entity_property_values` map pub fn ensure_all_property_values_are_already_added( - entity_property_values: &BTreeMap>, + entity_property_values: &BTreeMap>, new_property_values: &BTreeMap>, ) -> dispatch::Result { ensure!( @@ -2165,14 +2165,14 @@ impl Module { /// Filter `new_property_values` identical to `entity_property_values`. /// Return only `new_property_values`, that are not in `entity_property_values` pub fn try_filter_identical_property_values( - entity_property_values: &BTreeMap>, + entity_property_values: &BTreeMap>, new_property_values: BTreeMap>, ) -> BTreeMap> { new_property_values .into_iter() .filter(|(id, new_property_value)| { if let Some(entity_property_value) = entity_property_values.get(id) { - OutputPropertyValue::::from(new_property_value.to_owned()) + StoredPropertyValue::::from(new_property_value.to_owned()) != *entity_property_value } else { true @@ -2184,9 +2184,9 @@ impl Module { /// Update existing `entity_property_values` with `new_property_values`. /// if update performed, returns updated entity property values pub fn make_updated_property_values( - entity_property_values: &BTreeMap>, - new_output_property_values: &BTreeMap>, - ) -> Option>> { + entity_property_values: &BTreeMap>, + new_output_property_values: &BTreeMap>, + ) -> Option>> { // Used to check if updated performed let mut entity_property_values_updated = entity_property_values.to_owned(); @@ -2205,43 +2205,43 @@ impl Module { } } - /// Insert `InputValue` into `VecOutputPropertyValue` at `index_in_property_vector`. - /// Returns `VecOutputPropertyValue` wrapped in `OutputPropertyValue` + /// Insert `InputValue` into `VecStoredPropertyValue` at `index_in_property_vector`. + /// Returns `VecStoredPropertyValue` wrapped in `StoredPropertyValue` pub fn insert_at_index_in_property_vector( - mut property_value_vector: VecOutputPropertyValue, + mut property_value_vector: VecStoredPropertyValue, index_in_property_vector: VecMaxLength, value: InputValue, - ) -> OutputPropertyValue { + ) -> StoredPropertyValue { property_value_vector.insert_at(index_in_property_vector, value.into()); - OutputPropertyValue::Vector(property_value_vector) + StoredPropertyValue::Vector(property_value_vector) } /// Remove `InputValue` at `index_in_property_vector` in `VecInputPropertyValue`. /// Returns `VecInputPropertyValue` wrapped in `InputPropertyValue` pub fn remove_at_index_in_property_vector( - mut property_value_vector: VecOutputPropertyValue, + mut property_value_vector: VecStoredPropertyValue, index_in_property_vector: VecMaxLength, - ) -> OutputPropertyValue { + ) -> StoredPropertyValue { property_value_vector.remove_at(index_in_property_vector); - OutputPropertyValue::Vector(property_value_vector) + StoredPropertyValue::Vector(property_value_vector) } - /// Clear `VecOutputPropertyValue`. - /// Returns empty `VecOutputPropertyValue` wrapped in `OutputPropertyValue` + /// Clear `VecStoredPropertyValue`. + /// Returns empty `VecStoredPropertyValue` wrapped in `StoredPropertyValue` pub fn clear_property_vector( - mut property_value_vector: VecOutputPropertyValue, - ) -> OutputPropertyValue { + mut property_value_vector: VecStoredPropertyValue, + ) -> StoredPropertyValue { property_value_vector.clear(); - OutputPropertyValue::Vector(property_value_vector) + StoredPropertyValue::Vector(property_value_vector) } /// Insert `InputPropertyValue` into `entity_property_values` mapping at `in_class_schema_property_id`. /// Returns updated `entity_property_values` pub fn insert_at_in_class_schema_property_id( - mut entity_property_values: BTreeMap>, + mut entity_property_values: BTreeMap>, in_class_schema_property_id: PropertyId, - property_value: OutputPropertyValue, - ) -> BTreeMap> { + property_value: StoredPropertyValue, + ) -> BTreeMap> { entity_property_values.insert(in_class_schema_property_id, property_value); entity_property_values } diff --git a/runtime-modules/content-directory/src/schema.rs b/runtime-modules/content-directory/src/schema.rs index c444e30dde..c0e3bfac88 100644 --- a/runtime-modules/content-directory/src/schema.rs +++ b/runtime-modules/content-directory/src/schema.rs @@ -356,7 +356,7 @@ impl Property { pub fn ensure_property_value_can_be_inserted_at_property_vector( &self, single_value: &InputValue, - vec_value: &VecOutputPropertyValue, + vec_value: &VecStoredPropertyValue, index_in_property_vec: VecMaxLength, current_entity_controller: &EntityController, ) -> dispatch::Result { @@ -388,34 +388,34 @@ impl Property { property_type_vec.get_vec_type(), ) { // Single values - (InputValue::Bool(_), VecOutputValue::Bool(vec), Type::Bool) => { + (InputValue::Bool(_), VecStoredValue::Bool(vec), Type::Bool) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Uint16(_), VecOutputValue::Uint16(vec), Type::Uint16) => { + (InputValue::Uint16(_), VecStoredValue::Uint16(vec), Type::Uint16) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Uint32(_), VecOutputValue::Uint32(vec), Type::Uint32) => { + (InputValue::Uint32(_), VecStoredValue::Uint32(vec), Type::Uint32) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Uint64(_), VecOutputValue::Uint64(vec), Type::Uint64) => { + (InputValue::Uint64(_), VecStoredValue::Uint64(vec), Type::Uint64) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Int16(_), VecOutputValue::Int16(vec), Type::Int16) => { + (InputValue::Int16(_), VecStoredValue::Int16(vec), Type::Int16) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Int32(_), VecOutputValue::Int32(vec), Type::Int32) => { + (InputValue::Int32(_), VecStoredValue::Int32(vec), Type::Int32) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Int64(_), VecOutputValue::Int64(vec), Type::Int64) => { + (InputValue::Int64(_), VecStoredValue::Int64(vec), Type::Int64) => { validate_property_vector_length_after_value_insert(vec, max_vec_len) } - (InputValue::Text(text_item), VecOutputValue::Text(vec), Type::Text(text_max_len)) => { + (InputValue::Text(text_item), VecStoredValue::Text(vec), Type::Text(text_max_len)) => { Self::validate_max_len_of_text(text_item, *text_max_len)?; validate_property_vector_length_after_value_insert(vec, max_vec_len) } ( InputValue::TextToHash(text_item), - VecOutputValue::Hash(vec), + VecStoredValue::Hash(vec), Type::Hash(text_max_len), ) => { if let Some(text_max_len) = text_max_len { @@ -425,7 +425,7 @@ impl Property { } ( InputValue::Reference(entity_id), - VecOutputValue::Reference(vec), + VecStoredValue::Reference(vec), Type::Reference(class_id, same_controller_status), ) => { // Ensure class_id of Entity under provided entity_id references Entity, diff --git a/runtime-modules/content-directory/src/schema/convert.rs b/runtime-modules/content-directory/src/schema/convert.rs index 2eb578565c..8516a1350e 100644 --- a/runtime-modules/content-directory/src/schema/convert.rs +++ b/runtime-modules/content-directory/src/schema/convert.rs @@ -1,62 +1,62 @@ use super::*; use runtime_primitives::traits::Hash; -impl From> for OutputPropertyValue { +impl From> for StoredPropertyValue { fn from(input_property_value: InputPropertyValue) -> Self { match input_property_value { InputPropertyValue::Single(input_value) => { - OutputPropertyValue::Single(input_value.into()) + StoredPropertyValue::Single(input_value.into()) } InputPropertyValue::Vector(vector_input_value) => { let vec_output_property_value = - VecOutputPropertyValue::new(vector_input_value.into(), T::Nonce::default()); - OutputPropertyValue::Vector(vec_output_property_value) + VecStoredPropertyValue::new(vector_input_value.into(), T::Nonce::default()); + StoredPropertyValue::Vector(vec_output_property_value) } } } } -impl From> for OutputValue { +impl From> for StoredValue { fn from(input_value: InputValue) -> Self { match input_value { - InputValue::Bool(value) => OutputValue::Bool(value), - InputValue::Uint16(value) => OutputValue::Uint16(value), - InputValue::Uint32(value) => OutputValue::Uint32(value), - InputValue::Uint64(value) => OutputValue::Uint64(value), - InputValue::Int16(value) => OutputValue::Int16(value), - InputValue::Int32(value) => OutputValue::Int32(value), - InputValue::Int64(value) => OutputValue::Int64(value), - InputValue::Text(value) => OutputValue::Text(value), + InputValue::Bool(value) => StoredValue::Bool(value), + InputValue::Uint16(value) => StoredValue::Uint16(value), + InputValue::Uint32(value) => StoredValue::Uint32(value), + InputValue::Uint64(value) => StoredValue::Uint64(value), + InputValue::Int16(value) => StoredValue::Int16(value), + InputValue::Int32(value) => StoredValue::Int32(value), + InputValue::Int64(value) => StoredValue::Int64(value), + InputValue::Text(value) => StoredValue::Text(value), InputValue::TextToHash(value) => { let hash_value = value.using_encoded(::Hashing::hash); - OutputValue::Hash(hash_value) + StoredValue::Hash(hash_value) } - InputValue::Reference(value) => OutputValue::Reference(value), + InputValue::Reference(value) => StoredValue::Reference(value), } } } -impl From> for VecOutputValue { +impl From> for VecStoredValue { fn from(vec_input_value: VecInputValue) -> Self { match vec_input_value { - VecInputValue::Bool(vec_value) => VecOutputValue::Bool(vec_value), - VecInputValue::Uint16(vec_value) => VecOutputValue::Uint16(vec_value), - VecInputValue::Uint32(vec_value) => VecOutputValue::Uint32(vec_value), - VecInputValue::Uint64(vec_value) => VecOutputValue::Uint64(vec_value), - VecInputValue::Int16(vec_value) => VecOutputValue::Int16(vec_value), - VecInputValue::Int32(vec_value) => VecOutputValue::Int32(vec_value), - VecInputValue::Int64(vec_value) => VecOutputValue::Int64(vec_value), - VecInputValue::Text(vec_value) => VecOutputValue::Text(vec_value), + VecInputValue::Bool(vec_value) => VecStoredValue::Bool(vec_value), + VecInputValue::Uint16(vec_value) => VecStoredValue::Uint16(vec_value), + VecInputValue::Uint32(vec_value) => VecStoredValue::Uint32(vec_value), + VecInputValue::Uint64(vec_value) => VecStoredValue::Uint64(vec_value), + VecInputValue::Int16(vec_value) => VecStoredValue::Int16(vec_value), + VecInputValue::Int32(vec_value) => VecStoredValue::Int32(vec_value), + VecInputValue::Int64(vec_value) => VecStoredValue::Int64(vec_value), + VecInputValue::Text(vec_value) => VecStoredValue::Text(vec_value), VecInputValue::TextToHash(vec_value) => { let hash_vec_value: Vec<_> = vec_value .into_iter() .map(|value| value.using_encoded(::Hashing::hash)) .collect(); - VecOutputValue::Hash(hash_vec_value) + VecStoredValue::Hash(hash_vec_value) } - VecInputValue::Reference(value) => VecOutputValue::Reference(value), + VecInputValue::Reference(value) => VecStoredValue::Reference(value), } } } diff --git a/runtime-modules/content-directory/src/schema/output.rs b/runtime-modules/content-directory/src/schema/output.rs index 3ea699a23f..48c5c85db9 100644 --- a/runtime-modules/content-directory/src/schema/output.rs +++ b/runtime-modules/content-directory/src/schema/output.rs @@ -1,40 +1,40 @@ use super::*; use runtime_primitives::traits::Hash; -/// Enum, representing either `OutputValue` or `VecOutputPropertyValue` +/// Enum, representing either `StoredValue` or `VecStoredPropertyValue` #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug)] -pub enum OutputPropertyValue { - Single(OutputValue), - Vector(VecOutputPropertyValue), +pub enum StoredPropertyValue { + Single(StoredValue), + Vector(VecStoredPropertyValue), } -impl OutputPropertyValue { - pub fn as_single_value(&self) -> Option<&OutputValue> { - if let OutputPropertyValue::Single(single_value) = self { +impl StoredPropertyValue { + pub fn as_single_value(&self) -> Option<&StoredValue> { + if let StoredPropertyValue::Single(single_value) = self { Some(single_value) } else { None } } - pub fn as_vec_property_value(&self) -> Option<&VecOutputPropertyValue> { - if let OutputPropertyValue::Vector(vec_property_value) = self { + pub fn as_vec_property_value(&self) -> Option<&VecStoredPropertyValue> { + if let StoredPropertyValue::Vector(vec_property_value) = self { Some(vec_property_value) } else { None } } - pub fn as_vec_property_value_mut(&mut self) -> Option<&mut VecOutputPropertyValue> { - if let OutputPropertyValue::Vector(vec_property_value) = self { + pub fn as_vec_property_value_mut(&mut self) -> Option<&mut VecStoredPropertyValue> { + if let StoredPropertyValue::Vector(vec_property_value) = self { Some(vec_property_value) } else { None } } - /// Update `Self` with provided `OutputPropertyValue` + /// Update `Self` with provided `StoredPropertyValue` pub fn update(&mut self, mut new_value: Self) { if let (Some(vec_property_value), Some(new_vec_property_value)) = ( self.as_vec_property_value_mut(), @@ -45,17 +45,17 @@ impl OutputPropertyValue { *self = new_value } - /// Retrieve all involved `entity_id`'s, if current `OutputPropertyValue` is reference + /// Retrieve all involved `entity_id`'s, if current `StoredPropertyValue` is reference pub fn get_involved_entities(&self) -> Option> { match self { - OutputPropertyValue::Single(single_property_value) => { + StoredPropertyValue::Single(single_property_value) => { if let Some(entity_id) = single_property_value.get_involved_entity() { Some(vec![entity_id]) } else { None } } - OutputPropertyValue::Vector(vector_property_value) => vector_property_value + StoredPropertyValue::Vector(vector_property_value) => vector_property_value .get_vec_value_ref() .get_involved_entities(), } @@ -64,26 +64,26 @@ impl OutputPropertyValue { /// Compute hash from unique property value and its respective property_id pub fn compute_unique_hash(&self, property_id: PropertyId) -> T::Hash { match self { - OutputPropertyValue::Single(output_value) => { + StoredPropertyValue::Single(output_value) => { (property_id, output_value).using_encoded(::Hashing::hash) } - OutputPropertyValue::Vector(vector_output_value) => { + StoredPropertyValue::Vector(vector_output_value) => { vector_output_value.compute_unique_hash(property_id) } } } } -impl Default for OutputPropertyValue { +impl Default for StoredPropertyValue { fn default() -> Self { - OutputPropertyValue::Single(OutputValue::default()) + StoredPropertyValue::Single(StoredValue::default()) } } -/// OutputValue enum representation, related to corresponding `SingleOutputPropertyValue` structure +/// StoredValue enum representation, related to corresponding `SingleStoredPropertyValue` structure #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Hash, Clone, PartialEq, PartialOrd, Ord, Eq, Debug)] -pub enum OutputValue { +pub enum StoredValue { Bool(bool), Uint16(u16), Uint32(u32), @@ -96,16 +96,16 @@ pub enum OutputValue { Reference(T::EntityId), } -impl Default for OutputValue { - fn default() -> OutputValue { +impl Default for StoredValue { + fn default() -> StoredValue { Self::Bool(false) } } -impl OutputValue { - /// Retrieve involved `entity_id`, if current `OutputValue` is reference +impl StoredValue { + /// Retrieve involved `entity_id`, if current `StoredValue` is reference pub fn get_involved_entity(&self) -> Option { - if let OutputValue::Reference(entity_id) = self { + if let StoredValue::Reference(entity_id) = self { Some(*entity_id) } else { None @@ -113,15 +113,15 @@ impl OutputValue { } } -/// Consists of `VecOutputPropertyValue` enum representation and `nonce`, used to avoid vector data race update conditions +/// Consists of `VecStoredPropertyValue` enum representation and `nonce`, used to avoid vector data race update conditions #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Default, Clone, Debug, PartialEq, Eq)] -pub struct VecOutputPropertyValue { - vec_value: VecOutputValue, +pub struct VecStoredPropertyValue { + vec_value: VecStoredValue, nonce: T::Nonce, } -impl VecOutputPropertyValue { +impl VecStoredPropertyValue { /// Compute hash from unique vec property value and its respective property_id pub fn compute_unique_hash(&self, property_id: PropertyId) -> T::Hash { // Do not hash nonce @@ -134,48 +134,48 @@ impl VecOutputPropertyValue { self.nonce } - pub fn new(vec_value: VecOutputValue, nonce: T::Nonce) -> Self { + pub fn new(vec_value: VecStoredValue, nonce: T::Nonce) -> Self { Self { vec_value, nonce } } - /// Retrieve `VecOutputValue` - pub fn get_vec_value(self) -> VecOutputValue { + /// Retrieve `VecStoredValue` + pub fn get_vec_value(self) -> VecStoredValue { self.vec_value } - /// Retrieve `VecOutputValue` by reference - pub fn get_vec_value_ref(&self) -> &VecOutputValue { + /// Retrieve `VecStoredValue` by reference + pub fn get_vec_value_ref(&self) -> &VecStoredValue { &self.vec_value } fn len(&self) -> usize { match &self.vec_value { - VecOutputValue::Bool(vec) => vec.len(), - VecOutputValue::Uint16(vec) => vec.len(), - VecOutputValue::Uint32(vec) => vec.len(), - VecOutputValue::Uint64(vec) => vec.len(), - VecOutputValue::Int16(vec) => vec.len(), - VecOutputValue::Int32(vec) => vec.len(), - VecOutputValue::Int64(vec) => vec.len(), - VecOutputValue::Text(vec) => vec.len(), - VecOutputValue::Hash(vec) => vec.len(), - VecOutputValue::Reference(vec) => vec.len(), + VecStoredValue::Bool(vec) => vec.len(), + VecStoredValue::Uint16(vec) => vec.len(), + VecStoredValue::Uint32(vec) => vec.len(), + VecStoredValue::Uint64(vec) => vec.len(), + VecStoredValue::Int16(vec) => vec.len(), + VecStoredValue::Int32(vec) => vec.len(), + VecStoredValue::Int64(vec) => vec.len(), + VecStoredValue::Text(vec) => vec.len(), + VecStoredValue::Hash(vec) => vec.len(), + VecStoredValue::Reference(vec) => vec.len(), } } /// Clear current `vec_value` pub fn clear(&mut self) { match &mut self.vec_value { - VecOutputValue::Bool(vec) => *vec = vec![], - VecOutputValue::Uint16(vec) => *vec = vec![], - VecOutputValue::Uint32(vec) => *vec = vec![], - VecOutputValue::Uint64(vec) => *vec = vec![], - VecOutputValue::Int16(vec) => *vec = vec![], - VecOutputValue::Int32(vec) => *vec = vec![], - VecOutputValue::Int64(vec) => *vec = vec![], - VecOutputValue::Text(vec) => *vec = vec![], - VecOutputValue::Hash(vec) => *vec = vec![], - VecOutputValue::Reference(vec) => *vec = vec![], + VecStoredValue::Bool(vec) => *vec = vec![], + VecStoredValue::Uint16(vec) => *vec = vec![], + VecStoredValue::Uint32(vec) => *vec = vec![], + VecStoredValue::Uint64(vec) => *vec = vec![], + VecStoredValue::Int16(vec) => *vec = vec![], + VecStoredValue::Int32(vec) => *vec = vec![], + VecStoredValue::Int64(vec) => *vec = vec![], + VecStoredValue::Text(vec) => *vec = vec![], + VecStoredValue::Hash(vec) => *vec = vec![], + VecStoredValue::Reference(vec) => *vec = vec![], } } @@ -188,23 +188,23 @@ impl VecOutputPropertyValue { } match &mut self.vec_value { - VecOutputValue::Bool(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Uint16(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Uint32(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Uint64(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Int16(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Int32(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Int64(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Text(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Hash(vec) => remove_at_checked(vec, index_in_property_vec), - VecOutputValue::Reference(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Bool(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Uint16(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Uint32(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Uint64(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Int16(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Int32(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Int64(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Text(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Hash(vec) => remove_at_checked(vec, index_in_property_vec), + VecStoredValue::Reference(vec) => remove_at_checked(vec, index_in_property_vec), } self.increment_nonce(); } - /// Insert provided `OutputValue` at given `index_in_property_vec`, increment `nonce` - pub fn insert_at(&mut self, index_in_property_vec: VecMaxLength, single_value: OutputValue) { + /// Insert provided `StoredValue` at given `index_in_property_vec`, increment `nonce` + pub fn insert_at(&mut self, index_in_property_vec: VecMaxLength, single_value: StoredValue) { fn insert_at(vec: &mut Vec, index_in_property_vec: VecMaxLength, value: T) { if (index_in_property_vec as usize) < vec.len() { vec.insert(index_in_property_vec as usize, value); @@ -212,33 +212,33 @@ impl VecOutputPropertyValue { } match (&mut self.vec_value, single_value) { - (VecOutputValue::Bool(vec), OutputValue::Bool(value)) => { + (VecStoredValue::Bool(vec), StoredValue::Bool(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Uint16(vec), OutputValue::Uint16(value)) => { + (VecStoredValue::Uint16(vec), StoredValue::Uint16(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Uint32(vec), OutputValue::Uint32(value)) => { + (VecStoredValue::Uint32(vec), StoredValue::Uint32(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Uint64(vec), OutputValue::Uint64(value)) => { + (VecStoredValue::Uint64(vec), StoredValue::Uint64(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Int16(vec), OutputValue::Int16(value)) => { + (VecStoredValue::Int16(vec), StoredValue::Int16(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Int32(vec), OutputValue::Int32(value)) => { + (VecStoredValue::Int32(vec), StoredValue::Int32(value)) => { insert_at(vec, index_in_property_vec, value) } - (VecOutputValue::Int64(vec), OutputValue::Int64(value)) => { + (VecStoredValue::Int64(vec), StoredValue::Int64(value)) => { insert_at(vec, index_in_property_vec, value) } // Match by move, when https://github.com/rust-lang/rust/issues/68354 stableize - (VecOutputValue::Text(vec), OutputValue::Text(ref value)) => { + (VecStoredValue::Text(vec), StoredValue::Text(ref value)) => { insert_at(vec, index_in_property_vec, value.to_owned()) } - (VecOutputValue::Reference(vec), OutputValue::Reference(value)) => { + (VecStoredValue::Reference(vec), StoredValue::Reference(value)) => { insert_at(vec, index_in_property_vec, value) } _ => return, @@ -247,7 +247,7 @@ impl VecOutputPropertyValue { self.increment_nonce(); } - /// Ensure `VecOutputPropertyValue` nonce is equal to the provided one. + /// Ensure `VecStoredPropertyValue` nonce is equal to the provided one. /// Used to to avoid possible data races, when performing vector specific operations pub fn ensure_nonce_equality(&self, new_nonce: T::Nonce) -> dispatch::Result { ensure!( @@ -257,7 +257,7 @@ impl VecOutputPropertyValue { Ok(()) } - /// Ensure, provided `index_in_property_vec` is valid index of `VecOutputValue` + /// Ensure, provided `index_in_property_vec` is valid index of `VecStoredValue` pub fn ensure_index_in_property_vector_is_valid( &self, index_in_property_vec: VecMaxLength, @@ -271,10 +271,10 @@ impl VecOutputPropertyValue { } } -/// Vector value enum representation, related to corresponding `VecOutputPropertyValue` structure +/// Vector value enum representation, related to corresponding `VecStoredPropertyValue` structure #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug)] -pub enum VecOutputValue { +pub enum VecStoredValue { Bool(Vec), Uint16(Vec), Uint32(Vec), @@ -287,14 +287,14 @@ pub enum VecOutputValue { Reference(Vec), } -impl Default for VecOutputValue { +impl Default for VecStoredValue { fn default() -> Self { Self::Bool(vec![]) } } -impl VecOutputValue { - /// Retrieve all involved `entity_id`'s, if current `VecOutputValue` is reference +impl VecStoredValue { + /// Retrieve all involved `entity_id`'s, if current `VecStoredValue` is reference pub fn get_involved_entities(&self) -> Option> { if let Self::Reference(entity_ids) = self { Some(entity_ids.to_owned()) diff --git a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs index dd2a2716cc..6057dd4b7b 100644 --- a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs @@ -14,7 +14,7 @@ fn insert_at_entity_property_vector_success() { let number_of_events_before_calls = System::events().len(); // Insert `InputValue` at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` + // into `VecStoredPropertyValue` under `in_class_schema_property_id` let nonce = 0; let index_in_property_vector = 1; let input_value = InputValue::Reference(SECOND_ENTITY_ID); @@ -39,7 +39,7 @@ fn insert_at_entity_property_vector_success() { { second_schema_old_property_value.insert_at( index_in_property_vector, - OutputValue::Reference(SECOND_ENTITY_ID), + StoredValue::Reference(SECOND_ENTITY_ID), ); } @@ -91,7 +91,7 @@ fn insert_at_entity_property_vector_entity_not_found() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case when corresponding Entity does not exist + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case when corresponding Entity does not exist let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, actor.clone(), @@ -131,7 +131,7 @@ fn insert_at_entity_property_vector_lead_auth_failed() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` using unknown origin and lead actor + // into `VecStoredPropertyValue` under `in_class_schema_property_id` using unknown origin and lead actor let insert_at_entity_property_vector_result = insert_at_entity_property_vector( UNKNOWN_ORIGIN, actor, @@ -171,7 +171,7 @@ fn insert_at_entity_property_vector_member_auth_failed() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` using unknown origin and member actor + // into `VecStoredPropertyValue` under `in_class_schema_property_id` using unknown origin and member actor let insert_at_entity_property_vector_result = insert_at_entity_property_vector( UNKNOWN_ORIGIN, actor, @@ -218,7 +218,7 @@ fn insert_at_entity_property_vector_curator_group_is_not_active() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` using curator group, which is not active as actor + // into `VecStoredPropertyValue` under `in_class_schema_property_id` using curator group, which is not active as actor let insert_at_entity_property_vector_result = insert_at_entity_property_vector( FIRST_CURATOR_ORIGIN, actor, @@ -261,7 +261,7 @@ fn insert_at_entity_property_vector_curator_auth_failed() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` using unknown origin and curator actor + // into `VecStoredPropertyValue` under `in_class_schema_property_id` using unknown origin and curator actor let insert_at_entity_property_vector_result = insert_at_entity_property_vector( UNKNOWN_ORIGIN, actor, @@ -304,7 +304,7 @@ fn insert_at_entity_property_vector_curator_not_found_in_curator_group() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // using actor in group, which curator id was not added to corresponding group set let insert_at_entity_property_vector_result = insert_at_entity_property_vector( SECOND_CURATOR_ORIGIN, @@ -345,7 +345,7 @@ fn insert_at_entity_property_vector_access_denied() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // using origin, which corresponding actor is neither entity maintainer, nor controller. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( SECOND_CURATOR_ORIGIN, @@ -386,7 +386,7 @@ fn insert_at_entity_property_vector_values_locked_on_class_level() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // in the case, when all property values were locked on Class level. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, @@ -428,7 +428,7 @@ fn insert_at_entity_property_vector_class_property_not_found() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // in the case, when Property under corresponding PropertyId was not found on Class level. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, @@ -505,7 +505,7 @@ fn insert_at_entity_property_vector_is_locked_for_given_actor() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to remove value at given `index_in_property_vector` - // from `VecOutputPropertyValue` under `in_class_schema_property_id`, + // from `VecStoredPropertyValue` under `in_class_schema_property_id`, // under lead origin, which is current Entity controller, in the case, // when corresponding class Property was locked from controller on Class level let insert_at_entity_property_vector_result = insert_at_entity_property_vector( @@ -551,7 +551,7 @@ fn insert_at_entity_property_vector_unknown_entity_property_id() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // in the case, when property value was not added to current Entity values yet. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, @@ -618,7 +618,7 @@ fn insert_at_entity_property_vector_value_under_given_index_is_not_a_vector() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // in the case, when entity property value corresponding to a given in_class_schema_property_id is not a vector. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, @@ -663,7 +663,7 @@ fn insert_at_entity_property_vector_nonces_does_not_match() { let input_value = InputValue::Reference(FIRST_ENTITY_ID); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id`, + // into `VecStoredPropertyValue` under `in_class_schema_property_id`, // providing nonce that does not corresponding property value vector one. let insert_at_entity_property_vector_result = insert_at_entity_property_vector( LEAD_ORIGIN, @@ -720,7 +720,7 @@ fn insert_at_entity_property_vector_index_is_out_of_range() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when provided index_in_property_vector is out of range of the related vector let nonce = 0; let index_in_property_vector = entity_ids.len() as u16 + 1; @@ -781,7 +781,7 @@ fn insert_at_entity_property_vector_is_too_long() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding property_vector can not contain more values let nonce = 0; let index_in_property_vector = 1; @@ -858,7 +858,7 @@ fn insert_at_entity_property_vector_text_prop_is_too_long() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding property text value is too long let nonce = 0; let index_in_property_vector = 0; @@ -938,7 +938,7 @@ fn insert_at_entity_property_vector_hashed_text_prop_is_too_long() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding property text to hash value is too long let nonce = 0; let index_in_property_vector = 0; @@ -1017,7 +1017,7 @@ fn insert_at_entity_property_vector_prop_type_does_not_match_internal_vec_proper let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding property type does not match internal vector property type let nonce = 0; let index_in_property_vector = 0; @@ -1062,7 +1062,7 @@ fn insert_at_entity_property_vector_referenced_entity_not_found() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding input_value referes to unknown Entity let nonce = 0; let index_in_property_vector = 0; @@ -1118,7 +1118,7 @@ fn insert_at_entity_property_vector_entity_can_not_be_referenced() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding Entity can not be referenced let nonce = 0; let index_in_property_vector = 0; @@ -1180,7 +1180,7 @@ fn insert_at_entity_property_vector_same_controller_constraint_violation() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when corresponding Entity can only be referenced from Entity with the same controller. let nonce = 0; let index_in_property_vector = 0; @@ -1244,7 +1244,7 @@ fn insert_at_entity_property_vector_property_should_be_unique() { let number_of_events_before_call = System::events().len(); // Make an attempt to insert value at given `index_in_property_vector` - // into `VecOutputPropertyValue` under `in_class_schema_property_id` in case, + // into `VecStoredPropertyValue` under `in_class_schema_property_id` in case, // when in result we`ll get required & unique property value vector, // which is already added to another Entity of this Class. let nonce = 0; From 1b744b711c0e3a3dd055553e21948d04ffb36b02 Mon Sep 17 00:00:00 2001 From: iorveth Date: Fri, 14 Aug 2020 09:56:43 +0000 Subject: [PATCH 14/17] Make OutputValueForExistingProperty::from method infallible --- .../content-directory/src/helpers.rs | 18 ++++++------- runtime-modules/content-directory/src/lib.rs | 27 ++++++++----------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index a310608917..4bf277351b 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -133,22 +133,20 @@ impl<'a, T: Trait> DerefMut for StoredValuesForExistingProperties<'a, T> { impl<'a, T: Trait> StoredValuesForExistingProperties<'a, T> { /// Create `StoredValuesForExistingProperties` helper structure from provided `property_values` and their corresponding `Class` properties. - /// Throws an error, when `Class` `Property` under `property_id`, corresponding to provided `property_value` not found pub fn from( properties: &'a [Property], property_values: &'a BTreeMap>, - ) -> Result { + ) -> Self { let mut values_for_existing_properties = StoredValuesForExistingProperties::::default(); for (&property_id, property_value) in property_values { - let property = properties - .get(property_id as usize) - .ok_or(ERROR_CLASS_PROP_NOT_FOUND)?; - values_for_existing_properties.insert( - property_id, - StoredValueForExistingProperty::new(property, property_value), - ); + if let Some(property) = properties.get(property_id as usize) { + values_for_existing_properties.insert( + property_id, + StoredValueForExistingProperty::new(property, property_value), + ); + } } - Ok(values_for_existing_properties) + values_for_existing_properties } /// Used to compute hashes from `StoredPropertyValue`s and their respective property ids, which respective `Properties` have `unique` flag set diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index f1a53fc171..71d0f59a3f 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -717,7 +717,7 @@ decl_module! { let entity_property_values = entity.get_values(); // Create wrapper structure from provided entity_property_values and their corresponding Class properties - let values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &entity_property_values)?; + let values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &entity_property_values); // Filter provided values_for_existing_properties, leaving only `Reference`'s with `SameOwner` flag set // Retrieve the set of corresponding property ids @@ -783,7 +783,7 @@ decl_module! { let entities_inbound_rcs_delta = Self::get_updated_inbound_rcs_delta( entity_id, class_properties, entity_property_values, new_output_property_value_references_with_same_owner_flag_set - )?; + ); // Update InboundReferenceCounter, based on previously calculated ReferenceCounterSideEffects, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -925,7 +925,7 @@ decl_module! { let entity_values = entity.get_values(); - let unique_property_value_hashes = StoredValuesForExistingProperties::from(&class_properties, &entity_values)?.compute_unique_hashes(); + let unique_property_value_hashes = StoredValuesForExistingProperties::from(&class_properties, &entity_values).compute_unique_hashes(); // // == MUTATION SAFE == @@ -1008,7 +1008,7 @@ decl_module! { schema, entity_property_values, &new_output_property_values ); - let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; + let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); // Retrieve StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) @@ -1119,7 +1119,7 @@ decl_module! { // Calculate entities reference counter side effects for current operation (should always be safe) let entities_inbound_rcs_delta = - Self::get_updated_inbound_rcs_delta(entity_id, class_properties, entity_property_values, new_output_property_values)?; + Self::get_updated_inbound_rcs_delta(entity_id, class_properties, entity_property_values, new_output_property_values); // Update InboundReferenceCounter, based on previously calculated entities_inbound_rcs_delta, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -1696,7 +1696,7 @@ impl Module { class_properties: Vec>, entity_property_values: BTreeMap>, new_output_property_values: BTreeMap>, - ) -> Result>, &'static str> { + ) -> Option> { // Filter entity_property_values to get only those, which will be substituted with new_property_values let entity_property_values_to_update: BTreeMap> = entity_property_values @@ -1713,7 +1713,7 @@ impl Module { StoredValuesForExistingProperties::from( &class_properties, &entity_property_values_to_update, - )?, + ), DeltaMode::Decrement, ); @@ -1721,10 +1721,7 @@ impl Module { // as involved InputPropertyValue References will substitute the old ones let incremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( current_entity_id, - StoredValuesForExistingProperties::from( - &class_properties, - &new_output_property_values, - )?, + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values), DeltaMode::Increment, ); @@ -1735,7 +1732,7 @@ impl Module { incremental_reference_counter_side_effects, ); - Ok(reference_counter_side_effects) + reference_counter_side_effects } /// Add up both net first_reference_counter_side_effects and second_reference_counter_side_effects (if some) @@ -1897,10 +1894,8 @@ impl Module { ) -> Result, &'static str> { // Compute StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from( - &class_properties, - &new_output_property_values, - )?; + let new_output_values_for_existing_properties = + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); From f0c1f0dcc591da5a70eec82b6725a01d2970300b Mon Sep 17 00:00:00 2001 From: iorveth Date: Fri, 14 Aug 2020 12:37:50 +0000 Subject: [PATCH 15/17] Fix clippy --- runtime-modules/content-directory/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index 71d0f59a3f..3ccd9d0832 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -1727,12 +1727,10 @@ impl Module { // Add up both net decremental_reference_counter_side_effects and incremental_reference_counter_side_effects // to get one net sideffect per entity. - let reference_counter_side_effects = Self::calculate_updated_inbound_rcs_delta( + Self::calculate_updated_inbound_rcs_delta( decremental_reference_counter_side_effects, incremental_reference_counter_side_effects, - ); - - reference_counter_side_effects + ) } /// Add up both net first_reference_counter_side_effects and second_reference_counter_side_effects (if some) From 673fcb38407d012598e14158867d090e6a7846aa Mon Sep 17 00:00:00 2001 From: iorveth Date: Fri, 14 Aug 2020 22:31:40 +0000 Subject: [PATCH 16/17] ensure_new_property_values_respect_uniquness method refactoring --- runtime-modules/content-directory/src/lib.rs | 57 ++++++++++---------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index 3ccd9d0832..90b83ea625 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -42,12 +42,6 @@ pub use serde::{Deserialize, Serialize}; /// Type, used in diffrent numeric constraints representations type MaxNumber = u32; -/// Type representing a map for both new and old property values hashes -type PropertyValuesHashes = ( - BTreeMap::Hash>, - BTreeMap::Hash>, -); - pub trait Trait: system::Trait + ActorAuthenticator + Debug + Clone { /// The overarching event type. type Event: From> + Into<::Event>; @@ -752,16 +746,25 @@ decl_module! { let new_output_property_value_references_with_same_owner_flag_set = Self::make_output_property_values(new_property_value_references_with_same_owner_flag_set); - // Compute old and new unique property value hashes. + // Compute StoredPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_output_values_for_existing_properties = + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_value_references_with_same_owner_flag_set); + + // Compute new unique property value hashes. // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level - let (new_unique_hashes, old_unique_hashes) = Self::ensure_property_values_hashes( - class_id, &class_properties, &new_output_property_value_references_with_same_owner_flag_set, &entity_property_values + let new_unique_hashes = Self::ensure_new_property_values_respect_uniquness( + class_id, new_output_values_for_existing_properties, )?; // // == MUTATION SAFE == // + // Used to compute old unique hashes, that should be substituted with new ones. + let old_unique_hashes = + Self::compute_old_unique_hashes(&new_output_property_value_references_with_same_owner_flag_set, &entity_property_values); + // Add property values, that should be unique on Class level Self::add_unique_property_value_hashes(class_id, new_unique_hashes); @@ -1094,16 +1097,25 @@ decl_module! { let new_output_property_values = Self::make_output_property_values(new_property_values); - // Compute old and new unique property value hashes. + // Compute StoredPropertyValues, which respective Properties have unique flag set + // (skip PropertyIds, which respective property values under this Entity are default and non required) + let new_output_values_for_existing_properties = + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); + + // Compute new unique property value hashes. // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level - let (new_unique_hashes, old_unique_hashes) = Self::ensure_property_values_hashes( - class_id, &class_properties, &new_output_property_values, &entity_property_values + let new_unique_hashes = Self::ensure_new_property_values_respect_uniquness( + class_id, new_output_values_for_existing_properties, )?; // // == MUTATION SAFE == // + // Used to compute old unique hashes, that should be substituted with new ones. + let old_unique_hashes = + Self::compute_old_unique_hashes(&new_output_property_values, &entity_property_values); + // Add property value hashes, that should be unique on Class level Self::add_unique_property_value_hashes(class_id, new_unique_hashes); @@ -1882,19 +1894,12 @@ impl Module { Ok((new_property_value_hash, old_property_value_hash)) } - /// Compute old and new unique property value hashes. + /// Compute new unique property value hashes. /// Ensure new property value hashes with `unique` flag set are `unique` on `Class` level - pub fn ensure_property_values_hashes( + pub fn ensure_new_property_values_respect_uniquness( class_id: T::ClassId, - class_properties: &[Property], - new_output_property_values: &BTreeMap>, - entity_property_values: &BTreeMap>, - ) -> Result, &'static str> { - // Compute StoredPropertyValues, which respective Properties have unique flag set - // (skip PropertyIds, which respective property values under this Entity are default and non required) - let new_output_values_for_existing_properties = - StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); - + new_output_values_for_existing_properties: StoredValuesForExistingProperties, + ) -> Result, &'static str> { let new_unique_property_value_hashes = new_output_values_for_existing_properties.compute_unique_hashes(); @@ -1904,11 +1909,7 @@ impl Module { &new_unique_property_value_hashes, )?; - // Used to compute old unique hashes, that should be substituted with new ones. - let old_unique_hashes = - Self::compute_old_unique_hashes(&new_output_property_values, entity_property_values); - - Ok((new_unique_property_value_hashes, old_unique_hashes)) + Ok(new_unique_property_value_hashes) } /// Returns the stored `Class` if exist, error otherwise. From 828eb8d624f4e76066b8b5cc4ef18311cffcdec6 Mon Sep 17 00:00:00 2001 From: iorveth Date: Sat, 15 Aug 2020 00:07:28 +0000 Subject: [PATCH 17/17] Use debug_assert macro, when from method conversion should not fail --- .../content-directory/src/helpers.rs | 18 +++--- runtime-modules/content-directory/src/lib.rs | 58 ++++++++++++++----- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/runtime-modules/content-directory/src/helpers.rs b/runtime-modules/content-directory/src/helpers.rs index 4bf277351b..6436712bd3 100644 --- a/runtime-modules/content-directory/src/helpers.rs +++ b/runtime-modules/content-directory/src/helpers.rs @@ -136,17 +136,19 @@ impl<'a, T: Trait> StoredValuesForExistingProperties<'a, T> { pub fn from( properties: &'a [Property], property_values: &'a BTreeMap>, - ) -> Self { + ) -> Result { let mut values_for_existing_properties = StoredValuesForExistingProperties::::default(); + for (&property_id, property_value) in property_values { - if let Some(property) = properties.get(property_id as usize) { - values_for_existing_properties.insert( - property_id, - StoredValueForExistingProperty::new(property, property_value), - ); - } + let property = properties + .get(property_id as usize) + .ok_or(ERROR_CLASS_PROP_NOT_FOUND)?; + values_for_existing_properties.insert( + property_id, + StoredValueForExistingProperty::new(property, property_value), + ); } - values_for_existing_properties + Ok(values_for_existing_properties) } /// Used to compute hashes from `StoredPropertyValue`s and their respective property ids, which respective `Properties` have `unique` flag set diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index 90b83ea625..4e396b649b 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -39,6 +39,8 @@ use system::ensure_signed; #[cfg(feature = "std")] pub use serde::{Deserialize, Serialize}; +use core::debug_assert; + /// Type, used in diffrent numeric constraints representations type MaxNumber = u32; @@ -711,7 +713,13 @@ decl_module! { let entity_property_values = entity.get_values(); // Create wrapper structure from provided entity_property_values and their corresponding Class properties - let values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &entity_property_values); + let values_for_existing_properties = match StoredValuesForExistingProperties::from(&class_properties, &entity_property_values) { + Ok(values_for_existing_properties) => values_for_existing_properties, + Err(e) => { + debug_assert!(false, "Should not fail! {:?}", e); + return Err(e) + } + }; // Filter provided values_for_existing_properties, leaving only `Reference`'s with `SameOwner` flag set // Retrieve the set of corresponding property ids @@ -749,7 +757,7 @@ decl_module! { // Compute StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) let new_output_values_for_existing_properties = - StoredValuesForExistingProperties::from(&class_properties, &new_output_property_value_references_with_same_owner_flag_set); + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_value_references_with_same_owner_flag_set)?; // Compute new unique property value hashes. // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level @@ -786,7 +794,7 @@ decl_module! { let entities_inbound_rcs_delta = Self::get_updated_inbound_rcs_delta( entity_id, class_properties, entity_property_values, new_output_property_value_references_with_same_owner_flag_set - ); + )?; // Update InboundReferenceCounter, based on previously calculated ReferenceCounterSideEffects, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -928,7 +936,13 @@ decl_module! { let entity_values = entity.get_values(); - let unique_property_value_hashes = StoredValuesForExistingProperties::from(&class_properties, &entity_values).compute_unique_hashes(); + let unique_property_value_hashes = match StoredValuesForExistingProperties::from(&class_properties, &entity_values) { + Ok(values_for_existing_properties) => values_for_existing_properties.compute_unique_hashes(), + Err(e) => { + debug_assert!(false, "Should not fail! {:?}", e); + return Err(e) + } + }; // // == MUTATION SAFE == @@ -1011,7 +1025,7 @@ decl_module! { schema, entity_property_values, &new_output_property_values ); - let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); + let new_output_values_for_existing_properties = StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; // Retrieve StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) @@ -1100,7 +1114,7 @@ decl_module! { // Compute StoredPropertyValues, which respective Properties have unique flag set // (skip PropertyIds, which respective property values under this Entity are default and non required) let new_output_values_for_existing_properties = - StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values); + StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values)?; // Compute new unique property value hashes. // Ensure new property value hashes with `unique` flag set are `unique` on `Class` level @@ -1131,7 +1145,7 @@ decl_module! { // Calculate entities reference counter side effects for current operation (should always be safe) let entities_inbound_rcs_delta = - Self::get_updated_inbound_rcs_delta(entity_id, class_properties, entity_property_values, new_output_property_values); + Self::get_updated_inbound_rcs_delta(entity_id, class_properties, entity_property_values, new_output_property_values)?; // Update InboundReferenceCounter, based on previously calculated entities_inbound_rcs_delta, for each Entity involved Self::update_entities_rcs(&entities_inbound_rcs_delta); @@ -1708,7 +1722,7 @@ impl Module { class_properties: Vec>, entity_property_values: BTreeMap>, new_output_property_values: BTreeMap>, - ) -> Option> { + ) -> Result>, &'static str> { // Filter entity_property_values to get only those, which will be substituted with new_property_values let entity_property_values_to_update: BTreeMap> = entity_property_values @@ -1718,14 +1732,25 @@ impl Module { // Calculate entities reference counter side effects for update operation + let stored_values_for_entity_property_values_to_update = + match StoredValuesForExistingProperties::from( + &class_properties, + &entity_property_values_to_update, + ) { + Ok(stored_values_for_entity_property_values_to_update) => { + stored_values_for_entity_property_values_to_update + } + Err(e) => { + debug_assert!(false, "Should not fail! {:?}", e); + return Err(e); + } + }; + // Calculate entities inbound reference counter delta with Decrement DeltaMode for entity_property_values_to_update, // as involved InputPropertyValue References will be substituted with new ones let decremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( current_entity_id, - StoredValuesForExistingProperties::from( - &class_properties, - &entity_property_values_to_update, - ), + stored_values_for_entity_property_values_to_update, DeltaMode::Decrement, ); @@ -1733,16 +1758,19 @@ impl Module { // as involved InputPropertyValue References will substitute the old ones let incremental_reference_counter_side_effects = Self::calculate_entities_inbound_rcs_delta( current_entity_id, - StoredValuesForExistingProperties::from(&class_properties, &new_output_property_values), + StoredValuesForExistingProperties::from( + &class_properties, + &new_output_property_values, + )?, DeltaMode::Increment, ); // Add up both net decremental_reference_counter_side_effects and incremental_reference_counter_side_effects // to get one net sideffect per entity. - Self::calculate_updated_inbound_rcs_delta( + Ok(Self::calculate_updated_inbound_rcs_delta( decremental_reference_counter_side_effects, incremental_reference_counter_side_effects, - ) + )) } /// Add up both net first_reference_counter_side_effects and second_reference_counter_side_effects (if some)