From 1fb8e0c135402cd0517df069295e4db80a5247b2 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Mon, 6 Mar 2023 18:48:37 +0100 Subject: [PATCH 1/7] RED-6310: Refomatted sql query for readability --- .../repository/NotificationRepository.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationRepository.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationRepository.java index 3346bbb67..7fea3d1e9 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationRepository.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/NotificationRepository.java @@ -15,11 +15,19 @@ public interface NotificationRepository extends JpaRepository findAppNotificationsForUser(String userId); - @Query("select n from NotificationEntity n where " + " (exists (select e from NotificationPreferencesEntity e where e.userId = :userId) and n.notificationType in ( select apn from NotificationPreferencesEntity p join p.inAppNotifications apn where p.userId = :userId and p.inAppNotificationsEnabled = true )) " + " and n.seenDate is null and n.softDeleted is null and n.userId = :userId order by n.creationDate desc ") + @Query(""" + select n from NotificationEntity n where + (exists (select e from NotificationPreferencesEntity e where e.userId = :userId) and n.notificationType in + (select apn from NotificationPreferencesEntity p join p.inAppNotifications apn where p.userId = :userId and p.inAppNotificationsEnabled = true)) + and n.seenDate is null and n.softDeleted is null and n.userId = :userId order by n.creationDate desc""") List findUnseenNotificationsForUser(String userId); From ef36f5c10f21042ce74621dd8e48a754b6a7258f Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Mon, 6 Mar 2023 18:58:07 +0100 Subject: [PATCH 2/7] RED-6310: Added tests to check if concurrent access to notification-preferences works --- .../NotificationPreferencesServiceTest.java | 75 ++++++++++++++++ .../integration/tests/NotificationTest.java | 87 +++++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java 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 new file mode 100644 index 000000000..57f761f5f --- /dev/null +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationPreferencesServiceTest.java @@ -0,0 +1,75 @@ +package com.iqser.red.service.peristence.v1.server.integration.tests; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.Collections; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import com.iqser.red.service.peristence.v1.server.controller.NotificationController; +import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; +import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; + +import lombok.AccessLevel; +import lombok.SneakyThrows; +import lombok.experimental.FieldDefaults; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@FieldDefaults(level = AccessLevel.PRIVATE) +public class NotificationPreferencesServiceTest extends AbstractPersistenceServerServiceTest { + + @Autowired + NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; + + @Autowired + NotificationController notificationController; + + + // Disabled since tenant-id is not used correctly when directly using the service + @Disabled + @Test + @SneakyThrows + public void testNotificationPreferencesConcurrent() { + + final String userId = "1"; + + for (int i = 0; i < 1000; i++) { + var exceptions = Collections.synchronizedList(new ArrayList()); + + Thread t1 = new Thread(() -> { + try { + notificationController.getNotifications(userId, true); + notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); + } catch (Exception e) { + exceptions.add(e); + } + }); + Thread t2 = new Thread(() -> { + try { + notificationController.getNotifications(userId, true); + notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); + } catch (Exception e) { + exceptions.add(e); + } + }); + t1.start(); + t2.start(); + t1.join(); + t2.join(); + + notificationPreferencesPersistenceService.deleteNotificationPreferences(userId); + + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); + } + + assertThat(exceptions).isEmpty(); + } + + } + +} diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java index e76936759..8e248ae66 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java @@ -3,6 +3,8 @@ package com.iqser.red.service.peristence.v1.server.integration.tests; import static org.assertj.core.api.Assertions.assertThat; import java.time.OffsetDateTime; +import java.util.ArrayList; +import java.util.Collections; import java.util.Map; import org.assertj.core.util.Lists; @@ -17,6 +19,11 @@ import com.iqser.red.service.persistence.service.v1.api.model.common.JSONPrimiti import com.iqser.red.service.persistence.service.v1.api.model.notification.Notification; import com.iqser.red.service.persistence.service.v1.api.model.notification.NotificationType; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; + +@SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection") +@Slf4j public class NotificationTest extends AbstractPersistenceServerServiceTest { @Autowired @@ -106,4 +113,84 @@ public class NotificationTest extends AbstractPersistenceServerServiceTest { return currentNotifications.iterator().next(); } + + @Test + @SneakyThrows + public void testNotificationPreferencesConcurrent() { + + final String userId = "1"; + + for (int i = 0; i < 1000; i++) { + var exceptions = Collections.synchronizedList(new ArrayList()); + + Thread t1 = new Thread(() -> { + try { + notificationPreferencesClient.getNotificationPreferences(userId); + } catch (Exception e) { + exceptions.add(e); + } + }); + Thread t2 = new Thread(() -> { + try { + notificationPreferencesClient.getNotificationPreferences(userId); + } catch (Exception e) { + exceptions.add(e); + } + }); + t1.start(); + t2.start(); + t1.join(); + t2.join(); + + notificationPreferencesClient.deleteNotificationPreferences(userId); + + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); + } + + assertThat(exceptions).isEmpty(); + } + + } + + + @Test + @SneakyThrows + public void testNotificationsConcurrent() { + + final String userId = "1"; + + for (int i = 0; i < 1000; i++) { + var exceptions = Collections.synchronizedList(new ArrayList()); + + Thread t1 = new Thread(() -> { + try { + notificationClient.getNotifications(userId, false); + } catch (Exception e) { + exceptions.add(e); + } + }); + Thread t2 = new Thread(() -> { + try { + notificationClient.getNotifications(userId, false); + } catch (Exception e) { + exceptions.add(e); + } + }); + t1.start(); + t2.start(); + t1.join(); + t2.join(); + + notificationPreferencesClient.deleteNotificationPreferences(userId); + + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); + } + + assertThat(exceptions).isEmpty(); + } + + } + } From b1d79970c033822ddffb47be199c379cada2ce40 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 10:53:47 +0100 Subject: [PATCH 3/7] RED-6310: Added setup of tenant id to fix the service test --- .../tests/NotificationPreferencesServiceTest.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 57f761f5f..aecaf4575 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 @@ -5,13 +5,14 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; import java.util.Collections; -import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import com.iqser.red.service.peristence.v1.server.controller.NotificationController; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; +import com.iqser.red.service.persistence.management.v1.processor.utils.multitenancy.TenantContext; import lombok.AccessLevel; import lombok.SneakyThrows; @@ -28,9 +29,13 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe @Autowired NotificationController notificationController; + @BeforeEach + public void setup() { + + TenantContext.setTenantId("redaction"); + } + - // Disabled since tenant-id is not used correctly when directly using the service - @Disabled @Test @SneakyThrows public void testNotificationPreferencesConcurrent() { From fd7f39bc7e4b4cc67ed55d5e77ef84d93779b342 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 10:57:33 +0100 Subject: [PATCH 4/7] RED-6310: Removed not needed code from test --- .../tests/NotificationPreferencesServiceTest.java | 6 ------ 1 file changed, 6 deletions(-) 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 aecaf4575..9348d16b0 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 @@ -9,7 +9,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import com.iqser.red.service.peristence.v1.server.controller.NotificationController; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; import com.iqser.red.service.persistence.management.v1.processor.service.persistence.NotificationPreferencesPersistenceService; import com.iqser.red.service.persistence.management.v1.processor.utils.multitenancy.TenantContext; @@ -26,9 +25,6 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe @Autowired NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; - @Autowired - NotificationController notificationController; - @BeforeEach public void setup() { @@ -47,7 +43,6 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe Thread t1 = new Thread(() -> { try { - notificationController.getNotifications(userId, true); notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); } catch (Exception e) { exceptions.add(e); @@ -55,7 +50,6 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe }); Thread t2 = new Thread(() -> { try { - notificationController.getNotifications(userId, true); notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); } catch (Exception e) { exceptions.add(e); From 40d6961742436fd4f7c5e07616850d8b1eb31a43 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 11:36:44 +0100 Subject: [PATCH 5/7] RED-6310: Moved code for multithreaded tests to a helper class --- .../NotificationPreferencesServiceTest.java | 42 +++-------- .../integration/tests/NotificationTest.java | 75 ++++--------------- .../utils/MultithreadedTestRunner.java | 71 ++++++++++++++++++ 3 files changed, 96 insertions(+), 92 deletions(-) create mode 100644 persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/utils/MultithreadedTestRunner.java 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 9348d16b0..75428b5c3 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 @@ -2,14 +2,12 @@ package com.iqser.red.service.peristence.v1.server.integration.tests; import static org.assertj.core.api.Assertions.assertThat; -import java.util.ArrayList; -import java.util.Collections; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; 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.persistence.NotificationPreferencesPersistenceService; import com.iqser.red.service.persistence.management.v1.processor.utils.multitenancy.TenantContext; @@ -25,6 +23,9 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe @Autowired NotificationPreferencesPersistenceService notificationPreferencesPersistenceService; + final MultithreadedTestRunner multithreadedTestRunner = new MultithreadedTestRunner(2, 1000); + + @BeforeEach public void setup() { @@ -37,38 +38,15 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe public void testNotificationPreferencesConcurrent() { final String userId = "1"; + Runnable test = () -> notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); + Runnable afterTest = () -> notificationPreferencesPersistenceService.deleteNotificationPreferences(userId); + var exceptions = multithreadedTestRunner.runMutlithreadedCollectingExceptions(test, afterTest); - for (int i = 0; i < 1000; i++) { - var exceptions = Collections.synchronizedList(new ArrayList()); - - Thread t1 = new Thread(() -> { - try { - notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); - } catch (Exception e) { - exceptions.add(e); - } - }); - Thread t2 = new Thread(() -> { - try { - notificationPreferencesPersistenceService.getOrCreateNotificationPreferences(userId); - } catch (Exception e) { - exceptions.add(e); - } - }); - t1.start(); - t2.start(); - t1.join(); - t2.join(); - - notificationPreferencesPersistenceService.deleteNotificationPreferences(userId); - - for (Exception ex : exceptions) { - log.error("Exception during notification creation", ex); - } - - assertThat(exceptions).isEmpty(); + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); } + assertThat(exceptions).isEmpty(); } } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java index 8e248ae66..2dac99468 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/NotificationTest.java @@ -3,8 +3,6 @@ package com.iqser.red.service.peristence.v1.server.integration.tests; import static org.assertj.core.api.Assertions.assertThat; import java.time.OffsetDateTime; -import java.util.ArrayList; -import java.util.Collections; import java.util.Map; import org.assertj.core.util.Lists; @@ -14,6 +12,7 @@ import org.springframework.beans.factory.annotation.Autowired; import com.iqser.red.service.peristence.v1.server.integration.client.NotificationClient; import com.iqser.red.service.peristence.v1.server.integration.client.NotificationPreferencesClient; 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.service.v1.api.model.audit.AddNotificationRequest; import com.iqser.red.service.persistence.service.v1.api.model.common.JSONPrimitive; import com.iqser.red.service.persistence.service.v1.api.model.notification.Notification; @@ -32,6 +31,8 @@ public class NotificationTest extends AbstractPersistenceServerServiceTest { @Autowired private NotificationPreferencesClient notificationPreferencesClient; + private final MultithreadedTestRunner multithreadedTestRunner = new MultithreadedTestRunner(2, 1000); + @Test public void testNotificationPreferences() { @@ -119,38 +120,15 @@ public class NotificationTest extends AbstractPersistenceServerServiceTest { public void testNotificationPreferencesConcurrent() { final String userId = "1"; + Runnable test = () -> notificationPreferencesClient.getNotificationPreferences(userId); + Runnable afterTest = () -> notificationPreferencesClient.deleteNotificationPreferences(userId); + var exceptions = multithreadedTestRunner.runMutlithreadedCollectingExceptions(test, afterTest); - for (int i = 0; i < 1000; i++) { - var exceptions = Collections.synchronizedList(new ArrayList()); - - Thread t1 = new Thread(() -> { - try { - notificationPreferencesClient.getNotificationPreferences(userId); - } catch (Exception e) { - exceptions.add(e); - } - }); - Thread t2 = new Thread(() -> { - try { - notificationPreferencesClient.getNotificationPreferences(userId); - } catch (Exception e) { - exceptions.add(e); - } - }); - t1.start(); - t2.start(); - t1.join(); - t2.join(); - - notificationPreferencesClient.deleteNotificationPreferences(userId); - - for (Exception ex : exceptions) { - log.error("Exception during notification creation", ex); - } - - assertThat(exceptions).isEmpty(); + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); } + assertThat(exceptions).isEmpty(); } @@ -159,38 +137,15 @@ public class NotificationTest extends AbstractPersistenceServerServiceTest { public void testNotificationsConcurrent() { final String userId = "1"; + Runnable test = () -> notificationClient.getNotifications(userId, false); + Runnable afterTest = () -> notificationPreferencesClient.deleteNotificationPreferences(userId); + var exceptions = multithreadedTestRunner.runMutlithreadedCollectingExceptions(test, afterTest); - for (int i = 0; i < 1000; i++) { - var exceptions = Collections.synchronizedList(new ArrayList()); - - Thread t1 = new Thread(() -> { - try { - notificationClient.getNotifications(userId, false); - } catch (Exception e) { - exceptions.add(e); - } - }); - Thread t2 = new Thread(() -> { - try { - notificationClient.getNotifications(userId, false); - } catch (Exception e) { - exceptions.add(e); - } - }); - t1.start(); - t2.start(); - t1.join(); - t2.join(); - - notificationPreferencesClient.deleteNotificationPreferences(userId); - - for (Exception ex : exceptions) { - log.error("Exception during notification creation", ex); - } - - assertThat(exceptions).isEmpty(); + for (Exception ex : exceptions) { + log.error("Exception during notification creation", ex); } + assertThat(exceptions).isEmpty(); } } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/utils/MultithreadedTestRunner.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/utils/MultithreadedTestRunner.java new file mode 100644 index 000000000..2dd4cc6c8 --- /dev/null +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/utils/MultithreadedTestRunner.java @@ -0,0 +1,71 @@ +package com.iqser.red.service.peristence.v1.server.integration.utils; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; +import lombok.experimental.FieldDefaults; + +@RequiredArgsConstructor +@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) +public class MultithreadedTestRunner { + + int numberOfThreads; + int numberOfExecutions; + + + public List runMutlithreadedCollectingExceptions(boolean stopOnFirstRunWithExceptions, Runnable test, Runnable afterTest) { + + List allExceptions = new ArrayList<>(); + + for (int execution = 1; execution <= numberOfExecutions; execution++) { + var threads = new ArrayList(numberOfThreads); + var exceptions = Collections.synchronizedList(new ArrayList()); + + for (int threadNumber = 1; threadNumber <= numberOfThreads; threadNumber++) { + Thread t = new Thread(() -> { + try { + test.run(); + } catch (Exception e) { + exceptions.add(e); + } + }); + + threads.add(t); + } + + for (Thread t : threads) { + t.start(); + } + + for (Thread t : threads) { + try { + t.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + afterTest.run(); + + if (stopOnFirstRunWithExceptions) { + if (!exceptions.isEmpty()) { + return exceptions; + } + } else { + allExceptions.addAll(exceptions); + } + } + + return allExceptions; + } + + + public List runMutlithreadedCollectingExceptions(Runnable test, Runnable afterTest) { + + return runMutlithreadedCollectingExceptions(true, test, afterTest); + } + +} From 643ffc8723d923a7ede8ee52e25ea40e25045148 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 13:34:05 +0100 Subject: [PATCH 6/7] RED-6310: Corrected services so that they use the user id instead of wrongly using the entity id --- .../NotificationPreferencesPersistenceService.java | 8 +++----- .../repository/NotificationPreferencesRepository.java | 7 ++++++- 2 files changed, 9 insertions(+), 6 deletions(-) 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 cb125312b..57b270993 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 @@ -60,14 +60,14 @@ public class NotificationPreferencesPersistenceService { @Transactional public void deleteNotificationPreferences(String userId) { - notificationPreferencesRepository.deleteById(userId); + notificationPreferencesRepository.deleteByUserId(userId); } @Transactional public NotificationPreferencesEntity getOrCreateNotificationPreferences(String userId) { - return notificationPreferencesRepository.findById(userId).orElseGet(() -> { + return notificationPreferencesRepository.findByUserId(userId).orElseGet(() -> { var notificationPreference = new NotificationPreferencesEntity(); notificationPreference.setUserId(userId); @@ -82,9 +82,7 @@ public class NotificationPreferencesPersistenceService { @Transactional public void initializePreferencesIfNotExists(String userId) { - if (!notificationPreferencesRepository.existsByUserId(userId)) { - getOrCreateNotificationPreferences(userId); - } + getOrCreateNotificationPreferences(userId); } 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 fa804686d..f151527e4 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 @@ -1,11 +1,16 @@ package com.iqser.red.service.persistence.management.v1.processor.service.persistence.repository; +import java.util.Optional; + import org.springframework.data.jpa.repository.JpaRepository; import com.iqser.red.service.persistence.management.v1.processor.entity.notification.NotificationPreferencesEntity; public interface NotificationPreferencesRepository extends JpaRepository { - boolean existsByUserId(String userId); + Optional findByUserId(String userId); + + + void deleteByUserId(String userId); } From 45bd8e600328ce85c8457525b605da2c16dd70c8 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 18:32:33 +0100 Subject: [PATCH 7/7] RED-6310: Moved code to create user-preferences to a separate class so that the calling code can handle a persistence exception --- ...ficationPreferencesPersistenceService.java | 55 +++++++++++++++---- .../NotificationPreferencesRepository.java | 3 + 2 files changed, 46 insertions(+), 12 deletions(-) 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 57b270993..d6605b8ec 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 @@ -10,6 +10,8 @@ import java.util.stream.Collectors; import javax.transaction.Transactional; import org.springframework.beans.BeanUtils; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.stereotype.Component; import org.springframework.stereotype.Service; import com.iqser.red.service.persistence.management.v1.processor.entity.notification.NotificationPreferencesEntity; @@ -18,15 +20,20 @@ import com.iqser.red.service.persistence.management.v1.processor.service.persist import com.iqser.red.service.persistence.service.v1.api.model.notification.NotificationPreferences; import com.iqser.red.service.persistence.service.v1.api.model.notification.NotificationType; +import lombok.AccessLevel; import lombok.RequiredArgsConstructor; +import lombok.experimental.FieldDefaults; @Service @RequiredArgsConstructor +@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) public class NotificationPreferencesPersistenceService { - private final NotificationPreferencesRepository notificationPreferencesRepository; + NotificationPreferencesRepository notificationPreferencesRepository; - private final NotificationRepository notificationRepository; + NonThreadSafeNotificationPreferencesRepositoryWrapper notificationPreferencesRepositoryWrapper; + + NotificationRepository notificationRepository; @Transactional @@ -64,18 +71,17 @@ public class NotificationPreferencesPersistenceService { } - @Transactional + // This method intentionally does not have a @Transactional annotation, since it needs to handle an underlying transaction exception. public NotificationPreferencesEntity getOrCreateNotificationPreferences(String userId) { - return 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); - }); + 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 notificationPreferencesRepository.getByUserId(userId); + } } @@ -91,4 +97,29 @@ public class NotificationPreferencesPersistenceService { return notificationPreferencesRepository.findAll(); } + + @Component + @RequiredArgsConstructor + @FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) + private static class NonThreadSafeNotificationPreferencesRepositoryWrapper { + + NotificationPreferencesRepository notificationPreferencesRepository; + + + @Transactional(Transactional.TxType.REQUIRES_NEW) + public NotificationPreferencesEntity getOrCreateNotificationPreferences(String userId) { + + return 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); + }); + } + + } + } 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 f151527e4..4340130f8 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 @@ -11,6 +11,9 @@ public interface NotificationPreferencesRepository extends JpaRepository findByUserId(String userId); + NotificationPreferencesEntity getByUserId(String userId); + + void deleteByUserId(String userId); }