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

changelog/unreleased/SOLR-13706.yml

@@ -0,0 +1,7 @@
+title: The Config API now returns the highlight searchComponent, with child plugins keyed by their type instead of their name
+type: changed
+authors:
+  - name: Nick Shanin
+links:
+  - name: SOLR-13706
+    url: https://issues.apache.org/jira/browse/SOLR-13706

solr/core/src/java/org/apache/solr/core/PluginInfo.java

@@ -206,22 +206,15 @@ public void writeMap(EntryWriter ew) throws IOException {
       return;
     }
 
-    Map<String, Object> childrenGrouped = new LinkedHashMap<>();
+    // Group by type, not name: lookups are by type (getChildren), and keying by name
+    // would leave unnamed children under a null key, which fails on write.
+    Map<String, List<PluginInfo>> childrenByType = new LinkedHashMap<>();
     for (PluginInfo child : children) {
-      Object old = childrenGrouped.get(child.name);
-      if (old == null) {
-        childrenGrouped.put(child.name, child);
-      } else if (old instanceof List list) {
-        list.add(child);
-      } else {
-        List<Object> l = new ArrayList<>();
-        l.add(old);
-        l.add(child);
-        childrenGrouped.put(child.name, l);
-      }
+      childrenByType.computeIfAbsent(child.type, t -> new ArrayList<>()).add(child);
     }
-    for (Map.Entry<String, Object> entry : childrenGrouped.entrySet()) {
-      ew.put(entry.getKey(), entry.getValue());
+    for (Map.Entry<String, List<PluginInfo>> entry : childrenByType.entrySet()) {
+      List<PluginInfo> group = entry.getValue();
+      ew.put(entry.getKey(), group.size() == 1 ? group.get(0) : group);
     }
   }
 

solr/core/src/java/org/apache/solr/core/SolrConfig.java

@@ -983,8 +983,6 @@ public void writeMap(EntryWriter ew) throws IOException {
       if (plugin.options.contains(PluginOpts.REQUIRE_NAME)) {
         LinkedHashMap<String, Object> items = new LinkedHashMap<>();
         for (PluginInfo info : infos) {
-          // TODO remove after fixing https://issues.apache.org/jira/browse/SOLR-13706
-          if (info.type.equals("searchComponent") && info.name.equals("highlight")) continue;
           items.put(info.name, info);
         }
         for (Map.Entry<String, Map<String, Object>> e :

solr/core/src/test/org/apache/solr/core/PluginInfoTest.java

@@ -16,7 +16,10 @@
  */
 package org.apache.solr.core;
 
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.util.DOMUtilTestBase;
 import org.apache.solr.util.ErrorLogMuter;
 import org.junit.Test;
@@ -158,6 +161,72 @@ public void testChildren() throws Exception {
     }
   }
 
+  @Test
+  public void testChildrenGroupedByTypeInSerializedOutput() throws Exception {
+    // Two unrelated child plugin types sharing a name must not clobber each other in the
+    // serialized output; children are grouped by type (see SOLR-13706).
+    PluginInfo formatter =
+        new PluginInfo(
+            "formatter", Map.of("name", "html", "class", "com.example.HtmlFormatter"), null, null);
+    PluginInfo secondFormatter =
+        new PluginInfo(
+            "formatter", Map.of("name", "text", "class", "com.example.TextFormatter"), null, null);
+    PluginInfo encoder =
+        new PluginInfo(
+            "encoder", Map.of("name", "html", "class", "com.example.HtmlEncoder"), null, null);
+    PluginInfo parent =
+        new PluginInfo(
+            "searchComponent",
+            Map.of("name", "highlight", "class", "com.example.HighlightComponent"),
+            null,
+            List.of(formatter, encoder, secondFormatter));
+
+    Map<String, Object> out = new LinkedHashMap<>();
+    parent.writeMap(
+        new MapWriter.EntryWriter() {
+          @Override
+          public MapWriter.EntryWriter put(CharSequence k, Object v) {
+            out.put(k.toString(), v);
+            return this;
+          }
+        });
+
+    assertTrue(out.containsKey("formatter"));
+    assertTrue(out.containsKey("encoder"));
+    assertFalse("children must be grouped by type, not by shared name", out.containsKey("html"));
+    // repeated children of the same type become a list
+    assertTrue(out.get("formatter") instanceof List);
+    assertEquals(2, ((List<?>) out.get("formatter")).size());
+    // each child's own name is preserved in its attributes
+    PluginInfo gotEncoder = (PluginInfo) out.get("encoder");
+    assertEquals("html", gotEncoder.name);
+    assertEquals("com.example.HtmlEncoder", gotEncoder.className);
+  }
+
+  @Test
+  public void testUnnamedChildIsWrittenUnderItsType() throws Exception {
+    PluginInfo unnamed =
+        new PluginInfo("highlighting", Map.of("class", "com.example.Highlighting"), null, null);
+    PluginInfo parent =
+        new PluginInfo(
+            "searchComponent",
+            Map.of("name", "highlight", "class", "com.example.HighlightComponent"),
+            null,
+            List.of(unnamed));
+
+    Map<String, Object> out = new LinkedHashMap<>();
+    parent.writeMap(
+        new MapWriter.EntryWriter() {
+          @Override
+          public MapWriter.EntryWriter put(CharSequence k, Object v) {
+            out.put(k.toString(), v);
+            return this;
+          }
+        });
+
+    assertSame(unnamed, out.get("highlighting"));
+  }
+
   @Test
   public void testInitArgsCount() throws Exception {
     Node node = getNode(configWithNoChildren, "plugin");

solr/core/src/test/org/apache/solr/core/TestConfigHighlightOutput.java

@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.util.List;
+import java.util.Map;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.Utils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Tests that the serialized config, as returned by the Config API, includes the highlighter. */
+public class TestConfigHighlightOutput extends SolrTestCase {
+
+  @BeforeClass
+  public static void beforeClass() {
+    // Sets the randomized solr.tests.* properties the collection1 solrconfig.xml requires.
+    SolrTestCaseJ4.newRandomConfig();
+  }
+
+  @Test
+  public void testHighlightComponentIsWrittenWithItsChildren() throws Exception {
+    SolrConfig solrConfig =
+        new SolrConfig(SolrTestCaseJ4.TEST_PATH().resolve("collection1"), "solrconfig.xml");
+    Object config = Utils.fromJSONString(solrConfig.jsonStr());
+    Object highlighting =
+        Utils.getObjectByPath(
+            config, false, List.of("searchComponent", "highlight", "highlighting"));
+    assertTrue("highlighting must be written as an object", highlighting instanceof Map);
+
+    // the child types are keys, even though several children share the name "simple"
+    for (String type :
+        List.of(
+            "fragmenter", "formatter", "fragListBuilder", "fragmentsBuilder", "boundaryScanner")) {
+      assertNotNull("missing child type " + type, ((Map<?, ?>) highlighting).get(type));
+    }
+    assertFalse(((Map<?, ?>) highlighting).containsKey("simple"));
+
+    assertEquals(2, ((List<?>) ((Map<?, ?>) highlighting).get("fragmenter")).size());
+    assertEquals(2, ((List<?>) ((Map<?, ?>) highlighting).get("fragmentsBuilder")).size());
+    assertEquals(2, ((List<?>) ((Map<?, ?>) highlighting).get("boundaryScanner")).size());
+    assertEquals("html", ((Map<?, ?>) ((Map<?, ?>) highlighting).get("formatter")).get("name"));
+    assertEquals(
+        "simple", ((Map<?, ?>) ((Map<?, ?>) highlighting).get("fragListBuilder")).get("name"));
+  }
+}

solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc

@@ -41,3 +41,11 @@ bin/solr start -Dsolr.node.roles=data:on,overseer:preferred
 
 A node started this way asks the Overseer to re-run its node prioritization, so a preferred node takes over without waiting for the current Overseer to restart.
 Note that node roles are fixed for the lifetime of a node: unlike `ADDROLE`, they cannot be changed on a running node.
+
+== API Changes
+
+=== Child Plugins in Config API Output Are Keyed by Type
+
+In the Config API output for a plugin that has child plugins, each child is now keyed by the child's type instead of its name; the name, when the child has one, still appears inside the child's own entry.
+Clients that look up a plugin's children by name need to look them up by type instead.
+The `highlight` search component, which was previously omitted from this output, is now included.