From 5808d6425d895bc72c15b0f802be999a826cef09 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 7 Apr 2026 13:35:09 -0400 Subject: [PATCH 01/12] Make AdminHandlersProxy instantiable --- .../solr/handler/admin/AdminHandlersProxy.java | 15 +++++++-------- .../apache/solr/handler/admin/MetricsHandler.java | 2 +- .../solr/handler/admin/SystemInfoHandler.java | 2 +- .../apache/solr/handler/admin/api/GetMetrics.java | 4 ++-- .../solr/handler/admin/api/GetNodeSystemInfo.java | 4 ++-- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java index a91db17d9bb..a9989afa3c1 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java @@ -63,7 +63,7 @@ public class AdminHandlersProxy { * provided. For V2, use {@link AdminHandlersProxy#maybeProxyToNodes(String, SolrQueryRequest, * SolrQueryResponse, CoreContainer)} */ - public static boolean maybeProxyToNodes( + public boolean maybeProxyToNodes( SolrQueryRequest req, SolrQueryResponse rsp, CoreContainer container) throws IOException, SolrServerException, InterruptedException { return maybeProxyToNodes("V1", req, rsp, container); @@ -73,7 +73,7 @@ public static boolean maybeProxyToNodes( * Proxy this request to a different remote node's selected API version if 'node' or 'nodes' * parameter is provided */ - public static boolean maybeProxyToNodes( + public boolean maybeProxyToNodes( String apiVersion, SolrQueryRequest req, SolrQueryResponse rsp, CoreContainer container) throws IOException, SolrServerException, InterruptedException { @@ -112,7 +112,7 @@ public static boolean maybeProxyToNodes( } /** Handle non-Prometheus formats using the existing NamedList approach. */ - private static void handleNamedListFormat( + private void handleNamedListFormat( String apiVersion, Set nodes, String pathStr, @@ -146,7 +146,7 @@ private static void handleNamedListFormat( } /** Makes a remote request asynchronously. */ - private static CompletableFuture> callRemoteNode( + private CompletableFuture> callRemoteNode( String apiVersion, String nodeName, String uriPath, @@ -194,7 +194,7 @@ private static CompletableFuture> callRemoteNode( * @return set of resolved node names * @throws SolrException if node format is invalid */ - private static Set resolveNodes(String nodeNames, CoreContainer container) { + private Set resolveNodes(String nodeNames, CoreContainer container) { Set liveNodes = container.getZkController().zkStateReader.getClusterState().getLiveNodes(); @@ -223,7 +223,7 @@ private static Set resolveNodes(String nodeNames, CoreContainer containe * @param container the CoreContainer * @param rsp the response to populate */ - private static void handlePrometheusSingleNode( + private void handlePrometheusSingleNode( String apiVersion, String nodeName, String pathStr, @@ -251,8 +251,7 @@ private static void handlePrometheusSingleNode( } } - private static SolrRequest createRequest( - String apiVersion, String uriPath, SolrParams params) { + private SolrRequest createRequest(String apiVersion, String uriPath, SolrParams params) { if (apiVersion.equalsIgnoreCase("V1")) { return new GenericSolrRequest(SolrRequest.METHOD.GET, uriPath, params); } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index eaa5510bb02..eb51dcf5144 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -101,7 +101,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw + format); } - if (cc != null && AdminHandlersProxy.maybeProxyToNodes(req, rsp, cc)) { + if (cc != null && new AdminHandlersProxy().maybeProxyToNodes(req, rsp, cc)) { return; // Request was proxied to other node } SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp)); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java index 20fe09098d2..53733d1076f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java @@ -49,7 +49,7 @@ public SystemInfoHandler(CoreContainer cc) { public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.setHttpCaching(false); - if (AdminHandlersProxy.maybeProxyToNodes(req, rsp, getCoreContainer(req))) { + if (new AdminHandlersProxy().maybeProxyToNodes(req, rsp, getCoreContainer(req))) { return; // Request was proxied to other node } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java index ad45a8ad14b..35b50ab6c24 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java @@ -137,8 +137,8 @@ private void validateRequest(String acceptHeader) { private boolean proxyToNodes() { try { if (coreContainer != null - && AdminHandlersProxy.maybeProxyToNodes( - "V2", solrQueryRequest, solrQueryResponse, coreContainer)) { + && new AdminHandlersProxy() + .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse, coreContainer)) { return true; // Request was proxied to other node } } catch (Exception e) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java index 5885489f78e..9f8d58f55b2 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java @@ -68,8 +68,8 @@ public NodeSystemResponse getNodeSystemInfo(String nodes) { private boolean proxyToNodes() { try { if (coreContainer != null - && AdminHandlersProxy.maybeProxyToNodes( - "V2", solrQueryRequest, solrQueryResponse, coreContainer)) { + && new AdminHandlersProxy() + .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse, coreContainer)) { return true; // Request was proxied to other node } } catch (Exception e) { From 66a4d01abc4a028053453e884e739e3f0bc00627 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 8 Apr 2026 09:56:58 -0400 Subject: [PATCH 02/12] Extract CoreContainer to be constructor param --- .../handler/admin/AdminHandlersProxy.java | 23 +++++++++++-------- .../solr/handler/admin/MetricsHandler.java | 2 +- .../solr/handler/admin/SystemInfoHandler.java | 2 +- .../solr/handler/admin/api/GetMetrics.java | 4 ++-- .../handler/admin/api/GetNodeSystemInfo.java | 4 ++-- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java index a9989afa3c1..45561376b4c 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java @@ -58,23 +58,27 @@ public class AdminHandlersProxy { private static final String PARAM_NODE = "node"; private static final long PROMETHEUS_FETCH_TIMEOUT_SECONDS = 10; + private final CoreContainer container; + + public AdminHandlersProxy(CoreContainer container) { + this.container = container; + } + /** * Proxy this request to a different remote node's V1 API if 'node' or 'nodes' parameter is * provided. For V2, use {@link AdminHandlersProxy#maybeProxyToNodes(String, SolrQueryRequest, - * SolrQueryResponse, CoreContainer)} + * SolrQueryResponse)} */ - public boolean maybeProxyToNodes( - SolrQueryRequest req, SolrQueryResponse rsp, CoreContainer container) + public boolean maybeProxyToNodes(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException, SolrServerException, InterruptedException { - return maybeProxyToNodes("V1", req, rsp, container); + return maybeProxyToNodes("V1", req, rsp); } /** * Proxy this request to a different remote node's selected API version if 'node' or 'nodes' * parameter is provided */ - public boolean maybeProxyToNodes( - String apiVersion, SolrQueryRequest req, SolrQueryResponse rsp, CoreContainer container) + public boolean maybeProxyToNodes(String apiVersion, SolrQueryRequest req, SolrQueryResponse rsp) throws IOException, SolrServerException, InterruptedException { String pathStr = req.getPath(); @@ -95,7 +99,7 @@ public boolean maybeProxyToNodes( } params.remove(PARAM_NODE); - handlePrometheusSingleNode(apiVersion, nodeName, pathStr, params, container, rsp); + handlePrometheusSingleNode(apiVersion, nodeName, pathStr, params, rsp); } else { // Other formats (JSON/XML): use plural 'nodes' parameter for multi-node aggregation String nodeNames = req.getParams().get(PARAM_NODES); @@ -104,7 +108,7 @@ public boolean maybeProxyToNodes( } params.remove(PARAM_NODES); - Set nodes = resolveNodes(nodeNames, container); + Set nodes = resolveNodes(nodeNames); handleNamedListFormat(apiVersion, nodes, pathStr, params, container.getZkController(), rsp); } @@ -194,7 +198,7 @@ private CompletableFuture> callRemoteNode( * @return set of resolved node names * @throws SolrException if node format is invalid */ - private Set resolveNodes(String nodeNames, CoreContainer container) { + private Set resolveNodes(String nodeNames) { Set liveNodes = container.getZkController().zkStateReader.getClusterState().getLiveNodes(); @@ -228,7 +232,6 @@ private void handlePrometheusSingleNode( String nodeName, String pathStr, ModifiableSolrParams params, - CoreContainer container, SolrQueryResponse rsp) throws IOException, SolrServerException { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index eb51dcf5144..82b9b14eab9 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -101,7 +101,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw + format); } - if (cc != null && new AdminHandlersProxy().maybeProxyToNodes(req, rsp, cc)) { + if (cc != null && new AdminHandlersProxy(cc).maybeProxyToNodes(req, rsp)) { return; // Request was proxied to other node } SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp)); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java index 53733d1076f..c144edb59a8 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java @@ -49,7 +49,7 @@ public SystemInfoHandler(CoreContainer cc) { public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.setHttpCaching(false); - if (new AdminHandlersProxy().maybeProxyToNodes(req, rsp, getCoreContainer(req))) { + if (new AdminHandlersProxy(getCoreContainer(req)).maybeProxyToNodes(req, rsp)) { return; // Request was proxied to other node } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java index 35b50ab6c24..b3e9c75eab0 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java @@ -137,8 +137,8 @@ private void validateRequest(String acceptHeader) { private boolean proxyToNodes() { try { if (coreContainer != null - && new AdminHandlersProxy() - .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse, coreContainer)) { + && new AdminHandlersProxy(coreContainer) + .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse)) { return true; // Request was proxied to other node } } catch (Exception e) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java index 9f8d58f55b2..0be2ba9f199 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java @@ -68,8 +68,8 @@ public NodeSystemResponse getNodeSystemInfo(String nodes) { private boolean proxyToNodes() { try { if (coreContainer != null - && new AdminHandlersProxy() - .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse, coreContainer)) { + && new AdminHandlersProxy(coreContainer) + .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse)) { return true; // Request was proxied to other node } } catch (Exception e) { From 8740d2cfb2d5b44c27f8054e23b41b2d5c4c3f42 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 8 Apr 2026 09:58:36 -0400 Subject: [PATCH 03/12] Move proxy into its own package --- .../solr/handler/admin/LoggingHandler.java | 1 + .../solr/handler/admin/MetricsHandler.java | 1 + .../solr/handler/admin/SystemInfoHandler.java | 1 + .../solr/handler/admin/api/GetMetrics.java | 2 +- .../handler/admin/api/GetNodeSystemInfo.java | 2 +- .../admin/{ => proxy}/AdminHandlersProxy.java | 2 +- .../handler/admin/proxy/package-info.java | 19 +++++++++++++++++++ 7 files changed, 25 insertions(+), 3 deletions(-) rename solr/core/src/java/org/apache/solr/handler/admin/{ => proxy}/AdminHandlersProxy.java (99%) create mode 100644 solr/core/src/java/org/apache/solr/handler/admin/proxy/package-info.java diff --git a/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java index 7593bb7cbdd..121f807136b 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java @@ -31,6 +31,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.NodeLogging; +import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.handler.api.V2ApiUtils; import org.apache.solr.logging.LogWatcher; import org.apache.solr.request.SolrQueryRequest; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index 82b9b14eab9..b06c17bef1e 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -32,6 +32,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.GetMetrics; +import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.otel.FilterablePrometheusMetricReader; import org.apache.solr.request.SolrQueryRequest; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java index c144edb59a8..968b4daaf29 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java @@ -24,6 +24,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.GetNodeSystemInfo; +import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.handler.api.V2ApiUtils; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java index b3e9c75eab0..90833a0ac11 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java @@ -35,7 +35,7 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.core.CoreContainer; -import org.apache.solr.handler.admin.AdminHandlersProxy; +import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.jersey.PermissionName; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.otel.FilterablePrometheusMetricReader; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java index 0be2ba9f199..bd6681d7fc8 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java @@ -23,8 +23,8 @@ import org.apache.solr.client.api.model.NodeSystemResponse; import org.apache.solr.common.SolrException; import org.apache.solr.core.CoreContainer; -import org.apache.solr.handler.admin.AdminHandlersProxy; import org.apache.solr.handler.admin.SystemInfoProvider; +import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.jersey.PermissionName; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java similarity index 99% rename from solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java rename to solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java index 45561376b4c..460e3882bf6 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/AdminHandlersProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.solr.handler.admin; +package org.apache.solr.handler.admin.proxy; import java.io.IOException; import java.lang.invoke.MethodHandles; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/package-info.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/package-info.java new file mode 100644 index 00000000000..e8f03767637 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/package-info.java @@ -0,0 +1,19 @@ +/* + * 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. + */ + +/** Utilities for proxying requests to other nodes within the Solr cluster */ +package org.apache.solr.handler.admin.proxy; From c8f536c655451b598465fef0d14075b1eff05712 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 8 Apr 2026 12:43:03 -0400 Subject: [PATCH 04/12] Add delegating SolrRequest impl: WrappedSolrRequest Will be used by the proxy's nascent support for v2 APIs. --- .../solr/client/solrj/WrappedSolrRequest.java | 193 ++++++++++++++ .../client/solrj/WrappedSolrRequestTest.java | 242 ++++++++++++++++++ 2 files changed, 435 insertions(+) create mode 100644 solr/solrj/src/java/org/apache/solr/client/solrj/WrappedSolrRequest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/WrappedSolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/WrappedSolrRequest.java new file mode 100644 index 00000000000..4f2ab125122 --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/WrappedSolrRequest.java @@ -0,0 +1,193 @@ +/* + * 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.client.solrj; + +import java.io.IOException; +import java.security.Principal; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.response.ResponseParser; +import org.apache.solr.client.solrj.response.StreamingResponseCallback; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.NamedList; + +/** A {@link SolrRequest} that wrappeds all method calls to a wrapped instance. */ +public class WrappedSolrRequest extends SolrRequest { + + protected final SolrRequest wrapped; + + public WrappedSolrRequest(SolrRequest wrapped) { + super(wrapped.getMethod(), wrapped.getPath(), wrapped.getRequestType()); + this.wrapped = wrapped; + } + + // -- Abstract methods -- + + @Override + public SolrParams getParams() { + return wrapped.getParams(); + } + + @Override + protected T createResponse(NamedList namedList) { + return wrapped.createResponse(namedList); + } + + // -- Overrides for all non-final methods with private state in SolrRequest -- + + @Override + public METHOD getMethod() { + return wrapped.getMethod(); + } + + @Override + public void setMethod(METHOD method) { + wrapped.setMethod(method); + } + + @Override + public String getPath() { + return wrapped.getPath(); + } + + @Override + public void setPath(String path) { + wrapped.setPath(path); + } + + @Override + public ResponseParser getResponseParser() { + return wrapped.getResponseParser(); + } + + @Override + public void setResponseParser(ResponseParser responseParser) { + wrapped.setResponseParser(responseParser); + } + + @Override + public StreamingResponseCallback getStreamingResponseCallback() { + return wrapped.getStreamingResponseCallback(); + } + + @Override + public void setStreamingResponseCallback(StreamingResponseCallback callback) { + wrapped.setStreamingResponseCallback(callback); + } + + @Override + public Set getQueryParams() { + return wrapped.getQueryParams(); + } + + @Override + public void setQueryParams(Set queryParams) { + wrapped.setQueryParams(queryParams); + } + + @Override + public SolrRequestType getRequestType() { + return wrapped.getRequestType(); + } + + @Override + public void setRequestType(SolrRequestType requestType) { + wrapped.setRequestType(requestType); + } + + @Override + public List getPreferredNodes() { + return wrapped.getPreferredNodes(); + } + + @Override + public SolrRequest setPreferredNodes(List nodes) { + wrapped.setPreferredNodes(nodes); + return this; + } + + @Override + public Principal getUserPrincipal() { + return wrapped.getUserPrincipal(); + } + + @Override + public void setUserPrincipal(Principal userPrincipal) { + wrapped.setUserPrincipal(userPrincipal); + } + + @Override + public SolrRequest setBasicAuthCredentials(String user, String password) { + wrapped.setBasicAuthCredentials(user, password); + return this; + } + + @Override + public String getBasicAuthUser() { + return wrapped.getBasicAuthUser(); + } + + @Override + public String getBasicAuthPassword() { + return wrapped.getBasicAuthPassword(); + } + + @Override + public boolean requiresCollection() { + return wrapped.requiresCollection(); + } + + @Override + public ApiVersion getApiVersion() { + return wrapped.getApiVersion(); + } + + @Override + @Deprecated + public Collection getContentStreams() throws IOException { + return wrapped.getContentStreams(); + } + + @Override + public RequestWriter.ContentWriter getContentWriter(String expectedType) { + return wrapped.getContentWriter(expectedType); + } + + @Override + public String getCollection() { + return wrapped.getCollection(); + } + + @Override + public void addHeader(String key, String value) { + wrapped.addHeader(key, value); + } + + @Override + public void addHeaders(Map headers) { + wrapped.addHeaders(headers); + } + + @Override + public Map getHeaders() { + return wrapped.getHeaders(); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java new file mode 100644 index 00000000000..35830ee0a4d --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java @@ -0,0 +1,242 @@ +/* + * 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.client.solrj; + +import java.security.Principal; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.request.GenericSolrRequest; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.response.SimpleSolrResponse; +import org.apache.solr.client.solrj.response.StreamingResponseCallback; +import org.apache.solr.client.solrj.response.XMLResponseParser; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for {@link WrappedSolrRequest}; focused on ensuring that the "wrapped" and "wrapper" + * SolrRequest instances line up on retvals, etc. + * + *

Getters are tested by modifying the "wrapped" value and ensuring the "wrapper" reflects the + * change. Setters are tested by modifying the "wrapper" value and ensuring the change propagates to + * the "wrapped" instance. + */ +public class WrappedSolrRequestTest extends SolrTestCase { + + private GenericSolrRequest inner; + private WrappedSolrRequest wrapper; + + @Before + public void setUp() throws Exception { + super.setUp(); + inner = new GenericSolrRequest(SolrRequest.METHOD.GET, "/test"); + wrapper = new WrappedSolrRequest<>(inner); + } + + @Test + public void testGetMethod() { + inner.setMethod(SolrRequest.METHOD.POST); + assertEquals(inner.getMethod(), wrapper.getMethod()); + } + + @Test + public void testSetMethod() { + wrapper.setMethod(SolrRequest.METHOD.DELETE); + assertEquals(inner.getMethod(), wrapper.getMethod()); + } + + @Test + public void testGetPath() { + inner.setPath("/updated"); + assertEquals(inner.getPath(), wrapper.getPath()); + } + + @Test + public void testSetPath() { + wrapper.setPath("/updated"); + assertEquals(inner.getPath(), wrapper.getPath()); + } + + @Test + public void testGetResponseParser() { + inner.setResponseParser(new XMLResponseParser()); + assertSame(inner.getResponseParser(), wrapper.getResponseParser()); + } + + @Test + public void testSetResponseParser() { + XMLResponseParser rp = new XMLResponseParser(); + wrapper.setResponseParser(rp); + assertSame(inner.getResponseParser(), wrapper.getResponseParser()); + } + + @Test + public void testGetStreamingResponseCallback() { + StreamingResponseCallback cb = noopCallback(); + inner.setStreamingResponseCallback(cb); + assertSame(inner.getStreamingResponseCallback(), wrapper.getStreamingResponseCallback()); + } + + @Test + public void testSetStreamingResponseCallback() { + wrapper.setStreamingResponseCallback(noopCallback()); + assertSame(inner.getStreamingResponseCallback(), wrapper.getStreamingResponseCallback()); + } + + @Test + public void testGetQueryParams() { + inner.setQueryParams(Set.of("q", "rows")); + assertEquals(inner.getQueryParams(), wrapper.getQueryParams()); + } + + @Test + public void testSetQueryParams() { + wrapper.setQueryParams(Set.of("q", "rows")); + assertEquals(inner.getQueryParams(), wrapper.getQueryParams()); + } + + @Test + public void testGetRequestType() { + inner.setRequestType(SolrRequest.SolrRequestType.ADMIN); + assertEquals(inner.getRequestType(), wrapper.getRequestType()); + } + + @Test + public void testSetRequestType() { + wrapper.setRequestType(SolrRequest.SolrRequestType.QUERY); + assertEquals(inner.getRequestType(), wrapper.getRequestType()); + } + + @Test + public void testGetPreferredNodes() { + inner.setPreferredNodes(List.of("node1:8983_solr", "node2:8983_solr")); + assertEquals(inner.getPreferredNodes(), wrapper.getPreferredNodes()); + } + + @Test + public void testSetPreferredNodes() { + wrapper.setPreferredNodes(List.of("node1:8983_solr", "node2:8983_solr")); + assertEquals(inner.getPreferredNodes(), wrapper.getPreferredNodes()); + } + + @Test + public void testGetUserPrincipal() { + Principal principal = () -> "test-user"; + inner.setUserPrincipal(principal); + assertEquals(inner.getUserPrincipal(), wrapper.getUserPrincipal()); + } + + @Test + public void testSetUserPrincipal() { + wrapper.setUserPrincipal(() -> "test-user"); + assertEquals(inner.getUserPrincipal(), wrapper.getUserPrincipal()); + } + + @Test + public void testGetBasicAuthUser() { + inner.setBasicAuthCredentials("alice", "secret"); + assertEquals(inner.getBasicAuthUser(), wrapper.getBasicAuthUser()); + } + + @Test + public void testGetBasicAuthPassword() { + inner.setBasicAuthCredentials("alice", "secret"); + assertEquals(inner.getBasicAuthPassword(), wrapper.getBasicAuthPassword()); + } + + @Test + public void testSetBasicAuthCredentials() { + wrapper.setBasicAuthCredentials("alice", "secret"); + assertEquals(inner.getBasicAuthUser(), wrapper.getBasicAuthUser()); + assertEquals(inner.getBasicAuthPassword(), wrapper.getBasicAuthPassword()); + } + + @Test + public void testRequiresCollection() { + inner.setRequiresCollection(true); + assertEquals(inner.requiresCollection(), wrapper.requiresCollection()); + } + + @Test + public void testGetApiVersion() { + assertEquals(inner.getApiVersion(), wrapper.getApiVersion()); + } + + @Test + public void testGetContentStreams() throws Exception { + assertEquals(inner.getContentStreams(), wrapper.getContentStreams()); + } + + @Test + public void testGetContentWriter() { + RequestWriter.ContentWriter cw = + inner.withContent(new byte[] {1, 2, 3}, "application/octet-stream").contentWriter; + assertEquals( + inner.getContentWriter("application/octet-stream"), + wrapper.getContentWriter("application/octet-stream")); + assertSame(cw, wrapper.getContentWriter("application/octet-stream")); + } + + @Test + public void testGetCollection() { + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set("collection", "myCollection"); + GenericSolrRequest innerWithCollection = + new GenericSolrRequest(SolrRequest.METHOD.GET, "/test", params); + final var wrapperWithCollection = new WrappedSolrRequest<>(innerWithCollection); + assertEquals(innerWithCollection.getCollection(), wrapperWithCollection.getCollection()); + } + + @Test + public void testGetHeaders() { + inner.addHeader("X-Custom", "value"); + assertEquals(inner.getHeaders(), wrapper.getHeaders()); + } + + @Test + public void testAddHeader() { + wrapper.addHeader("X-Custom", "value"); + assertEquals(inner.getHeaders(), wrapper.getHeaders()); + } + + @Test + public void testAddHeaders() { + wrapper.addHeaders(Map.of("X-Foo", "foo", "X-Bar", "bar")); + assertEquals(inner.getHeaders(), wrapper.getHeaders()); + } + + @Test + public void testGetParams() { + SolrParams params = inner.getParams(); + assertSame(params, wrapper.getParams()); + } + + private static StreamingResponseCallback noopCallback() { + return new StreamingResponseCallback() { + @Override + public void streamSolrDocument(SolrDocument doc) {} + + @Override + public void streamDocListInfo(long numFound, long start, Float maxScore) {} + }; + } +} From 5628692be0d93a669aa0f4baa9d23bc6f9d7d2fb Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 8 Apr 2026 12:47:36 -0400 Subject: [PATCH 05/12] WIP: Partial proxy refactoring Code now compiles and tests mostly pass, with the notable exception of the metrics stuff which will definitely require some additional work. --- .../client/api/model/NodeSystemResponse.java | 16 ++ .../solr/handler/admin/LoggingHandler.java | 6 +- .../solr/handler/admin/MetricsHandler.java | 37 +++- .../solr/handler/admin/SystemInfoHandler.java | 7 +- .../solr/handler/admin/api/GetMetrics.java | 11 +- .../handler/admin/api/GetNodeSystemInfo.java | 28 ++- .../admin/proxy/AdminHandlersProxy.java | 194 +++++------------- .../admin/proxy/GenericV1RequestProxy.java | 76 +++++++ .../handler/admin/proxy/V2RequestProxy.java | 77 +++++++ 9 files changed, 289 insertions(+), 163 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java create mode 100644 solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java diff --git a/solr/api/src/java/org/apache/solr/client/api/model/NodeSystemResponse.java b/solr/api/src/java/org/apache/solr/client/api/model/NodeSystemResponse.java index 024f53cdffc..1811c05598d 100644 --- a/solr/api/src/java/org/apache/solr/client/api/model/NodeSystemResponse.java +++ b/solr/api/src/java/org/apache/solr/client/api/model/NodeSystemResponse.java @@ -16,6 +16,8 @@ */ package org.apache.solr.client.api.model; +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonProperty; import java.util.Date; import java.util.List; @@ -25,6 +27,20 @@ /** Response from /node/system */ public class NodeSystemResponse extends SolrJerseyResponse { + // TODO The typing here is kindof wonky - can I tighten 'Object' here to be NodeSystemResponse or + // will Jackson choke on that? + public Map remoteNodeData; + + @JsonAnyGetter + public Map remoteNodeData() { + return remoteNodeData; + } + + @JsonAnySetter + public void setRemoteNodeResponse(String field, Object value) { + remoteNodeData.put(field, value); + } + @JsonProperty public String host; @JsonProperty public String node; @JsonProperty public String mode; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java index 121f807136b..77e65ede715 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java @@ -31,7 +31,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.NodeLogging; -import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; +import org.apache.solr.handler.admin.proxy.GenericV1RequestProxy; import org.apache.solr.handler.api.V2ApiUtils; import org.apache.solr.logging.LogWatcher; import org.apache.solr.request.SolrQueryRequest; @@ -92,9 +92,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } rsp.setHttpCaching(false); - if (cc != null && AdminHandlersProxy.maybeProxyToNodes(req, rsp, cc)) { - return; // Request was proxied to other node - } + new GenericV1RequestProxy(cc, req, rsp).proxyRequest(); } private void squashV2Response(SolrQueryResponse rsp, LoggingResponse response) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index b06c17bef1e..a8c2c4901cc 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -26,13 +26,17 @@ import java.util.SortedMap; import java.util.function.BiConsumer; import org.apache.solr.api.JerseyResource; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.response.InputStreamResponseParser; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.GetMetrics; import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; +import org.apache.solr.handler.admin.proxy.GenericV1RequestProxy; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.otel.FilterablePrometheusMetricReader; import org.apache.solr.request.SolrQueryRequest; @@ -102,9 +106,12 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw + format); } - if (cc != null && new AdminHandlersProxy(cc).maybeProxyToNodes(req, rsp)) { + final var reqProxy = createMetricProxy(cc, req, rsp); + if (cc != null && reqProxy.shouldProxy()) { + reqProxy.proxyRequest(); return; // Request was proxied to other node } + SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp)); try { handleRequest(req.getParams(), (k, v) -> rsp.add(k, v)); @@ -144,6 +151,34 @@ public void handleRequest(SolrParams params, BiConsumer consumer consumer.accept("metrics", mergedSnapshots); } + public static AdminHandlersProxy createMetricProxy( + CoreContainer cc, SolrQueryRequest req, SolrQueryResponse rsp) { + return new GenericV1RequestProxy(cc, req, rsp) { + + // Metric requests use 'node' to proxy rather than the generally accepted "nodes" + @Override + protected String getDestinationNodeParamName() { + return "node"; + } + + // Metrics requests require a particular ResponseParser + @Override + protected SolrRequest createGenericRequest(String apiPath, SolrParams params) { + final var toProxy = super.createGenericRequest(apiPath, params); + String wt = params.get(CommonParams.WT, MetricUtils.PROMETHEUS_METRICS_WT); + toProxy.setResponseParser(new InputStreamResponseParser(wt)); + + return toProxy; + } + + // Metrics requests only proxy to single host, so proxied response is added at root level + @Override + public void processProxiedResponse(String nodeName, NamedList proxiedResponse) { + rsp.getValues().addAll(proxiedResponse); + } + }; + } + @Override public String getDescription() { return "A handler to return all the metrics gathered by Solr"; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java index 968b4daaf29..e11343f15da 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java @@ -24,7 +24,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.GetNodeSystemInfo; -import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; +import org.apache.solr.handler.admin.proxy.GenericV1RequestProxy; import org.apache.solr.handler.api.V2ApiUtils; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; @@ -50,8 +50,9 @@ public SystemInfoHandler(CoreContainer cc) { public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { rsp.setHttpCaching(false); - if (new AdminHandlersProxy(getCoreContainer(req)).maybeProxyToNodes(req, rsp)) { - return; // Request was proxied to other node + final var reqProxy = new GenericV1RequestProxy(getCoreContainer(req), req, rsp); + if (reqProxy.proxyRequest()) { + return; } SystemInfoProvider provider = new SystemInfoProvider(req); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java index 90833a0ac11..6f95a5c78fe 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java @@ -35,7 +35,7 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.core.CoreContainer; -import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; +import org.apache.solr.handler.admin.MetricsHandler; import org.apache.solr.jersey.PermissionName; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.otel.FilterablePrometheusMetricReader; @@ -69,6 +69,8 @@ public GetMetrics( this.enabled = coreContainer.getConfig().getMetricsConfig().isEnabled(); } + // TODO Rewrite implementing logic to use method parameters, rather than wrapping everything in a + // SolrParams. @Override @PermissionName(PermissionNameProvider.Name.METRICS_READ_PERM) public StreamingOutput getMetrics( @@ -136,9 +138,10 @@ private void validateRequest(String acceptHeader) { private boolean proxyToNodes() { try { - if (coreContainer != null - && new AdminHandlersProxy(coreContainer) - .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse)) { + if (coreContainer != null) { + final var reqProxy = + MetricsHandler.createMetricProxy(coreContainer, solrQueryRequest, solrQueryResponse); + reqProxy.proxyRequest(); return true; // Request was proxied to other node } } catch (Exception e) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java index bd6681d7fc8..d6eca67f0f1 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java @@ -21,10 +21,11 @@ import org.apache.solr.api.JerseyResource; import org.apache.solr.client.api.endpoint.NodeSystemInfoApi; import org.apache.solr.client.api.model.NodeSystemResponse; +import org.apache.solr.client.solrj.request.SystemApi; import org.apache.solr.common.SolrException; import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.admin.SystemInfoProvider; -import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; +import org.apache.solr.handler.admin.proxy.V2RequestProxy; import org.apache.solr.jersey.PermissionName; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; @@ -50,14 +51,15 @@ public GetNodeSystemInfo(SolrQueryRequest solrQueryRequest, SolrQueryResponse so @Override @PermissionName(PermissionNameProvider.Name.CONFIG_READ_PERM) public NodeSystemResponse getNodeSystemInfo(String nodes) { + NodeSystemResponse response = instantiateJerseyResponse(NodeSystemResponse.class); solrQueryResponse.setHttpCaching(false); - if (proxyToNodes()) { - return null; // Request handled via proxying + if (proxyToNodes(response, nodes)) { + return response; // Request handled via proxying } + // No proxying done; populate data for this node. SystemInfoProvider provider = new SystemInfoProvider(solrQueryRequest); - NodeSystemResponse response = instantiateJerseyResponse(NodeSystemResponse.class); provider.getNodeSystemInfo(response); if (log.isTraceEnabled()) { log.trace("Node {}, core root: {}", response.node, response.coreRoot); @@ -65,12 +67,20 @@ public NodeSystemResponse getNodeSystemInfo(String nodes) { return response; } - private boolean proxyToNodes() { + private boolean proxyToNodes(NodeSystemResponse response, String nodes) { try { - if (coreContainer != null - && new AdminHandlersProxy(coreContainer) - .maybeProxyToNodes("V2", solrQueryRequest, solrQueryResponse)) { - return true; // Request was proxied to other node + if (coreContainer != null) { + final var req = new SystemApi.GetNodeSystemInfo(); + req.setNodes(nodes); + final var reqProxy = + new V2RequestProxy(coreContainer, req) { + @Override + public void processTypedProxiedResponse( + String nodeName, NodeSystemResponse proxiedResponse) { + response.remoteNodeData.put(nodeName, proxiedResponse); + } + }; + return reqProxy.proxyRequest(); } } catch (Exception e) { throw new SolrException( diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java index 460e3882bf6..921001d0a20 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java @@ -21,6 +21,7 @@ import java.lang.invoke.MethodHandles; import java.net.URI; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; @@ -32,19 +33,9 @@ import java.util.concurrent.TimeoutException; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.request.GenericSolrRequest; -import org.apache.solr.client.solrj.request.GenericV2SolrRequest; -import org.apache.solr.client.solrj.response.InputStreamResponseParser; -import org.apache.solr.cloud.ZkController; import org.apache.solr.common.SolrException; -import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; -import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.core.CoreContainer; -import org.apache.solr.request.SolrQueryRequest; -import org.apache.solr.response.SolrQueryResponse; -import org.apache.solr.util.stats.MetricUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,111 +43,85 @@ * Static methods to proxy calls to an Admin (GET) API to other nodes in the cluster and return a * combined response */ -public class AdminHandlersProxy { +public abstract class AdminHandlersProxy { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private static final String PARAM_NODES = "nodes"; + protected static final String PARAM_NODES = "nodes"; private static final String PARAM_NODE = "node"; private static final long PROMETHEUS_FETCH_TIMEOUT_SECONDS = 10; - private final CoreContainer container; + protected final CoreContainer coreContainer; public AdminHandlersProxy(CoreContainer container) { - this.container = container; + this.coreContainer = container; } - /** - * Proxy this request to a different remote node's V1 API if 'node' or 'nodes' parameter is - * provided. For V2, use {@link AdminHandlersProxy#maybeProxyToNodes(String, SolrQueryRequest, - * SolrQueryResponse)} - */ - public boolean maybeProxyToNodes(SolrQueryRequest req, SolrQueryResponse rsp) - throws IOException, SolrServerException, InterruptedException { - return maybeProxyToNodes("V1", req, rsp); - } + /** Indicates whether a particular request should be proxied to other nodes or not. */ + public abstract boolean shouldProxy(); /** - * Proxy this request to a different remote node's selected API version if 'node' or 'nodes' - * parameter is provided + * Returns the nodes that the request should be proxied to. + * + *

Only called when {@link #shouldProxy()} returns true. Returned strings are in "live nodes" + * format (i.e. "someHost:8983_solr") */ - public boolean maybeProxyToNodes(String apiVersion, SolrQueryRequest req, SolrQueryResponse rsp) - throws IOException, SolrServerException, InterruptedException { - - String pathStr = req.getPath(); - ModifiableSolrParams params = new ModifiableSolrParams(req.getParams()); + public abstract Collection getDestinationNodes(); - // Check if response format is Prometheus/OpenMetrics - String wt = params.get(CommonParams.WT); - boolean isPrometheusFormat = - MetricUtils.PROMETHEUS_METRICS_WT.equals(wt) - || MetricUtils.OPEN_METRICS_WT.equals(wt) - || (wt == null && pathStr.endsWith("/metrics")); + /** Creates a {@link SolrRequest} instance representing the request being proxied. */ + public abstract SolrRequest prepareProxiedRequest(); - if (isPrometheusFormat) { - // Prometheus format: use singular 'node' parameter for single-node proxy - String nodeName = req.getParams().get(PARAM_NODE); - if (nodeName == null || nodeName.isEmpty()) { - return false; // No node parameter, handle locally - } - - params.remove(PARAM_NODE); - handlePrometheusSingleNode(apiVersion, nodeName, pathStr, params, rsp); - } else { - // Other formats (JSON/XML): use plural 'nodes' parameter for multi-node aggregation - String nodeNames = req.getParams().get(PARAM_NODES); - if (nodeNames == null || nodeNames.isEmpty()) { - return false; // No nodes parameter, handle locally - } + /** + * Process the response from request proxying to a particular node. + * + * @param nodeName the node that the proxied response is being received from, in "live node" + * format (i.e. "someHost:8983_solr") + * @param proxiedResponse the proxied response, as a NamedList + */ + public abstract void processProxiedResponse(String nodeName, NamedList proxiedResponse); - params.remove(PARAM_NODES); - Set nodes = resolveNodes(nodeNames); - handleNamedListFormat(apiVersion, nodes, pathStr, params, container.getZkController(), rsp); + public boolean proxyRequest() { + if (!shouldProxy()) { + return false; } + final var nodesToProxyTo = getDestinationNodes(); + final var solrRequest = prepareProxiedRequest(); + final var responseFutures = doProxyToNodes(nodesToProxyTo, solrRequest); + bulkProcessResponses(responseFutures); return true; } - /** Handle non-Prometheus formats using the existing NamedList approach. */ - private void handleNamedListFormat( - String apiVersion, - Set nodes, - String pathStr, - SolrParams params, - ZkController zkController, - SolrQueryResponse rsp) { - - Map>> responses = new LinkedHashMap<>(); - for (String node : nodes) { - responses.put(node, callRemoteNode(apiVersion, node, pathStr, params, zkController)); - } - - for (Map.Entry>> entry : responses.entrySet()) { + private void bulkProcessResponses(Map>> responseFutures) { + for (Map.Entry>> entry : responseFutures.entrySet()) { try { NamedList resp = entry.getValue().get(10, TimeUnit.SECONDS); - rsp.add(entry.getKey(), resp); - } catch (ExecutionException ee) { + processProxiedResponse(entry.getKey(), resp); + } catch (ExecutionException | InterruptedException ee) { log.warn( "Exception when fetching result from node {}", entry.getKey(), ee.getCause()); // nowarn - } catch (TimeoutException te) { - log.warn("Timeout when fetching result from node {}", entry.getKey()); - } catch (InterruptedException e) { - log.warn("Interrupted when fetching result from node {}", entry.getKey()); - Thread.currentThread().interrupt(); - break; // stop early + } catch (TimeoutException e) { + log.warn("Timeout exceeded waiting for response from proxied node {}", entry.getKey()); } } if (log.isDebugEnabled()) { - log.debug("Fetched response from {} nodes: {}", responses.size(), responses.keySet()); + log.debug( + "Fetched response from {} nodes: {}", responseFutures.size(), responseFutures.keySet()); + } + } + + private Map>> doProxyToNodes( + Collection nodesToProxyTo, SolrRequest solrRequest) { + Map>> responses = new LinkedHashMap<>(); + for (String node : nodesToProxyTo) { + responses.put(node, callRemoteNode(node, solrRequest)); } + return responses; } /** Makes a remote request asynchronously. */ private CompletableFuture> callRemoteNode( - String apiVersion, - String nodeName, - String uriPath, - SolrParams params, - ZkController zkController) { + String nodeName, SolrRequest solrRequest) { + final var zkController = coreContainer.getZkController(); // Validate that the node exists in the cluster if (!zkController.zkStateReader.getClusterState().getLiveNodes().contains(nodeName)) { throw new SolrException( @@ -164,25 +129,14 @@ private CompletableFuture> callRemoteNode( "Requested node " + nodeName + " is not part of cluster"); } - log.debug("Proxying {} request to node {}", uriPath, nodeName); + log.debug("Proxying {} request to node {}", solrRequest, nodeName); URI baseUri = URI.create(zkController.zkStateReader.getBaseUrlForNodeName(nodeName)); - SolrRequest proxyReq = createRequest(apiVersion, uriPath, params); - - // Set response parser based on wt parameter to ensure correct format is used - String wt = params.get(CommonParams.WT); - if (MetricUtils.PROMETHEUS_METRICS_WT.equals(wt) || MetricUtils.OPEN_METRICS_WT.equals(wt)) { - proxyReq.setResponseParser(new InputStreamResponseParser(wt)); - } - if (wt == null && uriPath.endsWith("/metrics")) { - proxyReq.setResponseParser(new InputStreamResponseParser(MetricUtils.PROMETHEUS_METRICS_WT)); - } - try { return zkController .getCoreContainer() .getDefaultHttpSolrClient() - .requestWithBaseUrl(baseUri.toString(), c -> c.requestAsync(proxyReq)); + .requestWithBaseUrl(baseUri.toString(), c -> c.requestAsync(solrRequest)); } catch (SolrServerException | IOException e) { // requestWithBaseUrl declares it throws these but it actually depends on the lambda assert false : "requestAsync doesn't throw; it returns a Future"; @@ -194,13 +148,12 @@ private CompletableFuture> callRemoteNode( * Resolve node names from the "nodes" parameter into a set of live node names. * * @param nodeNames the value of the "nodes" parameter ("all" or comma-separated node names) - * @param container the CoreContainer * @return set of resolved node names * @throws SolrException if node format is invalid */ - private Set resolveNodes(String nodeNames) { + protected Set validateNodeNames(String nodeNames) { Set liveNodes = - container.getZkController().zkStateReader.getClusterState().getLiveNodes(); + coreContainer.getZkController().zkStateReader.getClusterState().getLiveNodes(); if (nodeNames.equals("all")) { log.debug("All live nodes requested"); @@ -217,47 +170,4 @@ private Set resolveNodes(String nodeNames) { log.debug("Nodes requested: {}", nodes); return nodes; } - - /** - * Handle Prometheus format by proxying to a single node. * - * - * @param nodeName the name of the single node to proxy to - * @param pathStr the request path - * @param params the request parameters (with 'node' parameter already removed) - * @param container the CoreContainer - * @param rsp the response to populate - */ - private void handlePrometheusSingleNode( - String apiVersion, - String nodeName, - String pathStr, - ModifiableSolrParams params, - SolrQueryResponse rsp) - throws IOException, SolrServerException { - - // Keep wt=prometheus for the remote request so MetricsHandler accepts it - // The InputStreamResponseParser will return the Prometheus text in a "stream" key - Future> response = - callRemoteNode(apiVersion, nodeName, pathStr, params, container.getZkController()); - - try { - try { - NamedList resp = response.get(PROMETHEUS_FETCH_TIMEOUT_SECONDS, TimeUnit.SECONDS); - rsp.getValues().addAll(resp); - } catch (ExecutionException e) { - throw e.getCause(); - } - } catch (IOException | SolrServerException | RuntimeException | Error e) { - throw e; - } catch (Throwable t) { // unlikely? - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, t); - } - } - - private SolrRequest createRequest(String apiVersion, String uriPath, SolrParams params) { - if (apiVersion.equalsIgnoreCase("V1")) { - return new GenericSolrRequest(SolrRequest.METHOD.GET, uriPath, params); - } - return new GenericV2SolrRequest(SolrRequest.METHOD.GET, uriPath, params); - } } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java new file mode 100644 index 00000000000..7830aaa9fed --- /dev/null +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java @@ -0,0 +1,76 @@ +/* + * 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.handler.admin.proxy; + +import java.util.Collection; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.request.GenericSolrRequest; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; + +public class GenericV1RequestProxy extends AdminHandlersProxy { + + private final ModifiableSolrParams params; + private final SolrQueryRequest req; + private final SolrQueryResponse rsp; + + public GenericV1RequestProxy( + CoreContainer coreContainer, SolrQueryRequest req, SolrQueryResponse rsp) { + super(coreContainer); + this.req = req; + this.params = new ModifiableSolrParams(req.getParams()); + this.rsp = rsp; + } + + @Override + public boolean shouldProxy() { + String nodeNames = params.get(getDestinationNodeParamName()); + if (nodeNames == null || nodeNames.isEmpty()) { + return false; // No nodes parameter, handle locally + } + return true; + } + + @Override + public Collection getDestinationNodes() { + return validateNodeNames(params.get(getDestinationNodeParamName())); + } + + @Override + public SolrRequest prepareProxiedRequest() { + params.remove(getDestinationNodeParamName()); + return createGenericRequest(req.getPath(), params); + } + + @Override + public void processProxiedResponse(String nodeName, NamedList proxiedResponse) { + rsp.add(nodeName, proxiedResponse); + } + + /** The name of the query-param that indicates which node(s) should be proxied to. */ + protected String getDestinationNodeParamName() { + return PARAM_NODES; + } + + protected SolrRequest createGenericRequest(String apiPath, SolrParams params) { + return new GenericSolrRequest(SolrRequest.METHOD.GET, apiPath, params); + } +} diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java new file mode 100644 index 00000000000..4e4227f19bb --- /dev/null +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java @@ -0,0 +1,77 @@ +/* + * 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.handler.admin.proxy; + +import java.util.Collection; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.WrappedSolrRequest; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.core.CoreContainer; + +public abstract class V2RequestProxy extends AdminHandlersProxy { + + private SolrRequest solrRequest; + + public V2RequestProxy(CoreContainer coreContainer, SolrRequest solrRequest) { + super(coreContainer); + this.solrRequest = solrRequest; + } + + @Override + public boolean shouldProxy() { + String nodeNames = solrRequest.getParams().get(PARAM_NODES); + if (nodeNames == null || nodeNames.isEmpty()) { + return false; // No nodes parameter, handle locally + } + return true; + } + + @Override + public Collection getDestinationNodes() { + return validateNodeNames(solrRequest.getParams().get(PARAM_NODES)); + } + + @Override + public SolrRequest prepareProxiedRequest() { + return new WrappedSolrRequest<>(solrRequest) { + @Override + public SolrParams getParams() { + final var originalParams = this.wrapped.getParams(); + final var mutable = + (originalParams instanceof ModifiableSolrParams myMut) + ? myMut + : new ModifiableSolrParams(originalParams); + mutable.remove(PARAM_NODES); + return mutable; + } + }; + } + + @Override + @SuppressWarnings("unchecked") + public void processProxiedResponse(String nodeName, NamedList proxiedResponse) { + final var typedResponse = (T) proxiedResponse.get("response"); // TODO handle this cast better + processTypedProxiedResponse(nodeName, typedResponse); + } + + // TODO This is where I left off on this on 4/8 - let's see what I think of it when I return + // tomorrow + + public abstract void processTypedProxiedResponse(String nodeName, T proxiedResponse); +} From ad3aa10c15f6dd629a9f96a1cd029b750c7ce036 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 08:30:44 -0400 Subject: [PATCH 06/12] Fix minor bug --- .../java/org/apache/solr/handler/admin/MetricsHandler.java | 5 +++++ .../java/org/apache/solr/handler/admin/api/GetMetrics.java | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index a8c2c4901cc..adc2848d3b3 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -17,6 +17,8 @@ package org.apache.solr.handler.admin; +import static org.apache.solr.common.params.CommonParams.METRICS_PATH; + import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.ArrayList; @@ -165,6 +167,9 @@ protected String getDestinationNodeParamName() { @Override protected SolrRequest createGenericRequest(String apiPath, SolrParams params) { final var toProxy = super.createGenericRequest(apiPath, params); + // Metrics proxy might be called from either v1 or v2, but end up proxying to v1 for + // simplicity + toProxy.setPath(METRICS_PATH); String wt = params.get(CommonParams.WT, MetricUtils.PROMETHEUS_METRICS_WT); toProxy.setResponseParser(new InputStreamResponseParser(wt)); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java index 6f95a5c78fe..f4322af2c93 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetMetrics.java @@ -138,9 +138,9 @@ private void validateRequest(String acceptHeader) { private boolean proxyToNodes() { try { - if (coreContainer != null) { - final var reqProxy = - MetricsHandler.createMetricProxy(coreContainer, solrQueryRequest, solrQueryResponse); + final var reqProxy = + MetricsHandler.createMetricProxy(coreContainer, solrQueryRequest, solrQueryResponse); + if (coreContainer != null && reqProxy.shouldProxy()) { reqProxy.proxyRequest(); return true; // Request was proxied to other node } From 929908dfab5eae2666c3a473ba2accc591454380 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 11:14:36 -0400 Subject: [PATCH 07/12] Add javadocs 1 --- .../apache/solr/handler/admin/proxy/GenericV1RequestProxy.java | 1 + 1 file changed, 1 insertion(+) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java index 7830aaa9fed..7c7fd685137 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java @@ -26,6 +26,7 @@ import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; +/** Uses a v1 {@link GenericSolrRequest} to proxy the executing (v1) request to remote nodes */ public class GenericV1RequestProxy extends AdminHandlersProxy { private final ModifiableSolrParams params; From 966965d3b747540262e6f218b410c59de3459d4c Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 11:31:39 -0400 Subject: [PATCH 08/12] Rename v2 proxy implementation --- .../handler/admin/api/GetNodeSystemInfo.java | 4 ++-- ...Proxy.java => V2SolrRequestBasedProxy.java} | 18 +++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) rename solr/core/src/java/org/apache/solr/handler/admin/proxy/{V2RequestProxy.java => V2SolrRequestBasedProxy.java} (80%) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java index d6eca67f0f1..b865fb09d2e 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java @@ -25,7 +25,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.admin.SystemInfoProvider; -import org.apache.solr.handler.admin.proxy.V2RequestProxy; +import org.apache.solr.handler.admin.proxy.V2SolrRequestBasedProxy; import org.apache.solr.jersey.PermissionName; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; @@ -73,7 +73,7 @@ private boolean proxyToNodes(NodeSystemResponse response, String nodes) { final var req = new SystemApi.GetNodeSystemInfo(); req.setNodes(nodes); final var reqProxy = - new V2RequestProxy(coreContainer, req) { + new V2SolrRequestBasedProxy(coreContainer, req) { @Override public void processTypedProxiedResponse( String nodeName, NodeSystemResponse proxiedResponse) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java similarity index 80% rename from solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java rename to solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java index 4e4227f19bb..577508d7bf2 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2RequestProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java @@ -24,11 +24,18 @@ import org.apache.solr.common.util.NamedList; import org.apache.solr.core.CoreContainer; -public abstract class V2RequestProxy extends AdminHandlersProxy { +/** + * Uses a v2 {@link SolrRequest} instance to proxy requests to other nodes. + * + *

While this implementation is intended for v2 requests, callers may use it for v1 requests as + * well by overriding {@link #processProxiedResponse(String, NamedList)} to do something more + * appropriate with the response. + */ +public abstract class V2SolrRequestBasedProxy extends AdminHandlersProxy { private SolrRequest solrRequest; - public V2RequestProxy(CoreContainer coreContainer, SolrRequest solrRequest) { + public V2SolrRequestBasedProxy(CoreContainer coreContainer, SolrRequest solrRequest) { super(coreContainer); this.solrRequest = solrRequest; } @@ -66,12 +73,9 @@ public SolrParams getParams() { @Override @SuppressWarnings("unchecked") public void processProxiedResponse(String nodeName, NamedList proxiedResponse) { - final var typedResponse = (T) proxiedResponse.get("response"); // TODO handle this cast better + final var typedResponse = (T) proxiedResponse.get("response"); processTypedProxiedResponse(nodeName, typedResponse); } - // TODO This is where I left off on this on 4/8 - let's see what I think of it when I return - // tomorrow - - public abstract void processTypedProxiedResponse(String nodeName, T proxiedResponse); + protected abstract void processTypedProxiedResponse(String nodeName, T proxiedResponse); } From 1cbea8912f16937a31bde71a481069524eb9ed9d Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 11:37:00 -0400 Subject: [PATCH 09/12] Rename AdminHandlersProxy --- .../org/apache/solr/handler/admin/MetricsHandler.java | 4 ++-- .../handler/admin/proxy/GenericV1RequestProxy.java | 2 +- ...dminHandlersProxy.java => RemoteRequestProxy.java} | 11 +++-------- .../handler/admin/proxy/V2SolrRequestBasedProxy.java | 2 +- ...lersProxyTest.java => RemoteRequestProxyTest.java} | 2 +- 5 files changed, 8 insertions(+), 13 deletions(-) rename solr/core/src/java/org/apache/solr/handler/admin/proxy/{AdminHandlersProxy.java => RemoteRequestProxy.java} (94%) rename solr/core/src/test/org/apache/solr/handler/admin/{AdminHandlersProxyTest.java => RemoteRequestProxyTest.java} (98%) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index adc2848d3b3..784676656ae 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -37,8 +37,8 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.handler.admin.api.GetMetrics; -import org.apache.solr.handler.admin.proxy.AdminHandlersProxy; import org.apache.solr.handler.admin.proxy.GenericV1RequestProxy; +import org.apache.solr.handler.admin.proxy.RemoteRequestProxy; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.otel.FilterablePrometheusMetricReader; import org.apache.solr.request.SolrQueryRequest; @@ -153,7 +153,7 @@ public void handleRequest(SolrParams params, BiConsumer consumer consumer.accept("metrics", mergedSnapshots); } - public static AdminHandlersProxy createMetricProxy( + public static RemoteRequestProxy createMetricProxy( CoreContainer cc, SolrQueryRequest req, SolrQueryResponse rsp) { return new GenericV1RequestProxy(cc, req, rsp) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java index 7c7fd685137..7e54aeb28a3 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/GenericV1RequestProxy.java @@ -27,7 +27,7 @@ import org.apache.solr.response.SolrQueryResponse; /** Uses a v1 {@link GenericSolrRequest} to proxy the executing (v1) request to remote nodes */ -public class GenericV1RequestProxy extends AdminHandlersProxy { +public class GenericV1RequestProxy extends RemoteRequestProxy { private final ModifiableSolrParams params; private final SolrQueryRequest req; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java similarity index 94% rename from solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java rename to solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java index 921001d0a20..dc9ef560ac6 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/AdminHandlersProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java @@ -39,19 +39,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** - * Static methods to proxy calls to an Admin (GET) API to other nodes in the cluster and return a - * combined response - */ -public abstract class AdminHandlersProxy { +/** Proxies an API call to remote nodes; exposing hooks to process responses */ +public abstract class RemoteRequestProxy { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected static final String PARAM_NODES = "nodes"; - private static final String PARAM_NODE = "node"; - private static final long PROMETHEUS_FETCH_TIMEOUT_SECONDS = 10; protected final CoreContainer coreContainer; - public AdminHandlersProxy(CoreContainer container) { + public RemoteRequestProxy(CoreContainer container) { this.coreContainer = container; } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java index 577508d7bf2..d09757e9b52 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxy.java @@ -31,7 +31,7 @@ * well by overriding {@link #processProxiedResponse(String, NamedList)} to do something more * appropriate with the response. */ -public abstract class V2SolrRequestBasedProxy extends AdminHandlersProxy { +public abstract class V2SolrRequestBasedProxy extends RemoteRequestProxy { private SolrRequest solrRequest; diff --git a/solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java b/solr/core/src/test/org/apache/solr/handler/admin/RemoteRequestProxyTest.java similarity index 98% rename from solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java rename to solr/core/src/test/org/apache/solr/handler/admin/RemoteRequestProxyTest.java index 14411a38876..1aaa3b0970d 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/RemoteRequestProxyTest.java @@ -32,7 +32,7 @@ import org.junit.BeforeClass; import org.junit.Test; -public class AdminHandlersProxyTest extends SolrCloudTestCase { +public class RemoteRequestProxyTest extends SolrCloudTestCase { private CloudSolrClient solrClient; @BeforeClass From 4ca7464b30a70f2236b6362425bba70857c57eec Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 14:35:18 -0400 Subject: [PATCH 10/12] Add unit tests for proxy impls --- .../proxy/GenericV1RequestProxyTest.java | 103 +++++++++++++++++ .../proxy/V2SolrRequestBasedProxyTest.java | 106 ++++++++++++++++++ 2 files changed, 209 insertions(+) create mode 100644 solr/core/src/test/org/apache/solr/handler/admin/proxy/GenericV1RequestProxyTest.java create mode 100644 solr/core/src/test/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxyTest.java diff --git a/solr/core/src/test/org/apache/solr/handler/admin/proxy/GenericV1RequestProxyTest.java b/solr/core/src/test/org/apache/solr/handler/admin/proxy/GenericV1RequestProxyTest.java new file mode 100644 index 00000000000..b146533aa2b --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/proxy/GenericV1RequestProxyTest.java @@ -0,0 +1,103 @@ +/* + * 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.handler.admin.proxy; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Unit tests for {@link GenericV1RequestProxy} */ +public class GenericV1RequestProxyTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void ensureWorkingMockito() { + assumeWorkingMockito(); + } + + private ModifiableSolrParams solrParams; + private SolrQueryRequest mockRequest; + + @Before + public void setUpMocks() { + solrParams = new ModifiableSolrParams(); + mockRequest = mock(SolrQueryRequest.class); + + when(mockRequest.getParams()).thenReturn(solrParams); + } + + @Test + public void shouldProxyReflectsPresenceOfNodesParam() { + var proxy = new GenericV1RequestProxy(null, mockRequest, new SolrQueryResponse()); + assertFalse( + "Expected 'shouldProxy' to return false when 'nodes' param is absent", proxy.shouldProxy()); + + solrParams.add("nodes", "localhost:7574_solr"); + proxy = new GenericV1RequestProxy(null, mockRequest, new SolrQueryResponse()); + assertTrue( + "Expected 'shouldProxy' to return true when 'nodes' param is present", proxy.shouldProxy()); + } + + // ---- getDestinationNodes() tests ---- + // + // validateNodeNames() requires a live ZkController, so these tests use an anonymous subclass + // that stubs it out, keeping the focus on getDestinationNodes()'s own param-reading logic. + + @Test + public void testDestinationNodesExtractsValueFromNodes() { + solrParams.add("nodes", "somehost:8983_solr"); + + // Stub out live-node validation for tests. + GenericV1RequestProxy proxy = + new GenericV1RequestProxy(null, mockRequest, new SolrQueryResponse()) { + @Override + protected Set validateNodeNames(String nodeNames) { + return new HashSet<>(Arrays.asList(nodeNames.split(","))); + } + }; + + Collection nodes = proxy.getDestinationNodes(); + assertEquals(Set.of("somehost:8983_solr"), new HashSet<>(nodes)); + } + + @Test + public void testPreparedRequestMirrorsSolrQueryRequestVals() { + solrParams.set("nodes", "somehost:8983_solr"); + solrParams.set("wt", "json"); + when(mockRequest.getPath()).thenReturn("/admin/info/system"); + + GenericV1RequestProxy proxy = + new GenericV1RequestProxy(null, mockRequest, new SolrQueryResponse()); + SolrRequest prepared = proxy.prepareProxiedRequest(); + + assertNull( + "'nodes' param should be stripped from proxied request", prepared.getParams().get("nodes")); + assertEquals("json", prepared.getParams().get("wt")); + assertEquals("/admin/info/system", prepared.getPath()); + } +} diff --git a/solr/core/src/test/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxyTest.java b/solr/core/src/test/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxyTest.java new file mode 100644 index 00000000000..b0deb0ccdf7 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/proxy/V2SolrRequestBasedProxyTest.java @@ -0,0 +1,106 @@ +/* + * 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.handler.admin.proxy; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Unit tests for {@link V2SolrRequestBasedProxy} */ +public class V2SolrRequestBasedProxyTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void ensureWorkingMockito() { + assumeWorkingMockito(); + } + + private ModifiableSolrParams solrParams; + private SolrRequest mockSolrRequest; + + @Before + @SuppressWarnings("unchecked") + public void setUpMocks() { + solrParams = new ModifiableSolrParams(); + mockSolrRequest = mock(SolrRequest.class); + when(mockSolrRequest.getParams()).thenReturn(solrParams); + // WrappedSolrRequest's constructor calls these, so stub them to avoid NPEs + when(mockSolrRequest.getMethod()).thenReturn(SolrRequest.METHOD.GET); + when(mockSolrRequest.getPath()).thenReturn("/api/node/properties"); + when(mockSolrRequest.getRequestType()).thenReturn(SolrRequest.SolrRequestType.ADMIN); + } + + /** Minimal concrete subclass for testing. */ + private class TestProxy extends V2SolrRequestBasedProxy { + TestProxy() { + super(null, mockSolrRequest); + } + + @Override + protected void processTypedProxiedResponse(String nodeName, Object proxiedResponse) {} + + // Override the live-node validation that usually happens here. + @Override + protected Set validateNodeNames(String nodeNames) { + return new HashSet<>(Arrays.asList(nodeNames.split(","))); + } + } + + @Test + public void shouldProxyReflectsPresenceOfNodesParam() { + var proxy = new TestProxy(); + assertFalse( + "Expected 'shouldProxy' to return false when 'nodes' param is absent", proxy.shouldProxy()); + + solrParams.add("nodes", "localhost:7574_solr"); + assertTrue( + "Expected 'shouldProxy' to return true when 'nodes' param is present", proxy.shouldProxy()); + } + + @Test + public void testDestinationNodesExtractsValueFromNodes() { + solrParams.add("nodes", "somehost:8983_solr"); + + // Stub out live-node validation for tests. + final var proxy = new TestProxy(); + + Collection nodes = proxy.getDestinationNodes(); + assertEquals(Set.of("somehost:8983_solr"), new HashSet<>(nodes)); + } + + @Test + public void testPreparedRequestMirrorsSolrRequestVals() { + solrParams.set("nodes", "somehost:8983_solr"); + solrParams.set("wt", "json"); + + SolrRequest prepared = new TestProxy().prepareProxiedRequest(); + + assertNull( + "'nodes' param should be stripped from proxied request", prepared.getParams().get("nodes")); + assertEquals("json", prepared.getParams().get("wt")); + assertEquals("/api/node/properties", prepared.getPath()); + } +} From 8b40de190689d2a323512c665a759f80ae0e4ced Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 21 Apr 2026 14:41:06 -0400 Subject: [PATCH 11/12] javadoc tweak --- .../apache/solr/handler/admin/proxy/RemoteRequestProxy.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java b/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java index dc9ef560ac6..3520e880c14 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java @@ -39,7 +39,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Proxies an API call to remote nodes; exposing hooks to process responses */ +/** + * Proxies an API call to remote nodes; exposing hooks to process responses + * + * @lucene.experimental + */ public abstract class RemoteRequestProxy { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected static final String PARAM_NODES = "nodes"; From 24d1ad5e46510a224e0e3c3a915a5babd8910d48 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Wed, 22 Apr 2026 08:22:32 -0400 Subject: [PATCH 12/12] precommit fix, 1 --- .../org/apache/solr/client/solrj/WrappedSolrRequestTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java index 35830ee0a4d..6f8bfe58a64 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/WrappedSolrRequestTest.java @@ -46,6 +46,7 @@ public class WrappedSolrRequestTest extends SolrTestCase { private WrappedSolrRequest wrapper; @Before + @Override public void setUp() throws Exception { super.setUp(); inner = new GenericSolrRequest(SolrRequest.METHOD.GET, "/test"); @@ -182,6 +183,7 @@ public void testGetApiVersion() { } @Test + @SuppressWarnings("UndefinedEquals") // Reference-check equality here is fine. public void testGetContentStreams() throws Exception { assertEquals(inner.getContentStreams(), wrapper.getContentStreams()); }