From a044b12cf5b63e560ee1869d3eab16d153dcf118 Mon Sep 17 00:00:00 2001 From: Shai Erera Date: Thu, 26 Mar 2015 20:18:14 +0000 Subject: [PATCH] SOLR-7298: Fix Collections API SolrJ calls to not add name parameter when not needed git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1669426 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 + .../solrj/request/CollectionAdminRequest.java | 142 +++++++++---- .../solr/common/params/CoreAdminParams.java | 3 + ...lectionAdminRequestRequiredParamsTest.java | 200 ++++++++++++++++++ 4 files changed, 310 insertions(+), 38 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cfa246f6f95..df57006efef 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -328,6 +328,9 @@ Bug Fixes * SOLR-7293: Fix bug that Solr server does not listen on IPv6 interfaces by default. (Uwe Schindler, Sebastian Pesman) +* SOLR-7298: Fix Collections API calls (SolrJ) to not add name parameter when not needed. + (Shai Erera, Anshum Gupta) + Optimizations ---------------------- diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java index 1ad0b26c78a..18d9538c847 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -47,27 +47,25 @@ public class CollectionAdminRequest extends SolrRequest private static String PROPERTY_PREFIX = "property."; - protected void setAction( CollectionAction action ) { + protected void setAction(CollectionAction action) { this.action = action; } - public CollectionAdminRequest() - { - super( METHOD.GET, "/admin/collections" ); + public CollectionAdminRequest() { + super(METHOD.GET, "/admin/collections"); } - public CollectionAdminRequest( String path ) - { - super( METHOD.GET, path ); + public CollectionAdminRequest(String path) { + super(METHOD.GET, path); } @Override public SolrParams getParams() { - if( action == null ) { + if (action == null) { throw new RuntimeException( "no action specified!" ); } ModifiableSolrParams params = new ModifiableSolrParams(); - params.set( CoreAdminParams.ACTION, action.toString() ); + params.set(CoreAdminParams.ACTION, action.toString()); return params; } @@ -98,8 +96,7 @@ public class CollectionAdminRequest extends SolrRequest protected static class CollectionSpecificAdminRequest extends CollectionAdminRequest { protected String collection = null; - public final void setCollectionName( String collectionName ) - { + public final void setCollectionName(String collectionName) { this.collection = collectionName; } @@ -113,16 +110,30 @@ public class CollectionAdminRequest extends SolrRequest } - protected static class CollectionShardAdminRequest extends CollectionSpecificAdminRequest { + protected static class CollectionShardAdminRequest extends CollectionAdminRequest { protected String shardName = null; + protected String collection = null; - public void setShardName(String shard) { this.shardName = shard; } - public String getShardName() { return this.shardName; } + public void setCollectionName(String collectionName) { + this.collection = collectionName; + } + + public String getCollectionName() { + return collection; + } + + public void setShardName(String shard) { + this.shardName = shard; + } + + public String getShardName() { + return this.shardName; + } public ModifiableSolrParams getCommonParams() { ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); - params.set( "collection", collection ); - params.set( "shard", shardName); + params.set(CoreAdminParams.COLLECTION, collection); + params.set(CoreAdminParams.SHARD, shardName); return params; } @@ -180,7 +191,6 @@ public class CollectionAdminRequest extends SolrRequest protected Integer stateFormat; protected String asyncId; - public Create() { action = CollectionAction.CREATE; } @@ -297,8 +307,10 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = getCommonParams(); - params.set( "createNodeSet", nodeSet); - if(properties != null) { + if (nodeSet != null) { + params.set("createNodeSet", nodeSet); + } + if (properties != null) { addProperties(params, properties); } return params; @@ -376,13 +388,18 @@ public class CollectionAdminRequest extends SolrRequest action = CollectionAction.REQUESTSTATUS; } - public void setRequestId(String requestId) {this.requestId = requestId; } - public String getRequestId() { return this.requestId; } + public void setRequestId(String requestId) { + this.requestId = requestId; + } + + public String getRequestId() { + return this.requestId; + } @Override public SolrParams getParams() { ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); - params.set("requestid", requestId); + params.set(CoreAdminParams.REQUESTID, requestId); return params; } } @@ -404,8 +421,13 @@ public class CollectionAdminRequest extends SolrRequest return aliasName; } - public void setAliasedCollections(String alias) { this.aliasedCollections = alias; } - public String getAliasedCollections() { return this.aliasedCollections; } + public void setAliasedCollections(String alias) { + this.aliasedCollections = alias; + } + + public String getAliasedCollections() { + return this.aliasedCollections; + } @Deprecated public void setCollectionName(String aliasName) { @@ -415,8 +437,8 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); - params.set("name", aliasName); - params.set( "collections", aliasedCollections ); + params.set(CoreAdminParams.NAME, aliasName); + params.set("collections", aliasedCollections); return params; } } @@ -436,7 +458,7 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); - params.set("name", aliasName); + params.set(CoreAdminParams.NAME, aliasName); return params; } } @@ -498,13 +520,15 @@ public class CollectionAdminRequest extends SolrRequest public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); if (shardName == null || shardName.isEmpty()) { - params.remove("shard"); + params.remove(CoreAdminParams.SHARD); if (routeKey == null) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Either shard or routeKey must be provided"); } params.add(ShardParams._ROUTE_, routeKey); } - params.set("async", asyncId); + if (asyncId != null) { + params.set("async", asyncId); + } if (node != null) { params.add("node", node); } @@ -591,9 +615,10 @@ public class CollectionAdminRequest extends SolrRequest return this.propertyValue; } + @Override public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); - params.add("name", propertyName); + params.add(CoreAdminParams.NAME, propertyName); params.add("val", propertyValue); return params; @@ -602,7 +627,8 @@ public class CollectionAdminRequest extends SolrRequest } // MIGRATE request - public static class Migrate extends CollectionSpecificAdminRequest { + public static class Migrate extends CollectionAdminRequest { + private String collection; private String targetCollection; private String splitKey; private Integer forwardTimeout; @@ -613,6 +639,14 @@ public class CollectionAdminRequest extends SolrRequest action = CollectionAction.MIGRATE; } + public void setCollectionName(String collection) { + this.collection = collection; + } + + public String getCollectionName() { + return collection; + } + public void setTargetCollection(String targetCollection) { this.targetCollection = targetCollection; } @@ -648,15 +682,15 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); - params.set( "collection", collection ); + params.set(CoreAdminParams.COLLECTION, collection); params.set("target.collection", targetCollection); params.set("split.key", splitKey); - if(forwardTimeout != null) { + if (forwardTimeout != null) { params.set("forward.timeout", forwardTimeout); } params.set("async", asyncId); - if(properties != null) { + if (properties != null) { addProperties(params, properties); } @@ -694,12 +728,43 @@ public class CollectionAdminRequest extends SolrRequest } // CLUSTERSTATUS request - public static class ClusterStatus extends CollectionShardAdminRequest { + public static class ClusterStatus extends CollectionAdminRequest { + + protected String shardName = null; + protected String collection = null; public ClusterStatus () { action = CollectionAction.CLUSTERSTATUS; } + public void setCollectionName(String collectionName) { + this.collection = collectionName; + } + + public String getCollectionName() { + return collection; + } + + public void setShardName(String shard) { + this.shardName = shard; + } + + public String getShardName() { + return this.shardName; + } + + @Override + public SolrParams getParams() { + ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + if (collection != null) { + params.set(CoreAdminParams.COLLECTION, collection); + } + if (shardName != null) { + params.set(CoreAdminParams.SHARD, shardName); + } + return params; + } + } // LIST request @@ -755,12 +820,13 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); - params.set("replica", replica); + params.set(CoreAdminParams.REPLICA, replica); params.set("property", propertyName); params.set("property.value", propertyValue); - if(shardUnique != null) + if (shardUnique != null) { params.set("shardUnique", shardUnique); + } return params; } @@ -847,7 +913,7 @@ public class CollectionAdminRequest extends SolrRequest @Override public SolrParams getParams() { ModifiableSolrParams params = new ModifiableSolrParams(super.getParams()); - params.set("collection", collection); + params.set(CoreAdminParams.COLLECTION, collection); params.set("property", propertyName); if(onlyActiveNodes != null) params.set("onlyactivenodes", onlyActiveNodes); diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java index f673c6c9a71..fcafd92a6dd 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java @@ -74,6 +74,9 @@ public abstract class CoreAdminParams /** The collection name in solr cloud */ public final static String COLLECTION = "collection"; + /** The replica name in solr cloud */ + public final static String REPLICA = "replica"; + /** The shard id in solr cloud */ public final static String SHARD = "shard"; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java new file mode 100644 index 00000000000..c0596b57986 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java @@ -0,0 +1,200 @@ +package org.apache.solr.client.solrj; + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import static org.apache.solr.common.params.CoreAdminParams.*; + +import java.util.Iterator; +import java.util.Set; + +import org.apache.lucene.util.LuceneTestCase; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.common.params.ShardParams; +import org.apache.solr.common.params.SolrParams; + +import com.google.common.collect.Sets; + +/** + * Tests that default {@link CollectionAdminRequest#getParams()} returns only + * the required parameters of this request, and none other. + */ +public class CollectionAdminRequestRequiredParamsTest extends LuceneTestCase { + + public void testBalanceShardUnique() { + final CollectionAdminRequest.BalanceShardUnique request = new CollectionAdminRequest.BalanceShardUnique(); + request.setCollection("foo"); + request.setPropertyName("prop"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "property"); + } + + public void testClusterProp() { + final CollectionAdminRequest.ClusterProp request = new CollectionAdminRequest.ClusterProp(); + request.setPropertyName("foo"); + request.setPropertyValue("bar"); + assertContainsParams(request.getParams(), ACTION, NAME, "val"); + } + + public void testAddRole() { + final CollectionAdminRequest.AddRole request = new CollectionAdminRequest.AddRole(); + request.setNode("node"); + request.setRole("role"); + assertContainsParams(request.getParams(), ACTION, "node", "role"); + } + + public void testRemoveRole() { + final CollectionAdminRequest.RemoveRole request = new CollectionAdminRequest.RemoveRole(); + request.setNode("node"); + request.setRole("role"); + assertContainsParams(request.getParams(), ACTION, "node", "role"); + } + + public void testAddReplica() { + // with shard parameter + CollectionAdminRequest.AddReplica request = new CollectionAdminRequest.AddReplica(); + request.setShardName("shard"); + request.setCollectionName("collection"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + + // with route parameter + request = new CollectionAdminRequest.AddReplica(); + request.setRouteKey("route"); + request.setCollectionName("collection"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, ShardParams._ROUTE_); + } + + public void testAddReplicaProp() { + final CollectionAdminRequest.AddReplicaProp request = new CollectionAdminRequest.AddReplicaProp(); + request.setShardName("shard"); + request.setCollectionName("collection"); + request.setReplica("replica"); + request.setPropertyName("prop"); + request.setPropertyValue("value"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property", "property.value"); + } + + public void testClusterStatus() { + final CollectionAdminRequest.ClusterStatus request = new CollectionAdminRequest.ClusterStatus(); + assertContainsParams(request.getParams(), ACTION); + } + + public void testCreateShard() { + final CollectionAdminRequest.CreateShard request = new CollectionAdminRequest.CreateShard(); + request.setCollectionName("collection"); + request.setShardName("shard"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + } + + public void testDeleteReplica() { + final CollectionAdminRequest.DeleteReplica request = new CollectionAdminRequest.DeleteReplica(); + request.setCollectionName("collection"); + request.setShardName("shard"); + request.setReplica("replica"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA); + } + + public void testDeleteReplicaProp() { + final CollectionAdminRequest.DeleteReplicaProp request = new CollectionAdminRequest.DeleteReplicaProp(); + request.setCollectionName("collection"); + request.setShardName("shard"); + request.setReplica("replica"); + request.setPropertyName("foo"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property"); + } + + public void testDeleteShard() { + final CollectionAdminRequest.DeleteShard request = new CollectionAdminRequest.DeleteShard(); + request.setCollectionName("collection"); + request.setShardName("shard"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + } + + public void testSplitShard() { + final CollectionAdminRequest.SplitShard request = new CollectionAdminRequest.SplitShard(); + request.setCollectionName("collection"); + request.setShardName("shard"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + } + + public void testCreateCollection() { + final CollectionAdminRequest.Create request = new CollectionAdminRequest.Create(); + request.setCollectionName("collection"); + assertContainsParams(request.getParams(), ACTION, NAME); + } + + public void testReloadCollection() { + final CollectionAdminRequest.Reload request = new CollectionAdminRequest.Reload(); + request.setCollectionName("collection"); + assertContainsParams(request.getParams(), ACTION, NAME); + } + + public void testDeleteCollection() { + final CollectionAdminRequest.Delete request = new CollectionAdminRequest.Delete(); + request.setCollectionName("collection"); + assertContainsParams(request.getParams(), ACTION, NAME); + } + + public void testCreateAlias() { + final CollectionAdminRequest.CreateAlias request = new CollectionAdminRequest.CreateAlias(); + request.setAliasName("name"); + request.setAliasedCollections("collections"); + assertContainsParams(request.getParams(), ACTION, NAME, "collections"); + } + + public void testDeleteAlias() { + final CollectionAdminRequest.DeleteAlias request = new CollectionAdminRequest.DeleteAlias(); + request.setAliasName("name"); + assertContainsParams(request.getParams(), ACTION, NAME); + } + + public void testListCollections() { + final CollectionAdminRequest.List request = new CollectionAdminRequest.List(); + assertContainsParams(request.getParams(), ACTION); + } + + public void testMigrate() { + final CollectionAdminRequest.Migrate request = new CollectionAdminRequest.Migrate(); + request.setCollectionName("collection"); + request.setTargetCollection("target"); + request.setSplitKey("splitKey"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "target.collection", "split.key"); + } + + public void testOverseerStatus() { + final CollectionAdminRequest.OverseerStatus request = new CollectionAdminRequest.OverseerStatus(); + assertContainsParams(request.getParams(), ACTION); + } + + public void testRequestStatus() { + final CollectionAdminRequest.RequestStatus request = new CollectionAdminRequest.RequestStatus(); + request.setRequestId("request"); + assertContainsParams(request.getParams(), ACTION, REQUESTID); + } + + private void assertContainsParams(SolrParams solrParams, String... requiredParams) { + final Set requiredParamsSet = Sets.newHashSet(requiredParams); + final Set solrParamsSet = Sets.newHashSet(); + for (Iterator iter = solrParams.getParameterNamesIterator(); iter.hasNext();) { + solrParamsSet.add(iter.next()); + } + assertTrue("required params missing: required=" + requiredParamsSet + ", params=" + solrParamsSet, + solrParamsSet.containsAll(requiredParamsSet)); + assertTrue("extra parameters included in request: required=" + requiredParamsSet + ", params=" + solrParams, + Sets.difference(solrParamsSet, requiredParamsSet).isEmpty()); + } + +}