Skip to content

Commit 0a77f67

Browse files
chrisknollchrisknoll
committed
Added code to handle test context reloads.
Fixed permission tests.
1 parent 37aaef8 commit 0a77f67

File tree

9 files changed

+128
-90
lines changed

9 files changed

+128
-90
lines changed

‎src/main/java/org/ohdsi/webapi/cache/CacheService.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import java.util.ArrayList;
1919
import java.util.List;
20-
import java.util.Optional;
2120
import java.util.stream.StreamSupport;
2221
import javax.cache.Cache;
2322
import javax.cache.CacheManager;
@@ -54,6 +53,10 @@ public CacheService(CacheManager cacheManager) {
5453
this.cacheManager = cacheManager;
5554
}
5655

56+
public CacheService() {
57+
}
58+
59+
5760
@GET
5861
@Path("/")
5962
@Produces(MediaType.APPLICATION_JSON)
@@ -88,5 +91,4 @@ public ClearCacheResult clearAll() {
8891
}
8992
return result;
9093
}
91-
9294
}

‎src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,12 @@ public static class CachingSetup implements JCacheManagerCustomizer {
163163
@Override
164164
public void customize(CacheManager cacheManager) {
165165
// Evict when a cohort definition is created or updated, or permissions, or tags
166-
cacheManager.createCache(COHORT_DEFINITION_LIST_CACHE, new MutableConfiguration<String, List<CohortMetadataDTO>>()
167-
.setTypes(String.class, (Class<List<CohortMetadataDTO>>) (Class<?>) List.class)
168-
.setStoreByValue(false)
169-
.setStatisticsEnabled(true));
166+
if (!CacheHelper.getCacheNames(cacheManager).contains(COHORT_DEFINITION_LIST_CACHE)) {
167+
cacheManager.createCache(COHORT_DEFINITION_LIST_CACHE, new MutableConfiguration<String, List<CohortMetadataDTO>>()
168+
.setTypes(String.class, (Class<List<CohortMetadataDTO>>) (Class<?>) List.class)
169+
.setStoreByValue(false)
170+
.setStatisticsEnabled(true));
171+
}
170172
}
171173
}
172174

‎src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+11-5
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.ohdsi.webapi.conceptset.dto.ConceptSetVersionFullDTO;
4242
import org.ohdsi.webapi.exception.ConceptNotExistException;
4343
import org.ohdsi.webapi.security.PermissionService;
44+
import static org.ohdsi.webapi.service.CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE;
4445
import org.ohdsi.webapi.service.dto.ConceptSetDTO;
4546
import org.ohdsi.webapi.shiro.Entities.UserEntity;
4647
import org.ohdsi.webapi.shiro.Entities.UserRepository;
@@ -51,6 +52,7 @@
5152
import org.ohdsi.webapi.source.SourceService;
5253
import org.ohdsi.webapi.tag.domain.HasTags;
5354
import org.ohdsi.webapi.tag.dto.TagNameListRequestDTO;
55+
import org.ohdsi.webapi.util.CacheHelper;
5456
import org.ohdsi.webapi.util.ExportUtil;
5557
import org.ohdsi.webapi.util.NameUtils;
5658
import org.ohdsi.webapi.util.ExceptionUtils;
@@ -88,14 +90,18 @@ public static class CachingSetup implements JCacheManagerCustomizer {
8890

8991
@Override
9092
public void customize(CacheManager cacheManager) {
93+
// due to unit tests causing application contexts to reload cache manager caches, we
94+
// have to check for the existance of a cache before creating it
95+
Set<String> cacheNames = CacheHelper.getCacheNames(cacheManager);
9196
// Evict when a cohort definition is created or updated, or permissions, or tags
92-
cacheManager.createCache(CONCEPT_SET_LIST_CACHE, new MutableConfiguration<String, Collection<ConceptSetDTO>>()
93-
.setTypes(String.class, (Class<Collection<ConceptSetDTO>>) (Class<?>) List.class)
94-
.setStoreByValue(false)
95-
.setStatisticsEnabled(true));
97+
if (!cacheNames.contains(CONCEPT_SET_LIST_CACHE)) {
98+
cacheManager.createCache(CONCEPT_SET_LIST_CACHE, new MutableConfiguration<String, Collection<ConceptSetDTO>>()
99+
.setTypes(String.class, (Class<Collection<ConceptSetDTO>>) (Class<?>) List.class)
100+
.setStoreByValue(false)
101+
.setStatisticsEnabled(true));
102+
}
96103
}
97104
}
98-
99105
@Autowired
100106
private ConceptSetGenerationInfoRepository conceptSetGenerationInfoRepository;
101107

