From 83d2bc6f6586e7c4b32337537aba632d893d1110 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Fri, 15 May 2026 18:17:04 -0400 Subject: [PATCH 1/7] feat: consolidate methods --- .../consent/http/service/EmailService.java | 57 ++++++------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/broadinstitute/consent/http/service/EmailService.java b/src/main/java/org/broadinstitute/consent/http/service/EmailService.java index 7c4aa8f3d..1adc94edb 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/EmailService.java +++ b/src/main/java/org/broadinstitute/consent/http/service/EmailService.java @@ -18,10 +18,8 @@ import java.util.Date; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; -import javax.annotation.Nullable; import org.broadinstitute.consent.http.configurations.ConsentConfiguration; import org.broadinstitute.consent.http.db.DAOContainer; import org.broadinstitute.consent.http.db.DatasetDAO; @@ -73,37 +71,6 @@ public EmailService( this.fromAccount = config.getMailConfiguration().getGoogleAccount(); } - /** - * This method saves an email (either sent or unsent) with all available metadata from the - * SendGrid response. - */ - private void saveEmailAndResponse( - @Nullable Response response, - @Nullable String entityReferenceId, - @Nullable Integer voteId, - Integer userId, - EmailType emailType, - String content) { - Instant now = Instant.now(); - Date dateSent = - (Objects.nonNull(response) && response.getStatusCode() < 400) ? Date.from(now) : null; - String sendgridResponse = Objects.nonNull(response) ? response.getBody() : null; - Integer sendgridStatus = Objects.nonNull(response) ? response.getStatusCode() : null; - org.broadinstitute.consent.http.models.mail.MailMessage mailMessage = - new org.broadinstitute.consent.http.models.mail.MailMessage( - entityReferenceId, - null, - voteId, - userId, - emailType.getTypeInt(), - dateSent, - content, - sendgridResponse, - sendgridStatus, - Date.from(now)); - emailDAO.insert(mailMessage); - } - public void sendMessage(MailMessage mailMessage, Integer userId) throws IOException, TemplateException { Writer out = new StringWriter(); @@ -123,13 +90,23 @@ public void sendMessage(MailMessage mailMessage, Integer userId) } // Checks that the user has not disabled email before sending Response response = sendGridAPI.sendMessage(message, mailMessage.toUser.getEmail()); - saveEmailAndResponse( - response, - mailMessage.getEntityReferenceId(), - mailMessage.getVoteId(), - userId, - mailMessage.emailType, - content); + Instant now = Instant.now(); + Date dateSent = (response != null && response.getStatusCode() < 400) ? Date.from(now) : null; + String sendgridResponse = response != null ? response.getBody() : null; + Integer sendgridStatus = response != null ? response.getStatusCode() : null; + org.broadinstitute.consent.http.models.mail.MailMessage persistedMailMessage = + new org.broadinstitute.consent.http.models.mail.MailMessage( + mailMessage.getEntityReferenceId(), + null, + mailMessage.getVoteId(), + userId, + mailMessage.emailType.getTypeInt(), + dateSent, + content, + sendgridResponse, + sendgridStatus, + Date.from(now)); + emailDAO.insert(persistedMailMessage); } public List fetchEmailMessagesByType( From 9e6093c816087de5cbf63dd2a1ad4ba64e22bbb3 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Fri, 15 May 2026 18:40:05 -0400 Subject: [PATCH 2/7] feat: consolidate send message methods --- .../consent/http/db/MailMessageDAO.java | 5 +- .../http/models/mail/MailMessageInsert.java | 13 ++ .../consent/http/service/EmailService.java | 11 +- .../http/db/DataAccessRequestDAOTest.java | 8 +- .../consent/http/db/ElectionDAOTest.java | 8 +- .../consent/http/db/MailMessageDAOTest.java | 189 +++++++----------- .../consent/http/db/UserDAOTest.java | 15 +- .../http/service/EmailServiceTest.java | 6 +- 8 files changed, 107 insertions(+), 148 deletions(-) create mode 100644 src/main/java/org/broadinstitute/consent/http/models/mail/MailMessageInsert.java diff --git a/src/main/java/org/broadinstitute/consent/http/db/MailMessageDAO.java b/src/main/java/org/broadinstitute/consent/http/db/MailMessageDAO.java index 503cb7ca2..a637a24d1 100644 --- a/src/main/java/org/broadinstitute/consent/http/db/MailMessageDAO.java +++ b/src/main/java/org/broadinstitute/consent/http/db/MailMessageDAO.java @@ -4,6 +4,7 @@ import java.util.List; import org.broadinstitute.consent.http.db.mapper.MailMessageMapper; import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.jdbi.v3.sqlobject.config.RegisterRowMapper; import org.jdbi.v3.sqlobject.customizer.Bind; import org.jdbi.v3.sqlobject.customizer.BindMethods; @@ -19,11 +20,11 @@ WITH insterted_row AS ( INSERT INTO email_entity (entity_reference_id, vote_id, user_id, email_type, date_sent, email_text, sendgrid_response, sendgrid_status, create_date) VALUES - (:entityReferenceId, :voteId, :userId, :emailType, :dateSent, :emailText, :sendgridResponse, :sendgridStatus, :createDate) + (:entityReferenceId, :voteId, :userId, :emailType, :dateSent, :emailText, :sendgridResponse, :sendgridStatus, NOW()) RETURNING *) SELECT * FROM insterted_row """) - MailMessage insert(@BindMethods MailMessage mail); + MailMessage insert(@BindMethods MailMessageInsert mail); @SqlQuery( """ diff --git a/src/main/java/org/broadinstitute/consent/http/models/mail/MailMessageInsert.java b/src/main/java/org/broadinstitute/consent/http/models/mail/MailMessageInsert.java new file mode 100644 index 000000000..902d263c8 --- /dev/null +++ b/src/main/java/org/broadinstitute/consent/http/models/mail/MailMessageInsert.java @@ -0,0 +1,13 @@ +package org.broadinstitute.consent.http.models.mail; + +import java.util.Date; + +public record MailMessageInsert( + String entityReferenceId, + Integer voteId, + Integer userId, + Integer emailType, + Date dateSent, + String emailText, + String sendgridResponse, + Integer sendgridStatus) {} diff --git a/src/main/java/org/broadinstitute/consent/http/service/EmailService.java b/src/main/java/org/broadinstitute/consent/http/service/EmailService.java index 1adc94edb..7c85be323 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/EmailService.java +++ b/src/main/java/org/broadinstitute/consent/http/service/EmailService.java @@ -36,6 +36,7 @@ import org.broadinstitute.consent.http.models.StudyDatasetCountRecord; import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.UserVoteReminder; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.broadinstitute.consent.http.util.ConsentLogger; import org.jdbi.v3.core.result.ResultIterable; @@ -94,19 +95,17 @@ public void sendMessage(MailMessage mailMessage, Integer userId) Date dateSent = (response != null && response.getStatusCode() < 400) ? Date.from(now) : null; String sendgridResponse = response != null ? response.getBody() : null; Integer sendgridStatus = response != null ? response.getStatusCode() : null; - org.broadinstitute.consent.http.models.mail.MailMessage persistedMailMessage = - new org.broadinstitute.consent.http.models.mail.MailMessage( + MailMessageInsert mailMessageInsert = + new MailMessageInsert( mailMessage.getEntityReferenceId(), - null, mailMessage.getVoteId(), userId, mailMessage.emailType.getTypeInt(), dateSent, content, sendgridResponse, - sendgridStatus, - Date.from(now)); - emailDAO.insert(persistedMailMessage); + sendgridStatus); + emailDAO.insert(mailMessageInsert); } public List fetchEmailMessagesByType( diff --git a/src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java index bf666846f..a246f9ebb 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java @@ -38,7 +38,7 @@ import org.broadinstitute.consent.http.models.Election; import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.Vote; -import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.jdbi.v3.core.JdbiException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -1267,17 +1267,15 @@ void testFindAgedDARsByEmailTypeOlderThanIntervalExpiringEntriesDoesNotRepeatIfE dataAccessRequestDAO.insertDARDatasetRelation( darTwo.getReferenceId(), datasetThree.getDatasetId()); mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( darOne.getReferenceId(), null, - null, userOneId, EmailType.DAR_EXPIRATION_REMINDER.getTypeInt(), Date.from(Instant.now()), "hello world!", "success", - 200, - Date.from(Instant.now()))); + 200)); List dars = dataAccessRequestDAO.findAgedDARsByEmailTypeOlderThanInterval( EmailType.DAR_EXPIRATION_REMINDER.getTypeInt(), diff --git a/src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java index e7b9367df..4b9abf792 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java @@ -31,7 +31,7 @@ import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.UserVoteReminder; import org.broadinstitute.consent.http.models.Vote; -import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -692,17 +692,15 @@ void testFindVotesThatNeedReminders_AlreadyEmailed() { String referenceId = Instant.now().toString(); Integer emailType = EmailType.COLLECT.getTypeInt(); mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( referenceId, - null, vote.getVoteId(), vote.getUserId(), emailType, Date.from(Instant.now()), "Extra, Extra!", null, - null, - Date.from(Instant.now()))); + null)); List userVoteReminders = electionDAO.findElectionReminders(-1, emailType, referenceId); diff --git a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java index 8a6005e09..be6978f11 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java @@ -1,6 +1,7 @@ package org.broadinstitute.consent.http.db; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -14,6 +15,7 @@ import org.broadinstitute.consent.http.enumeration.EmailType; import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.jdbi.v3.core.statement.UnableToExecuteStatementException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -28,17 +30,15 @@ void testInsert_AllFields() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); } @@ -51,17 +51,15 @@ void testInsert_AllEmailTypes() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), t.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); }); } @@ -72,8 +70,7 @@ void testInsert_NullEntityReferenceId() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( - null, + new MailMessageInsert( null, randomInt(1, 1000), user.getUserId(), @@ -81,8 +78,7 @@ void testInsert_NullEntityReferenceId() { Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); } @@ -92,37 +88,32 @@ void testInsert_NullVoteId() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), null, - null, user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); } @Test void testInsert_NullDateSent() { User user = createUser(); - Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), null, randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); } @@ -132,17 +123,15 @@ void testInsert_NullSendGridResponse() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), null, - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); assertNotNull(mail); } @@ -152,17 +141,15 @@ void testInsert_NullSendGridStatus() { Instant now = Instant.now(); MailMessage mail = mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - null, - Date.from(now))); + null)); assertNotNull(mail); } @@ -177,17 +164,15 @@ void testInsert_MissingUserId() { UnableToExecuteStatementException.class, () -> mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( entityReferenceId, - null, voteId, null, null, Date.from(now), null, sendGridResponse, - sendGridStatus, - null))); + sendGridStatus))); } @Test @@ -202,17 +187,15 @@ void testInsert_MissingEmailType() { UnableToExecuteStatementException.class, () -> mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( entityReferenceId, - null, voteId, userId, null, Date.from(now), null, sendGridResponse, - sendGridStatus, - null))); + sendGridStatus))); } @Test @@ -228,44 +211,40 @@ void testInsert_MissingEmailText() { UnableToExecuteStatementException.class, () -> mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( entityReferenceId, - null, voteId, userId, emailType, Date.from(now), null, sendGridResponse, - sendGridStatus, - null))); + sendGridStatus))); } @Test - void testInsert_MissingCreateDate() { + void testInsert_ProvidesCreateDateFromDatabase() { + User user = createUser(); Instant now = Instant.now(); String entityReferenceId = randomAlphanumeric(10); Integer voteId = randomInt(1, 1000); - Integer userId = randomInt(1, 1000); Integer emailType = EmailType.COLLECT.getTypeInt(); String emailText = randomAlphanumeric(10); String sendGridResponse = randomAlphanumeric(10); Integer sendGridStatus = randomInt(200, 399); - assertThrows( - UnableToExecuteStatementException.class, - () -> - mailMessageDAO.insert( - new MailMessage( - entityReferenceId, - null, - voteId, - userId, - emailType, - Date.from(now), - emailText, - sendGridResponse, - sendGridStatus, - null))); + MailMessage savedMessage = + mailMessageDAO.insert( + new MailMessageInsert( + entityReferenceId, + voteId, + user.getUserId(), + emailType, + Date.from(now), + emailText, + sendGridResponse, + sendGridStatus)); + assertNotNull(savedMessage.createDate()); + assertNotEquals(Date.from(now), savedMessage.createDate()); } @Test @@ -276,17 +255,15 @@ void testFetch() { t -> { Instant now = Instant.now(); mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), t.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); }); EnumSet.allOf(EmailType.class) @@ -298,18 +275,7 @@ void testFetch() { void testFetchLimitAndOffset() { User user = createUser(); Instant now = Instant.now(); - mailMessageDAO.insert( - new MailMessage( - randomAlphanumeric(10), - null, - randomInt(1, 1000), - user.getUserId(), - EmailType.COLLECT.getTypeInt(), - Date.from(now), - randomAlphanumeric(10), - randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + MailMessage firstMail = generateMessage(user, now.minus(1, ChronoUnit.HOURS)); List mailMessageList = mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 1, 0); @@ -319,24 +285,12 @@ void testFetchLimitAndOffset() { mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 1, 1); assertEquals(0, mailMessageList2.size()); - MailMessage mail2 = - mailMessageDAO.insert( - new MailMessage( - randomAlphanumeric(10), - null, - randomInt(1, 1000), - user.getUserId(), - EmailType.COLLECT.getTypeInt(), - Date.from(now), - randomAlphanumeric(10), - randomAlphanumeric(10), - null, - Date.from(now))); + MailMessage mail2 = generateMessage(user, now); List mailMessageList3 = mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 1, 1); assertEquals(1, mailMessageList3.size()); - assertEquals(mail2.emailId(), mailMessageList3.getFirst().emailId()); + assertEquals(firstMail.emailId(), mailMessageList3.getFirst().emailId()); List mailMessageList4 = mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 20, 0); @@ -348,17 +302,15 @@ void testFetchByUserId() { Instant now = Instant.now(); User user = createUser(); mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); List mailMessageList = mailMessageDAO.fetchMessagesByUserId(user.getUserId(), 10, 0); @@ -366,17 +318,15 @@ void testFetchByUserId() { assertEquals(user.getUserId(), mailMessageList.getFirst().userId()); mailMessageDAO.insert( - new MailMessage( + new MailMessageInsert( randomAlphanumeric(10), - null, randomInt(1, 1000), user.getUserId(), EmailType.COLLECT.getTypeInt(), Date.from(now), randomAlphanumeric(10), randomAlphanumeric(10), - randomInt(200, 399), - Date.from(now))); + randomInt(200, 399))); List mailMessageList2 = mailMessageDAO.fetchMessagesByUserId(user.getUserId(), 10, 0); assertEquals(2, mailMessageList2.size()); @@ -447,18 +397,16 @@ void testInsert() { String emailText = randomAlphanumeric(1000); String sendGridResponse = randomAlphanumeric(1000); Integer sendGridStatus = 200; - MailMessage unsavedMessage = - new MailMessage( + MailMessageInsert unsavedMessage = + new MailMessageInsert( entityReferenceId, - null, voteId, user.getUserId(), emailType, nowDate, emailText, sendGridResponse, - sendGridStatus, - nowDate); + sendGridStatus); MailMessage savedMessage = mailMessageDAO.insert(unsavedMessage); assertNotNull(savedMessage); @@ -470,22 +418,33 @@ void testInsert() { assertEquals(emailText, savedMessage.emailText()); assertEquals(sendGridStatus, savedMessage.sendgridStatus()); assertEquals(sendGridResponse, savedMessage.sendgridResponse()); - assertEquals(nowDate, savedMessage.createDate()); + assertNotNull(savedMessage.createDate()); } private MailMessage generateMessage(Instant instant) { - User user = createUser(); - return mailMessageDAO.insert( - new MailMessage( - randomAlphanumeric(10), - null, - randomInt(1, 1000), - user.getUserId(), - EmailType.COLLECT.getTypeInt(), - Date.from(instant), - randomAlphanumeric(10), - randomAlphanumeric(10), - randomInt(200, 399), - Date.from(instant))); + return generateMessage(createUser(), instant); + } + + private MailMessage generateMessage(User user, Instant instant) { + MailMessage savedMessage = + mailMessageDAO.insert( + new MailMessageInsert( + randomAlphanumeric(10), + randomInt(1, 1000), + user.getUserId(), + EmailType.COLLECT.getTypeInt(), + Date.from(instant), + randomAlphanumeric(10), + randomAlphanumeric(10), + randomInt(200, 399))); + jdbi.useHandle( + handle -> + handle + .createUpdate( + "UPDATE email_entity SET create_date = :createDate WHERE email_entity_id = :emailId") + .bind("createDate", Date.from(instant)) + .bind("emailId", savedMessage.emailId()) + .execute()); + return mailMessageDAO.fetchMessageById(savedMessage.emailId()); } } diff --git a/src/test/java/org/broadinstitute/consent/http/db/UserDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/UserDAOTest.java index 8e12163ab..d9b6fb308 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/UserDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/UserDAOTest.java @@ -32,7 +32,7 @@ import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.UserProperty; import org.broadinstitute.consent.http.models.UserRole; -import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.broadinstitute.consent.http.util.gson.GsonUtil; import org.jdbi.v3.core.result.ResultIterable; import org.jdbi.v3.core.statement.UnableToExecuteStatementException; @@ -597,17 +597,8 @@ void testFindAllEmailReceivingThinlyPopulatedUsers() { // simulate having sent a message so we won't send one again. Date now = Date.from(Instant.now()); mailMessageDAO.insert( - new MailMessage( - referenceId, - null, - null, - user.getUserId(), - emailType, - now, - "", - null, - null, - now)); + new MailMessageInsert( + referenceId, null, user.getUserId(), emailType, now, "", null, null)); } } }); diff --git a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java index c4cffae69..a4c550056 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java @@ -45,6 +45,7 @@ import org.broadinstitute.consent.http.models.User; import org.broadinstitute.consent.http.models.UserVoteReminder; import org.broadinstitute.consent.http.models.mail.MailMessage; +import org.broadinstitute.consent.http.models.mail.MailMessageInsert; import org.jdbi.v3.core.Handle; import org.jdbi.v3.core.HandleConsumer; import org.jdbi.v3.core.Jdbi; @@ -171,7 +172,7 @@ public Integer getVoteId() { verify(emailDAO) .insert( argThat( - m -> + (MailMessageInsert m) -> Objects.equals(m.entityReferenceId(), entityReferenceId) && Objects.equals(m.voteId(), voteId) && Objects.equals(m.userId(), 1234) @@ -179,8 +180,7 @@ public Integer getVoteId() { && Objects.equals(m.dateSent(), Date.from(fixedInstant)) && Objects.equals(m.emailText(), emailText) && Objects.equals(m.sendgridResponse(), response.getBody()) - && Objects.equals(m.sendgridStatus(), response.getStatusCode()) - && Objects.equals(m.createDate(), Date.from(fixedInstant)))); + && Objects.equals(m.sendgridStatus(), response.getStatusCode()))); } @Test From 39e16a61b4ec149300ce54c91e6955a4fc77bf49 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Sat, 16 May 2026 07:03:10 -0400 Subject: [PATCH 3/7] chore: address sonar flag --- .../org/broadinstitute/consent/http/db/MailMessageDAOTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java index be6978f11..62455fc6a 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java @@ -285,7 +285,7 @@ void testFetchLimitAndOffset() { mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 1, 1); assertEquals(0, mailMessageList2.size()); - MailMessage mail2 = generateMessage(user, now); + generateMessage(user, now); List mailMessageList3 = mailMessageDAO.fetchMessagesByType(EmailType.COLLECT.getTypeInt(), 1, 1); From 5bc504eaf1cdf8b36c2e21b4d1d32245d6f1ad12 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Sat, 16 May 2026 07:06:27 -0400 Subject: [PATCH 4/7] test: add test for coverage --- .../consent/http/service/EmailServiceTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java index a4c550056..b1aacc935 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java @@ -190,6 +190,15 @@ void testFetchEmails() { assertEquals(2, service.fetchEmailMessagesByType(EmailType.COLLECT, 20, 0).size()); } + @Test + void testFetchEmailsByUserId() { + Integer userId = 123; + List mailMessages = generateMailMessageList(); + when(emailDAO.fetchMessagesByUserId(eq(userId), anyInt(), anyInt())).thenReturn(mailMessages); + + assertEquals(2, service.fetchEmailMessagesByUserId(userId, 20, 0).size()); + } + @Test void testFetchEmailsByCreateDate() { List mailMessages = generateMailMessageList(); From df8b10499586bfe8930e79fd2111223668c50e50 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Sat, 16 May 2026 07:15:31 -0400 Subject: [PATCH 5/7] test: update test for potential clock drift --- .../consent/http/db/MailMessageDAOTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java index 62455fc6a..4dfe16120 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java @@ -1,9 +1,9 @@ package org.broadinstitute.consent.http.db; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.Instant; import java.time.LocalDate; @@ -225,7 +225,7 @@ void testInsert_MissingEmailText() { @Test void testInsert_ProvidesCreateDateFromDatabase() { User user = createUser(); - Instant now = Instant.now(); + Instant historicalInstant = Instant.parse("2000-01-01T00:00:00Z"); String entityReferenceId = randomAlphanumeric(10); Integer voteId = randomInt(1, 1000); Integer emailType = EmailType.COLLECT.getTypeInt(); @@ -239,12 +239,12 @@ void testInsert_ProvidesCreateDateFromDatabase() { voteId, user.getUserId(), emailType, - Date.from(now), + Date.from(historicalInstant), emailText, sendGridResponse, sendGridStatus)); assertNotNull(savedMessage.createDate()); - assertNotEquals(Date.from(now), savedMessage.createDate()); + assertTrue(savedMessage.createDate().toInstant().isAfter(historicalInstant)); } @Test From 9e94a81f069de55cf27dbaa00829b78d24096a33 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Sat, 16 May 2026 07:18:14 -0400 Subject: [PATCH 6/7] chore: address sonar warnings --- .../consent/http/db/MailMessageDAOTest.java | 73 +++++++++---------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java index 4dfe16120..79d584f7f 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java @@ -160,66 +160,63 @@ void testInsert_MissingUserId() { Integer voteId = randomInt(1, 1000); String sendGridResponse = randomAlphanumeric(10); Integer sendGridStatus = randomInt(200, 399); + MailMessageInsert mailMessageInsert = + new MailMessageInsert( + entityReferenceId, + voteId, + null, + null, + Date.from(now), + null, + sendGridResponse, + sendGridStatus); assertThrows( - UnableToExecuteStatementException.class, - () -> - mailMessageDAO.insert( - new MailMessageInsert( - entityReferenceId, - voteId, - null, - null, - Date.from(now), - null, - sendGridResponse, - sendGridStatus))); + UnableToExecuteStatementException.class, () -> mailMessageDAO.insert(mailMessageInsert)); } @Test void testInsert_MissingEmailType() { + User user = createUser(); Instant now = Instant.now(); String entityReferenceId = randomAlphanumeric(10); Integer voteId = randomInt(1, 1000); - Integer userId = randomInt(1, 1000); String sendGridResponse = randomAlphanumeric(10); Integer sendGridStatus = randomInt(200, 399); + MailMessageInsert mailMessageInsert = + new MailMessageInsert( + entityReferenceId, + voteId, + user.getUserId(), + null, + Date.from(now), + null, + sendGridResponse, + sendGridStatus); assertThrows( - UnableToExecuteStatementException.class, - () -> - mailMessageDAO.insert( - new MailMessageInsert( - entityReferenceId, - voteId, - userId, - null, - Date.from(now), - null, - sendGridResponse, - sendGridStatus))); + UnableToExecuteStatementException.class, () -> mailMessageDAO.insert(mailMessageInsert)); } @Test void testInsert_MissingEmailText() { + User user = createUser(); Instant now = Instant.now(); String entityReferenceId = randomAlphanumeric(10); Integer voteId = randomInt(1, 1000); - Integer userId = randomInt(1, 1000); Integer emailType = EmailType.COLLECT.getTypeInt(); String sendGridResponse = randomAlphanumeric(10); Integer sendGridStatus = randomInt(200, 399); + MailMessageInsert mailMessageInsert = + new MailMessageInsert( + entityReferenceId, + voteId, + user.getUserId(), + emailType, + Date.from(now), + null, + sendGridResponse, + sendGridStatus); assertThrows( - UnableToExecuteStatementException.class, - () -> - mailMessageDAO.insert( - new MailMessageInsert( - entityReferenceId, - voteId, - userId, - emailType, - Date.from(now), - null, - sendGridResponse, - sendGridStatus))); + UnableToExecuteStatementException.class, () -> mailMessageDAO.insert(mailMessageInsert)); } @Test From f85cafe3cdfc14c41b95e2b49fc748378015b559 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Sat, 16 May 2026 07:26:05 -0400 Subject: [PATCH 7/7] test: address missing coverage --- .../http/service/EmailServiceTest.java | 234 ++++++++++++++++-- 1 file changed, 210 insertions(+), 24 deletions(-) diff --git a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java index b1aacc935..ce4bf079d 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java @@ -101,32 +101,13 @@ void testSendMessage() throws Exception { User user = new User(); user.setEmail(userEmail); String subject = "subject"; - var model = Map.of("key", "value"); + Map model = Map.of("key", "value"); String entityReferenceId = "entityReferenceId"; Integer userId = 1234; Integer voteId = 4567; var message = - new org.broadinstitute.consent.http.mail.message.MailMessage(user, EmailType.NEW_CASE) { - @Override - public String createSubject() { - return subject; - } - - @Override - public Object createModel(String serverUrl) { - return model; - } - - @Override - public String getEntityReferenceId() { - return entityReferenceId; - } - - @Override - public Integer getVoteId() { - return voteId; - } - }; + createMailMessage( + user, EmailType.NEW_CASE, subject, model, entityReferenceId, voteId, null); Template template = mock(); when(templateHelper.getTemplate(EmailType.NEW_CASE.templateName)).thenReturn(template); Response response = new Response(); @@ -183,6 +164,110 @@ public Integer getVoteId() { && Objects.equals(m.sendgridStatus(), response.getStatusCode()))); } + @Test + @SuppressWarnings("deprecation") + void testSendMessage_SkipsCategoriesAndPersistsNullSendGridValues_WhenResponseIsNull() + throws Exception { + String userEmail = "user@duos"; + User user = new User(); + user.setEmail(userEmail); + String subject = "subject"; + Map model = Map.of("key", "value"); + String entityReferenceId = "entityReferenceId"; + Integer userId = 1234; + String templateName = "test-template.ftl"; + var message = + createMailMessage( + user, EmailType.NEW_DAA_REQUEST, subject, model, entityReferenceId, null, templateName); + Template template = mock(); + when(templateHelper.getTemplate(templateName)).thenReturn(template); + when(sendGridAPI.sendMessage(any(), any())).thenReturn(null); + String emailText = "emailText"; + doNothing() + .when(template) + .process( + eq(model), + argThat( + writer -> { + try { + writer.append(emailText); + } catch (IOException e) { + throw new RuntimeException(e); + } + return true; + })); + + service.sendMessage(message, userId); + + var captor = ArgumentCaptor.forClass(Mail.class); + verify(sendGridAPI).sendMessage(captor.capture(), eq(user.getEmail())); + var mail = captor.getValue(); + assertTrue(mail.getCategories() == null || mail.getCategories().isEmpty()); + + verify(emailDAO) + .insert( + argThat( + (MailMessageInsert m) -> + Objects.equals(m.entityReferenceId(), entityReferenceId) + && m.voteId() == null + && Objects.equals(m.userId(), userId) + && Objects.equals(m.emailType(), EmailType.NEW_DAA_REQUEST.getTypeInt()) + && m.dateSent() == null + && Objects.equals(m.emailText(), emailText) + && m.sendgridResponse() == null + && m.sendgridStatus() == null)); + } + + @Test + void testSendMessage_DoesNotSetDateSent_WhenSendGridReturnsErrorResponse() throws Exception { + String userEmail = "user@duos"; + User user = new User(); + user.setEmail(userEmail); + String subject = "subject"; + Map model = Map.of("key", "value"); + String entityReferenceId = "entityReferenceId"; + Integer userId = 1234; + Integer voteId = 4567; + var message = + createMailMessage( + user, EmailType.NEW_CASE, subject, model, entityReferenceId, voteId, null); + Template template = mock(); + when(templateHelper.getTemplate(EmailType.NEW_CASE.templateName)).thenReturn(template); + Response response = new Response(); + response.setStatusCode(500); + response.setBody("error"); + when(sendGridAPI.sendMessage(any(), any())).thenReturn(response); + String emailText = "emailText"; + doNothing() + .when(template) + .process( + eq(model), + argThat( + writer -> { + try { + writer.append(emailText); + } catch (IOException e) { + throw new RuntimeException(e); + } + return true; + })); + + service.sendMessage(message, userId); + + verify(emailDAO) + .insert( + argThat( + (MailMessageInsert m) -> + Objects.equals(m.entityReferenceId(), entityReferenceId) + && Objects.equals(m.voteId(), voteId) + && Objects.equals(m.userId(), userId) + && Objects.equals(m.emailType(), EmailType.NEW_CASE.getTypeInt()) + && m.dateSent() == null + && Objects.equals(m.emailText(), emailText) + && Objects.equals(m.sendgridResponse(), response.getBody()) + && Objects.equals(m.sendgridStatus(), response.getStatusCode()))); + } + @Test void testFetchEmails() { List mailMessages = generateMailMessageList(); @@ -351,15 +436,116 @@ void testSendNewDatasetInDUOSNotifications_IOException() throws IOException { .getTemplate(EmailType.NEW_STUDY_DIGEST.templateName); EmailService.sendgridThrottleMessageCount = 1; EmailService.sendgridThrottleResetTime = 1; - Thread.currentThread().interrupt(); - service.sendNewDatasetInDUOSNotifications(); + try { + Thread.currentThread().interrupt(); + service.sendNewDatasetInDUOSNotifications(); + } finally { + boolean interruptStatusCleared = Thread.interrupted(); + assertTrue(interruptStatusCleared || !Thread.currentThread().isInterrupted()); + EmailService.sendgridThrottleMessageCount = 500; + EmailService.sendgridThrottleResetTime = 60; + } verify(userDAO, times(1)).getHandle(); } + @Test + void testSendNewDatasetInDUOSNotifications_ReinterruptsThreadWhenThrottleSleepIsInterrupted() + throws IOException { + User toUser = new User(); + toUser.setDisplayName("Test User"); + toUser.setUserId(1); + toUser.setEmail("test@example.com"); + toUser.setEmailPreference(true); + when(datasetDAO.getRecentDacApprovedDatasetStudyIds()).thenReturn(List.of()); + when(datasetDAO.getRecentlyCreatedOpenOrExternalDatasetStudyIds()).thenReturn(List.of()); + when(studyDAO.findNameAndDatasetCount(any())) + .thenReturn(List.of(new StudyDatasetCountRecord("New Study", 1, 7))); + Handle handle = mock(Handle.class); + Jdbi jdbi = mock(Jdbi.class); + when(userDAO.getHandle()).thenReturn(handle); + when(handle.getJdbi()).thenReturn(jdbi); + doAnswer( + invocation -> { + HandleConsumer consumer = invocation.getArgument(0); + consumer.useHandle(handle); + return null; + }) + .when(jdbi) + .useHandle(any()); + @SuppressWarnings("unchecked") + ResultIterator mockIterator = mock(ResultIterator.class); + when(mockIterator.hasNext()).thenReturn(true, false); + when(mockIterator.next()).thenReturn(toUser); + when(userDAO.allEmailReceivingThinlyPopulatedUsers(any(), any())) + .thenReturn(ResultIterable.of(mockIterator)); + when(templateHelper.getTemplate(EmailType.NEW_STUDY_DIGEST.templateName)).thenReturn(mock()); + Response response = new Response(); + response.setStatusCode(202); + response.setBody("accepted"); + when(sendGridAPI.sendMessage(any(), any())).thenReturn(response); + EmailService.sendgridThrottleMessageCount = 1; + EmailService.sendgridThrottleResetTime = 1; + + try { + Thread.currentThread().interrupt(); + service.sendNewDatasetInDUOSNotifications(); + assertTrue(Thread.currentThread().isInterrupted()); + } finally { + boolean interruptStatusCleared = Thread.interrupted(); + assertTrue(interruptStatusCleared || !Thread.currentThread().isInterrupted()); + EmailService.sendgridThrottleMessageCount = 500; + EmailService.sendgridThrottleResetTime = 60; + } + + verify(userDAO, times(1)).getHandle(); + verify(emailDAO, times(1)) + .insert( + argThat( + m -> + Objects.equals(m.userId(), toUser.getUserId()) + && Objects.equals(m.emailType(), EmailType.NEW_STUDY_DIGEST.getTypeInt()))); + } + private List generateMailMessageList() { return Collections.nCopies(2, generateMailMessage()); } + private org.broadinstitute.consent.http.mail.message.MailMessage createMailMessage( + User user, + EmailType emailType, + String subject, + Object model, + String entityReferenceId, + Integer voteId, + String templateNameOverride) { + return new org.broadinstitute.consent.http.mail.message.MailMessage(user, emailType) { + @Override + public String getTemplateName() { + return templateNameOverride != null ? templateNameOverride : super.getTemplateName(); + } + + @Override + public String createSubject() { + return subject; + } + + @Override + public Object createModel(String serverUrl) { + return model; + } + + @Override + public String getEntityReferenceId() { + return entityReferenceId; + } + + @Override + public Integer getVoteId() { + return voteId; + } + }; + } + private MailMessage generateMailMessage() { return new MailMessage( randomAlphanumeric(10),