You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2015/03/26 21:18:15 UTC
svn commit: r1669426 - in /lucene/dev/trunk/solr: ./
solrj/src/java/org/apache/solr/client/solrj/request/
solrj/src/java/org/apache/solr/common/params/
solrj/src/test/org/apache/solr/client/solrj/
Author: shaie
Date: Thu Mar 26 20:18:14 2015
New Revision: 1669426
URL: http://svn.apache.org/r1669426
Log:
SOLR-7298: Fix Collections API SolrJ calls to not add name parameter when not needed
Added:
lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1669426&r1=1669425&r2=1669426&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Thu Mar 26 20:18:14 2015
@@ -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
----------------------
Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java?rev=1669426&r1=1669425&r2=1669426&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java Thu Mar 26 20:18:14 2015
@@ -47,27 +47,25 @@ public class CollectionAdminRequest exte
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 exte
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 exte
}
- 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 exte
protected Integer stateFormat;
protected String asyncId;
-
public Create() {
action = CollectionAction.CREATE;
}
@@ -297,8 +307,10 @@ public class CollectionAdminRequest exte
@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 exte
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 exte
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 exte
@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 exte
@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 exte
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 exte
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 exte
}
// 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 exte
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 exte
@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 exte
}
// 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 exte
@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 exte
@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);
Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java?rev=1669426&r1=1669425&r2=1669426&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java Thu Mar 26 20:18:14 2015
@@ -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";
Added: lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java?rev=1669426&view=auto
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java (added)
+++ lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java Thu Mar 26 20:18:14 2015
@@ -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<String> requiredParamsSet = Sets.newHashSet(requiredParams);
+ final Set<String> solrParamsSet = Sets.newHashSet();
+ for (Iterator<String> 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());
+ }
+
+}