Skip to content

fix: change FeatureVariable type to string for forward compatibility #370

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

Merged
merged 2 commits into from
Apr 22, 2020
Merged

fix: change FeatureVariable type to string for forward compatibility #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6be691f
change FeatureVariable type from enum to string for forward compatibi…
jaeopt Apr 15, 2020
a336d5d
fix switch default error
jaeopt Apr 15, 2020
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
22 changes: 12 additions & 10 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey,
variableKey,
userId,
attributes,
FeatureVariable.VariableType.BOOLEAN
FeatureVariable.BOOLEAN_TYPE
);
}

Expand Down Expand Up @@ -501,7 +501,7 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey,
variableKey,
userId,
attributes,
FeatureVariable.VariableType.DOUBLE
FeatureVariable.DOUBLE_TYPE
);
} catch (Exception exception) {
logger.error("NumberFormatException while trying to parse \"" + variableValue +
Expand Down Expand Up @@ -551,7 +551,7 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey,
variableKey,
userId,
attributes,
FeatureVariable.VariableType.INTEGER
FeatureVariable.INTEGER_TYPE
);

} catch (Exception exception) {
Expand Down Expand Up @@ -598,15 +598,15 @@ public String getFeatureVariableString(@Nonnull String featureKey,
variableKey,
userId,
attributes,
FeatureVariable.VariableType.STRING);
FeatureVariable.STRING_TYPE);
}

