Skip to content

refactor: read the flag once per evaluation, in one place #381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abelonogov-ld wants to merge 2 commits into
base: main
Choose a base branch
from
+39 −75
Open

refactor: read the flag once per evaluation, in one place #381

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7efd98c
refactor: read the flag once per evaluation, in one place
abelonogov-ld Aug 11, 2026
6a7de48
fix: attribute an evaluation to the context it read the flag for
abelonogov-ld Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 39 additions & 75 deletions launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -553,137 +554,98 @@ public Map<String, LDValue> 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<Boolean> 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<Integer> 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<Double> 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<String> 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<LDValue> 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 and of the evaluation
* context.
* <p>
* 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<LDValue> 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(
"LDClient.jsonValueVariationDetail",
method,
key,
clientContextImpl.getEvaluationContext(),
LDValue.normalize(defaultValue),
() -> variationDetailInternal(key, LDValue.normalize(defaultValue), false, true)
context,
defaultValue,
() -> variationDetailInternal(key, defaultValue, checkType, needsReason, null, flag, context)
);
}

private <T> EvaluationDetail<T> convertDetailType(EvaluationDetail<LDValue> detail, LDValue.Converter<T> converter) {
return EvaluationDetail.fromValue(converter.toType(detail.getValue()), detail.getVariationIndex(), detail.getReason());
}

private EvaluationDetail<LDValue> variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason) {
return variationDetailInternal(key, defaultValue, checkType, needsReason, null);
}

private EvaluationDetail<LDValue> variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set<String> visited) {
LDContext context = clientContextImpl.getEvaluationContext();
Flag flag = contextDataManager.getNonDeletedFlag(key); // returns null for nonexistent *or* deleted flag
private EvaluationDetail<LDValue> variationDetailInternal(@NonNull String key, @NonNull LDValue defaultValue, boolean checkType, boolean needsReason, Set<String> visited, @Nullable Flag flag, @NonNull LDContext context) {
EvaluationDetail<LDValue> result;

if (flag == null) {
Expand Down Expand Up @@ -711,7 +673,9 @@ private EvaluationDetail<LDValue> 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);
Expand Down
Loading