‎src/main/java/org/ohdsi/webapi/service/VocabularyService.java

+22-12
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.ohdsi.webapi.source.SourceService;
5454
import org.ohdsi.webapi.source.SourceDaimon;
5555
import org.ohdsi.webapi.source.SourceInfo;
56+
import org.ohdsi.webapi.util.CacheHelper;
5657
import org.ohdsi.webapi.util.PreparedSqlRender;
5758
import org.ohdsi.webapi.util.PreparedStatementRenderer;
5859
import org.ohdsi.webapi.vocabulary.ConceptRecommendedNotInstalledException;
@@ -98,19 +99,28 @@ public static class CachingSetup implements JCacheManagerCustomizer {
9899

99100
@Override
100101
public void customize(CacheManager cacheManager) {
102+
// due to unit tests causing application contexts to reload cache manager caches, we
103+
// have to check for the existance of a cache before creating it
104+
Set<String> cacheNames = CacheHelper.getCacheNames(cacheManager);
101105
// Evict when a cohort definition is created or updated, or permissions, or tags
102-
cacheManager.createCache(CONCEPT_DETAIL_CACHE, new MutableConfiguration<String, Concept>()
103-
.setTypes(String.class, Concept.class)
104-
.setStoreByValue(false)
105-
.setStatisticsEnabled(true));
106-
cacheManager.createCache(CONCEPT_RELATED_CACHE, new MutableConfiguration<String, Collection<RelatedConcept>>()
107-
.setTypes(String.class, (Class<Collection<RelatedConcept>>) (Class<?>) Collection.class)
108-
.setStoreByValue(false)
109-
.setStatisticsEnabled(true));
110-
cacheManager.createCache(CONCEPT_HIERARCHY_CACHE, new MutableConfiguration<String, Collection<RelatedConcept>>()
111-
.setTypes(String.class, (Class<Collection<RelatedConcept>>) (Class<?>) Collection.class)
112-
.setStoreByValue(false)
113-
.setStatisticsEnabled(true));
106+
if (!cacheNames.contains(CONCEPT_DETAIL_CACHE)) {
107+
cacheManager.createCache(CONCEPT_DETAIL_CACHE, new MutableConfiguration<String, Concept>()
108+
.setTypes(String.class, Concept.class)
109+
.setStoreByValue(false)
110+
.setStatisticsEnabled(true));
111+
}
112+
if (!cacheNames.contains(CONCEPT_RELATED_CACHE)) {
113+
cacheManager.createCache(CONCEPT_RELATED_CACHE, new MutableConfiguration<String, Collection<RelatedConcept>>()
114+
.setTypes(String.class, (Class<Collection<RelatedConcept>>) (Class<?>) Collection.class)
115+
.setStoreByValue(false)
116+
.setStatisticsEnabled(true));
117+
}
118+
if (!cacheNames.contains(CONCEPT_HIERARCHY_CACHE)) {
119+
cacheManager.createCache(CONCEPT_HIERARCHY_CACHE, new MutableConfiguration<String, Collection<RelatedConcept>>()
120+
.setTypes(String.class, (Class<Collection<RelatedConcept>>) (Class<?>) Collection.class)
121+
.setStoreByValue(false)
122+
.setStatisticsEnabled(true));
123+
}
114124
}
115125
}
116126

‎src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java

