Add modeling of Protocol test details to manifest#312
Conversation
This avoids the use of form-url encoding and makes the query/update strings easier to read.
|
I wrote a ~200 line perl test runner based on this new manifest data. The dependency chain is very large, so may not be super useful for people without a normal perl setup. YMMV. |
|
I've also left the old text-based @afs – I tried using this code against Fuseki and got 3 failures. One is expected ( |
Tpt
left a comment
There was a problem hiding this comment.
Thank you for this work! I have not reviewed carefully the content yet. Just found a detail
What errors do you get? Did you start the server Trying to work out what is actually sent so I have have misunderstood ... Looking at
It's two operations superimposed. I think that it will dispatch to query (that happens to come before GSP - a legal choice would also be GSP and then conneg error on the content type). Query checks the query string and FWIW the ASK is false - a dataset description is complete - the FROM sets the default graph there are no named graphs unless FROM NAMED is used as well.
The request is https://www.w3.org/TR/sparql12-protocol/#update-dataset says it's an error. 400. |
|
@afs – I get these errors:
I used fuseki quickstart, creating a "test" dataset, and using As mentioned, I think For
Yes. Dataset specified in the HTTP query parameters, content-type specifying SPARQL Query, and the request body with the
I don't understand "two operations superimposed". I don't think there's anything GSP-related going on here.
No error. It returns a 200 with SRX encoding of
I don't think that's true. The For This one should be an error because of the mixed use of The test as written (and approved by the previous WG) expects a 4xx error. I think that's the correct thing here, but the spec text doesn't actually specify this (saying only that this is "an error"). Fuseki returns a 500, so disagrees with the test, but not technically with the spec text. |
Yes, there would be both query and update if started with
How are you setting up the data? If started with The test says: (FWIW If you could send me the log file (printed to stdout) I can see the setup details and data loading, ideally, with This is Fuseki 6.0.0? I didn't look at |
Use SILENT form of CLEAR GRAPH to ensure graph is empty before the test runs, regardless of whether the graph already exists.
Combination of
My test runner is replacing /sparql/ with the path from the endpoint URL supplied to the test runner. It feels a bit strange, but I think that's probably required if we continue to use
Yes.
I manually validated the test. Fuseki works as expected. The test runner just doesn't know that because the validation logic isn't encoded in the manifest. I'd like to follow up this PR with a change to this test to make it simpler for test runners. I am pushing a small fix for |
|
(I have a log file from @kasei) I extracted the requests for the log and then used == bad_update_dataset_conflict Fuseki bug fixed - I now get 400, and not 500. == query_multiple_dataset The query request is
It has There is one very long name. If I do not encode == update_base_uri This test has a security problem. The execution may be on a machine behind a firewall. Depending on proxy/gateway protocol, the URL at point of execution has the local machine name/IP address, not that of the public side of the gateway. (A second problem is that update and query might be separate URLs.) Fuseki uses a fixed, dummy base name I get a result set with: Hacking the code to use the servlet URL, and it is |
Good catch. Fixed (along with the addition of an rdfs:comment on the manifest itself, describing some of the expectations and issues of running the tests).
I'm not sure I see the security issue here. The test is not trying to validate the specific IRI that is resolved, but only that the relative IRI *is resolved to some absolute IRI. So I think Fuseki is already passing the test as written (even if the expectations of the test are only provided in prose). That being said, the more I think about this test, the more I'm of the opinion that it is not really a protocol test. This requirement surely applies to the Protocol and any other means of submitting an update to the service (e.g. API). I'd be happy to just remove this test entirely. Thoughts? |
True - I see it as encouraging/highlighting bad behavior. (A system that rejected relative URIs wouldn't be such a bad thing.)
Personally - remove the test. |
|
I suggest we merge this - it may get wider review that way. |
|
@Tpt – any other comments before I merge? |
This adds modeling for all the existing Protocol tests based heavily on Gregg's work in #79 (which I think has diverged enough from the current manifest as a result of #306 that it's not worth trying to reconcile).
I am reasonably confident that this new data is a faithful encoding as they were produced automatically by parsing the HTTP requests in the previous
rdfs:commentstrings and modeling the resulting data. I used some regex matching to figure out the input for the expected results modeling.There are a few of differences from Gregg's original model:
"application/sparql-query; charset=UTF-16"is not also broken down into the header name and parameter elements)ht:statusCodeValuewith literal values like"4XX"(more on this below), instead using values such asht:StatusCode2xxwith a newmf:expectedStatuspropertyI introduce four new manifest terms to model the expected results:
mf:expectedStatus- Expected HTTP status code (pointing to values likeht:StatusCode2xx); this differs from Gregg's model which used_:response ht:statusCodeValue "2XX"which did not conform to the semantics ofht:statusCodeValue, and was my biggest issue with Fix protocol manifest #79mf:expectedBoolean- Expected results for ASK queries.mf:expectedFormat- Expected serialization format of the results; the range here is one of the literals:"boolean","tabular", or"RDF"(I'm open to changing this to a controlled set of IRIs if desired and with suggestions on where such IRIs might live)mf:expectation- A textual description of the expected results for the singel testupdate_base_uriwhere modeling the expectation would have been prohibitive. (Changing the test to check the expectation as aFILTERin the query and then usingASKmight be a better approach here.)I also (ab)use the SPARQL Update
ut:graphDatamodeling to indicate the named graphs that should be loaded into the Dataset before the test is run:For now those
ut:graphDataproperties are hanging off of the test itself, which feels a bit strange, but it didn't feel any better to hang them off of the mf:action which in this manifest is aht:Request. This is another area where I'm open to suggestions.Finally, I made a few substantive (but I expect uncontroversial) changes to a few tests. For the following tests, I removed the requirement to return results in SPARQL XML format (removing the Accept header in the request, and the expected Content-Type of the result)
update_dataset_default_graphsupdate_base_uriupdate_dataset_default_graphupdate_dataset_named_graphsupdate_dataset_full