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

Send PUT requests instead of PATCH requests #16

Closed
RubenVerborgh opened this issue Feb 14, 2019 · 2 comments
Closed

Send PUT requests instead of PATCH requests #16

RubenVerborgh opened this issue Feb 14, 2019 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@RubenVerborgh
Copy link
Member

Unfortunately, PATCH requests are broken on node-solid server 4.x and 5.x in at least two ways:

  1. node-solid-server does not follow the SPARQL UPDATE spec when a DELETE clause when there are no or multiple matches (PATCH requests with non-matching deletions should succeed nodeSolidServer/node-solid-server#1085), so the client gets unexpected 409 responses.
  2. node-solid-server does not implement file locking (Locking files when writing to avoid concurrency issues nodeSolidServer/node-solid-server#137), so accidentally concurrent patches can destroy a file, so the client gets unexpected 500 responses.

The first thing is more or less easily fixable, but deciding about the right fix is a longer process (linkeddata/rdflib.js#299). But even if we fix it, the client doesn't know whether it is talking to a fixed or a legacy server, and both require different approaches.

The second thing is hard to fix, as code for writing files is spread all over node-solid-server.

Hence, HTTP PATCH is currently not a reliable way forward. As such, I propose to:

  • apply the patch locally on the client, and update the remote resource through PUT
  • use the eTag mechanism to ensure we are not overwriting other people's changes

This solution is not ideal because it is inefficient for bandwidth, and will cause performance problems with larger files. However, it avoids file corruption, which is what we are currently risking, and it does not require changes in the client code (which was not broken in the first place).

TL;DR: PATCH is broken on the server, but we cannot easily fix it there; instead of forcing a workaround in clients, we can put that workaround in LDflex.

@rubensworks
Copy link
Contributor

We could implement this in LDflex, but disable it by default behind a config option, which could then be enabled automatically when using query-ldflex.

@RubenVerborgh
Copy link
Member Author

Postponed because of nodeSolidServer/node-solid-server#1085 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants