From 249d11fbbe0d46c9c660d1799fe3c11698114d28 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Mon, 27 Apr 2026 12:36:56 -0400 Subject: [PATCH] add flagKey to SelectedVariant --- .../openfeature/MixpanelProvider.java | 2 +- .../openfeature/MixpanelProviderTest.java | 48 ++++++------ .../featureflags/model/SelectedVariant.java | 61 +++++++++++++-- .../provider/BaseFlagsProvider.java | 5 +- .../provider/LocalFlagsProvider.java | 11 +-- .../provider/RemoteFlagsProvider.java | 8 +- .../provider/LocalFlagsProviderTest.java | 76 +++++++++++++++++++ .../provider/RemoteFlagsProviderTest.java | 15 ++++ 8 files changed, 183 insertions(+), 43 deletions(-) diff --git a/openfeature-provider/src/main/java/com/mixpanel/openfeature/MixpanelProvider.java b/openfeature-provider/src/main/java/com/mixpanel/openfeature/MixpanelProvider.java index 251eea9..a094709 100644 --- a/openfeature-provider/src/main/java/com/mixpanel/openfeature/MixpanelProvider.java +++ b/openfeature-provider/src/main/java/com/mixpanel/openfeature/MixpanelProvider.java @@ -145,7 +145,7 @@ private ProviderEvaluation evaluate(String key, T defaultValue, Evaluatio } private SelectedVariant fetchVariant(String key, EvaluationContext ctx) { - SelectedVariant fallback = new SelectedVariant<>(null); + SelectedVariant fallback = new SelectedVariant<>(key, null, null, null, null, null); return flagsProvider.getVariant(key, fallback, convertContext(ctx), true); } diff --git a/openfeature-provider/src/test/java/com/mixpanel/openfeature/MixpanelProviderTest.java b/openfeature-provider/src/test/java/com/mixpanel/openfeature/MixpanelProviderTest.java index c5870bd..29b9842 100644 --- a/openfeature-provider/src/test/java/com/mixpanel/openfeature/MixpanelProviderTest.java +++ b/openfeature-provider/src/test/java/com/mixpanel/openfeature/MixpanelProviderTest.java @@ -41,7 +41,7 @@ public void testGetMetadataReturnsCorrectName() { @SuppressWarnings("unchecked") @Test public void testBooleanEvaluationSuccess() { - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("bool-flag", "on", true, null, null, null); when(mockFlagsProvider.getVariant(eq("bool-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -56,7 +56,7 @@ public void testBooleanEvaluationSuccess() { @SuppressWarnings("unchecked") @Test public void testBooleanEvaluationTypeMismatch() { - SelectedVariant variant = new SelectedVariant<>("on", "not-a-boolean", null, null, null); + SelectedVariant variant = new SelectedVariant<>("bool-flag", "on", "not-a-boolean", null, null, null); when(mockFlagsProvider.getVariant(eq("bool-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -86,7 +86,7 @@ public void testBooleanEvaluationFlagNotFound() { @SuppressWarnings("unchecked") @Test public void testStringEvaluationSuccess() { - SelectedVariant variant = new SelectedVariant<>("blue", "blue", null, null, null); + SelectedVariant variant = new SelectedVariant<>("color-flag", "blue", "blue", null, null, null); when(mockFlagsProvider.getVariant(eq("color-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -101,7 +101,7 @@ public void testStringEvaluationSuccess() { @SuppressWarnings("unchecked") @Test public void testStringEvaluationTypeMismatch() { - SelectedVariant variant = new SelectedVariant<>("on", 42, null, null, null); + SelectedVariant variant = new SelectedVariant<>("string-flag", "on", 42, null, null, null); when(mockFlagsProvider.getVariant(eq("string-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -131,7 +131,7 @@ public void testStringEvaluationFlagNotFound() { @SuppressWarnings("unchecked") @Test public void testIntegerEvaluationSuccess() { - SelectedVariant variant = new SelectedVariant<>("v1", 42, null, null, null); + SelectedVariant variant = new SelectedVariant<>("int-flag", "v1", 42, null, null, null); when(mockFlagsProvider.getVariant(eq("int-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -146,7 +146,7 @@ public void testIntegerEvaluationSuccess() { @SuppressWarnings("unchecked") @Test public void testIntegerEvaluationFromLong() { - SelectedVariant variant = new SelectedVariant<>("v1", 42L, null, null, null); + SelectedVariant variant = new SelectedVariant<>("int-flag", "v1", 42L, null, null, null); when(mockFlagsProvider.getVariant(eq("int-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -159,7 +159,7 @@ public void testIntegerEvaluationFromLong() { @SuppressWarnings("unchecked") @Test public void testIntegerEvaluationFromDouble() { - SelectedVariant variant = new SelectedVariant<>("v1", 42.0, null, null, null); + SelectedVariant variant = new SelectedVariant<>("int-flag", "v1", 42.0, null, null, null); when(mockFlagsProvider.getVariant(eq("int-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -172,7 +172,7 @@ public void testIntegerEvaluationFromDouble() { @SuppressWarnings("unchecked") @Test public void testIntegerEvaluationTypeMismatch() { - SelectedVariant variant = new SelectedVariant<>("v1", "not-a-number", null, null, null); + SelectedVariant variant = new SelectedVariant<>("int-flag", "v1", "not-a-number", null, null, null); when(mockFlagsProvider.getVariant(eq("int-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -200,7 +200,7 @@ public void testIntegerEvaluationFlagNotFound() { @SuppressWarnings("unchecked") @Test public void testIntegerEvaluationOverflowReturnsMismatch() { - SelectedVariant variant = new SelectedVariant<>("v1", Long.MAX_VALUE, null, null, null); + SelectedVariant variant = new SelectedVariant<>("int-flag", "v1", Long.MAX_VALUE, null, null, null); when(mockFlagsProvider.getVariant(eq("int-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -215,7 +215,7 @@ public void testIntegerEvaluationOverflowReturnsMismatch() { @SuppressWarnings("unchecked") @Test public void testDoubleEvaluationSuccess() { - SelectedVariant variant = new SelectedVariant<>("v1", 3.14, null, null, null); + SelectedVariant variant = new SelectedVariant<>("double-flag", "v1", 3.14, null, null, null); when(mockFlagsProvider.getVariant(eq("double-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -230,7 +230,7 @@ public void testDoubleEvaluationSuccess() { @SuppressWarnings("unchecked") @Test public void testDoubleEvaluationFromInteger() { - SelectedVariant variant = new SelectedVariant<>("v1", 42, null, null, null); + SelectedVariant variant = new SelectedVariant<>("double-flag", "v1", 42, null, null, null); when(mockFlagsProvider.getVariant(eq("double-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -243,7 +243,7 @@ public void testDoubleEvaluationFromInteger() { @SuppressWarnings("unchecked") @Test public void testDoubleEvaluationTypeMismatch() { - SelectedVariant variant = new SelectedVariant<>("v1", "not-a-number", null, null, null); + SelectedVariant variant = new SelectedVariant<>("double-flag", "v1", "not-a-number", null, null, null); when(mockFlagsProvider.getVariant(eq("double-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -275,7 +275,7 @@ public void testDoubleEvaluationFlagNotFound() { public void testObjectEvaluationSuccess() { Map objValue = new HashMap<>(); objValue.put("key", "value"); - SelectedVariant variant = new SelectedVariant<>("v1", objValue, null, null, null); + SelectedVariant variant = new SelectedVariant<>("obj-flag", "v1", objValue, null, null, null); when(mockFlagsProvider.getVariant(eq("obj-flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -307,7 +307,7 @@ public void testObjectEvaluationFlagNotFound() { @SuppressWarnings("unchecked") @Test public void testPerEvaluationContextIsForwarded() { - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); ArgumentCaptor> contextCaptor = ArgumentCaptor.forClass(Map.class); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), contextCaptor.capture(), eq(true))) .thenReturn(variant); @@ -327,7 +327,7 @@ public void testPerEvaluationContextIsForwarded() { public void testPerEvaluationContextIsForwardedForObjectEvaluation() { Map objValue = new HashMap<>(); objValue.put("key", "value"); - SelectedVariant variant = new SelectedVariant<>("v1", objValue, null, null, null); + SelectedVariant variant = new SelectedVariant<>("obj-flag", "v1", objValue, null, null, null); ArgumentCaptor> contextCaptor = ArgumentCaptor.forClass(Map.class); when(mockFlagsProvider.getVariant(eq("obj-flag"), any(SelectedVariant.class), contextCaptor.capture(), eq(true))) .thenReturn(variant); @@ -343,7 +343,7 @@ public void testPerEvaluationContextIsForwardedForObjectEvaluation() { @SuppressWarnings("unchecked") @Test public void testTargetingKeyIsRegularProperty() { - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); ArgumentCaptor> contextCaptor = ArgumentCaptor.forClass(Map.class); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), contextCaptor.capture(), eq(true))) .thenReturn(variant); @@ -362,7 +362,7 @@ public void testTargetingKeyIsRegularProperty() { @SuppressWarnings("unchecked") @Test public void testTargetingKeyFromGetTargetingKeyIsIncluded() { - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); ArgumentCaptor> contextCaptor = ArgumentCaptor.forClass(Map.class); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), contextCaptor.capture(), eq(true))) .thenReturn(variant); @@ -382,7 +382,7 @@ public void testTargetingKeyFromGetTargetingKeyIsIncluded() { @SuppressWarnings("unchecked") @Test public void testExplicitTargetingKeyAttributeOverriddenByGetTargetingKey() { - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); ArgumentCaptor> contextCaptor = ArgumentCaptor.forClass(Map.class); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), contextCaptor.capture(), eq(true))) .thenReturn(variant); @@ -435,7 +435,7 @@ public void testProviderNotReadyObjectEvaluation() { public void testProviderReadyWithLocalProvider() { LocalFlagsProvider mockLocal = mock(LocalFlagsProvider.class); when(mockLocal.areFlagsReady()).thenReturn(true); - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); when(mockLocal.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); MixpanelProvider localProvider = new MixpanelProvider(mockLocal); @@ -451,7 +451,7 @@ public void testProviderReadyWithLocalProvider() { @Test public void testProviderNotReadySkippedForNonLocalProvider() { // BaseFlagsProvider (non-local) should not check readiness - SelectedVariant variant = new SelectedVariant<>("on", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "on", true, null, null, null); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -498,7 +498,7 @@ public void testObjectEvaluationException() { @SuppressWarnings("unchecked") @Test public void testVariantKeyPassedThroughOnBooleanEvaluation() { - SelectedVariant variant = new SelectedVariant<>("my-variant", true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "my-variant", true, null, null, null); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -510,7 +510,7 @@ public void testVariantKeyPassedThroughOnBooleanEvaluation() { @SuppressWarnings("unchecked") @Test public void testVariantKeyPassedThroughOnObjectEvaluation() { - SelectedVariant variant = new SelectedVariant<>("obj-variant", "some-value", null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", "obj-variant", "some-value", null, null, null); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -522,7 +522,7 @@ public void testVariantKeyPassedThroughOnObjectEvaluation() { @SuppressWarnings("unchecked") @Test public void testNullVariantKeyTreatedAsFallbackOnBooleanEvaluation() { - SelectedVariant variant = new SelectedVariant<>(null, true, null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", null, true, null, null, null); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); @@ -536,7 +536,7 @@ public void testNullVariantKeyTreatedAsFallbackOnBooleanEvaluation() { @SuppressWarnings("unchecked") @Test public void testNullVariantKeyTreatedAsFallbackOnObjectEvaluation() { - SelectedVariant variant = new SelectedVariant<>(null, "some-value", null, null, null); + SelectedVariant variant = new SelectedVariant<>("flag", null, "some-value", null, null, null); when(mockFlagsProvider.getVariant(eq("flag"), any(SelectedVariant.class), anyMap(), eq(true))) .thenReturn(variant); diff --git a/src/main/java/com/mixpanel/mixpanelapi/featureflags/model/SelectedVariant.java b/src/main/java/com/mixpanel/mixpanelapi/featureflags/model/SelectedVariant.java index 4a41830..720cbeb 100644 --- a/src/main/java/com/mixpanel/mixpanelapi/featureflags/model/SelectedVariant.java +++ b/src/main/java/com/mixpanel/mixpanelapi/featureflags/model/SelectedVariant.java @@ -5,8 +5,10 @@ /** * Represents the result of a feature flag evaluation. *

- * Contains the selected variant key and its value. Both may be null if the - * fallback was returned (e.g., flag not found, evaluation error). + * Contains the originating flag key, the selected variant key, and the variant value. + * The variant key and value may be null if the fallback was returned (e.g., flag not + * found, evaluation error); the flag key is populated by the SDK whenever a variant is + * returned from {@code getVariant}, including on fallback paths. *

*

* This class is immutable and thread-safe. @@ -15,6 +17,7 @@ * @param the type of the variant value */ public final class SelectedVariant { + private final String flagKey; private final String variantKey; private final T variantValue; private final UUID experimentId; @@ -22,25 +25,45 @@ public final class SelectedVariant { private final Boolean isQaTester; /** - * Creates a SelectedVariant with only a value (key is null). - * This is typically used for fallback responses. + * Creates a SelectedVariant carrying only a value. Both the flag key and variant key are + * null. This is the typical form for constructing a fallback to pass into + * {@code getVariant}; the SDK will stamp the requested flag key onto the returned variant. * * @param variantValue the fallback value */ public SelectedVariant(T variantValue) { - this(null, variantValue, null, null, null); + this(null, null, variantValue, null, null, null); } /** - * Creates a new SelectedVariant with experimentation metadata. + * Creates a new SelectedVariant with experimentation metadata. The flag key will be null. * * @param variantKey the key of the selected variant (may be null for fallback) * @param variantValue the value of the selected variant (may be null for fallback) * @param experimentId the experiment ID (may be null) * @param isExperimentActive whether the experiment is active (may be null) * @param isQaTester whether the user is a QA tester (may be null) + * @deprecated Use {@link #SelectedVariant(String, String, Object, UUID, Boolean, Boolean)} + * which also accepts the originating flag key, so the resulting variant can be + * associated with the flag it was selected for. */ + @Deprecated public SelectedVariant(String variantKey, T variantValue, UUID experimentId, Boolean isExperimentActive, Boolean isQaTester) { + this(null, variantKey, variantValue, experimentId, isExperimentActive, isQaTester); + } + + /** + * Creates a new SelectedVariant with the originating flag key and experimentation metadata. + * + * @param flagKey the key of the flag this variant was selected for (may be null for fallback) + * @param variantKey the key of the selected variant (may be null for fallback) + * @param variantValue the value of the selected variant (may be null for fallback) + * @param experimentId the experiment ID (may be null) + * @param isExperimentActive whether the experiment is active (may be null) + * @param isQaTester whether the user is a QA tester (may be null) + */ + public SelectedVariant(String flagKey, String variantKey, T variantValue, UUID experimentId, Boolean isExperimentActive, Boolean isQaTester) { + this.flagKey = flagKey; this.variantKey = variantKey; this.variantValue = variantValue; this.experimentId = experimentId; @@ -48,6 +71,14 @@ public SelectedVariant(String variantKey, T variantValue, UUID experimentId, Boo this.isQaTester = isQaTester; } + /** + * @return the flag key this variant was selected for, or null if not set (e.g., for fallbacks + * or variants returned by code paths that don't propagate the flag key) + */ + public String getFlagKey() { + return flagKey; + } + /** * @return the variant key, or null if this is a fallback */ @@ -83,6 +114,20 @@ public Boolean getIsQaTester() { return isQaTester; } + /** + * Returns a SelectedVariant with the given flag key, copying all other fields from this instance. + * Returns this instance unchanged if the flag key already matches. + * + * @param flagKey the flag key to associate with the variant (may be null) + * @return a SelectedVariant with the requested flag key + */ + public SelectedVariant withFlagKey(String flagKey) { + if (flagKey == null ? this.flagKey == null : flagKey.equals(this.flagKey)) { + return this; + } + return new SelectedVariant<>(flagKey, variantKey, variantValue, experimentId, isExperimentActive, isQaTester); + } + /** * @return true if this represents a successfully selected variant (not a fallback) */ @@ -100,7 +145,8 @@ public boolean isFallback() { @Override public String toString() { return "SelectedVariant{" + - "variantKey='" + variantKey + '\'' + + "flagKey='" + flagKey + '\'' + + ", variantKey='" + variantKey + '\'' + ", variantValue=" + variantValue + ", experimentId=" + experimentId + ", isExperimentActive=" + isExperimentActive + @@ -115,6 +161,7 @@ public boolean equals(Object o) { SelectedVariant that = (SelectedVariant) o; + if (flagKey != null ? !flagKey.equals(that.flagKey) : that.flagKey != null) return false; if (variantKey != null ? !variantKey.equals(that.variantKey) : that.variantKey != null) return false; if (variantValue != null ? !variantValue.equals(that.variantValue) : that.variantValue != null) return false; if (experimentId != null ? !experimentId.equals(that.experimentId) : that.experimentId != null) return false; diff --git a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/BaseFlagsProvider.java b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/BaseFlagsProvider.java index 9e3f4ed..7b6ad78 100644 --- a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/BaseFlagsProvider.java +++ b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/BaseFlagsProvider.java @@ -164,7 +164,7 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall * @return the selected variant value or fallback */ public T getVariantValue(String flagKey, T fallbackValue, Map context) { - SelectedVariant fallback = new SelectedVariant<>(fallbackValue); + SelectedVariant fallback = new SelectedVariant<>(flagKey, null, fallbackValue, null, null, null); SelectedVariant result = getVariant(flagKey, fallback, context, true); return result.getVariantValue(); } @@ -181,7 +181,8 @@ public T getVariantValue(String flagKey, T fallbackValue, Map context) { - SelectedVariant result = getVariant(flagKey, new SelectedVariant<>(false), context, true); + SelectedVariant fallback = new SelectedVariant<>(flagKey, null, false, null, null, null); + SelectedVariant result = getVariant(flagKey, fallback, context, true); Object value = result.getVariantValue(); return value instanceof Boolean && (Boolean) value; diff --git a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java index 6176b85..316ff00 100644 --- a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java +++ b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java @@ -338,7 +338,7 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall if (flag == null) { logger.log(Level.WARNING, "Flag not found: " + flagKey); - return fallback; + return fallback.withFlagKey(flagKey); } // Extract context value @@ -346,7 +346,7 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall Object contextValueObj = context.get(contextProperty); if (contextValueObj == null) { logger.log(Level.WARNING, "Variant assignment key property '" + contextProperty + "' not found for flag: " + flagKey); - return fallback; + return fallback.withFlagKey(flagKey); } String contextValue = contextValueObj.toString(); @@ -403,11 +403,11 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall } // No rollout matched - return fallback; + return fallback.withFlagKey(flagKey); } catch (Exception e) { logger.log(Level.WARNING, "Error evaluating flag: " + flagKey, e); - return fallback; + return fallback.withFlagKey(flagKey); } } @@ -419,6 +419,7 @@ private SelectedVariant buildResult(Variant variant, ExperimentationFlag String flagKey, Map context, long startTime, boolean reportExposure) { SelectedVariant result = new SelectedVariant<>( + flagKey, variant.getKey(), (T) variant.getValue(), flag.getExperimentId(), @@ -625,7 +626,7 @@ public List> getAllVariants(Map context, Map definitions = flagDefinitions.get(); for (ExperimentationFlag flag : definitions.values()) { - SelectedVariant fallback = new SelectedVariant<>(null); + SelectedVariant fallback = new SelectedVariant<>(flag.getKey(), null, null, null, null, null); SelectedVariant result = getVariant(flag.getKey(), fallback, context, reportExposure); // Only include successfully selected variants (not fallbacks) diff --git a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProvider.java b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProvider.java index 4d8fd90..ae52068 100644 --- a/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProvider.java +++ b/src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProvider.java @@ -65,7 +65,7 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall if (flags == null || !flags.has(flagKey)) { logger.log(Level.WARNING, "Flag not found in response: " + flagKey); - return fallback; + return fallback.withFlagKey(flagKey); } JSONObject flagData = flags.getJSONObject(flagKey); @@ -73,7 +73,7 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall Object variantValue = flagData.opt("variant_value"); if (variantKey == null) { - return fallback; + return fallback.withFlagKey(flagKey); } // Parse experiment metadata @@ -104,12 +104,12 @@ public SelectedVariant getVariant(String flagKey, SelectedVariant fall } @SuppressWarnings("unchecked") - SelectedVariant result = new SelectedVariant<>(variantKey, (T) variantValue, experimentId, isExperimentActive, isQaTester); + SelectedVariant result = new SelectedVariant<>(flagKey, variantKey, (T) variantValue, experimentId, isExperimentActive, isQaTester); return result; } catch (Exception e) { logger.log(Level.WARNING, "Error evaluating flag remotely: " + flagKey, e); - return fallback; + return fallback.withFlagKey(flagKey); } } diff --git a/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java b/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java index 3e3e3af..3b3178e 100644 --- a/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java +++ b/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java @@ -403,6 +403,57 @@ public void testReturnFallbackWhenRequestedFlagDoesNotExist() { assertEquals(0, eventSender.getEvents().size()); } + @Test + public void testReturnedFallbackHasFlagKeyPopulated() { + List variants = Arrays.asList(new Variant("control", "blue", false, 1.0f)); + List rollouts = Arrays.asList(new Rollout(1.0f)); + String response = buildFlagsResponse("existing-flag", "distinct_id", variants, rollouts, null); + + provider = createProviderWithResponse(response); + provider.startPollingForDefinitions(); + + Map context = buildContext("user-123"); + SelectedVariant fallback = new SelectedVariant<>("fallback"); + SelectedVariant result = provider.getVariant("missing-flag", fallback, context, false); + + assertTrue(result.isFallback()); + assertEquals("fallback", result.getVariantValue()); + assertEquals("missing-flag", result.getFlagKey()); + } + + @Test + public void testReturnedFallbackHasFlagKeyOnMissingContextProperty() { + List variants = Arrays.asList(new Variant("variant-a", "value-a", false, 1.0f)); + List rollouts = Arrays.asList(new Rollout(1.0f)); + String response = buildFlagsResponse("test-flag", "distinct_id", variants, rollouts, null); + + provider = createProviderWithResponse(response); + provider.startPollingForDefinitions(); + + SelectedVariant fallback = new SelectedVariant<>("fallback"); + SelectedVariant result = provider.getVariant("test-flag", fallback, new HashMap<>(), false); + + assertTrue(result.isFallback()); + assertEquals("test-flag", result.getFlagKey()); + } + + @Test + public void testReturnedFallbackHasFlagKeyOnNoRolloutMatched() { + List variants = Arrays.asList(new Variant("variant-a", "value-a", false, 1.0f)); + List rollouts = Arrays.asList(new Rollout(0.0f)); + String response = buildFlagsResponse("test-flag", "distinct_id", variants, rollouts, null); + + provider = createProviderWithResponse(response); + provider.startPollingForDefinitions(); + + Map context = buildContext("user-123"); + SelectedVariant fallback = new SelectedVariant<>("fallback"); + SelectedVariant result = provider.getVariant("test-flag", fallback, context, false); + + assertTrue(result.isFallback()); + assertEquals("test-flag", result.getFlagKey()); + } + @Test public void testReturnFallbackWhenNoContextProvided() { List variants = Arrays.asList(new Variant("variant-a", "value-a", false, 1.0f)); @@ -1032,6 +1083,31 @@ public void testGetAllVariantsReturnsAllSuccessfullySelectedVariants() { } } + @Test + public void testGetAllVariantsPopulatesFlagKeyOnEachResult() { + List flags = Arrays.asList( + new FlagDefinition("flag-1", distinctIdContextKey, + Arrays.asList(new Variant("variant-a", "value-a", false, 1.0f)), + Arrays.asList(new Rollout(1.0f))), + new FlagDefinition("flag-2", distinctIdContextKey, + Arrays.asList(new Variant("variant-b", "value-b", false, 1.0f)), + Arrays.asList(new Rollout(1.0f))) + ); + + String response = buildMultipleFlagsResponse(flags); + provider = createProviderWithResponse(response); + provider.startPollingForDefinitions(); + + Map context = buildContext("user-123"); + List> results = provider.getAllVariants(context, false); + + Set flagKeys = new HashSet<>(); + for (SelectedVariant variant : results) { + flagKeys.add(variant.getFlagKey()); + } + assertEquals(new HashSet<>(Arrays.asList("flag-1", "flag-2")), flagKeys); + } + @Test public void testGetAllVariantsReturnsEmptyListWhenNoFlagsDefined() { String response = "{\"flags\":[]}"; diff --git a/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProviderTest.java b/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProviderTest.java index a904b75..4b76dc6 100644 --- a/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProviderTest.java +++ b/src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/RemoteFlagsProviderTest.java @@ -178,6 +178,21 @@ public void testReturnFallbackWhenFlagNotFoundInSuccessfulResponse() { assertEquals(0, eventSender.getEvents().size()); } + @Test + public void testReturnedFallbackHasFlagKeyPopulated() { + // Response contains a different flag, so the requested flag triggers the miss path + String response = buildRemoteFlagsResponse("other-flag", "variant-a", "value-a"); + provider = createProviderWithResponse(response); + + Map context = buildContext("user-123"); + SelectedVariant fallback = new SelectedVariant<>("fallback"); + SelectedVariant result = provider.getVariant("missing-flag", fallback, context, false); + + assertTrue(result.isFallback()); + assertEquals("fallback", result.getVariantValue()); + assertEquals("missing-flag", result.getFlagKey()); + } + // #endregion // #region Successful Variant Retrieval Tests