Skip to content

Add MatchRegistry for custom match implementations. #390

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.

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 15 commits into from
Sep 15, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.optimizely.ab.config.ProjectConfig;
import com.optimizely.ab.config.audience.match.Match;
import com.optimizely.ab.config.audience.match.MatchType;
import com.optimizely.ab.config.audience.match.UnexpectedValueTypeException;
import com.optimizely.ab.config.audience.match.UnknownMatchTypeException;
import com.optimizely.ab.config.audience.match.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -87,35 +84,36 @@ public Boolean evaluate(ProjectConfig config, Map<String, ?> attributes) {
}
// check user attribute value is equal
try {
Match matchType = MatchType.getMatchType(match, value).getMatcher();
Boolean result = matchType.eval(userAttributeValue);

Match matcher = MatchRegistry.getMatch(match);
Boolean result = matcher.eval(value, userAttributeValue);
if (result == null) {
if (!attributes.containsKey(name)) {
//Missing attribute value
logger.debug("Audience condition \"{}\" evaluated to UNKNOWN because no value was passed for user attribute \"{}\"", this, name);
throw new UnknownValueTypeException();
}

return result;
} catch(UnknownValueTypeException e) {
if (!attributes.containsKey(name)) {
//Missing attribute value
logger.debug("Audience condition \"{}\" evaluated to UNKNOWN because no value was passed for user attribute \"{}\"", this, name);
} else {
//if attribute value is not valid
if (userAttributeValue != null) {
logger.warn(
"Audience condition \"{}\" evaluated to UNKNOWN because a value of type \"{}\" was passed for user attribute \"{}\"",
this,
userAttributeValue.getClass().getCanonicalName(),
name);
} else {
//if attribute value is not valid
if (userAttributeValue != null) {
logger.warn(
"Audience condition \"{}\" evaluated to UNKNOWN because a value of type \"{}\" was passed for user attribute \"{}\"",
this,
userAttributeValue.getClass().getCanonicalName(),
name);
} else {
logger.debug(
"Audience condition \"{}\" evaluated to UNKNOWN because a null value was passed for user attribute \"{}\"",
this,
name);
}
logger.debug(
"Audience condition \"{}\" evaluated to UNKNOWN because a null value was passed for user attribute \"{}\"",
this,
name);
}
}
return result;
} catch (UnknownMatchTypeException | UnexpectedValueTypeException ex) {
logger.warn("Audience condition \"{}\" " + ex.getMessage(),
this);
} catch (NullPointerException np) {
logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", np);
} catch (UnknownMatchTypeException | UnexpectedValueTypeException e) {
logger.warn("Audience condition \"{}\" " + e.getMessage(), this);
} catch (NullPointerException e) {
logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", e);
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,17 +21,16 @@
/**
* This is a temporary class. It mimics the current behaviour for
* legacy custom attributes. This will be dropped for ExactMatch and the unit tests need to be fixed.
* @param <T>
*/
class DefaultMatchForLegacyAttributes<T> extends AttributeMatch<T> {
T value;

protected DefaultMatchForLegacyAttributes(T value) {
this.value = value;
}

class DefaultMatchForLegacyAttributes implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
return value.equals(castToValueType(attributeValue, value));
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
if (!(conditionValue instanceof String)) {
throw new UnexpectedValueTypeException();
}
if (attributeValue == null) {
return false;
}
return conditionValue.toString().equals(attributeValue.toString());
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,18 +18,31 @@

import javax.annotation.Nullable;

class ExactMatch<T> extends AttributeMatch<T> {
T value;

protected ExactMatch(T value) {
this.value = value;
}
import static com.optimizely.ab.internal.AttributesUtil.isValidNumber;

/**
* ExactMatch supports matching Numbers, Strings and Booleans. Numbers are first converted to doubles
* before the comparison is evaluated. See {@link NumberComparator} Strings and Booleans are evaulated
* via the Object equals method.
*/
class ExactMatch implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
T converted = castToValueType(attributeValue, value);
if (value != null && converted == null) return null;
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
if (isValidNumber(attributeValue)) {
if (isValidNumber(conditionValue)) {
return NumberComparator.compareUnsafe(attributeValue, conditionValue) == 0;
}
return null;
}

if (!(conditionValue instanceof String || conditionValue instanceof Boolean)) {
throw new UnexpectedValueTypeException();
}

if (attributeValue == null || attributeValue.getClass() != conditionValue.getClass()) {
return null;
}

return value == null ? attributeValue == null : value.equals(converted);
return conditionValue.equals(attributeValue);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,20 +16,14 @@
*/
package com.optimizely.ab.config.audience.match;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

import javax.annotation.Nullable;

/**
* ExistsMatch checks that the attribute value is NOT null.
*/
class ExistsMatch implements Match {
@SuppressFBWarnings("URF_UNREAD_FIELD")
Object value;

protected ExistsMatch(Object value) {
this.value = value;
}

@Nullable
public Boolean eval(Object attributeValue) {
public Boolean eval(Object conditionValue, Object attributeValue) {
return attributeValue != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,12 @@

import javax.annotation.Nullable;

import static com.optimizely.ab.internal.AttributesUtil.isValidNumber;

class GEMatch extends AttributeMatch<Number> {
Number value;

protected GEMatch(Number value) {
this.value = value;
}

/**
* GEMatch performs a "greater than or equal to" number comparison via {@link NumberComparator}.
*/
class GEMatch implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
try {
if(isValidNumber(attributeValue)) {
return castToValueType(attributeValue, value).doubleValue() >= value.doubleValue();
}
} catch (Exception e) {
return null;
}
return null;
public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException {
return NumberComparator.compare(attributeValue, conditionValue) >= 0;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,24 +18,12 @@

import javax.annotation.Nullable;

import static com.optimizely.ab.internal.AttributesUtil.isValidNumber;

class GTMatch extends AttributeMatch<Number> {
Number value;

protected GTMatch(Number value) {
this.value = value;
}

/**
* GTMatch performs a "greater than" number comparison via {@link NumberComparator}.
*/
class GTMatch implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
try {
if(isValidNumber(attributeValue)) {
return castToValueType(attributeValue, value).doubleValue() > value.doubleValue();
}
} catch (Exception e) {
return null;
}
return null;
public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException {
return NumberComparator.compare(attributeValue, conditionValue) > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,13 @@

import javax.annotation.Nullable;

import static com.optimizely.ab.internal.AttributesUtil.isValidNumber;

class LEMatch extends AttributeMatch<Number> {
Number value;

protected LEMatch(Number value) {
this.value = value;
}

/**
* GEMatch performs a "less than or equal to" number comparison via {@link NumberComparator}.
*/
class LEMatch implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
try {
if(isValidNumber(attributeValue)) {
return castToValueType(attributeValue, value).doubleValue() <= value.doubleValue();
}
} catch (Exception e) {
return null;
}
return null;
public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException {
return NumberComparator.compare(attributeValue, conditionValue) <= 0;
}
}

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,25 +18,12 @@

import javax.annotation.Nullable;

import static com.optimizely.ab.internal.AttributesUtil.isValidNumber;

class LTMatch extends AttributeMatch<Number> {
Number value;

protected LTMatch(Number value) {
this.value = value;
}

/**
* GTMatch performs a "less than" number comparison via {@link NumberComparator}.
*/
class LTMatch implements Match {
@Nullable
public Boolean eval(Object attributeValue) {
try {
if(isValidNumber(attributeValue)) {
return castToValueType(attributeValue, value).doubleValue() < value.doubleValue();
}
} catch (Exception e) {
return null;
}
return null;
public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException {
return NumberComparator.compare(attributeValue, conditionValue) < 0;
}
}

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2018-2019, Optimizely and contributors
* Copyright 2018-2020, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,5 +20,5 @@

public interface Match {
@Nullable
Boolean eval(Object attributeValue);
Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException, UnknownValueTypeException;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need separate exception types for UnexpectedValueTypeException and UnknownValueTypeException?
They're similar and not much gain for additional complexity to differentiate them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for backwards compatibility, albeit I can update the names perhaps. The difference is in the logging behavior. One error is communicated as an SDK compatibility issue warning to upgrade on the condition value, while the other is saying that the attribute value type is unknown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The distinction is since the conditionValue comes from the datafile, so if a value is of an "unexpected" type then its likely the result of an out of date SDK. Since the attributeValue is provided at runtime via the user attributes, then we log that the type is "unknown".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. We can consider adding some clarifications.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added doc strings for all the exception classes in this package.

}
Loading