Comments after implementing v0.2.0 editor's draft

joachimvh commented 1 year ago

I updated the CSS to be in line with the editor's draft of the notifications spec, and similarly to https://github.com/solid/notifications/issues/114 here are some considerations I had going through the spec. These are mostly more minor than the the previous issue though and perhaps a bit nitpicky. I also did not go through all the active issues so potentially I mention some things that are already discussed there. Also same disclaimer that I can create separate issues if necessary.

What is a data model?

Most of the issues I had are related to the concept of "data model". The data models, as they are described in the spec, do they represent an abstract "object" with the properties specified, or do they specifically mean the JSON-LD objects representing that data? So is a description resource "an object with an id property", or the JSON-LD serialization of that object with the notification context, and thus always having that specific JSON-LD format. In the first case other JSON-LD serialization of the same information, such as using @id instead or using http://www.w3.org/ns/solid/notifications#subscription as key in the JSON instead of subscription, would also be valid while in the second they wouldn't.

The reason to ask is to know what specific formats the server should support both for input as output. E.g., should we convert a subscription request to triples and interpret it like, or should we explicitly only support the JSON-LD framing as it is shown in the examples. Currently we do the latter, but I was thinking about potentially supporting the former.

Profile parameter

This is probably related to the previous point as I saw some discussions about using the profile parameter to indicate compacting or some form of framing. But as it currently is written in the spec this parameter has no use right? How I read the definition of profile is that only the behaviour of the values found there are defined, and other custom values should be defined in the corresponding spec. The notification spec only mentions the following:

Subscription Servers MUST respond with a 415 status code for HTTP requests with unsupported values in the profile parameter.

So is that currently the only thing a server has to do? Make sure that the profile parameter has the correct value if it is present, but not interpret it in any other way? I understand that this will probably be expanded upon in future versions.

receiveFrom

One receiveFrom property to identify the resource on the Notification Sender that can be used to establish a connection to receive notifications.

Just curious why this is not 0-1. I'm thinking of, for example, webhooks where this might not be relevant?

What to do with fields not supported by the context

As mentioned in https://github.com/solid/vocab/pull/85#issuecomment-1410066837 I still had some issues with the current version of the context. So with the goal of making sure the data we output is still valid RDF, we output the following when doing a GET on the webhook subscription:

{
  "@context": [
    "https://www.w3.org/ns/solid/notification/v1"
  ],
  "id": "http://localhost:3000/.notifications/WebSocketChannel2023/",
  "channelType": "http://www.w3.org/ns/solid/notifications#WebSocketChannel2023",
  "feature": [
    "accept",
    "endAt",
    "rate",
    "startAt",
    "state"
  ]
}

Was wondering if this is an OK solution (for now), or if we should perhaps introduce a second context to make sure the channelType can be shortened as expected.

CxRes commented 1 year ago

Some issues you raise are firmly WIP for inclusion in a v0.3 or later. Having said that, let me see if I can clarify some of your concerns:

Data Model

While the data models are meant to be abstract, servers MUST support at least JSON-LD as a format (must be able to output in it and must be able to parse it). Thus, as far as implementations are concerned, JSON-LD objects are enough for a minimum implementation. You are free to support other formats (say, e.g. turtle), though.

Having said this, if you find the specification text is mixing up abstract and implementation specific descriptions, please flag them through an issue or PR.

Profile Parameter

We had decided to remove the profile property for v0.2.0, given the flux in the protocol, with the intention of putting it back again at a later stage (hopefully v0.3.0). Therefore, I think we need to remove all references to profile for v0.2.0 release. I'll raise the matter in the meeting. You can leave it unimplemented (either ignore it or respond 415 in all cases where it is present), knowing that it will come back!

Receive From

The text you cite is conditional and specific to the case of Receiver initiated channels, therefore worded in that way. Refer to the line preceding the text you have quoted:

Where a Notification Receiver establishes the connection, the notification channel has the following additional property:

For channels where the Sender pushes, such as webHooks, there are separate paragraphs after the text you quoted:

Where a Notification Sender pushes notifications, the notification channel in the subscription request has the following additional property:

Zero or one sendTo property to identify the resource where the Notification Receiver can accept notifications. \ \ Where a Notification Sender pushes notifications, the notification channel in the subscription response has the following additional properties:

Zero or one sender property to identify the Notification Sender

Would you prefer these being placed under separate subheadings, or put another way, for greater clarity?

Fields...

The example is adequate (barring the minor typo of . in id). I think you can add a second context if you want, but it is not necessary. Though, I'll defer to the other authors on that.

joachimvh commented 1 year ago

While the data models are meant to be abstract, servers MUST support at least JSON-LD as a format

There are 2 issues I currently have with the data model then as I read it in the spec. Note that I understand that some of these things are still a work in progress and fully accept the answer "we know, this is something we're going to fix in the next version". As it stands now, the CSS implementation does assume the JSON-LD has the same formatting as the examples in the spec for input (but it would be not that hard to change to just interpret the RDF itself instead).

