From 160b4e70a5071ca937cf35073edd331ab0fdaefa Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Fri, 17 May 2024 15:34:26 +0200 Subject: [PATCH 01/21] feat: WIP adds NotificationViewDTO to get minimal information regarding the negotiation of the notification --- .../database/model/Notification.java | 1 + .../model/views/NotificationViewDTO.java | 23 +++++ .../repository/NotificationRepository.java | 31 +++++++ .../service/UserNotificationServiceImpl.java | 87 +++++++++++-------- .../templates/email-notification.html | 6 +- .../NegotiationLifecycleServiceImplTest.java | 12 +++ 6 files changed, 123 insertions(+), 37 deletions(-) create mode 100644 src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java index 992ef19c9..9946c572f 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java @@ -49,4 +49,5 @@ public class Notification extends AuditEntity { @Enumerated(EnumType.STRING) private NotificationEmailStatus emailStatus; + } diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java new file mode 100644 index 000000000..b6a59eab2 --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java @@ -0,0 +1,23 @@ +package eu.bbmri_eric.negotiator.database.model.views; + +import eu.bbmri_eric.negotiator.database.model.NotificationEmailStatus; +import eu.bbmri_eric.negotiator.database.model.Person; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import org.springframework.lang.Nullable; + +@Getter +@Setter +@AllArgsConstructor +@NoArgsConstructor +public class NotificationViewDTO { + private Long id; + private String message; + private NotificationEmailStatus emailStatus; + private String negotiationId; + private String negotiationTitle; + private String negotiationCreatorName; + private Person recipient; +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java index 16cb27073..5ef7f042e 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java @@ -2,18 +2,49 @@ import eu.bbmri_eric.negotiator.database.model.Notification; import eu.bbmri_eric.negotiator.database.model.NotificationEmailStatus; +import eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO; import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; public interface NotificationRepository extends JpaRepository { + @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE p.id = :recipientId") + List findViewByRecipientId(Long recipientId); + List findByRecipientId(Long personId); + @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE nt.emailStatus = :status") + List findViewByEmailStatus(NotificationEmailStatus status); + List findByEmailStatus(NotificationEmailStatus status); List findByEmailStatusAndMessageEndsWith( NotificationEmailStatus status, String messageSuffix); + @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE p.id = :recipientId AND " + + "nt.emailStatus = :status") + List findViewByRecipientIdAndEmailStatus( + Long recipientId, NotificationEmailStatus status); + List findByRecipientIdAndEmailStatus( Long recipientId, NotificationEmailStatus status); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index e1e40258f..4725ca82c 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -10,6 +10,7 @@ import eu.bbmri_eric.negotiator.database.model.Person; import eu.bbmri_eric.negotiator.database.model.Post; import eu.bbmri_eric.negotiator.database.model.Resource; +import eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO; import eu.bbmri_eric.negotiator.database.repository.NegotiationRepository; import eu.bbmri_eric.negotiator.database.repository.NotificationRepository; import eu.bbmri_eric.negotiator.database.repository.PersonRepository; @@ -128,7 +129,19 @@ public void notifyAdmins(Negotiation negotiation) { NotificationEmailStatus.EMAIL_SENT, "New Negotiation %s was added for review.".formatted(negotiation.getId())); notificationRepository.saveAll(newNotifications); - sendNotificationsToAdmins(newNotifications); + sendNotificationsToAdmins( + newNotifications.stream() + .map( + (notification) -> + new NotificationViewDTO( + notification.getId(), + notification.getMessage(), + notification.getEmailStatus(), + negotiation.getId(), + negotiation.getTitle(), + negotiation.getCreatedBy().getName(), + notification.getRecipient())) + .collect(Collectors.toList())); } @Override @@ -231,7 +244,7 @@ private static boolean postAuthorIsAlsoRequester(Post post) { private void createNotificationsForRepresentatives(Negotiation negotiation) { Set representatives = getRepresentativesForNegotiation(negotiation); for (Person representative : representatives) { - createNewNotification(negotiation, NotificationEmailStatus.EMAIL_NOT_SENT, representative); + createNewNotification(negotiation, representative); Set overlappingResources = getResourcesInNegotiationRepresentedBy(negotiation, representative); markReachableResources(negotiation, overlappingResources); @@ -261,8 +274,8 @@ private List createNotificationsForAdmins( return newNotifications; } - private void sendNotificationsToAdmins(List notifications) { - for (Notification notification : notifications) { + private void sendNotificationsToAdmins(List notifications) { + for (NotificationViewDTO notification : notifications) { sendEmail(notification.getRecipient(), Collections.singletonList(notification)); } } @@ -290,12 +303,11 @@ private void markReachableResources( } } - private void createNewNotification( - Negotiation negotiation, NotificationEmailStatus emailNotSent, Person representative) { + private void createNewNotification(Negotiation negotiation, Person representative) { notificationRepository.save( buildNewNotification( negotiation, - emailNotSent, + NotificationEmailStatus.EMAIL_NOT_SENT, representative, "New Negotiation %s ".formatted(negotiation.getId()))); } @@ -305,16 +317,16 @@ private Notification buildNewNotification( NotificationEmailStatus emailNotSent, Person representative, String message) { - Notification new_notification = + Notification newNotification = Notification.builder() .negotiation(negotiation) .emailStatus(emailNotSent) .recipient(representative) .message(message) .build(); - new_notification.setModifiedDate(LocalDateTime.now()); - new_notification.setCreationDate(LocalDateTime.now()); - return new_notification; + newNotification.setModifiedDate(LocalDateTime.now()); + newNotification.setCreationDate(LocalDateTime.now()); + return newNotification; } @Override @@ -328,67 +340,74 @@ public void sendEmailsForNewNotifications() { private void sendOutNotificationEmails(@NonNull Set recipients) { for (Person recipient : recipients) { - List notifications = getPendingNotifications(recipient); + List notifications = getPendingNotifications(recipient); sendEmail(recipient, notifications); markNotificationsAsEmailSent(notifications); } } - private void markNotificationsAsEmailSent(@NonNull List notifications) { - for (Notification notification : notifications) { + private void markNotificationsAsEmailSent(@NonNull List notifications) { + for (NotificationViewDTO notificationView : notifications) { + Notification notification = + notificationRepository.findById(notificationView.getId()).orElseThrow(); notification.setEmailStatus(NotificationEmailStatus.EMAIL_SENT); notification.setModifiedDate(LocalDateTime.now()); notificationRepository.save(notification); } } - private void sendEmail(@NonNull Person recipient, @NonNull List notifications) { + private void sendEmail( + @NonNull Person recipient, @NonNull List notifications) { sendEmail(recipient, notifications, "email-notification"); } private void sendEmail( - @NonNull Person recipient, @NonNull List notifications, String email_template) { + @NonNull Person recipient, + @NonNull List notifications, + String emailTemplate) { Context context = new Context(); - List negotiations = + List negotiationsIds = notifications.stream() - .map(notification -> notification.getNegotiation()) + .map(NotificationViewDTO::getNegotiationId) .distinct() .collect(Collectors.toList()); Map roleForNegotiation = populateRoleForNegotiationMap(notifications); Map titleForNegotiation = populateTitleForNegotiationMap(notifications); - Map> notificationsForNegotiation = - notifications.stream().collect(Collectors.groupingBy(Notification::getNegotiation)); + Map> notificationsForNegotiation = + notifications.stream() + .collect(Collectors.groupingBy(NotificationViewDTO::getNegotiationId)); context.setVariable("recipient", recipient); - context.setVariable("negotiations", negotiations); + context.setVariable("negotiations", negotiationsIds); context.setVariable("frontendUrl", frontendUrl); context.setVariable("roleForNegotiation", roleForNegotiation); context.setVariable("titleForNegotiation", titleForNegotiation); context.setVariable("notificationsForNegotiation", notificationsForNegotiation); - String emailContent = templateEngine.process(email_template, context); + String emailContent = templateEngine.process(emailTemplate, context); emailService.sendEmail(recipient, "New Notifications", emailContent); } - private Map populateRoleForNegotiationMap(List notifications) { + private Map populateRoleForNegotiationMap( + List notifications) { Map roleForNegotiation = new HashMap<>(); - for (Notification notification : notifications) { - String negotiationId = notification.getNegotiation().getId(); + for (NotificationViewDTO notification : notifications) { + String negotiationId = notification.getNegotiationId(); String role = extractRoleFromNotificationMessage(notification); roleForNegotiation.put(negotiationId, role); } return roleForNegotiation; } - private Map populateTitleForNegotiationMap(List notifications) { + private Map populateTitleForNegotiationMap( + List notifications) { Map titleForNegotiation = new HashMap<>(); - for (Notification notification : notifications) { - Negotiation negotiation = notification.getNegotiation(); - String negotiatorId = negotiation.getId(); - String title = parseTitleFromNegotiation(negotiation); + for (NotificationViewDTO notification : notifications) { + String negotiatorId = notification.getNegotiationId(); + String title = notification.getNegotiationTitle(); titleForNegotiation.put(negotiatorId, title); } return titleForNegotiation; @@ -406,7 +425,7 @@ private static String parseTitleFromNegotiation(Negotiation negotiation) { return title; } - private String extractRoleFromNotificationMessage(Notification notification) { + private String extractRoleFromNotificationMessage(NotificationViewDTO notification) { String message = notification.getMessage(); if (message.matches("New Negotiation .* was added for review\\.") || message.matches("The negotiation .* is awaiting review\\.")) { @@ -417,7 +436,7 @@ private String extractRoleFromNotificationMessage(Notification notification) { return "ROLE_RESEARCHER"; } else if (message.matches("Negotiation .* had a new post by .*")) { String[] parts = message.split("new post by"); - String negotiationCreator = notification.getNegotiation().getCreatedBy().getName(); + String negotiationCreator = notification.getNegotiationCreatorName(); String postCreator = parts[1].trim(); return (negotiationCreator.equals(postCreator)) ? "ROLE_REPRESENTATIVE" : "ROLE_RESEARCHER"; } else if (message.matches("New Negotiation .*") @@ -428,8 +447,8 @@ private String extractRoleFromNotificationMessage(Notification notification) { } } - private List getPendingNotifications(@NonNull Person recipient) { - return notificationRepository.findByRecipientIdAndEmailStatus( + private List getPendingNotifications(@NonNull Person recipient) { + return notificationRepository.findViewByRecipientIdAndEmailStatus( recipient.getId(), NotificationEmailStatus.EMAIL_NOT_SENT); } diff --git a/src/main/resources/templates/email-notification.html b/src/main/resources/templates/email-notification.html index 321115b35..7747600ad 100755 --- a/src/main/resources/templates/email-notification.html +++ b/src/main/resources/templates/email-notification.html @@ -64,12 +64,12 @@
  • - - + +
    • - +
    • diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index c6cd72efc..6881d80a5 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -14,7 +14,10 @@ import eu.bbmri_eric.negotiator.database.model.Negotiation; import eu.bbmri_eric.negotiator.database.model.NegotiationLifecycleRecord; import eu.bbmri_eric.negotiator.database.model.NegotiationResourceLifecycleRecord; +import eu.bbmri_eric.negotiator.database.model.Person; +import eu.bbmri_eric.negotiator.database.model.Request; import eu.bbmri_eric.negotiator.database.repository.NegotiationRepository; +import eu.bbmri_eric.negotiator.database.repository.RequestRepository; import eu.bbmri_eric.negotiator.dto.negotiation.NegotiationCreateDTO; import eu.bbmri_eric.negotiator.dto.negotiation.NegotiationDTO; import eu.bbmri_eric.negotiator.exceptions.EntityNotFoundException; @@ -49,6 +52,7 @@ public class NegotiationLifecycleServiceImplTest { @Autowired NegotiationRepository negotiationRepository; @Autowired NegotiationService negotiationService; @Autowired private WebApplicationContext context; + @Autowired RequestRepository requestRepository; private void checkNegotiationResourceRecordPresenceWithAssignedState( String negotiationId, NegotiationResourceState negotiationResourceState) { @@ -118,6 +122,14 @@ void sendEvent_abandonNegotiation_to_inProcess_Negotiation() throws IOException private NegotiationDTO saveNegotiation() throws IOException { NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); + Request request = requestRepository.findById("request-2").get(); + Negotiation negotiation = + Negotiation.builder() + .requests(Set.of(request)) + .payload(negotiationCreateDTO.getPayload().toString()) + .build(); + negotiation.setCreatedBy(Person.builder().id(101L).name("TheBuilder").build()); + negotiationRepository.save(negotiation); return negotiationService.create(negotiationCreateDTO, 101L); } From c69c7b337afaa33305792d4f246232cd50c82715 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Fri, 17 May 2024 16:46:42 +0200 Subject: [PATCH 02/21] fix: fixes memory consumption for list attachements endpoint --- .../database/model/Notification.java | 1 - .../model/views/NotificationViewDTO.java | 1 - .../repository/NegotiationRepository.java | 10 ++++ .../repository/NotificationRepository.java | 47 ++++++++++--------- .../service/NegotiationServiceImpl.java | 7 +-- .../service/UserNotificationServiceImpl.java | 10 ++-- 6 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java index 9946c572f..992ef19c9 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/Notification.java @@ -49,5 +49,4 @@ public class Notification extends AuditEntity { @Enumerated(EnumType.STRING) private NotificationEmailStatus emailStatus; - } diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java index b6a59eab2..bd953fe41 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java @@ -6,7 +6,6 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import org.springframework.lang.Nullable; @Getter @Setter diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java index 8ef351a6f..0770256a5 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java @@ -17,6 +17,16 @@ public interface NegotiationRepository Optional findDetailedById(String id); + // @Override + // @Query( + // value = + // "SELECT EXISTS (" + // + "SELECT n.id " + // + "FROM negotiation n " + // + "WHERE n.id = :negotiationId", + // nativeQuery = true) + // boolean existsById(@Nonnull String id); + @Query(value = "SELECT currentState from Negotiation where id = :id") Optional findNegotiationStateById(String id); diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java index 5ef7f042e..37b127078 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java @@ -9,24 +9,26 @@ public interface NotificationRepository extends JpaRepository { - @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + - "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + - "FROM Notification nt " + - "JOIN nt.negotiation ng " + - "JOIN ng.createdBy c " + - "JOIN nt.recipient p " + - "WHERE p.id = :recipientId") + @Query( + "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE p.id = :recipientId") List findViewByRecipientId(Long recipientId); List findByRecipientId(Long personId); - @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + - "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + - "FROM Notification nt " + - "JOIN nt.negotiation ng " + - "JOIN ng.createdBy c " + - "JOIN nt.recipient p " + - "WHERE nt.emailStatus = :status") + @Query( + "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE nt.emailStatus = :status") List findViewByEmailStatus(NotificationEmailStatus status); List findByEmailStatus(NotificationEmailStatus status); @@ -34,14 +36,15 @@ public interface NotificationRepository extends JpaRepository findByEmailStatusAndMessageEndsWith( NotificationEmailStatus status, String messageSuffix); - @Query("SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + - "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + - "FROM Notification nt " + - "JOIN nt.negotiation ng " + - "JOIN ng.createdBy c " + - "JOIN nt.recipient p " + - "WHERE p.id = :recipientId AND " + - "nt.emailStatus = :status") + @Query( + "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "FROM Notification nt " + + "JOIN nt.negotiation ng " + + "JOIN ng.createdBy c " + + "JOIN nt.recipient p " + + "WHERE p.id = :recipientId AND " + + "nt.emailStatus = :status") List findViewByRecipientIdAndEmailStatus( Long recipientId, NotificationEmailStatus status); diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java index 26b7d22c3..2c6eac26d 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java @@ -98,12 +98,7 @@ private List findAttachments(Set attachmentDT @Override public boolean exists(String negotiationId) { - try { - findEntityById(negotiationId, false); - return true; - } catch (EntityNotFoundException ex) { - return false; - } + return negotiationRepository.existsById(negotiationId); } private void addPersonToNegotiation( diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 4725ca82c..91f7beccb 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -36,8 +36,6 @@ import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.scheduling.annotation.Async; -import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; import org.thymeleaf.TemplateEngine; import org.thymeleaf.context.Context; @@ -330,8 +328,8 @@ private Notification buildNewNotification( } @Override - @Scheduled(cron = "${notification.cron-schedule-expression:0 0 * * * *}") - @Async + // @Scheduled(cron = "${notification.cron-schedule-expression:0 0 * * * *}") + // @Async public void sendEmailsForNewNotifications() { log.info("Sending new email notifications."); Set recipients = getPendingRecipients(); @@ -477,8 +475,8 @@ private void remindRepresentatives(Negotiation negotiation) { notificationRepository.saveAll(reminderNotifications); } - @Scheduled(cron = "${reminder.cron-schedule-expression:0 0 0 * * MON-FRI}") - @Async + // @Scheduled(cron = "${reminder.cron-schedule-expression:0 0 0 * * MON-FRI}") + // @Async public void createRemindersOldNegotiations() { log.info("Creating reminder email notifications."); Duration durationThreshold = Duration.parse(triggerDuration); From 391a218bd0a23931b523a576c465c98efb95e556 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Mon, 20 May 2024 11:07:47 +0200 Subject: [PATCH 03/21] fix: fixes memory consumption for notifications --- .../model/views/NotificationViewDTO.java | 1 - .../repository/NegotiationRepository.java | 20 ------------- .../repository/NotificationRepository.java | 29 +------------------ .../database/repository/PersonRepository.java | 10 +++++++ .../service/NegotiationServiceImpl.java | 4 +-- .../service/UserNotificationServiceImpl.java | 26 ++++++----------- 6 files changed, 22 insertions(+), 68 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java index bd953fe41..740499be8 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java @@ -17,6 +17,5 @@ public class NotificationViewDTO { private NotificationEmailStatus emailStatus; private String negotiationId; private String negotiationTitle; - private String negotiationCreatorName; private Person recipient; } diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java index 0770256a5..f0af88727 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NegotiationRepository.java @@ -17,16 +17,6 @@ public interface NegotiationRepository Optional findDetailedById(String id); - // @Override - // @Query( - // value = - // "SELECT EXISTS (" - // + "SELECT n.id " - // + "FROM negotiation n " - // + "WHERE n.id = :negotiationId", - // nativeQuery = true) - // boolean existsById(@Nonnull String id); - @Query(value = "SELECT currentState from Negotiation where id = :id") Optional findNegotiationStateById(String id); @@ -43,16 +33,6 @@ Optional findNegotiationResourceStateById( List findByModifiedDateBeforeAndCurrentState( LocalDateTime thresholdTime, NegotiationState currentState); - @Query( - value = - "SELECT EXISTS (" - + "SELECT n.id " - + "FROM negotiation n " - + "WHERE n.id = :negotiationId AND " - + "n.created_by = :personId)", - nativeQuery = true) - boolean isNegotiationCreator(String negotiationId, Long personId); - @Query( value = "SELECT EXISTS (" diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java index 37b127078..6cbf5c1ce 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/NotificationRepository.java @@ -9,45 +9,18 @@ public interface NotificationRepository extends JpaRepository { - @Query( - "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" - + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " - + "FROM Notification nt " - + "JOIN nt.negotiation ng " - + "JOIN ng.createdBy c " - + "JOIN nt.recipient p " - + "WHERE p.id = :recipientId") - List findViewByRecipientId(Long recipientId); - List findByRecipientId(Long personId); - @Query( - "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" - + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " - + "FROM Notification nt " - + "JOIN nt.negotiation ng " - + "JOIN ng.createdBy c " - + "JOIN nt.recipient p " - + "WHERE nt.emailStatus = :status") - List findViewByEmailStatus(NotificationEmailStatus status); - List findByEmailStatus(NotificationEmailStatus status); - List findByEmailStatusAndMessageEndsWith( - NotificationEmailStatus status, String messageSuffix); - @Query( "SELECT new eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO(" - + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, c.name, p) " + + "nt.id, nt.message, nt.emailStatus, ng.id, ng.title, p) " + "FROM Notification nt " + "JOIN nt.negotiation ng " - + "JOIN ng.createdBy c " + "JOIN nt.recipient p " + "WHERE p.id = :recipientId AND " + "nt.emailStatus = :status") List findViewByRecipientIdAndEmailStatus( Long recipientId, NotificationEmailStatus status); - - List findByRecipientIdAndEmailStatus( - Long recipientId, NotificationEmailStatus status); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/repository/PersonRepository.java b/src/main/java/eu/bbmri_eric/negotiator/database/repository/PersonRepository.java index 7833e39a4..4a2f7ccbd 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/repository/PersonRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/repository/PersonRepository.java @@ -35,6 +35,16 @@ public interface PersonRepository @EntityGraph(value = "person-detailed") boolean existsByIdAndResourcesIn(Long id, Set resources); + @Query( + value = + "SELECT EXISTS (" + + "SELECT n.id " + + "FROM negotiation n " + + "WHERE n.id = :negotiationId AND " + + "n.created_by = :personId)", + nativeQuery = true) + boolean isNegotiationCreator(Long personId, String negotiationId); + @Query( value = "SELECT EXISTS (SELECT rs.id " diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java index 2c6eac26d..ff20f78ef 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java @@ -51,8 +51,8 @@ public class NegotiationServiceImpl implements NegotiationService { @Override public boolean isNegotiationCreator(String negotiationId) { - return negotiationRepository.isNegotiationCreator( - negotiationId, NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId()); + return personRepository.isNegotiationCreator( + NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId(), negotiationId); } /** diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 91f7beccb..470560878 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -36,6 +36,8 @@ import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.scheduling.annotation.Async; +import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; import org.thymeleaf.TemplateEngine; import org.thymeleaf.context.Context; @@ -137,7 +139,6 @@ public void notifyAdmins(Negotiation negotiation) { notification.getEmailStatus(), negotiation.getId(), negotiation.getTitle(), - negotiation.getCreatedBy().getName(), notification.getRecipient())) .collect(Collectors.toList())); } @@ -328,8 +329,8 @@ private Notification buildNewNotification( } @Override - // @Scheduled(cron = "${notification.cron-schedule-expression:0 0 * * * *}") - // @Async + @Scheduled(cron = "${notification.cron-schedule-expression:0 0 * * * *}") + @Async public void sendEmailsForNewNotifications() { log.info("Sending new email notifications."); Set recipients = getPendingRecipients(); @@ -394,7 +395,7 @@ private Map populateRoleForNegotiationMap( Map roleForNegotiation = new HashMap<>(); for (NotificationViewDTO notification : notifications) { String negotiationId = notification.getNegotiationId(); - String role = extractRoleFromNotificationMessage(notification); + String role = extractRole(notification); roleForNegotiation.put(negotiationId, role); } return roleForNegotiation; @@ -423,25 +424,16 @@ private static String parseTitleFromNegotiation(Negotiation negotiation) { return title; } - private String extractRoleFromNotificationMessage(NotificationViewDTO notification) { + private String extractRole(NotificationViewDTO notification) { String message = notification.getMessage(); if (message.matches("New Negotiation .* was added for review\\.") || message.matches("The negotiation .* is awaiting review\\.")) { return "ROLE_ADMIN"; - } else if (message.matches("Negotiation .* had a change of status of .* to .*")) { - // TODO if status changed to "ACCESS_CONDITIONS_MET" role should be "ROLE_REPRESENTATIVE" - // (once notification also goes to REPRESENTATIVE) + } else if (personRepository.isNegotiationCreator( + notification.getRecipient().getId(), notification.getNegotiationId())) { return "ROLE_RESEARCHER"; - } else if (message.matches("Negotiation .* had a new post by .*")) { - String[] parts = message.split("new post by"); - String negotiationCreator = notification.getNegotiationCreatorName(); - String postCreator = parts[1].trim(); - return (negotiationCreator.equals(postCreator)) ? "ROLE_REPRESENTATIVE" : "ROLE_RESEARCHER"; - } else if (message.matches("New Negotiation .*") - || message.matches("The negotiation .* is stale and had no status change in a while\\.")) { - return "ROLE_REPRESENTATIVE"; } else { - return "ROLE_RESEARCHER"; + return "ROLE_REPRESENTATIVE"; } } From 747e7fc32145a6760fec8632f2bdad000eac50e7 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 21 May 2024 09:50:05 +0200 Subject: [PATCH 04/21] fix: restore schedule --- .../negotiator/service/UserNotificationServiceImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 470560878..7c0fa814c 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -467,8 +467,8 @@ private void remindRepresentatives(Negotiation negotiation) { notificationRepository.saveAll(reminderNotifications); } - // @Scheduled(cron = "${reminder.cron-schedule-expression:0 0 0 * * MON-FRI}") - // @Async + @Scheduled(cron = "${reminder.cron-schedule-expression:0 0 0 * * MON-FRI}") + @Async public void createRemindersOldNegotiations() { log.info("Creating reminder email notifications."); Duration durationThreshold = Duration.parse(triggerDuration); From 897cc6b0733e7ba645bdbd27547f11257c5dc1cf Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 21 May 2024 10:42:45 +0200 Subject: [PATCH 05/21] fix: increase coverage --- .../model/views/NotificationViewDTO.java | 2 - .../unit/service/NegotiationServiceTest.java | 39 ++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java index 740499be8..84f05186a 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java @@ -8,9 +8,7 @@ import lombok.Setter; @Getter -@Setter @AllArgsConstructor -@NoArgsConstructor public class NotificationViewDTO { private Long id; private String message; diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java index 87af08b34..ed684606c 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java @@ -20,12 +20,12 @@ import eu.bbmri_eric.negotiator.database.repository.RoleRepository; import eu.bbmri_eric.negotiator.dto.negotiation.NegotiationCreateDTO; import eu.bbmri_eric.negotiator.dto.negotiation.NegotiationDTO; -import eu.bbmri_eric.negotiator.exceptions.EntityNotFoundException; import eu.bbmri_eric.negotiator.integration.api.v3.TestUtils; import eu.bbmri_eric.negotiator.service.EmailService; import eu.bbmri_eric.negotiator.service.NegotiationLifecycleService; import eu.bbmri_eric.negotiator.service.NegotiationServiceImpl; import eu.bbmri_eric.negotiator.service.ResourceLifecycleService; +import eu.bbmri_eric.negotiator.unit.context.WithMockNegotiatorUser; import java.io.IOException; import java.util.List; import java.util.Optional; @@ -34,6 +34,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.InjectMocks; @@ -42,7 +43,11 @@ import org.modelmapper.ModelMapper; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.domain.Specification; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +@ExtendWith(SpringExtension.class) +@ContextConfiguration public class NegotiationServiceTest { @Mock NegotiationRepository negotiationRepository; @Mock RoleRepository roleRepository; @@ -91,10 +96,40 @@ void after() throws Exception { @Test public void test_Exist_IsFalse_WhenNegotiationIsNotFound() { - when(negotiationRepository.findById(any())).thenThrow(EntityNotFoundException.class); + when(negotiationRepository.existsById(any())).thenReturn(false); assertFalse(negotiationService.exists("unknown")); } + @Test + public void test_Exist_IsTrue_WhenNegotiationIsFound() { + when(negotiationRepository.existsById(any())).thenReturn(true); + assertTrue(negotiationService.exists("123")); + } + + @Test + @WithMockNegotiatorUser( + id = 2L, + authName = "researcher", + authSubject = "researcher@aai.eu", + authEmail = "researcher@aai.eu", + authorities = {"ROLE_RESEARCHER"}) + public void test_isNegotiatorCreator_IsFalse_WhenPersonRepositoryIsNegotiatiorCreator_IsFalse() { + when(personRepository.isNegotiationCreator(any(), any())).thenReturn(false); + assertFalse(negotiationService.isNegotiationCreator("123")); + } + + @Test + @WithMockNegotiatorUser( + id = 2L, + authName = "researcher", + authSubject = "researcher@aai.eu", + authEmail = "researcher@aai.eu", + authorities = {"ROLE_RESEARCHER"}) + public void test_isNegotiatorCreator_IsTrue_WhenPersonRepositoryIsNegotiatiorCreator_IsTrue() { + when(personRepository.isNegotiationCreator(any(), any())).thenReturn(true); + assertTrue(negotiationService.isNegotiationCreator("123")); + } + @Disabled void testCreateNegotiation() throws IOException { when(personRepository.findById(100L)).thenReturn(Optional.of(new Person())); From f13bef1487bcfe2b4c8ada8afa93bc51d97fb39a Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 21 May 2024 10:58:36 +0200 Subject: [PATCH 06/21] style: fixes formatting --- .../negotiator/database/model/views/NotificationViewDTO.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java index 84f05186a..194fbdb54 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/views/NotificationViewDTO.java @@ -4,8 +4,6 @@ import eu.bbmri_eric.negotiator.database.model.Person; import lombok.AllArgsConstructor; import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; @Getter @AllArgsConstructor From 5a83ae3e3e7df0dee8f7c78523a3d3794b67d972 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 21 May 2024 12:23:20 +0200 Subject: [PATCH 07/21] fix: fixes broken email for admins --- .../negotiator/service/UserNotificationServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 7c0fa814c..ed7e3bf30 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -138,7 +138,7 @@ public void notifyAdmins(Negotiation negotiation) { notification.getMessage(), notification.getEmailStatus(), negotiation.getId(), - negotiation.getTitle(), + parseTitleFromNegotiation(negotiation), notification.getRecipient())) .collect(Collectors.toList())); } From 5a2000afe772b239a52810406a1a688caba3a904 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 21 May 2024 13:17:28 +0200 Subject: [PATCH 08/21] test: adds test for notification repository --- .../NotificationRepositoryTest.java | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java new file mode 100644 index 000000000..ed1d6e554 --- /dev/null +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java @@ -0,0 +1,194 @@ +package eu.bbmri_eric.negotiator.integration.repository; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import eu.bbmri_eric.negotiator.configuration.state_machine.negotiation.NegotiationState; +import eu.bbmri_eric.negotiator.database.model.DiscoveryService; +import eu.bbmri_eric.negotiator.database.model.Negotiation; +import eu.bbmri_eric.negotiator.database.model.Notification; +import eu.bbmri_eric.negotiator.database.model.NotificationEmailStatus; +import eu.bbmri_eric.negotiator.database.model.Organization; +import eu.bbmri_eric.negotiator.database.model.Person; +import eu.bbmri_eric.negotiator.database.model.PersonNegotiationRole; +import eu.bbmri_eric.negotiator.database.model.Request; +import eu.bbmri_eric.negotiator.database.model.Resource; +import eu.bbmri_eric.negotiator.database.model.Role; +import eu.bbmri_eric.negotiator.database.model.views.NotificationViewDTO; +import eu.bbmri_eric.negotiator.database.repository.DiscoveryServiceRepository; +import eu.bbmri_eric.negotiator.database.repository.NegotiationRepository; +import eu.bbmri_eric.negotiator.database.repository.NotificationRepository; +import eu.bbmri_eric.negotiator.database.repository.OrganizationRepository; +import eu.bbmri_eric.negotiator.database.repository.PersonRepository; +import eu.bbmri_eric.negotiator.database.repository.RequestRepository; +import eu.bbmri_eric.negotiator.database.repository.ResourceRepository; +import eu.bbmri_eric.negotiator.database.repository.RoleRepository; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.sql.DataSource; +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.TestPropertySource; + +@DataJpaTest(showSql = false) +@ActiveProfiles("test") +@TestPropertySource(properties = {"spring.sql.init.mode=never"}) +@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD) +public class NotificationRepositoryTest { + @Autowired DataSource dbSource; + @Autowired PersonRepository personRepository; + @Autowired ResourceRepository resourceRepository; + @Autowired RequestRepository requestRepository; + @Autowired DiscoveryServiceRepository discoveryServiceRepository; + @Autowired OrganizationRepository organizationRepository; + @Autowired NegotiationRepository negotiationRepository; + @Autowired RoleRepository roleRepository; + @Autowired NotificationRepository notificationRepository; + + String payload = + " {\n" + + "\"project\": {\n" + + "\"title\": \"Title\",\n" + + "\"description\": \"Description\"\n" + + "},\n" + + " \"samples\": {\n" + + " \"sample-type\": \"DNA\",\n" + + " \"num-of-subjects\": 10,\n" + + " \"num-of-samples\": 20,\n" + + " \"volume-per-sample\": 5\n" + + " },\n" + + " \"ethics-vote\": {\n" + + " \"ethics-vote\": \"My ethic vote\"\n" + + " }\n" + + "}\n"; + private DiscoveryService discoveryService; + private Person person; + private Resource resource; + private Negotiation negotiation; + + public void addH2Function() { + String statementScript = + "create DOMAIN IF NOT EXISTS JSONB AS JSON; \n" + + "CREATE ALIAS IF NOT EXISTS JSONB_EXTRACT_PATH AS '\n" + + "import com.jayway.jsonpath.JsonPath;\n" + + " @CODE\n" + + " String jsonbExtractPath(String jsonString, String...jsonPaths) {\n" + + " String overallPath = String.join(\".\", jsonPaths);\n" + + " try {\n" + + " Object result = JsonPath.read(jsonString, overallPath);\n" + + " if (result != null) {\n" + + " return result.toString();\n" + + " }\n" + + " } catch (Exception e) {\n" + + " e.printStackTrace();\n" + + " }\n" + + " return null;\n" + + " }';"; + JdbcTemplate jdbcTemplate = new JdbcTemplate(dbSource); + jdbcTemplate.execute(statementScript); + } + + @BeforeEach + void setUp() { + addH2Function(); + Organization organization = + organizationRepository.save( + Organization.builder().name("test").externalId("biobank:1").build()); + this.discoveryService = + discoveryServiceRepository.save(DiscoveryService.builder().url("").name("").build()); + this.person = savePerson("test"); + this.resource = + resourceRepository.save( + Resource.builder() + .organization(organization) + .discoveryService(discoveryService) + .sourceId("collection:1") + .name("test") + .representatives(new HashSet<>(List.of(person))) + .build()); + this.negotiation = saveNegotiation(this.person); + } + + private Negotiation saveNegotiation(Person author) { + Set requests = new HashSet<>(); + Set resources = new HashSet<>(); + resources.add(resource); + Request request = + Request.builder() + .url("http://test") + .resources(resources) + .discoveryService(discoveryService) + .humanReadable("everything") + .build(); + request = requestRepository.save(request); + requests.add(request); + Negotiation negotiation = + Negotiation.builder() + .currentState(NegotiationState.SUBMITTED) + .requests(requests) + .postsEnabled(false) + .payload(payload) + .build(); + negotiation.setCreatedBy(author); + Role role = roleRepository.save(new Role(1L, "test")); + Set roles = new HashSet<>(); + PersonNegotiationRole personRole = new PersonNegotiationRole(author, negotiation, role); + roles.add(personRole); + negotiation.setPersons(roles); + request.setNegotiation(negotiation); + negotiationRepository.save(negotiation); + return negotiation; + } + + private Person savePerson(String subjectId) { + return personRepository.save( + Person.builder() + .subjectId(subjectId) + .name("John") + .email("test@test.com") + .resources(new HashSet<>()) + .build()); + } + + private Notification saveNotification() { + Notification notification = + Notification.builder() + .id(1L) + .negotiation(this.negotiation) + .recipient(this.person) + .emailStatus(NotificationEmailStatus.EMAIL_NOT_SENT) + .build(); + return notificationRepository.save(notification); + } + + @Test + public void testFindByRecipientAndEmailStatus_ok() { + saveNotification(); + List notificationViewDTOs = + notificationRepository.findViewByRecipientIdAndEmailStatus( + this.person.getId(), NotificationEmailStatus.EMAIL_NOT_SENT); + NotificationViewDTO notificationViewDTO = notificationViewDTOs.get(0); + assertEquals(person.getId(), notificationViewDTO.getRecipient().getId()); + assertEquals(NotificationEmailStatus.EMAIL_NOT_SENT, notificationViewDTO.getEmailStatus()); + assertEquals(this.negotiation.getId(), notificationViewDTO.getNegotiationId()); + assertEquals(parseTitleFromNegotiation(negotiation), notificationViewDTO.getNegotiationTitle()); + } + + private static String parseTitleFromNegotiation(Negotiation negotiation) { + String title; + try { + JSONObject payloadJson = new JSONObject(negotiation.getPayload()); + title = payloadJson.getJSONObject("project").getString("title"); + } catch (JSONException e) { + title = "Untitled negotiation"; + } + return title; + } +} From 0bffd132210e09fb6220feb96046dc98bfb36993 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:14:07 +0200 Subject: [PATCH 09/21] fix(negotiation): remove eager annotation from the Entity Signed-off-by: RadovanTomik --- .../negotiator/database/model/Negotiation.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java index 6fd6570f0..dce8c0141 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java @@ -11,7 +11,6 @@ import jakarta.persistence.Entity; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; -import jakarta.persistence.FetchType; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; @@ -76,18 +75,16 @@ public class Negotiation extends AuditEntity { @OneToMany( mappedBy = "negotiation", - cascade = {CascadeType.MERGE}, - fetch = FetchType.LAZY) + cascade = {CascadeType.MERGE}) private Set attachments; @OneToMany( mappedBy = "negotiation", - cascade = {CascadeType.PERSIST, CascadeType.REMOVE}, - fetch = FetchType.LAZY) + cascade = {CascadeType.PERSIST, CascadeType.REMOVE}) @Exclude private Set persons = new HashSet<>(); - @OneToMany(mappedBy = "negotiation", cascade = CascadeType.MERGE, fetch = FetchType.LAZY) + @OneToMany(mappedBy = "negotiation", cascade = CascadeType.MERGE) @Exclude private Set requests; @@ -104,7 +101,7 @@ public class Negotiation extends AuditEntity { @Enumerated(EnumType.STRING) private NegotiationState currentState = NegotiationState.SUBMITTED; - @ElementCollection(fetch = FetchType.EAGER) + @ElementCollection @CollectionTable( name = "resource_state_per_negotiation", joinColumns = {@JoinColumn(name = "negotiation_id", referencedColumnName = "id")}) @@ -116,7 +113,6 @@ public class Negotiation extends AuditEntity { private Map currentStatePerResource = new HashMap<>(); @OneToMany( - fetch = FetchType.EAGER, cascade = {CascadeType.ALL}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @@ -124,7 +120,6 @@ public class Negotiation extends AuditEntity { private Set lifecycleHistory = creteInitialHistory(); @OneToMany( - fetch = FetchType.EAGER, cascade = {CascadeType.ALL}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) From b088dc36efff24ffd5239971fb584fce4b9ca203 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:30:01 +0200 Subject: [PATCH 10/21] fix(negotiation): remove unnecessary variable Signed-off-by: RadovanTomik --- .../bbmri_eric/negotiator/service/NegotiationServiceImpl.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java index ff20f78ef..ae1304653 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/NegotiationServiceImpl.java @@ -64,9 +64,6 @@ public boolean isNegotiationCreator(String negotiationId) { */ @Override public boolean isAuthorizedForNegotiation(String negotiationId) { - boolean isrepre = - personService.isRepresentativeOfAnyResourceOfNegotiation( - NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId(), negotiationId); return isNegotiationCreator(negotiationId) || personService.isRepresentativeOfAnyResourceOfNegotiation( NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId(), negotiationId); From 25ffed14bdfdffe20d84a2d8f688d4d1d87f95fc Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:30:19 +0200 Subject: [PATCH 11/21] fix(negotiation): remove unnecessary variable Signed-off-by: RadovanTomik --- .../bbmri_eric/negotiator/database/model/Negotiation.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java index dce8c0141..ca24985df 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/database/model/Negotiation.java @@ -112,15 +112,13 @@ public class Negotiation extends AuditEntity { @Builder.Default private Map currentStatePerResource = new HashMap<>(); - @OneToMany( - cascade = {CascadeType.ALL}) + @OneToMany(cascade = {CascadeType.ALL}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @Builder.Default private Set lifecycleHistory = creteInitialHistory(); - @OneToMany( - cascade = {CascadeType.ALL}) + @OneToMany(cascade = {CascadeType.ALL}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @Builder.Default From 50c2ef99a4a31011d889d2d34ecd537958cb113d Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:44:57 +0200 Subject: [PATCH 12/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../resource/ResourcePersistStateChangeListener.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java index 63c374a5e..a2323c462 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java @@ -13,6 +13,7 @@ import org.springframework.context.annotation.Lazy; import org.springframework.lang.Nullable; import org.springframework.messaging.Message; +import org.springframework.scheduling.annotation.Async; import org.springframework.statemachine.StateMachine; import org.springframework.statemachine.recipes.persist.PersistStateMachineHandler; import org.springframework.statemachine.state.State; @@ -55,7 +56,8 @@ private static String parseResourceIdFromMessage(Message message) { return message.getHeaders().get("resourceId", String.class); } - private void notifyRequester(Negotiation negotiation, String resourceId) { + @Async + protected void notifyRequester(Negotiation negotiation, String resourceId) { userNotificationService.notifyRequesterAboutStatusChange( negotiation, negotiation.getResources().stream() From 236cd5ab7a08f221dc12d06d0015f9219ae079ba Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:50:12 +0200 Subject: [PATCH 13/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../resource/ResourcePersistStateChangeListener.java | 4 +--- .../negotiator/service/UserNotificationServiceImpl.java | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java index a2323c462..63c374a5e 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/resource/ResourcePersistStateChangeListener.java @@ -13,7 +13,6 @@ import org.springframework.context.annotation.Lazy; import org.springframework.lang.Nullable; import org.springframework.messaging.Message; -import org.springframework.scheduling.annotation.Async; import org.springframework.statemachine.StateMachine; import org.springframework.statemachine.recipes.persist.PersistStateMachineHandler; import org.springframework.statemachine.state.State; @@ -56,8 +55,7 @@ private static String parseResourceIdFromMessage(Message message) { return message.getHeaders().get("resourceId", String.class); } - @Async - protected void notifyRequester(Negotiation negotiation, String resourceId) { + private void notifyRequester(Negotiation negotiation, String resourceId) { userNotificationService.notifyRequesterAboutStatusChange( negotiation, negotiation.getResources().stream() diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index ed7e3bf30..e91ffd1ec 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -151,6 +151,8 @@ public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { } @Override + @Async + @Transactional public void notifyRequesterAboutStatusChange(Negotiation negotiation, Resource resource) { log.info("Notifying researcher about status change."); notificationRepository.save( From e883bfa85cc327995de2a5f49ad5969e6f74f167 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:52:04 +0200 Subject: [PATCH 14/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../negotiator/service/UserNotificationServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index e91ffd1ec..bf7003977 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -144,6 +144,7 @@ public void notifyAdmins(Negotiation negotiation) { } @Override + @Async public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { log.info("Notifying representatives about new negotiation."); createNotificationsForRepresentatives(negotiation); From 2e84f9e82588f3a624499edfb9333af13889da9b Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:52:36 +0200 Subject: [PATCH 15/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../negotiator/service/UserNotificationServiceImpl.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index bf7003977..2abd9cf1d 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -152,8 +152,6 @@ public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { } @Override - @Async - @Transactional public void notifyRequesterAboutStatusChange(Negotiation negotiation, Resource resource) { log.info("Notifying researcher about status change."); notificationRepository.save( From b4c4b0f38909b00b4f5bc1d857af6f95fec1da51 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 14:59:47 +0200 Subject: [PATCH 16/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../java/eu/bbmri_eric/negotiator/NegotiatorApplication.java | 2 ++ .../negotiation/InitializeStateForResourceAction.java | 2 -- .../negotiator/service/UserNotificationServiceImpl.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java b/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java index 4a152be1f..c0356adb9 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java +++ b/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java @@ -4,12 +4,14 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.domain.EntityScan; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; +import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.scheduling.annotation.EnableScheduling; @SpringBootApplication @EntityScan(basePackages = {"eu.bbmri_eric.negotiator.database.*"}) @EnableJpaRepositories(basePackages = {"eu.bbmri_eric.negotiator.database.repository"}) @EnableScheduling +@EnableAsync public class NegotiatorApplication { public static void main(String[] args) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java index 097a42283..602b098fe 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java @@ -9,7 +9,6 @@ import lombok.extern.apachecommons.CommonsLog; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; -import org.springframework.scheduling.annotation.Async; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.action.Action; import org.springframework.stereotype.Component; @@ -25,7 +24,6 @@ public class InitializeStateForResourceAction implements Action @Override @Transactional - @Async public void execute(StateContext context) { String negotiationId = context.getMessage().getHeaders().get("negotiationId", String.class); Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 2abd9cf1d..29110d13c 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -145,6 +145,7 @@ public void notifyAdmins(Negotiation negotiation) { @Override @Async + @Transactional public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { log.info("Notifying representatives about new negotiation."); createNotificationsForRepresentatives(negotiation); From 9f08eddf5923aa12e3a724e392ec61d553b818ea Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 15:16:31 +0200 Subject: [PATCH 17/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../negotiation/InitializeStateForResourceAction.java | 11 +++++++++++ .../service/UserNotificationServiceImpl.java | 2 -- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java index 602b098fe..435962dd0 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java @@ -9,6 +9,7 @@ import lombok.extern.apachecommons.CommonsLog; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; +import org.springframework.scheduling.annotation.Async; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.action.Action; import org.springframework.stereotype.Component; @@ -26,10 +27,20 @@ public class InitializeStateForResourceAction implements Action @Transactional public void execute(StateContext context) { String negotiationId = context.getMessage().getHeaders().get("negotiationId", String.class); + initializeStateMachine(negotiationId); + notifyReps(negotiationId); + } + + protected void initializeStateMachine(String negotiationId) { Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); for (Resource resource : negotiation.getResources()) { negotiation.setStateForResource(resource.getSourceId(), NegotiationResourceState.SUBMITTED); } + } + + @Async + protected void notifyReps(String negotiationId) { + Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); userNotificationService.notifyRepresentativesAboutNewNegotiation(negotiation); } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 29110d13c..ed7e3bf30 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -144,8 +144,6 @@ public void notifyAdmins(Negotiation negotiation) { } @Override - @Async - @Transactional public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { log.info("Notifying representatives about new negotiation."); createNotificationsForRepresentatives(negotiation); From 3afb418c7440534baaa9fc796993375cfd486519 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 15:50:13 +0200 Subject: [PATCH 18/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../configuration/AsyncEventsConfig.java | 18 +++++++++++++++++ .../InitializeStateForResourceAction.java | 13 +++++------- .../events/NewNegotiationEvent.java | 20 +++++++++++++++++++ .../NewNegotiationEventListener.java | 20 +++++++++++++++++++ .../service/UserNotificationService.java | 7 +++++++ .../service/UserNotificationServiceImpl.java | 8 ++++++++ 6 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java create mode 100644 src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java create mode 100644 src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java new file mode 100644 index 000000000..798205de5 --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java @@ -0,0 +1,18 @@ +package eu.bbmri_eric.negotiator.configuration; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.event.ApplicationEventMulticaster; +import org.springframework.context.event.SimpleApplicationEventMulticaster; +import org.springframework.core.task.SimpleAsyncTaskExecutor; + +@Configuration +public class AsyncEventsConfig { + @Bean(name = "applicationEventMulticaster") + public ApplicationEventMulticaster simpleApplicationEventMulticaster() { + SimpleApplicationEventMulticaster eventMulticaster = new SimpleApplicationEventMulticaster(); + + eventMulticaster.setTaskExecutor(new SimpleAsyncTaskExecutor()); + return eventMulticaster; + } +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java index 435962dd0..68ee41b2d 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java @@ -4,12 +4,13 @@ import eu.bbmri_eric.negotiator.database.model.Negotiation; import eu.bbmri_eric.negotiator.database.model.Resource; import eu.bbmri_eric.negotiator.database.repository.NegotiationRepository; +import eu.bbmri_eric.negotiator.events.NewNegotiationEvent; import eu.bbmri_eric.negotiator.service.UserNotificationService; import jakarta.transaction.Transactional; import lombok.extern.apachecommons.CommonsLog; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Lazy; -import org.springframework.scheduling.annotation.Async; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.action.Action; import org.springframework.stereotype.Component; @@ -23,12 +24,14 @@ public class InitializeStateForResourceAction implements Action @Autowired @Lazy UserNotificationService userNotificationService; + @Autowired @Lazy ApplicationEventPublisher eventPublisher; + @Override @Transactional public void execute(StateContext context) { String negotiationId = context.getMessage().getHeaders().get("negotiationId", String.class); initializeStateMachine(negotiationId); - notifyReps(negotiationId); + eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationId)); } protected void initializeStateMachine(String negotiationId) { @@ -37,10 +40,4 @@ protected void initializeStateMachine(String negotiationId) { negotiation.setStateForResource(resource.getSourceId(), NegotiationResourceState.SUBMITTED); } } - - @Async - protected void notifyReps(String negotiationId) { - Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); - userNotificationService.notifyRepresentativesAboutNewNegotiation(negotiation); - } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java b/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java new file mode 100644 index 000000000..e46fb98ea --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java @@ -0,0 +1,20 @@ +package eu.bbmri_eric.negotiator.events; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.context.ApplicationEvent; + +@Getter +@Setter +public class NewNegotiationEvent extends ApplicationEvent { + private String negotiationId; + + public NewNegotiationEvent(Object source) { + super(source); + } + + public NewNegotiationEvent(Object source, String negotiationId) { + super(source); + this.negotiationId = negotiationId; + } +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java b/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java new file mode 100644 index 000000000..5979afd5b --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java @@ -0,0 +1,20 @@ +package eu.bbmri_eric.negotiator.listeners; + +import eu.bbmri_eric.negotiator.events.NewNegotiationEvent; +import eu.bbmri_eric.negotiator.service.UserNotificationService; +import org.springframework.context.ApplicationListener; +import org.springframework.stereotype.Component; + +@Component +public class NewNegotiationEventListener implements ApplicationListener { + private final UserNotificationService userNotificationService; + + public NewNegotiationEventListener(UserNotificationService userNotificationService) { + this.userNotificationService = userNotificationService; + } + + @Override + public void onApplicationEvent(NewNegotiationEvent event) { + userNotificationService.notifyRepresentativesAboutNewNegotiation(event.getNegotiationId()); + } +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java index 5b6e44f7a..6e897249a 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java @@ -30,6 +30,13 @@ public interface UserNotificationService { */ void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation); + /** + * Create notifications for all representatives of resources involved in a new negotiation. + * + * @param negotiation that was created. + */ + void notifyRepresentativesAboutNewNegotiation(String negotiationId); + /** * Create a notification of a resource status change for the author of the request. * diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index ed7e3bf30..54ff86102 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -150,6 +150,14 @@ public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { markResourcesWithoutARepresentative(negotiation); } + @Override + public void notifyRepresentativesAboutNewNegotiation(String negotiationId) { + log.info("Notifying representatives about new negotiation."); + Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); + createNotificationsForRepresentatives(negotiation); + markResourcesWithoutARepresentative(negotiation); + } + @Override public void notifyRequesterAboutStatusChange(Negotiation negotiation, Resource resource) { log.info("Notifying researcher about status change."); From 443c6eccf9cbb302214cb3c741a5bfc5f47b56c7 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 16:15:29 +0200 Subject: [PATCH 19/21] fix(notifications): make representative notifications generation async Signed-off-by: RadovanTomik --- .../negotiator/service/ResourceLifecycleServiceImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java index 8b7b8e6d8..2e08c4bfe 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java @@ -129,6 +129,8 @@ private boolean isSecurityRuleMet( creatorId = NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId(); } catch (ClassCastException e) { return false; + } catch (NullPointerException e) { + return true; } return negotiationRepository.existsByIdAndCreatedBy_Id(negotiationId, creatorId); } else if (securityRule.getExpression().equals("isRepresentative")) { From 0fa2488eb1d6c17e99462e347149274cec4bd5fa Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 16:27:27 +0200 Subject: [PATCH 20/21] fix(notifications): remove async notification sending Signed-off-by: RadovanTomik --- .../InitializeStateForResourceAction.java | 10 +--------- .../events/NewNegotiationEvent.java | 20 ------------------- .../NewNegotiationEventListener.java | 20 ------------------- .../service/UserNotificationService.java | 7 ------- .../service/UserNotificationServiceImpl.java | 8 -------- 5 files changed, 1 insertion(+), 64 deletions(-) delete mode 100644 src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java delete mode 100644 src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java index 68ee41b2d..602b098fe 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java +++ b/src/main/java/eu/bbmri_eric/negotiator/configuration/state_machine/negotiation/InitializeStateForResourceAction.java @@ -4,12 +4,10 @@ import eu.bbmri_eric.negotiator.database.model.Negotiation; import eu.bbmri_eric.negotiator.database.model.Resource; import eu.bbmri_eric.negotiator.database.repository.NegotiationRepository; -import eu.bbmri_eric.negotiator.events.NewNegotiationEvent; import eu.bbmri_eric.negotiator.service.UserNotificationService; import jakarta.transaction.Transactional; import lombok.extern.apachecommons.CommonsLog; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Lazy; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.action.Action; @@ -24,20 +22,14 @@ public class InitializeStateForResourceAction implements Action @Autowired @Lazy UserNotificationService userNotificationService; - @Autowired @Lazy ApplicationEventPublisher eventPublisher; - @Override @Transactional public void execute(StateContext context) { String negotiationId = context.getMessage().getHeaders().get("negotiationId", String.class); - initializeStateMachine(negotiationId); - eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationId)); - } - - protected void initializeStateMachine(String negotiationId) { Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); for (Resource resource : negotiation.getResources()) { negotiation.setStateForResource(resource.getSourceId(), NegotiationResourceState.SUBMITTED); } + userNotificationService.notifyRepresentativesAboutNewNegotiation(negotiation); } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java b/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java deleted file mode 100644 index e46fb98ea..000000000 --- a/src/main/java/eu/bbmri_eric/negotiator/events/NewNegotiationEvent.java +++ /dev/null @@ -1,20 +0,0 @@ -package eu.bbmri_eric.negotiator.events; - -import lombok.Getter; -import lombok.Setter; -import org.springframework.context.ApplicationEvent; - -@Getter -@Setter -public class NewNegotiationEvent extends ApplicationEvent { - private String negotiationId; - - public NewNegotiationEvent(Object source) { - super(source); - } - - public NewNegotiationEvent(Object source, String negotiationId) { - super(source); - this.negotiationId = negotiationId; - } -} diff --git a/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java b/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java deleted file mode 100644 index 5979afd5b..000000000 --- a/src/main/java/eu/bbmri_eric/negotiator/listeners/NewNegotiationEventListener.java +++ /dev/null @@ -1,20 +0,0 @@ -package eu.bbmri_eric.negotiator.listeners; - -import eu.bbmri_eric.negotiator.events.NewNegotiationEvent; -import eu.bbmri_eric.negotiator.service.UserNotificationService; -import org.springframework.context.ApplicationListener; -import org.springframework.stereotype.Component; - -@Component -public class NewNegotiationEventListener implements ApplicationListener { - private final UserNotificationService userNotificationService; - - public NewNegotiationEventListener(UserNotificationService userNotificationService) { - this.userNotificationService = userNotificationService; - } - - @Override - public void onApplicationEvent(NewNegotiationEvent event) { - userNotificationService.notifyRepresentativesAboutNewNegotiation(event.getNegotiationId()); - } -} diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java index 6e897249a..5b6e44f7a 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationService.java @@ -30,13 +30,6 @@ public interface UserNotificationService { */ void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation); - /** - * Create notifications for all representatives of resources involved in a new negotiation. - * - * @param negotiation that was created. - */ - void notifyRepresentativesAboutNewNegotiation(String negotiationId); - /** * Create a notification of a resource status change for the author of the request. * diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java index 54ff86102..ed7e3bf30 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/UserNotificationServiceImpl.java @@ -150,14 +150,6 @@ public void notifyRepresentativesAboutNewNegotiation(Negotiation negotiation) { markResourcesWithoutARepresentative(negotiation); } - @Override - public void notifyRepresentativesAboutNewNegotiation(String negotiationId) { - log.info("Notifying representatives about new negotiation."); - Negotiation negotiation = negotiationRepository.findDetailedById(negotiationId).orElseThrow(); - createNotificationsForRepresentatives(negotiation); - markResourcesWithoutARepresentative(negotiation); - } - @Override public void notifyRequesterAboutStatusChange(Negotiation negotiation, Resource resource) { log.info("Notifying researcher about status change."); From 4c890a5fe2dec7ef621bda29738c61164d87c821 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Tue, 21 May 2024 16:29:39 +0200 Subject: [PATCH 21/21] fix(notifications): remove async notification sending Signed-off-by: RadovanTomik --- .../negotiator/NegotiatorApplication.java | 2 -- .../configuration/AsyncEventsConfig.java | 18 ------------------ .../service/ResourceLifecycleServiceImpl.java | 2 -- 3 files changed, 22 deletions(-) delete mode 100644 src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java b/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java index c0356adb9..4a152be1f 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java +++ b/src/main/java/eu/bbmri_eric/negotiator/NegotiatorApplication.java @@ -4,14 +4,12 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.domain.EntityScan; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; -import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.scheduling.annotation.EnableScheduling; @SpringBootApplication @EntityScan(basePackages = {"eu.bbmri_eric.negotiator.database.*"}) @EnableJpaRepositories(basePackages = {"eu.bbmri_eric.negotiator.database.repository"}) @EnableScheduling -@EnableAsync public class NegotiatorApplication { public static void main(String[] args) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java b/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java deleted file mode 100644 index 798205de5..000000000 --- a/src/main/java/eu/bbmri_eric/negotiator/configuration/AsyncEventsConfig.java +++ /dev/null @@ -1,18 +0,0 @@ -package eu.bbmri_eric.negotiator.configuration; - -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.event.ApplicationEventMulticaster; -import org.springframework.context.event.SimpleApplicationEventMulticaster; -import org.springframework.core.task.SimpleAsyncTaskExecutor; - -@Configuration -public class AsyncEventsConfig { - @Bean(name = "applicationEventMulticaster") - public ApplicationEventMulticaster simpleApplicationEventMulticaster() { - SimpleApplicationEventMulticaster eventMulticaster = new SimpleApplicationEventMulticaster(); - - eventMulticaster.setTaskExecutor(new SimpleAsyncTaskExecutor()); - return eventMulticaster; - } -} diff --git a/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java index 2e08c4bfe..8b7b8e6d8 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/service/ResourceLifecycleServiceImpl.java @@ -129,8 +129,6 @@ private boolean isSecurityRuleMet( creatorId = NegotiatorUserDetailsService.getCurrentlyAuthenticatedUserInternalId(); } catch (ClassCastException e) { return false; - } catch (NullPointerException e) { - return true; } return negotiationRepository.existsByIdAndCreatedBy_Id(negotiationId, creatorId); } else if (securityRule.getExpression().equals("isRepresentative")) {