fix: edge cases with flagd targeting (#567)

Signed-off-by: Todd Baert <[email protected]>

Author: toddbaert

providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java

@@ -161,7 +161,7 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key,
             throw new FlagNotFoundError("flag: " + key + " is disabled");
         }
 
-        final Object resolvedVariant;
+        final String resolvedVariant;
         final String reason;
 
         if (EMPTY_TARGETING_STRING.equals(flag.getTargeting())) {
@@ -174,7 +174,7 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key,
                     resolvedVariant = flag.getDefaultVariant();
                     reason = Reason.DEFAULT.toString();
                 } else {
-                    resolvedVariant = jsonResolved;
+                    resolvedVariant = jsonResolved.toString(); // convert to string to support shorthand
                     reason = Reason.TARGETING_MATCH.toString();
                 }
             } catch (TargetingRuleException e) {
@@ -198,15 +198,15 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key,
             // if this is a double and we are trying to resolve an integer, convert
             value = ((Double) value).intValue();
         }
-        if (!type.isAssignableFrom(value.getClass()) || !(resolvedVariant instanceof String)) {
+        if (!type.isAssignableFrom(value.getClass())) {
             String message = "returning default variant for flagKey: %s, type not valid";
             log.debug(String.format(message, key));
             throw new TypeMismatchError(message);
         }
 
         return ProviderEvaluation.<T>builder()
                 .value((T) value)
-                .variant((String) resolvedVariant)
+                .variant(resolvedVariant)
                 .reason(reason)
                 .build();
     }

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/StepDefinitions.java

@@ -374,20 +374,45 @@ public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_
         this.customEvaluatorContext = new ImmutableContext(outerMap);
     }
 
+    @And("a context containing a nested property with outer key {string} and inner key {string}, with value {int}")
+    public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value_int(String outerKey,
+            String innerKey, Integer value) throws InstantiationException {
+        Map<String, Value> innerMap = new HashMap<String, Value>();
+        innerMap.put(innerKey, new Value(value));
+        Map<String, Value> outerMap = new HashMap<String, Value>();
+        outerMap.put(outerKey, new Value(new ImmutableStructure(innerMap)));
+        this.customEvaluatorContext = new ImmutableContext(outerMap);
+    }
+
+
     @And("a context containing a key {string}, with value {string}")
     public void a_context_containing_a_key_with_value(String key, String value) {
         Map<String, Value> attrs = new HashMap<String, Value>();
         attrs.put(key, new Value(value));
         this.customEvaluatorContext = new ImmutableContext(attrs);
     }
 
+    @And("a context containing a key {string}, with value {double}")
+    public void a_context_containing_a_key_with_value_double(String key, Double value) {
+        Map<String, Value> attrs = new HashMap<String, Value>();
+        attrs.put(key, new Value(value));
+        this.customEvaluatorContext = new ImmutableContext(attrs);
+    }
+
     @Then("the returned value should be {string}")
     public void the_returned_value_should_be(String expected) {
         String value = client.getStringValue(this.stringFlagKey, this.stringFlagDefaultValue,
                 this.customEvaluatorContext);
         assertEquals(expected, value);
     }
 
+    @Then("the returned value should be {int}")
+    public void the_returned_value_should_be(Integer expectedValue) {
+        Integer value = client.getIntegerValue(this.intFlagKey, this.intFlagDefaultValue,
+                this.customEvaluatorContext);
+        assertEquals(expectedValue, value);
+    }
+
     /*
      * Events
      */

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java

@@ -5,13 +5,13 @@
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.DOUBLE_FLAG;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.FLAG_WIH_IF_IN_TARGET;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.FLAG_WIH_INVALID_TARGET;
+import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.FLAG_WIH_SHORTHAND_TARGETING;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.INT_FLAG;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.OBJECT_FLAG;
 import static dev.openfeature.contrib.providers.flagd.resolver.process.MockFlags.VARIANT_MISMATCH_FLAG;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
-import static org.junit.jupiter.api.Assertions.fail;
 
 import java.lang.reflect.Field;
 import java.time.Duration;
@@ -259,6 +259,24 @@ public void typeMismatchEvaluation() throws Exception {
         });
     }
 
+    @Test
+    public void booleanShorthandEvaluation() throws Exception {
+        // given
+        final Map<String, FeatureFlag> flagMap = new HashMap<>();
+        flagMap.put("shorthand", FLAG_WIH_SHORTHAND_TARGETING);
+
+        InProcessResolver inProcessResolver = getInProcessResolverWth(new MockStorage(flagMap), providerState -> {
+        });
+
+        ProviderEvaluation<Boolean> providerEvaluation = inProcessResolver.booleanEvaluation("shorthand", false,
+                new ImmutableContext());
+
+        // then
+        assertEquals(true, providerEvaluation.getValue());
+        assertEquals("true", providerEvaluation.getVariant());
+        assertEquals(Reason.TARGETING_MATCH.toString(), providerEvaluation.getReason());
+    }
+
     @Test
     public void targetingMatchedEvaluationFlag() throws Exception {
         // given

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/MockFlags.java

@@ -8,6 +8,7 @@
 public class MockFlags {
 
     static final Map<String, Object> booleanVariant;
+    static final Map<String, Object> shorthandVariant;
     static final Map<String, Object> stringVariants;
     static final Map<String, Object> doubleVariants;
     static final Map<String, Object> intVariants;
@@ -18,6 +19,10 @@ public class MockFlags {
         booleanVariant.put("on", true);
         booleanVariant.put("off", false);
 
+        shorthandVariant = new HashMap<>();
+        shorthandVariant.put("true", true);
+        shorthandVariant.put("false", false);
+
         stringVariants = new HashMap<>();
         stringVariants.put("loop", "loopAlg");
         stringVariants.put("binet", "binetAlg");
@@ -46,6 +51,9 @@ public class MockFlags {
     // correct flag - boolean
     static final FeatureFlag BOOLEAN_FLAG = new FeatureFlag("ENABLED", "on", booleanVariant, null);
 
+    // correct flag - boolean
+    static final FeatureFlag SHORTHAND_FLAG = new FeatureFlag("ENABLED", "false", booleanVariant, null);
+
     // correct flag - double
     static final FeatureFlag DOUBLE_FLAG = new FeatureFlag("ENABLED", "one", doubleVariants, null);
 
@@ -68,4 +76,8 @@ public class MockFlags {
     // flag with incorrect targeting rule
     static final FeatureFlag FLAG_WIH_INVALID_TARGET = new FeatureFlag("ENABLED", "loop", stringVariants,
             "{if this, then that}");
+
+    // flag with shorthand rule
+    static final FeatureFlag FLAG_WIH_SHORTHAND_TARGETING = new FeatureFlag("ENABLED", "false", shorthandVariant,
+    "{ \"if\": [true, true, false] }");
 }