Repository navigation
SOLR-16396: Migrate GET /config/requestHandler to JAX-RS - #5168
Conversation
| @QueryParam("componentName") | ||
| @Parameter(description = "Name of the specific request handler") | ||
| String component, |
epugh
left a comment
There was a problem hiding this comment.
double check some of the chnages I made in our last PR together, I think they are patterns we need on this one too!
so excited!
| int expectedParamsVersion); | ||
|
|
||
| @GET | ||
| @Path("/requestHandler") |
There was a problem hiding this comment.
doiuble check sheet, but I think this is now plural? "requestHandlers"????
There was a problem hiding this comment.
done.
V2 is @path("/requestHandlers") (plural), matching the sheet.
V1 stays singular requestHandler.
|
|
||
| @GET | ||
| @Path("/requestHandler") | ||
| @StoreApiParameters |
There was a problem hiding this comment.
what does @StoreApiParameters mean?
| summary = "Fetch request handlers (including implicit handler)", | ||
| tags = {"config"}) | ||
| ConfigInfoResponse getRequestHandler( | ||
| @QueryParam("componentName") |
There was a problem hiding this comment.
i think this is now a path param as well....
There was a problem hiding this comment.
Stated the reasoning here in my self-review: #5168 (comment)
There was a problem hiding this comment.
We can’t put the handler name in the path the way we do for searchComponents/{name} / queryResponseWriters/{name}.
Request-handler names are paths themselves (/select, /update/json/docs, /admin/ping, …). A @PathParam would be a single segment, so names with / break routing (and encoding gets ugly).
So for request handlers we keep filtering via query param:
GET .../config/requestHandlers?componentName=/update/json/docs
|
Thanks @epugh for the quick review! I have addressed the comments. PackageToolTest is failing right now, mostly looks like it is due to "requestHandler" -> "requestHandlers" in the v2 endpoint, but I am not yet able to debug & fix it. Trying more. |
…/jaykay12/solr into pr/5168 # Conflicts: # changelog/unreleased/SOLR-16396-config-query.yml # solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigApi.java # solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java # solr/core/src/java/org/apache/solr/handler/admin/api/GetConfig.java # solr/core/src/test/org/apache/solr/handler/admin/api/GetConfigTest.java
Solr forwards a v2 request to the node that actually hosts the target
collection using the short "/c/{collection}/..." path alias (see
HttpSolrCall#sendRemoteProxy), but Constants.INDEX_PATH_PREFIX's JAX-RS
@path regex only recognized "cores" or "collections", not "c". Any
JAX-RS resource built on that prefix therefore 404'd with "Cannot find
API for the path" whenever a request needed to be proxied cross-node.
Reproduced with a new 2-node test in GetConfigSolrCloudTest that
deliberately sends a GET /config/overlay request to the node that does
NOT host the collection's only replica, then fixed by accepting "c" as
an alias for "collections" in the path regex.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
CI builds with -Werror, and errorprone flags comparing JettySolrRunner instances with != instead of .equals(). Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
|
@epugh this looks fine to me now, this PR has changes from the fix PR as well now & the test passes locally. |
|
done |
…"boolean" values exposed
1. PackageManager.verify() builds its verify-command URL by taking the path straight from repository.json's template (e.g. /api/collections/${collection}/config/requestHandlers?componentName=${RH-HANDLER-PATH}&meta=true) and passing that whole string, including the embedded ?..., as the path argument to GenericSolrRequest, with an empty separate params object.
2. HttpJettySolrClient.makeRequest() builds GET requests as url + wparams.toQueryString() — and toQueryString() always starts with a literal ?. Since url already contains its own ?..., this produces a malformed URL with two ? characters: ...requestHandlers?componentName=/mypath2&meta=true?wt=json.
3. Per the URL spec, everything after the first ? is one query string. Splitting on & gives meta = "true?wt=json" — a garbled value, not "true".
4. V1's parameter binding (SolrParams.getBool → StrUtils.parseBool) uses startsWith("true"), so it's lenient enough to still resolve "true?wt=json" to true — masking the bug for years on the old /config/requestHandler endpoint.
5. V2's JAX-RS @QueryParam boolean meta binding does an exact match, so "true?wt=json" fails to parse as true and silently resolves to false — the test fixture's upgrade to the new plural /config/requestHandlers endpoint is what exposed this for the first time.
# Conflicts: # solr/core/src/test/org/apache/solr/handler/admin/api/GetConfigSolrCloudTest.java
…p plumbing
- PackageUtils.getJsonStringFromUrl now reuses the existing, more correct
SolrRequestParsers.parseQueryString (which %-decodes values) instead of a
hand-rolled split("&")/split("=") parser that didn't.
- GetConfig.requestHandlersWithImplicits had two incompatible calling
conventions: buildConfigMap relied on it mutating a full config-map
wrapper in place, while getRequestHandler only used its return value and
threw the wrapper away. Both callers now extract the requestHandler
submap themselves before calling it, so the shared method has a single,
consistent contract: take a handler map, merge+expand it in place, return
it.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5














https://issues.apache.org/jira/browse/SOLR-16396
Description
Migrate GET /config/requestHandler to JAX-RS
Already landed / related:
Solution
AI Disclaimer: Used Cursor for Approach formulation, code is fully vetted & reviewed by me.
Tests
GetConfigTest covers the scenarios.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.