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

Update notification channel discovery and conformance #104

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

csarven
Copy link
Member

@csarven csarven commented Aug 25, 2022

Resolves issue #58 by working with the notion of notification channel:

  • Adds definition for notification-channel-resource.
  • Describes notification-channel-discovery via notification-channel-resource (servers can advertise link relations describedby ( server-link-resource-notification-channel ) and http://www.w3.org/ns/solid#storageDescription ( server-link-storage-notification-channel ) and notification-channel description.
  • Adds notification-channel-conformance

Copy link
Member

@acoburn acoburn left a comment

Choose a reason for hiding this comment

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

This looks good, just a few clarifying questions

<span about="" id="server-notification-channel-resource-accept" rel="spec:requirement" resource="#server-notification-channel-resource-accept">
<span property="spec:statement">
<span rel="spec:requirementSubject" resource="spec:Server">Servers</span>
<span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> accept requests on the Notification Channel Resource when the value of the <code>Accept</code> header indicates a preferred representation in <code>application/ld+json</code> [<cite><a class="bibref" href="#bib-json-ld11">JSON-LD11</a></cite>].
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to require JSON-LD 1.1, or can we just say JSON-LD?

Copy link
Member Author

@csarven csarven Aug 25, 2022

Choose a reason for hiding this comment

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

I think JSON-LD will suffice and generally entails the latest published version.

But this reminds me to mention JSON-LD form - Should we require https://www.w3.org/TR/json-ld11/#compacted-document-form instead of @context with an array?

Copy link
Member Author

Choose a reason for hiding this comment

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

We know the spec works with JSON-LD11 so we should require and reference that specifically.

I will update the references to JSON-LD11 throughout the document separately - not part of this PR.

I can also follow-up on any changes to context and document-form in a separate PR. Discussion here or elsewhere can continue.

Copy link
Member

Choose a reason for hiding this comment

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

Should we require https://www.w3.org/TR/json-ld11/#compacted-document-form instead of @context with an array?

A document with "@context": [ ... ] is compacted document form

<section id="notification-channel-discovery" inlist="" rel="schema:hasPart" resource="#notification-channel-discovery">
<h4 property="schema:name">Notification Channel Discovery</h4>
<div datatype="rdf:HTML" property="schema:description">
<p about="" id="server-link-resource-notification-channel" rel="spec:requirement" resource="#server-link-resource-notification-channel"><span property="spec:statement">When a server wants to enable applications to discover <a href="#notification-channel">Notification Channels</a> available to a given <a href="https://solidproject.org/TR/protocol#resource">resource</a>, the <span rel="spec:requirementSubject" resource="spec:Server">server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> advertise the associated resources describing the channel information by responding to an HTTP request including a <code>Link</code> header with the <code>rel</code> value of <code>describedby</code> [<cite><a class="bibref" href="#bib-powder-dr">POWDER-DR</a></cite>] and the Description Resource as link target [<cite><a class="bibref" href="#bib-rfc8288">RFC8288</a></cite>].</span></p>
Copy link
Member

Choose a reason for hiding this comment

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

Powder (appendix D) puts no constraints on the format of the target of a describedby link relation (it could be an image). Should this requirement add a constraint such that a description be at least content-negotiatable as 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.

True. I think this is explained further in notification-channel-resource (RDF Document). Would that suffice?

Copy link
Member

Choose a reason for hiding this comment

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

That would suffice for me

@acoburn
Copy link
Member

acoburn commented Aug 26, 2022

I'm +1 to merge in the current form

@csarven csarven merged commit ba267dc into main Aug 26, 2022
@csarven csarven deleted the feature/notification-channel branch August 26, 2022 18:25
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.

2 participants