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

Basic prep #1797

Merged
merged 24 commits into from
Nov 4, 2024
Merged

Basic prep #1797

merged 24 commits into from
Nov 4, 2024

Conversation

bourgeoa
Copy link
Member

@bourgeoa bourgeoa commented Nov 4, 2024

No description provided.

CxRes and others added 24 commits August 31, 2024 01:28
Implements Per Resource Events notifications in Node Solid Server.

Implementation Notes:
+ Uses `--experimental-require-module` to load esm packages natively. Requires node > 22.0.0. Start scripts and test invocations have been appropriately modified.
+ NSS converts strings into an older streaming format which was not being detected by Express-PREP. Express-PREP was modified to ask the user if the body provided is a stream, thus circumventing this issue.
+ Notifications are triggered from a common middleware, which is invoked after the response has been succesfully sent.
+ Uses Express PREP supplied default template for notifications.
Add Solid/Activity Streams format notifications:
+ Provides notifictions in JSON-LD and Turtle.
+ Extends notifications to PUT and POST methods.
+ Add Event-ID header field to the response of a write method.
nyc does not work with ESM called with require().
Parent path is correctly determined only for non root resources. Parent notifications are only generated when resources have a parent.
Node 20.17.0 LTS supports require(ESM). Therefore, allowing current Node LTS as well.
When checking response `Content-Type` during integration tests, relaxed all media-type values to regex.
Picks the repository from where the PR branch exists.
Fixes the following issues:
+ Linebreaks are not sent after boundaries in the same notification, instead they are sent at the start of notification.
+ DELETE triggers lastEvent only if the path for notification is the same as the one where events occured.
PREP can be disabled when the server is started with the `--no-prep` flag.
PREP dependencies have been updated to the latest versions that use the newest HTTP Structured Fields specification RFC9651.
Notifications on the parent container are now sent when the parent is root.
Notification emitted when a resource is created as a result of POST on a container has  activity type set to `as:Add`.
Notification emitted when a resource is created as a result of POST on a container sets the `target` property to the `Location` header, consistent with the requirements of `as:Add`.
Notification emitted on the container when a resource in it is removed, sets the `origin` property to the location of the removed resource, consistent with the requirements of `as:Remove`.
Added a test to check that a PREP notification is correctly emitted when a container is created with POST.
This fixes "undefined" notifications being emitted when the requested notification format is not Solid PREP RDF.
`req.url` is not overwritten anymore when PUT is used to create a container. This allows subsequent middlewares to have access to the original request URL.
Added tests to check that PREP notifications are correctly emitted on a container, when a container inside it is created or deleted.
Added debugging statements for failed notifications. Also clarified that notification errors should not be passed on to Express for error handling, since the mutation that triggered the notification was already successful.
When the negotiated media-type for notifications is not RDF, the `location` property is correctly set to the `Location` header in the generated notifications. Express-PREP now handles this by default.
@bourgeoa bourgeoa merged commit fd5837c into main Nov 4, 2024
2 checks passed
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