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


##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -1066,13 +1066,14 @@ private Map<String, Integer> 
checkStateVersionsAreValid(String stateVer) {
 
   protected SolrCore getCoreByCollection(String collectionName, boolean 
isPreferLeader) {
     ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
-
     ClusterState clusterState = zkStateReader.getClusterState();
-    DocCollection collection = 
clusterState.getCollectionOrNull(collectionName, true);
+    DocCollection collection = resolveDocCollection(collectionName);
+    // the usage of getCoreByCollection assumes that if null is returned, 
collection is found, but
+    // replicas might not
+    // have been created. Hence returning null here would be misleading...

Review Comment:
   Seems like you could write javadocs for getCoreByCollection additionally or 
instead



##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -354,13 +352,15 @@ protected void init() throws Exception {
    * recursively try this all one more time while ensuring the alias and 
collection info is sync'ed
    * from ZK.
    */
-  protected DocCollection resolveDocCollection(List<String> collectionsList) {
+  protected DocCollection resolveDocCollection(String collectionName) {

Review Comment:
   Its javadocs appear outdated now.
   
   I like the signature change.



##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -354,13 +352,15 @@ protected void init() throws Exception {
    * recursively try this all one more time while ensuring the alias and 
collection info is sync'ed
    * from ZK.
    */
-  protected DocCollection resolveDocCollection(List<String> collectionsList) {
+  protected DocCollection resolveDocCollection(String collectionName) {
     if (!cores.isZooKeeperAware()) {
       throw new SolrException(
           SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode 
");
     }
+    if (collectionName == null || collectionName.trim().isEmpty()) {

Review Comment:
   ```suggestion
       if (collectionName == null || collectionName.isBlank()) {
   ```



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