@VisibleForTesting
<T> T getFeatureVariableValueForType(@Nonnull String featureKey,
@Nonnull String variableKey,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes,
@Nonnull FeatureVariable.VariableType variableType) {
@Nonnull String variableType) {
if (featureKey == null) {
logger.warn("The featureKey parameter must be nonnull.");
return null;
Expand Down Expand Up @@ -691,29 +691,31 @@ <T> T getFeatureVariableValueForType(@Nonnull String featureKey,

// Helper method which takes type and variable value and convert it to object to use in Listener DecisionInfo object variable value
@VisibleForTesting
Object convertStringToType(String variableValue, FeatureVariable.VariableType type) {
Object convertStringToType(String variableValue, String type) {
if (variableValue != null) {
switch (type) {
case DOUBLE:
case FeatureVariable.DOUBLE_TYPE:
try {
return Double.parseDouble(variableValue);
} catch (NumberFormatException exception) {
logger.error("NumberFormatException while trying to parse \"" + variableValue +
"\" as Double. " + exception);
}
break;
case STRING:
case FeatureVariable.STRING_TYPE:
return variableValue;
case BOOLEAN:
case FeatureVariable.BOOLEAN_TYPE:
return Boolean.parseBoolean(variableValue);
case INTEGER:
case FeatureVariable.INTEGER_TYPE:
try {
return Integer.parseInt(variableValue);
} catch (NumberFormatException exception) {
logger.error("NumberFormatException while trying to parse \"" + variableValue +
"\" as Integer. " + exception.toString());
}
break;
default:
return variableValue;
}
}

Expand Down
53 changes: 8 additions & 45 deletions core-api/src/main/java/com/optimizely/ab/config/FeatureVariable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,52 +61,15 @@ public static VariableStatus fromString(String variableStatusString) {
}
}

public enum VariableType {
@SerializedName("boolean")
BOOLEAN("boolean"),

@SerializedName("integer")
INTEGER("integer"),

@SerializedName("string")
STRING("string"),

@SerializedName("double")
DOUBLE("double");

private final String variableType;

VariableType(String variableType) {
this.variableType = variableType;
}

@JsonValue
public String getVariableType() {
return variableType;
}

public static VariableType fromString(String variableTypeString) {
if (variableTypeString != null) {
for (VariableType variableTypeEnum : VariableType.values()) {
if (variableTypeString.equals(variableTypeEnum.getVariableType())) {
return variableTypeEnum;
}
}
}

return null;
}

@Override
public String toString() {
return variableType;
}
}
public static final String STRING_TYPE = "string";
public static final String INTEGER_TYPE = "integer";
public static final String DOUBLE_TYPE = "double";
public static final String BOOLEAN_TYPE = "boolean";

private final String id;
private final String key;
private final String defaultValue;
private final VariableType type;
private final String type;
@Nullable
private final VariableStatus status;

Expand All @@ -115,7 +78,7 @@ public FeatureVariable(@JsonProperty("id") String id,
@JsonProperty("key") String key,
@JsonProperty("defaultValue") String defaultValue,
@JsonProperty("status") VariableStatus status,
@JsonProperty("type") VariableType type) {
@JsonProperty("type") String type) {
this.id = id;
this.key = key;
this.defaultValue = defaultValue;
Expand All @@ -140,7 +103,7 @@ public String getDefaultValue() {
return defaultValue;
}

public VariableType getType() {
public String getType() {
return type;
}

Expand All @@ -165,7 +128,7 @@ public boolean equals(Object o) {
if (!id.equals(variable.id)) return false;
if (!key.equals(variable.key)) return false;
if (!defaultValue.equals(variable.defaultValue)) return false;
if (type != variable.type) return false;
if (!type.equals(variable.type)) return false;
return status == variable.status;
}

Expand Down
2 changes: 1 addition & 1 deletion core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ private List<FeatureVariable> parseFeatureVariables(JSONArray featureVariablesJs
String id = FeatureVariableObject.getString("id");
String key = FeatureVariableObject.getString("key");
String defaultValue = FeatureVariableObject.getString("defaultValue");
FeatureVariable.VariableType type = FeatureVariable.VariableType.fromString(FeatureVariableObject.getString("type"));
String type = FeatureVariableObject.getString("type");
FeatureVariable.VariableStatus status = null;
if (FeatureVariableObject.has("status")) {
status = FeatureVariable.VariableStatus.fromString(FeatureVariableObject.getString("status"));
Expand Down
3 changes: 1 addition & 2 deletions core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.optimizely.ab.config.*;
import com.optimizely.ab.config.Experiment.ExperimentStatus;
import com.optimizely.ab.config.FeatureVariable.VariableStatus;
import com.optimizely.ab.config.FeatureVariable.VariableType;
import com.optimizely.ab.config.audience.Audience;
import com.optimizely.ab.config.audience.AudienceIdCondition;
import com.optimizely.ab.config.audience.Condition;
Expand Down Expand Up @@ -335,7 +334,7 @@ private List<FeatureVariable> parseFeatureVariables(JSONArray featureVariablesJs
String id = (String) featureVariableObject.get("id");
String key = (String) featureVariableObject.get("key");
String defaultValue = (String) featureVariableObject.get("defaultValue");
VariableType type = VariableType.fromString((String) featureVariableObject.get("type"));
String type = (String) featureVariableObject.get("type");
VariableStatus status = VariableStatus.fromString((String) featureVariableObject.get("status"));

featureVariables.add(new FeatureVariable(id, key, defaultValue, status, type));
Expand Down
4 changes: 2 additions & 2 deletions core-api/src/main/java/com/optimizely/ab/notification/DecisionNotification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public static class FeatureVariableDecisionNotificationBuilder {
private Boolean featureEnabled;
private FeatureDecision featureDecision;
private String variableKey;
private FeatureVariable.VariableType variableType;
private String variableType;
private Object variableValue;
private String userId;
private Map<String, ?> attributes;
Expand Down Expand Up @@ -283,7 +283,7 @@ public FeatureVariableDecisionNotificationBuilder withVariableKey(String variabl
return this;
}

public FeatureVariableDecisionNotificationBuilder withVariableType(FeatureVariable.VariableType variableType) {
public FeatureVariableDecisionNotificationBuilder withVariableType(String variableType) {
this.variableType = variableType;
return this;
}
Expand Down
4 changes: 2 additions & 2 deletions core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ Map<String, OptimizelyVariable> getMergedVariablesMap(Variation variation, Strin
featureVariableKeyMap.put(featureVariable.getKey(), new OptimizelyVariable(
featureVariable.getId(),
featureVariable.getKey(),
featureVariable.getType().getVariableType().toLowerCase(),
featureVariable.getType(),
variation.getFeatureEnabled() && tempVariableIdMap.get(featureVariable.getId()) != null
? tempVariableIdMap.get(featureVariable.getId()).getValue()
: featureVariable.getDefaultValue()
Expand Down Expand Up @@ -205,7 +205,7 @@ Map<String, OptimizelyVariable> getFeatureVariablesMap(List<FeatureVariable> fea
featureVariableKeyMap.put(featureVariable.getKey(), new OptimizelyVariable(
featureVariable.getId(),
featureVariable.getKey(),
featureVariable.getType().getVariableType().toLowerCase(),
featureVariable.getType(),
featureVariable.getDefaultValue()
));
}
Expand Down
Loading