GetKeyAndValue NPE fix (#50)

* Fix GetKeyAndValue possible NPE Added more test to getKeyAndValue Updated version to 10.1.1 * Remove unnecessary public modifier
configcat · Apr 23, 2024 · 21f8a26 · 21f8a26
1 parent fca46cd
commit 21f8a26
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 19 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,3 +1,3 @@
-version=10.1.0
+version=10.1.1
 
 org.gradle.jvmargs=-Xmx2g
diff --git a/src/main/java/com/configcat/ConfigCatClient.java b/src/main/java/com/configcat/ConfigCatClient.java
@@ -467,26 +467,30 @@ private <T> Map.Entry<String, T> getKeyAndValueFromSettingsMap(Class<T> classOfT
                     return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, setting.getSettingsValue(), setting.getType()));
                 }
 
-                for (TargetingRule targetingRule : setting.getTargetingRules()) {
-                    if (targetingRule.getSimpleValue() != null) {
-                        if (variationId.equals(targetingRule.getSimpleValue().getVariationId())) {
-                            return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, targetingRule.getSimpleValue().getValue(), setting.getType()));
+                if(setting.getTargetingRules() != null) {
+                    for (TargetingRule targetingRule : setting.getTargetingRules()) {
+                        if (targetingRule.getSimpleValue() != null) {
+                            if (variationId.equals(targetingRule.getSimpleValue().getVariationId())) {
+                                return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, targetingRule.getSimpleValue().getValue(), setting.getType()));
 
-                        }
-                    } else if (targetingRule.getPercentageOptions() != null) {
-                        for (PercentageOption percentageRule : targetingRule.getPercentageOptions()) {
-                            if (variationId.equals(percentageRule.getVariationId())) {
-                                return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, percentageRule.getValue(), setting.getType()));
                             }
+                        } else if (targetingRule.getPercentageOptions() != null) {
+                            for (PercentageOption percentageRule : targetingRule.getPercentageOptions()) {
+                                if (variationId.equals(percentageRule.getVariationId())) {
+                                    return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, percentageRule.getValue(), setting.getType()));
+                                }
+                            }
+                        } else {
+                            throw new UnsupportedOperationException("Targeting rule THEN part is missing or invalid.");
                         }
-                    } else {
-                        throw new UnsupportedOperationException("Targeting rule THEN part is missing or invalid.");
                     }
                 }
 
-                for (PercentageOption percentageOption : setting.getPercentageOptions()) {
-                    if (variationId.equals(percentageOption.getVariationId())) {
-                        return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, percentageOption.getValue(), setting.getType()));
+                if( setting.getPercentageOptions() != null) {
+                    for (PercentageOption percentageOption : setting.getPercentageOptions()) {
+                        if (variationId.equals(percentageOption.getVariationId())) {
+                            return new AbstractMap.SimpleEntry<>(settingKey, (T) this.parseObject(classOfT, percentageOption.getValue(), setting.getType()));
+                        }
                     }
                 }
             }

diff --git a/src/main/java/com/configcat/Utils.java b/src/main/java/com/configcat/Utils.java
@@ -30,7 +30,7 @@ private Constants() { /* prevent from instantiation*/ }
     static final long DISTANT_PAST = 0;
     static final String CONFIG_JSON_NAME = "config_v6.json";
     static final String SERIALIZATION_FORMAT_VERSION = "v2";
-    static final String VERSION = "10.1.0";
+    static final String VERSION = "10.1.1";
 
     static final String SDK_KEY_PROXY_PREFIX = "configcat-proxy/";
     static final String SDK_KEY_PREFIX = "configcat-sdk-1";

