Introduced a collections helper property on IHypermediaContainer

[alien-mcl]

As mentioned in HydraCG/Specifications#155, I've introduced a helper property to filter out all collections.

---

This change is 

[lanthaler]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

control.type.contains(hydra.Collection)

What if they don't have the type set but are nevertheless referenced through the collection property?

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

control.type.contains(hydra.Collection)

What if they don't have the type set but are nevertheless referenced through the collection property?

Not sure which property do you refer to. Hydra does not define any collection predicate. Also the client's implementation does not have give any special meaning to such a relation. AFAIR, we agreed on explicit typing in order to get rid of any client-side type inference or reasoning processes (at least at the early stage of development).

---

Comments from Reviewable

[lanthaler]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

Not sure which property do you refer to. Hydra does not define any collection predicate. Also the client's implementation does not have give any special meaning to such a relation. AFAIR, we agreed on explicit typing in order to get rid of any client-side type inference or reasoning processes (at least at the early stage of development).

I'm referring to the hydra:collection property. The entry point use case shows how it is used (and here's the issue in which we decided to introduce it)

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

I'm referring to the hydra:collection property. The entry point use case shows how it is used (and here's the issue in which we decided to introduce it)

Ugh. I wasn't aware of this. This issue is 4 years old and the spec has not yet been updated!

Anyway, I'm not sure whether I understand hydra:collection correctly. While I think I understand why hydra:manages is introduced (to re-assert the relation between the collection's owner and it's members, i.e. foaf:knows), the former's semantics is unknown to me.

It could tell the client that all resources in that relation are of type hydra:Collection enabling an alternative way of hooking up collections to the resources. Am I correct?

---

Comments from Reviewable

[lanthaler]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

Ugh. I wasn't aware of this. This issue is 4 years old and the spec has not yet been updated!

Anyway, I'm not sure whether I understand hydra:collection correctly. While I think I understand why hydra:manages is introduced (to re-assert the relation between the collection's owner and it's members, i.e. foaf:knows), the former's semantics is unknown to me.

It could tell the client that all resources in that relation are of type hydra:Collection enabling an alternative way of hooking up collections to the resources. Am I correct?

Yes, the range of the hydra:collectionproperty is hydra:Collection. It has very simple semantics. It just tells the client, "here are a few collections that you might be interested in too", so it's somewhat similar to rdfs:seeAlso.

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

Yes, the range of the hydra:collectionproperty is hydra:Collection. It has very simple semantics. It just tells the client, "here are a few collections that you might be interested in too", so it's somewhat similar to rdfs:seeAlso.

Ok - I'll add support for this. Where can I find a draft of the spec that has all the latest features?

---

Comments from Reviewable

[alien-mcl]

Review status: 3 of 15 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

src/DataModel/HypermediaContainer.ts, line 40 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

Ok - I'll add support for this. Where can I find a draft of the spec that has all the latest features?

Done.

---

Comments from Reviewable

[lanthaler]

