Skip to content

Commit e668d2a

Browse files
gileswGiles Westwoodamontenegro
authored
{PD-5580} fix/work-summary-client-details-fanout (#7516)
* client-details-bulk-prefetch * Arrays removed in error * More improvements, lets build the source from the manager so it build it only once outside the facade mapper * Some basic stats collected to compare with current code * Refactoring the SourceEntityUtil so it is less complex, not all functions are static and it removes duplicated functions * Improvements in the way we load the source names on retrieveWorkSummaryExtended * Fix unit tests * More improvements in the UI endpoint * Some logs and improvements * More improvements * Remove system outs * Remove system outs * Clean imports * Fix test by refactoring it to use mocks, so, we dont need transactional tests * Fix test by refactoring it to use mocks, so, we dont need transactional tests --------- Co-authored-by: Giles Westwood <giles@gileswestwood.com> Co-authored-by: amontenegro <a.montenegro@orcid.org>
1 parent c478932 commit e668d2a

48 files changed

Lines changed: 1204 additions & 892 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎orcid-core/src/main/java/org/orcid/core/adapter/v3/JpaJaxbWorkAdapter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@
22

33
import java.util.Collection;
44
import java.util.List;
5+
import java.util.Map;
56

7+
import org.orcid.jaxb.model.v3.release.common.Source;
68
import org.orcid.jaxb.model.v3.release.record.Work;
79
import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary;
810
import org.orcid.persistence.jpa.entities.MinimizedExtendedWorkEntity;
911
import org.orcid.persistence.jpa.entities.MinimizedWorkEntity;
12+
import org.orcid.persistence.jpa.entities.ClientDetailsEntity;
1013
import org.orcid.persistence.jpa.entities.WorkEntity;
1114
import org.orcid.pojo.WorkExtended;
1215
import org.orcid.pojo.WorkSummaryExtended;
@@ -34,6 +37,8 @@ public interface JpaJaxbWorkAdapter {
3437

3538
List<WorkSummary> toWorkSummaryFromMinimized(Collection<MinimizedWorkEntity> workEntities);
3639

40+
List<WorkSummary> toWorkSummaryFromMinimized(Collection<MinimizedWorkEntity> workEntities, Map<String, Source> sources);
41+
3742
List<WorkSummaryExtended> toWorkSummaryExtendedFromMinimized(Collection<MinimizedExtendedWorkEntity> workEntities);
3843

3944
WorkEntity toWorkEntity(Work work, WorkEntity existing);

‎orcid-core/src/main/java/org/orcid/core/adapter/v3/converter/ContributorsRolesAndSequencesConverter.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import ma.glasnost.orika.converter.BidirectionalConverter;
77
import ma.glasnost.orika.metadata.Type;
88
import org.orcid.core.contributors.roles.ContributorRoleConverter;
9+
import org.orcid.core.contributors.roles.ContributorRoleConverterImpl;
910
import org.orcid.core.contributors.roles.credit.CreditRole;
1011
import org.orcid.core.utils.JsonUtils;
1112
import org.orcid.core.utils.v3.ContributorUtils;
@@ -15,18 +16,19 @@
1516
import org.slf4j.Logger;
1617
import org.slf4j.LoggerFactory;
1718

19+
import javax.annotation.Resource;
1820
import java.util.ArrayList;
1921
import java.util.List;
2022

2123
public class ContributorsRolesAndSequencesConverter extends BidirectionalConverter<List<ContributorsRolesAndSequences>, String> {
2224

2325
private static final Logger LOGGER = LoggerFactory.getLogger(ContributorsRolesAndSequencesConverter.class);
2426

25-
private ContributorRoleConverter roleConverter;
27+
@Resource(name = "workContributorRoleConverter")
28+
private ContributorRoleConverter workContributorRoleConverter;
2629

27-
public ContributorsRolesAndSequencesConverter(ContributorRoleConverter roleConverter) {
28-
this.roleConverter = roleConverter;
29-
}
30+
@Resource(name = "contributorUtilsV3")
31+
private ContributorUtils contributorUtils;
3032

3133
@Override
3234
public String convertTo(List<ContributorsRolesAndSequences> source, Type<String> destinationType) {
@@ -39,7 +41,6 @@ public List<ContributorsRolesAndSequences> convertFrom(String source, Type<List<
3941
}
4042

4143
public List<ContributorsRolesAndSequences> getContributorsRolesAndSequencesList(String source) {
42-
ContributorUtils contributorUtils = new ContributorUtils(null);
4344
final ObjectMapper objectMapper = new ObjectMapper();
4445
List<ContributorsRolesAndSequences> contributorsRolesAndSequencesResult = new ArrayList<>();
4546
try {
@@ -54,7 +55,7 @@ public List<ContributorsRolesAndSequences> getContributorsRolesAndSequencesList(
5455
if (cr != null) {
5556
providedRoleValue = cr.name();
5657
}
57-
crs.setContributorRole(contributorUtils.getCreditRole(roleConverter.toRoleValue(providedRoleValue)));
58+
crs.setContributorRole(contributorUtils.getCreditRole(workContributorRoleConverter.toRoleValue(providedRoleValue)));
5859
}
5960
}
6061
}

‎orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/JpaJaxbWorkAdapterImpl.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22

33
import java.util.Collection;
44
import java.util.List;
5+
import java.util.Map;
56

7+
import ma.glasnost.orika.MappingContext;
68
import ma.glasnost.orika.MapperFacade;
79

10+
import org.orcid.core.utils.SourceEntityUtils;
811
import org.orcid.core.adapter.v3.JpaJaxbWorkAdapter;
12+
import org.orcid.jaxb.model.v3.release.common.Source;
913
import org.orcid.jaxb.model.v3.release.record.Work;
1014
import org.orcid.jaxb.model.v3.release.record.summary.WorkSummary;
15+
import org.orcid.persistence.jpa.entities.ClientDetailsEntity;
1116
import org.orcid.persistence.jpa.entities.MinimizedExtendedWorkEntity;
1217
import org.orcid.persistence.jpa.entities.MinimizedWorkEntity;
1318
import org.orcid.persistence.jpa.entities.WorkEntity;
@@ -100,6 +105,16 @@ public List<WorkSummary> toWorkSummaryFromMinimized(Collection<MinimizedWorkEnti
100105
return mapperFacade.mapAsList(workEntities, WorkSummary.class);
101106
}
102107

108+
@Override
109+
public List<WorkSummary> toWorkSummaryFromMinimized(Collection<MinimizedWorkEntity> workEntities, Map<String, Source> sourceMap) {
110+
if(workEntities == null) {
111+
return null;
112+
}
113+
MappingContext context = new MappingContext.Factory().getContext();
114+
context.setProperty(SourceEntityUtils.SOURCE_MAP, sourceMap);
115+
return mapperFacade.mapAsList(workEntities, WorkSummary.class, context);
116+
}
117+
103118
@Override
104119
public List<WorkSummaryExtended> toWorkSummaryExtendedFromMinimized(Collection<MinimizedExtendedWorkEntity> workEntities) {
105120
if(workEntities == null) {

‎orcid-core/src/main/java/org/orcid/core/adapter/v3/impl/MapperFacadeFactory.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,9 @@
3131
import org.orcid.core.contributors.roles.works.WorkContributorRoleConverter;
3232
import org.orcid.core.exception.OrcidValidationException;
3333
import org.orcid.core.locale.LocaleManager;
34-
import org.orcid.core.manager.ClientDetailsEntityCacheManager;
3534
import org.orcid.core.manager.EncryptionManager;
3635
import org.orcid.core.manager.IdentityProviderManager;
37-
import org.orcid.core.manager.SourceNameCacheManager;
3836
import org.orcid.core.manager.impl.OrcidUrlManager;
39-
import org.orcid.core.manager.v3.read_only.ClientDetailsManagerReadOnly;
4037
import org.orcid.core.utils.JsonUtils;
4138
import org.orcid.core.utils.SourceEntityUtils;
4239
import org.orcid.core.utils.v3.identifiers.PIDNormalizationService;
@@ -140,7 +137,6 @@
140137
import org.orcid.persistence.jpa.entities.SpamEntity;
141138
import org.orcid.persistence.jpa.entities.StartDateEntity;
142139
import org.orcid.persistence.jpa.entities.WorkEntity;
143-
import org.orcid.persistence.jpa.entities.keys.ClientRedirectUriPk;
144140
import org.orcid.pojo.WorkExtended;
145141
import org.orcid.pojo.WorkSummaryExtended;
146142
import org.orcid.pojo.ajaxForm.PojoUtil;
@@ -170,15 +166,6 @@ public class MapperFacadeFactory implements FactoryBean<MapperFacade> {
170166
@Resource
171167
private WorkDao workDao;
172168

173-
@Resource
174-
private SourceNameCacheManager sourceNameCacheManager;
175-
176-
@Resource
177-
private ClientDetailsEntityCacheManager clientDetailsEntityCacheManager;
178-
179-
@Resource(name = "clientDetailsManagerReadOnlyV3")
180-
private ClientDetailsManagerReadOnly clientDetailsManagerReadOnly;
181-
182169
@Resource
183170
private IdentityProviderManager identityProviderManager;
184171

@@ -200,6 +187,12 @@ public class MapperFacadeFactory implements FactoryBean<MapperFacade> {
200187
@Resource
201188
private FundingContributorRoleConverter fundingContributorsRoleConverter;
202189

190+
@Resource
191+
private SourceEntityUtils sourceEntityUtils;
192+
193+
@Resource
194+
private ContributorsRolesAndSequencesConverter contributorsRolesAndSequencesConverter;
195+
203196
@Override
204197
public MapperFacade getObject() throws Exception {
205198
MapperFactory mapperFactory = new DefaultMapperFactory.Builder().build();
@@ -387,10 +380,19 @@ public void registerSourceConverters(MapperFactory mapperFactory, ClassMapBuilde
387380
}
388381

389382
private class SourceMapper<T, U> extends CustomMapper<SourceAware, SourceAwareEntity<?>> {
383+
@SuppressWarnings("unchecked")
390384
@Override
391385
public void mapBtoA(SourceAwareEntity<?> b, SourceAware a, MappingContext context) {
392-
Source source = SourceEntityUtils.extractSourceFromEntityComplete(b, sourceNameCacheManager, orcidUrlManager, clientDetailsEntityCacheManager);
393-
a.setSource(source);
386+
if (context != null && context.getProperty(SourceEntityUtils.SOURCE_MAP) != null) {
387+
// The source map is set in the context, so we can use it to set the source.
388+
Map<String, Source> sourceMap = (Map<String, Source>) context.getProperty(SourceEntityUtils.SOURCE_MAP);
389+
Source source = sourceMap.get(sourceEntityUtils.getSourceKey(b));
390+
a.setSource(source);
391+
} else {
392+
// We have to manually build the source elements
393+
Source source = sourceEntityUtils.extractSourceFromEntityComplete(b);
394+
a.setSource(source);
395+
}
394396
}
395397
}
396398

@@ -536,8 +538,6 @@ public MapperFacade getWorkMapperFacade() {
536538
MapperFactory mapperFactory = new DefaultMapperFactory.Builder().build();
537539

538540
WorkContributorsConverter wcc = new WorkContributorsConverter(workContributorsRoleConverter);
539-
ContributorsRolesAndSequencesConverter contributorsRolesAndSequencesConverter = new ContributorsRolesAndSequencesConverter(workContributorsRoleConverter);
540-
541541
ConverterFactory converterFactory = mapperFactory.getConverterFactory();
542542
converterFactory.registerConverter("workExternalIdentifiersConverterId", new JSONWorkExternalIdentifiersConverterV3(norm, resolverService, localeManager));
543543
converterFactory.registerConverter("workContributorsConverterId", wcc);

‎orcid-core/src/main/java/org/orcid/core/cli/FilterTopContributors.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private void filter() {
6565

6666
private void filterTopContributors(Object[] workObject) {
6767
WorkEntity workEntity = workDao.find(((BigInteger) workObject[0]).longValue());
68-
ContributorUtils contributorUtils = new ContributorUtils(0);
68+
ContributorUtils contributorUtils = new ContributorUtils();
6969
WorkSummaryExtended wse = new WorkSummaryExtended.WorkSummaryExtendedBuilder(((BigInteger) workObject[0]))
7070
.contributors(workContributorsConverter.getContributorsList(isEmpty(workObject[1])))
7171
.build();

‎orcid-core/src/main/java/org/orcid/core/exception/InvalidPutCodeException.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.HashMap;
44
import java.util.Map;
55

6+
import org.apache.commons.lang3.StringUtils;
67
import org.orcid.core.utils.SourceEntityUtils;
78
import org.orcid.jaxb.model.v3.release.common.Source;
89

@@ -19,10 +20,10 @@ public InvalidPutCodeException(Map<String, String> params) {
1920
super(params);
2021
}
2122

22-
public static InvalidPutCodeException forSource(Source activeSource) {
23+
public static InvalidPutCodeException forSource(String sourceName) {
2324
Map<String, String> params = new HashMap<String, String>();
24-
if (activeSource != null) {
25-
params.put("clientName", SourceEntityUtils.getSourceName(activeSource));
25+
if (StringUtils.isNoneBlank(sourceName)) {
26+
params.put("clientName", sourceName);
2627
}
2728
return new InvalidPutCodeException(params);
2829
}

‎orcid-core/src/main/java/org/orcid/core/exception/OrcidCoreExceptionMapper.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ public class OrcidCoreExceptionMapper {
4747
@Resource(name = "sourceManagerV3")
4848
private SourceManager sourceManager;
4949

50+
@Resource
51+
private SourceEntityUtils sourceEntityUtils;
52+
5053
private static Map<Class<? extends Throwable>, Pair<Response.Status, Integer>> HTTP_STATUS_AND_ERROR_CODE_BY_THROWABLE_TYPE = new HashMap<>();
5154
{
5255
// 301
@@ -226,7 +229,7 @@ public org.orcid.jaxb.model.v3.release.error.OrcidError getOrcidErrorV3(int erro
226229
Map<String, String> params = null;
227230
if (t instanceof PutCodeFormatException) {
228231
params = new HashMap<String, String>();
229-
params.put("clientName", SourceEntityUtils.getSourceName(sourceManager.retrieveActiveSource()));
232+
params.put("clientName", sourceEntityUtils.getSourceName(sourceManager.retrieveActiveSource()));
230233
} else if (t instanceof ApplicationException) {
231234
params = ((ApplicationException) t).getParams();
232235
} else if (t instanceof IllegalEnumValueException) {

‎orcid-core/src/main/java/org/orcid/core/manager/ClientDetailsEntityCacheManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
package org.orcid.core.manager;
22

3+
import java.util.Collection;
4+
import java.util.Map;
5+
36
import org.orcid.persistence.jpa.entities.ClientDetailsEntity;
47

58
public interface ClientDetailsEntityCacheManager {
69

710
public ClientDetailsEntity retrieve(String clientId) throws IllegalArgumentException;
11+
12+
public Map<String, ClientDetailsEntity> retrieveAll(Collection<String> clientIds);
813

914
public ClientDetailsEntity retrieveByIdP(String clientId) throws IllegalArgumentException;
1015

‎orcid-core/src/main/java/org/orcid/core/manager/impl/AddressManagerImpl.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public class AddressManagerImpl extends AddressManagerReadOnlyImpl implements Ad
3333
@Resource
3434
protected SourceManager sourceManager;
3535

36+
@Resource
37+
private SourceEntityUtils sourceEntityUtils;
38+
3639
@Resource
3740
private ProfileEntityCacheManager profileEntityCacheManager;
3841

@@ -130,7 +133,7 @@ private boolean isDuplicated(AddressEntity existing, Address address, SourceEnti
130133
if (!existing.getId().equals(address.getPutCode())) {
131134
// If they have the same source
132135
String existingSourceId = existing.getElementSourceId();
133-
if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(SourceEntityUtils.getSourceId(source))) {
136+
if (!PojoUtil.isEmpty(existingSourceId) && existingSourceId.equals(sourceEntityUtils.getSourceId(source))) {
134137
if (existing.getIso2Country().equals(address.getCountry().getValue())) {
135138
return true;
136139
}

‎orcid-core/src/main/java/org/orcid/core/manager/impl/ClientDetailsEntityCacheManagerImpl.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package org.orcid.core.manager.impl;
22

3+
import java.util.ArrayList;
4+
import java.util.Collection;
35
import java.util.Date;
6+
import java.util.HashMap;
7+
import java.util.List;
8+
import java.util.Map;
49

510
import javax.annotation.Resource;
611

@@ -46,6 +51,44 @@ public ClientDetailsEntity retrieve(String clientId) throws IllegalArgumentExcep
4651
return clientDetails;
4752
}
4853

54+
@Override
55+
public Map<String, ClientDetailsEntity> retrieveAll(Collection<String> clientIds) {
56+
Map<String, ClientDetailsEntity> clientDetailsById = new HashMap<>();
57+
if (clientIds == null || clientIds.isEmpty()) {
58+
return clientDetailsById;
59+
}
60+
61+
List<String> clientIdList = new ArrayList<>(clientIds);
62+
Map<String, Date> lastModifiedByClientId = clientDetailsManager.getLastModifiedByClientIds(clientIdList);
63+
if (lastModifiedByClientId == null) {
64+
return clientDetailsById;
65+
}
66+
List<String> staleOrMissingClientIds = new ArrayList<>();
67+
68+
for (String clientId : clientIdList) {
69+
Date dbDate = lastModifiedByClientId.get(clientId);
70+
if (dbDate == null) {
71+
continue;
72+
}
73+
Object key = new ClientIdCacheKey(clientId, releaseName);
74+
ClientDetailsEntity clientDetails = clientDetailsCache.get(key);
75+
if (needsFresh(dbDate, clientDetails)) {
76+
staleOrMissingClientIds.add(clientId);
77+
} else {
78+
clientDetailsById.put(clientId, clientDetails);
79+
}
80+
}
81+
82+
if (!staleOrMissingClientIds.isEmpty()) {
83+
for (ClientDetailsEntity clientDetails : clientDetailsManager.findByClientIds(staleOrMissingClientIds)) {
84+
clientDetailsCache.put(new ClientIdCacheKey(clientDetails.getId(), releaseName), clientDetails);
85+
clientDetailsById.put(clientDetails.getId(), clientDetails);
86+
}
87+
}
88+
89+
return clientDetailsById;
90+
}
91+
4992
@Override
5093
public ClientDetailsEntity retrieveByIdP(String idp) throws IllegalArgumentException {
5194
Object key = new ClientIdCacheKey("IdP+" + idp, releaseName);

0 commit comments

Comments
 (0)