From cf4a650fb55bbc25ecd1991bea9285fd8f10d29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominique=20Eifl=C3=A4nder?= Date: Tue, 9 Jan 2024 10:38:09 +0100 Subject: [PATCH] RED-8174: Fixed transaction handling for notification preferences --- .../NotificationPreferencesController.java | 4 +- .../NotificationPreferencesService.java | 39 +++++++++ .../NotificationPersistenceService.java | 12 +-- ...ficationPreferencesPersistenceService.java | 87 ++++++------------- .../NotificationPreferencesRepository.java | 4 - .../NotificationPreferencesServiceTest.java | 7 +- 6 files changed, 81 insertions(+), 72 deletions(-) create mode 100644 persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/NotificationPreferencesService.java diff --git a/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/NotificationPreferencesController.java b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/NotificationPreferencesController.java index 08c00771f..ce468c250 100644 --- a/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/NotificationPreferencesController.java +++ b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/NotificationPreferencesController.java @@ -9,6 +9,7 @@ import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RestController; +import com.iqser.red.service.persistence.management.v1.processor.service.NotificationPreferencesService; import com.knecon.fforesight.keycloakcommons.security.KeycloakSecurity; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; import com.knecon.fforesight.databasetenantcommons.providers.utils.MagicConverter; @@ -21,13 +22,14 @@ import lombok.RequiredArgsConstructor; @RequiredArgsConstructor public class NotificationPreferencesController implements NotificationPreferencesResource { + private final NotificationPreferencesService notificationPreferencesService; private final NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; @Override @PreAuthorize("hasAuthority('" + READ_NOTIFICATIONS + "')") public NotificationPreferences getNotificationPreferences() { - return notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(KeycloakSecurity.getUserId()); + return notificationPreferencesService.getOrCreateNotificationPreferences(KeycloakSecurity.getUserId()); } diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/NotificationPreferencesService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/NotificationPreferencesService.java new file mode 100644 index 000000000..3f2738823 --- /dev/null +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/NotificationPreferencesService.java @@ -0,0 +1,39 @@ +package com.iqser.red.service.persistence.management.v1.processor.service; + +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.stereotype.Service; + +import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; +import com.iqser.red.service.persistence.service.v1.api.shared.model.notification.NotificationPreferences; + +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; +import lombok.experimental.FieldDefaults; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@Service +@RequiredArgsConstructor +@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) +public class NotificationPreferencesService { + + NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; + + public NotificationPreferences getOrCreateNotificationPreferences(String userId) { + + var notificationPreferences = notificationPreferencesPersistenceService.getByUserId(userId); + + if (notificationPreferences != null){ + return notificationPreferences; + } + + try { + notificationPreferencesPersistenceService.createNotificationPreferences(userId); + } catch (DataIntegrityViolationException ex) { + log.debug("NotificationPreferences might be already created by another Thread, ignoring...", ex); + } + + return notificationPreferencesPersistenceService.getByUserId(userId); + } + +} diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPersistenceService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPersistenceService.java index e6a4940d8..e7e08b387 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPersistenceService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPersistenceService.java @@ -4,17 +4,17 @@ import java.time.OffsetDateTime; import java.time.temporal.ChronoUnit; import java.util.List; -import jakarta.transaction.Transactional; - import org.springframework.beans.BeanUtils; import org.springframework.stereotype.Service; import com.iqser.red.service.persistence.management.v1.processor.entity.notification.NotificationEntity; import com.iqser.red.service.persistence.management.v1.processor.exception.NotFoundException; +import com.iqser.red.service.persistence.management.v1.processor.service.NotificationPreferencesService; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.repository.NotificationRepository; import com.iqser.red.service.persistence.service.v1.api.shared.model.audit.AddNotificationRequest; import com.iqser.red.service.persistence.service.v1.api.shared.model.notification.EmailNotificationType; +import jakarta.transaction.Transactional; import lombok.RequiredArgsConstructor; import lombok.SneakyThrows; @@ -24,14 +24,14 @@ public class NotificationPersistenceService { private final NotificationRepository notificationRepository; - private final NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; + private final NotificationPreferencesService notificationPreferencesService; private final NotificationEmailService notificationEmailService; public boolean hasNewNotificationsSince(String userId, OffsetDateTime since) { - notificationPreferencesPersistenceService.initializePreferencesIfNotExists(userId); + notificationPreferencesService.getOrCreateNotificationPreferences(userId); return notificationRepository.hasInAppNotificationForUser(userId, since.truncatedTo(ChronoUnit.MILLIS)) > 0; } @@ -44,7 +44,7 @@ public class NotificationPersistenceService { BeanUtils.copyProperties(addNotificationRequest, notification); notification.setCreationDate(OffsetDateTime.now()); - var notificationPreferences = notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(addNotificationRequest.getUserId()); + var notificationPreferences = notificationPreferencesService.getOrCreateNotificationPreferences(addNotificationRequest.getUserId()); if (!notificationPreferences.isInAppNotificationsEnabled() || !notificationPreferences.getInAppNotifications().contains(notification.getNotificationType())) { notification.setSoftDeleted(OffsetDateTime.now().truncatedTo(ChronoUnit.MILLIS)); } @@ -103,7 +103,7 @@ public class NotificationPersistenceService { public List getNotifications(String userId, boolean includeSeen) { - notificationPreferencesPersistenceService.initializePreferencesIfNotExists(userId); + notificationPreferencesService.getOrCreateNotificationPreferences(userId); if (includeSeen) { return notificationRepository.findAppNotificationsForUser(userId); } else { diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPreferencesPersistenceService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPreferencesPersistenceService.java index 31c1a88aa..53015482b 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPreferencesPersistenceService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/NotificationPreferencesPersistenceService.java @@ -5,6 +5,7 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import jakarta.transaction.Transactional; @@ -15,6 +16,7 @@ import org.springframework.stereotype.Component; import org.springframework.stereotype.Service; import com.iqser.red.service.persistence.management.v1.processor.entity.notification.NotificationPreferencesEntity; +import com.iqser.red.service.persistence.management.v1.processor.exception.NotFoundException; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.repository.NotificationPreferencesRepository; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.repository.NotificationRepository; import com.knecon.fforesight.databasetenantcommons.providers.utils.MagicConverter; @@ -32,11 +34,29 @@ public class NotificationPreferencesPersistenceService { NotificationPreferencesRepository notificationPreferencesRepository; - NonThreadSafeNotificationPreferencesRepositoryWrapper notificationPreferencesRepositoryWrapper; - NotificationRepository notificationRepository; + @Transactional + public void createNotificationPreferences(String userId) { + + var notificationPreference = new NotificationPreferencesEntity(); + notificationPreference.setUserId(userId); + notificationPreference.setEmailNotificationsEnabled(false); + notificationPreference.setInAppNotificationsEnabled(true); + notificationPreference.setInAppNotifications(Arrays.stream(NotificationType.values()).map(Enum::name).collect(Collectors.toList())); + notificationPreferencesRepository.save(notificationPreference); + } + + @Transactional + public NotificationPreferences getByUserId(String userId){ + + var notificationPreferencesEntityOptional = notificationPreferencesRepository.findByUserId(userId); + + return notificationPreferencesEntityOptional.map(this::convertPreferences).orElse(null); + } + + @Transactional public void setNotificationPreference(String userId, NotificationPreferences newNotificationPreferences) { @@ -72,71 +92,18 @@ public class NotificationPreferencesPersistenceService { } - // This method intentionally does not have a @Transactional annotation, since it needs to handle an underlying transaction exception. - public NotificationPreferences getOrCreateNotificationPreferences(String userId) { - - try { - // The method called here will fail if it is called concurrently (more than 1 thread), since it will always try to create - // the desired entity. But the exception only means, that the entity has been created by another thread. - // In that case we can just fetch the data from the db. - return notificationPreferencesRepositoryWrapper.getOrCreateNotificationPreferences(userId); - } catch (DataIntegrityViolationException ex) { - return notificationPreferencesRepositoryWrapper.getByUserId(userId); - } - } - - - @Transactional - public void initializePreferencesIfNotExists(String userId) { - - getOrCreateNotificationPreferences(userId); - } - - public List findAll() { return notificationPreferencesRepository.findAll(); } - @Component - @RequiredArgsConstructor - @FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) - private static class NonThreadSafeNotificationPreferencesRepositoryWrapper { - - NotificationPreferencesRepository notificationPreferencesRepository; - - @Transactional(Transactional.TxType.REQUIRES_NEW) - public NotificationPreferences getByUserId(String userId){ - return convertPreferences(notificationPreferencesRepository.getByUserId(userId)); - } - - - - @Transactional(Transactional.TxType.REQUIRES_NEW) - public NotificationPreferences getOrCreateNotificationPreferences(String userId) { - - var preferences = notificationPreferencesRepository.findByUserId(userId).orElseGet(() -> { - - var notificationPreference = new NotificationPreferencesEntity(); - notificationPreference.setUserId(userId); - notificationPreference.setEmailNotificationsEnabled(false); - notificationPreference.setInAppNotificationsEnabled(true); - notificationPreference.setInAppNotifications(Arrays.stream(NotificationType.values()).map(Enum::name).collect(Collectors.toList())); - return notificationPreferencesRepository.save(notificationPreference); - }); - - return convertPreferences(preferences); - - } - - private NotificationPreferences convertPreferences(NotificationPreferencesEntity preferences){ - var result = MagicConverter.convert(preferences, NotificationPreferences.class); - result.setInAppNotifications(new ArrayList<>(preferences.getInAppNotifications())); - result.setEmailNotifications(new ArrayList<>(preferences.getEmailNotifications())); - return result; - } + private NotificationPreferences convertPreferences(NotificationPreferencesEntity preferences){ + var result = MagicConverter.convert(preferences, NotificationPreferences.class); + result.setInAppNotifications(new ArrayList<>(preferences.getInAppNotifications())); + result.setEmailNotifications(new ArrayList<>(preferences.getEmailNotifications())); + return result; } } diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationPreferencesRepository.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationPreferencesRepository.java index 4340130f8..ed7e6c227 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationPreferencesRepository.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationPreferencesRepository.java @@ -10,10 +10,6 @@ public interface NotificationPreferencesRepository extends JpaRepository findByUserId(String userId); - - NotificationPreferencesEntity getByUserId(String userId); - - void deleteByUserId(String userId); } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java index 44ad766ba..8ee1e9c12 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java @@ -8,6 +8,7 @@ import org.springframework.beans.factory.annotation.Autowired; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; import com.iqser.red.service.peristence.v1.server.integration.utils.MultithreadedTestRunner; +import com.iqser.red.service.persistence.management.v1.processor.service.NotificationPreferencesService; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; import com.knecon.fforesight.tenantcommons.TenantContext; @@ -20,6 +21,10 @@ import lombok.extern.slf4j.Slf4j; @FieldDefaults(level = AccessLevel.PRIVATE) public class NotificationPreferencesServiceTest extends AbstractPersistenceServerServiceTest { + @Autowired + NotificationPreferencesService notificationPreferencesService; + + @Autowired NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; @@ -38,7 +43,7 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe public void testNotificationPreferencesConcurrent() { final String userId = "1"; - Runnable test = () -> notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); + Runnable test = () -> notificationPreferencesService.getOrCreateNotificationPreferences(userId); Runnable afterTest = () -> notificationPreferencesPersistenceService.deleteNotificationPreferences(userId); var exceptions = multithreadedTestRunner.runMutlithreadedCollectingExceptions(test, afterTest);