From d9461e21ff4a73198ab90c3f3bb6369fe9062067 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Fri, 27 Dec 2024 23:58:19 -0500 Subject: [PATCH] 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. --- .../org/apache/solr/cloud/ZkController.java | 20 ++++++++++--------- .../solr/cloud/OverseerTaskQueueTest.java | 4 +++- .../org/apache/solr/common/MapWriter.java | 7 +++++-- 3 files changed, 19 insertions(+), 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 index 7dd30a14d24..e81a4a20c2b 100644 --- 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 index 27039870857..0b748a0a25f 100644 --- 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 index 0bbe2dc50a0..c103461d694 100644 --- 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 toMap(Map 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 -> {