From e5bdb8c06428abd3e9fe9d1448ec8d4d613e750a Mon Sep 17 00:00:00 2001 From: "marius.marin" Date: Mon, 24 Nov 2025 11:35:10 +0200 Subject: [PATCH 1/8] 735 - Add inline validation and visual error for duplicate project titles --- config-templates/content_indices.properties | 4 +- .../ac/cam/cl/dtg/isaac/api/EventsFacade.java | 59 ++++++++++++++ .../api/managers/EventBookingManager.java | 7 ++ .../dao/EventBookingPersistenceManager.java | 19 +++++ .../isaac/dos/eventbookings/EventBooking.java | 7 ++ .../dos/eventbookings/EventBookings.java | 10 +++ .../dos/eventbookings/PgEventBooking.java | 11 ++- .../dos/eventbookings/PgEventBookings.java | 36 ++++++++- .../dto/eventbookings/EventBookingDTO.java | 22 +++++- .../ProjectTitlesRequestDTO.java | 77 +++++++++++++++++++ .../ProjectTitlesResponseDTO.java | 55 +++++++++++++ .../eventbookings/PgEventBookingsTest.java | 4 +- .../cl/dtg/isaac/mappers/EventMapperTest.java | 3 +- 13 files changed, 306 insertions(+), 8 deletions(-) create mode 100644 src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesRequestDTO.java create mode 100644 src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesResponseDTO.java diff --git a/config-templates/content_indices.properties b/config-templates/content_indices.properties index 76a435c336..678deaaed8 100644 --- a/config-templates/content_indices.properties +++ b/config-templates/content_indices.properties @@ -1,4 +1,4 @@ # #Mon Jan 28 2025 -latest=6b5a88a0d4b2c8eeaca34b8bca4ad3c4dfe80ad1 -live=6b5a88a0d4b2c8eeaca34b8bca4ad3c4dfe80ad1 +latest=b75f3eed0165f039664aa89a6c7a5a6902e14e97 +live=b75f3eed0165f039664aa89a6c7a5a6902e14e97 diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java index ce01133652..5779af431b 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java @@ -59,6 +59,7 @@ import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.DefaultValue; import jakarta.ws.rs.GET; @@ -114,6 +115,8 @@ import uk.ac.cam.cl.dtg.isaac.dto.eventbookings.CompetitionEntryDTO; import uk.ac.cam.cl.dtg.isaac.dto.eventbookings.DetailedEventBookingDTO; import uk.ac.cam.cl.dtg.isaac.dto.eventbookings.EventBookingDTO; +import uk.ac.cam.cl.dtg.isaac.dto.eventbookings.ProjectTitlesRequestDTO; +import uk.ac.cam.cl.dtg.isaac.dto.eventbookings.ProjectTitlesResponseDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO; import uk.ac.cam.cl.dtg.isaac.mappers.MainObjectMapper; @@ -461,6 +464,62 @@ public final Response getCountForAllEventBookings(@Context final HttpServletRequ } } + /** + * Count all event bookings. + * + * @param request so we can determine if the user is logged in + * @return a list of booking objects + */ + @POST + @Path("/bookings/titles") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @GZIP + @Operation(summary = "Get all project titles for the specified users in a competition") + public final Response getAllProjectTitlesForUsers( + @Context final HttpServletRequest request, + final ProjectTitlesRequestDTO requestBody) { + + try { + RegisteredUserDTO user = userManager.getCurrentRegisteredUser(request); + + if (!isUserTeacherOrAbove(userManager, user)) { + return SegueErrorResponse.getIncorrectRoleResponse(); + } + + if (requestBody == null) { + return new SegueErrorResponse(Status.BAD_REQUEST, + "Request body must be provided.").toResponse(); + } + + if (requestBody.getCompetitionId() == null || requestBody.getCompetitionId().isEmpty()) { + return new SegueErrorResponse(Status.BAD_REQUEST, + "Competition ID must be specified.").toResponse(); + } + + if (requestBody.getUserIds() == null || requestBody.getUserIds().isEmpty()) { + return new SegueErrorResponse(Status.BAD_REQUEST, + "User IDs must be specified.").toResponse(); + } + + List projectTitles = bookingManager.getCompetitionProjectTitlesForUsers( + requestBody.getCompetitionId(), + requestBody.getUserIds() + ); + + ProjectTitlesResponseDTO response = new ProjectTitlesResponseDTO(projectTitles); + + return Response.ok(response) + .cacheControl(getCacheControl(NEVER_CACHE_WITHOUT_ETAG_CHECK, false)) + .build(); + + } catch (NoUserLoggedInException e) { + return SegueErrorResponse.getNotLoggedInResponse(); + } catch (SegueDatabaseException e) { + throw new RuntimeException(e); + } + } + /** * Find a booking by id. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java index 2e1ac643fe..2501a4d20c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java @@ -1602,4 +1602,11 @@ private void removeUserFromEventGroup(final IsaacEventPageDTO event, final Regis user.getEmail(), event.getTitle())); } } + + public List getCompetitionProjectTitlesForUsers(String competitionId, List userIds) + throws SegueDatabaseException { + return this.bookingPersistenceManager.getBookingsByEventIdForUsers(competitionId, userIds) + .stream() + .map(EventBookingDTO::getEventTitle).toList(); + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java index 300fd5c4be..ae5a41afcf 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java @@ -101,6 +101,19 @@ public DetailedEventBookingDTO getBookingByEventIdAndUserId(final String eventId return this.convertToDTO(dao.findBookingByEventAndUser(eventId, userId)); } + /** + * Gets a specific event booking. + * + * @param eventId of interest + * @param usersId of interest + * @return event booking or null if we can't find one. + * @throws SegueDatabaseException if an error occurs. + */ + public List getBookingByEventIdAndUsersId(final String eventId, final List usersId) + throws SegueDatabaseException { + return this.convertToDTO(dao.findBookingByEventAndUsers(eventId, usersId)); + } + /** * Modify an existing event booking's status. * @@ -393,6 +406,7 @@ private DetailedEventBookingDTO convertToDTO(final EventBooking eb, final IsaacE result.setUpdated(eb.getUpdateDate()); result.setBookingStatus(eb.getBookingStatus()); result.setAdditionalInformation(eb.getAdditionalInformation()); + result.setProjectTitle(eb.projectTitle()); return result; } catch (NoUserException e) { @@ -472,4 +486,9 @@ private List convertToDTO(final List toCo return result; } + + public List getBookingsByEventIdForUsers(String competitionId, List userIds) + throws SegueDatabaseException { + return this.getBookingByEventIdAndUsersId(competitionId, userIds); + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBooking.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBooking.java index 7e96c5cc5a..1475206600 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBooking.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBooking.java @@ -55,6 +55,13 @@ public interface EventBooking { */ String getEventId(); + /** + * Getter project title + * + * @return event id + */ + String projectTitle(); + /** * Gets the current status of the booking. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java index 440501990d..b2f30634b1 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java @@ -175,6 +175,16 @@ Iterable findAllByEventIdAndStatus(String eventId, @Nullable Booki */ EventBooking findBookingByEventAndUser(String eventId, Long userId) throws SegueDatabaseException; + /** + * Find an event booking by event and users id. + * + * @param eventId - the event of interest. + * @param usersId - the users of interest. + * @return the event or an error. + * @throws SegueDatabaseException - if an error occurs. + */ + List findBookingByEventAndUsers(String eventId, List usersId) throws SegueDatabaseException; + /** * Find an event booking by id. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBooking.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBooking.java index f90a0ae31a..86fb05b824 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBooking.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBooking.java @@ -36,6 +36,8 @@ public class PgEventBooking implements EventBooking { private final BookingStatus bookingStatus; private final Instant created; private final Instant updated; + + private final String projectTitle; private final Map additionalInformation; /** @@ -48,11 +50,12 @@ public class PgEventBooking implements EventBooking { * @param bookingStatus - the status of the booking * @param created - the date the booking was made. * @param updated - the date the booking was last updated + * @param projectTitle * @param additionalInformation - additional information to be stored with this booking e.g. dietary requirements. */ public PgEventBooking(final Long bookingId, final Long userId, final Long reservedById, final String eventId, final BookingStatus bookingStatus, final Instant created, final Instant updated, - final Object additionalInformation) throws SegueDatabaseException { + String projectTitle, final Object additionalInformation) throws SegueDatabaseException { this.bookingId = bookingId; this.userId = userId; this.reservedById = reservedById; @@ -60,6 +63,7 @@ public PgEventBooking(final Long bookingId, final Long userId, final Long reserv this.bookingStatus = bookingStatus; this.updated = updated; this.created = created; + this.projectTitle = projectTitle; if (additionalInformation != null) { try { this.additionalInformation = this.convertFromJsonbToMap(additionalInformation); @@ -92,6 +96,11 @@ public String getEventId() { return eventId; } + @Override + public String projectTitle() { + return projectTitle; + } + @Override public BookingStatus getBookingStatus() { return bookingStatus; diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java index 4ca91fbacb..1617f4082e 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java @@ -25,6 +25,7 @@ import com.google.api.client.util.Lists; import com.google.api.client.util.Maps; import jakarta.annotation.Nullable; +import java.sql.Array; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -193,7 +194,7 @@ private PgEventBooking addEventBooking( if (generatedKeys.next()) { Long id = generatedKeys.getLong(1); return new PgEventBooking(id, userId, reserveById, eventId, status, creationDate, creationDate, - additionalEventInformation); + projectTitle, additionalEventInformation); } else { throw new SQLException("Creating event booking failed, no ID obtained."); } @@ -520,6 +521,38 @@ public EventBooking findBookingByEventAndUser(final String eventId, final Long u } } + @Override + public List findBookingByEventAndUsers(final String eventId, final List userIds) + throws SegueDatabaseException { + Validate.notBlank(eventId); + Validate.notNull(userIds); + Validate.notEmpty(userIds); + + String query = "SELECT * FROM event_bookings WHERE event_id = ? AND user_id = ANY(?)"; + + try (Connection conn = ds.getDatabaseConnection(); + PreparedStatement pst = conn.prepareStatement(query) + ) { + pst.setString(1, eventId); + + Long[] userIdArray = userIds.toArray(new Long[0]); + Array sqlArray = conn.createArrayOf("BIGINT", userIdArray); + pst.setArray(2, sqlArray); + + try (ResultSet results = pst.executeQuery()) { + List bookings = new ArrayList<>(); + + while (results.next()) { + bookings.add(buildPgEventBooking(results)); + } + + return bookings; + } + } catch (SQLException e) { + throw new SegueDatabaseException("Error retrieving event bookings", e); + } + } + /* * (non-Javadoc) * @@ -740,6 +773,7 @@ private PgEventBooking buildPgEventBooking(final ResultSet results) throws SQLEx BookingStatus.valueOf(results.getString("status")), getInstantFromTimestamp(results, "created"), getInstantFromTimestamp(results, "updated"), + results.getString("project_title"), results.getObject("additional_booking_information") ); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/EventBookingDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/EventBookingDTO.java index 008bc193a1..019e04a429 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/EventBookingDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/EventBookingDTO.java @@ -45,6 +45,8 @@ public class EventBookingDTO { private Instant bookingDate; + private String projectTitle; + /** * EventBookingDTO. */ @@ -76,7 +78,8 @@ public EventBookingDTO() { */ public EventBookingDTO(final Long bookingId, final UserSummaryDTO userBooked, final Long reservedById, final String eventId, final String eventTitle, final Instant eventDate, - final Instant bookingDate, final Instant lastUpdated, final BookingStatus status) { + final Instant bookingDate, final Instant lastUpdated, final BookingStatus status, + final String projectTitle) { this.bookingId = bookingId; this.userBooked = userBooked; this.reservedById = reservedById; @@ -86,6 +89,7 @@ public EventBookingDTO(final Long bookingId, final UserSummaryDTO userBooked, fi this.bookingStatus = status; this.lastUpdated = lastUpdated; this.bookingDate = bookingDate; + this.projectTitle = projectTitle; } /** @@ -255,4 +259,20 @@ public BookingStatus getBookingStatus() { public void setBookingStatus(final BookingStatus bookingStatus) { this.bookingStatus = bookingStatus; } + + public Instant getLastUpdated() { + return lastUpdated; + } + + public void setLastUpdated(Instant lastUpdated) { + this.lastUpdated = lastUpdated; + } + + public String getProjectTitle() { + return projectTitle; + } + + public void setProjectTitle(String projectTitle) { + this.projectTitle = projectTitle; + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesRequestDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesRequestDTO.java new file mode 100644 index 0000000000..07d40e55fb --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesRequestDTO.java @@ -0,0 +1,77 @@ +package uk.ac.cam.cl.dtg.isaac.dto.eventbookings; + +import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.List; + +/** + * DTO for requesting project titles for a competition. + */ +public class ProjectTitlesRequestDTO { + private String competitionId; + private List userIds; + + /** + * Default constructor for Jackson. + */ + public ProjectTitlesRequestDTO() { + } + + /** + * Constructor with fields. + * + * @param competitionId the competition/event ID + * @param userIds list of user IDs to get project titles for + */ + public ProjectTitlesRequestDTO(String competitionId, List userIds) { + this.competitionId = competitionId; + this.userIds = userIds; + } + + /** + * Gets the competition ID. + * + * @return the competition ID + */ + @JsonProperty("competitionId") + public String getCompetitionId() { + return competitionId; + } + + /** + * Sets the competition ID. + * + * @param competitionId the competition ID to set + */ + @JsonProperty("competitionId") + public void setCompetitionId(String competitionId) { + this.competitionId = competitionId; + } + + /** + * Gets the user IDs. + * + * @return the list of user IDs + */ + @JsonProperty("userIds") + public List getUserIds() { + return userIds; + } + + /** + * Sets the user IDs. + * + * @param userIds the list of user IDs to set + */ + @JsonProperty("userIds") + public void setUserIds(List userIds) { + this.userIds = userIds; + } + + @Override + public String toString() { + return "ProjectTitlesRequestDTO{" + + "competitionId='" + competitionId + '\'' + + ", userIds=" + userIds + + '}'; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesResponseDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesResponseDTO.java new file mode 100644 index 0000000000..5e7a209486 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/eventbookings/ProjectTitlesResponseDTO.java @@ -0,0 +1,55 @@ +package uk.ac.cam.cl.dtg.isaac.dto.eventbookings; + +import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.ArrayList; +import java.util.List; + +/** + * DTO for returning project titles for a competition. + */ +public class ProjectTitlesResponseDTO { + private List projectTitles; + + /** + * Default constructor for Jackson. + */ + public ProjectTitlesResponseDTO() { + this.projectTitles = new ArrayList<>(); + } + + /** + * Constructor with project titles. + * + * @param projectTitles the list of project titles + */ + public ProjectTitlesResponseDTO(List projectTitles) { + this.projectTitles = projectTitles != null ? projectTitles : new ArrayList<>(); + } + + /** + * Gets the project titles. + * + * @return the list of project titles + */ + @JsonProperty("projectTitles") + public List getProjectTitles() { + return projectTitles; + } + + /** + * Sets the project titles. + * + * @param projectTitles the list of project titles to set + */ + @JsonProperty("projectTitles") + public void setProjectTitles(List projectTitles) { + this.projectTitles = projectTitles; + } + + @Override + public String toString() { + return "ProjectTitlesResponseDTO{" + + "projectTitles=" + projectTitles + + '}'; + } +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java index ba59fd2b7c..b5a2f72d2c 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java @@ -119,9 +119,9 @@ void testFindAllByEventIds() throws SQLException, SegueDatabaseException { Instant now = now(); EventBooking expectedBooking1 = - new PgEventBooking(1L, 1L, 7L, "event1", BookingStatus.CONFIRMED, now, now, null); + new PgEventBooking(1L, 1L, 7L, "event1", BookingStatus.CONFIRMED, now, now, "projectTitle1", null); EventBooking expectedBooking2 = - new PgEventBooking(2L, 2L, 7L, "event2", BookingStatus.CONFIRMED, now, now, null); + new PgEventBooking(2L, 2L, 7L, "event2", BookingStatus.CONFIRMED, now, now, "projectTitle2", null); expect(dummyPostgresSqlDb.getDatabaseConnection()).andReturn(dummyConnection); expect(dummyConnection.prepareStatement(expectedQuery)).andReturn(dummyPreparedStatement); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/mappers/EventMapperTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/mappers/EventMapperTest.java index 2a7d2c4937..ec859315a3 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/mappers/EventMapperTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/mappers/EventMapperTest.java @@ -104,7 +104,8 @@ private static EventBookingDTO prepareEventBookingDTO() { testDate, testDate, newTestDate, - BookingStatus.CONFIRMED); + BookingStatus.CONFIRMED, + "projectTitle"); } private static DetailedEventBookingDTO prepareDetailedEventBookingDTO() { From a19b71b66f808b7b2f5e6af539f0458b463db266 Mon Sep 17 00:00:00 2001 From: "marius.marin" Date: Mon, 24 Nov 2025 15:09:40 +0200 Subject: [PATCH 2/8] MAILJET --- .../api/managers/ExternalAccountManager.java | 393 ++++++++++++++---- .../util/email/MailJetApiClientWrapper.java | 250 +++++++++-- 2 files changed, 524 insertions(+), 119 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index b0af27a09f..9c5753faab 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -19,6 +19,8 @@ import com.mailjet.client.errors.MailjetClientCommunicationException; import com.mailjet.client.errors.MailjetException; import com.mailjet.client.errors.MailjetRateLimitException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.List; import org.json.JSONObject; import org.slf4j.Logger; @@ -58,111 +60,348 @@ public ExternalAccountManager(final MailJetApiClientWrapper mailjetApi, final IE */ @Override public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchronisationException { + log.info("MMAILJETT - === Starting Mailjet user synchronisation ==="); + int totalUsersProcessed = 0; + int successfulSyncs = 0; + int failedSyncs = 0; + int skippedUsers = 0; + try { List userRecordsToUpdate = database.getRecentlyChangedRecords(); + log.info("MMAILJETT - Found {} users requiring synchronisation", userRecordsToUpdate.size()); for (UserExternalAccountChanges userRecord : userRecordsToUpdate) { - + totalUsersProcessed++; Long userId = userRecord.getUserId(); - log.debug(String.format("Processing user: %s", userId)); + String accountEmail = userRecord.getAccountEmail(); + + log.info("MMAILJETT - [User {}] Starting sync for email: {}", userId, sanitiseEmailForLogging(accountEmail)); + log.debug("MMAILJETT - [User {}] Full user record: deleted={}, emailVerificationStatus={}, providerUserId={}, " + + "allowsNews={}, allowsEvents={}, givenName={}, role={}, stage={}", + userId, + userRecord.isDeleted(), + userRecord.getEmailVerificationStatus(), + userRecord.getProviderUserId(), + userRecord.allowsNewsEmails(), + userRecord.allowsEventsEmails(), + userRecord.getGivenName(), + userRecord.getRole(), + userRecord.getStage()); + try { + boolean syncSuccess = processSingleUser(userRecord); - String accountEmail = userRecord.getAccountEmail(); - boolean accountEmailDeliveryFailed = - EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus()); - String mailjetId = userRecord.getProviderUserId(); - JSONObject mailjetDetails; - - if (null != mailjetId) { - // If there is a "mailjet_id", get the account from MailJet. - mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); - if (userRecord.isDeleted()) { - // Case: deleted from Isaac but still on MailJet: - // Expect: "deleted" but non-null "mailjet_id" - // Action: GDPR deletion, null out MailJet ID?, update provider_last_updated - log.debug("Case: deletion."); - deleteUserFromMailJet(mailjetId, userRecord); - } else if (accountEmailDeliveryFailed) { - // Case: DELIVERY_FAILED but already on MailJet - // Expect: DELIVERY_FAILED, but non-null "mailjet_id" - // Action: same as deletion? Or just remove from lists for now? - log.debug("Case: delivery failed."); - mailjetApi.updateUserSubscriptions(mailjetId, MailJetSubscriptionAction.REMOVE, - MailJetSubscriptionAction.REMOVE); - } else if (!accountEmail.toLowerCase().equals(mailjetDetails.getString("Email"))) { - // Case: account email change: - // Expect: non-null "mailjet_id", email in MailJet != email in database - // Action: delete old email, add new user for new email - log.debug("Case: account email change."); - mailjetApi.permanentlyDeleteAccountById(mailjetId); - mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); - updateUserOnMailJet(mailjetId, userRecord); - } else { - // Case: changed details/preferences: - // Expect: not deleted, not DELIVERY_FAILED - // Action: update details, update subscriptions, update provider_last_updated - log.debug("Case: generic preferences update."); - updateUserOnMailJet(mailjetId, userRecord); - } + if (syncSuccess) { + successfulSyncs++; + log.info("MMAILJETT - [User {}] Successfully synced to Mailjet", userId); } else { - if (!accountEmailDeliveryFailed && !userRecord.isDeleted()) { - // Case: new to Isaac, not on MailJet: - // Expect: null "mailjet_id", not DELIVERY_FAILED, not deleted - // Action: create MailJet ID, update details, update subscriptions, update provider_last_updated - // This will upload even users who are not subscribed to emails. - log.debug("Case: new to Isaac/not yet on MailJet"); - mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); - updateUserOnMailJet(mailjetId, userRecord); - } else { - // Case: not on MailJet, do not upload to Mailjet: - // Expect: either invalid email, deleted, or not subscribed at all so don't upload. - log.debug("Case: invalid/incorrect/already-unsubscribed user to skip."); - database.updateExternalAccount(userId, null); - } + skippedUsers++; + log.info("MMAILJETT - [User {}] Skipped (invalid/deleted user not requiring Mailjet)", userId); } - // Iff action done successfully, update the provider_last_updated time: - log.debug("Update provider_last_updated."); - database.updateProviderLastUpdated(userId); } catch (SegueDatabaseException e) { - log.error(String.format("Error storing record of MailJet update to user (%s)!", userId)); + failedSyncs++; + log.error(String.format("MMAILJETT - [User %s] Database error during sync - will retry on next run", userId), e); + // Don't update provider_last_updated so it will be retried + } catch (MailjetClientCommunicationException e) { - log.error("Failed to talk to MailJet!"); + failedSyncs++; + log.error(String.format("MMAILJETT - [User %s] Failed to communicate with Mailjet API", userId), e); + log.error("MMAILJETT - [User {}] MailjetClientCommunicationException details: {}", userId, e.getMessage()); throw new ExternalAccountSynchronisationException("Failed to successfully connect to MailJet!"); + } catch (MailjetRateLimitException e) { - log.warn("MailJet rate limiting!"); + failedSyncs++; + log.warn(String.format("MMAILJETT - [User %s] Hit Mailjet rate limit - stopping sync", userId), e); throw new ExternalAccountSynchronisationException("MailJet API rate limits exceeded!"); + } catch (MailjetException e) { - log.error(e.getMessage()); - throw new ExternalAccountSynchronisationException(e.getMessage()); + failedSyncs++; + log.error(String.format("MMAILJETT - [User %s] Mailjet API error during sync", userId), e); + log.error("MMAILJETT - [User {}] MailjetException type: {}, message: {}", + userId, e.getClass().getName(), e.getMessage()); + // Log the full stack trace for debugging + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + log.error("MMAILJETT - [User {}] Full stack trace: {}", userId, sw.toString()); + + // Don't throw - continue processing other users, but don't update provider_last_updated + // so this user will be retried next time + + } catch (Exception e) { + failedSyncs++; + log.error(String.format("MMAILJETT - [User %s] Unexpected error during sync", userId), e); + log.error("MMAILJETT - [User {}] Unexpected exception type: {}, message: {}", + userId, e.getClass().getName(), e.getMessage()); + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + log.error("MMAILJETT - [User {}] Full stack trace: {}", userId, sw.toString()); + // Continue processing other users } } + } catch (SegueDatabaseException e) { - log.error("Database error whilst collecting users whose details have changed!", e); + log.error("MMAILJETT - Database error whilst collecting users whose details have changed!", e); + throw new ExternalAccountSynchronisationException("Database error during user collection"); + } + + log.info("MMAILJETT - === Mailjet synchronisation complete ==="); + log.info("MMAILJETT - Total users processed: {}", totalUsersProcessed); + log.info("MMAILJETT - Successful syncs: {}", successfulSyncs); + log.info("MMAILJETT - Failed syncs: {}", failedSyncs); + log.info("MMAILJETT - Skipped users: {}", skippedUsers); + } + + /** + * Process a single user synchronisation. + * + * @param userRecord the user record to process + * @return true if sync succeeded, false if user was skipped + * @throws SegueDatabaseException if database operations fail + * @throws MailjetException if Mailjet API calls fail + */ + private boolean processSingleUser(UserExternalAccountChanges userRecord) + throws SegueDatabaseException, MailjetException { + + Long userId = userRecord.getUserId(); + String accountEmail = userRecord.getAccountEmail(); + boolean accountEmailDeliveryFailed = + EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus()); + String mailjetId = userRecord.getProviderUserId(); + + // Validate user data before attempting sync + if (!validateUserData(userRecord)) { + log.warn("MMAILJETT - [User {}] Skipping sync due to invalid user data", userId); + // Still update provider_last_updated to prevent repeated attempts + database.updateProviderLastUpdated(userId); + return false; + } + + if (null != mailjetId) { + log.debug("MMAILJETT - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); + + // Verify the user still exists in Mailjet + JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); + + if (mailjetDetails == null) { + log.warn("MMAILJETT - [User {}] Mailjet ID {} not found in Mailjet - will recreate", userId, mailjetId); + // Clear the mailjet ID and recreate + database.updateExternalAccount(userId, null); + mailjetId = null; + } else { + log.debug("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); + } + } + + if (null != mailjetId) { + // User exists in Mailjet - handle updates/deletions + JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); + + if (userRecord.isDeleted()) { + log.info("MMAILJETT - [User {}] Processing deletion from Mailjet", userId); + deleteUserFromMailJet(mailjetId, userRecord); + + } else if (accountEmailDeliveryFailed) { + log.info("MMAILJETT - [User {}] Email delivery failed - removing from lists", userId); + mailjetApi.updateUserSubscriptions(mailjetId, MailJetSubscriptionAction.REMOVE, + MailJetSubscriptionAction.REMOVE); + database.updateProviderLastUpdated(userId); + + } else if (!accountEmail.toLowerCase().equals(mailjetDetails.getString("Email"))) { + log.info("MMAILJETT - [User {}] Email changed from {} to {} - recreating Mailjet contact", + userId, sanitiseEmailForLogging(mailjetDetails.getString("Email")), + sanitiseEmailForLogging(accountEmail)); + mailjetApi.permanentlyDeleteAccountById(mailjetId); + log.debug("MMAILJETT - [User {}] Deleted old Mailjet contact", userId); + + mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); + log.info("MMAILJETT - [User {}] Created new Mailjet contact with ID: {}", userId, mailjetId); + + updateUserOnMailJet(mailjetId, userRecord); + + } else { + log.info("MMAILJETT - [User {}] Updating existing Mailjet contact", userId); + updateUserOnMailJet(mailjetId, userRecord); + } + + } else { + // User doesn't exist in Mailjet yet + if (!accountEmailDeliveryFailed && !userRecord.isDeleted()) { + log.info("MMAILJETT - [User {}] New user - creating Mailjet contact", userId); + + // Create the contact + mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); + + if (mailjetId == null) { + log.error("MMAILJETT - [User {}] Failed to create Mailjet contact - addNewUserOrGetUserIfExists returned null", userId); + throw new MailjetException("Failed to create Mailjet contact - returned null ID"); + } + + log.info("MMAILJETT - [User {}] Created Mailjet contact with ID: {}", userId, mailjetId); + + // Now update properties and subscriptions + // Use try-finally to ensure we record the mailjet ID even if updates fail + boolean updateSucceeded = false; + try { + updateUserOnMailJet(mailjetId, userRecord); + updateSucceeded = true; + } finally { + if (!updateSucceeded) { + log.error("MMAILJETT - [User {}] Failed to update properties/subscriptions for Mailjet ID {} - " + + "contact exists but may have incomplete data", userId, mailjetId); + // Store the mailjet ID anyway so we don't create duplicates + database.updateExternalAccount(userId, mailjetId); + } + } + + } else { + log.info("MMAILJETT - [User {}] Skipping - delivery failed or deleted (not uploading to Mailjet)", userId); + database.updateExternalAccount(userId, null); + database.updateProviderLastUpdated(userId); + return false; + } + } + + // If we got here, sync succeeded + database.updateProviderLastUpdated(userId); + return true; + } + + /** + * Validate user data before attempting Mailjet sync. + * + * @param userRecord the user record to validate + * @return true if data is valid, false otherwise + */ + private boolean validateUserData(UserExternalAccountChanges userRecord) { + Long userId = userRecord.getUserId(); + String accountEmail = userRecord.getAccountEmail(); + + if (accountEmail == null || accountEmail.trim().isEmpty()) { + log.error("MMAILJETT - [User {}] Invalid data: email is null or empty", userId); + return false; } + + if (!accountEmail.contains("@")) { + log.error("MMAILJETT - [User {}] Invalid data: email '{}' does not contain @", + userId, sanitiseEmailForLogging(accountEmail)); + return false; + } + + // Check for required fields (allowing nulls but logging them) + if (userRecord.getGivenName() == null) { + log.warn("MMAILJETT - [User {}] Warning: given_name is null", userId); + } + + if (userRecord.getRole() == null) { + log.warn("MMAILJETT - [User {}] Warning: role is null", userId); + } + + if (userRecord.getStage() == null) { + log.warn("MMAILJETT - [User {}] Warning: stage is null", userId); + } + + if (userRecord.getEmailVerificationStatus() == null) { + log.warn("MMAILJETT - [User {}] Warning: email_verification_status is null", userId); + } + + return true; } private void updateUserOnMailJet(final String mailjetId, final UserExternalAccountChanges userRecord) throws SegueDatabaseException, MailjetException { + Long userId = userRecord.getUserId(); - mailjetApi.updateUserProperties(mailjetId, userRecord.getGivenName(), userRecord.getRole().toString(), - userRecord.getEmailVerificationStatus().toString(), userRecord.getStage()); - - MailJetSubscriptionAction newsStatus = (userRecord.allowsNewsEmails() != null - && userRecord.allowsNewsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : - MailJetSubscriptionAction.UNSUBSCRIBE; - MailJetSubscriptionAction eventsStatus = (userRecord.allowsEventsEmails() != null - && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : - MailJetSubscriptionAction.UNSUBSCRIBE; - mailjetApi.updateUserSubscriptions(mailjetId, newsStatus, eventsStatus); - - database.updateExternalAccount(userId, mailjetId); + log.debug("MMAILJETT - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); + + // Update properties + try { + String firstName = userRecord.getGivenName(); + String role = userRecord.getRole() != null ? userRecord.getRole().toString() : ""; + String verificationStatus = userRecord.getEmailVerificationStatus() != null + ? userRecord.getEmailVerificationStatus().toString() : ""; + String stage = userRecord.getStage(); + + log.debug("MMAILJETT - [User {}] Setting properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", + userId, firstName, role, verificationStatus, stage); + + mailjetApi.updateUserProperties(mailjetId, firstName, role, verificationStatus, stage); + log.info("MMAILJETT - [User {}] Successfully updated Mailjet properties", userId); + + } catch (MailjetException e) { + log.error("MMAILJETT - [User {}] Failed to update Mailjet properties for ID {}", userId, mailjetId, e); + throw e; + } + + // Update subscriptions + try { + MailJetSubscriptionAction newsStatus = (userRecord.allowsNewsEmails() != null + && userRecord.allowsNewsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : + MailJetSubscriptionAction.UNSUBSCRIBE; + MailJetSubscriptionAction eventsStatus = (userRecord.allowsEventsEmails() != null + && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : + MailJetSubscriptionAction.UNSUBSCRIBE; + + log.debug("MMAILJETT - [User {}] Setting subscriptions - news: {}, events: {}", + userId, newsStatus, eventsStatus); + + mailjetApi.updateUserSubscriptions(mailjetId, newsStatus, eventsStatus); + log.info("MMAILJETT - [User {}] Successfully updated Mailjet subscriptions", userId); + + } catch (MailjetException e) { + log.error("MMAILJETT - [User {}] Failed to update Mailjet subscriptions for ID {}", userId, mailjetId, e); + throw e; + } + + // Store the Mailjet ID + try { + database.updateExternalAccount(userId, mailjetId); + log.debug("MMAILJETT - [User {}] Stored Mailjet ID in database: {}", userId, mailjetId); + } catch (SegueDatabaseException e) { + log.error("MMAILJETT - [User {}] Failed to store Mailjet ID {} in database", userId, mailjetId, e); + throw e; + } } private void deleteUserFromMailJet(final String mailjetId, final UserExternalAccountChanges userRecord) throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); - mailjetApi.permanentlyDeleteAccountById(mailjetId); - database.updateExternalAccount(userId, null); + log.info("MMAILJETT - [User {}] Deleting from Mailjet, ID: {}", userId, mailjetId); + + try { + mailjetApi.permanentlyDeleteAccountById(mailjetId); + log.info("MMAILJETT - [User {}] Successfully deleted from Mailjet", userId); + } catch (MailjetException e) { + log.error("MMAILJETT - [User {}] Failed to delete from Mailjet, ID: {}", userId, mailjetId, e); + throw e; + } + + try { + database.updateExternalAccount(userId, null); + log.debug("MMAILJETT - [User {}] Cleared Mailjet ID from database", userId); + } catch (SegueDatabaseException e) { + log.error("MMAILJETT - [User {}] Failed to clear Mailjet ID from database", userId, e); + throw e; + } + } + + /** + * Sanitise email for logging to prevent log injection and reduce PII exposure. + * + * @param email the email to sanitise + * @return sanitised email (e.g., "u***@example.com") + */ + private String sanitiseEmailForLogging(String email) { + if (email == null) { + return "null"; + } + if (!email.contains("@")) { + return "invalid-email"; + } + String[] parts = email.split("@"); + if (parts[0].length() <= 2) { + return parts[0].charAt(0) + "***@" + parts[1]; + } + return parts[0].charAt(0) + "***@" + parts[1]; } -} +} \ No newline at end of file diff --git a/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java b/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java index b1e5b160ed..440b55d414 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java +++ b/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java @@ -66,6 +66,9 @@ public MailJetApiClientWrapper(final String mailjetApiKey, final String mailjetA this.newsListId = mailjetNewsListId; this.eventsListId = mailjetEventsListId; this.legalListId = mailjetLegalListId; + + log.info("MMAILJETT - MailJetApiClientWrapper initialized with list IDs - News: {}, Events: {}, Legal: {}", + newsListId, eventsListId, legalListId); } /** @@ -77,15 +80,37 @@ public MailJetApiClientWrapper(final String mailjetApiKey, final String mailjetA */ public JSONObject getAccountByIdOrEmail(final String mailjetIdOrEmail) throws MailjetException { if (null == mailjetIdOrEmail) { + log.warn("MMAILJETT - getAccountByIdOrEmail called with null mailjetIdOrEmail"); return null; } - MailjetRequest request = new MailjetRequest(Contact.resource, mailjetIdOrEmail); - MailjetResponse response = mailjetClient.get(request); - JSONArray responseData = response.getData(); - if (response.getTotal() == 1) { - return responseData.getJSONObject(0); + + log.debug("MMAILJETT - Getting Mailjet account for: {}", mailjetIdOrEmail); + + try { + MailjetRequest request = new MailjetRequest(Contact.resource, mailjetIdOrEmail); + MailjetResponse response = mailjetClient.get(request); + + log.debug("MMAILJETT - Mailjet GET Contact response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); + + JSONArray responseData = response.getData(); + if (response.getTotal() == 1) { + JSONObject result = responseData.getJSONObject(0); + log.debug("MMAILJETT - Found Mailjet contact: ID={}, Email={}", + result.optInt("ID"), result.optString("Email")); + return result; + } + + log.warn("MMAILJETT - Mailjet account not found for: {}", mailjetIdOrEmail); + return null; + + } catch (MailjetException e) { + log.error("MMAILJETT - MailjetException in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); + throw e; + } catch (Exception e) { + log.error("MMAILJETT - Unexpected exception in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); + throw new MailjetException("Unexpected error getting account: " + e.getMessage(), e); } - return null; } /** @@ -96,8 +121,28 @@ public JSONObject getAccountByIdOrEmail(final String mailjetIdOrEmail) throws Ma */ public void permanentlyDeleteAccountById(final String mailjetId) throws MailjetException { requireNonNull(mailjetId); - MailjetRequest request = new MailjetRequest(Contacts.resource, mailjetId); - mailjetClient.delete(request); + + log.info("MMAILJETT - Permanently deleting Mailjet account: {}", mailjetId); + + try { + MailjetRequest request = new MailjetRequest(Contacts.resource, mailjetId); + MailjetResponse response = mailjetClient.delete(request); + + log.info("MMAILJETT - Mailjet DELETE response - Status: {}, mailjetId: {}", + response.getStatus(), mailjetId); + + if (response.getStatus() != 204 && response.getStatus() != 200) { + log.warn("MMAILJETT - Unexpected status code {} when deleting Mailjet account {}", + response.getStatus(), mailjetId); + } + + } catch (MailjetException e) { + log.error("MMAILJETT - MailjetException deleting account: {}", mailjetId, e); + throw e; + } catch (Exception e) { + log.error("MMAILJETT - Unexpected exception deleting account: {}", mailjetId, e); + throw new MailjetException("Unexpected error deleting account: " + e.getMessage(), e); + } } /** @@ -111,27 +156,67 @@ public void permanentlyDeleteAccountById(final String mailjetId) throws MailjetE */ public String addNewUserOrGetUserIfExists(final String email) throws MailjetException { if (null == email) { + log.error("MMAILJETT - addNewUserOrGetUserIfExists called with null email"); return null; } + + log.info("MMAILJETT - Creating Mailjet contact for email: {}", sanitiseEmailForLogging(email)); + try { MailjetRequest request = new MailjetRequest(Contact.resource).property(Contact.EMAIL, email); MailjetResponse response = mailjetClient.post(request); + + log.debug("MMAILJETT - Mailjet POST Contact response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); + // Get MailJet ID out: JSONObject responseData = response.getData().getJSONObject(0); - return Integer.toString(responseData.getInt("ID")); + String mailjetId = Integer.toString(responseData.getInt("ID")); + + log.info("MMAILJETT - Successfully created Mailjet contact with ID: {} for email: {}", + mailjetId, sanitiseEmailForLogging(email)); + + return mailjetId; + } catch (MailjetClientRequestException e) { + log.warn("MMAILJETT - MailjetClientRequestException creating contact for email: {} - Message: {}", + sanitiseEmailForLogging(email), e.getMessage()); + if (e.getMessage().contains("already exists")) { - // FIXME - we need to test that this response always comes back with "already exists" in the message - log.warn(String.format("Attempted to create a user with email (%s) that already existed!", email)); - JSONObject existingMailJetAccount = getAccountByIdOrEmail(email); - return Integer.toString(existingMailJetAccount.getInt("ID")); + log.info("MMAILJETT - Contact already exists, attempting to retrieve existing contact for: {}", + sanitiseEmailForLogging(email)); + + try { + JSONObject existingMailJetAccount = getAccountByIdOrEmail(email); + if (existingMailJetAccount != null) { + String existingId = Integer.toString(existingMailJetAccount.getInt("ID")); + log.info("MMAILJETT - Found existing Mailjet contact with ID: {} for email: {}", + existingId, sanitiseEmailForLogging(email)); + return existingId; + } else { + log.error("MMAILJETT - Contact 'already exists' but getAccountByIdOrEmail returned null for: {}", + sanitiseEmailForLogging(email)); + throw new MailjetException("Contact exists but could not be retrieved"); + } + } catch (JSONException jsonEx) { + log.error("MMAILJETT - JSONException retrieving existing contact for: {}", + sanitiseEmailForLogging(email), jsonEx); + throw new MailjetException("Failed to parse existing contact data", jsonEx); + } } else { - log.error(String.format("Failed to create user in MailJet with email: %s", email), e); + log.error("MMAILJETT - Failed to create user in MailJet with email: {} - Error: {}", + sanitiseEmailForLogging(email), e.getMessage(), e); + throw new MailjetException("Failed to create contact: " + e.getMessage(), e); } } catch (JSONException e) { - log.error(String.format("Failed to create user in MailJet with email: %s", email), e); + log.error("MMAILJETT - JSONException creating user in MailJet with email: {}", + sanitiseEmailForLogging(email), e); + throw new MailjetException("Failed to parse Mailjet response", e); + } catch (Exception e) { + log.error("MMAILJETT - Unexpected exception creating user in MailJet with email: {}", + sanitiseEmailForLogging(email), e); + throw new MailjetException("Unexpected error creating contact: " + e.getMessage(), e); } - return null; } /** @@ -147,16 +232,47 @@ public String addNewUserOrGetUserIfExists(final String email) throws MailjetExce public void updateUserProperties(final String mailjetId, final String firstName, final String role, final String emailVerificationStatus, String stage) throws MailjetException { requireNonNull(mailjetId); - MailjetRequest request = new MailjetRequest(Contactdata.resource, mailjetId) - .property(Contactdata.DATA, new JSONArray() - .put(new JSONObject().put("Name", "firstname").put("value", firstName)) - .put(new JSONObject().put("Name", "role").put("value", role)) - .put(new JSONObject().put("Name", "verification_status").put("value", emailVerificationStatus)) - .put(new JSONObject().put("Name", "stage").put("value", stage)) - ); - MailjetResponse response = mailjetClient.put(request); - if (response.getTotal() != 1) { - throw new MailjetException("Failed to update user!" + response.getTotal()); + + log.info("MMAILJETT - Updating properties for Mailjet ID: {}", mailjetId); + log.debug("MMAILJETT - Properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", + firstName, role, emailVerificationStatus, stage); + + try { + JSONArray propertiesArray = new JSONArray() + .put(new JSONObject().put("Name", "firstname").put("value", firstName)) + .put(new JSONObject().put("Name", "role").put("value", role)) + .put(new JSONObject().put("Name", "verification_status").put("value", emailVerificationStatus)) + .put(new JSONObject().put("Name", "stage").put("value", stage)); + + log.debug("MMAILJETT - Property JSON array: {}", propertiesArray.toString()); + + MailjetRequest request = new MailjetRequest(Contactdata.resource, mailjetId) + .property(Contactdata.DATA, propertiesArray); + + MailjetResponse response = mailjetClient.put(request); + + log.debug("MMAILJETT - Mailjet PUT Contactdata response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); + + if (response.getTotal() != 1) { + log.error("MMAILJETT - Failed to update user properties! Expected 1, got {} for Mailjet ID: {}", + response.getTotal(), mailjetId); + log.error("MMAILJETT - Response status: {}, Response data: {}", + response.getStatus(), response.getData().toString()); + throw new MailjetException("Failed to update user! Response total: " + response.getTotal()); + } + + log.info("MMAILJETT - Successfully updated properties for Mailjet ID: {}", mailjetId); + + } catch (MailjetException e) { + log.error("MMAILJETT - MailjetException updating properties for Mailjet ID: {}", mailjetId, e); + throw e; + } catch (JSONException e) { + log.error("MMAILJETT - JSONException creating property data for Mailjet ID: {}", mailjetId, e); + throw new MailjetException("Failed to create property JSON", e); + } catch (Exception e) { + log.error("MMAILJETT - Unexpected exception updating properties for Mailjet ID: {}", mailjetId, e); + throw new MailjetException("Unexpected error updating properties: " + e.getMessage(), e); } } @@ -171,22 +287,72 @@ public void updateUserProperties(final String mailjetId, final String firstName, public void updateUserSubscriptions(final String mailjetId, final MailJetSubscriptionAction newsEmails, final MailJetSubscriptionAction eventsEmails) throws MailjetException { requireNonNull(mailjetId); - MailjetRequest request = new MailjetRequest(ContactManagecontactslists.resource, mailjetId) - .property(ContactManagecontactslists.CONTACTSLISTS, new JSONArray() - .put(new JSONObject() - .put(ContactslistImportList.LISTID, legalListId) - .put(ContactslistImportList.ACTION, MailJetSubscriptionAction.FORCE_SUBSCRIBE.getValue())) - .put(new JSONObject() - .put(ContactslistImportList.LISTID, newsListId) - .put(ContactslistImportList.ACTION, newsEmails.getValue())) - .put(new JSONObject() - .put(ContactslistImportList.LISTID, eventsListId) - .put(ContactslistImportList.ACTION, eventsEmails.getValue())) - ); - MailjetResponse response = mailjetClient.post(request); - if (response.getTotal() != 1) { - throw new MailjetException("Failed to update user subscriptions!" + response.getTotal()); + + log.info("MMAILJETT - Updating subscriptions for Mailjet ID: {}", mailjetId); + log.debug("MMAILJETT - Subscriptions - news: {}, events: {}", newsEmails, eventsEmails); + log.debug("MMAILJETT - Using list IDs - Legal: {}, News: {}, Events: {}", legalListId, newsListId, eventsListId); + + try { + JSONArray subscriptionsArray = new JSONArray() + .put(new JSONObject() + .put(ContactslistImportList.LISTID, legalListId) + .put(ContactslistImportList.ACTION, MailJetSubscriptionAction.FORCE_SUBSCRIBE.getValue())) + .put(new JSONObject() + .put(ContactslistImportList.LISTID, newsListId) + .put(ContactslistImportList.ACTION, newsEmails.getValue())) + .put(new JSONObject() + .put(ContactslistImportList.LISTID, eventsListId) + .put(ContactslistImportList.ACTION, eventsEmails.getValue())); + + log.debug("MMAILJETT - Subscription JSON array: {}", subscriptionsArray.toString()); + + MailjetRequest request = new MailjetRequest(ContactManagecontactslists.resource, mailjetId) + .property(ContactManagecontactslists.CONTACTSLISTS, subscriptionsArray); + + MailjetResponse response = mailjetClient.post(request); + + log.debug("MMAILJETT - Mailjet POST ContactManagecontactslists response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); + + if (response.getTotal() != 1) { + log.error("MMAILJETT - Failed to update user subscriptions! Expected 1, got {} for Mailjet ID: {}", + response.getTotal(), mailjetId); + log.error("MMAILJETT - Response status: {}, Response data: {}", + response.getStatus(), response.getData().toString()); + throw new MailjetException("Failed to update user subscriptions! Response total: " + response.getTotal()); + } + + log.info("MMAILJETT - Successfully updated subscriptions for Mailjet ID: {}", mailjetId); + + } catch (MailjetException e) { + log.error("MMAILJETT - MailjetException updating subscriptions for Mailjet ID: {}", mailjetId, e); + throw e; + } catch (JSONException e) { + log.error("MMAILJETT - JSONException creating subscription data for Mailjet ID: {}", mailjetId, e); + throw new MailjetException("Failed to create subscription JSON", e); + } catch (Exception e) { + log.error("MMAILJETT - Unexpected exception updating subscriptions for Mailjet ID: {}", mailjetId, e); + throw new MailjetException("Unexpected error updating subscriptions: " + e.getMessage(), e); } } -} + /** + * Sanitise email for logging to prevent log injection and reduce PII exposure. + * + * @param email the email to sanitise + * @return sanitised email (e.g., "u***@example.com") + */ + private String sanitiseEmailForLogging(String email) { + if (email == null) { + return "null"; + } + if (!email.contains("@")) { + return "invalid-email"; + } + String[] parts = email.split("@"); + if (parts[0].length() <= 2) { + return parts[0].charAt(0) + "***@" + parts[1]; + } + return parts[0].charAt(0) + "***@" + parts[1]; + } +} \ No newline at end of file From e624a3a8d3e9f360e1eb5154bb5c9702c5beaf8d Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 07:40:43 +0200 Subject: [PATCH 3/8] PATCH 2 --- .../api/managers/ExternalAccountManager.java | 20 +++++++------- .../PgExternalAccountPersistenceManager.java | 26 +++++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index 9c5753faab..5d241fdb72 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -76,7 +76,7 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro String accountEmail = userRecord.getAccountEmail(); log.info("MMAILJETT - [User {}] Starting sync for email: {}", userId, sanitiseEmailForLogging(accountEmail)); - log.debug("MMAILJETT - [User {}] Full user record: deleted={}, emailVerificationStatus={}, providerUserId={}, " + + log.info("MMAILJETT - [User {}] Full user record: deleted={}, emailVerificationStatus={}, providerUserId={}, " + "allowsNews={}, allowsEvents={}, givenName={}, role={}, stage={}", userId, userRecord.isDeleted(), @@ -142,7 +142,7 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro } catch (SegueDatabaseException e) { log.error("MMAILJETT - Database error whilst collecting users whose details have changed!", e); - throw new ExternalAccountSynchronisationException("Database error during user collection"); + throw new ExternalAccountSynchronisationException("MMAILJETT - Database error during user collection"); } log.info("MMAILJETT - === Mailjet synchronisation complete ==="); @@ -178,7 +178,7 @@ private boolean processSingleUser(UserExternalAccountChanges userRecord) } if (null != mailjetId) { - log.debug("MMAILJETT - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); + log.info("MMAILJETT - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); // Verify the user still exists in Mailjet JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); @@ -189,7 +189,7 @@ private boolean processSingleUser(UserExternalAccountChanges userRecord) database.updateExternalAccount(userId, null); mailjetId = null; } else { - log.debug("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); + log.warn("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); } } @@ -212,7 +212,7 @@ private boolean processSingleUser(UserExternalAccountChanges userRecord) userId, sanitiseEmailForLogging(mailjetDetails.getString("Email")), sanitiseEmailForLogging(accountEmail)); mailjetApi.permanentlyDeleteAccountById(mailjetId); - log.debug("MMAILJETT - [User {}] Deleted old Mailjet contact", userId); + log.info("MMAILJETT - [User {}] Deleted old Mailjet contact", userId); mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); log.info("MMAILJETT - [User {}] Created new Mailjet contact with ID: {}", userId, mailjetId); @@ -312,7 +312,7 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); - log.debug("MMAILJETT - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); + log.info("MMAILJETT - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); // Update properties try { @@ -322,7 +322,7 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou ? userRecord.getEmailVerificationStatus().toString() : ""; String stage = userRecord.getStage(); - log.debug("MMAILJETT - [User {}] Setting properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", + log.info("MMAILJETT - [User {}] Setting properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", userId, firstName, role, verificationStatus, stage); mailjetApi.updateUserProperties(mailjetId, firstName, role, verificationStatus, stage); @@ -342,7 +342,7 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : MailJetSubscriptionAction.UNSUBSCRIBE; - log.debug("MMAILJETT - [User {}] Setting subscriptions - news: {}, events: {}", + log.info("MMAILJETT - [User {}] Setting subscriptions - news: {}, events: {}", userId, newsStatus, eventsStatus); mailjetApi.updateUserSubscriptions(mailjetId, newsStatus, eventsStatus); @@ -356,7 +356,7 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou // Store the Mailjet ID try { database.updateExternalAccount(userId, mailjetId); - log.debug("MMAILJETT - [User {}] Stored Mailjet ID in database: {}", userId, mailjetId); + log.info("MMAILJETT - [User {}] Stored Mailjet ID in database: {}", userId, mailjetId); } catch (SegueDatabaseException e) { log.error("MMAILJETT - [User {}] Failed to store Mailjet ID {} in database", userId, mailjetId, e); throw e; @@ -378,7 +378,7 @@ private void deleteUserFromMailJet(final String mailjetId, final UserExternalAcc try { database.updateExternalAccount(userId, null); - log.debug("MMAILJETT - [User {}] Cleared Mailjet ID from database", userId); + log.info("MMAILJETT - [User {}] Cleared Mailjet ID from database", userId); } catch (SegueDatabaseException e) { log.error("MMAILJETT - [User {}] Failed to clear Mailjet ID from database", userId, e); throw e; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java index 6fb0992236..bfc41455e0 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java @@ -40,17 +40,20 @@ public PgExternalAccountPersistenceManager(final PostgresSqlDb database) { @Override public List getRecentlyChangedRecords() throws SegueDatabaseException { String query = "SELECT id, provider_user_identifier, email, role, given_name, deleted, email_verification_status, " - + " news_prefs.preference_value AS news_emails, events_prefs.preference_value AS events_emails " - + "FROM users " - + " LEFT OUTER JOIN user_preferences AS news_prefs ON users.id = news_prefs.user_id " - + "AND news_prefs.preference_type='EMAIL_PREFERENCE' " - + "AND news_prefs.preference_name='NEWS_AND_UPDATES' " - + " LEFT OUTER JOIN user_preferences AS events_prefs ON users.id = events_prefs.user_id " - + "AND events_prefs.preference_type='EMAIL_PREFERENCE' " - + "AND events_prefs.preference_name='EVENTS' " - + " LEFT OUTER JOIN external_accounts ON users.id=external_accounts.user_id AND provider_name='MailJet' " - + "WHERE (users.last_updated >= provider_last_updated OR news_prefs.last_updated >= provider_last_updated " - + " OR events_prefs.last_updated >= provider_last_updated OR provider_last_updated IS NULL)"; + + " registered_contexts, " + + " news_prefs.preference_value AS news_emails, events_prefs.preference_value AS events_emails " + + "FROM users " + + " LEFT OUTER JOIN user_preferences AS news_prefs ON users.id = news_prefs.user_id " + + "AND news_prefs.preference_type='EMAIL_PREFERENCE' " + + "AND news_prefs.preference_name='NEWS_AND_UPDATES' " + + " LEFT OUTER JOIN user_preferences AS events_prefs ON users.id = events_prefs.user_id " + + "AND events_prefs.preference_type='EMAIL_PREFERENCE' " + + "AND events_prefs.preference_name='EVENTS' " + + " LEFT OUTER JOIN external_accounts ON users.id=external_accounts.user_id AND provider_name='MailJet' " + + "WHERE (users.last_updated >= provider_last_updated OR news_prefs.last_updated >= provider_last_updated " + + " OR events_prefs.last_updated >= provider_last_updated OR provider_last_updated IS NULL)"; + + log.info("QUERY :{}", query); try (Connection conn = database.getDatabaseConnection(); PreparedStatement pst = conn.prepareStatement(query); ResultSet results = pst.executeQuery() @@ -64,6 +67,7 @@ public List getRecentlyChangedRecords() throws Segue return listOfResults; } catch (SQLException e) { + log.info("QUERY :{}", query); throw new SegueDatabaseException("Postgres exception", e); } } From ae2cbd88ad2f4115588871d66d314163a2bbf62b Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 11:22:36 +0200 Subject: [PATCH 4/8] PATCH 3 --- .../SegueGuiceConfigurationModule.java | 4 +- .../PgExternalAccountPersistenceManager.java | 118 +++++++++++++++--- 2 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java index ac5cf51c3c..feda53b41c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java @@ -972,7 +972,7 @@ private static StatisticsManager getStatsManager(final UserAccountManager userMa static final String CRON_STRING_0700_DAILY = "0 0 7 * * ?"; static final String CRON_STRING_2000_DAILY = "0 0 20 * * ?"; static final String CRON_STRING_HOURLY = "0 0 * ? * * *"; - static final String CRON_STRING_EVERY_FOUR_HOURS = "0 0 0/4 ? * * *"; + static final String CRON_MAIL_JET = "0 0/30 * ? * * *"; // Every 4 hours static final String CRON_GROUP_NAME_SQL_MAINTENANCE = "SQLMaintenance"; static final String CRON_GROUP_NAME_JAVA_JOB = "JavaJob"; @@ -1054,7 +1054,7 @@ private static SegueJobService getSegueJobService(final PropertiesLoader propert "syncMailjetUsersJob", CRON_GROUP_NAME_JAVA_JOB, "Sync users to mailjet", - CRON_STRING_EVERY_FOUR_HOURS); + CRON_MAIL_JET); List configuredScheduledJobs = new ArrayList<>(Arrays.asList( piiSqlJob, diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java index bfc41455e0..e835255477 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java @@ -10,6 +10,7 @@ import java.time.Instant; import java.util.List; import org.json.JSONArray; +import org.json.JSONException; import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,21 +54,23 @@ public List getRecentlyChangedRecords() throws Segue + "WHERE (users.last_updated >= provider_last_updated OR news_prefs.last_updated >= provider_last_updated " + " OR events_prefs.last_updated >= provider_last_updated OR provider_last_updated IS NULL)"; - log.info("QUERY :{}", query); + log.info("MMAILJETT - QUERY :{}", query); try (Connection conn = database.getDatabaseConnection(); PreparedStatement pst = conn.prepareStatement(query); ResultSet results = pst.executeQuery() ) { + log.info("MMAILJETT - SIZE: {}", results.getFetchSize()); List listOfResults = Lists.newArrayList(); - + log.info("MMAILJETT - LIST SIZE BEFORE: {}", listOfResults.size()); while (results.next()) { listOfResults.add(buildUserExternalAccountChanges(results)); } - + log.info("MMAILJETT - LIST SIZE AFTER: {}", listOfResults.size()); return listOfResults; } catch (SQLException e) { - log.info("QUERY :{}", query); + log.info("MMAILJETT - QUERY :{}", query); + log.error("MMAILJETT - ERROR : {}", e.toString()); throw new SegueDatabaseException("Postgres exception", e); } } @@ -108,30 +111,107 @@ public void updateExternalAccount(final Long userId, final String providerUserId } private UserExternalAccountChanges buildUserExternalAccountChanges(final ResultSet results) throws SQLException { + log.info("MMAILJETT - buildUserExternalAccountChanges - START"); + String registeredContextsJson = results.getString("registered_contexts"); + log.info("MMAILJETT - registeredContextsJson: {}", registeredContextsJson); // Parse the JSON string if it's not null JSONObject registeredContexts = null; if (registeredContextsJson != null && !registeredContextsJson.isEmpty()) { - registeredContexts = new JSONObject(registeredContextsJson); + try { + JSONObject outer = new JSONObject(registeredContextsJson); + log.info("MMAILJETT - Parsed outer JSON object, length: {}", outer.length()); + + // Check if it's double-encoded (has a single key that's a JSON string) + if (outer.length() == 1) { + String[] keys = JSONObject.getNames(outer); + log.info("MMAILJETT - Single key found: {}", keys != null && keys.length > 0 ? keys[0] : "null"); + + if (keys != null && keys.length == 1) { + String firstKey = keys[0]; + // If the key itself looks like JSON (starts with {), it's double-encoded + if (firstKey.startsWith("{")) { + log.info("MMAILJETT - Detected double-encoded JSON, parsing inner object"); + registeredContexts = new JSONObject(firstKey); + log.info("MMAILJETT - Parsed inner JSON object: {}", registeredContexts.toString()); + } else { + log.info("MMAILJETT - Using outer object as-is"); + registeredContexts = outer; + } + } + } else if (outer.length() > 1) { + log.info("MMAILJETT - Multiple keys found, using outer object as-is"); + // Normal JSON object + registeredContexts = outer; + } else { + log.info("MMAILJETT - Empty JSON object, registeredContexts will be null"); + } + // If length is 0 (empty object {}), registeredContexts stays null + + } catch (JSONException e) { + log.warn("MMAILJETT - Failed to parse registered_contexts JSON for user {}: {}", + results.getLong("id"), e.getMessage()); + } + } else { + log.info("MMAILJETT - registeredContextsJson is null or empty"); } // Extract stage from the JSON object, or use a default value String stage = (registeredContexts != null && registeredContexts.has("stage")) - ? registeredContexts.getString("stage") - : "unknown"; - - return new UserExternalAccountChanges( - results.getLong("id"), - results.getString("provider_user_identifier"), - results.getString("email"), - Role.valueOf(results.getString("role")), - results.getString("given_name"), - results.getBoolean("deleted"), - EmailVerificationStatus.valueOf(results.getString("email_verification_status")), - results.getBoolean("news_emails"), - results.getBoolean("events_emails"), - stage // Pass the extracted stage as a string + ? registeredContexts.getString("stage") + : "unknown"; + log.info("MMAILJETT - Extracted stage: {}", stage); + + // Handle nullable booleans from LEFT OUTER JOIN + Boolean newsEmails = (Boolean) results.getObject("news_emails"); + log.info("MMAILJETT - newsEmails: {}", newsEmails); + + Boolean eventsEmails = (Boolean) results.getObject("events_emails"); + log.info("MMAILJETT - eventsEmails: {}", eventsEmails); + + Long id = results.getLong("id"); + log.info("MMAILJETT - id: {}", id); + + String providerUserId = results.getString("provider_user_identifier"); + log.info("MMAILJETT - provider_user_identifier: {}", providerUserId); + + String email = results.getString("email"); + log.info("MMAILJETT - email: {}", email); + + String roleStr = results.getString("role"); + log.info("MMAILJETT - role string: {}", roleStr); + Role role = Role.valueOf(roleStr); + log.info("MMAILJETT - role enum: {}", role); + + String givenName = results.getString("given_name"); + log.info("MMAILJETT - given_name: {}", givenName); + + boolean deleted = results.getBoolean("deleted"); + log.info("MMAILJETT - deleted: {}", deleted); + + String emailVerificationStatusStr = results.getString("email_verification_status"); + log.info("MMAILJETT - email_verification_status string: {}", emailVerificationStatusStr); + EmailVerificationStatus emailVerificationStatus = EmailVerificationStatus.valueOf(emailVerificationStatusStr); + log.info("MMAILJETT - email_verification_status enum: {}", emailVerificationStatus); + + log.info("MMAILJETT - buildUserExternalAccountChanges - Creating UserExternalAccountChanges object"); + + UserExternalAccountChanges result = new UserExternalAccountChanges( + id, + providerUserId, + email, + role, + givenName, + deleted, + emailVerificationStatus, + newsEmails, + eventsEmails, + stage ); + + log.info("MMAILJETT - buildUserExternalAccountChanges - END, returning object for user {}", id); + + return result; } } From e7a1848bdc2b3401f1b7dbf6823d8e23284969f6 Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 11:44:28 +0200 Subject: [PATCH 5/8] PATCH 4 --- .../api/managers/ExternalAccountManager.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index 5d241fdb72..0dd9678fc5 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -181,15 +181,28 @@ private boolean processSingleUser(UserExternalAccountChanges userRecord) log.info("MMAILJETT - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); // Verify the user still exists in Mailjet - JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); - - if (mailjetDetails == null) { - log.warn("MMAILJETT - [User {}] Mailjet ID {} not found in Mailjet - will recreate", userId, mailjetId); - // Clear the mailjet ID and recreate - database.updateExternalAccount(userId, null); - mailjetId = null; - } else { - log.warn("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); + try { + JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); + + if (mailjetDetails == null) { + log.warn("MMAILJETT - [User {}] Mailjet ID {} not found in Mailjet - will recreate", userId, mailjetId); + // Clear the mailjet ID and recreate + database.updateExternalAccount(userId, null); + mailjetId = null; + } else { + log.info("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); + } + } catch (MailjetException e) { + // If we get 404, clear the ID and recreate + if (e.getMessage() != null && (e.getMessage().contains("404") || e.getMessage().contains("Object not found"))) { + log.warn("MMAILJETT - [User {}] Mailjet contact {} returned 404 - clearing and recreating", + userId, mailjetId); + database.updateExternalAccount(userId, null); + mailjetId = null; + } else { + // Re-throw other errors + throw e; + } } } From 24c3833216211bced8ab82af73a1f7f0e6be93bc Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 12:29:22 +0200 Subject: [PATCH 6/8] PATCH 5 --- .../api/managers/ExternalAccountManager.java | 78 +++++++++++++++++-- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index 0dd9678fc5..41d67dd244 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -322,7 +322,7 @@ private boolean validateUserData(UserExternalAccountChanges userRecord) { } private void updateUserOnMailJet(final String mailjetId, final UserExternalAccountChanges userRecord) - throws SegueDatabaseException, MailjetException { + throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); log.info("MMAILJETT - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); @@ -332,16 +332,24 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou String firstName = userRecord.getGivenName(); String role = userRecord.getRole() != null ? userRecord.getRole().toString() : ""; String verificationStatus = userRecord.getEmailVerificationStatus() != null - ? userRecord.getEmailVerificationStatus().toString() : ""; + ? userRecord.getEmailVerificationStatus().toString() : ""; String stage = userRecord.getStage(); log.info("MMAILJETT - [User {}] Setting properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", - userId, firstName, role, verificationStatus, stage); + userId, firstName, role, verificationStatus, stage); mailjetApi.updateUserProperties(mailjetId, firstName, role, verificationStatus, stage); log.info("MMAILJETT - [User {}] Successfully updated Mailjet properties", userId); } catch (MailjetException e) { + // Check if this is a 404 - contact was deleted from Mailjet but we still have the ID + if (is404Error(e)) { + log.warn("MMAILJETT - [User {}] Contact with Mailjet ID {} not found (404) during property update - clearing stale ID", + userId, mailjetId); + handleStaleMailjetId(userId, mailjetId); + return; // Exit early - contact will be recreated on next sync + } + log.error("MMAILJETT - [User {}] Failed to update Mailjet properties for ID {}", userId, mailjetId, e); throw e; } @@ -349,19 +357,27 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou // Update subscriptions try { MailJetSubscriptionAction newsStatus = (userRecord.allowsNewsEmails() != null - && userRecord.allowsNewsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : - MailJetSubscriptionAction.UNSUBSCRIBE; + && userRecord.allowsNewsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : + MailJetSubscriptionAction.UNSUBSCRIBE; MailJetSubscriptionAction eventsStatus = (userRecord.allowsEventsEmails() != null - && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : - MailJetSubscriptionAction.UNSUBSCRIBE; + && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : + MailJetSubscriptionAction.UNSUBSCRIBE; log.info("MMAILJETT - [User {}] Setting subscriptions - news: {}, events: {}", - userId, newsStatus, eventsStatus); + userId, newsStatus, eventsStatus); mailjetApi.updateUserSubscriptions(mailjetId, newsStatus, eventsStatus); log.info("MMAILJETT - [User {}] Successfully updated Mailjet subscriptions", userId); } catch (MailjetException e) { + // Check if this is a 404 - contact was deleted from Mailjet but we still have the ID + if (is404Error(e)) { + log.warn("MMAILJETT - [User {}] Contact with Mailjet ID {} not found (404) during subscription update - clearing stale ID", + userId, mailjetId); + handleStaleMailjetId(userId, mailjetId); + return; // Exit early - contact will be recreated on next sync + } + log.error("MMAILJETT - [User {}] Failed to update Mailjet subscriptions for ID {}", userId, mailjetId, e); throw e; } @@ -376,6 +392,52 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou } } + /** + * Checks if a MailjetException is a 404 error. + * + * @param e The MailjetException to check + * @return true if this is a 404 error, false otherwise + */ + private boolean is404Error(MailjetException e) { + if (e == null) { + return false; + } + + String message = e.getMessage(); + if (message == null) { + return false; + } + + // Check for 404 status code in the error message + return message.contains("\"StatusCode\" : 404") || + message.contains("\"StatusCode\": 404") || + message.contains("Object not found"); + } + + /** + * Handles a stale Mailjet ID by clearing it from the database. + * The contact will be recreated on the next sync run. + * + * @param userId The user ID + * @param staleMailjetId The stale Mailjet ID to clear + * @throws SegueDatabaseException if clearing the ID fails + */ + private void handleStaleMailjetId(Long userId, String staleMailjetId) throws SegueDatabaseException { + log.info("MMAILJETT - [User {}] Clearing stale Mailjet ID: {}", userId, staleMailjetId); + + try { + // Clear the Mailjet ID from the database by setting it to null + database.updateExternalAccount(userId, null); + log.info("MMAILJETT - [User {}] Successfully cleared stale Mailjet ID {}. " + + "Contact will be recreated on next sync run.", userId, staleMailjetId); + + } catch (SegueDatabaseException e) { + log.error("MMAILJETT - [User {}] CRITICAL: Failed to clear stale Mailjet ID {} from database", + userId, staleMailjetId, e); + throw e; + } + } + private void deleteUserFromMailJet(final String mailjetId, final UserExternalAccountChanges userRecord) throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); From 184e2d331e9cdcc6703b2bb1eba45a74cc94dfb3 Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 20:52:23 +0200 Subject: [PATCH 7/8] PATCH 6 --- .../api/managers/ExternalAccountManager.java | 336 +++++++++--------- .../PgExternalAccountPersistenceManager.java | 229 ++++++------ .../util/email/MailJetApiClientWrapper.java | 175 +++++---- 3 files changed, 377 insertions(+), 363 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index 41d67dd244..7337823ba6 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -22,6 +22,8 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.util.List; + +import org.json.JSONException; import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +33,6 @@ import uk.ac.cam.cl.dtg.segue.dao.users.IExternalAccountDataManager; import uk.ac.cam.cl.dtg.util.email.MailJetApiClientWrapper; import uk.ac.cam.cl.dtg.util.email.MailJetSubscriptionAction; - public class ExternalAccountManager implements IExternalAccountManager { private static final Logger log = LoggerFactory.getLogger(ExternalAccountManager.class); @@ -60,7 +61,7 @@ public ExternalAccountManager(final MailJetApiClientWrapper mailjetApi, final IE */ @Override public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchronisationException { - log.info("MMAILJETT - === Starting Mailjet user synchronisation ==="); + log.info("MAILJET - === Starting Mailjet user synchronisation ==="); int totalUsersProcessed = 0; int successfulSyncs = 0; int failedSyncs = 0; @@ -68,88 +69,75 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro try { List userRecordsToUpdate = database.getRecentlyChangedRecords(); - log.info("MMAILJETT - Found {} users requiring synchronisation", userRecordsToUpdate.size()); + log.info("MAILJET - Found {} users requiring synchronisation", userRecordsToUpdate.size()); for (UserExternalAccountChanges userRecord : userRecordsToUpdate) { totalUsersProcessed++; Long userId = userRecord.getUserId(); String accountEmail = userRecord.getAccountEmail(); - log.info("MMAILJETT - [User {}] Starting sync for email: {}", userId, sanitiseEmailForLogging(accountEmail)); - log.info("MMAILJETT - [User {}] Full user record: deleted={}, emailVerificationStatus={}, providerUserId={}, " + - "allowsNews={}, allowsEvents={}, givenName={}, role={}, stage={}", - userId, - userRecord.isDeleted(), - userRecord.getEmailVerificationStatus(), - userRecord.getProviderUserId(), - userRecord.allowsNewsEmails(), - userRecord.allowsEventsEmails(), - userRecord.getGivenName(), - userRecord.getRole(), - userRecord.getStage()); + log.debug("MAILJET - [User {}] Starting sync for email: {}", userId, sanitiseEmailForLogging(accountEmail)); + log.debug("MAILJET - [User {}] Details: deleted={}, emailStatus={}, mailjetId={}, allowsNews={}, allowsEvents={}, role={}, stage={}", + userId, + userRecord.isDeleted(), + userRecord.getEmailVerificationStatus(), + userRecord.getProviderUserId(), + userRecord.allowsNewsEmails(), + userRecord.allowsEventsEmails(), + userRecord.getRole(), + userRecord.getStage()); try { boolean syncSuccess = processSingleUser(userRecord); if (syncSuccess) { successfulSyncs++; - log.info("MMAILJETT - [User {}] Successfully synced to Mailjet", userId); + log.info("MAILJET - [User {}] Successfully synced to Mailjet", userId); } else { skippedUsers++; - log.info("MMAILJETT - [User {}] Skipped (invalid/deleted user not requiring Mailjet)", userId); + log.debug("MAILJET - [User {}] Skipped (invalid/deleted/unsubscribed user)", userId); } } catch (SegueDatabaseException e) { failedSyncs++; - log.error(String.format("MMAILJETT - [User %s] Database error during sync - will retry on next run", userId), e); + log.error("MAILJET - [User {}] Database error during sync - will retry on next run: {}", userId, e.getMessage()); // Don't update provider_last_updated so it will be retried } catch (MailjetClientCommunicationException e) { failedSyncs++; - log.error(String.format("MMAILJETT - [User %s] Failed to communicate with Mailjet API", userId), e); - log.error("MMAILJETT - [User {}] MailjetClientCommunicationException details: {}", userId, e.getMessage()); + log.error("MAILJET - [User {}] Failed to communicate with Mailjet API: {}", userId, e.getMessage()); throw new ExternalAccountSynchronisationException("Failed to successfully connect to MailJet!"); } catch (MailjetRateLimitException e) { failedSyncs++; - log.warn(String.format("MMAILJETT - [User %s] Hit Mailjet rate limit - stopping sync", userId), e); + log.warn("MAILJET - [User {}] Hit Mailjet rate limit - stopping sync", userId); throw new ExternalAccountSynchronisationException("MailJet API rate limits exceeded!"); } catch (MailjetException e) { failedSyncs++; - log.error(String.format("MMAILJETT - [User %s] Mailjet API error during sync", userId), e); - log.error("MMAILJETT - [User {}] MailjetException type: {}, message: {}", - userId, e.getClass().getName(), e.getMessage()); - // Log the full stack trace for debugging - StringWriter sw = new StringWriter(); - e.printStackTrace(new PrintWriter(sw)); - log.error("MMAILJETT - [User {}] Full stack trace: {}", userId, sw.toString()); - - // Don't throw - continue processing other users, but don't update provider_last_updated - // so this user will be retried next time + log.error("MAILJET - [User {}] Mailjet API error: {} - {}", userId, e.getClass().getSimpleName(), e.getMessage()); + // Log stack trace at debug level to avoid flooding logs + log.debug("MAILJET - [User {}] Full stack trace:", userId, e); + // Don't throw - continue processing other users } catch (Exception e) { failedSyncs++; - log.error(String.format("MMAILJETT - [User %s] Unexpected error during sync", userId), e); - log.error("MMAILJETT - [User {}] Unexpected exception type: {}, message: {}", - userId, e.getClass().getName(), e.getMessage()); - StringWriter sw = new StringWriter(); - e.printStackTrace(new PrintWriter(sw)); - log.error("MMAILJETT - [User {}] Full stack trace: {}", userId, sw.toString()); + log.error("MAILJET - [User {}] Unexpected error: {} - {}", userId, e.getClass().getSimpleName(), e.getMessage()); + log.debug("MAILJET - [User {}] Full stack trace:", userId, e); // Continue processing other users } } } catch (SegueDatabaseException e) { - log.error("MMAILJETT - Database error whilst collecting users whose details have changed!", e); - throw new ExternalAccountSynchronisationException("MMAILJETT - Database error during user collection"); + log.error("MAILJET - Database error whilst collecting users: {}", e.getMessage(), e); + throw new ExternalAccountSynchronisationException("Database error during user collection"); } - log.info("MMAILJETT - === Mailjet synchronisation complete ==="); - log.info("MMAILJETT - Total users processed: {}", totalUsersProcessed); - log.info("MMAILJETT - Successful syncs: {}", successfulSyncs); - log.info("MMAILJETT - Failed syncs: {}", failedSyncs); - log.info("MMAILJETT - Skipped users: {}", skippedUsers); + log.info("MAILJET - === Mailjet synchronisation complete ==="); + log.info("MAILJET - Total users processed: {}", totalUsersProcessed); + log.info("MAILJET - Successful syncs: {}", successfulSyncs); + log.info("MAILJET - Failed syncs: {}", failedSyncs); + log.info("MAILJET - Skipped users: {}", skippedUsers); } /** @@ -161,46 +149,51 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro * @throws MailjetException if Mailjet API calls fail */ private boolean processSingleUser(UserExternalAccountChanges userRecord) - throws SegueDatabaseException, MailjetException { + throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); String accountEmail = userRecord.getAccountEmail(); boolean accountEmailDeliveryFailed = - EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus()); + EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus()); String mailjetId = userRecord.getProviderUserId(); // Validate user data before attempting sync if (!validateUserData(userRecord)) { - log.warn("MMAILJETT - [User {}] Skipping sync due to invalid user data", userId); + log.warn("MAILJET - [User {}] Skipping sync due to invalid user data", userId); // Still update provider_last_updated to prevent repeated attempts database.updateProviderLastUpdated(userId); return false; } + // Check if user should be synced to Mailjet at all + // Skip users who are not subscribed to any emails and don't already exist in Mailjet + if (mailjetId == null && !shouldSyncToMailjet(userRecord)) { + log.debug("MAILJET - [User {}] Skipping - user not subscribed to any emails", userId); + database.updateProviderLastUpdated(userId); + return false; + } + if (null != mailjetId) { - log.info("MMAILJETT - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); + log.debug("MAILJET - [User {}] Existing Mailjet user with ID: {}", userId, mailjetId); // Verify the user still exists in Mailjet try { JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); if (mailjetDetails == null) { - log.warn("MMAILJETT - [User {}] Mailjet ID {} not found in Mailjet - will recreate", userId, mailjetId); - // Clear the mailjet ID and recreate + log.warn("MAILJET - [User {}] Mailjet ID {} not found - will recreate if needed", userId, mailjetId); database.updateExternalAccount(userId, null); mailjetId = null; } else { - log.info("MMAILJETT - [User {}] Found existing Mailjet account: {}", userId, mailjetDetails.toString()); + log.debug("MAILJET - [User {}] Found existing Mailjet account", userId); } } catch (MailjetException e) { - // If we get 404, clear the ID and recreate - if (e.getMessage() != null && (e.getMessage().contains("404") || e.getMessage().contains("Object not found"))) { - log.warn("MMAILJETT - [User {}] Mailjet contact {} returned 404 - clearing and recreating", - userId, mailjetId); + // If we get 404, clear the ID + if (is404Error(e)) { + log.warn("MAILJET - [User {}] Mailjet contact {} returned 404 - clearing stale ID", userId, mailjetId); database.updateExternalAccount(userId, null); mailjetId = null; } else { - // Re-throw other errors throw e; } } @@ -208,114 +201,155 @@ private boolean processSingleUser(UserExternalAccountChanges userRecord) if (null != mailjetId) { // User exists in Mailjet - handle updates/deletions + return handleExistingMailjetUser(userRecord, mailjetId); + + } else { + // User doesn't exist in Mailjet yet + return handleNewMailjetUser(userRecord); + } + } + + /** + * Determines if a user should be synced to Mailjet. + * Users should be synced if they are subscribed to any emails OR if they already exist in Mailjet. + */ + private boolean shouldSyncToMailjet(UserExternalAccountChanges userRecord) { + // Don't sync deleted users or users with delivery failures + if (userRecord.isDeleted() || + EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus())) { + return false; + } + + // Sync if user is subscribed to news or events + return (userRecord.allowsNewsEmails() != null && userRecord.allowsNewsEmails()) + || (userRecord.allowsEventsEmails() != null && userRecord.allowsEventsEmails()); + } + + /** + * Handles sync for a user who already exists in Mailjet. + */ + private boolean handleExistingMailjetUser(UserExternalAccountChanges userRecord, String mailjetId) + throws SegueDatabaseException, MailjetException { + + Long userId = userRecord.getUserId(); + String accountEmail = userRecord.getAccountEmail(); + + try { JSONObject mailjetDetails = mailjetApi.getAccountByIdOrEmail(mailjetId); if (userRecord.isDeleted()) { - log.info("MMAILJETT - [User {}] Processing deletion from Mailjet", userId); + log.info("MAILJET - [User {}] Processing deletion from Mailjet", userId); deleteUserFromMailJet(mailjetId, userRecord); - } else if (accountEmailDeliveryFailed) { - log.info("MMAILJETT - [User {}] Email delivery failed - removing from lists", userId); + } else if (EmailVerificationStatus.DELIVERY_FAILED.equals(userRecord.getEmailVerificationStatus())) { + log.info("MAILJET - [User {}] Email delivery failed - removing from lists", userId); mailjetApi.updateUserSubscriptions(mailjetId, MailJetSubscriptionAction.REMOVE, - MailJetSubscriptionAction.REMOVE); + MailJetSubscriptionAction.REMOVE); database.updateProviderLastUpdated(userId); - } else if (!accountEmail.toLowerCase().equals(mailjetDetails.getString("Email"))) { - log.info("MMAILJETT - [User {}] Email changed from {} to {} - recreating Mailjet contact", - userId, sanitiseEmailForLogging(mailjetDetails.getString("Email")), - sanitiseEmailForLogging(accountEmail)); + } else if (!accountEmail.toLowerCase().equals(mailjetDetails.getString("Email").toLowerCase())) { + log.info("MAILJET - [User {}] Email changed - recreating Mailjet contact", userId); mailjetApi.permanentlyDeleteAccountById(mailjetId); - log.info("MMAILJETT - [User {}] Deleted old Mailjet contact", userId); + log.debug("MAILJET - [User {}] Deleted old Mailjet contact", userId); mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); - log.info("MMAILJETT - [User {}] Created new Mailjet contact with ID: {}", userId, mailjetId); + log.info("MAILJET - [User {}] Created new Mailjet contact with ID: {}", userId, mailjetId); updateUserOnMailJet(mailjetId, userRecord); } else { - log.info("MMAILJETT - [User {}] Updating existing Mailjet contact", userId); + log.debug("MAILJET - [User {}] Updating existing Mailjet contact", userId); updateUserOnMailJet(mailjetId, userRecord); } - } else { - // User doesn't exist in Mailjet yet - if (!accountEmailDeliveryFailed && !userRecord.isDeleted()) { - log.info("MMAILJETT - [User {}] New user - creating Mailjet contact", userId); + database.updateProviderLastUpdated(userId); + return true; - // Create the contact - mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); + } catch (JSONException e) { + log.error("MAILJET - [User {}] Failed to parse Mailjet account details: {}", userId, e.getMessage()); + throw new MailjetException("Failed to parse Mailjet response", e); + } + } - if (mailjetId == null) { - log.error("MMAILJETT - [User {}] Failed to create Mailjet contact - addNewUserOrGetUserIfExists returned null", userId); - throw new MailjetException("Failed to create Mailjet contact - returned null ID"); - } + /** + * Handles sync for a user who doesn't exist in Mailjet yet. + */ + private boolean handleNewMailjetUser(UserExternalAccountChanges userRecord) + throws SegueDatabaseException, MailjetException { - log.info("MMAILJETT - [User {}] Created Mailjet contact with ID: {}", userId, mailjetId); + Long userId = userRecord.getUserId(); + String accountEmail = userRecord.getAccountEmail(); - // Now update properties and subscriptions - // Use try-finally to ensure we record the mailjet ID even if updates fail - boolean updateSucceeded = false; - try { - updateUserOnMailJet(mailjetId, userRecord); - updateSucceeded = true; - } finally { - if (!updateSucceeded) { - log.error("MMAILJETT - [User {}] Failed to update properties/subscriptions for Mailjet ID {} - " + - "contact exists but may have incomplete data", userId, mailjetId); - // Store the mailjet ID anyway so we don't create duplicates - database.updateExternalAccount(userId, mailjetId); - } - } + // Only create new contacts if user is subscribed + if (!shouldSyncToMailjet(userRecord)) { + log.debug("MAILJET - [User {}] Skipping creation - not subscribed to any emails", userId); + database.updateProviderLastUpdated(userId); + return false; + } - } else { - log.info("MMAILJETT - [User {}] Skipping - delivery failed or deleted (not uploading to Mailjet)", userId); - database.updateExternalAccount(userId, null); - database.updateProviderLastUpdated(userId); - return false; + log.info("MAILJET - [User {}] New user - creating Mailjet contact", userId); + + // Create the contact + String mailjetId = mailjetApi.addNewUserOrGetUserIfExists(accountEmail); + + if (mailjetId == null) { + log.error("MAILJET - [User {}] Failed to create Mailjet contact - returned null ID", userId); + throw new MailjetException("Failed to create Mailjet contact - returned null ID"); + } + + log.info("MAILJET - [User {}] Created Mailjet contact with ID: {}", userId, mailjetId); + + // Update properties and subscriptions + boolean updateSucceeded = false; + try { + updateUserOnMailJet(mailjetId, userRecord); + updateSucceeded = true; + } finally { + if (!updateSucceeded) { + log.error("MAILJET - [User {}] Failed to update properties/subscriptions for Mailjet ID {} - contact exists but may have incomplete data", + userId, mailjetId); + // Store the mailjet ID anyway so we don't create duplicates + database.updateExternalAccount(userId, mailjetId); } } - // If we got here, sync succeeded database.updateProviderLastUpdated(userId); return true; } /** * Validate user data before attempting Mailjet sync. - * - * @param userRecord the user record to validate - * @return true if data is valid, false otherwise */ private boolean validateUserData(UserExternalAccountChanges userRecord) { Long userId = userRecord.getUserId(); String accountEmail = userRecord.getAccountEmail(); if (accountEmail == null || accountEmail.trim().isEmpty()) { - log.error("MMAILJETT - [User {}] Invalid data: email is null or empty", userId); + log.error("MAILJET - [User {}] Invalid data: email is null or empty", userId); return false; } if (!accountEmail.contains("@")) { - log.error("MMAILJETT - [User {}] Invalid data: email '{}' does not contain @", - userId, sanitiseEmailForLogging(accountEmail)); + log.error("MAILJET - [User {}] Invalid data: email '{}' does not contain @", + userId, sanitiseEmailForLogging(accountEmail)); return false; } - // Check for required fields (allowing nulls but logging them) + // Log warnings for null fields but don't fail validation if (userRecord.getGivenName() == null) { - log.warn("MMAILJETT - [User {}] Warning: given_name is null", userId); + log.debug("MAILJET - [User {}] Warning: given_name is null", userId); } if (userRecord.getRole() == null) { - log.warn("MMAILJETT - [User {}] Warning: role is null", userId); + log.warn("MAILJET - [User {}] Warning: role is null", userId); } if (userRecord.getStage() == null) { - log.warn("MMAILJETT - [User {}] Warning: stage is null", userId); + log.debug("MAILJET - [User {}] Warning: stage is null", userId); } if (userRecord.getEmailVerificationStatus() == null) { - log.warn("MMAILJETT - [User {}] Warning: email_verification_status is null", userId); + log.warn("MAILJET - [User {}] Warning: email_verification_status is null", userId); } return true; @@ -325,7 +359,7 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); - log.info("MMAILJETT - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); + log.debug("MAILJET - [User {}] Updating properties for Mailjet ID: {}", userId, mailjetId); // Update properties try { @@ -333,24 +367,22 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou String role = userRecord.getRole() != null ? userRecord.getRole().toString() : ""; String verificationStatus = userRecord.getEmailVerificationStatus() != null ? userRecord.getEmailVerificationStatus().toString() : ""; - String stage = userRecord.getStage(); + String stage = userRecord.getStage() != null ? userRecord.getStage() : "not_specified"; - log.info("MMAILJETT - [User {}] Setting properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", - userId, firstName, role, verificationStatus, stage); + log.debug("MAILJET - [User {}] Setting properties - firstName: {}, role: {}, stage: {}", + userId, firstName, role, stage); mailjetApi.updateUserProperties(mailjetId, firstName, role, verificationStatus, stage); - log.info("MMAILJETT - [User {}] Successfully updated Mailjet properties", userId); + log.debug("MAILJET - [User {}] Successfully updated Mailjet properties", userId); } catch (MailjetException e) { - // Check if this is a 404 - contact was deleted from Mailjet but we still have the ID if (is404Error(e)) { - log.warn("MMAILJETT - [User {}] Contact with Mailjet ID {} not found (404) during property update - clearing stale ID", + log.warn("MAILJET - [User {}] Contact {} not found (404) during property update - clearing stale ID", userId, mailjetId); handleStaleMailjetId(userId, mailjetId); - return; // Exit early - contact will be recreated on next sync + return; } - - log.error("MMAILJETT - [User {}] Failed to update Mailjet properties for ID {}", userId, mailjetId, e); + log.error("MAILJET - [User {}] Failed to update Mailjet properties for ID {}", userId, mailjetId); throw e; } @@ -363,108 +395,92 @@ private void updateUserOnMailJet(final String mailjetId, final UserExternalAccou && userRecord.allowsEventsEmails()) ? MailJetSubscriptionAction.FORCE_SUBSCRIBE : MailJetSubscriptionAction.UNSUBSCRIBE; - log.info("MMAILJETT - [User {}] Setting subscriptions - news: {}, events: {}", + log.debug("MAILJET - [User {}] Setting subscriptions - news: {}, events: {}", userId, newsStatus, eventsStatus); mailjetApi.updateUserSubscriptions(mailjetId, newsStatus, eventsStatus); - log.info("MMAILJETT - [User {}] Successfully updated Mailjet subscriptions", userId); + log.debug("MAILJET - [User {}] Successfully updated Mailjet subscriptions", userId); } catch (MailjetException e) { - // Check if this is a 404 - contact was deleted from Mailjet but we still have the ID if (is404Error(e)) { - log.warn("MMAILJETT - [User {}] Contact with Mailjet ID {} not found (404) during subscription update - clearing stale ID", + log.warn("MAILJET - [User {}] Contact {} not found (404) during subscription update - clearing stale ID", userId, mailjetId); handleStaleMailjetId(userId, mailjetId); - return; // Exit early - contact will be recreated on next sync + return; } - - log.error("MMAILJETT - [User {}] Failed to update Mailjet subscriptions for ID {}", userId, mailjetId, e); + log.error("MAILJET - [User {}] Failed to update Mailjet subscriptions for ID {}", userId, mailjetId); throw e; } // Store the Mailjet ID try { database.updateExternalAccount(userId, mailjetId); - log.info("MMAILJETT - [User {}] Stored Mailjet ID in database: {}", userId, mailjetId); + log.debug("MAILJET - [User {}] Stored Mailjet ID in database: {}", userId, mailjetId); } catch (SegueDatabaseException e) { - log.error("MMAILJETT - [User {}] Failed to store Mailjet ID {} in database", userId, mailjetId, e); + log.error("MAILJET - [User {}] Failed to store Mailjet ID {} in database", userId, mailjetId); throw e; } } /** * Checks if a MailjetException is a 404 error. - * - * @param e The MailjetException to check - * @return true if this is a 404 error, false otherwise */ private boolean is404Error(MailjetException e) { - if (e == null) { + if (e == null || e.getMessage() == null) { return false; } String message = e.getMessage(); - if (message == null) { - return false; - } - - // Check for 404 status code in the error message return message.contains("\"StatusCode\" : 404") || message.contains("\"StatusCode\": 404") || - message.contains("Object not found"); + message.contains("Object not found") || + message.contains("404"); } /** * Handles a stale Mailjet ID by clearing it from the database. - * The contact will be recreated on the next sync run. - * - * @param userId The user ID - * @param staleMailjetId The stale Mailjet ID to clear - * @throws SegueDatabaseException if clearing the ID fails */ private void handleStaleMailjetId(Long userId, String staleMailjetId) throws SegueDatabaseException { - log.info("MMAILJETT - [User {}] Clearing stale Mailjet ID: {}", userId, staleMailjetId); + log.info("MAILJET - [User {}] Clearing stale Mailjet ID: {}", userId, staleMailjetId); try { - // Clear the Mailjet ID from the database by setting it to null database.updateExternalAccount(userId, null); - log.info("MMAILJETT - [User {}] Successfully cleared stale Mailjet ID {}. " + - "Contact will be recreated on next sync run.", userId, staleMailjetId); - + log.info("MAILJET - [User {}] Cleared stale Mailjet ID. Will be recreated on next sync.", userId); } catch (SegueDatabaseException e) { - log.error("MMAILJETT - [User {}] CRITICAL: Failed to clear stale Mailjet ID {} from database", - userId, staleMailjetId, e); + log.error("MAILJET - [User {}] CRITICAL: Failed to clear stale Mailjet ID {}", userId, staleMailjetId); throw e; } } private void deleteUserFromMailJet(final String mailjetId, final UserExternalAccountChanges userRecord) - throws SegueDatabaseException, MailjetException { + throws SegueDatabaseException, MailjetException { Long userId = userRecord.getUserId(); - log.info("MMAILJETT - [User {}] Deleting from Mailjet, ID: {}", userId, mailjetId); + log.info("MAILJET - [User {}] Deleting from Mailjet, ID: {}", userId, mailjetId); try { mailjetApi.permanentlyDeleteAccountById(mailjetId); - log.info("MMAILJETT - [User {}] Successfully deleted from Mailjet", userId); + log.info("MAILJET - [User {}] Successfully deleted from Mailjet", userId); } catch (MailjetException e) { - log.error("MMAILJETT - [User {}] Failed to delete from Mailjet, ID: {}", userId, mailjetId, e); - throw e; + // If already deleted (404), that's fine + if (is404Error(e)) { + log.info("MAILJET - [User {}] Contact already deleted from Mailjet (404)", userId); + } else { + log.error("MAILJET - [User {}] Failed to delete from Mailjet, ID: {}", userId, mailjetId); + throw e; + } } try { database.updateExternalAccount(userId, null); - log.info("MMAILJETT - [User {}] Cleared Mailjet ID from database", userId); + log.debug("MAILJET - [User {}] Cleared Mailjet ID from database", userId); } catch (SegueDatabaseException e) { - log.error("MMAILJETT - [User {}] Failed to clear Mailjet ID from database", userId, e); + log.error("MAILJET - [User {}] Failed to clear Mailjet ID from database", userId); throw e; } } /** * Sanitise email for logging to prevent log injection and reduce PII exposure. - * - * @param email the email to sanitise - * @return sanitised email (e.g., "u***@example.com") */ private String sanitiseEmailForLogging(String email) { if (email == null) { diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java index e835255477..39287e0d2f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/PgExternalAccountPersistenceManager.java @@ -19,7 +19,6 @@ import uk.ac.cam.cl.dtg.isaac.dos.users.UserExternalAccountChanges; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; - /** * This class is responsible for managing and persisting user data. */ @@ -40,37 +39,42 @@ public PgExternalAccountPersistenceManager(final PostgresSqlDb database) { @Override public List getRecentlyChangedRecords() throws SegueDatabaseException { - String query = "SELECT id, provider_user_identifier, email, role, given_name, deleted, email_verification_status, " + String query = "SELECT users.id, provider_user_identifier, email, role, given_name, deleted, email_verification_status, " + " registered_contexts, " - + " news_prefs.preference_value AS news_emails, events_prefs.preference_value AS events_emails " + + " news_prefs.preference_value AS news_emails, " + + " events_prefs.preference_value AS events_emails " + "FROM users " + " LEFT OUTER JOIN user_preferences AS news_prefs ON users.id = news_prefs.user_id " - + "AND news_prefs.preference_type='EMAIL_PREFERENCE' " - + "AND news_prefs.preference_name='NEWS_AND_UPDATES' " + + " AND news_prefs.preference_type='EMAIL_PREFERENCE' " + + " AND news_prefs.preference_name='NEWS_AND_UPDATES' " + " LEFT OUTER JOIN user_preferences AS events_prefs ON users.id = events_prefs.user_id " - + "AND events_prefs.preference_type='EMAIL_PREFERENCE' " - + "AND events_prefs.preference_name='EVENTS' " - + " LEFT OUTER JOIN external_accounts ON users.id=external_accounts.user_id AND provider_name='MailJet' " - + "WHERE (users.last_updated >= provider_last_updated OR news_prefs.last_updated >= provider_last_updated " - + " OR events_prefs.last_updated >= provider_last_updated OR provider_last_updated IS NULL)"; + + " AND events_prefs.preference_type='EMAIL_PREFERENCE' " + + " AND events_prefs.preference_name='EVENTS' " + + " LEFT OUTER JOIN external_accounts ON users.id=external_accounts.user_id " + + " AND provider_name='MailJet' " + + "WHERE (users.last_updated >= provider_last_updated " + + " OR news_prefs.last_updated >= provider_last_updated " + + " OR events_prefs.last_updated >= provider_last_updated " + + " OR provider_last_updated IS NULL) " + + "ORDER BY users.id"; + + log.info("MAILJET - Fetching recently changed records for sync"); - log.info("MMAILJETT - QUERY :{}", query); try (Connection conn = database.getDatabaseConnection(); PreparedStatement pst = conn.prepareStatement(query); ResultSet results = pst.executeQuery() ) { - log.info("MMAILJETT - SIZE: {}", results.getFetchSize()); List listOfResults = Lists.newArrayList(); - log.info("MMAILJETT - LIST SIZE BEFORE: {}", listOfResults.size()); + while (results.next()) { listOfResults.add(buildUserExternalAccountChanges(results)); } - log.info("MMAILJETT - LIST SIZE AFTER: {}", listOfResults.size()); + + log.info("MAILJET - Retrieved {} users requiring synchronisation", listOfResults.size()); return listOfResults; } catch (SQLException e) { - log.info("MMAILJETT - QUERY :{}", query); - log.error("MMAILJETT - ERROR : {}", e.toString()); + log.error("MAILJET - Database error fetching recently changed records", e); throw new SegueDatabaseException("Postgres exception", e); } } @@ -92,12 +96,12 @@ public void updateProviderLastUpdated(final Long userId) throws SegueDatabaseExc @Override public void updateExternalAccount(final Long userId, final String providerUserIdentifier) - throws SegueDatabaseException { + throws SegueDatabaseException { // Upsert the value in, using Postgres 9.5 syntax 'ON CONFLICT DO UPDATE ...' String query = - "INSERT INTO external_accounts(user_id, provider_name, provider_user_identifier) VALUES (?, 'MailJet', ?)" - + " ON CONFLICT (user_id, provider_name) DO UPDATE SET" - + " provider_user_identifier=excluded.provider_user_identifier"; + "INSERT INTO external_accounts(user_id, provider_name, provider_user_identifier) VALUES (?, 'MailJet', ?)" + + " ON CONFLICT (user_id, provider_name) DO UPDATE SET" + + " provider_user_identifier=excluded.provider_user_identifier"; try (Connection conn = database.getDatabaseConnection(); PreparedStatement pst = conn.prepareStatement(query) ) { @@ -110,108 +114,103 @@ public void updateExternalAccount(final Long userId, final String providerUserId } } + /** + * Builds a UserExternalAccountChanges object from a ResultSet. + * Properly parses the registered_contexts array which contains JSON strings like: + * {"{\"stage\": \"gcse\", \"examBoard\": \"all\"}","{\"stage\": \"a_level\", \"examBoard\": \"all\"}"} + * + * @param results the ResultSet containing user data + * @return UserExternalAccountChanges object + * @throws SQLException if database access fails + */ private UserExternalAccountChanges buildUserExternalAccountChanges(final ResultSet results) throws SQLException { - log.info("MMAILJETT - buildUserExternalAccountChanges - START"); - - String registeredContextsJson = results.getString("registered_contexts"); - log.info("MMAILJETT - registeredContextsJson: {}", registeredContextsJson); - - // Parse the JSON string if it's not null - JSONObject registeredContexts = null; - if (registeredContextsJson != null && !registeredContextsJson.isEmpty()) { - try { - JSONObject outer = new JSONObject(registeredContextsJson); - log.info("MMAILJETT - Parsed outer JSON object, length: {}", outer.length()); - - // Check if it's double-encoded (has a single key that's a JSON string) - if (outer.length() == 1) { - String[] keys = JSONObject.getNames(outer); - log.info("MMAILJETT - Single key found: {}", keys != null && keys.length > 0 ? keys[0] : "null"); - - if (keys != null && keys.length == 1) { - String firstKey = keys[0]; - // If the key itself looks like JSON (starts with {), it's double-encoded - if (firstKey.startsWith("{")) { - log.info("MMAILJETT - Detected double-encoded JSON, parsing inner object"); - registeredContexts = new JSONObject(firstKey); - log.info("MMAILJETT - Parsed inner JSON object: {}", registeredContexts.toString()); - } else { - log.info("MMAILJETT - Using outer object as-is"); - registeredContexts = outer; - } - } - } else if (outer.length() > 1) { - log.info("MMAILJETT - Multiple keys found, using outer object as-is"); - // Normal JSON object - registeredContexts = outer; - } else { - log.info("MMAILJETT - Empty JSON object, registeredContexts will be null"); - } - // If length is 0 (empty object {}), registeredContexts stays null - - } catch (JSONException e) { - log.warn("MMAILJETT - Failed to parse registered_contexts JSON for user {}: {}", - results.getLong("id"), e.getMessage()); - } - } else { - log.info("MMAILJETT - registeredContextsJson is null or empty"); + Long userId = results.getLong("id"); + + try { + // Extract basic fields + String providerUserId = results.getString("provider_user_identifier"); + String email = results.getString("email"); + String roleStr = results.getString("role"); + Role role = Role.valueOf(roleStr); + String givenName = results.getString("given_name"); + boolean deleted = results.getBoolean("deleted"); + String emailVerificationStatusStr = results.getString("email_verification_status"); + EmailVerificationStatus emailVerificationStatus = EmailVerificationStatus.valueOf(emailVerificationStatusStr); + + // Handle nullable booleans from LEFT OUTER JOIN + Boolean newsEmails = (Boolean) results.getObject("news_emails"); + Boolean eventsEmails = (Boolean) results.getObject("events_emails"); + + // Parse registered_contexts array to extract stage + String stage = parseStageFromRegisteredContexts(results.getString("registered_contexts"), userId); + + return new UserExternalAccountChanges( + userId, + providerUserId, + email, + role, + givenName, + deleted, + emailVerificationStatus, + newsEmails, + eventsEmails, + stage + ); + + } catch (Exception e) { + log.error("MAILJET - Failed to build UserExternalAccountChanges for user {}: {}", userId, e.getMessage()); + throw new SQLException("Failed to parse user data for user " + userId, e); } + } - // Extract stage from the JSON object, or use a default value - String stage = (registeredContexts != null && registeredContexts.has("stage")) - ? registeredContexts.getString("stage") - : "unknown"; - log.info("MMAILJETT - Extracted stage: {}", stage); - - // Handle nullable booleans from LEFT OUTER JOIN - Boolean newsEmails = (Boolean) results.getObject("news_emails"); - log.info("MMAILJETT - newsEmails: {}", newsEmails); - - Boolean eventsEmails = (Boolean) results.getObject("events_emails"); - log.info("MMAILJETT - eventsEmails: {}", eventsEmails); - - Long id = results.getLong("id"); - log.info("MMAILJETT - id: {}", id); - - String providerUserId = results.getString("provider_user_identifier"); - log.info("MMAILJETT - provider_user_identifier: {}", providerUserId); - - String email = results.getString("email"); - log.info("MMAILJETT - email: {}", email); - - String roleStr = results.getString("role"); - log.info("MMAILJETT - role string: {}", roleStr); - Role role = Role.valueOf(roleStr); - log.info("MMAILJETT - role enum: {}", role); - - String givenName = results.getString("given_name"); - log.info("MMAILJETT - given_name: {}", givenName); - - boolean deleted = results.getBoolean("deleted"); - log.info("MMAILJETT - deleted: {}", deleted); + /** + * Parses the registered_contexts PostgreSQL array to extract stage information. + * The array contains JSON strings like: {"{\"stage\": \"gcse\"}","{\"stage\": \"a_level\"}"} + * or is empty: {} + * + * @param registeredContextsStr the raw string from the database + * @param userId the user ID for logging purposes + * @return a stage string: "gcse", "a_level", "gcse_and_a_level", or "not_specified" + */ + private String parseStageFromRegisteredContexts(String registeredContextsStr, Long userId) { + if (registeredContextsStr == null || registeredContextsStr.trim().isEmpty() || registeredContextsStr.equals("{}")) { + return "not_specified"; + } - String emailVerificationStatusStr = results.getString("email_verification_status"); - log.info("MMAILJETT - email_verification_status string: {}", emailVerificationStatusStr); - EmailVerificationStatus emailVerificationStatus = EmailVerificationStatus.valueOf(emailVerificationStatusStr); - log.info("MMAILJETT - email_verification_status enum: {}", emailVerificationStatus); + try { + // PostgreSQL array format: {"element1","element2"} + // Remove the outer braces and split by comma (handling quoted strings) + String cleaned = registeredContextsStr.trim(); + if (cleaned.startsWith("{") && cleaned.endsWith("}")) { + cleaned = cleaned.substring(1, cleaned.length() - 1); + } - log.info("MMAILJETT - buildUserExternalAccountChanges - Creating UserExternalAccountChanges object"); + boolean hasGcse = false; + boolean hasALevel = false; - UserExternalAccountChanges result = new UserExternalAccountChanges( - id, - providerUserId, - email, - role, - givenName, - deleted, - emailVerificationStatus, - newsEmails, - eventsEmails, - stage - ); + // Check if the string contains stage information + // The array elements are JSON strings like "{\"stage\": \"gcse\", \"examBoard\": \"all\"}" + if (cleaned.toLowerCase().contains("\"stage\"") || cleaned.toLowerCase().contains("stage")) { + // Simple pattern matching - more reliable than parsing complex nested structures + String lowerCleaned = cleaned.toLowerCase(); + hasGcse = lowerCleaned.contains("\"stage\": \"gcse\"") || lowerCleaned.contains("stage\": \"gcse"); + hasALevel = lowerCleaned.contains("\"stage\": \"a_level\"") || lowerCleaned.contains("stage\": \"a_level"); + } - log.info("MMAILJETT - buildUserExternalAccountChanges - END, returning object for user {}", id); + if (hasGcse && hasALevel) { + return "gcse_and_a_level"; + } else if (hasGcse) { + return "gcse"; + } else if (hasALevel) { + return "a_level"; + } else { + return "not_specified"; + } - return result; + } catch (Exception e) { + log.warn("MAILJET - Failed to parse registered_contexts for user {}: {}. Raw value: {}", + userId, e.getMessage(), registeredContextsStr); + return "not_specified"; + } } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java b/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java index 440b55d414..4a1a9e31c3 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java +++ b/src/main/java/uk/ac/cam/cl/dtg/util/email/MailJetApiClientWrapper.java @@ -35,7 +35,6 @@ import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - public class MailJetApiClientWrapper { private static final Logger log = LoggerFactory.getLogger(MailJetApiClientWrapper.class); @@ -58,17 +57,17 @@ public MailJetApiClientWrapper(final String mailjetApiKey, final String mailjetA final String mailjetNewsListId, final String mailjetEventsListId, final String mailjetLegalListId) { ClientOptions options = ClientOptions.builder() - .apiKey(mailjetApiKey) - .apiSecretKey(mailjetApiSecret) - .build(); + .apiKey(mailjetApiKey) + .apiSecretKey(mailjetApiSecret) + .build(); this.mailjetClient = new MailjetClient(options); this.newsListId = mailjetNewsListId; this.eventsListId = mailjetEventsListId; this.legalListId = mailjetLegalListId; - log.info("MMAILJETT - MailJetApiClientWrapper initialized with list IDs - News: {}, Events: {}, Legal: {}", - newsListId, eventsListId, legalListId); + log.info("MAILJET - MailJetApiClientWrapper initialized with list IDs - News: {}, Events: {}, Legal: {}", + newsListId, eventsListId, legalListId); } /** @@ -80,35 +79,35 @@ public MailJetApiClientWrapper(final String mailjetApiKey, final String mailjetA */ public JSONObject getAccountByIdOrEmail(final String mailjetIdOrEmail) throws MailjetException { if (null == mailjetIdOrEmail) { - log.warn("MMAILJETT - getAccountByIdOrEmail called with null mailjetIdOrEmail"); + log.warn("MAILJET - getAccountByIdOrEmail called with null mailjetIdOrEmail"); return null; } - log.debug("MMAILJETT - Getting Mailjet account for: {}", mailjetIdOrEmail); + log.debug("MAILJET - Getting Mailjet account for: {}", mailjetIdOrEmail); try { MailjetRequest request = new MailjetRequest(Contact.resource, mailjetIdOrEmail); MailjetResponse response = mailjetClient.get(request); - log.debug("MMAILJETT - Mailjet GET Contact response - Status: {}, Total: {}", - response.getStatus(), response.getTotal()); + log.debug("MAILJET - Mailjet GET Contact response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); JSONArray responseData = response.getData(); if (response.getTotal() == 1) { JSONObject result = responseData.getJSONObject(0); - log.debug("MMAILJETT - Found Mailjet contact: ID={}, Email={}", - result.optInt("ID"), result.optString("Email")); + log.debug("MAILJET - Found Mailjet contact: ID={}, Email={}", + result.optInt("ID"), result.optString("Email")); return result; } - log.warn("MMAILJETT - Mailjet account not found for: {}", mailjetIdOrEmail); + log.warn("MAILJET - Mailjet account not found for: {}", mailjetIdOrEmail); return null; } catch (MailjetException e) { - log.error("MMAILJETT - MailjetException in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); + log.error("MAILJET - MailjetException in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); throw e; } catch (Exception e) { - log.error("MMAILJETT - Unexpected exception in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); + log.error("MAILJET - Unexpected exception in getAccountByIdOrEmail for: {}", mailjetIdOrEmail, e); throw new MailjetException("Unexpected error getting account: " + e.getMessage(), e); } } @@ -122,25 +121,25 @@ public JSONObject getAccountByIdOrEmail(final String mailjetIdOrEmail) throws Ma public void permanentlyDeleteAccountById(final String mailjetId) throws MailjetException { requireNonNull(mailjetId); - log.info("MMAILJETT - Permanently deleting Mailjet account: {}", mailjetId); + log.info("MAILJET - Permanently deleting Mailjet account: {}", mailjetId); try { MailjetRequest request = new MailjetRequest(Contacts.resource, mailjetId); MailjetResponse response = mailjetClient.delete(request); - log.info("MMAILJETT - Mailjet DELETE response - Status: {}, mailjetId: {}", - response.getStatus(), mailjetId); + log.info("MAILJET - Mailjet DELETE response - Status: {}, mailjetId: {}", + response.getStatus(), mailjetId); if (response.getStatus() != 204 && response.getStatus() != 200) { - log.warn("MMAILJETT - Unexpected status code {} when deleting Mailjet account {}", - response.getStatus(), mailjetId); + log.warn("MAILJET - Unexpected status code {} when deleting Mailjet account {}", + response.getStatus(), mailjetId); } } catch (MailjetException e) { - log.error("MMAILJETT - MailjetException deleting account: {}", mailjetId, e); + log.error("MAILJET - MailjetException deleting account: {}", mailjetId, e); throw e; } catch (Exception e) { - log.error("MMAILJETT - Unexpected exception deleting account: {}", mailjetId, e); + log.error("MAILJET - Unexpected exception deleting account: {}", mailjetId, e); throw new MailjetException("Unexpected error deleting account: " + e.getMessage(), e); } } @@ -156,65 +155,65 @@ public void permanentlyDeleteAccountById(final String mailjetId) throws MailjetE */ public String addNewUserOrGetUserIfExists(final String email) throws MailjetException { if (null == email) { - log.error("MMAILJETT - addNewUserOrGetUserIfExists called with null email"); + log.error("MAILJET - addNewUserOrGetUserIfExists called with null email"); return null; } - log.info("MMAILJETT - Creating Mailjet contact for email: {}", sanitiseEmailForLogging(email)); + log.info("MAILJET - Creating Mailjet contact for email: {}", sanitiseEmailForLogging(email)); try { MailjetRequest request = new MailjetRequest(Contact.resource).property(Contact.EMAIL, email); MailjetResponse response = mailjetClient.post(request); - log.debug("MMAILJETT - Mailjet POST Contact response - Status: {}, Total: {}", - response.getStatus(), response.getTotal()); + log.debug("MAILJET - Mailjet POST Contact response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); // Get MailJet ID out: JSONObject responseData = response.getData().getJSONObject(0); String mailjetId = Integer.toString(responseData.getInt("ID")); - log.info("MMAILJETT - Successfully created Mailjet contact with ID: {} for email: {}", - mailjetId, sanitiseEmailForLogging(email)); + log.info("MAILJET - Successfully created Mailjet contact with ID: {} for email: {}", + mailjetId, sanitiseEmailForLogging(email)); return mailjetId; } catch (MailjetClientRequestException e) { - log.warn("MMAILJETT - MailjetClientRequestException creating contact for email: {} - Message: {}", - sanitiseEmailForLogging(email), e.getMessage()); + log.warn("MAILJET - MailjetClientRequestException creating contact for email: {} - Message: {}", + sanitiseEmailForLogging(email), e.getMessage()); if (e.getMessage().contains("already exists")) { - log.info("MMAILJETT - Contact already exists, attempting to retrieve existing contact for: {}", - sanitiseEmailForLogging(email)); + log.info("MAILJET - Contact already exists, attempting to retrieve existing contact for: {}", + sanitiseEmailForLogging(email)); try { JSONObject existingMailJetAccount = getAccountByIdOrEmail(email); if (existingMailJetAccount != null) { String existingId = Integer.toString(existingMailJetAccount.getInt("ID")); - log.info("MMAILJETT - Found existing Mailjet contact with ID: {} for email: {}", - existingId, sanitiseEmailForLogging(email)); + log.info("MAILJET - Found existing Mailjet contact with ID: {} for email: {}", + existingId, sanitiseEmailForLogging(email)); return existingId; } else { - log.error("MMAILJETT - Contact 'already exists' but getAccountByIdOrEmail returned null for: {}", - sanitiseEmailForLogging(email)); + log.error("MAILJET - Contact 'already exists' but getAccountByIdOrEmail returned null for: {}", + sanitiseEmailForLogging(email)); throw new MailjetException("Contact exists but could not be retrieved"); } } catch (JSONException jsonEx) { - log.error("MMAILJETT - JSONException retrieving existing contact for: {}", - sanitiseEmailForLogging(email), jsonEx); + log.error("MAILJET - JSONException retrieving existing contact for: {}", + sanitiseEmailForLogging(email), jsonEx); throw new MailjetException("Failed to parse existing contact data", jsonEx); } } else { - log.error("MMAILJETT - Failed to create user in MailJet with email: {} - Error: {}", - sanitiseEmailForLogging(email), e.getMessage(), e); + log.error("MAILJET - Failed to create user in MailJet with email: {} - Error: {}", + sanitiseEmailForLogging(email), e.getMessage(), e); throw new MailjetException("Failed to create contact: " + e.getMessage(), e); } } catch (JSONException e) { - log.error("MMAILJETT - JSONException creating user in MailJet with email: {}", - sanitiseEmailForLogging(email), e); + log.error("MAILJET - JSONException creating user in MailJet with email: {}", + sanitiseEmailForLogging(email), e); throw new MailjetException("Failed to parse Mailjet response", e); } catch (Exception e) { - log.error("MMAILJETT - Unexpected exception creating user in MailJet with email: {}", - sanitiseEmailForLogging(email), e); + log.error("MAILJET - Unexpected exception creating user in MailJet with email: {}", + sanitiseEmailForLogging(email), e); throw new MailjetException("Unexpected error creating contact: " + e.getMessage(), e); } } @@ -233,45 +232,45 @@ public void updateUserProperties(final String mailjetId, final String firstName, final String emailVerificationStatus, String stage) throws MailjetException { requireNonNull(mailjetId); - log.info("MMAILJETT - Updating properties for Mailjet ID: {}", mailjetId); - log.debug("MMAILJETT - Properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", - firstName, role, emailVerificationStatus, stage); + log.info("MAILJET - Updating properties for Mailjet ID: {}", mailjetId); + log.debug("MAILJET - Properties - firstName: {}, role: {}, verificationStatus: {}, stage: {}", + firstName, role, emailVerificationStatus, stage); try { JSONArray propertiesArray = new JSONArray() - .put(new JSONObject().put("Name", "firstname").put("value", firstName)) - .put(new JSONObject().put("Name", "role").put("value", role)) - .put(new JSONObject().put("Name", "verification_status").put("value", emailVerificationStatus)) - .put(new JSONObject().put("Name", "stage").put("value", stage)); + .put(new JSONObject().put("Name", "firstname").put("value", firstName)) + .put(new JSONObject().put("Name", "role").put("value", role)) + .put(new JSONObject().put("Name", "verification_status").put("value", emailVerificationStatus)) + .put(new JSONObject().put("Name", "stage").put("value", stage)); - log.debug("MMAILJETT - Property JSON array: {}", propertiesArray.toString()); + log.debug("MAILJET - Property JSON array: {}", propertiesArray.toString()); MailjetRequest request = new MailjetRequest(Contactdata.resource, mailjetId) - .property(Contactdata.DATA, propertiesArray); + .property(Contactdata.DATA, propertiesArray); MailjetResponse response = mailjetClient.put(request); - log.debug("MMAILJETT - Mailjet PUT Contactdata response - Status: {}, Total: {}", - response.getStatus(), response.getTotal()); + log.debug("MAILJET - Mailjet PUT Contactdata response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); if (response.getTotal() != 1) { - log.error("MMAILJETT - Failed to update user properties! Expected 1, got {} for Mailjet ID: {}", - response.getTotal(), mailjetId); - log.error("MMAILJETT - Response status: {}, Response data: {}", - response.getStatus(), response.getData().toString()); + log.error("MAILJET - Failed to update user properties! Expected 1, got {} for Mailjet ID: {}", + response.getTotal(), mailjetId); + log.error("MAILJET - Response status: {}, Response data: {}", + response.getStatus(), response.getData().toString()); throw new MailjetException("Failed to update user! Response total: " + response.getTotal()); } - log.info("MMAILJETT - Successfully updated properties for Mailjet ID: {}", mailjetId); + log.info("MAILJET - Successfully updated properties for Mailjet ID: {}", mailjetId); } catch (MailjetException e) { - log.error("MMAILJETT - MailjetException updating properties for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - MailjetException updating properties for Mailjet ID: {}", mailjetId, e); throw e; } catch (JSONException e) { - log.error("MMAILJETT - JSONException creating property data for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - JSONException creating property data for Mailjet ID: {}", mailjetId, e); throw new MailjetException("Failed to create property JSON", e); } catch (Exception e) { - log.error("MMAILJETT - Unexpected exception updating properties for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - Unexpected exception updating properties for Mailjet ID: {}", mailjetId, e); throw new MailjetException("Unexpected error updating properties: " + e.getMessage(), e); } } @@ -288,50 +287,50 @@ public void updateUserSubscriptions(final String mailjetId, final MailJetSubscri final MailJetSubscriptionAction eventsEmails) throws MailjetException { requireNonNull(mailjetId); - log.info("MMAILJETT - Updating subscriptions for Mailjet ID: {}", mailjetId); - log.debug("MMAILJETT - Subscriptions - news: {}, events: {}", newsEmails, eventsEmails); - log.debug("MMAILJETT - Using list IDs - Legal: {}, News: {}, Events: {}", legalListId, newsListId, eventsListId); + log.info("MAILJET - Updating subscriptions for Mailjet ID: {}", mailjetId); + log.debug("MAILJET - Subscriptions - news: {}, events: {}", newsEmails, eventsEmails); + log.debug("MAILJET - Using list IDs - Legal: {}, News: {}, Events: {}", legalListId, newsListId, eventsListId); try { JSONArray subscriptionsArray = new JSONArray() - .put(new JSONObject() - .put(ContactslistImportList.LISTID, legalListId) - .put(ContactslistImportList.ACTION, MailJetSubscriptionAction.FORCE_SUBSCRIBE.getValue())) - .put(new JSONObject() - .put(ContactslistImportList.LISTID, newsListId) - .put(ContactslistImportList.ACTION, newsEmails.getValue())) - .put(new JSONObject() - .put(ContactslistImportList.LISTID, eventsListId) - .put(ContactslistImportList.ACTION, eventsEmails.getValue())); - - log.debug("MMAILJETT - Subscription JSON array: {}", subscriptionsArray.toString()); + .put(new JSONObject() + .put(ContactslistImportList.LISTID, legalListId) + .put(ContactslistImportList.ACTION, MailJetSubscriptionAction.FORCE_SUBSCRIBE.getValue())) + .put(new JSONObject() + .put(ContactslistImportList.LISTID, newsListId) + .put(ContactslistImportList.ACTION, newsEmails.getValue())) + .put(new JSONObject() + .put(ContactslistImportList.LISTID, eventsListId) + .put(ContactslistImportList.ACTION, eventsEmails.getValue())); + + log.debug("MAILJET - Subscription JSON array: {}", subscriptionsArray.toString()); MailjetRequest request = new MailjetRequest(ContactManagecontactslists.resource, mailjetId) - .property(ContactManagecontactslists.CONTACTSLISTS, subscriptionsArray); + .property(ContactManagecontactslists.CONTACTSLISTS, subscriptionsArray); MailjetResponse response = mailjetClient.post(request); - log.debug("MMAILJETT - Mailjet POST ContactManagecontactslists response - Status: {}, Total: {}", - response.getStatus(), response.getTotal()); + log.debug("MAILJET - Mailjet POST ContactManagecontactslists response - Status: {}, Total: {}", + response.getStatus(), response.getTotal()); if (response.getTotal() != 1) { - log.error("MMAILJETT - Failed to update user subscriptions! Expected 1, got {} for Mailjet ID: {}", - response.getTotal(), mailjetId); - log.error("MMAILJETT - Response status: {}, Response data: {}", - response.getStatus(), response.getData().toString()); + log.error("MAILJET - Failed to update user subscriptions! Expected 1, got {} for Mailjet ID: {}", + response.getTotal(), mailjetId); + log.error("MAILJET - Response status: {}, Response data: {}", + response.getStatus(), response.getData().toString()); throw new MailjetException("Failed to update user subscriptions! Response total: " + response.getTotal()); } - log.info("MMAILJETT - Successfully updated subscriptions for Mailjet ID: {}", mailjetId); + log.info("MAILJET - Successfully updated subscriptions for Mailjet ID: {}", mailjetId); } catch (MailjetException e) { - log.error("MMAILJETT - MailjetException updating subscriptions for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - MailjetException updating subscriptions for Mailjet ID: {}", mailjetId, e); throw e; } catch (JSONException e) { - log.error("MMAILJETT - JSONException creating subscription data for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - JSONException creating subscription data for Mailjet ID: {}", mailjetId, e); throw new MailjetException("Failed to create subscription JSON", e); } catch (Exception e) { - log.error("MMAILJETT - Unexpected exception updating subscriptions for Mailjet ID: {}", mailjetId, e); + log.error("MAILJET - Unexpected exception updating subscriptions for Mailjet ID: {}", mailjetId, e); throw new MailjetException("Unexpected error updating subscriptions: " + e.getMessage(), e); } } From abac3078d282aba4a47ea291508ca55b74c0ee4b Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 17 Dec 2025 21:56:39 +0200 Subject: [PATCH 8/8] PATCH 7 --- .../api/managers/ExternalAccountManager.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java index 7337823ba6..049a3bd194 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/ExternalAccountManager.java @@ -33,6 +33,7 @@ import uk.ac.cam.cl.dtg.segue.dao.users.IExternalAccountDataManager; import uk.ac.cam.cl.dtg.util.email.MailJetApiClientWrapper; import uk.ac.cam.cl.dtg.util.email.MailJetSubscriptionAction; + public class ExternalAccountManager implements IExternalAccountManager { private static final Logger log = LoggerFactory.getLogger(ExternalAccountManager.class); @@ -61,6 +62,7 @@ public ExternalAccountManager(final MailJetApiClientWrapper mailjetApi, final IE */ @Override public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchronisationException { + long startTime = System.currentTimeMillis(); log.info("MAILJET - === Starting Mailjet user synchronisation ==="); int totalUsersProcessed = 0; int successfulSyncs = 0; @@ -76,6 +78,27 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro Long userId = userRecord.getUserId(); String accountEmail = userRecord.getAccountEmail(); + // Rate limiting: 2 second delay between each user + if (totalUsersProcessed > 1) { + try { + Thread.sleep(2000); // 2 seconds between users + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + log.warn("MAILJET - Thread interrupted during rate limit delay"); + } + } + + // Additional delay every 100 users to avoid rate limits + if (totalUsersProcessed % 100 == 0) { + try { + log.info("MAILJET - Processed {} users, pausing 10 seconds to avoid rate limits", totalUsersProcessed); + Thread.sleep(10000); // 10 seconds every 100 users + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + log.warn("MAILJET - Thread interrupted during extended rate limit delay"); + } + } + log.debug("MAILJET - [User {}] Starting sync for email: {}", userId, sanitiseEmailForLogging(accountEmail)); log.debug("MAILJET - [User {}] Details: deleted={}, emailStatus={}, mailjetId={}, allowsNews={}, allowsEvents={}, role={}, stage={}", userId, @@ -138,6 +161,9 @@ public synchronized void synchroniseChangedUsers() throws ExternalAccountSynchro log.info("MAILJET - Successful syncs: {}", successfulSyncs); log.info("MAILJET - Failed syncs: {}", failedSyncs); log.info("MAILJET - Skipped users: {}", skippedUsers); + + long duration = (System.currentTimeMillis() - startTime) / 1000; + log.info("MAILJET - Sync duration: {} seconds ({} minutes)", duration, duration / 60); } /**