Skip to content

Commit

Permalink
HWKALERTS-85 Feedback from review
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucasponce
lucasponce committed Sep 9, 2015
1 parent 924a738 commit 500589b
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 66 deletions.
6 changes: 3 additions & 3 deletions ...s-actions-aerogear/src/main/java/org/hawkular/alerts/actions/aerogear/AerogearPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
*/
@ActionPlugin(name = "aerogear")
public class AerogearPlugin implements ActionPluginListener {
static final String ROOT_SERVER_URL_PROPERTY = "org.hawkular.actions.aerogear.root.server.url";
static final String ROOT_SERVER_URL_PROPERTY = "org.hawkular.alerts.actions.aerogear.root.server.url";
static final String ROOT_SERVER_URL = System.getProperty(ROOT_SERVER_URL_PROPERTY);
static final String APPLICATION_ID_PROPERTY = "org.hawkular.actions.aerogear.application.id";
static final String APPLICATION_ID_PROPERTY = "org.hawkular.alerts.actions.aerogear.application.id";
static final String APPLICATION_ID = System.getProperty(APPLICATION_ID_PROPERTY);
static final String MASTER_SECRET_PROPERTY = "org.hawkular.actions.aerogear.master.secret";
static final String MASTER_SECRET_PROPERTY = "org.hawkular.alerts.actions.aerogear.master.secret";
static final String MASTER_SECRET = System.getProperty(MASTER_SECRET_PROPERTY);

private final MsgLogger msgLog = MsgLogger.LOGGER;
Expand Down
23 changes: 17 additions & 6 deletions ...lar-alerts-actions-email/src/main/java/org/hawkular/alerts/actions/email/EmailPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,23 @@
public class EmailPlugin implements ActionPluginListener {
public static final String PLUGIN_NAME = "email";

public static final String MAIL_SESSION_PROPERTY = "org.hawkular.actions.email.session";
public static final String MAIL_SESSION_PROPERTY = "org.hawkular.alerts.actions.email.session";

/**
* This property is used for testing porpuses.
* When present javax.mail.Session will not be initialized.
* This is useful on unit tests scenario where the objective is to validate the email composition instead of
* email transport cappabilities.
*/
public static final String MAIL_SESSION_OFFLINE = "org.hawkular.alerts.actions.email.session.offline";

public static final String MESSAGE_ID = "Message-ID";
public static final String IN_REPLY_TO = "in-reply-to";

public static final String DEFAULT_FROM_PROPERTY = "org.hawkular.actions.email.default.from";
public static final String DEFAULT_FROM_PROPERTY = "org.hawkular.alerts.actions.email.default.from";
public static final String DEFAULT_FROM = System.getProperty(DEFAULT_FROM_PROPERTY, "noreply@hawkular.org");

public static final String DEFAULT_FROM_NAME_PROPERTY = "org.hawkular.actions.email.default.from-name";
public static final String DEFAULT_FROM_NAME_PROPERTY = "org.hawkular.alerts.actions.email.default.from-name";
public static final String DEFAULT_FROM_NAME = System.getProperty(DEFAULT_FROM_NAME_PROPERTY, "Hawkular");

public static final String HAWKULAR_BASE_URL = "HAWKULAR_BASE_URL";
Expand Down Expand Up @@ -187,6 +195,12 @@ public EmailPlugin() {
defaultProperties.put(PROP_TEMPLATE_HTML, "");

emailTemplate = new EmailTemplate();

boolean offLine = System.getProperty(MAIL_SESSION_OFFLINE) != null;

if (mailSession == null && !offLine) {
initMailSession();
}
}

private void initMailSession() {
Expand All @@ -211,9 +225,6 @@ public Map<String, String> getDefaultProperties() {

@Override
public void process(PluginMessage msg) throws Exception {
if (mailSession == null) {
initMailSession();
}
Message message = createMimeMessage(msg);
Transport.send(message);

Expand Down
140 changes: 84 additions & 56 deletions ...tions-email/src/main/java/org/hawkular/alerts/actions/email/PluginMessageDescription.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,21 @@
*/
public class PluginMessageDescription {

/** Context property "resourceType". Supported at Trigger.getContext() level */
public static final String CONTEXT_PROPERTY_RESOURCE_TYPE = "resourceType";

/** Context property "resourceName". Supported at Trigger.getContext() level */
public static final String CONTEXT_PROPERTY_RESOURCE_NAME = "resourceName";

/** Context property "unit". Supported at Condition.getContext() level */
public static final String CONTEXT_PROPERTY_UNIT = "unit";

/** Context property "description". Supported at Condition.getContext() level */
public static final String CONTEXT_PROPERTY_DESCRIPTION = "description";

/** Context property "description". Supported at Condition.getContext() level with CompareCondition classes */
public static final String CONTEXT_PROPERTY_DESCRIPTION2 = "description2";

/** Shortcut for PluginMessage.getAction().message */
private String message;

Expand Down Expand Up @@ -202,10 +217,10 @@ public PluginMessageDescription(PluginMessage pm) {
trigger = alert.getTrigger();
if (trigger.getContext() != null &&
!trigger.getContext().isEmpty() &&
trigger.getContext().containsKey("resourceType") &&
trigger.getContext().containsKey("resourceName")) {
triggerDescription = trigger.getContext().get("resourceType") + " " +
trigger.getContext().get("resourceName");
trigger.getContext().containsKey(CONTEXT_PROPERTY_RESOURCE_TYPE) &&
trigger.getContext().containsKey(CONTEXT_PROPERTY_RESOURCE_NAME)) {
triggerDescription = trigger.getContext().get(CONTEXT_PROPERTY_RESOURCE_TYPE) + " " +
trigger.getContext().get(CONTEXT_PROPERTY_RESOURCE_NAME);
} else {
triggerDescription = trigger.getName();
}
Expand Down Expand Up @@ -249,6 +264,8 @@ private String dampeningDescription(Dampening d) {
case STRICT_TIMEOUT:
description += "after " + (d.getEvalTimeSetting()/1000) + " s";
break;
default:
throw new IllegalArgumentException(d.getType().name());
}
return description;
}
Expand Down Expand Up @@ -284,59 +301,62 @@ private void initConditions(Alert alert) {
for (int i = 0; i < numConditions; i++) {
ConditionDescription condDesc = conditions[i];
condDesc.average = ( condDesc.data.stream().reduce(0.0, (j,k) -> j+k ) ) / condDesc.data.size();
if (condDesc.condition.getContext().containsKey("unit")) {
if (condDesc.condition.getContext().containsKey(CONTEXT_PROPERTY_UNIT)) {
condDesc.averageDescription = decimalFormat.format(condDesc.average) + " " +
condDesc.condition.getContext().get("unit");
condDesc.condition.getContext().get(CONTEXT_PROPERTY_UNIT);
}
}
}

private Condition extractCondition(ConditionEval conditionEval) {
if (conditionEval == null) return null;
if (conditionEval instanceof AvailabilityConditionEval) {
return ((AvailabilityConditionEval) conditionEval).getCondition();
} else if (conditionEval instanceof CompareConditionEval) {
return ((CompareConditionEval) conditionEval).getCondition();
} else if (conditionEval instanceof ExternalConditionEval) {
return ((ExternalConditionEval) conditionEval).getCondition();
} else if (conditionEval instanceof StringConditionEval) {
return ((StringConditionEval) conditionEval).getCondition();
} else if (conditionEval instanceof ThresholdConditionEval) {
return ((ThresholdConditionEval) conditionEval).getCondition();
} else if (conditionEval instanceof ThresholdRangeConditionEval) {
return ((ThresholdRangeConditionEval) conditionEval).getCondition();
} else {
return null;
switch (conditionEval.getType()) {
case AVAILABILITY:
return ((AvailabilityConditionEval) conditionEval).getCondition();
case COMPARE:
return ((CompareConditionEval) conditionEval).getCondition();
case EXTERNAL:
return ((ExternalConditionEval) conditionEval).getCondition();
case STRING:
return ((StringConditionEval) conditionEval).getCondition();
case THRESHOLD:
return ((ThresholdConditionEval) conditionEval).getCondition();
case RANGE:
return ((ThresholdRangeConditionEval) conditionEval).getCondition();
default:
return null;
}
}

private Double extractValue(ConditionEval conditionEval) {
if (conditionEval == null) return 0d;
if (conditionEval instanceof ThresholdConditionEval) {
return ((ThresholdConditionEval) conditionEval).getValue();
} else if (conditionEval instanceof ThresholdRangeConditionEval) {
return ((ThresholdRangeConditionEval) conditionEval).getValue();
} else {
return 0d;
switch (conditionEval.getType()) {
case THRESHOLD:
return ((ThresholdConditionEval) conditionEval).getValue();
case RANGE:
return ((ThresholdRangeConditionEval) conditionEval).getValue();
default:
return 0d;
}
}

private String description(Condition condition) {
if (condition == null) return null;
if (condition instanceof AvailabilityCondition) {
return availability((AvailabilityCondition) condition);
} else if (condition instanceof CompareCondition) {
return compare((CompareCondition) condition);
} else if (condition instanceof ExternalCondition) {
return external((ExternalCondition) condition);
} else if (condition instanceof StringCondition) {
return string((StringCondition) condition);
} else if (condition instanceof ThresholdCondition) {
return threshold((ThresholdCondition) condition);
} else if (condition instanceof ThresholdRangeCondition) {
return range((ThresholdRangeCondition) condition);
} else {
return null;
switch (condition.getType()) {
case AVAILABILITY:
return availability((AvailabilityCondition) condition);
case COMPARE:
return compare((CompareCondition) condition);
case EXTERNAL:
return external((ExternalCondition) condition);
case STRING:
return string((StringCondition) condition);
case THRESHOLD:
return threshold((ThresholdCondition) condition);
case RANGE:
return range((ThresholdRangeCondition) condition);
default:
return null;
}
}

Expand All @@ -352,8 +372,8 @@ private String description(Condition condition) {
*/
public String availability(AvailabilityCondition condition) {
String description;
if (condition.getContext() != null && condition.getContext().get("description") != null) {
description = condition.getContext().get("description");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION) != null) {
description = condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION);
} else {
description = condition.getDataId();
}
Expand All @@ -368,6 +388,8 @@ public String availability(AvailabilityCondition condition) {
case UP:
description += " is up";
break;
default:
throw new IllegalArgumentException(operator.name());
}
return description;
}
Expand All @@ -388,8 +410,8 @@ public String availability(AvailabilityCondition condition) {
*/
public String compare(CompareCondition condition) {
String description;
if (condition.getContext() != null && condition.getContext().get("description") != null) {
description = condition.getContext().get("description");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION) != null) {
description = condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION);
} else {
description = condition.getDataId();
}
Expand All @@ -407,12 +429,14 @@ public String compare(CompareCondition condition) {
case GTE:
description += " greater or equals than ";
break;
default:
throw new IllegalArgumentException(operator.name());
}
if (condition.getData2Multiplier() != 1.0) {
description += "( " + condition.getData2Multiplier() + " ";
}
if (condition.getContext() != null && condition.getContext().get("description2") != null) {
description += condition.getContext().get("description2");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION2) != null) {
description += condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION2);
} else {
description += condition.getData2Id();
}
Expand Down Expand Up @@ -447,8 +471,8 @@ public String external(ExternalCondition condition) {
*/
public String string(StringCondition condition) {
String description;
if (condition.getContext() != null && condition.getContext().get("description") != null) {
description = condition.getContext().get("description");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION) != null) {
description = condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION);
} else {
description = condition.getDataId();
}
Expand All @@ -472,6 +496,8 @@ public String string(StringCondition condition) {
case MATCH:
description += "matches to ";
break;
default:
throw new IllegalArgumentException(operator.name());
}
description += condition.getPattern();
if (condition.isIgnoreCase()) {
Expand All @@ -495,8 +521,8 @@ public String string(StringCondition condition) {
*/
public String threshold(ThresholdCondition condition) {
String description;
if (condition.getContext() != null && condition.getContext().get("description") != null) {
description = condition.getContext().get("description");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION) != null) {
description = condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION);
} else {
description = condition.getDataId();
}
Expand All @@ -513,10 +539,12 @@ public String threshold(ThresholdCondition condition) {
case LTE:
description += " less or equal than ";
break;
default:
throw new IllegalArgumentException(condition.getOperator().name());
}
description += condition.getThreshold();
if (condition.getContext() != null && condition.getContext().get("unit") != null) {
description += " " + condition.getContext().get("unit");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_UNIT) != null) {
description += " " + condition.getContext().get(CONTEXT_PROPERTY_UNIT);
} else {
description += " (threshold)";
}
Expand All @@ -538,8 +566,8 @@ public String threshold(ThresholdCondition condition) {
*/
public String range(ThresholdRangeCondition condition) {
String description;
if (condition.getContext() != null && condition.getContext().get("description") != null) {
description = condition.getContext().get("description");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION) != null) {
description = condition.getContext().get(CONTEXT_PROPERTY_DESCRIPTION);
} else {
description = condition.getDataId();
}
Expand All @@ -563,8 +591,8 @@ public String range(ThresholdRangeCondition condition) {
} else {
description += ")";
}
if (condition.getContext() != null && condition.getContext().get("unit") != null) {
description += " " + condition.getContext().get("unit");
if (condition.getContext() != null && condition.getContext().get(CONTEXT_PROPERTY_UNIT) != null) {
description += " " + condition.getContext().get(CONTEXT_PROPERTY_UNIT);
} else {
description += " (range)";
}
Expand Down
7 changes: 7 additions & 0 deletions ...ular-alerts-actions-email/src/test/java/org/hawkular/alerts/actions/email/CommonTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import javax.mail.Message;

import org.junit.BeforeClass;

/**
* Helper methods for tests
*
Expand All @@ -29,6 +31,11 @@
*/
public class CommonTest {

@BeforeClass
public static void setUnitTest() {
System.setProperty(EmailPlugin.MAIL_SESSION_OFFLINE, "true");
}

protected void writeEmailFile(Message msg, String fileName) throws Exception {
File dir = new File("target/test-emails");
if (!dir.exists()) {
Expand Down
2 changes: 1 addition & 1 deletion ...wkular-alerts-actions-file/src/main/java/org/hawkular/alerts/actions/file/FilePlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class FilePlugin implements ActionPluginListener {

public FilePlugin() {
defaultProperties.put("path",
new File(System.getProperty("java.io.tmpdir"), "hawkular/actions/file").getAbsolutePath());
new File(System.getProperty("java.io.tmpdir"), "hawkular/alerts/actions/file").getAbsolutePath());
objectMapper = new ObjectMapper();
}

Expand Down

0 comments on commit 500589b

Please sign in to comment.