Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-15141: Remove V2RequestSupport SolrJ interface #2311

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ Other Changes
* SOLR-14067: StatelessScriptUpdateProcessorFactory moved to it's own /contrib/scripting/ package instead
of shipping as part of Solr due to security concerns. Renamed to ScriptUpdateProcessorFactory for simpler name. (Eric Pugh)

* SOLR-15141: The V2RequestSupport interface has been removed, along with the accompanying `setUseV2Request` SolrRequest
method. v2 APIs will be supported going forward in a way that doesn't require users to request v2-conversion. (Jason Gerlowski)

Bug Fixes
---------------------
* SOLR-14546: Fix for a relatively hard to hit issue in OverseerTaskProcessor that could lead to out of order execution
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,13 @@
*/
package org.apache.solr.cloud.api.collections;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;

import com.google.common.collect.Lists;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.V2Request;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.cloud.ZkTestServer;
import org.apache.solr.common.SolrException;
Expand All @@ -49,6 +40,13 @@
import org.apache.zookeeper.KeeperException;
import org.junit.Test;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public class TestCollectionAPI extends ReplicaPropertiesBase {

public static final String COLLECTION_NAME = "testcollection";
Expand All @@ -69,9 +67,7 @@ public void test() throws Exception {
} else {
req = CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf1",2, 1, 0, 1);
}
setV2(req);
client.request(req);
assertV2CallsCount();
createCollection(null, COLLECTION_NAME1, 1, 1, client, null, "conf1");
}

