SOLR-13583: Return 400 Bad Request instead of 500 Server Error when a complex

alias is found but a simple alias was expected.

solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java

@@ -202,13 +202,14 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
       zkStateReader.aliasesManager.update(); // aliases may have been stale; get latest from ZK
       aliases = zkStateReader.getAliases();
       aliasesRefs = referencedByAlias(extCollection, aliases, followAliases);
+      String collection = followAliases ? aliases.resolveSimpleAlias(extCollection) : extCollection;
       if (aliasesRefs.size() > 0) {
         for (String alias : aliasesRefs) {
           // for back-compat in 8.x we don't automatically remove other
           // aliases that point only to this collection
           if (!extCollection.equals(alias)) {
-            throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-                "Collection : " + extCollection + " is part of aliases: " + aliasesRefs + ", remove or modify the aliases before removing this collection.");
+                "Collection : " + collection + " is part of aliases: " + aliasesRefs + ", remove or modify the aliases before removing this collection.");
           } else {
             aliasesToDelete.add(alias);
           }

solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java

@@ -317,7 +317,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
     try {
       String resolved = stateProvider.resolveSimpleAlias(aliasName);
       fail("this is not a simple alias but it resolved to " + resolved);
-    } catch (IllegalArgumentException e) {
+    } catch (SolrException e) {
       // expected
     }
   }
@@ -517,10 +517,37 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
 
     // Now delete one of the collections, should fail since an alias points to it.
     RequestStatusState delResp = CollectionAdminRequest.deleteCollection("collection_one").processAndWait(cluster.getSolrClient(), 60);
-
+    // failed because the collection is a part of a compound alias
     assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
 
-    // Now redefine the alias to only point to colletion two
+    CollectionAdminRequest.Delete delete = CollectionAdminRequest.deleteCollection("collection_alias_pair");
+    delResp = delete.processAndWait(cluster.getSolrClient(), 60);
+    // failed because we tried to delete an alias with followAliases=false
+    assertEquals("Should have failed to delete alias: ", delResp, RequestStatusState.FAILED);
+
+    delete.setFollowAliases(true);
+    delResp = delete.processAndWait(cluster.getSolrClient(), 60);
+    // failed because we tried to delete compound alias
+    assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
+
+    CollectionAdminRequest.createAlias("collection_alias_one", "collection_one").process(cluster.getSolrClient());
+    lastVersion = waitForAliasesUpdate(lastVersion, zkStateReader);
+
+    delete = CollectionAdminRequest.deleteCollection("collection_one");
+    delResp = delete.processAndWait(cluster.getSolrClient(), 60);
+    // failed because we tried to delete collection referenced by multiple aliases
+    assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
+
+    delete = CollectionAdminRequest.deleteCollection("collection_alias_one");
+    delete.setFollowAliases(true);
+    delResp = delete.processAndWait(cluster.getSolrClient(), 60);
+    // failed because we tried to delete collection referenced by multiple aliases
+    assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
+
+    CollectionAdminRequest.deleteAlias("collection_alias_one").process(cluster.getSolrClient());
+    lastVersion = waitForAliasesUpdate(lastVersion, zkStateReader);
+
+    // Now redefine the alias to only point to collection two
     CollectionAdminRequest.createAlias("collection_alias_pair", "collection_two").process(cluster.getSolrClient());
     lastVersion = waitForAliasesUpdate(lastVersion, zkStateReader);
 

solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java

