From bbc58379f46daeaab8aae18ef441b49cda4c7a5d Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Mon, 6 Mar 2023 18:58:07 +0100 Subject: [PATCH 1/7] RED-6310: Added tests to check if concurrent access to notification-preferences works (cherry picked from commit ef36f5c10f21042ce74621dd8e48a754b6a7258f) --- .../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 8ad236e58..264d06274 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 d42d56e7f15c35a26263ab8bd442fb43ce9668c3 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 10:53:47 +0100 Subject: [PATCH 2/7] RED-6310: Added setup of tenant id to fix the service test (cherry picked from commit b1d79970c033822ddffb47be199c379cada2ce40) --- .../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 a0e0aadb615197470f33be47f25032bcadd4f12b Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 10:57:33 +0100 Subject: [PATCH 3/7] RED-6310: Removed not needed code from test (cherry picked from commit fd7f39bc7e4b4cc67ed55d5e77ef84d93779b342) --- .../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 551840a46f535f3e0d0818ba3f34878148eade96 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 11:36:44 +0100 Subject: [PATCH 4/7] RED-6310: Moved code for multithreaded tests to a helper class (cherry picked from commit 40d6961742436fd4f7c5e07616850d8b1eb31a43) --- .../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 264d06274..5ac460e5e 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 657a61609389bf58a0d39297ab4cc1c42c6a80e2 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 13:34:05 +0100 Subject: [PATCH 5/7] RED-6310: Corrected services so that they use the user id instead of wrongly using the entity id (cherry picked from commit 643ffc8723d923a7ede8ee52e25ea40e25045148) --- .../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 222e5915ed714fff5e5549ff28a1fa7718d4c71c Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Tue, 7 Mar 2023 18:32:33 +0100 Subject: [PATCH 6/7] RED-6310: Moved code to create user-preferences to a separate class so that the calling code can handle a persistence exception (cherry picked from commit 45bd8e600328ce85c8457525b605da2c16dd70c8) --- ...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); } From c698bf3a685dc906ce0e811f157122cb62caa93b Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Thu, 9 Mar 2023 15:56:32 +0100 Subject: [PATCH 7/7] RED-6310: Backported test to Junit 4 --- .../tests/NotificationPreferencesServiceTest.java | 6 +++--- 1 file changed, 3 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 75428b5c3..106993ab0 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,8 +2,8 @@ package com.iqser.red.service.peristence.v1.server.integration.tests; import static org.assertj.core.api.Assertions.assertThat; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.Before; +import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; @@ -26,7 +26,7 @@ public class NotificationPreferencesServiceTest extends AbstractPersistenceServe final MultithreadedTestRunner multithreadedTestRunner = new MultithreadedTestRunner(2, 1000); - @BeforeEach + @Before public void setup() { TenantContext.setTenantId("redaction");