nodeSolidServer / node-solid-server

Solid server on top of the file-system in NodeJS
https://solidproject.org/for-developers/pod-server
Other
1.78k stars 303 forks source link

Incorrect and incomplete Add activity data in Solid PREP responses #1799

Open csarven opened 2 weeks ago

csarven commented 2 weeks ago

Looking into implementing Solid PREP in dokieli. Cool stuff! Some bugs with the response data but they seem relatively minor.

Request (snippet):

$ curl -ki -H'Accept-Events: "prep"; accept="application/ld+json"' https://csarven.localhost:8443/dokieli/inbox/

Response (snippet):

HTTP/1.1 200 OK
X-Powered-By: solid-server/5.7.11
Accept-Events: "prep";accept=("message/rfc822" "application/ld+json" "text/turtle")
Events: protocol="prep", status=200, expires="Thu, 07 Nov 2024 15:13:48 GMT"
Content-Type: multipart/mixed; boundary="MAJ8bCuGxmcu9HkCOzUi"

--MAJ8bCuGxmcu9HkCOzUi
Content-Type: text/turtle

@prefix : <#>.
@prefix dct: <http://purl.org/dc/terms/>.
@prefix ldp: <http://www.w3.org/ns/ldp#>.
@prefix stat: <http://www.w3.org/ns/posix/stat#>.
@prefix xsd: <http://www.w3.org/2001/XMLSchema#>.
@prefix inbox: <>.
@prefix tur: <http://www.w3.org/ns/iana/media-types/text/turtle#>.

inbox:
    a ldp:BasicContainer, ldp:Container;
    dct:modified "2024-11-07T14:02:25Z"^^xsd:dateTime;
    ldp:contains
        <c86da0c8-087b-4d56-8c52-5cb682c07fe6.ttl>,
        <eb60ab84-80a5-4cd2-adf3-acc7a15180b4.ttl>,
        <eca044ed-c8b7-46fd-a469-c895df0ecd84.ttl>;
    stat:mtime 1730988145.945;
    stat:size 4096 .

--A+1x55W+mvT60TuzH1n5
Content-Type: application/ld+json

{
  "published": "Thu, 07 Nov 2024 14:15:58 GMT",
  "type": "Add",
  "object": "https://csarven.localhost/dokieli/inbox/",
  "target": "/dokieli/inbox/fa6ec1e0-dd3f-4b1f-b915-6dc3311351af.ttl",
  "state": "W/\"7-rM9AyJuqT6iOan/xHh+AW+7K/T8\"",
  "@context": [
    "https://www.w3.org/ns/activitystreams",
    "https://www.w3.org/ns/solid/notification/v1"
  ]
}

Note that the initial response is in text/turtle. Should that be application/ld+json?

Note the date format for as:published should have range in xsd:dateTime. Issue: NSS should convert the date format.


Another request (snippet):

$ curl -ki -H'Accept-Events: "prep"; accept="text/turtle"' https://csarven.localhost:8443/dokieli/inbox/

Response Headers

HTTP/1.1 200 OK
X-Powered-By: solid-server/5.7.11
Accept-Events: "prep";accept=("message/rfc822" "application/ld+json" "text/turtle")
Events: protocol="prep", status=200, expires="Thu, 07 Nov 2024 14:57:57 GMT"
Content-Type: multipart/mixed; boundary="KQman1p0csYxoyFdjhR2"

Notification

--chmdPhxmuTFKwWBW6gL2
Content-Type: text/turtle

@prefix as: <https://www.w3.org/ns/activitystreams#> .
@prefix notify: <https://www.w3.org/ns/solid/notification/v1#> .
@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .

<undefined> a as:Add ;
    notify:state "W/"7-rM9AyJuqT6iOan/xHh+AW+7K/T8"" ;
    as:object https://csarven.localhost/dokieli/inbox/ ;
    as:published "Thu, 07 Nov 2024 14:02:25 GMT"^^xsd:dateTime .

Note also data format here. Note undefined in subject IRI. Should probably be an http UUID if they can be eventually dereference but bnode with a UUID would do as well. The data also seems to be incomplete compared to response in application/ld+json.


