From a4b2c449752801d2c9341131e5569df84aaeef43 Mon Sep 17 00:00:00 2001 From: Dennis Menge Date: Tue, 12 Nov 2019 15:06:58 +0100 Subject: [PATCH 01/11] relates to #89: to Added first implementation (draft) of fallbackAction --- .../java/no/finn/unleash/DefaultUnleash.java | 41 +++++++++++++++---- .../java/no/finn/unleash/FakeUnleash.java | 29 +++++++++++++ .../java/no/finn/unleash/FallbackAction.java | 5 +++ src/main/java/no/finn/unleash/Unleash.java | 4 ++ 4 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 src/main/java/no/finn/unleash/FallbackAction.java diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index f7ee726c2..d6c6fb998 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -1,11 +1,5 @@ package no.finn.unleash; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; - import no.finn.unleash.event.EventDispatcher; import no.finn.unleash.event.ToggleEvaluated; import no.finn.unleash.metric.UnleashMetricService; @@ -17,6 +11,8 @@ import no.finn.unleash.strategy.*; import no.finn.unleash.util.UnleashConfig; +import java.util.*; + import static java.util.Optional.ofNullable; import static no.finn.unleash.Variant.DISABLED_VARIANT; import static no.finn.unleash.variant.VariantUtil.selectVariant; @@ -63,6 +59,14 @@ public DefaultUnleash(UnleashConfig unleashConfig, ToggleRepository toggleReposi metricService.register(strategyMap.keySet()); } + private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { + return new FeatureToggleRepository( + unleashConfig, + new HttpToggleFetcher(unleashConfig), + new ToggleBackupHandlerFile(unleashConfig) + ); + } + @Override public boolean isEnabled(final String toggleName) { return isEnabled(toggleName, false); @@ -73,12 +77,33 @@ public boolean isEnabled(final String toggleName, final boolean defaultSetting) return isEnabled(toggleName, contextProvider.getContext(), defaultSetting); } + public boolean isEnabled(final String toggleName, final FallbackAction fallbackAction) { + FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); + boolean enabled = isEnabled(featureToggle, contextProvider.getContext(), false); + + if(!enabled && fallbackAction != null) { + fallbackAction.apply(toggleName, contextProvider.getContext()); + } + + return enabled; + } + + public boolean isEnabled(final String toggleName, boolean defaultSetting, final FallbackAction fallbackAction) { + FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); + + if(featureToggle == null && fallbackAction != null) { + fallbackAction.apply(toggleName, contextProvider.getContext()); + } + + return isEnabled(featureToggle, contextProvider.getContext(), defaultSetting); + } + @Override - public boolean isEnabled(final String toggleName, final UnleashContext context ,final boolean defaultSetting) { + public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); boolean enabled = isEnabled(featureToggle, context, defaultSetting); count(toggleName, enabled); - eventDispatcher.dispatch(new ToggleEvaluated(toggleName,enabled)); + eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); return enabled; } diff --git a/src/main/java/no/finn/unleash/FakeUnleash.java b/src/main/java/no/finn/unleash/FakeUnleash.java index ea6d0105b..82aeb01f5 100644 --- a/src/main/java/no/finn/unleash/FakeUnleash.java +++ b/src/main/java/no/finn/unleash/FakeUnleash.java @@ -27,6 +27,35 @@ public boolean isEnabled(String toggleName, boolean defaultSetting) { } } + @Override + public boolean isEnabled(String toggleName, boolean defaultSetting, FallbackAction fallbackAction) { + if(enableAll) { + return true; + } else if(disableAll) { + + return false; + } else { + if(!features.containsKey(toggleName)) { + fallbackAction.apply(toggleName, UnleashContext.builder().build()); + } + return features.getOrDefault(toggleName, defaultSetting); + } + } + + @Override + public boolean isEnabled(String toggleName, FallbackAction fallbackAction) { + if(enableAll) { + return true; + } else if(disableAll) { + return false; + } else { + if(!features.containsKey(toggleName)) { + fallbackAction.apply(toggleName, UnleashContext.builder().build()); + } + return features.getOrDefault(toggleName, false); + } + } + @Override public Variant getVariant(String toggleName, UnleashContext context) { return getVariant(toggleName, Variant.DISABLED_VARIANT); diff --git a/src/main/java/no/finn/unleash/FallbackAction.java b/src/main/java/no/finn/unleash/FallbackAction.java new file mode 100644 index 000000000..1be7949c2 --- /dev/null +++ b/src/main/java/no/finn/unleash/FallbackAction.java @@ -0,0 +1,5 @@ +package no.finn.unleash; + +public interface FallbackAction { + void apply(String toggleName, UnleashContext unleashContext); +} diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index fe28dacdb..07be7125b 100644 --- a/src/main/java/no/finn/unleash/Unleash.java +++ b/src/main/java/no/finn/unleash/Unleash.java @@ -15,6 +15,10 @@ default boolean isEnabled(String toggleName, UnleashContext context, boolean def return isEnabled(toggleName, defaultSetting); } + boolean isEnabled(final String toggleName, boolean defaultSetting, final FallbackAction fallbackAction); + + boolean isEnabled(final String toggleName, final FallbackAction fallbackAction); + Variant getVariant(final String toggleName, final UnleashContext context); Variant getVariant(final String toggleName, final UnleashContext context, final Variant defaultValue); From 548b9b704fac9c495257ac4784cf2f26ed0bfe9f Mon Sep 17 00:00:00 2001 From: Dennis Menge Date: Tue, 12 Nov 2019 17:43:58 +0100 Subject: [PATCH 02/11] Migrated to BiFunction for definition of fallback action, refactored methods according to feedback, added tests --- .../java/no/finn/unleash/DefaultUnleash.java | 41 +++++++------------ .../java/no/finn/unleash/FakeUnleash.java | 14 +++++-- .../java/no/finn/unleash/FallbackAction.java | 5 --- src/main/java/no/finn/unleash/Unleash.java | 7 +++- .../java/no/finn/unleash/UnleashTest.java | 21 ++++++++++ 5 files changed, 51 insertions(+), 37 deletions(-) delete mode 100644 src/main/java/no/finn/unleash/FallbackAction.java diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index d6c6fb998..e4885987a 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -12,6 +12,7 @@ import no.finn.unleash.util.UnleashConfig; import java.util.*; +import java.util.function.BiFunction; import static java.util.Optional.ofNullable; import static no.finn.unleash.Variant.DISABLED_VARIANT; @@ -59,14 +60,6 @@ public DefaultUnleash(UnleashConfig unleashConfig, ToggleRepository toggleReposi metricService.register(strategyMap.keySet()); } - private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { - return new FeatureToggleRepository( - unleashConfig, - new HttpToggleFetcher(unleashConfig), - new ToggleBackupHandlerFile(unleashConfig) - ); - } - @Override public boolean isEnabled(final String toggleName) { return isEnabled(toggleName, false); @@ -77,33 +70,29 @@ public boolean isEnabled(final String toggleName, final boolean defaultSetting) return isEnabled(toggleName, contextProvider.getContext(), defaultSetting); } - public boolean isEnabled(final String toggleName, final FallbackAction fallbackAction) { - FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - boolean enabled = isEnabled(featureToggle, contextProvider.getContext(), false); - - if(!enabled && fallbackAction != null) { - fallbackAction.apply(toggleName, contextProvider.getContext()); - } - - return enabled; + @Override + public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) { + return isEnabled(toggleName, context, defaultSetting, null); } - public boolean isEnabled(final String toggleName, boolean defaultSetting, final FallbackAction fallbackAction) { - FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - - if(featureToggle == null && fallbackAction != null) { - fallbackAction.apply(toggleName, contextProvider.getContext()); - } + public boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { + return isEnabled(toggleName, false, fallbackAction); + } - return isEnabled(featureToggle, contextProvider.getContext(), defaultSetting); + public boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction) { + return isEnabled(toggleName, contextProvider.getContext(), defaultSetting, fallbackAction); } - @Override - public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) { + public boolean isEnabled(final String toggleName, UnleashContext context, boolean defaultSetting, final BiFunction fallbackAction) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); boolean enabled = isEnabled(featureToggle, context, defaultSetting); count(toggleName, enabled); eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); + + if(featureToggle == null && fallbackAction != null) { + return fallbackAction.apply(toggleName, context); + } + return enabled; } diff --git a/src/main/java/no/finn/unleash/FakeUnleash.java b/src/main/java/no/finn/unleash/FakeUnleash.java index 82aeb01f5..26d6543df 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.HashMap; import java.util.List; import java.util.Map; +import java.util.function.BiFunction; public final class FakeUnleash implements Unleash { private boolean enableAll = false; @@ -28,7 +29,7 @@ public boolean isEnabled(String toggleName, boolean defaultSetting) { } @Override - public boolean isEnabled(String toggleName, boolean defaultSetting, FallbackAction fallbackAction) { + public boolean isEnabled(String toggleName, boolean defaultSetting, BiFunction fallbackAction) { if(enableAll) { return true; } else if(disableAll) { @@ -36,21 +37,26 @@ public boolean isEnabled(String toggleName, boolean defaultSetting, FallbackActi return false; } else { if(!features.containsKey(toggleName)) { - fallbackAction.apply(toggleName, UnleashContext.builder().build()); + return fallbackAction.apply(toggleName, UnleashContext.builder().build()); } return features.getOrDefault(toggleName, defaultSetting); } } @Override - public boolean isEnabled(String toggleName, FallbackAction fallbackAction) { + public boolean isEnabled(String toggleName, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { + return isEnabled(toggleName, defaultSetting, fallbackAction); + } + + @Override + public boolean isEnabled(String toggleName, BiFunction fallbackAction) { if(enableAll) { return true; } else if(disableAll) { return false; } else { if(!features.containsKey(toggleName)) { - fallbackAction.apply(toggleName, UnleashContext.builder().build()); + return fallbackAction.apply(toggleName, UnleashContext.builder().build()); } return features.getOrDefault(toggleName, false); } diff --git a/src/main/java/no/finn/unleash/FallbackAction.java b/src/main/java/no/finn/unleash/FallbackAction.java deleted file mode 100644 index 1be7949c2..000000000 --- a/src/main/java/no/finn/unleash/FallbackAction.java +++ /dev/null @@ -1,5 +0,0 @@ -package no.finn.unleash; - -public interface FallbackAction { - void apply(String toggleName, UnleashContext unleashContext); -} diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index 07be7125b..4d47b6460 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,9 +16,11 @@ default boolean isEnabled(String toggleName, UnleashContext context, boolean def return isEnabled(toggleName, defaultSetting); } - boolean isEnabled(final String toggleName, boolean defaultSetting, final FallbackAction fallbackAction); + boolean isEnabled(final String toggleName, final BiFunction fallbackAction); - boolean isEnabled(final String toggleName, final FallbackAction fallbackAction); + boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction); + + boolean isEnabled(final String toggleName, UnleashContext context, boolean defaultSetting, final BiFunction fallbackAction); Variant getVariant(final String toggleName, final UnleashContext context); diff --git a/src/test/java/no/finn/unleash/UnleashTest.java b/src/test/java/no/finn/unleash/UnleashTest.java index 8f9f64303..cb15d4de3 100644 --- a/src/test/java/no/finn/unleash/UnleashTest.java +++ b/src/test/java/no/finn/unleash/UnleashTest.java @@ -81,6 +81,27 @@ public void unknown_feature_should_use_default_setting() { assertThat(unleash.isEnabled("test", true), is(true)); } + @Test + public void fallback_function_should_be_invoked() { + when(toggleRepository.getToggle("test")).thenReturn(null); + + assertThat(unleash.isEnabled("test", (name, unleashContext) -> true), is(true)); + } + + @Test + void fallback_function_should_override_default_fallback_value_when_toggle_not_defined() { + when(toggleRepository.getToggle("test")).thenReturn(null); + + assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> true), is(true)); + } + + @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)))); + + assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> true), is(true)); + } + @Test public void should_register_custom_strategies() { //custom strategy From 63b1621e24100f109458ef56753c52fcf1bfceaa Mon Sep 17 00:00:00 2001 From: Dennis Menge Date: Tue, 12 Nov 2019 17:46:12 +0100 Subject: [PATCH 03/11] changed one test case to reflect correct implementation --- src/test/java/no/finn/unleash/UnleashTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/no/finn/unleash/UnleashTest.java b/src/test/java/no/finn/unleash/UnleashTest.java index cb15d4de3..d69f62554 100644 --- a/src/test/java/no/finn/unleash/UnleashTest.java +++ b/src/test/java/no/finn/unleash/UnleashTest.java @@ -99,7 +99,7 @@ void fallback_function_should_override_default_fallback_value_when_toggle_not_de 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)))); - assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> true), is(true)); + assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> false), is(true)); } @Test From d6057c53e161f5e2de196af9dc41cf1a88a825c6 Mon Sep 17 00:00:00 2001 From: Dennis Menge Date: Tue, 12 Nov 2019 19:55:13 +0100 Subject: [PATCH 04/11] Added default implementation for isEnabled(String, BiFunction) --- src/main/java/no/finn/unleash/Unleash.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index 4d47b6460..a123c7d9f 100644 --- a/src/main/java/no/finn/unleash/Unleash.java +++ b/src/main/java/no/finn/unleash/Unleash.java @@ -16,7 +16,9 @@ default boolean isEnabled(String toggleName, UnleashContext context, boolean def return isEnabled(toggleName, defaultSetting); } - boolean isEnabled(final String toggleName, final BiFunction fallbackAction); + default boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { + return isEnabled(toggleName, false, fallbackAction); + } boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction); From b731dba3dd1f9f1f554b9174664b5da686cd8900 Mon Sep 17 00:00:00 2001 From: Dennis Menge Date: Wed, 13 Nov 2019 07:37:59 +0100 Subject: [PATCH 05/11] Incorporated feedback, changed logical method layout in DefaultUnleash implementation, added default implementation in Unleash interface to avoid breaking existing implementation --- .../java/no/finn/unleash/DefaultUnleash.java | 61 +++++++++++-------- src/main/java/no/finn/unleash/Unleash.java | 8 ++- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index e4885987a..9536d5cb7 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -1,5 +1,12 @@ package no.finn.unleash; +import java.util.Arrays; +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; import no.finn.unleash.metric.UnleashMetricService; @@ -8,12 +15,18 @@ import no.finn.unleash.repository.HttpToggleFetcher; import no.finn.unleash.repository.ToggleBackupHandlerFile; import no.finn.unleash.repository.ToggleRepository; -import no.finn.unleash.strategy.*; +import no.finn.unleash.strategy.ApplicationHostnameStrategy; +import no.finn.unleash.strategy.DefaultStrategy; +import no.finn.unleash.strategy.FlexibleRolloutStrategy; +import no.finn.unleash.strategy.GradualRolloutRandomStrategy; +import no.finn.unleash.strategy.GradualRolloutSessionIdStrategy; +import no.finn.unleash.strategy.GradualRolloutUserIdStrategy; +import no.finn.unleash.strategy.RemoteAddressStrategy; +import no.finn.unleash.strategy.Strategy; +import no.finn.unleash.strategy.UnknownStrategy; +import no.finn.unleash.strategy.UserWithIdStrategy; import no.finn.unleash.util.UnleashConfig; -import java.util.*; -import java.util.function.BiFunction; - import static java.util.Optional.ofNullable; import static no.finn.unleash.Variant.DISABLED_VARIANT; import static no.finn.unleash.variant.VariantUtil.selectVariant; @@ -38,14 +51,6 @@ public final class DefaultUnleash implements Unleash { private final UnleashConfig config; - private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { - return new FeatureToggleRepository( - unleashConfig, - new HttpToggleFetcher(unleashConfig), - new ToggleBackupHandlerFile(unleashConfig) - ); - } - public DefaultUnleash(UnleashConfig unleashConfig, Strategy... strategies) { this(unleashConfig, defaultToggleRepository(unleashConfig), strategies); } @@ -60,6 +65,14 @@ public DefaultUnleash(UnleashConfig unleashConfig, ToggleRepository toggleReposi metricService.register(strategyMap.keySet()); } + private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { + return new FeatureToggleRepository( + unleashConfig, + new HttpToggleFetcher(unleashConfig), + new ToggleBackupHandlerFile(unleashConfig) + ); + } + @Override public boolean isEnabled(final String toggleName) { return isEnabled(toggleName, false); @@ -72,7 +85,7 @@ public boolean isEnabled(final String toggleName, final boolean defaultSetting) @Override public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) { - return isEnabled(toggleName, context, defaultSetting, null); + return isEnabled(toggleName, context, defaultSetting, (name, unleashContext) -> defaultSetting); } public boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { @@ -85,30 +98,30 @@ public boolean isEnabled(final String toggleName, boolean defaultSetting, final public boolean isEnabled(final String toggleName, UnleashContext context, boolean defaultSetting, final BiFunction fallbackAction) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - boolean enabled = isEnabled(featureToggle, context, defaultSetting); - count(toggleName, enabled); - eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); - if(featureToggle == null && fallbackAction != null) { - return fallbackAction.apply(toggleName, context); - } - - return enabled; + return isEnabled(toggleName, featureToggle, context, defaultSetting, fallbackAction); } - private boolean isEnabled(FeatureToggle featureToggle, UnleashContext context, boolean defaultSetting) { + private boolean isEnabled(String toggleName, FeatureToggle featureToggle, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { boolean enabled; + UnleashContext enhancedContext = context.applyStaticFields(config); + if (featureToggle == null) { enabled = defaultSetting; + + if(fallbackAction != null) { + 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())); } + count(toggleName, enabled); + eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); return enabled; } @@ -120,7 +133,7 @@ public Variant getVariant(String toggleName, UnleashContext context) { @Override public Variant getVariant(String toggleName, UnleashContext context, Variant defaultValue) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - boolean enabled = isEnabled(featureToggle, context, false); + boolean enabled = isEnabled(toggleName, context, 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/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index a123c7d9f..a1c648e96 100644 --- a/src/main/java/no/finn/unleash/Unleash.java +++ b/src/main/java/no/finn/unleash/Unleash.java @@ -20,9 +20,13 @@ default boolean isEnabled(final String toggleName, final BiFunction fallbackAction); + default boolean isEnabled(String toggleName, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { + return isEnabled(toggleName, defaultSetting, fallbackAction); + } - boolean isEnabled(final String toggleName, UnleashContext context, boolean defaultSetting, final BiFunction fallbackAction); + default boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction) { + return isEnabled(toggleName, fallbackAction); + } Variant getVariant(final String toggleName, final UnleashContext context); From e96a1730d2803ac66e2941e2e67b0b698b438a43 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 16 Nov 2019 11:59:54 +0100 Subject: [PATCH 06/11] fix: fallbackAction and defaultSetting Should not be possible to define both a fallback action and a defaultSetting at the same time. --- .../java/no/finn/unleash/DefaultUnleash.java | 24 +++++-------------- .../java/no/finn/unleash/FakeUnleash.java | 19 ++------------- src/main/java/no/finn/unleash/Unleash.java | 10 +++----- .../java/no/finn/unleash/UnleashTest.java | 4 ++-- 4 files changed, 13 insertions(+), 44 deletions(-) diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index 9536d5cb7..30eba4f14 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -85,33 +85,21 @@ public boolean isEnabled(final String toggleName, final boolean defaultSetting) @Override public boolean isEnabled(final String toggleName, final UnleashContext context, final boolean defaultSetting) { - return isEnabled(toggleName, context, defaultSetting, (name, unleashContext) -> defaultSetting); + return isEnabled(toggleName, context, (n, c) -> defaultSetting); } + @Override public boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { - return isEnabled(toggleName, false, fallbackAction); - } - - public boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction) { - return isEnabled(toggleName, contextProvider.getContext(), defaultSetting, fallbackAction); + return isEnabled(toggleName, contextProvider.getContext(), fallbackAction); } - public boolean isEnabled(final String toggleName, UnleashContext context, boolean defaultSetting, final BiFunction fallbackAction) { + public boolean isEnabled(String toggleName, UnleashContext context, BiFunction fallbackAction) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - - return isEnabled(toggleName, featureToggle, context, defaultSetting, fallbackAction); - } - - private boolean isEnabled(String toggleName, FeatureToggle featureToggle, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { boolean enabled; UnleashContext enhancedContext = context.applyStaticFields(config); if (featureToggle == null) { - enabled = defaultSetting; - - if(fallbackAction != null) { - enabled = fallbackAction.apply(toggleName, enhancedContext); - } + enabled = fallbackAction.apply(toggleName, enhancedContext); } else if(!featureToggle.isEnabled()) { enabled = false; } else if(featureToggle.getStrategies().size() == 0) { @@ -133,7 +121,7 @@ public Variant getVariant(String toggleName, UnleashContext context) { @Override public Variant getVariant(String toggleName, UnleashContext context, Variant defaultValue) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - boolean enabled = isEnabled(toggleName, context, false); + boolean enabled = isEnabled(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 26d6543df..fe6fc4bb7 100644 --- a/src/main/java/no/finn/unleash/FakeUnleash.java +++ b/src/main/java/no/finn/unleash/FakeUnleash.java @@ -29,23 +29,8 @@ public boolean isEnabled(String toggleName, boolean defaultSetting) { } @Override - public boolean isEnabled(String toggleName, boolean defaultSetting, BiFunction fallbackAction) { - if(enableAll) { - return true; - } else if(disableAll) { - - return false; - } else { - if(!features.containsKey(toggleName)) { - return fallbackAction.apply(toggleName, UnleashContext.builder().build()); - } - return features.getOrDefault(toggleName, defaultSetting); - } - } - - @Override - public boolean isEnabled(String toggleName, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { - return isEnabled(toggleName, defaultSetting, fallbackAction); + public boolean isEnabled(String toggleName, UnleashContext context, BiFunction fallbackAction) { + return isEnabled(toggleName, fallbackAction); } @Override diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index a1c648e96..b86d9000b 100644 --- a/src/main/java/no/finn/unleash/Unleash.java +++ b/src/main/java/no/finn/unleash/Unleash.java @@ -17,15 +17,11 @@ default boolean isEnabled(String toggleName, UnleashContext context, boolean def } default boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { - return isEnabled(toggleName, false, fallbackAction); + return isEnabled(toggleName, false); } - default boolean isEnabled(String toggleName, UnleashContext context, boolean defaultSetting, BiFunction fallbackAction) { - return isEnabled(toggleName, defaultSetting, fallbackAction); - } - - default boolean isEnabled(final String toggleName, boolean defaultSetting, final BiFunction fallbackAction) { - return isEnabled(toggleName, fallbackAction); + default boolean isEnabled(String toggleName, UnleashContext context, BiFunction fallbackAction) { + return isEnabled(toggleName, context, false); } Variant getVariant(final String toggleName, final UnleashContext context); diff --git a/src/test/java/no/finn/unleash/UnleashTest.java b/src/test/java/no/finn/unleash/UnleashTest.java index d69f62554..c477a606f 100644 --- a/src/test/java/no/finn/unleash/UnleashTest.java +++ b/src/test/java/no/finn/unleash/UnleashTest.java @@ -92,14 +92,14 @@ public void fallback_function_should_be_invoked() { void fallback_function_should_override_default_fallback_value_when_toggle_not_defined() { when(toggleRepository.getToggle("test")).thenReturn(null); - assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> true), is(true)); + assertThat(unleash.isEnabled("test", (name, unleashContext) -> true), is(true)); } @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)))); - assertThat(unleash.isEnabled("test", false, (name, unleashContext) -> false), is(true)); + assertThat(unleash.isEnabled("test", (name, unleashContext) -> false), is(true)); } @Test From b4c8712806b6a62367d73c91c90ff0a7260d4bec Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 16 Nov 2019 12:17:47 +0100 Subject: [PATCH 07/11] fix: Improve fallbackAction unit-tests Use mocked functions and verify that the fallBack action is actually called with correct arguments. --- .../java/no/finn/unleash/UnleashTest.java | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/test/java/no/finn/unleash/UnleashTest.java b/src/test/java/no/finn/unleash/UnleashTest.java index c477a606f..ead646bd8 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.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.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 { @@ -82,24 +81,46 @@ public void unknown_feature_should_use_default_setting() { } @Test - public void fallback_function_should_be_invoked() { + public void fallback_function_should_be_invoked_and_return_true() { when(toggleRepository.getToggle("test")).thenReturn(null); + BiFunction fallbackAction = mock(BiFunction.class); + when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(true); - assertThat(unleash.isEnabled("test", (name, unleashContext) -> true), is(true)); + assertThat(unleash.isEnabled("test", fallbackAction), is(true)); + verify(fallbackAction, times(1)).apply(anyString(), any(UnleashContext.class)); } @Test - void fallback_function_should_override_default_fallback_value_when_toggle_not_defined() { + public void fallback_function_should_be_invoked_also_with_context() { when(toggleRepository.getToggle("test")).thenReturn(null); + BiFunction fallbackAction = mock(BiFunction.class); + when(fallbackAction.apply(eq("test"), any(UnleashContext.class))).thenReturn(true); - assertThat(unleash.isEnabled("test", (name, unleashContext) -> true), is(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 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)))); - assertThat(unleash.isEnabled("test", (name, unleashContext) -> false), is(true)); + BiFunction 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 From 2e8bfcc2958af3ec139c8b73a15ee5bc9f867ab9 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 16 Nov 2019 12:23:52 +0100 Subject: [PATCH 08/11] fix: remove final arguments in interface --- src/main/java/no/finn/unleash/Unleash.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/no/finn/unleash/Unleash.java b/src/main/java/no/finn/unleash/Unleash.java index b86d9000b..3bd04b80d 100644 --- a/src/main/java/no/finn/unleash/Unleash.java +++ b/src/main/java/no/finn/unleash/Unleash.java @@ -16,7 +16,7 @@ default boolean isEnabled(String toggleName, UnleashContext context, boolean def return isEnabled(toggleName, defaultSetting); } - default boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { + default boolean isEnabled(String toggleName, BiFunction fallbackAction) { return isEnabled(toggleName, false); } From 2b7ac4977ac72007057257426effb1ee9f2d1465 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 16 Nov 2019 12:29:27 +0100 Subject: [PATCH 09/11] fix: simplify FakeUnleash for fallbackAction --- src/main/java/no/finn/unleash/FakeUnleash.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/main/java/no/finn/unleash/FakeUnleash.java b/src/main/java/no/finn/unleash/FakeUnleash.java index fe6fc4bb7..3b6a8a4de 100644 --- a/src/main/java/no/finn/unleash/FakeUnleash.java +++ b/src/main/java/no/finn/unleash/FakeUnleash.java @@ -35,16 +35,10 @@ public boolean isEnabled(String toggleName, UnleashContext context, BiFunction fallbackAction) { - if(enableAll) { - return true; - } else if(disableAll) { - return false; - } else { - if(!features.containsKey(toggleName)) { - return fallbackAction.apply(toggleName, UnleashContext.builder().build()); - } - return features.getOrDefault(toggleName, false); + if(!features.containsKey(toggleName)) { + return fallbackAction.apply(toggleName, UnleashContext.builder().build()); } + return isEnabled(toggleName); } @Override From 5dc0061ade9b54ab76d3636333b14b7539bad521 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Tue, 19 Nov 2019 08:35:52 +0100 Subject: [PATCH 10/11] fix: formatting --- .../java/no/finn/unleash/DefaultUnleash.java | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index 30eba4f14..c93ac2f00 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -15,16 +15,7 @@ import no.finn.unleash.repository.HttpToggleFetcher; import no.finn.unleash.repository.ToggleBackupHandlerFile; import no.finn.unleash.repository.ToggleRepository; -import no.finn.unleash.strategy.ApplicationHostnameStrategy; -import no.finn.unleash.strategy.DefaultStrategy; -import no.finn.unleash.strategy.FlexibleRolloutStrategy; -import no.finn.unleash.strategy.GradualRolloutRandomStrategy; -import no.finn.unleash.strategy.GradualRolloutSessionIdStrategy; -import no.finn.unleash.strategy.GradualRolloutUserIdStrategy; -import no.finn.unleash.strategy.RemoteAddressStrategy; -import no.finn.unleash.strategy.Strategy; -import no.finn.unleash.strategy.UnknownStrategy; -import no.finn.unleash.strategy.UserWithIdStrategy; +import no.finn.unleash.strategy.*; import no.finn.unleash.util.UnleashConfig; import static java.util.Optional.ofNullable; @@ -50,6 +41,13 @@ public final class DefaultUnleash implements Unleash { private final EventDispatcher eventDispatcher; private final UnleashConfig config; + private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { + return new FeatureToggleRepository( + unleashConfig, + new HttpToggleFetcher(unleashConfig), + new ToggleBackupHandlerFile(unleashConfig) + ); + } public DefaultUnleash(UnleashConfig unleashConfig, Strategy... strategies) { this(unleashConfig, defaultToggleRepository(unleashConfig), strategies); @@ -65,14 +63,6 @@ public DefaultUnleash(UnleashConfig unleashConfig, ToggleRepository toggleReposi metricService.register(strategyMap.keySet()); } - private static FeatureToggleRepository defaultToggleRepository(UnleashConfig unleashConfig) { - return new FeatureToggleRepository( - unleashConfig, - new HttpToggleFetcher(unleashConfig), - new ToggleBackupHandlerFile(unleashConfig) - ); - } - @Override public boolean isEnabled(final String toggleName) { return isEnabled(toggleName, false); From e0eb4359830e4f1f76e7c8e43b2ab19fad52de09 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Tue, 19 Nov 2019 08:45:18 +0100 Subject: [PATCH 11/11] fix: counting at right place --- src/main/java/no/finn/unleash/DefaultUnleash.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java index c93ac2f00..c63ebcf0c 100644 --- a/src/main/java/no/finn/unleash/DefaultUnleash.java +++ b/src/main/java/no/finn/unleash/DefaultUnleash.java @@ -83,7 +83,15 @@ public boolean isEnabled(final String toggleName, final BiFunction fallbackAction) { + boolean enabled = checkEnabled(toggleName, context, fallbackAction); + count(toggleName, enabled); + eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); + return enabled; + } + + private boolean checkEnabled(String toggleName, UnleashContext context, BiFunction fallbackAction) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); boolean enabled; UnleashContext enhancedContext = context.applyStaticFields(config); @@ -98,8 +106,6 @@ public boolean isEnabled(String toggleName, UnleashContext context, BiFunction getStrategy(as.getName()).isEnabled(as.getParameters(), enhancedContext, as.getConstraints())); } - count(toggleName, enabled); - eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled)); return enabled; } @@ -111,7 +117,7 @@ public Variant getVariant(String toggleName, UnleashContext context) { @Override public Variant getVariant(String toggleName, UnleashContext context, Variant defaultValue) { FeatureToggle featureToggle = toggleRepository.getToggle(toggleName); - boolean enabled = isEnabled(toggleName, context, (n, c) -> false); + boolean enabled = checkEnabled(toggleName, context, (n, c) -> false); Variant variant = enabled ? selectVariant(featureToggle, context, defaultValue) : defaultValue; metricService.countVariant(toggleName, variant.getName()); return variant;