From 6fe8bc1664c4ece900a8be62205cab4ce775bae1 Mon Sep 17 00:00:00 2001 From: Roman Onufryk Date: Mon, 27 Feb 2017 15:24:40 -0800 Subject: [PATCH 1/5] Catch any exception occured during parsing project config Test Plan: unit tests Reviewers: elliot, vignesh, mike.ng JIRA Issues: OASIS-1008 Differential Revision: https://phabricator.optimizely.com/D14972 --- .../java/com/optimizely/ab/Optimizely.java | 7 ++++++ .../ab/config/parser/GsonConfigParser.java | 6 +++++ .../optimizely/ab/OptimizelyBuilderTest.java | 13 ++++++++++ .../config/parser/GsonConfigParserTest.java | 24 +++++++++++++++++++ .../parser/JacksonConfigParserTest.java | 24 +++++++++++++++++++ .../config/parser/JsonConfigParserTest.java | 24 +++++++++++++++++++ .../parser/JsonSimpleConfigParserTest.java | 22 +++++++++++++++++ 7 files changed, 120 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 334564508..989cb3fe5 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -498,6 +498,13 @@ private void track(@Nonnull String eventName, * @return a {@link ProjectConfig} instance given a json string */ private static ProjectConfig getProjectConfig(String datafile) throws ConfigParseException { + if (datafile == null) { + throw new ConfigParseException("Unable to parse null datafile."); + } + if (datafile.length() == 0) { + throw new ConfigParseException("Unable to parse empty datafile."); + } + return DefaultConfigParser.getInstance().parseProjectConfig(datafile); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java index 6a7f42a05..91b721dab 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java @@ -33,6 +33,12 @@ final class GsonConfigParser implements ConfigParser { @Override public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParseException { + if (json == null) { + throw new ConfigParseException("Unable to parse null datafile."); + } + if (json.length() == 0) { + throw new ConfigParseException("Unable to parse empty datafile."); + } Gson gson = new GsonBuilder() .registerTypeAdapter(ProjectConfig.class, new ProjectConfigGsonDeserializer()) .registerTypeAdapter(Audience.class, new AudienceGsonDeserializer()) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index 69ed103e6..13557bc76 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -164,6 +164,19 @@ public void withCustomClientVersion() throws Exception { assertThat(((EventBuilderV2)optimizelyClient.eventBuilder).clientVersion, is("0.0.0")); } + @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation") + @Test + public void builderThrowsConfigParseExceptionForNullDatafile() throws Exception { + thrown.expect(ConfigParseException.class); + Optimizely.builder(null, mockEventHandler).build(); + } + + @Test + public void builderThrowsConfigParseExceptionForEmptyDatafile() throws Exception { + thrown.expect(ConfigParseException.class); + Optimizely.builder("", mockEventHandler).build(); + } + @Test public void builderThrowsConfigParseExceptionForInvalidDatafile() throws Exception { thrown.expect(ConfigParseException.class); diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java index feff38be2..637f3e145 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java @@ -18,6 +18,7 @@ import com.optimizely.ab.config.ProjectConfig; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception { GsonConfigParser parser = new GsonConfigParser(); parser.parseProjectConfig("{\"valid\": \"json\"}"); } + + /** + * Verify that empty string JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + public void emptyJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + GsonConfigParser parser = new GsonConfigParser(); + parser.parseProjectConfig(""); + } + + /** + * Verify that null JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation") + public void nullJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + GsonConfigParser parser = new GsonConfigParser(); + parser.parseProjectConfig(null); + } } \ No newline at end of file diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java index 18e558082..8a0d7cf23 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java @@ -18,6 +18,7 @@ import com.optimizely.ab.config.ProjectConfig; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception { JacksonConfigParser parser = new JacksonConfigParser(); parser.parseProjectConfig("{\"valid\": \"json\"}"); } + + /** + * Verify that empty string JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + public void emptyJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JacksonConfigParser parser = new JacksonConfigParser(); + parser.parseProjectConfig(""); + } + + /** + * Verify that null JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation") + public void nullJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JacksonConfigParser parser = new JacksonConfigParser(); + parser.parseProjectConfig(null); + } } \ No newline at end of file diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java index 0e49c8204..890b7411a 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java @@ -18,6 +18,7 @@ import com.optimizely.ab.config.ProjectConfig; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception { JsonConfigParser parser = new JsonConfigParser(); parser.parseProjectConfig("{\"valid\": \"json\"}"); } + + /** + * Verify that empty string JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + public void emptyJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JsonConfigParser parser = new JsonConfigParser(); + parser.parseProjectConfig(""); + } + + /** + * Verify that null JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation") + public void nullJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JsonConfigParser parser = new JsonConfigParser(); + parser.parseProjectConfig(null); + } } \ No newline at end of file diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java index 57a07f78d..37d4c3ded 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java @@ -18,6 +18,7 @@ import com.optimizely.ab.config.ProjectConfig; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -86,4 +87,25 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception { JsonSimpleConfigParser parser = new JsonSimpleConfigParser(); parser.parseProjectConfig("{\"valid\": \"json\"}"); } + /** + * Verify that empty string JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + public void emptyJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JsonSimpleConfigParser parser = new JsonSimpleConfigParser(); + parser.parseProjectConfig(""); + } + /** + * Verify that null JSON results in a {@link ConfigParseException} being thrown. + */ + @Test + @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation") + public void nullJsonExceptionWrapping() throws Exception { + thrown.expect(ConfigParseException.class); + + JsonSimpleConfigParser parser = new JsonSimpleConfigParser(); + parser.parseProjectConfig(null); + } } \ No newline at end of file From 5e36e07e9609b3a6b00bece822edbcda96f8987b Mon Sep 17 00:00:00 2001 From: Roman Onufryk Date: Tue, 28 Feb 2017 14:58:21 -0800 Subject: [PATCH 2/5] Different message for GSON parser --- .../com/optimizely/ab/config/parser/GsonConfigParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java index 91b721dab..9bfa58981 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java @@ -34,10 +34,10 @@ final class GsonConfigParser implements ConfigParser { @Override public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParseException { if (json == null) { - throw new ConfigParseException("Unable to parse null datafile."); + throw new ConfigParseException("Unable to parse null json."); } if (json.length() == 0) { - throw new ConfigParseException("Unable to parse empty datafile."); + throw new ConfigParseException("Unable to parse empty json."); } Gson gson = new GsonBuilder() .registerTypeAdapter(ProjectConfig.class, new ProjectConfigGsonDeserializer()) From 5ba542021006759ef736678a0e91e912cce72fe9 Mon Sep 17 00:00:00 2001 From: Roman Onufryk Date: Tue, 28 Feb 2017 18:08:08 -0800 Subject: [PATCH 3/5] Trigger tests --- .../src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index 13557bc76..09cd16f21 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -180,6 +180,7 @@ public void builderThrowsConfigParseExceptionForEmptyDatafile() throws Exception @Test public void builderThrowsConfigParseExceptionForInvalidDatafile() throws Exception { thrown.expect(ConfigParseException.class); - Optimizely.builder("{invalidDatafile}", mockEventHandler).build(); + String invalidJson = "{invalidDatafile}"; + Optimizely.builder(invalidJson, mockEventHandler).build(); } } From f24e4c30f593e16fda8e1d00a50adfa6fffbf554 Mon Sep 17 00:00:00 2001 From: Roman Onufryk Date: Wed, 1 Mar 2017 10:10:17 -0800 Subject: [PATCH 4/5] Revert "Trigger tests" This reverts commit 5ba542021006759ef736678a0e91e912cce72fe9. --- .../src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index 09cd16f21..13557bc76 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -180,7 +180,6 @@ public void builderThrowsConfigParseExceptionForEmptyDatafile() throws Exception @Test public void builderThrowsConfigParseExceptionForInvalidDatafile() throws Exception { thrown.expect(ConfigParseException.class); - String invalidJson = "{invalidDatafile}"; - Optimizely.builder(invalidJson, mockEventHandler).build(); + Optimizely.builder("{invalidDatafile}", mockEventHandler).build(); } } From 2931888cabcec821eeff8f9a8fb920ceba0cf5e6 Mon Sep 17 00:00:00 2001 From: Roman Onufryk Date: Wed, 1 Mar 2017 10:17:15 -0800 Subject: [PATCH 5/5] Add EOF at the end of files --- .../optimizely/ab/config/parser/DefaultConfigParserTest.java | 2 +- .../com/optimizely/ab/config/parser/GsonConfigParserTest.java | 2 +- .../optimizely/ab/config/parser/JacksonConfigParserTest.java | 2 +- .../com/optimizely/ab/config/parser/JsonConfigParserTest.java | 2 +- .../optimizely/ab/config/parser/JsonSimpleConfigParserTest.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/DefaultConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/DefaultConfigParserTest.java index 0fa1db64e..f2e1a2df1 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/DefaultConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/DefaultConfigParserTest.java @@ -27,4 +27,4 @@ public class DefaultConfigParserTest { public void createThrowException() throws Exception { // FIXME - mdodsworth: hmmm, this isn't going to be the easiest thing to test } -} \ No newline at end of file +} diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java index 637f3e145..f66c3e401 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java @@ -110,4 +110,4 @@ public void nullJsonExceptionWrapping() throws Exception { GsonConfigParser parser = new GsonConfigParser(); parser.parseProjectConfig(null); } -} \ No newline at end of file +} diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java index 8a0d7cf23..3adac6c70 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java @@ -110,4 +110,4 @@ public void nullJsonExceptionWrapping() throws Exception { JacksonConfigParser parser = new JacksonConfigParser(); parser.parseProjectConfig(null); } -} \ No newline at end of file +} diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java index 890b7411a..064871471 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java @@ -110,4 +110,4 @@ public void nullJsonExceptionWrapping() throws Exception { JsonConfigParser parser = new JsonConfigParser(); parser.parseProjectConfig(null); } -} \ No newline at end of file +} diff --git a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java index 37d4c3ded..48f679e7a 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java @@ -108,4 +108,4 @@ public void nullJsonExceptionWrapping() throws Exception { JsonSimpleConfigParser parser = new JsonSimpleConfigParser(); parser.parseProjectConfig(null); } -} \ No newline at end of file +}