Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Just one query #6932

Merged
merged 4 commits into from
Nov 16, 2023
Merged

Just one query #6932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e375684
Do a single query to extract non verified emails that have not been n…
amontenegro Nov 16, 2023
f3fe4fc
Query # 1
amontenegro Nov 16, 2023
28879e7
Looks like it is working now
amontenegro Nov 16, 2023
5ba8f2a
date_trunc function is not available in the in memory db, so, we cann…
amontenegro Nov 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ public interface ProfileDao extends GenericDao<ProfileEntity, String> {

void updateLastModifiedDateAndIndexingStatusWithoutResult(String orcid, Date lastModified, IndexingStatus indexingStatus);

public List<Triple<String, Boolean, Date>> findEmailsUnverfiedDays(int daysUnverified, int maxResults);

public List<Triple<String, Boolean, String>> findEmailsUnverifiedDaysByEventType(int daysUnverified, int tooOldNumberOfDays);
public List<Triple<String, String, Boolean>> findEmailsUnverfiedDays(int daysUnverified, EmailEventType eventSent);

String retrieveOrcidType(String orcid);

Expand Down
65 changes: 25 additions & 40 deletions orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,35 @@
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<ProfileEntity, String> implements ProfileDao {

private static final String PRIVATE_VISIBILITY = "PRIVATE";

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;

Expand Down Expand Up @@ -136,47 +148,20 @@ public List<String> findUnclaimedNotIndexedAfterWaitPeriod(int waitPeriodDays, i

@SuppressWarnings("unchecked")
@Override
public List<Triple<String, Boolean, Date>> 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<Object[]> dbInfo = query.getResultList();
List<Triple<String, Boolean, Date>> results = new ArrayList<Triple<String, Boolean, Date>>();
dbInfo.stream().forEach(element -> {
Triple<String, Boolean, Date> pair = Triple.of((String) element[0], (Boolean) element[1], (Date) element[2]);
results.add(pair);
});
return results;
}

@SuppressWarnings("unchecked")
@Override
public List<Triple<String, Boolean, String>> 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<Triple<String, String, Boolean>> 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<Object[]> dbInfo = query.getResultList();
List<Triple<String, Boolean, String>> results = new ArrayList<Triple<String, Boolean, String>>();
List<Triple<String, String, Boolean>> results = new ArrayList<Triple<String, String, Boolean>>();
dbInfo.stream().forEach(element -> {
Triple<String, Boolean, String> pair = Triple.of((String) element[0], (Boolean) element[1], (String) element[2]);
results.add(pair);
Triple<String, String, Boolean> q = Triple.of((String) element[0], (String) element[1], (Boolean) element[2]);
results.add(q);
});
return results;
}
Expand Down
82 changes: 1 addition & 81 deletions orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("[email protected]", "bd22086b65b6259fe79f7844a6b6a369441733b9ef04eff762f3d640957b78f5", orcid, false, new Date()));

// Created a week ago
assertEquals(1, insertEmailWithDateCreated("[email protected]", "95770578974f683fb05c179a84f57c3fc7d4b260f8079fbc590080e51873bb67", orcid, false, LocalDateTime.now().minusDays(7).toDate()));

// Created 15 days ago
assertEquals(1, insertEmailWithDateCreated("[email protected]", "3cbebfc1de2500494fc95553c956e757cb1998149d366afb71888cdeb1550719", orcid, false, LocalDateTime.now().minusDays(15).toDate()));

// Created 7 days ago and verified
assertEquals(1, insertEmailWithDateCreated("[email protected]", "2f4812b9c675e9803a4bb616dd1bc241c8c9302ba5690a1ea9d48049a32e7c5f", orcid, true, LocalDateTime.now().minusDays(7).toDate()));

// Created 15 days ago and verified
assertEquals(1, insertEmailWithDateCreated("[email protected]", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", orcid, true, LocalDateTime.now().minusDays(15).toDate()));

List<Triple<String, Boolean, Date>> results = profileDao.findEmailsUnverfiedDays(7, 100);
assertNotNull(results);
assertEquals(2, results.size());

boolean found1 = false, found2 = false;

for(Triple<String, Boolean, Date> element : results) {
assertNotNull(element.getRight());
if(element.getLeft().equals("[email protected]")) {
found1 = true;
} else if(element.getLeft().equals("[email protected]")) {
found2 = true;
} else {
fail("Unexpected email id: " + element.getRight());
}
}

assertTrue(found1);
assertTrue(found2);

// Put an email event on '[email protected]' and verify there is only one result
emailEventDao.persist(new EmailEventEntity("[email protected]", EmailEventType.VERIFY_EMAIL_7_DAYS_SENT));

results = profileDao.findEmailsUnverfiedDays(7, 100);
assertNotNull(results);
assertEquals(1, results.size());
assertEquals("[email protected]", results.get(0).getLeft());

// Put an email event on '[email protected]' and verify there is no result anymore
emailEventDao.persist(new EmailEventEntity("[email protected]", 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();
}
}
}
92 changes: 16 additions & 76 deletions ...duler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<Triple<String, Boolean, Date>> elements = Collections.<Triple<String, Boolean, Date>> 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<String, Boolean, Date> 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<Triple<String, Boolean, String>> elements = Collections.<Triple<String, Boolean, String>> emptyList();

elements = profileDaoReadOnly.findEmailsUnverifiedDaysByEventType(verifyReminderAfterSevenDays, emailTooOld);
LOGGER.info("Got {} profiles with email event and unverified emails for 7 days reminder", elements.size());

HashMap<String, Triple<String, Boolean, String>> 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<String, Boolean, String> 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<String, Triple<String, Boolean, String>> hasEventTypes(List<Triple<String, Boolean, String>> elements, List<EmailEventType> eventTypes) {
HashMap<String, Triple<String, Boolean, String>> hasEventTypesMap = new HashMap<String, Triple<String, Boolean, String>>();
for (Triple<String, Boolean, String> 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<Triple<String, Boolean, String>> elements = Collections.<Triple<String, Boolean, String>> emptyList();
elements = profileDaoReadOnly.findEmailsUnverifiedDaysByEventType(verifyReminderAfterTwentyEightDays, emailTooOld);
LOGGER.info("Got {} profiles with email event and unverified emails for 28 days reminder", elements.size());

HashMap<String, Triple<String, Boolean, String>> 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<String, Boolean, String> 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<Triple<String, String, Boolean>> elements = Collections.<Triple<String, String, Boolean>> emptyList();
elements = profileDaoReadOnly.findEmailsUnverfiedDays(unverifiedDays, sent);
LOGGER.info("Got {} profiles with email event and unverified emails for {} days reminder", elements.size(), unverifiedDays);

for (Triple<String, String, Boolean> 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();
Expand All @@ -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);
Expand Down