search

Unleash / unleash-client-java

Unleash client SDK for Java
https://docs.getunleash.io
Apache License 2.0
119 stars 70 forks source link

FakeUnleash doesn't take into account global activation/deactivation for certain isEnabled method #239

Closed GFriedrich closed 5 months ago

GFriedrich commented 6 months ago

Describe the bug

The FakeUnleash method using the fallbackAction doesn't take into account the global activation/deactivation flags. Right now the code at https://github.com/Unleash/unleash-client-java/blob/b2ac9aee6aecbd3e968ed5c0e51bb5fca1c60452/src/main/java/io/getunleash/FakeUnleash.java#L39-L45 just checks whether a certain flag is configured. But if you use the global flags, then no specific feature is set. Therefore this method always falls back to the default action instead, which is unexpected. Therefore I would suggest to change the line of https://github.com/Unleash/unleash-client-java/blob/b2ac9aee6aecbd3e968ed5c0e51bb5fca1c60452/src/main/java/io/getunleash/FakeUnleash.java#L41 to something like

if ((!enableAll && !disableAll || excludedFeatures.containsKey(toggleName)) && !features.containsKey(toggleName)) {

Steps to reproduce the bug

  1. Use FakeUnleash and enable all flags by calling enableAll
  2. Run a method like isEnabled("abc", (a, b) -> false)
  3. See that it still returns false even though all flags have been enabled.

Expected behavior

FakeUnleash should correctly take into account global flags.

Logs, error output, etc.

No response

Screenshots

No response

Additional context

No response

Unleash version

9.2.0

Subscription type

None

Hosting type

None

SDK information (language and version)

No response

sighphyre commented 6 months ago

Oh that's a nice catch, this probably isn't something we'll tackle immediately but we'd be more than happy to take a PR

ivarconr commented 5 months ago

This is an oversight on our end. Your suggestion makes sense at first sight, feel free to push a PR!

Thanks šŸ‘šŸ¼