SOLR-8765: Enforce required parameters in SolrJ Collections API

This commit is contained in:
Alan Woodward 2016-03-07 19:53:09 +00:00 committed by Alan Woodward
parent b4eb4fb32c
commit c1277cda11
7 changed files with 666 additions and 199 deletions

View File

@ -24,6 +24,10 @@ Detailed Change List
.processAndWait() to wait for a call to finish without holding HTTP
collections open. (Alan Woodward)
* SOLR-8765: Enforce required parameters at query construction time in the SolrJ
Collections API, add static factory methods, and deprecate old setter methods.
(Alan Woodward, Jason Gerlowski)
New Features
----------------------

View File

@ -804,9 +804,7 @@ public class CoreContainer {
SolrCore core = null;
try {
MDCLoggingContext.setCore(core);
if (!SolrIdentifierValidator.validateCoreName(dcore.getName())) {
throw new SolrException(ErrorCode.BAD_REQUEST, SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE, dcore.getName()));
}
SolrIdentifierValidator.validateCoreName(dcore.getName());
if (zkSys.getZkController() != null) {
zkSys.getZkController().preRegister(dcore);
}
@ -1009,10 +1007,7 @@ public class CoreContainer {
}
public void rename(String name, String toName) {
if (!SolrIdentifierValidator.validateCoreName(toName)) {
throw new SolrException(ErrorCode.BAD_REQUEST, SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
toName));
}
SolrIdentifierValidator.validateCoreName(toName);
try (SolrCore core = getCore(name)) {
if (core != null) {
registerCore(toName, core, true);

View File

@ -16,45 +16,8 @@
*/
package org.apache.solr.handler.admin;
import static org.apache.solr.client.solrj.response.RequestStatusState.*;
import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.COLL_CONF;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.COLL_PROP_PREFIX;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.CREATE_NODE_SET;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.CREATE_NODE_SET_SHUFFLE;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.NUM_SLICES;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.ONLY_ACTIVE_NODES;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.ONLY_IF_DOWN;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.REQUESTID;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.SHARDS_PROP;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.SHARD_UNIQUE;
import static org.apache.solr.common.cloud.DocCollection.DOC_ROUTER;
import static org.apache.solr.common.cloud.DocCollection.RULE;
import static org.apache.solr.common.cloud.DocCollection.SNITCH;
import static org.apache.solr.common.cloud.DocCollection.STATE_FORMAT;
import static org.apache.solr.common.cloud.ZkStateReader.AUTO_ADD_REPLICAS;
import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.MAX_SHARDS_PER_NODE;
import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.*;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.VALUE_LONG;
import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_DATA_DIR;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_INDEX;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_INSTANCE_DIR;
import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
import static org.apache.solr.common.params.ShardParams._ROUTE_;
import static org.apache.solr.common.util.StrUtils.formatString;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@ -66,6 +29,8 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.solr.client.solrj.SolrResponse;
@ -117,8 +82,45 @@ import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import static org.apache.solr.client.solrj.response.RequestStatusState.COMPLETED;
import static org.apache.solr.client.solrj.response.RequestStatusState.FAILED;
import static org.apache.solr.client.solrj.response.RequestStatusState.NOT_FOUND;
import static org.apache.solr.client.solrj.response.RequestStatusState.RUNNING;
import static org.apache.solr.client.solrj.response.RequestStatusState.SUBMITTED;
import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.COLL_CONF;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.COLL_PROP_PREFIX;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.CREATE_NODE_SET;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.CREATE_NODE_SET_SHUFFLE;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.NUM_SLICES;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.ONLY_ACTIVE_NODES;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.ONLY_IF_DOWN;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.REQUESTID;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.SHARDS_PROP;
import static org.apache.solr.cloud.OverseerCollectionMessageHandler.SHARD_UNIQUE;
import static org.apache.solr.common.cloud.DocCollection.DOC_ROUTER;
import static org.apache.solr.common.cloud.DocCollection.RULE;
import static org.apache.solr.common.cloud.DocCollection.SNITCH;
import static org.apache.solr.common.cloud.DocCollection.STATE_FORMAT;
import static org.apache.solr.common.cloud.ZkStateReader.AUTO_ADD_REPLICAS;
import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.MAX_SHARDS_PER_NODE;
import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP;
import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.*;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.VALUE_LONG;
import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_DATA_DIR;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_INDEX;
import static org.apache.solr.common.params.CoreAdminParams.DELETE_INSTANCE_DIR;
import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
import static org.apache.solr.common.params.ShardParams._ROUTE_;
import static org.apache.solr.common.util.StrUtils.formatString;
public class CollectionsHandler extends RequestHandlerBase {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@ -348,11 +350,7 @@ public class CollectionsHandler extends RequestHandlerBase {
addMapObject(props, RULE);
addMapObject(props, SNITCH);
verifyRuleParams(h.coreContainer, props);
final String collectionName = (String) props.get(NAME);
if (!SolrIdentifierValidator.validateCollectionName(collectionName)) {
throw new SolrException(ErrorCode.BAD_REQUEST,
SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.COLLECTION, collectionName));
}
final String collectionName = SolrIdentifierValidator.validateCollectionName((String)props.get(NAME));
final String shardsParam = (String) props.get(SHARDS_PROP);
if (StringUtils.isNotEmpty(shardsParam)) {
verifyShardsParam(shardsParam);
@ -433,10 +431,7 @@ public class CollectionsHandler extends RequestHandlerBase {
@Override
Map<String, Object> call(SolrQueryRequest req, SolrQueryResponse rsp, CollectionsHandler handler)
throws Exception {
final String aliasName = req.getParams().get(NAME);
if (!SolrIdentifierValidator.validateCollectionName(aliasName)) {
throw new SolrException(ErrorCode.BAD_REQUEST, SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.ALIAS, aliasName));
}
final String aliasName = SolrIdentifierValidator.validateAliasName(req.getParams().get(NAME));
return req.getParams().required().getAll(null, NAME, "collections");
}
},
@ -505,11 +500,7 @@ public class CollectionsHandler extends RequestHandlerBase {
COLLECTION_PROP,
SHARD_ID_PROP);
ClusterState clusterState = handler.coreContainer.getZkController().getClusterState();
final String newShardName = req.getParams().get(SHARD_ID_PROP);
if (!SolrIdentifierValidator.validateShardName(newShardName)) {
throw new SolrException(ErrorCode.BAD_REQUEST, SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
newShardName));
}
final String newShardName = SolrIdentifierValidator.validateShardName(req.getParams().get(SHARD_ID_PROP));
if (!ImplicitDocRouter.NAME.equals(((Map) clusterState.getCollection(req.getParams().get(COLLECTION_PROP)).get(DOC_ROUTER)).get(NAME)))
throw new SolrException(ErrorCode.BAD_REQUEST, "shards can be added only to 'implicit' collections");
req.getParams().getAll(map,
@ -997,9 +988,7 @@ public class CollectionsHandler extends RequestHandlerBase {
private static void verifyShardsParam(String shardsParam) {
for (String shard : shardsParam.split(",")) {
if (!SolrIdentifierValidator.validateShardName(shard))
throw new SolrException(ErrorCode.BAD_REQUEST, SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
shard));
SolrIdentifierValidator.validateShardName(shard);
}
}

View File

@ -17,101 +17,129 @@
package org.apache.solr.cloud;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.response.RequestStatusState;
import org.junit.BeforeClass;
import org.junit.Test;
public class DeleteStatusTest extends AbstractFullDistribZkTestBase {
public class DeleteStatusTest extends SolrCloudTestCase {
public static final int MAX_WAIT_TIMEOUT = 30;
@BeforeClass
public static void createCluster() throws Exception {
configureCluster(2)
.addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
.configure();
}
// Basically equivalent to RequestStatus.waitFor(), but doesn't delete the id from the queue
private static RequestStatusState waitForRequestState(String id, SolrClient client, int timeout)
throws IOException, SolrServerException, InterruptedException {
RequestStatusState state = RequestStatusState.SUBMITTED;
long endTime = System.nanoTime() + TimeUnit.SECONDS.toNanos(MAX_WAIT_TIMEOUT);
while (System.nanoTime() < endTime) {
state = CollectionAdminRequest.requestStatus(id).process(client).getRequestStatus();
if (state == RequestStatusState.COMPLETED)
break;
assumeTrue("Error creating collection - skipping test", state != RequestStatusState.FAILED);
TimeUnit.SECONDS.sleep(1);
}
assumeTrue("Timed out creating collection - skipping test", state == RequestStatusState.COMPLETED);
return state;
}
@Test
public void testDeleteStatus() throws IOException, SolrServerException {
CollectionAdminRequest.Create create = new CollectionAdminRequest.Create();
create.setCollectionName("requeststatus")
.setConfigName("conf1")
.setReplicationFactor(1)
.setNumShards(1)
.setAsyncId("collectioncreate")
.process(cloudClient);
public void testAsyncIdsMayBeDeleted() throws Exception {
RequestStatusState state = getRequestStateAfterCompletion("collectioncreate", 30, cloudClient);
final CloudSolrClient client = cluster.getSolrClient();
final String collection = "deletestatus";
final String asyncId = CollectionAdminRequest.createCollection(collection, "conf1", 1, 1).processAsync(client);
waitForRequestState(asyncId, client, MAX_WAIT_TIMEOUT);
assertEquals(RequestStatusState.COMPLETED,
CollectionAdminRequest.requestStatus(asyncId).process(client).getRequestStatus());
CollectionAdminResponse rsp = CollectionAdminRequest.deleteAsyncId(asyncId).process(client);
assertEquals("successfully removed stored response for [" + asyncId + "]", rsp.getResponse().get("status"));
assertEquals(RequestStatusState.NOT_FOUND,
CollectionAdminRequest.requestStatus(asyncId).process(client).getRequestStatus());
}
@Test
public void testDeletingNonExistentRequests() throws Exception {
final CloudSolrClient client = cluster.getSolrClient();
CollectionAdminResponse rsp = CollectionAdminRequest.deleteAsyncId("foo").process(client);
assertEquals("[foo] not found in stored responses", rsp.getResponse().get("status"));
}
@Test
public void testProcessAndWaitDeletesAsyncIds() throws IOException, SolrServerException, InterruptedException {
final CloudSolrClient client = cluster.getSolrClient();
RequestStatusState state = CollectionAdminRequest.createCollection("requeststatus", "conf1", 1, 1)
.processAndWait("request1", client, MAX_WAIT_TIMEOUT);
assertSame(RequestStatusState.COMPLETED, state);
// Let's delete the stored response now
CollectionAdminRequest.DeleteStatus deleteStatus = new CollectionAdminRequest.DeleteStatus();
CollectionAdminResponse rsp = deleteStatus
.setRequestId("collectioncreate")
.process(cloudClient);
assertEquals("successfully removed stored response for [collectioncreate]", rsp.getResponse().get("status"));
// using processAndWait deletes the requestid
state = CollectionAdminRequest.requestStatus("request1").process(client).getRequestStatus();
assertSame("Request id was not deleted by processAndWait call", RequestStatusState.NOT_FOUND, state);
// Make sure that the response was deleted from zk
state = getRequestState("collectioncreate", cloudClient);
assertSame(RequestStatusState.NOT_FOUND, state);
// Try deleting the same requestid again
deleteStatus = new CollectionAdminRequest.DeleteStatus();
rsp = deleteStatus
.setRequestId("collectioncreate")
.process(cloudClient);
assertEquals("[collectioncreate] not found in stored responses", rsp.getResponse().get("status"));
// Let's try deleting a non-existent status
deleteStatus = new CollectionAdminRequest.DeleteStatus();
rsp = deleteStatus
.setRequestId("foo")
.process(cloudClient);
assertEquals("[foo] not found in stored responses", rsp.getResponse().get("status"));
}
@Test
public void testDeleteStatusFlush() throws Exception {
CollectionAdminRequest.Create create = new CollectionAdminRequest.Create();
create.setConfigName("conf1")
.setCollectionName("foo")
.setAsyncId("foo")
.setNumShards(1)
.setReplicationFactor(1)
.process(cloudClient);
create = new CollectionAdminRequest.Create();
create.setConfigName("conf1")
.setCollectionName("bar")
.setAsyncId("bar")
.setNumShards(1)
.setReplicationFactor(1)
.process(cloudClient);
final CloudSolrClient client = cluster.getSolrClient();
RequestStatusState state = getRequestStateAfterCompletion("foo", 30, cloudClient);
assertEquals(RequestStatusState.COMPLETED, state);
String id1 = CollectionAdminRequest.createCollection("flush1", "conf1", 1, 1).processAsync(client);
String id2 = CollectionAdminRequest.createCollection("flush2", "conf1", 1, 1).processAsync(client);
state = getRequestStateAfterCompletion("bar", 30, cloudClient);
assertEquals(RequestStatusState.COMPLETED, state);
assertEquals(RequestStatusState.COMPLETED, waitForRequestState(id1, client, MAX_WAIT_TIMEOUT));
assertEquals(RequestStatusState.COMPLETED, waitForRequestState(id2, client, MAX_WAIT_TIMEOUT));
CollectionAdminRequest.DeleteStatus deleteStatus = new CollectionAdminRequest.DeleteStatus();
deleteStatus.setFlush(true)
.process(cloudClient);
CollectionAdminRequest.deleteAllAsyncIds().process(client);
assertEquals(RequestStatusState.NOT_FOUND, getRequestState("foo", cloudClient));
assertEquals(RequestStatusState.NOT_FOUND, getRequestState("bar", cloudClient));
assertEquals(RequestStatusState.NOT_FOUND,
CollectionAdminRequest.requestStatus(id1).process(client).getRequestStatus());
assertEquals(RequestStatusState.NOT_FOUND,
CollectionAdminRequest.requestStatus(id2).process(client).getRequestStatus());
}
@Test
@SuppressWarnings("deprecation")
public void testDeprecatedConstructorValidation() throws Exception {
final CloudSolrClient client = cluster.getSolrClient();
deleteStatus = new CollectionAdminRequest.DeleteStatus();
try {
deleteStatus.process(cloudClient);
new CollectionAdminRequest.DeleteStatus().process(client);
fail("delete status should have failed");
} catch (HttpSolrClient.RemoteSolrException e) {
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Either requestid or flush parameter must be specified."));
}
deleteStatus = new CollectionAdminRequest.DeleteStatus();
try {
deleteStatus.setFlush(true)
new CollectionAdminRequest.DeleteStatus().setFlush(true)
.setRequestId("foo")
.process(cloudClient);
.process(client);
fail("delete status should have failed");
} catch (HttpSolrClient.RemoteSolrException e) {
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Both requestid and flush parameters can not be specified together."));
}
}

View File

@ -16,6 +16,11 @@
*/
package org.apache.solr.client.solrj.request;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
@ -29,11 +34,6 @@ import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
/**
* This class is experimental and subject to change.
*
@ -110,11 +110,7 @@ public class CoreAdminRequest extends SolrRequest<CoreAdminResponse> {
*/
@Override
public void setCoreName(String coreName) {
if (!SolrIdentifierValidator.validateCoreName(coreName)) {
throw new IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
coreName));
}
this.core = coreName;
this.core = SolrIdentifierValidator.validateCoreName(coreName);
}
@Override
@ -559,14 +555,9 @@ public class CoreAdminRequest extends SolrRequest<CoreAdminResponse> {
*/
public static CoreAdminResponse renameCore(String coreName, String newName, SolrClient client )
throws SolrServerException, IOException {
if (!SolrIdentifierValidator.validateCoreName(newName)) {
throw new IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
newName));
}
CoreAdminRequest req = new CoreAdminRequest();
req.setCoreName(coreName);
req.setOtherCoreName(newName);
req.setOtherCoreName(SolrIdentifierValidator.validateCoreName(newName));
req.setAction( CoreAdminAction.RENAME );
return req.process( client );
}

View File

@ -32,18 +32,28 @@ public class SolrIdentifierValidator {
SHARD, COLLECTION, CORE, ALIAS
}
public static boolean validateShardName(String shardName) {
return validateIdentifier(shardName);
public static String validateName(IdentifierType type, String name) {
if (!validateIdentifier(name))
throw new IllegalArgumentException(getIdentifierMessage(type, name));
return name;
}
public static String validateShardName(String shardName) {
return validateName(IdentifierType.SHARD, shardName);
}
public static boolean validateCollectionName(String collectionName) {
return validateIdentifier(collectionName);
public static String validateCollectionName(String collectionName) {
return validateName(IdentifierType.COLLECTION, collectionName);
}
public static boolean validateCoreName(String name) {
return validateIdentifier(name);
public static String validateAliasName(String alias) {
return validateName(IdentifierType.ALIAS, alias);
}
public static String validateCoreName(String coreName) {
return validateName(IdentifierType.CORE, coreName);
}
private static boolean validateIdentifier(String identifier) {
if (identifier == null || ! identifierPattern.matcher(identifier).matches()) {
return false;