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 2016/12/20 10:48:43 UTC

[38/44] lucene-solr:jira/solr-9854: SOLR-9874: CREATEALIAS should fail if target collections don't exist

SOLR-9874: CREATEALIAS should fail if target collections don't exist


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/84bbb8f7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/84bbb8f7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/84bbb8f7

Branch: refs/heads/jira/solr-9854
Commit: 84bbb8f797db55f6cb203d87d150f2b3a2fe92cf
Parents: 4d81eee
Author: Tomas Fernandez Lobbe <tf...@apache.org>
Authored: Mon Dec 19 16:54:13 2016 -0800
Committer: Tomas Fernandez Lobbe <tf...@apache.org>
Committed: Mon Dec 19 16:54:13 2016 -0800

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../solr/SolrMorphlineZkAliasTest.java          |  7 ---
 .../org/apache/solr/cloud/CreateAliasCmd.java   | 21 ++++++--
 .../solr/handler/admin/CollectionsHandler.java  |  2 +-
 .../apache/solr/cloud/AliasIntegrationTest.java | 50 ++++++++++++++++----
 .../solr/client/solrj/io/sql/JdbcTest.java      |  7 +--
 .../client/solrj/io/stream/JDBCStreamTest.java  |  9 ++--
 .../solrj/io/stream/StreamExpressionTest.java   |  7 +--
 .../client/solrj/io/stream/StreamingTest.java   | 10 ++--
 9 files changed, 81 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 35a2bb7..d04d491 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -304,6 +304,8 @@ Other Changes
 
 * SOLR-8959: Refactored TestSegmentSorting out of TestMiniSolrCloudCluster (hossman)
 
+* SOLR-9874: Solr will reject CREATEALIAS requests if target collections don't exist (Tom�s Fern�ndez L�bbe)
+
 
 ==================  6.3.0 ==================
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/contrib/morphlines-core/src/test/org/apache/solr/morphlines/solr/SolrMorphlineZkAliasTest.java
----------------------------------------------------------------------
diff --git a/solr/contrib/morphlines-core/src/test/org/apache/solr/morphlines/solr/SolrMorphlineZkAliasTest.java b/solr/contrib/morphlines-core/src/test/org/apache/solr/morphlines/solr/SolrMorphlineZkAliasTest.java
index ddaf2f6..74c8824 100644
--- a/solr/contrib/morphlines-core/src/test/org/apache/solr/morphlines/solr/SolrMorphlineZkAliasTest.java
+++ b/solr/contrib/morphlines-core/src/test/org/apache/solr/morphlines/solr/SolrMorphlineZkAliasTest.java
@@ -91,13 +91,6 @@ public class SolrMorphlineZkAliasTest extends AbstractSolrMorphlineZkTestBase {
     Notifications.notifyRollbackTransaction(morphline);
     Notifications.notifyShutdown(morphline);
 
-    CollectionAdminRequest.createAlias("aliascollection", "collection1,collection2")
-        .processAndWait(cluster.getSolrClient(), TIMEOUT);
-
-    expectThrows(IllegalArgumentException.class, () -> {
-      parse("test-morphlines" + File.separator + "loadSolrBasic", "aliascollection");
-    });
-
   }
 
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java b/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java
index b966ebd..339d75a 100644
--- a/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java
@@ -17,8 +17,11 @@
  */
 package org.apache.solr.cloud;
 
+import static org.apache.solr.common.params.CommonParams.NAME;
+
 import java.lang.invoke.MethodHandles;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
