From 84bbb8f797db55f6cb203d87d150f2b3a2fe92cf Mon Sep 17 00:00:00 2001 From: Tomas Fernandez Lobbe Date: Mon, 19 Dec 2016 16:54:13 -0800 Subject: [PATCH] SOLR-9874: CREATEALIAS should fail if target collections don't exist --- solr/CHANGES.txt | 2 + .../solr/SolrMorphlineZkAliasTest.java | 7 --- .../org/apache/solr/cloud/CreateAliasCmd.java | 21 ++++++-- .../handler/admin/CollectionsHandler.java | 2 +- .../solr/cloud/AliasIntegrationTest.java | 50 ++++++++++++++++--- .../solr/client/solrj/io/sql/JdbcTest.java | 7 +-- .../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(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 35a2bb7b3fa..d04d491af65 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 ================== 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 ddaf2f69e6e..74c8824ddc8 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"); - }); - } } 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 b966ebdd769..339d75ac02a 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> newAliasesMap = new HashMap<>(); - Map newCollectionAliasesMap = new HashMap<>(); ZkStateReader zkStateReader = ocmh.zkStateReader; Map prevColAliases = zkStateReader.getAliases().getCollectionAliasMap(); + validateAllCollectionsExist(collections, prevColAliases, zkStateReader.getClusterState()); + + Map> newAliasesMap = new HashMap<>(); + Map newCollectionAliasesMap = new HashMap<>(); if (prevColAliases != null) { newCollectionAliasesMap.putAll(prevColAliases); } @@ -81,6 +84,16 @@ public class CreateAliasCmd implements Cmd { } } + private void validateAllCollectionsExist(String collections, Map 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); 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 1915176070d..e683e96bddc 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)), 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 50e84e45d90..6ca072b1ba8 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"); + } } 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 d0a600d4947..885fe82d463 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) 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 c661fa2bc5b..e55c83747d9 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 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 ff5a0627e36..7d48c0e239f 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 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 d4304095190..cf60bc39f08 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);