Apply review comments

Author: przemekwitek

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java

@@ -24,8 +24,10 @@
 import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState;
 import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats;
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 
@@ -38,6 +40,9 @@ public class GetDatafeedsStatsAction extends StreamableResponseActionType<GetDat
 
     public static final String ALL = "_all";
     private static final String STATE = "state";
+    private static final String NODE = "node";
+    private static final String ASSIGNMENT_EXPLANATION = "assignment_explanation";
+    private static final String TIMING_STATS = "timing_stats";
 
     private GetDatafeedsStatsAction() {
         super(NAME);
@@ -181,7 +186,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                 builder.field(DatafeedConfig.ID.getPreferredName(), datafeedId);
                 builder.field(STATE, datafeedState.toString());
                 if (node != null) {
-                    builder.startObject("node");
+                    builder.startObject(NODE);
                     builder.field("id", node.getId());
                     builder.field("name", node.getName());
                     builder.field("ephemeral_id", node.getEphemeralId());
@@ -197,11 +202,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                     builder.endObject();
                 }
                 if (assignmentExplanation != null) {
-                    builder.field("assignment_explanation", assignmentExplanation);
+                    builder.field(ASSIGNMENT_EXPLANATION, assignmentExplanation);
                 }
                 if (timingStats != null) {
-                    builder.field("timing_stats");
-                    timingStats.toXContentWithCalculatedFields(builder);
+                    builder.field(
+                        TIMING_STATS,
+                        timingStats,
+                        new MapParams(Collections.singletonMap(ToXContentParams.INCLUDE_CALCULATED_FIELDS, "true")));
                 }
                 builder.endObject();
                 return builder;

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetJobsStatsAction.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.TimingStats;
 import org.elasticsearch.xpack.core.ml.stats.ForecastStats;
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -51,6 +52,8 @@ public class GetJobsStatsAction extends ActionType<GetJobsStatsAction.Response>
     private static final String FORECASTS_STATS = "forecasts_stats";
     private static final String STATE = "state";
     private static final String NODE = "node";
+    private static final String ASSIGNMENT_EXPLANATION = "assignment_explanation";
+    private static final String OPEN_TIME = "open_time";
     private static final String TIMING_STATS = "timing_stats";
 
     private GetJobsStatsAction() {
@@ -266,14 +269,16 @@ public XContentBuilder toUnwrappedXContent(XContentBuilder builder) throws IOExc
                     builder.endObject();
                 }
                 if (assignmentExplanation != null) {
-                    builder.field("assignment_explanation", assignmentExplanation);
+                    builder.field(ASSIGNMENT_EXPLANATION, assignmentExplanation);
                 }
                 if (openTime != null) {
-                    builder.field("open_time", openTime.getStringRep());
+                    builder.field(OPEN_TIME, openTime.getStringRep());
                 }
                 if (timingStats != null) {
-                    builder.field(TIMING_STATS);
-                    timingStats.toXContentWithCalculatedFields(builder);
+                    builder.field(
+                        TIMING_STATS,
+                        timingStats,
+                        new MapParams(Collections.singletonMap(ToXContentParams.INCLUDE_CALCULATED_FIELDS, "true")));
                 }
                 return builder;
             }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java

@@ -15,6 +15,7 @@
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -126,20 +127,12 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
-        return toXContent(builder, false);
-    }
-
-    public XContentBuilder toXContentWithCalculatedFields(XContentBuilder builder) throws IOException {
-        return toXContent(builder, true);
-    }
-
-    private XContentBuilder toXContent(XContentBuilder builder, boolean includeCalculatedFields) throws IOException {
         builder.startObject();
         builder.field(JOB_ID.getPreferredName(), jobId);
         builder.field(SEARCH_COUNT.getPreferredName(), searchCount);
         builder.field(BUCKET_COUNT.getPreferredName(), bucketCount);
         builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs);
-        if (includeCalculatedFields) {
+        if (params.paramAsBoolean(ToXContentParams.INCLUDE_CALCULATED_FIELDS, false)) {
             Double avgSearchTimePerBucket = getAvgSearchTimePerBucketMs();
             if (avgSearchTimePerBucket != null) {
                 builder.field(AVG_SEARCH_TIME_PER_BUCKET_MS.getPreferredName(), getAvgSearchTimePerBucketMs());

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java

@@ -920,9 +920,6 @@ private static void addDataCountsMapping(XContentBuilder builder) throws IOExcep
     private static void addTimingStatsExceptBucketCountMapping(XContentBuilder builder) throws IOException {
         builder
             // re-used: BUCKET_COUNT
-            .startObject(TimingStats.TOTAL_BUCKET_PROCESSING_TIME_MS.getPreferredName())
-                .field(TYPE, DOUBLE)
-            .endObject()
             .startObject(TimingStats.MIN_BUCKET_PROCESSING_TIME_MS.getPreferredName())
                 .field(TYPE, DOUBLE)
             .endObject()
@@ -951,9 +948,6 @@ private static void addDatafeedTimingStats(XContentBuilder builder) throws IOExc
             // re-used: BUCKET_COUNT
             .startObject(DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName())
                 .field(TYPE, DOUBLE)
-            .endObject()
-            .startObject(DatafeedTimingStats.AVG_SEARCH_TIME_PER_BUCKET_MS.getPreferredName())
-                .field(TYPE, DOUBLE)
             .endObject();
     }
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/process/autodetect/state/TimingStats.java

@@ -15,6 +15,7 @@
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.xpack.core.ml.job.config.Job;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -193,18 +194,10 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        return toXContent(builder, false);
-    }
-
-    public XContentBuilder toXContentWithCalculatedFields(XContentBuilder builder) throws IOException {
-        return toXContent(builder, true);
-    }
-
-    private XContentBuilder toXContent(XContentBuilder builder, boolean includeCalculatedFields) throws IOException {
         builder.startObject();
         builder.field(Job.ID.getPreferredName(), jobId);
         builder.field(BUCKET_COUNT.getPreferredName(), bucketCount);
-        if (includeCalculatedFields) {
+        if (params.paramAsBoolean(ToXContentParams.INCLUDE_CALCULATED_FIELDS, false)) {
             builder.field(TOTAL_BUCKET_PROCESSING_TIME_MS.getPreferredName(), getTotalBucketProcessingTimeMs());
         }
         if (minBucketProcessingTimeMs != null) {

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java

@@ -181,7 +181,6 @@ public final class ReservedFieldNames {
             Result.IS_INTERIM.getPreferredName(),
 
             TimingStats.BUCKET_COUNT.getPreferredName(),
-            TimingStats.TOTAL_BUCKET_PROCESSING_TIME_MS.getPreferredName(),
             TimingStats.MIN_BUCKET_PROCESSING_TIME_MS.getPreferredName(),
             TimingStats.MAX_BUCKET_PROCESSING_TIME_MS.getPreferredName(),
             TimingStats.AVG_BUCKET_PROCESSING_TIME_MS.getPreferredName(),
@@ -190,7 +189,6 @@ public final class ReservedFieldNames {
             DatafeedTimingStats.SEARCH_COUNT.getPreferredName(),
             DatafeedTimingStats.BUCKET_COUNT.getPreferredName(),
             DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(),
-            DatafeedTimingStats.AVG_SEARCH_TIME_PER_BUCKET_MS.getPreferredName(),
 
             GetResult._ID,
             GetResult._INDEX,

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/ToXContentParams.java

@@ -24,6 +24,12 @@ public final class ToXContentParams {
      */
     public static final String INCLUDE_TYPE = "include_type";
 
+    /**
+     * When serialising POJOs to X Content this indicates whether the calculated (i.e. not stored) fields
+     * should be included or not
+     */
+    public static final String INCLUDE_CALCULATED_FIELDS = "include_calculated_fields";
+
     private ToXContentParams() {
     }
 }