+45-35
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.apache.shiro.authz.Permission;
4242
import org.apache.shiro.authz.permission.WildcardPermission;
4343
import org.ohdsi.circe.helper.ResourceHelper;
44+
import org.ohdsi.webapi.util.CacheHelper;
4445
import org.springframework.beans.factory.annotation.Value;
4546
import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer;
4647
import org.springframework.cache.annotation.CacheEvict;
@@ -62,14 +63,18 @@ public static class CachingSetup implements JCacheManagerCustomizer {
6263

6364
@Override
6465
public void customize(CacheManager cacheManager) {
66+
// due to unit tests causing application contexts to reload cache manager caches, we
67+
// have to check for the existance of a cache before creating it
68+
Set<String> cacheNames = CacheHelper.getCacheNames(cacheManager);
6569
// Evict when a user, role or permission is modified/deleted.
66-
cacheManager.createCache(AUTH_INFO_CACHE, new MutableConfiguration<String, UserSimpleAuthorizationInfo>()
67-
.setTypes(String.class, UserSimpleAuthorizationInfo.class)
68-
.setStoreByValue(false)
69-
.setStatisticsEnabled(true));
70+
if (!cacheNames.contains(AUTH_INFO_CACHE)) {
71+
cacheManager.createCache(AUTH_INFO_CACHE, new MutableConfiguration<String, UserSimpleAuthorizationInfo>()
72+
.setTypes(String.class, UserSimpleAuthorizationInfo.class)
73+
.setStoreByValue(false)
74+
.setStatisticsEnabled(true));
75+
}
7076
}
7177
}
72-
7378

7479
@Value("${datasource.ohdsi.schema}")
7580
private String ohdsiSchema;
@@ -95,6 +100,8 @@ public void customize(CacheManager cacheManager) {
95100
@Autowired
96101
private JdbcTemplate jdbcTemplate;
97102

103+
private ThreadLocal<ConcurrentHashMap<String, UserSimpleAuthorizationInfo>> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new);
104+
98105
public static class PermissionsDTO {
99106

100107
public Map<String, List<String>> permissions = null;
@@ -163,39 +170,42 @@ public Iterable<RoleEntity> getRoles(boolean includePersonalRoles) {
163170
@Cacheable(cacheNames = CachingSetup.AUTH_INFO_CACHE)
164171
public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) {
165172

166-
final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo();
167-
168-
final UserEntity userEntity = userRepository.findByLogin(login);
169-
if(userEntity == null) {
170-
throw new UnknownAccountException("Account does not exist");
171-
}
172-
173-
info.setUserId(userEntity.getId());
174-
info.setLogin(userEntity.getLogin());
175-
176-
for (UserRoleEntity userRole: userEntity.getUserRoles()) {
177-
info.addRole(userRole.getRole().getName());
178-
}
179-
180-
// convert permission index from queryUserPermissions() into a map of WildcardPermissions
181-
Map<String, List<String>> permsIdx = this.queryUserPermissions(login).permissions;
182-
Map permissionMap = new HashMap<String, List<Permission>>();
183-
Set<String> permissionNames = new HashSet<>();
184-
185-
for(String permIdxKey : permsIdx.keySet()) {
186-
List<String> perms = permsIdx.get(permIdxKey);
187-
permissionNames.addAll(perms);
188-
// convert raw string permission into Wildcard perm for each element in this key's array.
189-
permissionMap.put(permIdxKey, perms.stream().map(perm -> new WildcardPermission(perm)).collect(Collectors.toList()));
190-
}
191-
192-
info.setStringPermissions(permissionNames);
193-
info.setPermissionIdx(permissionMap);
194-
return info;
195-
}
173+
return authorizationInfoCache.get().computeIfAbsent(login, newLogin -> {
174+
final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo();
175+
176+
final UserEntity userEntity = userRepository.findByLogin(login);
177+
if(userEntity == null) {
178+
throw new UnknownAccountException("Account does not exist");
179+
}
180+
181+
info.setUserId(userEntity.getId());
182+
info.setLogin(userEntity.getLogin());
183+
184+
for (UserRoleEntity userRole: userEntity.getUserRoles()) {
185+
info.addRole(userRole.getRole().getName());
186+
}
187+
188+
// convert permission index from queryUserPermissions() into a map of WildcardPermissions
189+
Map<String, List<String>> permsIdx = this.queryUserPermissions(login).permissions;
190+
Map permissionMap = new HashMap<String, List<Permission>>();
191+
Set<String> permissionNames = new HashSet<>();
192+
193+
for(String permIdxKey : permsIdx.keySet()) {
194+
List<String> perms = permsIdx.get(permIdxKey);
195+
permissionNames.addAll(perms);
196+
// convert raw string permission into Wildcard perm for each element in this key's array.
197+
permissionMap.put(permIdxKey, perms.stream().map(perm -> new WildcardPermission(perm)).collect(Collectors.toList()));
198+
}
199+
200+
info.setStringPermissions(permissionNames);
201+
info.setPermissionIdx(permissionMap);
202+
return info;
203+
});
204+
}
196205

