From 446e6c55a41947851757199593167fdf9267c8de Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 29 Mar 2026 00:16:02 -0400 Subject: [PATCH] Port testCompression to BATS; remove GzipHandler from JettySolrRunner The testCompression test in BasicHttpSolrClientTest verified that Solr's Jetty server responds with gzip-encoded content when the client sends Accept-Encoding: gzip. This behavior belongs in an integration test against the real packaged server, not the embedded test Jetty. - Add solr/packaging/test/test_compression.bats with three tests: server omits Content-Encoding without Accept-Encoding header, server includes Content-Encoding: gzip when requested, and curl can decompress and parse the response end-to-end. - Remove the GzipHandler setup from JettySolrRunner (it was already commented out with a TODO pointing here). - Remove testCompression from BasicHttpSolrClientTest. Refactor JettySolrRunner filter access to use generic getFilter() Replace specific filter accessors (getDebugFilter, getSolrRateLimitFilter) with a single generic getFilter(Class) method. Update call sites in ShardRoutingTest and TestInPlaceUpdatesDistrib to register DebugFilter via getExtraRequestFilters() rather than relying on it being added automatically. Broaden getExtraRequestFilters() return type from SortedMap to SequencedMap. --- .../apache/solr/cloud/ShardRoutingTest.java | 12 +- .../solr/servlet/TestRequestRateLimiter.java | 5 +- .../update/TestInPlaceUpdatesDistrib.java | 25 ++++- solr/packaging/test/test_compression.bats | 47 ++++++++ .../solr/BaseDistributedSearchTestCase.java | 3 +- .../apache/solr/embedded/JettySolrRunner.java | 104 ++++++------------ .../solrj/apache/BasicHttpSolrClientTest.java | 74 ------------- 7 files changed, 114 insertions(+), 156 deletions(-) create mode 100644 solr/packaging/test/test_compression.bats diff --git a/solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java b/solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java index cbb8df7d37f4..6fc5d589fd04 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ShardRoutingTest.java @@ -16,9 +16,12 @@ */ package org.apache.solr.cloud; +import jakarta.servlet.Filter; import java.lang.invoke.MethodHandles; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.SequencedMap; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.client.solrj.response.QueryResponse; @@ -327,10 +330,15 @@ public void doAtomicUpdate() throws Exception { } } + @Override + public SequencedMap, String> getExtraRequestFilters() { + return new LinkedHashMap<>(Map.of(JettySolrRunner.DebugFilter.class, "/*")); + } + long getNumRequests() { - long n = controlJetty.getDebugFilter().getTotalRequests(); + long n = controlJetty.getFilter(JettySolrRunner.DebugFilter.class).getTotalRequests(); for (JettySolrRunner jetty : jettys) { - n += jetty.getDebugFilter().getTotalRequests(); + n += jetty.getFilter(JettySolrRunner.DebugFilter.class).getTotalRequests(); } return n; } diff --git a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java index d551862227cf..43f627d917a2 100644 --- a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java +++ b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java @@ -72,7 +72,8 @@ public void testConcurrentQueries() throws Exception { CollectionAdminRequest.createCollection(FIRST_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(FIRST_COLLECTION, 1, 1); - RateLimitFilter rateLimitFilter = cluster.getJettySolrRunner(0).getSolrRateLimitFilter(); + RateLimitFilter rateLimitFilter = + cluster.getJettySolrRunner(0).getFilter(RateLimitFilter.class); RateLimiterConfig rateLimiterConfig = new RateLimiterConfig( @@ -291,7 +292,7 @@ public void testSlotBorrowing() throws Exception { CollectionAdminRequest.createCollection(SECOND_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(SECOND_COLLECTION, 1, 1); - RateLimitFilter rateLimitFilter = cluster.getJettySolrRunner(0).getSolrRateLimitFilter(); + var rateLimitFilter = cluster.getJettySolrRunner(0).getFilter(RateLimitFilter.class); RateLimiterConfig queryRateLimiterConfig = new RateLimiterConfig( diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java index 8576af702088..cb7406c1d2f6 100644 --- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java +++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java @@ -19,14 +19,17 @@ import static org.hamcrest.core.StringContains.containsString; +import jakarta.servlet.Filter; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.SequencedMap; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -202,9 +205,14 @@ public void test() throws Exception { // reorderedDBQsUsingUpdatedValueFromADroppedUpdate(); } + @Override + public SequencedMap, String> getExtraRequestFilters() { + return new LinkedHashMap<>(Map.of(JettySolrRunner.DebugFilter.class, "/*")); + } + private void resetDelays() { for (JettySolrRunner j : jettys) { - j.getDebugFilter().unsetDelay(); + j.getFilter(JettySolrRunner.DebugFilter.class).unsetDelay(); } } @@ -1236,7 +1244,7 @@ private void delayedReorderingFetchesMissingUpdateFromLeaderTest() throws Except .get(SHARD1) .get(1) .jetty - .getDebugFilter() + .getFilter(JettySolrRunner.DebugFilter.class) .addDelay("Waiting for dependant update to timeout", 1, 6000); ExecutorService threadpool = @@ -1324,7 +1332,12 @@ private void delayedReorderingFetchesMissingUpdateFromLeaderTest() throws Except { clearIndex(); commit(); - shardToJetty.get(SHARD1).get(1).jetty.getDebugFilter().unsetDelay(); + shardToJetty + .get(SHARD1) + .getFirst() + .jetty + .getFilter(JettySolrRunner.DebugFilter.class) + .unsetDelay(); updates.add(regularDeleteRequest(1)); @@ -1332,13 +1345,13 @@ private void delayedReorderingFetchesMissingUpdateFromLeaderTest() throws Except .get(SHARD1) .get(1) .jetty - .getDebugFilter() + .getFilter(JettySolrRunner.DebugFilter.class) .addDelay("Waiting for dependant update to timeout", 1, 5999); // the first update shardToJetty .get(SHARD1) .get(1) .jetty - .getDebugFilter() + .getFilter(JettySolrRunner.DebugFilter.class) .addDelay("Waiting for dependant update to timeout", 4, 5998); // the delete update threadpool = @@ -1635,7 +1648,7 @@ private void reorderedDBQsUsingUpdatedValueFromADroppedUpdate() throws Exception .get(SHARD1) .get(1) .jetty - .getDebugFilter() + .getFilter(JettySolrRunner.DebugFilter.class) .addDelay("Waiting for dependant update to timeout", 2, 8000); ExecutorService threadpool = diff --git a/solr/packaging/test/test_compression.bats b/solr/packaging/test/test_compression.bats new file mode 100644 index 000000000000..575490c8f99a --- /dev/null +++ b/solr/packaging/test/test_compression.bats @@ -0,0 +1,47 @@ +#!/usr/bin/env bats + +# 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. + +load bats_helper + +setup_file() { + common_clean_setup + solr start -e films +} + +setup() { + common_setup +} + +teardown_file() { + save_home_on_failure + solr stop --all >/dev/null 2>&1 +} + +@test "server does not compress response without Accept-Encoding header" { + run curl -s -D - -o /dev/null "http://localhost:${SOLR_PORT}/solr/films/select?q=*:*&rows=100" + refute_output --partial "Content-Encoding:" +} + +@test "server compresses response when Accept-Encoding: gzip is requested" { + run curl -s -D - -o /dev/null -H "Accept-Encoding: gzip" "http://localhost:${SOLR_PORT}/solr/films/select?q=*:*&rows=100" + assert_output --partial "gzip" +} + +@test "compressed response can be decompressed and parsed" { + run curl -s --compressed "http://localhost:${SOLR_PORT}/solr/films/select?q=*:*&rows=100" + assert_output --partial '"status":0' +} diff --git a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java index 726c736fda76..e72ffaa213cd 100644 --- a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java @@ -37,6 +37,7 @@ import java.util.Objects; import java.util.Properties; import java.util.Random; +import java.util.SequencedMap; import java.util.Set; import java.util.SortedMap; import java.util.concurrent.ConcurrentHashMap; @@ -439,7 +440,7 @@ public SortedMap getExtraServlets() { * Override this method to insert extra filters into the JettySolrRunners that are created using * createJetty() */ - public SortedMap, String> getExtraRequestFilters() { + public SequencedMap, String> getExtraRequestFilters() { return null; } diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 1c6405675ee8..4e96785a4a97 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -26,9 +26,6 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.UnavailableException; -import jakarta.servlet.http.HttpServlet; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintStream; import java.lang.invoke.MethodHandles; @@ -38,10 +35,12 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Properties; import java.util.Random; import java.util.Set; @@ -74,6 +73,7 @@ import org.apache.solr.util.configuration.SSLConfigurationsFactory; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; import org.eclipse.jetty.ee10.servlet.FilterHolder; +import org.eclipse.jetty.ee10.servlet.FilterMapping; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.ee10.servlet.ServletHolder; import org.eclipse.jetty.ee10.servlet.Source; @@ -91,7 +91,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.GracefulHandler; -import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.session.DefaultSessionIdManager; import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -116,12 +115,7 @@ public class JettySolrRunner implements SolrBackend { private Server server; - volatile FilterHolder debugFilter; - volatile FilterHolder requiredFilter; - volatile FilterHolder rateLimitFilter; - volatile FilterHolder authFilter; - volatile ServletHolder solrServlet; - private FilterHolder tracingFilter; + private volatile ServletHolder solrServlet; private int jettyPort = -1; @@ -131,8 +125,6 @@ public class JettySolrRunner implements SolrBackend { private volatile boolean startedBefore = false; - private List extraFilters; - private int proxyPort = -1; private final boolean enableProxy; @@ -371,7 +363,7 @@ private void init(int port) { { // Initialize the servlets final ServletContextHandler root = - new ServletContextHandler("/solr", ServletContextHandler.SESSIONS); + new ServletContextHandler("/solr", ServletContextHandler.NO_SESSIONS); root.setServer(server); root.setBaseResource(ResourceFactory.of(server).newResource(".")); root.addEventListener( @@ -397,11 +389,8 @@ public void contextInitialized(ServletContextEvent event) { } }); - debugFilter = root.addFilter(DebugFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); - extraFilters = new ArrayList<>(); for (Map.Entry, String> entry : config.extraFilters.entrySet()) { - extraFilters.add( - root.addFilter(entry.getKey(), entry.getValue(), EnumSet.of(DispatcherType.REQUEST))); + root.addFilter(entry.getKey(), entry.getValue(), EnumSet.of(DispatcherType.REQUEST)); } for (Map.Entry entry : config.extraServlets.entrySet()) { @@ -410,43 +399,29 @@ public void contextInitialized(ServletContextEvent event) { // TODO: This needs to be driven by a parsing of web.xml eventually // though we still want to avoid classpath scanning. - // required request setup - requiredFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); - requiredFilter.setHeldClass(RequiredSolrRequestFilter.class); - - // Ratelimit Requests - rateLimitFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); - rateLimitFilter.setHeldClass(RateLimitFilter.class); - - // Trace Requests - tracingFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); - tracingFilter.setHeldClass(TracingFilter.class); - - // Authenticate Requests - authFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); - authFilter.setHeldClass(AuthenticationFilter.class); - - // Map filters in same path as in web.xml - root.addFilter(requiredFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); - root.addFilter(rateLimitFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); - root.addFilter(tracingFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); - root.addFilter(authFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); - // This is our main workhorse - now a servlet instead of filter solrServlet = root.getServletHandler().newServletHolder(Source.EMBEDDED); + solrServlet.setName("SolrServlet"); solrServlet.setHeldClass(SolrServlet.class); root.addServlet(solrServlet, "/*"); - // Default servlet as a fall-through - ServletHolder defaultHolder = root.getServletHandler().newServletHolder(Source.EMBEDDED); - - // considered adding DefaultServlet.class here but perhaps that might grant our unit tests - // the power to serve static resources on the build machines? Not sure, so I'll just give a - // name to our existing hack. The tests passed without this, but it will ensure that if anyone - // ever hits the PathExcludeFilter in the unit test they get a 404 as before not a 500 - defaultHolder.setHeldClass(Servlet404.class); - defaultHolder.setName("default"); - root.addServlet(defaultHolder, "/"); + // Map filters to SolrServlet by name (same order as web.xml) + for (var filterClass : + List.>of( + RequiredSolrRequestFilter.class, + RateLimitFilter.class, + TracingFilter.class, + AuthenticationFilter.class)) { + FilterHolder fh = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + fh.setName(filterClass.getSimpleName()); + fh.setHeldClass(filterClass); + root.getServletHandler().addFilter(fh); + FilterMapping fm = new FilterMapping(); + fm.setFilterName(fh.getName()); + fm.setServletNames(new String[] {"SolrServlet"}); + fm.setDispatcherTypes(EnumSet.of(DispatcherType.REQUEST)); + root.getServletHandler().addFilterMapping(fm); + } // TODO: end area that should be driven by web.xml and webdefault.xml chain = root; @@ -462,13 +437,7 @@ public void contextInitialized(ServletContextEvent event) { chain = rwh; } - GzipHandler gzipHandler = new GzipHandler(); - gzipHandler.setHandler(chain); - - gzipHandler.setMinGzipSize(23); // https://github.com/eclipse/jetty.project/issues/4191 - gzipHandler.setIncludedMethods("GET"); - - server.setHandler(gzipHandler); + server.setHandler(chain); // Mimic "graceful.mod" GracefulHandler graceful = new GracefulHandler(); @@ -485,10 +454,15 @@ protected Handler.Wrapper injectJettyHandlers(Handler.Wrapper chain) { } /** - * @return the {@link RateLimitFilter} for this node + * @return the first filter implemented by the specified class, or throws an exception */ - public RateLimitFilter getSolrRateLimitFilter() { - return (RateLimitFilter) rateLimitFilter.getFilter(); + public T getFilter(Class filterClass) { + return Arrays.stream(solrServlet.getServletHandler().getFilters()) + .filter(fh -> fh.getHeldClass() == filterClass) + .map(fh -> filterClass.cast(fh.getFilter())) + .findFirst() + .orElseThrow( + () -> new NoSuchElementException("No filter of class: " + filterClass.getName())); } @Override @@ -848,21 +822,9 @@ public SolrClient newClient(int connectionTimeoutMillis, int socketTimeoutMillis .build(); } - public DebugFilter getDebugFilter() { - return (DebugFilter) debugFilter.getFilter(); - } - // -------------------------------------------------------------- // -------------------------------------------------------------- - /** This is a stupid hack to give jetty something to attach to */ - public static class Servlet404 extends HttpServlet { - @Override - public void service(HttpServletRequest req, HttpServletResponse res) throws IOException { - res.sendError(404, "Can not find: " + req.getRequestURI()); - } - } - /** A main class that starts jetty+solr This is useful for debugging */ public static void main(String[] args) throws Exception { JettySolrRunner jetty = new JettySolrRunner(".", 8983); diff --git a/solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java b/solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java index c7367bab551c..d016274be717 100644 --- a/solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java +++ b/solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.lang.invoke.MethodHandles; import java.net.URISyntaxException; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; @@ -32,14 +31,10 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.http.Header; -import org.apache.http.HttpEntity; import org.apache.http.HttpException; import org.apache.http.HttpRequest; import org.apache.http.HttpRequestInterceptor; -import org.apache.http.HttpResponse; import org.apache.http.client.CookieStore; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpRequestWrapper; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.client.utils.URIBuilder; @@ -61,13 +56,11 @@ import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.client.solrj.request.XMLRequestWriter; import org.apache.solr.client.solrj.response.JavaBinResponseParser; -import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.XMLResponseParser; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.EnvUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.embedded.JettyConfig; @@ -458,73 +451,6 @@ public void testRedirect() throws Exception { } } - @Test - public void testCompression() throws Exception { - final String debugPath = "/debug/foo"; - final SolrQuery q = new SolrQuery("*:*"); - final var queryRequest = new QueryRequest(q); - queryRequest.setPath(debugPath + queryRequest.getPath()); - - try (SolrClient client = getHttpSolrClient(solrTestRule.getBaseUrl())) { - // verify request header gets set - DebugServlet.clear(); - expectThrows(RemoteSolrException.class, () -> queryRequest.process(client)); - assertNull(DebugServlet.headers.toString(), DebugServlet.headers.get("accept-encoding")); - } - - try (SolrClient client = - new HttpSolrClient.Builder(solrTestRule.getBaseUrl()).allowCompression(true).build()) { - try { - queryRequest.process(client); - } catch (RemoteSolrException ignored) { - } - assertNotNull(DebugServlet.headers.get("accept-encoding")); - } - - try (SolrClient client = - new HttpSolrClient.Builder(solrTestRule.getBaseUrl()).allowCompression(false).build()) { - try { - queryRequest.process(client); - } catch (RemoteSolrException ignored) { - } - } - assertNull(DebugServlet.headers.get("accept-encoding")); - - // verify server compresses output - HttpGet get = - new HttpGet( - solrTestRule.getBaseUrl() + "/" + DEFAULT_TEST_CORENAME + "/select?q=foo&wt=xml"); - get.setHeader("Accept-Encoding", "gzip"); - ModifiableSolrParams params = new ModifiableSolrParams(); - params.set(HttpClientUtil.PROP_ALLOW_COMPRESSION, true); - - RequestConfig config = RequestConfig.custom().setDecompressionEnabled(false).build(); - get.setConfig(config); - - CloseableHttpClient httpclient = HttpClientUtil.createClient(params); - HttpEntity entity = null; - try { - HttpResponse response = - httpclient.execute(get, HttpClientUtil.createNewHttpClientRequestContext()); - entity = response.getEntity(); - Header ceheader = entity.getContentEncoding(); - assertNotNull(Arrays.asList(response.getAllHeaders()).toString(), ceheader); - assertEquals("gzip", ceheader.getValue()); - } finally { - if (entity != null) { - entity.getContent().close(); - } - HttpClientUtil.close(httpclient); - } - - // verify compressed response can be handled - try (SolrClient client = - getHttpSolrClient(solrTestRule.getBaseUrl(), DEFAULT_TEST_COLLECTION_NAME)) { - QueryResponse response = client.query(new SolrQuery("foo")); - assertEquals(0, response.getStatus()); - } - } - @Test public void testCollectionParameters() throws IOException, SolrServerException {