Submit long queries as form POST; fix Dataset reading, writing and negotiation - #37
Merged
Merged
Conversation
…meant to be. SPARQLClient has carried both branches since 2020 - GET with the query in the URL, and a form POST once that URL would outgrow maxGetRequestSize - but it measured the wrong map to choose between them. `params` holds the caller's default-graph-uri/named-graph-uri and nothing else, normally empty; the query is in `mergedParams`. So the length compared was the endpoint URI's, ~27 characters, and the POST branch was unreachable for a query of any size. Every query therefore went out as GET. That is invisible until the URL-encoded query passes whatever the hop in front of the endpoint allows on a request line - 32 KB for Varnish's default http_req_size, which a label lookup over a few hundred object URIs reaches - and the connection is then dropped mid-write, surfacing as a Broken pipe and a 500 rather than anything that names a size. SPARQLClientTest covers the switch in both directions, and the request never leaves the process: a ClientRequestFilter records the method and aborts. Without the one-word fix, testLargeQueryIsSentAsPost fails with expected: <POST> but was: <GET>. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects, both from reading ModelProvider too closely. isReadable/isWriteable asked for Model - `type == Model.class` and `Model.class.isAssignableFrom(type)` - in the provider parameterised on Dataset. JAX-RS selects a provider by the entity's Java type, so nothing could read or write a Dataset entity at all: QuadStoreClient's get/add/ replace and SPARQLClient.loadDataset had no message body worker and failed with "MessageBodyWriter not found" or a null entity. The mirror image is that BOTH providers claimed Model, and only the sort by declared generic type kept ModelProvider in front - had it gone the other way, writeTo would have been handed a ModelCom and thrown ClassCastException. writeTo then asked isTriples() first and reduced the dataset to its default graph. Jena registers JSON-LD as triples AND quads, so a dataset served as JSON-LD silently lost every named graph, on a format chosen by content negotiation rather than by the caller. Asking isQuads() first gives the whole dataset to any language that can carry it, and leaves the default-graph reduction to the languages that genuinely cannot. DatasetProviderTest covers provider selection by type, both quad and triples languages in each direction, and the named graphs surviving every quad format. ModelProviderTest gains the symmetric assertion, so the exclusivity that keeps the two providers from competing is pinned from both sides rather than resting on selection order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y its graphs. DatasetProvider has written a dataset as triples since efc8d3a, reducing it to its default graph for a language that cannot express named ones. Nothing could ask for that: the writable Dataset list was filtered to isQuads, so the only consumer of it - QuadStoreImpl's response variants - never offered a triples representation and answered 406. The branch has been unreachable ever since MediaTypes moved onto Jena's lang registries. The triples languages are appended in a second pass, which is the whole point of doing it in two: these variants carry no q of their own, so their order decides what a client accepting anything is served, and that must not be the lossy one. A wildcard Accept still gets quads; only a client that explicitly asks for Turtle gets the default graph. Reading stays quad-only by negotiation, deliberately: the Accept header is ours to choose and should never invite a lossy response, while DatasetProvider still parses triples if a remote endpoint answers a CONSTRUCT with them regardless - which is not ours to choose. MediaTypesTest pins the per-class offerings and the ordering invariant; QuadStoreClientTest covers the round-trips, the triples request body, the triples response, and a wildcard Accept keeping the named graphs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Three defects surfaced by a dataspace of ~15.6M quads, each with a regression test that fails without its fix. 93 tests green (was 66).
A query too long for a URL is submitted as a form POST
SPARQLClienthas carried both branches since 2020, but chose between them on the wrong map:paramsholds only the caller's graph URIs (normally empty) while the query is inmergedParams, so the length compared was the endpoint URI's ~27 characters and the POST branch was unreachable at any size.Every query went out as GET, which only shows once the URL-encoded query passes what the hop in front of the endpoint allows on a request line. Measured against Varnish (32 KB
http_req_sizeby default):The failure surfaces as
SocketException: Broken pipeand a 500 — nothing that names a size. After the fix all three are 200.DatasetProvider reads and writes datasets, and keeps their named graphs
isReadable/isWriteableasked forModelin the provider parameterised onDataset. Since JAX-RS selects by the entity's Java type, noDatasetcould be read or written at all —QuadStoreClient.get/add/replaceandSPARQLClient.loadDatasethad no message body worker. Both providers claimedModel, and only the sort by declared generic type keptModelProviderin front; the other way round,writeTowould have been handed aModelComand thrownClassCastException.writeToalso askedisTriples()first. Jena registers JSON-LD as triples and quads, so a dataset served as JSON-LD silently lost every named graph — on a format chosen by negotiation, not by the caller. AskingisQuads()first gives the whole dataset to any language that can carry it.A dataset is offered as triples too
The triples branch of
writeTohas existed since efc8d3a but was unreachable: the writableDatasetlist was filtered toisQuads, soQuadStoreImplnever offered a triples variant and answered 406. The triples languages are now appended in a second pass, so every quad format precedes them — these variants carry noq, so their order is what a client accepting anything gets, and that must not be the lossy one. A wildcardAcceptstill gets quads; only an explicitAccept: text/turtlegets the default graph.Reading stays quad-only by negotiation, deliberately: the
Acceptheader is ours to choose and should not invite a lossy response, while the provider still parses triples if a remote endpoint answers a CONSTRUCT with them anyway — which is not ours to choose.Verification
expected: <POST> but was: <GET>,named graph lost in application/ld+json).🤖 Generated with Claude Code