This is an automated email from the ASF dual-hosted git repository. houston pushed a commit to branch add-pki-caching in repository https://gitbox.apache.org/repos/asf/solr.git
commit 5afd8dac0c4d5f4d19fb2f789b9ca85f7ae278e6 Author: James Dyer <[email protected]> AuthorDate: Thu Oct 3 08:21:16 2024 -0500 SOLR-17464: Fixed Http2SolrClient bug in that 'requestAsync' triggered NPE when using a shared Jetty client (#2733) --- solr/CHANGES.txt | 2 ++ .../solr/client/solrj/impl/Http2SolrClient.java | 6 ++++ .../client/solrj/impl/Http2SolrClientTest.java | 40 ++++++++++++++++++++-- .../client/solrj/impl/HttpJdkSolrClientTest.java | 24 +++++++++++-- .../client/solrj/impl/HttpSolrClientTestBase.java | 9 ++--- 5 files changed, 71 insertions(+), 10 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 7af27d91614..a7be19b84d6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -144,6 +144,8 @@ Bug Fixes * SOLR-17417: Remove unnecessary code in PKIAuthPlugin and HttpSolrCall (Houston Putman, janhoy, Liu Huajin) +* SOLR-17464: Fixed Http2SolrClient bug in that 'requestAsync' triggered NPE when using a shared Jetty client (Jason Gerlowski, James Dyer) + Dependency Upgrades --------------------- * PR#2512: Update dependency com.carrotsearch:hppc to v0.10.0 (solrbot) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index d5e5367a2bc..0def45620e4 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -132,6 +132,9 @@ public class Http2SolrClient extends HttpSolrClientBase { if (builder.httpClient != null) { this.httpClient = builder.httpClient; + if (this.executor == null) { + this.executor = builder.executor; + } this.closeClient = false; } else { this.httpClient = createHttpClient(builder); @@ -1129,6 +1132,9 @@ public class Http2SolrClient extends HttpSolrClientBase { this.listenerFactory = new ArrayList<HttpListenerFactory>(); http2SolrClient.listenerFactory.forEach(this.listenerFactory::add); } + if (this.executor == null) { + this.executor = http2SolrClient.executor; + } return this; } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java index e84abb1218c..7217969ce76 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java @@ -20,6 +20,7 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.Collections; import java.util.HashMap; import java.util.concurrent.TimeUnit; import org.apache.solr.client.solrj.ResponseParser; @@ -265,17 +266,37 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase { @Test public void testDeprecatedAsyncGet() throws Exception { - super.testQueryAsync(true); + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, true); } @Test public void testAsyncGet() throws Exception { - super.testQueryAsync(false); + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, false); } @Test public void testDeprecatedAsyncPost() throws Exception { super.testUpdateAsync(true); + + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, true); } @Test @@ -293,6 +314,21 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase { super.testAsyncExceptionBase(false); } + @Test + public void testAsyncQueryWithSharedClient() throws Exception { + DebugServlet.clear(); + final var url = getBaseUrl() + DEBUG_SERVLET_PATH; + ResponseParser rp = new XMLResponseParser(); + final var queryParams = new MapSolrParams(Collections.singletonMap("q", "*:*")); + final var builder = + new Http2SolrClient.Builder(url).withDefaultCollection(DEFAULT_CORE).withResponseParser(rp); + try (Http2SolrClient originalClient = builder.build()) { + final var derivedBuilder = builder.withHttpClient(originalClient); + super.testQueryAsync(derivedBuilder, false); + } + } + + @Test public void testFollowRedirect() throws Exception { final String clientUrl = getBaseUrl() + REDIRECT_SERVLET_PATH; try (Http2SolrClient client = diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 12115983b9a..294b6ae9a48 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -200,17 +200,37 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase { @Test public void testDeprecatedAsyncGet() throws Exception { - super.testQueryAsync(true); + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, true); } @Test public void testAsyncGet() throws Exception { - super.testQueryAsync(false); + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, false); } @Test public void testDeprecatedAsyncPost() throws Exception { super.testUpdateAsync(true); + + HttpSolrClientBuilderBase<?, ?> b = + builder( + getBaseUrl() + DEBUG_SERVLET_PATH, + DEFAULT_CONNECTION_TIMEOUT, + DEFAULT_CONNECTION_TIMEOUT) + .withResponseParser(new XMLResponseParser()); + super.testQueryAsync(b, true); } @Test diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java index b545a83adf0..d74177a9e2a 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java @@ -608,13 +608,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase { } } - protected void testQueryAsync(boolean useDeprecatedApi) throws Exception { - ResponseParser rp = new XMLResponseParser(); + protected void testQueryAsync(HttpSolrClientBuilderBase<?, ?> builder, boolean useDeprecatedApi) + throws Exception { DebugServlet.clear(); DebugServlet.addResponseHeader("Content-Type", "application/xml; charset=UTF-8"); - String url = getBaseUrl() + DEBUG_SERVLET_PATH; - HttpSolrClientBuilderBase<?, ?> b = - builder(url, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT).withResponseParser(rp); int limit = 10; CountDownLatch latch = new CountDownLatch(limit); // Deprecated API use @@ -623,7 +620,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase { List<CompletableFuture<NamedList<Object>>> futures = new ArrayList<>(); - try (HttpSolrClientBase client = b.build()) { + try (HttpSolrClientBase client = builder.build()) { for (int i = 0; i < limit; i++) { DebugServlet.responseBodyByQueryFragment.put( ("id=KEY-" + i),
