SOLR-13706: Config API output for the "highlight" searchComponent is no longer dropped - #5015
nick-boss-tech wants to merge 10 commits into
Conversation
54c8a1b to
25fad2f
Compare
…g API child keying
| 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"). |
There was a problem hiding this comment.
🤖 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") |
There was a problem hiding this comment.
🤖 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 |
There was a problem hiding this comment.
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???
There was a problem hiding this comment.
🤖 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.
🤖 AI text below 🤖 (posted on behalf of Nick Shanin)
https://issues.apache.org/jira/browse/SOLR-13706
What happens today
The
highlightsearch component is missing from the Config API's/configoutput.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 aNullPointerException.SolrConfigworks around that for this one component: it skips thehighlightcomponent 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. TheSolrConfigworkaround that skipped thehighlightcomponent is removed, so the component appears in the/configoutput 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):testChildrenGroupedByTypeInSerializedOutputfails on itscontainsKey("formatter")assertion, because base keys the children by name, andtestUnnamedChildIsWrittenUnderItsTypefails with theNullPointerExceptionfrom the null key.TestConfigHighlightOutput: 1 of 1 passes with this change. On base code it fails with the sameNullPointerException(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
PluginInfoserialization 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
changedchangelog entry, with an upgrade note in the Solr 11 upgrade notes.Changelog:
changelog/unreleased/SOLR-13706.ymlAI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.