Expand Down Expand Up @@ -414,8 +410,7 @@ private void clusterStatusWithCollection() throws IOException, SolrServerExcepti
private void clusterStatusZNodeVersion() throws Exception {
String cname = "clusterStatusZNodeVersion";
try (CloudSolrClient client = createCloudClient(null)) {
setV2(CollectionAdminRequest.createCollection(cname, "conf1", 1, 1)).process(client);
assertV2CallsCount();
CollectionAdminRequest.createCollection(cname, "conf1", 1, 1).process(client);
waitForRecoveriesToFinish(cname, true);

ModifiableSolrParams params = new ModifiableSolrParams();
Expand All @@ -438,9 +433,7 @@ private void clusterStatusZNodeVersion() throws Exception {
assertNotNull(znodeVersion);

CollectionAdminRequest.AddReplica addReplica = CollectionAdminRequest.addReplicaToShard(cname, "shard1");
setV2(addReplica);
addReplica.process(client);
assertV2CallsCount();
waitForRecoveriesToFinish(cname, true);

rsp = client.request(request);
Expand All @@ -453,24 +446,6 @@ private void clusterStatusZNodeVersion() throws Exception {
}
}

private static long totalexpectedV2Calls;

@SuppressWarnings({"rawtypes"})
public static SolrRequest setV2(SolrRequest req) {
if (V2Request.v2Calls.get() == null) V2Request.v2Calls.set(new AtomicLong());
totalexpectedV2Calls = V2Request.v2Calls.get().get();
if (random().nextBoolean()) {
req.setUseV2(true);
req.setUseBinaryV2(random().nextBoolean());
totalexpectedV2Calls++;
}
return req;
}

public static void assertV2CallsCount() {
assertEquals(totalexpectedV2Calls, V2Request.v2Calls.get().get());
}

private void clusterStatusWithRouteKey() throws IOException, SolrServerException {
try (CloudSolrClient client = createCloudClient(DEFAULT_COLLECTION)) {
SolrInputDocument doc = new SolrInputDocument();
Expand Down
27 changes: 4 additions & 23 deletions solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
*/
package org.apache.solr.client.solrj;

import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;

import java.io.IOException;
import java.io.Serializable;
import java.security.Principal;
Expand All @@ -26,10 +30,6 @@
import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;

/**
*
*
Expand Down Expand Up @@ -82,25 +82,6 @@ public enum SolrClientContext {
private StreamingResponseCallback callback;
private Set<String> queryParams;

protected boolean usev2;
protected boolean useBinaryV2;

/**If set to true, every request that implements {@link V2RequestSupport} will be converted
* to a V2 API call
*/
@SuppressWarnings({"rawtypes"})
public SolrRequest setUseV2(boolean flag){
this.usev2 = flag;
return this;
}

/**If set to true use javabin instead of json (default)
*/
@SuppressWarnings({"rawtypes"})
public SolrRequest setUseBinaryV2(boolean flag){
this.useBinaryV2 = flag;
return this;
}

private String basicAuthUser, basicAuthPwd;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -877,9 +877,6 @@ protected NamedList<Object> requestWithRetryOnStaleState(@SuppressWarnings({"raw
String stateVerParam = null;
List<DocCollection> requestedCollections = null;
boolean isCollectionRequestOfV2 = false;
if (request instanceof V2RequestSupport) {
request = ((V2RequestSupport) request).getV2Request();
}
if (request instanceof V2Request) {
isCollectionRequestOfV2 = ((V2Request) request).isPerCollectionRequest();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,6 @@ private Request createRequest(@SuppressWarnings({"rawtypes"})SolrRequest solrReq
if (solrRequest.getBasePath() == null && serverBaseUrl == null)
throw new IllegalArgumentException("Destination node is not provided!");

if (solrRequest instanceof V2RequestSupport) {
solrRequest = ((V2RequestSupport) solrRequest).getV2Request();
}
SolrParams params = solrRequest.getParams();
RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest);
Collection<ContentStream> streams = contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,6 @@ static String changeV2RequestEndpoint(String basePath) throws MalformedURLExcept

@SuppressWarnings({"unchecked"})
protected HttpRequestBase createMethod(@SuppressWarnings({"rawtypes"})SolrRequest request, String collection) throws IOException, SolrServerException {
if (request instanceof V2RequestSupport) {
request = ((V2RequestSupport) request).getV2Request();
}
SolrParams params = request.getParams();
RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(request);
Collection<ContentStream> streams = contentWriter == null ? requestWriter.getContentStreams(request) : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,11 @@
*/
package org.apache.solr.client.solrj.request;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.TimeZone;
import java.util.UUID;
import java.util.concurrent.TimeUnit;

import org.apache.solr.client.solrj.RoutedAliasTypes;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.V2RequestSupport;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.response.RequestStatusState;
import org.apache.solr.client.solrj.util.SolrIdentifierValidator;
Expand All @@ -54,26 +40,29 @@
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.TimeZone;
import java.util.UUID;
import java.util.concurrent.TimeUnit;

import static org.apache.solr.common.cloud.DocCollection.PER_REPLICA_STATE;
import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
import static org.apache.solr.common.cloud.ZkStateReader.READ_ONLY;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR;
import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
import static org.apache.solr.common.params.CollectionAdminParams.ALIAS;
import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
import static org.apache.solr.common.params.CollectionAdminParams.COUNT_PROP;
import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM;
import static org.apache.solr.common.params.CollectionAdminParams.ROUTER_PREFIX;
import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
import static org.apache.solr.common.cloud.ZkStateReader.*;
import static org.apache.solr.common.params.CollectionAdminParams.*;

/**
* This class is experimental and subject to change.
*
* @since solr 4.5
*/
public abstract class CollectionAdminRequest<T extends CollectionAdminResponse> extends SolrRequest<T> implements V2RequestSupport, MapWriter {
public abstract class CollectionAdminRequest<T extends CollectionAdminResponse> extends SolrRequest<T> implements MapWriter {

/**
* The set of modifiable collection properties
Expand All @@ -97,14 +86,6 @@ public CollectionAdminRequest(String path, CollectionAction action) {
this.action = checkNotNull(CoreAdminParams.ACTION, action);
}

@Override
@SuppressWarnings({"rawtypes"})
public SolrRequest getV2Request() {
return usev2 ?
V1toV2ApiMapper.convert(this).useBinary(useBinaryV2).build() :
this;
}

@Override
public SolrParams getParams() {
ModifiableSolrParams params = new ModifiableSolrParams();
Expand Down