Reviewed 12 of 12 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

        },
        "required": "hydra:required",
        "collections": {

Please don't rename hydra properties here in the context (drop the "s" at the end) as it makes stuff unnecessarily complicated.

---

src/DataModel/HypermediaContainer.ts, line 42 at r2 (raw file):

entryPointCollections

This is not exclusive to entry points so you may want to rename this

---

src/DataModel/Collections/FilterableCollection.ts, line 64 at r2 (raw file):

   */
  public last(): T {
    let result: T;

Wouldn't this return undefined as you don't initialize it to null?

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

Please don't rename hydra properties here in the context (drop the "s" at the end) as it makes stuff unnecessarily complicated.

It was on purpose. I wanted to conceal as much of the RDF and JSON-LD as possible. Normally, I'd create a JSON structure where properties that normally would hold multiple values would end up with plural names. This is what I'm trying to do with that context.

---

src/DataModel/HypermediaContainer.ts, line 42 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

entryPointCollections

This is not exclusive to entry points so you may want to rename this

True that - done

---

src/DataModel/Collections/FilterableCollection.ts, line 64 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

Wouldn't this return undefined as you don't initialize it to null?

Oh yes. I'm wondering why my IDE complains about an unused variable initializer. Still, I'd prefer to return null instead of undefined.

---

Comments from Reviewable

[lanthaler]

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

It was on purpose. I wanted to conceal as much of the RDF and JSON-LD as possible. Normally, I'd create a JSON structure where properties that normally would hold multiple values would end up with plural names. This is what I'm trying to do with that context.

Then this should be done consistently. All other properties are singular. E.g. operation, supportedClass, supportedProperty, ... To keep changes at a manageable level in this PR I'd suggest keep this singular as well for now and if really desired switch to plural everywhere in a subsequent PR.

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

Then this should be done consistently. All other properties are singular. E.g. operation, supportedClass, supportedProperty, ... To keep changes at a manageable level in this PR I'd suggest keep this singular as well for now and if really desired switch to plural everywhere in a subsequent PR.

True that. I believe this was due to fact those you mentioned were added at the very beginning automatically as we're not using them in any way for the time being. Still, I'd update those to plural if that's OK.

---

Comments from Reviewable

[alien-mcl]

Can we move on with this PR?

---

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

Comments from Reviewable

[tpluscode]

Reviewed 1 of 5 files at r1, 2 of 12 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/DataModel/HypermediaContainer.ts, line 45 at r3 (raw file):

      itemsArray
        .filter(control => !!(control as any).collections)
        .map(control => Array.from((control as any).collections) as ICollection[])[0] || [];

Could you explain availableCollections? I don't understand from the code

---

src/DataModel/IHydraResource.ts, line 17 at r3 (raw file):

   * @returns {ResourceFilterableCollection<ICollection>}
   */
  readonly collections: ResourceFilterableCollection<ICollection>;

Do we really want hydra:collection on every resource? It strikes me as odd on templated link...

---

src/DataModel/Collections/FilterableCollection.ts, line 65 at r3 (raw file):

  public last(): T {
    let result: T = null;
    for (const item of this) {

Call it premature optimisation but looping over the items seems a bit inefficient.

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/DataModel/HypermediaContainer.ts, line 45 at r3 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…

Could you explain availableCollections? I don't understand from the code

I've updated code to be clearer about it. Hope it helps.

---

src/DataModel/IHydraResource.ts, line 17 at r3 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…

Do we really want hydra:collection on every resource? It strikes me as odd on templated link...

This interface is considered as an equivalent of hydra:Resource class. It currently has a hydra:operation predicate, thus operation property is created. But if the operations are available I see no objection to having also links exposed here. And if links are exposed, why not exposing collections separately.
Indeed, in many places these will be empty collections, but API user won't have to property-check on every attempt to access these.
I'm still opened on discussion which interfaces should extend IHydraResource though so we can get rid of some of these properties here and there.

---

src/DataModel/Collections/FilterableCollection.ts, line 65 at r3 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…

Call it premature optimisation but looping over the items seems a bit inefficient.

Hmm. This is not an Array, thus I cannot get the item at the array.length - 1. Also using Array.from will iterate all items. I see no other approach here that won't incorporate full iteration over all whole collection.

---

Comments from Reviewable

[lanthaler]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

True that. I believe this was due to fact those you mentioned were added at the very beginning automatically as we're not using them in any way for the time being. Still, I'd update those to plural if that's OK.

I'd prefer them to be exactly the same as in the Hydra vocab but as long as it is consistent I'm fine either way.

---

src/DataModel/HypermediaContainer.ts, line 45 at r3 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

I've updated code to be clearer about it. Hope it helps.

@tpluscode can we resolve this?

---

Comments from Reviewable

[tpluscode]

---

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[lanthaler]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

I'd prefer them to be exactly the same as in the Hydra vocab but as long as it is consistent I'm fine either way.

Karol. This is the only comment left. Would be nice to move on with this

---

Comments from Reviewable

[alien-mcl]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…

Karol. This is the only comment left. Would be nice to move on with this

Ugh - sorry, I missed that one. Done - it is as in the vocab now.

---

Comments from Reviewable

[lanthaler]

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

integration-tests/server/api/context.jsonld, line 47 at r2 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…

Ugh - sorry, I missed that one. Done - it is as in the vocab now.

Thanks

---

Comments from Reviewable

[lanthaler]

Please resolve the merge conflicts

[lanthaler]

Reviewed 1 of 9 files at r6.
Review status: 8 of 13 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[lanthaler]

Reviewed 5 of 9 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable