From 346644c8d5eff590f8909b6f2e36dae004f6dcd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Wed, 28 Oct 2026 13:34:09 +0000 Subject: [PATCH 1/7] SOLR-18603: Seal the NamedList class hierarchy NamedList is now sealed, permitting only SimpleOrderedMap (non-sealed). CloudSolrClient.RouteResponse extends SimpleOrderedMap instead of NamedList, since it cannot be permitted from another package without a module-info. NestableJsonFacetTest no longer uses anonymous NamedList subclasses. Adds a changelog entry and an 11.0 upgrade note. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../unreleased/SOLR-18603-seal-namedlist.yml | 13 +++ .../pages/major-changes-in-solr-11.adoc | 8 ++ .../client/solrj/impl/CloudSolrClient.java | 2 +- .../apache/solr/common/util/NamedList.java | 7 +- .../solr/common/util/SimpleOrderedMap.java | 2 +- .../solrj/response/NestableJsonFacetTest.java | 102 ++++-------------- 6 files changed, 50 insertions(+), 84 deletions(-) create mode 100644 changelog/unreleased/SOLR-18603-seal-namedlist.yml diff --git a/changelog/unreleased/SOLR-18603-seal-namedlist.yml b/changelog/unreleased/SOLR-18603-seal-namedlist.yml new file mode 100644 index 000000000000..9ab60a50ff22 --- /dev/null +++ b/changelog/unreleased/SOLR-18603-seal-namedlist.yml @@ -0,0 +1,13 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +title: > + NamedList is now a sealed class permitting only SimpleOrderedMap, which is non-sealed. Third-party + code can no longer extend NamedList directly; extend SimpleOrderedMap instead. + CloudSolrClient.RouteResponse now extends SimpleOrderedMap. +type: changed +authors: + - name: Ivan Šarić + nick: isaric +links: + - name: SOLR-18603 + url: https://issues.apache.org/jira/browse/SOLR-18603 diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc index ac2de0f81376..8d5de65a2a0f 100644 --- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc +++ b/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. + +== SolrJ + +=== NamedList is a sealed class + +`NamedList` is now a sealed class whose only permitted direct subclass is `SimpleOrderedMap`. +Plugins or client code that extended `NamedList` directly will no longer compile; extend `SimpleOrderedMap` instead, or hold a `NamedList` rather than being one. +`CloudSolrClient.RouteResponse` now extends `SimpleOrderedMap`; existing casts from `NamedList` keep working. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 07c8af0c5cd0..364b9381364b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -1682,7 +1682,7 @@ void evictStale() { } @SuppressWarnings({"rawtypes"}) - public static class RouteResponse extends NamedList { + public static class RouteResponse extends SimpleOrderedMap { private NamedList> routeResponses; private Map routes; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java index 13afd38e428a..697970420a24 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java @@ -53,10 +53,13 @@ * allows order to be easily preserved in various clients (i.e. not a straight map). If access by * key is more important for serialization, see {@link SimpleOrderedMap}, or simply use a regular * {@link Map} + * + *

This class is sealed: {@link SimpleOrderedMap} is its only direct subclass. */ @SuppressWarnings({"unchecked", "rawtypes"}) -public class NamedList - implements Cloneable, Serializable, Iterable>, MapWriter { +public sealed class NamedList + implements Cloneable, Serializable, Iterable>, MapWriter + permits SimpleOrderedMap { private static final long serialVersionUID = 1957981902839867821L; protected final List nvPairs; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java index d97990c35e9b..f7b5a6b658ac 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java @@ -43,7 +43,7 @@ * hold data to be serialized. It aims to minimize overhead and to be efficient at adding new * elements. */ -public class SimpleOrderedMap extends NamedList implements Map { +public non-sealed class SimpleOrderedMap extends NamedList implements Map { private static final SimpleOrderedMap EMPTY = new SimpleOrderedMap<>(List.of()); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java index e9230c358e0a..5d05e9d9997c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java @@ -29,46 +29,14 @@ public class NestableJsonFacetTest extends SolrTestCaseJ4 { public void testParsing() { NamedList list = new NamedList<>(); list.add("count", 12); - NamedList buckets = - new NamedList<>() { - { - add("val", "Nike"); - } - }; - NamedList vals = - new NamedList<>() { - { - add("numBuckets", 10); - add( - "allBuckets", - new NamedList<>() { - { - add("count", 12); - } - }); - add( - "before", - new NamedList<>() { - { - add("count", 1); - } - }); - add( - "after", - new NamedList<>() { - { - add("count", 2); - } - }); - add( - "between", - new NamedList<>() { - { - add("count", 9); - } - }); - } - }; + NamedList buckets = new NamedList<>(); + buckets.add("val", "Nike"); + NamedList vals = new NamedList<>(); + vals.add("numBuckets", 10); + vals.add("allBuckets", countList(12)); + vals.add("before", countList(1)); + vals.add("after", countList(2)); + vals.add("between", countList(9)); vals.add("buckets", List.of(buckets)); list.add("test", vals); NestableJsonFacet facet = new NestableJsonFacet(list); @@ -78,50 +46,24 @@ public void testParsing() { list.clear(); list.add("count", 12L); - buckets = - new NamedList<>() { - { - add("val", "Nike"); - } - }; - vals = - new NamedList<>() { - { - add("numBuckets", 10L); - add( - "allBuckets", - new NamedList<>() { - { - add("count", 12L); - } - }); - add( - "before", - new NamedList<>() { - { - add("count", 1L); - } - }); - add( - "after", - new NamedList<>() { - { - add("count", 2L); - } - }); - add( - "between", - new NamedList<>() { - { - add("count", 9L); - } - }); - } - }; + buckets = new NamedList<>(); + buckets.add("val", "Nike"); + vals = new NamedList<>(); + vals.add("numBuckets", 10L); + vals.add("allBuckets", countList(12L)); + vals.add("before", countList(1L)); + vals.add("after", countList(2L)); + vals.add("between", countList(9L)); vals.add("buckets", List.of(buckets)); list.add("test", vals); facet = new NestableJsonFacet(list); assertEquals(12L, facet.getCount()); assertEquals(2L, facet.getBucketBasedFacets("test").getAfter()); } + + private static NamedList countList(Object count) { + NamedList list = new NamedList<>(); + list.add("count", count); + return list; + } } From a737b30c67b4765d409cf8c14b4ba9e73f1575d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Thu, 29 Oct 2026 20:00:55 +0000 Subject: [PATCH 2/7] SOLR-18603: Make SimpleOrderedMap final and remove RouteResponse Per review: SimpleOrderedMap is final rather than non-sealed. CloudSolrClient.RouteResponse is removed; a routed update returns the condensed SimpleOrderedMap, whose responseHeader now carries each route's achieved replication factor under "rfByRoute", and getShardReplicationFactor reads that instead of casting. Tests verify routing via DocRouter and the new header entry. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../unreleased/SOLR-18603-seal-namedlist.yml | 7 +- .../pages/major-changes-in-solr-11.adoc | 9 +- .../client/solrj/impl/CloudSolrClient.java | 75 ++++++----------- .../solr/common/util/SimpleOrderedMap.java | 2 +- .../solrj/impl/CloudHttp2SolrClientTest.java | 84 +++++++------------ 5 files changed, 66 insertions(+), 111 deletions(-) diff --git a/changelog/unreleased/SOLR-18603-seal-namedlist.yml b/changelog/unreleased/SOLR-18603-seal-namedlist.yml index 9ab60a50ff22..7364b1708722 100644 --- a/changelog/unreleased/SOLR-18603-seal-namedlist.yml +++ b/changelog/unreleased/SOLR-18603-seal-namedlist.yml @@ -1,9 +1,10 @@ # See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc title: > - NamedList is now a sealed class permitting only SimpleOrderedMap, which is non-sealed. Third-party - code can no longer extend NamedList directly; extend SimpleOrderedMap instead. - CloudSolrClient.RouteResponse now extends SimpleOrderedMap. + NamedList is now a sealed class whose only subclass is SimpleOrderedMap, which is final. Third-party + code can no longer extend either. CloudSolrClient.RouteResponse is removed: a routed update now + returns a plain SimpleOrderedMap whose responseHeader carries each route's achieved replication + factor under "rfByRoute", which getShardReplicationFactor() reads. type: changed authors: - name: Ivan Šarić diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc index 8d5de65a2a0f..eb2e7b108abb 100644 --- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc +++ b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc @@ -46,6 +46,9 @@ Note that node roles are fixed for the lifetime of a node: unlike `ADDROLE`, the === NamedList is a sealed class -`NamedList` is now a sealed class whose only permitted direct subclass is `SimpleOrderedMap`. -Plugins or client code that extended `NamedList` directly will no longer compile; extend `SimpleOrderedMap` instead, or hold a `NamedList` rather than being one. -`CloudSolrClient.RouteResponse` now extends `SimpleOrderedMap`; existing casts from `NamedList` keep working. +`NamedList` is now a sealed class whose only permitted subclass is `SimpleOrderedMap`, which is `final`. +Plugins or client code that extended either class will no longer compile; hold a `NamedList` rather than being one. + +`CloudSolrClient.RouteResponse` has been removed. +A routed update request now returns a plain `SimpleOrderedMap`, whose `responseHeader` carries the achieved replication factor of each route under `rfByRoute` (`CloudSolrClient.REPFACT_BY_ROUTE`), keyed by route URL. +`CloudSolrClient.getShardReplicationFactor()` reads that entry, so its signature and behavior are unchanged; `condenseResponse()` now returns a `SimpleOrderedMap`. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 364b9381364b..236db54011d5 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -29,7 +29,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -46,7 +45,6 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; @@ -414,13 +412,8 @@ private NamedList directUpdate(UpdateRequest request, String collection) long end = System.nanoTime(); - @SuppressWarnings({"rawtypes"}) - RouteResponse rr = - condenseResponse( - shardResponses, (int) TimeUnit.MILLISECONDS.convert(end - start, TimeUnit.NANOSECONDS)); - rr.setRouteResponses(shardResponses); - rr.setRoutes(routes); - return rr; + return condenseResponse( + shardResponses, (int) TimeUnit.MILLISECONDS.convert(end - start, TimeUnit.NANOSECONDS)); } protected RouteException getRouteException( @@ -487,11 +480,22 @@ private Map> buildUrlMap( return urlMap; } - protected > T condenseResponse( - NamedList response, int timeMillis, Supplier supplier) { - T condensed = supplier.get(); + /** + * Key in the merged {@code responseHeader} of a routed update request: the achieved replication + * factor of each route, keyed by route URL. + */ + public static final String REPFACT_BY_ROUTE = "rfByRoute"; + + /** + * Merges the per-route responses of a routed update request into a single response. The merged + * {@code responseHeader} carries the lowest achieved replication factor as {@link + * UpdateRequest#REPFACT} and each route's own value under {@link #REPFACT_BY_ROUTE}. + */ + public SimpleOrderedMap condenseResponse(NamedList response, int timeMillis) { + SimpleOrderedMap condensed = new SimpleOrderedMap<>(); int status = 0; Integer rf = null; + SimpleOrderedMap rfByRoute = null; // TolerantUpdateProcessor List> toleratedErrors = null; @@ -511,6 +515,8 @@ protected > T condenseResponse( Object rfObj = header.get(UpdateRequest.REPFACT); if (rfObj != null && rfObj instanceof Integer routeRf) { if (rf == null || routeRf < rf) rf = routeRf; + if (rfByRoute == null) rfByRoute = new SimpleOrderedMap<>(); + rfByRoute.put(response.getName(i), routeRf); } @SuppressWarnings("unchecked") @@ -549,6 +555,7 @@ protected > T condenseResponse( cheader.add("status", status); cheader.add("QTime", timeMillis); if (rf != null) cheader.add(UpdateRequest.REPFACT, rf); + if (rfByRoute != null) cheader.add(REPFACT_BY_ROUTE, rfByRoute); if (null != toleratedErrors) { cheader.add("maxErrors", ToleratedUpdateError.getUserFriendlyMaxErrors(maxToleratedErrors)); cheader.add("errors", toleratedErrors); @@ -584,11 +591,6 @@ protected > T condenseResponse( return condensed; } - @SuppressWarnings({"rawtypes"}) - public RouteResponse condenseResponse(NamedList response, int timeMillis) { - return condenseResponse(response, timeMillis, RouteResponse::new); - } - @Override public NamedList request(SolrRequest request, String collection) throws SolrServerException, IOException { @@ -1241,14 +1243,15 @@ public int getMinAchievedReplicationFactor(String collection, NamedList resp) } /** - * Walks the NamedList response after performing an update request looking for the replication - * factor that was achieved in each shard involved in the request. For single doc updates, there + * Reads the replication factor that was achieved in each shard involved in a routed update + * request, from {@link #REPFACT_BY_ROUTE} in the response header. For single doc updates, there * will be only one shard in the return value. */ public Map getShardReplicationFactor(String collection, NamedList resp) { Map results = new HashMap<>(); - if (resp instanceof RouteResponse) { - NamedList> routes = ((RouteResponse) resp).getRouteResponses(); + NamedList header = (NamedList) resp.get("responseHeader"); + Object rfByRoute = header == null ? null : header.get(REPFACT_BY_ROUTE); + if (rfByRoute instanceof NamedList routes) { DocCollection coll = getDocCollection(collection, null); Map leaders = new HashMap<>(); for (Slice slice : coll.getActiveSlices()) { @@ -1261,13 +1264,9 @@ public Map getShardReplicationFactor(String collection, NamedLi } } - Iterator>> routeIter = routes.iterator(); - while (routeIter.hasNext()) { - Map.Entry> next = routeIter.next(); + for (Map.Entry next : routes) { String host = next.getKey(); - NamedList hostResp = next.getValue(); - Integer rf = - (Integer) ((NamedList) hostResp.get("responseHeader")).get(UpdateRequest.REPFACT); + Integer rf = (Integer) next.getValue(); if (rf != null) { String shard = leaders.get(host); if (shard == null) { @@ -1681,28 +1680,6 @@ void evictStale() { } } - @SuppressWarnings({"rawtypes"}) - public static class RouteResponse extends SimpleOrderedMap { - private NamedList> routeResponses; - private Map routes; - - public void setRouteResponses(NamedList> routeResponses) { - this.routeResponses = routeResponses; - } - - public NamedList> getRouteResponses() { - return routeResponses; - } - - public void setRoutes(Map routes) { - this.routes = routes; - } - - public Map getRoutes() { - return routes; - } - } - public static class RouteException extends SolrException { private NamedList throwables; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java index f7b5a6b658ac..e00870603772 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java @@ -43,7 +43,7 @@ * hold data to be serialized. It aims to minimize overhead and to be efficient at adding new * elements. */ -public non-sealed class SimpleOrderedMap extends NamedList implements Map { +public final class SimpleOrderedMap extends NamedList implements Map { private static final SimpleOrderedMap EMPTY = new SimpleOrderedMap<>(List.of()); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index ee80cbb85b9c..e5a965f66c05 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -18,7 +18,6 @@ import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST; import static org.apache.solr.client.solrj.impl.BaseHttpClusterStateProvider.SYS_PROP_CACHE_TIMEOUT_SECONDS; -import static org.apache.solr.client.solrj.impl.CloudSolrClient.RouteResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; @@ -378,29 +377,7 @@ public void testRouting() throws Exception { if (getRandomClient().isDirectUpdatesToLeadersOnly()) { checkSingleServer(response); } - RouteResponse rr = (RouteResponse) response; - Map routes = rr.getRoutes(); - Iterator> it = - routes.entrySet().iterator(); - while (it.hasNext()) { - Map.Entry entry = it.next(); - String coreUrl = entry.getKey(); - final String baseUrl = URLUtil.extractBaseUrl(coreUrl); - final String coreName = URLUtil.extractCoreFromCoreUrl(coreUrl); - UpdateRequest updateRequest = (UpdateRequest) entry.getValue().getRequest(); - SolrInputDocument doc = updateRequest.getDocuments().get(0); - String id = doc.getField("id").getValue().toString(); - ModifiableSolrParams params = new ModifiableSolrParams(); - params.add("q", "id:" + id); - params.add("distrib", "false"); - QueryRequest queryRequest = new QueryRequest(params); - - SolrClient solrClient = - new CollectionScopedSolrClient(cluster.getJetty(baseUrl).getSolrClient(), coreName); - QueryResponse queryResponse = queryRequest.process(solrClient); - SolrDocumentList docList = queryResponse.getResults(); - assertEquals(1, docList.getNumFound()); - } + assertDocsOnShardLeaders("routing_collection", "0", "2"); // Test the deleteById routing for UpdateRequest @@ -429,27 +406,7 @@ public void testRouting() throws Exception { if (threadedClient.isDirectUpdatesToLeadersOnly()) { checkSingleServer(response); } - rr = (RouteResponse) response; - routes = rr.getRoutes(); - it = routes.entrySet().iterator(); - while (it.hasNext()) { - Map.Entry entry = it.next(); - String coreUrl = entry.getKey(); - final String baseUrl = URLUtil.extractBaseUrl(coreUrl); - final String coreName = URLUtil.extractCoreFromCoreUrl(coreUrl); - UpdateRequest updateRequest = (UpdateRequest) entry.getValue().getRequest(); - SolrInputDocument doc = updateRequest.getDocuments().get(0); - String id = doc.getField("id").getValue().toString(); - ModifiableSolrParams params = new ModifiableSolrParams(); - params.add("q", "id:" + id); - params.add("distrib", "false"); - QueryRequest queryRequest = new QueryRequest(params); - SolrClient solrClient = - new CollectionScopedSolrClient(cluster.getJetty(baseUrl).getSolrClient(), coreName); - QueryResponse queryResponse = queryRequest.process(solrClient); - SolrDocumentList docList = queryResponse.getResults(); - assertEquals(1, docList.getNumFound()); - } + assertDocsOnShardLeaders("routing_collection", "0", "2"); } // Test that queries with _route_ params are routed by the client @@ -1096,17 +1053,34 @@ public void testRetryUpdatesWhenClusterStateIsStale() throws Exception { } } + /** Asserts that each doc is on the leader of the shard it routes to, querying that core alone. */ + private static void assertDocsOnShardLeaders(String collection, String... ids) throws Exception { + DocCollection col = cluster.getSolrClient().getClusterState().getCollection(collection); + for (String id : ids) { + String coreUrl = col.getRouter().getTargetSlice(id, null, null, null, col).getLeader().getCoreUrl(); + SolrClient solrClient = + new CollectionScopedSolrClient( + cluster.getJetty(URLUtil.extractBaseUrl(coreUrl)).getSolrClient(), + URLUtil.extractCoreFromCoreUrl(coreUrl)); + QueryResponse queryResponse = + new QueryRequest(params("q", "id:" + id, "distrib", "false")).process(solrClient); + assertEquals(1, queryResponse.getResults().getNumFound()); + } + } + + /** Asserts that a routed update was sent to shard leaders only, via the per-route rf header. */ private static void checkSingleServer(NamedList response) { - final RouteResponse rr = (RouteResponse) response; - final Map routes = rr.getRoutes(); - final Iterator> it = - routes.entrySet().iterator(); - while (it.hasNext()) { - Map.Entry entry = it.next(); - assertEquals( - "wrong number of servers: " + entry.getValue().getEndpoints(), - 1, - entry.getValue().getEndpoints().size()); + NamedList header = (NamedList) response.get("responseHeader"); + NamedList rfByRoute = (NamedList) header.get(CloudSolrClient.REPFACT_BY_ROUTE); + assertNotNull("no per-route rf in " + header, rfByRoute); + DocCollection col = + cluster.getSolrClient().getClusterState().getCollection("routing_collection"); + Set leaderUrls = new HashSet<>(); + for (Slice slice : col.getSlices()) { + leaderUrls.add(slice.getLeader().getCoreUrl()); + } + for (Map.Entry route : rfByRoute) { + assertTrue("not a leader: " + route.getKey(), leaderUrls.contains(route.getKey())); } } From fbc4c6949b54ead961d179d996a71f4bbb050581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Thu, 29 Oct 2026 20:04:24 +0000 Subject: [PATCH 3/7] SOLR-18603: gradlew tidy Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../solr/client/solrj/impl/CloudHttp2SolrClientTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index e5a965f66c05..b67854d21020 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -1057,7 +1057,8 @@ public void testRetryUpdatesWhenClusterStateIsStale() throws Exception { private static void assertDocsOnShardLeaders(String collection, String... ids) throws Exception { DocCollection col = cluster.getSolrClient().getClusterState().getCollection(collection); for (String id : ids) { - String coreUrl = col.getRouter().getTargetSlice(id, null, null, null, col).getLeader().getCoreUrl(); + String coreUrl = + col.getRouter().getTargetSlice(id, null, null, null, col).getLeader().getCoreUrl(); SolrClient solrClient = new CollectionScopedSolrClient( cluster.getJetty(URLUtil.extractBaseUrl(coreUrl)).getSolrClient(), From 4e99c4198ee9afdf1fda455f8ab096f35158f89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Thu, 29 Oct 2026 20:07:26 +0000 Subject: [PATCH 4/7] SOLR-18603: Fix a cast that sealing NamedList makes illegal A cast from a NamedList-typed expression to List is a compile error now that the hierarchy is sealed and no subtype implements List; the value there was never a NamedList, so type the containing map as NamedList. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../facet/DistributedFacetSimpleRefinementLongTailTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/test/org/apache/solr/search/facet/DistributedFacetSimpleRefinementLongTailTest.java b/solr/core/src/test/org/apache/solr/search/facet/DistributedFacetSimpleRefinementLongTailTest.java index 34e4d6461204..9ffb64f6c3bb 100644 --- a/solr/core/src/test/org/apache/solr/search/facet/DistributedFacetSimpleRefinementLongTailTest.java +++ b/solr/core/src/test/org/apache/solr/search/facet/DistributedFacetSimpleRefinementLongTailTest.java @@ -200,7 +200,7 @@ private void sanityCheckIndividualShards() throws Exception { { List bar_buckets = (List) - ((NamedList) shardFooBuckets[2].get(6).get("bar")).get("buckets"); + ((NamedList) shardFooBuckets[2].get(6).get("bar")).get("buckets"); assertEquals(6, bar_buckets.size()); for (int j = 0; j < 5; j++) { NamedList bucket = bar_buckets.get(j); From aa3e2ab91d523cb59f6cdc9db99578d311e112b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Fri, 30 Oct 2026 14:04:32 +0000 Subject: [PATCH 5/7] SOLR-18603: Address review comments Query the leader core via process(client, core) instead of wrapping it in a CollectionScopedSolrClient, and inline SimpleOrderedMap.of("count", ...) in NestableJsonFacetTest instead of a helper. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../solrj/impl/CloudHttp2SolrClientTest.java | 9 ++++---- .../solrj/response/NestableJsonFacetTest.java | 23 ++++++++----------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index b67854d21020..0941b3e15e07 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -1059,12 +1059,11 @@ private static void assertDocsOnShardLeaders(String collection, String... ids) t for (String id : ids) { String coreUrl = col.getRouter().getTargetSlice(id, null, null, null, col).getLeader().getCoreUrl(); - SolrClient solrClient = - new CollectionScopedSolrClient( - cluster.getJetty(URLUtil.extractBaseUrl(coreUrl)).getSolrClient(), - URLUtil.extractCoreFromCoreUrl(coreUrl)); QueryResponse queryResponse = - new QueryRequest(params("q", "id:" + id, "distrib", "false")).process(solrClient); + new QueryRequest(params("q", "id:" + id, "distrib", "false")) + .process( + cluster.getJetty(URLUtil.extractBaseUrl(coreUrl)).getSolrClient(), + URLUtil.extractCoreFromCoreUrl(coreUrl)); assertEquals(1, queryResponse.getResults().getNumFound()); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java index 5d05e9d9997c..8a3f6d3d6845 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java @@ -21,6 +21,7 @@ import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.response.json.NestableJsonFacet; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.junit.Test; public class NestableJsonFacetTest extends SolrTestCaseJ4 { @@ -33,10 +34,10 @@ public void testParsing() { buckets.add("val", "Nike"); NamedList vals = new NamedList<>(); vals.add("numBuckets", 10); - vals.add("allBuckets", countList(12)); - vals.add("before", countList(1)); - vals.add("after", countList(2)); - vals.add("between", countList(9)); + vals.add("allBuckets", SimpleOrderedMap.of("count",12)); + vals.add("before", SimpleOrderedMap.of("count",1)); + vals.add("after", SimpleOrderedMap.of("count",2)); + vals.add("between", SimpleOrderedMap.of("count",9)); vals.add("buckets", List.of(buckets)); list.add("test", vals); NestableJsonFacet facet = new NestableJsonFacet(list); @@ -50,20 +51,14 @@ public void testParsing() { buckets.add("val", "Nike"); vals = new NamedList<>(); vals.add("numBuckets", 10L); - vals.add("allBuckets", countList(12L)); - vals.add("before", countList(1L)); - vals.add("after", countList(2L)); - vals.add("between", countList(9L)); + vals.add("allBuckets", SimpleOrderedMap.of("count",12L)); + vals.add("before", SimpleOrderedMap.of("count",1L)); + vals.add("after", SimpleOrderedMap.of("count",2L)); + vals.add("between", SimpleOrderedMap.of("count",9L)); vals.add("buckets", List.of(buckets)); list.add("test", vals); facet = new NestableJsonFacet(list); assertEquals(12L, facet.getCount()); assertEquals(2L, facet.getBucketBasedFacets("test").getAfter()); } - - private static NamedList countList(Object count) { - NamedList list = new NamedList<>(); - list.add("count", count); - return list; - } } From 7f9acb9df610338c20537e1efd6ed75a6c64a13b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=C5=A0ari=C4=87?= Date: Fri, 30 Oct 2026 14:05:09 +0000 Subject: [PATCH 6/7] SOLR-18603: formatting Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XL2ChskWQPpS1niMQgCaQa --- .../solrj/response/NestableJsonFacetTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java index 8a3f6d3d6845..2e54375bbc60 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/NestableJsonFacetTest.java @@ -34,10 +34,10 @@ public void testParsing() { buckets.add("val", "Nike"); NamedList vals = new NamedList<>(); vals.add("numBuckets", 10); - vals.add("allBuckets", SimpleOrderedMap.of("count",12)); - vals.add("before", SimpleOrderedMap.of("count",1)); - vals.add("after", SimpleOrderedMap.of("count",2)); - vals.add("between", SimpleOrderedMap.of("count",9)); + vals.add("allBuckets", SimpleOrderedMap.of("count", 12)); + vals.add("before", SimpleOrderedMap.of("count", 1)); + vals.add("after", SimpleOrderedMap.of("count", 2)); + vals.add("between", SimpleOrderedMap.of("count", 9)); vals.add("buckets", List.of(buckets)); list.add("test", vals); NestableJsonFacet facet = new NestableJsonFacet(list); @@ -51,10 +51,10 @@ public void testParsing() { buckets.add("val", "Nike"); vals = new NamedList<>(); vals.add("numBuckets", 10L); - vals.add("allBuckets", SimpleOrderedMap.of("count",12L)); - vals.add("before", SimpleOrderedMap.of("count",1L)); - vals.add("after", SimpleOrderedMap.of("count",2L)); - vals.add("between", SimpleOrderedMap.of("count",9L)); + vals.add("allBuckets", SimpleOrderedMap.of("count", 12L)); + vals.add("before", SimpleOrderedMap.of("count", 1L)); + vals.add("after", SimpleOrderedMap.of("count", 2L)); + vals.add("between", SimpleOrderedMap.of("count", 9L)); vals.add("buckets", List.of(buckets)); list.add("test", vals); facet = new NestableJsonFacet(list); From b244ae94d6c84eec06bf1de5754e566b0fdb88d1 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Sat, 31 Oct 2026 00:51:56 -0400 Subject: [PATCH 7/7] SOLR-18603: cover per-shard replication-factor responses --- .../solrj/impl/CloudHttp2SolrClientTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index 0941b3e15e07..7f4a7971779a 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -360,6 +360,54 @@ private CloudSolrClient createHttpCSPBasedCloudSolrClient() { return new CloudSolrClient.Builder(solrUrls).build(); } + @Test + public void testShardReplicationFactors() throws Exception { + final String collection = "replication_factor_collection"; + CloudSolrClient client = cluster.getSolrClient(); + CollectionAdminRequest.createCollection(collection, "conf", 2, 1).process(client); + cluster.waitForActiveCollection(collection, 2, 2); + + DocCollection col = client.getClusterState().getCollection(collection); + String shard1Url = col.getSlice("shard1").getLeader().getCoreUrl(); + String shard2Url = col.getSlice("shard2").getLeader().getCoreUrl(); + NamedList> routeResponses = new NamedList<>(); + routeResponses.add( + shard1Url, + new NamedList<>( + Map.of( + "responseHeader", new NamedList<>(Map.of("status", 0, UpdateRequest.REPFACT, 2))))); + routeResponses.add( + shard2Url, + new NamedList<>( + Map.of( + "responseHeader", new NamedList<>(Map.of("status", 0, UpdateRequest.REPFACT, 3))))); + + NamedList response = client.condenseResponse(routeResponses, 7); + NamedList header = (NamedList) response.get("responseHeader"); + assertEquals(Map.of(shard1Url, 2, shard2Url, 3), header.get(CloudSolrClient.REPFACT_BY_ROUTE)); + assertEquals( + Map.of("shard1", 2, "shard2", 3), client.getShardReplicationFactor(collection, response)); + assertEquals(2, client.getMinAchievedReplicationFactor(collection, response)); + } + + @Test + public void testShardReplicationFactorsWithoutRouteFactors() { + CloudSolrClient client = cluster.getSolrClient(); + NamedList> routeResponses = new NamedList<>(); + routeResponses.add( + "http://unused/solr/core", + new NamedList<>(Map.of("responseHeader", new NamedList<>(Map.of("status", 0))))); + + NamedList response = client.condenseResponse(routeResponses, 7); + NamedList header = (NamedList) response.get("responseHeader"); + assertNull(header.get(CloudSolrClient.REPFACT_BY_ROUTE)); + assertNull(header.get(UpdateRequest.REPFACT)); + assertEquals(Map.of(), client.getShardReplicationFactor("missing_collection", response)); + assertEquals(-1, client.getMinAchievedReplicationFactor("missing_collection", response)); + assertEquals( + Map.of(), client.getShardReplicationFactor("missing_collection", new NamedList<>())); + } + @Test public void testRouting() throws Exception { CollectionAdminRequest.createCollection("routing_collection", "conf", 2, 1) @@ -1079,9 +1127,14 @@ private static void checkSingleServer(NamedList response) { for (Slice slice : col.getSlices()) { leaderUrls.add(slice.getLeader().getCoreUrl()); } + assertEquals("missing routes in " + rfByRoute, leaderUrls.size(), rfByRoute.size()); for (Map.Entry route : rfByRoute) { assertTrue("not a leader: " + route.getKey(), leaderUrls.contains(route.getKey())); + assertEquals(Integer.valueOf(1), route.getValue()); } + assertEquals( + Map.of("shard1", 1, "shard2", 1), + cluster.getSolrClient().getShardReplicationFactor("routing_collection", response)); } /**