Skip to content

Wrong uniqueness feature implementation removal #1035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bedeho merged 1 commit into from
Aug 4, 2020
Merged

Wrong uniqueness feature implementation removal #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 3 additions & 58 deletions runtime-modules/content-directory/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,24 +736,10 @@ decl_module! {

// Make updated entity_property_values from parameters provided
let entity_property_values_updated =
if let Some(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,
) {

// Create wrapper structure from provided entity_property_values_updated
// and their corresponding Class properties
let updated_values_for_existing_properties = OutputValuesForExistingProperties::from(
&class_properties, &entity_property_values_updated
)?;

// Traverse all updated_values_for_existing_properties to ensure unique option satisfied (if required)
Self::ensure_property_values_unique_option_satisfied(updated_values_for_existing_properties)?;
Some(entity_property_values_updated)
} else {
None
};
);

// Transfer entity ownership
let entities_inbound_rcs_delta = if let Some(entity_property_values_updated) = entity_property_values_updated {
Expand Down Expand Up @@ -979,16 +965,6 @@ decl_module! {
schema, entity_property_values, &new_output_property_values
);

// Create wrapper structure from updated entity values and their corresponding Class properties
let updated_values_for_existing_properties = OutputValuesForExistingProperties::from(
&class_properties, &entity_values_updated
)?;

// Traverse all updated_values_for_existing_properties to ensure unique option satisfied (if required)
Self::ensure_property_values_unique_option_satisfied(
updated_values_for_existing_properties
)?;

//
// == MUTATION SAFE ==
//
Expand Down Expand Up @@ -1063,20 +1039,8 @@ decl_module! {
let entity_property_values = entity.get_values();

// Make updated entity_property_values from current entity_property_values and new_property_values provided
let entity_property_values_updated = if let Some(entity_property_values_updated) =
Self::make_updated_property_values(&entity_property_values, &new_property_values) {

// Create wrapper structure from new_property_values and their corresponding Class properties
let updated_values_for_existing_properties = OutputValuesForExistingProperties::from(
&class_properties, &entity_property_values_updated
)?;

// Traverse all values_for_updated_properties to ensure unique option satisfied (if required)
Self::ensure_property_values_unique_option_satisfied(updated_values_for_existing_properties)?;
Some(entity_property_values_updated)
} else {
None
};
let entity_property_values_updated =
Self::make_updated_property_values(&entity_property_values, &new_property_values);

// If property values should be updated
if let Some(entity_property_values_updated) = entity_property_values_updated {
Expand Down Expand Up @@ -1863,25 +1827,6 @@ impl<T: Trait> Module<T> {
Ok(())
}

/// Ensure all `updated_values_for_existing_properties` provided satisfy unique option, if required
pub fn ensure_property_values_unique_option_satisfied(
updated_values_for_existing_properties: OutputValuesForExistingProperties<T>,
) -> dispatch::Result {
for (&property_id, updated_value_for_existing_property) in
updated_values_for_existing_properties.iter()
{
let (property, value) = updated_value_for_existing_property.unzip();

// Ensure all InputPropertyValue's with unique option set are unique, except of null non required ones
property.ensure_unique_option_satisfied(
property_id,
value,
&updated_values_for_existing_properties,
)?;
}
Ok(())
}

/// Validate all values, provided in `values_for_existing_properties`, against the type of its `Property`
/// and check any additional constraints
pub fn ensure_property_values_are_valid(
Expand Down
23 changes: 0 additions & 23 deletions runtime-modules/content-directory/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,29 +330,6 @@ impl<T: Trait> Property<T> {
Ok(())
}

/// Ensure all `OutputPropertyValue`'s with unique option set are unique, except of null non required ones
pub fn ensure_unique_option_satisfied(
&self,
new_value_property_id: PropertyId,
new_value: &OutputPropertyValue<T>,
updated_values_for_existing_properties: &OutputValuesForExistingProperties<T>,
) -> dispatch::Result {
if self.unique && (*new_value != OutputPropertyValue::default() || self.required) {
ensure!(
updated_values_for_existing_properties
.iter()
// Skip current property value
.filter(|(property_id, _)| **property_id != new_value_property_id)
.map(|(_, updated_value_for_existing_property)| {
updated_value_for_existing_property.get_value()
})
.all(|value| *value != *new_value),
ERROR_PROPERTY_VALUE_SHOULD_BE_UNIQUE
);
}
Ok(())
}

/// Validate new `InputPropertyValue` against the type of this `Property`
/// and check any additional constraints
pub fn ensure_property_value_to_update_is_valid(
Expand Down
180 changes: 90 additions & 90 deletions runtime-modules/content-directory/src/tests/add_schema_support_to_entity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1292,93 +1292,93 @@ 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::<Runtime>::single_text(TextMaxLengthConstraint::get());

// Create first text property

let first_property = Property::<Runtime>::with_name_and_type(
PropertyNameLengthConstraint::get().max() as usize,
property_type.clone(),
true,
true,
);

// Create second text property

let second_property = Property::<Runtime>::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::<Runtime>::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 entity
// assert_ok!(create_entity(LEAD_ORIGIN, FIRST_CLASS_ID, actor.to_owned()));

// let property_type = PropertyType::<Runtime>::single_text(TextMaxLengthConstraint::get());

// // Create first text property

// let first_property = Property::<Runtime>::with_name_and_type(
// PropertyNameLengthConstraint::get().max() as usize,
// property_type.clone(),
// true,
// true,
// );

// // Create second text property

// let second_property = Property::<Runtime>::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::<Runtime>::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,
// );
// })
// }
1 change: 1 addition & 0 deletions runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ fn insert_at_entity_property_vector_text_prop_is_too_long() {
})
}

#[test]
fn insert_at_entity_property_vector_hashed_text_prop_is_too_long() {
with_test_externalities(|| {
// Create class with default permissions
Expand Down
Loading