There is no explicit reference on how to go from the data model to RDF/JSON-LD (or I missed it).
Since there are no JSON-LD formatting restrictions the only correct way for servers/clients to interpret them is to convert to triples (which I'm fine with if that is the intended goal).

Regarding point 1, the spec states for example that a Description Resource has an id property and any amount of subscription and channel properties. But it doesn't say for example that if you want to interpret this as RDF it means having triples with the id property as subject and notify:subscription as predicate. Or that this should be interpreted as being a JSON-LD document with the id/subscription properties as being the same as those defined in the https://www.w3.org/ns/solid/notification/v1 context.

For point 2, the spec only states that JSON-LD is preferred (and required in certain situations), and that the context should be https://www.w3.org/ns/solid/notification/v1. But reusing my example from above, this is also a valid JSON-LD document that contains the same information for example:

{
  "@context": [
    "https://www.w3.org/ns/solid/notification/v1"
  ],
  "@id": "http://localhost:3000/.notifications/WebSocketChannel2023/",
  "notify:channelType": "http://www.w3.org/ns/solid/notifications#WebSocketChannel2023",
  "http://www.w3.org/ns/solid/notifications#feature": [ "..." ]
}

So the only way to interpret this the same way as the "standard" examples would be to first convert the JSON-LD to triples and interpreting those instead of the actual JSON.

The text you cite is conditional and specific to the case of Receiver initiated channels, therefore worded in that way.

Ah yes, I misread the line from the spec. I read that entire section as "these are all things that always happen to the receiver/sender" instead of "if this thing happens on the receiver/sender, you should do this".

Would you prefer these being placed under separate subheadings, or put another way, for greater clarity?

I'm not sure if there actually is something wrong with the text, could be that I just read it to fast or my mind just wanted to misinterpret. So you can just take it as a sample of 1 person who misread that.

barring the minor typo of . in id

Not sure what you mean as there is no typo in the example. http://localhost:3000/.notifications/WebSocketChannel2023/ is the default URL that the CSS will generate for the websocket channel if it is enabled.

elf-pavlik commented 1 year ago

Not sure what you mean as there is no typo in the example. http://localhost:3000/.notifications/WebSocketChannel2023/ is the default URL that the CSS will generate for the websocket channel if it is enabled.

I understand that http://localhost:3000/.notifications/WebSocketChannel2023/ is a Subscription Resource which subscription client can use to request Notification Channel of type WebSocketChannel2023.

@joachimvh can you help me with a short guidance on how to add support for StreamingHTTPChannel2023? I created dedicated issue for it https://github.com/CommunitySolidServer/CommunitySolidServer/issues/1574

CxRes commented 1 year ago

@elf-pavlik @joachimvh I presumed the path would be named /notifications/<channel>, and not /.notifications/<channel>. Though not technically wrong, it is a strange name for a path, that's all!

CxRes commented 1 year ago

Regarding the two points you raise: In my understanding the RDF is all that matters not the serializations, All the examples, (in the code boxes) are non-normative. So the second example, being equivalent to the first, is an equally valid response, and Clients must be prepared to handle that. Ditto for the server. I would also presume, like you describe, that internally you would handle these as triples; that way you can also support other formats (allowing clients to content-negotiate with you).

Based on your feedback (sample of one will do for now), we will discuss if we can make these things more explicit going forward.

csarven commented 1 year ago

I'll respond to the other points later but for now want to comment about the RDF/JSON-LD discussion:

RDF is the language and JSON-LD is the required format (for minimum interop.) Other concrete RDF syntaxes can be negotiated.

https://solid.github.io/notifications/protocol#json-ld-format (part of a normative section)
https://solid.github.io/notifications/protocol#resource-server-description-resource-accept (requirement)
https://solid.github.io/notifications/protocol#subscription-server-subscription-request-accept (requirement)
https://solid.github.io/notifications/protocol#subscription-server-subscription-request-content-type (requirement)
https://solid.github.io/notifications/protocol#notification-message (normative section - irrespective to using any RFC requirement level keywords. We/I'll update the section to have JSON-LD jump out more.)

joachimvh commented 1 year ago

Thanks for the explanation, I will update the CSS PR then to interpret the subscription requests as RDF instead of JSON. My only point related to that then is that I don't see in the specification how to go from the data model to RDF data as it doesn't specify the ontology and how the data model properties relate to that.

Though not technically wrong, it is a strange name for a path, that's all!

We generally use URLs with a dot in them for parts of the server that are not part of the core Solid protocol data management to differentiate between them, but can always be customized by whoever sets up the server.

elf-pavlik commented 1 year ago

I think the problem comes from the vocab and the JSON-LD context being stuck in a PR https://github.com/solid/vocab/pull/85

@joachimvh do you think JSON-LD frame would also be needed or we can reliably just compact it with provided JSON-LD context?

joachimvh commented 1 year ago

@joachimvh do you think JSON-LD frame would also be needed or we can reliably just compact it with provided JSON-LD context?

That depends on what your goal is. I was just talking about the link between the data model and the RDF ontology.

Framing would be necessary if you want server/clients to support a specific JSON-LD formatting. As a reference, for discovery CSS currently uses turtle by default, which you can content negotiate to JSON-LD but it's not going to be pretty JSON. If I dump that JSON into the JSON-LD playground I get the following JSON-LD:

{
  "@context": [ "https://www.w3.org/ns/solid/notification/v1" ]
  "@graph": [
    {
      "id": "http://localhost:3000/.well-known/solid",
      "type": "http://www.w3.org/ns/pim/space#Storage",
      "subscription": [
        "http://localhost:3000/.notifications/WebSocketChannel2023/"
      ]
    },
    {
      "id": "http://localhost:3000/.notifications/WebSocketChannel2023/",
      "channelType": "notify:WebSocketChannel2023",
      "feature": [
        "accept",
        "notify:endAt",
        "notify:rate",
        "notify:startAt",
        "state"
      ]
    }
  ]
}

Which is fine if you interpret it as an RDF resource but not if you expect a certain format to the JSON.

solid / notifications