Closed elf-pavlik closed 7 years ago
elf-pavlik
commented
7 years ago Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
property: "http://www.w3.org/1999/02/22-rdf-syntax-ns#type", object: "http://schema.org/CreateEvent" })
client could have more specific method for such common use case, something like getCollectionByMemberType('http://schema.org/Event')
Comments from Reviewable
tpluscode
commented
7 years ago Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
client could have more specific method for such common use case, something like `getCollectionByMemberType('http://schema.org/Event')`
I don't understand. You completely replaced the following of #events link with the getCollection call? In this case how does the client know which URL to retrieve?
Comments from Reviewable
tpluscode
commented
7 years ago Reviewed 2 of 7 files at r1. Review status: 2 of 7 files reviewed at latest revision, 4 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
"@id": "/api", "@type": "hydra:EntryPoint", "collection": {
Are you sure we should be changing this to hydra:collection? I guess this change answers my previous questions. But, I don't like how it loses the API-scecificness.
drafts/use-cases/7.searching-events.md, line 94 at r2 (raw file):
"@context": "/api/context.jsonld", "@id": "/api/events?search=some text", "@type": "hydra:Collection",
Is this really a collection or a partial view?
drafts/use-cases/8.filtering-events.md, line 97 at r2 (raw file):
"@context": "/api/context.jsonld", "@id": " /api/events?schema:eventName=.*test.*", "@type": "hydra:Collection",
Ditto, Is this really a collection or a partial view?
Comments from Reviewable
asbjornu
commented
7 years ago Why are we using Reviewable for code reviews? I much prefer the code review feature of GitHub, tbh.
elf-pavlik
commented
7 years ago @asbjornu plase see http://www.hydra-cg.com/minutes/2017-07-10/#resolution-1 (you will need to scroll up few lines)
elf-pavlik
commented
7 years ago Review status: 2 of 7 files reviewed at latest revision, 4 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file): Could you please explain what you mean by:
But, I don't like how it loses the API-scecificness.
To my understanding clients need a way to discover from the hydra:EntryPoint collection with members of type schema:Event. But this general conversation already happens in https://github.com/HydraCG/Specifications/issues/126
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
I don't understand. You completely replaced the following of `#events` link with the `getCollection` call? In this case how does the client know which URL to retrieve?
@id of the matching hydra:Collection I haven't thought of a case where multiple ones match but AFAIK hydra:Link also doesn't have restriction to reference only one resource
drafts/use-cases/7.searching-events.md, line 94 at r2 (raw file):
Is this really a collection or a partial view?
Good question, I hope Markus and Karol can chime in on that. I could always remove this change from this PR since it doesn't really belong in it.
drafts/use-cases/8.filtering-events.md, line 97 at r2 (raw file):
Ditto, Is this really a collection or a partial view?
I could also undo this change and leave it for another PR
Comments from Reviewable
tpluscode
commented
7 years ago Reviewed 4 of 7 files at r1, 1 of 1 files at r2. Review status: all files reviewed at latest revision, 2 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
Could you please explain what you mean by: > But, I don't like how it loses the API-scecificness. To my understanding clients need a way to discover from the `hydra:EntryPoint` collection with members of type `schema:Event`. But this general conversation already happens in https://github.com/HydraCG/Specifications/issues/126
What I mean is that replacing the API-specific term loses the link type between one resource (the Entrypoint) and the other (List of events). I think that the link relation trumps the manages block, so to speak.
As for the client though, a way to discover collection of a given type may be useful. How would you like it to be based on "@type": "hydra:Collection" and the manages-block and not the predicate type? This way you can keep a more informative link predicate
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
`@id` of the matching `hydra:Collection` I haven't thought of a case where multiple ones match but AFAIK `hydra:Link` also doesn't have restriction to reference only one resource
Not really what I had in mind but you are right. I meant more of what I wrote in the other comment about losing the link type.
In the end though I still don't entirely understand why you'd want a getCollection method instead of getLink. The former assumes what's on the other side. Not really hypermedia-y.
Comments from Reviewable
lanthaler
commented
7 years ago 
Reviewed 6 of 7 files at r1, 1 of 1 files at r2. Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/0.intro.md, line 89 at r2 (raw file):
"@type": "@id" }, "collection": {
I don't remember whether we introduced the rule afterwards but I think we agreed to not have terms that differ only in their capitalization. Maybe we should revisit this at some point.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
What I mean is that replacing the API-specific term loses the link type between one resource (the Entrypoint) and the other (List of events). I think that the link relation trumps the manages block, so to speak. As for the client though, a way to discover collection of a given type may be useful. How would you like it to be based on `"@type": "hydra:Collection"` and the `manages`-block and not the predicate type? This way you can keep a more informative link predicate
If such a relation exists, then yes, it should be used. This is for cases where no such relations exists yet and would only be created for the sake of being able to connect some entity with a collection.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Not really what I had in mind but you are right. I meant more of what I wrote in the other comment about losing the link type. In the end though I still don't entirely understand why you'd want a `getCollection` method instead of `getLink`. The former assumes what's on the other side. Not really hypermedia-y.
Think of it as a specialized link relation that Hydra clients understand natively. This is similar to how browsers use the icon or stylesheet link relation. You could also have multiple manages blocks that describe the relationship between the referencing entity and the collection members in more detail. See the page on the wiki where we worked on this (here's the summary).
Comments from Reviewable
tpluscode
commented
7 years ago Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
If such a relation exists, then yes, it should be used. This is for cases where no such relations exists yet and would only be created for the sake of being able to connect some entity with a collection.
Hm, wouldn't it be a good rule of thumb to mint an API-specific property for any such case? I think that is precisely what you did in events example...
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Think of it as a specialized link relation that Hydra clients understand natively. This is similar to how browsers use the `icon` or `stylesheet` link relation. You could also have multiple `manages` blocks that describe the relationship between the referencing entity and the collection members in more detail. See the [page on the wiki](https://www.w3.org/community/hydra/wiki/Avoid_that_collections_break_relationships) where we worked on this (here's the [summary](https://www.w3.org/community/hydra/wiki/Collection_Design)).
The problem with you analogy is that icon clearly defines the contents of the link's target. I'm not sure that is the case here.
Comments from Reviewable
lanthaler
commented
7 years ago Reviewed 1 of 7 files at r1. Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
Hm, wouldn't it be a good rule of thumb to mint an API-specific property for any such case? I think that is precisely what you did in events example...
Yeah, it's exactly what I did in the events example.. but only because there was nothing else available. The goal is to create clients that "understand" these APIs to some degree so that they can figure out how to use them. If everyone mints property like myvocab:events only clients that have been written specifically for that API will understand them. If we have a generic mechanisms as the one Pavlik proposes here on the other hand, we can implement client libraries that can find the collection that contains all events.
A custom link relation is only useful if it has some well-defined semantics that you can't convey otherwise.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
The problem with you analogy is that icon clearly defines the contents of the link's target. I'm not sure that is the case here.
Nope, unless I misunderstand you. It just describes in what relationship the target (the icon) stands to the context IRI (the document). The optional target attributes on a link are just hints that help a client to choose the supposedly best matching link. It must not trust them though but interpret what it actually gets when it dereferences the URL.
Comments from Reviewable
elf-pavlik
commented
7 years ago Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/0.intro.md, line 89 at r2 (raw file):
I don't remember whether we introduced the rule afterwards but I think we agreed to not have terms that differ only in their capitalization. Maybe we should revisit this at some point.
captured in https://github.com/HydraCG/Specifications/issues/133
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
Yeah, it's exactly what I did in the events example.. but only because there was nothing else available. The goal is to create clients that "understand" these APIs to some degree so that they can figure out how to use them. If everyone mints property like `myvocab:events` only clients that have been written specifically for that API will understand them. If we have a generic mechanisms as the one Pavlik proposes here on the other hand, we can implement client libraries that can *find the collection that contains all events*. A custom link relation is only useful if it has some well-defined semantics that you can't convey otherwise.
Since this PR proposes just one of possible solutions for the problem, I would like to suggest once again discussing and further clarifying the problem we try to address itself in https://github.com/HydraCG/Specifications/issues/126#issuecomment-322320409
Comments from Reviewable
tpluscode
commented
7 years ago Reviewed 1 of 7 files at r1. Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
Since this PR proposes just one of possible solutions for the problem, I would like to suggest once again discussing and further clarifying the problem we try to address itself in https://github.com/HydraCG/Specifications/issues/126#issuecomment-322320409
@mlanthaler only clients written for a specific API will understand the contents of the given collection anyway. the will understand hydra:Collection and partial views. other than that I don't see how having the collection is really useful as fallback. won't this lead to
{
"@id": "/some-person"
"hydra:collection": [
{
"@id": "/some-person/friends"
},
{
"@id": "/some-person/enemies"
},
{
"@id": "/some-person/interests"
}
]
}how useful is it now? Also, this PR assumes that the content of a collection is inline. If you only got the target URL, the hydra:collection conveys completely no information.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
[Nope](https://www.w3.org/TR/html5/links.html#rel-icon), unless I misunderstand you. It just describes in what relationship the target (the icon) stands to the context IRI (the document). The optional target attributes on a link are just hints that help a client to choose the supposedly best matching link. It must not trust them though but interpret what it actually gets when it dereferences the URL.
Well, my point precisely (see the comment above). If the representation contained multiple hydra:collectionlinks to multiple unrelated resources, the client would not be able to determine which one to dereference. Unless they are meant to be direct alternatives. Unless the manages-block will be mandatory...
Comments from Reviewable
elf-pavlik
commented
7 years ago drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
only clients written for a specific API will understand the contents of the given collection anyway.
I think we need to disambiguate between vocabs (schemas), APIs and datasets. Preferably around use cases included in our repo. In case where particular client wants to interact with multiple servers, each of those servers exposing a dataset via API but all those datasets use a common vocab (eg. schema.org). Preferably client only needs to implement Hydra spec and understand schema.org vocab to interact with all those servers exposing all those datasets.
If any of those servers defines some custom hydra:Link it can't expect such generic client to understand it. In our case relating collection of events from entry point with something as simple as rdfs:seeAlso and using hydra:manages block to hint type of all the members seems sufficient for client looking for 'collection of events in this dataset'
Also, this PR assumes that the content of a collection is inline. If you only got the target URL, the hydra:collection conveys completely no information.
I think while discussing collection design we have considered using rdfs:seeAlso but then decided to add hydra:collection to the vocab. I would see this property (the information it conveys) as something close to rdfs:seeAlso and 'void:subset'.
won't this lead to { ... } how useful is it now?
For example Solid mandates clients MUST load and parse all resources linked from personal WebID profile to get combined 'extended profile'. Clients which don't find enough information about collection inlined in entry point, can simply fetch it. We should still encourage publishers to inline information relevant for discovery in entry point.
BTW for the friends, enemies, interests manages block still seems more practical than trying to make up some hydra:Linkinstances
Comments from Reviewable
tpluscode
commented
7 years ago Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
> only clients written for a specific API will understand the contents of the given collection anyway. I think we need to disambiguate between *vocabs* (schemas), *APIs* and *datasets*. Preferably around use cases included in our repo. In case where particular client wants to interact with **multiple** servers, each of those servers exposing a dataset via API but all those datasets use a common vocab (eg. schema.org). Preferably client only needs to implement Hydra spec and understand schema.org vocab to interact with all those servers exposing all those datasets. If any of those servers defines some custom `hydra:Link` it can't expect such generic client to understand it. In our case relating collection of events from entry point with something as simple as `rdfs:seeAlso` and using `hydra:manages` block to hint type of all the members seems sufficient for client looking for 'collection of events in this dataset' > Also, this PR assumes that the content of a collection is inline. If you only got the target URL, the hydra:collection conveys completely no information. I think while discussing collection design we have considered using `rdfs:seeAlso` but then decided to add `hydra:collection` to the vocab. I would see this property (the information it conveys) as something close to `rdfs:seeAlso` and 'void:subset'. > won't this lead to { ... } how useful is it now? For example [Solid mandates](https://github.com/solid/solid-spec/blob/master/solid-webid-profiles.md#extended-profile) clients MUST load and parse all resources linked from personal WebID profile to get combined 'extended profile'. Clients which don't find enough information about collection inlined in entry point, can simply fetch it. We should still encourage publishers to inline information relevant for discovery in entry point. BTW for the *friends*, *enemies*, *interests* manages block still seems more practical than trying to make up some `hydra:Link`instances * http://purl.org/vocab/relationship/friendOf * http://purl.org/vocab/relationship/enemyOf * http://smiy.sourceforge.net/cco/spec/cognitivecharacteristics.html#interest
The element type of friends and enemies will both be a Person in the manages block? Again, the client cannot distinguish between two collection having clearly different meaning...
Comments from Reviewable
elf-pavlik
commented
7 years ago drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
The element type of `friends` and `enemies` will both be a Person in the manages block? Again, the client cannot distinguish between two collection having clearly different meaning...
In case above manages blocks wouldn't rely on 'type' property: rdf:type but instead on properties like:
Which capture the relation between /some-person and each member in the collection. Not the 'type' of the member - relation between some instance of rdfs:Class and each member in the collection.
Have you participated in defining Collection Design? Possibly it needs better documentation and clarified motivation beyond linking to gh issues. We didn't think about use case of rdf:type property when we discussed his design but it seems perfectly fit to handle it as well.
Comments from Reviewable
tpluscode
commented
7 years ago Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
In case above manages blocks wouldn't rely on 'type' `property: rdf:type` but instead on properties like: * http://purl.org/vocab/relationship/friendOf * http://purl.org/vocab/relationship/enemyOf * http://smiy.sourceforge.net/cco/spec/cognitivecharacteristics.html#interest Which capture the relation between `/some-person` and each member in the collection. Not the 'type' of the member - relation between some instance of `rdfs:Class` and each member in the collection. Have you participated in defining [Collection Design](https://www.w3.org/community/hydra/wiki/Collection_Design)? Possibly it needs better documentation and clarified motivation beyond linking to gh issues. We didn't think about use case of `rdf:type` property when we discussed his design but it seems perfectly fit to handle it as well.
Ah, I think I recall. This is for when one resource doesn't have an actual relation in dataset but only in hypermedia. So in RDF you don't have a api:entrypoint vocab:friends api:collection triple but want to be able to include a link between the two in Hydra representation. Correct?
Comments from Reviewable
lanthaler
commented
7 years ago Review status: all files reviewed at latest revision, 3 unresolved discussions.
drafts/use-cases/0.intro.md, line 89 at r2 (raw file):
captured in https://github.com/HydraCG/Specifications/issues/133
Thanks
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Well, my point precisely (see the comment above). If the representation contained multiple `hydra:collection`links to multiple unrelated resources, the client would not be able to determine which one to dereference. Unless they are meant to be direct alternatives. Unless the `manages`-block will be mandatory...
Correct, you do need the manages block to differentiate them. The advantage is that it allows you to describe things without having to invent new concepts. Assuming X points to Y via hydra:collection, manages can be used to express that
Y are of a specific type:
"manages": { "property": "rdf:type", "object": "schema:Event" }Ystand in a specific relationship with X:
"manages": { "subject": "X", "property": "schema:knows" }Y have a a specific property set to a specific value:
"manages": { "property": "schema:gender", "object": "schema:Female" }We can even allow multiple of them at the same time, so we would be able to express that there's a collection which manages entities of type schema:Person, that X schema:knowns and have schema:gender``schema:Female; in other words a collection containing X's female acquaintances. That's pretty powerful.
Is that concern here actually that it looks more complex/is more verbose or that it has weaker semantics? I can agree with the former but don't think the latter is true.
Comments from Reviewable
elf-pavlik
commented
7 years ago drafts/use-cases/1.1.security-considerations.md, line 18 at r2 (raw file):
Ah, I think I recall. This is for when one resource doesn't have an actual relation in dataset but only in hypermedia. So in RDF you don't have a `api:entrypoint vocab:friends api:collection` triple but want to be able to include a link between the two in Hydra representation. Correct?
Not sure if I understood. In case /some-person of socrel:friendOf or 'cco:interestrelations exist in the dataset, not between the/some-personandhydra:collectionbut between/some-personand each member. In case ofrdf:typeno relation meaningful for the dataset seems to exist between eachschema:Eventand thehydra:EntryPoint(except something likevoid:inDataset). If client would use SPARQL or Triple Pattern Fragments interface, thehydra:collectionseems not needed and IMO often may come just as artifact of trying to provide some tree-ish layout for REST inteface to access the graph-y dataset. I think to really clarify that, we have to keep working with more and more use cases and for each one take a look how accessing the same dataset would differ if using SPARQL interface or HyDRA interface - which data we add just for the sake of creating HyDRA interface layout. For the sake of this PR I think that Markus gave really good answer in another thread, the one with examples like *all entries in Y have a a specific property set to a specific value:*. Main improvement in this PR comes with switching to generalized approach relying on already existing terms in common vocabs and nod needing to improvise terms likemysnowflakevocab:events` which go against benefits of shared understanding provided by commonly used vocabs and enabling interoperability.
Comments from Reviewable
tpluscode
commented
7 years ago Review status: all files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Correct, you do need the manages block to differentiate them. The advantage is that it allows you to describe things without having to invent new concepts. Assuming `X` points to `Y` via `hydra:collection`, manages can be used to express that * all entries in `Y` are of a specific type: `"manages": { "property": "rdf:type", "object": "schema:Event" }` * all entries in `Y`stand in a specific relationship with `X`: `"manages": { "subject": "X", "property": "schema:knows" }` * all entries in `Y` have a a specific property set to a specific value: `"manages": { "property": "schema:gender", "object": "schema:Female" }` We can even allow multiple of them at the same time, so we would be able to express that there's a collection which manages entities of type `schema:Person`, that `X` `schema:knowns` and have `schema:gender``schema:Female`; in other words a collection containing `X`'s female acquaintances. That's pretty powerful. Is that concern here actually that it looks more complex/is more verbose or that it has weaker semantics? I can agree with the former but don't think the latter is true.
My concern is that this looks verbose and complicated. Especially that this structure addresses a RDF-specific problem. But, I don't have any more comments. Thank you for your patience :D
Comments from Reviewable
asbjornu
commented
7 years ago Review status: all files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
My concern is that this looks verbose and complicated. Especially that this structure addresses a RDF-specific problem. But, I don't have any more comments. Thank you for your patience :D
I agree with Tomasz that this looks complicated. With my lack of RDF knowledge, I hardly understand what's going on. :confused:
Comments from Reviewable
RubenVerborgh
commented
7 years ago Review status: all files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
I agree with Tomasz that this looks complicated. With my lack of RDF knowledge, I hardly understand what's going on. :confused:
Can't getCollection take JSON-LD (with an implicit or explicit context)? Then we can write this much more clearly.
Comments from Reviewable
elf-pavlik
commented
7 years ago drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Can't `getCollection` take JSON-LD (with an implicit or explicit context)? Then we can write this much more clearly.
I think the client should provide interface to control JSON-LD context. We just need to make it clear that developer shouldn't make any assumptions about JSON-LD context in received payloads and aways make sure to provide desired context to the client.
I would propose that once we agree on using manages block in the use case we should address client interface details in a future PR to https://github.com/HydraCG/Heracles.ts which implementation follows work on use cases...
Comments from Reviewable
lanthaler
commented
7 years ago Review status: all files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
I think the client should provide interface to control JSON-LD context. We just need to make it clear that developer shouldn't make any assumptions about JSON-LD context in received payloads and aways make sure to provide desired context to the client. I would propose that once we agree on using `manages` block in the use case we should address client interface details in a future PR to https://github.com/HydraCG/Heracles.ts which implementation follows work on use cases...
Yeah, I share that concerns as we "leak" quite a lot of RDF concepts here. Perhaps renaming manages to constraints or something similar and trying to come up with something replacing subject and object would solve that (we could eventually look at SHACL which recently became a W3C recommendation).
In the spirit of moving fast(er) I'd, howerver, like to propose to go ahead and merge this PR. If someone has a concrete proposal to improve this, it can be discussed in a separate PR. If no one objects, I'll merge this tomorrow.
Comments from Reviewable
alien-mcl
commented
7 years ago Review status: all files reviewed at latest revision, 1 unresolved discussion.
drafts/use-cases/5.creating-new-event.md, line 29 at r1 (raw file):
Yeah, I share that concerns as we "leak" quite a lot of RDF concepts here. Perhaps renaming `manages` to `constraints` or something similar and trying to come up with something replacing `subject` and `object` would solve that (we could eventually look at [SHACL](https://www.w3.org/TR/shacl/) which recently became a W3C recommendation). In the spirit of moving fast(er) I'd, howerver, like to propose to go ahead and merge this PR. If someone has a concrete proposal to improve this, it can be discussed in a separate PR. If no one objects, I'll merge this tomorrow.
Tha major issue I've got with getCollection is that the client need's to know ad-hoc that the result is a collection.
As for the JSON-LD filtering - I agree that we shouldn't be JSON-LD bound.
And yes - it seems like filtering. Do we have an agreement here?
Comments from Reviewable
Thise PR tries to address #126 by reusing already agreed on Collection Design and not introducing any new terms to hydra vocab.
This change is