Skip to content
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.

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

Content dir 2 #396

Merged
merged 165 commits into from
Jun 3, 2020
Merged

Content dir 2 #396

merged 165 commits into from
Jun 3, 2020

Conversation

iorveth
Copy link
Contributor

@iorveth iorveth commented May 7, 2020

Long-running PR for v2.0 of the content directory

VLOS

https://github.com/Joystream/joystream-landing/issues/175

  • Drop all events in VLOS

  • Enforce unique class names. <= Why? perhaps we should even drop them?

  • Enforce unique property names within each class. <= Why? perhaps we should even drop them?

  • Rename PropertyType::Internal to PropertyType::Reference, and same for PropertyValue::Internal, as we are dropping the no'tion of external references.

  • Remove PropertyType::None, was added to have some value for required Default trait, but has no semantic content. Instead just return some other value needed, we should not sacrifice our type safety due to a bad Substrate design decision.

  • Remove Class::id and Entity::id, as per convention Fix test for storage module and add new #36 (comment).

  • Make ClassId and EntityId, configurable through runtime.

  • Raw index types u16 are used for index types, replace with aliases for each use case, e.g. ClassId, EntityId, PropertyId etc. for clarity. Also make public

  • Add boolean field in ClassSchema which indicates whether a schema is active or not. It is only possible to add schema support to an entity for an active schema. Introduce method for updating this status also.

  • ClassSchema -> Schema

  • Change values field of Entity to BTreeMap<u16, PropertyValue> where key is index of property in class, and kill ClassPropertyValue.

  • Change in_class_schema_indexes in Entity to BTreeSet, and give better name to field, like supported_schemas.

  • Make vector properties updatable as vectors substrate-versioned-store-module#13

  • Entity removal substrate-versioned-store-module#22

  • Drop from storage these from storage, they need not be tuneable at runtime, just make them configurable through runtime trait.

pub PropertyNameConstraint get(property_name_constraint)
    config(): InputValidationLengthConstraint;

pub PropertyDescriptionConstraint get(property_description_constraint)
    config(): InputValidationLengthConstraint;

pub ClassNameConstraint get(class_name_constraint)
    config(): InputValidationLengthConstraint;

pub ClassDescriptionConstraint get(class_description_constraint)
    config(): InputValidationLengthConstraint;

paritytech/substrate#3263 - is not implemented yet

  • Remove InputValidationLengthConstraint, as it is no longer needed.

  • Introduce a new trait for Module, so that other modules depending on VLOS can consume this trait on runtime, which also allows for mocking.

  • Proposal: Merge permission and version store #380

  • Merge permission and version store

  • Class and ClassPermission, only have one instance per class, and only one map

  • Entity and EntityPermission, only have one instance per entity, and only one map.

  • Drop ClassPermission::properties_locked_from_controller, and instead enrich PropertyType to include a locking flag for controllers.

  • Drop ClassPermission::properties_locked_from_maintainers, and instead enrich PropertyType to include a locking flag for maintainers.

  • Drop ClassPermission::referenced_entity_must_have_same_controller, and instead enrich PropertyType::Reference to include a same owner boolean flag.

VLOS Permissions

Remove entity_id from Entity structure
…Map<u16, PropertyValue> where key is index of property in class
… representation, renamed to supported_schemas
…mentation. Begun work on insert_at implementation
iorveth added 18 commits May 26, 2020 23:07
…ropertyValue under in_class_schema_property_id is VecPropertyValue
…reluctant entity_creation_vouchers lookup, wrap EntityCreationVouchers runtime storage value in Option
…ation_voucher_upper_bound, update_entity_creation_voucher extrinsic: fix constraint check
@iorveth iorveth requested a review from bedeho May 30, 2020 14:29
Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my two comments to prior discussions, others are closed. Good job on trying to show where the change was made, that was very helpful in many cases.

As discussed in the chat, most of the calls to improve the clarity and reduce the usage of mutables did not result in sufficiently large improvements, and the introduction of EntitiesRc was a further step in this direction. So I think we should do a separate set of PRs to try to correct this step by step. This PR is already way to big.

Will merge this once we have concluded on the two replies I made.

@bedeho bedeho mentioned this pull request Jun 2, 2020
@iorveth iorveth requested a review from bedeho June 3, 2020 09:06
@iorveth iorveth mentioned this pull request Jun 3, 2020
@bedeho bedeho merged commit fe3bdee into Joystream:content_directory_second_try Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants