Introduce content directory graphql schema for query node #1458

metmirr · 2020-09-24T10:53:39Z

This PR introduces the content directory graphql input schema for the query node. All the types are defined in the schema expect #1348

bedeho

Why are the changes in 8a13fa7 here?

bedeho

I think @iorveth should be more deeply involved in making and reviewing this, he could have cought some of these issus.
Please introduce explicit Id field in all entities, and explain where value comes from. This is unclear here.

bedeho · 2020-09-24T18:22:19Z

query-node/joystream-query-node/schema.graphql

+	rootAccount: Bytes!
+
+	"When a member is worker"
+	worker: Worker @derivedFrom(field: "member")


A member can occupy 0 or many worker roles, both at the same time, and over time.

Done in 162095e all roles extends Worker interface will have a reference to the Member type.

bedeho · 2020-09-24T18:22:39Z

query-node/joystream-query-node/schema.graphql

+	worker: Worker @derivedFrom(field: "member")
+
+	"Virtual field required for 1:1 relationship"
+	entityController: EntityController @derivedFrom(field: "controllerWhenMember")


Why is this here?

Done 74b9b3d. It is my mistake, a member can control more than one entity.

bedeho · 2020-09-24T18:25:16Z

query-node/joystream-query-node/schema.graphql

+	# Commenting out this line because the properties are accessible from the schemas
+	# query { classes { schemas {properties { ... } } } }
+	# "All properties that have been used on this class across different class schemas"
+	# properties: [Property!]


Why is this commented out? I don't think the comment provides a good rationale, may have been me who wrote it long ago.

Done in f1bfa3b. No, it's my comment actually, after fixing Property entity (each property has a reference to a class) I uncomment it and it is a reverse lookup now.

bedeho · 2020-09-24T18:27:24Z

query-node/joystream-query-node/schema.graphql

+}
+
+"`Property` representation, related to a given `Class`"
+type Property @entity {


The Property and PropertyType types need to be coordinated better. we have fields such as ".... when the property is a vector, this is a symptom of not using algebraic types, which we should and can do. Look at runtime for guidance. @iorveth will have valuable input on this.

I refactored it to the model you suggested here #644 waiting @iorveth input on this.

#644 is not a solution, that draft was made before we even had algebraic types as a features.

bedeho · 2020-09-24T18:32:36Z

query-node/joystream-query-node/schema.graphql

+}
+
+"Fields encode a flattened representation of non-vector properties, only one will be non-null"
+type SingleValue @variant {


This should be modelled differently, this is not how to model this algebraically, @iorveth may help.

I refactored it to the model you suggested here #644 waiting for @iorveth input on this.

#644 is not a solution, that draft was made before we even had algebraic types as a features.

bedeho · 2020-09-24T18:32:45Z

query-node/joystream-query-node/schema.graphql

+Fields encode a flattened representation of vector properties, only one will be non-null
+"""
+type VectorValue @variant {
+	boolVec: [Boolean!]


This should be modelled differently, this is not how to model this algebraically, @iorveth may help.

I refactored it to the model you suggested here #644 waiting for @iorveth input on this.

#644 is not a solution, that draft was made before we even had algebraic types as a features.

bedeho · 2020-09-24T19:14:03Z

query-node/joystream-query-node/schema.graphql

+}
+
+"Working group participant: working-group/src/types.rs"
+type Worker @entity {


This should probably be an interface, as there are many kinds of workers, not just curators (even though we will only include curators in Babylon). The Curator entity can implement this interface. This would also require dropping curator field.

Done in 9540c34

bedeho · 2020-09-24T19:24:38Z

query-node/joystream-query-node/schema.graphql

+
+union StoredValue = SingleValue | VectorValue
+
+type PropertyValue @entity {


This is not being correctly modelled, please reevaluate. Include @iorveth here.

I refactored it to the model you suggested here #644 waiting for @iorveth input on this.

#644 is not a solution, that draft was made before we even had algebraic types as a features.

bedeho · 2020-09-24T19:28:32Z

query-node/joystream-query-node/schema.graphql

+	value: StoredValue!
+
+	"Virtual field for entity property values"
+	entities: [Entity!] @derivedFrom(field: "values")


A property value only corresponds to a single entity, and the valuesfield should derive from PropetyValue, not the other way around.

Done in d8b9033

bedeho · 2020-09-24T19:30:31Z

query-node/joystream-query-node/schema.graphql

+
+type PropertyValue @entity {
+	"Runtime identifier of property value in entity"
+	propertyValueId: BigInt! @unique


There is no unique property value id from the chain, the id in the chain is just an index in the given class, which will not be unique.

A property value is unique for a given combination and entity&property of the same class.

Done in d8b9033. Since PropertyValue has a reference to Property no longer needed that field.

There needs to be a propertyValueId, because it tells you the index of the value for the given entity. However, This value is no unique across all PropertyValue instances, precisely because lots of entities will have a property with index 0,1,2,...

If this is not clear, please review content director data model again.

…ore than one entity

metmirr · 2020-09-28T11:27:35Z

* I think @iorveth should be more deeply involved in making and reviewing this, he could have cought some of these issus.

* Please introduce explicit `Id` field in all entities, and explain where value comes from. This is unclear here.

Every entity defined in schema.graphql has an Id: ID! field by default.

…k entity

bedeho

The problem with algebraic type has not been resolved.
It needs to be explicitly stated what the Id field is for each entity, I believe a random one will be injected by Warthog or some other layer of the data model stack if we do not explicitly control it.

bedeho · 2020-10-01T13:46:43Z

query-node/joystream-query-node/schema.graphql

+"""
+type Class @entity {
+	"Runtime identifier for class"
+	classId: BigInt! @unique


replace with id: ID!, and the value should be "Runtime identifier for class".

bedeho · 2020-10-01T13:47:04Z

query-node/joystream-query-node/schema.graphql

+	classId: BigInt! @unique
+
+	"Name of class"
+	name: String!


Are class names non-null? are they unique? I forget.

bedeho · 2020-10-01T13:52:03Z

query-node/joystream-query-node/schema.graphql

+"'Property' representation, related to a given 'Class'"
+type Property @entity {
+	"Runtime identifier for property in class"
+	propertyId: Int!


Drop field, replace with new field "id:ID!`, and put value: ClassId+":"+PropertyId from runtime. Comment is then "Runtime class identifier concatenated with property runtime identifier, using colon as separator".

Correct me if I am wrong, ut runtime PropertyId is just an index relative to the class, so its not sufficient in of itself.

bedeho · 2020-10-01T13:54:41Z

query-node/joystream-query-node/schema.graphql

+"Represents a standard for what information must be associated with any entity of a given class"
+type Schema @entity {
+	"Runtime identifier of schema in class"
+	schemaId: Int!


Same idea here, replace with "id: ID!" where value is ClassId+":"+SchemaId, so comment says "Runtime class identifier concatinated with schema runtime identifier, using colon as separator".

bedeho · 2020-10-01T13:58:04Z

query-node/joystream-query-node/schema.graphql

+
+"Working group participant: working-group/src/types.rs"
+interface Worker @entity {
+	member: Member!


I think this needs Id ?

bedeho · 2020-10-01T13:58:46Z

query-node/joystream-query-node/schema.graphql

+"A voucher for 'Entity' creation"
+type EntityCreationVoucher @entity {
+	"Class to which voucher applies"
+	class: Class!


What is Id ?

bedeho · 2020-10-01T13:58:56Z

query-node/joystream-query-node/schema.graphql

+"Owner of an 'Entity'"
+type EntityController @entity {
+	"Current controller, which is initially set based on who created entity"
+	controllerType: EntityControllerType!


What is Id?

bedeho · 2020-10-01T13:59:05Z

query-node/joystream-query-node/schema.graphql

+"""
+type Entity @entity {
+	"Runtime identifier for entity"
+	entityId: BigInt! @unique


See Id comments.

bedeho · 2020-10-01T13:59:14Z

query-node/joystream-query-node/schema.graphql

+}
+
+type PropertyValue @entity {
+	entity: Entity!


What is Id?

bedeho · 2020-10-01T13:59:24Z

query-node/joystream-query-node/schema.graphql

+
+"Working group lead: curator lead content-working-group/src"
+type Lead implements Worker @entity {
+	leadId: BigInt! @unique


See Id comments.

…nel.owner field

metmirr added 2 commits September 16, 2020 12:59

fix: remove duplicate files

8a13fa7

introduce content directory graphql schema

2f8b9c2

This was referenced Sep 24, 2020

Content directory schema: Class entity defination #1399

Closed

Content directory schema: Entity entity defination #1400

Closed

metmirr added the query-node label Sep 24, 2020

metmirr requested review from bedeho and iorveth September 24, 2020 11:14

add Lead entity defination

96c65f7

bedeho reviewed Sep 24, 2020

View reviewed changes

bedeho requested changes Sep 24, 2020

View reviewed changes

metmirr added 7 commits September 24, 2020 21:33

add missing unique identifier for Lead and Channel entities

142c3ca

remove Worker reference field from Member

162095e

remove EntityController reference from Member: a member can control m…

74b9b3d

…ore than one entity

remove unique directive from schemaId: schemaId is unique per class

2a64bf5

remodel PropertyValue entity: remove variants, fix relations

d8b9033

convert Worker entity into an interface

9540c34

fix Class-Property relationship: each property has a class reference

f1bfa3b

metmirr added 3 commits September 30, 2020 18:38

implements Worker interface to Lead entity

5b5c3de

re-model Actor entity

5dcb3a0

replace HTTPMediaLocation fields(host, port) with url and remove Bloc…

8d352fe

…k entity

bedeho requested changes Oct 1, 2020

View reviewed changes

metmirr force-pushed the content_dir_schema branch from dc06056 to 8810bb5 Compare October 5, 2020 06:13

drop low level entities, refactor high level entities

71c1fa9

metmirr force-pushed the content_dir_schema branch from 8810bb5 to 71c1fa9 Compare October 5, 2020 06:17

metmirr added 3 commits October 5, 2020 19:56

update descriptions for id field

614095c

add ClassEntity type to keep entity-class relation

fb75d7f

remove unique directive from Channel.title field and comment out Chan…

91e14fb

…nel.owner field

iorveth mentioned this pull request Oct 13, 2020

Content dir entity_removed, entity_created mappings #1528

Closed

metmirr closed this Oct 29, 2020


		union StoredValue = SingleValue \| VectorValue

		type PropertyValue @entity {

Introduce content directory graphql schema for query node #1458

Introduce content directory graphql schema for query node #1458

Conversation

metmirr commented Sep 24, 2020

bedeho left a comment • edited Loading

Choose a reason for hiding this comment

bedeho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metmirr commented Sep 28, 2020

bedeho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedeho left a comment •

edited

Loading