Behavior for other SPARQL Update operations#323
Open
kjetilk wants to merge 2 commits intofeature/sparql-updatefrom
Open
Behavior for other SPARQL Update operations#323kjetilk wants to merge 2 commits intofeature/sparql-updatefrom
kjetilk wants to merge 2 commits intofeature/sparql-updatefrom
Conversation
ericprud
approved these changes
Oct 16, 2021
| <span about="" id="server-patch-sparql-not-other" rel="spec:requirement" resource="#server-patch-sparql-not-other"><span property="spec:statement"> except that <span rel="spec:requirementSubject" resource="spec:Server">servers</span> <span rel="spec:requirementLevel" resource="spec:MUSTNOT">MUST NOT</span> allow a request with a <code>PATCH</code> method to change other resources than the target resource.</span></span> [<a href="https://github.com/solid/specification/issues/125#issuecomment-873035679" rel="cito:citesAsSourceDocument">Source</a>] | ||
| <span about="" id="server-patch-sparql-outside-subset" rel="spec:requirement" resource="#server-patch-sparql-outside-subset"><span property="spec:statement"><span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a request body containing a SPARQL query that falls outside of the subset they are able to process <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error.</span></span> | ||
|
|
||
| <span about="" id="server-patch-sparql-not-update" rel="spec:requirement" resource="#server-patch-sparql-not-update"><span property="spec:statement">SPARQL Update [<cite><a class="bibref" href="#bib-sparql-overview">SPARQL</a></cite>] also defines operations <code>LOAD</code>, <code>CLEAR</code>, <code>CREATE</code>, <code>DROP</code>, <code>COPY</code>, <code>MOVE</code> and <code>ADD</code>. <span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a <code>PATCH</code> request body containing these operations <span rel="spec:requirementLevel" resource="spec:SHOULD">SHOULD</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error, or MUST ensure that any access control measures apply.</span></span> |
Member
There was a problem hiding this comment.
or MUST ensure that any access control measures apply.
The 'or' there is slightly confusing -- shouldn't that be an 'and'?
TallTed
reviewed
Nov 10, 2021
| <span about="" id="server-patch-sparql-not-other" rel="spec:requirement" resource="#server-patch-sparql-not-other"><span property="spec:statement"> except that <span rel="spec:requirementSubject" resource="spec:Server">servers</span> <span rel="spec:requirementLevel" resource="spec:MUSTNOT">MUST NOT</span> allow a request with a <code>PATCH</code> method to change other resources than the target resource.</span></span> [<a href="https://github.com/solid/specification/issues/125#issuecomment-873035679" rel="cito:citesAsSourceDocument">Source</a>] | ||
| <span about="" id="server-patch-sparql-outside-subset" rel="spec:requirement" resource="#server-patch-sparql-outside-subset"><span property="spec:statement"><span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a request body containing a SPARQL query that falls outside of the subset they are able to process <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error.</span></span> | ||
|
|
||
| <span about="" id="server-patch-sparql-not-update" rel="spec:requirement" resource="#server-patch-sparql-not-update"><span property="spec:statement">SPARQL Update [<cite><a class="bibref" href="#bib-sparql-overview">SPARQL</a></cite>] also defines operations <code>LOAD</code>, <code>CLEAR</code>, <code>CREATE</code>, <code>DROP</code>, <code>COPY</code>, <code>MOVE</code> and <code>ADD</code>. <span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a <code>PATCH</code> request body containing these operations <span rel="spec:requirementLevel" resource="spec:SHOULD">SHOULD</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error, or MUST ensure that any access control measures apply.</span></span> |
Contributor
There was a problem hiding this comment.
Suggested change
| <span about="" id="server-patch-sparql-not-update" rel="spec:requirement" resource="#server-patch-sparql-not-update"><span property="spec:statement">SPARQL Update [<cite><a class="bibref" href="#bib-sparql-overview">SPARQL</a></cite>] also defines operations <code>LOAD</code>, <code>CLEAR</code>, <code>CREATE</code>, <code>DROP</code>, <code>COPY</code>, <code>MOVE</code> and <code>ADD</code>. <span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a <code>PATCH</code> request body containing these operations <span rel="spec:requirementLevel" resource="spec:SHOULD">SHOULD</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error, or MUST ensure that any access control measures apply.</span></span> | |
| <span about="" id="server-patch-sparql-not-update" rel="spec:requirement" resource="#server-patch-sparql-not-update"><span property="spec:statement">SPARQL Update [<cite><a class="bibref" href="#bib-sparql-overview">SPARQL</a></cite>] also defines operations <code>LOAD</code>, <code>CLEAR</code>, <code>CREATE</code>, <code>DROP</code>, <code>COPY</code>, <code>MOVE</code> and <code>ADD</code>. <span rel="spec:requirementSubject" resource="spec:Server">Servers</span> that receives a <code>PATCH</code> request body containing these operations (1) <span rel="spec:requirementLevel" resource="spec:SHOULD">SHOULD</span> respond with a <code>422</code> status code [<cite><a class="bibref" href="#bib-rfc4918">RFC4918</a></cite>] and a message body that explains the error, and (2) MUST ensure that any access control measures apply.</span></span> |
Contributor
There was a problem hiding this comment.
I think it's an OR, not an AND.
EITHER ensure measures apply OR respond with an error.
Not both.
Contributor
There was a problem hiding this comment.
@michielbdejong -- Note that my suggestion sprang from @dmitrizagidulin's earlier comment. I don't think "EITHER MUST ensure measures apply OR SHOULD respond with an error" makes sense. Does one of these work?
- "MUST EITHER ensure measures apply OR respond with an error"
- "SHOULD EITHER ensure measures apply OR respond with an error"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As noted in #125, SPARQL Update contains several operations beyond DELETE and INSERT. It has been an open question what to do with them. Since they are not currently supported and thus falls outside of the supported subset, I suggest we simply disallow them on a SHOULD level.
I think we should review this soon with respect to
COPYandMOVEin light of #19 , but I think this is the most straightforward way to address it right now.I made this a separate PR to not tie up #320, but I hope we can get it in for completeness.