Remove MapWriter.append (#2919)

Undocumented and called in only 2 places. Since MapWriter is implemented by a great many things, we should be conservative adding methods there. append() is kind of clever but I think it's outside the scope of what MapWriter should be.
apache · Dec 28, 2024 · d9461e2 · d9461e2
1 parent 02cdf9e
commit d9461e2
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 12 deletions.
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -1702,16 +1702,18 @@ public void publish(
         }
         if (core != null && core.getDirectoryFactory().isSharedStorage()) {
           if (core.getDirectoryFactory().isSharedStorage()) {
+            // append additional entries to 'm'
+            MapWriter original = m;
             m =
-                m.append(
-                    props -> {
-                      props.put(ZkStateReader.SHARED_STORAGE_PROP, "true");
-                      props.put("dataDir", core.getDataDir());
-                      UpdateLog ulog = core.getUpdateHandler().getUpdateLog();
-                      if (ulog != null) {
-                        props.put("ulogDir", ulog.getUlogDir());
-                      }
-                    });
+                props -> {
+                  original.writeMap(props);
+                  props.put(ZkStateReader.SHARED_STORAGE_PROP, "true");
+                  props.put("dataDir", core.getDataDir());
+                  UpdateLog ulog = core.getUpdateHandler().getUpdateLog();
+                  if (ulog != null) {
+                    props.put("ulogDir", ulog.getUlogDir());
+                  }
+                };
           }
         }
       } catch (SolrCoreInitializationException ex) {

diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerTaskQueueTest.java
@@ -21,6 +21,7 @@
 import java.util.Map;
 import org.apache.solr.cloud.api.collections.CollectionHandlingUtils;
 import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.params.CommonAdminParams;
@@ -73,8 +74,9 @@ public void testContainsTaskWithRequestId() throws Exception {
     String watchID = tq.createResponseNode();
     String requestId2 = "baz";
 
+    // append async then submit
     tq.createRequestNode(
-        Utils.toJSON(props.append(ew -> ew.put(CommonAdminParams.ASYNC, requestId2))), watchID);
+        Utils.toJSON(new ZkNodeProps(props).plus(CommonAdminParams.ASYNC, requestId2)), watchID);
 
     // Set a SolrResponse as the response node by removing the QueueEvent, as done in
     // OverseerTaskProcessor

diff --git a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
@@ -33,6 +33,9 @@
  */
 public interface MapWriter extends MapSerializable, NavigableObject, JSONWriter.Writable {
 
+  /** Writes this object's entries out to {@code ew}. */
+  void writeMap(EntryWriter ew) throws IOException;
+
   default String jsonStr() {
     return Utils.toJSONString(this);
   }
@@ -42,6 +45,7 @@ default Map<String, Object> toMap(Map<String, Object> map) {
     return Utils.convertToMap(this, map);
   }
 
+  /** For implementing Noggit {@link org.noggit.JSONWriter.Writable}. */
   @Override
   default void write(JSONWriter writer) {
     writer.startObject();
@@ -70,8 +74,7 @@ public MapWriter.EntryWriter put(CharSequence k, Object v) {
     writer.endObject();
   }
 
-  void writeMap(EntryWriter ew) throws IOException;
-
+  @Deprecated
   default MapWriter append(MapWriter another) {
     MapWriter m = this;
     return ew -> {