From 9f877b289270722a541055b7f618b87663ae90ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Kupczy=C5=84ski?= Date: Thu, 5 Mar 2015 12:53:13 +0100 Subject: [PATCH 1/3] Not swalling exceptions --- .../src/main/java/io/searchbox/action/AbstractAction.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/jest-common/src/main/java/io/searchbox/action/AbstractAction.java b/jest-common/src/main/java/io/searchbox/action/AbstractAction.java index 0f2d3fca1..f826f95d1 100644 --- a/jest-common/src/main/java/io/searchbox/action/AbstractAction.java +++ b/jest-common/src/main/java/io/searchbox/action/AbstractAction.java @@ -88,11 +88,7 @@ protected T createNewElasticSearchResult(T result, String json, int statusCode, protected static JsonObject convertJsonStringToMapObject(String jsonTxt) { if (jsonTxt != null && !jsonTxt.trim().isEmpty()) { - try { return new JsonParser().parse(jsonTxt).getAsJsonObject(); - } catch (Exception e) { - log.error("An exception occurred while converting json string to map object"); - } } return new JsonObject(); } From e4236ad9c8377202b901ae8d86eaf321bf3e89a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Kupczy=C5=84ski?= Date: Thu, 5 Mar 2015 18:02:29 +0100 Subject: [PATCH 2/3] Added a test to verify that exception is not swallowed --- .../java/io/searchbox/action/AbstractActionTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java b/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java index 2c470519e..4feadd19d 100644 --- a/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java +++ b/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java @@ -2,6 +2,7 @@ import com.google.gson.Gson; import com.google.gson.JsonObject; +import com.google.gson.JsonSyntaxException; import io.searchbox.annotations.JestId; import io.searchbox.client.JestResult; import io.searchbox.core.Delete; @@ -9,7 +10,9 @@ import io.searchbox.core.Index; import io.searchbox.core.Update; import io.searchbox.indices.Flush; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import static org.junit.Assert.*; @@ -18,6 +21,9 @@ */ public class AbstractActionTest { + @Rule + public ExpectedException exception = ExpectedException.none(); + @Test public void buildRestUrlWithValidParameters() { String expected = "twitter/tweet/1"; @@ -179,6 +185,12 @@ public void convertNullJsonStringToMapObject() { assertNotNull(jsonMap); } + @Test + public void propagateExceptionWhenTheResponseIsNotJson() { + exception.expect(JsonSyntaxException.class); + AbstractAction.convertJsonStringToMapObject("401 Unauthorized"); + } + @Test public void getSuccessIndexResult() { From 36846d926802b0c79e678159bc3e842d53fad6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Kupczy=C5=84ski?= Date: Fri, 6 Mar 2015 23:27:30 +0100 Subject: [PATCH 3/3] Fixed the failing tests --- .../java/io/searchbox/action/AbstractAction.java | 2 +- .../java/io/searchbox/cluster/NodesHotThreads.java | 6 ++++++ .../io/searchbox/action/AbstractActionTest.java | 14 +++++--------- .../io/searchbox/core/SearchIntegrationTest.java | 5 ++++- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/jest-common/src/main/java/io/searchbox/action/AbstractAction.java b/jest-common/src/main/java/io/searchbox/action/AbstractAction.java index f826f95d1..300b2c7f3 100644 --- a/jest-common/src/main/java/io/searchbox/action/AbstractAction.java +++ b/jest-common/src/main/java/io/searchbox/action/AbstractAction.java @@ -86,7 +86,7 @@ protected T createNewElasticSearchResult(T result, String json, int statusCode, return result; } - protected static JsonObject convertJsonStringToMapObject(String jsonTxt) { + protected JsonObject convertJsonStringToMapObject(String jsonTxt) { if (jsonTxt != null && !jsonTxt.trim().isEmpty()) { return new JsonParser().parse(jsonTxt).getAsJsonObject(); } diff --git a/jest-common/src/main/java/io/searchbox/cluster/NodesHotThreads.java b/jest-common/src/main/java/io/searchbox/cluster/NodesHotThreads.java index beac6c42f..84575fb83 100644 --- a/jest-common/src/main/java/io/searchbox/cluster/NodesHotThreads.java +++ b/jest-common/src/main/java/io/searchbox/cluster/NodesHotThreads.java @@ -1,5 +1,6 @@ package io.searchbox.cluster; +import com.google.gson.JsonObject; import io.searchbox.action.AbstractMultiINodeActionBuilder; import io.searchbox.action.GenericResultAbstractAction; @@ -26,6 +27,11 @@ protected String buildURI() { return sb.toString(); } + @Override + protected JsonObject convertJsonStringToMapObject(String jsonTxt) { + return new JsonObject(); + } + @Override public String getRestMethodName() { return "GET"; diff --git a/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java b/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java index 4feadd19d..1ec8097cf 100644 --- a/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java +++ b/jest-common/src/test/java/io/searchbox/action/AbstractActionTest.java @@ -21,9 +21,6 @@ */ public class AbstractActionTest { - @Rule - public ExpectedException exception = ExpectedException.none(); - @Test public void buildRestUrlWithValidParameters() { String expected = "twitter/tweet/1"; @@ -164,7 +161,7 @@ public void convertJsonStringToMapObject() { " \"_type\" : \"tweet\",\n" + " \"_id\" : \"1\"\n" + "}"; - JsonObject jsonMap = AbstractAction.convertJsonStringToMapObject(json); + JsonObject jsonMap = new DummyAction.Builder().build().convertJsonStringToMapObject(json); assertNotNull(jsonMap); assertEquals(4, jsonMap.entrySet().size()); assertEquals(true, jsonMap.get("ok").getAsBoolean()); @@ -175,20 +172,19 @@ public void convertJsonStringToMapObject() { @Test public void convertEmptyJsonStringToMapObject() { - JsonObject jsonMap = AbstractAction.convertJsonStringToMapObject(""); + JsonObject jsonMap = new DummyAction.Builder().build().convertJsonStringToMapObject(""); assertNotNull(jsonMap); } @Test public void convertNullJsonStringToMapObject() { - JsonObject jsonMap = AbstractAction.convertJsonStringToMapObject(null); + JsonObject jsonMap = new DummyAction.Builder().build().convertJsonStringToMapObject(null); assertNotNull(jsonMap); } - @Test + @Test(expected = JsonSyntaxException.class) public void propagateExceptionWhenTheResponseIsNotJson() { - exception.expect(JsonSyntaxException.class); - AbstractAction.convertJsonStringToMapObject("401 Unauthorized"); + new DummyAction.Builder().build().convertJsonStringToMapObject("401 Unauthorized"); } diff --git a/jest/src/test/java/io/searchbox/core/SearchIntegrationTest.java b/jest/src/test/java/io/searchbox/core/SearchIntegrationTest.java index 7e6158f34..6479c3d44 100644 --- a/jest/src/test/java/io/searchbox/core/SearchIntegrationTest.java +++ b/jest/src/test/java/io/searchbox/core/SearchIntegrationTest.java @@ -73,7 +73,10 @@ public void searchWithValidQueryAndExplain() throws IOException { @Test public void searchWithQueryBuilder() throws IOException { - Index index = new Index.Builder("{\"user\":\"kimchy\"}").setParameter(Parameters.REFRESH, true).build(); + Index index = new Index.Builder("{\"user\":\"kimchy\"}") + .index("twitter") + .type("tweet") + .setParameter(Parameters.REFRESH, true).build(); client.execute(index); SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.query(QueryBuilders.matchQuery("user", "kimchy"));