Skip to content

SOLR-16396: Migrate remaining GET /config/{component} sections to JAX-RS - #5174

Draft
jaykay12 wants to merge 11 commits into
apache:mainfrom
jaykay12:SOLR-16396-config-remaining-get-sections
Draft

jaykay12 wants to merge 11 commits into
apache:mainfrom
jaykay12:SOLR-16396-config-remaining-get-sections

Conversation

@jaykay12

@jaykay12 jaykay12 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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

Description

Migrate remaining GET /config/{component} sections to JAX-RS

Solution

This will be a follow-up PR after: #5168

Will convert this from Draft -> ready-for-review once that PR is merged.

With this PR merged, all GET config sections will get be fully JAX-RS compatible.

Tests

TBA

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 and others added 2 commits October 17, 2026 19:14
circuitBreaker, initParams, listener, and updateRequestProcessorChain
are declared without REQUIRE_NAME in SolrConfig's plugin registry, so
SolrConfig#writeMap serializes them as a List rather than a name-keyed
Map. GetConfig#filterByComponentName's unconditional (Map) cast then
threw ClassCastException for any by-name lookup against these sections
(reachable both via v1's componentName query param, pre-existing, and
the new v2 GET /config/{component}/{componentName} path added here),
surfacing as an uncaught 500 instead of the documented 404 convention.

- filterByComponentName now checks the section's shape and throws a
  clean NOT_FOUND when by-name filtering doesn't apply, fixing the v1
  path that predates this PR.
- Also drop the 4 non-named sections from the v2 by-name @path regex,
  since offering a route that can never find a match by name isn't a
  useful API surface.
- cache has the same REQUIRE_NAME+MULTI_OK shape as the other sections
  already pluralized here (listener->listeners, queryParser->
  queryParsers, etc.) but was left singular; renamed to caches for
  consistency.

Verified via a throwaway reproduction test: GET .../config/circuitBreaker/x
500'd with ClassCastException before this change, now returns a clean 404.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
@epugh

epugh commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Found a bug while reviewing this against `dev-docs/apis-v2-conventions.adoc`: `circuitBreaker`, `initParams`, `listener`, and `updateRequestProcessorChain` are declared without `REQUIRE_NAME` in `SolrConfig`'s plugin registry, so `SolrConfig#writeMap` serializes them as a `List` rather than a name-keyed `Map`. `GetConfig#filterByComponentName`'s unconditional `(Map)` cast then throws a `ClassCastException` for any by-name lookup against these sections — reachable both via v1's `componentName` query param (pre-existing bug, predates this PR) and the new v2 `GET /config/{component}/{componentName}` path this PR adds. That surfaces as an uncaught 500 instead of the documented NOT_FOUND/404 convention.

Reproduced with a throwaway test: `GET .../config/circuitBreaker/x` 500'd with the ClassCastException before this fix, returns a clean 404 after.

Pushed a fix:

  • `filterByComponentName` now checks the section's shape and throws a clean `NOT_FOUND` when by-name filtering doesn't apply (fixes the v1 path too).
  • Dropped those 4 non-named sections from the v2 by-name `@Path` regex, since a route that can never match by name isn't useful API surface.
  • Also noticed `cache` has the same `REQUIRE_NAME + MULTI_OK` shape as the other sections this PR already pluralizes (`listener`→`listeners`, `queryParser`→`queryParsers`, etc.) but was left singular — renamed to `caches` for consistency.

All existing tests pass (`GetConfigTest`, `GetConfigSolrCloudTest`, `V2ConfigAPIMappingTest`, `TestSolrConfigHandler`, `TestSolrConfigHandlerCloud`).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants