Make sure that IdFieldType#isAggregatable is accurate.

Before, it always returned 'true' even when the setting
"indices.id_field_data.enabled" was false.

Author: jtibshirani

server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

@@ -54,6 +54,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.BooleanSupplier;
 import java.util.function.Supplier;
 
 /**
@@ -95,15 +96,18 @@ public static class Defaults {
         }
     }
 
-    public static final TypeParser PARSER = new FixedTypeParser(c -> new IdFieldMapper());
+    public static final TypeParser PARSER = new FixedTypeParser(c -> {
+        BooleanSupplier fieldDataEnabled = c.mapperService().isIdFieldDataEnabled();
+        return new IdFieldMapper(fieldDataEnabled);
+    });
 
     static final class IdFieldType extends TermBasedFieldType {
+        private final BooleanSupplier fieldDataEnabled;
 
-        public static final IdFieldType INSTANCE = new IdFieldType();
-
-        private IdFieldType() {
+        IdFieldType(BooleanSupplier fieldDataEnabled) {
             super(NAME, true, true, false, TextSearchInfo.SIMPLE_MATCH_ONLY, Collections.emptyMap());
             setIndexAnalyzer(Lucene.KEYWORD_ANALYZER);
+            this.fieldDataEnabled = fieldDataEnabled;
         }
 
         @Override
@@ -143,6 +147,11 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
 
         @Override
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
+            if (fieldDataEnabled.getAsBoolean() == false) {
+                throw new IllegalArgumentException("Fielddata access on the _id field is disallowed, "
+                    + "you can re-enable it by updating the dynamic cluster setting: "
+                    + IndicesService.INDICES_ID_FIELD_DATA_ENABLED_SETTING.getKey());
+            }
             final IndexFieldData.Builder fieldDataBuilder = new PagedBytesIndexFieldData.Builder(
                     name(),
                     TextFieldMapper.Defaults.FIELDDATA_MIN_FREQUENCY,
@@ -156,11 +165,6 @@ public IndexFieldData<?> build(
                     CircuitBreakerService breakerService,
                     MapperService mapperService
                 ) {
-                    if (mapperService.isIdFieldDataEnabled() == false) {
-                        throw new IllegalArgumentException("Fielddata access on the _id field is disallowed, "
-                            + "you can re-enable it by updating the dynamic cluster setting: "
-                            + IndicesService.INDICES_ID_FIELD_DATA_ENABLED_SETTING.getKey());
-                    }
                     deprecationLogger.deprecate("id_field_data", ID_FIELD_DATA_DEPRECATION_MESSAGE);
                     final IndexFieldData<?> fieldData = fieldDataBuilder.build(cache,
                         breakerService, mapperService);
@@ -251,8 +255,8 @@ public boolean advanceExact(int doc) throws IOException {
         };
     }
 
-    private IdFieldMapper() {
-        super(new IdFieldType());
+    private IdFieldMapper(BooleanSupplier fieldDataEnabled) {
+        super(new IdFieldType(fieldDataEnabled));
     }
 
     @Override

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -464,8 +464,8 @@ public Analyzer searchQuoteAnalyzer() {
     /**
      * Returns <code>true</code> if fielddata is enabled for the {@link IdFieldMapper} field, <code>false</code> otherwise.
      */
-    public boolean isIdFieldDataEnabled() {
-        return idFieldDataEnabled.getAsBoolean();
+    public BooleanSupplier isIdFieldDataEnabled() {
+        return idFieldDataEnabled;
     }
 
     @Override

server/src/test/java/org/elasticsearch/index/mapper/IdFieldMapperTests.java

@@ -88,6 +88,7 @@ public void testEnableFieldData() throws IOException {
                 throw new UnsupportedOperationException();
             }).build(null, null, mapperService));
         assertThat(exc.getMessage(), containsString(IndicesService.INDICES_ID_FIELD_DATA_ENABLED_SETTING.getKey()));
+        assertFalse(ft.isAggregatable());
 
         client().admin().cluster().prepareUpdateSettings()
             .setTransientSettings(Settings.builder().put(IndicesService.INDICES_ID_FIELD_DATA_ENABLED_SETTING.getKey(), true))
@@ -97,6 +98,7 @@ public void testEnableFieldData() throws IOException {
                 throw new UnsupportedOperationException();
             }).build(null, null, mapperService);
             assertWarnings(ID_FIELD_DATA_DEPRECATION_MESSAGE);
+            assertTrue(ft.isAggregatable());
         } finally {
             // unset cluster setting
             client().admin().cluster().prepareUpdateSettings()

server/src/test/java/org/elasticsearch/index/mapper/IdFieldTypeTests.java

@@ -32,7 +32,7 @@
 public class IdFieldTypeTests extends ESTestCase {
 
     public void testRangeQuery() {
-        MappedFieldType ft = IdFieldMapper.IdFieldType.INSTANCE;
+        MappedFieldType ft = new IdFieldMapper.IdFieldType(() -> false);
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
                 () -> ft.rangeQuery(null, null, randomBoolean(), randomBoolean(), null, null, null, null));
         assertEquals("Field [_id] of type [_id] does not support range queries", e.getMessage());
@@ -53,8 +53,16 @@ public void testTermsQuery() throws Exception {
         MapperService mapperService = Mockito.mock(MapperService.class);
         Mockito.when(context.getMapperService()).thenReturn(mapperService);
 
-        MappedFieldType ft = IdFieldMapper.IdFieldType.INSTANCE;
+        MappedFieldType ft = new IdFieldMapper.IdFieldType(() -> false);
         Query query = ft.termQuery("id", context);
         assertEquals(new TermInSetQuery("_id", Uid.encodeId("id")), query);
     }
+
+    public void testIsAggregatable() {
+        MappedFieldType ft = new IdFieldMapper.IdFieldType(() -> false);
+        assertFalse(ft.isAggregatable());
+
+        ft = new IdFieldMapper.IdFieldType(() -> true);
+        assertTrue(ft.isAggregatable());
+    }
 }