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

fix: id and state in Data Model #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

CxRes
Copy link
Member

@CxRes CxRes commented Nov 12, 2024

  • id is a IRI or bnode (not string).
  • notify:state (not id) can be used to set Last-Event-ID.
  • Add prefixes to properties in Data Model.

Fixes #11.

+ `id` is a IRI or bnode (not string).
+ `notify:state` (not `id`) can be used to set `Last-Event-ID`.
+ Add prefixes to properties in Data Model.
@CxRes CxRes requested review from csarven and elf-pavlik November 12, 2024 22:47
<dt id="notification-property-id"><code>*id* &lt;xs:string></code>
<dd> an opaque identifier for the notification. Can be used to set `Last-Event-ID` in a subsequent [PREP] notifications request.
<dt id="notification-property-id"><code>*id* &lt;rdfs:subject></code>
<dd> an unique identifier for a notification.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<dd> an unique identifier for a notification.
<dd> a unique identifier for a notification.

But why not just say URI or IRI here? I don't have the full view of the spec right now, if that's clear from the rest of the text, okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I cannot find an authoritative source for that. I researched RDF specifications quite a bit.

Copy link
Member

Choose a reason for hiding this comment

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

OK, perhaps it would suffice to staying close JSON-LD language here. See @id in https://www.w3.org/TR/json-ld11/#syntax-tokens-and-keywords

Copy link
Member Author

@CxRes CxRes Nov 12, 2024

Choose a reason for hiding this comment

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

I have seen that. I am convinced a notification should not be a bnode e.g. para 3 here.

But, I cannot find any such description of subjects, closest being one you point to above, which allows bnodes. Even that does not have any analogue in RDF specs. The language used in specifications is obtuse and all over the place.

@@ -8,13 +8,13 @@ A [PREP] notification from an LDP Resource on a [=Solid server=] MUST have the f

<dl>

<dt id="notification-property-id"><code>*id* &lt;xs:string></code>
<dd> an opaque identifier for the notification. Can be used to set `Last-Event-ID` in a subsequent [PREP] notifications request.
<dt id="notification-property-id"><code>*id* &lt;rdfs:subject></code>
Copy link
Member

Choose a reason for hiding this comment

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

Is mention of rdfs:subject necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the closest thing I could find that indicates, that id is expected to be IRI. How else can I specify the type of id using RDF?

Copy link
Member

Choose a reason for hiding this comment

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

See also https://www.w3.org/TR/xmlschema11-2/#anyURI

I don't have a strong opinion on only using IRI here. So, I'll dial back and suggest that a Note about what implementers may want to consider can be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy for you to suggest one. But in another PR so that it can be traced!

Copy link
Member

Choose a reason for hiding this comment

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

Saying IRI should be enough; using rdf:subject seems confusing and possibly incorrect.

Copy link
Member Author

Choose a reason for hiding this comment

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

@elf-pavlik The context above as @csarven points out is that, JSON-LD spec allows @id to be IRI or bnode. The latter, though not meaningful, is not expressly prohibited, and I don't want to add additional constraints without reason. I open to suggestion as to how to specify type for id.

Copy link
Member

Choose a reason for hiding this comment

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

in #18 :Notification IRI { requires it to be an IRI

Co-authored-by: Sarven Capadisli <info@csarven.ca>
@CxRes CxRes requested a review from csarven November 13, 2024 00:14
Copy link
Member

@elf-pavlik elf-pavlik left a comment

Choose a reason for hiding this comment

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

I think it is an improvement once rdfs:subject is replaced with IRI

I created #18 to possibly improve it even further

@@ -8,13 +8,13 @@ A [PREP] notification from an LDP Resource on a [=Solid server=] MUST have the f

<dl>

<dt id="notification-property-id"><code>*id* &lt;xs:string></code>
<dd> an opaque identifier for the notification. Can be used to set `Last-Event-ID` in a subsequent [PREP] notifications request.
<dt id="notification-property-id"><code>*id* &lt;rdfs:subject></code>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<dt id="notification-property-id"><code>*id* &lt;rdfs:subject></code>
<dt id="notification-property-id"><code>*id* IRI</code>

Copy link
Member Author

Choose a reason for hiding this comment

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

IRI itself is not a data type...

Copy link
Member

Choose a reason for hiding this comment

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

only literals have data type, not named or blanked nodes https://www.w3.org/TR/rdf11-concepts/#section-Graph-Literal

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.

Add a Note about the subject IRI of the Activity
3 participants