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

feat: Add support for N3 Patch #1122

Merged
merged 1 commit into from
Jan 25, 2022
Merged

feat: Add support for N3 Patch #1122

merged 1 commit into from
Jan 25, 2022

Conversation

joachimvh
Copy link
Member

@joachimvh joachimvh commented Jan 20, 2022

📁 Related issues

Closes #1060

✍️ Description

Depends on #1119 due to changes in the PATCH configuration there.

Fully adds support for N3 patch as described in the spec.

Integration tests are based on those in NSS, making N3 Patch probably the most comprehensively tested part of the codebase.

@joachimvh joachimvh added the semver.minor Requires a minor version bump label Jan 20, 2022
@joachimvh
Copy link
Member Author

One thing I forgot to do in this PR (just saw it in my notes but forgot to implement): the n3 patch and sparql update body parsers should be wrapped in a class that only can handle PATCH requests. Probably the same class as in #1119 . Otherwise it would be impossible to store non-patch N3 documents after this change.

Copy link
Member

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

Great stuff! Good to have this in.

@@ -14,11 +14,11 @@
{
"comment": "Makes sure PATCH operations on containers target the metadata.",
"@type": "ContainerPatcher",
"patcher": { "@type": "SparqlUpdatePatcher" }
"patcher": { "@id": "urn:solid-server:default:PatchHandler_Type" }
Copy link
Member

Choose a reason for hiding this comment

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

I find the _Type suffix weird. Why is it not just urn:solid-server:default:PatchHandlers?

Copy link
Member Author

Choose a reason for hiding this comment

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

I find that to be a bit too generic. Could go for urn:solid-server:default:PatchHandler_RDF since this is the handler that is wrapped by other patch handlers that convert to quads first.

Copy link
Member

Choose a reason for hiding this comment

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

Perfect.

config/storage/middleware/stores/patching.json Outdated Show resolved Hide resolved
src/authorization/permissions/N3PatchModesExtractor.ts Outdated Show resolved Hide resolved
src/authorization/permissions/N3PatchModesExtractor.ts Outdated Show resolved Hide resolved
src/authorization/permissions/N3PatchModesExtractor.ts Outdated Show resolved Hide resolved
src/storage/patch/N3Patcher.ts Show resolved Hide resolved
src/storage/patch/N3Patcher.ts Outdated Show resolved Hide resolved
src/storage/patch/N3Patcher.ts Outdated Show resolved Hide resolved
src/storage/patch/N3Patcher.ts Outdated Show resolved Hide resolved
src/storage/patch/N3Patcher.ts Outdated Show resolved Hide resolved
Base automatically changed from feat/static-throw-handlers to versions/3.0.0 January 21, 2022 16:10
@joachimvh joachimvh merged commit a9941eb into versions/3.0.0 Jan 25, 2022
@joachimvh joachimvh deleted the feat/n3-patch branch January 25, 2022 10:30
@joachimvh joachimvh mentioned this pull request Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver.minor Requires a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants