Skip to content

Commit

Permalink
Notification code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
ilgrosso committed Oct 15, 2024
1 parent 37b7c88 commit d5e3caa
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void execute(
OpEvent.CategoryType.TASK,
this.getClass().getSimpleName(),
null,
this.getClass().getSimpleName(), // searching for before object is too much expensive ...
this.getClass().getSimpleName(),
result,
task,
execution);
Expand All @@ -165,10 +165,10 @@ public void execute(
OpEvent.CategoryType.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(Job.OPERATION_ID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,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());
Expand All @@ -104,12 +105,8 @@ public TaskExec<NotificationTask> executeSingle(final NotificationTask task, fin
execution.setMessage(message);
}
} else {
LOG.debug("About to send notifications:\n{}\n{}\n{}\n{}\n{}",
task.getRecipients(),
task.getSender(),
task.getSubject(),
task.getHtmlBody(),
task.getTextBody());
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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,17 +181,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);
Expand All @@ -202,47 +201,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));
Expand Down Expand Up @@ -296,8 +292,6 @@ public List<NotificationTask> createTasks(
final Object output,
final Object... input) {

String currentEvent = OpEvent.toString(type, category, subcategory, op, outcome);

Optional<? extends Any<?>> any = Optional.empty();

if (before instanceof UserTO userTO) {
Expand Down Expand Up @@ -341,6 +335,8 @@ public List<NotificationTask> createTasks(
}

if (notification.isActive()) {
String currentEvent = OpEvent.toString(type, category, subcategory, op, outcome);

if (!notification.getEvents().contains(currentEvent)) {
LOG.debug("No events found about {}", any);
} else if (anyType == null || any.isEmpty()
Expand All @@ -350,31 +346,31 @@ public List<NotificationTask> createTasks(

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", op);
model.put("condition", outcome);
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", op);
jexlVars.put("condition", outcome);
jexlVars.put("before", before);
jexlVars.put("output", output);
jexlVars.put("input", input);

any.ifPresent(a -> {
switch (a) {
case User user ->
model.put("user", userDataBinder.getUserTO(user, true));
jexlVars.put("user", userDataBinder.getUserTO(user, true));
case Group group ->
model.put("group", groupDataBinder.getGroupTO(group, true));
jexlVars.put("group", groupDataBinder.getGroupTO(group, true));
case AnyObject anyObject ->
model.put("anyObject", anyObjectDataBinder.getAnyObjectTO(anyObject, true));
jexlVars.put("anyObject", anyObjectDataBinder.getAnyObjectTO(anyObject, true));
default -> {
}
}
});

NotificationTask notificationTask = getNotificationTask(notification, any.orElse(null), model);
NotificationTask notificationTask = getNotificationTask(notification, any.orElse(null), jexlVars);
notificationTask = taskDAO.save(notificationTask);
notifications.add(notificationTask);
}
Expand All @@ -399,11 +395,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 = intAttrName.getMembershipOfGroup() == null
? null
: groupDAO.findByName(intAttrName.getMembershipOfGroup()).
flatMap(group -> user.getMembership(group.getKey())).
orElse(null);
UMembership membership = Optional.ofNullable(intAttrName.getMembershipOfGroup()).
flatMap(groupDAO::findByName).
flatMap(group -> user.getMembership(group.getKey())).
orElse(null);

switch (intAttrName.getSchemaType()) {
case PLAIN -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit d5e3caa

Please sign in to comment.