Create a class to hold field capabilities for one index. (#51721)

jtibshirani · web-flow · commit fef0e1dd8973 · 2020-02-03T16:20:22.000-08:00
Currently, the same class `FieldCapabilities` is used both to represent the capabilities for one index, and also the merged capabilities across indices. To help clarify the logic, this PR proposes to create a separate class `IndexFieldCapabilities` for the capabilities in one index. The refactor will also help when adding `source_path` information in #49264, since the merged source path field will have a different structure from the field for a single index. Individual changes: * Add a new class IndexFieldCapabilities. * Remove extra constructor from FieldCapabilities. * Combine the add and merge methods in FieldCapabilities.Builder.
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java
@@ -1242,7 +1242,7 @@ public void testFieldCaps() throws IOException {
         assertEquals(1, fieldResponse.size());
 
         FieldCapabilities expectedTextCapabilities = new FieldCapabilities(
-            "field", "text", true, false, Collections.emptyMap());
+            "field", "text", true, false, null, null, null, Collections.emptyMap());
         assertEquals(expectedTextCapabilities, fieldResponse.get("text"));
     }
 
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
@@ -57,12 +57,6 @@ public class FieldCapabilities implements Writeable, ToXContentObject {
     private static final ParseField NON_AGGREGATABLE_INDICES_FIELD = new ParseField("non_aggregatable_indices");
     private static final ParseField META_FIELD = new ParseField("meta");
 
-    private static Map<String, Set<String>> mapToMapOfSets(Map<String, String> map) {
-        final Function<Map.Entry<String, String>, String> entryValueFunction = Map.Entry::getValue;
-        return map.entrySet().stream().collect(
-                Collectors.toUnmodifiableMap(Map.Entry::getKey, entryValueFunction.andThen(Set::of)));
-    }
-
     private final String name;
     private final String type;
     private final boolean isSearchable;
@@ -74,19 +68,6 @@ private static Map<String, Set<String>> mapToMapOfSets(Map<String, String> map)
 
     private final Map<String, Set<String>> meta;
 
-    /**
-     * Constructor for a single index.
-     * @param name The name of the field.
-     * @param type The type associated with the field.
-     * @param isSearchable Whether this field is indexed for search.
-     * @param isAggregatable Whether this field can be aggregated on.
-     * @param meta Metadata about the field.
-     */
-    public FieldCapabilities(String name, String type, boolean isSearchable, boolean isAggregatable,
-            Map<String, String> meta) {
-        this(name, type, isSearchable, isAggregatable, null, null, null, mapToMapOfSets(Objects.requireNonNull(meta)));
-    }
-
     /**
      * Constructor for a set of indices.
      * @param name The name of the field
@@ -102,11 +83,11 @@ public FieldCapabilities(String name, String type, boolean isSearchable, boolean
      * @param meta Merged metadata across indices.
      */
     public FieldCapabilities(String name, String type,
-                      boolean isSearchable, boolean isAggregatable,
-                      String[] indices,
-                      String[] nonSearchableIndices,
-                      String[] nonAggregatableIndices,
-                      Map<String, Set<String>> meta) {
+                             boolean isSearchable, boolean isAggregatable,
+                             String[] indices,
+                             String[] nonSearchableIndices,
+                             String[] nonAggregatableIndices,
+                             Map<String, Set<String>> meta) {
         this.name = name;
         this.type = type;
         this.isSearchable = isSearchable;
@@ -117,7 +98,7 @@ public FieldCapabilities(String name, String type,
         this.meta = Objects.requireNonNull(meta);
     }
 
-    public FieldCapabilities(StreamInput in) throws IOException {
+    FieldCapabilities(StreamInput in) throws IOException {
         this.name = in.readString();
         this.type = in.readString();
         this.isSearchable = in.readBoolean();
@@ -309,35 +290,20 @@ static class Builder {
             this.meta = new HashMap<>();
         }
 
-        private void add(String index, boolean search, boolean agg) {
+        /**
+         * Collect the field capabilities for an index.
+         */
+        void add(String index, boolean search, boolean agg, Map<String, String> meta) {
             IndexCaps indexCaps = new IndexCaps(index, search, agg);
             indiceList.add(indexCaps);
             this.isSearchable &= search;
             this.isAggregatable &= agg;
-        }
-
-        /**
-         * Collect capabilities of an index.
-         */
-        void add(String index, boolean search, boolean agg, Map<String, String> meta) {
-            add(index, search, agg);
             for (Map.Entry<String, String> entry : meta.entrySet()) {
                 this.meta.computeIfAbsent(entry.getKey(), key -> new HashSet<>())
                         .add(entry.getValue());
             }
         }
 
-        /**
-         * Merge another capabilities instance.
-         */
-        void merge(String index, boolean search, boolean agg, Map<String, Set<String>> meta) {
-            add(index, search, agg);
-            for (Map.Entry<String, Set<String>> entry : meta.entrySet()) {
-                this.meta.computeIfAbsent(entry.getKey(), key -> new HashSet<>())
-                        .addAll(entry.getValue());
-            }
-        }
-
         List<String> getIndices() {
             return indiceList.stream().map(c -> c.name).collect(Collectors.toList());
         }
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesIndexResponse.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesIndexResponse.java
@@ -29,13 +29,13 @@
 import java.util.Objects;
 
 /**
- * Response for {@link FieldCapabilitiesIndexRequest} requests.
+ * Response for {@link TransportFieldCapabilitiesIndexAction}.
  */
 public class FieldCapabilitiesIndexResponse extends ActionResponse implements Writeable {
     private String indexName;
-    private Map<String, FieldCapabilities> responseMap;
+    private Map<String, IndexFieldCapabilities> responseMap;
 
-    FieldCapabilitiesIndexResponse(String indexName, Map<String, FieldCapabilities> responseMap) {
+    FieldCapabilitiesIndexResponse(String indexName, Map<String, IndexFieldCapabilities> responseMap) {
         this.indexName = indexName;
         this.responseMap = responseMap;
     }
@@ -44,10 +44,9 @@ public class FieldCapabilitiesIndexResponse extends ActionResponse implements Wr
         super(in);
         this.indexName = in.readString();
         this.responseMap =
-            in.readMap(StreamInput::readString, FieldCapabilities::new);
+            in.readMap(StreamInput::readString, IndexFieldCapabilities::new);
     }
 
-
     /**
      * Get the index name
      */
@@ -58,15 +57,15 @@ public String getIndexName() {
     /**
      * Get the field capabilities map
      */
-    public Map<String, FieldCapabilities> get() {
+    public Map<String, IndexFieldCapabilities> get() {
         return responseMap;
     }
 
     /**
      *
      * Get the field capabilities for the provided {@code field}
      */
-    public FieldCapabilities getField(String field) {
+    public IndexFieldCapabilities getField(String field) {
         return responseMap.get(field);
     }
 
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java
@@ -0,0 +1,136 @@
+/*
+ * 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.
+ */
+
+package org.elasticsearch.action.fieldcaps;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Writeable;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Describes the capabilities of a field in a single index.
+ */
+public class IndexFieldCapabilities implements Writeable {
+
+    private final String name;
+    private final String type;
+    private final boolean isSearchable;
+    private final boolean isAggregatable;
+    private final Map<String, String> meta;
+
+    /**
+     * @param name The name of the field.
+     * @param type The type associated with the field.
+     * @param isSearchable Whether this field is indexed for search.
+     * @param isAggregatable Whether this field can be aggregated on.
+     * @param meta Metadata about the field.
+     */
+    IndexFieldCapabilities(String name, String type,
+                           boolean isSearchable, boolean isAggregatable,
+                           Map<String, String> meta) {
+
+        this.name = name;
+        this.type = type;
+        this.isSearchable = isSearchable;
+        this.isAggregatable = isAggregatable;
+        this.meta = meta;
+    }
+
+    IndexFieldCapabilities(StreamInput in) throws IOException {
+        if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
+            this.name = in.readString();
+            this.type = in.readString();
+            this.isSearchable = in.readBoolean();
+            this.isAggregatable = in.readBoolean();
+            this.meta = in.readMap(StreamInput::readString, StreamInput::readString);
+        } else {
+            // Previously we reused the FieldCapabilities class to represent index field capabilities.
+            FieldCapabilities fieldCaps = new FieldCapabilities(in);
+            this.name = fieldCaps.getName();
+            this.type = fieldCaps.getType();
+            this.isSearchable = fieldCaps.isSearchable();
+            this.isAggregatable = fieldCaps.isAggregatable();
+            this.meta = fieldCaps.meta().entrySet().stream().collect(Collectors.toMap(
+                Map.Entry::getKey,
+                entry -> entry.getValue().iterator().next()));
+        }
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+            out.writeString(name);
+            out.writeString(type);
+            out.writeBoolean(isSearchable);
+            out.writeBoolean(isAggregatable);
+            out.writeMap(meta, StreamOutput::writeString, StreamOutput::writeString);
+        } else {
+            // Previously we reused the FieldCapabilities class to represent index field capabilities.
+            Map<String, Set<String>> wrappedMeta = meta.entrySet().stream().collect(Collectors.toMap(
+                Map.Entry::getKey,
+                entry -> Set.of(entry.getValue())));
+            FieldCapabilities fieldCaps = new FieldCapabilities(name, type, isSearchable, isAggregatable, null, null, null, wrappedMeta);
+            fieldCaps.writeTo(out);
+        }
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public String getType() {
+        return type;
+    }
+
+    public boolean isAggregatable() {
+        return isAggregatable;
+    }
+
+    public boolean isSearchable() {
+        return isSearchable;
+    }
+
+    public Map<String, String> meta() {
+        return meta;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        IndexFieldCapabilities that = (IndexFieldCapabilities) o;
+        return isSearchable == that.isSearchable &&
+            isAggregatable == that.isAggregatable &&
+            Objects.equals(name, that.name) &&
+            Objects.equals(type, that.type) &&
+            Objects.equals(meta, that.meta);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(name, type, isSearchable, isAggregatable, meta);
+    }
+}
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
@@ -182,14 +182,14 @@ private void addUnmappedFields(String[] indices, String field, Map<String, Field
     }
 
     private void innerMerge(Map<String, Map<String, FieldCapabilities.Builder>> responseMapBuilder,
-                                String indexName, Map<String, FieldCapabilities> map) {
-        for (Map.Entry<String, FieldCapabilities> entry : map.entrySet()) {
+                                String indexName, Map<String, IndexFieldCapabilities> map) {
+        for (Map.Entry<String, IndexFieldCapabilities> entry : map.entrySet()) {
             final String field = entry.getKey();
-            final FieldCapabilities fieldCap = entry.getValue();
+            final IndexFieldCapabilities fieldCap = entry.getValue();
             Map<String, FieldCapabilities.Builder> typeMap = responseMapBuilder.computeIfAbsent(field, f -> new HashMap<>());
             FieldCapabilities.Builder builder = typeMap.computeIfAbsent(fieldCap.getType(),
                 key -> new FieldCapabilities.Builder(field, key));
-            builder.merge(indexName, fieldCap.isSearchable(), fieldCap.isAggregatable(), fieldCap.meta());
+            builder.add(indexName, fieldCap.isSearchable(), fieldCap.isAggregatable(), fieldCap.meta());
         }
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java
@@ -84,14 +84,14 @@ protected FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesI
             fieldNames.addAll(mapperService.simpleMatchToFullName(field));
         }
         Predicate<String> fieldPredicate = indicesService.getFieldFilter().apply(shardId.getIndexName());
-        Map<String, FieldCapabilities> responseMap = new HashMap<>();
+        Map<String, IndexFieldCapabilities> responseMap = new HashMap<>();
         for (String field : fieldNames) {
             MappedFieldType ft = mapperService.fullName(field);
             if (ft != null) {
                 if (indicesService.isMetaDataField(mapperService.getIndexSettings().getIndexVersionCreated(), field)
                         || fieldPredicate.test(ft.name())) {
-                    FieldCapabilities fieldCap = new FieldCapabilities(field, ft.typeName(), ft.isSearchable(), ft.isAggregatable(),
-                            ft.meta());
+                    IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(field, ft.typeName(),
+                        ft.isSearchable(), ft.isAggregatable(), ft.meta());
                     responseMap.put(field, fieldCap);
                 } else {
                     continue;
@@ -109,7 +109,8 @@ protected FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesI
                         // no field type, it must be an object field
                         ObjectMapper mapper = mapperService.getObjectMapper(parentField);
                         String type = mapper.nested().isNested() ? "nested" : "object";
-                        FieldCapabilities fieldCap = new FieldCapabilities(parentField, type, false, false, Collections.emptyMap());
+                        IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(parentField, type,
+                            false, false, Collections.emptyMap());
                         responseMap.put(parentField, fieldCap);
                     }
                     dotIndex = parentField.lastIndexOf('.');
diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesResponseTests.java
@@ -49,17 +49,35 @@ protected Writeable.Reader<FieldCapabilitiesResponse> instanceReader() {
     }
 
     private FieldCapabilitiesIndexResponse createRandomIndexResponse() {
-        Map<String, FieldCapabilities> responses = new HashMap<>();
+        Map<String, IndexFieldCapabilities> responses = new HashMap<>();
 
         String[] fields = generateRandomStringArray(5, 10, false, true);
         assertNotNull(fields);
 
         for (String field : fields) {
-            responses.put(field, FieldCapabilitiesTests.randomFieldCaps(field));
+            responses.put(field, randomFieldCaps(field));
         }
         return new FieldCapabilitiesIndexResponse(randomAsciiLettersOfLength(10), responses);
     }
 
+    private static IndexFieldCapabilities randomFieldCaps(String fieldName) {
+        Map<String, String> meta;
+        switch (randomInt(2)) {
+            case 0:
+                meta = Collections.emptyMap();
+                break;
+            case 1:
+                meta = Map.of("key", "value");
+                break;
+            default:
+                meta = Map.of("key1", "value1", "key2", "value2");
+                break;
+        }
+
+        return new IndexFieldCapabilities(fieldName, randomAlphaOfLengthBetween(5, 20),
+            randomBoolean(), randomBoolean(), meta);
+    }
+
     @Override
     protected FieldCapabilitiesResponse mutateInstance(FieldCapabilitiesResponse response) {
         Map<String, Map<String, FieldCapabilities>> mutatedResponses = new HashMap<>(response.get());
diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/MergedFieldCapabilitiesResponseTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/MergedFieldCapabilitiesResponseTests.java
@@ -152,7 +152,7 @@ public void testEmptyResponse() throws IOException {
 
     private static FieldCapabilitiesResponse createSimpleResponse() {
         Map<String, FieldCapabilities> titleCapabilities = new HashMap<>();
-        titleCapabilities.put("text", new FieldCapabilities("title", "text", true, false, Collections.emptyMap()));
+        titleCapabilities.put("text", new FieldCapabilities("title", "text", true, false, null, null, null, Collections.emptyMap()));
 
         Map<String, FieldCapabilities> ratingCapabilities = new HashMap<>();
         ratingCapabilities.put("long", new FieldCapabilities("rating", "long",
diff --git a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java
diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/extractor/ExtractedFieldsDetectorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/extractor/ExtractedFieldsDetectorTests.java
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java

Original file line number	Diff line number	Diff line change
`@@ -1242,7 +1242,7 @@ public void testFieldCaps() throws IOException {`
`1242`	`1242`	`assertEquals(1, fieldResponse.size());`
`1243`	`1243`
`1244`	`1244`	`FieldCapabilities expectedTextCapabilities = new FieldCapabilities(`
`1245`		`- "field", "text", true, false, Collections.emptyMap());`
	`1245`	`+ "field", "text", true, false, null, null, null, Collections.emptyMap());`
`1246`	`1246`	`assertEquals(expectedTextCapabilities, fieldResponse.get("text"));`
`1247`	`1247`	`}`
`1248`	`1248`
Original file line number	Diff line number	Diff line change
`@@ -182,14 +182,14 @@ private void addUnmappedFields(String[] indices, String field, Map<String, Field`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`private void innerMerge(Map<String, Map<String, FieldCapabilities.Builder>> responseMapBuilder,`
`185`		`- String indexName, Map<String, FieldCapabilities> map) {`
`186`		`- for (Map.Entry<String, FieldCapabilities> entry : map.entrySet()) {`
	`185`	`+ String indexName, Map<String, IndexFieldCapabilities> map) {`
	`186`	`+ for (Map.Entry<String, IndexFieldCapabilities> entry : map.entrySet()) {`
`187`	`187`	`final String field = entry.getKey();`
`188`		`- final FieldCapabilities fieldCap = entry.getValue();`
	`188`	`+ final IndexFieldCapabilities fieldCap = entry.getValue();`
`189`	`189`	`Map<String, FieldCapabilities.Builder> typeMap = responseMapBuilder.computeIfAbsent(field, f -> new HashMap<>());`
`190`	`190`	`FieldCapabilities.Builder builder = typeMap.computeIfAbsent(fieldCap.getType(),`
`191`	`191`	`key -> new FieldCapabilities.Builder(field, key));`
`192`		`- builder.merge(indexName, fieldCap.isSearchable(), fieldCap.isAggregatable(), fieldCap.meta());`
	`192`	`+ builder.add(indexName, fieldCap.isSearchable(), fieldCap.isAggregatable(), fieldCap.meta());`
`193`	`193`	`}`
`194`	`194`	`}`
`195`	`195`	`}`