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

SPARQL update support #19

Merged
merged 1 commit into from
Dec 8, 2023
Merged

SPARQL update support #19

merged 1 commit into from
Dec 8, 2023

Conversation

steve-bate
Copy link
Contributor

@steve-bate steve-bate commented Dec 7, 2023

  • Added support for SPARQL update requests
  • Added support for "direct" query (and update) requests
  • Added option to CLI to enable updates
    • Optional RDFLIB_APIKEY can be used to authorize updates.
  • Added unit tests for SPARQL update
    • POST only
    • With and without API key
    • request provided as body form, body direct and URL query params
  • Added unit test for "direct" query
  • Modified unit tests to use dictionaries for form-based data arguments so the content-type will be set correctly.

@vemonet

Closes #17

@steve-bate
Copy link
Contributor Author

steve-bate commented Dec 7, 2023

The builds seem to be failing because of a missing SonarCloud key?

@steve-bate steve-bate force-pushed the sparql-updates branch 4 times, most recently from 1bc23d5 to 1e2d82f Compare December 7, 2023 14:07
@vemonet
Copy link
Owner

vemonet commented Dec 7, 2023

Yes, no problem, I'll fix the workflow so that it does not try to push coverage when it's a PR, you should be able to rebase easily since you made no changes to the workflows

Thanks a lot @steve-bate! Really nice improvements! Users can easily choose to enable or disable update, and set an API key.

Just one thing: I know RDFLib have a different function for query and update. But I don't really like this approach, a SPARQL query is a SPARQL query, the user (and YASGUI) should not need to have to think about which param they need to use, they just send a SPARQL query to the endpoint. We can easily parse it to know if it's a query or an update

Would it be possible to make the changes to have only a query param please? But if there is a good argument for having query and update in different params feel free to share it :)

@steve-bate
Copy link
Contributor Author

But if there is a good argument for having query and update in different params feel free to share it :)

Yasgui uses an update form parameter. I think that's consistent with what's required by the SPARQL 1.1 Protocol Recommendation:

The update operation is used to send a SPARQL update request to a service. The update operation must be invoked using the HTTP POST method. Client requests for this operation must include exactly one SPARQL update request string (parameter name: update) ...
https://www.w3.org/TR/sparql11-protocol/#update-via-post-urlencoded

However, after rereading that section, it appears that update is not supported in POST URL query string (only as a URL-encoded form parameter in the body or as a direct body). In fact, neither query nor update are supported as URL query parameters for a POST. I can remove support for that.

@codecov-commenter
Copy link

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (52bf504) 94.37% compared to head (856d9ad) 94.20%.
Report is 1 commits behind head on main.

Files Patch % Lines
src/rdflib_endpoint/sparql_router.py 91.30% 6 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #19      +/-   ##
==========================================
- Coverage   94.37%   94.20%   -0.17%     
==========================================
  Files           4        4              
  Lines         231      259      +28     
==========================================
+ Hits          218      244      +26     
- Misses         13       15       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vemonet
Copy link
Owner

vemonet commented Dec 8, 2023

Thanks for digging up the specs @steve-bate ! They don't make it really clear, but I think there is nothing in there that prevents to use the same API endpoint for query and update, they never mention they should be distinct endpoints

I asked chatGPT 4 to preprocess it for us and according to it:


Based on the specification for the SPARQL Protocol that you provided, it is indeed possible to use the same API endpoint to serve both query and update operations. The protocol defines different HTTP methods and content types for distinguishing between query and update requests, allowing them to be processed differently even if they are sent to the same endpoint.

Key Points:

  1. Query Operation:
    • Can be invoked using either the HTTP GET or POST method.
    • Requires a SPARQL query string (query parameter) and may include graph URIs.
    • Supports different content types for requests (application/x-www-form-urlencoded for URL-encoded POST, application/sparql-query for POST directly).
  2. Update Operation:
    • Must be invoked using the HTTP POST method.
    • Requires a SPARQL update request string (update parameter) and may include graph URIs.
    • Supports application/x-www-form-urlencoded for URL-encoded POST and application/sparql-update for POST directly.

Implementation Considerations:

  • Endpoint Design: The same endpoint can be configured to handle both types of operations. The server-side logic should inspect the HTTP method and content type to determine whether the request is a query or an update.
  • Security and Permissions: It's crucial to implement proper security measures, especially for update operations, as they modify the data. Authentication and authorization mechanisms should be in place to control access.
  • Error Handling: Proper error responses should be implemented as per the protocol specification, distinguishing between query-related and update-related errors.

I noticed that to push changes you are fixing and force-pushing the original commit, which makes it really hard to see the evolution of your implementation

Please don't do it in the future! Especially when contributing to another repository (even for you own repo, you will see with the time it is nicer to have many commits to manage your code)

It is much better for the reviewers (and you!) to create multiple commits, even if some of the changes you made in the first commits are overridden latter. This way we can better see the different changes that the code went through

@steve-bate
Copy link
Contributor Author

Maybe I'm misunderstanding your comments. This PR supports both types of requests being performed with the same URL. For example a POST to https://server.example/sparql/ could be either a query or an update depending on the content type and the body form parameter if the content type is application/x-www-form-urlencoded. I think that's consistent with the ChatGPT information (and what I quoted earlier from the spec), but let me know what I'm missing. Thanks.

@vemonet
Copy link
Owner

vemonet commented Dec 8, 2023

You are right @steve-bate, I just checked it more in details on my laptop, and your implementation seems to fit perfectly the specs defined on the w3c website for update

And I see that YASGUI automatically set the query or update params depending if it is a SELECT or INSERT, so nothing to do on this side

Really nice job, thanks for all the improvements!

@vemonet vemonet merged commit 79c7c55 into vemonet:main Dec 8, 2023
8 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.

SPARQL Updates
3 participants