AMBARI-1480. Comparison predicate should account for null values.

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1449784 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -366,6 +366,8 @@ Trunk (unreleased changes):
 
  BUG FIXES
 
+ AMBARI-1480. Comparison predicate should account for null values. (tbeerbower)
+
  AMBARI-1467. UI should block on cluster metric api call before making
  subsequent one. (yusaku)
 

ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/EqualsPredicate.java

@@ -31,8 +31,12 @@ public class EqualsPredicate<T> extends ComparisonPredicate<T> {
 
   @Override
   public boolean evaluate(Resource resource) {
-    Object propertyValue = resource.getPropertyValue(getPropertyId());
-    return propertyValue != null && compareValueTo(propertyValue) == 0;
+    Object propertyValue  = resource.getPropertyValue(getPropertyId());
+    Object predicateValue = getValue();
+
+    return predicateValue == null ?
+        propertyValue == null :
+        propertyValue != null && compareValueTo(propertyValue) == 0;
   }
 
   @Override

ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/GreaterEqualsPredicate.java

@@ -25,8 +25,19 @@ import org.apache.ambari.server.controller.spi.Resource;
  */
 public class GreaterEqualsPredicate<T> extends ComparisonPredicate<T> {
 
+  /**
+   * Construct a GreaterEqualsPredicate.
+   *
+   * @param propertyId  the property id
+   * @param value       the value
+   *
+   * @throws IllegalArgumentException if the given value is null
+   */
   public GreaterEqualsPredicate(String propertyId, Comparable<T> value) {
     super(propertyId, value);
+    if (value == null) {
+      throw new IllegalArgumentException("Value can't be null.");
+    }
   }
 
   @Override

ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/GreaterPredicate.java

@@ -24,8 +24,19 @@ import org.apache.ambari.server.controller.spi.Resource;
  */
 public class GreaterPredicate<T> extends ComparisonPredicate<T> {
 
+  /**
+   * Construct a GreaterPredicate.
+   *
+   * @param propertyId  the property id
+   * @param value       the value
+   *
+   * @throws IllegalArgumentException if the given value is null
+   */
   public GreaterPredicate(String propertyId, Comparable<T> value) {
     super(propertyId, value);
+    if (value == null) {
+      throw new IllegalArgumentException("Value can't be null.");
+    }
   }
 
   @Override

ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/LessEqualsPredicate.java

@@ -25,8 +25,19 @@ import org.apache.ambari.server.controller.spi.Resource;
  */
 public class LessEqualsPredicate<T> extends ComparisonPredicate<T> {
 
+  /**
+   * Construct a LessEqualsPredicate.
+   *
+   * @param propertyId  the property id
+   * @param value       the value
+   *
+   * @throws IllegalArgumentException if the given value is null
+   */
   public LessEqualsPredicate(String propertyId, Comparable<T> value) {
     super(propertyId, value);
+    if (value == null) {
+      throw new IllegalArgumentException("Value can't be null.");
+    }
   }
 
   @Override

ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/LessPredicate.java

@@ -24,8 +24,19 @@ import org.apache.ambari.server.controller.spi.Resource;
  */
 public class LessPredicate<T> extends ComparisonPredicate<T> {
 
+  /**
+   * Construct a LessPredicate.
+   *
+   * @param propertyId  the property id
+   * @param value       the value
+   *
+   * @throws IllegalArgumentException if the given value is null
+   */
   public LessPredicate(String propertyId, Comparable<T> value) {
     super(propertyId, value);
+    if (value == null) {
+      throw new IllegalArgumentException("Value can't be null.");
+    }
   }
 
   @Override

ambari-server/src/test/java/org/apache/ambari/server/controller/predicate/EqualsPredicateTest.java

@@ -27,7 +27,7 @@ import org.junit.Test;
 import java.util.Set;
 
 /**
- *
+ * Equals predicate tests.
  */
 public class EqualsPredicateTest {
 
@@ -50,6 +50,19 @@ public class EqualsPredicateTest {
     Assert.assertFalse(predicate.evaluate(resource));
   }
 
+  @Test
+  public void testApplyNullValue() {
+    Resource resource = new ResourceImpl(Resource.Type.HostComponent);
+    String propertyId = PropertyHelper.getPropertyId("category1", "foo");
+    Predicate predicate = new EqualsPredicate<String>(propertyId, null);
+
+    resource.setProperty(propertyId, "monkey");
+    Assert.assertFalse(predicate.evaluate(resource));
+
+    resource.setProperty(propertyId, null);
+    Assert.assertTrue(predicate.evaluate(resource));
+  }
+
   @Test
   public void testGetProperties() {
     String propertyId = PropertyHelper.getPropertyId("category1", "foo");

ambari-server/src/test/java/org/apache/ambari/server/controller/predicate/GreaterEqualsPredicateTest.java

@@ -47,6 +47,16 @@ public class GreaterEqualsPredicateTest {
     Assert.assertTrue(predicate.evaluate(resource));
   }
 
+  @Test
+  public void testNullValue() {
+    try {
+      new GreaterEqualsPredicate<Integer>("category/foo", null);
+      Assert.fail("Expected IllegalArgumentException for null value.");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
   @Test
   public void testGetProperties() {
     String propertyId = PropertyHelper.getPropertyId("category1", "foo");

ambari-server/src/test/java/org/apache/ambari/server/controller/predicate/GreaterPredicateTest.java

@@ -47,6 +47,16 @@ public class GreaterPredicateTest {
     Assert.assertFalse(predicate.evaluate(resource));
   }
 
+  @Test
+  public void testNullValue() {
+    try {
+      new GreaterPredicate<Integer>("category/foo", null);
+      Assert.fail("Expected IllegalArgumentException for null value.");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
   @Test
   public void testGetProperties() {
     String propertyId = PropertyHelper.getPropertyId("category1", "foo");

ambari-server/src/test/java/org/apache/ambari/server/controller/predicate/LessEqualsPredicateTest.java

@@ -47,6 +47,16 @@ public class LessEqualsPredicateTest {
     Assert.assertTrue(predicate.evaluate(resource));
   }
 
+  @Test
+  public void testNullValue() {
+    try {
+      new LessEqualsPredicate<Integer>("category/foo", null);
+      Assert.fail("Expected IllegalArgumentException for null value.");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
   @Test
   public void testGetProperties() {
     String propertyId = PropertyHelper.getPropertyId("category1", "foo");

ambari-server/src/test/java/org/apache/ambari/server/controller/predicate/LessPredicateTest.java

@@ -47,6 +47,16 @@ public class LessPredicateTest {
     Assert.assertFalse(predicate.evaluate(resource));
   }
 
+  @Test
+  public void testNullValue() {
+    try {
+      new LessPredicate<Integer>("category/foo", null);
+      Assert.fail("Expected IllegalArgumentException for null value.");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
   @Test
   public void testGetProperties() {
     String propertyId = PropertyHelper.getPropertyId("category1", "foo");