@@ -35,8 +38,6 @@ import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.solr.common.params.CommonParams.NAME;
-
 
 public class CreateAliasCmd implements Cmd {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -52,10 +53,12 @@ public class CreateAliasCmd implements Cmd {
     String aliasName = message.getStr(NAME);
     String collections = message.getStr("collections");
 
-    Map<String, Map<String, String>> newAliasesMap = new HashMap<>();
-    Map<String, String> newCollectionAliasesMap = new HashMap<>();
     ZkStateReader zkStateReader = ocmh.zkStateReader;
     Map<String, String> prevColAliases = zkStateReader.getAliases().getCollectionAliasMap();
+    validateAllCollectionsExist(collections, prevColAliases, zkStateReader.getClusterState());
+
+    Map<String, Map<String, String>> newAliasesMap = new HashMap<>();
+    Map<String, String> newCollectionAliasesMap = new HashMap<>();
     if (prevColAliases != null) {
       newCollectionAliasesMap.putAll(prevColAliases);
     }
@@ -81,6 +84,16 @@ public class CreateAliasCmd implements Cmd {
     }
   }
 
+  private void validateAllCollectionsExist(String collections, Map<String,String> prevColAliases, ClusterState clusterState) {
+    String[] collectionArr = collections.split(",");
+    for (String collection:collectionArr) {
+      if (clusterState.getCollectionOrNull(collection) == null && (prevColAliases == null || !prevColAliases.containsKey(collection))) {
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+            String.format(Locale.ROOT,  "Can't create collection alias for collections='%s', '%s' is not an existing collection or alias", collections, collection));
+      }
+    }
+  }
+
   private void checkForAlias(String name, String value) {
 
     TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index 1915176..e683e96 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -437,7 +437,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       return null;
     }),
     CREATEALIAS_OP(CREATEALIAS, (req, rsp, h) -> {
-      final String aliasName = SolrIdentifierValidator.validateAliasName(req.getParams().get(NAME));
+      SolrIdentifierValidator.validateAliasName(req.getParams().get(NAME));
       return req.getParams().required().getAll(null, NAME, "collections");
     }),
     DELETEALIAS_OP(DELETEALIAS, (req, rsp, h) -> req.getParams().required().getAll(null, NAME)),

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
----------------------------------------------------------------------
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 50e84e4..6ca072b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
@@ -16,8 +16,6 @@
  */
 package org.apache.solr.cloud;
 
-import java.lang.invoke.MethodHandles;
-
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
@@ -28,13 +26,9 @@ import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrException;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public class AliasIntegrationTest extends SolrCloudTestCase {
 
-  private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-  
   @BeforeClass
   public static void setupCluster() throws Exception {
     configureCluster(2)
@@ -160,9 +154,49 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
       cluster.getSolrClient().query(q);
     });
     assertTrue("Unexpected exception message: " + e.getMessage(), e.getMessage().contains("Collection not found: testalias"));
-
-    logger.info("### FINISHED ACTUAL TEST");
   }
 
