Skip to content

SOLR-13706: Config API output for the "highlight" searchComponent is no longer dropped - #5015

Open
nick-boss-tech wants to merge 10 commits into
apache:mainfrom
nick-boss-tech:solr-13706-submit
Open

nick-boss-tech wants to merge 10 commits into
apache:mainfrom
nick-boss-tech:solr-13706-submit

Conversation

@nick-boss-tech

@nick-boss-tech nick-boss-tech commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

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

What happens today

The highlight search component is missing from the Config API's /config output. PluginInfo's serialization keys each child of a plugin by the child's name, and a child without a name produces a null key that fails serialization with a NullPointerException. SolrConfig works around that for this one component: it skips the highlight component when writing the configuration, under a TODO comment pointing at this ticket, so instead of an error the component is silently dropped from the output.

What this change does

PluginInfo's serialization now groups children by their type instead of their name, so every child is written under its type, with the child's name, when it has one, kept inside its own entry; an unnamed child no longer fails serialization. The SolrConfig workaround that skipped the highlight component is removed, so the component appears in the /config output again.

Proof

Verified on the fork's GitHub Actions test runner at head b062f3b on 2026-10-04, with Error Prone enabled; the base-code comparisons ran the same test classes against a tree with base production code and this PR's test files.

  • PluginInfoTest: 11 of 11 pass with this change. On base code the new cases fail (8 failing test executions under randomization): testChildrenGroupedByTypeInSerializedOutput fails on its containsKey("formatter") assertion, because base keys the children by name, and testUnnamedChildIsWrittenUnderItsType fails with the NullPointerException from the null key.
  • TestConfigHighlightOutput: 1 of 1 passes with this change. On base code it fails with the same NullPointerException (4 failing test executions).

A choice to check

The fix could have been local to the highlighting output, which is where the failure was reported. This PR fixes the shared PluginInfo serialization instead, because any plugin with unnamed children hits the same failure; the cost is that the serialized shape changes for every plugin that has children at all, named or not, not only the highlighter. Was fixing the shared serialization the right scope?

Limits

Children are now keyed by type in the serialized output for every plugin that has children, so anything that reads plugin info output and looks children up by name will not find them; each child's name is still present inside its own entry. This is recorded as a changed changelog entry, with an upgrade note in the Solr 11 upgrade notes.

Changelog: changelog/unreleased/SOLR-13706.yml

AI assistance

AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.

@github-actions github-actions Bot added the tests label Oct 2, 2026
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 4, 2026
for (PluginInfo child : children) {
Object old = childrenGrouped.get(child.name);
// Group by type (not name): children are looked up by type via getChildren(type),
// and unrelated plugin types may share a name (e.g. formatter/encoder both named "html").

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it less verbose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

Thanks, trimmed it down: the grouping is now a single computeIfAbsent accumulation into a Map<String, List>, and the comment is cut to the two lines explaining why the key is the type rather than the name. Behavior and output shape are unchanged (a type with one child still writes the child itself, several write a list), and PluginInfoTest (11/11) and TestConfigHighlightOutput (1/1) pass as before. Pushed in 51075ab.

solrTestRule
.newCollection()
.withConfigSet(SolrTestCaseJ4.TEST_COLL1_CONF())
.withSchemaFile("schema.xml")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? Augh

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

Fair point. The withSchemaFile line just restated the default (schema.xml is already the default schema name), so it is gone, and the test no longer boots a server or a core at all: it builds the SolrConfig directly from the collection1 solrconfig, the pattern TestConfig uses, and asserts on the same serialized output, since it never indexed or queried anything. Pushed in c96d7ee.


== API Changes

=== Child Plugins in Config API Output Are Keyed by Type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So…. I wonder if instead of fixing the v1 api, should this be done in v2?? That’s our future. We will be removing v1 APIs starting in Solr 11(???) and so is this fix in the v2 api???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

Good question; I checked. The v2 config API is not a separate implementation: GetConfigAPI (solr/core/src/java/org/apache/solr/handler/admin/api/GetConfigAPI.java:46) delegates every call to SolrConfigHandler, the same handler behind the v1 /config path, and SolrConfigHandler.getApis (solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:1063) is what registers those v2 endpoints. Both surfaces build the response from SolrConfig (SolrConfigHandler.java:341) and serialize each plugin through PluginInfo.writeMap (solr/core/src/java/org/apache/solr/core/PluginInfo.java:197), which is the method this PR changes. So on the base code the highlight component was missing from the v2 output too, for the same reason, and this fix covers v2 as it stands. There is no v2-only config output path in the tree that would need its own version of the change.

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

Labels

documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants