Skip to content

SOLR-18168: Simplify JettySolrRunner#4262

Open
dsmiley wants to merge 3 commits intoapache:mainfrom
dsmiley:SimplifyJettySolrRunner2
Open

SOLR-18168: Simplify JettySolrRunner#4262
dsmiley wants to merge 3 commits intoapache:mainfrom
dsmiley:SimplifyJettySolrRunner2

Conversation

@dsmiley
Copy link
Copy Markdown
Contributor

@dsmiley dsmiley commented Apr 3, 2026

(1) 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.

(2) 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.

(3) remove Servlet404.

https://issues.apache.org/jira/browse/SOLR-18168 -- not totally same but follow-on

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.
Copy link
Copy Markdown
Contributor Author

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will merge ~Friday


long getNumRequests() {
long n = controlJetty.getDebugFilter().getTotalRequests();
long n = controlJetty.getFilter(JettySolrRunner.DebugFilter.class).getTotalRequests();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO DebugFilter should be evicted from JettySolrRunner... perhaps moving to inside ServletFixtures -- WDYT?


@Override
public SequencedMap<Class<? extends Filter>, String> getExtraRequestFilters() {
return new LinkedHashMap<>(Map.of(JettySolrRunner.DebugFilter.class, "/*"));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-longer auto-registered. If you want it, you register it. Interestingly, nobody was overriding this before (I think). Moving to SequencedMap makes this easier to construct and more sensible than a sorted map.

// Initialize the servlets
final ServletContextHandler root =
new ServletContextHandler("/solr", ServletContextHandler.SESSIONS);
new ServletContextHandler("/solr", ServletContextHandler.NO_SESSIONS);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solr doesn't use sessions.

for (var filterClass :
List.<Class<? extends Filter>>of(
RequiredSolrRequestFilter.class,
RateLimitFilter.class,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be interesting to make the non-required filters opt-in. A test using them would register an extra filter (as they would do for DebugFilter), and the code here would pluck it out of the extra filter list if it's there. Any way, I'm not in the mood to pursue.

dsmiley added 2 commits April 10, 2026 17:57
Added javadocs.
Delay is now a record.
# Conflicts:
#	solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java
@dsmiley
Copy link
Copy Markdown
Contributor Author

dsmiley commented Apr 10, 2026

Given that the inner Debug Filter is only for "delay", I renamed it DelayFilter and moved it to ServletFixtures where it's in good company with other Servlet test utilities. Also made the inner class Delay a record and added some javadocs to DelayFilter generally.

JettySolrRunner is now more focused than ever.

Will merge Sunday without further input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant