Watcher: Fix hipchat message failure serialization (elastic/x-pack-elasticsearch#3939)

The current toXContent serialization of a failed hipchat message writes
the same field called status twice and thus cannot be stored in the
watch history.

This commit ensures the field gets only written once.

relates elastic/x-pack-elasticsearch#3919

Original commit: elastic/x-pack-elasticsearch@fb499e8055

plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/SentMessages.java

@@ -119,9 +119,9 @@ public class SentMessages implements ToXContentObject, Iterable<SentMessages.Sen
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
             builder.startObject();
-            builder.field(STATUS.getPreferredName(), isSuccess() ? "success" : "failure");
+            boolean success = isSuccess();
-            if (isSuccess() == false) {
+            builder.field(STATUS.getPreferredName(), success ? "success" : "failure");
-                builder.field(STATUS.getPreferredName(), "failure");
+            if (success == false) {
                 if (request != null) {
                     builder.field(REQUEST.getPreferredName());
                     request.toXContent(builder, params);

plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/hipchat/HipChatActionTests.java

@@ -8,9 +8,12 @@ package org.elasticsearch.xpack.watcher.actions.hipchat;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.MapBuilder;
+import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.core.watcher.actions.Action;
@@ -30,6 +33,7 @@ import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.junit.Before;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -103,9 +107,9 @@ public class HipChatActionTests extends ESTestCase {
         HipChatMessage message = new HipChatMessage(body.getTemplate(), rooms, null, null, null, null, null);
         HipChatAccount account = mock(HipChatAccount.class);
         when(account.render(wid.watchId(), "_id", templateEngine, messageTemplate, expectedModel)).thenReturn(message);
-        HttpResponse response = mock(HttpResponse.class);
+        boolean responseFailure = randomBoolean();
-        when(response.status()).thenReturn(200);
+        HttpResponse response = new HttpResponse(responseFailure ? 404 : 200);
-        HttpRequest request = mock(HttpRequest.class);
+        HttpRequest request = HttpRequest.builder("localhost", 12345).path("/").build();
         SentMessages sentMessages = new SentMessages(accountName, Arrays.asList(
                 SentMessages.SentMessage.responded("_r1", SentMessages.SentMessage.TargetType.ROOM, message, request, response)
         ));
@@ -116,8 +120,13 @@ public class HipChatActionTests extends ESTestCase {
 
         assertThat(result, notNullValue());
         assertThat(result, instanceOf(HipChatAction.Result.Executed.class));
-        assertThat(result.status(), equalTo(Action.Result.Status.SUCCESS));
+        if (responseFailure) {
+            assertThat(result.status(), equalTo(Action.Result.Status.FAILURE));
+        } else {
+            assertThat(result.status(), equalTo(Action.Result.Status.SUCCESS));
+        }
         assertThat(((HipChatAction.Result.Executed) result).sentMessages(), sameInstance(sentMessages));
+        assertValidToXContent(result);
     }
 
     public void testParser() throws Exception {
@@ -267,4 +276,18 @@ public class HipChatActionTests extends ESTestCase {
             assertThat(e.getMessage(), is("failed to parse [hipchat] action [_watch/_action]. unexpected token [VALUE_STRING]"));
         }
     }
+
+    // ensure that toXContent can be serialized and read again
+    private void assertValidToXContent(Action.Result result) throws IOException {
+        try (XContentBuilder builder = jsonBuilder()) {
+            builder.startObject();
+            result.toXContent(builder, ToXContent.EMPTY_PARAMS);
+            builder.endObject();
+            builder.string();
+            try (XContentParser parser = XContentType.JSON.xContent()
+                    .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, builder.string())) {
+                parser.map();
+            }
+        }
+    }
 }