Alon Bar-Lev has uploaded a new change for review. Change subject: notifier: remove NotificationConfigurator as superseded by LocalConfig ......................................................................
notifier: remove NotificationConfigurator as superseded by LocalConfig cleanup validations of parameters to be acquired the LocalConfig way. move most of validation to main function in more structural manner. Change-Id: I9f8ebea59fabac945b95b6422d1bfbf8a2b1d208 Signed-off-by: Alon Bar-Lev <[email protected]> --- M .gitignore M Makefile M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/EngineMonitorService.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/Notifier.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodFactoryEmailImpl.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodMapBuilder.java D backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationConfigurator.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/JavaMailSender.java M backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/EngineMonitorServiceTest.java M backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/NotificationServiceTest.java D backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationConfiguratorTest.java A backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationPropertiesTest.java M backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java A backend/manager/tools/src/test/resources/conf/notifier-mail-test-plain.conf A backend/manager/tools/src/test/resources/conf/notifier-mail-test-secured.conf R backend/manager/tools/src/test/resources/conf/notifier-prop-test.conf R packaging/conf/notifier.conf.defaults.in 20 files changed, 199 insertions(+), 251 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/48/14448/1 diff --git a/.gitignore b/.gitignore index 6a70492..45f7398 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ ########################### packaging/bin/engine-prolog.sh packaging/conf/engine.conf.defaults +packaging/conf/notifier.conf.defaults packaging/etc/engine-config/log4j.xml packaging/etc/engine-manage-domains/log4j.xml packaging/etc/notifier/log4j.xml diff --git a/Makefile b/Makefile index a994bf4..7a8feb5 100644 --- a/Makefile +++ b/Makefile @@ -134,6 +134,7 @@ GENERATED = \ packaging/bin/engine-prolog.sh \ packaging/conf/engine.conf.defaults \ + packaging/conf/notifier.conf.defaults \ packaging/etc/engine-config/log4j.xml \ packaging/etc/engine-manage-domains/log4j.xml \ packaging/etc/notifier/log4j.xml \ diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/EngineMonitorService.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/EngineMonitorService.java index 62a653b..d55e400 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/EngineMonitorService.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/EngineMonitorService.java @@ -36,7 +36,6 @@ import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.config.ConfigCommon; import org.ovirt.engine.core.common.config.ConfigValues; -import org.ovirt.engine.core.notifier.utils.NotificationConfigurator; import org.ovirt.engine.core.notifier.utils.NotificationProperties; import org.ovirt.engine.core.tools.common.db.StandaloneDataSource; import org.ovirt.engine.core.utils.EngineLocalConfig; @@ -57,11 +56,8 @@ private static final String ENGINE_RESPONDING_MESSAGE = "Engine server is up and running."; private static final String HEALTH_SERVLET_PATH = "/OvirtEngineWeb/HealthStatus"; private static final String CERTIFICATION_TYPE = "PKCS12"; - private static final String DEFAULT_SSL_PROTOCOL = "TLS"; - private static final long DEFAULT_SERVER_MONITOR_TIMEOUT_IN_SECONDS = 30; - private static final int DEFAULT_SERVER_MONITOR_RETRIES = 3; private DataSource ds; - private Map<String, String> prop = null; + private NotificationProperties prop = null; private long serverMonitorTimeout; private URL serverUrl; private boolean isServerUp = true; @@ -84,50 +80,20 @@ * notification configuration contains service properties * @throws NotificationServiceException */ - public EngineMonitorService(NotificationConfigurator notificationConf) throws NotificationServiceException { - this.prop = notificationConf.getProperties(); + public EngineMonitorService(NotificationProperties prop) throws NotificationServiceException { + this.prop = prop; initConnectivity(); initServerConnectivity(); initServerMonitorInterval(); - initServerMonitorRetries(); - initPidFile(); + serverMonitorRetries = prop.getInteger(NotificationProperties.ENGINE_MONITOR_RETRIES); + pidFile = prop.getProperty(NotificationProperties.ENGINE_PID); // Boolean.valueOf always returns false unless gets a true expression. - repeatNonResponsiveNotification = - Boolean.valueOf(this.prop.get(NotificationProperties.REPEAT_NON_RESPONSIVE_NOTIFICATION)); + repeatNonResponsiveNotification = this.prop.getBoolean(NotificationProperties.REPEAT_NON_RESPONSIVE_NOTIFICATION); if (log.isDebugEnabled()) { log.debug(MessageFormat.format("Checking server status using {0}, {1}ignoring SSL errors.", isHttpsProtocol ? "HTTPS" : "HTTP", sslIgnoreCertErrors ? "" : "without ")); } - } - - private void initPidFile() { - pidFile = prop.get(NotificationProperties.ENGINE_PID); - if(pidFile == null) { - pidFile = NotificationProperties.DEFAULT_ENGINE_PID; - } - } - - /** - * Reads number of server monitoring retries for each iteration of the monitor service.<br> - * If a property wasn't configured, uses the default from {@code SERVER_MONITOR_RETRIES} - * @throws NotificationServiceException - * if a number is malformed - */ - private void initServerMonitorRetries() throws NotificationServiceException { - int retries; - if (prop.containsKey(NotificationProperties.ENGINE_MONITOR_RETRIES)) { - try { - retries = - NotificationConfigurator.extractNumericProperty(this.prop.get(NotificationProperties.ENGINE_MONITOR_RETRIES)); - } catch (NumberFormatException e) { - throw new NotificationServiceException(NotificationProperties.ENGINE_MONITOR_RETRIES - + " value must be a positive integer number"); - } - } else { - retries = DEFAULT_SERVER_MONITOR_RETRIES; - } - serverMonitorRetries = retries; } /** @@ -136,21 +102,10 @@ * misconfigured, throws exception. */ private void initServerMonitorInterval() throws NotificationServiceException { - long interval; - if (prop.containsKey(NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS)) { - String timeout = prop.get(NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS); - try { - interval = Long.valueOf(timeout); - if (interval < 0) { - throw new NotificationServiceException(NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS - + " value must be a positive integer number"); - } - } catch (NumberFormatException e) { - throw new NotificationServiceException(String.format("Invalid format of property [%s]", - NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS), e); - } - } else { - interval = DEFAULT_SERVER_MONITOR_TIMEOUT_IN_SECONDS; + long interval = prop.getLong(NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS); + if (interval < 0) { + throw new NotificationServiceException(NotificationProperties.ENGINE_TIMEOUT_IN_SECONDS + + " value must be a positive integer number"); } serverMonitorTimeout = TimeUnit.SECONDS.convert(interval, TimeUnit.MILLISECONDS); } @@ -162,9 +117,9 @@ * @throws NotificationServiceException */ private void initServerConnectivity() throws NotificationServiceException { - isHttpsProtocol = Boolean.valueOf(prop.get(NotificationProperties.IS_HTTPS_PROTOCOL)); - sslIgnoreCertErrors = Boolean.valueOf(prop.get(NotificationProperties.SSL_IGNORE_CERTIFICATE_ERRORS)); - sslIgnoreHostVerification = Boolean.valueOf(prop.get(NotificationProperties.SSL_IGNORE_HOST_VERIFICATION)); + isHttpsProtocol = prop.getBoolean(NotificationProperties.IS_HTTPS_PROTOCOL); + sslIgnoreCertErrors = prop.getBoolean(NotificationProperties.SSL_IGNORE_CERTIFICATE_ERRORS); + sslIgnoreHostVerification = prop.getBoolean(NotificationProperties.SSL_IGNORE_HOST_VERIFICATION); // Setting SSL_IGNORE_HOST_VERIFICATION in configuration file implies that SSL certification errors should be // ignored as well @@ -207,10 +162,7 @@ String keystoreUrl = config.getPKIEngineStore().getAbsolutePath(); try { - String sslProtocol = prop.get(NotificationProperties.SSL_PROTOCOL); - if (StringUtils.isEmpty(sslProtocol)) { - sslProtocol = DEFAULT_SSL_PROTOCOL; - } + String sslProtocol = prop.getProperty(NotificationProperties.SSL_PROTOCOL); KeyStore keyStore = EncryptionUtils.getKeyStore(keystoreUrl, keystorePass, CERTIFICATION_TYPE); TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); tmf.init(keyStore); @@ -229,7 +181,7 @@ */ private void createDummySSLSocketFactory() throws NotificationServiceException { try { - SSLContext sslContext = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL); + SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { new X509TrustManager() { @Override diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java index 6743a1d..b44070e 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java @@ -27,7 +27,6 @@ import org.ovirt.engine.core.notifier.methods.EventMethodFiller; import org.ovirt.engine.core.notifier.methods.NotificationMethodMapBuilder; import org.ovirt.engine.core.notifier.methods.NotificationMethodMapBuilder.NotificationMethodFactoryMapper; -import org.ovirt.engine.core.notifier.utils.NotificationConfigurator; import org.ovirt.engine.core.notifier.utils.NotificationProperties; import org.ovirt.engine.core.notifier.utils.sender.EventSender; import org.ovirt.engine.core.notifier.utils.sender.EventSenderResult; @@ -43,13 +42,13 @@ private static final Logger log = Logger.getLogger(NotificationService.class); private DataSource ds; - private Map<String, String> prop = null; + private NotificationProperties prop = null; private NotificationMethodFactoryMapper methodsMapper = null; private int daysToKeepHistory = 0; private int daysToSendOnStartup = 0; - public NotificationService(NotificationConfigurator notificationConf) throws NotificationServiceException { - this.prop = notificationConf.getProperties(); + public NotificationService(NotificationProperties prop) throws NotificationServiceException { + this.prop = prop; initConnectivity(); initConfigurationProperties(); initEvents(); @@ -72,7 +71,7 @@ private int getNonNegativeIntegerProperty(final String name) throws NotificationServiceException { // Get the text of the property: - final String text = prop.get(name); + final String text = prop.getProperty(name); // Validate it: if (StringUtils.isNotEmpty(text)) { diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/Notifier.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/Notifier.java index 78e2c93..066a60b 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/Notifier.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/Notifier.java @@ -1,14 +1,17 @@ package org.ovirt.engine.core.notifier; +import java.net.InetAddress; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import javax.mail.internet.InternetAddress; +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; -import org.ovirt.engine.core.notifier.utils.NotificationConfigurator; import org.ovirt.engine.core.notifier.utils.NotificationProperties; import sun.misc.Signal; @@ -38,38 +41,68 @@ NotificationService notificationService = null; EngineMonitorService engineMonitorService = null; - NotificationConfigurator notificationConf = null; long engineMonitorInterval; long notificationInterval; try { - notificationConf = new NotificationConfigurator(); + NotificationProperties prop = NotificationProperties.getInstance(); - // This check will be not mandatory when SMS is implemented. - String mailServer = notificationConf.getProperties().get(NotificationProperties.MAIL_SERVER); - if ( mailServer == null || mailServer.isEmpty() ) { - throw new IllegalArgumentException("Check configuration file, MAIL_SERVER is missing"); + for (String property : new String[] { + NotificationProperties.DAYS_TO_KEEP_HISTORY, + 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(prop.getProperty(property))) { + throw new IllegalArgumentException( + String.format( + "Check configuration file, '%s' is missing", + property + ) + ); + } } - notificationService = new NotificationService(notificationConf); - engineMonitorService = new EngineMonitorService(notificationConf); + InetAddress.getAllByName(prop.getProperty(NotificationProperties.MAIL_SERVER)); - notificationInterval = - notificationConf.getTimerInterval(NotificationProperties.INTERVAL_IN_SECONDS, - DEFAULT_NOTIFICATION_INTERVAL_IN_SECONDS); - engineMonitorInterval = - notificationConf.getTimerInterval(NotificationProperties.ENGINE_INTERVAL_IN_SECONDS, - DEFAULT_ENGINE_MONITOR_INTERVAL_IN_SECONDS); + for (String property : new String[] { + NotificationProperties.MAIL_USER, + NotificationProperties.MAIL_FROM, + NotificationProperties.MAIL_REPLY_TO, + }) { + String candidate = prop.getProperty(property); + if (!StringUtils.isEmpty(candidate)) { + try { + new InternetAddress(candidate); + } + catch(Exception e) { + throw new IllegalArgumentException( + String.format( + "Check configuration file, invalid format in '%s'", + property + ), + e + ); + } + } + } + + notificationService = new NotificationService(prop); + engineMonitorService = new EngineMonitorService(prop); // add notification service to scheduler with its configurable interval handler.addServiceHandler(notifyScheduler.scheduleWithFixedDelay(notificationService, 1, - notificationInterval, + prop.getLong(NotificationProperties.INTERVAL_IN_SECONDS), TimeUnit.SECONDS)); // add engine monitor service to scheduler with its configurable interval handler.addServiceHandler(monitorScheduler.scheduleWithFixedDelay(engineMonitorService, 1, - engineMonitorInterval, + prop.getLong(NotificationProperties.ENGINE_INTERVAL_IN_SECONDS), TimeUnit.SECONDS)); } catch (Exception e) { log.error("Failed to run the event notification service. ", e); diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodFactoryEmailImpl.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodFactoryEmailImpl.java index 9b74986..9bec28d 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodFactoryEmailImpl.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodFactoryEmailImpl.java @@ -1,7 +1,6 @@ package org.ovirt.engine.core.notifier.methods; -import java.util.Map; - +import org.ovirt.engine.core.notifier.utils.NotificationProperties; import org.ovirt.engine.core.notifier.utils.sender.mail.EventSenderMailImpl; /** @@ -14,7 +13,7 @@ private EventSenderMailImpl senderMailImpl = null; - public NotificationMethodFactoryEmailImpl(Map<String,String> properties) { + public NotificationMethodFactoryEmailImpl(NotificationProperties properties) { senderMailImpl = new EventSenderMailImpl(properties); } diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodMapBuilder.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodMapBuilder.java index 36e7cce..c39a0fd 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodMapBuilder.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/methods/NotificationMethodMapBuilder.java @@ -6,6 +6,7 @@ import org.ovirt.engine.core.common.EventNotificationMethods; import org.ovirt.engine.core.common.businessentities.EventNotificationMethod; +import org.ovirt.engine.core.notifier.utils.NotificationProperties; import org.ovirt.engine.core.notifier.utils.sender.EventSender; /** @@ -61,7 +62,7 @@ * @return a map of notification map and its factory */ public NotificationMethodFactoryMapper createMethodsMapper(List<EventNotificationMethod> notificationMethods, - Map<String, String> properties) { + NotificationProperties properties) { NotificationMethodFactoryMapper methodMapper = new NotificationMethodFactoryMapper(); for (EventNotificationMethod method : notificationMethods) { diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationConfigurator.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationConfigurator.java deleted file mode 100644 index 75fae30..0000000 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationConfigurator.java +++ /dev/null @@ -1,70 +0,0 @@ -package org.ovirt.engine.core.notifier.utils; - -import java.io.File; -import java.util.Map; - -import org.apache.commons.lang.StringUtils; -import org.apache.log4j.Logger; -import org.ovirt.engine.core.notifier.NotificationServiceException; - -/** - * The <code>NotificationConfigurator</class> reads and stores properties from a configuration file.<br> - * It provides simple properties querying for timer interval properties types, e.g. {@link #getTimerInterval(String)} - */ -public class NotificationConfigurator { - - private static final Logger log = Logger.getLogger(NotificationConfigurator.class); - private Map<String, String> prop = null; - - public NotificationConfigurator() { - prop = NotificationProperties.getInstance().getProperties(); - } - - /** - * Returns properties which read from file. - * - * @return - */ - public Map<String, String> getProperties() { - return prop; - } - - /** - * Gets a value for timer interval by a given property name - * @param intervalPropertyName - * an interval property key - * @param defaultInterval - * a default interval value - * @return an interval - * @throws NotificationServiceException - */ - public long getTimerInterval(String intervalPropertyName, long defaultInterval) throws NotificationServiceException { - long interval; - - if (!prop.containsKey(intervalPropertyName)) { - interval = defaultInterval; - log.info(String.format("%s property is missing, using default %d.", intervalPropertyName, defaultInterval)); - } else { - try { - interval = extractNumericProperty(prop.get(intervalPropertyName)); - } catch (NumberFormatException e) { - throw new NotificationServiceException("Invalid format for property: " + intervalPropertyName, e); - } - } - return interval; - } - - /** - * Extract a positive numeric property out of a property value - * @param intervalProp property value - * @return a numeric value represents the property - * @throws NotificationServiceException - */ - static public int extractNumericProperty(String intervalProp) throws NumberFormatException { - int interval = Integer.valueOf(intervalProp); - if (interval <= 0) { - throw new NumberFormatException(String.format("[%s] value should be a positive number", intervalProp)); - } - return interval; - } -} 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 b60cc3e..c26179f 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 @@ -43,7 +43,6 @@ public static final String SSL_IGNORE_CERTIFICATE_ERRORS = "SSL_IGNORE_CERTIFICATE_ERRORS"; public static final String SSL_IGNORE_HOST_VERIFICATION = "SSL_IGNORE_HOST_VERIFICATION"; public static final String ENGINE_PID = "ENGINE_PID"; - public static final String DEFAULT_ENGINE_PID = "/var/run/ovirt-engine.pid"; /** * This parameter specifies how many days of old events are processed and @@ -60,9 +59,12 @@ private static String VARS_PATH = "/etc/ovirt-engine/notifier/notifier.conf"; // This is a singleton and this is the instance: - private static final NotificationProperties instance = new NotificationProperties(); + private static NotificationProperties instance; - public static NotificationProperties getInstance() { + public static synchronized NotificationProperties getInstance() { + if (instance == null) { + instance = new NotificationProperties(); + } return instance; } @@ -71,6 +73,10 @@ VARS_PATH = varsPath; } + public static void release() { + instance = null; + } + private NotificationProperties() { // Locate the defaults file and add it to the list: String defaultsPath = System.getenv("ENGINE_NOTIFIER_DEFAULTS"); diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java index a733a2f..f44b642 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java @@ -3,7 +3,6 @@ import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.Map; import javax.mail.MessagingException; @@ -34,9 +33,9 @@ private String hostName; private boolean isBodyHtml = false; - public EventSenderMailImpl(Map<String, String> mailProp) { + public EventSenderMailImpl(NotificationProperties mailProp) { mailSender = new JavaMailSender(mailProp); - String isBodyHtmlStr = mailProp.get(NotificationProperties.HTML_MESSAGE_FORMAT); + String isBodyHtmlStr = mailProp.getProperty(NotificationProperties.HTML_MESSAGE_FORMAT); if (StringUtils.isNotEmpty(isBodyHtmlStr)) { isBodyHtml = Boolean.valueOf(isBodyHtmlStr); } diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/JavaMailSender.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/JavaMailSender.java index 5782d45..0c0463e 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/JavaMailSender.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/JavaMailSender.java @@ -38,15 +38,15 @@ * @param aMailProps * properties required for creating a mail session */ - public JavaMailSender(Map<String, String> aMailProps) { + public JavaMailSender(NotificationProperties aMailProps) { Properties mailSessionProps = setCommonProperties(aMailProps); - mailSessionProps.put("mail.smtp.host", aMailProps.get(NotificationProperties.MAIL_SERVER)); + mailSessionProps.put("mail.smtp.host", aMailProps.getProperty(NotificationProperties.MAIL_SERVER)); // enable SSL - if (Boolean.valueOf(aMailProps.get(NotificationProperties.MAIL_ENABLE_SSL))) { + if (aMailProps.getBoolean(NotificationProperties.MAIL_ENABLE_SSL, false)) { mailSessionProps.put("mail.transport.protocol", "smtps"); - mailSessionProps.put("mail.smtp.port", aMailProps.get(NotificationProperties.MAIL_PORT_SSL)); - mailSessionProps.put("mail.smtps.socketFactory.port", aMailProps.get(NotificationProperties.MAIL_PORT_SSL)); + mailSessionProps.put("mail.smtp.port", aMailProps.getProperty(NotificationProperties.MAIL_PORT_SSL)); + mailSessionProps.put("mail.smtps.socketFactory.port", aMailProps.getProperty(NotificationProperties.MAIL_PORT_SSL)); mailSessionProps.put("mail.smtps.auth", "true"); mailSessionProps.put("mail.smtps.socketFactory.class", "javax.net.ssl.SSLSocketFactory"); mailSessionProps.put("mail.smtps.socketFactory.fallback", false); @@ -54,14 +54,13 @@ this.isSSL = true; } else { mailSessionProps.put("mail.transport.protocol", "smtp"); - mailSessionProps.put("mail.smtp.port", aMailProps.get(NotificationProperties.MAIL_PORT)); + mailSessionProps.put("mail.smtp.port", aMailProps.getProperty(NotificationProperties.MAIL_PORT)); } - String password = aMailProps.get(NotificationProperties.MAIL_PASSWORD); - boolean isAuthenticated = StringUtils.isNotEmpty(password); - if (isAuthenticated) { - auth = new EmailAuthenticator(aMailProps.get(NotificationProperties.MAIL_USER), - aMailProps.get(NotificationProperties.MAIL_PASSWORD)); + String password = aMailProps.getProperty(NotificationProperties.MAIL_PASSWORD, true); + if (StringUtils.isNotEmpty(password)) { + auth = new EmailAuthenticator(aMailProps.getProperty(NotificationProperties.MAIL_USER, true), + password); this.session = Session.getDefaultInstance(mailSessionProps, auth); } else { this.session = Session.getInstance(mailSessionProps); @@ -75,21 +74,18 @@ * mail configuration properties * @return a session common properties */ - private Properties setCommonProperties(Map<String, String> aMailProps) { + private Properties setCommonProperties(NotificationProperties aMailProps) { Properties mailSessionProps = new Properties(); if (log.isTraceEnabled()) { mailSessionProps.put("mail.debug", "true"); } - String mailHost = aMailProps.get(NotificationProperties.MAIL_SERVER); - if (StringUtils.isEmpty(mailHost)) { - throw new IllegalArgumentException(NotificationProperties.MAIL_SERVER + " must not be null or empty"); - } + String mailHost = aMailProps.getProperty(NotificationProperties.MAIL_SERVER); - String emailUser = aMailProps.get(NotificationProperties.MAIL_USER); + String emailUser = aMailProps.getProperty(NotificationProperties.MAIL_USER, true); if (StringUtils.isEmpty(emailUser)) { - if (Boolean.valueOf(aMailProps.get(NotificationProperties.MAIL_ENABLE_SSL)) || - StringUtils.isNotEmpty(aMailProps.get(NotificationProperties.MAIL_PASSWORD))) { + if (aMailProps.getBoolean(NotificationProperties.MAIL_ENABLE_SSL, false) || + StringUtils.isNotEmpty(aMailProps.getProperty(NotificationProperties.MAIL_PASSWORD, true))) { throw new IllegalArgumentException(NotificationProperties.MAIL_USER + " must be set when SSL is enabled or when password is set"); } else { @@ -98,12 +94,12 @@ } } - if (StringUtils.isNotEmpty(aMailProps.get(NotificationProperties.MAIL_FROM))) { + if (StringUtils.isNotEmpty(aMailProps.getProperty(NotificationProperties.MAIL_FROM, true))) { try { - from = new InternetAddress(aMailProps.get(NotificationProperties.MAIL_FROM)); + from = new InternetAddress(aMailProps.getProperty(NotificationProperties.MAIL_FROM)); } catch (AddressException e) { log.error(MessageFormat.format("Failed to parse 'from' user {0} provided by property {1}", - aMailProps.get(NotificationProperties.MAIL_FROM), + aMailProps.getProperty(NotificationProperties.MAIL_FROM), NotificationProperties.MAIL_FROM), e); } } else { @@ -118,20 +114,18 @@ } } - if (StringUtils.isNotEmpty(aMailProps.get(NotificationProperties.MAIL_REPLY_TO))) { + if (StringUtils.isNotEmpty(aMailProps.getProperty(NotificationProperties.MAIL_REPLY_TO, true))) { try { - replyTo = new InternetAddress(aMailProps.get(NotificationProperties.MAIL_REPLY_TO)); + replyTo = new InternetAddress(aMailProps.getProperty(NotificationProperties.MAIL_REPLY_TO)); } catch (AddressException e) { log.error(MessageFormat.format("Failed to parse 'replyTo' email {0} provided by property {1}", - aMailProps.get(NotificationProperties.MAIL_REPLY_TO), + aMailProps.getProperty(NotificationProperties.MAIL_REPLY_TO), NotificationProperties.MAIL_REPLY_TO), e); } } - String isBodyHtmlStr = aMailProps.get(NotificationProperties.HTML_MESSAGE_FORMAT); - if (StringUtils.isNotEmpty(isBodyHtmlStr)) { - isBodyHtml = Boolean.valueOf(isBodyHtmlStr); - } + isBodyHtml = aMailProps.getBoolean(NotificationProperties.HTML_MESSAGE_FORMAT, false); + return mailSessionProps; } diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/EngineMonitorServiceTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/EngineMonitorServiceTest.java index 2b73aef..89ab4fe 100644 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/EngineMonitorServiceTest.java +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/EngineMonitorServiceTest.java @@ -4,7 +4,6 @@ import java.io.File; import org.junit.Test; -import org.ovirt.engine.core.notifier.utils.NotificationConfigurator; import org.ovirt.engine.core.notifier.utils.NotificationProperties; /** @@ -18,7 +17,7 @@ File config = new File("src/test/resources/conf/notifier.conf"); NotificationProperties.setDefaults(config.getAbsolutePath(), null); - engineMonitorService = new EngineMonitorService(new NotificationConfigurator()); + engineMonitorService = new EngineMonitorService(NotificationProperties.getInstance()); } catch (NotificationServiceException e) { e.printStackTrace(); } diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/NotificationServiceTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/NotificationServiceTest.java index c0bbe60..a66f226 100644 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/NotificationServiceTest.java +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/NotificationServiceTest.java @@ -4,7 +4,6 @@ import java.io.File; import org.junit.Test; -import org.ovirt.engine.core.notifier.utils.NotificationConfigurator; import org.ovirt.engine.core.notifier.utils.NotificationProperties; /** @@ -38,7 +37,7 @@ NotificationProperties.setDefaults(config.getAbsolutePath(), null); notificationService = - new NotificationService(new NotificationConfigurator()); + new NotificationService(NotificationProperties.getInstance()); } catch (NotificationServiceException e) { e.printStackTrace(); } diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationConfiguratorTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationConfiguratorTest.java deleted file mode 100644 index 5dd5d3c..0000000 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationConfiguratorTest.java +++ /dev/null @@ -1,51 +0,0 @@ -package org.ovirt.engine.core.notifier.utils; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - -import java.io.File; -import org.junit.BeforeClass; -import org.junit.Test; -import org.ovirt.engine.core.notifier.NotificationServiceException; - -public class NotificationConfiguratorTest { - - private static NotificationConfigurator config = null; - - @BeforeClass - static public void initConfigurator() { - File c = new File("src/test/resources/conf/notifier.conf"); - NotificationProperties.setDefaults(c.getAbsolutePath(), null); - - config = new NotificationConfigurator(); - assertNotNull(config); - } - - @Test - public void testConfiguration() { - NotificationConfigurator configurator = null; - configurator = new NotificationConfigurator(); - - assertNotNull(configurator); - - long timerInterval = -1; - try { - timerInterval = configurator.getTimerInterval(NotificationProperties.INTERVAL_IN_SECONDS, 10); - } catch (NotificationServiceException e) { - } - assertEquals(timerInterval, 60); - } - - @Test - public void testIntervalTimerWrongValue() { - boolean failure = false; - try { - config.getTimerInterval(NotificationProperties.INTERVAL_IN_SECONDS, 0); - } catch (NotificationServiceException e) { - failure = true; - } - assertTrue(failure); - } -} - diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationPropertiesTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationPropertiesTest.java new file mode 100644 index 0000000..e926abf --- /dev/null +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/NotificationPropertiesTest.java @@ -0,0 +1,31 @@ +package org.ovirt.engine.core.notifier.utils; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import org.junit.BeforeClass; +import org.junit.Test; +import org.ovirt.engine.core.notifier.NotificationServiceException; + +public class NotificationPropertiesTest { + + private static NotificationProperties prop = null; + + @BeforeClass + static public void beforeClass() { + NotificationProperties.setDefaults( + "src/test/resources/conf/notifier-prop-test.conf", + "src/test/resources/conf/missing.conf" + ); + prop = NotificationProperties.getInstance(); + assertNotNull(prop); + } + + @Test + public void testProperties() { + assertEquals(60, prop.getLong(NotificationProperties.INTERVAL_IN_SECONDS)); + } +} + diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java index 5330a22..f77c854 100644 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java @@ -6,6 +6,7 @@ import java.util.HashMap; import java.util.Map; +import org.junit.BeforeClass; import org.junit.Test; import org.ovirt.engine.core.common.businessentities.EventAuditLogSubscriber; import org.ovirt.engine.core.notifier.utils.NotificationProperties; @@ -18,6 +19,11 @@ * over SSL. */ public class MailSenderTest { + + @BeforeClass + public static void beforeClass() { + NotificationProperties.release(); + } /** * The test covers sending an email using non-secured mail client (SMTP) configuration. <br> @@ -40,7 +46,11 @@ eventData.setstorage_domain_name("a test storage pool domain"); eventData.setseverity(3); - EventSenderMailImpl mailSender = new EventSenderMailImpl(getMailProperties()); + NotificationProperties.setDefaults( + "src/test/resources/conf/notifier-mail-test-plain.conf", + "src/test/resources/conf/missing.conf" + ); + EventSenderMailImpl mailSender = new EventSenderMailImpl(NotificationProperties.getInstance()); eventData.setmessage("a test message to be sent via non-secured mode"); EventSenderResult sentResult = null; try { @@ -73,7 +83,11 @@ eventData.setstorage_domain_name("a test storage pool domain"); eventData.setseverity(3); - EventSenderMailImpl mailSender = new EventSenderMailImpl(getSecuredMailProperties()); + NotificationProperties.setDefaults( + "src/test/resources/conf/notifier-mail-test-secured.conf", + "src/test/resources/conf/missing.conf" + ); + EventSenderMailImpl mailSender = new EventSenderMailImpl(NotificationProperties.getInstance()); eventData.setmessage("a test message to be sent via secured mode"); mailSender.send(eventData, null); diff --git a/backend/manager/tools/src/test/resources/conf/notifier-mail-test-plain.conf b/backend/manager/tools/src/test/resources/conf/notifier-mail-test-plain.conf new file mode 100644 index 0000000..29605ac --- /dev/null +++ b/backend/manager/tools/src/test/resources/conf/notifier-mail-test-plain.conf @@ -0,0 +1,18 @@ +# Notification settings +INTERVAL_IN_SECONDS=60 + +# Engine server monitor settings +ENGINE_INTERVAL_IN_SECONDS=300 +ENGINE_TIMEOUT_IN_SECONDS=30 + +# Days to keep history in the event_notification_history table +DAYS_TO_KEEP_HISTORY=30 + +# oVirt lib location +engineLib=/usr/local/jboss-eap-5.1/jboss-as/server/default/deploy/engine.ear/lib + +MAIL_SERVER=smtp.redhat.com +MAIL_PORT=25 +MAIL_PORT_SSL=465 [email protected] +HTML_MESSAGE_FORMAT=true diff --git a/backend/manager/tools/src/test/resources/conf/notifier-mail-test-secured.conf b/backend/manager/tools/src/test/resources/conf/notifier-mail-test-secured.conf new file mode 100644 index 0000000..d57face --- /dev/null +++ b/backend/manager/tools/src/test/resources/conf/notifier-mail-test-secured.conf @@ -0,0 +1,20 @@ +# Notification settings +INTERVAL_IN_SECONDS=60 + +# Engine server monitor settings +ENGINE_INTERVAL_IN_SECONDS=300 +ENGINE_TIMEOUT_IN_SECONDS=30 + +# Days to keep history in the event_notification_history table +DAYS_TO_KEEP_HISTORY=30 + +# oVirt lib location +engineLib=/usr/local/jboss-eap-5.1/jboss-as/server/default/deploy/engine.ear/lib + +MAIL_SERVER=smtp.gmail.com +MAIL_PORT=25 +MAIL_PORT_SSL=465 [email protected] +MAIL_PASSWORD="q1!w2@e3#!" [email protected] +MAIL_ENABLE_SSL=true diff --git a/backend/manager/tools/src/test/resources/conf/notifier.conf b/backend/manager/tools/src/test/resources/conf/notifier-prop-test.conf similarity index 100% rename from backend/manager/tools/src/test/resources/conf/notifier.conf rename to backend/manager/tools/src/test/resources/conf/notifier-prop-test.conf diff --git a/packaging/conf/notifier.conf.defaults b/packaging/conf/notifier.conf.defaults.in similarity index 97% rename from packaging/conf/notifier.conf.defaults rename to packaging/conf/notifier.conf.defaults.in index fee57bb..c0e5273 100644 --- a/packaging/conf/notifier.conf.defaults +++ b/packaging/conf/notifier.conf.defaults.in @@ -75,3 +75,6 @@ # Specifies whether to repeat auditing of failure messages of non-responding engine server. false means # repeated failure messages will NOT be sent to the subscribers. REPEAT_NON_RESPONSIVE_NOTIFICATION=false + +# Location of engine pid +ENGINE_PID=@ENGINE_VAR@/ovirt-engine.pid -- To view, visit http://gerrit.ovirt.org/14448 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9f8ebea59fabac945b95b6422d1bfbf8a2b1d208 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
