Remove and forbid use of com.google.common.base.Joiner

This commit removes and now forbids all uses of
com.google.common.base.Joiner across the codebase. This is one of many
steps in the eventual removal of Guava as a dependency.

Relates #13224

core/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java

@@ -19,11 +19,12 @@
 
 package org.elasticsearch.common.util.concurrent;
 
-import com.google.common.base.Joiner;
 import org.elasticsearch.common.settings.Settings;
 
+import java.util.Arrays;
 import java.util.concurrent.*;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
 
 /**
  *
@@ -81,7 +82,12 @@ public class EsExecutors {
     }
 
     public static String threadName(Settings settings, String ... names) {
-        return threadName(settings, "[" +  Joiner.on(".").skipNulls().join(names) + "]");
+        String namePrefix =
+                Arrays
+                        .stream(names)
+                        .filter(name -> name != null)
+                        .collect(Collectors.joining(".", "[", "]"));
+        return threadName(settings, namePrefix);
     }
 
     public static String threadName(Settings settings, String namePrefix) {

core/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

@@ -19,14 +19,9 @@
 
 package org.elasticsearch.repositories;
 
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableMap;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.cluster.AckedClusterStateUpdateTask;
-import org.elasticsearch.cluster.ClusterChangedEvent;
-import org.elasticsearch.cluster.ClusterService;
-import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ClusterStateListener;
+import org.elasticsearch.cluster.*;
 import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest;
 import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.metadata.MetaData;
@@ -45,10 +40,8 @@ import org.elasticsearch.snapshots.SnapshotsService;
 import org.elasticsearch.transport.TransportService;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
 
@@ -571,9 +564,10 @@ public class RepositoriesService extends AbstractComponent implements ClusterSta
         }
 
         public String failureDescription() {
-            StringBuilder builder = new StringBuilder('[');
-            Joiner.on(", ").appendTo(builder, failures);
-            return builder.append(']').toString();
+            return Arrays
+                    .stream(failures)
+                    .map(failure -> failure.toString())
+                    .collect(Collectors.joining(", ", "[", "]"));
         }
 
     }

core/src/main/java/org/elasticsearch/search/suggest/context/CategoryContextMapping.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.search.suggest.context;
 
-import com.google.common.base.Joiner;
 import org.apache.lucene.analysis.PrefixAnalyzer;
 import org.apache.lucene.analysis.TokenStream;
 import org.apache.lucene.index.IndexableField;
@@ -36,6 +35,8 @@ import org.elasticsearch.index.mapper.ParseContext.Document;
 
 import java.io.IOException;
 import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 /**
  * The {@link CategoryContextMapping} is used to define a {@link ContextMapping} that
@@ -258,14 +259,18 @@ public class CategoryContextMapping extends ContextMapping {
         public String toString() {
             StringBuilder sb = new StringBuilder("FieldConfig(" + fieldname + " = [");
             if (this.values != null && this.values.iterator().hasNext()) {
-                sb.append("(").append(Joiner.on(", ").join(this.values.iterator())).append(")");
+                sb.append(delimitValues(this.values));
             }
             if (this.defaultValues != null && this.defaultValues.iterator().hasNext()) {
-                sb.append(" default(").append(Joiner.on(", ").join(this.defaultValues.iterator())).append(")");
+                sb.append(" default").append(delimitValues(this.defaultValues));
             }
             return sb.append("])").toString();
         }
 
+        private String delimitValues(Iterable<? extends CharSequence> values) {
+            return StreamSupport.stream(values.spliterator(), false).collect(Collectors.joining(", ", "(", ")"));
+        }
+
     }
 
     private static class FieldQuery extends ContextQuery {

core/src/test/java/org/elasticsearch/NamingConventionTests.java

@@ -19,9 +19,6 @@
 package org.elasticsearch;
 
 import junit.framework.TestCase;
-
-import com.google.common.base.Joiner;
-
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.test.ESIntegTestCase;
@@ -40,6 +37,7 @@ import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Simple class that ensures that all subclasses concrete of ESTestCase end with either Test | Tests
@@ -149,24 +147,27 @@ public class NamingConventionTests extends ESTestCase {
         assertTrue(pureUnitTest.remove(PlainUnit.class));
         assertTrue(pureUnitTest.remove(PlainUnitTheSecond.class));
 
-        String classesToSubclass = Joiner.on(',').join(
-            ESTestCase.class.getSimpleName(),
-            ESTestCase.class.getSimpleName(),
-            ESTokenStreamTestCase.class.getSimpleName(),
-            LuceneTestCase.class.getSimpleName());
-        assertTrue("Not all subclasses of " + ESTestCase.class.getSimpleName() +
- " match the naming convention. Concrete classes must end with [Tests]:\n" + Joiner.on('\n').join(missingSuffix),
-            missingSuffix.isEmpty());
-        assertTrue("Classes ending with [Tests] are abstract or interfaces:\n" + Joiner.on('\n').join(notRunnable),
-            notRunnable.isEmpty());
-        assertTrue("Found inner classes that are tests, which are excluded from the test runner:\n" + Joiner.on('\n').join(innerClasses),
-            innerClasses.isEmpty());
-        assertTrue("Pure Unit-Test found must subclass one of [" + classesToSubclass +"]:\n" + Joiner.on('\n').join(pureUnitTest),
-            pureUnitTest.isEmpty());
-        assertTrue("Classes ending with [Tests] must subclass [" + classesToSubclass + "]:\n" + Joiner.on('\n').join(notImplementing),
-            notImplementing.isEmpty());
-        assertTrue("Subclasses of ESIntegTestCase should end with IT as they are integration tests:\n" + Joiner.on('\n').join(integTestsInDisguise),
-            integTestsInDisguise.isEmpty());
+        String classesToSubclass = String.join(
+                ",",
+                ESTestCase.class.getSimpleName(),
+                ESTestCase.class.getSimpleName(),
+                ESTokenStreamTestCase.class.getSimpleName(),
+                LuceneTestCase.class.getSimpleName()
+        );
+        assertNoViolations("Not all subclasses of " + ESTestCase.class.getSimpleName() + " match the naming convention. Concrete classes must end with [Tests]:\n", missingSuffix);
+        assertNoViolations("Classes ending with [Tests] are abstract or interfaces:\n", notRunnable);
+        assertNoViolations("Found inner classes that are tests, which are excluded from the test runner:\n", innerClasses);
+        assertNoViolations("Pure Unit-Test found must subclass one of [" + classesToSubclass + "]:\n", pureUnitTest);
+        assertNoViolations("Classes ending with [Tests] must subclass [" + classesToSubclass + "]:\n", notImplementing);
+        assertNoViolations("Subclasses of ESIntegTestCase should end with IT as they are integration tests:\n", integTestsInDisguise);
+    }
+
+    private String join(Set<Class> set) {
+        return set.stream().map(Object::toString).collect(Collectors.joining("\n"));
+    }
+
+    private void assertNoViolations(String message, Set<Class> set) {
+        assertTrue(message + join(set), set.isEmpty());
     }
 
     /*

core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java

@@ -793,7 +793,7 @@ public abstract class ESIntegTestCase extends ESTestCase {
     }
 
     private Settings.Builder getExcludeSettings(String index, int num, Settings.Builder builder) {
-        String exclude = Joiner.on(',').join(internalCluster().allDataNodesButN(num));
+        String exclude = String.join(",", internalCluster().allDataNodesButN(num));
         builder.put("index.routing.allocation.exclude._name", exclude);
         return builder;
     }

core/src/test/java/org/elasticsearch/test/rest/client/http/HttpRequestBuilder.java

@@ -18,14 +18,7 @@
  */
 package org.elasticsearch.test.rest.client.http;
 
