From a2397060d32e8e081c9eade8caf2a082f388ad60 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Thu, 12 Sep 2024 16:47:13 -0400 Subject: [PATCH] Implemented caching for cohort def list, concept set list, permissions. Implementation uses ehCache 3.9. Added test for CohortDefinitionService caching. --- pom.xml | 13 ++ .../java/org/ohdsi/webapi/CacheConfig.java | 10 ++ .../java/org/ohdsi/webapi/JerseyConfig.java | 18 +-- .../org/ohdsi/webapi/cache/CacheInfo.java | 30 +++++ .../org/ohdsi/webapi/cache/CacheService.java | 92 ++++++++++++++ .../webapi/security/PermissionService.java | 16 +++ .../service/CohortDefinitionService.java | 30 ++++- .../webapi/service/ConceptSetService.java | 27 +++- .../webapi/service/VocabularyService.java | 47 ++++++- .../webapi/service/dto/CommonEntityDTO.java | 3 +- .../ohdsi/webapi/shiro/PermissionManager.java | 83 ++++++++----- .../webapi/shiro/filters/CacheFilter.java | 38 ------ .../shiro/management/AtlasSecurity.java | 1 + .../org/ohdsi/webapi/user/dto/UserDTO.java | 4 +- .../org/ohdsi/webapi/util/CacheHelper.java | 36 ++++++ src/main/resources/appCache.xml | 33 +++++ src/main/resources/application.properties | 4 + .../service/CohortDefinitionServiceTest.java | 115 ++++++++++++++++++ 18 files changed, 520 insertions(+), 80 deletions(-) create mode 100644 src/main/java/org/ohdsi/webapi/CacheConfig.java create mode 100644 src/main/java/org/ohdsi/webapi/cache/CacheInfo.java create mode 100644 src/main/java/org/ohdsi/webapi/cache/CacheService.java delete mode 100644 src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java create mode 100644 src/main/java/org/ohdsi/webapi/util/CacheHelper.java create mode 100644 src/main/resources/appCache.xml create mode 100644 src/test/java/org/ohdsi/webapi/service/CohortDefinitionServiceTest.java diff --git a/pom.xml b/pom.xml index f44b0e08a0..7be1e911ca 100644 --- a/pom.xml +++ b/pom.xml @@ -236,6 +236,10 @@ info warn + + jcache + + 10 20 2147483647 @@ -272,6 +276,7 @@ 3 true + true 47 @@ -742,6 +747,10 @@ provided + javax.cache + cache-api + 1.1.1 + org.ohdsi.sql SqlRender ${SqlRender.version} @@ -1189,6 +1198,10 @@ 1.1.7 + org.ehcache + ehcache + 3.9.11 + com.opentable.components otj-pg-embedded 0.13.1 diff --git a/src/main/java/org/ohdsi/webapi/CacheConfig.java b/src/main/java/org/ohdsi/webapi/CacheConfig.java new file mode 100644 index 0000000000..8e9eecf418 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/CacheConfig.java @@ -0,0 +1,10 @@ +package org.ohdsi.webapi; + +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.context.annotation.Configuration; + +@Configuration +@EnableCaching +public class CacheConfig { + +} diff --git a/src/main/java/org/ohdsi/webapi/JerseyConfig.java b/src/main/java/org/ohdsi/webapi/JerseyConfig.java index 91dbd2d15f..eabc3f8189 100644 --- a/src/main/java/org/ohdsi/webapi/JerseyConfig.java +++ b/src/main/java/org/ohdsi/webapi/JerseyConfig.java @@ -37,6 +37,7 @@ import javax.inject.Singleton; import javax.ws.rs.ext.RuntimeDelegate; +import org.ohdsi.webapi.cache.CacheService; /** * @@ -59,31 +60,32 @@ public JerseyConfig() { public void afterPropertiesSet() throws Exception { packages(this.rootPackage); register(ActivityService.class); + register(CacheService.class); + register(CcController.class); register(CDMResultsService.class); register(CohortAnalysisService.class); register(CohortDefinitionService.class); register(CohortResultsService.class); register(CohortService.class); register(ConceptSetService.class); + register(DDLService.class); register(EvidenceService.class); register(FeasibilityService.class); + register(FeatureExtractionService.class); register(InfoService.class); register(IRAnalysisResource.class); register(JobService.class); + register(MultiPartFeature.class); + register(PermissionController.class); register(PersonService.class); + register(ScriptExecutionController.class); + register(ScriptExecutionCallbackController.class); register(SourceController.class); register(SqlRenderService.class); - register(DDLService.class); + register(SSOController.class); register(TherapyPathResultsService.class); register(UserService.class); register(VocabularyService.class); - register(ScriptExecutionController.class); - register(ScriptExecutionCallbackController.class); - register(MultiPartFeature.class); - register(FeatureExtractionService.class); - register(CcController.class); - register(SSOController.class); - register(PermissionController.class); register(new AbstractBinder() { @Override protected void configure() { diff --git a/src/main/java/org/ohdsi/webapi/cache/CacheInfo.java b/src/main/java/org/ohdsi/webapi/cache/CacheInfo.java new file mode 100644 index 0000000000..d1ebd357d8 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/cache/CacheInfo.java @@ -0,0 +1,30 @@ +/* + * Copyright 2019 cknoll1. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.ohdsi.webapi.cache; + +import java.io.Serializable; +import java.util.Optional; +import javax.cache.management.CacheStatisticsMXBean; + +/** + * + * @author cknoll1 + */ +public class CacheInfo implements Serializable{ + public String cacheName; + public Long entries; + public CacheStatisticsMXBean cacheStatistics; +} diff --git a/src/main/java/org/ohdsi/webapi/cache/CacheService.java b/src/main/java/org/ohdsi/webapi/cache/CacheService.java new file mode 100644 index 0000000000..f58107725a --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/cache/CacheService.java @@ -0,0 +1,92 @@ +/* + * Copyright 2019 cknoll1. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.ohdsi.webapi.cache; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.stream.StreamSupport; +import javax.cache.Cache; +import javax.cache.CacheManager; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import org.ohdsi.webapi.util.CacheHelper; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * + * @author cknoll1 + */ +@Path("/cache") +@Component +public class CacheService { + + public static class ClearCacheResult { + + public List clearedCaches; + + private ClearCacheResult() { + this.clearedCaches = new ArrayList<>(); + } + } + + private CacheManager cacheManager; + + @Autowired(required = false) + public CacheService(CacheManager cacheManager) { + + this.cacheManager = cacheManager; + } + + @GET + @Path("/") + @Produces(MediaType.APPLICATION_JSON) + public List getCacheInfoList() { + List caches = new ArrayList<>(); + + if (cacheManager == null) return caches; //caching is disabled + + for (String cacheName : cacheManager.getCacheNames()) { + Cache cache = cacheManager.getCache(cacheName); + CacheInfo info = new CacheInfo(); + info.cacheName = cacheName; + info.entries = StreamSupport.stream(cache.spliterator(), false).count(); + info.cacheStatistics = CacheHelper.getCacheStats(cacheManager , cacheName); + caches.add(info); + } + return caches; + } + @GET + @Path("/clear") + @Produces(MediaType.APPLICATION_JSON) + public ClearCacheResult clearAll() { + ClearCacheResult result = new ClearCacheResult(); + + for (String cacheName : cacheManager.getCacheNames()) { + Cache cache = cacheManager.getCache(cacheName); + CacheInfo info = new CacheInfo(); + info.cacheName = cacheName; + info.entries = StreamSupport.stream(cache.spliterator(), false).count(); + result.clearedCaches.add(info); + cache.clear(); + } + return result; + } + +} diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionService.java b/src/main/java/org/ohdsi/webapi/security/PermissionService.java index cc510579dd..2d6aa35c50 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionService.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionService.java @@ -62,6 +62,9 @@ public class PermissionService { @Value("#{!'${security.provider}'.equals('DisabledSecurity')}") private boolean securityEnabled; + @Value("${security.defaultGlobalReadPermissions}") + private boolean defaultGlobalReadPermissions; + private final EntityGraph PERMISSION_ENTITY_GRAPH = EntityGraphUtils.fromAttributePaths("rolePermissions", "rolePermissions.role"); public PermissionService( @@ -227,4 +230,17 @@ public void fillReadAccess(CommonEntity entity, CommonEntityDTO entityDTO) { public boolean isSecurityEnabled() { return this.securityEnabled; } + + // Use this key for cache (asset lists) that may be associated to a user or shared across users. + public String getAssetListCacheKey() { + if (this.isSecurityEnabled() && !defaultGlobalReadPermissions) + return permissionManager.getSubjectName(); + else + return "ALL_USERS"; + } + + // use this cache key when the cache is associated to a user + public String getSubjectCacheKey() { + return this.isSecurityEnabled() ? permissionManager.getSubjectName() : "ALL_USERS"; + } } diff --git a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java index 4de4e872e6..92caf34201 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java @@ -131,6 +131,8 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import javax.ws.rs.core.Response.ResponseBuilder; import static org.ohdsi.webapi.Constants.Params.COHORT_DEFINITION_ID; @@ -138,6 +140,9 @@ import static org.ohdsi.webapi.Constants.Params.SOURCE_ID; import org.ohdsi.webapi.source.SourceService; import static org.ohdsi.webapi.util.SecurityUtils.whitelist; +import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Cacheable; /** * Provides REST services for working with cohort definitions. @@ -149,6 +154,22 @@ @Component public class CohortDefinitionService extends AbstractDaoService implements HasTags { + //create cache + @Component + public static class CachingSetup implements JCacheManagerCustomizer { + + public static final String COHORT_DEFINITION_LIST_CACHE = "cohortDefinitionList"; + + @Override + public void customize(CacheManager cacheManager) { + // Evict when a cohort definition is created or updated, or permissions, or tags + cacheManager.createCache(COHORT_DEFINITION_LIST_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) List.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + } + private static final CohortExpressionQueryBuilder queryBuilder = new CohortExpressionQueryBuilder(); @Autowired @@ -205,7 +226,7 @@ public class CohortDefinitionService extends AbstractDaoService implements HasTa @Autowired private VersionService versionService; - @Value("${security.defaultGlobalReadPermissions}") + @Value("${security.defaultGlobalReadPermissions}") private boolean defaultGlobalReadPermissions; private final MarkdownRender markdownPF = new MarkdownRender(); @@ -408,6 +429,7 @@ public GenerateSqlResult generateSql(GenerateSqlRequest request) { @Path("/") @Produces(MediaType.APPLICATION_JSON) @Transactional + @Cacheable(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, key = "@permissionService.getAssetListCacheKey()") public List getCohortDefinitionList() { List definitions = cohortDefinitionRepository.list(); return definitions.stream() @@ -436,6 +458,7 @@ public List getCohortDefinitionList() { @Transactional @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) public CohortDTO createCohortDefinition(CohortDTO dto) { Date currentTime = Calendar.getInstance().getTime(); @@ -538,6 +561,7 @@ public int getCountCDefWithSameName(@PathParam("id") @DefaultValue("0") final in @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) @Transactional + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) public CohortDTO saveCohortDefinition(@PathParam("id") final int id, CohortDTO def) { Date currentTime = Calendar.getInstance().getTime(); @@ -670,6 +694,7 @@ public List getInfo(@PathParam("id") final int id) { @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/copy") @Transactional + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) public CohortDTO copy(@PathParam("id") final int id) { CohortDTO sourceDef = getCohortDefinition(id); sourceDef.setId(null); // clear the ID @@ -954,6 +979,7 @@ private Response printFrindly(String markdown, String format) { @POST @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/tag/") + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) @Transactional public void assignTag(@PathParam("id") final Integer id, final int tagId) { CohortDefinition entity = cohortDefinitionRepository.findOne(id); @@ -971,6 +997,7 @@ public void assignTag(@PathParam("id") final Integer id, final int tagId) { @DELETE @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/tag/{tagId}") + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) @Transactional public void unassignTag(@PathParam("id") final Integer id, @PathParam("tagId") final int tagId) { CohortDefinition entity = cohortDefinitionRepository.findOne(id); @@ -1106,6 +1133,7 @@ public void deleteVersion(@PathParam("id") final int id, @PathParam("version") f @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/version/{version}/createAsset") @Transactional + @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) public CohortDTO copyAssetFromVersion(@PathParam("id") final int id, @PathParam("version") final int version) { checkVersion(id, version, false); CohortVersion cohortVersion = versionService.getById(VersionType.COHORT, id, version); diff --git a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java index 2f80ef991e..9055ef172f 100644 --- a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java +++ b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java @@ -19,6 +19,8 @@ import java.util.*; import java.util.stream.Collectors; import java.util.stream.StreamSupport; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import javax.transaction.Transactional; import javax.ws.rs.*; @@ -30,6 +32,7 @@ import org.ohdsi.vocabulary.Concept; import org.ohdsi.webapi.check.CheckResult; import org.ohdsi.webapi.check.checker.conceptset.ConceptSetChecker; +import org.ohdsi.webapi.cohortdefinition.dto.CohortMetadataDTO; import org.ohdsi.webapi.conceptset.ConceptSet; import org.ohdsi.webapi.conceptset.ConceptSetExport; import org.ohdsi.webapi.conceptset.ConceptSetGenerationInfo; @@ -60,6 +63,9 @@ import org.ohdsi.webapi.versioning.service.VersionService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.cache.annotation.CacheEvict; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.stereotype.Component; @@ -74,7 +80,22 @@ @Transactional @Path("/conceptset/") public class ConceptSetService extends AbstractDaoService implements HasTags { - + //create cache + @Component + public static class CachingSetup implements JCacheManagerCustomizer { + + public static final String CONCEPT_SET_LIST_CACHE = "conceptSetList"; + + @Override + public void customize(CacheManager cacheManager) { + // Evict when a cohort definition is created or updated, or permissions, or tags + cacheManager.createCache(CONCEPT_SET_LIST_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) List.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + } + @Autowired private ConceptSetGenerationInfoRepository conceptSetGenerationInfoRepository; @@ -135,6 +156,7 @@ public ConceptSetDTO getConceptSet(@PathParam("id") final int id) { @GET @Path("/") @Produces(MediaType.APPLICATION_JSON) + @Cacheable(cacheNames = ConceptSetService.CachingSetup.CONCEPT_SET_LIST_CACHE, key = "@permissionService.getAssetListCacheKey()") public Collection getConceptSets() { return getTransactionTemplate().execute( transactionStatus -> StreamSupport.stream(getConceptSetRepository().findAll().spliterator(), false) @@ -453,6 +475,7 @@ public Response exportConceptSetToCSV(@PathParam("id") final String id) throws E @POST @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public ConceptSetDTO createConceptSet(ConceptSetDTO conceptSetDTO) { UserEntity user = userRepository.findByLogin(security.getSubject()); @@ -508,6 +531,7 @@ public List getNamesLike(String copyName) { @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @Transactional + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public ConceptSetDTO updateConceptSet(@PathParam("id") final int id, ConceptSetDTO conceptSetDTO) throws Exception { ConceptSet updated = getConceptSetRepository().findById(id); @@ -795,6 +819,7 @@ public void deleteVersion(@PathParam("id") final int id, @PathParam("version") f @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/version/{version}/createAsset") @Transactional + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public ConceptSetDTO copyAssetFromVersion(@PathParam("id") final int id, @PathParam("version") final int version) { checkVersion(id, version, false); ConceptSetVersion conceptSetVersion = versionService.getById(VersionType.CONCEPT_SET, id, version); diff --git a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java index e034fb41c2..f4523a2e12 100644 --- a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java +++ b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java @@ -11,6 +11,8 @@ import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import javax.ws.rs.Consumes; import javax.ws.rs.DefaultValue; @@ -66,6 +68,10 @@ import org.ohdsi.webapi.vocabulary.VocabularySearchService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.cache.annotation.Caching; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.jdbc.core.RowCallbackHandler; @@ -82,7 +88,33 @@ @Component public class VocabularyService extends AbstractDaoService { - private static Hashtable vocabularyInfoCache = null; + //create cache + @Component + public static class CachingSetup implements JCacheManagerCustomizer { + + public static final String CONCEPT_DETAIL_CACHE = "conceptDetail"; + public static final String CONCEPT_RELATED_CACHE = "conceptRelated"; + public static final String CONCEPT_HIERARCHY_CACHE = "conceptHierarchy"; + + @Override + public void customize(CacheManager cacheManager) { + // Evict when a cohort definition is created or updated, or permissions, or tags + cacheManager.createCache(CONCEPT_DETAIL_CACHE, new MutableConfiguration() + .setTypes(String.class, Concept.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + cacheManager.createCache(CONCEPT_RELATED_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) Collection.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + cacheManager.createCache(CONCEPT_HIERARCHY_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) Collection.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + } + + private static Hashtable vocabularyInfoCache = null; public static final String DEFAULT_SEARCH_ROWS = "20000"; @Autowired @@ -717,6 +749,7 @@ public Collection executeSearch(@PathParam("query") String query) { @GET @Path("{sourceKey}/concept/{id}") @Produces(MediaType.APPLICATION_JSON) + @Cacheable(cacheNames = CachingSetup.CONCEPT_DETAIL_CACHE, key = "#sourceKey.concat('/').concat(#id)") public Concept getConcept(@PathParam("sourceKey") final String sourceKey, @PathParam("id") final long id) { Source source = getSourceRepository().findBySourceKey(sourceKey); String sqlPath = "/resources/vocabulary/sql/getConcept.sql"; @@ -768,6 +801,7 @@ public Concept getConcept(@PathParam("id") final long id) { @GET @Path("{sourceKey}/concept/{id}/related") @Produces(MediaType.APPLICATION_JSON) + @Cacheable(cacheNames = CachingSetup.CONCEPT_RELATED_CACHE, key = "#sourceKey.concat('/').concat(#id)") public Collection getRelatedConcepts(@PathParam("sourceKey") String sourceKey, @PathParam("id") final Long id) { final Map concepts = new HashMap<>(); Source source = getSourceRepository().findBySourceKey(sourceKey); @@ -795,6 +829,7 @@ public Collection getRelatedConcepts(@PathParam("sourceKey") Str @GET @Path("{sourceKey}/concept/{id}/ancestorAndDescendant") @Produces(MediaType.APPLICATION_JSON) + @Cacheable(cacheNames = CachingSetup.CONCEPT_HIERARCHY_CACHE, key = "#sourceKey.concat('/').concat(#id)") public Collection getConceptAncestorAndDescendant(@PathParam("sourceKey") String sourceKey, @PathParam("id") final Long id) { final Map concepts = new HashMap<>(); Source source = getSourceRepository().findBySourceKey(sourceKey); @@ -1211,9 +1246,19 @@ public VocabularyInfo mapRow(final ResultSet resultSet, final int arg1) throws S return vocabularyInfoCache.get(sourceKey); } + @Caching(evict = { + @CacheEvict(value=CachingSetup.CONCEPT_DETAIL_CACHE, allEntries = true), + @CacheEvict(value=CachingSetup.CONCEPT_RELATED_CACHE, allEntries = true), + @CacheEvict(value=CachingSetup.CONCEPT_RELATED_CACHE, allEntries = true) + }) public void clearVocabularyInfoCache() { vocabularyInfoCache = null; } + + + public void clearCaches() { + + } /** * Get the descendant concepts of the selected ancestor vocabulary and diff --git a/src/main/java/org/ohdsi/webapi/service/dto/CommonEntityDTO.java b/src/main/java/org/ohdsi/webapi/service/dto/CommonEntityDTO.java index 287894aef9..ea820ecffc 100644 --- a/src/main/java/org/ohdsi/webapi/service/dto/CommonEntityDTO.java +++ b/src/main/java/org/ohdsi/webapi/service/dto/CommonEntityDTO.java @@ -2,13 +2,14 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import java.io.Serializable; import org.ohdsi.webapi.user.dto.UserDTO; import java.util.Date; import org.ohdsi.webapi.CommonDTO; @JsonInclude(JsonInclude.Include.NON_NULL) -public abstract class CommonEntityDTO implements CommonDTO { +public abstract class CommonEntityDTO implements CommonDTO, Serializable { @JsonProperty(access = JsonProperty.Access.READ_ONLY) private UserDTO createdBy; @JsonProperty(access = JsonProperty.Access.READ_ONLY) diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 43ff4ace64..2715ee04f9 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -35,11 +35,16 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import org.apache.commons.lang3.StringUtils; import org.apache.shiro.authz.Permission; import org.apache.shiro.authz.permission.WildcardPermission; import org.ohdsi.circe.helper.ResourceHelper; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Cacheable; import org.springframework.jdbc.core.JdbcTemplate; /** @@ -49,6 +54,22 @@ @Component @Transactional public class PermissionManager { + //create cache + @Component + public static class CachingSetup implements JCacheManagerCustomizer { + + public static final String AUTH_INFO_CACHE = "authorizationInfo"; + + @Override + public void customize(CacheManager cacheManager) { + // Evict when a user, role or permission is modified/deleted. + cacheManager.createCache(AUTH_INFO_CACHE, new MutableConfiguration() + .setTypes(String.class, UserSimpleAuthorizationInfo.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + } + @Value("${datasource.ohdsi.schema}") private String ohdsiSchema; @@ -74,8 +95,6 @@ public class PermissionManager { @Autowired private JdbcTemplate jdbcTemplate; - private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); - public static class PermissionsDTO { public Map> permissions = null; @@ -93,10 +112,12 @@ public RoleEntity addRole(String roleName, boolean isSystem) { return role; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, key = "#login") public String addUserToRole(String roleName, String login) { return addUserToRole(roleName, login, UserOrigin.SYSTEM); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, key = "#login") public String addUserToRole(String roleName, String login, UserOrigin userOrigin) { Guard.checkNotEmpty(roleName); Guard.checkNotEmpty(login); @@ -108,6 +129,7 @@ public String addUserToRole(String roleName, String login, UserOrigin userOrigin return userRole.getStatus(); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, key = "#login") public void removeUserFromRole(String roleName, String login, UserOrigin origin) { Guard.checkNotEmpty(roleName); Guard.checkNotEmpty(login); @@ -138,43 +160,42 @@ public Iterable getRoles(boolean includePersonalRoles) { * @param login The login to fetch the authorization info * @return A UserSimpleAuthorizationInfo containing roles and permissions. */ + @Cacheable(cacheNames = CachingSetup.AUTH_INFO_CACHE) public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) { - return authorizationInfoCache.get().computeIfAbsent(login, newLogin -> { - final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo(); + final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo(); - final UserEntity userEntity = userRepository.findByLogin(newLogin); - if(userEntity == null) { - throw new UnknownAccountException("Account does not exist"); - } + final UserEntity userEntity = userRepository.findByLogin(login); + if(userEntity == null) { + throw new UnknownAccountException("Account does not exist"); + } - info.setUserId(userEntity.getId()); - info.setLogin(userEntity.getLogin()); + info.setUserId(userEntity.getId()); + info.setLogin(userEntity.getLogin()); - for (UserRoleEntity userRole: userEntity.getUserRoles()) { - info.addRole(userRole.getRole().getName()); - } + for (UserRoleEntity userRole: userEntity.getUserRoles()) { + info.addRole(userRole.getRole().getName()); + } - // convert permission index from queryUserPermissions() into a map of WildcardPermissions - Map> permsIdx = this.queryUserPermissions(newLogin).permissions; - Map permissionMap = new HashMap>(); - Set permissionNames = new HashSet<>(); - - for(String permIdxKey : permsIdx.keySet()) { - List perms = permsIdx.get(permIdxKey); - permissionNames.addAll(perms); - // convert raw string permission into Wildcard perm for each element in this key's array. - permissionMap.put(permIdxKey, perms.stream().map(perm -> new WildcardPermission(perm)).collect(Collectors.toList())); - } + // convert permission index from queryUserPermissions() into a map of WildcardPermissions + Map> permsIdx = this.queryUserPermissions(login).permissions; + Map permissionMap = new HashMap>(); + Set permissionNames = new HashSet<>(); + + for(String permIdxKey : permsIdx.keySet()) { + List perms = permsIdx.get(permIdxKey); + permissionNames.addAll(perms); + // convert raw string permission into Wildcard perm for each element in this key's array. + permissionMap.put(permIdxKey, perms.stream().map(perm -> new WildcardPermission(perm)).collect(Collectors.toList())); + } - info.setStringPermissions(permissionNames); - info.setPermissionIdx(permissionMap); - return info; - }); + info.setStringPermissions(permissionNames); + info.setPermissionIdx(permissionMap); + return info; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void clearAuthorizationInfoCache() { - this.authorizationInfoCache.set(new ConcurrentHashMap<>()); } @Transactional @@ -261,6 +282,7 @@ public Set getUserPermissions(Long userId) { return permissions; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void removeRole(Long roleId) { eventPublisher.publishEvent(new DeleteRoleEvent(this, roleId)); this.roleRepository.delete(roleId); @@ -283,6 +305,7 @@ public void addPermission(RoleEntity role, PermissionEntity permission) { this.addPermission(role, permission, null); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void removePermission(Long permissionId, Long roleId) { RolePermissionEntity rolePermission = this.rolePermissionRepository.findByRoleIdAndPermissionId(roleId, permissionId); if (rolePermission != null) @@ -308,6 +331,7 @@ public void removeUser(Long userId, Long roleId) { this.userRoleRepository.delete(userRole); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void removePermission(String value) { PermissionEntity permission = this.permissionRepository.findByValueIgnoreCase(value); if (permission != null) @@ -473,6 +497,7 @@ private PermissionEntity getPermissionById(Long permissionId) { return permission; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) private RolePermissionEntity addPermission(final RoleEntity role, final PermissionEntity permission, final String status) { RolePermissionEntity relation = this.rolePermissionRepository.findByRoleAndPermission(role, permission); if (relation == null) { diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java deleted file mode 100644 index d76aa8a921..0000000000 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java +++ /dev/null @@ -1,38 +0,0 @@ -package org.ohdsi.webapi.shiro.filters; - -import org.ohdsi.webapi.security.PermissionService; -import org.ohdsi.webapi.shiro.PermissionManager; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Component; - -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import java.io.IOException; - -@Component -public class CacheFilter implements Filter { - - @Autowired - private PermissionManager permissionManager; - - @Override - public void init(FilterConfig filterConfig) throws ServletException { - - } - - @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - - permissionManager.clearAuthorizationInfoCache(); - chain.doFilter(request, response); - } - - @Override - public void destroy() { - - } -} diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java index 47e086a572..b6c95b3758 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java @@ -99,6 +99,7 @@ protected FilterChainBuilder setupProtectedPaths(FilterChainBuilder filterChainB return filterChainBuilder // version info .addRestPath("/info") + .addRestPath("/cache/*") // move this to protected path before PR // DDL service .addRestPath("/ddl/results") .addRestPath("/ddl/cemresults") diff --git a/src/main/java/org/ohdsi/webapi/user/dto/UserDTO.java b/src/main/java/org/ohdsi/webapi/user/dto/UserDTO.java index 4ecce3ea78..eae323ac87 100644 --- a/src/main/java/org/ohdsi/webapi/user/dto/UserDTO.java +++ b/src/main/java/org/ohdsi/webapi/user/dto/UserDTO.java @@ -1,6 +1,8 @@ package org.ohdsi.webapi.user.dto; -public class UserDTO { +import java.io.Serializable; + +public class UserDTO implements Serializable { private Long id; private String name; diff --git a/src/main/java/org/ohdsi/webapi/util/CacheHelper.java b/src/main/java/org/ohdsi/webapi/util/CacheHelper.java new file mode 100644 index 0000000000..581dbe14a3 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/util/CacheHelper.java @@ -0,0 +1,36 @@ +package org.ohdsi.webapi.util; + +import java.lang.management.ManagementFactory; +import java.util.Set; +import javax.cache.CacheManager; +import javax.cache.management.CacheStatisticsMXBean; +import javax.management.MBeanServer; +import javax.management.MBeanServerInvocationHandler; +import javax.management.ObjectInstance; +import javax.management.ObjectName; + +/** + * + * @author cknoll1 + */ +public class CacheHelper { + public static CacheStatisticsMXBean getCacheStats(CacheManager cacheManager, String cacheName) throws RuntimeException { + try { + final MBeanServer beanServer = ManagementFactory.getPlatformMBeanServer(); + + final Set cacheBeans = beanServer.queryMBeans( + ObjectName.getInstance("javax.cache:type=CacheStatistics,CacheManager=*,Cache=*"), + null); + String cacheManagerName = cacheManager.getURI().toString().replace(":", "."); + ObjectInstance cacheBean = cacheBeans.stream() + .filter(b -> + b.getObjectName().getKeyProperty("CacheManager").equals(cacheManagerName) + && b.getObjectName().getKeyProperty("Cache").equals(cacheName) + ).findFirst().orElseThrow(() -> new IllegalArgumentException(String.format("No cache found for cache manager = %s, cache = %s", cacheManagerName, cacheName))); + final CacheStatisticsMXBean cacheStatisticsMXBean = MBeanServerInvocationHandler.newProxyInstance(beanServer, cacheBean.getObjectName(), CacheStatisticsMXBean.class, false); + return cacheStatisticsMXBean; + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/src/main/resources/appCache.xml b/src/main/resources/appCache.xml new file mode 100644 index 0000000000..d75377403e --- /dev/null +++ b/src/main/resources/appCache.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + 10 + + + + + 100 + + + + + 100 + 10 + + + + \ No newline at end of file diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index cd1afb2013..beaafd1e08 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -64,6 +64,10 @@ spring.jpa.properties.hibernate.generate_statistics=${spring.jpa.properties.hibe spring.jpa.properties.hibernate.jdbc.batch_size=${spring.jpa.properties.hibernate.jdbc.batch_size} spring.jpa.properties.hibernate.order_inserts=${spring.jpa.properties.hibernate.order_inserts} +#Spring Cache +spring.cache.jcache.config=classpath:appCache.xml +spring.cache.type=${spring.cache.type} + #JAX-RS jersey.resources.root.package=org.ohdsi.webapi diff --git a/src/test/java/org/ohdsi/webapi/service/CohortDefinitionServiceTest.java b/src/test/java/org/ohdsi/webapi/service/CohortDefinitionServiceTest.java new file mode 100644 index 0000000000..294fff7106 --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/service/CohortDefinitionServiceTest.java @@ -0,0 +1,115 @@ +package org.ohdsi.webapi.service; + +import java.util.Arrays; +import java.util.List; +import javax.cache.Cache; +import javax.cache.CacheManager; +import javax.cache.management.CacheStatisticsMXBean; +import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.web.mgt.DefaultWebSecurityManager; +import static org.assertj.core.api.Assertions.assertThat; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.ohdsi.webapi.AbstractDatabaseTest; +import org.ohdsi.webapi.cohortdefinition.CohortDefinitionRepository; +import org.ohdsi.webapi.cohortdefinition.dto.CohortDTO; +import org.ohdsi.webapi.cohortdefinition.dto.CohortMetadataDTO; +import org.springframework.beans.factory.annotation.Autowired; + +import org.ohdsi.webapi.util.CacheHelper; + +public class CohortDefinitionServiceTest extends AbstractDatabaseTest { + + @Autowired + private CohortDefinitionService cdService; + @Autowired + protected CohortDefinitionRepository cdRepository; + @Autowired(required = false) + private CacheManager cacheManager ; + @Autowired + private DefaultWebSecurityManager securityManager; + + // in JUnit 4 it's impossible to mark methods inside interface with annotations, it was implemented in JUnit 5. After upgrade it's needed + // to mark interface methods with @Test, @Before, @After and to remove them from this class + @After + public void tearDownDB() { + cdRepository.deleteAll(); + } + + @Before + public void setup() { + // Set the SecurityManager for the current thread + SimplePrincipalCollection principalCollection = new SimplePrincipalCollection(); + principalCollection.addAll(Arrays.asList("permsTest"), "testRealm"); + Subject subject = new Subject.Builder(securityManager) + .authenticated(true) + .principals(principalCollection) + .buildSubject(); + ThreadContext.bind(subject); + } + + private CohortDTO createEntity(String name) { + CohortDTO dto = new CohortDTO(); + dto.setName(name); + return cdService.createCohortDefinition(dto); + } + + @Test + public void cohortDefinitionListCacheTest() throws Exception { + + if (cacheManager == null) return; // cache is disabled, so nothing to test + + CacheStatisticsMXBean cacheStatistics = CacheHelper.getCacheStats(cacheManager , CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE); + Cache cohortListCache = cacheManager.getCache(CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE); + + // reset the cache and statistics for this test + cacheStatistics.clear(); + cohortListCache.clear(); + int cacheHits = 0; + int cacheMisses = 0; + List cohortDefList; + cohortDefList = cdService.getCohortDefinitionList(); + cacheMisses++; + assertThat(cacheStatistics.getCacheMisses()).isEqualTo(cacheMisses); + assertThat(cacheStatistics.getCacheHits()).isEqualTo(cacheHits); + + cohortDefList = cdService.getCohortDefinitionList(); + cacheHits++; + assertThat(cacheStatistics.getCacheMisses()).isEqualTo(cacheMisses); + assertThat(cacheStatistics.getCacheHits()).isEqualTo(cacheHits); + + } + + @Test + public void cohortDefinitionListEvictTest() throws Exception { + + if (cacheManager == null) return; // cache is disabled, so nothing to test + + CacheStatisticsMXBean cacheStatistics = CacheHelper.getCacheStats(cacheManager , CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE); + Cache cohortListCache = cacheManager.getCache(CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE); + + // reset the cache and statistics for this test + cacheStatistics.clear(); + cohortListCache.clear(); + int cacheHits = 0; + int cacheMisses = 0; + List cohortDefList; + cohortDefList = cdService.getCohortDefinitionList(); + cacheMisses++; + assertThat(cacheStatistics.getCacheMisses()).isEqualTo(cacheMisses); + assertThat(cacheStatistics.getCacheHits()).isEqualTo(cacheHits); + CohortDTO c = createEntity("Cohort 1"); + cohortDefList = cdService.getCohortDefinitionList(); + cacheMisses++; + assertThat(cacheStatistics.getCacheMisses()).isEqualTo(cacheMisses); + assertThat(cacheStatistics.getCacheHits()).isEqualTo(cacheHits); + c = cdService.saveCohortDefinition(c.getId(), c); + cohortDefList = cdService.getCohortDefinitionList(); + cacheMisses++; + assertThat(cacheStatistics.getCacheMisses()).isEqualTo(cacheMisses); + assertThat(cacheStatistics.getCacheHits()).isEqualTo(cacheHits); + } +}