mooli tayer has uploaded a new change for review. Change subject: notifier: Notification properties changes. ......................................................................
notifier: Notification properties changes. Before this change validation was done both in NotificationService.initConfigurationProperties and in NotificationProperties itself. all moved to NotificationProperties. The properties themself are not read from the notification service to be instance variables inside the notifier but are read each time they are needed. Change-Id: I5753ba2cad2de90a7f2d14bd4675edc1dd35045e Signed-off-by: Mooli Tayer <[email protected]> --- 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/utils/NotificationProperties.java 2 files changed, 104 insertions(+), 130 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/80/23080/1 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 ee14ae0..2ef0400 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 @@ -44,67 +44,17 @@ private DataSource ds; private NotificationProperties prop = null; private NotificationMethodsMapper notificationMethodsMapper; - private int daysToKeepHistory = 0; - private int daysToSendOnStartup = 0; private EventSender failedQueriesEventSender; private List<EventAuditLogSubscriber> failedQueriesEventSubscribers = Collections.emptyList(); - private int failedQueriesNotificationThreshold; private int failedQueries = 0; public NotificationService(NotificationProperties prop) throws NotificationServiceException { this.prop = prop; initConnectivity(); - initConfigurationProperties(); - initEvents(); - initFailedQueriesEventSubscribers(); - } - - /** - * Validates the correctness of properties set in the configuration file.<br> - * If any of the properties is invalid, an error will be sent and service initialization fails.<br> - * Validated properties are: <li>DAYS_TO_KEEP_HISTORY - property could be omitted, if specified should be a positive - * <li>INTERVAL_IN_SECONDS - property is mandatory, if specified should be a positive <li>DB Connectivity - * Credentials - if failed to obtain connection to database, fails - * <li>FAILED_QUERIES_NOTIFICATION_THRESHOLD - send db connectivity notification once every x query attempts. - * - * @throws NotificationServiceException - * configuration setting error - */ - private void initConfigurationProperties() throws NotificationServiceException { - daysToKeepHistory = getNonNegativeIntegerProperty(NotificationProperties.DAYS_TO_KEEP_HISTORY); - daysToSendOnStartup = getNonNegativeIntegerProperty(NotificationProperties.DAYS_TO_SEND_ON_STARTUP); - failedQueriesNotificationThreshold = - getNonNegativeIntegerProperty(NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD); - if (failedQueriesNotificationThreshold == 0) { - failedQueriesNotificationThreshold = 1; - } notificationMethodsMapper = new NotificationMethodsMapper(prop); failedQueriesEventSender = notificationMethodsMapper.getEventSender(EventNotificationMethod.EMAIL); - } - - private int getNonNegativeIntegerProperty(final String name) throws NotificationServiceException { - // Get the text of the property: - final String text = prop.getProperty(name); - - // Validate it: - if (StringUtils.isNotEmpty(text)) { - try { - int value = Integer.parseInt(text); - if (value < 0) { - throw new NumberFormatException(name + " value should be a positive number"); - } - return value; - } - catch (NumberFormatException exception) { - String err = - String.format("Invalid format of %s: %s", name, text); - log.error(err, exception); - throw new NotificationServiceException(err, exception); - } - } - - // If the property can't be found then return 0 as the value: - return 0; + initEvents(); + initFailedQueriesEventSubscribers(); } /** @@ -115,9 +65,7 @@ try { log.debug("Start event notification service iteration"); processEvents(); - if (daysToKeepHistory > 0) { - deleteObseleteHistoryData(); - } + deleteObsoleteHistoryData(); log.debug("Finish event notification service iteration"); } catch (Throwable e) { if (!Thread.interrupted()) { @@ -126,37 +74,38 @@ } } - // TODO: Consider adding deleteObseleteHistoryData() as a separate scheduled thread run on a daily basis - private void deleteObseleteHistoryData() throws SQLException { - Calendar cal = Calendar.getInstance(); - cal.setTime(new Date()); - cal.add(Calendar.DATE, -daysToKeepHistory); - Timestamp startDeleteFrom = new Timestamp(cal.getTimeInMillis()); - Connection connection = null; - PreparedStatement deleteStmt = null; - int deletedRecords; - try { - connection = ds.getConnection(); - deleteStmt = connection.prepareStatement("delete from event_notification_hist where sent_at < ?"); - deleteStmt.setTimestamp(1, startDeleteFrom); - deletedRecords = deleteStmt.executeUpdate(); - } finally { - if (deleteStmt != null) { - deleteStmt.close(); + private void deleteObsoleteHistoryData() throws SQLException { + if (prop.getInteger(NotificationProperties.DAYS_TO_KEEP_HISTORY) > 0) { + Calendar cal = Calendar.getInstance(); + cal.setTime(new Date()); + cal.add(Calendar.DATE, -prop.getInteger(NotificationProperties.DAYS_TO_KEEP_HISTORY)); + Timestamp startDeleteFrom = new Timestamp(cal.getTimeInMillis()); + Connection connection = null; + PreparedStatement deleteStmt = null; + int deletedRecords; + try { + connection = ds.getConnection(); + deleteStmt = connection.prepareStatement("delete from event_notification_hist where sent_at < ?"); + deleteStmt.setTimestamp(1, startDeleteFrom); + deletedRecords = deleteStmt.executeUpdate(); + } finally { + if (deleteStmt != null) { + deleteStmt.close(); + } + if (connection != null) { + connection.close(); + } } - if (connection != null) { - connection.close(); - } - } - if (deletedRecords > 0) { - log.debug(deletedRecords + " records were deleted from \"event_notification_hist\" table."); + if (deletedRecords > 0) { + log.debug(deletedRecords + " records were deleted from \"event_notification_hist\" table."); + } } } private void markOldEventsAsProcessed() throws SQLException { Calendar calendar = Calendar.getInstance(); - calendar.add(Calendar.DATE, -daysToSendOnStartup); + calendar.add(Calendar.DATE, -prop.getInteger(NotificationProperties.DAYS_TO_SEND_ON_STARTUP)); Timestamp ts = new Timestamp(calendar.getTimeInMillis()); Connection connection = null; PreparedStatement statement = null; @@ -267,6 +216,11 @@ // Don't rethrow. we don't want to mask the original query exception. } } + int failedQueriesNotificationThreshold = + prop.getInteger(NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD); + if (failedQueriesNotificationThreshold == 0) { + failedQueriesNotificationThreshold = 1; + } failedQueries = (failedQueries + 1) % failedQueriesNotificationThreshold; } 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 5142ab7..54fcb50 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,7 +1,6 @@ package org.ovirt.engine.core.notifier.utils; import java.net.InetAddress; - import javax.mail.internet.InternetAddress; import org.apache.commons.lang.StringUtils; @@ -11,6 +10,41 @@ * Defines properties uses by the event notification service */ public class NotificationProperties extends LocalConfig { + + /** + * Service parameters + */ + public static final String DAYS_TO_KEEP_HISTORY = "DAYS_TO_KEEP_HISTORY"; + public static final String INTERVAL_IN_SECONDS = "INTERVAL_IN_SECONDS"; + public static final String ENGINE_INTERVAL_IN_SECONDS = "ENGINE_INTERVAL_IN_SECONDS"; + public static final String ENGINE_TIMEOUT_IN_SECONDS = "ENGINE_TIMEOUT_IN_SECONDS"; + public static final String IS_HTTPS_PROTOCOL = "IS_HTTPS_PROTOCOL"; + public static final String SSL_PROTOCOL = "SSL_PROTOCOL"; + public static final String REPEAT_NON_RESPONSIVE_NOTIFICATION = "REPEAT_NON_RESPONSIVE_NOTIFICATION"; + public static final String ENGINE_MONITOR_RETRIES = "ENGINE_MONITOR_RETRIES"; + public static final String SSL_IGNORE_CERTIFICATE_ERRORS = "SSL_IGNORE_CERTIFICATE_ERRORS"; + 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. + */ + 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. + */ + 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. + */ + public static final String DAYS_TO_SEND_ON_STARTUP = "DAYS_TO_SEND_ON_STARTUP"; + /** * Email parameters */ @@ -37,41 +71,6 @@ * SMTP transport encryption using TLS (SMTP with STARTTLS) */ public static final String MAIL_SMTP_ENCRYPTION_TLS = "tls"; - - /** - * Service parameters - */ - public static final String DAYS_TO_KEEP_HISTORY = "DAYS_TO_KEEP_HISTORY"; - public static final String INTERVAL_IN_SECONDS = "INTERVAL_IN_SECONDS"; - public static final String ENGINE_INTERVAL_IN_SECONDS = "ENGINE_INTERVAL_IN_SECONDS"; - public static final String ENGINE_TIMEOUT_IN_SECONDS = "ENGINE_TIMEOUT_IN_SECONDS"; - public static final String IS_HTTPS_PROTOCOL = "IS_HTTPS_PROTOCOL"; - public static final String SSL_PROTOCOL = "SSL_PROTOCOL"; - public static final String REPEAT_NON_RESPONSIVE_NOTIFICATION = "REPEAT_NON_RESPONSIVE_NOTIFICATION"; - public static final String ENGINE_MONITOR_RETRIES = "ENGINE_MONITOR_RETRIES"; - 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"; - - /** - * 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"; - - /** - * 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. - */ - public static final String FAILED_QUERIES_NOTIFICATION_THRESHOLD = "FAILED_QUERIES_NOTIFICATION_THRESHOLD"; // Default files for defaults and overridden values: private static String DEFAULTS_PATH = "/usr/share/ovirt-engine/conf/notifier.conf.defaults"; @@ -118,9 +117,11 @@ * @throws IllegalArgumentException if some properties has invalid values */ public void validate() { - // validate mandatory properties - for (String property : new String[] { + // validate mandatory and non empty properties + for (String property : new String[]{ 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, @@ -136,15 +137,34 @@ } } + // validate non negative args + for (String property : new String[]{ + NotificationProperties.DAYS_TO_KEEP_HISTORY, + NotificationProperties.DAYS_TO_SEND_ON_STARTUP, + NotificationProperties.FAILED_QUERIES_NOTIFICATION_THRESHOLD}) { + final String stringVal = getProperty(property); + try { + int value = Integer.parseInt(stringVal); + if (value < 0) { + throw new NumberFormatException(); + } + } catch (NumberFormatException exception) { + throw new IllegalArgumentException( + String.format( + "'%s' must be a non negative integer.", + 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 - )); + "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 + )); } // try to resolve MAIL_SERVER host @@ -167,7 +187,7 @@ if (!StringUtils.isEmpty(candidate)) { try { new InternetAddress(candidate); - } catch(Exception ex) { + } catch (Exception ex) { throw new IllegalArgumentException( String.format( "Check configuration file, invalid format in '%s'", @@ -181,12 +201,12 @@ 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( - "'%s' must be set when SSL or TLS is enabled or when password is set", - NotificationProperties.MAIL_USER)); + || MAIL_SMTP_ENCRYPTION_TLS.equals(getProperty(MAIL_SMTP_ENCRYPTION, true)) + || StringUtils.isNotEmpty(getProperty(NotificationProperties.MAIL_PASSWORD, true)))) { + throw new IllegalArgumentException( + String.format( + "'%s' must be set when SSL or TLS is enabled or when password is set", + NotificationProperties.MAIL_USER)); } } -- To view, visit http://gerrit.ovirt.org/23080 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5753ba2cad2de90a7f2d14bd4675edc1dd35045e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
