SOLR-16396 | Migrate GET /config/overlay and GET /config/znodeVersion to JAX-RS.

[jaykay12]

https://issues.apache.org/jira/browse/SOLR-16396

Description

Migrate GET /config/overlay and GET /config/znodeVersion to JAX-RS.

Solution

• Migrate GET /config/overlay and GET /config/znodeVersion to JAX-RS on ConfigApi.Get / GetConfig
• Keep v1 behavior via squash into SolrConfigHandler, including znodeVersion stale-config refresh
• Add SolrJ/HTTP coverage in GetConfigTest and GetConfigSolrCloudTest

Yes, Used Cursor for approach formulation.

Future Plan

• Full GET /config: SOLR-16396 | Migration of Get Config V2 API to JAX-RS construct #4951
• This PR: overlay + znodeVersion
• Related: SOLR-16396 | Migrate GET config sections updateHandler, directoryFactory, indexConfig, and codecFactory to JAX-RS. #5023

Later follow-ups (not next PR)

• Remaining GET /config/{component} (e.g. requestHandler, searchComponent, …) with componentName, expandParams, meta
• GET /config/params and GET /config/params/{paramset}

Tests

• GetConfigTest / GetConfigSolrCloudTest: SolrJ GetOverlay and GetZnodeVersion
  (overlay map has znodeVersion; versions >= -1).
• Existing TestSolrConfigHandler overlay/v1 paths still exercise squash + refresh.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide
• I have added a changelog entry for my change

[jaykay12]

self-review ✅

[epugh]

Okay, I did some testing, and with some ai help apparently event hoguh we have v2 apis, the code path is going via the v1 path! See this curl where we ask for xml but get back json? and we expect a 406 not a 200:

curl -sv "http://localhost:8983/api/collections/techproducts/config/overlay" -H "Accept: application/xml"
* Host localhost:8983 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:8983...
* connect to ::1 port 8983 from ::1 port 55336 failed: Connection refused
*   Trying 127.0.0.1:8983...
* Connected to localhost (127.0.0.1) port 8983
> GET /api/collections/techproducts/config/overlay HTTP/1.1
> Host: localhost:8983
> User-Agent: curl/8.7.1
> Accept: application/xml
> 
* Request completely sent off
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< Content-Type: application/json;charset=utf-8
< Content-Length: 99
< 
{
  "responseHeader":{
    "status":0,
    "QTime":0
  },
  "overlay":{
    "znodeVersion":-1
  }

[epugh]

The new JAX-RS registrations for /overlay and /znodeVersion appear to never actually run through Jersey. GetConfigAPI.getComponentConfig() (GetConfigAPI.java:58-64) still registers the generic @EndPoint(path={"/config/{component}"}) wildcard in ApiBag, and V2HttpCall.init() resolves ApiBag matches before ever consulting Jersey (executeCoreRequest() only invokes Jersey if (api == null)). Since "overlay"/"znodeVersion" both match that wildcard, api is never null for these requests, so the legacy path (SolrConfigHandler.handleGET()) always wins and Jersey is never invoked — confirmed this empirically too (a request with Accept: application/xml still returns JSON with no 406, which is what you'd expect from the legacy response writer rather than real JAX-RS content negotiation).

Functionally this is fine today since handleGET() now manually constructs GetConfig and reuses the same logic, so responses are correct. But it means the ConfigApi.Get.getOverlay()/getZnodeVersion() JAX-RS methods — and their @PermissionName annotations — are currently dead code at the dispatch layer; they only serve SolrJ client codegen and OpenAPI docs. Contrast with #4951, which removed the old @EndPoint(path={"/config"}) registration entirely when it migrated the base path to JAX-RS, so Jersey would actually own that route.

Given the PR description already lists the remaining /config/{component} values as a planned follow-up, is the intent here that the wildcard stays until every sub-path is migrated, with the last one finally removing it (mirroring #4951)? If so this is probably fine as an intermediate state — just flagging so it's a conscious choice rather than an oversight, and so a future PR doesn't have to rediscover this.

[epugh]

I think we need to make {"/config/{component}"}) be more specific, or otherwise make {"/config/overlay"}) and {"/config/znodeVersion"}) come first in some way?

