From 30e9b51af71161d2e775ec438e0d14295537cf34 Mon Sep 17 00:00:00 2001 From: Anshum Gupta Date: Wed, 5 Jul 2017 11:46:22 -0700 Subject: [PATCH] SOLR-11004: Consolidate SolrClient builder code into an abstract base class to reduce duplication. --- solr/CHANGES.txt | 3 + .../client/solrj/impl/CloudSolrClient.java | 43 ++--------- .../impl/ConcurrentUpdateSolrClient.java | 43 ++--------- .../client/solrj/impl/HttpSolrClient.java | 51 ++----------- .../client/solrj/impl/LBHttpSolrClient.java | 52 ++------------ .../client/solrj/impl/SolrClientBuilder.java | 72 +++++++++++++++++++ 6 files changed, 99 insertions(+), 165 deletions(-) create mode 100644 solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8d90b0249e9..296f470bfb9 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -449,6 +449,9 @@ Other Changes * SOLR-10456: Deprecate timeout related setters from SolrClients, and replace with Builder based implementation (Jason Gerlowski, Anshum Gupta) +* SOLR-11004: Consolidate SolrClient builder code in an abstract base class (Jason Gerlowski, + Anshum Gupta) + ================== 6.7.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index e660fd5a1c5..9948857e4a2 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -1367,18 +1367,15 @@ public class CloudSolrClient extends SolrClient { /** * Constructs {@link CloudSolrClient} instances from provided configuration. */ - public static class Builder { + public static class Builder extends SolrClientBuilder { protected Collection zkHosts; protected List solrUrls; - protected HttpClient httpClient; protected String zkChroot; protected LBHttpSolrClient loadBalancedSolrClient; protected LBHttpSolrClient.Builder lbClientBuilder; protected boolean shardLeadersOnly; protected boolean directUpdatesToLeadersOnly; protected ClusterStateProvider stateProvider; - protected Integer connectionTimeoutMillis; - protected Integer socketTimeoutMillis; public Builder() { @@ -1436,15 +1433,6 @@ public class CloudSolrClient extends SolrClient { return this; } - /** - * Provides a {@link HttpClient} for the builder to use when creating clients. - */ - public Builder withHttpClient(HttpClient httpClient) { - this.httpClient = httpClient; - return this; - } - - /** * Provide a series of ZooKeeper client endpoints for the builder to use when creating clients. * @@ -1515,30 +1503,6 @@ public class CloudSolrClient extends SolrClient { return this; } - /** - * Tells {@link Builder} that created clients should obey the following timeout when connecting to Solr servers. - */ - public Builder withConnectionTimeout(int connectionTimeoutMillis) { - if (connectionTimeoutMillis <= 0) { - throw new IllegalArgumentException("connectionTimeoutMillis must be a positive integer."); - } - - this.connectionTimeoutMillis = connectionTimeoutMillis; - return this; - } - - /** - * Tells {@link Builder} that created clients should set the following read timeout on all sockets. - */ - public Builder withSocketTimeout(int socketTimeoutMillis) { - if (socketTimeoutMillis <= 0) { - throw new IllegalArgumentException("socketTimeoutMillis must be a positive integer."); - } - - this.socketTimeoutMillis = socketTimeoutMillis; - return this; - } - /** * Create a {@link CloudSolrClient} based on the provided configuration. */ @@ -1560,5 +1524,10 @@ public class CloudSolrClient extends SolrClient { } return new CloudSolrClient(this); } + + @Override + public Builder getThis() { + return this; + } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java index 2d9bfb105f5..f14d9533f33 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java @@ -770,15 +770,12 @@ public class ConcurrentUpdateSolrClient extends SolrClient { /** * Constructs {@link ConcurrentUpdateSolrClient} instances from provided configuration. */ - public static class Builder { + public static class Builder extends SolrClientBuilder { protected String baseSolrUrl; - protected HttpClient httpClient; protected int queueSize; protected int threadCount; protected ExecutorService executorService; protected boolean streamDeletes; - protected Integer connectionTimeoutMillis; - protected Integer socketTimeoutMillis; /** * Create a Builder object, based on the provided Solr URL. @@ -804,14 +801,6 @@ public class ConcurrentUpdateSolrClient extends SolrClient { public Builder(String baseSolrUrl) { this.baseSolrUrl = baseSolrUrl; } - - /** - * Provides a {@link HttpClient} for the builder to use when creating clients. - */ - public Builder withHttpClient(HttpClient httpClient) { - this.httpClient = httpClient; - return this; - } /** * The number of documents to batch together before sending to Solr. @@ -860,31 +849,6 @@ public class ConcurrentUpdateSolrClient extends SolrClient { return this; } - /** - * Tells {@link Builder} that created clients should obey the following timeout when connecting to Solr servers. - */ - public Builder withConnectionTimeout(int connectionTimeoutMillis) { - if (connectionTimeoutMillis <= 0) { - throw new IllegalArgumentException("connectionTimeoutMillis must be a positive integer."); - } - - this.connectionTimeoutMillis = connectionTimeoutMillis; - return this; - } - - /** - * Tells {@link Builder} that created clients should set the following read timeout on all sockets. - */ - public Builder withSocketTimeout(int socketTimeoutMillis) { - if (socketTimeoutMillis <= 0) { - throw new IllegalArgumentException("socketTimeoutMillis must be a positive integer."); - } - - this.socketTimeoutMillis = socketTimeoutMillis; - return this; - } - - /** * Create a {@link ConcurrentUpdateSolrClient} based on the provided configuration options. */ @@ -895,5 +859,10 @@ public class ConcurrentUpdateSolrClient extends SolrClient { return new ConcurrentUpdateSolrClient(this); } + + @Override + public Builder getThis() { + return this; + } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index 1b0354f2732..7566ba047c3 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -827,14 +827,10 @@ s * @deprecated since 7.0 Use {@link Builder} methods instead. /** * Constructs {@link HttpSolrClient} instances from provided configuration. */ - public static class Builder { + public static class Builder extends SolrClientBuilder { protected String baseSolrUrl; - protected HttpClient httpClient; - protected ResponseParser responseParser; protected boolean compression; protected ModifiableSolrParams invariantParams = new ModifiableSolrParams(); - protected Integer connectionTimeoutMillis; - protected Integer socketTimeoutMillis; public Builder() { this.responseParser = new BinaryResponseParser(); @@ -894,22 +890,6 @@ s * @deprecated since 7.0 Use {@link Builder} methods instead. this.responseParser = new BinaryResponseParser(); } - /** - * Provides a {@link HttpClient} for the builder to use when creating clients. - */ - public Builder withHttpClient(HttpClient httpClient) { - this.httpClient = httpClient; - return this; - } - - /** - * Provides a {@link ResponseParser} for created clients to use when handling requests. - */ - public Builder withResponseParser(ResponseParser responseParser) { - this.responseParser = responseParser; - return this; - } - /** * Chooses whether created {@link HttpSolrClient}s use compression by default. */ @@ -941,30 +921,6 @@ s * @deprecated since 7.0 Use {@link Builder} methods instead. this.invariantParams.add(params); return this; } - - /** - * Tells {@link Builder} that created clients should obey the following timeout when connecting to Solr servers. - */ - public Builder withConnectionTimeout(int connectionTimeoutMillis) { - if (connectionTimeoutMillis <= 0) { - throw new IllegalArgumentException("connectionTimeoutMillis must be a positive integer."); - } - - this.connectionTimeoutMillis = connectionTimeoutMillis; - return this; - } - - /** - * Tells {@link Builder} that created clients should set the following read timeout on all sockets. - */ - public Builder withSocketTimeout(int socketTimeoutMillis) { - if (socketTimeoutMillis <= 0) { - throw new IllegalArgumentException("socketTimeoutMillis must be a positive integer."); - } - - this.socketTimeoutMillis = socketTimeoutMillis; - return this; - } /** * Create a {@link HttpSolrClient} based on provided configuration. @@ -980,5 +936,10 @@ s * @deprecated since 7.0 Use {@link Builder} methods instead. return new DelegationTokenHttpSolrClient(this); } } + + @Override + public Builder getThis() { + return this; + } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java index 2dfd4b4a9e9..6c2737d334c 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java @@ -881,13 +881,9 @@ public class LBHttpSolrClient extends SolrClient { /** * Constructs {@link LBHttpSolrClient} instances from provided configuration. */ - public static class Builder { + public static class Builder extends SolrClientBuilder { protected final List baseSolrUrls; - protected HttpClient httpClient; - protected ResponseParser responseParser; protected HttpSolrClient.Builder httpSolrClientBuilder; - protected Integer connectionTimeoutMillis; - protected Integer socketTimeoutMillis; public Builder() { this.baseSolrUrls = new ArrayList<>(); @@ -955,23 +951,6 @@ public class LBHttpSolrClient extends SolrClient { } return this; } - - - /** - * Provides a {@link HttpClient} for the builder to use when creating clients. - */ - public Builder withHttpClient(HttpClient httpClient) { - this.httpClient = httpClient; - return this; - } - - /** - * Provides a {@link ResponseParser} for created clients to use when handling requests. - */ - public Builder withResponseParser(ResponseParser responseParser) { - this.responseParser = responseParser; - return this; - } /** * Provides a {@link HttpSolrClient.Builder} to be used for building the internally used clients. @@ -980,30 +959,6 @@ public class LBHttpSolrClient extends SolrClient { this.httpSolrClientBuilder = builder; return this; } - - /** - * Tells {@link Builder} that created clients should obey the following timeout when connecting to Solr servers. - */ - public Builder withConnectionTimeout(int connectionTimeoutMillis) { - if (connectionTimeoutMillis <= 0) { - throw new IllegalArgumentException("connectionTimeoutMillis must be a positive integer."); - } - - this.connectionTimeoutMillis = connectionTimeoutMillis; - return this; - } - - /** - * Tells {@link Builder} that created clients should set the following read timeout on all sockets. - */ - public Builder withSocketTimeout(int socketTimeoutMillis) { - if (socketTimeoutMillis <= 0) { - throw new IllegalArgumentException("socketTimeoutMillis must be a positive integer."); - } - - this.socketTimeoutMillis = socketTimeoutMillis; - return this; - } /** * Create a {@link HttpSolrClient} based on provided configuration. @@ -1011,5 +966,10 @@ public class LBHttpSolrClient extends SolrClient { public LBHttpSolrClient build() { return new LBHttpSolrClient(this); } + + @Override + public Builder getThis() { + return this; + } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java new file mode 100644 index 00000000000..ec149bd39fb --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java @@ -0,0 +1,72 @@ +/* + * 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. + */ +package org.apache.solr.client.solrj.impl; + +import org.apache.http.client.HttpClient; +import org.apache.solr.client.solrj.ResponseParser; +import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder; + +public abstract class SolrClientBuilder> { + + protected HttpClient httpClient; + protected ResponseParser responseParser; + protected Integer connectionTimeoutMillis; + protected Integer socketTimeoutMillis; + + /** The solution for the unchecked cast warning. */ + public abstract B getThis(); + + /** + * Provides a {@link HttpClient} for the builder to use when creating clients. + */ + public B withHttpClient(HttpClient httpClient) { + this.httpClient = httpClient; + return getThis(); + } + + /** + * Provides a {@link ResponseParser} for created clients to use when handling requests. + */ + public B withResponseParser(ResponseParser responseParser) { + this.responseParser = responseParser; + return getThis(); + } + + /** + * Tells {@link Builder} that created clients should obey the following timeout when connecting to Solr servers. + */ + public B withConnectionTimeout(int connectionTimeoutMillis) { + if (connectionTimeoutMillis <= 0) { + throw new IllegalArgumentException("connectionTimeoutMillis must be a positive integer."); + } + + this.connectionTimeoutMillis = connectionTimeoutMillis; + return getThis(); + } + + /** + * Tells {@link Builder} that created clients should set the following read timeout on all sockets. + */ + public B withSocketTimeout(int socketTimeoutMillis) { + if (socketTimeoutMillis <= 0) { + throw new IllegalArgumentException("socketTimeoutMillis must be a positive integer."); + } + + this.socketTimeoutMillis = socketTimeoutMillis; + return getThis(); + } +}