You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2019/07/05 07:21:33 UTC
[lucene-solr] branch master updated: SOLR-13583: Return 400 Bad
Request instead of 500 Server Error when a complex alias is found but a
simple alias was expected.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new dd4813d SOLR-13583: Return 400 Bad Request instead of 500 Server Error when a complex alias is found but a simple alias was expected.
dd4813d is described below
commit dd4813d5b82d7e983a3541be54cb9b0e04f246ce
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()));