From 3cca8c0644ca86d03b4301f23dd94fb6dd669dd6 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Mon, 1 Jul 2019 20:15:24 +0500 Subject: [PATCH 1/6] Added DEFAULT_PARSER env variable check if set and present than initialize using that parser --- .../ab/config/parser/DefaultConfigParser.java | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java index d75435e1a..85bc83829 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java @@ -37,6 +37,23 @@ public static ConfigParser getInstance() { //======== Helper methods ========// + public enum ConfigParsers { + GSON_CONFIG_PARSER("com.google.gson.Gson"), + JACKSON_CONFIG_PARSER("com.fasterxml.jackson.databind.ObjectMapper"), + JSON_CONFIG_PARSER("org.json.JSONObject"), + JSON_SIMPLE_CONFIG_PARSER("org.json.simple.JSONObject"); + + private final String configParserValue; + + ConfigParsers(String configParserValue) { + this.configParserValue = configParserValue; + } + + @Override + public String toString() { + return configParserValue; + } + } /** * Creates and returns a {@link ConfigParser} using a json parser available on the classpath. * @@ -46,8 +63,25 @@ public static ConfigParser getInstance() { private static @Nonnull ConfigParser create() { ConfigParser configParser; + String configParserName = null; + if(System.getenv().containsKey("DEFAULT_PARSER") && isPresent(System.getenv("DEFAULT_PARSER"))) { + configParserName = System.getenv("DEFAULT_PARSER"); + } - if (isPresent("com.fasterxml.jackson.databind.ObjectMapper")) { + if(configParserName != null){ + if (ConfigParsers.JACKSON_CONFIG_PARSER.toString().equals(configParserName)) { + configParser = new JacksonConfigParser(); + } else if (ConfigParsers.GSON_CONFIG_PARSER.toString().equals(configParserName)) { + configParser = new GsonConfigParser(); + } else if (ConfigParsers.JSON_SIMPLE_CONFIG_PARSER.toString().equals(configParserName)) { + configParser = new JsonSimpleConfigParser(); + } else if (ConfigParsers.JSON_CONFIG_PARSER.toString().equals(configParserName)) { + configParser = new JsonConfigParser(); + } else { + throw new MissingJsonParserException("unable to locate a JSON parser. " + + "Please see for more information"); + } + } else if (isPresent("com.fasterxml.jackson.databind.ObjectMapper")) { configParser = new JacksonConfigParser(); } else if (isPresent("com.google.gson.Gson")) { configParser = new GsonConfigParser(); @@ -66,7 +100,8 @@ ConfigParser create() { private static boolean isPresent(@Nonnull String className) { try { - Class.forName(className); + if (!className.equals(null)) + Class.forName(className); return true; } catch (ClassNotFoundException e) { return false; From b6dbe794db082d4591fe821cdf8d5354055dc3ae Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Mon, 1 Jul 2019 12:23:37 -0700 Subject: [PATCH 2/6] trying to test all parsers --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index e9f71e4dc..089917d26 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,11 @@ jdk: install: true addons: srcclr: true +env: + - DEFAULT_PARSER="com.google.gson.Gson" + - DEFAULT_PARSER="com.fasterxml.jackson.databind.ObjectMapper" + - DEFAULT_PARSER="org.json.JSONObject" + - DEFAULT_PARSER="org.json.simple.JSONObject" script: - "./gradlew clean" - "./gradlew exhaustiveTest" From c9abbc5ad6260e42bc0d53352be04dd6cef35ee1 Mon Sep 17 00:00:00 2001 From: msohailhussain Date: Mon, 1 Jul 2019 12:40:23 -0700 Subject: [PATCH 3/6] removed double quote --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 089917d26..a55f3e946 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,10 +8,10 @@ install: true addons: srcclr: true env: - - DEFAULT_PARSER="com.google.gson.Gson" - - DEFAULT_PARSER="com.fasterxml.jackson.databind.ObjectMapper" - - DEFAULT_PARSER="org.json.JSONObject" - - DEFAULT_PARSER="org.json.simple.JSONObject" + - DEFAULT_PARSER=com.google.gson.Gson + - DEFAULT_PARSER=com.fasterxml.jackson.databind.ObjectMapper + - DEFAULT_PARSER=org.json.JSONObject + - DEFAULT_PARSER=org.json.simple.JSONObject script: - "./gradlew clean" - "./gradlew exhaustiveTest" From 2b72e77782bfb1dd7ad9d5ceeabe3429f5aff7e9 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Tue, 2 Jul 2019 16:20:25 +0500 Subject: [PATCH 4/6] Updated defaultConfigParser changed default parser variable to optimizely_default_parser because picking up this var from property utils Added unit test of default conifg parser will be usefull for travis ci env var optimizely_default_parser gets changed --- .travis.yml | 8 +-- .../ab/config/parser/DefaultConfigParser.java | 14 ++-- .../parser/DefaultConfigParserTest.java | 67 ++++++++++++++++++- 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index a55f3e946..d21b057a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,10 +8,10 @@ install: true addons: srcclr: true env: - - DEFAULT_PARSER=com.google.gson.Gson - - DEFAULT_PARSER=com.fasterxml.jackson.databind.ObjectMapper - - DEFAULT_PARSER=org.json.JSONObject - - DEFAULT_PARSER=org.json.simple.JSONObject + - optimizely_default_parser=com.google.gson.Gson + - optimizely_default_parser=com.fasterxml.jackson.databind.ObjectMapper + - optimizely_default_parser=org.json.JSONObject + - optimizely_default_parser=org.json.simple.JSONObject script: - "./gradlew clean" - "./gradlew exhaustiveTest" diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java index 85bc83829..df4ac4b53 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.config.parser; +import com.optimizely.ab.internal.PropertyUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,12 +64,10 @@ public String toString() { private static @Nonnull ConfigParser create() { ConfigParser configParser; - String configParserName = null; - if(System.getenv().containsKey("DEFAULT_PARSER") && isPresent(System.getenv("DEFAULT_PARSER"))) { - configParserName = System.getenv("DEFAULT_PARSER"); - } - if(configParserName != null){ + String configParserName = PropertyUtils.get("default_parser"); + + if(configParserName != null && isPresent(configParserName)) { if (ConfigParsers.JACKSON_CONFIG_PARSER.toString().equals(configParserName)) { configParser = new JacksonConfigParser(); } else if (ConfigParsers.GSON_CONFIG_PARSER.toString().equals(configParserName)) { @@ -94,14 +93,13 @@ ConfigParser create() { + "Please see for more information"); } - logger.info("using json parser: {}", configParser.getClass().getSimpleName()); + logger.debug("using json parser: {}", configParser.getClass().getSimpleName()); return configParser; } private static boolean isPresent(@Nonnull String className) { try { - if (!className.equals(null)) - Class.forName(className); + Class.forName(className); return true; } catch (ClassNotFoundException e) { return false; 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 5911932f8..10e30d997 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2017, 2019, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,15 +16,76 @@ */ package com.optimizely.ab.config.parser; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.internal.PropertyUtils; +import org.hamcrest.CoreMatchers; +import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; + +import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.*; +import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.validConfigJsonV4; /** * Tests for {@link DefaultConfigParser}. */ +@RunWith(Parameterized.class) public class DefaultConfigParserTest { + @Parameterized.Parameters(name = "{index}") + public static Collection data() throws IOException { + return Arrays.asList(new Object[][]{ + { + validConfigJsonV2(), + validProjectConfigV2() + }, + { + validConfigJsonV3(), + validProjectConfigV3() + }, + { + validConfigJsonV4(), + validProjectConfigV4() + } + }); + } + + @Parameterized.Parameter(0) + public String validDatafile; + + @Parameterized.Parameter(1) + public ProjectConfig validProjectConfig; + + /** + * This method is to test DefaultConfigParser when different default_parser gets set. + * For example: when optimizely_default_parser environment variable will be set to "com.google.gson.Gson" than + * "DefaultConfigParser.getInstance()" returns "GsonConfigParser" and parse ProjectConfig using it. Also + * this test will assertThat "configParser" is instance of "GsonConfigParser.class" + * + * @throws Exception + */ @Test - public void createThrowException() throws Exception { - // FIXME - mdodsworth: hmmm, this isn't going to be the easiest thing to test + public void testPropertyDefaultParser() throws Exception { + String defaultParser = PropertyUtils.get("default_parser"); + ConfigParser configParser = DefaultConfigParser.getInstance(); + ProjectConfig actual = configParser.parseProjectConfig(validDatafile); + ProjectConfig expected = validProjectConfig; + verifyProjectConfig(actual, expected); + if(defaultParser != null) { + if (DefaultConfigParser.ConfigParsers.GSON_CONFIG_PARSER.toString().equals(defaultParser)) { + Assert.assertThat(configParser, CoreMatchers.instanceOf(GsonConfigParser.class)); + } else if (DefaultConfigParser.ConfigParsers.JACKSON_CONFIG_PARSER.toString().equals(defaultParser)) { + Assert.assertThat(configParser, CoreMatchers.instanceOf(JacksonConfigParser.class)); + } else if (DefaultConfigParser.ConfigParsers.JSON_CONFIG_PARSER.toString().equals(defaultParser)) { + Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonConfigParser.class)); + } else if (DefaultConfigParser.ConfigParsers.JSON_SIMPLE_CONFIG_PARSER.toString().equals(defaultParser)) { + Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonSimpleConfigParser.class)); + } + } } } From bb2ff6d10bb5370cbbc3cf97ba1f1194cdf6d8a4 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Thu, 4 Jul 2019 19:32:57 +0500 Subject: [PATCH 5/6] Updated defaultConfig and using Supplier --- .travis.yml | 8 +- .../ab/config/parser/DefaultConfigParser.java | 90 +++++++++---------- .../parser/DefaultConfigParserTest.java | 16 ++-- 3 files changed, 59 insertions(+), 55 deletions(-) diff --git a/.travis.yml b/.travis.yml index d21b057a0..10a95d197 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,10 +8,10 @@ install: true addons: srcclr: true env: - - optimizely_default_parser=com.google.gson.Gson - - optimizely_default_parser=com.fasterxml.jackson.databind.ObjectMapper - - optimizely_default_parser=org.json.JSONObject - - optimizely_default_parser=org.json.simple.JSONObject + - optimizely_default_parser=GSON_CONFIG_PARSER + - optimizely_default_parser=JACKSON_CONFIG_PARSER + - optimizely_default_parser=JSON_CONFIG_PARSER + - optimizely_default_parser=JSON_SIMPLE_CONFIG_PARSER script: - "./gradlew clean" - "./gradlew exhaustiveTest" diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java index df4ac4b53..0d103eb11 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DefaultConfigParser.java @@ -21,6 +21,7 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; +import java.util.function.Supplier; /** * Factory for generating {@link ConfigParser} instances, based on the json parser available on the classpath. @@ -38,21 +39,31 @@ public static ConfigParser getInstance() { //======== Helper methods ========// - public enum ConfigParsers { - GSON_CONFIG_PARSER("com.google.gson.Gson"), - JACKSON_CONFIG_PARSER("com.fasterxml.jackson.databind.ObjectMapper"), - JSON_CONFIG_PARSER("org.json.JSONObject"), - JSON_SIMPLE_CONFIG_PARSER("org.json.simple.JSONObject"); + public enum ConfigParserSupplier { + GSON_CONFIG_PARSER("com.google.gson.Gson", GsonConfigParser::new), + JACKSON_CONFIG_PARSER("com.fasterxml.jackson.databind.ObjectMapper", JacksonConfigParser::new), + JSON_CONFIG_PARSER("org.json.JSONObject", JsonConfigParser::new), + JSON_SIMPLE_CONFIG_PARSER("org.json.simple.JSONObject", JsonSimpleConfigParser::new); - private final String configParserValue; + private final String className; + private final Supplier supplier; - ConfigParsers(String configParserValue) { - this.configParserValue = configParserValue; + ConfigParserSupplier(String className, Supplier supplier) { + this.className = className; + this.supplier = supplier; } - @Override - public String toString() { - return configParserValue; + ConfigParser get() { + return supplier.get(); + } + + private boolean isPresent() { + try { + Class.forName(className); + return true; + } catch (ClassNotFoundException e) { + return false; + } } } /** @@ -63,50 +74,39 @@ public String toString() { */ private static @Nonnull ConfigParser create() { - ConfigParser configParser; String configParserName = PropertyUtils.get("default_parser"); - if(configParserName != null && isPresent(configParserName)) { - if (ConfigParsers.JACKSON_CONFIG_PARSER.toString().equals(configParserName)) { - configParser = new JacksonConfigParser(); - } else if (ConfigParsers.GSON_CONFIG_PARSER.toString().equals(configParserName)) { - configParser = new GsonConfigParser(); - } else if (ConfigParsers.JSON_SIMPLE_CONFIG_PARSER.toString().equals(configParserName)) { - configParser = new JsonSimpleConfigParser(); - } else if (ConfigParsers.JSON_CONFIG_PARSER.toString().equals(configParserName)) { - configParser = new JsonConfigParser(); - } else { - throw new MissingJsonParserException("unable to locate a JSON parser. " - + "Please see for more information"); + if (configParserName != null) { + try { + ConfigParserSupplier supplier = ConfigParserSupplier.valueOf(configParserName); + if (supplier.isPresent()) { + ConfigParser configParser = supplier.get(); + logger.debug("using json parser: {}, based on override config", configParser.getClass().getSimpleName()); + return configParser; + } + + logger.warn("configured parser {} is not available in the classpath", configParserName); + } catch (IllegalArgumentException e) { + logger.warn("configured parser {} is not a valid value", configParserName); } - } else if (isPresent("com.fasterxml.jackson.databind.ObjectMapper")) { - configParser = new JacksonConfigParser(); - } else if (isPresent("com.google.gson.Gson")) { - configParser = new GsonConfigParser(); - } else if (isPresent("org.json.simple.JSONObject")) { - configParser = new JsonSimpleConfigParser(); - } else if (isPresent("org.json.JSONObject")) { - configParser = new JsonConfigParser(); - } else { - throw new MissingJsonParserException("unable to locate a JSON parser. " - + "Please see for more information"); } - logger.debug("using json parser: {}", configParser.getClass().getSimpleName()); - return configParser; - } + for (ConfigParserSupplier supplier: ConfigParserSupplier.values()) { + if (!supplier.isPresent()) { + continue; + } - private static boolean isPresent(@Nonnull String className) { - try { - Class.forName(className); - return true; - } catch (ClassNotFoundException e) { - return false; + ConfigParser configParser = supplier.get(); + logger.info("using json parser: {}", configParser.getClass().getSimpleName()); + return configParser; } + + throw new MissingJsonParserException("unable to locate a JSON parser. " + + "Please see for more information"); } - //======== Lazy-init Holder ========// + //======== Lazy-init Holder ========// private static class LazyHolder { private static final ConfigParser INSTANCE = create(); 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 10e30d997..841d66eb9 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 @@ -63,9 +63,9 @@ public static Collection data() throws IOException { /** * This method is to test DefaultConfigParser when different default_parser gets set. - * For example: when optimizely_default_parser environment variable will be set to "com.google.gson.Gson" than + * For example: when optimizely_default_parser environment variable will be set to "GSON_CONFIG_PARSER" than * "DefaultConfigParser.getInstance()" returns "GsonConfigParser" and parse ProjectConfig using it. Also - * this test will assertThat "configParser" is instance of "GsonConfigParser.class" + * this test will assertThat "configParser" (Provided in env variable) is instance of "GsonConfigParser.class" * * @throws Exception */ @@ -77,15 +77,19 @@ public void testPropertyDefaultParser() throws Exception { ProjectConfig expected = validProjectConfig; verifyProjectConfig(actual, expected); if(defaultParser != null) { - if (DefaultConfigParser.ConfigParsers.GSON_CONFIG_PARSER.toString().equals(defaultParser)) { + DefaultConfigParser.ConfigParserSupplier defaultParserSupplier = DefaultConfigParser.ConfigParserSupplier.valueOf(defaultParser); + + if (DefaultConfigParser.ConfigParserSupplier.GSON_CONFIG_PARSER.equals(defaultParserSupplier)) { Assert.assertThat(configParser, CoreMatchers.instanceOf(GsonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParsers.JACKSON_CONFIG_PARSER.toString().equals(defaultParser)) { + } else if (DefaultConfigParser.ConfigParserSupplier.JACKSON_CONFIG_PARSER.equals(defaultParserSupplier)) { Assert.assertThat(configParser, CoreMatchers.instanceOf(JacksonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParsers.JSON_CONFIG_PARSER.toString().equals(defaultParser)) { + } else if (DefaultConfigParser.ConfigParserSupplier.JSON_CONFIG_PARSER.equals(defaultParserSupplier)) { Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParsers.JSON_SIMPLE_CONFIG_PARSER.toString().equals(defaultParser)) { + } else if (DefaultConfigParser.ConfigParserSupplier.JSON_SIMPLE_CONFIG_PARSER.equals(defaultParserSupplier)) { Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonSimpleConfigParser.class)); } + } else { + Assert.assertThat(configParser, CoreMatchers.instanceOf(GsonConfigParser.class)); } } } From 630b2d0a9ff4c57ef2efdc5e327a609c19e5a7eb Mon Sep 17 00:00:00 2001 From: Muhammad Noman Date: Mon, 8 Jul 2019 12:17:50 -0700 Subject: [PATCH 6/6] Updated defaultparser unittest added switch statements --- .../parser/DefaultConfigParserTest.java | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 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 841d66eb9..dfc130f21 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 @@ -30,6 +30,7 @@ import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.*; import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.validConfigJsonV4; +import static junit.framework.TestCase.fail; /** * Tests for {@link DefaultConfigParser}. @@ -76,20 +77,28 @@ public void testPropertyDefaultParser() throws Exception { ProjectConfig actual = configParser.parseProjectConfig(validDatafile); ProjectConfig expected = validProjectConfig; verifyProjectConfig(actual, expected); + Class expectedParser = GsonConfigParser.class; + if(defaultParser != null) { DefaultConfigParser.ConfigParserSupplier defaultParserSupplier = DefaultConfigParser.ConfigParserSupplier.valueOf(defaultParser); - - if (DefaultConfigParser.ConfigParserSupplier.GSON_CONFIG_PARSER.equals(defaultParserSupplier)) { - Assert.assertThat(configParser, CoreMatchers.instanceOf(GsonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParserSupplier.JACKSON_CONFIG_PARSER.equals(defaultParserSupplier)) { - Assert.assertThat(configParser, CoreMatchers.instanceOf(JacksonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParserSupplier.JSON_CONFIG_PARSER.equals(defaultParserSupplier)) { - Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonConfigParser.class)); - } else if (DefaultConfigParser.ConfigParserSupplier.JSON_SIMPLE_CONFIG_PARSER.equals(defaultParserSupplier)) { - Assert.assertThat(configParser, CoreMatchers.instanceOf(JsonSimpleConfigParser.class)); + switch (defaultParserSupplier) { + case GSON_CONFIG_PARSER: + expectedParser = GsonConfigParser.class; + break; + case JACKSON_CONFIG_PARSER: + expectedParser = JacksonConfigParser.class; + break; + case JSON_CONFIG_PARSER: + expectedParser = JsonConfigParser.class; + break; + case JSON_SIMPLE_CONFIG_PARSER: + expectedParser = JsonSimpleConfigParser.class; + break; + default: + fail("Not a valid config parser"); } - } else { - Assert.assertThat(configParser, CoreMatchers.instanceOf(GsonConfigParser.class)); } + + Assert.assertThat(configParser, CoreMatchers.instanceOf(expectedParser)); } }