Skip to content

Simplify SolrRequest path usage: prefer ctor not setPath - #4689

Merged
dsmiley merged 2 commits into
apache:mainfrom
dsmiley:setPathAvoidance
Aug 1, 2026
Merged

Simplify SolrRequest path usage: prefer ctor not setPath#4689
dsmiley merged 2 commits into
apache:mainfrom
dsmiley:setPathAvoidance

Conversation

@dsmiley

@dsmiley dsmiley commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Simplify qt/path handling in QueryRequest callers

QueryRequest's constructors already resolve the "qt" param into the
request path, so callers no longer need to manually extract/remove
qt and call setPath themselves.

Tests: SolrRequest path usage: prefer ctor not setPath

And don't use QueryRequest for non-SearchHandler.

dsmiley added 2 commits July 31, 2026 13:00
QueryRequest's constructors already resolve the "qt" param into the
request path, so callers no longer need to manually extract/remove
qt and call setPath themselves.
And don't use QueryRequest for non-SearchHandler.

@epugh epugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

}
QueryRequest request = new QueryRequest(params);
request.setPath("/admin/collections");
var request =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Normally when I see a GSR I think "We should be using one of our strongly typed OpenAPI classes" ;-). But in this case, I think it's better ;-).

throws Exception {
QueryRequest query = new QueryRequest(args);
query.setResponseParser(new SmileResponseParser());
String path = args.get("qt");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Love seeing another qt bite the dust.

@dsmiley
dsmiley merged commit b47aaca into apache:main Aug 1, 2026
4 of 6 checks passed
@dsmiley
dsmiley deleted the setPathAvoidance branch August 1, 2026 16:59
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.

2 participants