From b9feca71edfaae43e3e85cba23267b4095c36c4f Mon Sep 17 00:00:00 2001 From: Anne Marsan Date: Fri, 13 Sep 2024 11:09:57 -0400 Subject: [PATCH 01/10] Update the database security configuration to match the instructions in the wiki. --- sample_settings.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sample_settings.xml b/sample_settings.xml index 4eacb6134f..bf562edd51 100644 --- a/sample_settings.xml +++ b/sample_settings.xml @@ -61,12 +61,12 @@ 3 10 10 - ${datasource.ohdsi.schema} + atlas_security ${datasource.url} ${datasource.driverClassName} ${datasource.username} ${datasource.password} - select password from ${security.db.datasource.schema}.users where lower(email) = lower(?) + select password,firstName,middleName,lastName from ${security.db.datasource.schema}.users where username = ? From faa0f574a892363ba61e20ee65efa9aa4f7b8d02 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Mon, 4 Nov 2024 12:13:48 -0500 Subject: [PATCH 02/10] Fix for compile error seen for Eclipse (issue 2390) (#2395) (#2396) * Fix for compile error seen for Eclipse (issue 2390) (#2395) --------- Co-authored-by: greshje <36824929+greshje@users.noreply.github.com> --- .../AbstractForEachValidatorBuilder.java | 29 +++++++++++++++++-- .../check/builder/ValidatorGroupBuilder.java | 26 +++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/check/builder/AbstractForEachValidatorBuilder.java b/src/main/java/org/ohdsi/webapi/check/builder/AbstractForEachValidatorBuilder.java index ea93a7c0c2..b19b3ce3b3 100644 --- a/src/main/java/org/ohdsi/webapi/check/builder/AbstractForEachValidatorBuilder.java +++ b/src/main/java/org/ohdsi/webapi/check/builder/AbstractForEachValidatorBuilder.java @@ -41,9 +41,34 @@ public final AbstractForEachValidatorBuilder groups(ValidatorGroupBuilder< } protected List> initGroups() { - return initAndBuildList(this.validatorGroupBuilders); + return initAndBuildGroupList(this.validatorGroupBuilders); } + // Note: exact same functionality as initAndBuildList, just needed a different call signature. + // This method was added to enable development using Eclipse + private List> initAndBuildGroupList(List> builders) { + + builders.forEach(builder -> { + if (Objects.isNull(builder.getBasePath())) { + builder.basePath(createChildPath()); + } + if (Objects.isNull(builder.getErrorMessage())) { + builder.errorMessage(this.errorMessage); + } + if (Objects.isNull(builder.getSeverity())) { + builder.severity(this.severity); + } + if (Objects.isNull(builder.getAttrName())) { + builder.attrName(this.attrName); + } + }); + return builders.stream() + .map(ValidatorBaseBuilder::build) + .collect(Collectors.toList()); + + + } + protected List> initValidators() { return initAndBuildList(this.validatorBuilders); } @@ -70,4 +95,4 @@ private List initAndBuildList(List build() { - List> groups = initAndBuildList(this.validatorGroupBuilders); + List> groups = initAndBuildGroupList(this.validatorGroupBuilders); List> validators = initAndBuildList(this.validatorBuilders); return new ValidatorGroup<>(validators, groups, valueGetter, conditionGetter); } + // Note: exact same functionality as initAndBuildList, just needed a different call signature. + // This method was added to enable development using Eclipse + private List> initAndBuildGroupList(List> builders) { + + builders.forEach(builder -> { + if (Objects.nonNull(this.errorMessage)) { + builder.errorMessage(this.errorMessage); + } + if (Objects.isNull(builder.getBasePath())) { + builder.basePath(createChildPath()); + } + if (Objects.isNull(builder.severity)) { + builder.severity(this.severity); + } + }); + return builders.stream() + .map(ValidatorBaseBuilder::build) + .collect(Collectors.toList()); + + + } + private List initAndBuildList(List> builders) { builders.forEach(builder -> { @@ -89,4 +111,4 @@ private List initAndBuildList(List Date: Fri, 8 Nov 2024 09:29:27 -0500 Subject: [PATCH 03/10] Bug Fix: Return correct count values for RC/DRC and PC/DPC (#2405) Fixes #2296 --- pom.xml | 6 ++ .../cdmresults/service/CDMCacheService.java | 6 +- .../results/achilles_result_concept_count.sql | 8 ++ .../webapi/test/CDMResultsServiceIT.java | 81 +++++++++++++++++++ .../java/org/ohdsi/webapi/test/WebApiIT.java | 3 +- .../resources/application-test.properties | 4 + src/test/resources/database/cdm_results.sql | 2 + 7 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 src/main/resources/ddl/results/achilles_result_concept_count.sql create mode 100644 src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java create mode 100644 src/test/resources/database/cdm_results.sql diff --git a/pom.xml b/pom.xml index f44b0e08a0..f82a63ccb3 100644 --- a/pom.xml +++ b/pom.xml @@ -769,6 +769,12 @@ json-smart 2.4.9 + + net.minidev + asm + 1.0.2 + test + org.apache.commons commons-lang3 diff --git a/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java b/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java index a4ec88b38e..e3870609d4 100644 --- a/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java +++ b/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java @@ -145,9 +145,9 @@ private void cacheRecordsById(Source source, List ids) { PreparedStatementRenderer psr = getPartialPreparedStatementRenderer(source, idsSlice); Set cachedIds = loadCache(source, psr, mapper, jdbcTemplate); // in this batch, need to save any concepts that were not found when loading cache - idsSlice.removeAll(cachedIds); - if (!idsSlice.isEmpty()) { // store zeros in cache - List zeroConcepts = idsSlice.stream().map(id -> { + List notFoundIds = new ArrayList(CollectionUtils.subtract(idsSlice, cachedIds)); + if (!notFoundIds.isEmpty()) { // store zeros in cache + List zeroConcepts = notFoundIds.stream().map(id -> { CDMCacheEntity ce = new CDMCacheEntity(); ce.setConceptId(id); ce.setRecordCount(0L); diff --git a/src/main/resources/ddl/results/achilles_result_concept_count.sql b/src/main/resources/ddl/results/achilles_result_concept_count.sql new file mode 100644 index 0000000000..b11087a543 --- /dev/null +++ b/src/main/resources/ddl/results/achilles_result_concept_count.sql @@ -0,0 +1,8 @@ +IF OBJECT_ID('@results_schema.achilles_result_concept_count', 'U') IS NULL +CREATE TABLE @results_schema.achilles_result_concept_count ( + concept_id int, + record_count bigint, + descendant_record_count bigint, + person_count bigint, + descendant_person_count bigint +); \ No newline at end of file diff --git a/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java b/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java new file mode 100644 index 0000000000..72635bad7a --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java @@ -0,0 +1,81 @@ +package org.ohdsi.webapi.test; + +import com.odysseusinc.arachne.commons.types.DBMSType; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.ohdsi.circe.helper.ResourceHelper; +import org.ohdsi.sql.SqlRender; +import org.ohdsi.sql.SqlTranslate; +import org.ohdsi.webapi.source.SourceRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.ResponseEntity; + +public class CDMResultsServiceIT extends WebApiIT { + private static final String CDM_RESULTS_FILE_PATH = "/database/cdm_results.sql"; + + @Value("${cdmResultsService.endpoint.conceptRecordCount}") + private String conceptRecordCountEndpoint; + + @Autowired + private SourceRepository sourceRepository; + + @Before + public void init() throws Exception { + truncateTable(String.format("%s.%s", "public", "source")); + resetSequence(String.format("%s.%s", "public", "source_sequence")); + sourceRepository.saveAndFlush(getCdmSource()); + prepareCdmSchema(); + prepareResultSchema(); + addCDMResults(); + } + + private void addCDMResults() { + String resultSql = SqlRender.renderSql(ResourceHelper.GetResourceAsString( + CDM_RESULTS_FILE_PATH), + new String[] { "results_schema" }, new String[] { RESULT_SCHEMA_NAME }); + String sql = SqlTranslate.translateSql(resultSql, DBMSType.POSTGRESQL.getOhdsiDB()); + jdbcTemplate.execute(sql); + } + + @Test + public void requestConceptRecordCounts_firstTime_returnsResults() { + + // Arrange + List conceptIds = Arrays.asList(1); + Map queryParameters = new HashMap(); + queryParameters.put("sourceName", SOURCE_KEY); + + List>> list = new ArrayList<>(); + @SuppressWarnings("unchecked") + Class>>> returnClass = (Class>>>)list.getClass(); + + // Act + final ResponseEntity>>> entity = getRestTemplate().postForEntity(this.conceptRecordCountEndpoint, conceptIds, + returnClass, queryParameters ); + + // Assertion + assertOK(entity); + List>> results = entity.getBody(); + assertEquals(1, results.size()); + LinkedHashMap> resultHashMap = results.get(0); + assertEquals(1, resultHashMap.size()); + assertTrue(resultHashMap.containsKey("1")); + List counts = resultHashMap.get("1"); + assertEquals(100, counts.get(0).intValue()); + assertEquals(101, counts.get(1).intValue()); + assertEquals(102, counts.get(2).intValue()); + assertEquals(103, counts.get(3).intValue()); + } +} diff --git a/src/test/java/org/ohdsi/webapi/test/WebApiIT.java b/src/test/java/org/ohdsi/webapi/test/WebApiIT.java index dd7dd57c0d..ed2af1e0c5 100644 --- a/src/test/java/org/ohdsi/webapi/test/WebApiIT.java +++ b/src/test/java/org/ohdsi/webapi/test/WebApiIT.java @@ -64,7 +64,8 @@ public abstract class WebApiIT { "/ddl/results/pathway_analysis_codes.sql", "/ddl/results/pathway_analysis_events.sql", "/ddl/results/pathway_analysis_paths.sql", - "/ddl/results/pathway_analysis_stats.sql" + "/ddl/results/pathway_analysis_stats.sql", + "/ddl/results/achilles_result_concept_count.sql" ); diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index f11c6df1b7..3d9a789b5b 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -1,6 +1,7 @@ baseUri=http://localhost:${local.server.port}${server.context-path} security.db.datasource.url=http://localhost:${datasource.url}/arachne_portal_enterprise vocabularyservice.endpoint=${baseUri}/vocabulary +cdmResultsService.endpoint=${baseUri}/cdmresults #GET vocabularies vocabularyservice.endpoint.vocabularies=${vocabularyservice.endpoint}/vocabularies #GET domains @@ -10,6 +11,9 @@ vocabularyservice.endpoint.concept=${vocabularyservice.endpoint}/concept/1 #GET cohortdefinitions cohortdefinitionservice.endpoint.cohortdefinitions=${baseUri}/cohortdefinition +#POST cdmResults +cdmResultsService.endpoint.conceptRecordCount=${cdmResultsService.endpoint}/{sourceName}/conceptRecordCount + #Example application service exampleservice.endpoint=${baseUri}/example diff --git a/src/test/resources/database/cdm_results.sql b/src/test/resources/database/cdm_results.sql new file mode 100644 index 0000000000..ce261c3c5a --- /dev/null +++ b/src/test/resources/database/cdm_results.sql @@ -0,0 +1,2 @@ +insert into @results_schema.achilles_result_concept_count (concept_id, record_count, descendant_record_count, person_count, descendant_person_count) +values (1,100,101,102,103); \ No newline at end of file From bc7306722c53a3d90694889c1e6f57330fc7f67d Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Fri, 22 Nov 2024 12:28:23 -0500 Subject: [PATCH 04/10] Remove the wild card permissions from the CohortCharacterizationPermissionSchema. (#2413) Fixes #2412 --- .../security/model/CohortCharacterizationPermissionSchema.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java index f6ea10012a..17f34501be 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java @@ -16,8 +16,6 @@ public class CohortCharacterizationPermissionSchema extends EntityPermissionSche private static Map readPermissions = new HashMap() {{ put("cohort-characterization:%s:get", "Get cohort characterization"); put("cohort-characterization:%s:generation:get", "Get cohort characterization generations"); - put("cohort-characterization:generation:*:get", "Get cohort characterization generation"); - put("cohort-characterization:design:get", "cohort-characterization:design:get"); put("cohort-characterization:%s:design:get", "Get cohort characterization design"); put("cohort-characterization:design:%s:get", "view cohort characterization with id %s"); put("cohort-characterization:%s:version:get", "Get list of characterization versions"); From fce763f2904fc43488af8ff71a258860f9fb8b75 Mon Sep 17 00:00:00 2001 From: oleg-odysseus Date: Tue, 26 Nov 2024 15:18:26 +0100 Subject: [PATCH 05/10] [issue-2379][issue-2408] Migrated JDBC Snowflake driver to v3.20.0 (#2414) --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f82a63ccb3..2549b9fd23 100644 --- a/pom.xml +++ b/pom.xml @@ -1828,7 +1828,7 @@ webapi-snowflake true - 3.13.22 + 3.20.0 From e41dc29d0e6f9563be3628989dbf64b67b7f9d9b Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Wed, 27 Nov 2024 09:31:42 -0500 Subject: [PATCH 06/10] Removed dependency on commons lang (#2415) * Code base has both commons-lang and commons-lang3 libraries loaded. I replaced existing references with newer library variant. * WordUtils was moved to commons-text --------- Co-authored-by: edwincruz --- pom.xml | 5 +++++ .../checker/ir/helper/IRAnalysisExpressionHelper.java | 2 +- .../converter/EstimationToEstimationShortDTOConverter.java | 2 +- .../ExampleApplicationWithJobService.java | 2 +- .../org/ohdsi/webapi/generationcache/CleanupScheduler.java | 2 +- .../org/ohdsi/webapi/report/mapper/GenericRowMapper.java | 2 +- .../webapi/shiro/filters/UpdateAccessTokenFilter.java | 2 +- .../webapi/shiro/management/AtlasRegularSecurity.java | 2 +- src/main/java/org/ohdsi/webapi/shiro/realms/ADRealm.java | 2 +- .../user/importer/providers/ActiveDirectoryProvider.java | 4 +--- .../org/ohdsi/webapi/util/PreparedStatementRenderer.java | 7 ++----- 11 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pom.xml b/pom.xml index 2549b9fd23..8c8e946b61 100644 --- a/pom.xml +++ b/pom.xml @@ -780,6 +780,11 @@ commons-lang3 3.12.0 + + org.apache.commons + commons-text + 1.10.0 + org.flywaydb flyway-core diff --git a/src/main/java/org/ohdsi/webapi/check/checker/ir/helper/IRAnalysisExpressionHelper.java b/src/main/java/org/ohdsi/webapi/check/checker/ir/helper/IRAnalysisExpressionHelper.java index 6a1f59056a..8a6d8f2341 100644 --- a/src/main/java/org/ohdsi/webapi/check/checker/ir/helper/IRAnalysisExpressionHelper.java +++ b/src/main/java/org/ohdsi/webapi/check/checker/ir/helper/IRAnalysisExpressionHelper.java @@ -1,6 +1,6 @@ package org.ohdsi.webapi.check.checker.ir.helper; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.ohdsi.webapi.check.builder.IterableForEachValidatorBuilder; import org.ohdsi.webapi.check.builder.NotNullNotEmptyValidatorBuilder; import org.ohdsi.webapi.check.builder.PredicateValidatorBuilder; diff --git a/src/main/java/org/ohdsi/webapi/estimation/converter/EstimationToEstimationShortDTOConverter.java b/src/main/java/org/ohdsi/webapi/estimation/converter/EstimationToEstimationShortDTOConverter.java index 1aafbd9dc2..3d8af2813d 100644 --- a/src/main/java/org/ohdsi/webapi/estimation/converter/EstimationToEstimationShortDTOConverter.java +++ b/src/main/java/org/ohdsi/webapi/estimation/converter/EstimationToEstimationShortDTOConverter.java @@ -1,6 +1,6 @@ package org.ohdsi.webapi.estimation.converter; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.ohdsi.webapi.estimation.Estimation; import org.ohdsi.webapi.estimation.dto.EstimationShortDTO; import org.ohdsi.webapi.service.converters.BaseCommonEntityToDTOConverter; diff --git a/src/main/java/org/ohdsi/webapi/exampleapplication/ExampleApplicationWithJobService.java b/src/main/java/org/ohdsi/webapi/exampleapplication/ExampleApplicationWithJobService.java index 0c34950ea4..db275544b4 100644 --- a/src/main/java/org/ohdsi/webapi/exampleapplication/ExampleApplicationWithJobService.java +++ b/src/main/java/org/ohdsi/webapi/exampleapplication/ExampleApplicationWithJobService.java @@ -1,6 +1,6 @@ package org.ohdsi.webapi.exampleapplication; -import org.apache.commons.lang.RandomStringUtils; +import org.apache.commons.lang3.RandomStringUtils; import org.ohdsi.circe.vocabulary.Concept; import org.ohdsi.webapi.exampleapplication.model.Widget; import org.ohdsi.webapi.exampleapplication.repository.WidgetRepository; diff --git a/src/main/java/org/ohdsi/webapi/generationcache/CleanupScheduler.java b/src/main/java/org/ohdsi/webapi/generationcache/CleanupScheduler.java index 37e951eef2..d89b278907 100644 --- a/src/main/java/org/ohdsi/webapi/generationcache/CleanupScheduler.java +++ b/src/main/java/org/ohdsi/webapi/generationcache/CleanupScheduler.java @@ -1,7 +1,7 @@ package org.ohdsi.webapi.generationcache; import com.cosium.spring.data.jpa.entity.graph.domain.EntityGraphUtils; -import org.apache.commons.lang.time.DateUtils; +import org.apache.commons.lang3.time.DateUtils; import org.springframework.beans.factory.annotation.Value; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; diff --git a/src/main/java/org/ohdsi/webapi/report/mapper/GenericRowMapper.java b/src/main/java/org/ohdsi/webapi/report/mapper/GenericRowMapper.java index 64d4707c99..c84dcaed26 100644 --- a/src/main/java/org/ohdsi/webapi/report/mapper/GenericRowMapper.java +++ b/src/main/java/org/ohdsi/webapi/report/mapper/GenericRowMapper.java @@ -3,7 +3,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; -import org.apache.commons.lang.WordUtils; +import org.apache.commons.text.WordUtils; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.support.JdbcUtils; diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java index f5597058e8..20dce484e0 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java @@ -18,7 +18,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import javax.ws.rs.core.UriBuilder; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.shiro.SecurityUtils; import org.apache.shiro.session.Session; import org.apache.shiro.subject.PrincipalCollection; diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java index e19a1ebc8b..6b083b9e82 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java @@ -4,7 +4,7 @@ import io.buji.pac4j.filter.CallbackFilter; import io.buji.pac4j.filter.SecurityFilter; import io.buji.pac4j.realm.Pac4jRealm; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.shiro.realm.Realm; import org.apache.shiro.realm.activedirectory.ActiveDirectoryRealm; import org.apache.shiro.realm.ldap.DefaultLdapRealm; diff --git a/src/main/java/org/ohdsi/webapi/shiro/realms/ADRealm.java b/src/main/java/org/ohdsi/webapi/shiro/realms/ADRealm.java index 6594666d84..3b0d4d6e82 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/realms/ADRealm.java +++ b/src/main/java/org/ohdsi/webapi/shiro/realms/ADRealm.java @@ -1,6 +1,6 @@ package org.ohdsi.webapi.shiro.realms; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.SimpleAuthenticationInfo; diff --git a/src/main/java/org/ohdsi/webapi/user/importer/providers/ActiveDirectoryProvider.java b/src/main/java/org/ohdsi/webapi/user/importer/providers/ActiveDirectoryProvider.java index 9ad48582c4..66c6708be9 100644 --- a/src/main/java/org/ohdsi/webapi/user/importer/providers/ActiveDirectoryProvider.java +++ b/src/main/java/org/ohdsi/webapi/user/importer/providers/ActiveDirectoryProvider.java @@ -1,7 +1,7 @@ package org.ohdsi.webapi.user.importer.providers; import com.google.common.collect.ImmutableSet; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.ohdsi.webapi.user.importer.model.LdapGroup; import org.ohdsi.webapi.user.importer.model.LdapUser; import org.springframework.beans.factory.annotation.Value; @@ -13,11 +13,9 @@ import org.springframework.ldap.support.LdapUtils; import org.springframework.stereotype.Component; -import javax.annotation.PostConstruct; import javax.naming.NamingException; import javax.naming.directory.Attributes; import javax.naming.directory.SearchControls; -import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.stream.Collectors; diff --git a/src/main/java/org/ohdsi/webapi/util/PreparedStatementRenderer.java b/src/main/java/org/ohdsi/webapi/util/PreparedStatementRenderer.java index 96436da4f6..de7632c9c5 100644 --- a/src/main/java/org/ohdsi/webapi/util/PreparedStatementRenderer.java +++ b/src/main/java/org/ohdsi/webapi/util/PreparedStatementRenderer.java @@ -1,6 +1,5 @@ package org.ohdsi.webapi.util; -import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.HashMap; import java.util.List; @@ -11,10 +10,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import com.google.common.collect.ImmutableList; -import com.odysseusinc.arachne.commons.types.DBMSType; -import org.apache.commons.lang.ArrayUtils; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.StringUtils; import org.ohdsi.circe.helper.ResourceHelper; import org.ohdsi.sql.BigQuerySparkTranslate; import org.ohdsi.sql.SqlRender; From e7e6055463d96191cd616307d97aaaf147d866f2 Mon Sep 17 00:00:00 2001 From: Anne Marsan Date: Wed, 27 Nov 2024 09:37:30 -0500 Subject: [PATCH 07/10] Add endpoint to clear the cdm_cache and achilles_cache (#2406) Truncates cdm_cache and achilles_cache before warming cache. --- .editorconfig | 12 ++ pom.xml | 65 +++--- .../repository/AchillesCacheRepository.java | 6 +- .../service/AchillesCacheService.java | 204 ++++++++++-------- .../repository/CDMCacheRepository.java | 5 + .../cdmresults/service/CDMCacheService.java | 19 ++ .../model/SourcePermissionSchema.java | 1 + .../webapi/service/CDMResultsService.java | 60 +++++- ...41113115700__add_clearcache_permission.sql | 29 +++ .../webapi/test/CDMResultsServiceIT.java | 79 ++++++- .../java/org/ohdsi/webapi/test/ITStarter.java | 3 +- .../resources/application-test.properties | 4 +- 12 files changed, 351 insertions(+), 136 deletions(-) create mode 100644 .editorconfig create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20241113115700__add_clearcache_permission.sql diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000000..88bf97c508 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +# EditorConfig is awesome: https://EditorConfig.org + +# top-most EditorConfig file +root = true + +[*] +indent_style = space +indent_size = 2 +end_of_line = crlf +charset = utf-8 +trim_trailing_whitespace = false +insert_final_newline = false \ No newline at end of file diff --git a/pom.xml b/pom.xml index 8c8e946b61..184483c9e7 100644 --- a/pom.xml +++ b/pom.xml @@ -1,5 +1,5 @@ - + 4.0.0 @@ -122,7 +122,7 @@ (&(objectClass=person)(cn=%s)) true - 30000 + 30000 public (&(objectClass=person)(userPrincipalName=%s)) displayname @@ -191,8 +191,8 @@ - true - + true + 8080 @@ -216,7 +216,7 @@ false PBEWithMD5AndDES - + OHDSI @@ -298,6 +298,7 @@ /tmp/atlas/audit/audit.log /tmp/atlas/audit/audit-extra.log + WebAPI ${basedir}/src/main/java @@ -368,8 +369,8 @@ - git.branch - git.commit.id.abbrev + git.branch + git.commit.id.abbrev @@ -575,6 +576,7 @@ + com.fasterxml.jackson.core @@ -813,12 +815,12 @@ jackson-databind - org.slf4j - slf4j-log4j12 + org.slf4j + slf4j-log4j12 - log4j - log4j + log4j + log4j @@ -1164,7 +1166,7 @@ ${pac4j.version} - com.fasterxml.jackson.core + com.fasterxml.jackson.core jackson-databind @@ -1210,12 +1212,12 @@ spring-boot-starter-test ${spring.boot.version} test - - - com.vaadin.external.google - android-json - - + + + com.vaadin.external.google + android-json + + org.dbunit @@ -1236,12 +1238,13 @@ test - com.github.mjeanroy - dbunit-plus - 2.0.1 - test + com.github.mjeanroy + dbunit-plus + 2.0.1 + test + webapi-oracle @@ -1334,17 +1337,17 @@ lower(email) = lower(?) - + ohdsi.snapshots repo.ohdsi.org-snapshots https://repo.ohdsi.org/nexus/content/repositories/snapshots false - + true - - + + @@ -1417,7 +1420,7 @@ true 2.6.15 - ...path/to/impala/jdbc/drivers... + ...path/to/impala/jdbc/drivers... @@ -1522,9 +1525,9 @@ v2-rev20220326-1.32.1 - com.google.cloud - google-cloud-bigquery - 1.2.15 + com.google.cloud + google-cloud-bigquery + 1.2.15 com.google.http-client diff --git a/src/main/java/org/ohdsi/webapi/achilles/repository/AchillesCacheRepository.java b/src/main/java/org/ohdsi/webapi/achilles/repository/AchillesCacheRepository.java index 2479cc7da7..9e5e8c3805 100644 --- a/src/main/java/org/ohdsi/webapi/achilles/repository/AchillesCacheRepository.java +++ b/src/main/java/org/ohdsi/webapi/achilles/repository/AchillesCacheRepository.java @@ -2,7 +2,7 @@ import org.ohdsi.webapi.achilles.domain.AchillesCacheEntity; import org.ohdsi.webapi.source.Source; -import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.query.Param; @@ -16,4 +16,8 @@ public interface AchillesCacheRepository extends CrudRepository findBySourceAndNames(@Param("source") Source source, @Param("names") List names); + + @Modifying + @Query("delete from AchillesCacheEntity where source = :source") + void deleteBySource(@Param("source") Source source); } diff --git a/src/main/java/org/ohdsi/webapi/achilles/service/AchillesCacheService.java b/src/main/java/org/ohdsi/webapi/achilles/service/AchillesCacheService.java index c55180929b..023f6d7d69 100644 --- a/src/main/java/org/ohdsi/webapi/achilles/service/AchillesCacheService.java +++ b/src/main/java/org/ohdsi/webapi/achilles/service/AchillesCacheService.java @@ -5,9 +5,12 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import org.ohdsi.webapi.achilles.domain.AchillesCacheEntity; import org.ohdsi.webapi.achilles.repository.AchillesCacheRepository; +import org.ohdsi.webapi.shiro.management.datasource.SourceAccessor; import org.ohdsi.webapi.source.Source; +import org.ohdsi.webapi.source.SourceRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Propagation; @@ -22,103 +25,124 @@ @Service public class AchillesCacheService { - private static final Logger LOG = LoggerFactory.getLogger(AchillesCacheService.class); - - private final AchillesCacheRepository cacheRepository; - - private final ObjectMapper objectMapper; - - @Value("${spring.jpa.properties.hibernate.jdbc.batch_size}") - private int batchSize; - - public AchillesCacheService(AchillesCacheRepository cacheRepository, - ObjectMapper objectMapper) { - this.cacheRepository = cacheRepository; - this.objectMapper = objectMapper; - } - - @Transactional(readOnly = true) - public AchillesCacheEntity getCache(Source source, String cacheName) { - return cacheRepository.findBySourceAndCacheName(source, cacheName); - } - - @Transactional(readOnly = true) - public List findBySourceAndNames(Source source, List names) { - return cacheRepository.findBySourceAndNames(source, names); + private static final Logger LOG = LoggerFactory.getLogger(AchillesCacheService.class); + + private final AchillesCacheRepository cacheRepository; + + private final ObjectMapper objectMapper; + + @Value("${spring.jpa.properties.hibernate.jdbc.batch_size}") + private int batchSize; + + @Autowired + private SourceRepository sourceRepository; + + @Autowired + private SourceAccessor sourceAccessor; + + public AchillesCacheService(AchillesCacheRepository cacheRepository, + ObjectMapper objectMapper) { + this.cacheRepository = cacheRepository; + this.objectMapper = objectMapper; + } + + @Transactional(readOnly = true) + public AchillesCacheEntity getCache(Source source, String cacheName) { + return cacheRepository.findBySourceAndCacheName(source, cacheName); + } + + @Transactional(readOnly = true) + public List findBySourceAndNames(Source source, List names) { + return cacheRepository.findBySourceAndNames(source, names); + } + + @Transactional(propagation = Propagation.REQUIRES_NEW) + public AchillesCacheEntity createCache(Source source, String cacheName, Object result) + throws JsonProcessingException { + AchillesCacheEntity cacheEntity = getCache(source, cacheName); + String cache = objectMapper.writeValueAsString(result); + if (Objects.nonNull(cacheEntity)) { + cacheEntity.setCache(cache); + } else { + cacheEntity = new AchillesCacheEntity(); + cacheEntity.setSource(source); + cacheEntity.setCacheName(cacheName); + cacheEntity.setCache(cache); } - @Transactional(propagation = Propagation.REQUIRES_NEW) - public AchillesCacheEntity createCache(Source source, String cacheName, Object result) throws JsonProcessingException { - AchillesCacheEntity cacheEntity = getCache(source, cacheName); - String cache = objectMapper.writeValueAsString(result); - if (Objects.nonNull(cacheEntity)) { - cacheEntity.setCache(cache); - } else { - cacheEntity = new AchillesCacheEntity(); - cacheEntity.setSource(source); - cacheEntity.setCacheName(cacheName); - cacheEntity.setCache(cache); - } + return cacheRepository.save(cacheEntity); + } - return cacheRepository.save(cacheEntity); + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void saveDrilldownCacheMap(Source source, String domain, Map conceptNodes) { + if (conceptNodes.isEmpty()) { // nothing to cache + LOG.warn( + "Cannot cache drilldown reports for {}, domain {}. Check if result schema contains achilles results tables.", + source.getSourceKey(), domain); + return; } - @Transactional(propagation = Propagation.REQUIRES_NEW) - public void saveDrilldownCacheMap(Source source, String domain, Map conceptNodes) { - if (conceptNodes.isEmpty()) { // nothing to cache - LOG.warn("Cannot cache drilldown reports for {}, domain {}. Check if result schema contains achilles results tables.", - source.getSourceKey(), domain); - return; - } - - Map nodes = new HashMap<>(batchSize); - for (Map.Entry entry : conceptNodes.entrySet()) { - if (nodes.size() >= batchSize) { - createCacheEntities(source, nodes); - nodes.clear(); - } - Integer key = entry.getKey(); - String cacheName = getCacheName(domain, key); - nodes.put(cacheName, entry.getValue()); - } + Map nodes = new HashMap<>(batchSize); + for (Map.Entry entry : conceptNodes.entrySet()) { + if (nodes.size() >= batchSize) { createCacheEntities(source, nodes); nodes.clear(); + } + Integer key = entry.getKey(); + String cacheName = getCacheName(domain, key); + nodes.put(cacheName, entry.getValue()); } - - private void createCacheEntities(Source source, Map nodes) { - List cacheEntities = getEntities(source, nodes); - cacheRepository.save(cacheEntities); - } - - private List getEntities(Source source, Map nodes) { - List cacheNames = new ArrayList<>(nodes.keySet()); - List cacheEntities = findBySourceAndNames(source, cacheNames); - nodes.forEach((key, value) -> { - // check if the entity with given cache name already exists - Optional cacheEntity = cacheEntities.stream() - .filter(entity -> entity.getCacheName().equals(key)) - .findAny(); - try { - String newValue = objectMapper.writeValueAsString(value); - if (cacheEntity.isPresent()) { - // if cache entity already exists update its value - cacheEntity.get().setCache(newValue); - } else { - // if cache entity does not exist - create new one - AchillesCacheEntity newEntity = new AchillesCacheEntity(); - newEntity.setCacheName(key); - newEntity.setSource(source); - newEntity.setCache(newValue); - cacheEntities.add(newEntity); - } - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - }); - return cacheEntities; - } - - private String getCacheName(String domain, int conceptId) { - return String.format("drilldown_%s_%d", domain, conceptId); + createCacheEntities(source, nodes); + nodes.clear(); + } + + @Transactional() + public void clearCache() { + List sources = sourceRepository.findAll(); + sources.stream().forEach(this::clearCache); + } + + @Transactional() + public void clearCache(Source source) { + if (sourceAccessor.hasAccess(source)) { + cacheRepository.deleteBySource(source); } + } + + private void createCacheEntities(Source source, Map nodes) { + List cacheEntities = getEntities(source, nodes); + cacheRepository.save(cacheEntities); + } + + private List getEntities(Source source, Map nodes) { + List cacheNames = new ArrayList<>(nodes.keySet()); + List cacheEntities = findBySourceAndNames(source, cacheNames); + nodes.forEach((key, value) -> { + // check if the entity with given cache name already exists + Optional cacheEntity = cacheEntities.stream() + .filter(entity -> entity.getCacheName().equals(key)) + .findAny(); + try { + String newValue = objectMapper.writeValueAsString(value); + if (cacheEntity.isPresent()) { + // if cache entity already exists update its value + cacheEntity.get().setCache(newValue); + } else { + // if cache entity does not exist - create new one + AchillesCacheEntity newEntity = new AchillesCacheEntity(); + newEntity.setCacheName(key); + newEntity.setSource(source); + newEntity.setCache(newValue); + cacheEntities.add(newEntity); + } + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + }); + return cacheEntities; + } + + private String getCacheName(String domain, int conceptId) { + return String.format("drilldown_%s_%d", domain, conceptId); + } } diff --git a/src/main/java/org/ohdsi/webapi/cdmresults/repository/CDMCacheRepository.java b/src/main/java/org/ohdsi/webapi/cdmresults/repository/CDMCacheRepository.java index f329ddfcd2..140bc48bfa 100644 --- a/src/main/java/org/ohdsi/webapi/cdmresults/repository/CDMCacheRepository.java +++ b/src/main/java/org/ohdsi/webapi/cdmresults/repository/CDMCacheRepository.java @@ -1,6 +1,7 @@ package org.ohdsi.webapi.cdmresults.repository; import org.ohdsi.webapi.cdmresults.domain.CDMCacheEntity; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.query.Param; @@ -12,4 +13,8 @@ public interface CDMCacheRepository extends CrudRepository { @Query("select c from CDMCacheEntity c where c.sourceId = :sourceId and c.conceptId in :conceptIds") List findBySourceAndConceptIds(@Param("sourceId") int sourceId, @Param("conceptIds") List conceptIds); + + @Modifying + @Query("delete from CDMCacheEntity c where c.sourceId = :sourceId") + void deleteBySource(@Param("sourceId") int sourceId); } diff --git a/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java b/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java index e3870609d4..8b7a6e520b 100644 --- a/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java +++ b/src/main/java/org/ohdsi/webapi/cdmresults/service/CDMCacheService.java @@ -10,15 +10,18 @@ import org.ohdsi.webapi.cdmresults.mapper.DescendantRecordCountMapper; import org.ohdsi.webapi.cdmresults.repository.CDMCacheRepository; import org.ohdsi.webapi.service.AbstractDaoService; +import org.ohdsi.webapi.shiro.management.datasource.SourceAccessor; import org.ohdsi.webapi.source.Source; import org.ohdsi.webapi.source.SourceDaimon; import org.ohdsi.webapi.util.PreparedSqlRender; import org.ohdsi.webapi.util.PreparedStatementRenderer; import org.ohdsi.webapi.util.SessionUtils; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.convert.ConversionService; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionTemplate; import java.util.ArrayList; @@ -51,6 +54,9 @@ public class CDMCacheService extends AbstractDaoService { private final ConversionService conversionService; + @Autowired + private SourceAccessor sourceAccessor; + public CDMCacheService(CDMCacheBatchService cdmCacheBatchService, ConversionService conversionService, CDMCacheRepository cdmCacheRepository) { @@ -95,6 +101,19 @@ public List findAndCache(Source source, List conceptIds return cacheEntities; } + @Transactional() + public void clearCache() { + List sources = getSourceRepository().findAll(); + sources.stream().forEach(this::clearCache); + } + + @Transactional() + public void clearCache(Source source) { + if (sourceAccessor.hasAccess(source)) { + cdmCacheRepository.deleteBySource(source.getSourceId()); + } + } + private List find(Source source, List conceptIds) { if (CollectionUtils.isEmpty(conceptIds)) { return Collections.emptyList(); diff --git a/src/main/java/org/ohdsi/webapi/security/model/SourcePermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/SourcePermissionSchema.java index c510776be8..ae457d252f 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/SourcePermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/SourcePermissionSchema.java @@ -33,6 +33,7 @@ public class SourcePermissionSchema extends EntityPermissionSchema { put("cdmresults:%s:*:get", "Get Achilles reports on Source with SourceKey = %s"); put("cdmresults:%s:conceptRecordCount:post", "Get Achilles concept counts on Source with SourceKey = %s"); put("cdmresults:%s:*:*:get", "Get Achilles reports details on Source with SourceKey = %s"); + put("cdmresults:%s:clearcache:post", "Clear the Achilles and CDM results caches on Source with SourceKey = %s"); put("cohortresults:%s:*:*:get", "Get cohort results on Source with SourceKey = %s"); put("cohortresults:%s:*:*:*:get", "Get cohort results details on Source with SourceKey = %s"); put("cohortresults:%s:*:healthcareutilization:*:*:get", "Get cohort results baseline on period for Source with SourceKey = %s"); diff --git a/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java b/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java index 7b83d9d3bd..c887962d1c 100644 --- a/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java +++ b/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java @@ -44,8 +44,10 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; import javax.ws.rs.Consumes; +import javax.ws.rs.ForbiddenException; import javax.ws.rs.GET; import javax.ws.rs.POST; import javax.ws.rs.Path; @@ -282,7 +284,44 @@ public JobExecutionResource refreshCache(@PathParam("sourceKey") final String so } return new JobExecutionResource(); } + + /** + * Clear the cdm_cache and achilles_cache for all sources + * + * @summary Clear the cdm_cache and achilles_cache for all sources + * @return void + * @throws ForbiddenException if the user is not an admin + */ + @POST + @Path("{sourceKey}/clearCache") + @Transactional() + public void clearCacheForSource(@PathParam("sourceKey") final String sourceKey) { + if (!isSecured() || !isAdmin()) { + throw new ForbiddenException(); + } + Source source = getSourceRepository().findBySourceKey(sourceKey); + cacheService.clearCache(source); + cdmCacheService.clearCache(source); + } + /** + * Clear the cdm_cache and achilles_cache for all sources + * + * @summary Clear the cdm_cache and achilles_cache for all sources + * @return void + * @throws ForbiddenException if the user is not an admin + */ + @POST + @Path("clearCache") + @Transactional() + public void clearCache() { + if (!isSecured() || !isAdmin()) { + throw new ForbiddenException(); + } + cacheService.clearCache(); + cdmCacheService.clearCache(); + } + /** * Queries for data density report for the given sourceKey * @@ -441,9 +480,7 @@ private JobExecutionResource warmCaches(Source source) { String jobName = getWarmCacheJobName(String.valueOf(source.getSourceId()), source.getSourceKey()); List jobSteps = createCacheWarmingJobSteps(source, jobName); - SimpleJobBuilder builder = createJob(String.valueOf(source.getSourceId()), - source.getSourceKey(), - jobSteps); + SimpleJobBuilder builder = createJob(jobName, jobSteps); return runJob(source.getSourceKey(), source.getSourceId(), jobName, builder); } @@ -488,9 +525,9 @@ private void warmCaches(Collection sources) { if (counter++ >= bucketSizes[bucketIndex] - 1) { if (!allJobSteps.isEmpty()) { - SimpleJobBuilder builder = createJob(sourceIds.stream().map(String::valueOf).collect(Collectors.joining(",")), - String.join(",", sourceKeys), - allJobSteps); + String compositeJobName = getWarmCacheJobName(sourceIds.stream().map(String::valueOf) + .collect(Collectors.joining(",")), String.join(",", sourceKeys)); + SimpleJobBuilder builder = createJob(compositeJobName, allJobSteps); runJob(source.getSourceKey(), source.getSourceId(), jobName, builder); } @@ -503,9 +540,8 @@ private void warmCaches(Collection sources) { } } - private SimpleJobBuilder createJob(String sourceIds, String sourceKeys, List steps) { + private SimpleJobBuilder createJob(String jobName, List steps) { final SimpleJobBuilder[] stepBuilder = {null}; - String jobName = getWarmCacheJobName(sourceIds, sourceKeys); if (jobService.findJobByName(jobName, jobName) == null && !steps.isEmpty()) { JobBuilder jobBuilder = jobBuilders.get(jobName); @@ -576,11 +612,15 @@ private int getResultsDaimonPriority(Source source) { } private String getWarmCacheJobName(String sourceIds, String sourceKeys) { + return getJobName("warming cache", sourceIds, sourceKeys); + } + + private String getJobName(String jobType, String sourceIds, String sourceKeys) { // for multiple sources: try to compose a job name from source keys, and if it is too long - use source ids - String jobName = String.format("warming cache: %s", sourceKeys); + String jobName = String.format("%s: %s", jobType, sourceKeys); if (jobName.length() >= 100) { // job name in batch_job_instance is varchar(100) - jobName = String.format("warming cache: %s", sourceIds); + jobName = String.format("%s: %s", jobType, sourceIds); if (jobName.length() >= 100) { // if we still have more than 100 symbols jobName = jobName.substring(0, 88); diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20241113115700__add_clearcache_permission.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20241113115700__add_clearcache_permission.sql new file mode 100644 index 0000000000..080e455933 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20241113115700__add_clearcache_permission.sql @@ -0,0 +1,29 @@ +INSERT INTO ${ohdsiSchema}.sec_permission (id, value, description) +SELECT nextval('${ohdsiSchema}.sec_permission_id_seq'), + 'cdmresults:clearcache:post', + 'Clear the Achilles and CDM results caches'; + +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 + ( + 'cdmresults:clearcache:post' + ) + AND sr.name IN ('admin'); + +INSERT INTO ${ohdsiSchema}.sec_permission (id, value, description) +SELECT nextval('${ohdsiSchema}.sec_permission_id_seq'), + 'cdmresults:*:clearcache:post', + 'Clear the Achilles and CDM results caches'; + +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 + ( + 'cdmresults:*:clearcache:post' + ) + AND sr.name IN ('admin'); diff --git a/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java b/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java index 72635bad7a..51e0a356c5 100644 --- a/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java +++ b/src/test/java/org/ohdsi/webapi/test/CDMResultsServiceIT.java @@ -17,6 +17,8 @@ import org.ohdsi.circe.helper.ResourceHelper; import org.ohdsi.sql.SqlRender; import org.ohdsi.sql.SqlTranslate; +import org.ohdsi.webapi.achilles.service.AchillesCacheService; +import org.ohdsi.webapi.cdmresults.service.CDMCacheService; import org.ohdsi.webapi.source.SourceRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -28,9 +30,18 @@ public class CDMResultsServiceIT extends WebApiIT { @Value("${cdmResultsService.endpoint.conceptRecordCount}") private String conceptRecordCountEndpoint; + @Value("${cdmResultsService.endpoint.clearCache}") + private String clearCacheEndpoint; + @Autowired private SourceRepository sourceRepository; + @Autowired + private AchillesCacheService achillesService; + + @Autowired + private CDMCacheService cdmCacheService; + @Before public void init() throws Exception { truncateTable(String.format("%s.%s", "public", "source")); @@ -65,7 +76,7 @@ public void requestConceptRecordCounts_firstTime_returnsResults() { final ResponseEntity>>> entity = getRestTemplate().postForEntity(this.conceptRecordCountEndpoint, conceptIds, returnClass, queryParameters ); - // Assertion + // Assert assertOK(entity); List>> results = entity.getBody(); assertEquals(1, results.size()); @@ -78,4 +89,68 @@ public void requestConceptRecordCounts_firstTime_returnsResults() { assertEquals(102, counts.get(2).intValue()); assertEquals(103, counts.get(3).intValue()); } -} + + @Test + public void achillesService_clearCache_nothingInCache_doesNothing() { + + // Arrange + + // Act + achillesService.clearCache(); + + // Assert + String sql = "SELECT COUNT(*) FROM achilles_cache"; + Integer count = jdbcTemplate.queryForObject(sql, Integer.class); + assertEquals(0, count.intValue()); + } + + @Test + public void achillesService_clearCache_somethingInCache_clearsAllRowsForSource() { + + // Arrange + String insertSqlRow1 = "INSERT INTO achilles_cache (id, source_id, cache_name, cache) VALUES (1, 1, 'cache1', 'cache1')"; + jdbcTemplate.execute(insertSqlRow1); + String insertSqlRow2 = "INSERT INTO achilles_cache (id, source_id, cache_name, cache) VALUES (2, 1, 'cache2', 'cache2')"; + jdbcTemplate.execute(insertSqlRow2); + + // Act + achillesService.clearCache(); + + // Assert + String sql = "SELECT COUNT(*) FROM achilles_cache"; + Integer count = jdbcTemplate.queryForObject(sql, Integer.class); + assertEquals(0, count.intValue()); + } + + @Test + public void cdmCacheService_clearCache_nothingInCache_doesNothing() { + + // Arrange + + // Act + cdmCacheService.clearCache(); + + // Assert + String sql = "SELECT COUNT(*) FROM cdm_cache"; + Integer count = jdbcTemplate.queryForObject(sql, Integer.class); + assertEquals(0, count.intValue()); + } + + @Test + public void cdmCacheService_clearCache_somethingInCache_clearsAllRowsForSource() { + + // Arrange + String insertSqlRow1 = "INSERT INTO cdm_cache (id, concept_id, source_id, record_count, descendant_record_count, person_count, descendant_person_count) VALUES (1, 1, 1, 100, 101, 102, 103)"; + jdbcTemplate.execute(insertSqlRow1); + String insertSqlRow2 = "INSERT INTO cdm_cache (id, concept_id, source_id, record_count, descendant_record_count, person_count, descendant_person_count) VALUES (2, 2, 1, 200, 201, 202, 203)"; + jdbcTemplate.execute(insertSqlRow2); + + // Act + cdmCacheService.clearCache(); + + // Assert + String sql = "SELECT COUNT(*) FROM cdm_cache"; + Integer count = jdbcTemplate.queryForObject(sql, Integer.class); + assertEquals(0, count.intValue()); + } + } diff --git a/src/test/java/org/ohdsi/webapi/test/ITStarter.java b/src/test/java/org/ohdsi/webapi/test/ITStarter.java index 1c12bdd850..cf7593e17f 100644 --- a/src/test/java/org/ohdsi/webapi/test/ITStarter.java +++ b/src/test/java/org/ohdsi/webapi/test/ITStarter.java @@ -21,7 +21,8 @@ SecurityIT.class, JobServiceIT.class, CohortAnalysisServiceIT.class, - VocabularyServiceIT.class + VocabularyServiceIT.class, + CDMResultsServiceIT.class }) @TestPropertySource(locations = "/application-test.properties") public class ITStarter extends AbstractShiro { diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index 3d9a789b5b..6b79fe5e0e 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -11,8 +11,10 @@ vocabularyservice.endpoint.concept=${vocabularyservice.endpoint}/concept/1 #GET cohortdefinitions cohortdefinitionservice.endpoint.cohortdefinitions=${baseUri}/cohortdefinition -#POST cdmResults +#POST conceptRecordCount cdmResultsService.endpoint.conceptRecordCount=${cdmResultsService.endpoint}/{sourceName}/conceptRecordCount +#GET clearCache +cdmResultsService.endpoint.clearCache=${cdmResultsService.endpoint}/clearCache #Example application service exampleservice.endpoint=${baseUri}/example From 02a9c9fec1143fcba530ab598b4081d15813596c Mon Sep 17 00:00:00 2001 From: Anthony Sena Date: Tue, 3 Dec 2024 09:25:07 -0500 Subject: [PATCH 08/10] Add handling for Snowflake IN clause limit (#2404) --- src/main/java/org/ohdsi/webapi/util/PreparedSqlRender.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/ohdsi/webapi/util/PreparedSqlRender.java b/src/main/java/org/ohdsi/webapi/util/PreparedSqlRender.java index 910efa3f8c..b2ad03c236 100644 --- a/src/main/java/org/ohdsi/webapi/util/PreparedSqlRender.java +++ b/src/main/java/org/ohdsi/webapi/util/PreparedSqlRender.java @@ -84,7 +84,7 @@ public static int getParameterLimit(Source source) { returnVal = 990; } else if (sourceDialect.equals(DBMSType.MS_SQL_SERVER.getOhdsiDB()) || sourceDialect.equals(DBMSType.PDW.getOhdsiDB())) { returnVal = 2000; - } else if (sourceDialect.equals(DBMSType.BIGQUERY.getOhdsiDB())) { + } else if (sourceDialect.equals(DBMSType.BIGQUERY.getOhdsiDB()) || sourceDialect.equals(DBMSType.SNOWFLAKE.getOhdsiDB())) { returnVal = 10000; } return returnVal; From 67f8816961c129715409b22d654779ec28d8be58 Mon Sep 17 00:00:00 2001 From: alex-odysseus <75131044+alex-odysseus@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:17:47 +0100 Subject: [PATCH 09/10] Extending Concept Set Items with Annotations (#2403) Co-authored-by: Hernaldo Urbina Co-authored-by: hernaldo.urbina Co-authored-by: oleg-odysseus --- .../ohdsi/webapi/conceptset/ConceptSet.java | 1 + .../annotation/ConceptSetAnnotation.java | 108 ++++++ .../ConceptSetAnnotationRepository.java | 20 ++ .../model/ConceptSetPermissionSchema.java | 6 +- .../webapi/security/model/EntityType.java | 1 - .../webapi/service/AbstractDaoService.java | 7 + .../webapi/service/ConceptSetService.java | 340 +++++++++++++----- .../annotations/SearchDataTransformer.java | 100 ++++++ .../webapi/service/dto/AnnotationDTO.java | 80 +++++ .../service/dto/AnnotationDetailsDTO.java | 37 ++ .../webapi/service/dto/ConceptSetDTO.java | 1 + .../service/dto/CopyAnnotationsRequest.java | 23 ++ .../dto/SaveConceptSetAnnotationsRequest.java | 27 ++ ...20240716100000__conceptset_annotations.sql | 67 ++++ src/main/resources/i18n/messages_en.json | 10 +- src/main/resources/i18n/messages_ko.json | 7 +- src/main/resources/i18n/messages_ru.json | 7 +- src/main/resources/i18n/messages_zh.json | 7 +- .../SearchDataTransformerTest.java | 70 ++++ 19 files changed, 813 insertions(+), 106 deletions(-) create mode 100644 src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotation.java create mode 100644 src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotationRepository.java create mode 100644 src/main/java/org/ohdsi/webapi/service/annotations/SearchDataTransformer.java create mode 100644 src/main/java/org/ohdsi/webapi/service/dto/AnnotationDTO.java create mode 100644 src/main/java/org/ohdsi/webapi/service/dto/AnnotationDetailsDTO.java create mode 100644 src/main/java/org/ohdsi/webapi/service/dto/CopyAnnotationsRequest.java create mode 100644 src/main/java/org/ohdsi/webapi/service/dto/SaveConceptSetAnnotationsRequest.java create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20240716100000__conceptset_annotations.sql create mode 100644 src/test/java/org/ohdsi/webapi/service/annotations/SearchDataTransformerTest.java diff --git a/src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java b/src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java index ee49b11226..e19f97ebca 100644 --- a/src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java +++ b/src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java @@ -26,6 +26,7 @@ import javax.persistence.JoinTable; import javax.persistence.ManyToMany; import javax.persistence.Table; +import javax.persistence.OneToOne; import org.hibernate.annotations.GenericGenerator; import org.hibernate.annotations.Parameter; import org.ohdsi.webapi.model.CommonEntity; diff --git a/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotation.java b/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotation.java new file mode 100644 index 0000000000..f13fd19db7 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotation.java @@ -0,0 +1,108 @@ +package org.ohdsi.webapi.conceptset.annotation; + +import org.hibernate.annotations.GenericGenerator; +import org.hibernate.annotations.Parameter; +import org.ohdsi.webapi.model.CommonEntity; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Table; +import java.io.Serializable; + +@Entity(name = "ConceptSetAnnotation") +@Table(name = "concept_set_annotation") +public class ConceptSetAnnotation implements Serializable { + /** + * + */ + private static final long serialVersionUID = 1L; + + @Id + @GenericGenerator( + name = "concept_set_annotation_generator", + strategy = "org.hibernate.id.enhanced.SequenceStyleGenerator", + parameters = { + @Parameter(name = "sequence_name", value = "concept_set_annotation_sequence"), + @Parameter(name = "increment_size", value = "1") + } + ) + @GeneratedValue(generator = "concept_set_annotation_generator") + @Column(name = "concept_set_annotation_id") + private Integer id; + + @Column(name = "concept_set_id", nullable = false) + private Integer conceptSetId; + + @Column(name = "concept_id") + private Integer conceptId; + + @Column(name = "annotation_details") + private String annotationDetails; + + @Column(name = "vocabulary_version") + private String vocabularyVersion; + + @Column(name = "concept_set_version") + private Integer conceptSetVersion; + + @Column(name = "copied_from_concept_set_ids") + private String copiedFromConceptSetIds; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public Integer getConceptSetId() { + return conceptSetId; + } + + public void setConceptSetId(Integer conceptSetId) { + this.conceptSetId = conceptSetId; + } + + public Integer getConceptId() { + return conceptId; + } + + public void setConceptId(Integer conceptId) { + this.conceptId = conceptId; + } + + public String getAnnotationDetails() { + return annotationDetails; + } + + public void setAnnotationDetails(String annotationDetails) { + this.annotationDetails = annotationDetails; + } + + public String getVocabularyVersion() { + return vocabularyVersion; + } + + public void setVocabularyVersion(String vocabularyVersion) { + this.vocabularyVersion = vocabularyVersion; + } + + public Integer getConceptSetVersion() { + return conceptSetVersion; + } + + public void setConceptSetVersion(Integer conceptSetVersion) { + this.conceptSetVersion = conceptSetVersion; + } + + public String getCopiedFromConceptSetIds() { + return copiedFromConceptSetIds; + } + + public void setCopiedFromConceptSetIds(String copiedFromConceptSetIds) { + this.copiedFromConceptSetIds = copiedFromConceptSetIds; + } +} diff --git a/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotationRepository.java b/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotationRepository.java new file mode 100644 index 0000000000..f44bd01c72 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotationRepository.java @@ -0,0 +1,20 @@ +package org.ohdsi.webapi.conceptset.annotation; + +import java.util.List; +import java.util.Optional; + +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; + +public interface ConceptSetAnnotationRepository extends JpaRepository { + + @Query("DELETE FROM ConceptSetAnnotation cc WHERE cc.conceptSetId = :conceptSetId and cc.conceptId in :conceptId") + void deleteAnnotationByConceptSetIdAndInConceptId(int conceptSetId, List conceptId); + + void deleteAnnotationByConceptSetIdAndConceptId(int conceptSetId, int conceptId); + + List findByConceptSetId(int conceptSetId); + ConceptSetAnnotation findById(int id); + void deleteById(int id); + Optional findConceptSetAnnotationByConceptIdAndConceptId(int conceptSetId, int conceptId); +} diff --git a/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java index 66b4b1a4b2..e5d9c72cdc 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java @@ -11,17 +11,21 @@ public class ConceptSetPermissionSchema extends EntityPermissionSchema { private static Map writePermissions = new HashMap() {{ put("conceptset:%s:put", "Update Concept Set with ID = %s"); put("conceptset:%s:items:put", "Update Items of Concept Set with ID = %s"); + put("conceptset:*:annotation:put", "Create Concept Set Annotation"); + put("conceptset:%s:annotation:*:delete", "Delete Annotations of Concept Set with ID = %s"); + put("conceptset:*:annotation:*:delete", "Delete Annotations of any Concept Set"); put("conceptset:%s:delete", "Delete Concept Set with ID = %s"); }}; private static Map readPermissions = new HashMap() {{ put("conceptset:%s:get", "view conceptset definition with id %s"); put("conceptset:%s:expression:get", "Resolve concept set %s expression"); + put("conceptset:%s:annotation:get", "Resolve concept set annotations"); + put("conceptset:*:annotation:get", "Resolve concept set annotations"); put("conceptset:%s:version:*:expression:get", "Get expression for concept set %s items for default source"); }}; public ConceptSetPermissionSchema() { - super(EntityType.CONCEPT_SET, readPermissions, writePermissions); } } diff --git a/src/main/java/org/ohdsi/webapi/security/model/EntityType.java b/src/main/java/org/ohdsi/webapi/security/model/EntityType.java index d5a9d63acf..c294cdb17a 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/EntityType.java +++ b/src/main/java/org/ohdsi/webapi/security/model/EntityType.java @@ -27,7 +27,6 @@ public enum EntityType { COHORT_SAMPLE(CohortSample.class), TAG(Tag.class), REUSABLE(Reusable.class); - private final Class entityClass; EntityType(Class entityClass) { diff --git a/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java b/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java index 95277cbe68..b84ef71890 100644 --- a/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java +++ b/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java @@ -20,6 +20,7 @@ import org.ohdsi.webapi.conceptset.ConceptSetComparison; import org.ohdsi.webapi.conceptset.ConceptSetItemRepository; import org.ohdsi.webapi.conceptset.ConceptSetRepository; +import org.ohdsi.webapi.conceptset.annotation.ConceptSetAnnotationRepository; import org.ohdsi.webapi.exception.BadRequestAtlasException; import org.ohdsi.webapi.ircalc.IncidenceRateAnalysis; import org.ohdsi.webapi.model.CommonEntity; @@ -106,6 +107,9 @@ public abstract class AbstractDaoService extends AbstractAdminService { @Autowired private ConceptSetItemRepository conceptSetItemRepository; + @Autowired + private ConceptSetAnnotationRepository conceptSetAnnotationRepository; + @Autowired protected Security security; @@ -120,6 +124,9 @@ public abstract class AbstractDaoService extends AbstractAdminService { public ConceptSetItemRepository getConceptSetItemRepository() { return conceptSetItemRepository; } + public ConceptSetAnnotationRepository getConceptSetAnnotationRepository() { + return conceptSetAnnotationRepository; + } @Autowired private ConceptSetRepository conceptSetRepository; diff --git a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java index 2f80ef991e..84bc8e7787 100644 --- a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java +++ b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java @@ -25,6 +25,9 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; + import org.apache.shiro.authz.UnauthorizedException; import org.ohdsi.circe.vocabulary.ConceptSetExpression; import org.ohdsi.vocabulary.Concept; @@ -36,9 +39,15 @@ import org.ohdsi.webapi.conceptset.ConceptSetGenerationInfoRepository; import org.ohdsi.webapi.conceptset.ConceptSetItem; import org.ohdsi.webapi.conceptset.dto.ConceptSetVersionFullDTO; +import org.ohdsi.webapi.conceptset.annotation.ConceptSetAnnotation; import org.ohdsi.webapi.exception.ConceptNotExistException; import org.ohdsi.webapi.security.PermissionService; +import org.ohdsi.webapi.service.annotations.SearchDataTransformer; +import org.ohdsi.webapi.service.dto.AnnotationDetailsDTO; import org.ohdsi.webapi.service.dto.ConceptSetDTO; +import org.ohdsi.webapi.service.dto.SaveConceptSetAnnotationsRequest; +import org.ohdsi.webapi.service.dto.AnnotationDTO; +import org.ohdsi.webapi.service.dto.CopyAnnotationsRequest; import org.ohdsi.webapi.shiro.Entities.UserEntity; import org.ohdsi.webapi.shiro.Entities.UserRepository; import org.ohdsi.webapi.shiro.management.Security; @@ -64,12 +73,12 @@ import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.stereotype.Component; - /** - * Provides REST services for working with - * concept sets. - * - * @summary Concept Set - */ +/** + * Provides REST services for working with + * concept sets. + * + * @summary Concept Set + */ @Component @Transactional @Path("/conceptset/") @@ -105,14 +114,21 @@ public class ConceptSetService extends AbstractDaoService implements HasTags versionService; + @Autowired + private SearchDataTransformer searchDataTransformer; + + @Autowired + private ObjectMapper mapper; + + @Value("${security.defaultGlobalReadPermissions}") private boolean defaultGlobalReadPermissions; - + public static final String COPY_NAME = "copyName"; /** * Get the concept set based in the identifier - * + * * @summary Get concept set by ID * @param id The concept set ID * @return The concept set definition @@ -128,7 +144,7 @@ public ConceptSetDTO getConceptSet(@PathParam("id") final int id) { /** * Get the full list of concept sets in the WebAPI database - * + * * @summary Get all concept sets * @return A list of all concept sets in the WebAPI database */ @@ -151,7 +167,7 @@ public Collection getConceptSets() { /** * Get the concept set items for a selected concept set ID. - * + * * @summary Get the concept set items * @param id The concept set identifier * @return A list of concept set items @@ -165,7 +181,7 @@ public Iterable getConceptSetItems(@PathParam("id") final int id /** * Get the concept set expression for a selected version of the expression - * + * * @summary Get concept set expression by version * @param id The concept set ID * @param version The version identifier @@ -188,7 +204,7 @@ public ConceptSetExpression getConceptSetExpression(@PathParam("id") final int i * source key. NOTE: This method requires the specification * of a source key but it does not appear to be used by the underlying * code. - * + * * @summary Get concept set expression by version and source. * @param id The concept set identifier * @param version The version of the concept set @@ -210,7 +226,7 @@ public ConceptSetExpression getConceptSetExpression(@PathParam("id") final int i /** * Get the concept set expression by identifier - * + * * @summary Get concept set by ID * @param id The concept set identifier * @return The concept set expression @@ -228,7 +244,7 @@ public ConceptSetExpression getConceptSetExpression(@PathParam("id") final int i /** * Get the concept set expression by identifier and source key - * + * * @summary Get concept set by ID and source * @param id The concept set ID * @param sourceKey The source key @@ -288,26 +304,26 @@ private ConceptSetExpression getConceptSetExpression(int id, Integer version, So throw new ConceptNotExistException("Current data source does not contain required concepts " + ids); } for(Concept concept : concepts) { - map.put(concept.conceptId, concept); // associate the concept object to the conceptID in the map + map.put(concept.conceptId, concept); // associate the concept object to the conceptID in the map } - // put the concept information into the expression along with the concept set item information + // put the concept information into the expression along with the concept set item information for (ConceptSetItem repositoryItem : repositoryItems) { - ConceptSetExpression.ConceptSetItem currentItem = new ConceptSetExpression.ConceptSetItem(); - currentItem.concept = map.get(repositoryItem.getConceptId()); - currentItem.includeDescendants = (repositoryItem.getIncludeDescendants() == 1); - currentItem.includeMapped = (repositoryItem.getIncludeMapped() == 1); - currentItem.isExcluded = (repositoryItem.getIsExcluded() == 1); - expressionItems.add(currentItem); + ConceptSetExpression.ConceptSetItem currentItem = new ConceptSetExpression.ConceptSetItem(); + currentItem.concept = map.get(repositoryItem.getConceptId()); + currentItem.includeDescendants = (repositoryItem.getIncludeDescendants() == 1); + currentItem.includeMapped = (repositoryItem.getIncludeMapped() == 1); + currentItem.isExcluded = (repositoryItem.getIsExcluded() == 1); + expressionItems.add(currentItem); } expression.items = expressionItems.toArray(new ConceptSetExpression.ConceptSetItem[0]); // this will return a new array - + return expression; } /** * Check if the concept set name exists (DEPRECATED) - * + * * @summary DO NOT USE * @deprecated * @param id The concept set ID @@ -328,11 +344,11 @@ public Response getConceptSetExistsDeprecated(@PathParam("id") final int id, @Pa * Check if a concept set with the same name exists in the WebAPI * database. The name is checked against the selected concept set ID * to ensure that only the selected concept set ID has the name specified. - * + * * @summary Concept set with same name exists * @param id The concept set ID * @param name The name of the concept set - * @return The count of concept sets with the name, excluding the + * @return The count of concept sets with the name, excluding the * specified concept set ID. */ @GET @@ -345,11 +361,11 @@ public int getCountCSetWithSameName(@PathParam("id") @DefaultValue("0") final in /** * Update the concept set items for the selected concept set ID in the * WebAPI database. - * + * * The concept set has two parts: 1) the elements of the ConceptSetDTO that - * consist of the identifier, name, etc. 2) the concept set items which + * consist of the identifier, name, etc. 2) the concept set items which * contain the concepts and their mapping (i.e. include descendants). - * + * * @summary Update concept set items * @param id The concept set ID * @param items An array of ConceptSetItems @@ -376,12 +392,12 @@ public boolean saveConceptSetItems(@PathParam("id") final int id, ConceptSetItem * Exports a list of concept sets, based on the conceptSetList argument, * to one or more comma separated value (CSV) file(s), compresses the files * into a ZIP file and sends the ZIP file to the client. - * + * * @summary Export concept set list to CSV files * @param conceptSetList A list of concept set identifiers in the format * conceptset=++ * @return - * @throws Exception + * @throws Exception */ @GET @Path("/exportlist") @@ -411,7 +427,7 @@ public Response exportConceptSetList(@QueryParam("conceptsets") final String con // Get the concept set information cs.add(getConceptSetForExport(conceptSetIds.get(i), new SourceInfo(source))); } - // Write Concept Set Expression to a CSV + // Write Concept Set Expression to a CSV baos = ExportUtil.writeConceptSetExportToCSVAndZip(cs); response = Response @@ -427,12 +443,12 @@ public Response exportConceptSetList(@QueryParam("conceptsets") final String con } /** - * Exports a single concept set to a comma separated value (CSV) + * Exports a single concept set to a comma separated value (CSV) * file, compresses to a ZIP file and sends to the client. * @param id The concept set ID * @return A zip file containing the exported concept set - * @throws Exception + * @throws Exception */ @GET @Path("{id}/export") @@ -444,7 +460,7 @@ public Response exportConceptSetToCSV(@PathParam("id") final String id) throws E /** * Save a new concept set to the WebAPI database - * + * * @summary Create a new concept set * @param conceptSetDTO The concept set to save * @return The concept set saved with the concept set identifier @@ -470,7 +486,7 @@ public ConceptSetDTO createConceptSet(ConceptSetDTO conceptSetDTO) { * that is used when generating a copy of an existing concept set. This * function is generally used in conjunction with the copy endpoint to * create a unique name and then save a copy of an existing concept set. - * + * * @sumamry Get concept set name suggestion for copying * @param id The concept set ID * @return A map of the new concept set name and the existing concept set @@ -492,17 +508,17 @@ public List getNamesLike(String copyName) { /** * Updates the concept set for the selected concept set. - * + * * The concept set has two parts: 1) the elements of the ConceptSetDTO that - * consist of the identifier, name, etc. 2) the concept set items which + * consist of the identifier, name, etc. 2) the concept set items which * contain the concepts and their mapping (i.e. include descendants). - * + * * @summary Update concept set * @param id The concept set identifier * @param conceptSetDTO The concept set header * @return The - * @throws Exception - */ + * @throws Exception + */ @Path("/{id}") @PUT @Consumes(MediaType.APPLICATION_JSON) @@ -512,7 +528,7 @@ public ConceptSetDTO updateConceptSet(@PathParam("id") final int id, ConceptSetD ConceptSet updated = getConceptSetRepository().findById(id); if (updated == null) { - throw new Exception("Concept Set does not exist."); + throw new Exception("Concept Set does not exist."); } saveVersion(id); @@ -528,11 +544,11 @@ private ConceptSet updateConceptSet(ConceptSet dst, ConceptSet src) { dst.setDescription(src.getDescription()); dst.setModifiedDate(new Date()); dst.setModifiedBy(user); - + dst = this.getConceptSetRepository().save(dst); return dst; } - + private ConceptSetExport getConceptSetForExport(int conceptSetId, SourceInfo vocabSource) { ConceptSetExport cs = new ConceptSetExport(); @@ -556,63 +572,63 @@ private ConceptSetExport getConceptSetForExport(int conceptSetId, SourceInfo voc * Get the concept set generation information for the selected concept * set ID. This function only works with the configuration of the CEM * data source. - * + * * @link https://github.com/OHDSI/CommonEvidenceModel/wiki - * + * * @summary Get concept set generation info * @param id The concept set identifier. * @return A collection of concept set generation info objects */ - @GET - @Path("{id}/generationinfo") - @Produces(MediaType.APPLICATION_JSON) - public Collection getConceptSetGenerationInfo(@PathParam("id") final int id) { - return this.conceptSetGenerationInfoRepository.findAllByConceptSetId(id); - } - - /** - * Delete the selected concept set by concept set identifier - * - * @summary Delete concept set - * @param id The concept set ID - */ - @DELETE - @Transactional(rollbackOn = Exception.class, dontRollbackOn = EmptyResultDataAccessException.class) - @Path("{id}") - public void deleteConceptSet(@PathParam("id") final int id) { - // Remove any generation info - try { - this.conceptSetGenerationInfoRepository.deleteByConceptSetId(id); - } catch (EmptyResultDataAccessException e) { - // Ignore - there may be no data - log.warn("Failed to delete Generation Info by ConceptSet with ID = {}, {}", id, e); - } - catch (Exception e) { - throw e; - } - - // Remove the concept set items - try { - getConceptSetItemRepository().deleteByConceptSetId(id); - } catch (EmptyResultDataAccessException e) { - // Ignore - there may be no data - log.warn("Failed to delete ConceptSet items with ID = {}, {}", id, e); - } - catch (Exception e) { - throw e; - } - - // Remove the concept set - try { - getConceptSetRepository().delete(id); - } catch (EmptyResultDataAccessException e) { - // Ignore - there may be no data - log.warn("Failed to delete ConceptSet with ID = {}, {}", id, e); - } - catch (Exception e) { - throw e; - } - } + @GET + @Path("{id}/generationinfo") + @Produces(MediaType.APPLICATION_JSON) + public Collection getConceptSetGenerationInfo(@PathParam("id") final int id) { + return this.conceptSetGenerationInfoRepository.findAllByConceptSetId(id); + } + + /** + * Delete the selected concept set by concept set identifier + * + * @summary Delete concept set + * @param id The concept set ID + */ + @DELETE + @Transactional(rollbackOn = Exception.class, dontRollbackOn = EmptyResultDataAccessException.class) + @Path("{id}") + public void deleteConceptSet(@PathParam("id") final int id) { + // Remove any generation info + try { + this.conceptSetGenerationInfoRepository.deleteByConceptSetId(id); + } catch (EmptyResultDataAccessException e) { + // Ignore - there may be no data + log.warn("Failed to delete Generation Info by ConceptSet with ID = {}, {}", id, e); + } + catch (Exception e) { + throw e; + } + + // Remove the concept set items + try { + getConceptSetItemRepository().deleteByConceptSetId(id); + } catch (EmptyResultDataAccessException e) { + // Ignore - there may be no data + log.warn("Failed to delete ConceptSet items with ID = {}, {}", id, e); + } + catch (Exception e) { + throw e; + } + + // Remove the concept set + try { + getConceptSetRepository().delete(id); + } catch (EmptyResultDataAccessException e) { + // Ignore - there may be no data + log.warn("Failed to delete ConceptSet with ID = {}, {}", id, e); + } + catch (Exception e) { + throw e; + } + } /** * Assign tag to Concept Set @@ -683,10 +699,10 @@ public void unassignPermissionProtectedTag(@PathParam("id") final int id, @PathP } /** - * Checks a concept set for diagnostic problems. At this time, + * Checks a concept set for diagnostic problems. At this time, * this appears to be an endpoint used to check to see which tags * are applied to a concept set. - * + * * @summary Concept set tag check * @since v2.10.0 * @param conceptSetDTO The concept set @@ -857,4 +873,130 @@ private ConceptSetVersion saveVersion(int id) { version.setCreatedDate(versionDate); return versionService.create(VersionType.CONCEPT_SET, version); } -} + + /** + * Update the concept set annotation for each concept in concept set ID in the + * WebAPI database. + *

+ * The body has two parts: 1) the elements new concept which added to the + * concept set. 2) the elements concept which remove from concept set. + * + * @param conceptSetId The concept set ID + * @param request An object of 2 Array new annotation and remove annotation + * @return Boolean: true if the save is successful + * @summary Create new or delete concept set annotation items + */ + @PUT + @Path("/{id}/annotation") + @Produces(MediaType.APPLICATION_JSON) + @Transactional + public boolean saveConceptSetAnnotation(@PathParam("id") final int conceptSetId, SaveConceptSetAnnotationsRequest request) { + removeAnnotations(conceptSetId, request); + if (request.getNewAnnotation() != null && !request.getNewAnnotation().isEmpty()) { + List annotationList = request.getNewAnnotation() + .stream() + .map(newAnnotationData -> { + ConceptSetAnnotation conceptSetAnnotation = new ConceptSetAnnotation(); + conceptSetAnnotation.setConceptSetId(conceptSetId); + try { + AnnotationDetailsDTO annotationDetailsDTO = new AnnotationDetailsDTO(); + annotationDetailsDTO.setId(newAnnotationData.getId()); + annotationDetailsDTO.setConceptId(newAnnotationData.getConceptId()); + annotationDetailsDTO.setSearchData(newAnnotationData.getSearchData()); + conceptSetAnnotation.setAnnotationDetails(mapper.writeValueAsString(annotationDetailsDTO)); + } catch (JsonProcessingException e) { + log.error("Could not serialize Concept Set AnnotationDetailsDTO", e); + throw new RuntimeException(e); + } + conceptSetAnnotation.setVocabularyVersion(newAnnotationData.getVocabularyVersion()); + conceptSetAnnotation.setConceptSetVersion(newAnnotationData.getConceptSetVersion()); + conceptSetAnnotation.setConceptId(newAnnotationData.getConceptId()); + return conceptSetAnnotation; + }).collect(Collectors.toList()); + + this.getConceptSetAnnotationRepository().save(annotationList); + } + + return true; + } + private void removeAnnotations(int id, SaveConceptSetAnnotationsRequest request){ + if (request.getRemoveAnnotation() != null && !request.getRemoveAnnotation().isEmpty()) { + for (AnnotationDTO annotationDTO : request.getRemoveAnnotation()) { + this.getConceptSetAnnotationRepository().deleteAnnotationByConceptSetIdAndConceptId(id, annotationDTO.getConceptId()); + } + } + } + @POST + @Path("/copy-annotations") + @Produces(MediaType.APPLICATION_JSON) + @Transactional + public void copyAnnotations(CopyAnnotationsRequest copyAnnotationsRequest ) { + List sourceAnnotations = getConceptSetAnnotationRepository().findByConceptSetId(copyAnnotationsRequest.getSourceConceptSetId()); + List copiedAnnotations= sourceAnnotations.stream() + .map(sourceAnnotation -> copyAnnotation(sourceAnnotation, copyAnnotationsRequest.getSourceConceptSetId(), copyAnnotationsRequest.getTargetConceptSetId())) + .collect(Collectors.toList()); + getConceptSetAnnotationRepository().save(copiedAnnotations); + } + private ConceptSetAnnotation copyAnnotation(ConceptSetAnnotation sourceConceptSetAnnotation, int sourceConceptSetId, int targetConceptSetId){ + ConceptSetAnnotation targetConceptSetAnnotation = new ConceptSetAnnotation(); + targetConceptSetAnnotation.setConceptSetId(targetConceptSetId); + targetConceptSetAnnotation.setConceptSetVersion(sourceConceptSetAnnotation.getConceptSetVersion()); + targetConceptSetAnnotation.setAnnotationDetails(sourceConceptSetAnnotation.getAnnotationDetails()); + targetConceptSetAnnotation.setConceptId(sourceConceptSetAnnotation.getConceptId()); + targetConceptSetAnnotation.setVocabularyVersion(sourceConceptSetAnnotation.getVocabularyVersion()); + targetConceptSetAnnotation.setCopiedFromConceptSetIds(appendCopiedFromConceptSetId(sourceConceptSetAnnotation.getCopiedFromConceptSetIds(), sourceConceptSetId)); + return targetConceptSetAnnotation; + } + private String appendCopiedFromConceptSetId(String copiedFromConceptSetIds, int sourceConceptSetId) { + if(copiedFromConceptSetIds == null || copiedFromConceptSetIds.isEmpty()){ + return Integer.toString(sourceConceptSetId); + } + return copiedFromConceptSetIds.concat(",").concat(Integer.toString(sourceConceptSetId)); + } + + @GET + @Path("/{id}/annotation") + @Produces(MediaType.APPLICATION_JSON) + public List getConceptSetAnnotation(@PathParam("id") final int id) { + List annotationList = getConceptSetAnnotationRepository().findByConceptSetId(id); + return annotationList.stream() + .map(this::convertAnnotationEntityToDTO) + .collect(Collectors.toList()); + } + + + private AnnotationDTO convertAnnotationEntityToDTO(ConceptSetAnnotation conceptSetAnnotation) { + AnnotationDetailsDTO annotationDetails; + try { + annotationDetails = mapper.readValue(conceptSetAnnotation.getAnnotationDetails(), AnnotationDetailsDTO.class); + } catch (JsonProcessingException e) { + log.error("Could not deserialize Concept Set AnnotationDetailsDTO", e); + throw new RuntimeException(e); + } + + AnnotationDTO annotationDTO = new AnnotationDTO(); + + annotationDTO.setId(conceptSetAnnotation.getId()); + annotationDTO.setConceptId(conceptSetAnnotation.getConceptId()); + + String searchDataJSON = annotationDetails.getSearchData(); + String humanReadableData = searchDataTransformer.convertJsonToReadableFormat(searchDataJSON); + annotationDTO.setSearchData(humanReadableData); + + annotationDTO.setVocabularyVersion(conceptSetAnnotation.getVocabularyVersion()); + annotationDTO.setConceptSetVersion(conceptSetAnnotation.getConceptSetVersion()); + annotationDTO.setCopiedFromConceptSetIds(conceptSetAnnotation.getCopiedFromConceptSetIds()); + return annotationDTO; + } + + @DELETE + @Path("/{conceptSetId}/annotation/{annotationId}") + @Produces(MediaType.APPLICATION_JSON) + public Response deleteConceptSetAnnotation(@PathParam("conceptSetId") final int conceptSetId, @PathParam("annotationId") final int annotationId) { + ConceptSetAnnotation conceptSetAnnotation = getConceptSetAnnotationRepository().findById(annotationId); + if (conceptSetAnnotation != null) { + getConceptSetAnnotationRepository().deleteById(annotationId); + return Response.ok().build(); + } else throw new NotFoundException("Concept set annotation not found"); + } +} \ No newline at end of file diff --git a/src/main/java/org/ohdsi/webapi/service/annotations/SearchDataTransformer.java b/src/main/java/org/ohdsi/webapi/service/annotations/SearchDataTransformer.java new file mode 100644 index 0000000000..e013adfb0f --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/service/annotations/SearchDataTransformer.java @@ -0,0 +1,100 @@ +package org.ohdsi.webapi.service.annotations; + +import org.apache.commons.lang3.StringUtils; +import org.json.JSONArray; +import org.json.JSONObject; +import org.springframework.stereotype.Service; + +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +@Service +public class SearchDataTransformer { + + private static final String FILTER_DATA = "filterData"; + private static final String TITLE = "title"; + private static final String VALUE = "value"; + private static final String KEY = "key"; + private static final String FILTER_SOURCE = "filterSource"; + private static final String FILTER_SOURCE_LABEL = "Filtered By"; + private static final String SEARCH_TEXT = "searchText"; + private static final String DEFAULT_FILTER_SOURCE = "Search"; + private static final String DELIMITER = ", "; + private static final String ENTRY_FORMAT = "%s: \"%s\""; + + public String convertJsonToReadableFormat(String jsonInput) { + JSONObject searchObject = new JSONObject(Optional.ofNullable(jsonInput).orElse("{}")); + + if (searchObject.isEmpty()) { + return ""; + } + + StringBuilder result = new StringBuilder(); + + String filterSource = processFilterSource(searchObject); + append(result, getDefaultOrActual(filterSource, DEFAULT_FILTER_SOURCE)); + + JSONObject filterDataObject = searchObject.optJSONObject(FILTER_DATA); + JSONArray filterDataArray = searchObject.optJSONArray(FILTER_DATA); + + if (filterDataObject != null) { + Optional.ofNullable(filterDataObject).map(this::processSearchText).ifPresent(searchText -> appendCommaSeparated(result, formatQuoted(searchText))); + Optional.ofNullable(filterDataObject.optJSONArray("filterColumns")).map(this::formatKeyValuePairs).ifPresent( + fdResult -> appendCommaSeparated(result, FILTER_SOURCE_LABEL + ": \"" + fdResult + "\"") + ); + } else if (filterDataArray != null) { + String extractedData = formatKeyValuePairs(filterDataArray); + if (!extractedData.isEmpty()) { + appendCommaSeparated(result, FILTER_SOURCE_LABEL + ": \"" + extractedData + "\""); + } + } + + return result.toString().trim(); + } + + private String processFilterSource(JSONObject jsonObject) { + return jsonObject.optString(FILTER_SOURCE, ""); + } + + private String processSearchText(JSONObject filterData) { + return filterData.optString(SEARCH_TEXT, ""); + } + + private String formatKeyValuePairs(JSONArray filterDataArray) { + return IntStream.range(0, filterDataArray.length()) + .mapToObj(index -> formatEntry(filterDataArray.getJSONObject(index))) + .collect(Collectors.joining(DELIMITER)); + } + + private String formatEntry(JSONObject item) { + String title = optString(item, TITLE); + String key = StringUtils.unwrap(optString(item, KEY), '"'); + return String.format(ENTRY_FORMAT, title, key); + } + + private void appendCommaSeparated(StringBuilder builder, String part) { + if (!part.isEmpty()) { + append(builder, part); + } + } + + private void append(StringBuilder builder, String part) { + if (builder.length() > 0) { + builder.append(DELIMITER); + } + builder.append(part); + } + + private String optString(JSONObject item, String key) { + return item.optString(key, ""); + } + + private String getDefaultOrActual(String actual, String defaultVal) { + return actual.isEmpty() ? defaultVal : actual; + } + + private String formatQuoted(String text) { + return String.format("\"%s\"", text); + } +} \ No newline at end of file diff --git a/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDTO.java b/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDTO.java new file mode 100644 index 0000000000..0ed0808034 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDTO.java @@ -0,0 +1,80 @@ +package org.ohdsi.webapi.service.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +@JsonIgnoreProperties(ignoreUnknown = true) +public class AnnotationDTO { + + private Integer id; + private String createdBy; + private String createdDate; + private String vocabularyVersion; + private Integer conceptSetVersion; + private String searchData; + private String copiedFromConceptSetIds; + private Integer conceptId; + + public String getCreatedBy() { + return createdBy; + } + + public void setCreatedBy(String createdBy) { + this.createdBy = createdBy; + } + + public String getCreatedDate() { + return createdDate; + } + + public void setCreatedDate(String createdDate) { + this.createdDate = createdDate; + } + + public String getVocabularyVersion() { + return vocabularyVersion; + } + + public void setVocabularyVersion(String vocabularyVersion) { + this.vocabularyVersion = vocabularyVersion; + } + + public Integer getConceptSetVersion() { + return conceptSetVersion; + } + + public void setConceptSetVersion(Integer conceptSetVersion) { + this.conceptSetVersion = conceptSetVersion; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getSearchData() { + return searchData; + } + + public void setSearchData(String searchData) { + this.searchData = searchData; + } + + public Integer getConceptId() { + return conceptId; + } + + public void setConceptId(Integer conceptId) { + this.conceptId = conceptId; + } + + public String getCopiedFromConceptSetIds() { + return copiedFromConceptSetIds; + } + + public void setCopiedFromConceptSetIds(String copiedFromConceptSetIds) { + this.copiedFromConceptSetIds = copiedFromConceptSetIds; + } +} diff --git a/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDetailsDTO.java b/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDetailsDTO.java new file mode 100644 index 0000000000..d62acb2493 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/service/dto/AnnotationDetailsDTO.java @@ -0,0 +1,37 @@ +package org.ohdsi.webapi.service.dto; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +@JsonIgnoreProperties(ignoreUnknown = true) +public class AnnotationDetailsDTO { + private Integer id; + + private String searchData; + + private Integer conceptId; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getSearchData() { + return searchData; + } + + public void setSearchData(String searchData) { + this.searchData = searchData; + } + + public Integer getConceptId() { + return conceptId; + } + + public void setConceptId(Integer conceptId) { + this.conceptId = conceptId; + } + +} diff --git a/src/main/java/org/ohdsi/webapi/service/dto/ConceptSetDTO.java b/src/main/java/org/ohdsi/webapi/service/dto/ConceptSetDTO.java index e91993332d..1323d338ed 100644 --- a/src/main/java/org/ohdsi/webapi/service/dto/ConceptSetDTO.java +++ b/src/main/java/org/ohdsi/webapi/service/dto/ConceptSetDTO.java @@ -29,4 +29,5 @@ public String getDescription() { public void setDescription(String description) { this.description = description; } + } diff --git a/src/main/java/org/ohdsi/webapi/service/dto/CopyAnnotationsRequest.java b/src/main/java/org/ohdsi/webapi/service/dto/CopyAnnotationsRequest.java new file mode 100644 index 0000000000..1d9313b5e9 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/service/dto/CopyAnnotationsRequest.java @@ -0,0 +1,23 @@ +package org.ohdsi.webapi.service.dto; + +public class CopyAnnotationsRequest { + + private int sourceConceptSetId; + private int targetConceptSetId; + + public int getSourceConceptSetId() { + return sourceConceptSetId; + } + + public int getTargetConceptSetId() { + return targetConceptSetId; + } + + public void setSourceConceptSetId(int sourceConceptSetId) { + this.sourceConceptSetId = sourceConceptSetId; + } + + public void setTargetConceptSetId(int targetConceptSetId) { + this.targetConceptSetId = targetConceptSetId; + } +} diff --git a/src/main/java/org/ohdsi/webapi/service/dto/SaveConceptSetAnnotationsRequest.java b/src/main/java/org/ohdsi/webapi/service/dto/SaveConceptSetAnnotationsRequest.java new file mode 100644 index 0000000000..3d8469ab99 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/service/dto/SaveConceptSetAnnotationsRequest.java @@ -0,0 +1,27 @@ +package org.ohdsi.webapi.service.dto; + +import java.util.List; + +public class SaveConceptSetAnnotationsRequest { + + private List newAnnotation; + + private List removeAnnotation; + + public List getNewAnnotation() { + return newAnnotation; + } + + public void setNewAnnotation(List newAnnotation) { + this.newAnnotation = newAnnotation; + } + + public List getRemoveAnnotation() { + return removeAnnotation; + } + + public void setRemoveAnnotation(List removeAnnotation) { + this.removeAnnotation = removeAnnotation; + } + +} diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20240716100000__conceptset_annotations.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20240716100000__conceptset_annotations.sql new file mode 100644 index 0000000000..04da4e9153 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20240716100000__conceptset_annotations.sql @@ -0,0 +1,67 @@ +CREATE SEQUENCE ${ohdsiSchema}.concept_set_annotation_sequence; + +CREATE TABLE ${ohdsiSchema}.concept_set_annotation +( + concept_set_annotation_id int4 NOT NULL DEFAULT nextval('${ohdsiSchema}.concept_set_annotation_sequence'), + concept_set_id integer NOT NULL, + concept_id integer, + annotation_details VARCHAR, + vocabulary_version VARCHAR, + created_by_id INTEGER, + created_date TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT (now()), + modified_by_id INTEGER, + modified_date TIMESTAMP WITH TIME ZONE, + concept_set_version VARCHAR(100), + copied_from_concept_set_ids VARCHAR(1000), + CONSTRAINT pk_concept_set_annotation_id PRIMARY KEY (concept_set_annotation_id), + CONSTRAINT fk_concept_set FOREIGN KEY (concept_set_id) + REFERENCES ${ohdsiSchema}.concept_set (concept_set_id) + ON DELETE CASCADE +); + +DELETE FROM ${ohdsiSchema}.sec_role_permission +WHERE permission_id IN ( + SELECT id FROM ${ohdsiSchema}.sec_permission + WHERE value like '%:annotation:%' +); + +DELETE FROM ${ohdsiSchema}.sec_permission +WHERE value like '%:annotation:%'; + +INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES + (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'conceptset:*:annotation:put', 'Create Concept Set Annotation'); + +INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES + (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'conceptset:%s:annotation:get', 'List Concept Set Annotations'); + +INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES + (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'conceptset:*:annotation:get', 'View Concept Set Annotation'); + +INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES + (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'conceptset:%s:annotation:*:delete', 'Delete Owner`s Concept Set Annotations'); + +INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES + (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'conceptset:*:annotation:*:delete', 'Delete Any Concept Set Annotation'); + +INSERT INTO ${ohdsiSchema}.sec_role_permission(id, role_id, permission_id) +SELECT nextval('${ohdsiSchema}.sec_role_permission_sequence'), sr.id, sp.id +FROM ${ohdsiSchema}.sec_permission SP, ${ohdsiSchema}.sec_role sr +WHERE sp.value IN ( + 'conceptset:*:annotation:put', + 'conceptset:*:annotation:*:delete', + 'conceptset:%s:annotation:*:delete', + 'conceptset:%s:annotation:get', + 'conceptset:*:annotation:get' + ) AND sr.name IN ('admin'); + +INSERT INTO ${ohdsiSchema}.sec_role_permission(id, role_id, permission_id) +SELECT nextval('${ohdsiSchema}.sec_role_permission_sequence'), sr.id, sp.id +FROM ${ohdsiSchema}.sec_permission SP, ${ohdsiSchema}.sec_role sr +WHERE sp.value IN ( + 'conceptset:*:annotation:put', + 'conceptset:%s:annotation:*:delete', + 'conceptset:%s:annotation:get', + 'conceptset:*:annotation:get' + ) AND sr.name IN ('Atlas users'); + +ALTER TABLE ${ohdsiSchema}.concept_set_annotation ALTER COLUMN concept_set_version TYPE INTEGER USING (concept_set_version::integer); \ No newline at end of file diff --git a/src/main/resources/i18n/messages_en.json b/src/main/resources/i18n/messages_en.json index 5a247d4bb4..e4ea6edf60 100644 --- a/src/main/resources/i18n/messages_en.json +++ b/src/main/resources/i18n/messages_en.json @@ -1408,6 +1408,7 @@ "notPrevalent": "Not Prevalent", "optimizedOut": "Optimized Out", "options": "Options", + "originConceptSets": "Origin Concept Sets", "outcomeCohortName": "Outcome Cohort Name", "outcomeId": "Outcome Id", "outcomeModel": "Outcome Model", @@ -1521,7 +1522,11 @@ "rcTooltip": "Record Count", "drcTooltip": "Descendant Record Count", "pcTooltip": "Person Count", - "dpcTooltip": "Descendant Person Count" + "dpcTooltip": "Descendant Person Count", + "conceptID": "Concept Id", + "searchData": "Search Data", + "createdBy": "Created By", + "createdDate": "Created Date" }, "dataSources": { "const": { @@ -2446,7 +2451,8 @@ "alerts": { "clearLocalCache": "Local Storage has been cleared. Please refresh the page to reload configuration information.", "clearServerCache": "Server cache has been cleared.", - "failUpdatePrioritySourceDaimon": "Failed to update priority source daimon" + "failUpdatePrioritySourceDaimon": "Failed to update priority source daimon", + "failUpdateCurrentVocabVersion": "Failed to update current vocabulary version" }, "buttons": { "check": "check", diff --git a/src/main/resources/i18n/messages_ko.json b/src/main/resources/i18n/messages_ko.json index 63fe140849..9ec9712441 100644 --- a/src/main/resources/i18n/messages_ko.json +++ b/src/main/resources/i18n/messages_ko.json @@ -1408,6 +1408,7 @@ "notPrevalent": "만연하지 않음(Not Prevalent)", "optimizedOut": "최적화", "options": "옵션", + "originConceptSets": "오리진 컨셉트 세트", "outcomeCohortName": "아웃컴(outcome) 코호트 이름", "outcomeId": "아웃컴(outcome) ID", "outcomeModel": "결과 모델", @@ -1521,7 +1522,11 @@ "rcTooltip": "Record Count", "drcTooltip": "Descendant Record Count", "pcTooltip": "Person Count", - "dpcTooltip": "Descendant Person Count" + "dpcTooltip": "Descendant Person Count", + "conceptID": "개념 ID", + "searchData": "데이터 검색", + "createdBy": "만든 사람", + "createdDate": "생성 날짜" }, "dataSources": { "const": { diff --git a/src/main/resources/i18n/messages_ru.json b/src/main/resources/i18n/messages_ru.json index 6b576d00e3..aae125875e 100644 --- a/src/main/resources/i18n/messages_ru.json +++ b/src/main/resources/i18n/messages_ru.json @@ -1338,6 +1338,7 @@ "binary": "Двоичный", "model": "Модель", "options": "Опции", + "originConceptSets": "Исходные наборы концепций", "firstExposureOnly": "Только первое знакомство", "washoutPeriod": "Период вымывания", "includeAllOutcomes": "Включить все результаты", @@ -1521,7 +1522,11 @@ "pcTooltip": "Количество пациентов", "dpcTooltip": "Количество пациентов-потомков", "validStartDate": "Действительная дата начала", - "validEndDate": "Действительная дата конца" + "validEndDate": "Действительная дата конца", + "conceptID": "Идентификатор концепции", + "searchData": "Данные поиска", + "createdBy": "Автор", + "createdDate": "Дата создания" }, "dataSources": { "headingTitle": "Источники данных", diff --git a/src/main/resources/i18n/messages_zh.json b/src/main/resources/i18n/messages_zh.json index bc0688ba71..207cfcd706 100644 --- a/src/main/resources/i18n/messages_zh.json +++ b/src/main/resources/i18n/messages_zh.json @@ -1408,6 +1408,7 @@ "notPrevalent": "不普遍", "optimizedOut": "优化了", "options": "选择", + "originConceptSets": "起源概念集", "outcomeCohortName": "结果队列名称", "outcomeId": "结果编号", "outcomeModel": "结果模型", @@ -1521,7 +1522,11 @@ "rcTooltip": "Record Count", "drcTooltip": "Descendant Record Count", "pcTooltip": "Person Count", - "dpcTooltip": "Descendant Person Count" + "dpcTooltip": "Descendant Person Count", + "conceptID": "概念 ID", + "searchData": "搜索数据", + "createdBy": "由...制作", + "createdDate": "创建日期" }, "dataSources": { "const": { diff --git a/src/test/java/org/ohdsi/webapi/service/annotations/SearchDataTransformerTest.java b/src/test/java/org/ohdsi/webapi/service/annotations/SearchDataTransformerTest.java new file mode 100644 index 0000000000..ea24b5ac76 --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/service/annotations/SearchDataTransformerTest.java @@ -0,0 +1,70 @@ +package org.ohdsi.webapi.service.annotations; + +import org.json.JSONObject; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.isEmptyString; + +public class SearchDataTransformerTest { + + private SearchDataTransformer sut; + + @Before + public void setUp() { + sut = new SearchDataTransformer(); + } + + @Test + public void shouldReturnEmptyStringWhenInputIsEmpty() { + JSONObject emptyJson = new JSONObject(); + String transformed = sut.convertJsonToReadableFormat(emptyJson.toString()); + assertThat(transformed, isEmptyString()); + } + + @Test + public void shouldHandleSearchText() { + String input = "{\"filterData\":{\"searchText\":\"testSearch\"}}"; + String result = sut.convertJsonToReadableFormat(input); + assertThat(result, is("Search, \"testSearch\"")); + } + + @Test + public void shouldHandleFilterSource() { + String input = "{\"filterSource\":\"Search\"}"; + String result = sut.convertJsonToReadableFormat(input); + assertThat(result, is("Search")); + } + + @Test + public void shouldHandleFilterColumns() { + String input = "{\"filterData\":{\"filterColumns\":[{\"title\":\"Domain\",\"key\":\"Drug\"}]} }"; + String result = sut.convertJsonToReadableFormat(input); + assertThat(result, is("Search, \"\", Filtered By: \"Domain: \"Drug\"\"")); + } + + @Test + public void shouldCombineFilterDataAndFilterSource() { + String input = "{\"filterData\":{\"searchText\":\"testSearch\",\"filterColumns\":[{\"title\":\"Domain\",\"key\":\"Drug\"}]},\"filterSource\":\"Search\"}"; + String result = sut.convertJsonToReadableFormat(input); + String expected = "Search, \"testSearch\", Filtered By: \"Domain: \"Drug\"\""; + assertThat(result, is(expected)); + } + + @Test + public void shouldHandleMultipleFilterColumns() { + String input = "{\"filterData\":{\"filterColumns\":[{\"title\":\"Domain\",\"key\":\"Drug\"},{\"title\":\"Class\",\"key\":\"Medication\"}]}}"; + String result = sut.convertJsonToReadableFormat(input); + String expected = "Search, \"\", Filtered By: \"Domain: \"Drug\"" + ", Class: \"Medication\"\""; + assertThat(result, is(expected)); + } + + @Test + public void shouldHandleNullValuesGracefully() { + String input = "{\"filterData\":{\"filterColumns\":[{\"title\":null,\"key\":null}], \"searchText\":null}, \"filterSource\":null}"; + String result = sut.convertJsonToReadableFormat(input); + assertThat(result, is("Search, \"\", Filtered By: \": \"\"\"")); + } +} \ No newline at end of file From 70dc4e259603ea2966ab8f839f45dc8ecc457d76 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Tue, 17 Dec 2024 10:22:03 -0500 Subject: [PATCH 10/10] Re-introduce Caching (#2393) * Implemented caching for cohort def list, concept set list, permissions, and data sources. Implementation uses ehCache 3.9. Changed editor config to tabs. Made cache endpoints auth restricted. --- .editorconfig | 2 +- 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 | 94 ++++++++++++++ .../webapi/security/PermissionService.java | 16 +++ .../service/CohortDefinitionService.java | 32 ++++- .../webapi/service/ConceptSetService.java | 43 ++++++- .../webapi/service/VocabularyService.java | 57 ++++++++- .../webapi/service/dto/CommonEntityDTO.java | 3 +- .../ohdsi/webapi/shiro/PermissionManager.java | 102 +++++++++++----- .../ohdsi/webapi/source/SourceController.java | 6 +- .../ohdsi/webapi/source/SourceService.java | 46 ++++--- .../org/ohdsi/webapi/user/dto/UserDTO.java | 4 +- .../org/ohdsi/webapi/util/CacheHelper.java | 44 +++++++ src/main/resources/appCache.xml | 34 ++++++ src/main/resources/application.properties | 4 + ...0241203000001__webapi_cache_permission.sql | 14 +++ .../ohdsi/webapi/security/PermissionTest.java | 4 +- .../service/CohortDefinitionServiceTest.java | 115 ++++++++++++++++++ .../resources/permission/permsTest_PREP.json | 42 +++---- .../permission/wildcardTest_PREP.json | 18 +-- 23 files changed, 655 insertions(+), 96 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 create mode 100644 src/main/java/org/ohdsi/webapi/util/CacheHelper.java create mode 100644 src/main/resources/appCache.xml create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20241203000001__webapi_cache_permission.sql create mode 100644 src/test/java/org/ohdsi/webapi/service/CohortDefinitionServiceTest.java 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/pom.xml b/pom.xml index 184483c9e7..222100829e 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 @@ -744,6 +749,10 @@ provided + javax.cache + cache-api + 1.1.1 + org.ohdsi.sql SqlRender ${SqlRender.version} @@ -1202,6 +1211,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..86875fbd51 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/cache/CacheService.java @@ -0,0 +1,94 @@ +/* + * 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.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; + } + + public CacheService() { + } + + + @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..867635d64c 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,24 @@ @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 + 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)); + } + } + } + private static final CohortExpressionQueryBuilder queryBuilder = new CohortExpressionQueryBuilder(); @Autowired @@ -205,7 +228,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 +431,7 @@ public GenerateSqlResult generateSql(GenerateSqlRequest request) { @Path("/") @Produces(MediaType.APPLICATION_JSON) @Transactional + @Cacheable(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, key = "@permissionService.getSubjectCacheKey()") public List getCohortDefinitionList() { List definitions = cohortDefinitionRepository.list(); return definitions.stream() @@ -436,6 +460,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 +563,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 +696,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 +981,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 +999,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 +1135,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 84bc8e7787..1b92c4e769 100644 --- a/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java +++ b/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java @@ -27,6 +27,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import javax.cache.CacheManager; +import javax.cache.configuration.MutableConfiguration; import org.apache.shiro.authz.UnauthorizedException; import org.ohdsi.circe.vocabulary.ConceptSetExpression; @@ -57,6 +59,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; @@ -69,6 +72,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.CacheEvict; +import org.springframework.cache.annotation.Cacheable; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.stereotype.Component; @@ -83,7 +89,26 @@ @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) { + // 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 + 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; @@ -151,7 +176,8 @@ public ConceptSetDTO getConceptSet(@PathParam("id") final int id) { @GET @Path("/") @Produces(MediaType.APPLICATION_JSON) - public Collection getConceptSets() { + @Cacheable(cacheNames = ConceptSetService.CachingSetup.CONCEPT_SET_LIST_CACHE, key = "@permissionService.getSubjectCacheKey()") + public Collection getConceptSets() { return getTransactionTemplate().execute( transactionStatus -> StreamSupport.stream(getConceptSetRepository().findAll().spliterator(), false) .filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true) @@ -469,7 +495,8 @@ public Response exportConceptSetToCSV(@PathParam("id") final String id) throws E @POST @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) - public ConceptSetDTO createConceptSet(ConceptSetDTO conceptSetDTO) { + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) + public ConceptSetDTO createConceptSet(ConceptSetDTO conceptSetDTO) { UserEntity user = userRepository.findByLogin(security.getSubject()); ConceptSet conceptSet = conversionService.convert(conceptSetDTO, ConceptSet.class); @@ -524,7 +551,8 @@ public List getNamesLike(String copyName) { @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @Transactional - public ConceptSetDTO updateConceptSet(@PathParam("id") final int id, ConceptSetDTO conceptSetDTO) throws Exception { + @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); if (updated == null) { @@ -595,7 +623,8 @@ public Collection getConceptSetGenerationInfo(@PathPar @DELETE @Transactional(rollbackOn = Exception.class, dontRollbackOn = EmptyResultDataAccessException.class) @Path("{id}") - public void deleteConceptSet(@PathParam("id") final int id) { + @CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true) + public void deleteConceptSet(@PathParam("id") final int id) { // Remove any generation info try { this.conceptSetGenerationInfoRepository.deleteByConceptSetId(id); @@ -642,7 +671,8 @@ public void deleteConceptSet(@PathParam("id") final int id) { @Produces(MediaType.APPLICATION_JSON) @Path("/{id}/tag/") @Transactional - public void assignTag(@PathParam("id") final Integer id, final int tagId) { + @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); assignTag(entity, tagId); @@ -811,6 +841,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..7a418de59a 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; @@ -51,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; @@ -66,6 +69,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 +89,42 @@ @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) { + // 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 + 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)); + } + } + } + + private static Hashtable vocabularyInfoCache = null; public static final String DEFAULT_SEARCH_ROWS = "20000"; @Autowired @@ -717,6 +759,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 +811,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 +839,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 +1256,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..7e30062ae6 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -35,11 +35,17 @@ 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.ohdsi.webapi.util.CacheHelper; 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 +55,26 @@ @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) { + // 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. + 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; @@ -74,8 +100,8 @@ public class PermissionManager { @Autowired private JdbcTemplate jdbcTemplate; - private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); - + private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); + public static class PermissionsDTO { public Map> permissions = null; @@ -93,10 +119,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 +136,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 +167,45 @@ 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(); + return authorizationInfoCache.get().computeIfAbsent(login, newLogin -> { + 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<>(); - info.setStringPermissions(permissionNames); - info.setPermissionIdx(permissionMap); - return info; - }); - } + 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() { - this.authorizationInfoCache.set(new ConcurrentHashMap<>()); + authorizationInfoCache.set(new ConcurrentHashMap<>()); } @Transactional @@ -261,6 +292,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); @@ -272,6 +304,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); @@ -279,10 +312,12 @@ 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); } + @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) @@ -295,6 +330,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); @@ -302,12 +338,14 @@ 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) 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 +511,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) { @@ -528,6 +567,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); @@ -537,11 +577,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/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/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..6c258f1492 --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/util/CacheHelper.java @@ -0,0 +1,44 @@ +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; +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); + } + } + + public static Set getCacheNames(CacheManager cacheManager) { + Set cacheNames = new HashSet<>(); + cacheManager.getCacheNames().forEach((name) -> cacheNames.add(name)); + return cacheNames; + } +} diff --git a/src/main/resources/appCache.xml b/src/main/resources/appCache.xml new file mode 100644 index 0000000000..ebc1bb19e0 --- /dev/null +++ b/src/main/resources/appCache.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + 50 + + + + + 500 + + + + + 500 + 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/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'); 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/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); + } +} 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" } ]