From c902837bb237e579f9fdd0db0a8b35680b294ad1 Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Wed, 9 Sep 2020 09:19:48 -0500 Subject: [PATCH] SOLR-14846 Clean up Optional use (#1843) * Remove Optional.ofNullable.orElse pattern * Remove use of Optional as method parameter --- .../solr/cloud/api/collections/BackupCmd.java | 2 +- .../solr/cloud/api/collections/RestoreCmd.java | 3 +-- .../java/org/apache/solr/core/CoreContainer.java | 6 +++--- .../core/backup/repository/BackupRepository.java | 4 ++-- .../apache/solr/handler/ReplicationHandler.java | 5 ++--- .../apache/solr/handler/admin/BackupCoreOp.java | 3 +-- .../solr/handler/admin/CollectionsHandler.java | 5 ++--- .../apache/solr/handler/admin/RestoreCoreOp.java | 3 +-- .../apache/solr/search/facet/RelatednessAgg.java | 7 +++---- .../ConfigurableInternodeAuthHadoopPlugin.java | 3 +-- .../solrj/impl/HttpClientBuilderFactory.java | 3 +-- .../solr/client/solrj/impl/HttpClientUtil.java | 3 +-- .../client/solrj/impl/Krb5HttpClientBuilder.java | 5 ++--- .../PreemptiveBasicAuthClientBuilderFactory.java | 8 ++------ .../RequestReplicaListTransformerGenerator.java | 16 ++++++++-------- 15 files changed, 31 insertions(+), 45 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java index 6ff3797d3df..61be096277a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java @@ -81,7 +81,7 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd { Instant startTime = Instant.now(); CoreContainer cc = ocmh.overseer.getCoreContainer(); - BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo)); + BackupRepository repository = cc.newBackupRepository(repo); BackupManager backupMgr = new BackupManager(repository, ocmh.zkStateReader); // Backup location diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java index 552c40a0dea..0aa4389c9bb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java @@ -28,7 +28,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -95,7 +94,7 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd { String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY); CoreContainer cc = ocmh.overseer.getCoreContainer(); - BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo)); + BackupRepository repository = cc.newBackupRepository(repo); URI location = repository.createURI(message.getStr(CoreAdminParams.BACKUP_LOCATION)); URI backupPath = repository.resolve(location, backupName); diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 0453c6104ca..6480fa827ba 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -265,10 +265,10 @@ public class CoreContainer { * If not specified, a default implementation is used. * @return a new instance of {@linkplain BackupRepository}. */ - public BackupRepository newBackupRepository(Optional repositoryName) { + public BackupRepository newBackupRepository(String repositoryName) { BackupRepository repository; - if (repositoryName.isPresent()) { - repository = backupRepoFactory.newInstance(getResourceLoader(), repositoryName.get()); + if (repositoryName != null) { + repository = backupRepoFactory.newInstance(getResourceLoader(), repositoryName); } else { repository = backupRepoFactory.newInstance(getResourceLoader()); } diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java index 875be18559e..d750f590039 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java +++ b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java @@ -20,7 +20,6 @@ import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; import java.net.URI; -import java.util.Optional; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; @@ -48,7 +47,8 @@ public interface BackupRepository extends NamedListInitializedPlugin, Closeable * Otherwise return the default configuration value for the {@linkplain CoreAdminParams#BACKUP_LOCATION} parameter. */ default String getBackupLocation(String override) { - return Optional.ofNullable(override).orElse(getConfigProperty(CoreAdminParams.BACKUP_LOCATION)); + // If override is null and default backup location is unset, what do we do? + return override != null ? override : getConfigProperty(CoreAdminParams.BACKUP_LOCATION); } /** diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index 1cf89a90d33..b200ecc5001 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java @@ -39,7 +39,6 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Properties; import java.util.Random; import java.util.concurrent.ExecutorService; @@ -484,7 +483,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw CoreContainer cc = core.getCoreContainer(); BackupRepository repo = null; if (repoName != null) { - repo = cc.newBackupRepository(Optional.of(repoName)); + repo = cc.newBackupRepository(repoName); location = repo.getBackupLocation(location); if (location == null) { throw new IllegalArgumentException("location is required"); @@ -593,7 +592,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw CoreContainer cc = core.getCoreContainer(); BackupRepository repo = null; if (repoName != null) { - repo = cc.newBackupRepository(Optional.of(repoName)); + repo = cc.newBackupRepository(repoName); location = repo.getBackupLocation(location); if (location == null) { throw new IllegalArgumentException("location is required"); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java b/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java index 503eed03a3b..c295ad5d22d 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java @@ -18,7 +18,6 @@ package org.apache.solr.handler.admin; import java.net.URI; -import java.util.Optional; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CoreAdminParams; @@ -39,7 +38,7 @@ class BackupCoreOp implements CoreAdminHandler.CoreAdminOp { String name = params.required().get(NAME); String repoName = params.get(CoreAdminParams.BACKUP_REPOSITORY); - BackupRepository repository = it.handler.coreContainer.newBackupRepository(Optional.ofNullable(repoName)); + BackupRepository repository = it.handler.coreContainer.newBackupRepository(repoName); String location = repository.getBackupLocation(params.get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index 71380024839..4ef43cd2355 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -96,7 +96,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -1062,7 +1061,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission CoreContainer cc = h.coreContainer; String repo = req.getParams().get(CoreAdminParams.BACKUP_REPOSITORY); - BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo)); + BackupRepository repository = cc.newBackupRepository(repo); String location = repository.getBackupLocation(req.getParams().get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { @@ -1114,7 +1113,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission final CoreContainer cc = h.coreContainer; final String repo = req.getParams().get(CoreAdminParams.BACKUP_REPOSITORY); - final BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo)); + final BackupRepository repository = cc.newBackupRepository(repo); String location = repository.getBackupLocation(req.getParams().get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java index a53324ae98c..65214c2226a 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java @@ -18,7 +18,6 @@ package org.apache.solr.handler.admin; import java.net.URI; -import java.util.Optional; import org.apache.solr.cloud.CloudDescriptor; import org.apache.solr.cloud.ZkController; @@ -46,7 +45,7 @@ class RestoreCoreOp implements CoreAdminHandler.CoreAdminOp { } String repoName = params.get(CoreAdminParams.BACKUP_REPOSITORY); - BackupRepository repository = it.handler.coreContainer.newBackupRepository(Optional.ofNullable(repoName)); + BackupRepository repository = it.handler.coreContainer.newBackupRepository(repoName); String location = repository.getBackupLocation(params.get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { diff --git a/solr/core/src/java/org/apache/solr/search/facet/RelatednessAgg.java b/solr/core/src/java/org/apache/solr/search/facet/RelatednessAgg.java index df4d11f5abc..835de9bf1a7 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/RelatednessAgg.java +++ b/solr/core/src/java/org/apache/solr/search/facet/RelatednessAgg.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Map; import java.util.function.IntFunction; @@ -652,10 +651,10 @@ public class RelatednessAgg extends AggValueSource { public void merge(Object facetResult, Context mcontext) { @SuppressWarnings({"unchecked"}) final NamedList shardData = (NamedList)facetResult; + + final boolean shardImplied = Objects.requireNonNullElse((Boolean)shardData.remove(IMPLIED_KEY), false); - final boolean shardImplied = Optional.ofNullable((Boolean)shardData.remove(IMPLIED_KEY)).orElse(false); - - // regardless of wether this shard is implied, we want to know it's size info... + // regardless of whether this shard is implied, we want to know its size info... mergedData.incSizes((Long)shardData.remove(FG_SIZE), (Long)shardData.remove(BG_SIZE)); if (! shardImplied) { diff --git a/solr/core/src/java/org/apache/solr/security/ConfigurableInternodeAuthHadoopPlugin.java b/solr/core/src/java/org/apache/solr/security/ConfigurableInternodeAuthHadoopPlugin.java index 8fe00709b91..010fa16cca3 100644 --- a/solr/core/src/java/org/apache/solr/security/ConfigurableInternodeAuthHadoopPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/ConfigurableInternodeAuthHadoopPlugin.java @@ -19,7 +19,6 @@ package org.apache.solr.security; import java.io.IOException; import java.util.Map; import java.util.Objects; -import java.util.Optional; import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.impl.HttpClientBuilderFactory; @@ -60,7 +59,7 @@ public class ConfigurableInternodeAuthHadoopPlugin extends HadoopAuthPlugin impl @Override public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder builder) { - return factory.getHttpClientBuilder(Optional.ofNullable(builder)); + return factory.getHttpClientBuilder(builder); } @Override diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientBuilderFactory.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientBuilderFactory.java index b552f66872a..fa175ee1f1a 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientBuilderFactory.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientBuilderFactory.java @@ -17,7 +17,6 @@ package org.apache.solr.client.solrj.impl; import java.io.Closeable; -import java.util.Optional; /** * Factory interface for configuring {@linkplain SolrHttpClientBuilder}. This @@ -36,7 +35,7 @@ public interface HttpClientBuilderFactory extends Closeable { * by configured (optional). * @return the {@linkplain SolrHttpClientBuilder} */ - public SolrHttpClientBuilder getHttpClientBuilder(Optional builder); + public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder builder); public default void setup(Http2SolrClient client) { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java index c685a0d3302..31203f2d305 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java @@ -21,7 +21,6 @@ import java.io.InputStream; import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.util.List; -import java.util.Optional; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; @@ -170,7 +169,7 @@ public class HttpClientUtil { log.debug ("Using {}", factoryClassName); try { HttpClientBuilderFactory factory = (HttpClientBuilderFactory)Class.forName(factoryClassName).getConstructor().newInstance(); - httpClientBuilder = factory.getHttpClientBuilder(Optional.of(SolrHttpClientBuilder.create())); + httpClientBuilder = factory.getHttpClientBuilder(SolrHttpClientBuilder.create()); } catch (InstantiationException | IllegalAccessException | ClassNotFoundException | InvocationTargetException | NoSuchMethodException e) { throw new RuntimeException("Unable to instantiate Solr HttpClientBuilderFactory", e); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Krb5HttpClientBuilder.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Krb5HttpClientBuilder.java index 6c4b44a24ca..aa3eb85a847 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Krb5HttpClientBuilder.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Krb5HttpClientBuilder.java @@ -26,7 +26,6 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Set; import org.apache.http.HttpEntity; @@ -82,8 +81,8 @@ public class Krb5HttpClientBuilder implements HttpClientBuilderFactory { } @Override - public SolrHttpClientBuilder getHttpClientBuilder(Optional builder) { - return builder.isPresent() ? getBuilder(builder.get()) : getBuilder(); + public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder builder) { + return builder == null ? getBuilder() : getBuilder(builder); } private SPNEGOAuthentication createSPNEGOAuthentication() { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/PreemptiveBasicAuthClientBuilderFactory.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/PreemptiveBasicAuthClientBuilderFactory.java index 3ec0bfb0ee8..8df7248a6b6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/PreemptiveBasicAuthClientBuilderFactory.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/PreemptiveBasicAuthClientBuilderFactory.java @@ -23,7 +23,6 @@ import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; -import java.util.Optional; import java.util.Properties; import org.apache.http.auth.AuthScope; @@ -122,17 +121,14 @@ public class PreemptiveBasicAuthClientBuilderFactory implements HttpClientBuilde } @Override - public SolrHttpClientBuilder getHttpClientBuilder(Optional optionalBuilder) { + public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder builder) { final String basicAuthUser = defaultParams.get(HttpClientUtil.PROP_BASIC_AUTH_USER); final String basicAuthPass = defaultParams.get(HttpClientUtil.PROP_BASIC_AUTH_PASS); if(basicAuthUser == null || basicAuthPass == null) { throw new IllegalArgumentException("username & password must be specified with " + getClass().getName()); } - SolrHttpClientBuilder builder = optionalBuilder.isPresent() ? - initHttpClientBuilder(optionalBuilder.get(), basicAuthUser, basicAuthPass) - : initHttpClientBuilder(SolrHttpClientBuilder.create(), basicAuthUser, basicAuthPass); - return builder; + return initHttpClientBuilder(builder == null ? SolrHttpClientBuilder.create() : builder, basicAuthUser, basicAuthPass); } private SolrHttpClientBuilder initHttpClientBuilder(SolrHttpClientBuilder builder, String basicAuthUser, String basicAuthPass) { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java b/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java index 4853787783c..a6bd477fae8 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/routing/RequestReplicaListTransformerGenerator.java @@ -20,7 +20,7 @@ import java.lang.invoke.MethodHandles; import java.util.Arrays; import java.util.Iterator; import java.util.List; -import java.util.Optional; +import java.util.Objects; import java.util.Random; import org.apache.solr.common.SolrException; @@ -64,9 +64,9 @@ public class RequestReplicaListTransformerGenerator { } public RequestReplicaListTransformerGenerator(ReplicaListTransformerFactory defaultRltFactory, ReplicaListTransformerFactory stableRltFactory, String defaultShardPreferences, String nodeName, String localHostAddress, NodesSysPropsCacher sysPropsCacher) { - this.defaultRltFactory = Optional.ofNullable(defaultRltFactory).orElse(RANDOM_RLTF); - this.stableRltFactory = Optional.ofNullable(stableRltFactory).orElseGet(AffinityReplicaListTransformerFactory::new); - this.defaultShardPreferences = Optional.ofNullable(defaultShardPreferences).orElse(""); + this.defaultRltFactory = Objects.requireNonNullElse(defaultRltFactory, RANDOM_RLTF); + this.stableRltFactory = Objects.requireNonNullElseGet(stableRltFactory, AffinityReplicaListTransformerFactory::new); + this.defaultShardPreferences = Objects.requireNonNullElse(defaultShardPreferences, ""); this.nodeName = nodeName; this.localHostAddress = localHostAddress; this.sysPropsCacher = sysPropsCacher; @@ -83,7 +83,7 @@ public class RequestReplicaListTransformerGenerator { public ReplicaListTransformer getReplicaListTransformer(final SolrParams requestParams, String defaultShardPreferences, String nodeName, String localHostAddress, NodesSysPropsCacher sysPropsCacher) { @SuppressWarnings("deprecation") final boolean preferLocalShards = requestParams.getBool(CommonParams.PREFER_LOCAL_SHARDS, false); - defaultShardPreferences = Optional.ofNullable(defaultShardPreferences).orElse(this.defaultShardPreferences); + defaultShardPreferences = Objects.requireNonNullElse(defaultShardPreferences, this.defaultShardPreferences); final String shardsPreferenceSpec = requestParams.get(ShardParams.SHARDS_PREFERENCE, defaultShardPreferences); if (preferLocalShards || !shardsPreferenceSpec.isEmpty()) { @@ -102,9 +102,9 @@ public class RequestReplicaListTransformerGenerator { new NodePreferenceRulesComparator( preferenceRules, requestParams, - Optional.ofNullable(nodeName).orElse(this.nodeName), - Optional.ofNullable(localHostAddress).orElse(this.localHostAddress), - Optional.ofNullable(sysPropsCacher).orElse(this.sysPropsCacher), + nodeName != null ? nodeName : this.nodeName, // could be still null + localHostAddress != null ? localHostAddress : this.localHostAddress, // could still be null + sysPropsCacher != null ? sysPropsCacher : this.sysPropsCacher, // could still be null defaultRltFactory, stableRltFactory); ReplicaListTransformer baseReplicaListTransformer = replicaComp.getBaseReplicaListTransformer();