jkmuriithi commented on code in PR #3238:
URL: https://github.com/apache/solr/pull/3238#discussion_r1999136119


##########
solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java:
##########
@@ -185,8 +193,21 @@ public void setQueryParams(Set<String> queryParams) {
     this.queryParams = queryParams;
   }
 
-  /** This method defines the type of this Solr request. */
-  public abstract String getRequestType();
+  /**
+   * The type of this Solr request.
+   *
+   * <p>Pattern matches {@link SolrRequest#getPath} to identify ADMIN requests 
and other special
+   * cases. Overriding this method may affect request routing within various 
clients (i.e. {@link
+   * CloudSolrClient}).
+   */
+  public SolrRequestType getRequestType() {
+    String path = getPath();
+    if (path != null && CommonParams.ADMIN_PATHS.contains(path)) {

Review Comment:
   The reason this is necessary at the moment is because of this pattern, which 
is present in several unit tests across the codebase (example from 
[TestReplicationHandler](https://github.com/apache/solr/blob/af97ef7037aec0c1fb028e173b6d8931cd12588d/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java#L221-L225)):
   ```java
   ModifiableSolrParams params = new ModifiableSolrParams();
   params.set("action", "reload");
   params.set("core", core);
   params.set("qt", "/admin/cores");
   QueryRequest req = new QueryRequest(params);
   ```
   Doing pattern matching on `getPath` allows this type of path manipulation to 
"just work" with `CloudSolrClient`, at the cost of introducing more complexity 
into `SolrRequest`. I considered going through and changing each instance of 
this pattern across all unit tests, but I felt like it might expand the scope 
of the PR a bit too much.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to