From 9e557e7ad67e987e0c64357feccab7e9595782bd Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 May 2024 13:22:06 +0200 Subject: [PATCH 1/2] fix: enableAll and disableAll overrides fallback As discussed in #239 - When all is enabled, we had a bit of a surprising behaviour where we'd fallback to fallback action for `isEnabled(featureName, fallback)` even if all was enabled and feature did not exist. This PR fixes that, and adds tests to confirm this behaviour is intentional. closes: #239 --- src/main/java/io/getunleash/FakeUnleash.java | 2 +- .../java/io/getunleash/FakeUnleashTest.java | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/getunleash/FakeUnleash.java b/src/main/java/io/getunleash/FakeUnleash.java index 11b5bf425..f2efa024a 100644 --- a/src/main/java/io/getunleash/FakeUnleash.java +++ b/src/main/java/io/getunleash/FakeUnleash.java @@ -38,7 +38,7 @@ public boolean isEnabled( @Override public boolean isEnabled( String toggleName, BiPredicate fallbackAction) { - if (!features.containsKey(toggleName)) { + if ((!enableAll && !disableAll || excludedFeatures.containsKey(toggleName)) && !features.containsKey(toggleName)) { return fallbackAction.test(toggleName, UnleashContext.builder().build()); } return isEnabled(toggleName); diff --git a/src/test/java/io/getunleash/FakeUnleashTest.java b/src/test/java/io/getunleash/FakeUnleashTest.java index a4c00977d..c45368c66 100644 --- a/src/test/java/io/getunleash/FakeUnleashTest.java +++ b/src/test/java/io/getunleash/FakeUnleashTest.java @@ -179,4 +179,25 @@ public void should_countVariant_and_not_throw_an_error() { FakeUnleash fakeUnleash = new FakeUnleash(); fakeUnleash.more().countVariant("toggleName", "variantName"); } + + @Test + public void if_all_is_enabled_should_return_true_even_if_feature_does_not_exist_and_fallback_returns_false() { + FakeUnleash fakeUnleash = new FakeUnleash(); + fakeUnleash.enableAll(); + assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> false)).isTrue(); + } + + @Test + public void if_all_is_disabled_should_return_false_even_if_feature_does_not_exist_and_fallback_returns_true() { + FakeUnleash fakeUnleash = new FakeUnleash(); + fakeUnleash.disableAll(); + assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> true)).isFalse(); + } + + @Test + public void all_enabled_and_exclusion_toggle_returns_expected_result() { + FakeUnleash fakeUnleash = new FakeUnleash(); + fakeUnleash.enableAllExcept("my.feature.that.should.be.disabled"); + assertThat(fakeUnleash.isEnabled("my.feature.that.should.be.disabled", (name, context) -> false)).isFalse(); + } } From e4d1e527727d12c34470d0682f05bde2239e2291 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 May 2024 13:26:48 +0200 Subject: [PATCH 2/2] chore: spotlesS --- src/main/java/io/getunleash/FakeUnleash.java | 3 ++- .../java/io/getunleash/FakeUnleashTest.java | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/getunleash/FakeUnleash.java b/src/main/java/io/getunleash/FakeUnleash.java index f2efa024a..0e88d135a 100644 --- a/src/main/java/io/getunleash/FakeUnleash.java +++ b/src/main/java/io/getunleash/FakeUnleash.java @@ -38,7 +38,8 @@ public boolean isEnabled( @Override public boolean isEnabled( String toggleName, BiPredicate fallbackAction) { - if ((!enableAll && !disableAll || excludedFeatures.containsKey(toggleName)) && !features.containsKey(toggleName)) { + if ((!enableAll && !disableAll || excludedFeatures.containsKey(toggleName)) + && !features.containsKey(toggleName)) { return fallbackAction.test(toggleName, UnleashContext.builder().build()); } return isEnabled(toggleName); diff --git a/src/test/java/io/getunleash/FakeUnleashTest.java b/src/test/java/io/getunleash/FakeUnleashTest.java index c45368c66..daaaf09c2 100644 --- a/src/test/java/io/getunleash/FakeUnleashTest.java +++ b/src/test/java/io/getunleash/FakeUnleashTest.java @@ -181,23 +181,30 @@ public void should_countVariant_and_not_throw_an_error() { } @Test - public void if_all_is_enabled_should_return_true_even_if_feature_does_not_exist_and_fallback_returns_false() { + public void + if_all_is_enabled_should_return_true_even_if_feature_does_not_exist_and_fallback_returns_false() { FakeUnleash fakeUnleash = new FakeUnleash(); fakeUnleash.enableAll(); - assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> false)).isTrue(); + assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> false)) + .isTrue(); } @Test - public void if_all_is_disabled_should_return_false_even_if_feature_does_not_exist_and_fallback_returns_true() { + public void + if_all_is_disabled_should_return_false_even_if_feature_does_not_exist_and_fallback_returns_true() { FakeUnleash fakeUnleash = new FakeUnleash(); fakeUnleash.disableAll(); - assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> true)).isFalse(); + assertThat(fakeUnleash.isEnabled("my.non.existing.feature", (name, context) -> true)) + .isFalse(); } @Test public void all_enabled_and_exclusion_toggle_returns_expected_result() { FakeUnleash fakeUnleash = new FakeUnleash(); fakeUnleash.enableAllExcept("my.feature.that.should.be.disabled"); - assertThat(fakeUnleash.isEnabled("my.feature.that.should.be.disabled", (name, context) -> false)).isFalse(); + assertThat( + fakeUnleash.isEnabled( + "my.feature.that.should.be.disabled", (name, context) -> false)) + .isFalse(); } }