diff --git a/CHANGELOG.md b/CHANGELOG.md index cd4835e4a87..ddfcbfecd4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.10 - 2023-11-16 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.9...v2.44.10) + +- [#6932](https://github.com/ORCID/ORCID-Source/pull/6932): Just one query + ## v2.44.9 - 2023-11-14 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.8...v2.44.9) diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java index 09bf55902b6..9984f40bf9e 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java @@ -76,9 +76,7 @@ public interface ProfileDao extends GenericDao { void updateLastModifiedDateAndIndexingStatusWithoutResult(String orcid, Date lastModified, IndexingStatus indexingStatus); - public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults); - - public List> findEmailsUnverifiedDaysByEventType(int daysUnverified, int tooOldNumberOfDays); + public List> findEmailsUnverfiedDays(int daysUnverified, EmailEventType eventSent); String retrieveOrcidType(String orcid); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java index d916513f83a..88a08c3b939 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java @@ -13,16 +13,16 @@ import org.apache.commons.lang3.tuple.Triple; import org.orcid.persistence.aop.UpdateProfileLastModifiedAndIndexingStatus; import org.orcid.persistence.dao.ProfileDao; +import org.orcid.persistence.jpa.entities.EmailEventType; import org.orcid.persistence.jpa.entities.IndexingStatus; import org.orcid.persistence.jpa.entities.OrcidGrantedAuthority; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.persistence.jpa.entities.ProfileEventEntity; import org.orcid.persistence.jpa.entities.ProfileEventType; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.transaction.annotation.Transactional; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.transaction.annotation.Transactional; public class ProfileDaoImpl extends GenericDaoImpl implements ProfileDao { @@ -30,6 +30,18 @@ public class ProfileDaoImpl extends GenericDaoImpl implem private static final Logger LOGGER = LoggerFactory.getLogger(ProfileDaoImpl.class); + private static final String FIND_EMAILS_TO_SEND_VERIFICATION_REMINTER = "SELECT e.orcid, e.email, e.is_primary " + + "FROM email e, profile p " + + "WHERE e.is_verified = false " + + " AND p.orcid = e.orcid " + + " AND p.deprecated_date is null " + + " AND p.profile_deactivation_date is null " + + " AND p.account_expiry is null " + + " AND e.date_created BETWEEN (DATE_TRUNC('day', now()) - CAST('{RANGE_START}' AS INTERVAL DAY)) AND (DATE_TRUNC('day', now()) - CAST('{RANGE_END}' AS INTERVAL DAY)) " + + " AND NOT EXISTS " + + " (SELECT x.email FROM email_event x WHERE x.email = e.email AND email_event_type IN ('{EVENT_SENT}')) " + + "order by e.last_modified"; + @Value("${org.orcid.postgres.query.timeout:30000}") private Integer queryTimeout; @@ -136,47 +148,20 @@ public List findUnclaimedNotIndexedAfterWaitPeriod(int waitPeriodDays, i @SuppressWarnings("unchecked") @Override - public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults) { - StringBuilder queryString = new StringBuilder("SELECT e.email, e.is_primary, e.date_created FROM email e "); - queryString.append("LEFT JOIN email_event ev ON e.email = ev.email "); - queryString.append("JOIN profile p on p.orcid = e.orcid and p.claimed = true "); - queryString.append("AND p.deprecated_date is null AND p.profile_deactivation_date is null AND p.account_expiry is null "); - queryString.append("where ev.email IS NULL " + "and e.is_verified = false "); - queryString.append("and e.date_created < (now() - CAST('").append(daysUnverified).append("' AS INTERVAL DAY)) "); - queryString.append("and (e.source_id = e.orcid OR e.source_id is null)"); - queryString.append(" ORDER BY e.last_modified"); - - Query query = entityManager.createNativeQuery(queryString.toString()); - query.setMaxResults(maxResults); - List dbInfo = query.getResultList(); - List> results = new ArrayList>(); - dbInfo.stream().forEach(element -> { - Triple pair = Triple.of((String) element[0], (Boolean) element[1], (Date) element[2]); - results.add(pair); - }); - return results; - } - - @SuppressWarnings("unchecked") - @Override - public List> findEmailsUnverifiedDaysByEventType(int daysUnverified, int tooOldNumberOfDays) { - StringBuilder queryString = new StringBuilder("SELECT e.email, e.is_primary, ev.email_event_type FROM email e "); - queryString.append("LEFT JOIN email_event ev ON e.email = ev.email "); - queryString.append("JOIN profile p on p.orcid = e.orcid and p.claimed = true "); - queryString.append("AND p.deprecated_date is null AND p.profile_deactivation_date is null AND p.account_expiry is null "); - queryString.append("where e.is_verified = false "); - queryString.append("and e.date_created between (now() - CAST('").append(tooOldNumberOfDays).append("' AS INTERVAL DAY)) and (now() - CAST('") - .append(daysUnverified).append("' AS INTERVAL DAY)) "); - queryString.append("and e.date_created < (now() - CAST('").append(daysUnverified).append("' AS INTERVAL DAY)) "); - queryString.append("and (e.source_id = e.orcid OR e.source_id is null)"); - queryString.append(" ORDER BY e.last_modified"); + public List> findEmailsUnverfiedDays(int daysUnverified, EmailEventType eventSent) { + int rangeStart = daysUnverified; + int rangeEnd = daysUnverified - 1; + String queryString = FIND_EMAILS_TO_SEND_VERIFICATION_REMINTER.replace("{RANGE_START}", String.valueOf(rangeStart)) + .replace("{RANGE_END}", String.valueOf(rangeEnd)).replace("{EVENT_SENT}", eventSent.name()); + LOGGER.debug("Query to search unverified emails"); + LOGGER.debug(queryString); Query query = entityManager.createNativeQuery(queryString.toString()); List dbInfo = query.getResultList(); - List> results = new ArrayList>(); + List> results = new ArrayList>(); dbInfo.stream().forEach(element -> { - Triple pair = Triple.of((String) element[0], (Boolean) element[1], (String) element[2]); - results.add(pair); + Triple q = Triple.of((String) element[0], (String) element[1], (Boolean) element[2]); + results.add(q); }); return results; } diff --git a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java index 96e6551b8e9..2d14ac9c1cc 100644 --- a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java +++ b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java @@ -5,7 +5,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.Arrays; import java.util.Calendar; @@ -376,84 +375,5 @@ public void testDisable2FA() { profileDao.disable2FA("2000-0000-0000-0002"); profile = profileDao.find("2000-0000-0000-0002"); assertFalse(profile.getUsing2FA()); - } - - @Test - @Transactional - @Rollback(true) - public void findEmailsUnverfiedDaysTest() throws IllegalAccessException { - String orcid = "9999-9999-9999-999X"; - ProfileEntity profile = new ProfileEntity(); - profile.setId(orcid); - profile.setClaimed(true); - profileDao.persist(profile); - profileDao.flush(); - emailDao.removeAll(); - - // Created today - assertEquals(1, insertEmailWithDateCreated("unverified_1@test.orcid.org", "bd22086b65b6259fe79f7844a6b6a369441733b9ef04eff762f3d640957b78f5", orcid, false, new Date())); - - // Created a week ago - assertEquals(1, insertEmailWithDateCreated("unverified_2@test.orcid.org", "95770578974f683fb05c179a84f57c3fc7d4b260f8079fbc590080e51873bb67", orcid, false, LocalDateTime.now().minusDays(7).toDate())); - - // Created 15 days ago - assertEquals(1, insertEmailWithDateCreated("unverified_3@test.orcid.org", "3cbebfc1de2500494fc95553c956e757cb1998149d366afb71888cdeb1550719", orcid, false, LocalDateTime.now().minusDays(15).toDate())); - - // Created 7 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_1@test.orcid.org", "2f4812b9c675e9803a4bb616dd1bc241c8c9302ba5690a1ea9d48049a32e7c5f", orcid, true, LocalDateTime.now().minusDays(7).toDate())); - - // Created 15 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_2@test.orcid.org", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", orcid, true, LocalDateTime.now().minusDays(15).toDate())); - - List> results = profileDao.findEmailsUnverfiedDays(7, 100); - assertNotNull(results); - assertEquals(2, results.size()); - - boolean found1 = false, found2 = false; - - for(Triple element : results) { - assertNotNull(element.getRight()); - if(element.getLeft().equals("unverified_2@test.orcid.org")) { - found1 = true; - } else if(element.getLeft().equals("unverified_3@test.orcid.org")) { - found2 = true; - } else { - fail("Unexpected email id: " + element.getRight()); - } - } - - assertTrue(found1); - assertTrue(found2); - - // Put an email event on 'unverified_2@test.orcid.org' and verify there is only one result - emailEventDao.persist(new EmailEventEntity("unverified_2@test.orcid.org", EmailEventType.VERIFY_EMAIL_7_DAYS_SENT)); - - results = profileDao.findEmailsUnverfiedDays(7, 100); - assertNotNull(results); - assertEquals(1, results.size()); - assertEquals("unverified_3@test.orcid.org", results.get(0).getLeft()); - - // Put an email event on 'unverified_3@test.orcid.org' and verify there is no result anymore - emailEventDao.persist(new EmailEventEntity("unverified_3@test.orcid.org", EmailEventType.VERIFY_EMAIL_TOO_OLD)); - results = profileDao.findEmailsUnverfiedDays(7, 100); - assertNotNull(results); - assertTrue(results.isEmpty()); - } - - private int insertEmailWithDateCreated(String email, String emailHash, String orcid, boolean isVerified, Date dateCreated) { - Query q = entityManager.createNativeQuery( - "INSERT INTO email(email,email_hash,orcid,source_id,visibility,is_primary,is_current,is_verified,date_created,last_modified) " - + "values(:email, :emailHash, :orcid, :sourceId, :visibility, :isPrimary, :isCurrent, :isVerified, :dateCreated, :lastModified)"); - q.setParameter("email", email); - q.setParameter("emailHash", emailHash); - q.setParameter("orcid", orcid); - q.setParameter("sourceId", orcid); - q.setParameter("visibility", "PUBLIC"); - q.setParameter("isPrimary", false); - q.setParameter("isCurrent", false); - q.setParameter("isVerified", isVerified); - q.setParameter("dateCreated", dateCreated); - q.setParameter("lastModified", dateCreated); - return q.executeUpdate(); - } + } } diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java index d69ee216fa3..d25758a8e45 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java @@ -25,7 +25,6 @@ import org.apache.commons.lang3.LocaleUtils; import org.apache.commons.lang3.time.DurationFormatUtils; import org.apache.commons.lang3.tuple.Triple; -import org.joda.time.LocalDateTime; import org.orcid.core.constants.EmailConstants; import org.orcid.core.locale.LocaleManager; import org.orcid.core.manager.EmailMessage; @@ -70,8 +69,6 @@ import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionTemplate; -import liquibase.repackaged.org.apache.commons.lang3.StringUtils; - /** * * @author Will Simpson @@ -572,84 +569,38 @@ private String getHtmlBody(NotificationAdministrative notificationAdministrative @Override synchronized public void processUnverifiedEmails2Days() { - LOGGER.info("About to process unverIfied emails for 2 days reminder"); - List> elements = Collections.> emptyList(); - do { - elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterTwoDays, 100); - LOGGER.info("Got batch of {} profiles with unverified emails for 2 days reminder", elements.size()); - LocalDateTime now = LocalDateTime.now(); - // togglz here - Date tooOld = now.minusDays(emailTooOldLegacy).toDate(); - if (Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { - tooOld = now.minusDays(emailTooOld).toDate(); - } - for (Triple element : elements) { - if (element.getRight() == null || element.getRight().after(tooOld)) { - processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_2_DAYS_SENT, - EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED); - } else { - // Mark is as too old to send the verification email - markUnverifiedEmailAsTooOld(element.getLeft()); - } - } - } while (!elements.isEmpty()); + processUnverifiedEmails(verifyReminderAfterTwoDays, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED); } synchronized public void processUnverifiedEmails7Days() { - if (Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { - LOGGER.info("About to process unverIfied emails for 7 days reminder"); - List> elements = Collections.> emptyList(); - - elements = profileDaoReadOnly.findEmailsUnverifiedDaysByEventType(verifyReminderAfterSevenDays, emailTooOld); - LOGGER.info("Got {} profiles with email event and unverified emails for 7 days reminder", elements.size()); - - HashMap> hasEventTypesMap = hasEventTypes(elements, - List.of(EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED, EmailEventType.VERIFY_EMAIL_TOO_OLD)); - for (Triple element : elements) { - if (!hasEventTypesMap.containsKey(element.getLeft())) - processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, - EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED); - hasEventTypesMap.put(element.getLeft(), element); - } - } - } - - // helper method to get the triple map for an event type list - private HashMap> hasEventTypes(List> elements, List eventTypes) { - HashMap> hasEventTypesMap = new HashMap>(); - for (Triple element : elements) { - if (eventTypes.contains(EmailEventType.valueOf(element.getRight()))) { - hasEventTypesMap.put(element.getLeft(), element); - } - } - return hasEventTypesMap; + processUnverifiedEmails(verifyReminderAfterSevenDays, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED); } synchronized public void processUnverifiedEmails28Days() { + processUnverifiedEmails(verifyReminderAfterTwentyEightDays, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED); + } + + private void processUnverifiedEmails(int unverifiedDays, EmailEventType sent, EmailEventType failed) { if (Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { - LOGGER.info("About to process unverIfied emails for 28 days reminder"); - List> elements = Collections.> emptyList(); - elements = profileDaoReadOnly.findEmailsUnverifiedDaysByEventType(verifyReminderAfterTwentyEightDays, emailTooOld); - LOGGER.info("Got {} profiles with email event and unverified emails for 28 days reminder", elements.size()); - - HashMap> hasEventTypesMap = hasEventTypes(elements, - List.of(EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED, EmailEventType.VERIFY_EMAIL_TOO_OLD)); - for (Triple element : elements) { - if (!hasEventTypesMap.containsKey(element.getLeft())) - processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, - EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED); - hasEventTypesMap.put(element.getLeft(), element); + LOGGER.info("About to process unverIfied emails for {} days reminder", unverifiedDays); + List> elements = Collections.> emptyList(); + elements = profileDaoReadOnly.findEmailsUnverfiedDays(unverifiedDays, sent); + LOGGER.info("Got {} profiles with email event and unverified emails for {} days reminder", elements.size(), unverifiedDays); + + for (Triple element : elements) { + processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), element.getRight(), sent, + failed); } } } - private void processUnverifiedEmailsInTransaction(final String email, final Boolean isPrimaryEmail, EmailEventType eventSent, EmailEventType eventSkipped) { + private void processUnverifiedEmailsInTransaction(final String userOrcid, final String email, final Boolean isPrimaryEmail, EmailEventType eventSent, EmailEventType eventSkipped) { transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override @Transactional protected void doInTransactionWithoutResult(TransactionStatus status) { try { - String userOrcid = emailManagerReadOnly.findOrcidIdByEmail(email); + LOGGER.debug("Sending reminder {} to email address {}, orcid {}", eventSent, email, userOrcid); sendVerificationReminderEmail(userOrcid, email, isPrimaryEmail); emailEventDao.persist(new EmailEventEntity(email, eventSent)); emailEventDao.flush(); @@ -662,17 +613,6 @@ protected void doInTransactionWithoutResult(TransactionStatus status) { }); } - private void markUnverifiedEmailAsTooOld(final String email) { - transactionTemplate.execute(new TransactionCallbackWithoutResult() { - @Override - @Transactional - protected void doInTransactionWithoutResult(TransactionStatus status) { - emailEventDao.persist(new EmailEventEntity(email, EmailEventType.VERIFY_EMAIL_TOO_OLD)); - emailEventDao.flush(); - } - }); - } - private void sendVerificationReminderEmail(String userOrcid, String email, Boolean isPrimaryEmail) { ProfileEntity profile = profileEntityCacheManager.retrieve(userOrcid); Locale locale = getUserLocaleFromProfileEntity(profile);