From 7efd98ceced7e6e2882482b118b11037441c7403 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Mon, 10 Aug 2026 18:42:32 -0700 Subject: [PATCH 1/2] refactor: read the flag once per evaluation, in one place The ten variation methods each spelled out the same call to the hook runner around the same evaluation, and left the flag read to the evaluation itself. They now collapse onto a helper that does the read, so one place expresses the order of the read, the hooks and the evaluation, and what an evaluation is about to return can be described without reading the store a second time. Co-authored-by: Cursor --- .../launchdarkly/sdk/android/LDClient.java | 106 ++++++------------ 1 file changed, 36 insertions(+), 70 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index c74e75cb..aeb68058 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -3,6 +3,7 @@ import android.app.Application; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.launchdarkly.logging.LDLogger; @@ -553,123 +554,87 @@ public Map allFlags() { @Override public boolean boolVariation(@NonNull String key, boolean defaultValue) { - return hookRunner.withEvaluation( - "LDClient.boolVariation", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, false) - ).getValue().booleanValue(); + return evaluateWithHooks("LDClient.boolVariation", key, LDValue.of(defaultValue), true, false) + .getValue().booleanValue(); } @Override public EvaluationDetail boolVariationDetail(@NonNull String key, boolean defaultValue) { return convertDetailType( - hookRunner.withEvaluation( - "LDClient.boolVariationDetail", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, true) - ), + evaluateWithHooks("LDClient.boolVariationDetail", key, LDValue.of(defaultValue), true, true), LDValue.Convert.Boolean ); } @Override public int intVariation(@NonNull String key, int defaultValue) { - return hookRunner.withEvaluation( - "LDClient.intVariation", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, false) - ).getValue().intValue(); + return evaluateWithHooks("LDClient.intVariation", key, LDValue.of(defaultValue), true, false) + .getValue().intValue(); } @Override public EvaluationDetail intVariationDetail(@NonNull String key, int defaultValue) { return convertDetailType( - hookRunner.withEvaluation( - "LDClient.intVariationDetail", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, true) - ), + evaluateWithHooks("LDClient.intVariationDetail", key, LDValue.of(defaultValue), true, true), LDValue.Convert.Integer ); } @Override public double doubleVariation(@NonNull String key, double defaultValue) { - return hookRunner.withEvaluation( - "LDClient.doubleVariation", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, false) - ).getValue().doubleValue(); + return evaluateWithHooks("LDClient.doubleVariation", key, LDValue.of(defaultValue), true, false) + .getValue().doubleValue(); } @Override public EvaluationDetail doubleVariationDetail(@NonNull String key, double defaultValue) { return convertDetailType( - hookRunner.withEvaluation( - "LDClient.doubleVariationDetail", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, true) - ), + evaluateWithHooks("LDClient.doubleVariationDetail", key, LDValue.of(defaultValue), true, true), LDValue.Convert.Double ); } @Override public String stringVariation(@NonNull String key, String defaultValue) { - return hookRunner.withEvaluation( - "LDClient.stringVariation", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, false) - ).getValue().stringValue(); + return evaluateWithHooks("LDClient.stringVariation", key, LDValue.of(defaultValue), true, false) + .getValue().stringValue(); } @Override public EvaluationDetail stringVariationDetail(@NonNull String key, String defaultValue) { return convertDetailType( - hookRunner.withEvaluation( - "LDClient.stringVariationDetail", - key, - clientContextImpl.getEvaluationContext(), - LDValue.of(defaultValue), - () -> variationDetailInternal(key, LDValue.of(defaultValue), true, true) - ), + evaluateWithHooks("LDClient.stringVariationDetail", key, LDValue.of(defaultValue), true, true), LDValue.Convert.String ); } @Override public LDValue jsonValueVariation(@NonNull String key, LDValue defaultValue) { - return hookRunner.withEvaluation( - "LDClient.jsonValueVariation", - key, - clientContextImpl.getEvaluationContext(), - LDValue.normalize(defaultValue), - () -> variationDetailInternal(key, LDValue.normalize(defaultValue), false, false) - ).getValue(); + return evaluateWithHooks("LDClient.jsonValueVariation", key, LDValue.normalize(defaultValue), false, false) + .getValue(); } @Override public EvaluationDetail jsonValueVariationDetail(@NonNull String key, LDValue defaultValue) { + return evaluateWithHooks("LDClient.jsonValueVariationDetail", key, LDValue.normalize(defaultValue), false, true); + } + + /** + * Runs an evaluation, and the hooks around it, against one read of the flag. + *

+ * The read is done here rather than left to the evaluation so that there is one place expressing + * the order of the read, the hooks and the evaluation, and so that what an evaluation is about to + * return can be described to a hook without reading the store a second time. + */ + private EvaluationDetail evaluateWithHooks(String method, String key, LDValue defaultValue, + boolean checkType, boolean needsReason) { + Flag flag = contextDataManager.getNonDeletedFlag(key); // returns null for nonexistent *or* deleted flag return hookRunner.withEvaluation( - "LDClient.jsonValueVariationDetail", + method, key, clientContextImpl.getEvaluationContext(), - LDValue.normalize(defaultValue), - () -> variationDetailInternal(key, LDValue.normalize(defaultValue), false, true) + defaultValue, + () -> variationDetailInternal(key, defaultValue, checkType, needsReason, null, flag) ); } @@ -677,13 +642,14 @@ private EvaluationDetail convertDetailType(EvaluationDetail deta return EvaluationDetail.fromValue(converter.toType(detail.getValue()), detail.getVariationIndex(), detail.getReason()); } - private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason) { - return variationDetailInternal(key, defaultValue, checkType, needsReason, null); + private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited) { + // returns null for nonexistent *or* deleted flag + return variationDetailInternal(key, defaultValue, checkType, needsReason, visited, + contextDataManager.getNonDeletedFlag(key)); } - private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited) { + private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited, @Nullable Flag flag) { LDContext context = clientContextImpl.getEvaluationContext(); - Flag flag = contextDataManager.getNonDeletedFlag(key); // returns null for nonexistent *or* deleted flag EvaluationDetail result; if (flag == null) { From 6a7de483a9214907ee0a49608d6eef3ed4b3b2c8 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Mon, 10 Aug 2026 20:09:41 -0700 Subject: [PATCH 2/2] fix: attribute an evaluation to the context it read the flag for evaluateWithHooks snapshotted the flag before hooks ran, but variationDetailInternal still re-read the evaluation context when recording events. An identify landing in between left the returned value from the prior context's flag attributed to the new context. Both are now read together and threaded through the evaluation so the series, the result and the events all describe one pair. Co-authored-by: Cursor --- .../launchdarkly/sdk/android/LDClient.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index aeb68058..ea097d60 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -620,21 +620,24 @@ public EvaluationDetail jsonValueVariationDetail(@NonNull String key, L } /** - * Runs an evaluation, and the hooks around it, against one read of the flag. + * Runs an evaluation, and the hooks around it, against one read of the flag and of the evaluation + * context. *

- * The read is done here rather than left to the evaluation so that there is one place expressing - * the order of the read, the hooks and the evaluation, and so that what an evaluation is about to - * return can be described to a hook without reading the store a second time. + * Both are read here rather than left to the evaluation so that there is one place expressing the + * order of the reads, the hooks and the evaluation. Reading only the flag here and letting the + * evaluation re-read the context would leave an identify landing in between attributing the prior + * context's flag to the new context in events. */ private EvaluationDetail evaluateWithHooks(String method, String key, LDValue defaultValue, boolean checkType, boolean needsReason) { + LDContext context = clientContextImpl.getEvaluationContext(); Flag flag = contextDataManager.getNonDeletedFlag(key); // returns null for nonexistent *or* deleted flag return hookRunner.withEvaluation( method, key, - clientContextImpl.getEvaluationContext(), + context, defaultValue, - () -> variationDetailInternal(key, defaultValue, checkType, needsReason, null, flag) + () -> variationDetailInternal(key, defaultValue, checkType, needsReason, null, flag, context) ); } @@ -642,14 +645,7 @@ private EvaluationDetail convertDetailType(EvaluationDetail deta return EvaluationDetail.fromValue(converter.toType(detail.getValue()), detail.getVariationIndex(), detail.getReason()); } - private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited) { - // returns null for nonexistent *or* deleted flag - return variationDetailInternal(key, defaultValue, checkType, needsReason, visited, - contextDataManager.getNonDeletedFlag(key)); - } - - private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited, @Nullable Flag flag) { - LDContext context = clientContextImpl.getEvaluationContext(); + private EvaluationDetail variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set visited, @Nullable Flag flag, @NonNull LDContext context) { EvaluationDetail result; if (flag == null) { @@ -677,7 +673,9 @@ private EvaluationDetail variationDetailInternal(@NonNull String key, @ // value and reason (below) are unchanged. continue; } - variationDetailInternal(prereqKey, LDValue.ofNull(), false, false, visited); + // The prerequisite is evaluated as part of the same call, so it is attributed to the same context. + variationDetailInternal(prereqKey, LDValue.ofNull(), false, false, visited, + contextDataManager.getNonDeletedFlag(prereqKey), context); } } finally { visited.remove(key);