Notification code improvements

client/idrepo/console/src/main/resources/org/apache/syncope/client/console/implementations/MyRecipientsProvider.groovy

@@ -17,15 +17,17 @@
  * under the License.
  */
 import groovy.transform.CompileStatic
+import java.util.Map
 import java.util.Set
+import org.apache.syncope.core.persistence.api.entity.Any
 import org.apache.syncope.core.persistence.api.entity.Notification
 import org.apache.syncope.core.provisioning.api.notification.RecipientsProvider
 
 @CompileStatic
 class MyRecipientsProvider implements RecipientsProvider {
 
   @Override
-  Set<String> provideRecipients(Notification notification) {
+  Set<String> provideRecipients(Notification notification, Any<?> any, Map<String, Object> jexlVars) {
     return Set.of();
   }
 }

common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/AuditLoggerName.java

@@ -133,11 +133,8 @@ public static String buildEvent(
             eventBuilder.append(event);
         }
         eventBuilder.append(']');
-
         if (result != null) {
-            eventBuilder.append(":[").
-                    append(result).
-                    append(']');
+            eventBuilder.append(":[").append(result).append(']');
         }
 
         return eventBuilder.toString();

core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/notification/RecipientsProvider.java

@@ -18,11 +18,13 @@
  */
 package org.apache.syncope.core.provisioning.api.notification;
 
+import java.util.Map;
 import java.util.Set;
+import org.apache.syncope.core.persistence.api.entity.Any;
 import org.apache.syncope.core.persistence.api.entity.Notification;
 
 @FunctionalInterface
 public interface RecipientsProvider {
 
-    Set<String> provideRecipients(Notification notification);
+    Set<String> provideRecipients(Notification notification, Any<?> any, Map<String, Object> jexlVars);
 }

core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultAuditManager.java

@@ -96,18 +96,16 @@ public boolean auditRequested(
         auditEntry.setDate(OffsetDateTime.now());
 
         AuditConf auditConf = auditConfDAO.find(auditEntry.getLogger().toAuditKey());
-        boolean auditRequested = auditConf != null && auditConf.isActive();
 
-        if (auditRequested) {
+        if (auditConf != null && auditConf.isActive()) {
             return true;
         }
 
         auditEntry.setLogger(new AuditLoggerName(type, category, subcategory, event, Result.FAILURE));
 
         auditConf = auditConfDAO.find(auditEntry.getLogger().toAuditKey());
-        auditRequested = auditConf != null && auditConf.isActive();
 
-        return auditRequested;
+        return auditConf != null && auditConf.isActive();
     }
 
     @Transactional(propagation = Propagation.NOT_SUPPORTED)

core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/AbstractSchedTaskJobDelegate.java

@@ -177,7 +177,7 @@ public void execute(
                 AuditElements.EventCategoryType.TASK,
                 this.getClass().getSimpleName(),
                 null,
-                this.getClass().getSimpleName(), // searching for before object is too much expensive ...
+                this.getClass().getSimpleName(),
                 result,
                 task,
                 execution);
@@ -187,10 +187,10 @@ public void execute(
                 AuditElements.EventCategoryType.TASK,
                 task.getClass().getSimpleName(),
                 null,
-                null, // searching for before object is too much expensive ...
+                this.getClass().getSimpleName(),
                 result,
                 task,
-                null);
+                execution);
 
         if (manageOperationId) {
             MDC.remove(OPERATION_ID);

core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/notification/AbstractNotificationJobDelegate.java

@@ -102,12 +102,13 @@ public TaskExec<NotificationTask> executeSingle(final NotificationTask task, fin
         if (StringUtils.isBlank(task.getSubject()) || task.getRecipients().isEmpty()
                 || StringUtils.isBlank(task.getHtmlBody()) || StringUtils.isBlank(task.getTextBody())) {
 
-            String message = "Could not fetch all required information for sending e-mails:\n"
-                    + task.getRecipients() + '\n'
-                    + task.getSender() + '\n'
-                    + task.getSubject() + '\n'
-                    + task.getHtmlBody() + '\n'
-                    + task.getTextBody();
+            String message = "Could not fetch all required information for sending out notifications:"
+                    + "\nFrom: " + task.getSender()
+                    + "\nTo: " + task.getRecipients()
+                    + "\nSubject: " + task.getSubject()
+                    + "\nHTML body:\n" + task.getHtmlBody()
+                    + "\nText body:\n" + task.getTextBody()
+                    + "\n";
             LOG.error(message);
 
             execution.setStatus(NotificationJob.Status.NOT_SENT.name());
@@ -117,14 +118,8 @@ public TaskExec<NotificationTask> executeSingle(final NotificationTask task, fin
                 execution.setMessage(message);
             }
         } else {
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("About to send notifications:\n"
-                        + task.getRecipients() + '\n'
-                        + task.getSender() + '\n'
-                        + task.getSubject() + '\n'
-                        + task.getHtmlBody() + '\n'
-                        + task.getTextBody() + '\n');
-            }
+            LOG.debug("About to send notifications:\nFrom: {}\nTo: {}\nSubject: {}\nHTML body:\n{}\nText body:\n{}\n",
+                    task.getSender(), task.getRecipients(), task.getSubject(), task.getHtmlBody(), task.getTextBody());
 
             setStatus("Sending notifications to " + task.getRecipients());
 

core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/notification/DefaultNotificationManager.java

@@ -183,17 +183,16 @@ protected NotificationTask getNotificationTask(
             final Any<?> any,
             final Map<String, Object> jexlVars) {
 
-        if (any != null) {
-            virAttrHandler.getValues(any);
-        }
+        jexlVars.put("syncopeConf", confParamOps.list(SyncopeConstants.MASTER_DOMAIN));
+        jexlVars.put("events", notification.getEvents());
+
+        Optional.ofNullable(any).ifPresent(virAttrHandler::getValues);
 
         List<User> recipients = new ArrayList<>();
 
-        if (notification.getRecipientsFIQL() != null) {
-            recipients.addAll(anySearchDAO.<User>search(
-                    SearchCondConverter.convert(searchCondVisitor, notification.getRecipientsFIQL()),
-                    List.of(), AnyTypeKind.USER));
-        }
+        Optional.ofNullable(notification.getRecipientsFIQL()).
+                ifPresent(fiql -> recipients.addAll(anySearchDAO.<User>search(
+                SearchCondConverter.convert(searchCondVisitor, fiql), List.of(), AnyTypeKind.USER)));
 
         if (notification.isSelfAsRecipient() && any instanceof User) {
             recipients.add((User) any);
@@ -204,47 +203,44 @@ protected NotificationTask getNotificationTask(
         recipients.forEach(recipient -> {
             virAttrHandler.getValues(recipient);
 
-            String email = getRecipientEmail(notification.getRecipientAttrName(), recipient);
-            if (email == null) {
-                LOG.warn("{} cannot be notified: {} not found", recipient, notification.getRecipientAttrName());
-            } else {
-                recipientEmails.add(email);
-                recipientTOs.add(userDataBinder.getUserTO(recipient, true));
-            }
+            Optional.ofNullable(getRecipientEmail(notification.getRecipientAttrName(), recipient)).
+                    ifPresentOrElse(
+                            email -> {
+                                recipientEmails.add(email);
+                                recipientTOs.add(userDataBinder.getUserTO(recipient, true));
+                            },
+                            () -> LOG.warn("{} cannot be notified: {} not found",
+                                    recipient, notification.getRecipientAttrName()));
         });
+        jexlVars.put("recipients", recipientTOs);
 
-        if (notification.getStaticRecipients() != null) {
-            recipientEmails.addAll(notification.getStaticRecipients());
-        }
+        Optional.ofNullable(notification.getStaticRecipients()).ifPresent(recipientEmails::addAll);
 
-        if (notification.getRecipientsProvider() != null) {
+        Optional.ofNullable(notification.getRecipientsProvider()).ifPresent(impl -> {
             try {
                 RecipientsProvider recipientsProvider = ImplementationManager.build(
-                        notification.getRecipientsProvider(),
+                        impl,
                         () -> perContextRecipientsProvider.orElse(null),
                         instance -> perContextRecipientsProvider = Optional.of(instance));
 
-                recipientEmails.addAll(recipientsProvider.provideRecipients(notification));
+                recipientEmails.addAll(recipientsProvider.provideRecipients(notification, any, jexlVars));
             } catch (Exception e) {
                 LOG.error("While building {}", notification.getRecipientsProvider(), e);
             }
-        }
+        });
 
-        jexlVars.put("recipients", recipientTOs);
-        jexlVars.put("syncopeConf", confParamOps.list(SyncopeConstants.MASTER_DOMAIN));
-        jexlVars.put("events", notification.getEvents());
         JexlContext ctx = new MapContext(jexlVars);
 
         NotificationTask task = entityFactory.newEntity(NotificationTask.class);
         task.setNotification(notification);
-        if (any != null) {
-            task.setEntityKey(any.getKey());
-            task.setAnyTypeKind(any.getType().getKind());
-        }
+        Optional.ofNullable(any).ifPresent(a -> {
+            task.setEntityKey(a.getKey());
+            task.setAnyTypeKind(a.getType().getKind());
+        });
         task.setTraceLevel(notification.getTraceLevel());
         task.getRecipients().addAll(recipientEmails);
         task.setSender(notification.getSender());
-        task.setSubject(notification.getSubject());
+        task.setSubject(JexlUtils.evaluateTemplate(notification.getSubject(), ctx));
 
         if (StringUtils.isNotBlank(notification.getTemplate().getTextTemplate())) {
             task.setTextBody(JexlUtils.evaluateTemplate(notification.getTemplate().getTextTemplate(), ctx));
@@ -263,8 +259,9 @@ public boolean notificationsAvailable(
             final String subcategory,
             final String event) {
 
-        final String successEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, Result.SUCCESS);
-        final String failureEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, Result.FAILURE);
+        String successEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, Result.SUCCESS);
+        String failureEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, Result.FAILURE);
+
         return notificationDAO.findAll().stream().
                 anyMatch(notification -> notification.isActive()
                 && (notification.getEvents().contains(successEvent)
@@ -297,8 +294,6 @@ public List<NotificationTask> createTasks(
             final Object output,
             final Object... input) {
 
-        String currentEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, condition);
-
         Any<?> any = null;
 
         if (before instanceof UserTO) {
@@ -342,35 +337,37 @@ public List<NotificationTask> createTasks(
             }
 
             if (notification.isActive()) {
+                String currentEvent = AuditLoggerName.buildEvent(type, category, subcategory, event, condition);
+
                 if (!notification.getEvents().contains(currentEvent)) {
-                    LOG.debug("No events found about {}", any);
+                    LOG.debug("Notification {} does not match {}", notification, currentEvent);
                 } else if (anyType == null || any == null
                         || notification.getAbout(anyType).isEmpty()
                         || anyMatchDAO.matches(any, SearchCondConverter.convert(
                                 searchCondVisitor, notification.getAbout(anyType).get().get()))) {
 
                     LOG.debug("Creating notification task for event {} about {}", currentEvent, any);
 
-                    Map<String, Object> model = new HashMap<>();
-                    model.put("who", who);
-                    model.put("type", type);
-                    model.put("category", category);
-                    model.put("subcategory", subcategory);
-                    model.put("event", event);
-                    model.put("condition", condition);
-                    model.put("before", before);
-                    model.put("output", output);
-                    model.put("input", input);
+                    Map<String, Object> jexlVars = new HashMap<>();
+                    jexlVars.put("who", who);
+                    jexlVars.put("type", type);
+                    jexlVars.put("category", category);
+                    jexlVars.put("subcategory", subcategory);
+                    jexlVars.put("event", event);
+                    jexlVars.put("condition", condition);
+                    jexlVars.put("before", before);
+                    jexlVars.put("output", output);
+                    jexlVars.put("input", input);
 
                     if (any instanceof User) {
-                        model.put("user", userDataBinder.getUserTO((User) any, true));
+                        jexlVars.put("user", userDataBinder.getUserTO((User) any, true));
                     } else if (any instanceof Group) {
-                        model.put("group", groupDataBinder.getGroupTO((Group) any, true));
+                        jexlVars.put("group", groupDataBinder.getGroupTO((Group) any, true));
                     } else if (any instanceof AnyObject) {
-                        model.put("anyObject", anyObjectDataBinder.getAnyObjectTO((AnyObject) any, true));
+                        jexlVars.put("anyObject", anyObjectDataBinder.getAnyObjectTO((AnyObject) any, true));
                     }
 
-                    NotificationTask notificationTask = getNotificationTask(notification, any, model);
+                    NotificationTask notificationTask = getNotificationTask(notification, any, jexlVars);
                     notificationTask = taskDAO.save(notificationTask);
                     notifications.add(notificationTask);
                 }
@@ -395,13 +392,10 @@ protected String getRecipientEmail(final String recipientAttrName, final User us
         if ("username".equals(intAttrName.getField())) {
             email = user.getUsername();
         } else if (intAttrName.getSchemaType() != null) {
-            UMembership membership = null;
-            if (intAttrName.getMembershipOfGroup() != null) {
-                Group group = groupDAO.findByName(intAttrName.getMembershipOfGroup());
-                if (group != null) {
-                    membership = user.getMembership(group.getKey()).orElse(null);
-                }
-            }
+            UMembership membership = Optional.ofNullable(intAttrName.getMembershipOfGroup()).
+                    map(groupDAO::findByName).
+                    flatMap(group -> user.getMembership(group.getKey())).
+                    orElse(null);
 
             switch (intAttrName.getSchemaType()) {
                 case PLAIN:

fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestNotificationRecipientsProvider.java

@@ -18,7 +18,9 @@
  */
 package org.apache.syncope.fit.core.reference;
 
+import java.util.Map;
 import java.util.Set;
+import org.apache.syncope.core.persistence.api.entity.Any;
 import org.apache.syncope.core.persistence.api.entity.Notification;
 import org.apache.syncope.core.provisioning.api.notification.RecipientsProvider;
 import org.springframework.transaction.annotation.Transactional;
@@ -27,7 +29,11 @@ public class TestNotificationRecipientsProvider implements RecipientsProvider {
 
     @Transactional(readOnly = true)
     @Override
-    public Set<String> provideRecipients(final Notification notification) {
+    public Set<String> provideRecipients(
+            final Notification notification,
+            final Any<?> any,
+            final Map<String, Object> jexlVars) {
+
         return Set.of(getClass().getSimpleName() + "@syncope.apache.org");
     }
 }

fit/core-reference/src/test/java/org/apache/syncope/fit/core/NotificationTaskITCase.java

@@ -273,7 +273,7 @@ public void issueSYNCOPE446() throws Exception {
         assertNotNull(taskTO);
         assertNotNull(taskTO.getNotification());
         assertTrue(taskTO.getRecipients().containsAll(
-                new TestNotificationRecipientsProvider().provideRecipients(null)));
+                new TestNotificationRecipientsProvider().provideRecipients(null, null, null)));
 
         execNotificationTask(TASK_SERVICE, taskTO.getKey(), MAX_WAIT_SECONDS);