Repository: Unleash/unleash-client-java
Pull request #93 (2019-11-12 14:09:48)

feat: Add support for fallback-action 
Today it is possible to define a fallback value which the `isEnabled` will return if the feature toggle is not defined.

We want to also allow the user to send in a fallbackFunction instead of a fallback value (which will take precedence over fallback value).

The fallback function should get to arguments injected: `toggleName` and the enhanced `unleashContext`. The function is implemented by the caller and should return a boolean value. 

Example on how it would work:
`isEnabled("someToggle", context, (toggleName, eContext) -> false)`

coveralls: 
[![Coverage Status](https://coveralls.io/builds/26941009/badge)](https://coveralls.io/builds/26941009)

Coverage decreased (-1.4%) to 92.212% when pulling **63b1621e24100f109458ef56753c52fcf1bfceaa on dennis-menge:master** into **d26b67ab27e5211f55ae73cb07e757e87e920de2 on Unleash:master**.

ivarconr: This is really closing in to something good now :rocket:. 

I have limited time today, but how to be able to do a proper validation tomorrow!

Feel free to also [join us on slack](https://join.slack.com/t/unleash-community/shared_invite/enQtNjUxMjU2MDc0MTAxLTJjYmViYjkwYmE0ODVlNmY1YjcwZGRmZWU5MTU1YTQ1Nzg5ZWQ2YzBlY2U1MjlmZDg5ZDRmZTMzNmQ5YmEyOGE) for more in depth discussions: 
ivarconr: Hi again @dennis-menge. I did a few adjustment to this PR. Could you look it over and see if iy make sense?

https://github.com/Unleash/unleash-client-java/pull/93/commits/e96a1730d2803ac66e2941e2e67b0b698b438a43
dennis-menge: > Hi again @dennis-menge. I did a few adjustment to this PR. Could you look it over and see if iy make sense?
> 
> [e96a173](https://github.com/Unleash/unleash-client-java/commit/e96a1730d2803ac66e2941e2e67b0b698b438a43)

Hi Ivar, looks nice, I misunderstood all whole time that defaultSetting and fallbackAction should be provided at the same time with fallbackAction taking precedence, which obviously makes no sense.

Thank you! 

Patch:
diff --git a/README.md b/README.md
index 733eac51..1f229e8e 100644
--- a/README.md
+++ b/README.md
@@ -1,10 +1,11 @@
-# The Unleash Client 4 Java
+# Unleash Client SDK for Java
+This is the Unleash Client SDK for Java. It is compatible with the [Unlesah-hosted.com SaaS](https://www.unleash-hosted.com/) offering and [Unleash Open-Soruce](https://github.com/unleash/unleash).
+
 
 [![Build Status](https://travis-ci.org/Unleash/unleash-client-java.svg?branch=master)](https://travis-ci.org/Unleash/unleash-client-java)
 [![Coverage Status](https://coveralls.io/repos/github/Unleash/unleash-client-java/badge.svg?branch=master)](https://coveralls.io/github/Unleash/unleash-client-java?branch=master)
 
-This is the java client for Unleash. Read more about the [Unleash project](https://github.com/finn-no/unleash)
-**Version 3.x of the client requires `unleash-server` >= v3.x**
+
 
 ## Getting started
 You will require unleash on your class path, pop it in to your pom:
diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java
index f7ee726c..c63ebcf0 100644
--- a/src/main/java/no/finn/unleash/DefaultUnleash.java
+++ b/src/main/java/no/finn/unleash/DefaultUnleash.java
@@ -5,6 +5,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.BiFunction;
 
 import no.finn.unleash.event.EventDispatcher;
 import no.finn.unleash.event.ToggleEvaluated;
@@ -40,7 +41,6 @@ public final class DefaultUnleash implements Unleash {
     private final EventDispatcher eventDispatcher;
     private final UnleashConfig config;
 
-
     private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) {
         return new FeatureToggleRepository(
                 unleashConfig,
@@ -74,24 +74,35 @@ public final class DefaultUnleash implements Unleash {
     }
 
     @Override
-    public boolean isEnabled(final String toggleName, final UnleashContext context ,final boolean defaultSetting) {
-        FeatureToggle featureToggle = toggleRepository.getToggle(toggleName);
-        boolean enabled = isEnabled(featureToggle, context, defaultSetting);
+    public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) {
+        return isEnabled(toggleName, context, (n, c) -> defaultSetting);
+    }
+
+    @Override
+    public boolean isEnabled(final String toggleName, final BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        return isEnabled(toggleName, contextProvider.getContext(), fallbackAction);
+    }
+
+    @Override
+    public boolean isEnabled(String toggleName, UnleashContext context, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        boolean enabled = checkEnabled(toggleName, context, fallbackAction);
         count(toggleName, enabled);
-        eventDispatcher.dispatch(new ToggleEvaluated(toggleName,enabled));
+        eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled));
         return enabled;
     }
 
-    private boolean isEnabled(FeatureToggle featureToggle, UnleashContext context, boolean defaultSetting) {
+    private boolean checkEnabled(String toggleName, UnleashContext context, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        FeatureToggle featureToggle = toggleRepository.getToggle(toggleName);
         boolean enabled;
+        UnleashContext enhancedContext = context.applyStaticFields(config);
+
         if (featureToggle == null) {
-            enabled = defaultSetting;
+            enabled = fallbackAction.apply(toggleName, enhancedContext);
         } else if(!featureToggle.isEnabled()) {
             enabled = false;
         } else if(featureToggle.getStrategies().size() == 0) {
             return true;
         } else {
-            UnleashContext enhancedContext = context.applyStaticFields(config);
             enabled = featureToggle.getStrategies().stream()
                     .anyMatch(as -> getStrategy(as.getName()).isEnabled(as.getParameters(), enhancedContext, as.getConstraints()));
         }
@@ -106,7 +117,7 @@ public final class DefaultUnleash implements Unleash {
     @Override
     public Variant getVariant(String toggleName, UnleashContext context, Variant defaultValue) {
         FeatureToggle featureToggle = toggleRepository.getToggle(toggleName);
-        boolean enabled = isEnabled(featureToggle, context, false);
+        boolean enabled = checkEnabled(toggleName, context, (n, c) -> false);
         Variant variant = enabled ? selectVariant(featureToggle, context, defaultValue) : defaultValue;
         metricService.countVariant(toggleName, variant.getName());
         return variant;
diff --git a/src/main/java/no/finn/unleash/FakeUnleash.java b/src/main/java/no/finn/unleash/FakeUnleash.java
index ea6d0105..3b6a8a4d 100644
--- a/src/main/java/no/finn/unleash/FakeUnleash.java
+++ b/src/main/java/no/finn/unleash/FakeUnleash.java
@@ -4,6 +4,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiFunction;
 
 public final class FakeUnleash implements Unleash {
     private boolean enableAll = false;
@@ -27,6 +28,19 @@ public final class FakeUnleash implements Unleash {
         }
     }
 
+    @Override
+    public boolean isEnabled(String toggleName, UnleashContext context, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        return isEnabled(toggleName, fallbackAction);
+    }
+
+    @Override
+    public boolean isEnabled(String toggleName, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        if(!features.containsKey(toggleName)) {
+            return fallbackAction.apply(toggleName, UnleashContext.builder().build());
+        }
+        return isEnabled(toggleName);
+    }
+
     @Override
     public Variant getVariant(String toggleName, UnleashContext context) {
         return getVariant(toggleName, Variant.DISABLED_VARIANT);
diff --git a/src/main/java/no/finn/unleash/FeatureToggle.java b/src/main/java/no/finn/unleash/FeatureToggle.java
index 9a46b82e..fa0bcf33 100644
--- a/src/main/java/no/finn/unleash/FeatureToggle.java
+++ b/src/main/java/no/finn/unleash/FeatureToggle.java
@@ -1,5 +1,6 @@
 package no.finn.unleash;
 
+import java.util.Collections;
 import java.util.List;
 
 import no.finn.unleash.variant.VariantDefinition;
@@ -36,7 +37,11 @@ public final class FeatureToggle {
     }
 
     public List<VariantDefinition> getVariants() {
-        return variants;
+        if(variants == null) {
+            return Collections.emptyList();
+        } else {
+            return variants;
+        }
     }
 
     @Override
diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java
index fe28dacd..3bd04b80 100644
--- a/src/main/java/no/finn/unleash/Unleash.java
+++ b/src/main/java/no/finn/unleash/Unleash.java
@@ -1,6 +1,7 @@
 package no.finn.unleash;
 
 import java.util.List;
+import java.util.function.BiFunction;
 
 public interface Unleash {
     boolean isEnabled(String toggleName);
@@ -15,6 +16,14 @@ public interface Unleash {
         return isEnabled(toggleName, defaultSetting);
     }
 
+    default boolean isEnabled(String toggleName, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        return isEnabled(toggleName, false);
+    }
+
+    default boolean isEnabled(String toggleName, UnleashContext context, BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        return isEnabled(toggleName, context, false);
+    }
+
     Variant getVariant(final String toggleName, final UnleashContext context);
 
     Variant getVariant(final String toggleName, final UnleashContext context, final Variant defaultValue);


Test patch:
diff --git a/src/test/java/no/finn/unleash/UnleashTest.java b/src/test/java/no/finn/unleash/UnleashTest.java
index 8f9f6430..ead646bd 100644
--- a/src/test/java/no/finn/unleash/UnleashTest.java
+++ b/src/test/java/no/finn/unleash/UnleashTest.java
@@ -7,6 +7,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
 
 import no.finn.unleash.repository.ToggleRepository;
 import no.finn.unleash.strategy.Strategy;
@@ -15,21 +17,18 @@ import no.finn.unleash.util.UnleashConfig;
 
 import no.finn.unleash.variant.Payload;
 import no.finn.unleash.variant.VariantDefinition;
+import org.hamcrest.CoreMatchers;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.ArgumentMatchers.isNull;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.anyMap;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
 
 public class UnleashTest {
 
@@ -81,6 +80,49 @@ public class UnleashTest {
         assertThat(unleash.isEnabled("test", true), is(true));
     }
 
+    @Test
+    public void fallback_function_should_be_invoked_and_return_true() {
+        when(toggleRepository.getToggle("test")).thenReturn(null);
+        BiFunction<String, UnleashContext, Boolean>  fallbackAction = mock(BiFunction.class);
+        when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(true);
+
+        assertThat(unleash.isEnabled("test", fallbackAction), is(true));
+        verify(fallbackAction, times(1)).apply(anyString(), any(UnleashContext.class));
+    }
+
+    @Test
+    public void fallback_function_should_be_invoked_also_with_context() {
+        when(toggleRepository.getToggle("test")).thenReturn(null);
+        BiFunction<String, UnleashContext, Boolean>  fallbackAction = mock(BiFunction.class);
+        when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(true);
+
+        UnleashContext context = UnleashContext.builder().userId("123").build();
+
+        assertThat(unleash.isEnabled("test", context, fallbackAction), is(true));
+        verify(fallbackAction, times(1)).apply(anyString(), any(UnleashContext.class));
+    }
+
+    @Test
+    void fallback_function_should_be_invoked_and_return_false() {
+        when(toggleRepository.getToggle("test")).thenReturn(null);
+        BiFunction<String, UnleashContext, Boolean>  fallbackAction = mock(BiFunction.class);
+        when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(false);
+
+        assertThat(unleash.isEnabled("test", fallbackAction), is(false));
+        verify(fallbackAction, times(1)).apply(anyString(), any(UnleashContext.class));
+    }
+
+	@Test
+	void fallback_function_should_not_be_called_when_toggle_is_defined() {
+		when(toggleRepository.getToggle("test")).thenReturn(new FeatureToggle("test", true, asList(new ActivationStrategy("default", null))));
+
+        BiFunction<String, UnleashContext, Boolean>  fallbackAction = mock(BiFunction.class);
+        when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(false);
+
+		assertThat(unleash.isEnabled("test", fallbackAction), is(true));
+        verify(fallbackAction, never()).apply(anyString(), any(UnleashContext.class));
+	}
+
     @Test
     public void should_register_custom_strategies() {
         //custom strategy