Refactor FieldCapabilities creation by adding a proper builder object (#121310)

Reduce boilerplate associated with creating `FieldCapabilities` instances.
Since it's a class with a huge number of fields, it makes sense to define a builder object, as that can also help with all the Boolean and null blindness going on.
Note while there is a static Builder class in `FieldCapabilities`, it is not a proper builder object (no setters, still need to pass a lot of otherwise default parameters) and also package-private. To avoid changing that, I defined a new `FieldCapabilitiesBuilder` class. I also went over the code and refactored places which used the old constructor.

Author: GalLalouche

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/FieldCapsRankFeatureTests.java

@@ -10,6 +10,7 @@
 package org.elasticsearch.index.mapper.extras;
 
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
+import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.plugins.Plugin;
@@ -19,7 +20,6 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Map;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
@@ -59,10 +59,7 @@ public void testRankFeatureInIndex() {
         Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
         assertEquals(1, fooRankField.size());
         assertThat(fooRankField, Matchers.hasKey("rank_feature"));
-        assertEquals(
-            new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
-            fooRankField.get("rank_feature")
-        );
+        assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
     }
 
     public void testRankFeatureInIndexAfterRestart() throws Exception {
@@ -79,10 +76,7 @@ public void testRankFeatureInIndexAfterRestart() throws Exception {
         Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
         assertEquals(1, fooRankField.size());
         assertThat(fooRankField, Matchers.hasKey("rank_feature"));
-        assertEquals(
-            new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
-            fooRankField.get("rank_feature")
-        );
+        assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
     }
 
     public void testAllRankFeatureReturnedIfOneIsPresent() {
@@ -98,18 +92,16 @@ public void testAllRankFeatureReturnedIfOneIsPresent() {
         Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
         assertEquals(1, fooRankField.size());
         assertThat(fooRankField, Matchers.hasKey("rank_feature"));
-        assertEquals(
-            new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
-            fooRankField.get("rank_feature")
-        );
+        assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
         assertThat(response.get(), Matchers.hasKey("barRank"));
         // Check the capabilities for the 'barRank' field.
         Map<String, FieldCapabilities> barRankField = response.getField("barRank");
         assertEquals(1, barRankField.size());
         assertThat(barRankField, Matchers.hasKey("rank_feature"));
-        assertEquals(
-            new FieldCapabilities("barRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
-            barRankField.get("rank_feature")
-        );
+        assertEquals(fieldCapabilities("barRank"), barRankField.get("rank_feature"));
+    }
+
+    private static FieldCapabilities fieldCapabilities(String fieldName) {
+        return new FieldCapabilitiesBuilder(fieldName, "rank_feature").isAggregatable(false).build();
     }
 }

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java

@@ -17,6 +17,7 @@
 import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
+import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesFailure;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
@@ -219,24 +220,11 @@ public void testFieldAlias() {
         assertEquals(2, distance.size());
 
         assertTrue(distance.containsKey("double"));
-        assertEquals(
-            new FieldCapabilities(
-                "distance",
-                "double",
-                false,
-                true,
-                true,
-                new String[] { "old_index" },
-                null,
-                null,
-                Collections.emptyMap()
-            ),
-            distance.get("double")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("distance", "double").indices("old_index").build(), distance.get("double"));
 
         assertTrue(distance.containsKey("text"));
         assertEquals(
-            new FieldCapabilities("distance", "text", false, true, false, new String[] { "new_index" }, null, null, Collections.emptyMap()),
+            new FieldCapabilitiesBuilder("distance", "text").isAggregatable(false).indices("new_index").build(),
             distance.get("text")
         );
 
@@ -245,10 +233,7 @@ public void testFieldAlias() {
         assertEquals(1, routeLength.size());
 
         assertTrue(routeLength.containsKey("double"));
-        assertEquals(
-            new FieldCapabilities("route_length_miles", "double", false, true, true, null, null, null, Collections.emptyMap()),
-            routeLength.get("double")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("route_length_miles", "double").build(), routeLength.get("double"));
     }
 
     public void testFieldAliasWithWildcard() {
@@ -284,35 +269,19 @@ public void testWithUnmapped() {
         assertEquals(2, oldField.size());
 
         assertTrue(oldField.containsKey("long"));
-        assertEquals(
-            new FieldCapabilities("old_field", "long", false, true, true, new String[] { "old_index" }, null, null, Collections.emptyMap()),
-            oldField.get("long")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("old_field", "long").indices("old_index").build(), oldField.get("long"));
 
         assertTrue(oldField.containsKey("unmapped"));
         assertEquals(
-            new FieldCapabilities(
-                "old_field",
-                "unmapped",
-                false,
-                false,
-                false,
-                new String[] { "new_index" },
-                null,
-                null,
-                Collections.emptyMap()
-            ),
+            new FieldCapabilitiesBuilder("old_field", "unmapped").isSearchable(false).isAggregatable(false).indices("new_index").build(),
             oldField.get("unmapped")
         );
 
         Map<String, FieldCapabilities> newField = response.getField("new_field");
         assertEquals(1, newField.size());
 
         assertTrue(newField.containsKey("long"));
-        assertEquals(
-            new FieldCapabilities("new_field", "long", false, true, true, null, null, null, Collections.emptyMap()),
-            newField.get("long")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("new_field", "long").build(), newField.get("long"));
     }
 
     public void testWithIndexAlias() {
@@ -431,18 +400,15 @@ public void testMetadataFields() {
 
             assertTrue(idField.containsKey("_id"));
             assertEquals(
-                new FieldCapabilities("_id", "_id", true, true, false, null, null, null, Collections.emptyMap()),
+                new FieldCapabilitiesBuilder("_id", "_id").isMetadataField(true).isAggregatable(false).build(),
                 idField.get("_id")
             );
 
             Map<String, FieldCapabilities> testField = response.getField("_test");
             assertEquals(1, testField.size());
 
             assertTrue(testField.containsKey("keyword"));
-            assertEquals(
-                new FieldCapabilities("_test", "keyword", true, true, true, null, null, null, Collections.emptyMap()),
-                testField.get("keyword")
-            );
+            assertEquals(new FieldCapabilitiesBuilder("_test", "keyword").isMetadataField(true).build(), testField.get("keyword"));
         }
     }
 

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapsHasValueTests.java

@@ -11,6 +11,7 @@
 
 import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
+import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.support.ActiveShardCount;
@@ -76,10 +77,7 @@ public void testOnlyFieldsWithValueInIndex() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testOnlyFieldsWithValueInAlias() {
@@ -94,10 +92,7 @@ public void testOnlyFieldsWithValueInAlias() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testOnlyFieldsWithValueInSpecifiedIndex() {
@@ -112,10 +107,7 @@ public void testOnlyFieldsWithValueInSpecifiedIndex() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testOnlyFieldsWithValueInSpecifiedAlias() {
@@ -130,10 +122,7 @@ public void testOnlyFieldsWithValueInSpecifiedAlias() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testFieldsWithValueAfterUpdate() {
@@ -150,18 +139,12 @@ public void testFieldsWithValueAfterUpdate() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
         // Check the capabilities for the 'bar' field.
         Map<String, FieldCapabilities> barField = response.getField("bar");
         assertEquals(1, barField.size());
         assertThat(barField, Matchers.hasKey("keyword"));
-        assertEquals(
-            new FieldCapabilities("bar", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
-            barField.get("keyword")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").build(), barField.get("keyword"));
     }
 
     public void testOnlyFieldsWithValueAfterNodesRestart() throws Exception {
@@ -177,10 +160,7 @@ public void testOnlyFieldsWithValueAfterNodesRestart() throws Exception {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testFieldsAndAliasWithValue() {
@@ -198,26 +178,17 @@ public void testFieldsAndAliasWithValue() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
         // Check the capabilities for the 'bar' field.
         Map<String, FieldCapabilities> barField = response.getField("bar");
         assertEquals(1, barField.size());
         assertThat(barField, Matchers.hasKey("keyword"));
-        assertEquals(
-            new FieldCapabilities("bar", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
-            barField.get("keyword")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").build(), barField.get("keyword"));
         // Check the capabilities for the 'bar-alias' field.
         Map<String, FieldCapabilities> barAlias = response.getField("bar-alias");
         assertEquals(1, barAlias.size());
         assertThat(barAlias, Matchers.hasKey("keyword"));
-        assertEquals(
-            new FieldCapabilities("bar-alias", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
-            barAlias.get("keyword")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar-alias", "keyword").build(), barAlias.get("keyword"));
     }
 
     public void testUnmappedFieldsWithValueAfterRestart() throws Exception {
@@ -238,7 +209,7 @@ public void testUnmappedFieldsWithValueAfterRestart() throws Exception {
         assertEquals(2, unmappedField.size());
         assertThat(unmappedField, Matchers.hasKey("text"));
         assertEquals(
-            new FieldCapabilities("unmapped", "text", false, true, false, new String[] { INDEX1 }, null, null, Collections.emptyMap()),
+            new FieldCapabilitiesBuilder("unmapped", "text").isAggregatable(false).indices(INDEX1).build(),
             unmappedField.get("text")
         );
     }
@@ -257,18 +228,12 @@ public void testTwoFieldsNameTwoIndices() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
         // Check the capabilities for the 'bar' field.
         Map<String, FieldCapabilities> barField = response.getField("bar");
         assertEquals(1, barField.size());
         assertThat(barField, Matchers.hasKey("date"));
-        assertEquals(
-            new FieldCapabilities("bar", "date", false, true, true, null, null, null, Collections.emptyMap()),
-            barField.get("date")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar", "date").build(), barField.get("date"));
     }
 
     public void testSameFieldNameTwoIndices() {
@@ -284,15 +249,9 @@ public void testSameFieldNameTwoIndices() {
         Map<String, FieldCapabilities> barField = response.getField("bar");
         assertEquals(2, barField.size());
         assertThat(barField, Matchers.hasKey("keyword"));
-        assertEquals(
-            new FieldCapabilities("bar", "keyword", false, true, true, new String[] { INDEX1 }, null, null, Collections.emptyMap()),
-            barField.get("keyword")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").indices(INDEX1).build(), barField.get("keyword"));
         assertThat(barField, Matchers.hasKey("date"));
-        assertEquals(
-            new FieldCapabilities("bar", "date", false, true, true, new String[] { INDEX2 }, null, null, Collections.emptyMap()),
-            barField.get("date")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("bar", "date").indices(INDEX2).build(), barField.get("date"));
     }
 
     public void testDeletedDocsReturned() {
@@ -311,10 +270,7 @@ public void testDeletedDocsReturned() {
         Map<String, FieldCapabilities> fooField = response.getField("foo");
         assertEquals(1, fooField.size());
         assertThat(fooField, Matchers.hasKey("text"));
-        assertEquals(
-            new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
-            fooField.get("text")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
     }
 
     public void testNoNestedFieldsInEmptyIndex() {
@@ -339,15 +295,15 @@ public void testNestedFields() {
         assertEquals(1, nestedTypeField.size());
         assertThat(nestedTypeField, Matchers.hasKey("nested"));
         assertEquals(
-            new FieldCapabilities("nested_type", "nested", false, false, false, null, null, null, Collections.emptyMap()),
+            new FieldCapabilitiesBuilder("nested_type", "nested").isSearchable(false).isAggregatable(false).build(),
             nestedTypeField.get("nested")
         );
         // Check the capabilities for the 'nested_type.nested_field' field.
         Map<String, FieldCapabilities> nestedTypeNestedField = response.getField("nested_type.nested_field");
         assertEquals(1, nestedTypeNestedField.size());
         assertThat(nestedTypeNestedField, Matchers.hasKey("text"));
         assertEquals(
-            new FieldCapabilities("nested_type.nested_field", "text", false, true, false, null, null, null, Collections.emptyMap()),
+            new FieldCapabilitiesBuilder("nested_type.nested_field", "text").isAggregatable(false).build(),
             nestedTypeNestedField.get("text")
         );
     }
@@ -374,17 +330,14 @@ public void testObjectFields() {
         assertEquals(1, objectTypeField.size());
         assertThat(objectTypeField, Matchers.hasKey("object"));
         assertEquals(
-            new FieldCapabilities("object", "object", false, false, false, null, null, null, Collections.emptyMap()),
+            new FieldCapabilitiesBuilder("object", "object").isSearchable(false).isAggregatable(false).build(),
             objectTypeField.get("object")
         );
         // Check the capabilities for the 'object.sub_field' field.
         Map<String, FieldCapabilities> objectSubfield = response.getField("object.sub_field");
         assertEquals(1, objectSubfield.size());
         assertThat(objectSubfield, Matchers.hasKey("keyword"));
-        assertEquals(
-            new FieldCapabilities("object.sub_field", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
-            objectSubfield.get("keyword")
-        );
+        assertEquals(new FieldCapabilitiesBuilder("object.sub_field", "keyword").build(), objectSubfield.get("keyword"));
     }
 
     public void testWithIndexFilter() throws InterruptedException {