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

Enable hydra to expect and return non RDF resources #200

Conversation

alien-mcl
Copy link
Member

Summary

I've changed the rdfs:range of hydra:expects and hydra:returns predicates from hydra:Class to hydra:Resource and introduced schema:rangeIncludes for both.
I've also mentioned that fact in the spec.

More details

This PR addresses issues #22 and #199 but does NOT resolves them. It is also an alternative approach to PRs #186 and #187.

Sole purpose is to enable the vocabulary to expect and return non-RDF resources. Previous specification placed a hydra:Class in the range the proper predicates making it difficult to use other types of resources.
Unfortunately, modification within the rdfs:range of the aforementioned predicates may introduce possible issues within current implementations and specifications built on top of the vocabulary. Still, it seems to be the least intrusive method due to facts:

  • the hydra:Class used previously is still compliant with newly introduced hydra:Resource
  • schema:rangeIncludes introduced does not apply strict RDF type entailing rules yet still provides semantic hints on possible values.

@@ -253,7 +254,8 @@
"label": "expects",
"comment": "The information expected by the Web API.",
"domain": "hydra:Operation",
"range": "hydra:Class",
"range": "hydra:Resource",
"rangeIncludes": ["hydra:Resource", "hydra:Class"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's redundant to repeat hydra:Resource here. In fact, a Resource itself is not useful as an object of expects/returns. It has to be something more specific, like Class

Suggested change
"rangeIncludes": ["hydra:Resource", "hydra:Class"],
"rangeIncludes": ["hydra:Class"],

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't agree. I think it's useful to have both predicates to contain hydra:Resource. Previously used rdfs:range is more for compatibility, but I'd like to use schema:rangeIncludes in future implementations - it has weaker semantics.

I'd leave it as I suggested, at least untill some more specific alternatives are available.

Copy link
Member

@asbjornu asbjornu Sep 10, 2019

Choose a reason for hiding this comment

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

Isn't hydra:Resource what enables non-RDF payloads? I.e., won't hydra:Class restrict payloads to RDF?

Copy link
Member Author

Choose a reason for hiding this comment

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

@asbjornu, schema:rangeIncludes doesn't have that strong semantics as rdfs:range. Putting only hydra:Class inside schema:rangeIncludes doesn't infer no other types can be used - it's more of a hint (well the name has it - range includes, but not restricts). Still, I'd prefer to have both so clients doesn't have to concatenate both rdfs:range and schema:rangeIncludes to discover what's actually allowed.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for the explanation, @alien-mcl!

Copy link
Contributor

Choose a reason for hiding this comment

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

But Resource is range includes is actually not truthful. Most classes of Hydra are subclasses of hydra:Resource but they will not make sense as expects/returns. This hint is misleading.

@@ -262,7 +264,8 @@
"label": "returns",
"comment": "The information returned by the Web API on success",
"domain": "hydra:Operation",
"range": "hydra:Class",
"range": "hydra:Resource",
"rangeIncludes": ["hydra:Resource", "hydra:Class"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Suggested change
"rangeIncludes": ["hydra:Resource", "hydra:Class"],
"rangeIncludes": ["hydra:Class"],

spec/latest/core/index.html Outdated Show resolved Hide resolved
spec/latest/core/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@tpluscode tpluscode left a comment

Choose a reason for hiding this comment

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

Having discussed this again with @alien-mcl we came to an agreement how keeping hydra:Resource can actually be quite useful for a client and allow great flexibility at little cost.

Will expand in a prose PR explaining the client's behavior.

@alien-mcl alien-mcl merged commit adee1cf into HydraCG:master Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants