From 249a635ce11f707659baf7eb954a8e02c71f9975 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 19 Apr 2016 17:11:42 -0400 Subject: [PATCH] Fix BulkItemResponse.Failure.toString It was busted and causing intermittent test failures. --- .../action/bulk/BulkItemResponse.java | 2 +- .../org/elasticsearch/common/Strings.java | 18 +++++++++- .../action/bulk/BulkItemResponseTests.java | 35 +++++++++++++++++++ .../elasticsearch/common/StringsTests.java | 23 ++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java diff --git a/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java b/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java index 1062b397af5..ca2c0fc9f32 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java @@ -177,7 +177,7 @@ public class BulkItemResponse implements Streamable, StatusToXContent { @Override public String toString() { - return Strings.toString(this); + return Strings.toString(this, true); } } diff --git a/core/src/main/java/org/elasticsearch/common/Strings.java b/core/src/main/java/org/elasticsearch/common/Strings.java index bd1ba64c747..d180f8a8f80 100644 --- a/core/src/main/java/org/elasticsearch/common/Strings.java +++ b/core/src/main/java/org/elasticsearch/common/Strings.java @@ -20,6 +20,7 @@ package org.elasticsearch.common; import org.apache.lucene.util.BytesRefBuilder; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.FastStringReader; import org.elasticsearch.common.util.CollectionUtils; @@ -1062,12 +1063,27 @@ public class Strings { * {@link ToXContent}. */ public static String toString(ToXContent toXContent) { + return toString(toXContent, false); + } + + /** + * Return a {@link String} that is the json representation of the provided + * {@link ToXContent}. + * @param wrapInObject set this to true if the ToXContent instance expects to be inside an object + */ + public static String toString(ToXContent toXContent, boolean wrapInObject) { try { XContentBuilder builder = JsonXContent.contentBuilder(); + if (wrapInObject) { + builder.startObject(); + } toXContent.toXContent(builder, ToXContent.EMPTY_PARAMS); + if (wrapInObject) { + builder.endObject(); + } return builder.string(); } catch (IOException e) { - throw new AssertionError("Cannot happen", e); + return "Error building toString out of XContent: " + ExceptionsHelper.stackTrace(e); } } diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java new file mode 100644 index 00000000000..ec82c481e6c --- /dev/null +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java @@ -0,0 +1,35 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.bulk; + +import org.elasticsearch.action.bulk.BulkItemResponse.Failure; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.containsString; + +public class BulkItemResponseTests extends ESTestCase { + public void testFailureToString() { + Failure failure = new Failure("index", "type", "id", new RuntimeException("test")); + String toString = failure.toString(); + assertThat(toString, containsString("\"type\":\"runtime_exception\"")); + assertThat(toString, containsString("\"reason\":\"test\"")); + assertThat(toString, containsString("\"status\":500")); + } +} diff --git a/core/src/test/java/org/elasticsearch/common/StringsTests.java b/core/src/test/java/org/elasticsearch/common/StringsTests.java index 07465acaaae..65d697a63ae 100644 --- a/core/src/test/java/org/elasticsearch/common/StringsTests.java +++ b/core/src/test/java/org/elasticsearch/common/StringsTests.java @@ -19,8 +19,14 @@ package org.elasticsearch.common; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.test.ESTestCase; +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; + public class StringsTests extends ESTestCase { public void testToCamelCase() { assertEquals("foo", Strings.toCamelCase("foo")); @@ -60,4 +66,21 @@ public class StringsTests extends ESTestCase { assertEquals("o", Strings.cleanTruncate("o\uD83D\uDEAB", 1)); assertEquals("", Strings.cleanTruncate("foo", 0)); } + + public void testEvilToString() { + ToXContent needsEnclosingObject = new ToXContent() { + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.field("ok", "here").field("catastrophe", ""); + } + }; + String toString = Strings.toString(needsEnclosingObject); + assertThat(toString, containsString("Error building toString out of XContent")); + assertThat(toString, containsString("Can not write a field name, expecting a value")); + + // We can salvage it! + toString = Strings.toString(needsEnclosingObject, true); + assertThat(toString, containsString("\"ok\":\"here\"")); + assertThat(toString, containsString("\"catastrophe\":\"\"")); + } }