197206
@CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true)
198207
public void clearAuthorizationInfoCache() {
208+
authorizationInfoCache.remove();
199209
}
200210

201211
@Transactional

‎src/main/java/org/ohdsi/webapi/util/CacheHelper.java

+8
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package org.ohdsi.webapi.util;
22

33
import java.lang.management.ManagementFactory;
4+
import java.util.HashSet;
45
import java.util.Set;
56
import javax.cache.CacheManager;
7+
import javax.cache.configuration.MutableConfiguration;
68
import javax.cache.management.CacheStatisticsMXBean;
79
import javax.management.MBeanServer;
810
import javax.management.MBeanServerInvocationHandler;
@@ -33,4 +35,10 @@ public static CacheStatisticsMXBean getCacheStats(CacheManager cacheManager, Str
3335
throw new RuntimeException(e);
3436
}
3537
}
38+
39+
public static Set<String> getCacheNames(CacheManager cacheManager) {
40+
Set<String> cacheNames = new HashSet<>();
41+
cacheManager.getCacheNames().forEach((name) -> cacheNames.add(name));
42+
return cacheNames;
43+
}
3644
}

‎src/test/java/org/ohdsi/webapi/security/PermissionTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void setup() {
6565
ThreadContext.bind(subject);
6666
}
6767

68-
@Ignore
68+
// @Ignore
6969
@Test
7070
public void permsTest() throws Exception {
7171
// need to clear authorization cache before each test
@@ -88,7 +88,7 @@ public void permsTest() throws Exception {
8888

8989
}
9090

91-
@Ignore
91+
// @Ignore
9292
@Test
9393
public void wildcardTest() throws Exception {
9494
// need to clear authorization cache before each test

‎src/test/resources/permission/permsTest_PREP.json

+21-21
Original file line numberDiff line numberDiff line change
@@ -1,64 +1,64 @@
11
{
22
"public.sec_user": [
33
{
4-
"id":1001,
4+
"id":100001,
55
"login":"permsTest",
66
"name":"Perms Test User",
77
"origin":"SYSTEM"
88
}
99
],
1010
"public.sec_role": [
1111
{
12-
"id": 1001,
12+
"id": 100001,
1313
"name":"permsTest",
1414
"system_role":false
1515
}
1616
],
1717
"public.sec_permission" : [
1818
{
19-
"id": 1001,
19+
"id": 100001,
2020
"value":"printer:manage:printer1"
2121
},
2222
{
23-
"id": 1002,
23+
"id": 100002,
2424
"value":"printer:manage:printer2"
2525
},
2626
{
27-
"id": 1003,
27+
"id": 100003,
2828
"value":"printer:query:*"
2929
},
3030
{
31-
"id": 1004,
31+
"id": 100004,
3232
"value":"printer:print"
3333
}
3434
],
3535
"public.sec_role_permission" : [
3636
{
37-
"id":1001,
38-
"role_id":1001,
39-
"permission_id":1001
37+
"id":100001,
38+
"role_id":100001,
39+
"permission_id":100001
4040
},
4141
{
42-
"id":1002,
43-
"role_id":1001,
44-
"permission_id":1002
42+
"id":100002,
43+
"role_id":100001,
44+
"permission_id":100002
4545
},
4646
{
47-
"id":1003,
48-
"role_id":1001,
49-
"permission_id":1003
47+
"id":103,
48+
"role_id":100001,
49+
"permission_id":100003
5050
},
5151
{
52-
"id":1004,
53-
"role_id":1001,
54-
"permission_id":1004
52+
"id":100004,
53+
"role_id":100001,
54+
"permission_id":100004
5555
}
5656
],
5757
"public.sec_user_role": [
5858
{
59-
"id": 1001,
60-
"user_id":1001,
61-
"role_id":1001,
59+
"id": 100001,
60+
"user_id":100001,
61+
"role_id":100001,
6262
"origin":"SYSTEM"
6363
}
6464
]

0 commit comments

Comments
 (0)