I think the description of Add also needs a minor fix; swap object and target values. Taking the JSON-LD example from above, it should be like this:

{
  "id": "http://example.com/.well-known/genid/0798c962-1879-448f-af44-f83162a029be",
  "published": "2024-11-07T14:15:58Z",
  "type": "Add",
  "object": "/dokieli/inbox/fa6ec1e0-dd3f-4b1f-b915-6dc3311351af.ttl",
  "target": "https://csarven.localhost/dokieli/inbox/",
  "state": "W/\"7-rM9AyJuqT6iOan/xHh+AW+7K/T8\"",
  "@context": [
    "https://www.w3.org/ns/activitystreams",
    "https://www.w3.org/ns/solid/notification/v1"
  ]
}
CxRes commented 2 weeks ago

This is really a few independent issues (and I have taken the liberty to fix your request response pairs):

Note that the initial response is in text/turtle. Should that be application/ld+json

This is only one that's not a bug, but an idiosyncrasy introduced due to HTTP being originally designed for single request response. In the absence of an Accept header, NSS assumes that the preferred response for the first part is text/turtle. Whereas for notifications, you have requested application/ld+json. PREP has no intelligence to infer Accept from Accept-Events or vice-versa.

In your case, either set:

Accept-Events: "prep";accept="text/turtle"

or

Accept: application/ld+json
Accept-Events: "prep";accept="application/ld+json"

Note the date format for as:published should have range in xsd:dateTime. Issue: NSS should convert the date format.

This is a bug in the template. Do you know if NSS provide xsd:DateTime natively? Or I can do the conversion manually.


Note also data format here. Note undefined in subject IRI. ...

I need to show you the code and discuss with you. Lets continue this on Matrix.


The data also seems to be incomplete compared to response in application/ld+json.

I am generating the turtle by hand and missed these. I need to eventually switch to using rdflib.js. But I'll fix the template for now.


I think the description of Add also needs a minor fix; swap object and target values.

Can you check if a swap is also needed between origin and object for Delete? Fixed. Release Pending!

csarven commented 2 weeks ago

an idiosyncrasy

True that. I can live with that. Though from a consumer's perspective, the intuitive thing here would've been the whole payload defaulting to the format in Accept-Events when Accept is not provided. But, that's fine technically given Content-Type: multipart/mixed. Normally the application (at least mine) is sending Accept. It was just in the curl that I didn't include it.

Do you know if NSS provide xsd:DateTime natively? Or I can do the conversion manually.

Not sure what you mean natively but NSS does include dcterm:modified with xsd:dateTime as per https://solidproject.org/ED/protocol#contained-resource-metadata . So perhaps you can reuse/hook up to whatever that's generating that. Alternatively, you can probably just use the date object like date.toISOString() in your template?


The activity doesn't have to use an HTTP URI that such that when fetched it provides a representation. So, HTTP URI is okay to use independently, and can use /.well-known/genid/{uuid} for example. If and when it is dereferenced, it'll be a 404 until of course a representation can be provided in the future. urn:uuid:{uuid} can also work. But of course a bnode will do as well. Just for if nothing but aesthetics (besides collisions), I'd suggest to use a UUID value any way, _:{uuid}.I would suggest against using bnodes. These events should have an identity.

I don't have a strong opinion but I'd prefer the HTTP URI. It leaves the option open for the future to have it dereferenecable when everything else it depends on is worked out.

Most importantly, don't bake any semantics into the URI or bnode string. Move any significant information to a property describing the purpose and explain that in the specification so that consumers can implement it consistently.

CxRes commented 2 weeks ago

Move any significant information to a property describing the purpose and explain that in the specification so that consumers can implement it consistently.

Sure, but understand my reluctance about adding a new property without corresponding change in Solid Notifications Protocol which does not have semantics for last-event-id and uses only eTags (which are not always available on parent path notifications)! Introducing an event-id is a more general HTTP consideration than Solid (and inspired from SSE).

CxRes commented 1 week ago

I believe https://github.com/CxRes/node-solid-server/tree/basic-prep-fix-1 should address all your concerns. Once we can get eventID in the Solid Notifications vocabulary. I shall propose the PR.