Merge pull request elastic#18110 from jasontedor/strings-split-as-array

Remove Strings#splitStringToArray Remove arbitrary separator/wildcard from PathTrie
brwe · May 4, 2016 · 78d615f · 78d615f
2 parents 247b5c8 + 9fe5ce9
commit 78d615f
Show file tree

Hide file tree

Showing 16 changed files with 29 additions and 110 deletions.
diff --git a/core/src/main/java/org/elasticsearch/action/search/TransportSearchHelper.java b/core/src/main/java/org/elasticsearch/action/search/TransportSearchHelper.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.common.Base64;
 import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.util.concurrent.AtomicArray;
 import org.elasticsearch.search.SearchPhaseResult;
 import org.elasticsearch.search.internal.InternalScrollSearchRequest;
@@ -89,7 +88,7 @@ static ParsedScrollId parseScrollId(String scrollId) {
         } catch (Exception e) {
             throw new IllegalArgumentException("Failed to decode scrollId", e);
         }
-        String[] elements = Strings.splitStringToArray(spare.get(), ';');
+        String[] elements = spare.get().toString().split(";");
         if (elements.length < 2) {
             throw new IllegalArgumentException("Malformed scrollId [" + scrollId + "]");
         }

diff --git a/core/src/main/java/org/elasticsearch/common/Strings.java b/core/src/main/java/org/elasticsearch/common/Strings.java
@@ -38,7 +38,6 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Properties;
-import java.util.Random;
 import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
