SOLR-18332: More 'qt' removal, rd 6 - #4773
Open
gerlowskija wants to merge 5 commits into
Open
Conversation
CacheHeaderTest/CacheHeaderTestBase built raw /select URLs with a vestigial qt=standard param that's never read (HttpSolrCall dispatches by URL path alone), so it's dropped outright. SolrRequestParserTest used "qt" purely as an arbitrary param name to test percent-decoding, unrelated to handler dispatch, so it's renamed to "misc"/"misc2".
checkQueries() switches to the explicit-handler query(String, SolrParams) overload instead of embedding qt in the varargs. getIndexVersion(Replica) and logReplicationDetails(Replica, StringBuilder) build a QueryRequest with an explicit path instead of relying on SolrClient.query(SolrParams)'s client-side qt-to-path conversion. Also closes out the ShardRoutingTest.java doRTG() call site deferred from the earlier BaseDistributedSearchTestCase.query(...) cleanup, via a new doQuery(List<String>, String, String...) overload that takes the expected doc ids as a List<String> instead of a comma-delimited String.
TestJavaBinResponseWriter, TestSubQueryTransformer, EnumFieldTest, and
TestGroupingSearch read qt off a request that never had it set (built
via req(...)/lrf.makeRequest(...) with no qt param), then passed the
always-null result to h.queryAndResponse(...)/core.execute(...). Same
shape in BasicFunctionalityTest's 3 remaining call sites. Replaced with
literal null, a behavior-preserving simplification since the value was
always null anyway.
ConvertedLegacyTest is the one real case: makeParams(...) embedded qt
in a SolrParams fed directly into `new SolrQueryRequestBase(core, params)`,
which never sets getPath(), so assertQ's handler resolution fell back to
reading qt. Every caller passed "/select", so qtype is dropped from
makeParams entirely and call sites switch to reqWithPath("/select", ...).
Clarify that a null handlerName doesn't just return null - it resolves to the core's default request handler via the empty-string alias RequestHandlers registers at startup.
TestTlogReplayVsRecovery already dispatched via an explicit path on its GenericSolrRequest, so the "qt","/get" pair alongside it was pure vestige. DistribCursorPagingTest.assertFullWalkNoDups read qt off its params (with a "/select" default) to pick a request handler for QueryRequest. Mirrors the fix already applied to the analogous non-distributed CursorPagingTest: add an explicit-handler overload, have assertFullWalkNoDupsElevated call it with "/elevate" instead of embedding qt in params, and have the no-handler overload default to "/select" explicitly (QueryRequest's 2-arg constructor requires a non-null path, unlike GenericSolrRequest).
github-actions
Bot
added
test-framework
jetty-server
tests
cat:search
cat:cloud
cat:schema
labels
gerlowskija
added
no-changelog
and removed
test-framework
jetty-server
tests
cat:search
cat:cloud
cat:schema
labels
dsmiley
reviewed
Aug 20, 2026
| * Get the request handler registered to a given name. | ||
| * | ||
| * <p>A {@code null} handlerName resolves to the core's default request handler (whichever handler | ||
| * is aliased to the empty string, normally the one registered at "/select", or "standard" for |
Contributor
There was a problem hiding this comment.
@epugh I recall you killed the "standard" pattern
| assertEquals(expectedIds, obtainedIds); | ||
| } | ||
|
|
||
| void doQuery(List<String> expectedIds, String path, String... queryParams) throws Exception { |
Contributor
There was a problem hiding this comment.
I very much disklike the param structure. Can you please require a QueryRequest?
| params.set("qt", "/replication"); | ||
| params.set(ReplicationHandler.COMMAND, ReplicationHandler.CMD_SHOW_COMMITS); | ||
| try { | ||
| QueryResponse response = client.query(params); |
Contributor
There was a problem hiding this comment.
another one bites the dust; thanks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 'qt' parameter and several related methods in SolrJ are deprecated.
This deprecation may not stick, but it's still worth minimizing use of this
feature as much as possible.
Many tests rely on it unnecessarily; this PR is one in a number of batches
slowly removing these test usages. Unlike previous batches, the removals
in this PR are a bit more of a catch-all. A number of different patterns are
addressed here. (They've been broken up into separate commits, for the
purpose of making review easier.)