[jaykay12]

Thanks for tracing this, Eric, and for the Accept: application/xml curl. You are right that /config/{component} still matches overlay and znodeVersion, so those requests stay on the AnnotatedApi path and Jersey is not invoked. handleGET calls GetConfig, so the response still comes from the shared implementation. The JAX-RS methods are what SolrJ and the OpenAPI spec see.

That is intentional for this PR. The remaining /config/{component} values are still on that wildcard, and one follow-up PRs for them are already in flight & few more are planned. The last of those will remove /config/{component}, the same way #4951 removed the old /config endpoint once that path had moved. I will carry this note onto those PRs as well.

On your third comment. I would rather leave the single wildcard in place and do that cutover once, in the last follow-up, when nothing is left on {component}.

[jaykay12]

Test Evidences GET overlay

http://localhost:8983/solr/techproducts/config/overlay
http://localhost:8983/api/collections/techproducts/config/overlay

Scenario	Prod	Feature
Start - V1 & V2
-	-	-
Ingesting 1 sample config update
-	-	-
Ingesting 1 more sample config update
-	-	-
V1 API
-	-	-
V2 API
-	-	-
V1 API with omitHeader
-	-	-
V2 API with omitHeader

[jaykay12]

Test Evidences GET znodeVersion

http://localhost:8983/solr/techproducts/config/znodeVersion
http://localhost:8983/api/collections/techproducts/config/znodeVersion
http://localhost:8983/solr/techproducts/config/znodeVersion?params=0&overlay=2
http://localhost:8983/api/collections/techproducts/config/znodeVersion?params=0&overlay=2

Scenario	Prod	Feature
First Update
-	-	-
Second Update
-	-	-
V1 API with queryParams
-	-	-
V2 API with queryParams

[jaykay12]

self-review ✅
local testing ✅

@epugh can please trigger workflows on this?

[epugh]

can you give some context on why this rename is needed? Is this a tidy, or was the name clashing? for soem reason i get nervoius every time we touch the mustache templating, maybe cause it seems like magic!

[jaykay12]

Not a tidy - it was a name clash.
I hit it via a failing test -
https://github.com/jaykay12/solr/blob/cf8b0220fd4c94759e20a857a2b6ba29d64206e2/solr/core/src/test/org/apache/solr/handler/admin/api/GetConfigTest.java#L72

Actually getZnodeVersion has @QueryParam("params"), so codegen produces a field/local named params. The template then emitted params.add(...) on a local also called params, which failed to compile. Renaming the local to solrParams avoids colliding with any query param named params. Behavior of the generated clients is unchanged.

this change in mustache templating was suggested to me by Cursor, when debugging the failed test, since it was local variable renaming, & test passed post this renaming so felt safe to me!

[epugh]

Thanks! thats interesting that we have to think about this...

[epugh]

i wonder if there are otehr non migrated V2 apis that are going to trigger this type of thing?

[epugh]

Thanks for tracing this, Eric, and for the Accept: application/xml curl. You are right that /config/{component} still matches overlay and znodeVersion, so those requests stay on the AnnotatedApi path and Jersey is not invoked. handleGET calls GetConfig, so the response still comes from the shared implementation. The JAX-RS methods are what SolrJ and the OpenAPI spec see.

That is intentional for this PR. The remaining /config/{component} values are still on that wildcard, and one follow-up PRs for them are already in flight & few more are planned. The last of those will remove /config/{component}, the same way #4951 removed the old /config endpoint once that path had moved. I will carry this note onto those PRs as well.

On your third comment. I would rather leave the single wildcard in place and do that cutover once, in the last follow-up, when nothing is left on {component}.

Thanks for explaining this to me.... With the final cutover being at the end, it does mean that each of these PR's isn't a complete feature, in that from an external perspective, we never actually exercise these new apis... Which I suppose is fine as long as we make it to that final cut over!!!

Does teh fact that we never exercise the new apis mean the screen shots aren't actually meaningful? Because the v2 calls are just routed to the old v1 endpoint and returned right? Whcih is fine if our goal is to have java code validation, and then do a cut over at the end??? Just want to make sure I am underrstanding things....