-import com.google.common.base.Joiner;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
-import org.apache.http.client.methods.HttpHead;
-import org.apache.http.client.methods.HttpOptions;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpPut;
-import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.methods.*;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.elasticsearch.client.support.Headers;
@@ -44,6 +37,7 @@ import java.net.URLEncoder;
 import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 /**
  * Executable builder for an http request
@@ -194,7 +188,7 @@ public class HttpRequestBuilder {
             //(e.g. '+' will stay as is) hence when trying to properly encode params manually they will end up double encoded (+ becomes %252B instead of %2B).
             StringBuilder uriBuilder = new StringBuilder(protocol).append("://").append(host).append(":").append(port).append(uri.getRawPath());
             if (params.size() > 0) {
-                uriBuilder.append("?").append(Joiner.on('&').withKeyValueSeparator("=").join(params));
+                uriBuilder.append("?").append(params.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining("&")));
             }
             return URI.create(uriBuilder.toString());
         } catch(URISyntaxException e) {

core/src/test/java/org/elasticsearch/test/rest/section/ApiCallSection.java

@@ -18,14 +18,9 @@
  */
 package org.elasticsearch.test.rest.section;
 
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableMap;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 /**
  * Represents a test fragment that contains the information needed to call an api
@@ -52,7 +47,7 @@ public class ApiCallSection {
     public void addParam(String key, String value) {
         String existingValue = params.get(key);
         if (existingValue != null) {
-            value = Joiner.on(",").join(existingValue, value);
+            value = existingValue + "," + value;
         }
         this.params.put(key, value);
     }

dev-tools/src/main/resources/forbidden/#all-signatures.txt#

@@ -0,0 +1,129 @@
+# Licensed to Elasticsearch under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Elasticsearch 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.
+
+@defaultMessage Convert to URI
+java.net.URL#getPath()
+java.net.URL#getFile()
+
+@defaultMessage Usage of getLocalHost is discouraged
+java.net.InetAddress#getLocalHost()
+
+@defaultMessage Use java.nio.file instead of java.io.File API
+java.util.jar.JarFile
+java.util.zip.ZipFile
+java.io.File
+java.io.FileInputStream
+java.io.FileOutputStream
+java.io.PrintStream#<init>(java.lang.String,java.lang.String)
+java.io.PrintWriter#<init>(java.lang.String,java.lang.String)
+java.util.Formatter#<init>(java.lang.String,java.lang.String,java.util.Locale)
+java.io.RandomAccessFile
+java.nio.file.Path#toFile()
+
+@defaultMessage Don't use deprecated lucene apis
+org.apache.lucene.index.DocsEnum
+org.apache.lucene.index.DocsAndPositionsEnum
+org.apache.lucene.queries.TermFilter
+org.apache.lucene.queries.TermsFilter
+org.apache.lucene.search.TermRangeFilter
+org.apache.lucene.search.NumericRangeFilter
+org.apache.lucene.search.PrefixFilter
+
+java.nio.file.Paths @ Use PathUtils.get instead.
+java.nio.file.FileSystems#getDefault() @ use PathUtils.getDefault instead.
+
+@defaultMessage Specify a location for the temp file/directory instead.
+java.nio.file.Files#createTempDirectory(java.lang.String,java.nio.file.attribute.FileAttribute[])
+java.nio.file.Files#createTempFile(java.lang.String,java.lang.String,java.nio.file.attribute.FileAttribute[])
+
+@defaultMessage Don't use java serialization - this can break BWC without noticing it
+java.io.ObjectOutputStream
+java.io.ObjectOutput
+java.io.ObjectInputStream
+java.io.ObjectInput
+
+java.nio.file.Files#isHidden(java.nio.file.Path) @ Dependent on the operating system, use FileSystemUtils.isHidden instead
+
+java.nio.file.Files#getFileStore(java.nio.file.Path) @ Use Environment.getFileStore() instead, impacted by JDK-8034057
+java.nio.file.Files#isWritable(java.nio.file.Path) @ Use Environment.isWritable() instead, impacted by JDK-8034057
+
+@defaultMessage Resolve hosts explicitly to the address(es) you want with InetAddress.
+java.net.InetSocketAddress#<init>(java.lang.String,int)
+java.net.Socket#<init>(java.lang.String,int)
+java.net.Socket#<init>(java.lang.String,int,java.net.InetAddress,int)
+
+@defaultMessage Don't bind to wildcard addresses. Be specific.
+java.net.DatagramSocket#<init>()
+java.net.DatagramSocket#<init>(int)
+java.net.InetSocketAddress#<init>(int)
+java.net.MulticastSocket#<init>()
+java.net.MulticastSocket#<init>(int)
+java.net.ServerSocket#<init>(int)
+java.net.ServerSocket#<init>(int,int)
+
+@defaultMessage use NetworkAddress format/formatAddress to print IP or IP+ports
+java.net.InetAddress#toString()
+java.net.InetAddress#getHostAddress()
+java.net.Inet4Address#getHostAddress()
+java.net.Inet6Address#getHostAddress()
+java.net.InetSocketAddress#toString()
+
+@defaultMessage avoid DNS lookups by accident: if you have a valid reason, then @SuppressWarnings with that reason so its completely clear
+java.net.InetAddress#getHostName()
+java.net.InetAddress#getCanonicalHostName()
+
+java.net.InetSocketAddress#getHostName() @ Use getHostString() instead, which avoids a DNS lookup
+
+@defaultMessage avoid adding additional dependencies on Guava
+com.google.common.collect.Lists
+com.google.common.collect.ImmutableList
+com.google.common.base.Objects
+com.google.common.base.Predicate
+com.google.common.base.Predicates
+com.google.common.base.Strings
+com.google.common.base.Throwables
+com.google.common.collect.Maps
+com.google.common.collect.Sets
+com.google.common.base.Preconditions
+com.google.common.collect.ImmutableSortedSet
+com.google.common.collect.Queues
+com.google.common.util.concurrent.ListenableFuture
+com.google.common.util.concurrent.SettableFuture
+com.google.common.util.concurrent.Futures
+com.google.common.util.concurrent.MoreExecutors
+com.google.common.collect.ImmutableSortedMap
+com.google.common.base.Charsets
+com.google.common.base.Function
+com.google.common.collect.Collections2
+com.google.common.cache.LoadingCache
+com.google.common.cache.CacheLoader
+com.google.common.collect.Iterables
+<<<<<<< HEAD
+com.google.common.util.concurrent.UncheckedExecutionException
+com.google.common.util.concurrent.AtomicLongMap
+com.google.common.primitives.Longs
+com.google.common.io.ByteStreams
+com.google.common.collect.UnmodifiableIterator
+com.google.common.collect.ObjectArrays
+com.google.common.collect.Multimap
+com.google.common.collect.MultimapBuilder
+=======
+com.google.common.base.Joiner
+>>>>>>> Remove and forbid use of com.google.common.base.Joiner
+
+@defaultMessage Do not violate java's access system
+java.lang.reflect.AccessibleObject#setAccessible(boolean)
+java.lang.reflect.AccessibleObject#setAccessible(java.lang.reflect.AccessibleObject[], boolean)

dev-tools/src/main/resources/forbidden/all-signatures.txt

@@ -120,6 +120,7 @@ com.google.common.collect.ObjectArrays
 com.google.common.collect.Multimap
 com.google.common.collect.MultimapBuilder
 com.google.common.math.LongMath
+com.google.common.base.Joiner
 
 @defaultMessage Do not violate java's access system
 java.lang.reflect.AccessibleObject#setAccessible(boolean)