diff --git a/orcid-core/src/main/java/org/orcid/core/adapter/v3/JpaJaxbWorkAdapter.java b/orcid-core/src/main/java/org/orcid/core/adapter/v3/JpaJaxbWorkAdapter.java index cc949717eac..921a7329662 100644 --- a/orcid-core/src/main/java/org/orcid/core/adapter/v3/JpaJaxbWorkAdapter.java +++ b/orcid-core/src/main/java/org/orcid/core/adapter/v3/JpaJaxbWorkAdapter.java @@ -2,11 +2,14 @@ import java.util.Collection; import java.util.List; +import java.util.Map; +import org.orcid.jaxb.model.v3.release.common.Source; import org.orcid.jaxb.model.v3.release.record.Work; import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary; import org.orcid.persistence.jpa.entities.MinimizedExtendedWorkEntity; import org.orcid.persistence.jpa.entities.MinimizedWorkEntity; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.WorkEntity; import org.orcid.pojo.WorkExtended; import org.orcid.pojo.WorkSummaryExtended; @@ -34,6 +37,8 @@ public interface JpaJaxbWorkAdapter { List toWorkSummaryFromMinimized(Collection workEntities); + List toWorkSummaryFromMinimized(Collection workEntities, Map sources); + List toWorkSummaryExtendedFromMinimized(Collection workEntities); WorkEntity toWorkEntity(Work work, WorkEntity existing); diff --git a/orcid-core/src/main/java/org/orcid/core/adapter/v3/converter/ContributorsRolesAndSequencesConverter.java b/orcid-core/src/main/java/org/orcid/core/adapter/v3/converter/ContributorsRolesAndSequencesConverter.java index be6df9376f1..9f79a278390 100644 --- a/orcid-core/src/main/java/org/orcid/core/adapter/v3/converter/ContributorsRolesAndSequencesConverter.java +++ b/orcid-core/src/main/java/org/orcid/core/adapter/v3/converter/ContributorsRolesAndSequencesConverter.java @@ -6,6 +6,7 @@ import ma.glasnost.orika.converter.BidirectionalConverter; import ma.glasnost.orika.metadata.Type; import org.orcid.core.contributors.roles.ContributorRoleConverter; +import org.orcid.core.contributors.roles.ContributorRoleConverterImpl; import org.orcid.core.contributors.roles.credit.CreditRole; import org.orcid.core.utils.JsonUtils; import org.orcid.core.utils.v3.ContributorUtils; @@ -15,6 +16,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Resource; import java.util.ArrayList; import java.util.List; @@ -22,11 +24,11 @@ public class ContributorsRolesAndSequencesConverter extends BidirectionalConvert private static final Logger LOGGER = LoggerFactory.getLogger(ContributorsRolesAndSequencesConverter.class); - private ContributorRoleConverter roleConverter; + @Resource(name = "workContributorRoleConverter") + private ContributorRoleConverter workContributorRoleConverter; - public ContributorsRolesAndSequencesConverter(ContributorRoleConverter roleConverter) { - this.roleConverter = roleConverter; - } + @Resource(name = "contributorUtilsV3") + private ContributorUtils contributorUtils; @Override public String convertTo(List source, Type destinationType) { @@ -39,7 +41,6 @@ public List convertFrom(String source, Type getContributorsRolesAndSequencesList(String source) { - ContributorUtils contributorUtils = new ContributorUtils(null); final ObjectMapper objectMapper = new ObjectMapper(); List contributorsRolesAndSequencesResult = new ArrayList<>(); try { @@ -54,7 +55,7 @@ public List getContributorsRolesAndSequencesList( if (cr != null) { providedRoleValue = cr.name(); } - crs.setContributorRole(contributorUtils.getCreditRole(roleConverter.toRoleValue(providedRoleValue))); + crs.setContributorRole(contributorUtils.getCreditRole(workContributorRoleConverter.toRoleValue(providedRoleValue))); } } } diff --git a/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/JpaJaxbWorkAdapterImpl.java b/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/JpaJaxbWorkAdapterImpl.java index 5bdc7b62412..40f7bf35c53 100644 --- a/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/JpaJaxbWorkAdapterImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/JpaJaxbWorkAdapterImpl.java @@ -2,12 +2,17 @@ import java.util.Collection; import java.util.List; +import java.util.Map; +import ma.glasnost.orika.MappingContext; import ma.glasnost.orika.MapperFacade; +import org.orcid.core.utils.SourceEntityUtils; import org.orcid.core.adapter.v3.JpaJaxbWorkAdapter; +import org.orcid.jaxb.model.v3.release.common.Source; import org.orcid.jaxb.model.v3.release.record.Work; import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.MinimizedExtendedWorkEntity; import org.orcid.persistence.jpa.entities.MinimizedWorkEntity; import org.orcid.persistence.jpa.entities.WorkEntity; @@ -100,6 +105,16 @@ public List toWorkSummaryFromMinimized(Collection toWorkSummaryFromMinimized(Collection workEntities, Map sourceMap) { + if(workEntities == null) { + return null; + } + MappingContext context = new MappingContext.Factory().getContext(); + context.setProperty(SourceEntityUtils.SOURCE_MAP, sourceMap); + return mapperFacade.mapAsList(workEntities, WorkSummary.class, context); + } + @Override public List toWorkSummaryExtendedFromMinimized(Collection workEntities) { if(workEntities == null) { diff --git a/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/MapperFacadeFactory.java b/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/MapperFacadeFactory.java index cc4b1a9daf3..0f4885a8072 100644 --- a/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/MapperFacadeFactory.java +++ b/orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/MapperFacadeFactory.java @@ -31,12 +31,9 @@ import org.orcid.core.contributors.roles.works.WorkContributorRoleConverter; import org.orcid.core.exception.OrcidValidationException; import org.orcid.core.locale.LocaleManager; -import org.orcid.core.manager.ClientDetailsEntityCacheManager; import org.orcid.core.manager.EncryptionManager; import org.orcid.core.manager.IdentityProviderManager; -import org.orcid.core.manager.SourceNameCacheManager; import org.orcid.core.manager.impl.OrcidUrlManager; -import org.orcid.core.manager.v3.read_only.ClientDetailsManagerReadOnly; import org.orcid.core.utils.JsonUtils; import org.orcid.core.utils.SourceEntityUtils; import org.orcid.core.utils.v3.identifiers.PIDNormalizationService; @@ -140,7 +137,6 @@ import org.orcid.persistence.jpa.entities.SpamEntity; import org.orcid.persistence.jpa.entities.StartDateEntity; import org.orcid.persistence.jpa.entities.WorkEntity; -import org.orcid.persistence.jpa.entities.keys.ClientRedirectUriPk; import org.orcid.pojo.WorkExtended; import org.orcid.pojo.WorkSummaryExtended; import org.orcid.pojo.ajaxForm.PojoUtil; @@ -170,15 +166,6 @@ public class MapperFacadeFactory implements FactoryBean { @Resource private WorkDao workDao; - @Resource - private SourceNameCacheManager sourceNameCacheManager; - - @Resource - private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; - - @Resource(name = "clientDetailsManagerReadOnlyV3") - private ClientDetailsManagerReadOnly clientDetailsManagerReadOnly; - @Resource private IdentityProviderManager identityProviderManager; @@ -200,6 +187,12 @@ public class MapperFacadeFactory implements FactoryBean { @Resource private FundingContributorRoleConverter fundingContributorsRoleConverter; + @Resource + private SourceEntityUtils sourceEntityUtils; + + @Resource + private ContributorsRolesAndSequencesConverter contributorsRolesAndSequencesConverter; + @Override public MapperFacade getObject() throws Exception { MapperFactory mapperFactory = new DefaultMapperFactory.Builder().build(); @@ -387,10 +380,19 @@ public void registerSourceConverters(MapperFactory mapperFactory, ClassMapBuilde } private class SourceMapper extends CustomMapper> { + @SuppressWarnings("unchecked") @Override public void mapBtoA(SourceAwareEntity b, SourceAware a, MappingContext context) { - Source source = SourceEntityUtils.extractSourceFromEntityComplete(b, sourceNameCacheManager, orcidUrlManager, clientDetailsEntityCacheManager); - a.setSource(source); + if (context != null && context.getProperty(SourceEntityUtils.SOURCE_MAP) != null) { + // The source map is set in the context, so we can use it to set the source. + Map sourceMap = (Map) context.getProperty(SourceEntityUtils.SOURCE_MAP); + Source source = sourceMap.get(sourceEntityUtils.getSourceKey(b)); + a.setSource(source); + } else { + // We have to manually build the source elements + Source source = sourceEntityUtils.extractSourceFromEntityComplete(b); + a.setSource(source); + } } } @@ -536,8 +538,6 @@ public MapperFacade getWorkMapperFacade() { MapperFactory mapperFactory = new DefaultMapperFactory.Builder().build(); WorkContributorsConverter wcc = new WorkContributorsConverter(workContributorsRoleConverter); - ContributorsRolesAndSequencesConverter contributorsRolesAndSequencesConverter = new ContributorsRolesAndSequencesConverter(workContributorsRoleConverter); - ConverterFactory converterFactory = mapperFactory.getConverterFactory(); converterFactory.registerConverter("workExternalIdentifiersConverterId", new JSONWorkExternalIdentifiersConverterV3(norm, resolverService, localeManager)); converterFactory.registerConverter("workContributorsConverterId", wcc); diff --git a/orcid-core/src/main/java/org/orcid/core/cli/FilterTopContributors.java b/orcid-core/src/main/java/org/orcid/core/cli/FilterTopContributors.java index e8614a69c1b..b26492c6f45 100644 --- a/orcid-core/src/main/java/org/orcid/core/cli/FilterTopContributors.java +++ b/orcid-core/src/main/java/org/orcid/core/cli/FilterTopContributors.java @@ -65,7 +65,7 @@ private void filter() { private void filterTopContributors(Object[] workObject) { WorkEntity workEntity = workDao.find(((BigInteger) workObject[0]).longValue()); - ContributorUtils contributorUtils = new ContributorUtils(0); + ContributorUtils contributorUtils = new ContributorUtils(); WorkSummaryExtended wse = new WorkSummaryExtended.WorkSummaryExtendedBuilder(((BigInteger) workObject[0])) .contributors(workContributorsConverter.getContributorsList(isEmpty(workObject[1]))) .build(); diff --git a/orcid-core/src/main/java/org/orcid/core/exception/InvalidPutCodeException.java b/orcid-core/src/main/java/org/orcid/core/exception/InvalidPutCodeException.java index 88805d9f44a..8e419539274 100644 --- a/orcid-core/src/main/java/org/orcid/core/exception/InvalidPutCodeException.java +++ b/orcid-core/src/main/java/org/orcid/core/exception/InvalidPutCodeException.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.Map; +import org.apache.commons.lang3.StringUtils; import org.orcid.core.utils.SourceEntityUtils; import org.orcid.jaxb.model.v3.release.common.Source; @@ -19,10 +20,10 @@ public InvalidPutCodeException(Map params) { super(params); } - public static InvalidPutCodeException forSource(Source activeSource) { + public static InvalidPutCodeException forSource(String sourceName) { Map params = new HashMap(); - if (activeSource != null) { - params.put("clientName", SourceEntityUtils.getSourceName(activeSource)); + if (StringUtils.isNoneBlank(sourceName)) { + params.put("clientName", sourceName); } return new InvalidPutCodeException(params); } diff --git a/orcid-core/src/main/java/org/orcid/core/exception/OrcidCoreExceptionMapper.java b/orcid-core/src/main/java/org/orcid/core/exception/OrcidCoreExceptionMapper.java index d09e1f386e6..8409f0657a4 100644 --- a/orcid-core/src/main/java/org/orcid/core/exception/OrcidCoreExceptionMapper.java +++ b/orcid-core/src/main/java/org/orcid/core/exception/OrcidCoreExceptionMapper.java @@ -47,6 +47,9 @@ public class OrcidCoreExceptionMapper { @Resource(name = "sourceManagerV3") private SourceManager sourceManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + private static Map, Pair> HTTP_STATUS_AND_ERROR_CODE_BY_THROWABLE_TYPE = new HashMap<>(); { // 301 @@ -226,7 +229,7 @@ public org.orcid.jaxb.model.v3.release.error.OrcidError getOrcidErrorV3(int erro Map params = null; if (t instanceof PutCodeFormatException) { params = new HashMap(); - params.put("clientName", SourceEntityUtils.getSourceName(sourceManager.retrieveActiveSource())); + params.put("clientName", sourceEntityUtils.getSourceName(sourceManager.retrieveActiveSource())); } else if (t instanceof ApplicationException) { params = ((ApplicationException) t).getParams(); } else if (t instanceof IllegalEnumValueException) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/ClientDetailsEntityCacheManager.java b/orcid-core/src/main/java/org/orcid/core/manager/ClientDetailsEntityCacheManager.java index c8c7cd078bf..8ba3d563483 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/ClientDetailsEntityCacheManager.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/ClientDetailsEntityCacheManager.java @@ -1,10 +1,15 @@ package org.orcid.core.manager; +import java.util.Collection; +import java.util.Map; + import org.orcid.persistence.jpa.entities.ClientDetailsEntity; public interface ClientDetailsEntityCacheManager { public ClientDetailsEntity retrieve(String clientId) throws IllegalArgumentException; + + public Map retrieveAll(Collection clientIds); public ClientDetailsEntity retrieveByIdP(String clientId) throws IllegalArgumentException; diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/AddressManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/AddressManagerImpl.java index b2df894a4c2..b5d82f4d6a6 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/AddressManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/AddressManagerImpl.java @@ -33,6 +33,9 @@ public class AddressManagerImpl extends AddressManagerReadOnlyImpl implements Ad @Resource protected SourceManager sourceManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Resource private ProfileEntityCacheManager profileEntityCacheManager; @@ -130,7 +133,7 @@ private boolean isDuplicated(AddressEntity existing, Address address, SourceEnti if (!existing.getId().equals(address.getPutCode())) { // If they have the same source String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(SourceEntityUtils.getSourceId(source))) { + if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(sourceEntityUtils.getSourceId(source))) { if (existing.getIso2Country().equals(address.getCountry().getValue())) { return true; } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImpl.java index b1e331c8235..1b48cff5f60 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImpl.java @@ -1,6 +1,11 @@ package org.orcid.core.manager.impl; +import java.util.ArrayList; +import java.util.Collection; import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import javax.annotation.Resource; @@ -46,6 +51,44 @@ public ClientDetailsEntity retrieve(String clientId) throws IllegalArgumentExcep return clientDetails; } + @Override + public Map retrieveAll(Collection clientIds) { + Map clientDetailsById = new HashMap<>(); + if (clientIds == null || clientIds.isEmpty()) { + return clientDetailsById; + } + + List clientIdList = new ArrayList<>(clientIds); + Map lastModifiedByClientId = clientDetailsManager.getLastModifiedByClientIds(clientIdList); + if (lastModifiedByClientId == null) { + return clientDetailsById; + } + List staleOrMissingClientIds = new ArrayList<>(); + + for (String clientId : clientIdList) { + Date dbDate = lastModifiedByClientId.get(clientId); + if (dbDate == null) { + continue; + } + Object key = new ClientIdCacheKey(clientId, releaseName); + ClientDetailsEntity clientDetails = clientDetailsCache.get(key); + if (needsFresh(dbDate, clientDetails)) { + staleOrMissingClientIds.add(clientId); + } else { + clientDetailsById.put(clientId, clientDetails); + } + } + + if (!staleOrMissingClientIds.isEmpty()) { + for (ClientDetailsEntity clientDetails : clientDetailsManager.findByClientIds(staleOrMissingClientIds)) { + clientDetailsCache.put(new ClientIdCacheKey(clientDetails.getId(), releaseName), clientDetails); + clientDetailsById.put(clientDetails.getId(), clientDetails); + } + } + + return clientDetailsById; + } + @Override public ClientDetailsEntity retrieveByIdP(String idp) throws IllegalArgumentException { Object key = new ClientIdCacheKey("IdP+" + idp, releaseName); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/ExternalIdentifierManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/ExternalIdentifierManagerImpl.java index 52c41dacf33..edc8e3a8e30 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/ExternalIdentifierManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/ExternalIdentifierManagerImpl.java @@ -34,10 +34,10 @@ public class ExternalIdentifierManagerImpl extends ExternalIdentifierManagerRead private OrcidSecurityManager orcidSecurityManager; @Resource - private ProfileEntityManager profileEntityManager; + private ProfileEntityCacheManager profileEntityCacheManager; @Resource - private ProfileEntityCacheManager profileEntityCacheManager; + private SourceEntityUtils sourceEntityUtils; @Override public PersonExternalIdentifier createExternalIdentifier(String orcid, PersonExternalIdentifier externalIdentifier, boolean isApiRequest) { @@ -109,7 +109,7 @@ private boolean isDuplicated(ExternalIdentifierEntity existing, PersonExternalId if (!existing.getId().equals(newExternalIdentifier.getPutCode())) { // If they have the same source String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(SourceEntityUtils.getSourceId(source))) { + if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(sourceEntityUtils.getSourceId(source))) { // And they have the same reference if ((PojoUtil.isEmpty(existing.getExternalIdReference()) && PojoUtil.isEmpty(newExternalIdentifier.getValue())) || (!PojoUtil.isEmpty(existing.getExternalIdReference()) && existing.getExternalIdReference().equals(newExternalIdentifier.getValue()))) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/PeerReviewManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/PeerReviewManagerImpl.java index 58d161de081..f4114c01653 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/PeerReviewManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/PeerReviewManagerImpl.java @@ -78,6 +78,9 @@ public class PeerReviewManagerImpl extends PeerReviewManagerReadOnlyImpl impleme @Resource private ProfileEntityCacheManager profileEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override public PeerReview createPeerReview(String orcid, PeerReview peerReview, boolean isApiRequest) { SourceEntity sourceEntity = sourceManager.retrieveSourceEntity(); @@ -90,7 +93,7 @@ public PeerReview createPeerReview(String orcid, PeerReview peerReview, boolean List peerReviews = peerReviewDao.getByUser(orcid, getLastModified(orcid)); // If it is the user adding the peer review, allow him to add // duplicates - if (!SourceEntityUtils.getSourceId(sourceEntity).equals(orcid)) { + if (!sourceEntityUtils.getSourceId(sourceEntity).equals(orcid)) { if (peerReviews != null) { for (PeerReviewEntity entity : peerReviews) { PeerReview existing = jpaJaxbPeerReviewAdapter.toPeerReview(entity); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/SourceNameCacheManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/SourceNameCacheManagerImpl.java index c567e21a9d7..96fc8bd1f6b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/SourceNameCacheManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/SourceNameCacheManagerImpl.java @@ -4,7 +4,6 @@ import org.apache.commons.lang.StringUtils; import org.ehcache.Cache; -import org.orcid.core.manager.ProfileEntityManager; import org.orcid.core.manager.SourceNameCacheManager; import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; import org.orcid.core.utils.ReleaseNameUtils; @@ -35,8 +34,6 @@ public class SourceNameCacheManagerImpl implements SourceNameCacheManager { private ClientDetailsDao clientDetailsDao; - private ProfileEntityManager profileEntityManager; - @Resource(name = "recordNameManagerReadOnlyV3") private RecordNameManagerReadOnly recordNameManagerReadOnlyV3; @@ -47,10 +44,7 @@ public void setRecordNameDao(RecordNameDao recordNameDao) { public void setClientDetailsDao(ClientDetailsDao clientDetailsDao) { this.clientDetailsDao = clientDetailsDao; } - - public void setProfileEntityManager(ProfileEntityManager profileEntityManager) { - this.profileEntityManager = profileEntityManager; - } + @Override public String retrieve(String sourceId) throws IllegalArgumentException { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/WorkManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/WorkManagerImpl.java index f56fb8dc841..29c8d73a551 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/impl/WorkManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/WorkManagerImpl.java @@ -95,6 +95,9 @@ public class WorkManagerImpl extends WorkManagerReadOnlyImpl implements WorkMana @Resource private ContributorsRolesAndSequencesConverterV2 contributorsRolesAndSequencesConverterV2; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Value("${org.orcid.core.work.contributors.ui.max:50}") private int maxContributorsForUI; @@ -160,7 +163,7 @@ public Work createWork(String orcid, Work work, boolean isApiRequest) { activityValidator.validateWork(work, sourceEntity, true, isApiRequest, null); // If it is the user adding the peer review, allow him to add // duplicates - if (!SourceEntityUtils.getSourceId(sourceEntity).equals(orcid)) { + if (!sourceEntityUtils.getSourceId(sourceEntity).equals(orcid)) { List existingWorks = this.findWorks(orcid); if (existingWorks != null) { if ((existingWorks.size() + 1) > this.maxNumOfActivities) { @@ -222,7 +225,7 @@ public WorkBulk createWorks(String orcid, WorkBulk workBulk) { if (workBulk.getBulk() != null && !workBulk.getBulk().isEmpty()) { List bulk = workBulk.getBulk(); Map extIDPutCodeMap = new HashMap(); - Set existingExternalIdentifiers = buildExistingExternalIdsSet(existingWorks, SourceEntityUtils.getSourceId(sourceEntity), extIDPutCodeMap); + Set existingExternalIdentifiers = buildExistingExternalIdsSet(existingWorks, sourceEntityUtils.getSourceId(sourceEntity), extIDPutCodeMap); if ((existingWorks.size() + bulk.size()) > this.maxNumOfActivities) { throw new ExceedMaxNumberOfElementsException(); } @@ -248,7 +251,7 @@ public WorkBulk createWorks(String orcid, WorkBulk workBulk) { // identifier, then mark it as duplicated if (existingExternalIdentifiers.contains(extId) && Relationship.SELF.equals(extId.getRelationship())) { Map params = new HashMap(); - params.put("clientName", sourceNameCacheManager.retrieve(SourceEntityUtils.getSourceId(sourceEntity))); + params.put("clientName", sourceNameCacheManager.retrieve(sourceEntityUtils.getSourceId(sourceEntity))); if (extIDPutCodeMap.containsKey(extId)) { params.put("putCode", String.valueOf(extIDPutCodeMap.get(extId))); } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/read_only/ClientDetailsManagerReadOnly.java b/orcid-core/src/main/java/org/orcid/core/manager/read_only/ClientDetailsManagerReadOnly.java index 8cbdf99badd..efae4e3e17b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/read_only/ClientDetailsManagerReadOnly.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/read_only/ClientDetailsManagerReadOnly.java @@ -2,6 +2,7 @@ import java.util.Date; import java.util.List; +import java.util.Map; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.springframework.security.oauth2.provider.ClientDetailsService; @@ -13,6 +14,10 @@ public interface ClientDetailsManagerReadOnly extends ClientDetailsService { Date getLastModified(String clientId); + Map getLastModifiedByClientIds(List clientIds); + + List findByClientIds(List clientIds); + Date getLastModifiedByIdp(String idp); boolean exists(String cliendId); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/read_only/impl/ClientDetailsManagerReadOnlyImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/read_only/impl/ClientDetailsManagerReadOnlyImpl.java index fed04a51b54..ccfa5a69902 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/read_only/impl/ClientDetailsManagerReadOnlyImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/read_only/impl/ClientDetailsManagerReadOnlyImpl.java @@ -3,6 +3,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.annotation.Resource; @@ -156,6 +157,16 @@ public Date getLastModified(String clientId) { return clientDetailsDao.getLastModified(clientId); } + @Override + public Map getLastModifiedByClientIds(List clientIds) { + return clientDetailsDao.getLastModifiedByClientIds(clientIds); + } + + @Override + public List findByClientIds(List clientIds) { + return clientDetailsDao.findByClientIds(clientIds); + } + @Override public Date getLastModifiedByIdp(String idp) { try { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AddressManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AddressManagerImpl.java index 996eac8440e..e217ba7b13b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AddressManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AddressManagerImpl.java @@ -40,6 +40,9 @@ public class AddressManagerImpl extends AddressManagerReadOnlyImpl implements Ad @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override @Transactional public Address updateAddress(String orcid, Long putCode, Address address, boolean isApiRequest) { @@ -48,7 +51,7 @@ public Address updateAddress(String orcid, Long putCode, Address address, boolea Visibility originalVisibility = Visibility.fromValue(updatedEntity.getVisibility()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(updatedEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(updatedEntity); // If it is an update from the API, check the source and preserve the // original visibility @@ -75,7 +78,7 @@ public Address updateAddress(String orcid, Long putCode, Address address, boolea adapter.toAddressEntity(address, updatedEntity); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedEntity); addressDao.merge(updatedEntity); return adapter.toAddress(updatedEntity); @@ -101,7 +104,7 @@ public Address createAddress(String orcid, Address address, boolean isApiRequest ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); newEntity.setOrcid(orcid); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(newEntity, isApiRequest); setIncomingPrivacy(newEntity, profile); @@ -127,7 +130,7 @@ private boolean isDuplicated(AddressEntity existing, Address address, Source act if (!existing.getId().equals(address.getPutCode())) { // If they have the same source String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existing, clientDetailsEntityCacheManager)) { + if (!PojoUtil.isEmpty(existingSourceId) && sourceEntityUtils.isTheSameSource(activeSource, existing)) { // TODO: Not sure this works! String vs Iso3166Country enum if (existing.getIso2Country().equals(address.getCountry().getValue())) { return true; @@ -188,7 +191,7 @@ public Addresses updateAddresses(String orcid, Addresses addresses) { AddressEntity newAddress = adapter.toAddressEntity(updatedOrNew); Source activeSource = sourceManager.retrieveActiveSource(); newAddress.setOrcid(orcid); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newAddress); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newAddress); newAddress.setVisibility(updatedOrNew.getVisibility().name()); newAddress.setDisplayIndex(updatedOrNew.getDisplayIndex()); addressDao.persist(newAddress); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AffiliationsManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AffiliationsManagerImpl.java index d8cb5665d6e..b01b07c024b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AffiliationsManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/AffiliationsManagerImpl.java @@ -55,6 +55,9 @@ public class AffiliationsManagerImpl extends AffiliationsManagerReadOnlyImpl imp @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + /** * Add a new distinction to the given user * @@ -292,7 +295,7 @@ private Affiliation createAffiliation(String orcid, Affiliation affiliation, boo OrgEntity updatedOrganization = orgManager.getOrgEntity(affiliation); entity.setOrg(updatedOrganization); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, entity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, entity); ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); setIncomingWorkPrivacy(entity, profile, isApiRequest); @@ -342,7 +345,7 @@ public Affiliation updateAffiliation(String orcid, Affiliation affiliation, bool Source activeSource = sourceManager.retrieveActiveSource(); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(entity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(entity); String originalVisibility = entity.getVisibility(); orcidSecurityManager.checkSourceAndThrow(entity); @@ -385,7 +388,7 @@ public Affiliation updateAffiliation(String orcid, Affiliation affiliation, bool // Populate display index in case it is missing DisplayIndexCalculatorHelper.setDisplayIndexOnExistingEntity(entity, isApiRequest); - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, entity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, entity); // Updates the give organization with the latest organization from // database, or, create a new one diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ExternalIdentifierManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ExternalIdentifierManagerImpl.java index 4bff791ffcd..f15baa6245b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ExternalIdentifierManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ExternalIdentifierManagerImpl.java @@ -34,14 +34,11 @@ public class ExternalIdentifierManagerImpl extends ExternalIdentifierManagerRead @Resource(name = "orcidSecurityManagerV3") private OrcidSecurityManager orcidSecurityManager; - @Resource(name = "profileEntityManagerV3") - private ProfileEntityManager profileEntityManager; - @Resource private ProfileEntityCacheManager profileEntityCacheManager; @Resource - private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + private SourceEntityUtils sourceEntityUtils; @Override public PersonExternalIdentifier createExternalIdentifier(String orcid, PersonExternalIdentifier externalIdentifier, boolean isApiRequest) { @@ -63,7 +60,7 @@ public PersonExternalIdentifier createExternalIdentifier(String orcid, PersonExt ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); newEntity.setOrcid(orcid); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); setIncomingPrivacy(newEntity, profile); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(newEntity, isApiRequest); @@ -77,7 +74,7 @@ public PersonExternalIdentifier updateExternalIdentifier(String orcid, PersonExt ExternalIdentifierEntity updatedExternalIdentifierEntity = externalIdentifierDao.getExternalIdentifierEntity(orcid, externalIdentifier.getPutCode()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(updatedExternalIdentifierEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(updatedExternalIdentifierEntity); Visibility originalVisibility = Visibility.valueOf(updatedExternalIdentifierEntity.getVisibility()); // Validate external identifier @@ -96,7 +93,7 @@ public PersonExternalIdentifier updateExternalIdentifier(String orcid, PersonExt jpaJaxbExternalIdentifierAdapter.toExternalIdentifierEntity(externalIdentifier, updatedExternalIdentifierEntity); // Set source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedExternalIdentifierEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedExternalIdentifierEntity); externalIdentifierDao.merge(updatedExternalIdentifierEntity); return jpaJaxbExternalIdentifierAdapter.toExternalIdentifier(updatedExternalIdentifierEntity); @@ -106,7 +103,7 @@ private boolean isDuplicated(ExternalIdentifierEntity existing, PersonExternalId if (!existing.getId().equals(newExternalIdentifier.getPutCode())) { // If they have the same source String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existing, clientDetailsEntityCacheManager)) { + if (!PojoUtil.isEmpty(existingSourceId) && sourceEntityUtils.isTheSameSource(activeSource, existing)) { // And they have the same reference if ((PojoUtil.isEmpty(existing.getExternalIdReference()) && PojoUtil.isEmpty(newExternalIdentifier.getValue())) || (!PojoUtil.isEmpty(existing.getExternalIdReference()) && existing.getExternalIdReference().equals(newExternalIdentifier.getValue()))) { 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 894e448237f..0966ce45b34 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 @@ -114,6 +114,9 @@ public class OrcidSecurityManagerImpl implements OrcidSecurityManager { @Resource private OrcidUserDetailsService orcidUserDetailsService; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override public boolean isAdmin() { return orcidUserDetailsService.isAdmin(); @@ -217,7 +220,7 @@ private boolean isOldEnough(ProfileEntity profile) { @Override public void checkSourceAndThrow(SourceAwareEntity existingEntity) { Source activeSource = sourceManager.retrieveActiveSource(); - if (activeSource != null && !SourceEntityUtils.isTheSameForPermissionChecking(activeSource, existingEntity, clientDetailsEntityCacheManager)) { + if (activeSource != null && !sourceEntityUtils.isTheSameSource(activeSource, existingEntity)) { Map params = new HashMap(); params.put("activity", "work"); throw new WrongSourceException(params); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OtherNameManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OtherNameManagerImpl.java index e12b77b40a9..fd9113af3d5 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OtherNameManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/OtherNameManagerImpl.java @@ -40,6 +40,9 @@ public class OtherNameManagerImpl extends OtherNameManagerReadOnlyImpl implement @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override @Transactional public boolean deleteOtherName(String orcid, Long putCode, boolean checkSource) { @@ -78,7 +81,7 @@ public OtherName createOtherName(String orcid, OtherName otherName, boolean isAp ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); newEntity.setOrcid(orcid); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); setIncomingPrivacy(newEntity, profile); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(newEntity, isApiRequest); @@ -93,7 +96,7 @@ public OtherName updateOtherName(String orcid, Long putCode, OtherName otherName OtherNameEntity updatedOtherNameEntity = otherNameDao.getOtherName(orcid, putCode); Visibility originalVisibility = Visibility.fromValue(updatedOtherNameEntity.getVisibility()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(updatedOtherNameEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(updatedOtherNameEntity); // Validate the other name PersonValidator.validateOtherName(otherName, activeSource, false, isApiRequest, originalVisibility); @@ -111,7 +114,7 @@ public OtherName updateOtherName(String orcid, Long putCode, OtherName otherName orcidSecurityManager.checkSourceAndThrow(updatedOtherNameEntity); jpaJaxbOtherNameAdapter.toOtherNameEntity(otherName, updatedOtherNameEntity); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedOtherNameEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedOtherNameEntity); otherNameDao.merge(updatedOtherNameEntity); return jpaJaxbOtherNameAdapter.toOtherName(updatedOtherNameEntity); @@ -160,7 +163,7 @@ public OtherNames updateOtherNames(String orcid, OtherNames otherNames) { Source activeSource = sourceManager.retrieveActiveSource(); newOtherName.setOrcid(orcid); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newOtherName); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newOtherName); newOtherName.setVisibility(updatedOrNew.getVisibility().name()); newOtherName.setDisplayIndex(updatedOrNew.getDisplayIndex()); otherNameDao.persist(newOtherName); @@ -173,7 +176,7 @@ public OtherNames updateOtherNames(String orcid, OtherNames otherNames) { private boolean isDuplicated(OtherNameEntity existing, OtherName otherName, Source activeSource) { if (!existing.getId().equals(otherName.getPutCode())) { String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existing, clientDetailsEntityCacheManager)) { + if (!PojoUtil.isEmpty(existingSourceId) && sourceEntityUtils.isTheSameSource(activeSource, existing)) { if (existing.getDisplayName() != null && existing.getDisplayName().equals(otherName.getContent())) { return true; } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/PeerReviewManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/PeerReviewManagerImpl.java index 2d392b760a3..625ed5ce552 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/PeerReviewManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/PeerReviewManagerImpl.java @@ -82,6 +82,9 @@ public class PeerReviewManagerImpl extends PeerReviewManagerReadOnlyImpl impleme @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override public PeerReview createPeerReview(String orcid, PeerReview peerReview, boolean isApiRequest) { Source activeSource = sourceManager.retrieveActiveSource(); @@ -117,7 +120,7 @@ public PeerReview createPeerReview(String orcid, PeerReview peerReview, boolean entity.setOrg(updatedOrganization); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, entity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, entity); ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); setIncomingPrivacy(entity, profile); @@ -137,7 +140,7 @@ public PeerReview updatePeerReview(String orcid, PeerReview peerReview, boolean Source activeSource = sourceManager.retrieveActiveSource(); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(existingEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(existingEntity); // If request comes from the API perform validations if (isApiRequest) { @@ -162,7 +165,7 @@ public PeerReview updatePeerReview(String orcid, PeerReview peerReview, boolean existingEntity.setVisibility(originalVisibility.name()); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, existingEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, existingEntity); createIssnGroupIdIfNecessary(peerReview); if (peerReview.getOrganization() != null) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileFundingManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileFundingManagerImpl.java index 62eb974beb4..9e35872679c 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileFundingManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileFundingManagerImpl.java @@ -55,6 +55,9 @@ public class ProfileFundingManagerImpl extends ProfileFundingManagerReadOnlyImpl @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + /** * Removes the relationship that exists between a funding and a profile. * @@ -143,7 +146,7 @@ public Funding createFunding(String orcid, Funding funding, boolean isApiRequest profileFundingEntity.setOrg(updatedOrganization); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, profileFundingEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, profileFundingEntity); ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); setIncomingPrivacy(profileFundingEntity, profile, isApiRequest); @@ -183,7 +186,7 @@ public Funding updateFunding(String orcid, Funding funding, boolean isApiRequest Visibility originalVisibility = Visibility.valueOf(pfe.getVisibility()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(pfe, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(pfe); activityValidator.validateFunding(funding, activeSOurce, false, isApiRequest, originalVisibility); if (isApiRequest) { @@ -203,7 +206,7 @@ public Funding updateFunding(String orcid, Funding funding, boolean isApiRequest } // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, pfe); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, pfe); // Updates the give organization with the latest organization from // database, or, create a new one diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileKeywordManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileKeywordManagerImpl.java index 9eea8781519..e75a0f3db82 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileKeywordManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileKeywordManagerImpl.java @@ -40,6 +40,9 @@ public class ProfileKeywordManagerImpl extends ProfileKeywordManagerReadOnlyImpl @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override public boolean deleteKeyword(String orcid, Long putCode, boolean checkSource) { ProfileKeywordEntity entity = profileKeywordDao.getProfileKeyword(orcid, putCode); @@ -77,7 +80,7 @@ public Keyword createKeyword(String orcid, Keyword keyword, boolean isApiRequest newEntity.setOrcid(orcid); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); setIncomingPrivacy(newEntity, profile); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(newEntity, isApiRequest); @@ -93,7 +96,7 @@ public Keyword updateKeyword(String orcid, Long putCode, Keyword keyword, boolea Visibility originalVisibility = Visibility.valueOf(updatedEntity.getVisibility()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(updatedEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(updatedEntity); // Validate the keyword PersonValidator.validateKeyword(keyword, activeSource, false, isApiRequest, originalVisibility); @@ -113,7 +116,7 @@ public Keyword updateKeyword(String orcid, Long putCode, Keyword keyword, boolea adapter.toProfileKeywordEntity(keyword, updatedEntity); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedEntity); profileKeywordDao.merge(updatedEntity); return adapter.toKeyword(updatedEntity); @@ -162,7 +165,7 @@ public Keywords updateKeywords(String orcid, Keywords keywords) { Source activeSource = sourceManager.retrieveActiveSource(); newKeyword.setOrcid(orcid); // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newKeyword); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newKeyword); newKeyword.setVisibility(updatedOrNew.getVisibility().name()); newKeyword.setDisplayIndex(updatedOrNew.getDisplayIndex()); profileKeywordDao.persist(newKeyword); @@ -177,7 +180,7 @@ public Keywords updateKeywords(String orcid, Keywords keywords) { private boolean isDuplicated(ProfileKeywordEntity existing, org.orcid.jaxb.model.v3.release.record.Keyword keyword, Source activeSource) { if (!existing.getId().equals(keyword.getPutCode())) { String existingSourceId = existing.getElementSourceId(); - if (!PojoUtil.isEmpty(existingSourceId) && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existing, clientDetailsEntityCacheManager)) { + if (!PojoUtil.isEmpty(existingSourceId) && sourceEntityUtils.isTheSameSource(activeSource, existing)) { if (existing.getKeywordName() != null && existing.getKeywordName().equals(keyword.getContent())) { return true; } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearchResourceManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearchResourceManagerImpl.java index 457cb357d48..133091b8592 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearchResourceManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearchResourceManagerImpl.java @@ -59,6 +59,9 @@ public class ResearchResourceManagerImpl extends ResearchResourceManagerReadOnly @Resource(name = "notificationManagerV3") private NotificationManager notificationManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override @Transactional public ResearchResource createResearchResource(String orcid, ResearchResource rr, boolean isApiRequest) { @@ -88,7 +91,7 @@ public ResearchResource createResearchResource(String orcid, ResearchResource rr } // Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, researchResourceEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, researchResourceEntity); ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); setIncomingPrivacy(researchResourceEntity, profile); @@ -108,7 +111,7 @@ public ResearchResource updateResearchResource(String orcid, ResearchResource rr Visibility originalVisibility = Visibility.valueOf(rre.getVisibility()); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(rre, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(rre); activityValidator.validateResearchResource(rr, activeSource, false, isApiRequest, originalVisibility); if (!isApiRequest) { @@ -127,7 +130,7 @@ public ResearchResource updateResearchResource(String orcid, ResearchResource rr rre.setVisibility(originalVisibility.name()); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, rre); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, rre); // update orgs (ordering managed by @OrderColumn on lists) List updatedOrganizations = orgManager.getOrgEntities(rr.getProposal().getHosts()); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearcherUrlManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearcherUrlManagerImpl.java index f99d412b013..fbe42ccbb0a 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearcherUrlManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ResearcherUrlManagerImpl.java @@ -44,6 +44,9 @@ public class ResearcherUrlManagerImpl extends ResearcherUrlManagerReadOnlyImpl i @Resource private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Override public boolean deleteResearcherUrl(String orcid, Long id, boolean checkSource) { boolean result = true; @@ -113,7 +116,7 @@ public ResearcherUrls updateResearcherUrls(String orcid, ResearcherUrls research ProfileEntity profile = new ProfileEntity(orcid); newResearcherUrl.setOrcid(orcid); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newResearcherUrl); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newResearcherUrl); newResearcherUrl.setVisibility(updatedOrNew.getVisibility().name()); newResearcherUrl.setDisplayIndex(updatedOrNew.getDisplayIndex()); @@ -133,7 +136,7 @@ public ResearcherUrl updateResearcherUrl(String orcid, ResearcherUrl researcherU Source activeSource = sourceManager.retrieveActiveSource(); // Save the original source - Source originalSource = SourceEntityUtils.extractSourceFromEntity(updatedResearcherUrlEntity, clientDetailsEntityCacheManager); + Source originalSource = sourceEntityUtils.extractSourceFromEntity(updatedResearcherUrlEntity); // Validate the researcher url PersonValidator.validateResearcherUrl(researcherUrl, activeSource, false, isApiRequest, originalVisibility); @@ -153,7 +156,7 @@ public ResearcherUrl updateResearcherUrl(String orcid, ResearcherUrl researcherU jpaJaxbResearcherUrlAdapter.toResearcherUrlEntity(researcherUrl, updatedResearcherUrlEntity); // Be sure it doesn't overwrite the source - SourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedResearcherUrlEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(originalSource, updatedResearcherUrlEntity); researcherUrlDao.merge(updatedResearcherUrlEntity); return jpaJaxbResearcherUrlAdapter.toResearcherUrl(updatedResearcherUrlEntity); @@ -179,7 +182,7 @@ public ResearcherUrl createResearcherUrl(String orcid, ResearcherUrl researcherU ProfileEntity profile = profileEntityCacheManager.retrieve(orcid); newEntity.setOrcid(orcid); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, newEntity); setIncomingPrivacy(newEntity, profile); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(newEntity, isApiRequest); @@ -192,7 +195,7 @@ private boolean isDuplicated(ResearcherUrlEntity existing, ResearcherUrl newRese // If they have the same source String existingSourceId = existing.getElementSourceId(); // If they have the same source - if (!PojoUtil.isEmpty(existingSourceId) && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existing, clientDetailsEntityCacheManager)) { + if (!PojoUtil.isEmpty(existingSourceId) && sourceEntityUtils.isTheSameSource(activeSource, existing)) { // If the url is the same if (existing.getUrl() != null && existing.getUrl().equals(newResearcherUrl.getUrl().getValue())) { return true; diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/WorkManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/WorkManagerImpl.java index d48199e9feb..097b5294114 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/WorkManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/WorkManagerImpl.java @@ -88,12 +88,6 @@ public class WorkManagerImpl extends WorkManagerReadOnlyImpl implements WorkMana @Resource(name = "activityValidatorV3") private ActivityValidator activityValidator; - @Resource(name = "groupingSuggestionManagerV3") - private GroupingSuggestionManager groupingSuggestionManager; - - @Resource(name = "groupingSuggestionManagerReadOnlyV3") - private GroupingSuggestionManagerReadOnly groupingSuggestionManagerReadOnly; - @Resource private MessageSource messageSource; @@ -115,6 +109,9 @@ public class WorkManagerImpl extends WorkManagerReadOnlyImpl implements WorkMana @Resource private WorkContributorRoleConverter workContributorsRoleConverter; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Value("${org.orcid.core.work.contributors.ui.max:50}") private int maxContributorsForUI; @@ -213,7 +210,7 @@ public Work createWork(String orcid, Work work, boolean isApiRequest) { workEntity.setFeaturedDisplayIndex(0); //Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); setIncomingWorkPrivacy(workEntity, profile, isApiRequest); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(workEntity, isApiRequest); @@ -273,7 +270,7 @@ public WorkBulk createWorks(String orcid, WorkBulk workBulk) { // If the external id exists and is a SELF identifier, then mark it as duplicated if(existingExternalIdentifiers.contains(extId) && Relationship.SELF.equals(extId.getRelationship())) { Map params = new HashMap(); - params.put("clientName", SourceEntityUtils.getSourceName(activeSource)); + params.put("clientName", sourceEntityUtils.getSourceName(activeSource)); if(extIDPutCodeMap.containsKey(extId)) { params.put("putCode", String.valueOf(extIDPutCodeMap.get(extId))); } @@ -287,7 +284,7 @@ public WorkBulk createWorks(String orcid, WorkBulk workBulk) { workEntity.setAddedToProfileDate(new Date()); workEntity.setFeaturedDisplayIndex(0); - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); setIncomingWorkPrivacy(workEntity, profile); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(workEntity, true); @@ -332,7 +329,7 @@ private Set buildExistingExternalIdsSet(List existingWorks, So Set existingExternalIds = new HashSet(); for(Work work : existingWorks) { //If it is the same source - if(SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, work.getSource())) { + if(sourceEntityUtils.isTheSameSource(activeSource, work.getSource())) { if(work.getExternalIdentifiers() != null && work.getExternalIdentifiers().getExternalIdentifier() != null) { for(ExternalID extId : work.getExternalIdentifiers().getExternalIdentifier()) { if(extIdsPutCodeMap != null) { @@ -547,7 +544,7 @@ public Work createWork(String orcid, WorkForm workForm) { workEntity.setFeaturedDisplayIndex(0); //Set the source - SourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); + sourceEntityUtils.populateSourceAwareEntityFromSource(activeSource, workEntity); setIncomingWorkPrivacy(workEntity, profile, false); DisplayIndexCalculatorHelper.setDisplayIndexOnNewEntity(workEntity, false); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ClientDetailsManagerReadOnly.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ClientDetailsManagerReadOnly.java index 74c2b38e6b5..2df876cbc7b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ClientDetailsManagerReadOnly.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ClientDetailsManagerReadOnly.java @@ -2,6 +2,7 @@ import java.util.Date; import java.util.List; +import java.util.Map; import org.orcid.jaxb.model.v3.release.client.ClientSummary; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; @@ -14,6 +15,10 @@ public interface ClientDetailsManagerReadOnly extends ClientDetailsService { Date getLastModified(String clientId); + Map getLastModifiedByClientIds(List clientIds); + + List findByClientIds(List clientIds); + Date getLastModifiedByIdp(String idp); boolean exists(String cliendId); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ClientDetailsManagerReadOnlyImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ClientDetailsManagerReadOnlyImpl.java index 50819febf2b..e5c93abf860 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ClientDetailsManagerReadOnlyImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ClientDetailsManagerReadOnlyImpl.java @@ -3,6 +3,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.annotation.Resource; @@ -157,6 +158,16 @@ public Date getLastModified(String clientId) { return clientDetailsDao.getLastModified(clientId); } + @Override + public Map getLastModifiedByClientIds(List clientIds) { + return clientDetailsDao.getLastModifiedByClientIds(clientIds); + } + + @Override + public List findByClientIds(List clientIds) { + return clientDetailsDao.findByClientIds(clientIds); + } + @Override public Date getLastModifiedByIdp(String idp) { try { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/RecordManagerReadOnlyImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/RecordManagerReadOnlyImpl.java index efb9a5bcba7..768d210270b 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/RecordManagerReadOnlyImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/RecordManagerReadOnlyImpl.java @@ -43,7 +43,10 @@ public class RecordManagerReadOnlyImpl implements RecordManagerReadOnly { @Resource protected SourceNameCacheManager sourceNameCacheManager; - + + @Resource + private SourceEntityUtils sourceEntityUtils; + protected ProfileEntityCacheManager profileEntityCacheManager; protected EmailManagerReadOnly emailManager; @@ -146,7 +149,7 @@ private History getHistory(String orcid) { } if(profile.getSource() != null) { - Source source = SourceEntityUtils.extractSourceFromProfileComplete(profile, sourceNameCacheManager, orcidUrlManager); + Source source = sourceEntityUtils.extractSourceFromProfileComplete(profile); history.setSource(source); } diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/WorkManagerReadOnlyImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/WorkManagerReadOnlyImpl.java index 1e8e27389b3..e37d95a2830 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/WorkManagerReadOnlyImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/WorkManagerReadOnlyImpl.java @@ -1,22 +1,10 @@ package org.orcid.core.manager.v3.read_only.impl; -import java.math.BigInteger; -import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.concurrent.ForkJoinPool; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - -import javax.annotation.PostConstruct; -import javax.annotation.Resource; - +import org.apache.commons.lang3.StringUtils; import org.orcid.core.adapter.jsonidentifier.converter.JSONWorkExternalIdentifiersConverterV3; import org.orcid.core.adapter.v3.JpaJaxbWorkAdapter; import org.orcid.core.adapter.v3.converter.ContributorsRolesAndSequencesConverter; import org.orcid.core.adapter.v3.converter.WorkContributorsConverter; -import org.orcid.core.contributors.roles.works.WorkContributorRoleConverter; import org.orcid.core.exception.ExceedMaxNumberOfPutCodesException; import org.orcid.core.exception.OrcidCoreExceptionMapper; import org.orcid.core.exception.PutCodeFormatException; @@ -25,37 +13,24 @@ import org.orcid.core.manager.WorkEntityCacheManager; import org.orcid.core.manager.v3.GroupingSuggestionManager; import org.orcid.core.manager.v3.WorksExtendedCacheManager; -import org.orcid.core.manager.v3.read_only.ClientDetailsManagerReadOnly; import org.orcid.core.manager.v3.read_only.WorkManagerReadOnly; import org.orcid.core.togglz.Features; +import org.orcid.core.utils.SourceEntityUtils; import org.orcid.core.utils.v3.ContributorUtils; -import org.orcid.core.utils.v3.activities.ActivitiesGroup; -import org.orcid.core.utils.v3.activities.ActivitiesGroupGenerator; -import org.orcid.core.utils.v3.activities.WorkComparators; -import org.orcid.core.utils.v3.activities.WorkGroupAndGroupingSuggestionGenerator; +import org.orcid.core.utils.v3.activities.*; import org.orcid.jaxb.model.record.bulk.BulkElement; import org.orcid.jaxb.model.v3.release.common.PublicationDate; -import org.orcid.jaxb.model.v3.release.record.ExternalID; -import org.orcid.jaxb.model.v3.release.record.ExternalIDs; -import org.orcid.jaxb.model.v3.release.record.GroupAble; -import org.orcid.jaxb.model.v3.release.record.GroupableActivity; -import org.orcid.jaxb.model.v3.release.record.Work; -import org.orcid.jaxb.model.v3.release.record.WorkBulk; +import org.orcid.jaxb.model.v3.release.common.Source; +import org.orcid.jaxb.model.v3.release.record.*; import org.orcid.jaxb.model.v3.release.record.summary.WorkGroup; import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary; import org.orcid.jaxb.model.v3.release.record.summary.Works; import org.orcid.persistence.dao.WorkDao; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.MinimizedWorkEntity; import org.orcid.persistence.jpa.entities.WorkEntity; import org.orcid.persistence.jpa.entities.WorkLastModifiedEntity; -import org.orcid.pojo.ActivityTitle; -import org.orcid.pojo.ActivityTitleSearchResult; -import org.orcid.pojo.ContributorsRolesAndSequences; -import org.orcid.pojo.WorkContributorsList; -import org.orcid.pojo.WorkExtended; -import org.orcid.pojo.WorkGroupExtended; -import org.orcid.pojo.WorkSummaryExtended; -import org.orcid.pojo.WorksExtended; +import org.orcid.pojo.*; import org.orcid.pojo.ajaxForm.PojoUtil; import org.orcid.pojo.ajaxForm.WorkForm; import org.orcid.pojo.grouping.WorkGroupingSuggestion; @@ -63,6 +38,12 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import javax.annotation.Resource; +import java.math.BigInteger; +import java.sql.Timestamp; +import java.util.*; +import java.util.stream.Collectors; + import static org.orcid.pojo.ajaxForm.PojoUtil.getWorkForm; public class WorkManagerReadOnlyImpl extends ManagerReadOnlyBaseImpl implements WorkManagerReadOnly { @@ -92,18 +73,12 @@ public class WorkManagerReadOnlyImpl extends ManagerReadOnlyBaseImpl implements @Resource(name = "contributorUtilsV3") private ContributorUtils contributorUtils; - @Resource - private WorkContributorRoleConverter workContributorsRoleConverter; - @Resource(name = "workContributorsConverter") private WorkContributorsConverter workContributorsConverter; @Resource private JSONWorkExternalIdentifiersConverterV3 jsonWorkExternalIdentifiersConverterV3; - @Resource(name = "clientDetailsManagerReadOnlyV3") - private ClientDetailsManagerReadOnly clientDetailsManagerReadOnly; - @Resource protected ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; @@ -113,17 +88,12 @@ public class WorkManagerReadOnlyImpl extends ManagerReadOnlyBaseImpl implements @Resource private ContributorsRolesAndSequencesConverter contributorsRolesAndSequencesConverter; + @Resource + private SourceEntityUtils sourceEntityUtils; + @Value("${org.orcid.core.work.contributors.ui.max:50}") private int maxContributorsForUI; - @Value("${org.orcid.core.manager.v3.read_only.impl.WorkManagerReadOnlyImpl.normalize.min.works.to.parallelize:20}") - private int minWorksToParallelize; - - @PostConstruct - public void init() { - LOGGER.info("Initializing WorkManagerReadOnlyImpl: minWorksToParallelize = " + minWorksToParallelize); - } - public WorkManagerReadOnlyImpl(@Value("${org.orcid.core.works.bulk.read.max:100}") Integer bulkReadSize) { this.maxWorksToRead = (bulkReadSize == null) ? 100 : bulkReadSize; } @@ -206,22 +176,27 @@ public WorkSummary getWorkSummary(String orcid, Long workId) { @Override public List getWorksSummaryList(String orcid) { List works = workEntityCacheManager.retrieveMinimizedWorks(orcid, getLastModified(orcid)); - List workSummaryResult = Arrays.asList(); - // Lets implement a parallel stream for when there are more than 10 works to be processed - if(works.size() < minWorksToParallelize) { - workSummaryResult = jpaJaxbWorkAdapter.toWorkSummaryFromMinimized(works); - } else { - try { - // Execute the parallel stream within the custom thread pool - workSummaryResult = ForkJoinPool.commonPool().submit(() -> - works.parallelStream().map(minimizedWorkEntity -> jpaJaxbWorkAdapter.toWorkSummary(minimizedWorkEntity)).collect(Collectors.toList()) - ).get(); // use .get() to wait for completion and propagate exceptions - - } catch (Exception e) { - LOGGER.error("Error while generating the list of work summaries in parallel", e); + Set clientIds = works.stream() + .map(MinimizedWorkEntity::getClientSourceId) + .filter(clientId -> !PojoUtil.isEmpty(clientId)) + .collect(Collectors.toSet()); + + // Get the client details from the database + Map clientDetailsById = clientDetailsEntityCacheManager.retrieveAll(clientIds); + Map sources = new HashMap<>(); + works.stream().forEach(workEntity -> { + String sourceKey = sourceEntityUtils.getSourceKey(workEntity); + if(!sources.containsKey(sourceKey)) { + Source source = sourceEntityUtils.extractSourceFromEntityComplete(workEntity); + sources.put(sourceKey, source); } + }); + if (clientIds.isEmpty()) { + return jpaJaxbWorkAdapter.toWorkSummaryFromMinimized(works); } - return workSummaryResult; + + // This map should be read-only + return jpaJaxbWorkAdapter.toWorkSummaryFromMinimized(works, Collections.unmodifiableMap(sources)); } /** @@ -250,6 +225,7 @@ public List getWorksSummaryExtendedList(String orcid, boole private List retrieveWorkSummaryExtended(String orcid, boolean featuredOnly) { List workSummaryExtendedList = new ArrayList<>(); + Map isUserOBOEnabled = new HashMap(); List list = workDao.getWorksByOrcid(orcid, featuredOnly); for (Object[] q1 : list) { BigInteger putCode = (BigInteger) q1[0]; @@ -276,21 +252,34 @@ private List retrieveWorkSummaryExtended(String orcid, bool } String sourceName = null; String assertionOriginName = null; - if (clientSourceId != null) { - assertionOriginSourceId = contributorUtils.getAssertionOriginOrcid(clientSourceId, orcid, putCode.longValue(), clientDetailsEntityCacheManager, workDao); + if (StringUtils.isNotBlank(clientSourceId)) { + //Set the source name + sourceName = sourceNameCacheManager.retrieve(clientSourceId); + // Check if user OBO is enabled + if (!PojoUtil.isEmpty(assertionOriginSourceId)) { + if(!isUserOBOEnabled.containsKey(clientSourceId)) { + ClientDetailsEntity clientEntity = clientDetailsEntityCacheManager.retrieve(clientSourceId); + if(clientEntity != null && clientEntity.isUserOBOEnabled()) { + isUserOBOEnabled.put(clientSourceId, true); + } else { + isUserOBOEnabled.put(clientSourceId, false); + } + } + if(isUserOBOEnabled.get(clientSourceId)) { + assertionOriginName = sourceNameCacheManager.retrieve(assertionOriginSourceId); + } + } } - if (!PojoUtil.isEmpty(assertionOriginSourceId)) { - assertionOriginName = contributorUtils.getSourceName(assertionOriginSourceId, sourceNameCacheManager); + + // Check the sourceId name only if there is no clientSourceId + if (PojoUtil.isEmpty(sourceName) && !PojoUtil.isEmpty(sourceId)) { + sourceName = sourceNameCacheManager.retrieve(sourceId); } + if (!PojoUtil.isEmpty(assertionOriginClientSourceId)) { - assertionOriginName = contributorUtils.getSourceName(assertionOriginClientSourceId, sourceNameCacheManager); - } - if (!PojoUtil.isEmpty(sourceId)) { - sourceName = contributorUtils.getSourceName(sourceId, sourceNameCacheManager); - } - if (!PojoUtil.isEmpty(clientSourceId)) { - sourceName = contributorUtils.getSourceName(clientSourceId, sourceNameCacheManager); + assertionOriginName = sourceNameCacheManager.retrieve(assertionOriginClientSourceId); } + List contributorList = new ArrayList<>(); List contributorsRolesAndSequencesList = new ArrayList<>(); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/ActivityValidator.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/ActivityValidator.java index 8f310d1a961..ba33bfab4df 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/ActivityValidator.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/ActivityValidator.java @@ -58,7 +58,6 @@ import org.orcid.jaxb.model.v3.release.common.Year; import org.orcid.jaxb.model.v3.release.groupid.GroupIdRecord; import org.orcid.jaxb.model.v3.release.record.Affiliation; -import org.orcid.jaxb.model.v3.release.record.AffiliationType; import org.orcid.jaxb.model.v3.release.record.Education; import org.orcid.jaxb.model.v3.release.record.Employment; import org.orcid.jaxb.model.v3.release.record.ExternalID; @@ -286,7 +285,7 @@ public void validateWork(Work work, Source activeSource, boolean createFlag, boo } if (work.getPutCode() != null && createFlag) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(sourceEntityUtils.getSourceName(activeSource)); } // Check that we are not changing the visibility @@ -340,7 +339,7 @@ public void validateFunding(Funding funding, Source activeSource, boolean create } if (funding.getPutCode() != null && createFlag) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(sourceEntityUtils.getSourceName(activeSource)); } if (isApiRequest) { @@ -447,7 +446,7 @@ private void validateDisambiguatedOrg(MultipleOrganizationHolder organizationHol public void validateAffiliation(Affiliation affiliation, Source activeSource, boolean createFlag, boolean isApiRequest, Visibility originalVisibility) { if (affiliation.getPutCode() != null && createFlag) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(sourceEntityUtils.getSourceName(activeSource)); } // Check that we are not changing the visibility @@ -486,7 +485,7 @@ public void validatePeerReview(PeerReview peerReview, Source activeSource, boole } if (peerReview.getPutCode() != null && createFlag) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(sourceEntityUtils.getSourceName(activeSource)); } if (peerReview.getType() == null) { @@ -555,9 +554,9 @@ public void checkExternalIdentifiersForDuplicates(ExternalIdentifiersAwareActivi existingId.setNormalized(new TransientNonEmptyString(norm.normalise(existingId.getType(), existingId.getValue()))); } if (areRelationshipsSameAndSelf(existingId.getRelationship(), newId.getRelationship()) && newId.equals(existingId) - && SourceEntityUtils.isTheSameForDuplicateChecking(activeSource, existingSource)) { + && sourceEntityUtils.isTheSameSource(activeSource, existingSource)) { Map params = new HashMap(); - params.put("clientName", SourceEntityUtils.getSourceName(activeSource)); + params.put("clientName", sourceEntityUtils.getSourceName(activeSource)); params.put("putCode", String.valueOf(theExisting.getPutCode())); throw new OrcidDuplicatedActivityException(params); } @@ -607,7 +606,7 @@ public void validateResearchResource(ResearchResource rr, Source activeSource, b } if (rr.getPutCode() != null && createFlag) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(sourceEntityUtils.getSourceName(activeSource)); } // Check that we are not changing the visibility diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/PersonValidator.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/PersonValidator.java index fd30b6de1e1..a19335dfc79 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/PersonValidator.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/validator/PersonValidator.java @@ -7,6 +7,7 @@ import org.orcid.core.exception.OrcidValidationException; import org.orcid.core.exception.PutCodeRequiredException; import org.orcid.core.exception.VisibilityMismatchException; +import org.orcid.core.utils.SourceEntityUtils; import org.orcid.jaxb.model.common.Relationship; import org.orcid.jaxb.model.v3.release.common.Source; import org.orcid.jaxb.model.v3.release.common.Visibility; @@ -21,10 +22,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Resource; + public class PersonValidator { private static final Logger LOGGER = LoggerFactory.getLogger(PersonValidator.class); - + public static void validateResearcherUrl(ResearcherUrl researcherUrl, Source activeSource, boolean createFlag, boolean isApiRequest, Visibility originalVisibility) { if(researcherUrl == null) return; @@ -50,7 +53,7 @@ public static void validateResearcherUrl(ResearcherUrl researcherUrl, Source act } if(createFlag) { if(researcherUrl.getPutCode() != null) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(SourceEntityUtils.getSourceName(activeSource)); } } else { if(researcherUrl.getPutCode() == null) { @@ -66,7 +69,7 @@ public static void validateResearcherUrl(ResearcherUrl researcherUrl, Source act public static void validateOtherName(OtherName otherName, Source activeSource, boolean createFlag, boolean isApiRequest, Visibility originalVisibility) { if(createFlag) { if(otherName.getPutCode() != null) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(SourceEntityUtils.getSourceName(activeSource)); } } else { if(otherName.getPutCode() == null) { @@ -128,7 +131,7 @@ public static void validateExternalIdentifier(PersonExternalIdentifier externalI if(createFlag) { if(externalIdentifier.getPutCode() != null) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(SourceEntityUtils.getSourceName(activeSource)); } } else { if(externalIdentifier.getPutCode() == null) { @@ -144,7 +147,7 @@ public static void validateExternalIdentifier(PersonExternalIdentifier externalI public static void validateKeyword(Keyword keyword, Source activeSource, boolean createFlag, boolean isApiRequest, Visibility originalVisibility) { if(createFlag) { if(keyword.getPutCode() != null) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(SourceEntityUtils.getSourceName(activeSource)); } } else { if(keyword.getPutCode() == null) { @@ -166,7 +169,7 @@ public static void validateKeyword(Keyword keyword, Source activeSource, boolean public static void validateAddress(Address address, Source activeSource, boolean createFlag, boolean isApiRequest, Visibility originalVisibility) { if(createFlag) { if(address.getPutCode() != null) { - throw InvalidPutCodeException.forSource(activeSource); + throw InvalidPutCodeException.forSource(SourceEntityUtils.getSourceName(activeSource)); } } else { if(address.getPutCode() == null) { diff --git a/orcid-core/src/main/java/org/orcid/core/manager/validator/ActivityValidator.java b/orcid-core/src/main/java/org/orcid/core/manager/validator/ActivityValidator.java index f4e86b931f0..c56c74b7919 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/validator/ActivityValidator.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/validator/ActivityValidator.java @@ -470,7 +470,7 @@ public void checkExternalIdentifiersForDuplicates(ExternalIdentifiersAwareActivi for (ExternalID existingId : existingExtIds.getExternalIdentifier()) { for (ExternalID newId : newExtIds.getExternalIdentifier()) { if (areRelationshipsSameButNotBothPartOf(existingId.getRelationship(), newId.getRelationship()) && newId.equals(existingId) - && SourceEntityUtils.getSourceId(sourceEntity).equals(getExistingSource(existingSource))) { + && sourceEntityUtils.getSourceId(sourceEntity).equals(getExistingSource(existingSource))) { Map params = new HashMap(); params.put("clientName", sourceEntityUtils.getSourceName(sourceEntity)); params.put("putCode", String.valueOf(theExisting.getPutCode())); diff --git a/orcid-core/src/main/java/org/orcid/core/utils/SourceEntityUtils.java b/orcid-core/src/main/java/org/orcid/core/utils/SourceEntityUtils.java index 747c178af4f..fcabb8accba 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/SourceEntityUtils.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/SourceEntityUtils.java @@ -1,5 +1,7 @@ package org.orcid.core.utils; +import java.util.Optional; + import javax.annotation.Resource; import org.apache.commons.lang3.StringUtils; @@ -19,53 +21,58 @@ public class SourceEntityUtils { + public static final String SOURCE_MAP = "sourceMap"; + @Resource(name = "recordNameManagerReadOnlyV3") private RecordNameManagerReadOnly recordNameManagerReadOnlyV3; + @Resource(name = "sourceNameCacheManager") + private SourceNameCacheManager sourceNameCacheManager; + + @Resource + private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + @Resource private OrcidUrlManager orcidUrlManager; - public String getSourceName(SourceEntity sourceEntity) { - if (sourceEntity.getCachedSourceName() != null) { - return sourceEntity.getCachedSourceName(); + public static String getSourceId(SourceEntity sourceEntity) { + if (sourceEntity.getCachedSourceId() != null) { + return sourceEntity.getCachedSourceId(); } if (sourceEntity.getSourceClient() != null) { - return sourceEntity.getSourceClient().getClientName(); + return sourceEntity.getSourceClient().getClientId(); } if (sourceEntity.getSourceProfile() != null) { - String orcid = sourceEntity.getSourceProfile().getId(); - // Set the source name - return recordNameManagerReadOnlyV3.fetchDisplayablePublicName(orcid); + return sourceEntity.getSourceProfile().getId(); } return null; } - /** - * Call this method before storing in cache to prevent a whole profile or - * client being serialized. - * - * WARNING: The entity must be detached (using DAO) so that the source is - * not made null in DB. - */ - public void prepareForCache(SourceEntity sourceEntity) { - if (!sourceEntity.isDetached()) { - throw new IllegalStateException("Must not prepare source entity for cache, unless it is detached"); - } - sourceEntity.setCachedSourceId(getSourceId(sourceEntity)); - sourceEntity.setCachedSourceName(getSourceName(sourceEntity)); - sourceEntity.setSourceClient(null); - sourceEntity.setSourceProfile(null); + public static String getSourceName(Source activeSource) { + if (activeSource != null && activeSource.getSourceName() != null && !StringUtils.isEmpty(activeSource.getSourceName().getContent())) + return activeSource.getSourceName().getContent(); + else + return null; } - public static String getSourceId(SourceEntity sourceEntity) { - if (sourceEntity.getCachedSourceId() != null) { - return sourceEntity.getCachedSourceId(); + public static String getSourceKey(SourceAwareEntity e) { + String sourceOrcid = Optional.ofNullable(e.getSourceId()).filter(StringUtils::isNotEmpty).orElse("-"); + String clientSourceId = Optional.ofNullable(e.getClientSourceId()).filter(StringUtils::isNotEmpty).orElse("-"); + String assertionOriginClientSourceId = Optional.ofNullable(e.getAssertionOriginClientSourceId()).filter(StringUtils::isNotEmpty).orElse("-"); + return String.join("|", sourceOrcid, clientSourceId, assertionOriginClientSourceId); + } + + public String getSourceName(SourceEntity sourceEntity) { + if (sourceEntity.getCachedSourceName() != null) { + return sourceEntity.getCachedSourceName(); } if (sourceEntity.getSourceClient() != null) { - return sourceEntity.getSourceClient().getClientId(); + return sourceEntity.getSourceClient().getClientName(); } if (sourceEntity.getSourceProfile() != null) { - return sourceEntity.getSourceProfile().getId(); + String orcid = sourceEntity.getSourceProfile().getId(); + // Set the source name + return recordNameManagerReadOnlyV3.fetchDisplayablePublicName(orcid); } return null; } @@ -78,7 +85,7 @@ public static String getSourceId(SourceEntity sourceEntity) { * @param to */ @SuppressWarnings("deprecation") - public static void populateSourceAwareEntityFromSource(Source from, SourceAwareEntity to) { + public void populateSourceAwareEntityFromSource(Source from, SourceAwareEntity to) { // Set the source if (from.getSourceOrcid() != null && from.getSourceOrcid().getPath() != null) { to.setSourceId(from.getSourceOrcid().getPath()); @@ -94,9 +101,8 @@ public static void populateSourceAwareEntityFromSource(Source from, SourceAwareE /** * Utility that copies source ids from entity into new Source model. - * */ - public static Source extractSourceFromEntity(SourceAwareEntity e, ClientDetailsEntityCacheManager clientDetailsEntityCacheManager) { + public Source extractSourceFromEntity(SourceAwareEntity e) { Source source = new Source(); // orcid if (!StringUtils.isEmpty(e.getSourceId())) { @@ -108,11 +114,8 @@ public static Source extractSourceFromEntity(SourceAwareEntity e, ClientDetai source.setSourceClientId(new SourceClientId(e.getClientSourceId())); if(e instanceof OrcidAware) { ClientDetailsEntity clientSource = clientDetailsEntityCacheManager.retrieve(e.getClientSourceId()); - if (clientSource.isUserOBOEnabled()) { - String orcidId = null; - if (e instanceof OrcidAware) { - orcidId = ((OrcidAware) e).getOrcid(); - } + if (clientSource != null && clientSource.isUserOBOEnabled()) { + String orcidId = ((OrcidAware) e).getOrcid(); source.setAssertionOriginOrcid(new SourceOrcid(orcidId)); } } @@ -126,7 +129,7 @@ public static Source extractSourceFromEntity(SourceAwareEntity e, ClientDetai return source; } - public static Source extractSourceFromProfileComplete(ProfileEntity profile, SourceNameCacheManager sourceNameCacheManager, OrcidUrlManager orcidUrlManager) { + public Source extractSourceFromProfileComplete(ProfileEntity profile) { Source source = new Source(); SourceEntity entity = profile.getSource(); if (entity.getSourceProfile() != null) { @@ -135,18 +138,17 @@ public static Source extractSourceFromProfileComplete(ProfileEntity profile, Sou if (entity.getSourceClient() != null) { source.setSourceClientId(new SourceClientId(entity.getSourceClient().getId())); } - populateSource(source, sourceNameCacheManager, orcidUrlManager); + populateSource(source); return source; } - public static Source extractSourceFromEntityComplete(SourceAwareEntity b, SourceNameCacheManager sourceNameCacheManager, OrcidUrlManager orcidUrlManager, - ClientDetailsEntityCacheManager clientDetailsEntityCacheManager) { - Source s = extractSourceFromEntity(b, clientDetailsEntityCacheManager); - populateSource(s, sourceNameCacheManager, orcidUrlManager); + public Source extractSourceFromEntityComplete(SourceAwareEntity b) { + Source s = extractSourceFromEntity(b); + populateSource(s); return s; } - public static void populateSource(Source s, SourceNameCacheManager sourceNameCacheManager, OrcidUrlManager orcidUrlManager) { + private void populateSource(Source s) { // Set the source if (s.getSourceOrcid() != null && s.getSourceOrcid().getPath() != null) { s.getSourceOrcid().setHost(orcidUrlManager.getBaseHost()); @@ -186,47 +188,12 @@ public static void populateSource(Source s, SourceNameCacheManager sourceNameCac // ================================= // utils to help refactoring for OBO // ================================= - - /** - * Used to check for duplicates adding via API. - * - * @param active - * @param existing - * @return - */ - public static boolean isTheSameForDuplicateChecking(Source activeSource, SourceAwareEntity existingEntity, - ClientDetailsEntityCacheManager clientDetailsEntityCacheManager) { - Source existing = extractSourceFromEntity(existingEntity, clientDetailsEntityCacheManager); - return existing.equals(activeSource); - } - - public static boolean isTheSameForDuplicateChecking(Source active, Source existing) { + public boolean isTheSameSource(Source active, Source existing) { return existing.equals(active); } - /** - * Used only for errors when validating I think... - * - * @param activeSource - * @return - */ - public static String getSourceName(Source activeSource) { - if (activeSource.getSourceName() != null && !StringUtils.isEmpty(activeSource.getSourceName().getContent())) - return activeSource.getSourceName().getContent(); - else - return null; - } - - /** - * Used to check if activeSource can update/delete an item. - * - * @param activeSource - * @param existingEntity - * @return - */ - public static boolean isTheSameForPermissionChecking(Source activeSource, SourceAwareEntity existingEntity, - ClientDetailsEntityCacheManager clientDetailsEntityCacheManager) { - Source existing = extractSourceFromEntity(existingEntity, clientDetailsEntityCacheManager); - return existing.equals(activeSource); + public boolean isTheSameSource(Source active, SourceAwareEntity existingEntity) { + Source existing = extractSourceFromEntity(existingEntity); + return existing.equals(active); } } diff --git a/orcid-core/src/main/java/org/orcid/core/utils/v3/ContributorUtils.java b/orcid-core/src/main/java/org/orcid/core/utils/v3/ContributorUtils.java index cbabb769235..8c910bc2e85 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/v3/ContributorUtils.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/v3/ContributorUtils.java @@ -1,57 +1,40 @@ package org.orcid.core.utils.v3; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - import org.apache.commons.lang3.StringUtils; -import org.orcid.core.aop.ProfileLastModifiedAspect; import org.orcid.core.contributors.roles.credit.CreditRole; -import org.orcid.core.manager.ClientDetailsEntityCacheManager; -import org.orcid.core.manager.SourceNameCacheManager; -import org.orcid.core.manager.v3.ActivityManager; import org.orcid.core.manager.v3.ProfileEntityManager; +import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; import org.orcid.jaxb.model.v3.release.common.Contributor; import org.orcid.jaxb.model.v3.release.common.ContributorAttributes; import org.orcid.jaxb.model.v3.release.common.CreditName; import org.orcid.jaxb.model.v3.release.record.Funding; import org.orcid.jaxb.model.v3.release.record.FundingContributor; -import org.orcid.persistence.dao.WorkDao; -import org.orcid.persistence.jpa.entities.ClientDetailsEntity; -import org.orcid.persistence.jpa.entities.OrcidAware; -import org.orcid.persistence.jpa.entities.WorkEntity; import org.orcid.pojo.ContributorsRolesAndSequences; import org.orcid.pojo.ajaxForm.PojoUtil; -import org.springframework.beans.factory.annotation.Value; + +import javax.annotation.Resource; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; public class ContributorUtils { - private final Integer BATCH_SIZE; - - private ActivityManager cacheManager; + @Resource(name = "recordNameManagerReadOnlyV3") + private RecordNameManagerReadOnly recordNameManagerReadOnlyV3; - private ProfileEntityManager profileEntityManager; + @Resource(name = "profileEntityManagerV3") + private ProfileEntityManager profileEntityManagerV3; - protected ProfileLastModifiedAspect profileLastModifiedAspect; - - public ContributorUtils(@Value("${org.orcid.contributor.names.batch_size:2500}") Integer batchSize) { - if(batchSize == null) { - BATCH_SIZE = 2500; - } else { - BATCH_SIZE = batchSize; - } - } - public void filterContributorPrivateData(Funding funding) { if (funding.getContributors() != null && funding.getContributors().getContributor() != null) { for (FundingContributor contributor : funding.getContributors().getContributor()) { contributor.setContributorEmail(null); if (!PojoUtil.isEmpty(contributor.getContributorOrcid())) { String contributorOrcid = contributor.getContributorOrcid().getPath(); - if (profileEntityManager.orcidExists(contributorOrcid)) { + if (profileEntityManagerV3.orcidExists(contributorOrcid)) { // contributor is an ORCID user - visibility of user's // name in record must be taken into account - String publicContributorCreditName = cacheManager.getPublicCreditName(contributorOrcid); + String publicContributorCreditName = recordNameManagerReadOnlyV3.fetchDisplayablePublicName(contributorOrcid); CreditName creditName = new CreditName(publicContributorCreditName != null ? publicContributorCreditName : ""); contributor.setCreditName(creditName); } @@ -60,18 +43,6 @@ public void filterContributorPrivateData(Funding funding) { } } - public void setCacheManager(ActivityManager cacheManager) { - this.cacheManager = cacheManager; - } - - public void setProfileEntityManager(ProfileEntityManager profileEntityManager) { - this.profileEntityManager = profileEntityManager; - } - - public void setProfileLastModifiedAspect(ProfileLastModifiedAspect profileLastModifiedAspect) { - this.profileLastModifiedAspect = profileLastModifiedAspect; - } - public List getContributorsGroupedByOrcid(List contributors, Integer maxContributorsForUI) { List contributorsRolesAndSequencesList = new ArrayList<>(); for(Contributor contributor : contributors) { @@ -156,32 +127,12 @@ private ContributorsRolesAndSequences addContributor(Contributor contributor) { return crs; } - public String getCreditRole(String contributorRole) { + public static String getCreditRole(String contributorRole) { try { - CreditRole cr = CreditRole.fromValue(contributorRole); - return cr.getUiValue(); - } catch(IllegalArgumentException e) { + return CreditRole.fromValue(contributorRole).getUiValue(); + } catch(Exception e) { return contributorRole; } } - public String getAssertionOriginOrcid(String clientSourceId, String orcid, Long putCode, ClientDetailsEntityCacheManager clientDetailsEntityCacheManager, WorkDao workDao) { - String assertionOriginOrcid = null; - ClientDetailsEntity clientSource = clientDetailsEntityCacheManager.retrieve(clientSourceId); - if (clientSource.isUserOBOEnabled()) { - WorkEntity e = workDao.getWork(orcid, putCode); - - String orcidId = null; - if (e instanceof OrcidAware) { - orcidId = ((OrcidAware) e).getOrcid(); - } - assertionOriginOrcid = orcidId; - } - - return assertionOriginOrcid; - } - - public String getSourceName(String sourceId, SourceNameCacheManager sourceNameCacheManager) { - return sourceNameCacheManager.retrieve(sourceId); - } } diff --git a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/ActivitiesGroupGenerator.java b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/ActivitiesGroupGenerator.java index ef4d4f7dcd0..a01a17f5939 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/ActivitiesGroupGenerator.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/ActivitiesGroupGenerator.java @@ -55,7 +55,7 @@ protected ActivitiesGroup createNewGroup(GroupableActivity activity) { protected List generateBelongsToList(GroupableActivity activity) { List belongsTo = new ArrayList(); ActivitiesGroup thisGroup = new ActivitiesGroup(activity); - for (GroupAble g :thisGroup.getGroupKeys()){ + for (GroupAble g : thisGroup.getGroupKeys()){ if (lookup.containsKey(g)) { belongsTo.add(lookup.get(g)); } diff --git a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/WorkGroupAndGroupingSuggestionGenerator.java b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/WorkGroupAndGroupingSuggestionGenerator.java index 227b52fd1f3..60e07df241f 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/WorkGroupAndGroupingSuggestionGenerator.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/v3/activities/WorkGroupAndGroupingSuggestionGenerator.java @@ -33,7 +33,6 @@ public void group(GroupableActivity activity) { workSummary = (WorkSummary) activity; } - if (groups.isEmpty()) { // If it is the first activity, create a new group for it ActivitiesGroup newGroup = createNewGroup(activity); diff --git a/orcid-core/src/main/java/org/orcid/pojo/ContributorsRolesAndSequences.java b/orcid-core/src/main/java/org/orcid/pojo/ContributorsRolesAndSequences.java index ba9c9a59ade..b60fb8bdfee 100644 --- a/orcid-core/src/main/java/org/orcid/pojo/ContributorsRolesAndSequences.java +++ b/orcid-core/src/main/java/org/orcid/pojo/ContributorsRolesAndSequences.java @@ -59,10 +59,9 @@ public boolean compare(Object obj) { } AtomicBoolean isDifferent = new AtomicBoolean(false); WorkContributorRoleConverter roleConverter = new WorkContributorRoleConverter(); - ContributorUtils contributorUtils = new ContributorUtils(null); for (int i = 0; i < rolesAndSequences.size() ; i++) { if (rolesAndSequences.get(i).getContributorRole() != null && other.rolesAndSequences.get(i).getContributorRole() != null) { - if (!WorkForm.compareStrings(rolesAndSequences.get(i).getContributorRole(), contributorUtils.getCreditRole(roleConverter.toRoleValue(other.rolesAndSequences.get(i).getContributorRole())))) { + if (!WorkForm.compareStrings(rolesAndSequences.get(i).getContributorRole(), ContributorUtils.getCreditRole(roleConverter.toRoleValue(other.rolesAndSequences.get(i).getContributorRole())))) { isDifferent.set(true); break; } diff --git a/orcid-core/src/main/java/org/orcid/pojo/grouping/WorkGroup.java b/orcid-core/src/main/java/org/orcid/pojo/grouping/WorkGroup.java index 2dbf34923d8..5ca5b5d2daf 100644 --- a/orcid-core/src/main/java/org/orcid/pojo/grouping/WorkGroup.java +++ b/orcid-core/src/main/java/org/orcid/pojo/grouping/WorkGroup.java @@ -110,14 +110,14 @@ public static WorkGroup valueOf(org.orcid.jaxb.model.v3.release.record.summary.W public static WorkGroup valueOf(WorkGroupExtended workGroup, int id, String orcid) { WorkGroup group = new WorkGroup(); group.setGroupId(id); - group.setWorks(new ArrayList<>()); + List works = new ArrayList<>(); WorkType workType = null; Long maxDisplayIndex = null; for (WorkSummaryExtended workSummary : workGroup.getWorkSummary()) { WorkForm workForm = getWorkForm(workSummary); - group.getWorks().add(workForm); + works.add(workForm); Long displayIndex = Long.parseLong(workSummary.getDisplayIndex()); if (maxDisplayIndex == null || displayIndex > maxDisplayIndex) { @@ -134,6 +134,8 @@ public static WorkGroup valueOf(WorkGroupExtended workGroup, int id, String orci workType = workSummary.getType(); } + group.setWorks(works); + if (workGroup.getIdentifiers() != null) { List workExternalIdentifiersList = new ArrayList(); for (ExternalID extId : workGroup.getIdentifiers().getExternalIdentifier()) { diff --git a/orcid-core/src/main/resources/orcid-core-context.xml b/orcid-core/src/main/resources/orcid-core-context.xml index fff62b31795..3a8ff324a41 100644 --- a/orcid-core/src/main/resources/orcid-core-context.xml +++ b/orcid-core/src/main/resources/orcid-core-context.xml @@ -298,7 +298,7 @@ - + @@ -379,17 +379,10 @@ - - - - - - - - + @@ -410,27 +403,17 @@ - + - - - - - - - - - - - + - + - + @@ -480,7 +463,7 @@ - + @@ -1232,9 +1215,7 @@ - - - + diff --git a/orcid-core/src/test/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImplTest.java b/orcid-core/src/test/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImplTest.java new file mode 100644 index 00000000000..610b4a4b324 --- /dev/null +++ b/orcid-core/src/test/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImplTest.java @@ -0,0 +1,86 @@ +package org.orcid.core.manager.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Date; +import java.util.List; +import java.util.Map; + +import org.ehcache.Cache; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.orcid.core.manager.ClientDetailsManager; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; +import org.springframework.test.util.ReflectionTestUtils; + +public class ClientDetailsEntityCacheManagerImplTest { + + private ClientDetailsEntityCacheManagerImpl cacheManager; + + @Mock + private ClientDetailsManager clientDetailsManager; + + @Mock + private Cache clientDetailsCache; + + @Mock + private Cache clientDetailsIdPCache; + + @Before + public void before() { + MockitoAnnotations.initMocks(this); + cacheManager = new ClientDetailsEntityCacheManagerImpl(); + ReflectionTestUtils.setField(cacheManager, "clientDetailsManager", clientDetailsManager); + ReflectionTestUtils.setField(cacheManager, "clientDetailsCache", clientDetailsCache); + ReflectionTestUtils.setField(cacheManager, "clientDetailsIdPCache", clientDetailsIdPCache); + } + + @Test + public void retrieveAllReturnsFreshCachedClientsWithoutSingleClientValidation() { + Date lastModified = new Date(); + ClientDetailsEntity cached = client("A", lastModified); + when(clientDetailsManager.getLastModifiedByClientIds(Arrays.asList("A"))).thenReturn(java.util.Collections.singletonMap("A", lastModified)); + when(clientDetailsCache.get(any())).thenReturn(cached); + + Map result = cacheManager.retrieveAll(Arrays.asList("A")); + + assertEquals(1, result.size()); + assertSame(cached, result.get("A")); + verify(clientDetailsManager).getLastModifiedByClientIds(Arrays.asList("A")); + verify(clientDetailsManager, never()).getLastModified("A"); + verify(clientDetailsManager, never()).findByClientIds(any(List.class)); + } + + @Test + public void retrieveAllReloadsStaleClientsInBulk() { + Date oldLastModified = new Date(1L); + Date newLastModified = new Date(2L); + ClientDetailsEntity cached = client("A", oldLastModified); + ClientDetailsEntity fresh = client("A", newLastModified); + when(clientDetailsManager.getLastModifiedByClientIds(Arrays.asList("A"))).thenReturn(java.util.Collections.singletonMap("A", newLastModified)); + when(clientDetailsCache.get(any())).thenReturn(cached); + when(clientDetailsManager.findByClientIds(Arrays.asList("A"))).thenReturn(Arrays.asList(fresh)); + + Map result = cacheManager.retrieveAll(Arrays.asList("A")); + + assertEquals(1, result.size()); + assertSame(fresh, result.get("A")); + verify(clientDetailsManager).findByClientIds(Arrays.asList("A")); + verify(clientDetailsCache).put(any(), any(ClientDetailsEntity.class)); + } + + private ClientDetailsEntity client(String clientId, Date lastModified) { + ClientDetailsEntity entity = new ClientDetailsEntity(); + entity.setId(clientId); + ReflectionTestUtils.setField(entity, "lastModified", lastModified); + return entity; + } +} diff --git a/orcid-core/src/test/java/org/orcid/core/utils/SourceEntityUtilsTest.java b/orcid-core/src/test/java/org/orcid/core/utils/SourceEntityUtilsTest.java new file mode 100644 index 00000000000..886dcb74b71 --- /dev/null +++ b/orcid-core/src/test/java/org/orcid/core/utils/SourceEntityUtilsTest.java @@ -0,0 +1,265 @@ +package org.orcid.core.utils; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.orcid.core.manager.ClientDetailsEntityCacheManager; +import org.orcid.core.manager.SourceNameCacheManager; +import org.orcid.core.manager.impl.OrcidUrlManager; +import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; +import org.orcid.jaxb.model.v3.release.common.Source; +import org.orcid.jaxb.model.v3.release.common.SourceClientId; +import org.orcid.jaxb.model.v3.release.common.SourceName; +import org.orcid.jaxb.model.v3.release.common.SourceOrcid; +import org.orcid.persistence.jpa.entities.AddressEntity; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; +import org.orcid.persistence.jpa.entities.ProfileEntity; +import org.orcid.persistence.jpa.entities.SourceAwareEntity; +import org.orcid.persistence.jpa.entities.SourceEntity; + +public class SourceEntityUtilsTest { + + @Mock + private RecordNameManagerReadOnly recordNameManagerReadOnlyV3; + + @Mock + private SourceNameCacheManager sourceNameCacheManager; + + @Mock + private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager; + + @Mock + private OrcidUrlManager orcidUrlManager; + + @InjectMocks + private SourceEntityUtils sourceEntityUtils; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + when(orcidUrlManager.getBaseHost()).thenReturn("orcid.org"); + when(orcidUrlManager.getBaseUrl()).thenReturn("https://orcid.org"); + } + + @Test + public void testGetSourceId() { + SourceEntity sourceEntity = new SourceEntity(); + + // Test cached + sourceEntity.setCachedSourceId("cached-id"); + assertEquals("cached-id", SourceEntityUtils.getSourceId(sourceEntity)); + + // Test client + sourceEntity.setCachedSourceId(null); + ClientDetailsEntity client = new ClientDetailsEntity("client-id"); + sourceEntity.setSourceClient(client); + assertEquals("client-id", SourceEntityUtils.getSourceId(sourceEntity)); + + // Test profile + sourceEntity.setSourceClient(null); + ProfileEntity profile = new ProfileEntity("orcid-id"); + sourceEntity.setSourceProfile(profile); + assertEquals("orcid-id", SourceEntityUtils.getSourceId(sourceEntity)); + + // Test null + sourceEntity.setSourceProfile(null); + assertNull(SourceEntityUtils.getSourceId(sourceEntity)); + } + + @Test + public void testGetSourceNameFromSource() { + Source source = new Source(); + assertNull(SourceEntityUtils.getSourceName(source)); + + source.setSourceName(new SourceName("Source Name")); + assertEquals("Source Name", SourceEntityUtils.getSourceName(source)); + + source.setSourceName(new SourceName("")); + assertNull(SourceEntityUtils.getSourceName(source)); + } + + @Test + public void testGetSourceKey() { + // Mocking SourceAwareEntity since it's abstract + SourceAwareEntity entity = mock(SourceAwareEntity.class); + when(entity.getSourceId()).thenReturn("orcid"); + when(entity.getClientSourceId()).thenReturn("client"); + when(entity.getAssertionOriginClientSourceId()).thenReturn("assertion"); + + assertEquals("orcid|client|assertion", SourceEntityUtils.getSourceKey(entity)); + + when(entity.getSourceId()).thenReturn(null); + when(entity.getClientSourceId()).thenReturn(""); + when(entity.getAssertionOriginClientSourceId()).thenReturn(null); + assertEquals("-|-|-", SourceEntityUtils.getSourceKey(entity)); + } + + @Test + public void testGetSourceNameFromEntity() { + SourceEntity sourceEntity = new SourceEntity(); + + // Test cached + sourceEntity.setCachedSourceName("cached-name"); + assertEquals("cached-name", sourceEntityUtils.getSourceName(sourceEntity)); + + // Test client + sourceEntity.setCachedSourceName(null); + ClientDetailsEntity client = new ClientDetailsEntity("client-id", "Client Name"); + sourceEntity.setSourceClient(client); + assertEquals("Client Name", sourceEntityUtils.getSourceName(sourceEntity)); + + // Test profile + sourceEntity.setSourceClient(null); + ProfileEntity profile = new ProfileEntity("orcid-id"); + sourceEntity.setSourceProfile(profile); + when(recordNameManagerReadOnlyV3.fetchDisplayablePublicName("orcid-id")).thenReturn("Profile Name"); + assertEquals("Profile Name", sourceEntityUtils.getSourceName(sourceEntity)); + + // Test null + sourceEntity.setSourceProfile(null); + assertNull(sourceEntityUtils.getSourceName(sourceEntity)); + } + + @Test + public void testPopulateSourceAwareEntityFromSource() { + Source source = new Source(); + source.setSourceOrcid(new SourceOrcid("orcid-id")); + source.setSourceClientId(new SourceClientId("client-id")); + source.setAssertionOriginClientId(new SourceClientId("assertion-id")); + + SourceAwareEntity entity = new AddressEntity(); // Using a concrete subclass + sourceEntityUtils.populateSourceAwareEntityFromSource(source, entity); + + assertEquals("orcid-id", entity.getSourceId()); + assertEquals("client-id", entity.getClientSourceId()); + assertEquals("assertion-id", entity.getAssertionOriginClientSourceId()); + } + + @Test + public void testExtractSourceFromEntity() { + AddressEntity entity = new AddressEntity(); + entity.setSourceId("orcid-id"); + entity.setClientSourceId("client-id"); + entity.setAssertionOriginClientSourceId("assertion-id"); + entity.setOrcid("user-orcid"); + + ClientDetailsEntity clientDetails = new ClientDetailsEntity(); + clientDetails.setUserOBOEnabled(true); + when(clientDetailsEntityCacheManager.retrieve("client-id")).thenReturn(clientDetails); + + Source source = sourceEntityUtils.extractSourceFromEntity(entity); + + assertEquals("orcid-id", source.getSourceOrcid().getPath()); + assertEquals("client-id", source.getSourceClientId().getPath()); + assertEquals("assertion-id", source.getAssertionOriginClientId().getPath()); + assertEquals("user-orcid", source.getAssertionOriginOrcid().getPath()); + + // Test without OBO + clientDetails.setUserOBOEnabled(false); + source = sourceEntityUtils.extractSourceFromEntity(entity); + assertNull(source.getAssertionOriginOrcid()); + } + + @Test + public void testExtractSourceFromProfileComplete() { + ProfileEntity profile = new ProfileEntity("user-orcid"); + SourceEntity sourceEntity = new SourceEntity(); + sourceEntity.setSourceProfile(new ProfileEntity("source-orcid")); + sourceEntity.setSourceClient(new ClientDetailsEntity("source-client")); + profile.setSource(sourceEntity); + + when(sourceNameCacheManager.retrieve("source-orcid")).thenReturn("Source Orcid Name"); + when(sourceNameCacheManager.retrieve("source-client")).thenReturn("Source Client Name"); + + Source source = sourceEntityUtils.extractSourceFromProfileComplete(profile); + + assertEquals("source-orcid", source.getSourceOrcid().getPath()); + assertEquals("orcid.org", source.getSourceOrcid().getHost()); + assertEquals("https://orcid.org/source-orcid", source.getSourceOrcid().getUri()); + + assertEquals("source-client", source.getSourceClientId().getPath()); + assertEquals("orcid.org", source.getSourceClientId().getHost()); + assertEquals("https://orcid.org/client/source-client", source.getSourceClientId().getUri()); + + assertEquals("Source Client Name", source.getSourceName().getContent()); + } + + @Test + public void testExtractSourceFromEntityComplete() { + AddressEntity entity = new AddressEntity(); + entity.setSourceId("orcid-id"); + entity.setClientSourceId("client-id"); + + ClientDetailsEntity clientDetails = new ClientDetailsEntity(); + clientDetails.setUserOBOEnabled(false); + when(clientDetailsEntityCacheManager.retrieve("client-id")).thenReturn(clientDetails); + + when(sourceNameCacheManager.retrieve("orcid-id")).thenReturn("Orcid Name"); + when(sourceNameCacheManager.retrieve("client-id")).thenReturn("Client Name"); + + Source source = sourceEntityUtils.extractSourceFromEntityComplete(entity); + + assertEquals("orcid-id", source.getSourceOrcid().getPath()); + assertEquals("client-id", source.getSourceClientId().getPath()); + assertEquals("Client Name", source.getSourceName().getContent()); + } + + @Test + public void testIsTheSameSource_SourceOrcid() { + Source s1 = new Source(); + s1.setSourceOrcid(new SourceOrcid("orcid-1")); + + Source s2 = new Source(); + s2.setSourceOrcid(new SourceOrcid("orcid-1")); + + assertTrue(sourceEntityUtils.isTheSameSource(s1, s2)); + + s2.setSourceOrcid(new SourceOrcid("orcid-2")); + assertFalse(sourceEntityUtils.isTheSameSource(s1, s2)); + + AddressEntity entity = new AddressEntity(); + entity.setSourceId("orcid-1"); + assertTrue(sourceEntityUtils.isTheSameSource(s1, entity)); + + entity.setSourceId("orcid-2"); + assertFalse(sourceEntityUtils.isTheSameSource(s1, entity)); + } + + @Test + public void testIsTheSameSource_SourceClientId() { + ClientDetailsEntity clientDetails1 = new ClientDetailsEntity(); + clientDetails1.setId("client-1"); + clientDetails1.setUserOBOEnabled(true); + when(clientDetailsEntityCacheManager.retrieve("client-1")).thenReturn(clientDetails1); + + Source s1 = new Source(); + s1.setSourceClientId(new SourceClientId("client-1")); + + Source s2 = new Source(); + s2.setSourceClientId(new SourceClientId("client-1")); + + assertTrue(sourceEntityUtils.isTheSameSource(s1, s2)); + + Source s3 = new Source(); + s3.setSourceClientId(new SourceClientId("client-1")); + s3.setAssertionOriginOrcid(new SourceOrcid("orcid-1")); + + AddressEntity entity = new AddressEntity(); + entity.setOrcid("orcid-1"); + entity.setClientSourceId("client-1"); + assertTrue(sourceEntityUtils.isTheSameSource(s3, entity)); + + entity.setClientSourceId("client-2"); + assertFalse(sourceEntityUtils.isTheSameSource(s3, entity)); + } +} diff --git a/orcid-core/src/test/java/org/orcid/core/utils/v3/ContributorUtilsTest.java b/orcid-core/src/test/java/org/orcid/core/utils/v3/ContributorUtilsTest.java index ed25b0ee1a5..00dcaeb5445 100644 --- a/orcid-core/src/test/java/org/orcid/core/utils/v3/ContributorUtilsTest.java +++ b/orcid-core/src/test/java/org/orcid/core/utils/v3/ContributorUtilsTest.java @@ -24,6 +24,7 @@ import org.orcid.core.manager.ProfileEntityCacheManager; import org.orcid.core.manager.v3.ActivityManager; import org.orcid.core.manager.v3.ProfileEntityManager; +import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; import org.orcid.jaxb.model.v3.release.common.Contributor; import org.orcid.jaxb.model.v3.release.common.ContributorEmail; import org.orcid.jaxb.model.v3.release.common.ContributorOrcid; @@ -51,13 +52,13 @@ public class ContributorUtilsTest { @Mock private ProfileEntityManager profileEntityManager; - - @Mock - private RecordNameDao recordNameDao; - + @Mock private ProfileLastModifiedAspect profileLastModifiedAspect; - + + @Mock + private RecordNameManagerReadOnly recordNameManagerReadOnlyV3; + @Mock private Cache contributorsNameCache; @@ -67,9 +68,6 @@ public class ContributorUtilsTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); - contributorUtils.setCacheManager(cacheManager); - contributorUtils.setProfileEntityManager(profileEntityManager); - contributorUtils.setProfileLastModifiedAspect(profileLastModifiedAspect); when(profileLastModifiedAspect.retrieveLastModifiedDate(anyString())).thenReturn(new Date()); when(contributorsNameCache.containsKey(anyString())).thenReturn(false); } @@ -93,6 +91,7 @@ public void testFilterContributorPrivateDataForFundingWithPublicName() { when(profileEntityManager.orcidExists(anyString())).thenReturn(true); when(profileEntityCacheManager.retrieve(anyString())).thenReturn(new ProfileEntity()); when(cacheManager.getPublicCreditName(any(String.class))).thenReturn("a public name"); + when(recordNameManagerReadOnlyV3.fetchDisplayablePublicName(anyString())).thenReturn("a public name"); Funding funding = getFundingWithOrcidContributor(); contributorUtils.filterContributorPrivateData(funding); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ClientDetailsDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ClientDetailsDao.java index 60c155b5805..a03f4b5f849 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ClientDetailsDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ClientDetailsDao.java @@ -2,6 +2,7 @@ import java.util.Date; import java.util.List; +import java.util.Map; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.ClientSecretEntity; @@ -16,6 +17,10 @@ public interface ClientDetailsDao extends GenericDao getLastModifiedByClientIds(List clientIds); + + List findByClientIds(List clientIds); Date getLastModifiedByIdP(String idp); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ClientDetailsDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ClientDetailsDaoImpl.java index a1db541f4d8..35d625cab90 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ClientDetailsDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ClientDetailsDaoImpl.java @@ -1,7 +1,9 @@ package org.orcid.persistence.dao.impl; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.persistence.NoResultException; import javax.persistence.PersistenceContext; @@ -51,6 +53,30 @@ public Date getLastModified(String clientId) { return query.getSingleResult(); } + @Override + public Map getLastModifiedByClientIds(List clientIds) { + if (clientIds == null || clientIds.isEmpty()) { + return new HashMap<>(); + } + TypedQuery query = entityManager.createQuery("select id, lastModified from ClientDetailsEntity where id in :clientIds", Object[].class); + query.setParameter("clientIds", clientIds); + Map lastModifiedByClientId = new HashMap<>(); + for (Object[] result : query.getResultList()) { + lastModifiedByClientId.put((String) result[0], (Date) result[1]); + } + return lastModifiedByClientId; + } + + @Override + public List findByClientIds(List clientIds) { + if (clientIds == null || clientIds.isEmpty()) { + return java.util.Collections.emptyList(); + } + TypedQuery query = entityManager.createQuery("from ClientDetailsEntity where id in :clientIds", ClientDetailsEntity.class); + query.setParameter("clientIds", clientIds); + return query.getResultList(); + } + @Override public Date getLastModifiedByIdP(String idp) { TypedQuery query = entityManager.createQuery("select lastModified from ClientDetailsEntity where authenticationProviderId = :idp", Date.class); diff --git a/orcid-web/src/main/resources/orcid-core-context-spam.xml b/orcid-web/src/main/resources/orcid-core-context-spam.xml index 124a0da9ad7..1312dd78f0d 100644 --- a/orcid-web/src/main/resources/orcid-core-context-spam.xml +++ b/orcid-web/src/main/resources/orcid-core-context-spam.xml @@ -366,15 +366,8 @@ - - - - - - - - - + + @@ -401,31 +394,17 @@ - + - - - - - - - - - - - - - - - - - + + + - + @@ -475,7 +454,7 @@ - + diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageMembersControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageMembersControllerTest.java index 4b2275aa7ca..4b8a127ecde 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageMembersControllerTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageMembersControllerTest.java @@ -1,544 +1,474 @@ package org.orcid.frontend.web.controllers; -/** - * @author Angel Montenegro (amontenegro) Date: 29/08/2013 - */ -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -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 static org.mockito.Mockito.when; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import javax.annotation.Resource; -import javax.transaction.Transactional; - -import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.orcid.core.common.manager.EmailFrequencyManager; +import org.mockito.junit.MockitoJUnitRunner; import org.orcid.core.exception.ClientAlreadyActiveException; import org.orcid.core.exception.ClientAlreadyDeactivatedException; +import org.orcid.core.locale.LocaleManager; import org.orcid.core.manager.v3.ClientDetailsManager; +import org.orcid.core.manager.v3.ClientManager; +import org.orcid.core.manager.v3.EmailManager; import org.orcid.core.manager.v3.MembersManager; -import org.orcid.core.manager.v3.ProfileHistoryEventManager; -import org.orcid.core.manager.v3.SourceManager; +import org.orcid.core.manager.v3.read_only.ClientDetailsManagerReadOnly; +import org.orcid.core.manager.v3.read_only.ClientManagerReadOnly; import org.orcid.core.security.OrcidRoles; import org.orcid.jaxb.model.clientgroup.MemberType; -import org.orcid.persistence.dao.ClientDetailsDao; -import org.orcid.persistence.dao.ProfileDao; -import org.orcid.persistence.jpa.entities.ProfileEntity; -import org.orcid.persistence.jpa.entities.SourceEntity; +import org.orcid.jaxb.model.clientgroup.RedirectUriType; +import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.pojo.ClientActivationRequest; import org.orcid.pojo.ajaxForm.Client; import org.orcid.pojo.ajaxForm.Member; -import org.orcid.pojo.ajaxForm.PojoUtil; import org.orcid.pojo.ajaxForm.RedirectUri; import org.orcid.pojo.ajaxForm.Text; -import org.orcid.test.DBUnitTest; -import org.orcid.test.OrcidJUnit4ClassRunner; -import org.orcid.test.TargetProxyHelper; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.web.WebAppConfiguration; -import org.springframework.test.util.ReflectionTestUtils; - -@RunWith(OrcidJUnit4ClassRunner.class) -@WebAppConfiguration -@ContextConfiguration(locations = { "classpath:test-frontend-web-servlet.xml" }) -public class ManageMembersControllerTest extends DBUnitTest { - - @Resource(name = "membersManagerV3") - MembersManager membersManager; - - @Resource - ManageMembersController manageMembers; - - @Resource - private ProfileDao profileDao; - - @Resource - ClientsController groupAdministratorController; - - @Resource - ClientDetailsDao clientDetailsDao; - - @Resource(name = "sourceManagerV3") - SourceManager sourceManager; - +import org.springframework.web.servlet.ModelAndView; + +import java.util.*; + +import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +@RunWith(MockitoJUnitRunner.class) +public class ManageMembersControllerTest { + + @Mock(name = "membersManagerV3") + private MembersManager membersManager; + + @Mock(name = "clientManagerV3") + private ClientManager clientManager; + + @Mock(name = "clientManagerReadOnlyV3") + private ClientManagerReadOnly clientManagerReadOnly; + + @Mock(name = "clientDetailsManagerReadOnlyV3") + private ClientDetailsManagerReadOnly clientDetailsManagerReadOnly; + + @Mock(name = "clientDetailsManagerV3") + private ClientDetailsManager clientDetailsManager; + + @Mock(name = "emailManagerV3") + private EmailManager emailManager; + @Mock - SourceManager mockSourceManager; - - @Mock - ProfileHistoryEventManager mockProfileHistoryEventManager; - - @Resource - ProfileHistoryEventManager profileHistoryEventManager; - - @Resource - EmailFrequencyManager emailFrequencyManager; - + private ClientsController groupAdministratorController; + @Mock - private ClientDetailsManager mockClientDetailsManager; - - @Resource(name = "clientDetailsManagerV3") - private ClientDetailsManager clientDetailsManager; - + private LocaleManager localeManager; + + @InjectMocks + private ManageMembersController manageMembers; + @Before - public void beforeInstance() { + public void setUp() { SecurityContextHolder.getContext().setAuthentication(getAuthentication()); - MockitoAnnotations.initMocks(this); - TargetProxyHelper.injectIntoProxy(membersManager, "sourceManager", mockSourceManager); - TargetProxyHelper.injectIntoProxy(emailFrequencyManager, "profileHistoryEventManager", mockProfileHistoryEventManager); - - SourceEntity sourceEntity = new SourceEntity(); - sourceEntity.setSourceProfile(new ProfileEntity("5555-5555-5555-0000")); - when(mockSourceManager.retrieveActiveSourceEntity()).thenReturn(sourceEntity); - } - - @BeforeClass - public static void beforeClass() throws Exception { - initDBUnitData(Arrays.asList("/data/EmptyEntityData.xml", "/data/PremiumInstitutionMemberData.xml")); - } - - @After - public void after() { - TargetProxyHelper.injectIntoProxy(membersManager, "sourceManager", sourceManager); - TargetProxyHelper.injectIntoProxy(emailFrequencyManager, "profileHistoryEventManager", profileHistoryEventManager); - } - - @AfterClass - public static void afterClass() throws Exception { - removeDBUnitData(Arrays.asList("/data/PremiumInstitutionMemberData.xml")); - } - - protected Authentication getAuthentication() { + // Mock localeManager to return the message code as the translated message + when(localeManager.resolveMessage(anyString(), any())).thenAnswer(invocation -> invocation.getArgument(0)); + } + + private Authentication getAuthentication() { String orcid = "4444-4444-4444-4440"; - UserDetails details = new User(orcid, - "password", Arrays.asList(new SimpleGrantedAuthority(OrcidRoles.ROLE_ADMIN.name()))); + UserDetails details = new User(orcid, "password", List.of(new SimpleGrantedAuthority(OrcidRoles.ROLE_ADMIN.name()))); UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(orcid, "password", details.getAuthorities()); auth.setDetails(details); return auth; } - - @Test - @Transactional - public void createMemberProfileWithInvalidEmailsTest() throws Exception { - String existingEmail = "premium_institution@group.com"; + + @Test + public void testGetManageMembersPage() { + ModelAndView mav = manageMembers.getManageMembersPage(); + assertEquals("/admin/manage_members", mav.getViewName()); + } + + @Test + public void testGetEmptyGroup() { + Member member = manageMembers.getEmptyGroup(); + assertNotNull(member); + assertEquals("", member.getEmail().getValue()); + assertEquals("", member.getGroupName().getValue()); + assertEquals("", member.getGroupOrcid().getValue()); + assertEquals("", member.getSalesforceId().getValue()); + assertEquals(MemberType.BASIC.value(), member.getType().getValue()); + } + + @Test + public void testFind_ClientBranch() { + String id = "APP-123"; + when(clientDetailsManagerReadOnly.exists(id)).thenReturn(true); - Member group = new Member(); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("basic")); - group.setSalesforceId(Text.valueOf("")); - - // Validate already existing email address - group.setEmail(Text.valueOf(existingEmail)); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.email.already_used", new ArrayList()), group.getErrors().get(0)); - - // Validate empty email address - group.setEmail(Text.valueOf("")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("NotBlank.group.email", new ArrayList()), group.getErrors().get(0)); - - // Validate invalid email address - group.setEmail(Text.valueOf("invalidemail")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.email.invalid_email", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void createMemberProfileWithInvalidGroupNameTest() throws Exception { - Member group = new Member(); - group.setEmail(Text.valueOf("group@email.com")); - group.setType(Text.valueOf("basic")); - group.setSalesforceId(Text.valueOf("")); - - // Validate empty group name - group.setGroupName(Text.valueOf("")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("NotBlank.group.name", new ArrayList()), group.getErrors().get(0)); - - // validate too long group name - 151 chars - group.setGroupName(Text - .valueOf("1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.name.too_long", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void createMemberProfileWithInvalidTypeTest() throws Exception { - Member group = new Member(); - group.setEmail(Text.valueOf("group@email.com")); - group.setGroupName(Text.valueOf("Group Name")); - group.setSalesforceId(Text.valueOf("")); - - // Validate empty type - group.setType(Text.valueOf("")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("NotBlank.group.type", new ArrayList()), group.getErrors().get(0)); - - // Validate invalid type - group.setType(Text.valueOf("invalid")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.type.invalid", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void createMemberProfileWithInvalidSalesforceIdTest() throws Exception { - Member group = new Member(); - group.setEmail(Text.valueOf("group@email.com")); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("basic")); - - // Validate empty type - group.setSalesforceId(Text.valueOf("1")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.salesforce_id.invalid_length", new ArrayList()), group.getErrors().get(0)); - - // Validate invalid type - group.setSalesforceId(Text.valueOf("1234567890abcd!")); - group = manageMembers.createMember(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.salesforce_id.invalid", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void createMemberProfileTest() throws Exception { - Member group = new Member(); - String email = "group" + System.currentTimeMillis() + "@email.com"; - group.setEmail(Text.valueOf(email)); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("")); - group = manageMembers.createMember(group); - assertEquals(0, group.getErrors().size()); - assertFalse(PojoUtil.isEmpty(group.getGroupOrcid())); - } - - @Test - public void findMemberByOrcidTest() throws Exception { - Member group = new Member(); - String email = "group" + System.currentTimeMillis() + "@email.com"; - group.setEmail(Text.valueOf(email)); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcde")); - group = manageMembers.createMember(group); - assertEquals(0, group.getErrors().size()); - assertFalse(PojoUtil.isEmpty(group.getGroupOrcid())); - - // Test find by orcid - String orcid = group.getGroupOrcid().getValue(); - Member newGroup = manageMembers.findMember(orcid); - assertNotNull(newGroup); - - assertFalse(PojoUtil.isEmpty(newGroup.getGroupOrcid())); - assertFalse(PojoUtil.isEmpty(newGroup.getEmail())); - assertFalse(PojoUtil.isEmpty(newGroup.getSalesforceId())); - assertFalse(PojoUtil.isEmpty(newGroup.getGroupName())); - - assertEquals(email, newGroup.getEmail().getValue()); - assertEquals("Group Name", newGroup.getGroupName().getValue()); - assertEquals("1234567890abcde", newGroup.getSalesforceId().getValue()); - assertEquals(orcid, newGroup.getGroupOrcid().getValue()); - - // Test find by email - Member newGroup2 = manageMembers.findMember(email); - assertNotNull(newGroup2); - - assertFalse(PojoUtil.isEmpty(newGroup2.getGroupOrcid())); - assertFalse(PojoUtil.isEmpty(newGroup2.getEmail())); - assertFalse(PojoUtil.isEmpty(newGroup2.getSalesforceId())); - assertFalse(PojoUtil.isEmpty(newGroup2.getGroupName())); - - assertEquals(email, newGroup2.getEmail().getValue()); - assertEquals("Group Name", newGroup2.getGroupName().getValue()); - assertEquals("1234567890abcde", newGroup2.getSalesforceId().getValue()); - assertEquals(orcid, newGroup2.getGroupOrcid().getValue()); - - // Test: Find member by ORCID with clients and check deactivated status - Member newGroup3 = manageMembers.findMember("5555-5555-5555-0000"); - - List clients = newGroup3.getClients(); - - Client activeClient1 = findClientById(clients, "APP-0000000000000001"); - Client activeClient2 = findClientById(clients, "APP-0000000000000002"); - Client deactivatedClient = findClientById(clients, "APP-0000000000000003"); - - assertNotNull(newGroup3); - - assertEquals(3, clients.size()); - assertEquals(false, activeClient1.isDeactivated()); - assertEquals(false, activeClient2.isDeactivated()); - assertEquals(true, deactivatedClient.isDeactivated()); - } - - - @Test - public void editMemberTest() throws Exception { - Member group = new Member(); - group.setEmail(Text.valueOf("group@email.com")); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcde")); - group = manageMembers.createMember(group); - assertEquals(0, group.getErrors().size()); - assertFalse(PojoUtil.isEmpty(group.getGroupOrcid())); + // Mock findClient behavior + org.orcid.jaxb.model.v3.release.client.Client modelClient = mock(org.orcid.jaxb.model.v3.release.client.Client.class); + when(modelClient.getId()).thenReturn(id); + when(modelClient.getClientType()).thenReturn(org.orcid.jaxb.model.clientgroup.ClientType.CREATOR); + when(clientManagerReadOnly.get(id)).thenReturn(modelClient); - group.setEmail(Text.valueOf("new_email@user.com")); - group.setSalesforceId(Text.valueOf("")); - group.setGroupName(Text.valueOf("Updated Group Name")); + ClientDetailsEntity clientDetails = new ClientDetailsEntity(); + when(clientDetailsManagerReadOnly.findByClientId(id)).thenReturn(clientDetails); + + Object resultObj = manageMembers.find(id); + assertTrue((Boolean) reflectGet(resultObj, "isClient")); + Object clientObj = reflectGet(resultObj, "clientObject"); + assertNotNull(clientObj); + assertEquals(id, ((Client) clientObj).getClientId().getValue()); + } + + private Object reflectGet(Object obj, String fieldName) { + try { + java.lang.reflect.Field field = obj.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + return field.get(obj); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @Test + public void testFind_MemberBranch() { + String id = "0000-0000-0000-0001"; + when(clientDetailsManagerReadOnly.exists(id)).thenReturn(false); - manageMembers.updateMember(group); - Member updatedGroup = manageMembers.findMember(group.getGroupOrcid().getValue()); - assertNotNull(updatedGroup); - assertEquals(group.getGroupOrcid().getValue(), updatedGroup.getGroupOrcid().getValue()); - assertEquals("Updated Group Name", updatedGroup.getGroupName().getValue()); - } - - @Test - public void editMemberWithInvalidEmailTest() throws Exception { - //Create one member - Member group = new Member(); - String email = "group1" + System.currentTimeMillis() + "@email.com"; - group.setEmail(Text.valueOf(email)); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcde")); - group = manageMembers.createMember(group); - assertNotNull(group); - assertEquals(0, group.getErrors().size()); - //Try to create another member with the same email - group = new Member(); - group.setEmail(Text.valueOf(email)); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcde")); - group = manageMembers.createMember(group); - assertNotNull(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.email.already_used", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void editMemberWithInvalidSalesforceIdTest() throws Exception { - //Create one member - Member group = new Member(); - String email = "group" + System.currentTimeMillis() + "@email.com"; - group.setEmail(Text.valueOf(email)); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcde")); - group = manageMembers.createMember(group); - assertNotNull(group); - assertEquals(0, group.getErrors().size()); - //Try to create another member with the same email - group = new Member(); - group.setEmail(Text.valueOf("group2@email.com")); - group.setGroupName(Text.valueOf("Group Name")); - group.setType(Text.valueOf("premium-institution")); - group.setSalesforceId(Text.valueOf("1234567890abcd!")); - group = manageMembers.createMember(group); - assertNotNull(group); - assertEquals(1, group.getErrors().size()); - assertEquals(manageMembers.getMessage("group.salesforce_id.invalid", new ArrayList()), group.getErrors().get(0)); - } - - @Test - public void findClientTest() throws Exception { - //Client with all redirect uris default - Client client_0002 = manageMembers.findClient("APP-0000000000000002"); - assertNotNull(client_0002); - assertNotNull(client_0002.getDisplayName()); - assertEquals("Client # 2", client_0002.getDisplayName().getValue()); - assertNotNull(client_0002.getRedirectUris()); - assertEquals(1, client_0002.getRedirectUris().size()); - assertEquals("http://www.google.com/APP-0000000000000002/redirect/oauth", client_0002.getRedirectUris().get(0).getValue().getValue()); - assertEquals(false, client_0002.isDeactivated()); - - //Client with redirect uri not default - Client client_0003 = manageMembers.findClient("APP-0000000000000003"); - assertNotNull(client_0003); - assertNotNull(client_0003.getDisplayName()); - assertEquals("Client # 3", client_0003.getDisplayName().getValue()); - assertNotNull(client_0003.getRedirectUris()); - assertEquals(2, client_0003.getRedirectUris().size()); - assertEquals(true, client_0003.isDeactivated()); + Member member = new Member(); + member.setGroupOrcid(Text.valueOf(id)); + when(membersManager.getMember(id)).thenReturn(member); + + Object resultObj = manageMembers.find(id); + assertFalse((Boolean) reflectGet(resultObj, "isClient")); + Object memberObj = reflectGet(resultObj, "memberObject"); + assertNotNull(memberObj); + assertEquals(id, ((Member) memberObj).getGroupOrcid().getValue()); + } + + @Test + public void testFindMember_Empty() { + Member result = manageMembers.findMember(""); + assertEquals(1, result.getErrors().size()); + assertEquals("manage_member.not_blank", result.getErrors().get(0)); + } + + @Test + public void testFindMember_Success() { + String id = "some-id"; + Member member = new Member(); + when(membersManager.getMember(id)).thenReturn(member); + Member result = manageMembers.findMember(id); + assertSame(member, result); + } + + @Test + public void testCreateMember_Validation_EmailEmpty() { + Member member = createValidMember(); + member.setEmail(Text.valueOf("")); - RedirectUri rUri1 = client_0003.getRedirectUris().get(0); - if("http://www.google.com/APP-0000000000000003/redirect/oauth".equals(rUri1.getValue().getValue())) { - assertNotNull(rUri1.getType()); - assertEquals("default", rUri1.getType().getValue()); - assertNotNull(rUri1.getScopes()); - assertEquals(0, rUri1.getScopes().size()); - } else if ("http://www.google.com/APP-0000000000000003/redirect/oauth/grant_read_wizard".equals(rUri1.getValue().getValue())) { - assertNotNull(rUri1.getType()); - assertEquals("grant-read-wizard", rUri1.getType().getValue()); - assertNotNull(rUri1.getScopes()); - assertEquals(1, rUri1.getScopes().size()); - assertEquals("/funding/read-limited", rUri1.getScopes().get(0)); - } else { - fail("Invalid redirect uri: " + rUri1.getValue().getValue()); - } + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("NotBlank.group.email")); + } - RedirectUri rUri2 = client_0003.getRedirectUris().get(1); - if("http://www.google.com/APP-0000000000000003/redirect/oauth".equals(rUri2.getValue().getValue())) { - assertNotNull(rUri2.getType()); - assertEquals("default", rUri2.getType().getValue()); - assertNotNull(rUri2.getScopes()); - assertEquals(0, rUri2.getScopes().size()); - } else if ("http://www.google.com/APP-0000000000000003/redirect/oauth/grant_read_wizard".equals(rUri2.getValue().getValue())) { - assertNotNull(rUri2.getType()); - assertEquals("grant-read-wizard", rUri2.getType().getValue()); - assertNotNull(rUri2.getScopes()); - assertEquals(1, rUri2.getScopes().size()); - assertEquals("/funding/read-limited", rUri2.getScopes().get(0)); - } else { - fail("Invalid redirect uri: " + rUri2.getValue().getValue()); - } + @Test + public void testCreateMember_Validation_EmailInvalid() { + Member member = createValidMember(); + member.setEmail(Text.valueOf("invalid-email")); + + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.email.invalid_email")); } - - @Test - public void editClientWithInvalidRedirectUriTest() throws Exception { - //Client with all redirect uris default - Client client_0002 = manageMembers.findClient("APP-0000000000000002"); - assertNotNull(client_0002); - RedirectUri rUri = new RedirectUri(); - rUri.setType(Text.valueOf("default")); - rUri.setValue(Text.valueOf("http://érm.com")); + + @Test + public void testCreateMember_Validation_EmailExists() { + Member member = createValidMember(); + when(emailManager.emailExists(member.getEmail().getValue())).thenReturn(true); - client_0002.getRedirectUris().add(rUri); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.email.already_used")); + } + + @Test + public void testCreateMember_Validation_GroupNameEmpty() { + Member member = createValidMember(); + member.setGroupName(Text.valueOf("")); - client_0002 = manageMembers.updateClient(client_0002); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("NotBlank.group.name")); + } + + @Test + public void testCreateMember_Validation_GroupNameTooLong() { + Member member = createValidMember(); + member.setGroupName(Text.valueOf("a".repeat(151))); - assertNotNull(client_0002); - assertEquals(1, client_0002.getErrors().size()); - assertEquals(manageMembers.getMessage("manage.developer_tools.invalid_redirect_uri"), client_0002.getErrors().get(0)); - } - - @Test - public void editMemberDoesntChangePersistentTokenEnabledValueTest() throws Exception { - Client clientWithPersistentTokensEnabled = manageMembers.findClient("APP-0000000000000001"); - assertNotNull(clientWithPersistentTokensEnabled); - assertNotNull(clientWithPersistentTokensEnabled.getDisplayName()); - assertEquals("Client # 1", clientWithPersistentTokensEnabled.getDisplayName().getValue()); - assertNotNull(clientWithPersistentTokensEnabled.getPersistentTokenEnabled()); - assertTrue(clientWithPersistentTokensEnabled.getPersistentTokenEnabled().getValue()); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.name.too_long")); + } + + @Test + public void testCreateMember_Validation_TypeEmpty() { + Member member = createValidMember(); + member.setType(Text.valueOf("")); - clientWithPersistentTokensEnabled.getDisplayName().setValue("Updated Name"); - manageMembers.updateClient(clientWithPersistentTokensEnabled); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("NotBlank.group.type")); + } + + @Test + public void testCreateMember_Validation_TypeInvalid() { + Member member = createValidMember(); + member.setType(Text.valueOf("INVALID_TYPE")); - Client updatedClient = manageMembers.findClient("APP-0000000000000001"); - assertNotNull(updatedClient); - assertNotNull(updatedClient.getDisplayName()); - assertEquals("Updated Name", updatedClient.getDisplayName().getValue()); - assertNotNull(updatedClient.getPersistentTokenEnabled()); - assertTrue(updatedClient.getPersistentTokenEnabled().getValue()); - } - - @Test - public void editGroupTypeTest() throws Exception { - Member group_0000 = manageMembers.findMember("5555-5555-5555-0000"); - assertNotNull(group_0000); - assertNotNull(group_0000.getType()); - assertEquals(MemberType.PREMIUM_INSTITUTION.value(), group_0000.getType().getValue()); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.type.invalid")); + } + + @Test + public void testCreateMember_Validation_SalesforceIdInvalidLength() { + Member member = createValidMember(); + member.setSalesforceId(Text.valueOf("123")); - // Update group type to basic - group_0000.setType(Text.valueOf(MemberType.BASIC.value())); - manageMembers.updateMember(group_0000); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.salesforce_id.invalid_length")); + } + + @Test + public void testCreateMember_Validation_SalesforceIdInvalidPattern() { + Member member = createValidMember(); + member.setSalesforceId(Text.valueOf("12345678901234!")); // 15 chars but with '!' - group_0000 = manageMembers.findMember("5555-5555-5555-0000"); - assertNotNull(group_0000); - assertNotNull(group_0000.getType()); - assertEquals(MemberType.BASIC.value(), group_0000.getType().getValue()); + Member result = manageMembers.createMember(member); + assertTrue(result.getErrors().contains("group.salesforce_id.invalid")); + } + + @Test + public void testCreateMember_Success() { + Member member = createValidMember(); + when(membersManager.createMember(any(Member.class))).thenReturn(member); + Member result = manageMembers.createMember(member); + assertEquals(0, result.getErrors().size()); + verify(membersManager).createMember(member); } - + @Test - public void testDeactivateClient() throws ClientAlreadyDeactivatedException { - SecurityContextHolder.getContext().setAuthentication(getAuthentication()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", mockClientDetailsManager); - Mockito.when(mockClientDetailsManager.exists(Mockito.eq("test"))).thenReturn(true); - Mockito.doNothing().when(mockClientDetailsManager).deactivateClientDetails(Mockito.eq("test"), Mockito.eq("4444-4444-4444-4440")); - ClientActivationRequest clientDeactivation = new ClientActivationRequest(); - clientDeactivation.setClientId("test"); - clientDeactivation = manageMembers.deactivateClient(clientDeactivation); - assertNull(clientDeactivation.getError()); - Mockito.verify(mockClientDetailsManager, Mockito.times(1)).deactivateClientDetails(Mockito.eq("test"), Mockito.eq("4444-4444-4444-4440")); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", clientDetailsManager); - } - - @Test - public void testActivateClient() throws ClientAlreadyActiveException { - SecurityContextHolder.getContext().setAuthentication(getAuthentication()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", mockClientDetailsManager); - Mockito.when(mockClientDetailsManager.exists(Mockito.eq("test"))).thenReturn(true); - Mockito.doNothing().when(mockClientDetailsManager).activateClientDetails(Mockito.eq("test")); - ClientActivationRequest clientActivation = new ClientActivationRequest(); - clientActivation.setClientId("test"); - clientActivation = manageMembers.activateClient(clientActivation); - assertNull(clientActivation.getError()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", clientDetailsManager); - } - - @Test - public void testDeactivateClientAlreadyDeactivated() throws ClientAlreadyDeactivatedException { - SecurityContextHolder.getContext().setAuthentication(getAuthentication()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", mockClientDetailsManager); - Mockito.when(mockClientDetailsManager.exists(Mockito.eq("test"))).thenReturn(true); - Mockito.doThrow(new ClientAlreadyDeactivatedException("already-deactivated")).when(mockClientDetailsManager).deactivateClientDetails(Mockito.eq("test"), Mockito.eq("4444-4444-4444-4440")); - ClientActivationRequest clientDeactivation = new ClientActivationRequest(); - clientDeactivation.setClientId("test"); - clientDeactivation = manageMembers.deactivateClient(clientDeactivation); - assertNotNull(clientDeactivation.getError()); - assertEquals("already-deactivated", clientDeactivation.getError()); - Mockito.verify(mockClientDetailsManager, Mockito.times(1)).deactivateClientDetails(Mockito.eq("test"), Mockito.eq("4444-4444-4444-4440")); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", clientDetailsManager); - } - - @Test - public void testActivateClientAlreadyActive() throws ClientAlreadyActiveException { - SecurityContextHolder.getContext().setAuthentication(getAuthentication()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", mockClientDetailsManager); - Mockito.when(mockClientDetailsManager.exists(Mockito.eq("test"))).thenReturn(true); - Mockito.doThrow(new ClientAlreadyActiveException("already-active")).when(mockClientDetailsManager).activateClientDetails(Mockito.eq("test")); - ClientActivationRequest clientActivation = new ClientActivationRequest(); - clientActivation.setClientId("test"); - clientActivation = manageMembers.activateClient(clientActivation); - assertNotNull(clientActivation.getError()); - assertEquals("already-active", clientActivation.getError()); - ReflectionTestUtils.setField(manageMembers, "clientDetailsManager", clientDetailsManager); - } - - private Client findClientById(List clients, String id) { - return clients.stream() - .filter(c -> id.equals(c.getClientId().getValue())) - .findFirst() - .orElse(null); + public void testUpdateMember_Success() { + Member member = createValidMember(); + member.setGroupOrcid(Text.valueOf("0000-0000-0000-0001")); + when(membersManager.updateMemeber(any(Member.class))).thenReturn(member); + + Member result = manageMembers.updateMember(member); + assertEquals(0, result.getErrors().size()); + verify(membersManager).updateMemeber(member); + } + + @Test + public void testUpdateMember_EmailOwnership_SameOwner() { + Member member = createValidMember(); + String orcid = "0000-0000-0000-0001"; + member.setGroupOrcid(Text.valueOf(orcid)); + String email = "test@orcid.org"; + member.setEmail(Text.valueOf(email)); + + when(emailManager.emailExists(email)).thenReturn(true); + Map owners = new HashMap<>(); + owners.put(email, orcid); + when(emailManager.findOricdIdsByCommaSeparatedEmails(email)).thenReturn(owners); + when(membersManager.updateMemeber(any(Member.class))).thenReturn(member); + + Member result = manageMembers.updateMember(member); + assertEquals(0, result.getErrors().size()); + } + + @Test + public void testUpdateMember_EmailOwnership_DifferentOwner() { + Member member = createValidMember(); + String orcid = "0000-0000-0000-0001"; + member.setGroupOrcid(Text.valueOf(orcid)); + String email = "test@orcid.org"; + member.setEmail(Text.valueOf(email)); + + when(emailManager.emailExists(email)).thenReturn(true); + Map owners = new HashMap<>(); + owners.put(email, "0000-0000-0000-0002"); // different owner + when(emailManager.findOricdIdsByCommaSeparatedEmails(email)).thenReturn(owners); + + Member result = manageMembers.updateMember(member); + assertTrue(result.getErrors().contains("group.email.already_used")); + } + + @Test + public void testFindClient_Empty() { + Client result = manageMembers.findClient(""); + assertEquals(1, result.getErrors().size()); + assertEquals("manage_member.not_blank", result.getErrors().get(0)); + } + + @Test + public void testFindClient_Success() { + String clientId = "APP-123"; + org.orcid.jaxb.model.v3.release.client.Client model = mock(org.orcid.jaxb.model.v3.release.client.Client.class); + when(model.getId()).thenReturn(clientId); + when(model.getClientType()).thenReturn(org.orcid.jaxb.model.clientgroup.ClientType.CREATOR); + + when(clientManagerReadOnly.get(clientId)).thenReturn(model); + + ClientDetailsEntity details = new ClientDetailsEntity(); + details.setDeactivatedDate(new Date()); + when(clientDetailsManagerReadOnly.findByClientId(clientId)).thenReturn(details); + + Client result = manageMembers.findClient(clientId); + assertEquals(clientId, result.getClientId().getValue()); + assertTrue(result.isDeactivated()); + } + + @Test + public void testUpdateClient_Success() { + Client client = new Client(); + client.setDisplayName(Text.valueOf("Name")); + client.setWebsite(Text.valueOf("http://website.com")); + client.setShortDescription(Text.valueOf("Desc")); + client.setRedirectUris(new ArrayList<>()); + + org.orcid.jaxb.model.v3.release.client.Client model = mock(org.orcid.jaxb.model.v3.release.client.Client.class); + when(model.getId()).thenReturn("APP-123"); + when(model.getClientType()).thenReturn(org.orcid.jaxb.model.clientgroup.ClientType.CREATOR); + + when(clientManager.edit(any(), eq(true))).thenReturn(model); + + Client result = manageMembers.updateClient(client); + assertEquals(0, result.getErrors().size()); + verify(clientManager).edit(any(), eq(true)); + } + + @Test + public void testUpdateClient_WithIdP_NoInstitutionalRedirectUri() { + Client client = new Client(); + client.setDisplayName(Text.valueOf("Name")); + client.setAuthenticationProviderId(Text.valueOf("IDP-123")); + client.setRedirectUris(new ArrayList<>()); + + Client result = manageMembers.updateClient(client); + assertTrue(result.getErrors().contains("manage.developer_tools.client.idp.error.no_redirect_uri_found")); + } + + @Test + public void testUpdateClient_WithIdP_WithInstitutionalRedirectUri() { + Client client = new Client(); + client.setDisplayName(Text.valueOf("Name")); + client.setAuthenticationProviderId(Text.valueOf("IDP-123")); + RedirectUri rUri = new RedirectUri(); + rUri.setType(Text.valueOf(RedirectUriType.INSTITUTIONAL_SIGN_IN.value())); + rUri.setValue(Text.valueOf("http://redirect.uri")); + client.setRedirectUris(List.of(rUri)); + + org.orcid.jaxb.model.v3.release.client.Client model = mock(org.orcid.jaxb.model.v3.release.client.Client.class); + when(model.getId()).thenReturn("APP-123"); + when(model.getClientType()).thenReturn(org.orcid.jaxb.model.clientgroup.ClientType.CREATOR); + when(clientManager.edit(any(), eq(true))).thenReturn(model); + + Client result = manageMembers.updateClient(client); + assertFalse(result.getErrors().contains("manage.developer_tools.client.idp.error.no_redirect_uri_found")); + } + + @Test + public void testGetEmptyRedirectUri() { + RedirectUri result = manageMembers.getEmptyRedirectUri(); + assertEquals(RedirectUriType.DEFAULT.value(), result.getType().getValue()); + assertEquals("", result.getValue().getValue()); + } + + @Test + public void testDeactivateClient_NotFound() { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-MISSING"); + when(clientDetailsManager.exists("APP-MISSING")).thenReturn(false); + + ClientActivationRequest result = manageMembers.deactivateClient(request); + assertEquals("Client not found", result.getError()); + } + + @Test + public void testDeactivateClient_AlreadyDeactivated() throws ClientAlreadyDeactivatedException { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-123"); + when(clientDetailsManager.exists("APP-123")).thenReturn(true); + doThrow(new ClientAlreadyDeactivatedException("already deactivated")) + .when(clientDetailsManager).deactivateClientDetails(eq("APP-123"), anyString()); + + ClientActivationRequest result = manageMembers.deactivateClient(request); + assertEquals("already deactivated", result.getError()); + } + + @Test + public void testDeactivateClient_Success() throws ClientAlreadyDeactivatedException { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-123"); + when(clientDetailsManager.exists("APP-123")).thenReturn(true); + + ClientActivationRequest result = manageMembers.deactivateClient(request); + assertNull(result.getError()); + verify(clientDetailsManager).deactivateClientDetails(eq("APP-123"), eq("4444-4444-4444-4440")); + } + + @Test + public void testActivateClient_NotFound() { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-MISSING"); + when(clientDetailsManager.exists("APP-MISSING")).thenReturn(false); + + ClientActivationRequest result = manageMembers.activateClient(request); + assertEquals("Client not found", result.getError()); + } + + @Test + public void testActivateClient_AlreadyActive() throws ClientAlreadyActiveException { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-123"); + when(clientDetailsManager.exists("APP-123")).thenReturn(true); + doThrow(new ClientAlreadyActiveException("already active")) + .when(clientDetailsManager).activateClientDetails("APP-123"); + + ClientActivationRequest result = manageMembers.activateClient(request); + assertEquals("already active", result.getError()); + } + + @Test + public void testActivateClient_Success() throws ClientAlreadyActiveException { + ClientActivationRequest request = new ClientActivationRequest(); + request.setClientId("APP-123"); + when(clientDetailsManager.exists("APP-123")).thenReturn(true); + + ClientActivationRequest result = manageMembers.activateClient(request); + assertNull(result.getError()); + verify(clientDetailsManager).activateClientDetails("APP-123"); + } + + @Test + public void testGetRedirectUriTypes() { + Map types = manageMembers.getRedirectUriTypes(); + assertFalse(types.containsKey(RedirectUriType.SSO_AUTHENTICATION.value())); + assertTrue(types.containsKey(RedirectUriType.DEFAULT.value())); + } + + @Test + public void testRetrieveGroupTypes() { + Map types = manageMembers.retrieveGroupTypes(); + assertFalse(types.containsKey(MemberType.BASIC_INSTITUTION.value())); + assertFalse(types.containsKey(MemberType.PREMIUM_INSTITUTION.value())); + assertTrue(types.containsKey(MemberType.BASIC.value())); + assertEquals("basic", types.get(MemberType.BASIC.value())); + } + + private Member createValidMember() { + Member member = new Member(); + member.setEmail(Text.valueOf("test@orcid.org")); + member.setGroupName(Text.valueOf("Test Group")); + member.setType(Text.valueOf(MemberType.BASIC.value())); + member.setSalesforceId(Text.valueOf("1234567890abcde")); + return member; } }