dsmiley commented on code in PR #2455:
URL: https://github.com/apache/solr/pull/2455#discussion_r1621361826


##########
solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java:
##########
@@ -56,6 +63,51 @@ public static Collection<ContentStream> toContentStreams(
     return streams;
   }
 
+  /**
+   * Create the full URL for a SolrRequest (excepting query parameters) as a 
String
+   *
+   * @param solrRequest the {@link SolrRequest} to build the URL for
+   * @param requestWriter a {@link RequestWriter} from the {@link SolrClient} 
that will be sending
+   *     the request
+   * @param serverRootUrl the root URL of the Solr server being targeted. May 
by overridden {@link
+   *     SolrRequest#getBasePath()}, if present.
+   * @param collection the collection to send the request to. May be null if 
no collection is
+   *     needed.
+   * @throws MalformedURLException if {@code serverRootUrl} or {@link 
SolrRequest#getBasePath()}
+   *     contain a malformed URL string
+   */
+  public static String buildRequestUrl(
+      SolrRequest<?> solrRequest,
+      RequestWriter requestWriter,
+      String serverRootUrl,
+      String collection)
+      throws MalformedURLException {
+    String basePath = solrRequest.getBasePath() == null ? serverRootUrl : 
solrRequest.getBasePath();
+
+    if (solrRequest instanceof V2Request) {
+      if (System.getProperty("solr.v2RealPath") == null) {
+        basePath = changeV2RequestEndpoint(basePath);
+      } else {
+        basePath = serverRootUrl + "/____v2";
+      }
+    }
+
+    if (solrRequest.requiresCollection() && collection != null) basePath += 
"/" + collection;
+
+    String path = requestWriter.getPath(solrRequest);

Review Comment:
   This part feels bizarre; what business does the request writer have in the 
path construction?!  It turns out it merely calls SolrRequest.getPath -- okay.  
Only *one* place does something different happen, which is only in a test.  I 
think we should remove this API oddity.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java:
##########
@@ -56,6 +63,51 @@ public static Collection<ContentStream> toContentStreams(
     return streams;
   }
 
+  /**
+   * Create the full URL for a SolrRequest (excepting query parameters) as a 
String
+   *
+   * @param solrRequest the {@link SolrRequest} to build the URL for
+   * @param requestWriter a {@link RequestWriter} from the {@link SolrClient} 
that will be sending
+   *     the request
+   * @param serverRootUrl the root URL of the Solr server being targeted. May 
by overridden {@link
+   *     SolrRequest#getBasePath()}, if present.
+   * @param collection the collection to send the request to. May be null if 
no collection is
+   *     needed.
+   * @throws MalformedURLException if {@code serverRootUrl} or {@link 
SolrRequest#getBasePath()}
+   *     contain a malformed URL string
+   */
+  public static String buildRequestUrl(
+      SolrRequest<?> solrRequest,
+      RequestWriter requestWriter,
+      String serverRootUrl,
+      String collection)
+      throws MalformedURLException {
+    String basePath = solrRequest.getBasePath() == null ? serverRootUrl : 
solrRequest.getBasePath();
+
+    if (solrRequest instanceof V2Request) {
+      if (System.getProperty("solr.v2RealPath") == null) {
+        basePath = changeV2RequestEndpoint(basePath);
+      } else {
+        basePath = serverRootUrl + "/____v2";

Review Comment:
   suspicious to see solrRequest.getBasePath() is ignored in this scenario.  
Hmm.  I don't know enough about V2 path construction but it looks complicated 
:-(



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