[epugh]

/config/{component}

A quck thoguh, what if instead of a wildcard, we just had explicit /config/componentA, /config/componentB, /componentC that explicitly listed out each of the end points, then we could migrate completely and just delete them one by one?

[jaykay12]

Thanks for explaining this to me.... With the final cutover being at the end, it does mean that each of these PR's isn't a complete feature, in that from an external perspective, we never actually exercise these new apis... Which I suppose is fine as long as we make it to that final cut over!!!

Does teh fact that we never exercise the new apis mean the screen shots aren't actually meaningful? Because the v2 calls are just routed to the old v1 endpoint and returned right? Whcih is fine if our goal is to have java code validation, and then do a cut over at the end??? Just want to make sure I am underrstanding things....

Yeah, you’ve got the picture right.

One small nuance on “routed to the old v1 endpoint”: the /api/.../config/overlay and /znodeVersion calls still come in on the V2 path (V2HttpCall). What happens is ApiBag matches the existing AnnotatedApi wildcard /config/{component} in GetConfigAPI before Jersey ever gets a look, so they go AnnotatedApi → SolrConfigHandler.handleGET → GetConfig (via squash). They never hit the Jersey-registered getOverlay() / getZnodeVersion() methods - which is why your Accept: application/xml curl still comes back as JSON/200 instead of a 406.

So the screenshots are meaningful as a regression check that response bodies stay equivalent after moving the logic into GetConfig, not as proof that Jersey owns the route.

also, screenshots are my own personal ways of actually understanding what actually changes to the end user with my each ongoing change, & those testing also aids as an actual testing evidence for me. I will do similar ones, once we do final cutover.

[jaykay12]

A quck thoguh, what if instead of a wildcard, we just had explicit /config/componentA, /config/componentB, /componentC that explicitly listed out each of the end points, then we could migrate completely and just delete them one by one?

That’s a fair idea - if we listed each /config/... path explicitly under AnnotatedApi, we could delete them one by one as each migrates and Jersey would actually own the route in that PR.

My preference would be to skip that detour if we can. Expanding the wildcard into temporary endpoints just so we can delete them again feels like throwaway work, and V2 is still experimental. The plan is to land the remaining /config/{component} pieces and then cut over once at the end (same as #4951), so I was hoping to keep the single wildcard until then.

That said, I’m not stuck on it - if you've strong inclination on us doing that, I’m happy to do that.

[epugh]

If you don't mind, lets do it, because then I can feel like we ahve a "complete feature" in each PR. Especially if we get stuck, and maybe some lands on 10.2 and some on 10.3.. Though I do suspect it will all be on 10.2!!!!

Also, then I get the visual satisfaction of seeing us eliminate each one of htose lines from this PR forward ;-)

Also, this lets us update Google spreadsheet after each merge to flip JAX RS to "true" ;-)

[jaykay12]

If you don't mind, lets do it, because then I can feel like we ahve a "complete feature" in each PR. Especially if we get stuck, and maybe some lands on 10.2 and some on 10.3.. Though I do suspect it will all be on 10.2!!!!

Also, then I get the visual satisfaction of seeing us eliminate each one of htose lines from this PR forward ;-)

Also, this lets us update Google spreadsheet after each merge to flip JAX RS to "true" ;-)

Ha, fair enough - the “flip it to true in the spreadsheet” argument is pretty convincing 😄

Quick process question before I dig in: for the explicit AnnotatedApi registrations (replacing /config/{component} with a concrete path list so we can peel them off one by one), would you rather I:

1. Do that in this PR, and also drop overlay / znodeVersion from that list so Jersey owns those two here, or
2. Land a small precursor PR that only expands the wildcard into explicit paths (no behavior change), merge that to main, and then rebase this PR on top so overlay / znodeVersion are the first real deletions?

I’m fine either way, I just don’t want to bake the wrong sequencing in.

For now, i am working via route 2, if you suggest otherwise i will move those changes in this PR.

[epugh]

i think this PR is fine.. We;ll merge this one very very soon!

[jaykay12]

verified local testing again ✅
screenshots updates ✅

[epugh]

just did the curl test, yay!