diff --git a/src/test/java/com/configcat/VariationIdTests.java b/src/test/java/com/configcat/VariationIdTests.java
@@ -16,7 +16,8 @@
 
 class VariationIdTests {
 
-    private static final String TEST_JSON = "{ p: { s: 'test-salt' },  f: { key1: { v: { b: true }, i: 'fakeId1', p: [] ,r: [] }, key2: { v: { b: false }, i: 'fakeId2', p: [] ,r: [] } } }";
+    private static final String TEST_JSON = "{ 'p':{ 'u': 'https://cdn-global.configcat.com', 'r': '0 ', 's': 'test-salt'}, 'f':{ 'key1':{ 't':0, 'r':[ { 'c':[ { 'u':{ 'a': 'Email', 'c': 2 , 'l ':[ '@configcat.com' ] } } ], 's':{ 'v': { 'b':true }, 'i': 'rolloutId1' } }, { 'c': [ { 'u' :{ 'a': 'Email', 'c': 2, 'l' : [ '@test.com' ] } } ], 's' : { 'v' : { 'b': false }, 'i': 'rolloutId2' } } ], 'p':[ { 'p':50, 'v' : { 'b': true }, 'i' : 'percentageId1'  },  { 'p' : 50, 'v' : { 'b': false }, 'i': 'percentageId2' } ], 'v':{ 'b':true }, 'i': 'fakeId1' }, 'key2': { 't':0, 'v': { 'b': false }, 'i': 'fakeId2' }, 'key3': { 't': 0, 'r':[ { 'c': [ { 'u':{ 'a': 'Email', 'c':2,  'l':[ '@configcat.com' ] } } ], 'p': [{ 'p':50, 'v':{ 'b': true  }, 'i' : 'targetPercentageId1' },  { 'p': 50, 'v': { 'b':false }, 'i' : 'targetPercentageId2' } ] } ], 'v':{ 'b': false  }, 'i': 'fakeId3' } } }";
+    private static final String TEST_JSON_INCORRECT = "{ 'p':{ 'u': 'https://cdn-global.configcat.com', 'r': '0 ', 's': 'test-salt' }, 'f' :{ 'incorrect' : { 't': 0, 'r': [ {'c': [ {'u': {'a': 'Email', 'c': 2, 'l': ['@configcat.com'] } } ] } ],'v': {'b': false}, 'i': 'incorrectId' } } }";
     private ConfigCatClient client;
     private MockWebServer server;
 
@@ -64,9 +65,11 @@ void getAllVariationIdsWorks() {
         server.enqueue(new MockResponse().setResponseCode(200).setBody(TEST_JSON));
 
         List<EvaluationDetails<?>> allValueDetails = client.getAllValueDetails(null);
-        assertEquals(2, allValueDetails.size());
+        assertEquals(3, allValueDetails.size());
         assertEquals("fakeId1", allValueDetails.get(0).getVariationId());
         assertEquals("fakeId2", allValueDetails.get(1).getVariationId());
+        assertEquals("fakeId3", allValueDetails.get(2).getVariationId());
+
     }
 
     @Test
@@ -82,9 +85,11 @@ void getAllVariationIdsAsyncWorks() throws ExecutionException, InterruptedExcept
         server.enqueue(new MockResponse().setResponseCode(200).setBody(TEST_JSON));
 
         List<EvaluationDetails<?>> allValueDetails = client.getAllValueDetailsAsync(null).get();
-        assertEquals(2, allValueDetails.size());
+        assertEquals(3, allValueDetails.size());
         assertEquals("fakeId1", allValueDetails.get(0).getVariationId());
         assertEquals("fakeId2", allValueDetails.get(1).getVariationId());
+        assertEquals("fakeId3", allValueDetails.get(2).getVariationId());
+
     }
 
     @Test
@@ -93,6 +98,18 @@ void getKeyAndValueWorks() {
         Map.Entry<String, Boolean> result = client.getKeyAndValue(boolean.class, "fakeId2");
         assertEquals("key2", result.getKey());
         assertFalse(result.getValue());
+
+        Map.Entry<String, Boolean> result2 = client.getKeyAndValue(boolean.class, "percentageId2");
+        assertEquals("key1", result2.getKey());
+        assertFalse(result2.getValue());
+
+        Map.Entry<String, Boolean> result3 = client.getKeyAndValue(boolean.class, "rolloutId1");
+        assertEquals("key1", result3.getKey());
+        assertTrue(result3.getValue());
+
+        Map.Entry<String, Boolean> result4 = client.getKeyAndValue(boolean.class, "targetPercentageId2");
+        assertEquals("key3", result4.getKey());
+        assertFalse(result4.getValue());
     }
 
     @Test
@@ -109,4 +126,11 @@ void getKeyAndValueNotFound() {
         Map.Entry<String, Boolean> result = client.getKeyAndValue(boolean.class, "nonexisting");
         assertNull(result);
     }
+
+    @Test
+    void getKeyAndValueIncorrectTargetingRule() {
+        server.enqueue(new MockResponse().setResponseCode(200).setBody(TEST_JSON_INCORRECT));
+        Map.Entry<String, Boolean> result = client.getKeyAndValue(boolean.class, "targetPercentageId2");
+        assertNull(result);
+    }
 }