From 327948179e7bbe26605e09d6ecfe906e1cc546e0 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Tue, 27 Feb 2024 13:37:16 -0600 Subject: [PATCH 1/4] Deactivated records should get 409 on GET requests --- .../orcid/api/common/jaxb/OrcidExceptionMapper.java | 2 ++ .../impl/MemberV3ApiServiceDelegatorImpl.java | 12 ++---------- .../manager/v3/impl/OrcidSecurityManagerImpl.java | 6 +++--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/orcid-api-common/src/main/java/org/orcid/api/common/jaxb/OrcidExceptionMapper.java b/orcid-api-common/src/main/java/org/orcid/api/common/jaxb/OrcidExceptionMapper.java index 81f74a10d2b..d96850ceb37 100644 --- a/orcid-api-common/src/main/java/org/orcid/api/common/jaxb/OrcidExceptionMapper.java +++ b/orcid-api-common/src/main/java/org/orcid/api/common/jaxb/OrcidExceptionMapper.java @@ -111,6 +111,8 @@ public Response toResponse(Throwable t) { logShortError(t, clientId); } else if (t instanceof LockedException) { logShortError(t, clientId); + } else if (t instanceof DeactivatedException) { + logShortError(t, clientId); } else if (t instanceof ClientDeactivatedException) { logShortError(t, clientId); } else if (t instanceof OrcidNonPublicElementException) { diff --git a/orcid-api-web/src/main/java/org/orcid/api/memberV3/server/delegator/impl/MemberV3ApiServiceDelegatorImpl.java b/orcid-api-web/src/main/java/org/orcid/api/memberV3/server/delegator/impl/MemberV3ApiServiceDelegatorImpl.java index 64c6535d0bf..a3fb41eb9a5 100644 --- a/orcid-api-web/src/main/java/org/orcid/api/memberV3/server/delegator/impl/MemberV3ApiServiceDelegatorImpl.java +++ b/orcid-api-web/src/main/java/org/orcid/api/memberV3/server/delegator/impl/MemberV3ApiServiceDelegatorImpl.java @@ -1642,16 +1642,8 @@ public Response deleteResearchResource(String orcid, Long putCode) { return Response.noContent().build(); } - private void checkProfileStatus(String orcid, boolean readOperation) { - try { - orcidSecurityManager.checkProfile(orcid); - } catch (DeactivatedException e) { - // If it is a read operation, ignore the deactivated status since we - // are going to return the empty element with the deactivation date - if (!readOperation) { - throw e; - } - } + private void checkProfileStatus(String orcid, boolean readOperation) throws DeactivatedException { + orcidSecurityManager.checkProfile(orcid); } private Map addParmsMismatchedPutCode(Long urlPutCode, Long bodyPutCode) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OrcidSecurityManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OrcidSecurityManagerImpl.java index 59b415afdc0..5067b4195e3 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OrcidSecurityManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OrcidSecurityManagerImpl.java @@ -202,15 +202,15 @@ public void checkProfile(String orcid) throws NoResultException, OrcidDeprecated // Check if the user record is locked if (!profile.isAccountNonLocked()) { - LockedException lockedException = new LockedException(); + LockedException lockedException = new LockedException(orcid + " is locked"); lockedException.setOrcid(profile.getId()); throw lockedException; } // Check if the user record is deactivated if (profile.getDeactivationDate() != null) { - DeactivatedException exception = new DeactivatedException(); - exception.setOrcid(orcid); + DeactivatedException exception = new DeactivatedException(orcid + " is deactivated"); + exception.setOrcid(orcid); throw exception; } } From 2f8657f5385a497d9618b16bc3da0f73bae8479d Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 17 Jul 2024 09:48:51 -0600 Subject: [PATCH 2/4] Add better error message for when a issn couldnt be found --- ...mberV2ApiServiceDelegator_GeneralTest.java | 2 +- ...mberV2ApiServiceDelegator_GroupIdTest.java | 10 ++-- ...V2ApiServiceDelegator_PeerReviewsTest.java | 2 +- ...mberV3ApiServiceDelegator_GeneralTest.java | 2 +- ...mberV3ApiServiceDelegator_GroupIdTest.java | 10 ++-- ...V3ApiServiceDelegator_PeerReviewsTest.java | 2 +- .../exception/TooManyRequestsException.java | 17 ++++++ .../orcid/core/groupIds/issn/IssnClient.java | 28 +++++----- .../impl/GroupIdRecordManagerImpl.java | 56 ++++++++++++------- .../v3/impl/GroupIdRecordManagerImpl.java | 55 +++++++++++------- .../manager/impl/IssnLoadManagerImpl.java | 2 +- .../loader/source/issn/IssnLoadSource.java | 36 +++++++++--- 12 files changed, 148 insertions(+), 74 deletions(-) create mode 100644 orcid-core/src/main/java/org/orcid/core/exception/TooManyRequestsException.java diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GeneralTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GeneralTest.java index b98fdf4d9cc..12be925f9b8 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GeneralTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GeneralTest.java @@ -108,7 +108,7 @@ public class MemberV2ApiServiceDelegator_GeneralTest extends DBUnitTest { private IssnClient issnClient; @Before - public void before() { + public void before() throws Exception { MockitoAnnotations.initMocks(this); Map map = new HashMap(); map.put(EmailFrequencyManager.ADMINISTRATIVE_CHANGE_NOTIFICATIONS, String.valueOf(Float.MAX_VALUE)); diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GroupIdTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GroupIdTest.java index 5275a265fc9..f2e4f0d25e3 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GroupIdTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_GroupIdTest.java @@ -94,7 +94,7 @@ public void testGetGroupIdRecord() { } @Test - public void testCreateGroupIdRecord() { + public void testCreateGroupIdRecord() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -208,7 +208,7 @@ public void testFindGroupIdByGroupId() { } @Test - public void testFindGroupIdRecordByNonExistentIssnGroupId() { + public void testFindGroupIdRecordByNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -262,7 +262,7 @@ public void testFindGroupIdRecordByNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithNonExistentIssnGroupId() { + public void testCreateGroupIdRecordWithNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -316,7 +316,7 @@ public void testCreateGroupIdRecordWithNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() { + public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -370,7 +370,7 @@ public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithInvalidIssnGroupId() { + public void testCreateGroupIdRecordWithInvalidIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_PeerReviewsTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_PeerReviewsTest.java index 6e2ba6ad725..91b34708d7c 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_PeerReviewsTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV2/server/delegator/MemberV2ApiServiceDelegator_PeerReviewsTest.java @@ -110,7 +110,7 @@ public class MemberV2ApiServiceDelegator_PeerReviewsTest extends DBUnitTest { private IssnClient issnClient; @Before - public void before() { + public void before() throws Exception { MockitoAnnotations.initMocks(this); Map map = new HashMap(); map.put(EmailFrequencyManager.ADMINISTRATIVE_CHANGE_NOTIFICATIONS, String.valueOf(Float.MAX_VALUE)); diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GeneralTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GeneralTest.java index d3e9359f071..7de4afe0aad 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GeneralTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GeneralTest.java @@ -123,7 +123,7 @@ public class MemberV3ApiServiceDelegator_GeneralTest extends DBUnitTest { private ArgumentCaptor>> searchParamsCaptor; @Before - public void before() { + public void before() throws Exception { MockitoAnnotations.initMocks(this); Map map = new HashMap(); map.put(EmailFrequencyManager.ADMINISTRATIVE_CHANGE_NOTIFICATIONS, String.valueOf(Float.MAX_VALUE)); diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GroupIdTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GroupIdTest.java index 7393ae62964..c30dfd7eb4e 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GroupIdTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_GroupIdTest.java @@ -100,7 +100,7 @@ public void testGetGroupIdRecord() { } @Test - public void testCreateGroupIdRecord() { + public void testCreateGroupIdRecord() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -212,7 +212,7 @@ public void testFindGroupIdByGroupId() { } @Test - public void testFindGroupIdRecordByNonExistentIssnGroupId() { + public void testFindGroupIdRecordByNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -267,7 +267,7 @@ public void testFindGroupIdRecordByNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithNonExistentIssnGroupId() { + public void testCreateGroupIdRecordWithNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -322,7 +322,7 @@ public void testCreateGroupIdRecordWithNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() { + public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); @@ -377,7 +377,7 @@ public void testCreateGroupIdRecordWithAnotherNonExistentIssnGroupId() { } @Test - public void testCreateGroupIdRecordWithInvalidIssnGroupId() { + public void testCreateGroupIdRecordWithInvalidIssnGroupId() throws Exception { GroupIdRecordManager groupIdRecordManager = (GroupIdRecordManager) ReflectionTestUtils.getField(serviceDelegator, "groupIdRecordManager"); GroupIdRecordDao groupIdRecordDao = (GroupIdRecordDao) ReflectionTestUtils.getField(groupIdRecordManager, "groupIdRecordDao"); IssnClient issnClient = (IssnClient) ReflectionTestUtils.getField(groupIdRecordManager, "issnClient"); diff --git a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_PeerReviewsTest.java b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_PeerReviewsTest.java index 86c04e83604..09e3bd54003 100644 --- a/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_PeerReviewsTest.java +++ b/orcid-api-web/src/test/java/org/orcid/api/memberV3/server/delegator/MemberV3ApiServiceDelegator_PeerReviewsTest.java @@ -118,7 +118,7 @@ public class MemberV3ApiServiceDelegator_PeerReviewsTest extends DBUnitTest { private IssnClient issnClient; @Before - public void before() { + public void before() throws Exception { MockitoAnnotations.initMocks(this); Map map = new HashMap(); map.put(EmailFrequencyManager.ADMINISTRATIVE_CHANGE_NOTIFICATIONS, String.valueOf(Float.MAX_VALUE)); diff --git a/orcid-core/src/main/java/org/orcid/core/exception/TooManyRequestsException.java b/orcid-core/src/main/java/org/orcid/core/exception/TooManyRequestsException.java new file mode 100644 index 00000000000..1a40b37daaa --- /dev/null +++ b/orcid-core/src/main/java/org/orcid/core/exception/TooManyRequestsException.java @@ -0,0 +1,17 @@ +package org.orcid.core.exception; + +import java.util.Map; + +public class TooManyRequestsException extends ApplicationException { + + private static final long serialVersionUID = 1L; + + public TooManyRequestsException() { + super(); + } + + public TooManyRequestsException(Map params) { + super(params); + } + +} diff --git a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java index 679c02c8304..e042ebaaa54 100644 --- a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java +++ b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.net.URISyntaxException; import java.net.http.HttpResponse; +import java.util.Map; import javax.annotation.Resource; @@ -10,6 +11,9 @@ import org.codehaus.jettison.json.JSONArray; import org.codehaus.jettison.json.JSONException; import org.codehaus.jettison.json.JSONObject; +import org.eclipse.jetty.http.HttpStatus; +import org.orcid.core.exception.TooManyRequestsException; +import org.orcid.core.exception.UnexpectedResponseCodeException; import org.orcid.core.utils.http.HttpRequestUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,19 +36,12 @@ public class IssnClient { @Resource private HttpRequestUtils httpRequestUtils; - public IssnData getIssnData(String issn) { + public IssnData getIssnData(String issn) throws TooManyRequestsException, UnexpectedResponseCodeException, IOException, URISyntaxException, InterruptedException { if(StringUtils.isEmpty(issn)) { return null; } - String json = null; - try { - LOG.debug("Extracting ISSN for " + issn); - // ensure any lower case x is X otherwise issn portal won't work - json = getJsonDataFromIssnPortal(issn.toUpperCase()); - } catch (IOException | InterruptedException | URISyntaxException e) { - LOG.error("Error when getting the issn data from issn portal " + issn, e); - return null; - } + LOG.debug("Extracting ISSN for " + issn); + String json = getJsonDataFromIssnPortal(issn.toUpperCase()); try { if (json != null) { IssnData data = extractIssnData(issn.toUpperCase(), json); @@ -138,11 +135,16 @@ private IssnData extractIssnData(String issn, String json) throws JSONException throw new IllegalArgumentException("Unable to extract name, couldn't find the Key Title nor the main resource for " + issn); } - private String getJsonDataFromIssnPortal(String issn) throws IOException, InterruptedException, URISyntaxException { + private String getJsonDataFromIssnPortal(String issn) throws TooManyRequestsException, UnexpectedResponseCodeException, IOException, InterruptedException, URISyntaxException { String issnUrl = issnPortalUrlBuilder.buildJsonIssnPortalUrlForIssn(issn); HttpResponse response = httpRequestUtils.doGet(issnUrl); - if (response.statusCode() != 200) { - return null; + if (response.statusCode() != HttpStatus.OK_200) { + if(response.statusCode() == HttpStatus.TOO_MANY_REQUESTS_429) { + throw new TooManyRequestsException(); + } else { + LOG.warn(issnUrl + " returned status code " + response.statusCode()); + throw new UnexpectedResponseCodeException(response.statusCode()); + } } return response.body(); } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java index e02f779ebe6..6e667415956 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java @@ -1,13 +1,12 @@ package org.orcid.core.manager.impl; +import java.io.IOException; +import java.net.URISyntaxException; import java.util.GregorianCalendar; import javax.annotation.Resource; -import org.orcid.core.exception.DuplicatedGroupIdRecordException; -import org.orcid.core.exception.GroupIdRecordNotFoundException; -import org.orcid.core.exception.InvalidIssnException; -import org.orcid.core.exception.OrcidElementCantBeDeletedException; +import org.orcid.core.exception.*; import org.orcid.core.groupIds.issn.IssnClient; import org.orcid.core.groupIds.issn.IssnData; import org.orcid.core.groupIds.issn.IssnValidator; @@ -23,15 +22,16 @@ import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; import org.orcid.persistence.jpa.entities.SourceEntity; import org.orcid.core.utils.DateUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; public class GroupIdRecordManagerImpl extends GroupIdRecordManagerReadOnlyImpl implements GroupIdRecordManager { - @Resource - private SourceManager sourceManager; + private static final Logger LOG = LoggerFactory.getLogger(GroupIdRecordManagerImpl.class); @Resource - private LocaleManager localeManager; + private SourceManager sourceManager; @Resource private OrcidSecurityManager orcidSecurityManager; @@ -118,20 +118,38 @@ private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { if (!issnValidator.issnValid(issn)) { throw new InvalidIssnException(); } - - IssnData issnData = issnClient.getIssnData(issn); - if (issnData == null) { + + try { + IssnData issnData = issnClient.getIssnData(issn); + if (issnData == null) { + throw new InvalidIssnException(); + } + + GroupIdRecord record = new GroupIdRecord(); + record.setGroupId(groupId); + GregorianCalendar cal = new GregorianCalendar(); + record.setCreatedDate(new CreatedDate(DateUtils.convertToXMLGregorianCalendar(cal))); + record.setLastModifiedDate(new LastModifiedDate(DateUtils.convertToXMLGregorianCalendar(cal))); + record.setName(issnData.getMainTitle()); + record.setType("journal"); + return record; + } catch(TooManyRequestsException tmre) { + //TODO: We are being rate limited, we have to pause + LOG.warn("We are being rate limited by the issn portal"); + throw new InvalidIssnException(); + } catch(UnexpectedResponseCodeException urce) { + LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); + throw new InvalidIssnException(); + } catch (IOException e) { + LOG.warn("IOException for issn {}", issn); + throw new InvalidIssnException(); + } catch (URISyntaxException e) { + LOG.warn("URISyntaxException for issn {}", issn); + throw new InvalidIssnException(); + } catch (InterruptedException e) { + LOG.warn("InterruptedException for issn {}", issn); throw new InvalidIssnException(); } - - GroupIdRecord record = new GroupIdRecord(); - record.setGroupId(groupId); - GregorianCalendar cal = new GregorianCalendar(); - record.setCreatedDate(new CreatedDate(DateUtils.convertToXMLGregorianCalendar(cal))); - record.setLastModifiedDate(new LastModifiedDate(DateUtils.convertToXMLGregorianCalendar(cal))); - record.setName(issnData.getMainTitle()); - record.setType("journal"); - return record; } private void validateDuplicate(GroupIdRecord newGroupIdRecord) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java index 76eaffe0781..e6b251d73cf 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java @@ -1,13 +1,12 @@ package org.orcid.core.manager.v3.impl; +import java.io.IOException; +import java.net.URISyntaxException; import java.util.GregorianCalendar; import javax.annotation.Resource; -import org.orcid.core.exception.DuplicatedGroupIdRecordException; -import org.orcid.core.exception.GroupIdRecordNotFoundException; -import org.orcid.core.exception.InvalidIssnException; -import org.orcid.core.exception.OrcidElementCantBeDeletedException; +import org.orcid.core.exception.*; import org.orcid.core.groupIds.issn.IssnClient; import org.orcid.core.groupIds.issn.IssnData; import org.orcid.core.groupIds.issn.IssnValidator; @@ -23,16 +22,17 @@ import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; import org.orcid.persistence.jpa.entities.SourceEntity; import org.orcid.core.utils.DateUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; public class GroupIdRecordManagerImpl extends GroupIdRecordManagerReadOnlyImpl implements GroupIdRecordManager { + private static final Logger LOG = LoggerFactory.getLogger(GroupIdRecordManagerImpl.class); + @Resource(name = "sourceManagerV3") private SourceManager sourceManager; - @Resource - private LocaleManager localeManager; - @Resource(name = "orcidSecurityManagerV3") private OrcidSecurityManager orcidSecurityManager; @@ -118,20 +118,37 @@ private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { if (!issnValidator.issnValid(issn)) { throw new InvalidIssnException(); } - - IssnData issnData = issnClient.getIssnData(issn); - if (issnData == null) { + + try { + IssnData issnData = issnClient.getIssnData(issn); + if (issnData == null) { + throw new InvalidIssnException(); + } + GroupIdRecord record = new GroupIdRecord(); + record.setGroupId(groupId); + GregorianCalendar cal = new GregorianCalendar(); + record.setCreatedDate(new CreatedDate(DateUtils.convertToXMLGregorianCalendar(cal))); + record.setLastModifiedDate(new LastModifiedDate(DateUtils.convertToXMLGregorianCalendar(cal))); + record.setName(issnData.getMainTitle()); + record.setType("journal"); + return record; + } catch(TooManyRequestsException tmre) { + //TODO: We are being rate limited, we have to pause + LOG.warn("We are being rate limited by the issn portal"); + throw new InvalidIssnException(); + } catch(UnexpectedResponseCodeException urce) { + LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); + throw new InvalidIssnException(); + } catch (IOException e) { + LOG.warn("IOException for issn {}", issn); + throw new InvalidIssnException(); + } catch (URISyntaxException e) { + LOG.warn("URISyntaxException for issn {}", issn); + throw new InvalidIssnException(); + } catch (InterruptedException e) { + LOG.warn("InterruptedException for issn {}", issn); throw new InvalidIssnException(); } - - GroupIdRecord record = new GroupIdRecord(); - record.setGroupId(groupId); - GregorianCalendar cal = new GregorianCalendar(); - record.setCreatedDate(new CreatedDate(DateUtils.convertToXMLGregorianCalendar(cal))); - record.setLastModifiedDate(new LastModifiedDate(DateUtils.convertToXMLGregorianCalendar(cal))); - record.setName(issnData.getMainTitle()); - record.setType("journal"); - return record; } private void validateDuplicate(GroupIdRecord newGroupIdRecord) { diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java index aa0de5d5b83..de57046d74a 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java @@ -33,7 +33,7 @@ public class IssnLoadManagerImpl implements IssnLoadManager { @Override public void loadIssn() { try { - LOGGER.debug("Load ISSN for client : " + issnSource); + LOGGER.info("Load ISSN for client : " + issnSource); issnLoadSource.loadIssn(issnSource); slackManager.sendAlert("Issn succesfully updated for client " + issnSource, slackChannel, slackUser); diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java index 1424912b39a..e689116f764 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java @@ -1,5 +1,7 @@ package org.orcid.scheduler.loader.source.issn; +import java.io.IOException; +import java.net.URISyntaxException; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -7,6 +9,8 @@ import javax.annotation.Resource; import org.apache.commons.lang3.StringUtils; +import org.orcid.core.exception.TooManyRequestsException; +import org.orcid.core.exception.UnexpectedResponseCodeException; import org.orcid.core.groupIds.issn.IssnClient; import org.orcid.core.groupIds.issn.IssnData; import org.orcid.core.groupIds.issn.IssnValidator; @@ -76,18 +80,34 @@ private void updateIssnGroupIdRecords() { int total = 0; while (!issnEntities.isEmpty()) { for (GroupIdRecordEntity issnEntity : issnEntities) { + LOG.info("Processing entity {}", new Object[]{ issnEntity.getId() }); String issn = getIssn(issnEntity); if (issn != null && issnValidator.issnValid(issn)) { batchCount++; total++; - IssnData issnData = issnClient.getIssnData(issn); - if (issnData != null) { - updateIssnEntity(issnEntity, issnData); - LOG.info("Updated group id record {} - {}, processed count now {}", - new Object[] { issnEntity.getId(), issnEntity.getGroupId(), Integer.toString(total) }); - } else { - LOG.warn("ISSN data not found for {}", issn); - recordFailure(issnEntity.getId(), "Data not found"); + try { + IssnData issnData = issnClient.getIssnData(issn); + if (issnData != null) { + updateIssnEntity(issnEntity, issnData); + LOG.info("Updated group id record {} - {}, processed count now {}", + new Object[]{issnEntity.getId(), issnEntity.getGroupId(), Integer.toString(total)}); + } + } catch(TooManyRequestsException tmre) { + //TODO: We are being rate limited, we have to pause + LOG.warn("We are being rate limited by the issn portal"); + recordFailure(issnEntity.getId(), "RATE_LIMIT reached"); + } catch(UnexpectedResponseCodeException urce) { + LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); + recordFailure(issnEntity.getId(), "Unexpected response code " + urce.getReceivedCode()); + } catch (IOException e) { + LOG.warn("IOException for issn {}", issn); + recordFailure(issnEntity.getId(), "IOException"); + } catch (URISyntaxException e) { + LOG.warn("URISyntaxException for issn {}", issn); + recordFailure(issnEntity.getId(), "URISyntaxException"); + } catch (InterruptedException e) { + LOG.warn("InterruptedException for issn {}", issn); + recordFailure(issnEntity.getId(), "InterruptedException"); } } else { LOG.info("Issn for group record {} not valid: {}", issnEntity.getId(), issnEntity.getGroupId()); From 49c3dd8cbe1b271947b290d085159416788c3807 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 17 Jul 2024 13:32:10 -0600 Subject: [PATCH 3/4] Running locally --- .../orcid/core/groupIds/issn/IssnClient.java | 2 +- .../persistence/dao/GroupIdRecordDao.java | 3 +- .../dao/impl/GroupIdRecordDaoImpl.java | 18 ++++--- .../jpa/entities/GroupIdRecordEntity.java | 36 ++++++++++++- .../InvalidIssnGroupIdRecordEntity.java | 40 -------------- .../main/resources/META-INF/persistence.xml | 3 +- .../src/main/resources/db-master.xml | 1 + ...eason_and_sync_date_to_group_id_record.xml | 50 +++++++++++++++++ .../resources/orcid-persistence-context.xml | 4 -- .../manager/impl/IssnLoadManagerImpl.java | 1 - .../loader/source/issn/IssnLoadSource.java | 54 +++++++++---------- 11 files changed, 129 insertions(+), 83 deletions(-) delete mode 100644 orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/InvalidIssnGroupIdRecordEntity.java create mode 100644 orcid-persistence/src/main/resources/db/updates/add_fail_count_fail_reason_and_sync_date_to_group_id_record.xml diff --git a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java index e042ebaaa54..8df8e935147 100644 --- a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java +++ b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java @@ -51,7 +51,7 @@ public IssnData getIssnData(String issn) throws TooManyRequestsException, Unexpe return null; } } catch (Exception e) { - LOG.warn("Error extracting issn data from json returned from issn portal "+ issn, e); + LOG.warn("Error extracting issn data from json returned from issn portal "+ issn); return null; } } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/GroupIdRecordDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/GroupIdRecordDao.java index 7a7b0c22a1d..18e2d33d415 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/GroupIdRecordDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/GroupIdRecordDao.java @@ -1,5 +1,6 @@ package org.orcid.persistence.dao; +import java.util.Date; import java.util.List; import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; @@ -16,5 +17,5 @@ public interface GroupIdRecordDao extends GenericDao boolean duplicateExists(Long putCode, String groupId); - List getIssnRecordsSortedById(int batchSize, long initialId); + List getIssnRecordsSortedBySyncDate(int batchSize, Date syncTime); } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/GroupIdRecordDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/GroupIdRecordDaoImpl.java index 7befb1feaa5..332011883f9 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/GroupIdRecordDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/GroupIdRecordDaoImpl.java @@ -1,5 +1,6 @@ package org.orcid.persistence.dao.impl; +import java.util.Date; import java.util.List; import javax.persistence.Query; @@ -7,9 +8,13 @@ import org.orcid.persistence.dao.GroupIdRecordDao; import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; +import org.springframework.beans.factory.annotation.Value; + +public class GroupIdRecordDaoImpl extends GenericDaoImpl implements GroupIdRecordDao { + + @Value("${org.orcid.persistence.groupIdRecord.retry.max:5}") + private int maxRetries; -public class GroupIdRecordDaoImpl extends GenericDaoImpl implements GroupIdRecordDao { - public GroupIdRecordDaoImpl() { super(GroupIdRecordEntity.class); } @@ -73,10 +78,11 @@ public boolean duplicateExists(Long putCode, String groupId) { } @Override - public List getIssnRecordsSortedById(int batchSize, long initialId) { - Query query = entityManager.createNativeQuery("SELECT * FROM group_id_record g LEFT OUTER JOIN invalid_issn_group_id_record p ON g.id = p.id where p.id IS NULL AND g.group_id like 'issn:%' and g.id > :initialId order by g.id", GroupIdRecordEntity.class); - query.setParameter("initialId", initialId); - query.setMaxResults(batchSize); + public List getIssnRecordsSortedBySyncDate(int batchSize, Date syncTime) { + Query query = entityManager.createNativeQuery("SELECT * FROM group_id_record g WHERE g.issn_loader_fail_count < :max AND g.group_id LIKE 'issn:%' AND (g.sync_date is null OR g.sync_date < :syncTime) ORDER BY g.sync_date", GroupIdRecordEntity.class); + query.setParameter("max", maxRetries); + query.setParameter("syncTime", syncTime); + query.setMaxResults(batchSize); return query.getResultList(); } } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/GroupIdRecordEntity.java b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/GroupIdRecordEntity.java index ec55352ecee..12720bd5fb8 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/GroupIdRecordEntity.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/GroupIdRecordEntity.java @@ -7,6 +7,7 @@ import javax.persistence.Id; import javax.persistence.SequenceGenerator; import javax.persistence.Table; +import java.util.Date; /** * The persistent class for the group_id_record database table. @@ -26,7 +27,13 @@ public class GroupIdRecordEntity extends SourceAwareEntity implements Comp private String groupDescription; - private String groupType; + private String groupType; + + private Integer issnLoaderFailCount; + + private String failReason; + + private Date syncDate; @Id @GeneratedValue(strategy = GenerationType.AUTO, generator = "group_id_record_seq") @@ -75,6 +82,33 @@ public void setGroupType(String groupType) { this.groupType = groupType; } + @Column(name = "issn_loader_fail_count") + public Integer getIssnLoaderFailCount() { + return issnLoaderFailCount; + } + + public void setIssnLoaderFailCount(Integer issnLoaderFailCount) { + this.issnLoaderFailCount = issnLoaderFailCount; + } + + @Column(name = "fail_reason") + public String getFailReason() { + return failReason; + } + + public void setFailReason(String failReason) { + this.failReason = failReason; + } + + @Column(name = "sync_date") + public Date getSyncDate() { + return syncDate; + } + + public void setSyncDate(Date syncDate) { + this.syncDate = syncDate; + } + @Override public int compareTo(GroupIdRecordEntity o) { return 0; diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/InvalidIssnGroupIdRecordEntity.java b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/InvalidIssnGroupIdRecordEntity.java deleted file mode 100644 index 3e69411d5c4..00000000000 --- a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/InvalidIssnGroupIdRecordEntity.java +++ /dev/null @@ -1,40 +0,0 @@ -package org.orcid.persistence.jpa.entities; - -import java.io.Serializable; - -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.Table; - -@Entity -@Table(name = "invalid_issn_group_id_record") -public class InvalidIssnGroupIdRecordEntity extends BaseEntity implements Serializable { - - private static final long serialVersionUID = 1L; - - private String notes; - - private Long id; - - @Override - @Id - @Column - public Long getId() { - return id; - } - - public void setId(Long id) { - this.id = id; - } - - @Column - public String getNotes() { - return notes; - } - - public void setNotes(String note) { - this.notes = note; - } - -} diff --git a/orcid-persistence/src/main/resources/META-INF/persistence.xml b/orcid-persistence/src/main/resources/META-INF/persistence.xml index 2ef97de7a61..418d0cfa39d 100644 --- a/orcid-persistence/src/main/resources/META-INF/persistence.xml +++ b/orcid-persistence/src/main/resources/META-INF/persistence.xml @@ -89,8 +89,7 @@ org.orcid.persistence.jpa.entities.RejectedGroupingSuggestionEntity org.orcid.persistence.jpa.entities.ValidatedPublicProfileEntity org.orcid.persistence.jpa.entities.MemberOBOWhitelistedClientEntity - org.orcid.persistence.jpa.entities.InvalidIssnGroupIdRecordEntity - org.orcid.persistence.jpa.entities.OrgImportLogEntity + org.orcid.persistence.jpa.entities.OrgImportLogEntity org.orcid.statistics.jpa.entities.StatisticValuesEntity diff --git a/orcid-persistence/src/main/resources/db-master.xml b/orcid-persistence/src/main/resources/db-master.xml index b1fc81b796a..da9e14ed2b7 100644 --- a/orcid-persistence/src/main/resources/db-master.xml +++ b/orcid-persistence/src/main/resources/db-master.xml @@ -391,4 +391,5 @@ + diff --git a/orcid-persistence/src/main/resources/db/updates/add_fail_count_fail_reason_and_sync_date_to_group_id_record.xml b/orcid-persistence/src/main/resources/db/updates/add_fail_count_fail_reason_and_sync_date_to_group_id_record.xml new file mode 100644 index 00000000000..eca7c094ec7 --- /dev/null +++ b/orcid-persistence/src/main/resources/db/updates/add_fail_count_fail_reason_and_sync_date_to_group_id_record.xml @@ -0,0 +1,50 @@ + + + + + + + + + ALTER TABLE group_id_record ADD COLUMN issn_loader_fail_count INTEGER default 0; + + + + + + + + + ALTER TABLE group_id_record ADD COLUMN fail_reason VARCHAR(50); + + + + + + + + + ALTER TABLE group_id_record ADD COLUMN sync_date timestamp; + + + + + + + + + create index group_id_record_issn_loader_fail_count_index on group_id_record(issn_loader_fail_count); + + + + + + + + + create index group_id_record_sync_date_index on group_id_record(sync_date); + + + \ No newline at end of file diff --git a/orcid-persistence/src/main/resources/orcid-persistence-context.xml b/orcid-persistence/src/main/resources/orcid-persistence-context.xml index 285312d0a1b..cedd58617ac 100644 --- a/orcid-persistence/src/main/resources/orcid-persistence-context.xml +++ b/orcid-persistence/src/main/resources/orcid-persistence-context.xml @@ -271,10 +271,6 @@ - - - - diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java index de57046d74a..75c74f7d4ca 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/manager/impl/IssnLoadManagerImpl.java @@ -36,7 +36,6 @@ public void loadIssn() { LOGGER.info("Load ISSN for client : " + issnSource); issnLoadSource.loadIssn(issnSource); slackManager.sendAlert("Issn succesfully updated for client " + issnSource, slackChannel, slackUser); - } catch (Exception ex) { LOGGER.error("Error when running ISSN for client" + issnSource, ex); slackManager.sendAlert("Error when running ISSN for client " + issnSource, slackChannel, slackUser); diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java index e689116f764..a4e88d73fb9 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.net.URISyntaxException; +import java.util.Date; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -15,11 +16,9 @@ import org.orcid.core.groupIds.issn.IssnData; import org.orcid.core.groupIds.issn.IssnValidator; import org.orcid.persistence.dao.ClientDetailsDao; -import org.orcid.persistence.dao.GenericDao; import org.orcid.persistence.dao.GroupIdRecordDao; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; -import org.orcid.persistence.jpa.entities.InvalidIssnGroupIdRecordEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; @@ -35,7 +34,7 @@ public class IssnLoadSource { @Value("${org.orcid.scheduler.issnLoadSource.batchSize:5000}") private int batchSize; - @Value("${org.orcid.scheduler.issnLoadSource.waitBetweenBatches:30000}") + @Value("${org.orcid.scheduler.issnLoadSource.waitBetweenBatches:10000}") private int waitBetweenBatches; @Resource @@ -43,9 +42,6 @@ public class IssnLoadSource { @Resource(name="groupIdRecordDaoReadOnly") private GroupIdRecordDao groupIdRecordDaoReadOnly; - - @Resource - private GenericDao invalidIssnGroupIdRecordDao; @Resource(name="clientDetailsDaoReadOnly") private ClientDetailsDao clientDetailsDaoReadOnly; @@ -72,10 +68,10 @@ public void loadIssn(String issnSource) { } private void updateIssnGroupIdRecords() { - Long nextBatchStartId = 0L; + Date startTime = new Date(); // Get the first batch of issn's - LOG.info("Loading batch of ISSN's, starting id: " + nextBatchStartId + " batch size: " + batchSize); - List issnEntities = groupIdRecordDaoReadOnly.getIssnRecordsSortedById(batchSize, nextBatchStartId); + LOG.info("Running the process to load ISSN info, starting time: " + startTime + " batch size: " + batchSize); + List issnEntities = groupIdRecordDaoReadOnly.getIssnRecordsSortedBySyncDate(batchSize, startTime); int batchCount = 0; int total = 0; while (!issnEntities.isEmpty()) { @@ -95,23 +91,23 @@ private void updateIssnGroupIdRecords() { } catch(TooManyRequestsException tmre) { //TODO: We are being rate limited, we have to pause LOG.warn("We are being rate limited by the issn portal"); - recordFailure(issnEntity.getId(), "RATE_LIMIT reached"); + recordFailure(issnEntity, "RATE_LIMIT reached"); } catch(UnexpectedResponseCodeException urce) { LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); - recordFailure(issnEntity.getId(), "Unexpected response code " + urce.getReceivedCode()); + recordFailure(issnEntity, "Unexpected response code " + urce.getReceivedCode()); } catch (IOException e) { LOG.warn("IOException for issn {}", issn); - recordFailure(issnEntity.getId(), "IOException"); + recordFailure(issnEntity, "IOException"); } catch (URISyntaxException e) { LOG.warn("URISyntaxException for issn {}", issn); - recordFailure(issnEntity.getId(), "URISyntaxException"); + recordFailure(issnEntity, "URISyntaxException"); } catch (InterruptedException e) { LOG.warn("InterruptedException for issn {}", issn); - recordFailure(issnEntity.getId(), "InterruptedException"); + recordFailure(issnEntity, "InterruptedException"); } } else { LOG.info("Issn for group record {} not valid: {}", issnEntity.getId(), issnEntity.getGroupId()); - recordFailure(issnEntity.getId(), "Invalid record"); + recordFailure(issnEntity, "Invalid record"); } try { // Lets sleep for 30 secs after processing one batch @@ -125,21 +121,22 @@ private void updateIssnGroupIdRecords() { // TODO Auto-generated catch block LOG.warn("Exception while pausing the issn loader", e); } - - if (issnEntity.getId() > nextBatchStartId) { - nextBatchStartId = issnEntity.getId(); - } } - LOG.info("Loading batch of ISSN's, starting id: " + nextBatchStartId); - issnEntities = groupIdRecordDaoReadOnly.getIssnRecordsSortedById(batchSize, nextBatchStartId); + LOG.info("Loading next batch of ISSN's"); + issnEntities = groupIdRecordDaoReadOnly.getIssnRecordsSortedBySyncDate(batchSize, startTime); } + LOG.info("All ISSN records processed"); } - private void recordFailure(Long id, String notes) { - InvalidIssnGroupIdRecordEntity invalidIssn = new InvalidIssnGroupIdRecordEntity(); - invalidIssn.setId(id); - invalidIssn.setNotes(notes); - invalidIssnGroupIdRecordDao.persist(invalidIssn); + private void recordFailure(GroupIdRecordEntity issnEntity, String notes) { + issnEntity.setFailReason(notes); + issnEntity.setSyncDate(new Date()); + if(issnEntity.getIssnLoaderFailCount() == null) { + issnEntity.setIssnLoaderFailCount(1); + } else { + issnEntity.setIssnLoaderFailCount(issnEntity.getIssnLoaderFailCount() + 1); + } + groupIdRecordDao.merge(issnEntity); } private void updateIssnEntity(GroupIdRecordEntity issnEntity, IssnData issnData) { @@ -149,10 +146,13 @@ private void updateIssnEntity(GroupIdRecordEntity issnEntity, IssnData issnData) if(!StringUtils.equals(currentGroupName, updatedGroupName)) { issnEntity.setGroupName(updatedGroupName); issnEntity.setClientSourceId(orcidSource.getId()); + issnEntity.setSyncDate(new Date()); LOG.info("Updating Group id: " + issnEntity.getGroupId() + " | current group name: " + currentGroupName + " | group name to be updated: " + issnEntity.getGroupName()); groupIdRecordDao.merge(issnEntity); } else { - LOG.info("Group id: " + issnEntity.getGroupId() + " is up to date"); + issnEntity.setSyncDate(new Date()); + groupIdRecordDao.merge(issnEntity); + LOG.debug("Group id: " + issnEntity.getGroupId() + " is up to date"); } } From e43ac75dcc132a7ef6077afbb354530ecc4e6e08 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 18 Jul 2024 10:54:40 -0600 Subject: [PATCH 4/4] Banned exception and more logging --- .../orcid/core/exception/BannedException.java | 17 +++++++ .../orcid/core/groupIds/issn/IssnClient.java | 21 ++++---- .../impl/GroupIdRecordManagerImpl.java | 4 ++ .../v3/impl/GroupIdRecordManagerImpl.java | 12 ++--- .../loader/source/issn/IssnLoadSource.java | 50 +++++++++++++++++-- .../utils/alerting/impl/SlackManagerImpl.java | 2 +- 6 files changed, 87 insertions(+), 19 deletions(-) create mode 100644 orcid-core/src/main/java/org/orcid/core/exception/BannedException.java diff --git a/orcid-core/src/main/java/org/orcid/core/exception/BannedException.java b/orcid-core/src/main/java/org/orcid/core/exception/BannedException.java new file mode 100644 index 00000000000..e5c7b1f90bc --- /dev/null +++ b/orcid-core/src/main/java/org/orcid/core/exception/BannedException.java @@ -0,0 +1,17 @@ +package org.orcid.core.exception; + +import java.util.Map; + +public class BannedException extends ApplicationException { + + private static final long serialVersionUID = 1L; + + public BannedException() { + super(); + } + + public BannedException(Map params) { + super(params); + } + +} diff --git a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java index 8df8e935147..0bae68dbd03 100644 --- a/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java +++ b/orcid-core/src/main/java/org/orcid/core/groupIds/issn/IssnClient.java @@ -12,6 +12,7 @@ import org.codehaus.jettison.json.JSONException; import org.codehaus.jettison.json.JSONObject; import org.eclipse.jetty.http.HttpStatus; +import org.orcid.core.exception.BannedException; import org.orcid.core.exception.TooManyRequestsException; import org.orcid.core.exception.UnexpectedResponseCodeException; import org.orcid.core.utils.http.HttpRequestUtils; @@ -36,23 +37,25 @@ public class IssnClient { @Resource private HttpRequestUtils httpRequestUtils; - public IssnData getIssnData(String issn) throws TooManyRequestsException, UnexpectedResponseCodeException, IOException, URISyntaxException, InterruptedException { + public IssnData getIssnData(String issn) throws BannedException, TooManyRequestsException, UnexpectedResponseCodeException, IOException, URISyntaxException, InterruptedException, JSONException { if(StringUtils.isEmpty(issn)) { return null; } LOG.debug("Extracting ISSN for " + issn); String json = getJsonDataFromIssnPortal(issn.toUpperCase()); try { - if (json != null) { - IssnData data = extractIssnData(issn.toUpperCase(), json); - data.setIssn(issn); - return data; - } else { + IssnData data = extractIssnData(issn.toUpperCase(), json); + data.setIssn(issn); + return data; + } catch (JSONException e) { + LOG.warn("Error extracting issn data from json returned from issn portal "+ issn); + if(json == null) { return null; + } else if(json.contains("you have been banned")) { + throw new BannedException(); + } else { + throw e; } - } catch (Exception e) { - LOG.warn("Error extracting issn data from json returned from issn portal "+ issn); - return null; } } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java index 6e667415956..7eb87b7ad89 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/GroupIdRecordManagerImpl.java @@ -6,6 +6,7 @@ import javax.annotation.Resource; +import org.codehaus.jettison.json.JSONException; import org.orcid.core.exception.*; import org.orcid.core.groupIds.issn.IssnClient; import org.orcid.core.groupIds.issn.IssnData; @@ -149,6 +150,9 @@ private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { } catch (InterruptedException e) { LOG.warn("InterruptedException for issn {}", issn); throw new InvalidIssnException(); + } catch(JSONException e) { + LOG.warn("JSONException for issn {}", issn, e); + throw new InvalidIssnException(); } } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java index e6b251d73cf..54bc666057d 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/GroupIdRecordManagerImpl.java @@ -6,6 +6,7 @@ import javax.annotation.Resource; +import org.codehaus.jettison.json.JSONException; import org.orcid.core.exception.*; import org.orcid.core.groupIds.issn.IssnClient; import org.orcid.core.groupIds.issn.IssnData; @@ -66,7 +67,7 @@ public GroupIdRecord createGroupIdRecord(GroupIdRecord groupIdRecord) { } @Override - public GroupIdRecord createOrcidSourceIssnGroupIdRecord(String groupId, String issn) { + public GroupIdRecord createOrcidSourceIssnGroupIdRecord(String groupId, String issn) throws TooManyRequestsException, BannedException { GroupIdRecord issnRecord = createIssnGroupIdRecord(groupId, issn); GroupIdRecordEntity entity = jpaJaxbGroupIdRecordAdapter.toGroupIdRecordEntity(issnRecord); entity.setClientSourceId(orcidSourceClientDetailsId); @@ -114,7 +115,7 @@ public void deleteGroupIdRecord(Long putCode) { } } - private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { + private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) throws TooManyRequestsException, BannedException { if (!issnValidator.issnValid(issn)) { throw new InvalidIssnException(); } @@ -132,10 +133,6 @@ private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { record.setName(issnData.getMainTitle()); record.setType("journal"); return record; - } catch(TooManyRequestsException tmre) { - //TODO: We are being rate limited, we have to pause - LOG.warn("We are being rate limited by the issn portal"); - throw new InvalidIssnException(); } catch(UnexpectedResponseCodeException urce) { LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); throw new InvalidIssnException(); @@ -148,6 +145,9 @@ private GroupIdRecord createIssnGroupIdRecord(String groupId, String issn) { } catch (InterruptedException e) { LOG.warn("InterruptedException for issn {}", issn); throw new InvalidIssnException(); + } catch(JSONException e) { + LOG.warn("JSONException for issn {}", issn); + throw new InvalidIssnException(); } } diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java index a4e88d73fb9..f59ea373cef 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/loader/source/issn/IssnLoadSource.java @@ -1,7 +1,7 @@ package org.orcid.scheduler.loader.source.issn; import java.io.IOException; -import java.net.URISyntaxException; +import java.net.*; import java.util.Date; import java.util.List; import java.util.regex.Matcher; @@ -10,6 +10,8 @@ import javax.annotation.Resource; import org.apache.commons.lang3.StringUtils; +import org.codehaus.jettison.json.JSONException; +import org.orcid.core.exception.BannedException; import org.orcid.core.exception.TooManyRequestsException; import org.orcid.core.exception.UnexpectedResponseCodeException; import org.orcid.core.groupIds.issn.IssnClient; @@ -19,6 +21,7 @@ import org.orcid.persistence.dao.GroupIdRecordDao; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.GroupIdRecordEntity; +import org.orcid.utils.alerting.SlackManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; @@ -36,7 +39,10 @@ public class IssnLoadSource { @Value("${org.orcid.scheduler.issnLoadSource.waitBetweenBatches:10000}") private int waitBetweenBatches; - + + @Value("${org.orcid.scheduler.issnLoadSource.rateLimit.pause:600000}") + private int pause; + @Resource private GroupIdRecordDao groupIdRecordDao; @@ -53,6 +59,9 @@ public class IssnLoadSource { @Resource private IssnClient issnClient; + + @Resource + private SlackManager slackManager; public void loadIssn(String issnSource) { @@ -74,6 +83,7 @@ private void updateIssnGroupIdRecords() { List issnEntities = groupIdRecordDaoReadOnly.getIssnRecordsSortedBySyncDate(batchSize, startTime); int batchCount = 0; int total = 0; + while (!issnEntities.isEmpty()) { for (GroupIdRecordEntity issnEntity : issnEntities) { LOG.info("Processing entity {}", new Object[]{ issnEntity.getId() }); @@ -89,9 +99,29 @@ private void updateIssnGroupIdRecords() { new Object[]{issnEntity.getId(), issnEntity.getGroupId(), Integer.toString(total)}); } } catch(TooManyRequestsException tmre) { - //TODO: We are being rate limited, we have to pause + //We are being rate limited, we have to pause for 'pause' minutes LOG.warn("We are being rate limited by the issn portal"); recordFailure(issnEntity, "RATE_LIMIT reached"); + if(pause() != 1) { + LOG.warn("Unable to pause, finishing the process"); + return; + } + } catch(BannedException be) { + LOG.error("We have been banned from the issn portal, the sync process will finish now"); + try { + InetAddress id = InetAddress.getLocalHost(); + slackManager.sendSystemAlert("We have bee banned from the issn portal on " + id.getHostName()); + } catch(UnknownHostException uhe) { + // Lets try to get the IP address + try(final DatagramSocket socket = new DatagramSocket()){ + socket.connect(InetAddress.getByName("8.8.8.8"), 10002); + String ip = socket.getLocalAddress().getHostAddress(); + slackManager.sendSystemAlert("We have bee banned from the issn portal on " + ip); + } catch(SocketException | UnknownHostException se) { + slackManager.sendSystemAlert("We have bee banned from the issn portal on - Couldn't identify the machine"); + } + } + return; } catch(UnexpectedResponseCodeException urce) { LOG.warn("Unexpected response code {} for issn {}", urce.getReceivedCode(), issn); recordFailure(issnEntity, "Unexpected response code " + urce.getReceivedCode()); @@ -104,6 +134,9 @@ private void updateIssnGroupIdRecords() { } catch (InterruptedException e) { LOG.warn("InterruptedException for issn {}", issn); recordFailure(issnEntity, "InterruptedException"); + } catch(JSONException e) { + LOG.warn("InterruptedException for issn {}", issn); + recordFailure(issnEntity, "InterruptedException"); } } else { LOG.info("Issn for group record {} not valid: {}", issnEntity.getId(), issnEntity.getGroupId()); @@ -164,4 +197,15 @@ private String getIssn(GroupIdRecordEntity issnEntity) { return null; } + private int pause() { + try { + LOG.warn("Pause do to rate limit"); + Thread.sleep(pause); + return 1; + } catch (InterruptedException e) { + LOG.warn("Unable to pause", e); + return -1; + } + } + } diff --git a/orcid-utils/src/main/java/org/orcid/utils/alerting/impl/SlackManagerImpl.java b/orcid-utils/src/main/java/org/orcid/utils/alerting/impl/SlackManagerImpl.java index 14efca1a2b2..478f67d9136 100644 --- a/orcid-utils/src/main/java/org/orcid/utils/alerting/impl/SlackManagerImpl.java +++ b/orcid-utils/src/main/java/org/orcid/utils/alerting/impl/SlackManagerImpl.java @@ -25,7 +25,7 @@ */ public class SlackManagerImpl implements SlackManager { - @Value("${org.orcid.core.slack.webhookUrl:}") + @Value("${org.orcid.core.slack.webhookUrl}") private String webhookUrl; @Value("${org.orcid.core.slack.channel}")