From d7e07eed9d97a449b5e867d9ab93e4380ec54e75 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Fri, 3 Nov 2023 14:45:52 -0600 Subject: [PATCH 01/23] Is there is more than one org with no parent, we should not suggest any org for that domain --- .../loader/cli/EmailDomainToRorLoader.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/cli/EmailDomainToRorLoader.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/cli/EmailDomainToRorLoader.java index 8037f5d62d0..7dce97592fa 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/cli/EmailDomainToRorLoader.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/cli/EmailDomainToRorLoader.java @@ -7,10 +7,8 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import org.orcid.core.common.manager.EmailDomainManager; import org.orcid.core.common.manager.impl.EmailDomainManagerImpl.STATUS; @@ -30,11 +28,11 @@ public class EmailDomainToRorLoader { private String filePath; private EmailDomainManager emailDomainManager; private List> csvData; - private Set invalidDomains = new HashSet(); private Map map = new HashMap(); private int updatedEntries = 0; private int createdEntries = 0; + private int invalidEntires = 0; public EmailDomainToRorLoader(String filePath) { this.filePath = filePath; @@ -123,8 +121,8 @@ private void storeDomainToRorMap() { } else if (STATUS.UPDATED.equals(s)) { updatedEntries++; } - } else if(element.getIdsWithParent().size() == 1) { - // Else, if the domain has only one entry with parent, store that one + } else if(element.getIdsWithNoParent().isEmpty() && element.getIdsWithParent().size() == 1) { + // Else, if the domain doesn't have an org with no parents and only have one entry with parent, store that one STATUS s = emailDomainManager.createOrUpdateEmailDomain(element.getDomain(), element.getIdsWithParent().get(0)); if(STATUS.CREATED.equals(s)) { createdEntries++; @@ -133,17 +131,12 @@ private void storeDomainToRorMap() { } } else { // Else log a warning because there is no way to provide a suggestion - invalidDomains.add(element.getDomain()); + LOG.warn("Domain {} couldnt be mapped, it have {} rows with parent and {} rows with no parent", element.getDomain(), element.getIdsWithParent().size(), element.getIdsWithNoParent().size()); + invalidEntires++; } } - if(!invalidDomains.isEmpty()) { - LOG.warn("The following domains couldn't be mapped ({} In total):", invalidDomains.size()); - for(String invalidDomain : invalidDomains) { - LOG.warn("{}", invalidDomain); - } - } - LOG.info("Created entries: {}, updated entries: {}", createdEntries, updatedEntries); + LOG.info("Created entries: {}, updated entries: {}, invalid entries {}", createdEntries, updatedEntries, invalidEntires); } private class DomainToRorMap { From 2526b676732760b925d7a73732e7ea0a4242c220 Mon Sep 17 00:00:00 2001 From: github actions Date: Fri, 3 Nov 2023 21:18:42 +0000 Subject: [PATCH 02/23] v2.44.5 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bf11883fdd..3f7cdfa64c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.5 - 2023-11-03 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.4...v2.44.5) + +- [#6925](https://github.com/ORCID/ORCID-Source/pull/6925): Is there is more than one org with no parent, we should not suggest a… + ## v2.44.4 - 2023-11-02 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.3...v2.44.4) From c13418b94e3ef28550e06d7ec054bfb4f5ad3320 Mon Sep 17 00:00:00 2001 From: Daniel Palafox Date: Wed, 8 Nov 2023 15:40:14 -0500 Subject: [PATCH 03/23] fix: Add create event to ShibbolethController and Add missing member name --- .../core/common/manager/EventManager.java | 2 +- .../common/manager/impl/EventManagerImpl.java | 37 +++++++++++++++---- .../frontend/oauth2/OauthController.java | 2 +- .../AjaxAuthenticationSuccessHandler.java | 2 +- .../web/controllers/LoginController.java | 6 +-- .../controllers/OauthAuthorizeController.java | 2 +- .../controllers/PublicRecordController.java | 2 +- .../controllers/RegistrationController.java | 6 +-- .../web/controllers/ShibbolethController.java | 10 +++++ 9 files changed, 51 insertions(+), 18 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java index eaecd912705..323f76a9534 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java @@ -14,6 +14,6 @@ public interface EventManager { boolean removeEvents(String orcid); - void createEvent(String orcid, EventType eventType, HttpServletRequest request, RequestInfoForm requestInfoForm); + void createEvent(String orcid, EventType eventType, HttpServletRequest request); } diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java index 11c7b13d6fa..96b6b540bf9 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java @@ -7,10 +7,14 @@ import org.orcid.core.common.manager.EventManager; import org.orcid.core.constants.OrcidOauth2Constants; import org.orcid.core.manager.ClientDetailsEntityCacheManager; +import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; import org.orcid.core.utils.EventType; +import org.orcid.jaxb.model.clientgroup.ClientType; +import org.orcid.jaxb.model.v3.release.record.Name; import org.orcid.persistence.dao.EventDao; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.EventEntity; +import org.orcid.pojo.ajaxForm.PojoUtil; import org.orcid.pojo.ajaxForm.RequestInfoForm; import java.io.UnsupportedEncodingException; @@ -30,13 +34,16 @@ public class EventManagerImpl implements EventManager { @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource(name = "recordNameManagerReadOnlyV3") + private RecordNameManagerReadOnly recordNameManagerReadOnly; + @Override public boolean removeEvents(String orcid) { return eventDao.removeEvents(orcid); } @Override - public void createEvent(String orcid, EventType eventType, HttpServletRequest request, RequestInfoForm requestInfoForm) { + public void createEvent(String orcid, EventType eventType, HttpServletRequest request) { String label = "Website"; String clientId = null; String redirectUrl = null; @@ -48,17 +55,33 @@ public void createEvent(String orcid, EventType eventType, HttpServletRequest re } else { if (request != null) { Boolean isOauth2ScreensRequest = (Boolean) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_2SCREENS); - if (isOauth2ScreensRequest != null && isOauth2ScreensRequest) { + RequestInfoForm requestInfoForm = (RequestInfoForm) request.getSession().getAttribute("requestInfoForm"); + if (requestInfoForm != null) { + clientId = requestInfoForm.getClientId(); + redirectUrl = removeAttributesFromUrl(requestInfoForm.getRedirectUrl()); + label = "OAuth " + requestInfoForm.getMemberName() + " " + requestInfoForm.getClientName(); + } else if (isOauth2ScreensRequest != null && isOauth2ScreensRequest) { String queryString = (String) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_QUERY_STRING); clientId = getParameterValue(queryString, "client_id"); redirectUrl = getParameterValue(queryString, "redirect_uri"); ClientDetailsEntity clientDetailsEntity = clientDetailsEntityCacheManager.retrieve(clientId); - label = "OAuth " + clientDetailsEntity.getClientName(); + String memberName = ""; + String clientName = clientDetailsEntity.getClientName(); + + if (ClientType.PUBLIC_CLIENT.equals(clientDetailsEntity.getClientType())) { + memberName = "PubApp"; + } else if (!PojoUtil.isEmpty(clientDetailsEntity.getGroupProfileId())) { + Name name = recordNameManagerReadOnly.getRecordName(clientDetailsEntity.getGroupProfileId()); + if (name != null) { + memberName = name.getCreditName() != null ? name.getCreditName().getContent() : ""; + } + } + + if (StringUtils.isBlank(memberName)) { + memberName = clientName; + } + label = "OAuth " + memberName + " " + clientName; } - } else if (requestInfoForm != null) { - clientId = requestInfoForm.getClientId(); - redirectUrl = removeAttributesFromUrl(requestInfoForm.getRedirectUrl()); - label = "OAuth " + requestInfoForm.getClientName(); } } diff --git a/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java b/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java index 0b24e1ec2f5..f11d4ef4f55 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java @@ -97,7 +97,7 @@ public class OauthController { if (responseParam != null && !responseParam.isEmpty() && !PojoUtil.isEmpty(responseParam.get(0))) { isResponseSet = true; if (Features.EVENTS.isActive()) { - eventManager.createEvent(requestInfoForm.getUserOrcid(), EventType.REAUTHORIZE, null, requestInfoForm); + eventManager.createEvent(requestInfoForm.getUserOrcid(), EventType.REAUTHORIZE, request); } } } diff --git a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java index bd660794e1c..6bb187adfbb 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java +++ b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java @@ -26,7 +26,7 @@ public class AjaxAuthenticationSuccessHandler extends AjaxAuthenticationSuccessH public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { String targetUrl = getTargetUrl(request, response, authentication); if (Features.EVENTS.isActive()) { - eventManager.createEvent(authentication.getPrincipal().toString(), EventType.SIGN_IN, request, null); + eventManager.createEvent(authentication.getPrincipal().toString(), EventType.SIGN_IN, request); } response.setContentType("application/json"); response.getWriter().println("{\"success\": true, \"url\": \"" + targetUrl.replaceAll("^/", "") + "\"}"); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java index dff31541f06..98a34a18d04 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java @@ -326,9 +326,6 @@ private ModelAndView processSocialLogin(HttpServletRequest request, HttpServletR userConnectionId = userConnection.getId().getUserid(); // Store relevant data in the session socialSignInUtils.setSignedInData(request, userData); - if (Features.EVENTS.isActive()) { - eventManager.createEvent(userConnection.getOrcid(), EventType.SIGN_IN, request, null); - } if(userConnection.isLinked()) { // If user exists and is linked update user connection info @@ -351,6 +348,9 @@ private ModelAndView processSocialLogin(HttpServletRequest request, HttpServletR if (userConnectionId == null) { throw new IllegalArgumentException("Unable to find userConnectionId for providerUserId = " + providerUserId); } + if (Features.EVENTS.isActive()) { + eventManager.createEvent(userConnection.getOrcid(), EventType.SIGN_IN, request); + } userCookieGenerator.addCookie(userConnectionId, response); if ("social_2FA".equals(view.getViewName())) { diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java index dbed4118a0d..8bdf7fc2bf8 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java @@ -260,7 +260,7 @@ public ModelAndView loginGetHandler(HttpServletRequest request, HttpServletRespo } else { orcid = auth.getPrincipal().toString(); } - eventManager.createEvent(orcid, eventType, null, requestInfoForm); + eventManager.createEvent(orcid, eventType, request); } if(new HttpSessionRequestCache().getRequest(request, response) != null) new HttpSessionRequestCache().removeRequest(request, response); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java index 63f56614557..3f2f78febc4 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java @@ -185,7 +185,7 @@ PublicRecord getPublicRecord(@PathVariable("orcid") String orcid) { try { if (Features.EVENTS.isActive()) { - eventManager.createEvent(orcid, EventType.PUBLIC_PAGE, null, null); + eventManager.createEvent(orcid, EventType.PUBLIC_PAGE, null); } // Check if the profile is deprecated or locked orcidSecurityManager.checkProfile(orcid); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java index 78f299e9c1d..9979dfd533f 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java @@ -281,6 +281,9 @@ public void validateGrcaptcha(HttpServletRequest request, @RequestBody Registrat Locale locale = RequestContextUtils.getLocale(request); // Ip String ip = OrcidRequestUtil.getIpAddress(request); + if (Features.EVENTS.isActive()) { + eventManager.createEvent(getCurrentUserOrcid(), EventType.NEW_REGISTRATION, request); + } createMinimalRegistrationAndLogUserIn(request, response, reg, usedCaptcha, locale, ip); } catch (Exception e) { LOGGER.error("Error registering a new user", e); @@ -304,9 +307,6 @@ public void validateGrcaptcha(HttpServletRequest request, @RequestBody Registrat redirectUrl = calculateRedirectUrl(request, response, true, true); } r.setUrl(redirectUrl); - if (Features.EVENTS.isActive()) { - eventManager.createEvent(getCurrentUserOrcid(), EventType.NEW_REGISTRATION, request, null); - } return r; } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java index 9f8e25031c0..9f0dc458c06 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java @@ -10,6 +10,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.orcid.core.common.manager.EventManager; import org.orcid.core.constants.OrcidOauth2Constants; import org.orcid.core.manager.BackupCodeManager; import org.orcid.core.manager.IdentityProviderManager; @@ -20,6 +21,8 @@ import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; import org.orcid.core.oauth.OrcidProfileUserDetails; import org.orcid.core.security.OrcidUserDetailsService; +import org.orcid.core.togglz.Features; +import org.orcid.core.utils.EventType; import org.orcid.core.utils.JsonUtils; import org.orcid.frontend.web.exception.FeatureDisabledException; import org.orcid.persistence.jpa.entities.ProfileEntity; @@ -81,6 +84,9 @@ public class ShibbolethController extends BaseController { @Resource private OrcidUserDetailsService orcidUserDetailsService; + + @Resource + private EventManager eventManager; @RequestMapping(value = { "/2FA/authenticationCode.json" }, method = RequestMethod.GET) public @ResponseBody TwoFactorAuthenticationCodes getTwoFactorCodeWrapper() { @@ -165,6 +171,10 @@ public ModelAndView signinHandler(HttpServletRequest request, HttpServletRespons try { notifyUser(shibIdentityProvider, userConnectionEntity); processAuthentication(remoteUser, userConnectionEntity); + if (Features.EVENTS.isActive()) { + OrcidProfileUserDetails orcidProfileUserDetails = getOrcidProfileUserDetails(userConnectionEntity.getOrcid()); + eventManager.createEvent(orcidProfileUserDetails.getOrcid(), EventType.SIGN_IN, request); + } } catch (AuthenticationException e) { // this should never happen SecurityContextHolder.getContext().setAuthentication(null); From 2274efb5e0be7f1da5214be0ca5d219fa95c7ae4 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 8 Nov 2023 15:25:22 -0600 Subject: [PATCH 04/23] Add more logging for when the request changes during oauth --- .../controllers/OauthAuthorizeController.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java index dbed4118a0d..0abf1d5c47c 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java @@ -27,9 +27,11 @@ import org.slf4j.LoggerFactory; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.oauth2.common.exceptions.InvalidRequestException; import org.springframework.security.oauth2.common.exceptions.InvalidScopeException; import org.springframework.security.oauth2.common.util.OAuth2Utils; import org.springframework.security.oauth2.provider.AuthorizationRequest; +import org.springframework.security.oauth2.provider.endpoint.AuthorizationEndpoint; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestBody; @@ -249,8 +251,22 @@ public ModelAndView loginGetHandler(HttpServletRequest request, HttpServletRespo } // Approve - RedirectView view = (RedirectView) authorizationEndpoint.approveOrDeny(approvalParams, model, status, auth); - requestInfoForm.setRedirectUrl(view.getUrl()); + try { + RedirectView view = (RedirectView) authorizationEndpoint.approveOrDeny(approvalParams, model, status, auth); + requestInfoForm.setRedirectUrl(view.getUrl()); + } catch (InvalidRequestException ire) { + LOGGER.error("Something changed on the request, here are the authorization request and original authorization request values:"); + LOGGER.error("Client id: original '{}' latest '{}'", originalRequest.get(OrcidOauth2Constants.CLIENT_ID), authorizationRequest.getClientId()); + LOGGER.error("State: original '{}' latest '{}'", originalRequest.get(OrcidOauth2Constants.STATE_PARAM), authorizationRequest.getState()); + LOGGER.error("Redirect uri: original '{}' latest '{}'", originalRequest.get(OrcidOauth2Constants.REDIRECT_URI_PARAM), authorizationRequest.getRedirectUri()); + LOGGER.error("Response type: original '{}' latest '{}'", originalRequest.get(OrcidOauth2Constants.RESPONSE_TYPE_PARAM), authorizationRequest.getResponseTypes()); + LOGGER.error("Scope: original '{}' latest '{}'", originalRequest.get(OrcidOauth2Constants.SCOPE_PARAM), authorizationRequest.getScope()); + LOGGER.error("Approved: original '{}' latest '{}'", originalRequest.get("approved"), authorizationRequest.isApproved()); + LOGGER.error("Resource Ids: original '{}' latest '{}'", originalRequest.get("resourceIds"), authorizationRequest.getResourceIds()); + LOGGER.error("Authorities: original '{}' latest '{}'", originalRequest.get("authorities"), authorizationRequest.getAuthorities()); + // Propagate the exception + throw ire; + } if (Features.EVENTS.isActive()) { EventType eventType = "true".equals(approvalParams.get("user_oauth_approval")) ? EventType.AUTHORIZE : EventType.AUTHORIZE_DENY; String orcid = null; From 7fe1bb324ca8b6c70ab83711dea77d5dbfc3c132 Mon Sep 17 00:00:00 2001 From: github actions Date: Wed, 8 Nov 2023 22:06:57 +0000 Subject: [PATCH 05/23] v2.44.6 changelog update --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f7cdfa64c6..b78b182299e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## v2.44.6 - 2023-11-08 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.5...v2.44.6) + +- [#6926](https://github.com/ORCID/ORCID-Source/pull/6926): fix: Add create event to ShibbolethController and Add missing member … + +### Fix + +- Add create event to ShibbolethController and Add missing member name + ## v2.44.5 - 2023-11-03 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.4...v2.44.5) From aa1f1cbbf080b2b3808f1a64a5188caaaaf3abcb Mon Sep 17 00:00:00 2001 From: github actions Date: Thu, 9 Nov 2023 16:53:28 +0000 Subject: [PATCH 06/23] v2.44.7 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b78b182299e..6e364a50a17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.7 - 2023-11-09 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.6...v2.44.7) + +- [#6927](https://github.com/ORCID/ORCID-Source/pull/6927): Add more logging for when the request changes during oauth + ## v2.44.6 - 2023-11-08 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.5...v2.44.6) From 05c8a6a6889a298a9439f158642c28f977b3538f Mon Sep 17 00:00:00 2001 From: "c.dumitru@orcid.org" Date: Tue, 14 Nov 2023 11:36:12 +0000 Subject: [PATCH 07/23] Changed the code to use triples --- .../org/orcid/persistence/dao/ProfileDao.java | 2 + .../persistence/dao/impl/ProfileDaoImpl.java | 32 +++++++++- .../cli/manager/EmailMessageSenderImpl.java | 62 ++++++++++--------- 3 files changed, 66 insertions(+), 30 deletions(-) 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 1737562b61a..61652824719 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 @@ -78,6 +78,8 @@ public interface ProfileDao extends GenericDao { public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults); + public List> findEmailsUnverifiedDaysByEventType(int daysUnverified, int tooOldNumberOfDays); + String retrieveOrcidType(String orcid); List findInfoForDecryptionAnalysis(); 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 baf0d02de86..27e5948d70f 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 @@ -21,9 +21,14 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.transaction.annotation.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class ProfileDaoImpl extends GenericDaoImpl implements ProfileDao { private static final String PRIVATE_VISIBILITY = "PRIVATE"; + + private static final Logger LOGGER = LoggerFactory.getLogger(ProfileDaoImpl.class); @Value("${org.orcid.postgres.query.timeout:30000}") private Integer queryTimeout; @@ -150,7 +155,32 @@ public List> findEmailsUnverfiedDays(int daysUnver 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"); + + Query query = entityManager.createNativeQuery(queryString.toString()); + List dbInfo = query.getResultList(); + List> results = new ArrayList>(); + dbInfo.stream().forEach(element -> { + Triple pair = Triple.of((String) element[0], (Boolean) element[1], (String) element[2]); + results.add(pair); + }); + return results; + } @SuppressWarnings("unchecked") @Override 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 c4deda62955..134285ddb23 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 @@ -70,6 +70,8 @@ import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionTemplate; +import liquibase.repackaged.org.apache.commons.lang3.StringUtils; + /** * * @author Will Simpson @@ -596,43 +598,45 @@ synchronized public void processUnverifiedEmails2Days() { 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(); - 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 (Triple element : elements) { - if(element.getRight() == null || element.getRight().after(tooOld)) { + 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); - } else { - // Mark is as too old to send the verification email - markUnverifiedEmailAsTooOld(element.getLeft()); - } + hasEventTypesMap.put(element.getLeft(), element); + } } - } while (!elements.isEmpty()); - } } + //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; + } + synchronized public void processUnverifiedEmails28Days() { if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { LOGGER.info("About to process unverIfied emails for 28 days reminder"); - List> elements = Collections.> 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 (Triple element : elements) { - if(element.getRight() == null || element.getRight().after(tooOld)) { - processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), 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()); + 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); + } } } From c1bf046a392640c96e88500c706c76503dfc19c4 Mon Sep 17 00:00:00 2001 From: "c.dumitru@orcid.org" Date: Tue, 14 Nov 2023 11:37:47 +0000 Subject: [PATCH 08/23] Formatting --- .../org/orcid/persistence/dao/ProfileDao.java | 8 +- .../persistence/dao/impl/ProfileDaoImpl.java | 23 +- .../cli/manager/EmailMessageSenderImpl.java | 221 +++++++++--------- 3 files changed, 127 insertions(+), 125 deletions(-) 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 61652824719..09bf55902b6 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 @@ -77,9 +77,9 @@ 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); - + String retrieveOrcidType(String orcid); List findInfoForDecryptionAnalysis(); @@ -158,10 +158,10 @@ public interface ProfileDao extends GenericDao { public List getSigninLock(String orcid); public void startSigninLock(String orcid); - + public void resetSigninLock(String orcid); public void updateSigninLock(String orcid, Integer count); - + boolean haveMemberPushedWorksOrAffiliationsToRecord(String orcid, String clientId); } 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 27e5948d70f..d916513f83a 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 @@ -27,7 +27,7 @@ public class ProfileDaoImpl extends GenericDaoImpl implements ProfileDao { private static final String PRIVATE_VISIBILITY = "PRIVATE"; - + private static final Logger LOGGER = LoggerFactory.getLogger(ProfileDaoImpl.class); @Value("${org.orcid.postgres.query.timeout:30000}") @@ -155,9 +155,8 @@ public List> findEmailsUnverfiedDays(int daysUnver results.add(pair); }); return results; - } - - + } + @SuppressWarnings("unchecked") @Override public List> findEmailsUnverifiedDaysByEventType(int daysUnverified, int tooOldNumberOfDays) { @@ -166,8 +165,8 @@ public List> findEmailsUnverifiedDaysByEventType 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 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"); @@ -180,7 +179,7 @@ public List> findEmailsUnverifiedDaysByEventType results.add(pair); }); return results; - } + } @SuppressWarnings("unchecked") @Override @@ -815,7 +814,7 @@ public void startSigninLock(String orcid) { query.executeUpdate(); return; } - + @Override @Transactional public void resetSigninLock(String orcid) { @@ -839,7 +838,7 @@ public void updateSigninLock(String orcid, Integer count) { query.executeUpdate(); return; } - + public boolean haveMemberPushedWorksOrAffiliationsToRecord(String orcid, String clientId) { try { String queryString = "select p.orcid from profile p where p.orcid = :orcid and ( exists (select 1 from work w where w.orcid = p.orcid and w.client_source_id = :clientId) or exists (select 1 from org_affiliation_relation o where o.orcid = p.orcid and o.client_source_id = :clientId));"; @@ -847,10 +846,10 @@ public boolean haveMemberPushedWorksOrAffiliationsToRecord(String orcid, String query.setParameter("orcid", orcid); query.setParameter("clientId", clientId); String result = (String) query.getSingleResult(); - if(orcid.equals(result)) { + if (orcid.equals(result)) { return true; - } - } catch(NoResultException nre) { + } + } catch (NoResultException nre) { return false; } return false; 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 134285ddb23..d69ee216fa3 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 @@ -80,17 +80,17 @@ public class EmailMessageSenderImpl implements EmailMessageSender { private static final Logger LOGGER = LoggerFactory.getLogger(EmailMessageSenderImpl.class); - + private final Integer MAX_RETRY_COUNT; - + ExecutorService pool; - + private int verifyReminderAfterTwoDays = 2; - + private int verifyReminderAfterSevenDays = 7; - + private int verifyReminderAfterTwentyEightDays = 28; - + @Resource private NotificationDao notificationDao; @@ -105,7 +105,7 @@ public class EmailMessageSenderImpl implements EmailMessageSender { @Resource private EmailDao emailDao; - + @Resource private TransactionTemplate transactionTemplate; @@ -126,33 +126,33 @@ public class EmailMessageSenderImpl implements EmailMessageSender { @Resource private ProfileEntityCacheManager profileEntityCacheManager; - + @Resource(name = "emailManagerReadOnlyV3") private EmailManagerReadOnly emailManagerReadOnly; - + @Resource private GenericDao emailEventDao; - + @Resource private ProfileDao profileDaoReadOnly; - + @Resource(name = "recordNameManagerV3") private RecordNameManager recordNameManagerV3; - + @Resource private VerifyEmailUtils verifyEmailUtils; - + @Value("${org.notifications.service_announcements.batchSize:60000}") private Integer batchSize; - + @Value("${org.notifications.max_elements_to_show:20}") - private Integer maxNotificationsToShowPerClient; - + private Integer maxNotificationsToShowPerClient; + @Value("${org.orcid.core.email.verify.tooOld:45}") - private int emailTooOld; - - private int emailTooOldLegacy = 15; - + 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) { if (maxThreads == null || maxThreads > 64 || maxThreads < 1) { @@ -163,7 +163,7 @@ public EmailMessageSenderImpl(@Value("${org.notifications.service_announcements. MAX_RETRY_COUNT = maxRetry; } - + @Override public EmailMessage createDigest(String orcid, Collection notifications) { ProfileEntity record = profileEntityCacheManager.retrieve(orcid); @@ -173,7 +173,7 @@ public EmailMessage createDigest(String orcid, Collection notifica String bodyHtmlDelegate = null; String bodyHtmlDelegateRecipient = null; String bodyHtmlAdminDelegate = null; - + Set memberIds = new HashSet<>(); DigestEmail digestEmail = new DigestEmail(); @@ -229,19 +229,21 @@ public EmailMessage createDigest(String orcid, Collection notifica } } } - + List sortedClientIds = updatesByClient.keySet().stream().sorted().collect(Collectors.toList()); List sortedClientUpdates = new ArrayList(); - sortedClientIds.stream().forEach(s -> {sortedClientUpdates.add(updatesByClient.get(s));}); + sortedClientIds.stream().forEach(s -> { + sortedClientUpdates.add(updatesByClient.get(s)); + }); String emailName = recordNameManagerV3.deriveEmailFriendlyName(record.getId()); String subject = messages.getMessage("email.subject.digest", new String[] { emailName }, locale); Map params = new HashMap<>(); params.put("locale", locale); params.put("messages", messages); - params.put("messageArgs", new Object[0]); + params.put("messageArgs", new Object[0]); params.put("emailName", emailName); - params.put("digestEmail", digestEmail); + params.put("digestEmail", digestEmail); params.put("orcidMessageCount", orcidMessageCount); params.put("baseUri", orcidUrlManager.getBaseUrl()); params.put("subject", subject); @@ -264,17 +266,17 @@ public EmailMessage createDigest(String orcid, Collection notifica emailMessage.setBodyHtml(bodyHtml); return emailMessage; } - + private Locale getUserLocaleFromProfileEntity(ProfileEntity profile) { String locale = profile.getLocale(); if (locale != null) { AvailableLocales loc = AvailableLocales.valueOf(locale); return LocaleUtils.toLocale(loc.value()); } - + return LocaleUtils.toLocale("en"); } - + private String encryptAndEncodePutCode(Long putCode) { String encryptedPutCode = encryptionManager.encryptForExternalUse(String.valueOf(putCode)); try { @@ -287,26 +289,26 @@ private String encryptAndEncodePutCode(Long putCode) { @Override public void sendEmailMessages() { List orcidsWithUnsentNotifications = new ArrayList(); - orcidsWithUnsentNotifications = notificationDaoReadOnly.findRecordsWithUnsentNotifications(); - + orcidsWithUnsentNotifications = notificationDaoReadOnly.findRecordsWithUnsentNotifications(); + for (final Object[] element : orcidsWithUnsentNotifications) { - String orcid = (String) element[0]; + String orcid = (String) element[0]; try { Float emailFrequencyDays = null; Date recordActiveDate = null; recordActiveDate = (Date) element[1]; - + List notifications = notificationManager.findNotificationsToSend(orcid, emailFrequencyDays, recordActiveDate); - + EmailEntity primaryEmail = emailDao.findPrimaryEmail(orcid); if (primaryEmail == null) { LOGGER.info("No primary email for orcid: " + orcid); return; } - - if(!notifications.isEmpty()) { + + if (!notifications.isEmpty()) { LOGGER.info("Found {} messages to send for orcid: {}", notifications.size(), orcid); - EmailMessage digestMessage = createDigest(orcid, notifications); + EmailMessage digestMessage = createDigest(orcid, notifications); digestMessage.setFrom(EmailConstants.DO_NOT_REPLY_NOTIFY_ORCID_ORG); digestMessage.setTo(primaryEmail.getEmail()); boolean successfullySent = mailGunManager.sendEmail(digestMessage.getFrom(), digestMessage.getTo(), digestMessage.getSubject(), @@ -352,11 +354,11 @@ public Boolean call() throws Exception { e.printStackTrace(); } } - + @Override public void sendTips(Integer customBatchSize, String fromAddress) { LOGGER.info("About to send Tips messages"); - + List serviceAnnouncementsOrTips = new ArrayList(); try { long startTime = System.currentTimeMillis(); @@ -383,11 +385,11 @@ public Boolean call() throws Exception { e.printStackTrace(); } } - + private void processServiceAnnouncementOrTipNotification(NotificationEntity n) { processServiceAnnouncementOrTipNotification(n, null); } - + private void processServiceAnnouncementOrTipNotification(NotificationEntity n, String fromAddress) { String orcid = n.getOrcid(); EmailEntity primaryEmail = emailDao.findPrimaryEmail(orcid); @@ -396,7 +398,7 @@ private void processServiceAnnouncementOrTipNotification(NotificationEntity n, S flagAsFailed(orcid, n); return; } - if(!primaryEmail.getVerified()) { + if (!primaryEmail.getVerified()) { LOGGER.info("Primary email not verified for: " + orcid); flagAsFailed(orcid, n); return; @@ -404,23 +406,21 @@ private void processServiceAnnouncementOrTipNotification(NotificationEntity n, S try { boolean successfullySent = false; String fromAddressParam = EmailConstants.DO_NOT_REPLY_NOTIFY_ORCID_ORG; - if(!PojoUtil.isEmpty(fromAddress)) { + if (!PojoUtil.isEmpty(fromAddress)) { fromAddressParam = fromAddress; } if (n instanceof NotificationServiceAnnouncementEntity) { NotificationServiceAnnouncementEntity nc = (NotificationServiceAnnouncementEntity) n; // They might be custom notifications to have the // html/text ready to be sent - successfullySent = mailGunManager.sendMarketingEmail(fromAddressParam, primaryEmail.getEmail(), nc.getSubject(), nc.getBodyText(), - nc.getBodyHtml()); + successfullySent = mailGunManager.sendMarketingEmail(fromAddressParam, primaryEmail.getEmail(), nc.getSubject(), nc.getBodyText(), nc.getBodyHtml()); } else if (n instanceof NotificationTipEntity) { NotificationTipEntity nc = (NotificationTipEntity) n; // They might be custom notifications to have the // html/text ready to be sent - successfullySent = mailGunManager.sendMarketingEmail(fromAddressParam, primaryEmail.getEmail(), nc.getSubject(), nc.getBodyText(), - nc.getBodyHtml()); + successfullySent = mailGunManager.sendMarketingEmail(fromAddressParam, primaryEmail.getEmail(), nc.getSubject(), nc.getBodyText(), nc.getBodyHtml()); } - + if (successfullySent) { flagAsSent(n.getId()); } else { @@ -430,58 +430,58 @@ private void processServiceAnnouncementOrTipNotification(NotificationEntity n, S LOGGER.warn("Problem sending service announcement message to user: " + orcid, e); } } - + private void flagAsSent(Long id) { - transactionTemplate.execute(new TransactionCallbackWithoutResult() { + transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus status) { notificationDao.flagAsSent(Arrays.asList(id)); } - }); + }); } - + private void flagAsFailed(String orcid, NotificationEntity n) { - transactionTemplate.execute(new TransactionCallbackWithoutResult() { + transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus status) { - if(n.getRetryCount() != null && n.getRetryCount() >= MAX_RETRY_COUNT) { + if (n.getRetryCount() != null && n.getRetryCount() >= MAX_RETRY_COUNT) { notificationDao.flagAsNonSendable(orcid, n.getId()); } else { - if(n.getRetryCount() == null) { + if (n.getRetryCount() == null) { notificationDao.updateRetryCount(orcid, n.getId(), 1L); } else { notificationDao.updateRetryCount(orcid, n.getId(), (n.getRetryCount() + 1)); } - } + } } - }); + }); } - + public class ClientUpdates { String clientId; String clientName; String clientDescription; Integer counter = 0; Locale userLocale; - - Map>> updates = new HashMap<>(); - + + Map>> updates = new HashMap<>(); + public void setClientId(String clientId) { this.clientId = clientId; } - + public void setClientName(String clientName) { this.clientName = clientName; } - + public void setClientDescription(String clientDescription) { this.clientDescription = clientDescription; } - + public void setUserLocale(Locale locale) { this.userLocale = locale; } - + public String getClientId() { return clientId; } @@ -497,11 +497,11 @@ public String getClientDescription() { public Locale getUserLocale() { return userLocale; } - + public Map>> getUpdates() { return updates; } - + public Integer getCounter() { return counter; } @@ -509,11 +509,11 @@ public Integer getCounter() { private String renderCreationDate(XMLGregorianCalendar createdDate) { String result = new String(); result += createdDate.getYear(); - result += "-" + (createdDate.getMonth() < 10 ? "0" + createdDate.getMonth() : createdDate.getMonth()); - result += "-" + (createdDate.getDay() < 10 ? "0" + createdDate.getDay() : createdDate.getDay()); + result += "-" + (createdDate.getMonth() < 10 ? "0" + createdDate.getMonth() : createdDate.getMonth()); + result += "-" + (createdDate.getDay() < 10 ? "0" + createdDate.getDay() : createdDate.getDay()); return result; - } - + } + public void addElement(XMLGregorianCalendar createdDate, Item item) { init(item.getItemType().name(), item.getActionType() == null ? null : item.getActionType().name()); String value = null; @@ -550,7 +550,7 @@ public void addElement(XMLGregorianCalendar createdDate, Item item) { counter += 1; } - } + } private void init(String itemType, String actionType) { if (!updates.containsKey(itemType)) { @@ -569,7 +569,7 @@ private String getHtmlBody(NotificationAdministrative notificationAdministrative int bodyTagClose = notificationAdministrative.getBodyHtml().indexOf(""); return notificationAdministrative.getBodyHtml().substring(bodyTag + 6, bodyTagClose); } - + @Override synchronized public void processUnverifiedEmails2Days() { LOGGER.info("About to process unverIfied emails for 2 days reminder"); @@ -578,14 +578,15 @@ synchronized public void processUnverifiedEmails2Days() { 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 + // togglz here Date tooOld = now.minusDays(emailTooOldLegacy).toDate(); - if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { + 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); + 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()); @@ -593,53 +594,55 @@ 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> elements = Collections.> emptyList(); - + 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)); + 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); - } + 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 + + // 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()))) { + if (eventTypes.contains(EmailEventType.valueOf(element.getRight()))) { hasEventTypesMap.put(element.getLeft(), element); } } return hasEventTypesMap; } - synchronized public void processUnverifiedEmails28Days() { - 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); + 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); - } + } } } - + private void processUnverifiedEmailsInTransaction(final String email, final Boolean isPrimaryEmail, EmailEventType eventSent, EmailEventType eventSkipped) { transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override @@ -657,19 +660,19 @@ protected void doInTransactionWithoutResult(TransactionStatus status) { } } }); - } - + } + private void markUnverifiedEmailAsTooOld(final String email) { transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override @Transactional - protected void doInTransactionWithoutResult(TransactionStatus status) { + 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); @@ -682,5 +685,5 @@ private void sendVerificationReminderEmail(String userOrcid, String email, Boole String body = templateManager.processTemplate("verification_email_v2.ftl", templateParams); String htmlBody = templateManager.processTemplate("verification_email_html_v2.ftl", templateParams); mailGunManager.sendEmail(EmailConstants.DO_NOT_REPLY_VERIFY_ORCID_ORG, email, subject, body, htmlBody); - } + } } From f9202309b89c0509b9016b6b356cc1a65871477f Mon Sep 17 00:00:00 2001 From: github actions Date: Tue, 14 Nov 2023 16:11:09 +0000 Subject: [PATCH 09/23] v2.44.8 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e364a50a17..e3ca315bbab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.8 - 2023-11-14 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.7...v2.44.8) + +- [#6929](https://github.com/ORCID/ORCID-Source/pull/6929): Changed the code to use triples + ## v2.44.7 - 2023-11-09 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.6...v2.44.7) From 6f466eb69bb4b14421780ff0d149ddcd1d184e8d Mon Sep 17 00:00:00 2001 From: amontenegro Date: Tue, 14 Nov 2023 11:37:54 -0600 Subject: [PATCH 10/23] Remove orcid, redirect_url, public_page and last_modified from event --- .../core/common/manager/EventManager.java | 7 +- .../common/manager/impl/EventManagerImpl.java | 24 ++---- .../org/orcid/persistence/dao/EventDao.java | 9 +-- .../persistence/dao/impl/EventDaoImpl.java | 36 +++------ .../persistence/jpa/entities/EventEntity.java | 74 ++++++++++++------- .../src/main/resources/db-master.xml | 1 + .../resources/db/updates/dw_alter_event_2.xml | 62 ++++++++++++++++ .../org/orcid/persistence/EventDaoTest.java | 68 ++++++++--------- .../main/resources/data/EventEntityData.xml | 12 +-- 9 files changed, 166 insertions(+), 127 deletions(-) create mode 100644 orcid-persistence/src/main/resources/db/updates/dw_alter_event_2.xml diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java index 323f76a9534..04593fce26c 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java @@ -1,10 +1,9 @@ package org.orcid.core.common.manager; -import org.orcid.core.utils.EventType; -import org.orcid.pojo.ajaxForm.RequestInfoForm; - import javax.servlet.http.HttpServletRequest; +import org.orcid.core.utils.EventType; + /** * * @author Daniel Palafox @@ -12,8 +11,6 @@ */ public interface EventManager { - boolean removeEvents(String orcid); - void createEvent(String orcid, EventType eventType, HttpServletRequest request); } diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java index 96b6b540bf9..4fbc9291f1e 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java @@ -1,5 +1,10 @@ package org.orcid.core.common.manager.impl; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.Date; + import javax.annotation.Resource; import javax.servlet.http.HttpServletRequest; @@ -17,10 +22,6 @@ import org.orcid.pojo.ajaxForm.PojoUtil; import org.orcid.pojo.ajaxForm.RequestInfoForm; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; - /** * * @author Daniel Palafox @@ -35,18 +36,12 @@ public class EventManagerImpl implements EventManager { private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; @Resource(name = "recordNameManagerReadOnlyV3") - private RecordNameManagerReadOnly recordNameManagerReadOnly; - - @Override - public boolean removeEvents(String orcid) { - return eventDao.removeEvents(orcid); - } + private RecordNameManagerReadOnly recordNameManagerReadOnly; @Override public void createEvent(String orcid, EventType eventType, HttpServletRequest request) { String label = "Website"; String clientId = null; - String redirectUrl = null; String publicPage = null; if (eventType == EventType.PUBLIC_PAGE) { @@ -58,12 +53,10 @@ public void createEvent(String orcid, EventType eventType, HttpServletRequest re RequestInfoForm requestInfoForm = (RequestInfoForm) request.getSession().getAttribute("requestInfoForm"); if (requestInfoForm != null) { clientId = requestInfoForm.getClientId(); - redirectUrl = removeAttributesFromUrl(requestInfoForm.getRedirectUrl()); label = "OAuth " + requestInfoForm.getMemberName() + " " + requestInfoForm.getClientName(); } else if (isOauth2ScreensRequest != null && isOauth2ScreensRequest) { String queryString = (String) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_QUERY_STRING); clientId = getParameterValue(queryString, "client_id"); - redirectUrl = getParameterValue(queryString, "redirect_uri"); ClientDetailsEntity clientDetailsEntity = clientDetailsEntityCacheManager.retrieve(clientId); String memberName = ""; String clientName = clientDetailsEntity.getClientName(); @@ -87,13 +80,10 @@ public void createEvent(String orcid, EventType eventType, HttpServletRequest re EventEntity eventEntity = new EventEntity(); - eventEntity.setOrcid(orcid); eventEntity.setEventType(eventType.getValue()); eventEntity.setClientId(clientId); - eventEntity.setRedirectUrl(redirectUrl); eventEntity.setLabel(label); - eventEntity.setPublicPage(publicPage); - + eventEntity.setDateCreated(new Date()); eventDao.createEvent(eventEntity); } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/EventDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/EventDao.java index 99787a759c5..93108c62255 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/EventDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/EventDao.java @@ -2,19 +2,14 @@ import org.orcid.persistence.jpa.entities.EventEntity; -import java.util.List; - /** * * @author Daniel Palafox * */ -public interface EventDao extends GenericDao{ - - boolean removeEvents(String orcid); - - List getEvents(String orcid); +public interface EventDao { void createEvent(EventEntity eventEntity); + EventEntity find(long id); } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/EventDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/EventDaoImpl.java index 2d74fee4168..6a07a45c018 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/EventDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/EventDaoImpl.java @@ -1,32 +1,23 @@ package org.orcid.persistence.dao.impl; -import org.orcid.persistence.aop.UpdateProfileLastModified; +import javax.annotation.Resource; +import javax.persistence.EntityManager; + import org.orcid.persistence.dao.EventDao; -import org.orcid.persistence.jpa.entities.EmailEntity; import org.orcid.persistence.jpa.entities.EventEntity; -import org.orcid.persistence.jpa.entities.SpamEntity; import org.springframework.transaction.annotation.Transactional; -import javax.persistence.Query; -import javax.persistence.TypedQuery; -import java.util.List; - /** * @author Daniel Palafox */ -public class EventDaoImpl extends GenericDaoImpl implements EventDao { +public class EventDaoImpl implements EventDao { + @Resource(name="entityManager") + protected EntityManager entityManager; + public EventDaoImpl() { - super(EventEntity.class); - } - - @Override - public List getEvents(String orcid) { - TypedQuery query = entityManager.createQuery("from EventEntity where orcid=:orcid", EventEntity.class); - query.setParameter("orcid", orcid); - List results = query.getResultList(); - return results.isEmpty() ? null : results; - } + + } @Override @Transactional @@ -35,11 +26,8 @@ public void createEvent(EventEntity eventEntity) { } @Override - @Transactional - public boolean removeEvents(String orcid) { - Query query = entityManager.createQuery("delete from EventEntity where orcid = :orcid"); - query.setParameter("orcid", orcid); - query.executeUpdate(); - return query.executeUpdate() > 0; + public EventEntity find(long id) { + return entityManager.find(EventEntity.class, id); } + } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/EventEntity.java b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/EventEntity.java index 019a710041c..c656f6fd44b 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/EventEntity.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/EventEntity.java @@ -1,5 +1,8 @@ package org.orcid.persistence.jpa.entities; +import java.util.Date; +import java.util.Objects; + import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.GeneratedValue; @@ -15,15 +18,13 @@ */ @Entity @Table(name = "event") -public class EventEntity extends BaseEntity implements OrcidAware { +public class EventEntity { private static final long serialVersionUID = 1L; private Long id; - private String orcid; - private String eventType; private String clientId; - private String redirectUrl; + private String eventType; private String label; - private String publicPage; + private Date dateCreated; @Id @Column(name = "id") @@ -37,37 +38,58 @@ public void setId(Long id) { this.id = id; } - @Column(name = "orcid") - public String getOrcid() { - return orcid; + @Column(name = "event_type") + public String getEventType() { + return eventType; } - public void setOrcid(String orcid) { - this.orcid = orcid; + public void setEventType(String eventType) { + this.eventType = eventType; } - @Column(name = "event_type") - public String getEventType() { return eventType; } - - public void setEventType(String eventType) { this.eventType = eventType; } - @Column(name = "client_id") - public String getClientId() { return clientId; } + public String getClientId() { + return clientId; + } + + public void setClientId(String client_id) { + this.clientId = client_id; + } - public void setClientId(String client_id) { this.clientId = client_id; } + @Column(name = "label") + public String getLabel() { + return label; + } - @Column(name = "redirect_url") - public String getRedirectUrl() { return redirectUrl; } + public void setLabel(String label) { + this.label = label; + } - public void setRedirectUrl(String redirect_url) { this.redirectUrl = redirect_url; } + @Column(name = "date_created") + public Date getDateCreated() { + return dateCreated; + } - @Column(name = "label") - public String getLabel() { return label; } + public void setDateCreated(Date date) { + this.dateCreated = date; + } - public void setLabel(String label) { this.label = label; } + @Override + public int hashCode() { + return Objects.hash(clientId, dateCreated, eventType, id, label); + } - @Column(name = "public_page") - public String getPublicPage() { return publicPage; } + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + EventEntity other = (EventEntity) obj; + return Objects.equals(clientId, other.clientId) && Objects.equals(dateCreated, other.dateCreated) && Objects.equals(eventType, other.eventType) + && Objects.equals(id, other.id) && Objects.equals(label, other.label); + } - public void setPublicPage(String public_page) { this.publicPage = public_page; } } diff --git a/orcid-persistence/src/main/resources/db-master.xml b/orcid-persistence/src/main/resources/db-master.xml index da8321423b4..0fd77537b67 100644 --- a/orcid-persistence/src/main/resources/db-master.xml +++ b/orcid-persistence/src/main/resources/db-master.xml @@ -378,4 +378,5 @@ + diff --git a/orcid-persistence/src/main/resources/db/updates/dw_alter_event_2.xml b/orcid-persistence/src/main/resources/db/updates/dw_alter_event_2.xml new file mode 100644 index 00000000000..10211c3dec7 --- /dev/null +++ b/orcid-persistence/src/main/resources/db/updates/dw_alter_event_2.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + SELECT event_type, client_id, COUNT(id), DATE_TRUNC('day', date_created), DATE_TRUNC('day', date_created) as last_modified + FROM event + GROUP BY event_type, client_id, DATE_TRUNC('day', date_created) + ORDER BY DATE_TRUNC('day', date_created) DESC; + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/orcid-persistence/src/test/java/org/orcid/persistence/EventDaoTest.java b/orcid-persistence/src/test/java/org/orcid/persistence/EventDaoTest.java index 84ff1491e49..a48fd69fea6 100644 --- a/orcid-persistence/src/test/java/org/orcid/persistence/EventDaoTest.java +++ b/orcid-persistence/src/test/java/org/orcid/persistence/EventDaoTest.java @@ -1,36 +1,31 @@ package org.orcid.persistence; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import java.util.Arrays; +import java.util.Date; + +import javax.annotation.Resource; + import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.orcid.persistence.dao.EventDao; -import org.orcid.persistence.dao.SpamDao; import org.orcid.persistence.jpa.entities.EventEntity; -import org.orcid.persistence.jpa.entities.SourceType; -import org.orcid.persistence.jpa.entities.SpamEntity; import org.orcid.test.DBUnitTest; import org.orcid.test.OrcidJUnit4ClassRunner; import org.springframework.test.context.ContextConfiguration; -import javax.annotation.Resource; -import javax.persistence.NoResultException; -import javax.transaction.Transactional; -import java.util.Arrays; -import java.util.Date; -import java.util.List; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; - @RunWith(OrcidJUnit4ClassRunner.class) @ContextConfiguration(inheritInitializers = false, inheritLocations = false, locations = {"classpath:test-orcid-persistence-context.xml"}) public class EventDaoTest extends DBUnitTest { - private static String USER_ORCID = "4444-4444-4444-4497"; - private static String OTHER_USER_ORCID = "4444-4444-4444-4499"; - + private static String CLIENT_ID = "APP-5555555555555555"; + @Resource(name = "eventDao") private EventDao eventDao; @@ -45,33 +40,28 @@ public static void removeDBUnitData() throws Exception { } @Test - @Transactional - public void testFindByOrcid() { - List eventEntityList = eventDao.getEvents(OTHER_USER_ORCID); - assertNotNull(eventEntityList); - assertEquals(OTHER_USER_ORCID, eventEntityList.get(0).getOrcid()); - } - - @Test - public void testWriteSpam() throws IllegalAccessException { + public void testWriteEvent() throws IllegalAccessException { EventEntity eventEntity = new EventEntity(); eventEntity.setEventType("Sign-In"); - Date date = new Date(); - FieldUtils.writeField(eventEntity, "dateCreated", date, true); - FieldUtils.writeField(eventEntity, "lastModified", date, true); - eventEntity.setOrcid(USER_ORCID); + eventEntity.setLabel("This is the label"); + FieldUtils.writeField(eventEntity, "dateCreated", new Date(), true); + eventEntity.setClientId(CLIENT_ID); - eventDao.createEvent(eventEntity); + // Id should be null before creating the event + assertNull(eventEntity.getId()); - List eventEntities = eventDao.getEvents(USER_ORCID); - assertNotNull(eventEntities); - assertEquals(USER_ORCID, eventEntities.get(0).getOrcid()); - } + eventDao.createEvent(eventEntity); - @Test - public void testRemoveSpam() throws NoResultException { - List eventEntities = eventDao.getEvents(USER_ORCID); - assertNotNull(eventEntities); - eventDao.removeEvents(eventEntities.get(0).getOrcid()); + // Id should be populated now + assertNotNull(eventEntity.getId()); + + EventEntity fromDb = eventDao.find(eventEntity.getId()); + assertNotNull(fromDb); + assertEquals(eventEntity.getClientId(), fromDb.getClientId()); + assertEquals(eventEntity.getEventType(), fromDb.getEventType()); + assertEquals(eventEntity.getId(), fromDb.getId()); + assertEquals(eventEntity.getLabel(), fromDb.getLabel()); + assertNotNull(fromDb.getDateCreated()); } + } diff --git a/orcid-test/src/main/resources/data/EventEntityData.xml b/orcid-test/src/main/resources/data/EventEntityData.xml index c5fa142a18e..9ac989cc1f1 100644 --- a/orcid-test/src/main/resources/data/EventEntityData.xml +++ b/orcid-test/src/main/resources/data/EventEntityData.xml @@ -1,27 +1,21 @@ From 7b4062c9c1ad522938038988e844d94732b30bc8 Mon Sep 17 00:00:00 2001 From: github actions Date: Tue, 14 Nov 2023 20:21:44 +0000 Subject: [PATCH 11/23] v2.44.9 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3ca315bbab..cd4835e4a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.9 - 2023-11-14 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.8...v2.44.9) + +- [#6930](https://github.com/ORCID/ORCID-Source/pull/6930): Remove orcid, redirect_url, public_page and last_modified from event + ## v2.44.8 - 2023-11-14 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.7...v2.44.8) From e3756840cf298946e674b68e3655666d819ec55d Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 15 Nov 2023 18:44:35 -0600 Subject: [PATCH 12/23] Do a single query to extract non verified emails that have not been notified yet --- .../org/orcid/persistence/dao/ProfileDao.java | 4 +- .../persistence/dao/impl/ProfileDaoImpl.java | 69 ++++++-------- .../orcid/persistence/dao/ProfileDaoTest.java | 68 +++++++------- .../cli/manager/EmailMessageSenderImpl.java | 92 ++++--------------- 4 files changed, 82 insertions(+), 151 deletions(-) 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..0eca19bc9ff 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,22 @@ 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 orcid, email, is_verified \n" + + "from email \n" + + "where email in \n" + + " (SELECT distinct(e.email) \n" + + " FROM email e, email_event ev, profile p \n" + + " WHERE e.is_verified = false \n" + + " AND p.orcid = e.orcid \n" + + " AND p.deprecated_date is null \n" + + " AND p.profile_deactivation_date is null \n" + + " AND p.account_expiry is null \n" + + " 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)) \n" + + " AND NOT EXISTS \n" + + " (SELECT email FROM email_event x WHERE x.email = e.email AND email_event_type IN ('{EVENT_SENT}')\n" + + " )\n" + + ") order by last_modified;"; + @Value("${org.orcid.postgres.query.timeout:30000}") private Integer queryTimeout; @@ -136,47 +152,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..0eaf5017474 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; @@ -17,7 +16,6 @@ import javax.persistence.EntityManager; import javax.persistence.Query; -import org.apache.commons.lang3.tuple.Triple; import org.dbunit.dataset.DataSetException; import org.joda.time.LocalDateTime; import org.junit.AfterClass; @@ -26,11 +24,11 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.orcid.persistence.jpa.entities.EmailEventEntity; -import org.orcid.persistence.jpa.entities.EmailEventType; import org.orcid.persistence.jpa.entities.IndexingStatus; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.persistence.jpa.entities.ProfileEventEntity; import org.orcid.persistence.jpa.entities.ProfileEventType; +import org.orcid.persistence.util.Quadruple; import org.orcid.test.DBUnitTest; import org.orcid.test.OrcidJUnit4ClassRunner; import org.springframework.test.annotation.Rollback; @@ -393,51 +391,57 @@ public void findEmailsUnverfiedDaysTest() throws IllegalAccessException { // 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 2 days ago + assertEquals(1, insertEmailWithDateCreated("unverified_2@test.orcid.org", "95770578974f683fb05c179a84f57c3fc7d4b260f8079fbc590080e51873bb67", orcid, false, LocalDateTime.now().minusDays(2).toDate())); + + // Created 7 days ago + assertEquals(1, insertEmailWithDateCreated("unverified_3@test.orcid.org", "3cbebfc1de2500494fc95553c956e757cb1998149d366afb71888cdeb1550719", 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())); + assertEquals(1, insertEmailWithDateCreated("unverified_4@test.orcid.org", "8fccc2e14b546b968033dee2efb1644623a31a7878b55ae8ad52951961ca3719", orcid, false, LocalDateTime.now().minusDays(15).toDate())); + + // Created 2 days ago and verified + assertEquals(1, insertEmailWithDateCreated("verified_1@test.orcid.org", "2f4812b9c675e9803a4bb616dd1bc241c8c9302ba5690a1ea9d48049a32e7c5f", orcid, true, LocalDateTime.now().minusDays(2).toDate())); // Created 7 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_1@test.orcid.org", "2f4812b9c675e9803a4bb616dd1bc241c8c9302ba5690a1ea9d48049a32e7c5f", orcid, true, LocalDateTime.now().minusDays(7).toDate())); + assertEquals(1, insertEmailWithDateCreated("verified_2@test.orcid.org", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", 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())); + assertEquals(1, insertEmailWithDateCreated("verified_3@test.orcid.org", "f98cce12446df199b852583ce677ecf9870ebe1b58df21bc4e7b01dea67daf01", orcid, true, LocalDateTime.now().minusDays(15).toDate())); - List> results = profileDao.findEmailsUnverfiedDays(7, 100); + List> results = profileDao.findEmailsUnverfiedDays(2); assertNotNull(results); - assertEquals(2, results.size()); + assertEquals(1, results.size()); + assertEquals(orcid, results.get(0).getFirst()); + assertEquals("unverified_2@test.orcid.org", results.get(0).getSecond()); - boolean found1 = false, found2 = false; + results = profileDao.findEmailsUnverfiedDays(7); + assertNotNull(results); + assertEquals(1, results.size()); + assertEquals(orcid, results.get(0).getFirst()); + assertEquals("unverified_3@test.orcid.org", results.get(0).getSecond()); - 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()); - } - } + results = profileDao.findEmailsUnverfiedDays(15); + assertNotNull(results); + assertEquals(1, results.size()); + assertEquals(orcid, results.get(0).getFirst()); + assertEquals("unverified_4@test.orcid.org", results.get(0).getSecond()); - assertTrue(found1); - assertTrue(found2); + results = profileDao.findEmailsUnverfiedDays(6); + assertNotNull(results); + assertTrue(results.isEmpty()); - // 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(8); + assertNotNull(results); + assertTrue(results.isEmpty()); - results = profileDao.findEmailsUnverfiedDays(7, 100); + results = profileDao.findEmailsUnverfiedDays(14); assertNotNull(results); - assertEquals(1, results.size()); - assertEquals("unverified_3@test.orcid.org", results.get(0).getLeft()); + assertTrue(results.isEmpty()); - // 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); + results = profileDao.findEmailsUnverfiedDays(16); assertNotNull(results); - assertTrue(results.isEmpty()); + assertTrue(results.isEmpty()); } private int insertEmailWithDateCreated(String email, String emailHash, String orcid, boolean isVerified, Date dateCreated) { 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); From f3fe4fc31867583ba5a51113bf9cbe91beda031b Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 15 Nov 2023 19:05:47 -0600 Subject: [PATCH 13/23] Query # 1 --- .../persistence/dao/impl/ProfileDaoImpl.java | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) 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 0eca19bc9ff..cec2dbc4834 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 @@ -30,21 +30,17 @@ 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 orcid, email, is_verified \n" - + "from email \n" - + "where email in \n" - + " (SELECT distinct(e.email) \n" - + " FROM email e, email_event ev, profile p \n" - + " WHERE e.is_verified = false \n" - + " AND p.orcid = e.orcid \n" - + " AND p.deprecated_date is null \n" - + " AND p.profile_deactivation_date is null \n" - + " AND p.account_expiry is null \n" - + " 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)) \n" - + " AND NOT EXISTS \n" - + " (SELECT email FROM email_event x WHERE x.email = e.email AND email_event_type IN ('{EVENT_SENT}')\n" - + " )\n" - + ") order by last_modified;"; + private static final String FIND_EMAILS_TO_SEND_VERIFICATION_REMINTER = "SELECT e.orcid, e.email, e.is_verified" + + "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; From 28879e79fc615c54fab83c0a9affeeae304d5a08 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 15 Nov 2023 19:29:03 -0600 Subject: [PATCH 14/23] Looks like it is working now --- .../persistence/dao/impl/ProfileDaoImpl.java | 6 +-- .../orcid/persistence/dao/ProfileDaoTest.java | 39 ++++++------------- 2 files changed, 15 insertions(+), 30 deletions(-) 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 cec2dbc4834..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 @@ -30,16 +30,16 @@ 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_verified" + 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 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}'))" + + " (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}") 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 0eaf5017474..c1d8d0565af 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 @@ -16,6 +16,7 @@ import javax.persistence.EntityManager; import javax.persistence.Query; +import org.apache.commons.lang3.tuple.Triple; import org.dbunit.dataset.DataSetException; import org.joda.time.LocalDateTime; import org.junit.AfterClass; @@ -24,11 +25,11 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.orcid.persistence.jpa.entities.EmailEventEntity; +import org.orcid.persistence.jpa.entities.EmailEventType; import org.orcid.persistence.jpa.entities.IndexingStatus; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.persistence.jpa.entities.ProfileEventEntity; import org.orcid.persistence.jpa.entities.ProfileEventType; -import org.orcid.persistence.util.Quadruple; import org.orcid.test.DBUnitTest; import org.orcid.test.OrcidJUnit4ClassRunner; import org.springframework.test.annotation.Rollback; @@ -407,41 +408,25 @@ public void findEmailsUnverfiedDaysTest() throws IllegalAccessException { assertEquals(1, insertEmailWithDateCreated("verified_2@test.orcid.org", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", orcid, true, LocalDateTime.now().minusDays(7).toDate())); // Created 15 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_3@test.orcid.org", "f98cce12446df199b852583ce677ecf9870ebe1b58df21bc4e7b01dea67daf01", orcid, true, LocalDateTime.now().minusDays(15).toDate())); + assertEquals(1, insertEmailWithDateCreated("verified_3@test.orcid.org", "f98cce12446df199b852583ce677ecf9870ebe1b58df21bc4e7b01dea67daf01", orcid, true, LocalDateTime.now().minusDays(28).toDate())); - List> results = profileDao.findEmailsUnverfiedDays(2); + List> results = profileDao.findEmailsUnverfiedDays(2, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT); assertNotNull(results); assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getFirst()); - assertEquals("unverified_2@test.orcid.org", results.get(0).getSecond()); + assertEquals(orcid, results.get(0).getLeft()); + assertEquals("unverified_2@test.orcid.org", results.get(0).getMiddle()); - results = profileDao.findEmailsUnverfiedDays(7); + results = profileDao.findEmailsUnverfiedDays(7, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT); assertNotNull(results); assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getFirst()); - assertEquals("unverified_3@test.orcid.org", results.get(0).getSecond()); + assertEquals(orcid, results.get(0).getLeft()); + assertEquals("unverified_3@test.orcid.org", results.get(0).getMiddle()); - results = profileDao.findEmailsUnverfiedDays(15); + results = profileDao.findEmailsUnverfiedDays(28, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT); assertNotNull(results); assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getFirst()); - assertEquals("unverified_4@test.orcid.org", results.get(0).getSecond()); - - results = profileDao.findEmailsUnverfiedDays(6); - assertNotNull(results); - assertTrue(results.isEmpty()); - - results = profileDao.findEmailsUnverfiedDays(8); - assertNotNull(results); - assertTrue(results.isEmpty()); - - results = profileDao.findEmailsUnverfiedDays(14); - assertNotNull(results); - assertTrue(results.isEmpty()); - - results = profileDao.findEmailsUnverfiedDays(16); - assertNotNull(results); - assertTrue(results.isEmpty()); + assertEquals(orcid, results.get(0).getLeft()); + assertEquals("unverified_4@test.orcid.org", results.get(0).getMiddle()); } private int insertEmailWithDateCreated(String email, String emailHash, String orcid, boolean isVerified, Date dateCreated) { From 5ba8f2a0ff888f352f1a8bb5005249e023df2b56 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 15 Nov 2023 19:46:04 -0600 Subject: [PATCH 15/23] date_trunc function is not available in the in memory db, so, we cannot run that query there --- .../orcid/persistence/dao/ProfileDaoTest.java | 71 +------------------ 1 file changed, 1 insertion(+), 70 deletions(-) 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 c1d8d0565af..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 @@ -375,74 +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 2 days ago - assertEquals(1, insertEmailWithDateCreated("unverified_2@test.orcid.org", "95770578974f683fb05c179a84f57c3fc7d4b260f8079fbc590080e51873bb67", orcid, false, LocalDateTime.now().minusDays(2).toDate())); - - // Created 7 days ago - assertEquals(1, insertEmailWithDateCreated("unverified_3@test.orcid.org", "3cbebfc1de2500494fc95553c956e757cb1998149d366afb71888cdeb1550719", orcid, false, LocalDateTime.now().minusDays(7).toDate())); - - // Created 15 days ago - assertEquals(1, insertEmailWithDateCreated("unverified_4@test.orcid.org", "8fccc2e14b546b968033dee2efb1644623a31a7878b55ae8ad52951961ca3719", orcid, false, LocalDateTime.now().minusDays(15).toDate())); - - // Created 2 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_1@test.orcid.org", "2f4812b9c675e9803a4bb616dd1bc241c8c9302ba5690a1ea9d48049a32e7c5f", orcid, true, LocalDateTime.now().minusDays(2).toDate())); - - // Created 7 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_2@test.orcid.org", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", orcid, true, LocalDateTime.now().minusDays(7).toDate())); - - // Created 15 days ago and verified - assertEquals(1, insertEmailWithDateCreated("verified_3@test.orcid.org", "f98cce12446df199b852583ce677ecf9870ebe1b58df21bc4e7b01dea67daf01", orcid, true, LocalDateTime.now().minusDays(28).toDate())); - - List> results = profileDao.findEmailsUnverfiedDays(2, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT); - assertNotNull(results); - assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getLeft()); - assertEquals("unverified_2@test.orcid.org", results.get(0).getMiddle()); - - results = profileDao.findEmailsUnverfiedDays(7, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT); - assertNotNull(results); - assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getLeft()); - assertEquals("unverified_3@test.orcid.org", results.get(0).getMiddle()); - - results = profileDao.findEmailsUnverfiedDays(28, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT); - assertNotNull(results); - assertEquals(1, results.size()); - assertEquals(orcid, results.get(0).getLeft()); - assertEquals("unverified_4@test.orcid.org", results.get(0).getMiddle()); - } - - 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(); - } + } } From ee62a5f185bd195ed461b3c3f816dd77d135c4ff Mon Sep 17 00:00:00 2001 From: github actions Date: Thu, 16 Nov 2023 02:59:50 +0000 Subject: [PATCH 16/23] v2.44.10 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) 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) From a32ba402566db96b15d5d8575fae549511e3dc81 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 16 Nov 2023 08:02:51 -0600 Subject: [PATCH 17/23] Add missing keys --- .../src/main/resources/i18n/email_common_lr.properties | 5 ++++- .../src/main/resources/i18n/email_common_rl.properties | 3 +++ .../src/main/resources/i18n/email_common_xx.properties | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/orcid-core/src/main/resources/i18n/email_common_lr.properties b/orcid-core/src/main/resources/i18n/email_common_lr.properties index 6834ef87286..8aae8bdbea8 100644 --- a/orcid-core/src/main/resources/i18n/email_common_lr.properties +++ b/orcid-core/src/main/resources/i18n/email_common_lr.properties @@ -41,4 +41,7 @@ email.common.email.preferences=LR email.common.privacy_policy=LR email.common.privacy_policy_2=LR email.common.address1=LR -email.common.address2=LR \ No newline at end of file +email.common.address2=LR +email.welcome.your_id.id=LR +email.welcome.your_id.link=LR +email.common.received_email_as_service=LR \ No newline at end of file diff --git a/orcid-core/src/main/resources/i18n/email_common_rl.properties b/orcid-core/src/main/resources/i18n/email_common_rl.properties index 48376543ff2..6c5002e123d 100644 --- a/orcid-core/src/main/resources/i18n/email_common_rl.properties +++ b/orcid-core/src/main/resources/i18n/email_common_rl.properties @@ -42,3 +42,6 @@ email.common.privacy_policy=RL email.common.privacy_policy_2=RL email.common.address1=RL email.common.address2=RL +email.welcome.your_id.id=RL +email.welcome.your_id.link=RL +email.common.received_email_as_service=RL \ No newline at end of file diff --git a/orcid-core/src/main/resources/i18n/email_common_xx.properties b/orcid-core/src/main/resources/i18n/email_common_xx.properties index 909e53485f0..42aafaa374d 100644 --- a/orcid-core/src/main/resources/i18n/email_common_xx.properties +++ b/orcid-core/src/main/resources/i18n/email_common_xx.properties @@ -42,3 +42,6 @@ email.common.privacy_policy=X email.common.privacy_policy_2=X email.common.address1=X email.common.address2=X +email.welcome.your_id.id=X +email.welcome.your_id.link=X +email.common.received_email_as_service=X \ No newline at end of file From b531efe2d45b2911fdbf64317d3656ad75a8a4b0 Mon Sep 17 00:00:00 2001 From: github actions Date: Thu, 16 Nov 2023 15:43:39 +0000 Subject: [PATCH 18/23] v2.44.11 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddfcbfecd4a..f919f5c2807 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v2.44.11 - 2023-11-16 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.10...v2.44.11) + +- [#6933](https://github.com/ORCID/ORCID-Source/pull/6933): Add missing keys + ## v2.44.10 - 2023-11-16 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.9...v2.44.10) From 60faa2784e2baf47254eef58f757bdd728fd3535 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 16 Nov 2023 09:58:27 -0600 Subject: [PATCH 19/23] Fix NPE on social sign in --- .../core/common/manager/EventManager.java | 2 +- .../common/manager/impl/EventManagerImpl.java | 52 ++++++++----------- .../web/controllers/LoginController.java | 12 ++--- 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java index 04593fce26c..51439b40d4b 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/EventManager.java @@ -11,6 +11,6 @@ */ public interface EventManager { - void createEvent(String orcid, EventType eventType, HttpServletRequest request); + void createEvent(EventType eventType, HttpServletRequest request); } diff --git a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java index 4fbc9291f1e..82cd09d7501 100644 --- a/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/common/manager/impl/EventManagerImpl.java @@ -39,42 +39,36 @@ public class EventManagerImpl implements EventManager { private RecordNameManagerReadOnly recordNameManagerReadOnly; @Override - public void createEvent(String orcid, EventType eventType, HttpServletRequest request) { + public void createEvent(EventType eventType, HttpServletRequest request) { String label = "Website"; String clientId = null; - String publicPage = null; - if (eventType == EventType.PUBLIC_PAGE) { - publicPage = orcid; - orcid = null; - } else { - if (request != null) { - Boolean isOauth2ScreensRequest = (Boolean) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_2SCREENS); - RequestInfoForm requestInfoForm = (RequestInfoForm) request.getSession().getAttribute("requestInfoForm"); - if (requestInfoForm != null) { - clientId = requestInfoForm.getClientId(); - label = "OAuth " + requestInfoForm.getMemberName() + " " + requestInfoForm.getClientName(); - } else if (isOauth2ScreensRequest != null && isOauth2ScreensRequest) { - String queryString = (String) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_QUERY_STRING); - clientId = getParameterValue(queryString, "client_id"); - ClientDetailsEntity clientDetailsEntity = clientDetailsEntityCacheManager.retrieve(clientId); - String memberName = ""; - String clientName = clientDetailsEntity.getClientName(); + if (request != null) { + Boolean isOauth2ScreensRequest = (Boolean) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_2SCREENS); + RequestInfoForm requestInfoForm = (RequestInfoForm) request.getSession().getAttribute("requestInfoForm"); + if (requestInfoForm != null) { + clientId = requestInfoForm.getClientId(); + label = "OAuth " + requestInfoForm.getMemberName() + " " + requestInfoForm.getClientName(); + } else if (isOauth2ScreensRequest != null && isOauth2ScreensRequest) { + String queryString = (String) request.getSession().getAttribute(OrcidOauth2Constants.OAUTH_QUERY_STRING); + clientId = getParameterValue(queryString, "client_id"); + ClientDetailsEntity clientDetailsEntity = clientDetailsEntityCacheManager.retrieve(clientId); + String memberName = ""; + String clientName = clientDetailsEntity.getClientName(); - if (ClientType.PUBLIC_CLIENT.equals(clientDetailsEntity.getClientType())) { - memberName = "PubApp"; - } else if (!PojoUtil.isEmpty(clientDetailsEntity.getGroupProfileId())) { - Name name = recordNameManagerReadOnly.getRecordName(clientDetailsEntity.getGroupProfileId()); - if (name != null) { - memberName = name.getCreditName() != null ? name.getCreditName().getContent() : ""; - } + if (ClientType.PUBLIC_CLIENT.equals(clientDetailsEntity.getClientType())) { + memberName = "PubApp"; + } else if (!PojoUtil.isEmpty(clientDetailsEntity.getGroupProfileId())) { + Name name = recordNameManagerReadOnly.getRecordName(clientDetailsEntity.getGroupProfileId()); + if (name != null) { + memberName = name.getCreditName() != null ? name.getCreditName().getContent() : ""; } + } - if (StringUtils.isBlank(memberName)) { - memberName = clientName; - } - label = "OAuth " + memberName + " " + clientName; + if (StringUtils.isBlank(memberName)) { + memberName = clientName; } + label = "OAuth " + memberName + " " + clientName; } } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java index 98a34a18d04..3184290f6f7 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/LoginController.java @@ -3,7 +3,6 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; -import java.util.Iterator; import java.util.UUID; import javax.annotation.Resource; @@ -335,7 +334,7 @@ private ModelAndView processSocialLogin(HttpServletRequest request, HttpServletR } else { // Forward to account link page view = socialLinking(request); - } + } } else { // Store relevant data in the session socialSignInUtils.setSignedInData(request, userData); @@ -343,16 +342,17 @@ private ModelAndView processSocialLogin(HttpServletRequest request, HttpServletR userConnectionId = createUserConnection(socialType, providerUserId, userData.getString(OrcidOauth2Constants.EMAIL), userData.getString(OrcidOauth2Constants.DISPLAY_NAME), accessToken, expiresIn); // Forward to account link page - view = socialLinking(request); + view = socialLinking(request); } if (userConnectionId == null) { throw new IllegalArgumentException("Unable to find userConnectionId for providerUserId = " + providerUserId); } + if (Features.EVENTS.isActive()) { - eventManager.createEvent(userConnection.getOrcid(), EventType.SIGN_IN, request); + eventManager.createEvent(EventType.SIGN_IN, request); } - userCookieGenerator.addCookie(userConnectionId, response); - + userCookieGenerator.addCookie(userConnectionId, response); + if ("social_2FA".equals(view.getViewName())) { return new ModelAndView("redirect:" + calculateRedirectUrl("/2fa-signin?social=true")); } From 963a781b8810762a5a99f05045ba876b64f42c9f Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 16 Nov 2023 10:29:15 -0600 Subject: [PATCH 20/23] Fix compilation issues --- .../main/java/org/orcid/frontend/oauth2/OauthController.java | 2 +- .../orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java | 2 +- .../frontend/web/controllers/OauthAuthorizeController.java | 2 +- .../orcid/frontend/web/controllers/PublicRecordController.java | 2 +- .../orcid/frontend/web/controllers/RegistrationController.java | 2 +- .../orcid/frontend/web/controllers/ShibbolethController.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java b/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java index f11d4ef4f55..a544d63b92f 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/oauth2/OauthController.java @@ -97,7 +97,7 @@ public class OauthController { if (responseParam != null && !responseParam.isEmpty() && !PojoUtil.isEmpty(responseParam.get(0))) { isResponseSet = true; if (Features.EVENTS.isActive()) { - eventManager.createEvent(requestInfoForm.getUserOrcid(), EventType.REAUTHORIZE, request); + eventManager.createEvent(EventType.REAUTHORIZE, request); } } } diff --git a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java index 6bb187adfbb..599baf46329 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java +++ b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationSuccessHandler.java @@ -26,7 +26,7 @@ public class AjaxAuthenticationSuccessHandler extends AjaxAuthenticationSuccessH public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { String targetUrl = getTargetUrl(request, response, authentication); if (Features.EVENTS.isActive()) { - eventManager.createEvent(authentication.getPrincipal().toString(), EventType.SIGN_IN, request); + eventManager.createEvent(EventType.SIGN_IN, request); } response.setContentType("application/json"); response.getWriter().println("{\"success\": true, \"url\": \"" + targetUrl.replaceAll("^/", "") + "\"}"); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java index 1858acb7338..d1f258b4550 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthAuthorizeController.java @@ -276,7 +276,7 @@ public ModelAndView loginGetHandler(HttpServletRequest request, HttpServletRespo } else { orcid = auth.getPrincipal().toString(); } - eventManager.createEvent(orcid, eventType, request); + eventManager.createEvent(eventType, request); } if(new HttpSessionRequestCache().getRequest(request, response) != null) new HttpSessionRequestCache().removeRequest(request, response); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java index 3f2f78febc4..b4f8ab05aa4 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PublicRecordController.java @@ -185,7 +185,7 @@ PublicRecord getPublicRecord(@PathVariable("orcid") String orcid) { try { if (Features.EVENTS.isActive()) { - eventManager.createEvent(orcid, EventType.PUBLIC_PAGE, null); + eventManager.createEvent(EventType.PUBLIC_PAGE, null); } // Check if the profile is deprecated or locked orcidSecurityManager.checkProfile(orcid); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java index 9979dfd533f..a3e86e1fce9 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RegistrationController.java @@ -282,7 +282,7 @@ public void validateGrcaptcha(HttpServletRequest request, @RequestBody Registrat // Ip String ip = OrcidRequestUtil.getIpAddress(request); if (Features.EVENTS.isActive()) { - eventManager.createEvent(getCurrentUserOrcid(), EventType.NEW_REGISTRATION, request); + eventManager.createEvent(EventType.NEW_REGISTRATION, request); } createMinimalRegistrationAndLogUserIn(request, response, reg, usedCaptcha, locale, ip); } catch (Exception e) { diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java index 9f0dc458c06..818670ab8f6 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ShibbolethController.java @@ -173,7 +173,7 @@ public ModelAndView signinHandler(HttpServletRequest request, HttpServletRespons processAuthentication(remoteUser, userConnectionEntity); if (Features.EVENTS.isActive()) { OrcidProfileUserDetails orcidProfileUserDetails = getOrcidProfileUserDetails(userConnectionEntity.getOrcid()); - eventManager.createEvent(orcidProfileUserDetails.getOrcid(), EventType.SIGN_IN, request); + eventManager.createEvent(EventType.SIGN_IN, request); } } catch (AuthenticationException e) { // this should never happen From c21173d5f904cc8c493ed8ca5be280821602f127 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 16 Nov 2023 11:00:50 -0600 Subject: [PATCH 21/23] allways send 2 days reminder --- .../email/cli/manager/EmailMessageSenderImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 d25758a8e45..e4ca740e29e 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 @@ -569,19 +569,19 @@ private String getHtmlBody(NotificationAdministrative notificationAdministrative @Override synchronized public void processUnverifiedEmails2Days() { - processUnverifiedEmails(verifyReminderAfterTwoDays, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED); + processUnverifiedEmails(true, verifyReminderAfterTwoDays, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED); } synchronized public void processUnverifiedEmails7Days() { - processUnverifiedEmails(verifyReminderAfterSevenDays, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED); + processUnverifiedEmails(false, 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); + processUnverifiedEmails(false, 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()) { + private void processUnverifiedEmails(boolean forceSending, int unverifiedDays, EmailEventType sent, EmailEventType failed) { + if (forceSending || Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { LOGGER.info("About to process unverIfied emails for {} days reminder", unverifiedDays); List> elements = Collections.> emptyList(); elements = profileDaoReadOnly.findEmailsUnverfiedDays(unverifiedDays, sent); From 37b49e62c74db558202f1d1530c3fe8d3a2033dc Mon Sep 17 00:00:00 2001 From: Daniel Palafox Date: Thu, 16 Nov 2023 13:12:07 -0500 Subject: [PATCH 22/23] fix: Add missing capture event in shibboleth account link --- .../ShibbolethAjaxAuthenticationSuccessHandler.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/orcid-web/src/main/java/org/orcid/frontend/spring/ShibbolethAjaxAuthenticationSuccessHandler.java b/orcid-web/src/main/java/org/orcid/frontend/spring/ShibbolethAjaxAuthenticationSuccessHandler.java index 35afa2a2818..c515ff5d4a1 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/spring/ShibbolethAjaxAuthenticationSuccessHandler.java +++ b/orcid-web/src/main/java/org/orcid/frontend/spring/ShibbolethAjaxAuthenticationSuccessHandler.java @@ -11,11 +11,15 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.orcid.core.common.manager.EventManager; import org.orcid.core.manager.InstitutionalSignInManager; +import org.orcid.core.togglz.Features; +import org.orcid.core.utils.EventType; import org.orcid.frontend.web.exception.FeatureDisabledException; import org.orcid.pojo.RemoteUser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.security.core.Authentication; @@ -30,10 +34,16 @@ public class ShibbolethAjaxAuthenticationSuccessHandler extends AjaxAuthenticati @Resource private InstitutionalSignInManager institutionalSignInManager; + + @Autowired + EventManager eventManager; public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { linkShibbolethAccount(request, response); String targetUrl = getTargetUrl(request, response, authentication); + if (Features.EVENTS.isActive()) { + eventManager.createEvent(EventType.SIGN_IN, request); + } response.setContentType("application/json"); response.getWriter().println("{\"success\": true, \"url\": \"" + targetUrl.replaceAll("^/", "") + "\"}"); } From e5687dc0b85185e5ad288aa7dbddb7e9b78a15f1 Mon Sep 17 00:00:00 2001 From: github actions Date: Thu, 16 Nov 2023 22:15:27 +0000 Subject: [PATCH 23/23] v2.44.12 changelog update --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f919f5c2807..661257f2524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## v2.44.12 - 2023-11-16 + +[Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.11...v2.44.12) + +- [#6936](https://github.com/ORCID/ORCID-Source/pull/6936): fix: Add missing capture event in shibboleth account link +- [#6935](https://github.com/ORCID/ORCID-Source/pull/6935): allways send 2 days reminder +- [#6934](https://github.com/ORCID/ORCID-Source/pull/6934): Fix NPE on social sign in + +### Fix + +- Add missing capture event in shibboleth account link + ## v2.44.11 - 2023-11-16 [Full Changelog](https://github.com/ORCID/ORCID-Source/compare/v2.44.10...v2.44.11)