diff --git a/src/main/java/com/yahoo/bullet/common/Validator.java b/src/main/java/com/yahoo/bullet/common/Validator.java index b2ff7474..59b32804 100644 --- a/src/main/java/com/yahoo/bullet/common/Validator.java +++ b/src/main/java/com/yahoo/bullet/common/Validator.java @@ -20,6 +20,7 @@ import java.util.function.BiPredicate; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; /** * This class validates instances of {@link BulletConfig}. Use {@link Validator.Entry} to define @@ -32,12 +33,15 @@ public class Validator { private static final Predicate UNARY_IDENTITY = o -> true; private static final BiPredicate BINARY_IDENTITY = (oA, oB) -> true; + private static final Predicate> NARY_IDENTITY = o -> true; + private static final String COMMA = ", "; /** * This represents a field in the Validator. It applies a {@link Predicate} to the value of the field and uses a * default value (see {@link Entry#defaultTo(Object)} if the predicate fails. It can also apply an arbitrary * conversion using {@link Entry#castTo(Function)}. These are all applied when you call * {@link Entry#normalize(BulletConfig)} with a {@link BulletConfig} containing a field that matches the Entry. + * You can also ask that the check cause a failure using {@link #orFail()}. */ public static class Entry { private String key; @@ -45,11 +49,13 @@ public static class Entry { private Predicate guard; private Object defaultValue; private Function adapter; + private boolean fail; private Entry(String key) { this.validation = UNARY_IDENTITY; this.guard = UNARY_IDENTITY.negate(); this.key = key; + this.fail = false; } private Entry copy() { @@ -58,6 +64,7 @@ private Entry copy() { entry.defaultValue = defaultValue; entry.validation = validation; entry.guard = guard; + entry.fail = fail; return entry; } @@ -90,6 +97,16 @@ public Entry checkIf(Predicate validator) { return this; } + /** + * Fail if this entry fails to hold. + * + * @return This Entry for chaining. + */ + public Entry orFail() { + fail = true; + return this; + } + /** * Use a default value for the field represented by this Entry. This is used if the validation fails. Note that * the {@link Entry#castTo(Function)} will be applied to this if present. @@ -142,6 +159,10 @@ void normalize(BulletConfig config) { } boolean isValid = validation.test(value); if (!isValid) { + if (fail) { + log.error("Key: {} had an invalid value: {}. Erroring out as default not permitted.", key, value); + throw new IllegalStateException("Check cannot be satisfied or fixed for " + key); + } log.warn("Key: {} had an invalid value: {}. Using default: {}", key, value, defaultValue); value = defaultValue; } @@ -157,7 +178,8 @@ void normalize(BulletConfig config) { * This represents a binary relationship between two fields in a {@link BulletConfig}. You should have defined * {@link Entry} for these fields before you try to define relationships between them. You can use this to apply a * {@link BiPredicate} to these fields and provide or use their defined defaults (defined using - * {@link Entry#defaultTo(Object)}) if the check fails. + * {@link Entry#defaultTo(Object)}) if the check fails. You may also ask the relationship to fail with + * {@link #orFail()} if you do not want it to default.. */ public static class Relationship { private String keyA; @@ -166,6 +188,7 @@ public static class Relationship { private BiPredicate binaryRelation; private Object defaultA; private Object defaultB; + private boolean fail; private Relationship(String description, String keyA, String keyB, Map entries) { this.description = description; @@ -175,6 +198,7 @@ private Relationship(String description, String keyA, String keyB, Map entries) { @@ -182,6 +206,7 @@ private Relationship copy(Map entries) { relation.binaryRelation = binaryRelation; relation.defaultA = defaultA; relation.defaultB = defaultB; + relation.fail = fail; return relation; } @@ -209,6 +234,13 @@ public void orElseUse(Object objectA, Object objectB) { this.defaultB = objectB; } + /** + * Fail if this relationship fails to hold. + */ + public void orFail() { + fail = true; + } + /** * Normalize the given {@link BulletConfig} for the fields defined by this relationship. This applies the check * and uses the defaults (provided using {@link Relationship#orElseUse(Object, Object)} or the Entry defaults @@ -220,20 +252,101 @@ void normalize(BulletConfig config) { Object objectA = config.get(keyA); Object objectB = config.get(keyB); boolean isValid = binaryRelation.test(objectA, objectB); - if (!isValid) { - log.warn("{}: {} and {}: {} do not satisfy: {}. Using their defaults", keyA, objectA, keyB, objectB, description); - log.warn("Using default {} for {}", defaultA, keyA); - log.warn("Using default {} for {}", defaultB, keyB); - config.set(keyA, defaultA); - config.set(keyB, defaultB); + if (isValid) { + return; + } + if (fail) { + log.error("{}: {} and {}: {} do not satisfy: {}. Erroring out as using defaults was not permitted...", + keyA, objectA, keyB, objectB, description); + throw new IllegalStateException("Relationship cannot be satisfied or fixed: " + description); } + log.warn("{}: {} and {}: {} do not satisfy: {}. Using their defaults", keyA, objectA, keyB, objectB, description); + log.warn("Using default {} for {}", defaultA, keyA); + log.warn("Using default {} for {}", defaultB, keyB); + config.set(keyA, defaultA); + config.set(keyB, defaultB); } } + /** + * This represents a n-ary validation for the config. You can specify as many fields that need to participate in this + * validation. As with a {@link Relationship}, the {@link Entry} must have been defined for these fields already. + * You may provide a check for the values of these fields using {@link #checkIf(Predicate)}, which may be chained + * onto. If it fails and unless you ask the check to fail with {@link #orFail()}, the defaults from the entries + * defined for these fields will be used for all of them. + */ + public static class State { + private final String description; + private final List keys; + private Predicate> validation; + private boolean fail; + + private State(String description, List keys) { + this.description = description; + this.keys = keys; + this.validation = NARY_IDENTITY; + this.fail = false; + } + + private State copy() { + State state = new State(description, keys); + state.validation = validation; + state.fail = fail; + return state; + } + + /** + * Provide the {@link Predicate} that accepts the values that this state is checking. + * validations and they will be ANDed on the existing ones. + * + * @param validation A check for this relationship. + * @return This Relationship for chaining. + */ + public State checkIf(Predicate> validation) { + this.validation = this.validation.and(validation); + return this; + } + + /** + * Fail if this state check fails. + */ + public void orFail() { + fail = true; + } + + /* + * Normalize the given {@link BulletConfig} for the fields defined by this relationship. This applies the check + * and uses the defaults (provided using {@link Relationship#orElseUse(Object, Object)} or the Entry defaults + * for these fields if the check fails. + * + * @param config The config to validate. + * @param entries The {@link Map} of names to {@link Entry} that are relevant for this config. + */ + void normalize(BulletConfig config, Map entries) { + // Sequential stream so order is the same + List values = keys.stream().map(config::get).collect(Collectors.toList()); + boolean result = validation.test(values); + if (result) { + return; + } + log.warn("State validation: {} failed for values {}", description, values); + if (fail) { + log.error("Erroring out as using defaults was not permitted"); + throw new IllegalStateException("Unsupported values for " + values); + } + for (String key : keys) { + Object defaultValue = entries.get(key).getDefaultValue(); + log.warn("Using default value of {} for {}", defaultValue, key); + config.set(key, defaultValue); + } + } + } + + // For testing @Getter(AccessLevel.PACKAGE) private final Map entries; - @Getter(AccessLevel.PACKAGE) private final List relations; + private final List states; /** * Default constructor. @@ -241,13 +354,15 @@ void normalize(BulletConfig config) { public Validator() { entries = new HashMap<>(); relations = new ArrayList<>(); + states = new ArrayList<>(); } - private Validator(Map entries, List relations) { + private Validator(Map entries, List relations, List states) { // Copy constructor. this(); entries.forEach((name, entry) -> this.entries.put(name, entry.copy())); relations.forEach(relation -> this.relations.add(relation.copy(entries))); + states.forEach(state -> this.states.add(state.copy())); } /** @@ -283,6 +398,27 @@ public Relationship relate(String description, String keyA, String keyB) { return relation; } + /** + * Create a state with a description for the given fields. This lets you validate values for as many fields as you + * want. By default, the state check will hold true unless you provide one with {@link State#checkIf(Predicate)}, + * which provides you the values for the fields you are defining this for. Unless you ask the the check to fail with + * {@link State#orFail()}, it will use the defaults for each of the entries. + * + * @param description A string description for this state validation. + * @param keys The non-null fields for this state validation. They must already be defined as entries. + * @return The create {@link State}. + */ + public State evaluate(String description, String... keys) { + Objects.requireNonNull(keys, "You must provide the relevant keys for this state validation"); + List missingKeys = Arrays.stream(keys).filter(k -> entries.get(k) == null).collect(Collectors.toList()); + if (!missingKeys.isEmpty()) { + throw new NullPointerException("You must evaluate entries for " + missingKeys.stream().collect(Collectors.joining(COMMA))); + } + State state = new State(description, Arrays.asList(keys)); + states.add(state); + return state; + } + /** * Validate and normalize the provided {@link BulletConfig} for the defined entries and relationships. Then entries * are used to validate the config first. @@ -292,6 +428,7 @@ public Relationship relate(String description, String keyA, String keyB) { public void validate(BulletConfig config) { entries.values().forEach(e -> e.normalize(config)); relations.forEach(r -> r.normalize(config)); + states.forEach(s -> s.normalize(config, entries)); } /** @@ -301,7 +438,7 @@ public void validate(BulletConfig config) { * @return A copy of this validator with all its defined {@link Entry} and {@link Relationship}. */ public Validator copy() { - return new Validator(entries, relations); + return new Validator(entries, relations, states); } // Type Adapters @@ -538,4 +675,17 @@ public static Predicate isInRange(T min, T max) { public static boolean isGreaterOrEqual(Object first, Object second) { return ((Number) first).doubleValue() >= ((Number) second).doubleValue(); } + + // Binary Predicate makers. + + /** + * Returns a {@link BiPredicate} that checks to see if the first argument is at least the given times + * more than the second. + * + * @param n The number of times the second argument must be smaller than the first. + * @return The created {@link BiPredicate}. + */ + public static BiPredicate isAtleastNTimes(double n) { + return (greater, smaller) -> ((Number) greater).doubleValue() >= n * ((Number) smaller).doubleValue(); + } } diff --git a/src/test/java/com/yahoo/bullet/common/ValidatorTest.java b/src/test/java/com/yahoo/bullet/common/ValidatorTest.java index d72acdf1..e08e6d88 100644 --- a/src/test/java/com/yahoo/bullet/common/ValidatorTest.java +++ b/src/test/java/com/yahoo/bullet/common/ValidatorTest.java @@ -7,6 +7,7 @@ import com.yahoo.bullet.common.Validator.Entry; import com.yahoo.bullet.common.Validator.Relationship; +import com.yahoo.bullet.common.Validator.State; import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -17,6 +18,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.function.BiPredicate; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -124,6 +126,17 @@ public void testEntryUnless() { Assert.assertTrue(empty.get("foo") == original); } + @Test(expectedExceptions = IllegalStateException.class) + public void testEntryFailing() { + Validator validator = new Validator(); + Entry entry = validator.define("foo") + .checkIf(Validator::isNotNull) + .checkIf(Validator.isIn("baz", "bar")) + .orFail(); + empty.set("foo", "qux"); + entry.normalize(empty); + } + @Test public void testIsNotNull() { Assert.assertTrue(Validator.isNotNull("foo")); @@ -252,6 +265,20 @@ public void testIsInRange() { Assert.assertFalse(Validator.isInRange(-1, 2.0).test(null)); } + @Test + public void testIsAtleastNTimes() { + BiPredicate isAtleastTwice = Validator.isAtleastNTimes(2.0); + Assert.assertTrue(isAtleastTwice.test(2L, 1L)); + Assert.assertTrue(isAtleastTwice.test(3L, 1L)); + Assert.assertFalse(isAtleastTwice.test(0L, 1L)); + + BiPredicate isAtleastThrice = Validator.isAtleastNTimes(3.0); + Assert.assertFalse(isAtleastThrice.test(2.0, 1L)); + Assert.assertTrue(isAtleastThrice.test(3.0, 1L)); + Assert.assertTrue(isAtleastThrice.test(4.0, 1L)); + Assert.assertFalse(isAtleastThrice.test(0.1, 1L)); + } + @Test public void testIntegerConversion() { Assert.assertEquals(Validator.asInt(3.4), 3); @@ -396,6 +423,86 @@ public void testRelationshipFailureUsesCustomDefaults() { Assert.assertEquals(empty.get("bar"), "norf"); } + @Test(expectedExceptions = IllegalStateException.class) + public void testRelationshipFailureFailsOut() { + Validator validator = new Validator(); + validator.define("foo").defaultTo(0); + validator.define("bar").defaultTo(42); + + Relationship relation = validator.relate("Test", "foo", "bar").checkIf(Validator::isGreaterOrEqual); + relation.orFail(); + + empty.set("foo", -1L); + empty.set("bar", 0.2); + relation.normalize(empty); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testStateWithoutEntry() { + Validator validator = new Validator(); + validator.evaluate("Test", "foo", "bar", "baz"); + } + + @Test + public void testStateDefaulting() { + Validator validator = new Validator(); + + validator.define("foo").defaultTo(42); + validator.define("bar").defaultTo(11).castTo(Validator::asDouble); + validator.define("baz").defaultTo(true).checkIf(Validator::isBoolean); + + validator.evaluate("Test", "foo", "bar", "baz").checkIf(o -> false); + + empty.set("foo", 22); + empty.set("bar", -1.4); + empty.set("baz", false); + validator.validate(empty); + + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 11.0); + Assert.assertEquals(empty.get("baz"), true); + } + + @Test + public void testStateMultipleChecks() { + Validator validator = new Validator(); + + validator.define("foo").defaultTo(42); + validator.define("bar").defaultTo(11).castTo(Validator::asDouble); + validator.define("baz").defaultTo(true).checkIf(Validator::isBoolean); + + validator.evaluate("Test", "foo", "bar", "baz") + .checkIf((o) -> o.get(0).equals("1") && o.get(1).equals(2.0) && o.get(2).equals(false)) + .checkIf((o) -> Double.valueOf(o.get(0).toString()) >= 0.0); + + empty.set("foo", "1"); + empty.set("bar", 2.0); + empty.set("baz", false); + validator.validate(empty); + + Assert.assertEquals(empty.get("foo"), "1"); + Assert.assertEquals(empty.get("bar"), 2.0); + Assert.assertEquals(empty.get("baz"), false); + } + + @Test(expectedExceptions = IllegalStateException.class) + public void testStateCheckFailureErrorsOut() { + Validator validator = new Validator(); + + validator.define("foo").defaultTo(42); + validator.define("bar").defaultTo(11).castTo(Validator::asDouble); + validator.define("baz").defaultTo(true).checkIf(Validator::isBoolean); + + validator.evaluate("Test", "foo", "bar", "baz") + .checkIf((o) -> false) + .orFail(); + + empty.set("foo", 22); + empty.set("bar", -1.4); + empty.set("baz", false); + validator.validate(empty); + } + @Test public void testCopyingPreservesOriginal() { Validator validator = new Validator(); @@ -450,39 +557,58 @@ public void testCopyingDoesNotDeepCopy() { validator.define("foo").checkIf(whitelist::contains).defaultTo(0); validator.define("bar").defaultTo(42); + validator.define("baz").defaultTo(false); Relationship relation = validator.relate("Test", "foo", "bar").checkIf(Validator::isGreaterOrEqual); relation.orElseUse("qux", "norf"); + State state = validator.evaluate("Test", "foo", "bar", "baz").checkIf(o -> true); + empty.set("foo", -1L); empty.set("bar", -1.4); + empty.set("baz", true); validator.validate(empty); // Defaults to 0 since -1 is not in the whitelist Assert.assertEquals(empty.get("foo"), 0); Assert.assertEquals(empty.get("bar"), -1.4); + Assert.assertEquals(empty.get("baz"), true); // Allow -1 through whitelist.add(-1L); empty.set("foo", -1L); empty.set("bar", -1.4); + empty.set("baz", false); validator.validate(empty); Assert.assertEquals(empty.get("foo"), -1L); Assert.assertEquals(empty.get("bar"), -1.4); + Assert.assertEquals(empty.get("baz"), false); Validator copy = validator.copy(); // The copy also has the shallow copy of contains since -1 goes through empty.set("foo", -1L); empty.set("bar", -1.4); + empty.set("baz", false); copy.validate(empty); Assert.assertEquals(empty.get("foo"), -1L); Assert.assertEquals(empty.get("bar"), -1.4); + Assert.assertEquals(empty.get("baz"), false); // Removing the white whitelist.clear(); copy.validate(empty); Assert.assertEquals(empty.get("foo"), 0); Assert.assertEquals(empty.get("bar"), -1.4); + Assert.assertEquals(empty.get("baz"), false); + + // Changing the state to fail, doesn't change the copy + state.checkIf(o -> false); + state.orFail(); + + copy.validate(empty); + Assert.assertEquals(empty.get("foo"), 0); + Assert.assertEquals(empty.get("bar"), -1.4); + Assert.assertEquals(empty.get("baz"), false); } }