+  public void testErrorChecks() throws Exception {
+
+    CollectionAdminRequest.createCollection("testErrorChecks-collection", "conf", 2, 1).process(cluster.getSolrClient());
+    waitForState("Expected testErrorChecks-collection to be created with 2 shards and 1 replica", "testErrorChecks-collection", clusterShape(2, 1));
+    
+    ignoreException(".");
+    
+    // Invalid Alias name
+    SolrException e = expectThrows(SolrException.class, () -> {
+      CollectionAdminRequest.createAlias("test:alias", "testErrorChecks-collection").process(cluster.getSolrClient());
+    });
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST, SolrException.ErrorCode.getErrorCode(e.code()));
+
+    // Target collection doesn't exists
+    e = expectThrows(SolrException.class, () -> {
+      CollectionAdminRequest.createAlias("testalias", "doesnotexist").process(cluster.getSolrClient());
+    });
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST, SolrException.ErrorCode.getErrorCode(e.code()));
+    assertTrue(e.getMessage().contains("Can't create collection alias for collections='doesnotexist', 'doesnotexist' is not an existing collection or alias"));
+
+    // One of the target collections doesn't exist
+    e = expectThrows(SolrException.class, () -> {
+      CollectionAdminRequest.createAlias("testalias", "testErrorChecks-collection,doesnotexist").process(cluster.getSolrClient());
+    });
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST, SolrException.ErrorCode.getErrorCode(e.code()));
+    assertTrue(e.getMessage().contains("Can't create collection alias for collections='testErrorChecks-collection,doesnotexist', 'doesnotexist' is not an existing collection or alias"));
+
+    // Valid
+    CollectionAdminRequest.createAlias("testalias", "testErrorChecks-collection").process(cluster.getSolrClient());
+    CollectionAdminRequest.createAlias("testalias2", "testalias").process(cluster.getSolrClient());
+
+    // Alias + invalid
+    e = expectThrows(SolrException.class, () -> {
+      CollectionAdminRequest.createAlias("testalias3", "testalias2,doesnotexist").process(cluster.getSolrClient());
+    });
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST, SolrException.ErrorCode.getErrorCode(e.code()));
+    unIgnoreException(".");
+
+    CollectionAdminRequest.deleteAlias("testalias").process(cluster.getSolrClient());
+    CollectionAdminRequest.deleteAlias("testalias2").process(cluster.getSolrClient());
+    CollectionAdminRequest.deleteCollection("testErrorChecks-collection");
+  }
 
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java
index d0a600d..885fe82 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java
@@ -62,16 +62,17 @@ public class JdbcTest extends SolrCloudTestCase {
 
     String collection;
     boolean useAlias = random().nextBoolean();
-    if(useAlias) {
+    if (useAlias) {
       collection = COLLECTIONORALIAS + "_collection";
-      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
     } else {
       collection = COLLECTIONORALIAS;
     }
-
     CollectionAdminRequest.createCollection(collection, "conf", 2, 1).process(cluster.getSolrClient());
     AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, cluster.getSolrClient().getZkStateReader(),
         false, true, DEFAULT_TIMEOUT);
+    if (useAlias) {
+      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
+    }
 
     new UpdateRequest()
         .add(id, "0", "a_s", "hello0", "a_i", "0", "a_f", "1", "testnull_i", null)

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/JDBCStreamTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/JDBCStreamTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/JDBCStreamTest.java
index c661fa2..e55c837 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/JDBCStreamTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/JDBCStreamTest.java
@@ -62,18 +62,19 @@ public class JDBCStreamTest extends SolrCloudTestCase {
         .addConfig("conf", getFile("solrj").toPath().resolve("solr").resolve("configsets").resolve("streaming").resolve("conf"))
         .configure();
 
-    String collection;
     boolean useAlias = random().nextBoolean();
-    if(useAlias) {
+    String collection;
+    if (useAlias) {
       collection = COLLECTIONORALIAS + "_collection";
-      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
     } else {
       collection = COLLECTIONORALIAS;
     }
-
     CollectionAdminRequest.createCollection(collection, "conf", 2, 1).process(cluster.getSolrClient());
     AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, cluster.getSolrClient().getZkStateReader(),
         false, true, TIMEOUT);
+    if (useAlias) {
+      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
+    }
   }
 
   @BeforeClass

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
index ff5a062..7d48c0e 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
@@ -80,16 +80,17 @@ public class StreamExpressionTest extends SolrCloudTestCase {
 
     String collection;
     useAlias = random().nextBoolean();
-    if(useAlias) {
+    if (useAlias) {
       collection = COLLECTIONORALIAS + "_collection";
-      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
     } else {
       collection = COLLECTIONORALIAS;
     }
-
     CollectionAdminRequest.createCollection(collection, "conf", 2, 1).process(cluster.getSolrClient());
     AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, cluster.getSolrClient().getZkStateReader(),
         false, true, TIMEOUT);
+    if (useAlias) {
+      CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
+    }
   }
 
   @Before

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/84bbb8f7/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java
index d430409..cf60bc3 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamingTest.java
@@ -94,15 +94,17 @@ public static void configureCluster() throws Exception {
 
   String collection;
   useAlias = random().nextBoolean();
-  if(useAlias) {
+  if (useAlias) {
     collection = COLLECTIONORALIAS + "_collection";
-    CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
   } else {
     collection = COLLECTIONORALIAS;
   }
-
   CollectionAdminRequest.createCollection(collection, "conf", numShards, 1).process(cluster.getSolrClient());
-  AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, cluster.getSolrClient().getZkStateReader(), false, true, DEFAULT_TIMEOUT);
+  AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, cluster.getSolrClient().getZkStateReader(),
+      false, true, DEFAULT_TIMEOUT);
+  if (useAlias) {
+    CollectionAdminRequest.createAlias(COLLECTIONORALIAS, collection).process(cluster.getSolrClient());
+  }
 
   zkHost = cluster.getZkServer().getZkAddress();
   streamFactory.withCollectionZkHost(COLLECTIONORALIAS, zkHost);