Fix bug in 'type' attribute handling in JacksonDeserializer

There was a bug in the 'type' attribute handling in deserializeCondition. But after some examination I simplified things a bit by: - Have the ConditionEval subclasses set their 'type' at construction time. There is only one valid type, just set it. This is now analogous to Condition, where the type is also set at construction. - Given that we can depend on the type field being set, simplify the 'type' handling in the deserializer. - Also, convert to switch stmts on the enum for clarity. - Note, this fixed a bug in ExternalConditionEval where type was not being set correctly.
hawkular · Jul 17, 2015 · b5e0dff · b5e0dff
1 parent ea91916
commit b5e0dff
Show file tree

Hide file tree

Showing 9 changed files with 183 additions and 164 deletions.
diff --git a/hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/json/JacksonDeserializer.java b/hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/json/JacksonDeserializer.java
diff --git a/...-api/src/main/java/org/hawkular/alerts/api/model/condition/AvailabilityConditionEval.java b/...-api/src/main/java/org/hawkular/alerts/api/model/condition/AvailabilityConditionEval.java
@@ -16,6 +16,7 @@
  */
 package org.hawkular.alerts.api.model.condition;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.Availability;
 import org.hawkular.alerts.api.model.data.Availability.AvailabilityType;
 
@@ -37,18 +38,15 @@ public class AvailabilityConditionEval extends ConditionEval {
     private AvailabilityType value;
 
     public AvailabilityConditionEval() {
-        super(false, 0, null);
+        super(Type.AVAILABILITY, false, 0, null);
         this.condition = null;
         this.value = null;
     }
 
     public AvailabilityConditionEval(AvailabilityCondition condition, Availability avail) {
-        super(condition.match(avail.getValue()), avail.getTimestamp(), avail.getContext());
+        super(Type.AVAILABILITY, condition.match(avail.getValue()), avail.getTimestamp(), avail.getContext());
         this.condition = condition;
         this.value = avail.getValue();
-        if (this.condition != null) {
-            this.type = this.condition.getType();
-        }
     }
 
     public AvailabilityCondition getCondition() {

diff --git a/...lerts-api/src/main/java/org/hawkular/alerts/api/model/condition/CompareConditionEval.java b/...lerts-api/src/main/java/org/hawkular/alerts/api/model/condition/CompareConditionEval.java
@@ -18,6 +18,7 @@
 
 import java.util.Map;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.NumericData;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -44,23 +45,20 @@ public class CompareConditionEval extends ConditionEval {
     protected Map<String, String> context2;
 
     public CompareConditionEval() {
-        super(false, 0, null);
+        super(Type.COMPARE, false, 0, null);
         this.condition = null;
         this.value1 = null;
         this.value2 = null;
         this.context2 = null;
     }
 
     public CompareConditionEval(CompareCondition condition, NumericData data1, NumericData data2) {
-        super(condition.match(data1.getValue(), data2.getValue()),
+        super(Type.COMPARE, condition.match(data1.getValue(), data2.getValue()),
                 ((data1.getTimestamp() > data1.getTimestamp()) ? data1.getTimestamp() : data2.getTimestamp()),
                 data1.getContext());
         this.condition = condition;
         this.value1 = data1.getValue();
         this.value2 = data2.getValue();
-        if (this.condition != null) {
-            this.type = this.condition.getType();
-        }
         this.context2 = data2.getContext();
     }
 

diff --git a/hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/model/condition/ConditionEval.java b/hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/model/condition/ConditionEval.java
@@ -19,6 +19,7 @@
 import java.util.Map;
 
 import org.hawkular.alerts.api.json.JacksonDeserializer;
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -60,7 +61,8 @@ public ConditionEval() {
         // for json assembly
     }
 
-    public ConditionEval(boolean match, long dataTimestamp, Map<String, String> context) {
+    public ConditionEval(Type type, boolean match, long dataTimestamp, Map<String, String> context) {
+        this.type = type;
         this.match = match;
         this.dataTimestamp = dataTimestamp;
         this.evalTimestamp = System.currentTimeMillis();

diff --git a/...erts-api/src/main/java/org/hawkular/alerts/api/model/condition/ExternalConditionEval.java b/...erts-api/src/main/java/org/hawkular/alerts/api/model/condition/ExternalConditionEval.java
@@ -16,6 +16,7 @@
  */
 package org.hawkular.alerts.api.model.condition;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.StringData;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -37,13 +38,13 @@ public class ExternalConditionEval extends ConditionEval {
     private String value;
 
     public ExternalConditionEval() {
-        super(false, 0, null);
+        super(Type.EXTERNAL, false, 0, null);
         this.condition = null;
         this.value = null;
     }
 
     public ExternalConditionEval(ExternalCondition condition, StringData data) {
-        super(condition.match(data.getValue()), data.getTimestamp(), data.getContext());
+        super(Type.EXTERNAL, condition.match(data.getValue()), data.getTimestamp(), data.getContext());
         this.condition = condition;
         this.value = data.getValue();
     }

diff --git a/...alerts-api/src/main/java/org/hawkular/alerts/api/model/condition/StringConditionEval.java b/...alerts-api/src/main/java/org/hawkular/alerts/api/model/condition/StringConditionEval.java
@@ -16,6 +16,7 @@
  */
 package org.hawkular.alerts.api.model.condition;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.StringData;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -36,18 +37,15 @@ public class StringConditionEval extends ConditionEval {
     private String value;
 
     public StringConditionEval() {
-        super(false, 0, null);
+        super(Type.STRING, false, 0, null);
         this.condition = null;
         this.value = null;
     }
 
     public StringConditionEval(StringCondition condition, StringData data) {
-        super(condition.match(data.getValue()), data.getTimestamp(), data.getContext());
+        super(Type.STRING, condition.match(data.getValue()), data.getTimestamp(), data.getContext());
         this.condition = condition;
         this.value = data.getValue();
-        if (this.condition != null) {
-            this.type = this.condition.getType();
-        }
     }
 
     public StringCondition getCondition() {

diff --git a/...rts-api/src/main/java/org/hawkular/alerts/api/model/condition/ThresholdConditionEval.java b/...rts-api/src/main/java/org/hawkular/alerts/api/model/condition/ThresholdConditionEval.java
@@ -16,6 +16,7 @@
  */
 package org.hawkular.alerts.api.model.condition;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.NumericData;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -36,17 +37,14 @@ public class ThresholdConditionEval extends ConditionEval {
     private Double value;
 
     public ThresholdConditionEval() {
-        super(false, 0, null);
+        super(Type.THRESHOLD, false, 0, null);
         this.value = Double.NaN;
     }
 
     public ThresholdConditionEval(ThresholdCondition condition, NumericData data) {
-        super(condition.match(data.getValue()), data.getTimestamp(), data.getContext());
+        super(Type.THRESHOLD, condition.match(data.getValue()), data.getTimestamp(), data.getContext());
         this.condition = condition;
         this.value = data.getValue();
-        if (this.condition != null) {
-            this.type = this.condition.getType();
-        }
     }
 
     public ThresholdCondition getCondition() {

diff --git a/...pi/src/main/java/org/hawkular/alerts/api/model/condition/ThresholdRangeConditionEval.java b/...pi/src/main/java/org/hawkular/alerts/api/model/condition/ThresholdRangeConditionEval.java
@@ -16,6 +16,7 @@
  */
 package org.hawkular.alerts.api.model.condition;
 
+import org.hawkular.alerts.api.model.condition.Condition.Type;
 import org.hawkular.alerts.api.model.data.NumericData;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -36,18 +37,15 @@ public class ThresholdRangeConditionEval extends ConditionEval {
     private Double value;
 
     public ThresholdRangeConditionEval() {
-        super(false, 0, null);
+        super(Type.RANGE, false, 0, null);
         this.condition = null;
         this.value = null;
     }
 
     public ThresholdRangeConditionEval(ThresholdRangeCondition condition, NumericData data) {
-        super(condition.match(data.getValue()), data.getTimestamp(), data.getContext());
+        super(Type.RANGE, condition.match(data.getValue()), data.getTimestamp(), data.getContext());
         this.condition = condition;
         this.value = data.getValue();
-        if (this.condition != null) {
-            this.type = this.condition.getType();
-        }
     }
 
     public ThresholdRangeCondition getCondition() {

diff --git a/hawkular-alerts-api/src/test/java/org/hawkular/alerts/api/JsonJacksonTest.java b/hawkular-alerts-api/src/test/java/org/hawkular/alerts/api/JsonJacksonTest.java
@@ -253,8 +253,8 @@ public void jsonAvailabilityConditionTest() throws Exception {
     @Test
     public void jsonAvailabilityConditionEvalTest() throws Exception {
         String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"AVAILABILITY\"," +
-                "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"dataId\":\"Default\",\"operator\":\"UP\"}," +
+                "\"condition\":{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"AVAILABILITY\"," +
+                "\"dataId\":\"Default\",\"operator\":\"UP\"}," +
                 "\"value\":\"UP\",\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
         AvailabilityConditionEval eval = objectMapper.readValue(str, AvailabilityConditionEval.class);
 
@@ -357,12 +357,12 @@ public void jsonCompareConditionTest() throws Exception {
 
     @Test
     public void jsonCompareConditionEvalTest() throws Exception {
-        String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"COMPARE\"," +
-                "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\"," +
-                "\"dataId\":\"Default1\",\"operator\":\"LT\",\"data2Id\":\"Default2\",\"data2Multiplier\":1.2}," +
-                "\"value1\":10.0," +
-                "\"value2\":15.0,\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"},\"context2\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
+        String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"COMPARE\","
+                + "\"condition\":"
+                + "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"COMPARE\","
+                + "\"dataId\":\"Default1\",\"operator\":\"LT\",\"data2Id\":\"Default2\",\"data2Multiplier\":1.2},"
+                + "\"value1\":10.0,\"value2\":15.0,"
+                + "\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"},\"context2\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
         CompareConditionEval eval = objectMapper.readValue(str, CompareConditionEval.class);
 
         assertTrue(eval.getEvalTimestamp() == 1);
@@ -468,7 +468,7 @@ public void jsonStringConditionTest() throws Exception {
     public void jsonStringConditionEvalTest() throws Exception {
         String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"STRING\"," +
                 "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\"," +
+                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"STRING\"," +
                 "\"dataId\":\"Default\",\"operator\":\"MATCH\",\"pattern\":\"test-pattern\",\"ignoreCase\":false}," +
                 "\"value\":\"test-value\",\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
         StringConditionEval eval = objectMapper.readValue(str, StringConditionEval.class);
@@ -563,7 +563,7 @@ public void jsonThresholdConditionTest() throws Exception {
     public void jsonThresholdConditionEvalTest() throws Exception {
         String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"THRESHOLD\"," +
                 "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\"," +
+                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"THRESHOLD\"," +
                 "\"dataId\":\"Default\",\"operator\":\"LT\",\"threshold\":10.5}," +
                 "\"value\":1.0,\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
         ThresholdConditionEval eval = objectMapper.readValue(str, ThresholdConditionEval.class);
@@ -703,7 +703,7 @@ public void jsonThresholdRangeConditionTest() throws Exception {
     public void jsonThresholdRangeConditionEvalTest() throws Exception {
         String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"RANGE\"," +
                 "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\"," +
+                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"RANGE\"," +
                 "\"dataId\":\"Default\",\"operatorLow\":\"INCLUSIVE\",\"operatorHigh\":\"INCLUSIVE\"," +
                 "\"thresholdLow\":10.5,\"thresholdHigh\":20.5,\"inRange\":true}," +
                 "\"value\":1.0,\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";
@@ -748,7 +748,7 @@ public void jsonExternalConditionTest() throws Exception {
     public void jsonExternalConditionEvalTest() throws Exception {
         String str = "{\"evalTimestamp\":1,\"dataTimestamp\":1,\"type\":\"EXTERNAL\"," +
                 "\"condition\":" +
-                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\"," +
+                "{\"triggerId\":\"test\",\"triggerMode\":\"FIRING\",\"type\":\"EXTERNAL\"," +
                 "\"dataId\":\"Default\",\"systemId\":\"HawkularMetrics\"," +
                 "\"expression\":\"metric:5:avg(foo > 100.5)\"}," +
                 "\"value\":\"foo\",\"context\":{\"n1\":\"v1\",\"n2\":\"v2\"}}";