Skip to content

SOLR-17742: remove handleSelect requestDispatcher option #3409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

wildtusker
Copy link
Contributor

…nd HttpSolrCall.java

https://issues.apache.org/jira/browse/SOLR-17742

Description

handleSelect is removed from solrconfig.xml .The relevant code has been removed

Solution

  1. Searched handleSelect in codebase
  2. Relevant files came up.The primary file was SolrConfig.java .
  3. SolrConfig.java showed the usage of handleSelect in other files
  4. Removed the code
  5. Ran the tests and refactored the relevant code in iterative manner
  6. Finally all the test cases passed

Tests

All the tests passed

Checklist

@dsmiley dsmiley self-requested a review June 28, 2025 15:30
Copy link
Contributor

@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.

Don't comment out the code we want to go away. Remove it!

I think you would benefit from a self-review of the PR. Meaning, review the diff in GitHub, and then add comments for anything not clear.

@@ -128,7 +128,7 @@ public enum PluginOpts {

private int formUploadLimitKB;

private boolean handleSelect;
// private boolean handleSelect;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this outright; don't leave commented

@@ -363,7 +363,7 @@ private SolrConfig(SolrResourceLoader loader, String name, Properties substituta
log.warn("Ignored deprecated enableStreamBody in config; use sys-prop");
}

handleSelect = get("requestDispatcher").boolAttr("handleSelect", false);
// handleSelect = get("requestDispatcher").boolAttr("handleSelect", false);
Copy link
Contributor

Choose a reason for hiding this comment

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

again

return handleSelect;
}

/** public boolean isHandleSelect() { return handleSelect; } */
Copy link
Contributor

Choose a reason for hiding this comment

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

uh?

@@ -1010,7 +1007,7 @@ public Map<String, Object> toMap(Map<String, Object> result) {
m, filterCacheConfig, queryResultCacheConfig, documentCacheConfig, fieldValueCacheConfig);
m = new LinkedHashMap<>();
result.put("requestDispatcher", m);
m.put("handleSelect", handleSelect);
// m.put("handleSelect", handleSelect);
Copy link
Contributor

Choose a reason for hiding this comment

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

again

@@ -454,25 +453,18 @@ protected void extractHandlerFromURLPath(SolrRequestParsers parser) throws Excep
if (handler == null && path.length() > 1) { // don't match "" or "/" as valid path
handler = core.getRequestHandler(path);
// no handler yet but <requestDispatcher> allows us to handle /select with a 'qt' param
Copy link
Contributor

Choose a reason for hiding this comment

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

and this comment is now obsolete/false

Copy link
Contributor

Choose a reason for hiding this comment

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

still should remove this comment

@dsmiley
Copy link
Contributor

dsmiley commented Jun 28, 2025

Precommit failed. Looking at the logs, this is why:

> Task :solr:solrj:validateLogCalls
/home/runner/work/solr/solr/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:456: warning: [NotJavadoc] Avoid using `/**` for comments which aren't actually Javadoc.

      /**
> Task :solr:core:compileJava
      ^
    (see https://errorprone.info/bugpattern/NotJavadoc)
  Did you mean '/*'?
error: warnings found and -Werror specified
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
1 warning

Copy link
Contributor

@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.

Needs a CHANGES.txt entry under 10 in "Deprecated Removals"

@@ -454,25 +453,18 @@ protected void extractHandlerFromURLPath(SolrRequestParsers parser) throws Excep
if (handler == null && path.length() > 1) { // don't match "" or "/" as valid path
handler = core.getRequestHandler(path);
// no handler yet but <requestDispatcher> allows us to handle /select with a 'qt' param
Copy link
Contributor

Choose a reason for hiding this comment

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

still should remove this comment

@dsmiley
Copy link
Contributor

dsmiley commented Jul 1, 2025

Please search for "handleSelect" more thoroughly. Your changes here are good but incomplete.

@cpoerschke cpoerschke changed the title SOLR-17742 SOLR-17742: remove handleSelect requestDispatcher option Jul 11, 2025
@cpoerschke
Copy link
Contributor

Wondering if ref-guide references could be removed too: https://github.com/search?q=repo%3Aapache%2Fsolr+handleSelect+language%3AAsciiDoc&type=code&l=AsciiDoc

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.

3 participants