Hello mooli tayer,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/24696
to review the following change.
Change subject: tools: notifier: Remove auxiliary email address from
EventSender interface
......................................................................
tools: notifier: Remove auxiliary email address from EventSender interface
Prior to this change the interface EventSender.java
contained the following logic: an email address can come from wither
The subscriber table(event_subscriber) or if it does not exist the user
table (users). This patch moves this logic from the interface to the
component loading the email subscriber, as this logic is email specific.
Simpler interfaces are better.
Change-Id: Ic69ff7541fca8347fb7cb85665934ca704fa9d06
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/sender/EventSender.java
M
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/mail/EventSenderMailImpl.java
M
backend/manager/tools/src/test/java/org/ovirt/engine/core/notifier/utils/mail/MailSenderTest.java
M packaging/dbscripts/create_views.sql
5 files changed, 37 insertions(+), 67 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/96/24696/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 2ef0400..03eb1d4 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
@@ -6,7 +6,6 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
-import java.sql.Statement;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Calendar;
@@ -21,7 +20,6 @@
import org.apache.log4j.Logger;
import org.ovirt.engine.core.common.AuditLogSeverity;
import org.ovirt.engine.core.common.EventNotificationMethod;
-import org.ovirt.engine.core.common.businessentities.DbUser;
import org.ovirt.engine.core.common.businessentities.EventAuditLogSubscriber;
import org.ovirt.engine.core.common.businessentities.event_notification_hist;
import org.ovirt.engine.core.compat.Guid;
@@ -121,8 +119,7 @@
);
statement.setTimestamp(1, ts);
updatedRecords = statement.executeUpdate();
- }
- finally {
+ } finally {
DbUtils.closeQuietly(statement, connection);
}
@@ -144,8 +141,7 @@
// all of them:
try {
markOldEventsAsProcessed();
- }
- catch (SQLException exception) {
+ } catch (SQLException exception) {
throw new NotificationServiceException("Failed mark old events as
processed.", exception);
}
}
@@ -154,7 +150,7 @@
Connection connection = null;
PreparedStatement ps = null;
ResultSet rs = null;
- List<EventAuditLogSubscriber> eventSubscribers = new ArrayList<>();
+ List<EventAuditLogSubscriber> eventSubscribers = new ArrayList<>();
try {
connection = ds.getConnection();
ps =
@@ -169,33 +165,29 @@
}
} catch (SQLException e) {
- if (isConnectionException(e)){
+ if (isConnectionException(e)) {
handleQueryFailure();
}
throw e;
} finally {
DbUtils.closeQuietly(rs, ps, connection);
}
- DbUser dbUser = null;
- for (EventAuditLogSubscriber eventSubscriber:eventSubscribers) {
- dbUser = getUserByUserId(eventSubscriber.getsubscriber_id());
- if (dbUser != null) {
- EventSender method =
-
notificationMethodsMapper.getEventSender(eventSubscriber.getEventNotificationMethod());
- EventSenderResult sendResult = null;
- try {
- sendResult = method.send(eventSubscriber,
dbUser.getEmail());
- } catch (Exception e) {
- log.error("Failed to dispatch message", e);
- sendResult = new EventSenderResult();
- sendResult.setSent(false);
- sendResult.setReason(e.getMessage());
- }
-
addEventNotificationHistory(geteventNotificationHist(eventSubscriber,
- sendResult.isSent(),
- sendResult.getReason()));
- updateAuditLogEventProcessed(eventSubscriber);
+ for (EventAuditLogSubscriber eventSubscriber : eventSubscribers) {
+ EventSender method =
+
notificationMethodsMapper.getEventSender(eventSubscriber.getEventNotificationMethod());
+ EventSenderResult sendResult = null;
+ try {
+ sendResult = method.send(eventSubscriber);
+ } catch (Exception e) {
+ log.error("Failed to dispatch message", e);
+ sendResult = new EventSenderResult();
+ sendResult.setSent(false);
+ sendResult.setReason(e.getMessage());
}
+
addEventNotificationHistory(geteventNotificationHist(eventSubscriber,
+ sendResult.isSent(),
+ sendResult.getReason()));
+ updateAuditLogEventProcessed(eventSubscriber);
}
}
@@ -206,10 +198,10 @@
private void handleQueryFailure() {
if (failedQueries == 0) {
try {
- for( EventAuditLogSubscriber
failedQueriesEventSubscriber:failedQueriesEventSubscribers){
+ for (EventAuditLogSubscriber failedQueriesEventSubscriber :
failedQueriesEventSubscribers) {
failedQueriesEventSubscriber.setlog_time(new Date());
failedQueriesEventSender.
- send(failedQueriesEventSubscriber,
failedQueriesEventSubscriber.getmethod_address());
+ send(failedQueriesEventSubscriber);
}
} catch (Exception e) {
log.error("Failed to dispatch query failure email message", e);
@@ -276,37 +268,17 @@
return eventHistory;
}
- private DbUser getUserByUserId(Guid userId) throws SQLException {
- // Using preparedStatement instead of STP GetUserByUserId to skip
handling supporting dialects
- // for MSSQL and PG. PG doesn't support parameter name which matches a
column name. This is supported
- // by the backend, since using a plan JDBC, bypassing this issue by
prepared statement.
- // in additional, required only partial email field of the DbUser
- Connection connection = null;
- Statement ps = null;
- ResultSet rs = null;
- DbUser dbUser = null;
- try {
- connection = ds.getConnection();
- ps = connection.createStatement();
- rs = ps.executeQuery(String.format("SELECT email FROM users WHERE
user_id = '%s'", userId.toString()));
- if (rs.next()) {
- dbUser = new DbUser();
- dbUser.setId(userId);
- dbUser.setEmail(rs.getString("email"));
- }
- } finally {
- DbUtils.closeQuietly(rs, ps, connection);
- }
- return dbUser;
- }
-
private EventAuditLogSubscriber getEventAuditLogSubscriber(ResultSet rs)
throws SQLException {
EventAuditLogSubscriber eals = new EventAuditLogSubscriber();
eals.setevent_type(rs.getInt("event_type"));
eals.setsubscriber_id(Guid.createGuidFromStringDefaultEmpty(rs.getString("subscriber_id")));
eals.setevent_up_name(rs.getString("event_up_name"));
eals.setEventNotificationMethod(EventNotificationMethod.valueOf(rs.getString("notification_method")));
- eals.setmethod_address(rs.getString("method_address"));
+ String methodAddress = rs.getString("method_address");
+ if (methodAddress == null) {
+
+ }
+ eals.setmethod_address(methodAddress);
eals.settag_name(rs.getString("tag_name"));
eals.setaudit_log_id(rs.getLong("audit_log_id"));
eals.setuser_id(Guid.createGuidFromString(rs.getString("user_id")));
@@ -333,7 +305,7 @@
return;
}
List<EventAuditLogSubscriber> failedQueriesEventSubscribers = new
LinkedList<>();
- for (String email:emailRecipients.split(",")){
+ for (String email : emailRecipients.split(",")) {
EventAuditLogSubscriber eals = new EventAuditLogSubscriber();
eals.setevent_type(MessageHelper.MessageType.alertMessage.getEventType());
eals.setevent_up_name("DATABASE_UNREACHABLE");
diff --git
a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/EventSender.java
b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/EventSender.java
index dbaffcc..23f2632 100644
---
a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/EventSender.java
+++
b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/sender/EventSender.java
@@ -10,10 +10,9 @@
* Sends a message which constructed by the implementing class to be send
as a notification to the subscriber
* @param eventData
* contains data required for constructing a message
- * @param methodAddress
- * an alternate method address if not provided by {@link
event_audit_log_subscriber.getmethod_address()}
+ *
* @return the result of the notification send action
*/
- public EventSenderResult send(EventAuditLogSubscriber eventData, String
methodAddress);
+ public EventSenderResult send(EventAuditLogSubscriber eventData);
}
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 d2d8839..058d9b3 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
@@ -51,15 +51,12 @@
/**
* {@link #EventSender}
*/
- public EventSenderResult send(EventAuditLogSubscriber eventData, String
methodAddress) {
+ public EventSenderResult send(EventAuditLogSubscriber eventData) {
EventSenderResult result = new EventSenderResult();
EventMessageContent message = new EventMessageContent();
message.prepareMessage(hostName, eventData, isBodyHtml);
String recipient = eventData.getmethod_address();
- if (StringUtils.isEmpty(recipient)) {
- recipient = methodAddress;
- }
if ( StringUtils.isEmpty(recipient) ) {
log.error("Email recipient is not known, please check user table (
email ) or event_subscriber ( method_address ), unable to send email for
subscriber " + eventData.getsubscriber_id() + ", message was " +
message.getMessageSubject() + ":" + message.getMessageBody());
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 dbc7617..09e3ae4 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
@@ -53,7 +53,7 @@
eventData.setmessage("a test message to be sent via non-secured mode");
EventSenderResult sentResult = null;
try {
- sentResult = mailSender.send(eventData, null);
+ sentResult = mailSender.send(eventData);
} catch (Exception e) {
sentResult = new EventSenderResult();
sentResult.setSent(false);
@@ -88,11 +88,11 @@
);
EventSenderMailImpl mailSender = new
EventSenderMailImpl(NotificationProperties.getInstance());
eventData.setmessage("a test message to be sent via secured mode");
- mailSender.send(eventData, null);
+ mailSender.send(eventData);
EventSenderResult sentResult = null;
try {
- sentResult = mailSender.send(eventData, null);
+ sentResult = mailSender.send(eventData);
} catch (Exception e) {
sentResult = new EventSenderResult();
sentResult.setSent(false);
diff --git a/packaging/dbscripts/create_views.sql
b/packaging/dbscripts/create_views.sql
index e72b1b2..ef460e5 100644
--- a/packaging/dbscripts/create_views.sql
+++ b/packaging/dbscripts/create_views.sql
@@ -928,22 +928,24 @@
AS
SELECT 1 as event_type, event_subscriber_1.subscriber_id as subscriber_id,
event_subscriber_1.event_up_name as event_up_name,
event_subscriber_1.notification_method as notification_method,
- event_subscriber_1.method_address as method_address,
event_subscriber_1.tag_name as tag_name, audit_log_1.audit_log_id as
audit_log_id, audit_log_1.user_id as user_id, audit_log_1.user_name as
user_name,
+ event_subscriber_1.method_address as method_address,
event_subscriber_1.tag_name as tag_name, users.email as email,
audit_log_1.audit_log_id as audit_log_id, audit_log_1.user_id as user_id,
audit_log_1.user_name as user_name,
audit_log_1.vm_id as vm_id, audit_log_1.vm_name as
vm_name, audit_log_1.vm_template_id as vm_template_id,
audit_log_1.vm_template_name as vm_template_name, audit_log_1.vds_id as vds_id,
audit_log_1.vds_name as vds_name,
audit_log_1.storage_pool_id as storage_pool_id,
audit_log_1.storage_pool_name as storage_pool_name,
audit_log_1.storage_domain_id as storage_domain_id,
audit_log_1.storage_domain_name as storage_domain_name,
audit_log_1.log_time as log_time, audit_log_1.severity
as severity, audit_log_1.message as message
FROM audit_log AS audit_log_1 INNER JOIN
event_subscriber AS event_subscriber_1 ON audit_log_1.log_type_name =
event_subscriber_1.event_up_name
+INNER JOIN users ON event_subscriber_1.subscriber_id = users.user_id
WHERE (audit_log_1.processed = false)
UNION
SELECT distinct 0 as event_type, event_subscriber.subscriber_id as
subscriber_id, audit_log.log_type_name as event_up_name,
event_subscriber.notification_method as notification_method,
event_subscriber.method_address as method_address,
- event_subscriber.tag_name as tag_name,
audit_log.audit_log_id as audit_log_id, audit_log.user_id as user_id,
audit_log.user_name as user_name, audit_log.vm_id as vm_id, audit_log.vm_name
as vm_name,
+ event_subscriber.tag_name as tag_name, users_1.email as
email, audit_log.audit_log_id as audit_log_id, audit_log.user_id as user_id,
audit_log.user_name as user_name, audit_log.vm_id as vm_id, audit_log.vm_name
as vm_name,
audit_log.vm_template_id as vm_template_id,
audit_log.vm_template_name as vm_template_name, audit_log.vds_id as vds_id,
audit_log.vds_name as vds_name, audit_log.storage_pool_id as storage_pool_id,
audit_log.storage_pool_name as storage_pool_name,
audit_log.storage_domain_id as storage_domain_id, audit_log.storage_domain_name
as storage_domain_name, audit_log.log_time as log_time, audit_log.severity as
severity,
audit_log.message as message
FROM audit_log AS audit_log INNER JOIN
event_map ON audit_log.log_type_name = event_map.event_down_name INNER JOIN
event_subscriber AS event_subscriber ON event_subscriber.event_up_name =
event_map.event_up_name
+INNER JOIN users AS users_1 ON event_subscriber.subscriber_id = users_1.user_id
WHERE (audit_log.processed = false);
--
To view, visit http://gerrit.ovirt.org/24696
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic69ff7541fca8347fb7cb85665934ca704fa9d06
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
