Merge pull request #162 from jshaughn/hwkalerts-131

HWKALERTS-131 Re-use member dataIdMappings when updating group trigger conditions

hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/model/trigger/Trigger.java

@@ -103,6 +103,12 @@ public class Trigger implements Serializable {
     @JsonInclude
     private Match autoResolveMatch;
 
+    // Only set for MEMBER triggers, the dataIdMap used when adding the member. Is re-used
+    // for group condition updates unless a new dataIdMap is provided.
+    @JsonInclude(Include.NON_EMPTY)
+    protected Map<String, String> dataIdMap;
+
+    // Only set for MEMBER triggers, the group trigger for which this is a member.
     @JsonInclude(Include.NON_EMPTY)
     private String memberOf;
 
@@ -173,6 +179,7 @@ public Trigger(String tenantId, String id, String name, Map<String, String> cont
         this.autoResolve = false;
         this.autoResolveAlerts = true;
         this.autoResolveMatch = Match.ALL;
+        this.dataIdMap = null;
         this.eventCategory = null;
         this.eventText = null;
         this.eventType = EventType.ALERT; // Is this an OK default, or should it be a constructor parameter?
@@ -346,6 +353,14 @@ public void setActions(Set<TriggerAction> actions) {
         this.actions = actions;
     }
 
+    public Map<String, String> getDataIdMap() {
+        return dataIdMap;
+    }
+
+    public void setDataIdMap(Map<String, String> dataIdMap) {
+        this.dataIdMap = dataIdMap;
+    }
+
     public void addAction(TriggerAction triggerAction) {
         getActions().add(triggerAction);
     }
@@ -488,7 +503,8 @@ public String toString() {
                 + ", context=" + context + ", actions=" + actions + ", autoDisable=" + autoDisable
                 + ", autoEnable=" + autoEnable + ", autoResolve=" + autoResolve + ", autoResolveAlerts="
                 + autoResolveAlerts + ", autoResolveMatch=" + autoResolveMatch + ", memberOf=" + memberOf
-                + ", enabled=" + enabled + ", firingMatch=" + firingMatch + ", mode=" + mode + ", tags=" + tags + "]";
+                + ", dataIdMap=" + dataIdMap + ", enabled=" + enabled + ", firingMatch=" + firingMatch
+                + ", mode=" + mode + ", tags=" + tags + "]";
     }
 
 }

hawkular-alerts-api/src/main/java/org/hawkular/alerts/api/services/DefinitionsService.java

@@ -448,20 +448,35 @@ Collection<Condition> setConditions(String tenantId, String triggerId, Mode trig
             Collection<Condition> conditions) throws Exception;
 
     /**
-     * The condition set for the specified Group Trigger and trigger mode.  The set conditions are ordered
-     * using the Collection ordering as the conditionSet ordering (assuming an ordered Collection implementation
-     * is supplied).  Any existing conditions are replaced. The non-orphan member triggers will have the new condition
-     * set applied, using the provided dataIdMemberMap.
-     * See {@link org.hawkular.alerts.api.json.GroupConditions#setDataIdMemberMap(Map)} for
-     * an example of the <code>dataIdMemberMap</code> parameter.  The following Condition fields are ignored for
-     * the incoming conditions, and set in the returned collection set:
+     * The condition set for the specified Group Trigger and trigger mode.  The conditionSet is ordered
+     * using the Collection ordering (assuming an ordered Collection implementation is supplied).  Any existing
+     * conditions are replaced. The non-orphan member triggers will have the new condition set applied, using the
+     * provided dataIdMemberMap. See {@link org.hawkular.alerts.api.json.GroupConditionsInfo#setDataIdMemberMap(Map)}
+     * for an example of the <code>dataIdMemberMap</code> parameter.
+     * <p>
+     * The <code>dataIdMemberMap</code> should be null if the group has no members.
+     * </p>
+     * <p>
+     * The <code>dataIdMemberMap</code> should be null if this is a DataDriven group trigger.  In this
+     * case the member triggers are removed and will be re-populated as incoming data demands.
+     * </p>
+     * <p>
+     * For [non-data-driven] group triggers with existing members the <code>dataIdMemberMap</code> is handled
+     * as follows. For members not included in the <code>dataIdMemberMap</code> their most recently supplied
+     * dataIdMap will be used. This means that it is not necessary to supply mappings if the new condition set
+     * uses only dataIds found in the old condition set. If the new conditions introduce new dataIds a full
+     * <code>dataIdMemberMap</code> must be supplied.
+     * </p>
+     * <p>
+     * The following Condition fields are ignored for the incoming conditions, and set in the returned collection set:
      * <pre>
      *   conditionId
      *   triggerId
      *   triggerMode
      *   conditionSetSize
      *   conditionSetIndex
      * </pre>
+     * </p>
      * @param tenantId Tenant where trigger is stored
      * @param groupId Group Trigger adding the condition and whose non-orphan members will also have it added.
      * @param triggerMode Mode where condition is applied

hawkular-alerts-engine/src/main/java/org/hawkular/alerts/engine/impl/CassDefinitionsServiceImpl.java

@@ -282,9 +282,9 @@ private void addTrigger(Trigger trigger) throws Exception {
         try {
             session.execute(insertTrigger.bind(trigger.getTenantId(), trigger.getId(), trigger.isAutoDisable(),
                     trigger.isAutoEnable(), trigger.isAutoResolve(), trigger.isAutoResolveAlerts(),
-                    trigger.getAutoResolveMatch().name(), trigger.getContext(), trigger.getDescription(),
-                    trigger.isEnabled(), trigger.getEventCategory(), trigger.getEventText(), trigger.getEventType(),
-                    trigger.getFiringMatch().name(), trigger.getMemberOf(), trigger.getName(),
+                    trigger.getAutoResolveMatch().name(), trigger.getContext(), trigger.getDataIdMap(),
+                    trigger.getDescription(), trigger.isEnabled(), trigger.getEventCategory(), trigger.getEventText(),
+                    trigger.getEventType(), trigger.getFiringMatch().name(), trigger.getMemberOf(), trigger.getName(),
                     trigger.getSeverity().name(), trigger.getSource(), trigger.getTags(), trigger.getType().name()));
 
             insertTriggerActions(trigger);
@@ -313,9 +313,9 @@ private void insertTriggerActions(Trigger trigger) throws Exception {
                 triggerAction.setTenantId(trigger.getTenantId());
             });
             List<ResultSetFuture> futures = trigger.getActions().stream().map(triggerAction -> session.
-            executeAsync(insertTriggerActions.bind(trigger.getTenantId(), trigger.getId(),
-                    triggerAction.getActionPlugin(), triggerAction.getActionId(),
-                    JsonUtil.toJson(triggerAction)))).collect(Collectors.toList());
+                    executeAsync(insertTriggerActions.bind(trigger.getTenantId(), trigger.getId(),
+                            triggerAction.getActionPlugin(), triggerAction.getActionId(),
+                            JsonUtil.toJson(triggerAction)))).collect(Collectors.toList());
             Futures.allAsList(futures).get();
         }
     }
@@ -536,6 +536,7 @@ private Trigger copyGroupTrigger(Trigger group, Trigger member) {
         member.setAutoResolveAlerts(group.isAutoResolveAlerts());
         member.setAutoResolveMatch(group.getAutoResolveMatch());
         member.setContext(group.getContext());
+        member.setDataIdMap(group.getDataIdMap()); // irrelevant but here for completeness
         member.setDescription(group.getDescription());
         member.setEnabled(group.isEnabled());
         member.setEventType(group.getEventType());
@@ -558,10 +559,10 @@ private Trigger updateTrigger(Trigger trigger, Set<TriggerAction> existingAction
         try {
             session.execute(updateTrigger.bind(trigger.isAutoDisable(), trigger.isAutoEnable(),
                     trigger.isAutoResolve(), trigger.isAutoResolveAlerts(), trigger.getAutoResolveMatch().name(),
-                    trigger.getContext(), trigger.getDescription(), trigger.isEnabled(), trigger.getEventCategory(),
-                    trigger.getEventText(), trigger.getFiringMatch().name(), trigger.getMemberOf(), trigger.getName(),
-                    trigger.getSeverity().name(), trigger.getSource(), trigger.getTags(), trigger.getType().name(),
-                    trigger.getTenantId(), trigger.getId()));
+                    trigger.getContext(), trigger.getDataIdMap(), trigger.getDescription(), trigger.isEnabled(),
+                    trigger.getEventCategory(), trigger.getEventText(), trigger.getFiringMatch().name(),
+                    trigger.getMemberOf(), trigger.getName(), trigger.getSeverity().name(), trigger.getSource(),
+                    trigger.getTags(), trigger.getType().name(), trigger.getTenantId(), trigger.getId()));
             if (!trigger.getActions().equals(existingActions)) {
                 deleteTriggerActions(trigger.getTenantId(), trigger.getId());
                 insertTriggerActions(trigger);
@@ -902,9 +903,9 @@ public Collection<Trigger> getAllTriggersByTag(String name, String value) throws
             List<ResultSetFuture> futures = nameOnly ? tenants.stream()
                     .map(tenantId -> session.executeAsync(selectTags.bind(tenantId, TagType.TRIGGER, name)))
                     .collect(Collectors.toList()) : tenants.stream()
-                            .map(tenantId -> session.executeAsync(selectTags.bind(tenantId, TagType.TRIGGER, name,
-                                    value)))
-                            .collect(Collectors.toList());
+                    .map(tenantId -> session.executeAsync(selectTags.bind(tenantId, TagType.TRIGGER, name,
+                            value)))
+                    .collect(Collectors.toList());
             List<ResultSet> rsTriggerIds = Futures.allAsList(futures).get();
             rsTriggerIds.stream().forEach(rs -> {
                 for (Row row : rs) {
@@ -1011,6 +1012,7 @@ private Trigger mapTrigger(Row row) {
         trigger.setAutoResolveAlerts(row.getBool("autoResolveAlerts"));
         trigger.setAutoResolveMatch(Match.valueOf(row.getString("autoResolveMatch")));
         trigger.setContext(row.getMap("context", String.class, String.class));
+        trigger.setDataIdMap(row.getMap("dataIdMap", String.class, String.class));
         trigger.setDescription(row.getString("description"));
         trigger.setEnabled(row.getBool("enabled"));
         trigger.setEventCategory(row.getString("eventCategory"));
@@ -1091,6 +1093,9 @@ public Trigger addMemberTrigger(String tenantId, String groupId, String memberId
             member.setTags(combinedTags);
         }
 
+        // store the dataIdMap so that it can be used for future condition updates (where the mappings are unchanged)
+        member.setDataIdMap(dataIdMap);
+
         addTrigger(member);
 
         // add any conditions
@@ -1724,13 +1729,39 @@ public Collection<Condition> setGroupConditions(String tenantId, String groupId,
             memberTriggers.clear();
         }
 
-        // allow the dataIdMap to be empty when there are no member triggers
         if (!memberTriggers.isEmpty()) {
-            if (dataIdMemberMap == null) {
-                throw new IllegalArgumentException("DataIdMemberMap must be not null when member triggers exist.");
+            // fill in dataIdMap entries not supplied using the most recently supplied mapping
+            if (null == dataIdMemberMap) {
+                dataIdMemberMap = new HashMap<>();
+            }
+            for (Trigger member : memberTriggers) {
+                String memberId = member.getId();
+
+                for (Map.Entry<String, String> entry : member.getDataIdMap().entrySet()) {
+                    String groupDataId = entry.getKey();
+                    String memberDataId = entry.getValue();
+
+                    Map<String, String> memberIdMap = dataIdMemberMap.get(groupDataId);
+                    if (null == memberIdMap) {
+                        memberIdMap = new HashMap<>(memberTriggers.size());
+                        dataIdMemberMap.put(groupDataId, memberIdMap);
+                    }
+                    if (memberIdMap.containsKey(memberId)) {
+                        // supplied mapping has a mapping for this groupDataId and member. If it is
+                        // a new mapping for this member then update the member's dataIdMap
+                        if (!memberIdMap.get(memberId).equals(member.getDataIdMap().get(groupDataId))) {
+                            Map<String, String> updatedDataIdMap = new HashMap<>(member.getDataIdMap());
+                            updatedDataIdMap.put(groupDataId, memberIdMap.get(memberId));
+                            updateMemberTriggerDataIdMap(tenantId, memberId, updatedDataIdMap);
+                        }
+                    } else {
+                        // supplied map did not have the previously stored mapping, use the existing mapping
+                        memberIdMap.put(memberId, memberDataId);
+                    }
+                }
             }
 
-            // first, validate the dataIdMap
+            // validate the dataIdMemberMap
             for (Condition groupCondition : groupConditions) {
                 if (!dataIdMemberMap.containsKey(groupCondition.getDataId())) {
                     throw new IllegalArgumentException("Missing dataIdMap entry for dataId token ["
@@ -1795,6 +1826,32 @@ public Collection<Condition> setGroupConditions(String tenantId, String groupId,
         return setConditions(tenantId, groupId, triggerMode, groupConditions);
     }
 
+    private void updateMemberTriggerDataIdMap(String tenantId, String memberTriggerId, Map<String, String> dataIdMap)
+            throws Exception {
+        if (isEmpty(tenantId)) {
+            throw new IllegalArgumentException("TenantId must be not null");
+        }
+        if (isEmpty(memberTriggerId)) {
+            throw new IllegalArgumentException("TriggerId must be not null");
+        }
+        if (isEmpty(dataIdMap)) {
+            throw new IllegalArgumentException("DatIdMap must be not null");
+        }
+
+        Session session = CassCluster.getSession();
+        PreparedStatement updateTriggerDataIdMap = CassStatement
+                .get(session, CassStatement.UPDATE_TRIGGER_DATA_ID_MAP);
+        if (updateTriggerDataIdMap == null) {
+            throw new RuntimeException("updateTriggerDataIdMap PreparedStatement is null");
+        }
+        try {
+            session.execute(updateTriggerDataIdMap.bind(dataIdMap, tenantId, memberTriggerId));
+        } catch (Exception e) {
+            msgLog.errorDatabaseException(e.getMessage());
+            throw e;
+        }
+    }
+
     private Collection<Condition> addCondition(Condition condition) throws Exception {
         String tenantId = condition.getTenantId();
         String triggerId = condition.getTriggerId();
@@ -2302,7 +2359,8 @@ private Condition mapCondition(Row row) throws Exception {
                     rCondition.setConditionSetIndex(row.getInt("conditionSetIndex"));
                     rCondition.setDataId(row.getString("dataId"));
                     rCondition.setOperatorLow(ThresholdRangeCondition.Operator.valueOf(row.getString("operatorLow")));
-                    rCondition.setOperatorHigh(ThresholdRangeCondition.Operator.valueOf(row.getString("operatorHigh")));
+                    rCondition
+                            .setOperatorHigh(ThresholdRangeCondition.Operator.valueOf(row.getString("operatorHigh")));
                     rCondition.setThresholdLow(row.getDouble("thresholdLow"));
                     rCondition.setThresholdHigh(row.getDouble("thresholdHigh"));
                     rCondition.setInRange(row.getBool("inRange"));

hawkular-alerts-engine/src/main/java/org/hawkular/alerts/engine/impl/CassStatement.java

@@ -149,6 +149,7 @@ public class CassStatement {
     public static final String UPDATE_DAMPENING_ID;
     public static final String UPDATE_EVENT;
     public static final String UPDATE_TRIGGER;
+    public static final String UPDATE_TRIGGER_DATA_ID_MAP;
     public static final String UPDATE_TRIGGER_ENABLED;
 
     static {
@@ -317,9 +318,9 @@ public class CassStatement {
 
         INSERT_TRIGGER = "INSERT INTO " + keyspace + ".triggers " +
                 "(tenantId, id, autoDisable, autoEnable, autoResolve, autoResolveAlerts, autoResolveMatch, "
-                + "context, description, enabled, eventCategory, eventText, eventType, firingMatch, memberOf, name, "
-                + "severity, source, tags, type) "
-                + "values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ";
+                + "context, dataIdMap, description, enabled, eventCategory, eventText, eventType, firingMatch, "
+                + "memberOf, name, severity, source, tags, type) "
+                + "values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ";
 
         INSERT_TRIGGER_ACTIONS = "INSERT INTO " + keyspace + ".triggers_actions "
                 + "(tenantId, triggerId, actionPlugin, actionId, payload) VALUES (?, ?, ?, ?, ?) ";
@@ -467,7 +468,7 @@ public class CassStatement {
                 + "WHERE tenantId = ? AND type = ? and name = ? AND value = ? ";
 
         SELECT_TRIGGER = "SELECT tenantId, id, autoDisable, autoEnable, autoResolve, autoResolveAlerts, "
-                + "autoResolveMatch, context, description, enabled, eventCategory, eventText, eventType, "
+                + "autoResolveMatch, context, dataIdMap, description, enabled, eventCategory, eventText, eventType, "
                 + "firingMatch, memberOf, name, severity, source, tags, type "
                 + "FROM " + keyspace + ".triggers "
                 + "WHERE tenantId = ? AND id = ? ";
@@ -501,12 +502,12 @@ public class CassStatement {
                 + "WHERE tenantId = ? AND triggerId = ? and triggerMode = ? ";
 
         SELECT_TRIGGERS_ALL = "SELECT tenantId, id, autoDisable, autoEnable, autoResolve, autoResolveAlerts, "
-                + "autoResolveMatch, context, description, enabled, eventCategory, eventText, eventType, "
+                + "autoResolveMatch, context, dataIdMap, description, enabled, eventCategory, eventText, eventType, "
                 + "firingMatch, memberOf, name, severity, source, tags, type "
                 + "FROM " + keyspace + ".triggers ";
 
         SELECT_TRIGGERS_TENANT = "SELECT tenantId, id, autoDisable, autoEnable, autoResolve, autoResolveAlerts, "
-                + "autoResolveMatch, context, description, enabled, eventCategory, eventText, eventType, "
+                + "autoResolveMatch, context, dataIdMap, description, enabled, eventCategory, eventText, eventType, "
                 + "firingMatch, memberOf, name, severity, source, tags, type "
                 + "FROM " + keyspace + ".triggers WHERE tenantId = ? ";
 
@@ -532,10 +533,13 @@ public class CassStatement {
 
         UPDATE_TRIGGER = "UPDATE " + keyspace + ".triggers "
                 + "SET autoDisable = ?, autoEnable = ?, autoResolve = ?, autoResolveAlerts = ?, autoResolveMatch = ?, "
-                + "context = ?, description = ?,  enabled = ?, eventCategory = ?, eventText = ?, firingMatch = ?, "
-                + "memberOf = ?, name = ?, severity = ?, source = ?, tags = ?, type = ? "
+                + "context = ?, dataIdMap = ?, description = ?,  enabled = ?, eventCategory = ?, eventText = ?, "
+                + "firingMatch = ?, memberOf = ?, name = ?, severity = ?, source = ?, tags = ?, type = ? "
                 + "WHERE tenantId = ? AND id = ? ";
 
+        UPDATE_TRIGGER_DATA_ID_MAP = "UPDATE " + keyspace + ".triggers "
+                + "SET dataIdMap = ? WHERE tenantId = ? AND id = ? ";
+
         UPDATE_TRIGGER_ENABLED = "UPDATE " + keyspace + ".triggers "
                 + "SET enabled = ? WHERE tenantId = ? AND id = ? ";
 

hawkular-alerts-engine/src/main/resources/hawkular-alerts-schema.cql

@@ -37,6 +37,7 @@ CREATE TABLE ${keyspace}.triggers (
     autoResolveAlerts boolean,
     autoResolveMatch text,
     context map<text,text>,
+    dataIdMap map<text,text>,
     description text,
     enabled boolean,
     eventCategory text,