From bdbe98dc0b1aa3105914d14f0831bd9d2de680c3 Mon Sep 17 00:00:00 2001 From: itchyny Date: Thu, 28 Dec 2023 13:54:06 +0900 Subject: [PATCH] fix: [go-feature-flag] Fix NullPointerException on checking anonymous user --- .../gofeatureflag/bean/GoFeatureFlagUser.java | 19 ++++++++++----- .../gofeatureflag/hook/DataCollectorHook.java | 5 ++-- .../GoFeatureFlagProviderTest.java | 23 +++++++++++++++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/GoFeatureFlagUser.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/GoFeatureFlagUser.java index ccc63d721..4d106830e 100644 --- a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/GoFeatureFlagUser.java +++ b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/GoFeatureFlagUser.java @@ -28,18 +28,25 @@ public class GoFeatureFlagUser { */ public static GoFeatureFlagUser fromEvaluationContext(EvaluationContext ctx) { String key = ctx.getTargetingKey(); - if (key == null || "".equals(key)) { + if (key == null || key.isEmpty()) { throw new TargetingKeyMissingError(); } - Value anonymousValue = ctx.getValue(anonymousFieldName); - if (anonymousValue == null) { - anonymousValue = new Value(Boolean.FALSE); - } - boolean anonymous = anonymousValue.asBoolean(); + boolean anonymous = isAnonymousUser(ctx); Map custom = new HashMap<>(ctx.asObjectMap()); if (ctx.getValue(anonymousFieldName) != null) { custom.remove(anonymousFieldName); } return GoFeatureFlagUser.builder().anonymous(anonymous).key(key).custom(custom).build(); } + + /** + * isAnonymousUser is checking if the user in the evaluationContext is anonymous. + * + * @param ctx - EvaluationContext from open-feature + * @return true if the user is anonymous, false otherwise + */ + public static boolean isAnonymousUser(EvaluationContext ctx) { + Value value = ctx.getValue(anonymousFieldName); + return value != null && value.asBoolean(); + } } diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/DataCollectorHook.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/DataCollectorHook.java index 11fab7de0..7022a88c7 100644 --- a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/DataCollectorHook.java +++ b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/DataCollectorHook.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import dev.openfeature.contrib.providers.gofeatureflag.GoFeatureFlagProvider; +import dev.openfeature.contrib.providers.gofeatureflag.bean.GoFeatureFlagUser; import dev.openfeature.contrib.providers.gofeatureflag.exception.InvalidOptions; import dev.openfeature.contrib.providers.gofeatureflag.hook.events.Event; import dev.openfeature.contrib.providers.gofeatureflag.hook.events.Events; @@ -73,7 +74,7 @@ public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { Event event = Event.builder() .key(ctx.getFlagKey()) .kind("feature") - .contextKind(ctx.getCtx().getValue("anonymous").asBoolean() ? "anonymousUser" : "user") + .contextKind(GoFeatureFlagUser.isAnonymousUser(ctx.getCtx()) ? "anonymousUser" : "user") .defaultValue(false) .variation(details.getVariant()) .value(details.getValue()) @@ -88,7 +89,7 @@ public void error(HookContext ctx, Exception error, Map hints) { Event event = Event.builder() .key(ctx.getFlagKey()) .kind("feature") - .contextKind(ctx.getCtx().getValue("anonymous").asBoolean() ? "anonymousUser" : "user") + .contextKind(GoFeatureFlagUser.isAnonymousUser(ctx.getCtx()) ? "anonymousUser" : "user") .creationDate(System.currentTimeMillis()) .defaultValue(true) .variation("SdkDefault") diff --git a/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java b/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java index f66810582..fddd9120b 100644 --- a/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java +++ b/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java @@ -750,6 +750,29 @@ void should_publish_events() { assertEquals(6, publishEventsRequestsReceived, "we have call 6 time more, so we should consider only those new calls"); } + @SneakyThrows + @Test + void should_publish_events_context_without_anonymous() { + this.evaluationContext = new MutableContext("d45e303a-38c2-11ed-a261-0242ac120002"); + GoFeatureFlagProvider g = new GoFeatureFlagProvider(GoFeatureFlagProviderOptions.builder() + .endpoint(this.baseUrl.toString()) + .timeout(1000) + .enableCache(true) + .flushIntervalMs(50L) + .build()); + String providerName = this.testName; + OpenFeatureAPI.getInstance().setProviderAndWait(providerName, g); + Client client = OpenFeatureAPI.getInstance().getClient(providerName); + client.getBooleanDetails("fail_500", false, this.evaluationContext); + Thread.sleep(100L); + assertEquals(1, publishEventsRequestsReceived, "We should have 1 event waiting to be publish"); + client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext); + client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext); + client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext); + Thread.sleep(100); + assertEquals(3, publishEventsRequestsReceived, "We pass the flush interval, we should have 3 events"); + } + private String readMockResponse(String filename) throws Exception { URL url = getClass().getClassLoader().getResource("mock_responses/" + filename); assert url != null;