@@ -557,7 +556,8 @@ public static Set<String> splitStringByCommaToSet(final String s) {
     }
 
     public static String[] splitStringByCommaToArray(final String s) {
-        return splitStringToArray(s, ',');
+        if (s == null || s.isEmpty()) return Strings.EMPTY_ARRAY;
+        else return s.split(",");
     }
 
     public static Set<String> splitStringToSet(final String s, final char c) {
@@ -588,42 +588,6 @@ public static Set<String> splitStringToSet(final String s, final char c) {
         return result;
     }
 
-    public static String[] splitStringToArray(final CharSequence s, final char c) {
-        if (s == null || s.length() == 0) {
-            return Strings.EMPTY_ARRAY;
-        }
-        int count = 1;
-        for (int i = 0; i < s.length(); i++) {
-            if (s.charAt(i) == c) {
-                count++;
-            }
-        }
-        final String[] result = new String[count];
-        final StringBuilder builder = new StringBuilder();
-        int res = 0;
-        for (int i = 0; i < s.length(); i++) {
-            if (s.charAt(i) == c) {
-                if (builder.length() > 0) {
-                    result[res++] = builder.toString();
-                    builder.setLength(0);
-                }
-
-            } else {
-                builder.append(s.charAt(i));
-            }
-        }
-        if (builder.length() > 0) {
-            result[res++] = builder.toString();
-        }
-        if (res != count) {
-            // we have empty strings, copy over to a new array
-            String[] result1 = new String[res];
-            System.arraycopy(result, 0, result1, 0, res);
-            return result1;
-        }
-        return result;
-    }
-
     /**
      * Split a String at the first occurrence of the delimiter.
      * Does not include the delimiter in the result.

diff --git a/core/src/main/java/org/elasticsearch/common/Table.java b/core/src/main/java/org/elasticsearch/common/Table.java
@@ -149,7 +149,7 @@ public Table addCell(Object value, String attributes) {
                 // get the attributes of the header cell we are going to add
                 mAttr.putAll(headers.get(currentCells.size()).attr);
             }
-            String[] sAttrs = Strings.splitStringToArray(attributes, ';');
+            String[] sAttrs = attributes.split(";");
             for (String sAttr : sAttrs) {
                 if (sAttr.length() == 0) {
                     continue;

diff --git a/core/src/main/java/org/elasticsearch/common/path/PathTrie.java b/core/src/main/java/org/elasticsearch/common/path/PathTrie.java
@@ -19,17 +19,12 @@
 
 package org.elasticsearch.common.path;
 
-import org.elasticsearch.common.Strings;
-
 import java.util.HashMap;
 import java.util.Map;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableMap;
 
-/**
- *
- */
 public class PathTrie<T> {
 
     public interface Decoder {
@@ -38,17 +33,14 @@ public interface Decoder {
 
     private final Decoder decoder;
     private final TrieNode root;
-    private final char separator;
     private T rootValue;
 
-    public PathTrie(Decoder decoder) {
-        this('/', "*", decoder);
-    }
+    private static final String SEPARATOR = "/";
+    private static final String WILDCARD = "*";
 
-    public PathTrie(char separator, String wildcard, Decoder decoder) {
+    public PathTrie(Decoder decoder) {
         this.decoder = decoder;
-        this.separator = separator;
-        root = new TrieNode(new String(new char[]{separator}), null, wildcard);
+        root = new TrieNode(SEPARATOR, null, WILDCARD);
     }
 
     public class TrieNode {
@@ -196,7 +188,7 @@ public String toString() {
     }
 
     public void insert(String path, T value) {
-        String[] strings = Strings.splitStringToArray(path, separator);
+        String[] strings = path.split(SEPARATOR);
         if (strings.length == 0) {
             rootValue = value;
             return;
@@ -217,7 +209,7 @@ public T retrieve(String path, Map<String, String> params) {
         if (path.length() == 0) {
             return rootValue;
         }
-        String[] strings = Strings.splitStringToArray(path, separator);
+        String[] strings = path.split(SEPARATOR);
         if (strings.length == 0) {
             return rootValue;
         }

diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java
@@ -899,19 +899,6 @@ public Builder put(Dictionary<Object,Object> properties) {
             return this;
         }
 
-        public Builder loadFromDelimitedString(String value, char delimiter) {
-            String[] values = Strings.splitStringToArray(value, delimiter);
-            for (String s : values) {
-                int index = s.indexOf('=');
-                if (index == -1) {
-                    throw new IllegalArgumentException(
-                            "value [" + s + "] for settings loaded with delimiter [" + delimiter + "] is malformed, missing =");
-                }
-                map.put(s.substring(0, index), s.substring(index + 1));
-            }
-            return this;
-        }
-
         /**
          * Loads settings from the actual string content that represents them using the
          * {@link SettingsLoaderFactory#loaderFromSource(String)}.

diff --git a/core/src/main/java/org/elasticsearch/common/util/BloomFilter.java b/core/src/main/java/org/elasticsearch/common/util/BloomFilter.java
@@ -24,7 +24,6 @@
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.RamUsageEstimator;
 import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.hash.MurmurHash3;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -75,7 +74,7 @@ public static Factory buildFromString(@Nullable String config) {
             if (config == null) {
                 return buildDefault();
             }
-            String[] sEntries = Strings.splitStringToArray(config, ',');
+            String[] sEntries = config.split(",");
             if (sEntries.length == 0) {
                 if (config.length() > 0) {
                     return new Factory(new Entry[]{new Entry(0, Double.parseDouble(config))});

diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java
@@ -40,7 +40,7 @@ public class XContentMapValues {
      */
     public static List<Object> extractRawValues(String path, Map<String, Object> map) {
         List<Object> values = new ArrayList<>();
-        String[] pathElements = Strings.splitStringToArray(path, '.');
+        String[] pathElements = path.split("\\.");
         if (pathElements.length == 0) {
             return values;
         }
@@ -93,7 +93,7 @@ private static void extractRawValues(List values, List<Object> part, String[] pa
     }
 
     public static Object extractValue(String path, Map<String, Object> map) {
-        String[] pathElements = Strings.splitStringToArray(path, '.');
+        String[] pathElements = path.split("\\.");
         if (pathElements.length == 0) {
             return null;
         }

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -19,13 +19,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.Version;
@@ -51,6 +44,13 @@
 import org.elasticsearch.index.mapper.object.ArrayValueMapperParser;
 import org.elasticsearch.index.mapper.object.ObjectMapper;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+
 /** A parser for documents, given mappings from a DocumentMapper */
 final class DocumentParser {
 
@@ -829,8 +829,7 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
             // The path of the dest field might be completely different from the current one so we need to reset it
             context = context.overridePath(new ContentPath(0));
 
-            // TODO: why Strings.splitStringToArray instead of String.split?
-            final String[] paths = Strings.splitStringToArray(field, '.');
+            final String[] paths = field.split("\\.");
             final String fieldName = paths[paths.length-1];
             ObjectMapper mapper = context.root();
             ObjectMapper[] mappers = new ObjectMapper[paths.length-1];

diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
@@ -33,7 +33,6 @@
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.ShapesAvailability;
 import org.elasticsearch.common.geo.SpatialStrategy;
@@ -379,7 +378,7 @@ private ShapeBuilder fetch(Client client, GetRequest getRequest, String path) th
             throw new IllegalArgumentException("Shape with ID [" + getRequest.id() + "] in type [" + getRequest.type() + "] not found");
         }
 
-        String[] pathElements = Strings.splitStringToArray(path, '.');
+        String[] pathElements = path.split("\\.");
         int currentPathSlot = 0;
 
         try (XContentParser parser = XContentHelper.createParser(response.getSourceAsBytesRef())) {

diff --git a/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java b/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java
@@ -622,8 +622,8 @@ private FieldPath(String path) {
                     newPath = path;
                 }
             }
-            this.pathElements = Strings.splitStringToArray(newPath, '.');
-            if (pathElements.length == 0) {
+            this.pathElements = newPath.split("\\.");
+            if (pathElements.length == 1 && pathElements[0].isEmpty()) {
                 throw new IllegalArgumentException("path [" + path + "] is not valid");
             }
         }

diff --git a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java
@@ -492,7 +492,7 @@ public void tearDown() throws Exception {
     private void registerTaskManageListeners(String actionMasks) {
         for (String nodeName : internalCluster().getNodeNames()) {
             DiscoveryNode node = internalCluster().getInstance(ClusterService.class, nodeName).localNode();
-            RecordingTaskManagerListener listener = new RecordingTaskManagerListener(node, Strings.splitStringToArray(actionMasks, ','));
+            RecordingTaskManagerListener listener = new RecordingTaskManagerListener(node, actionMasks.split(","));
             ((MockTaskManager) internalCluster().getInstance(TransportService.class, nodeName).getTaskManager()).addListener(listener);
             RecordingTaskManagerListener oldListener = listeners.put(new Tuple<>(node.getName(), actionMasks), listener);
             assertNull(oldListener);

diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
@@ -38,24 +38,6 @@
 
 public class SettingsTests extends ESTestCase {
 
-    public void testLoadFromDelimitedString() {
-        Settings settings = Settings.builder()
-                .loadFromDelimitedString("key1=value1;key2=value2", ';')
-                .build();
-        assertThat(settings.get("key1"), equalTo("value1"));
-        assertThat(settings.get("key2"), equalTo("value2"));
-        assertThat(settings.getAsMap().size(), equalTo(2));
-        assertThat(settings.toDelimitedString(';'), equalTo("key1=value1;key2=value2;"));
-
-        settings = Settings.builder()
-                .loadFromDelimitedString("key1=value1;key2=value2;", ';')
-                .build();
-        assertThat(settings.get("key1"), equalTo("value1"));
-        assertThat(settings.get("key2"), equalTo("value2"));
-        assertThat(settings.getAsMap().size(), equalTo(2));
-        assertThat(settings.toDelimitedString(';'), equalTo("key1=value1;key2=value2;"));
-    }
-
     public void testReplacePropertiesPlaceholderSystemProperty() {
         String value = System.getProperty("java.home");
         assertFalse(value.isEmpty());

diff --git a/plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/network/GceNameResolver.java b/plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/network/GceNameResolver.java
@@ -92,7 +92,7 @@ private InetAddress[] resolve(String value) throws IOException {
         } else if (value.startsWith(GceAddressResolverType.PRIVATE_IP.configName)) {
             // We extract the network interface from gce:privateIp:XX
             String network = "0";
-            String[] privateIpConfig = Strings.splitStringToArray(value, ':');
+            String[] privateIpConfig = value.split(":");
             if (privateIpConfig != null && privateIpConfig.length == 3) {
                 network = privateIpConfig[2];
             }

diff --git a/.../repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java b/.../repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java
@@ -116,7 +116,7 @@ public AzureRepository(RepositoryName name, RepositorySettings repositorySetting
             // Remove starting / if any
             basePath = Strings.trimLeadingCharacter(basePath, '/');
             BlobPath path = new BlobPath();
-            for(String elem : Strings.splitStringToArray(basePath, '/')) {
+            for(String elem : basePath.split("/")) {
                 path = path.add(elem);
             }
             this.basePath = path;

diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java
@@ -287,7 +287,7 @@ public S3Repository(RepositoryName name, RepositorySettings repositorySettings,
         String basePath = getValue(repositorySettings, Repository.BASE_PATH_SETTING, Repositories.BASE_PATH_SETTING);
         if (Strings.hasLength(basePath)) {
             BlobPath path = new BlobPath();
-            for(String elem : Strings.splitStringToArray(basePath, '/')) {
+            for(String elem : basePath.split("/")) {
                 path = path.add(elem);
             }
             this.basePath = path;

diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java b/test/framework/src/main/java/org/elasticsearch/ingest/RandomDocumentPicks.java
@@ -22,8 +22,6 @@
 import com.carrotsearch.randomizedtesting.generators.RandomInts;
 import com.carrotsearch.randomizedtesting.generators.RandomPicks;
 import com.carrotsearch.randomizedtesting.generators.RandomStrings;
-import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.ingest.core.IngestDocument;
 
 import java.util.ArrayList;
@@ -105,7 +103,7 @@ public static String addRandomField(Random random, IngestDocument ingestDocument
      * that each node of the tree either doesn't exist or is a map, otherwise new fields cannot be added.
      */
     public static boolean canAddField(String path, IngestDocument ingestDocument) {
-        String[] pathElements = Strings.splitStringToArray(path, '.');
+        String[] pathElements = path.split("\\.");
         Map<String, Object> innerMap = ingestDocument.getSourceAndMetadata();
         if (pathElements.length > 1) {
             for (int i = 0; i < pathElements.length - 1; i++) {