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.

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

8875 update the email verification schedule to send emails 2 7 and 28 days after email creation #6915

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion orcid-core/src/main/java/org/orcid/core/togglz/Features.java
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ public enum Features implements Feature {
ACCOUNT_LOCKOUT_SIMULATION,

@Label("Enable the new 100M ID's range")
ENABLE_NEW_IDS;
ENABLE_NEW_IDS,

@Label("Send verification emails for 2, 7 and 28 days. If disabled 2 days only verification emails will be sent.")
SEND_ALL_VERIFICATION_EMAILS;

public boolean isActive() {
return FeatureContext.getFeatureManager().isActive(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.List;

import org.apache.commons.lang3.tuple.Pair;
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;
Expand Down Expand Up @@ -76,6 +77,8 @@ public interface ProfileDao extends GenericDao<ProfileEntity, String> {
void updateLastModifiedDateAndIndexingStatusWithoutResult(String orcid, Date lastModified, IndexingStatus indexingStatus);

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

public List<Pair<String, Date>> findEmailsUnverifiedDaysAndEventType(int daysUnverified, int maxResults, List<EmailEventType> eventTypes);

String retrieveOrcidType(String orcid);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;

import javax.persistence.NoResultException;
import javax.persistence.Query;
Expand All @@ -13,13 +16,15 @@
import org.apache.commons.lang3.tuple.Pair;
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.springframework.util.StringUtils;

public class ProfileDaoImpl extends GenericDaoImpl<ProfileEntity, String> implements ProfileDao {

Expand Down Expand Up @@ -151,6 +156,36 @@ public List<Pair<String, Date>> findEmailsUnverfiedDays(int daysUnverified, int
});
return results;
}

@SuppressWarnings("unchecked")
@Override
public List<Pair<String, Date>> findEmailsUnverifiedDaysAndEventType(int daysUnverified, int maxResults, List<EmailEventType> eventTypes) {
List<String> stringList = eventTypes.stream().map(Optional::ofNullable) //Stream<Optional<..>>
.map(opt -> opt.orElse(null))
.map(Objects::toString)
.map(opt -> "'"+opt+"'")
.collect(Collectors.toList());

String eventsTypeStr = StringUtils.collectionToCommaDelimitedString(stringList);
StringBuilder queryString = new StringBuilder("SELECT e.email, 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 or email_event_type NOT IN ("+ eventsTypeStr+"))" + "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(" GROUP BY e.email, e.date_created, e.last_modified");
queryString.append(" ORDER BY e.last_modified");
Query query = entityManager.createNativeQuery(queryString.toString());
query.setMaxResults(maxResults);
List<Object[]> dbInfo = query.getResultList();
List<Pair<String, Date>> results = new ArrayList<Pair<String, Date>>();
dbInfo.stream().forEach(element -> {
Pair<String, Date> pair = Pair.of((String) element[0], (Date) element[1]);
results.add(pair);
});
return results;
}

@SuppressWarnings("unchecked")
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
*
*/
public enum EmailEventType {

VERIFY_EMAIL_28_DAYS_SENT,
VERIFY_EMAIL_28_DAYS_SENT_SKIPPED,
VERIFY_EMAIL_7_DAYS_SENT,
VERIFY_EMAIL_7_DAYS_SENT_SKIPPED, /* we are going to skip notifying email address that where already in the system before launching this */
VERIFY_EMAIL_2_DAYS_SENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.orcid.core.manager.v3.NotificationManager;
import org.orcid.core.manager.v3.RecordNameManager;
import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly;
import org.orcid.core.togglz.Features;
import org.orcid.core.utils.VerifyEmailUtils;
import org.orcid.jaxb.model.common.ActionType;
import org.orcid.jaxb.model.common.AvailableLocales;
Expand Down Expand Up @@ -82,7 +83,11 @@ public class EmailMessageSenderImpl implements EmailMessageSender {

ExecutorService pool;

private int verifyReminderAfterDays = 2;
private int verifyReminderAfterTwoDays = 2;

private int verifyReminderAfterSevenDays = 7;

private int verifyReminderAfterTwentyEightDays = 28;

@Resource
private NotificationDao notificationDao;
Expand Down Expand Up @@ -141,8 +146,10 @@ public class EmailMessageSenderImpl implements EmailMessageSender {
@Value("${org.notifications.max_elements_to_show:20}")
private Integer maxNotificationsToShowPerClient;

@Value("${org.orcid.core.email.verify.tooOld:15}")
private int emailTooOld;
@Value("${org.orcid.core.email.verify.tooOld:45}")
private int emailTooOld;

private int emailTooOldLegacy = 15;

public EmailMessageSenderImpl(@Value("${org.notifications.service_announcements.maxThreads:8}") Integer maxThreads,
@Value("${org.notifications.service_announcements.maxRetry:3}") Integer maxRetry) {
Expand Down Expand Up @@ -563,13 +570,17 @@ private String getHtmlBody(NotificationAdministrative notificationAdministrative

@Override
synchronized public void processUnverifiedEmails2Days() {
LOGGER.info("About to process unverIfied emails for reminder");
LOGGER.info("About to process unverIfied emails for 2 days reminder");
List<Pair<String, Date>> elements = Collections.<Pair<String, Date>> emptyList();
do {
elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterDays, 100);
LOGGER.info("Got batch of {} profiles with unverified emails for reminder", elements.size());
elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterTwoDays, 100);
LOGGER.info("Got batch of {} profiles with unverified emails for 2 days reminder", elements.size());
LocalDateTime now = LocalDateTime.now();
Date tooOld = now.minusDays(emailTooOld).toDate();
//togglz here
Date tooOld = now.minusDays(emailTooOldLegacy).toDate();
if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) {
tooOld = now.minusDays(emailTooOld).toDate();
}
for (Pair<String, Date> element : elements) {
if(element.getRight() == null || element.getRight().after(tooOld)) {
processUnverifiedEmails2DaysInTransaction(element.getLeft());
Expand All @@ -580,7 +591,70 @@ synchronized public void processUnverifiedEmails2Days() {
}
} while (!elements.isEmpty());
}



synchronized public void processUnverifiedEmails7Days() {
if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) {
LOGGER.info("About to process unverIfied emails for 7 days reminder");
List<Pair<String, Date>> elements = Collections.<Pair<String, Date>> emptyList();
do {
elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterSevenDays, 100);
LOGGER.info("Got batch of {} profiles with unverified emails for 7 days reminder", elements.size());
LocalDateTime now = LocalDateTime.now();
Date tooOld = now.minusDays(emailTooOld).toDate();
for (Pair<String, Date> element : elements) {
if(element.getRight() == null || element.getRight().after(tooOld)) {
processUnverifiedEmailsInTransaction(element.getLeft(), EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED);
} else {
// Mark is as too old to send the verification email
markUnverifiedEmailAsTooOld(element.getLeft());
}
}
} while (!elements.isEmpty());
}
}


synchronized public void processUnverifiedEmails28Days() {
if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) {
LOGGER.info("About to process unverIfied emails for 28 days reminder");
List<Pair<String, Date>> elements = Collections.<Pair<String, Date>> emptyList();
do {
elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterTwentyEightDays, 100);
LOGGER.info("Got batch of {} profiles with unverified emails for 28 days reminder", elements.size());
LocalDateTime now = LocalDateTime.now();
Date tooOld = now.minusDays(emailTooOld).toDate();
for (Pair<String, Date> element : elements) {
if(element.getRight() == null || element.getRight().after(tooOld)) {
processUnverifiedEmailsInTransaction(element.getLeft(),EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED);
} else {
// Mark is as too old to send the verification email
markUnverifiedEmailAsTooOld(element.getLeft());
}
}
} while (!elements.isEmpty());
}
}

private void processUnverifiedEmailsInTransaction(final String email, EmailEventType eventSent, EmailEventType eventSkipped) {
transactionTemplate.execute(new TransactionCallbackWithoutResult() {
@Override
@Transactional
protected void doInTransactionWithoutResult(TransactionStatus status) {
try {
String userOrcid = emailManagerReadOnly.findOrcidIdByEmail(email);
sendVerificationReminderEmail(userOrcid, email);
emailEventDao.persist(new EmailEventEntity(email, eventSent));
emailEventDao.flush();
} catch (Exception e) {
LOGGER.error("Unable to send unverified email reminder to email: " + email, e);
emailEventDao.persist(new EmailEventEntity(email, eventSkipped));
emailEventDao.flush();
}
}
});
}

private void processUnverifiedEmails2DaysInTransaction(final String email) {
transactionTemplate.execute(new TransactionCallbackWithoutResult() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
<task:scheduled ref="cleanOldClientKeysCronJob" method="cleanOldClientKeys" cron="${org.orcid.scheduler.web.cleanOldClientKeys:0 0 0/1 * * ?}" />
<task:scheduled ref="emailMessageSender" method="sendEmailMessages" cron="${org.orcid.scheduler.web.sendEmailMessages:35 */5 * * * *}" />
<task:scheduled ref="emailMessageSender" method="processUnverifiedEmails2Days" cron="${org.orcid.scheduler.web.processUnverifiedEmails2Days:0 10 * * * *}"/>
<task:scheduled ref="emailMessageSender" method="processUnverifiedEmails7Days" cron="${org.orcid.scheduler.web.processUnverifiedEmails7Days:0 0 0 * * FRI}"/>
<task:scheduled ref="emailMessageSender" method="processUnverifiedEmails28Days" cron="${org.orcid.scheduler.web.processUnverifiedEmails28Days:0 0 0 */28 * * }"/>
<task:scheduled ref="identityProviderLoader" method="loadIdentityProviders" cron="${org.orcid.scheduler.web.loadIdentityProviders:05 05 0-2 * * *}"/>
<task:scheduled ref="notificationManager" method="processOldNotificationsToAutoArchive" cron="${org.orcid.scheduler.web.processOldNotificationsToAutoArchive:06 06 1 * * *}"/>
<task:scheduled ref="notificationManager" method="processOldNotificationsToAutoDelete" cron="${org.orcid.scheduler.web.processOldNotificationsToAutoDelete:07 07 2 * * *}"/>
Expand Down