@@ -27,6 +27,7 @@ import java.util.Objects;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
@@ -207,29 +208,29 @@ public class Aliases {
    * @param aliasName alias name
    * @return original name if there's no such alias, or a resolved name. If an alias points to more than 1
    * collection (directly or indirectly) an exception is thrown
-   * @throws IllegalArgumentException if either direct or indirect alias points to more than 1 name.
+   * @throws SolrException if either direct or indirect alias points to more than 1 name.
    */
-  public String resolveSimpleAlias(String aliasName) throws IllegalArgumentException {
+  public String resolveSimpleAlias(String aliasName) throws SolrException {
     return resolveSimpleAliasGivenAliasMap(collectionAliases, aliasName);
   }
 
   /** @lucene.internal */
   @SuppressWarnings("JavaDoc")
   public static String resolveSimpleAliasGivenAliasMap(Map<String, List<String>> collectionAliasListMap,
-                                                       String aliasName) throws IllegalArgumentException {
+                                                       String aliasName) throws SolrException {
     List<String> level1 = collectionAliasListMap.get(aliasName);
     if (level1 == null || level1.isEmpty()) {
       return aliasName; // simple collection name
     }
     if (level1.size() > 1) {
-      throw new IllegalArgumentException("Simple alias '" + aliasName + "' points to more than 1 collection: " + level1);
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Simple alias '" + aliasName + "' points to more than 1 collection: " + level1);
     }
     List<String> level2 = collectionAliasListMap.get(level1.get(0));
     if (level2 == null || level2.isEmpty()) {
       return level1.get(0); // simple alias
     }
     if (level2.size() > 1) {
-      throw new IllegalArgumentException("Simple alias '" + aliasName + "' resolves to '"
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Simple alias '" + aliasName + "' resolves to '"
           + level1.get(0) + "' which points to more than 1 collection: " + level2);
     }
     return level2.get(0);
@@ -318,10 +319,10 @@ public class Aliases {
    * @param after new alias name. If this is null then it's equivalent to calling {@link #cloneWithCollectionAlias(String, String)}
    *              with the second argument set to null, ie. removing an alias.
    * @return new instance with the renamed alias
-   * @throws IllegalArgumentException when either <code>before</code> or <code>after</code> is empty, or
+   * @throws SolrException when either <code>before</code> or <code>after</code> is empty, or
    * the <code>before</code> name is a routed alias
    */
-  public Aliases cloneWithRename(String before, String after) {
+  public Aliases cloneWithRename(String before, String after) throws SolrException {
     if (before == null) {
       throw new NullPointerException("'before' and 'after' cannot be null");
     }
@@ -329,7 +330,7 @@ public class Aliases {
       return cloneWithCollectionAlias(before, after);
     }
     if (before.isEmpty() || after.isEmpty()) {
-      throw new IllegalArgumentException("'before' and 'after' cannot be empty");
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "'before' and 'after' cannot be empty");
     }
     if (before.equals(after)) {
       return this;
@@ -337,7 +338,7 @@ public class Aliases {
     Map<String, String> props = collectionAliasProperties.get(before);
     if (props != null) {
       if (props.keySet().stream().anyMatch(k -> k.startsWith(CollectionAdminParams.ROUTER_PREFIX))) {
-        throw new IllegalArgumentException("source name '" + before + "' is a routed alias.");
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "source name '" + before + "' is a routed alias.");
       }
     }
     Map<String, Map<String, String>> newColProperties = new LinkedHashMap<>(this.collectionAliasProperties);
@@ -397,12 +398,12 @@ public class Aliases {
    * @param properties the properties to add/replace, null values in the map will remove the key.
    * @return An immutable copy of the aliases with the new properties.
    */
-  public Aliases cloneWithCollectionAliasProperties(String alias, Map<String,String> properties) {
+  public Aliases cloneWithCollectionAliasProperties(String alias, Map<String,String> properties) throws SolrException {
     if (!collectionAliases.containsKey(alias)) {
-      throw new IllegalArgumentException(alias + " is not a valid alias");
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, alias + " is not a valid alias");
     }
     if (properties == null) {
-      throw new IllegalArgumentException("Null is not a valid properties map");
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Null is not a valid properties map");
     }
     Map<String,Map<String,String>> newColProperties = new LinkedHashMap<>(this.collectionAliasProperties);//clone to modify
     Map<String, String> newMetaMap = new LinkedHashMap<>(newColProperties.getOrDefault(alias, Collections.emptyMap()));