modules/core/domain/src/main/java/org/rhq/core/domain/alert/AlertDefinition.java
| 30 ++-
modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/alert/definitions/TemplateAlertDefinitionsView.java
| 4
modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/gwt/AlertTemplateGWTService.java
| 4
modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/server/gwt/AlertTemplateGWTServiceImpl.java
| 4
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertDefinitionManagerBean.java
| 4
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertTemplateManagerBean.java
| 6
modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/alert/test/AlertConditionTest.java
| 78 +++++++++-
7 files changed, 109 insertions(+), 21 deletions(-)
New commits:
commit 2c5974fbda6a05ab0d7d233c190740d45df067d5
Author: Jay Shaughnessy <jshaughn(a)redhat.com>
Date: Fri Jun 22 16:09:36 2012 -0400
[830463 - Re-enabling (actually, updating) an alert definition clears condition logs
for existing alerts]
Some follow-on work. Add optimization such that if conditions are not being
updated we don't detach the AlertCondition from the def, since they are
unchanged.
- Add related unit testing
- re-enable the tests in AlertConditionTest, which pass but for some
reason were not enabled.
- Fix a few parameter names that I missed updating last time.
diff --git
a/modules/core/domain/src/main/java/org/rhq/core/domain/alert/AlertDefinition.java
b/modules/core/domain/src/main/java/org/rhq/core/domain/alert/AlertDefinition.java
index 397e254..71ca46c 100644
--- a/modules/core/domain/src/main/java/org/rhq/core/domain/alert/AlertDefinition.java
+++ b/modules/core/domain/src/main/java/org/rhq/core/domain/alert/AlertDefinition.java
@@ -62,7 +62,7 @@ import org.rhq.core.domain.resource.group.ResourceGroup;
* @author Joseph Marques
*/
@Entity
-@NamedQueries( { //
+@NamedQueries({ //
@NamedQuery(name = AlertDefinition.QUERY_FIND_ALL, query = "" //
+ "SELECT a " //
+ " FROM AlertDefinition a " //
@@ -409,6 +409,16 @@ public class AlertDefinition implements Serializable {
}
public void update(AlertDefinition alertDef) {
+ update(alertDef, true);
+ }
+
+ /**
+ * @param alertDef
+ * @param updateConditions set to true if conditions are being updated. This incurs
overhead because the
+ * old AlertConditions get detached and replaced for no good reason, becoming a
cleanup burden later on. Only
+ * set to false if you are sure the conditions are not being updated. If false the
conditions will not be updated.
+ */
+ public void update(AlertDefinition alertDef, boolean updateConditions) {
/*
* Don't copy the id, ctime, or mtime.
*/
@@ -440,15 +450,17 @@ public class AlertDefinition implements Serializable {
this.recoveryId = alertDef.recoveryId;
- // copy conditions
- List<AlertCondition> copiedConditions = new
ArrayList<AlertCondition>();
- for (AlertCondition oldCondition : alertDef.getConditions()) {
- AlertCondition newCondition = new AlertCondition(oldCondition);
- newCondition.setAlertDefinition(this);
- copiedConditions.add(newCondition);
+ // copy conditions if necessary
+ if (updateConditions) {
+ List<AlertCondition> copiedConditions = new
ArrayList<AlertCondition>();
+ for (AlertCondition oldCondition : alertDef.getConditions()) {
+ AlertCondition newCondition = new AlertCondition(oldCondition);
+ newCondition.setAlertDefinition(this);
+ copiedConditions.add(newCondition);
+ }
+ this.removeAllConditions();
+ this.getConditions().addAll(copiedConditions);
}
- this.removeAllConditions();
- this.getConditions().addAll(copiedConditions);
// copy notifications
List<AlertNotification> copiedNotifications = new
ArrayList<AlertNotification>();
diff --git
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/alert/definitions/TemplateAlertDefinitionsView.java
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/alert/definitions/TemplateAlertDefinitionsView.java
index 987e62f..0c710b4 100644
---
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/alert/definitions/TemplateAlertDefinitionsView.java
+++
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/alert/definitions/TemplateAlertDefinitionsView.java
@@ -206,7 +206,7 @@ public class TemplateAlertDefinitionsView extends
AbstractAlertDefinitionsView {
}
@Override
- protected void commitAlertDefinition(final AlertDefinition alertDefinition, boolean
purgeInternals) {
+ protected void commitAlertDefinition(final AlertDefinition alertDefinition, boolean
resetMatching) {
if (alertDefinition.getId() == 0) {
GWTServiceLookup.getAlertTemplateService().createAlertTemplate(alertDefinition,
Integer.valueOf(this.resourceType.getId()), new
AsyncCallback<Integer>() {
@@ -224,7 +224,7 @@ public class TemplateAlertDefinitionsView extends
AbstractAlertDefinitionsView {
}
});
} else {
-
GWTServiceLookup.getAlertTemplateService().updateAlertTemplate(alertDefinition,
purgeInternals,
+
GWTServiceLookup.getAlertTemplateService().updateAlertTemplate(alertDefinition,
resetMatching,
new AsyncCallback<AlertDefinition>() {
@Override
public void onSuccess(AlertDefinition result) {
diff --git
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/gwt/AlertTemplateGWTService.java
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/gwt/AlertTemplateGWTService.java
index e9af2cc..0a7f27b 100644
---
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/gwt/AlertTemplateGWTService.java
+++
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/gwt/AlertTemplateGWTService.java
@@ -30,11 +30,11 @@ public interface AlertTemplateGWTService extends RemoteService {
* Updates a alert template definition.
*
* @param alertDefinition
- * @param purgeInternals must be true if you are updating conditions or dampening
settings, can be false otherwise
+ * @param resetMatching must be true if you are updating conditions, condition
expression, or dampening settings, can be false otherwise
* @return the updated definition
* @throws Exception
*/
- AlertDefinition updateAlertTemplate(AlertDefinition alertDefinition, boolean
purgeInternals)
+ AlertDefinition updateAlertTemplate(AlertDefinition alertDefinition, boolean
resetMatching)
throws RuntimeException;
void enableAlertTemplates(Integer[] alertDefinitionIds) throws RuntimeException;
diff --git
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/server/gwt/AlertTemplateGWTServiceImpl.java
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/server/gwt/AlertTemplateGWTServiceImpl.java
index ceddfa7..7e1fd44 100644
---
a/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/server/gwt/AlertTemplateGWTServiceImpl.java
+++
b/modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/server/gwt/AlertTemplateGWTServiceImpl.java
@@ -41,11 +41,11 @@ public class AlertTemplateGWTServiceImpl extends
AbstractGWTServiceImpl implemen
}
@Override
- public AlertDefinition updateAlertTemplate(AlertDefinition alertDefinition, boolean
purgeInternals)
+ public AlertDefinition updateAlertTemplate(AlertDefinition alertDefinition, boolean
resetMatching)
throws RuntimeException {
try {
AlertDefinition results =
alertTemplateManager.updateAlertTemplate(getSessionSubject(), alertDefinition,
- purgeInternals);
+ resetMatching);
return SerialUtility.prepare(results,
"AlertTemplateService.updateAlertTemplate");
} catch (Throwable t) {
throw getExceptionToThrowToClient(t);
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertDefinitionManagerBean.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertDefinitionManagerBean.java
index 5da131e..c4fdcda 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertDefinitionManagerBean.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertDefinitionManagerBean.java
@@ -60,8 +60,8 @@ import org.rhq.enterprise.server.cloud.StatusManagerLocal;
import org.rhq.enterprise.server.plugin.pc.alert.AlertSender;
import org.rhq.enterprise.server.plugin.pc.alert.AlertSenderPluginManager;
import org.rhq.enterprise.server.util.CriteriaQueryGenerator;
-import org.rhq.enterprise.server.util.CriteriaQueryRunner;
import org.rhq.enterprise.server.util.CriteriaQueryGenerator.AuthorizationTokenType;
+import org.rhq.enterprise.server.util.CriteriaQueryRunner;
/**
* @author Joseph Marques
@@ -519,7 +519,7 @@ public class AlertDefinitionManagerBean implements
AlertDefinitionManagerLocal,
alertDefinition.setConditionExpression(BooleanExpression.ANY);
}
- oldAlertDefinition.update(alertDefinition);
+ oldAlertDefinition.update(alertDefinition, resetMatching);
if (LOG.isDebugEnabled()) {
LOG.debug("Updating: " + oldAlertDefinition);
for (AlertCondition nextCondition : oldAlertDefinition.getConditions()) {
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertTemplateManagerBean.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertTemplateManagerBean.java
index 94f1d35..5b4ad77 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertTemplateManagerBean.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/alert/AlertTemplateManagerBean.java
@@ -231,7 +231,7 @@ public class AlertTemplateManagerBean implements
AlertTemplateManagerLocal {
@RequiredPermission(Permission.MANAGE_SETTINGS)
@TransactionAttribute(TransactionAttributeType.NOT_SUPPORTED)
- public AlertDefinition updateAlertTemplate(Subject user, AlertDefinition
alertTemplate, boolean purgeInternals)
+ public AlertDefinition updateAlertTemplate(Subject user, AlertDefinition
alertTemplate, boolean resetMatching)
throws InvalidAlertDefinitionException, AlertDefinitionUpdateException,
AlertNotificationValidationException {
if (LOG.isDebugEnabled()) {
LOG.debug("updateAlertTemplate: " + alertTemplate);
@@ -241,7 +241,7 @@ public class AlertTemplateManagerBean implements
AlertTemplateManagerLocal {
AlertDefinition updated = null;
try {
updated = alertDefinitionManager.updateAlertDefinition(user,
alertTemplate.getId(), alertTemplate,
- purgeInternals); // do not allow direct undeletes of an alert definition
+ resetMatching); // do not allow direct undeletes of an alert definition
} catch (Throwable t) {
throw new AlertDefinitionUpdateException("Failed to update an
AlertTemplate "
+ alertTemplate.toSimpleString(), t);
@@ -260,7 +260,7 @@ public class AlertTemplateManagerBean implements
AlertTemplateManagerLocal {
for (Integer alertDefinitionId : alertDefinitions) {
try {
alertDefinitionManager
- .updateAlertDefinition(overlord, alertDefinitionId, alertTemplate,
purgeInternals);
+ .updateAlertDefinition(overlord, alertDefinitionId, alertTemplate,
resetMatching);
} catch (Throwable t) {
// continue on error, update as many as possible
if (firstThrowable == null) {
diff --git
a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/alert/test/AlertConditionTest.java
b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/alert/test/AlertConditionTest.java
index 645d049..47ba274 100644
---
a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/alert/test/AlertConditionTest.java
+++
b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/alert/test/AlertConditionTest.java
@@ -20,12 +20,14 @@
package org.rhq.enterprise.server.alert.test;
import java.util.HashSet;
+import java.util.List;
import java.util.Set;
import javax.persistence.EntityManager;
import javax.persistence.NoResultException;
import javax.transaction.TransactionManager;
+import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
@@ -70,7 +72,7 @@ import org.rhq.test.TransactionCallbackWithContext;
@Test
public class AlertConditionTest extends UpdatePluginMetadataTestBase {
- private static final boolean ENABLED = false;
+ private static final boolean ENABLED = true;
// this must match the constant found in ServerManagerBean
private static final String RHQ_SERVER_NAME_PROPERTY =
"rhq.server.high-availability.name";
@@ -343,10 +345,84 @@ public class AlertConditionTest extends UpdatePluginMetadataTestBase
{
}
}
+ @Test(enabled = ENABLED)
+ public void testBZ830463_updateDef() throws Exception {
+ // create our resource with alert definition
+ MeasurementDefinition metricDef = createResourceWithMetricSchedule();
+ AlertDefinition alertDef =
createAlertDefinitionWithOneInsideRangeCondition(metricDef, resource.getId());
+
+ assert alertDef.getConditions().size() == 1 : "1 alertDef condition should
exist";
+ AlertCondition condition = alertDef.getConditions().iterator().next();
+ int conditionId = condition.getId();
+
+ // re-load the resource so we get the measurement schedule
+ Resource resourceWithSchedules = loadResourceWithSchedules(resource.getId());
+ MeasurementSchedule schedule =
resourceWithSchedules.getSchedules().iterator().next();
+
+ // simulate a measurement report coming from the agent - one values that fits in
our range, so 1 alert is fired
+ MeasurementScheduleRequest request = new MeasurementScheduleRequest(schedule);
+ MeasurementReport report = new MeasurementReport();
+ report.addData(new MeasurementDataNumeric(getTimestamp(15), request,
Double.valueOf(50.0))); // 50 < 60 AND 50 > 40
+ MeasurementDataManagerLocal dataManager =
LookupUtil.getMeasurementDataManager();
+ dataManager.mergeMeasurementReport(report);
+
+ // wait for our JMS messages to process and see if we get any alerts
+ Thread.sleep(5000);
+
+ // make sure one alert was triggered
+ List<Alert> alerts = getAlerts(resourceWithSchedules.getId());
+ assert alerts.size() == 1 : "1 alert should have fired: " + alerts;
+ Alert alert = alerts.get(0);
+
+ assert alert.getConditionLogs().size() == 1 : "1 condition log should
exist";
+ AlertConditionLog conditionLog = alert.getConditionLogs().iterator().next();
+ Assert.assertEquals(conditionLog.getCondition().getId(), conditionId,
+ "original condition should have been associated with the alert");
+
+ // update a non-condition aspect of the def and then update the def
+ String updatedDesc = "Updated Description";
+ alertDef.setDescription(updatedDesc);
+ AlertDefinition updatedAlertDef =
LookupUtil.getAlertDefinitionManager().updateAlertDefinition(getOverlord(),
+ alertDef.getId(), alertDef, false); // note that resetMatching is false
+ assert updatedDesc.equals(updatedAlertDef.getDescription()) : "Description
should be updated";
+ assert updatedAlertDef.getConditions().size() == 1 : "1 alertDef condition
should exist after the update";
+ assert updatedAlertDef.getConditions().iterator().next().getId() ==
condition.getId() : "condition should not be updated";
+
+ // get the alert again, and make sure it still has a log with the same condition
+ alerts = getAlerts(resourceWithSchedules.getId());
+ assert alerts.size() == 1 : "1 alert should have fired: " + alerts;
+ alert = alerts.get(0);
+
+ assert alert.getConditionLogs().size() == 1 : "1 condition log should exist
after the update";
+ conditionLog = alert.getConditionLogs().iterator().next();
+ Assert.assertEquals(conditionLog.getCondition().getId(), conditionId,
+ "original condition should still have been associated with the
alert");
+
+ // update the condition on the def and then update the def
+ condition.setThreshold(Double.valueOf(41.0));
+ updatedAlertDef =
LookupUtil.getAlertDefinitionManager().updateAlertDefinition(getOverlord(),
alertDef.getId(),
+ alertDef, true); // note that resetMatching is true
+ assert updatedAlertDef.getConditions().size() == 1 : "1 alertDef condition
should exist after the update";
+ assert updatedAlertDef.getConditions().iterator().next().getId() !=
condition.getId() : "condition should be updated";
+
+ // get the alert again, and make sure it still has a log with the same condition
+ alerts = getAlerts(resourceWithSchedules.getId());
+ assert alerts.size() == 1 : "1 alert should have fired: " + alerts;
+ alert = alerts.get(0);
+
+ assert alert.getConditionLogs().size() == 1 : "1 condition log should exist
after the update";
+ conditionLog = alert.getConditionLogs().iterator().next();
+ Assert.assertEquals(conditionLog.getCondition().getId(), conditionId,
+ "original condition should still have been associated with the
alert");
+
+ return;
+ }
+
private PageList<Alert> getAlerts(int resourceId) {
AlertManagerLocal alertManager = LookupUtil.getAlertManager();
AlertCriteria alertCriteria = new AlertCriteria();
alertCriteria.addFilterResourceIds(resourceId);
+ alertCriteria.fetchConditionLogs(true);
PageList<Alert> alerts = alertManager.findAlertsByCriteria(getOverlord(),
alertCriteria);
return alerts;
}