Fix transaction

edeandrea · Jan 2, 2025 · 0440c9b · 0440c9b
1 parent 4723e1d
commit 0440c9b
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 74 deletions.
diff --git a/src/main/java/org/ericoleg/ndnp/ai/NotificationService.java b/src/main/java/org/ericoleg/ndnp/ai/NotificationService.java
@@ -1,6 +1,7 @@
 package org.ericoleg.ndnp.ai;
 
 import java.time.Duration;
+import java.util.Optional;
 import java.util.concurrent.ForkJoinPool;
 
 import jakarta.enterprise.context.ApplicationScoped;
@@ -48,35 +49,32 @@ public class NotificationService {
 	@WithSpan("NotificationService.updateClaimStatus")
 	public String updateClaimStatus(@SpanAttribute("arg.claimId") long claimId, @SpanAttribute("arg.status") String status) {
 		// Only want to actually do anything if the passed in status has at least 3 characters
-		return ((status != null) && (status.strip().length() > 2)) ?
-		       updateStatus(claimId, status) :
-		       INVALID_STATUS.formatted(status);
+		return Optional.ofNullable(status)
+			.filter(s -> s.strip().length() > 2)
+			.map(s -> updateStatus(claimId, s))
+			.orElseGet(() -> INVALID_STATUS.formatted(status));
 	}
 
 	private String updateStatus(long claimId, String status) {
 		// Only want to actually do anything if there is a corresponding claim in the database for the given claimId
-//		var txTimeout = ConfigUtils.isProfileActive("test") ? 5 * 60 : 0;
-		var updatedClaim = QuarkusTransaction.joiningExisting()
-//													.timeout(txTimeout)
-                          .call(() -> updateStatusIfFound(claimId, status));
-
-		return (updatedClaim != null) ?
-		       sendEmail(updatedClaim) :
-		       NOTIFICATION_NO_CLAIMANT_FOUND;
+		return updateStatusIfFound(claimId, status)
+			.map(this::sendEmail)
+			.orElse(NOTIFICATION_NO_CLAIMANT_FOUND);
 	}
 
-	private Claim updateStatusIfFound(long claimId, String status) {
-		var claim = Claim.<Claim>findById(claimId);
+	private Optional<Claim> updateStatusIfFound(long claimId, String status) {
+		return QuarkusTransaction.joiningExisting().call(() ->
+				Claim.<Claim>findByIdOptional(claimId)
+					.map(claim -> {
+						// Capitalize the first letter
+						claim.status = status.strip().substring(0, 1).toUpperCase() + status.strip().substring(1);
 
-		if (claim != null) {
-			// Capitalize the first letter
-			claim.status = status.strip().substring(0, 1).toUpperCase() + status.strip().substring(1);
+						// Save the claim with updated status
+						Claim.persist(claim);
 
-			// Save the claim with updated status
-			Claim.persist(claim);
-		}
-
-		return claim;
+						return claim;
+					})
+			);
 	}
 
 	private String sendEmail(Claim claim) {
@@ -89,7 +87,7 @@ private String sendEmail(Claim claim) {
 			.setFrom(MESSAGE_FROM);
 
 		// Send the email to the user
-		// Need to move this to another thread because the mailer blocks the...which causes deadlock
+		// Need to move this to another thread because the mailer blocks the thread...which causes deadlock
 		// Fail if it doesn't finish in 15 seconds
 		this.mailer.send(email)
 			.runSubscriptionOn(ForkJoinPool.commonPool())

diff --git a/src/test/java/org/ericoleg/ndnp/ai/NotificationServiceTests.java b/src/test/java/org/ericoleg/ndnp/ai/NotificationServiceTests.java
@@ -15,7 +15,7 @@
 import org.junit.jupiter.params.provider.NullAndEmptySource;
 import org.junit.jupiter.params.provider.ValueSource;
 
-import io.quarkus.runtime.annotations.RegisterForReflection;
+import io.quarkus.narayana.jta.QuarkusTransaction;
 import io.quarkus.test.TestTransaction;
 import io.quarkus.test.junit.QuarkusTest;
 
@@ -27,6 +27,8 @@
 @QuarkusTest
 @WithMailbox
 class NotificationServiceTests {
+	private static final Duration WAIT_DURATION = Duration.ofMinutes(5);
+
 	@InjectMailbox
 	Mailbox mailbox;
 
@@ -39,58 +41,65 @@ void afterEach() {
 	}
 
 	@Test
-	@TestTransaction
 	void emailSendsWhenUserExists() {
-		var status = "Denied";
-		var claimId = 1L;
-		var claim = Claim.<Claim>findByIdOptional(claimId)
-		                 .orElseThrow(() -> new IllegalArgumentException("Marty McFly's claim should be found!"));
-
-		assertThat(this.emailService.updateClaimStatus(claimId, status))
-			.isNotNull()
-			.isEqualTo(NotificationService.NOTIFICATION_SUCCESS, claim.emailAddress, claim.claimNumber, status);
-
-		await()
-			.atMost(Duration.ofMinutes(5))
-			.until(() -> findFirstMessage().isPresent());
-
-		// Find the message in Mailpit
-		var message = findFirstMessage();
-
-		// Assert the message has the correct subject & email address
-		assertThat(message)
-			.isNotNull()
-			.get()
-			.extracting(
-				Message::getSubject,
-				m -> m.getTo().getFirst().getAddress()
-			)
-			.containsExactly(
-				NotificationService.MESSAGE_SUBJECT,
-				claim.emailAddress
-			);
-
-		// Assert that the message has the correct info in the body
-		assertThat(message.get().getText().strip())
-			.isNotNull();
-
-		assertThat(message.get().getText().strip().replaceAll("\r\n", "\n"))
-			.isNotNull()
-			.startsWith(GenerateEmailService.EMAIL_STARTING.strip())
-			.endsWith(GenerateEmailService.EMAIL_ENDING.strip())
-			.contains(claim.clientName)
-			.containsIgnoringCase(claim.claimNumber)
-			.containsIgnoringCase(status);
-
-		// Assert that the claim status was updated in the database
-		var updatedClaim = Claim.findById(claimId);
-		assertThat(updatedClaim)
-			.isNotNull()
-			.extracting("status")
-			.isEqualTo(status);
+		QuarkusTransaction.begin(QuarkusTransaction.beginOptions().timeout((int) WAIT_DURATION.toSeconds()));
+
+		try {
+			var status = "Denied";
+			var claimId = 1L;
+			var claim = Claim.<Claim>findByIdOptional(claimId)
+			                 .orElseThrow(() -> new IllegalArgumentException("Marty McFly's claim should be found!"));
+
+			assertThat(this.emailService.updateClaimStatus(claimId, status))
+				.isNotNull()
+				.isEqualTo(NotificationService.NOTIFICATION_SUCCESS, claim.emailAddress, claim.claimNumber, status);
+
+			await()
+				.atMost(WAIT_DURATION)
+				.until(() -> findFirstMessage().isPresent());
+
+			// Find the message in Mailpit
+			var message = findFirstMessage();
+
+			// Assert the message has the correct subject & email address
+			assertThat(message)
+				.isNotNull()
+				.get()
+				.extracting(
+					Message::getSubject,
+					m -> m.getTo().getFirst().getAddress()
+				)
+				.containsExactly(
+					NotificationService.MESSAGE_SUBJECT,
+					claim.emailAddress
+				);
+
+			// Assert that the message has the correct info in the body
+			assertThat(message.get().getText().strip())
+				.isNotNull();
+
+			assertThat(message.get().getText().strip().replaceAll("\r\n", "\n"))
+				.isNotNull()
+				.startsWith(GenerateEmailService.EMAIL_STARTING.strip())
+				.endsWith(GenerateEmailService.EMAIL_ENDING.strip())
+				.contains(claim.clientName)
+				.containsIgnoringCase(claim.claimNumber)
+				.containsIgnoringCase(status);
+
+			// Assert that the claim status was updated in the database
+			var updatedClaim = Claim.findById(claimId);
+			assertThat(updatedClaim)
+				.isNotNull()
+				.extracting("status")
+				.isEqualTo(status);
+		}
+		finally {
+			QuarkusTransaction.rollback();
+		}
 	}
 
 	@Test
+	@TestTransaction
 	void noEmailSentWhenClaimantNotFound() {
 		assertThat(this.emailService.updateClaimStatus(-1L, "Under investigation"))
 			.isNotNull()
@@ -119,7 +128,4 @@ private void assertNoEmailSent() {
 	private Optional<Message> findFirstMessage() {
 		return Optional.ofNullable(this.mailbox.findFirst(NotificationService.MESSAGE_FROM));
 	}
-
-	@RegisterForReflection
-	public record ClaimStatus(String status) { }
 }