From a2397060d32e8e081c9eade8caf2a082f388ad60 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Thu, 12 Sep 2024 16:47:13 -0400 Subject: [PATCH 1/6] 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); + } +} From 37aaef83f45311b7dd9b93e947e7753c018b4d9c Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Fri, 13 Sep 2024 14:06:40 -0400 Subject: [PATCH 2/6] Evict cache when role permissions are updated. Increased size of small-heap configuration. --- .../java/org/ohdsi/webapi/shiro/PermissionManager.java | 7 +++++++ src/main/resources/appCache.xml | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 2715ee04f9..13256af83d 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -294,6 +294,7 @@ public Set getRolePermissions(Long roleId) { return permissions; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void addPermission(Long roleId, Long permissionId) { PermissionEntity permission = this.getPermissionById(permissionId); RoleEntity role = this.getRoleById(roleId); @@ -301,6 +302,7 @@ public void addPermission(Long roleId, Long permissionId) { this.addPermission(role, permission, null); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void addPermission(RoleEntity role, PermissionEntity permission) { this.addPermission(role, permission, null); } @@ -318,6 +320,7 @@ public Set getRoleUsers(Long roleId) { return users; } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void addUser(Long userId, Long roleId) { UserEntity user = this.getUserById(userId); RoleEntity role = this.getRoleById(roleId); @@ -325,6 +328,7 @@ public void addUser(Long userId, Long roleId) { this.addUser(user, role, UserOrigin.SYSTEM, null); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void removeUser(Long userId, Long roleId) { UserRoleEntity userRole = this.userRoleRepository.findByUserIdAndRoleId(userId, roleId); if (userRole != null) @@ -553,6 +557,7 @@ public RoleEntity updateRole(RoleEntity roleEntity) { return this.roleRepository.save(roleEntity); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void addPermissionsFromTemplate(RoleEntity roleEntity, Map template, String value) { for (Map.Entry entry : template.entrySet()) { String permission = String.format(entry.getKey(), value); @@ -562,11 +567,13 @@ public void addPermissionsFromTemplate(RoleEntity roleEntity, Map template, String value) { RoleEntity currentUserPersonalRole = getCurrentUserPersonalRole(); addPermissionsFromTemplate(currentUserPersonalRole, template, value); } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void removePermissionsFromTemplate(Map template, String value) { for (Map.Entry entry : template.entrySet()) { String permission = String.format(entry.getKey(), value); diff --git a/src/main/resources/appCache.xml b/src/main/resources/appCache.xml index d75377403e..fa20a83325 100644 --- a/src/main/resources/appCache.xml +++ b/src/main/resources/appCache.xml @@ -15,17 +15,17 @@ - 10 + 50 - 100 + 500 - 100 + 500 10 From 0a77f67785c0917a009116d68139a5e5f9023dbb Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Fri, 27 Sep 2024 11:46:46 -0400 Subject: [PATCH 3/6] Added code to handle test context reloads. Fixed permission tests. --- .../org/ohdsi/webapi/cache/CacheService.java | 6 +- .../service/CohortDefinitionService.java | 10 ++- .../webapi/service/ConceptSetService.java | 16 ++-- .../webapi/service/VocabularyService.java | 34 +++++--- .../ohdsi/webapi/shiro/PermissionManager.java | 80 +++++++++++-------- .../org/ohdsi/webapi/util/CacheHelper.java | 8 ++ .../ohdsi/webapi/security/PermissionTest.java | 4 +- .../resources/permission/permsTest_PREP.json | 42 +++++----- .../permission/wildcardTest_PREP.json | 18 ++--- 9 files changed, 128 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/cache/CacheService.java b/src/main/java/org/ohdsi/webapi/cache/CacheService.java index f58107725a..86875fbd51 100644 --- a/src/main/java/org/ohdsi/webapi/cache/CacheService.java +++ b/src/main/java/org/ohdsi/webapi/cache/CacheService.java @@ -17,7 +17,6 @@ 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; @@ -54,6 +53,10 @@ public CacheService(CacheManager cacheManager) { this.cacheManager = cacheManager; } + public CacheService() { + } + + @GET @Path("/") @Produces(MediaType.APPLICATION_JSON) @@ -88,5 +91,4 @@ public ClearCacheResult clearAll() { } return result; } - } diff --git a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java index 92caf34201..8b47dd1171 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java @@ -163,10 +163,12 @@ public static class CachingSetup implements JCacheManagerCustomizer { @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)); + if (!CacheHelper.getCacheNames(cacheManager).contains(COHORT_DEFINITION_LIST_CACHE)) { + cacheManager.createCache(COHORT_DEFINITION_LIST_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) List.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } } } diff --git a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java index 9055ef172f..4381cdbbd3 100644 --- a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java +++ b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java @@ -41,6 +41,7 @@ import org.ohdsi.webapi.conceptset.dto.ConceptSetVersionFullDTO; import org.ohdsi.webapi.exception.ConceptNotExistException; import org.ohdsi.webapi.security.PermissionService; +import static org.ohdsi.webapi.service.CohortDefinitionService.CachingSetup.COHORT_DEFINITION_LIST_CACHE; import org.ohdsi.webapi.service.dto.ConceptSetDTO; import org.ohdsi.webapi.shiro.Entities.UserEntity; import org.ohdsi.webapi.shiro.Entities.UserRepository; @@ -51,6 +52,7 @@ import org.ohdsi.webapi.source.SourceService; import org.ohdsi.webapi.tag.domain.HasTags; import org.ohdsi.webapi.tag.dto.TagNameListRequestDTO; +import org.ohdsi.webapi.util.CacheHelper; import org.ohdsi.webapi.util.ExportUtil; import org.ohdsi.webapi.util.NameUtils; import org.ohdsi.webapi.util.ExceptionUtils; @@ -88,14 +90,18 @@ public static class CachingSetup implements JCacheManagerCustomizer { @Override public void customize(CacheManager cacheManager) { + // due to unit tests causing application contexts to reload cache manager caches, we + // have to check for the existance of a cache before creating it + Set cacheNames = CacheHelper.getCacheNames(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)); + if (!cacheNames.contains(CONCEPT_SET_LIST_CACHE)) { + cacheManager.createCache(CONCEPT_SET_LIST_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) List.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } } } - @Autowired private ConceptSetGenerationInfoRepository conceptSetGenerationInfoRepository; diff --git a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java index f4523a2e12..7a418de59a 100644 --- a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java +++ b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java @@ -53,6 +53,7 @@ import org.ohdsi.webapi.source.SourceService; import org.ohdsi.webapi.source.SourceDaimon; import org.ohdsi.webapi.source.SourceInfo; +import org.ohdsi.webapi.util.CacheHelper; import org.ohdsi.webapi.util.PreparedSqlRender; import org.ohdsi.webapi.util.PreparedStatementRenderer; import org.ohdsi.webapi.vocabulary.ConceptRecommendedNotInstalledException; @@ -98,19 +99,28 @@ public static class CachingSetup implements JCacheManagerCustomizer { @Override public void customize(CacheManager cacheManager) { + // due to unit tests causing application contexts to reload cache manager caches, we + // have to check for the existance of a cache before creating it + Set cacheNames = CacheHelper.getCacheNames(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)); + if (!cacheNames.contains(CONCEPT_DETAIL_CACHE)) { + cacheManager.createCache(CONCEPT_DETAIL_CACHE, new MutableConfiguration() + .setTypes(String.class, Concept.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + if (!cacheNames.contains(CONCEPT_RELATED_CACHE)) { + cacheManager.createCache(CONCEPT_RELATED_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) Collection.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + if (!cacheNames.contains(CONCEPT_HIERARCHY_CACHE)) { + cacheManager.createCache(CONCEPT_HIERARCHY_CACHE, new MutableConfiguration>() + .setTypes(String.class, (Class>) (Class) Collection.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } } } diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 13256af83d..10c345c594 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -41,6 +41,7 @@ import org.apache.shiro.authz.Permission; import org.apache.shiro.authz.permission.WildcardPermission; import org.ohdsi.circe.helper.ResourceHelper; +import org.ohdsi.webapi.util.CacheHelper; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; import org.springframework.cache.annotation.CacheEvict; @@ -62,14 +63,18 @@ public static class CachingSetup implements JCacheManagerCustomizer { @Override public void customize(CacheManager cacheManager) { + // due to unit tests causing application contexts to reload cache manager caches, we + // have to check for the existance of a cache before creating it + Set cacheNames = CacheHelper.getCacheNames(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)); + if (!cacheNames.contains(AUTH_INFO_CACHE)) { + cacheManager.createCache(AUTH_INFO_CACHE, new MutableConfiguration() + .setTypes(String.class, UserSimpleAuthorizationInfo.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } } } - @Value("${datasource.ohdsi.schema}") private String ohdsiSchema; @@ -95,6 +100,8 @@ public void customize(CacheManager cacheManager) { @Autowired private JdbcTemplate jdbcTemplate; + private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); + public static class PermissionsDTO { public Map> permissions = null; @@ -163,39 +170,42 @@ public Iterable getRoles(boolean includePersonalRoles) { @Cacheable(cacheNames = CachingSetup.AUTH_INFO_CACHE) public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) { - final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo(); - - final UserEntity userEntity = userRepository.findByLogin(login); - if(userEntity == null) { - throw new UnknownAccountException("Account does not exist"); - } - - info.setUserId(userEntity.getId()); - info.setLogin(userEntity.getLogin()); - - for (UserRoleEntity userRole: userEntity.getUserRoles()) { - info.addRole(userRole.getRole().getName()); - } - - // 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; - } + return authorizationInfoCache.get().computeIfAbsent(login, newLogin -> { + final UserSimpleAuthorizationInfo info = new UserSimpleAuthorizationInfo(); + + final UserEntity userEntity = userRepository.findByLogin(login); + if(userEntity == null) { + throw new UnknownAccountException("Account does not exist"); + } + + info.setUserId(userEntity.getId()); + info.setLogin(userEntity.getLogin()); + + for (UserRoleEntity userRole: userEntity.getUserRoles()) { + info.addRole(userRole.getRole().getName()); + } + + // 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; + }); + } @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void clearAuthorizationInfoCache() { + authorizationInfoCache.remove(); } @Transactional diff --git a/src/main/java/org/ohdsi/webapi/util/CacheHelper.java b/src/main/java/org/ohdsi/webapi/util/CacheHelper.java index 581dbe14a3..6c258f1492 100644 --- a/src/main/java/org/ohdsi/webapi/util/CacheHelper.java +++ b/src/main/java/org/ohdsi/webapi/util/CacheHelper.java @@ -1,8 +1,10 @@ package org.ohdsi.webapi.util; import java.lang.management.ManagementFactory; +import java.util.HashSet; import java.util.Set; import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import javax.cache.management.CacheStatisticsMXBean; import javax.management.MBeanServer; import javax.management.MBeanServerInvocationHandler; @@ -33,4 +35,10 @@ public static CacheStatisticsMXBean getCacheStats(CacheManager cacheManager, Str throw new RuntimeException(e); } } + + public static Set getCacheNames(CacheManager cacheManager) { + Set cacheNames = new HashSet<>(); + cacheManager.getCacheNames().forEach((name) -> cacheNames.add(name)); + return cacheNames; + } } diff --git a/src/test/java/org/ohdsi/webapi/security/PermissionTest.java b/src/test/java/org/ohdsi/webapi/security/PermissionTest.java index 1aedd9a44c..dddcbe2bfd 100644 --- a/src/test/java/org/ohdsi/webapi/security/PermissionTest.java +++ b/src/test/java/org/ohdsi/webapi/security/PermissionTest.java @@ -65,7 +65,7 @@ public void setup() { ThreadContext.bind(subject); } - @Ignore +// @Ignore @Test public void permsTest() throws Exception { // need to clear authorization cache before each test @@ -88,7 +88,7 @@ public void permsTest() throws Exception { } - @Ignore +// @Ignore @Test public void wildcardTest() throws Exception { // need to clear authorization cache before each test diff --git a/src/test/resources/permission/permsTest_PREP.json b/src/test/resources/permission/permsTest_PREP.json index 8ad261396e..dd5ce28715 100644 --- a/src/test/resources/permission/permsTest_PREP.json +++ b/src/test/resources/permission/permsTest_PREP.json @@ -1,7 +1,7 @@ { "public.sec_user": [ { - "id":1001, + "id":100001, "login":"permsTest", "name":"Perms Test User", "origin":"SYSTEM" @@ -9,56 +9,56 @@ ], "public.sec_role": [ { - "id": 1001, + "id": 100001, "name":"permsTest", "system_role":false } ], "public.sec_permission" : [ { - "id": 1001, + "id": 100001, "value":"printer:manage:printer1" }, { - "id": 1002, + "id": 100002, "value":"printer:manage:printer2" }, { - "id": 1003, + "id": 100003, "value":"printer:query:*" }, { - "id": 1004, + "id": 100004, "value":"printer:print" } ], "public.sec_role_permission" : [ { - "id":1001, - "role_id":1001, - "permission_id":1001 + "id":100001, + "role_id":100001, + "permission_id":100001 }, { - "id":1002, - "role_id":1001, - "permission_id":1002 + "id":100002, + "role_id":100001, + "permission_id":100002 }, { - "id":1003, - "role_id":1001, - "permission_id":1003 + "id":103, + "role_id":100001, + "permission_id":100003 }, { - "id":1004, - "role_id":1001, - "permission_id":1004 + "id":100004, + "role_id":100001, + "permission_id":100004 } ], "public.sec_user_role": [ { - "id": 1001, - "user_id":1001, - "role_id":1001, + "id": 100001, + "user_id":100001, + "role_id":100001, "origin":"SYSTEM" } ] diff --git a/src/test/resources/permission/wildcardTest_PREP.json b/src/test/resources/permission/wildcardTest_PREP.json index 6f08990e8f..716ccbb4d9 100644 --- a/src/test/resources/permission/wildcardTest_PREP.json +++ b/src/test/resources/permission/wildcardTest_PREP.json @@ -1,7 +1,7 @@ { "public.sec_user": [ { - "id":1001, + "id":100001, "login":"permsTest", "name":"Perms Test User", "origin":"SYSTEM" @@ -9,29 +9,29 @@ ], "public.sec_role": [ { - "id": 1001, + "id": 100001, "name":"permsTest", "system_role":false } ], "public.sec_permission" : [ { - "id": 1001, + "id": 100001, "value":"*" } ], "public.sec_role_permission" : [ { - "id":1001, - "role_id":1001, - "permission_id":1001 + "id":100001, + "role_id":100001, + "permission_id":100001 } ], "public.sec_user_role": [ { - "id": 1001, - "user_id":1001, - "role_id":1001, + "id": 100001, + "user_id":100001, + "role_id":100001, "origin":"SYSTEM" } ] From 5d62dee953bbf10b7acdc876c4dc4c705f2225f0 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Tue, 8 Oct 2024 13:59:46 -0400 Subject: [PATCH 4/6] Fixed cache-evict for concept set delete. Changed asset cache key to use login always due to read/write permissions being assigned at a per-user basis. --- .../org/ohdsi/webapi/service/CohortDefinitionService.java | 2 +- .../java/org/ohdsi/webapi/service/ConceptSetService.java | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java index 8b47dd1171..867635d64c 100644 --- a/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java +++ b/src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java @@ -431,7 +431,7 @@ public GenerateSqlResult generateSql(GenerateSqlRequest request) { @Path("/") @Produces(MediaType.APPLICATION_JSON) @Transactional - @Cacheable(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, key = "@permissionService.getAssetListCacheKey()") + @Cacheable(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, key = "@permissionService.getSubjectCacheKey()") public List getCohortDefinitionList() { List definitions = cohortDefinitionRepository.list(); return definitions.stream() diff --git a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java index 4381cdbbd3..d60992fd57 100644 --- a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java +++ b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java @@ -162,7 +162,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()") + @Cacheable(cacheNames = ConceptSetService.CachingSetup.CONCEPT_SET_LIST_CACHE, key = "@permissionService.getSubjectCacheKey()") public Collection getConceptSets() { return getTransactionTemplate().execute( transactionStatus -> StreamSupport.stream(getConceptSetRepository().findAll().spliterator(), false) @@ -481,7 +481,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) + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public ConceptSetDTO createConceptSet(ConceptSetDTO conceptSetDTO) { UserEntity user = userRepository.findByLogin(security.getSubject()); @@ -609,6 +609,7 @@ public Collection getConceptSetGenerationInfo(@PathPar @DELETE @Transactional(rollbackOn = Exception.class, dontRollbackOn = EmptyResultDataAccessException.class) @Path("{id}") + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public void deleteConceptSet(@PathParam("id") final int id) { // Remove any generation info try { @@ -656,6 +657,7 @@ public void deleteConceptSet(@PathParam("id") final int id) { @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/tag/") @Transactional + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) public void assignTag(@PathParam("id") final Integer id, final int tagId) { ConceptSet entity = getConceptSetRepository().findById(id); checkOwnerOrAdminOrGranted(entity); From ab807439b845042688bf5c74ffd57e6ca6ecc6bf Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Fri, 11 Oct 2024 11:40:50 -0400 Subject: [PATCH 5/6] Restore thread-local auth cache so that it still applies some caching (old style) when caching is disabled. --- .../ohdsi/webapi/shiro/PermissionManager.java | 2 +- .../webapi/shiro/filters/CacheFilter.java | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 10c345c594..7e30062ae6 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -205,7 +205,7 @@ public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) { @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void clearAuthorizationInfoCache() { - authorizationInfoCache.remove(); + authorizationInfoCache.set(new ConcurrentHashMap<>()); } @Transactional diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java new file mode 100644 index 0000000000..d76aa8a921 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java @@ -0,0 +1,38 @@ +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() { + + } +} From 230dcda95819d71788a244bfd4e5d1a677ca10e9 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Fri, 6 Dec 2024 12:39:01 -0500 Subject: [PATCH 6/6] added source list caching. Changed editor config to tabs. Made cache endpoints auth restricted. --- .editorconfig | 2 +- .../shiro/management/AtlasSecurity.java | 1 - .../ohdsi/webapi/source/SourceController.java | 6 ++- .../ohdsi/webapi/source/SourceService.java | 46 +++++++++++++------ src/main/resources/appCache.xml | 1 + ...0241203000001__webapi_cache_permission.sql | 14 ++++++ 6 files changed, 53 insertions(+), 17 deletions(-) create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20241203000001__webapi_cache_permission.sql diff --git a/.editorconfig b/.editorconfig index 88bf97c508..eaad2c21c8 100644 --- a/.editorconfig +++ b/.editorconfig @@ -4,7 +4,7 @@ root = true [*] -indent_style = space +indent_style = tab indent_size = 2 end_of_line = crlf charset = utf-8 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 b6c95b3758..47e086a572 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java @@ -99,7 +99,6 @@ 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/source/SourceController.java b/src/main/java/org/ohdsi/webapi/source/SourceController.java index ee32d2f4de..3ebb2450c6 100644 --- a/src/main/java/org/ohdsi/webapi/source/SourceController.java +++ b/src/main/java/org/ohdsi/webapi/source/SourceController.java @@ -28,8 +28,8 @@ import java.io.IOException; import java.io.InputStream; import java.util.*; -import java.util.function.Predicate; import java.util.stream.Collectors; +import org.springframework.cache.annotation.CacheEvict; @Path("/source/") @Component @@ -157,6 +157,7 @@ public SourceDetails getSourceDetails(@PathParam("sourceId") Integer sourceId) { @POST @Consumes(MediaType.MULTIPART_FORM_DATA) @Produces(MediaType.APPLICATION_JSON) + @CacheEvict(cacheNames = SourceService.CachingSetup.SOURCE_LIST_CACHE, allEntries = true) public SourceInfo createSource(@FormDataParam("keyfile") InputStream file, @FormDataParam("keyfile") FormDataContentDisposition fileDetail, @FormDataParam("source") SourceRequest request) throws Exception { if (!securityEnabled) { throw new NotAuthorizedException(SECURE_MODE_ERROR); @@ -219,6 +220,7 @@ public SourceInfo createSource(@FormDataParam("keyfile") InputStream file, @Form @Consumes(MediaType.MULTIPART_FORM_DATA) @Produces(MediaType.APPLICATION_JSON) @Transactional + @CacheEvict(cacheNames = SourceService.CachingSetup.SOURCE_LIST_CACHE, allEntries = true) public SourceInfo updateSource(@PathParam("sourceId") Integer sourceId, @FormDataParam("keyfile") InputStream file, @FormDataParam("keyfile") FormDataContentDisposition fileDetail, @FormDataParam("source") SourceRequest request) throws IOException { if (!securityEnabled) { throw new NotAuthorizedException(SECURE_MODE_ERROR); @@ -318,6 +320,7 @@ private void setKeyfileData(Source updated, Source source, InputStream file) thr @Path("{sourceId}") @DELETE @Transactional + @CacheEvict(cacheNames = SourceService.CachingSetup.SOURCE_LIST_CACHE, allEntries = true) public Response delete(@PathParam("sourceId") Integer sourceId) throws Exception { if (!securityEnabled){ return getInsecureModeResponse(); @@ -384,6 +387,7 @@ public Map getPriorityDaimons() { @Path("{sourceKey}/daimons/{daimonType}/set-priority") @POST @Produces(MediaType.APPLICATION_JSON) + @CacheEvict(cacheNames = SourceService.CachingSetup.SOURCE_LIST_CACHE, allEntries = true) public Response updateSourcePriority( @PathParam("sourceKey") final String sourceKey, @PathParam("daimonType") final String daimonTypeName diff --git a/src/main/java/org/ohdsi/webapi/source/SourceService.java b/src/main/java/org/ohdsi/webapi/source/SourceService.java index 65551781a2..77a749977b 100644 --- a/src/main/java/org/ohdsi/webapi/source/SourceService.java +++ b/src/main/java/org/ohdsi/webapi/source/SourceService.java @@ -17,12 +17,33 @@ import javax.annotation.PostConstruct; import java.util.*; import java.util.stream.Collectors; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; +import org.ohdsi.webapi.util.CacheHelper; +import org.springframework.boot.autoconfigure.cache.JCacheManagerCustomizer; +import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.stereotype.Component; @Service public class SourceService extends AbstractDaoService { - private static Collection cachedSources = null; - + @Component + public static class CachingSetup implements JCacheManagerCustomizer { + + public static final String SOURCE_LIST_CACHE = "sourceList"; + + @Override + public void customize(CacheManager cacheManager) { + // Evict when a cohort definition is created or updated, or permissions, or tags + if (!CacheHelper.getCacheNames(cacheManager).contains(SOURCE_LIST_CACHE)) { + cacheManager.createCache(SOURCE_LIST_CACHE, new MutableConfiguration>() + .setTypes(Object.class, (Class>) (Class) List.class) + .setStoreByValue(false) + .setStatisticsEnabled(true)); + } + } + } @Value("${jasypt.encryptor.enabled}") private boolean encryptorEnabled; @@ -77,15 +98,13 @@ protected void doInTransactionWithoutResult(TransactionStatus transactionStatus) } } - public Collection getSources() { + @Cacheable(cacheNames = CachingSetup.SOURCE_LIST_CACHE) + public Collection getSources() { - if (cachedSources == null) { - List sources = sourceRepository.findAll(); - Collections.sort(sources, new SortByKey()); - cachedSources = sources; - } - return cachedSources; - } + List sources = sourceRepository.findAll(); + Collections.sort(sources, new SortByKey()); + return sources; + } public Source findBySourceKey(final String sourceKey) { @@ -160,10 +179,9 @@ public SourceInfo getPriorityVocabularySourceInfo() { return new SourceInfo(source); } - public void invalidateCache() { - - this.cachedSources = null; - } + @CacheEvict(cacheNames = CachingSetup.SOURCE_LIST_CACHE, allEntries = true) + public void invalidateCache() { + } private boolean checkConnectionSafe(Source source) { diff --git a/src/main/resources/appCache.xml b/src/main/resources/appCache.xml index fa20a83325..ebc1bb19e0 100644 --- a/src/main/resources/appCache.xml +++ b/src/main/resources/appCache.xml @@ -11,6 +11,7 @@ + diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20241203000001__webapi_cache_permission.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20241203000001__webapi_cache_permission.sql new file mode 100644 index 0000000000..9f3cbb27c1 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20241203000001__webapi_cache_permission.sql @@ -0,0 +1,14 @@ +INSERT INTO ${ohdsiSchema}.sec_permission (id, value, description) +SELECT nextval('${ohdsiSchema}.sec_permission_id_seq'), + 'cache:get', + 'fetch WebAPI cache information'; + +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +SELECT sr.id, sp.id +FROM ${ohdsiSchema}.sec_permission sp, + ${ohdsiSchema}.sec_role sr +WHERE sp.value in + ( + 'cache:get' + ) + AND sr.name IN ('admin');