From a9fd4c7a3f1f6b768a510437b9d64341df696f43 Mon Sep 17 00:00:00 2001 From: Mark Robert Miller Date: Thu, 14 Mar 2013 21:01:00 +0000 Subject: [PATCH] SOLR-4574: The Collections API will silently return success on an unknown ACTION parameter. SOLR-4576: Collections API validation errors should cause an exception on clients and otherwise act as validation errors with the Core Admin API. SOLR-4577: The collections API should return responses (success or failure) for each node it attempts to work with. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1456683 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 10 ++ .../cloud/OverseerCollectionProcessor.java | 95 ++++++------ .../handler/admin/CollectionsHandler.java | 67 +++++---- .../handler/component/HttpShardHandler.java | 3 + .../solr/handler/component/ShardRequest.java | 3 + .../solr/handler/component/ShardResponse.java | 12 ++ .../CollectionsAPIDistributedZkTest.java | 136 ++++++++++++++++-- .../OverseerCollectionProcessorTest.java | 3 +- .../client/solrj/request/TestCoreAdmin.java | 23 +++ 9 files changed, 266 insertions(+), 86 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 505885a27c2..03d9aeaf3a5 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -93,6 +93,16 @@ Bug Fixes close it in SolrCloud mode when a core with the same name already exists. (Mark Miller) +* SOLR-4574: The Collections API will silently return success on an unknown + ACTION parameter. (Mark Miller) + +* SOLR-4576: Collections API validation errors should cause an exception on + clients and otherwise act as validation errors with the Core Admin API. + (Mark Miller) + +* SOLR-4577: The collections API should return responses (success or failure) + for each node it attempts to work with. (Mark Miller) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java index 19854abda0c..3bd1f0d91e0 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java @@ -41,6 +41,7 @@ import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; import org.apache.solr.handler.component.ShardHandler; import org.apache.solr.handler.component.ShardRequest; @@ -156,22 +157,22 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { NamedList results = new NamedList(); try { if (CREATECOLLECTION.equals(operation)) { - createCollection(zkStateReader.getClusterState(), message); + createCollection(zkStateReader.getClusterState(), message, results); } else if (DELETECOLLECTION.equals(operation)) { ModifiableSolrParams params = new ModifiableSolrParams(); params.set(CoreAdminParams.ACTION, CoreAdminAction.UNLOAD.toString()); params.set(CoreAdminParams.DELETE_INSTANCE_DIR, true); - collectionCmd(zkStateReader.getClusterState(), message, params); + collectionCmd(zkStateReader.getClusterState(), message, params, results); } else if (RELOADCOLLECTION.equals(operation)) { ModifiableSolrParams params = new ModifiableSolrParams(); params.set(CoreAdminParams.ACTION, CoreAdminAction.RELOAD.toString()); - collectionCmd(zkStateReader.getClusterState(), message, params); + collectionCmd(zkStateReader.getClusterState(), message, params, results); } else if (CREATEALIAS.equals(operation)) { createAlias(zkStateReader.getAliases(), message); } else if (DELETEALIAS.equals(operation)) { deleteAlias(zkStateReader.getAliases(), message); } else { - throw new SolrException(ErrorCode.BAD_REQUEST, "Unknow the operation:" + throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown operation:" + operation); } int failed = 0; @@ -194,6 +195,10 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { SolrException.log(log, "Collection " + operation + " of " + operation + " failed", ex); results.add("Operation " + operation + " caused exception:", ex); + SimpleOrderedMap nl = new SimpleOrderedMap(); + nl.add("msg", ex.getMessage()); + nl.add("rspCode", ex instanceof SolrException ? ((SolrException)ex).code() : -1); + results.add("exception", nl); } finally { return new OverseerSolrResponse(results); } @@ -300,11 +305,10 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { } - private boolean createCollection(ClusterState clusterState, ZkNodeProps message) { + private void createCollection(ClusterState clusterState, ZkNodeProps message, NamedList results) { String collectionName = message.getStr("name"); if (clusterState.getCollections().contains(collectionName)) { - SolrException.log(log, "collection already exists: " + collectionName); - return false; + throw new SolrException(ErrorCode.BAD_REQUEST, "collection already exists: " + collectionName); } try { @@ -319,12 +323,11 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { if (repFactor <= 0) { SolrException.log(log, REPLICATION_FACTOR + " must be > 0"); - return false; + throw new SolrException(ErrorCode.BAD_REQUEST, "collection already exists: " + collectionName); } if (numSlices < 0) { - SolrException.log(log, NUM_SLICES + " must be > 0"); - return false; + throw new SolrException(ErrorCode.BAD_REQUEST, NUM_SLICES + " must be > 0"); } String configName = message.getStr("collection.configName"); @@ -343,9 +346,8 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { Collections.shuffle(nodeList); if (nodeList.size() <= 0) { - log.error("Cannot create collection " + collectionName - + ". No live Solr-instaces" + ((createNodeList != null)?" among Solr-instances specified in " + CREATE_NODE_SET:"")); - return false; + throw new SolrException(ErrorCode.BAD_REQUEST, "Cannot create collection " + collectionName + + ". No live Solr-instances" + ((createNodeList != null)?" among Solr-instances specified in " + CREATE_NODE_SET + ":" + createNodeSetStr:"")); } if (repFactor > nodeList.size()) { @@ -363,7 +365,7 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { int maxShardsAllowedToCreate = maxShardsPerNode * nodeList.size(); int requestedShardsToCreate = numSlices * repFactor; if (maxShardsAllowedToCreate < requestedShardsToCreate) { - log.error("Cannot create collection " + collectionName + ". Value of " + throw new SolrException(ErrorCode.BAD_REQUEST, "Cannot create collection " + collectionName + ". Value of " + MAX_SHARDS_PER_NODE + " is " + maxShardsPerNode + ", and the number of live nodes is " + nodeList.size() + ". This allows a maximum of " + maxShardsAllowedToCreate @@ -371,7 +373,6 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { + " and value of " + REPLICATION_FACTOR + " is " + repFactor + ". This requires " + requestedShardsToCreate + " shards to be created (higher than the allowed number)"); - return false; } for (int i = 1; i <= numSlices; i++) { @@ -394,6 +395,7 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { params.set(ZkStateReader.NUM_SHARDS_PROP, numSlices); ShardRequest sreq = new ShardRequest(); + sreq.nodeName = nodeName; params.set("qt", adminPath); sreq.purpose = 1; String replica = zkStateReader.getZkClient() @@ -408,36 +410,25 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { } } - int failed = 0; ShardResponse srsp; do { srsp = shardHandler.takeCompletedOrError(); if (srsp != null) { - Throwable e = srsp.getException(); - if (e != null) { - // should we retry? - // TODO: we should return errors to the client - // TODO: what if one fails and others succeed? - failed++; - log.error("Error talking to shard: " + srsp.getShard(), e); - } + processResponse(results, srsp); } } while (srsp != null); - - // if all calls succeeded, return true - if (failed > 0) { - return false; - } - log.info("Successfully created all shards for collection " + + log.info("Finished create command on all shards for collection: " + collectionName); - return true; + + } catch (SolrException ex) { + throw ex; } catch (Exception ex) { - // Expecting that the necessary logging has already been performed - return false; + throw new SolrException(ErrorCode.SERVER_ERROR, null, ex); } } - private boolean collectionCmd(ClusterState clusterState, ZkNodeProps message, ModifiableSolrParams params) { + private boolean collectionCmd(ClusterState clusterState, ZkNodeProps message, ModifiableSolrParams params, NamedList results) { log.info("Executing Collection Cmd : " + params); String collectionName = message.getStr("name"); @@ -463,7 +454,7 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { String replica = node.getStr(ZkStateReader.BASE_URL_PROP); ShardRequest sreq = new ShardRequest(); - + sreq.nodeName = node.getStr(ZkStateReader.NODE_NAME_PROP); // yes, they must use same admin handler path everywhere... cloneParams.set("qt", adminPath); sreq.purpose = 1; @@ -484,14 +475,7 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { do { srsp = shardHandler.takeCompletedOrError(); if (srsp != null) { - Throwable e = srsp.getException(); - if (e != null) { - // should we retry? - // TODO: we should return errors to the client - // TODO: what if one fails and others succeed? - failed++; - log.error("Error talking to shard: " + srsp.getShard(), e); - } + processResponse(results, srsp); } } while (srsp != null); @@ -502,6 +486,31 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread { } return true; } + + private void processResponse(NamedList results, ShardResponse srsp) { + Throwable e = srsp.getException(); + if (e != null) { + log.error("Error from shard: " + srsp.getShard(), e); + + SimpleOrderedMap failure = (SimpleOrderedMap) results.get("failure"); + if (failure == null) { + failure = new SimpleOrderedMap(); + results.add("failure", failure); + } + + failure.add(srsp.getNodeName(), e.getClass().getName() + ":" + e.getMessage()); + + } else { + + SimpleOrderedMap success = (SimpleOrderedMap) results.get("success"); + if (success == null) { + success = new SimpleOrderedMap(); + results.add("success", success); + } + + success.add(srsp.getNodeName(), srsp.getSolrResponse().getResponse()); + } + } private int msgStrToInt(ZkNodeProps message, String key, Integer def) throws Exception { 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 a77c5c58035..cb21826ccc8 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 @@ -40,6 +40,7 @@ import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.request.SolrQueryRequest; @@ -102,35 +103,38 @@ public class CollectionsHandler extends RequestHandlerBase { if (a != null) { action = CollectionAction.get(a); } - if (action != null) { - switch (action) { - case CREATE: { - this.handleCreateAction(req, rsp); - break; - } - case DELETE: { - this.handleDeleteAction(req, rsp); - break; - } - case RELOAD: { - this.handleReloadAction(req, rsp); - break; - } - case SYNCSHARD: { - this.handleSyncShardAction(req, rsp); - break; - } - case CREATEALIAS: { - this.handleCreateAliasAction(req, rsp); - break; - } - case DELETEALIAS: { - this.handleDeleteAliasAction(req, rsp); - break; - } - default: { - throw new RuntimeException("Unknown action: " + action); - } + if (action == null) { + throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown action: " + a); + } + + switch (action) { + case CREATE: { + this.handleCreateAction(req, rsp); + break; + } + case DELETE: { + this.handleDeleteAction(req, rsp); + break; + } + case RELOAD: { + this.handleReloadAction(req, rsp); + break; + } + case SYNCSHARD: { + this.handleSyncShardAction(req, rsp); + break; + } + case CREATEALIAS: { + this.handleCreateAliasAction(req, rsp); + break; + } + case DELETEALIAS: { + this.handleDeleteAliasAction(req, rsp); + break; + } + default: { + throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown action: " + + action); } } @@ -148,6 +152,11 @@ public class CollectionsHandler extends RequestHandlerBase { if (event.getBytes() != null) { SolrResponse response = SolrResponse.deserialize(event.getBytes()); rsp.getValues().addAll(response.getResponse()); + SimpleOrderedMap exp = (SimpleOrderedMap) response.getResponse().get("exception"); + if (exp != null) { + Integer code = (Integer) exp.get("rspCode"); + rsp.setException(new SolrException(code != null && code != -1 ? ErrorCode.getErrorCode(code) : ErrorCode.SERVER_ERROR, (String)exp.get("msg"))); + } } else { if (System.currentTimeMillis() - time >= DEFAULT_ZK_TIMEOUT) { throw new SolrException(ErrorCode.SERVER_ERROR, operation diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java index fe15354ee9a..cea5a7b3200 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java @@ -137,6 +137,9 @@ public class HttpShardHandler extends ShardHandler { public ShardResponse call() throws Exception { ShardResponse srsp = new ShardResponse(); + if (sreq.nodeName != null) { + srsp.setNodeName(sreq.nodeName); + } srsp.setShardRequest(sreq); srsp.setShard(shard); SimpleSolrResponse ssr = new SimpleSolrResponse(); diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java b/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java index e9d69f5fe8f..57ceb437dea 100755 --- a/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ShardRequest.java @@ -53,6 +53,9 @@ public class ShardRequest { /** actual shards to send the request to, filled out by framework */ public String[] actualShards; + /** may be null */ + public String nodeName; + // TODO: one could store a list of numbers to correlate where returned docs // go in the top-level response rather than looking up by id... // this would work well if we ever transitioned to using internal ids and diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java index 07ef8ee2890..d2296806777 100755 --- a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java @@ -22,6 +22,7 @@ import org.apache.solr.common.SolrException; public final class ShardResponse { private ShardRequest req; private String shard; + private String nodeName; private String shardAddress; // the specific shard that this response was received from private int rspCode; private Throwable exception; @@ -56,6 +57,11 @@ public final class ShardResponse { return shard; } + public String getNodeName() + { + return nodeName; + } + public void setShardRequest(ShardRequest rsp) { this.req = rsp; @@ -80,9 +86,15 @@ public final class ShardResponse { { this.rspCode = rspCode; } + + void setNodeName(String nodeName) + { + this.nodeName = nodeName; + } /** What was the shard address that returned this response. Example: "http://localhost:8983/solr" */ public String getShardAddress() { return this.shardAddress; } void setShardAddress(String addr) { this.shardAddress = addr; } + } diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java index c83a002cc24..7f768e0f878 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java @@ -17,9 +17,9 @@ package org.apache.solr.cloud; * limitations under the License. */ +import java.io.File; import java.io.IOException; import java.lang.management.ManagementFactory; -import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -43,12 +43,14 @@ import javax.management.ObjectName; import org.apache.lucene.util.LuceneTestCase.Slow; import org.apache.lucene.util._TestUtil; +import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrServer; import org.apache.solr.client.solrj.impl.HttpSolrServer; import org.apache.solr.client.solrj.request.CoreAdminRequest; import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.request.CoreAdminRequest.Create; import org.apache.solr.client.solrj.response.CoreAdminResponse; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrException; @@ -63,9 +65,12 @@ import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionParams.CollectionAction; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrInfoMBean.Category; +import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.update.DirectUpdateHandler2; import org.apache.solr.update.SolrCmdDistributor.Request; import org.apache.solr.util.DefaultSolrThreadFactory; @@ -139,12 +144,115 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa testNodesUsedByCreate(); testCollectionsAPI(); + testErrorHandling(); if (DEBUG) { super.printLayout(); } } + private void testErrorHandling() throws Exception { + final String baseUrl = getBaseUrl((HttpSolrServer) clients.get(0)); + + + // try a bad action + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set("action", "BADACTION"); + String collectionName = "badactioncollection"; + params.set("name", collectionName); + QueryRequest request = new QueryRequest(params); + request.setPath("/admin/collections"); + boolean gotExp = false; + NamedList resp = null; + try { + resp = createNewSolrServer("", baseUrl).request(request); + } catch (SolrException e) { + gotExp = true; + } + assertTrue(gotExp); + + + // leave out required param name + params = new ModifiableSolrParams(); + params.set("action", CollectionAction.CREATE.toString()); + collectionName = "collection"; + // No Name + // params.set("name", collectionName); + request = new QueryRequest(params); + request.setPath("/admin/collections"); + gotExp = false; + resp = null; + try { + resp = createNewSolrServer("", baseUrl).request(request); + } catch (SolrException e) { + gotExp = true; + } + assertTrue(gotExp); + + // Too many replicas + params = new ModifiableSolrParams(); + params.set("action", CollectionAction.CREATE.toString()); + collectionName = "collection"; + params.set("name", collectionName); + params.set("numShards", 2); + params.set(OverseerCollectionProcessor.REPLICATION_FACTOR, 10); + request = new QueryRequest(params); + request.setPath("/admin/collections"); + gotExp = false; + resp = null; + try { + resp = createNewSolrServer("", baseUrl).request(request); + } catch (SolrException e) { + gotExp = true; + } + assertTrue(gotExp); + + // Fail on one node + + // first we make a core with the core name the collections api + // will try and use - this will cause our mock fail + Create createCmd = new Create(); + createCmd.setCoreName("halfcollection_shard1_replica1"); + createCmd.setCollection("halfcollectionblocker"); + String dataDir = SolrTestCaseJ4.dataDir.getAbsolutePath() + File.separator + + System.currentTimeMillis() + "halfcollection" + "_3n"; + createCmd.setDataDir(dataDir); + createCmd.setNumShards(1); + createNewSolrServer("", baseUrl).request(createCmd); + + createCmd = new Create(); + createCmd.setCoreName("halfcollection_shard1_replica1"); + createCmd.setCollection("halfcollectionblocker2"); + dataDir = SolrTestCaseJ4.dataDir.getAbsolutePath() + File.separator + + System.currentTimeMillis() + "halfcollection" + "_3n"; + createCmd.setDataDir(dataDir); + createCmd.setNumShards(1); + createNewSolrServer("", getBaseUrl((HttpSolrServer) clients.get(1))).request(createCmd); + + params = new ModifiableSolrParams(); + params.set("action", CollectionAction.CREATE.toString()); + collectionName = "halfcollection"; + params.set("name", collectionName); + params.set("numShards", 2); + params.set("wt", "xml"); + + String nn1 = ((SolrDispatchFilter) jettys.get(0).getDispatchFilter().getFilter()).getCores().getZkController().getNodeName(); + String nn2 = ((SolrDispatchFilter) jettys.get(1).getDispatchFilter().getFilter()).getCores().getZkController().getNodeName(); + + params.set(OverseerCollectionProcessor.CREATE_NODE_SET, nn1 + "," + nn2); + request = new QueryRequest(params); + request.setPath("/admin/collections"); + gotExp = false; + resp = createNewSolrServer("", baseUrl).request(request); + + SimpleOrderedMap success = (SimpleOrderedMap) resp.get("success"); + SimpleOrderedMap failure = (SimpleOrderedMap) resp.get("failure"); + + String val1 = success.getVal(0).toString(); + String val2 = failure.getVal(0).toString(); + assertTrue(val1.contains("SolrException") || val2.contains("SolrException")); + } + private void testNodesUsedByCreate() throws Exception { // we can use this client because we just want base url final String baseUrl = getBaseUrl((HttpSolrServer) clients.get(0)); @@ -324,7 +432,13 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa request = new QueryRequest(params); request.setPath("/admin/collections"); - createNewSolrServer("", baseUrl).request(request); + boolean exp = false; + try { + createNewSolrServer("", baseUrl).request(request); + } catch (SolrException e) { + exp = true; + } + assertTrue("Expected exception", exp); // create another collection should still work params = new ModifiableSolrParams(); @@ -363,14 +477,17 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa collectionInfos = new HashMap>(); CloudSolrServer client = createCloudClient("awholynewcollection_" + cnt); try { + exp = false; + try { createCollection(collectionInfos, "awholynewcollection_" + cnt, numShards, replicationFactor, maxShardsPerNode, client, null); + } catch (SolrException e) { + exp = true; + } + assertTrue("expected exception", exp); } finally { client.shutdown(); } - - // TODO: REMOVE THE SLEEP IN THE METHOD CALL WHEN WE HAVE COLLECTION API - // RESPONSES - checkCollectionIsNotCreated(collectionInfos.keySet().iterator().next()); + // Test createNodeSet numLiveNodes = getCommonCloudSolrServer().getZkStateReader().getClusterState().getLiveNodes().size(); @@ -508,13 +625,6 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa fail("Could not find the new collection - " + exp.code() + " : " + collectionClient.getBaseURL()); } - - private void checkCollectionIsNotCreated(String collectionName) - throws Exception { - // TODO: REMOVE THIS SLEEP WHEN WE HAVE COLLECTION API RESPONSES - Thread.sleep(10000); - assertFalse(collectionName + " not supposed to exist", getCommonCloudSolrServer().getZkStateReader().getClusterState().getCollections().contains(collectionName)); - } private void checkForMissingCollection(String collectionName) throws Exception { diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionProcessorTest.java index f3e9ee6609e..ce7b19939f4 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionProcessorTest.java @@ -38,6 +38,7 @@ import java.util.Set; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrResponse; +import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.cloud.DistributedQueue.QueueEvent; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.SolrZkClient; @@ -46,7 +47,6 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction; import org.apache.solr.common.params.ModifiableSolrParams; -import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.StrUtils; import org.apache.solr.handler.component.ShardHandler; import org.apache.solr.handler.component.ShardRequest; @@ -248,6 +248,7 @@ public class OverseerCollectionProcessorTest extends SolrTestCaseJ4 { expectLastCall(); submitCaptures.add(submitCapture); ShardResponse shardResponseWithoutException = new ShardResponse(); + shardResponseWithoutException.setSolrResponse(new QueryResponse()); expect(shardHandlerMock.takeCompletedOrError()).andReturn( shardResponseWithoutException); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java index 21179582208..9be49721784 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java @@ -23,6 +23,9 @@ import org.apache.solr.SolrIgnoredThreadsFilter; import org.apache.solr.client.solrj.SolrServer; import org.apache.solr.client.solrj.embedded.AbstractEmbeddedSolrServerTestCase; import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.NamedList; import org.apache.solr.core.SolrCore; import org.apache.commons.io.FileUtils; import org.junit.After; @@ -94,6 +97,26 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { } + @Test + public void testErrorCases() throws Exception { + + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set("action", "BADACTION"); + String collectionName = "badactioncollection"; + params.set("name", collectionName); + QueryRequest request = new QueryRequest(params); + request.setPath("/admin/cores"); + boolean gotExp = false; + NamedList resp = null; + try { + resp = getSolrAdmin().request(request); + } catch (SolrException e) { + gotExp = true; + } + + assertTrue(gotExp); + } + @BeforeClass public static void before() { // wtf?