mooli tayer has uploaded a new change for review. Change subject: tools: more changes in NotificationProperties ......................................................................
tools: more changes in NotificationProperties split validate into sub calls (validateBasic, validateEmail ...) to sections. as more sections are expected in following commits (validateFilter, validateSnmp ). Order methods (constructor befor any other method). Apply code style & add requireAll and requireOne methods. Change-Id: I2ebbf76a23c79d354cb9026dc3ad0f4310a804be Signed-off-by: Mooli Tayer <mta...@redhat.com> --- M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java 1 file changed, 135 insertions(+), 83 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/24297/1 diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java index 9787f77..3da5534 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java @@ -1,10 +1,10 @@ package org.ovirt.engine.core.notifier.utils; -import java.net.InetAddress; -import javax.mail.internet.InternetAddress; - import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.utils.LocalConfig; + +import javax.mail.internet.InternetAddress; +import java.net.InetAddress; /** * Defines properties uses by the event notification service @@ -26,22 +26,21 @@ public static final String SSL_IGNORE_HOST_VERIFICATION = "SSL_IGNORE_HOST_VERIFICATION"; /** - * Comma separated list of recipients to be informed in case - * the notification service cannot connect to the DB. can be empty. + * Comma separated list of recipients to be informed in case the notification service cannot connect to the DB. can + * be empty. */ public static final String FAILED_QUERIES_NOTIFICATION_RECIPIENTS = "FAILED_QUERIES_NOTIFICATION_RECIPIENTS"; /** - * Send a notification email after first failure to fetch notifications, - * and then once every failedQueriesNotificationThreshold times. + * Send a notification email after first failure to fetch notifications, and then once every + * failedQueriesNotificationThreshold times. */ public static final String FAILED_QUERIES_NOTIFICATION_THRESHOLD = "FAILED_QUERIES_NOTIFICATION_THRESHOLD"; /** - * This parameter specifies how many days of old events are processed and - * sent when the notifier starts. If set to 2, for example, the notifier - * will process and send the events of the last two days, older events will - * just be marked as processed and won't be sent. + * This parameter specifies how many days of old events are processed and sent when the notifier starts. If set to + * 2, for example, the notifier will process and send the events of the last two days, older events will just be + * marked as processed and won't be sent. */ public static final String DAYS_TO_SEND_ON_STARTUP = "DAYS_TO_SEND_ON_STARTUP"; @@ -72,28 +71,14 @@ */ public static final String MAIL_SMTP_ENCRYPTION_TLS = "tls"; + private static final String GENERIC_MESSAGE = "Check configuration file, "; + // Default files for defaults and overridden values: private static String DEFAULTS_PATH = "/usr/share/ovirt-engine/conf/notifier.conf.defaults"; private static String VARS_PATH = "/etc/ovirt-engine/notifier/notifier.conf"; // This is a singleton and this is the instance: private static NotificationProperties instance; - - public static synchronized NotificationProperties getInstance() { - if (instance == null) { - instance = new NotificationProperties(); - } - return instance; - } - - public static void setDefaults(String defaultsPath, String varsPath) { - DEFAULTS_PATH = defaultsPath; - VARS_PATH = varsPath; - } - - public static void release() { - instance = null; - } private NotificationProperties() { // Locate the defaults file and add it to the list: @@ -111,37 +96,54 @@ loadConfig(defaultsPath, varsPath); } + public static synchronized NotificationProperties getInstance() { + if (instance == null) { + instance = new NotificationProperties(); + } + return instance; + } + + public static void setDefaults(String defaultsPath, String varsPath) { + DEFAULTS_PATH = defaultsPath; + VARS_PATH = varsPath; + } + + public static void release() { + instance = null; + } + /** * Validates properties values. - * - * @throws IllegalArgumentException if some properties has invalid values + * + * @throws IllegalArgumentException + * if some properties has invalid values */ public void validate() { - // validate mandatory and non empty properties - for (String property : new String[]{ - NotificationProperties.DAYS_TO_KEEP_HISTORY, + validateCommon(); + + validateEmailBasic(); + if (isConfigured(NotificationProperties.MAIL_SERVER)) { + validateEmailAvailability(); + } + } + + private void validateCommon() { + requireAll(NotificationProperties.DAYS_TO_KEEP_HISTORY, NotificationProperties.DAYS_TO_SEND_ON_STARTUP, NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD, NotificationProperties.ENGINE_INTERVAL_IN_SECONDS, NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS, NotificationProperties.INTERVAL_IN_SECONDS, NotificationProperties.IS_HTTPS_PROTOCOL, - NotificationProperties.MAIL_PORT, - NotificationProperties.MAIL_SERVER, - NotificationProperties.REPEAT_NON_RESPONSIVE_NOTIFICATION }) { - if (StringUtils.isEmpty(getProperty(property))) { - throw new IllegalArgumentException( - String.format( - "Check configuration file, '%s' is missing", - property)); - } - } + NotificationProperties.REPEAT_NON_RESPONSIVE_NOTIFICATION); + // validate mandatory and non empty properties + requireOne(NotificationProperties.MAIL_SERVER); // validate non negative args - for (String property : new String[]{ + for (String property : new String[] { NotificationProperties.DAYS_TO_KEEP_HISTORY, NotificationProperties.DAYS_TO_SEND_ON_STARTUP, - NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD}) { + NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD }) { final String stringVal = getProperty(property); try { int value = Integer.parseInt(stringVal); @@ -155,18 +157,11 @@ property)); } } + } - if (!isSmtpEncryptionOptionValid()) { - throw new IllegalArgumentException( - String.format( - "Check configuration file, '%s' value has to be one of: '%s', '%s', '%s'.", - NotificationProperties.MAIL_SMTP_ENCRYPTION, - NotificationProperties.MAIL_SMTP_ENCRYPTION_NONE, - NotificationProperties.MAIL_SMTP_ENCRYPTION_SSL, - NotificationProperties.MAIL_SMTP_ENCRYPTION_TLS - )); - } - + private void validateEmailBasic() { + // validate MAIL_PORT + requireAll(MAIL_PORT); boolean mailPortValid = false; try { int port = new Integer(getProperty(MAIL_PORT)); @@ -182,15 +177,27 @@ getProperty(MAIL_PORT))); } - // try to resolve MAIL_SERVER host - try { - InetAddress.getAllByName(getProperty(NotificationProperties.MAIL_SERVER)); - } catch (Exception ex) { + // validate MAIL_USER value + String emailUser = getProperty(NotificationProperties.MAIL_USER, true); + if (StringUtils.isEmpty(emailUser) + && (MAIL_SMTP_ENCRYPTION_SSL.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) + || MAIL_SMTP_ENCRYPTION_TLS.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) + || StringUtils.isNotEmpty(getProperty(NotificationProperties.MAIL_PASSWORD, true)))) { throw new IllegalArgumentException( String.format( - "Check configuration file, cannot verify '%s' value", - NotificationProperties.MAIL_SERVER), - ex); + "'%s' must be set when SSL or TLS is enabled or when password is set", + NotificationProperties.MAIL_USER)); + } + + if (!isSmtpEncryptionOptionValid()) { + throw new IllegalArgumentException( + String.format( + GENERIC_MESSAGE + "'%s' value has to be one of: '%s', '%s', '%s'.", + NotificationProperties.MAIL_SMTP_ENCRYPTION, + NotificationProperties.MAIL_SMTP_ENCRYPTION_NONE, + NotificationProperties.MAIL_SMTP_ENCRYPTION_SSL, + NotificationProperties.MAIL_SMTP_ENCRYPTION_TLS + )); } // validate email addresses @@ -199,35 +206,80 @@ NotificationProperties.MAIL_FROM, NotificationProperties.MAIL_REPLY_TO }) { String candidate = getProperty(property); - if (!StringUtils.isEmpty(candidate)) { - try { - new InternetAddress(candidate); - } catch (Exception ex) { - throw new IllegalArgumentException( - String.format( - "Check configuration file, invalid format in '%s'", - property), - ex); - } - } + validateEmail(property, candidate); } + } - // validate mail user value - String emailUser = getProperty(NotificationProperties.MAIL_USER, true); - if (StringUtils.isEmpty(emailUser) - && (MAIL_SMTP_ENCRYPTION_SSL.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) - || MAIL_SMTP_ENCRYPTION_TLS.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) - || StringUtils.isNotEmpty(getProperty(NotificationProperties.MAIL_PASSWORD, true)))) { + public boolean isConfigured(String property) { + return !StringUtils.isEmpty(getProperty(property, true)); + } + + // Availability + private void validateEmailAvailability() { + // try to resolve MAIL_SERVER host + validateHost(NotificationProperties.MAIL_SERVER, getProperty(NotificationProperties.MAIL_SERVER)); + } + + private void validateHost(String propName, String propVal) { + try { + InetAddress.getAllByName(propVal); + } catch (Exception ex) { + throw new IllegalArgumentException( + String.format( + GENERIC_MESSAGE + "cannot verify '%s' value", + propName), + ex); + } + } + + private void requireAll(String... mandatoryProperties) { + for (String property : mandatoryProperties) { + if (StringUtils.isEmpty(getProperty(property, true))) { throw new IllegalArgumentException( String.format( - "'%s' must be set when SSL or TLS is enabled or when password is set", - NotificationProperties.MAIL_USER)); + GENERIC_MESSAGE + "'%s' is missing", + property)); + } + } + } + + private void requireOne(String... mandatoryProperties) { + boolean provided = false; + for (String property : mandatoryProperties) { + if (isConfigured(property)) { + provided = true; + } + } + if (!provided) { + StringBuilder sb = new StringBuilder(100); + sb.append(GENERIC_MESSAGE); + String prefix = " "; + for (String property : mandatoryProperties) { + sb.append(prefix).append(property); + prefix = " or "; + } + sb.append(" must be defined"); + throw new IllegalArgumentException( + sb.toString()); + } + } + + private void validateEmail(String propName, String propVal) { + if (!StringUtils.isEmpty(propVal)) { + try { + new InternetAddress(propVal); + } catch (Exception ex) { + throw new IllegalArgumentException( + String.format( + GENERIC_MESSAGE + "invalid format in '%s'", + propName), + ex); + } } } /** - * Returns {@code true} if mail transport encryption type {@link MAIL_SMTP_ENCRYPTION} is correctly specified, - * otherwise {@code false} + * Returns {@code true} if mail transport encryption type is correctly specified, otherwise {@code false} */ public boolean isSmtpEncryptionOptionValid() { return MAIL_SMTP_ENCRYPTION_NONE.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) -- To view, visit http://gerrit.ovirt.org/24297 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2ebbf76a23c79d354cb9026dc3ad0f4310a804be Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <mta...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches