lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a.@apache.org
Subject [lucene-solr] branch branch_8x updated: SOLR-13583: Return 400 Bad Request instead of 500 Server Error when a complex alias is found but a simple alias was expected.
Date Fri, 05 Jul 2019 08:14:42 GMT
This is an automated email from the ASF dual-hosted git repository.

ab pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new e616ed4  SOLR-13583: Return 400 Bad Request instead of 500 Server Error when a complex
alias is found but a simple alias was expected.
e616ed4 is described below

commit e616ed49a6688c011e4854afc509dd13a7222b6f
Author: Andrzej Bialecki <ab@apache.org>
AuthorDate: Fri Jul 5 09:17:57 2019 +0200

    SOLR-13583: Return 400 Bad Request instead of 500 Server Error when a complex
    alias is found but a simple alias was expected.
---
 .../cloud/api/collections/DeleteCollectionCmd.java |  5 ++--
 .../apache/solr/cloud/AliasIntegrationTest.java    | 31 ++++++++++++++++++++--
 .../java/org/apache/solr/common/cloud/Aliases.java | 25 ++++++++---------
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
index 98c7ca3..6fe02ee 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
+++ b/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,
-                "Collection : " + extCollection + " is part of aliases: " + aliasesRefs +
", remove or modify the aliases before removing this collection.");
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+                "Collection : " + collection + " is part of aliases: " + aliasesRefs + ",
remove or modify the aliases before removing this collection.");
           } else {
             aliasesToDelete.add(alias);
           }
diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
index c0d63ba..60d2e3b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/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);
+
+    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);
 
-    // Now redefine the alias to only point to colletion two
+    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);
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
index f1fa3f1..3ba61b7 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
+++ b/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()));


Mime
View raw message