Watcher: Ensure all json builders use try-with-resources (elastic/x-pack-elasticsearch#650)

Some json builders in the codebase were not closed. even
though this is not needed for the BytesStreamOutput being used,
there is more closing logic in the jackson classes, which we
should not rely on, that those never change or are ok to not
close.

Original commit: elastic/x-pack-elasticsearch@05a43d80ff

plugin/src/main/java/org/elasticsearch/xpack/watcher/actions/index/ExecutableIndexAction.java

@@ -13,6 +13,7 @@ import org.elasticsearch.action.bulk.BulkResponse;
 import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentType;
@@ -87,18 +88,23 @@ public class ExecutableIndexAction extends ExecutableAction<IndexAction> {
         indexRequest.id(docId);
 
         data = addTimestampToDocument(data, ctx.executionTime());
-        indexRequest.source(jsonBuilder().prettyPrint().map(data));
+        IndexResponse response;
+        BytesReference bytesReference;
+        try (XContentBuilder builder = jsonBuilder()) {
+            indexRequest.source(builder.prettyPrint().map(data));
+        }
 
         if (ctx.simulateAction(actionId)) {
             return new IndexAction.Simulated(indexRequest.index(), action.docType, docId, new XContentSource(indexRequest.source(),
                     XContentType.JSON));
         }
 
-        IndexResponse response = client.index(indexRequest, timeout);
+        response = client.index(indexRequest, timeout);
-        XContentBuilder jsonBuilder = jsonBuilder();
+        try (XContentBuilder builder = jsonBuilder()) {
-        indexResponseToXContent(jsonBuilder, response);
+            indexResponseToXContent(builder, response);
-
+            bytesReference = builder.bytes();
-        return new IndexAction.Result(Status.SUCCESS, new XContentSource(jsonBuilder));
+        }
+        return new IndexAction.Result(Status.SUCCESS, new XContentSource(bytesReference, XContentType.JSON));
     }
 
     Action.Result indexBulk(Iterable list, String actionId, WatchExecutionContext ctx) throws Exception {
@@ -121,24 +127,27 @@ public class ExecutableIndexAction extends ExecutableAction<IndexAction> {
                 indexRequest.id(doc.remove(ID_FIELD).toString());
             }
             doc = addTimestampToDocument(doc, ctx.executionTime());
-            indexRequest.source(jsonBuilder().prettyPrint().map(doc));
+            try (XContentBuilder builder = jsonBuilder()) {
+                indexRequest.source(builder.prettyPrint().map(doc));
+            }
             bulkRequest.add(indexRequest);
         }
         BulkResponse bulkResponse = client.bulk(bulkRequest, action.timeout);
-        XContentBuilder jsonBuilder = jsonBuilder().startArray();
+        try (XContentBuilder jsonBuilder = jsonBuilder().startArray()) {
-        for (BulkItemResponse item : bulkResponse) {
+            for (BulkItemResponse item : bulkResponse) {
-            itemResponseToXContent(jsonBuilder, item);
+                itemResponseToXContent(jsonBuilder, item);
-        }
+            }
-        jsonBuilder.endArray();
+            jsonBuilder.endArray();
 
-        // different error states, depending on how successful the bulk operation was
+            // different error states, depending on how successful the bulk operation was
-        long failures = Stream.of(bulkResponse.getItems()).filter(BulkItemResponse::isFailed).count();
+            long failures = Stream.of(bulkResponse.getItems()).filter(BulkItemResponse::isFailed).count();
-        if (failures == 0) {
+            if (failures == 0) {
-            return new IndexAction.Result(Status.SUCCESS, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
+                return new IndexAction.Result(Status.SUCCESS, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
-        } else if (failures == bulkResponse.getItems().length) {
+            } else if (failures == bulkResponse.getItems().length) {
-            return new IndexAction.Result(Status.FAILURE, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
+                return new IndexAction.Result(Status.FAILURE, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
-        } else {
+            } else {
-            return new IndexAction.Result(Status.PARTIAL_FAILURE, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
+                return new IndexAction.Result(Status.PARTIAL_FAILURE, new XContentSource(jsonBuilder.bytes(), XContentType.JSON));
+            }
         }
     }
 

plugin/src/main/java/org/elasticsearch/xpack/watcher/client/WatchSourceBuilder.java

@@ -117,7 +117,9 @@ public class WatchSourceBuilder extends ToXContentToBytes implements ToXContent
     }
 
     public XContentSource build() throws IOException {
-        return new XContentSource(toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS));
+        try (XContentBuilder builder = jsonBuilder()) {
+            return new XContentSource(toXContent(builder, ToXContent.EMPTY_PARAMS));
+        }
     }
 
     @Override

plugin/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java

@@ -22,6 +22,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
@@ -109,7 +110,9 @@ public class TriggeredWatchStore extends AbstractComponent {
         for (TriggeredWatch triggeredWatch : triggeredWatches) {
             try {
                 IndexRequest indexRequest = new IndexRequest(INDEX_NAME, DOC_TYPE, triggeredWatch.id().value());
-                indexRequest.source(XContentFactory.jsonBuilder().value(triggeredWatch));
+                try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
+                    indexRequest.source(xContentBuilder.value(triggeredWatch));
+                }
                 indexRequest.opType(IndexRequest.OpType.CREATE);
                 request.add(indexRequest);
             } catch (IOException e) {
@@ -141,7 +144,9 @@ public class TriggeredWatchStore extends AbstractComponent {
             BulkRequest request = new BulkRequest();
             for (TriggeredWatch triggeredWatch : triggeredWatches) {
                 IndexRequest indexRequest = new IndexRequest(INDEX_NAME, DOC_TYPE, triggeredWatch.id().value());
-                indexRequest.source(XContentFactory.jsonBuilder().value(triggeredWatch));
+                try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
+                    indexRequest.source(xContentBuilder.value(triggeredWatch));
+                }
                 indexRequest.opType(IndexRequest.OpType.CREATE);
                 request.add(indexRequest);
             }

plugin/src/main/java/org/elasticsearch/xpack/watcher/history/HistoryStore.java

@@ -106,9 +106,11 @@ public class HistoryStore extends AbstractComponent {
         } catch (VersionConflictEngineException vcee) {
             watchRecord = new WatchRecord.MessageWatchRecord(watchRecord, ExecutionState.EXECUTED_MULTIPLE_TIMES,
                     "watch record [{ " + watchRecord.id() + " }] has been stored before, previous state [" + watchRecord.state() + "]");
-            IndexRequest request = new IndexRequest(index, DOC_TYPE, watchRecord.id().value())
+            try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
-                    .source(XContentFactory.jsonBuilder().value(watchRecord));
+                IndexRequest request = new IndexRequest(index, DOC_TYPE, watchRecord.id().value())
-            client.index(request, (TimeValue) null);
+                        .source(xContentBuilder.value(watchRecord));
+                client.index(request, (TimeValue) null);
+            }
         } catch (IOException ioe) {
             throw ioException("failed to persist watch record [{}]", ioe, watchRecord);
         } finally {

plugin/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestHijackOperationAction.java

@@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.rest.action;
 
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.rest.BytesRestResponse;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -56,13 +55,16 @@ public class RestHijackOperationAction extends WatcherRestHandler {
         if (request.hasParam("id")) {
             request.param("id");
         }
-        XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
+        return channel -> {
-        jsonBuilder.startObject().field("error", "This endpoint is not supported for " +
+            try (XContentBuilder builder = channel.newErrorBuilder()) {
-                request.method().name() + " on " + Watch.INDEX + " index. Please use " +
+                builder.startObject().field("error", "This endpoint is not supported for " +
-                request.method().name() + " " + URI_BASE + "/watch/<watch_id> instead");
+                        request.method().name() + " on " + Watch.INDEX + " index. Please use " +
-        jsonBuilder.field("status", RestStatus.BAD_REQUEST.getStatus());
+                        request.method().name() + " " + URI_BASE + "/watch/<watch_id> instead");
-        jsonBuilder.endObject();
+                builder.field("status", RestStatus.BAD_REQUEST.getStatus());
-        return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, jsonBuilder));
+                builder.endObject();
+                channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, builder));
+            }
+        };
     }
 
     private static class UnsupportedHandler extends WatcherRestHandler {
@@ -77,14 +79,16 @@ public class RestHijackOperationAction extends WatcherRestHandler {
             if (request.hasParam("id")) {
                 request.param("id");
             }
-            XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
+
-            jsonBuilder.startObject().field("error", "This endpoint is not supported for " +
+            return channel -> {
-                    request.method().name() + " on " + Watch.INDEX + " index.");
+                try (XContentBuilder builder = channel.newErrorBuilder()) {
-            jsonBuilder.field("status", RestStatus.BAD_REQUEST.getStatus());
+                    builder.startObject().field("error", "This endpoint is not supported for " +
-            jsonBuilder.endObject();
+                            request.method().name() + " on " + Watch.INDEX + " index.");
-            return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, jsonBuilder));
+                    builder.field("status", RestStatus.BAD_REQUEST.getStatus());
+                    builder.endObject();
+                    channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, builder));
+                }
+            };
         }
-
     }
-
 }

plugin/src/main/java/org/elasticsearch/xpack/watcher/support/xcontent/XContentSource.java

@@ -51,10 +51,6 @@ public class XContentSource implements ToXContent {
         this(builder.bytes(), builder.contentType());
     }
 
-    public XContentSource(ToXContent content) throws IOException {
-        this(content.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS));
-    }
-
     /**
      * @return The bytes reference of the source
      */

plugin/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/activate/TransportActivateWatchAction.java

@@ -109,14 +109,16 @@ public class TransportActivateWatchAction extends WatcherTransportAction<Activat
     }
 
     private XContentBuilder activateWatchBuilder(boolean active, DateTime now) throws IOException {
-        XContentBuilder builder = jsonBuilder().startObject()
+        try (XContentBuilder builder = jsonBuilder()) {
-                .startObject(Watch.Field.STATUS.getPreferredName())
+            builder.startObject()
-                .startObject(WatchStatus.Field.STATE.getPreferredName())
+                    .startObject(Watch.Field.STATUS.getPreferredName())
-                .field(WatchStatus.Field.ACTIVE.getPreferredName(), active);
+                    .startObject(WatchStatus.Field.STATE.getPreferredName())
+                    .field(WatchStatus.Field.ACTIVE.getPreferredName(), active);
 
-        writeDate(WatchStatus.Field.TIMESTAMP.getPreferredName(), builder, now);
+            writeDate(WatchStatus.Field.TIMESTAMP.getPreferredName(), builder, now);
-        builder.endObject().endObject().endObject();
+            builder.endObject().endObject().endObject();
-        return builder;
+            return builder;
+        }
     }
 
     @Override

plugin/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java

@@ -5,12 +5,10 @@
  */
 package org.elasticsearch.xpack.monitoring;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.XPackFeatureSet;
@@ -21,6 +19,11 @@ import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
 import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource;
 import org.junit.Before;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.core.Is.is;
@@ -99,7 +102,11 @@ public class MonitoringFeatureSetTests extends ESTestCase {
         for (XPackFeatureSet.Usage usage : Arrays.asList(monitoringUsage, serializedUsage)) {
             assertThat(usage.name(), is(featureSet.name()));
             assertThat(usage.enabled(), is(featureSet.enabled()));
-            XContentSource source = new XContentSource(usage);
+            XContentSource source;
+            try (XContentBuilder builder = jsonBuilder()) {
+                usage.toXContent(builder, ToXContent.EMPTY_PARAMS);
+                source = new XContentSource(builder);
+            }
             assertThat(source.getValue("enabled_exporters"), is(notNullValue()));
             if (localCount > 0) {
                 assertThat(source.getValue("enabled_exporters.local"), is(localCount));

plugin/src/test/java/org/elasticsearch/xpack/security/SecurityFeatureSetTests.java

@@ -5,14 +5,12 @@
  */
 package org.elasticsearch.xpack.security;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
 import org.elasticsearch.common.collect.MapBuilder;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.XPackFeatureSet;
@@ -26,6 +24,11 @@ import org.elasticsearch.xpack.security.user.AnonymousUser;
 import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource;
 import org.junit.Before;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.hasEntry;
@@ -156,7 +159,11 @@ public class SecurityFeatureSetTests extends ESTestCase {
             assertThat(usage.name(), is(XPackPlugin.SECURITY));
             assertThat(usage.enabled(), is(enabled));
             assertThat(usage.available(), is(authcAuthzAvailable));
-            XContentSource source = new XContentSource(usage);
+            XContentSource source;
+            try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
+                usage.toXContent(builder, ToXContent.EMPTY_PARAMS);
+                source = new XContentSource(builder);
+            }
 
             if (enabled) {
                 if (authcAuthzAvailable) {