From a6e23debb796326ba530d747e57ef11345c471e4 Mon Sep 17 00:00:00 2001 From: Reto Wettstein Date: Wed, 4 Dec 2024 10:24:06 +0100 Subject: [PATCH 1/3] use BPMN errorMessageVariable --- .../report/service/DownloadReport.java | 5 +- .../process/report/service/InsertReport.java | 5 +- src/main/resources/bpe/report-receive.bpmn | 96 +++++++++---------- 3 files changed, 50 insertions(+), 56 deletions(-) diff --git a/src/main/java/de/medizininformatik_initiative/process/report/service/DownloadReport.java b/src/main/java/de/medizininformatik_initiative/process/report/service/DownloadReport.java index 25a586f..e7fd7b6 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/service/DownloadReport.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/service/DownloadReport.java @@ -64,15 +64,12 @@ protected void doExecute(DelegateExecution execution, Variables variables) ConstantsReport.CODESYSTEM_REPORT_STATUS_VALUE_RECEIVE_ERROR, "Download report failed")); variables.updateTask(task); - variables.setString(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_RECEIVE_ERROR_MESSAGE, - "Download report failed"); - logger.warn( "Downloading report with id '{}' from organization '{}' referenced in Task with id '{}' failed - {}", reportReference.getValue(), task.getRequester().getIdentifier().getValue(), task.getId(), exception.getMessage()); throw new BpmnError(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_RECEIVE_ERROR, - "Download report - " + exception.getMessage()); + "Download report failed - " + exception.getMessage()); } } diff --git a/src/main/java/de/medizininformatik_initiative/process/report/service/InsertReport.java b/src/main/java/de/medizininformatik_initiative/process/report/service/InsertReport.java index 2fb0460..a63241e 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/service/InsertReport.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/service/InsertReport.java @@ -82,13 +82,10 @@ protected void doExecute(DelegateExecution execution, Variables variables) ConstantsReport.CODESYSTEM_REPORT_STATUS_VALUE_RECEIVE_ERROR, "Insert report failed")); variables.updateTask(task); - variables.setString(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_RECEIVE_ERROR_MESSAGE, - "Insert report failed"); - logger.warn("Storing report from organization '{}' for Task with id '{}' failed - {}", sendingOrganization, task.getId(), exception.getMessage()); throw new BpmnError(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_RECEIVE_ERROR, - "Insert report - " + exception.getMessage()); + "Insert report failed - " + exception.getMessage()); } } diff --git a/src/main/resources/bpe/report-receive.bpmn b/src/main/resources/bpe/report-receive.bpmn index d8a568b..780099a 100644 --- a/src/main/resources/bpe/report-receive.bpmn +++ b/src/main/resources/bpe/report-receive.bpmn @@ -1,5 +1,5 @@ - + @@ -40,11 +40,11 @@ Flow_01x9gay - + Flow_0lhidy1 - + Flow_0epmqlh @@ -69,51 +69,26 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + @@ -121,16 +96,6 @@ - - - - - - - - - - @@ -146,6 +111,41 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 4b3fc2fcf1118d60ab834a8a9e03ff04f4226eb6 Mon Sep 17 00:00:00 2001 From: Reto Wettstein Date: Wed, 4 Dec 2024 11:15:49 +0100 Subject: [PATCH 2/3] refactor 'check search bundle' service task implementation --- pom.xml | 6 - .../report/service/CheckSearchBundle.java | 205 ++------------ .../report/spring/config/ReportConfig.java | 10 +- .../report/util/SearchQueryCheckService.java | 205 ++++++++++++++ .../bpe/CheckSearchBundleServiceTest.java | 257 ------------------ .../bpe/SearchBundleCheckServiceTest.java | 158 +++++++++++ 6 files changed, 388 insertions(+), 453 deletions(-) create mode 100644 src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java delete mode 100644 src/test/java/de/medizininformatik_initiative/process/report/bpe/CheckSearchBundleServiceTest.java create mode 100644 src/test/java/de/medizininformatik_initiative/process/report/bpe/SearchBundleCheckServiceTest.java diff --git a/pom.xml b/pom.xml index 42ed073..2d2956e 100644 --- a/pom.xml +++ b/pom.xml @@ -75,12 +75,6 @@ ${dsf.version} test - - org.mockito - mockito-core - 5.12.0 - test - junit junit diff --git a/src/main/java/de/medizininformatik_initiative/process/report/service/CheckSearchBundle.java b/src/main/java/de/medizininformatik_initiative/process/report/service/CheckSearchBundle.java index dfda77a..91afbe8 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/service/CheckSearchBundle.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/service/CheckSearchBundle.java @@ -1,57 +1,38 @@ package de.medizininformatik_initiative.process.report.service; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.regex.Pattern; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.camunda.bpm.engine.delegate.DelegateExecution; import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.ResourceType; import org.hl7.fhir.r4.model.Task; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.util.MultiValueMap; -import org.springframework.web.util.UriComponents; -import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.beans.factory.InitializingBean; import de.medizininformatik_initiative.process.report.ConstantsReport; +import de.medizininformatik_initiative.process.report.util.SearchQueryCheckService; import dev.dsf.bpe.v1.ProcessPluginApi; import dev.dsf.bpe.v1.activity.AbstractServiceDelegate; import dev.dsf.bpe.v1.variables.Target; import dev.dsf.bpe.v1.variables.Variables; -public class CheckSearchBundle extends AbstractServiceDelegate +public class CheckSearchBundle extends AbstractServiceDelegate implements InitializingBean { private static final Logger logger = LoggerFactory.getLogger(CheckSearchBundle.class); - private static final Pattern MODIFIERS = Pattern.compile(":.*"); - private static final Pattern YEAR_ONLY = Pattern.compile("\\b20\\d{2}(?!\\S)"); - private static final String DATE_EQUALITY_FILTER = "eq"; + private final SearchQueryCheckService searchQueryCheckService; - private static final String CAPABILITY_STATEMENT_PATH = "metadata"; - private static final String SUMMARY_SEARCH_PARAM = "_summary"; - private static final String SUMMARY_SEARCH_PARAM_VALUE_COUNT = "count"; - private static final String TYPE_SEARCH_PARAM = "type"; - - private static final Set ALL_RESOURCE_TYPES = EnumSet.allOf(ResourceType.class).stream() - .map(ResourceType::name).collect(Collectors.toSet()); - - private static final List DATE_SEARCH_PARAMS = List.of("date", "recorded-date", "onset-date", "effective", - "effective-time", "authored", "collected", "issued", "period", "location-period", "occurrence"); - private static final List TOKEN_SEARCH_PARAMS = List.of("code", "ingredient-code", "type"); - private static final List OTHER_SEARCH_PARAMS = List.of("_profile", "_summary"); - private static final List VALID_SEARCH_PARAMS = Stream - .of(DATE_SEARCH_PARAMS.stream(), TOKEN_SEARCH_PARAMS.stream(), OTHER_SEARCH_PARAMS.stream()).flatMap(s -> s) - .toList(); - - public CheckSearchBundle(ProcessPluginApi api) + public CheckSearchBundle(ProcessPluginApi api, SearchQueryCheckService searchQueryCheckService) { super(api); + this.searchQueryCheckService = searchQueryCheckService; + } + + @Override + public void afterPropertiesSet() throws Exception + { + super.afterPropertiesSet(); + Objects.requireNonNull(searchQueryCheckService, "searchQueryCheckService"); } @Override @@ -66,15 +47,12 @@ protected void doExecute(DelegateExecution execution, Variables variables) try { - List searches = bundle.getEntry(); - - testNoResources(searches); - testRequestMethod(searches); - testRequestUrls(searches); + searchQueryCheckService.checkBundle(bundle); logger.info( "Search Bundle downloaded from HRP '{}' as part of Task with id '{}' contains only valid requests of type GET and valid search params {}", - target.getOrganizationIdentifierValue(), task.getId(), VALID_SEARCH_PARAMS); + target.getOrganizationIdentifierValue(), task.getId(), + searchQueryCheckService.getValidSearchParams()); } catch (Exception exception) { @@ -86,155 +64,4 @@ protected void doExecute(DelegateExecution execution, Variables variables) exception); } } - - private void testNoResources(List searches) - { - if (searches.stream().map(Bundle.BundleEntryComponent::getResource).anyMatch(Objects::nonNull)) - throw new RuntimeException("Search Bundle contains resources"); - } - - private void testRequestMethod(List searches) - { - long searchesCount = searches.size(); - long httpGetCount = searches.stream().filter(Bundle.BundleEntryComponent::hasRequest) - .map(Bundle.BundleEntryComponent::getRequest).filter(Bundle.BundleEntryRequestComponent::hasMethod) - .map(Bundle.BundleEntryRequestComponent::getMethod).filter(Bundle.HTTPVerb.GET::equals).count(); - - if (searchesCount != httpGetCount) - throw new RuntimeException("Search Bundle contains HTTP method other then GET"); - } - - private void testRequestUrls(List searches) - { - int searchesCount = searches.size(); - List requests = searches.stream() - .filter(Bundle.BundleEntryComponent::hasRequest).map(Bundle.BundleEntryComponent::getRequest) - .filter(Bundle.BundleEntryRequestComponent::hasUrl).toList(); - int requestCount = requests.size(); - - if (searchesCount != requestCount) - throw new RuntimeException("Search Bundle contains request without url"); - - List uriComponents = requests.stream() - .map(r -> UriComponentsBuilder.fromUriString(r.getUrl()).build()).toList(); - - testContainsOnlyResourcePath(uriComponents); - testContainsValidSummaryCount(uriComponents); - testContainsValidSearchParams(uriComponents); - testContainsValidDateSearchParams(uriComponents); - testContainsValidTokenSearchParams(uriComponents); - } - - private void testContainsOnlyResourcePath(List uriComponents) - { - uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())).forEach(this::testPath); - } - - private void testPath(UriComponents uriComponents) - { - if (!ALL_RESOURCE_TYPES.contains(uriComponents.getPath())) - { - throw new RuntimeException( - "Search Bundle contains request url with forbidden path - [" + uriComponents.getPath() + "]"); - } - } - - private void testContainsValidSummaryCount(List uriComponents) - { - uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) - .map(UriComponents::getQueryParams).forEach(this::testSummaryCount); - } - - private void testSummaryCount(MultiValueMap queryParams) - { - List summaryParams = queryParams.get(SUMMARY_SEARCH_PARAM); - - if (summaryParams == null || summaryParams.isEmpty()) - { - throw new RuntimeException("Search Bundle contains request url without _summary parameter"); - } - - if (summaryParams.size() > 1) - { - throw new RuntimeException("Search Bundle contains request url with more than one _summary parameter"); - } - - if (!SUMMARY_SEARCH_PARAM_VALUE_COUNT.equals(summaryParams.get(0))) - { - throw new RuntimeException( - "Search Bundle contains request url with unexpected _summary parameter value (expected: count, actual: " - + summaryParams.get(0) + ")"); - } - } - - private void testContainsValidSearchParams(List uriComponents) - { - uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) - .map(UriComponents::getQueryParams).forEach(this::testSearchParamNames); - } - - private void testSearchParamNames(MultiValueMap queryParams) - { - if (queryParams.keySet().stream().map(s -> MODIFIERS.matcher(s).replaceAll("")) - .anyMatch(s -> !VALID_SEARCH_PARAMS.contains(s))) - throw new RuntimeException("Search Bundle contains invalid search params, only allowed search params are " - + VALID_SEARCH_PARAMS); - } - - private void testContainsValidDateSearchParams(List uriComponents) - { - uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) - .map(UriComponents::getQueryParams).forEach(this::testSearchParamDateValues); - } - - private void testSearchParamDateValues(MultiValueMap queryParams) - { - List> dateParams = queryParams.entrySet().stream() - .filter(e -> DATE_SEARCH_PARAMS.contains(MODIFIERS.matcher(e.getKey()).replaceAll(""))) - .flatMap(e -> e.getValue().stream().map(v -> Map.entry(e.getKey(), v))).toList(); - - List> erroneousDateFilters = dateParams.stream() - .filter(e -> !e.getValue().startsWith(DATE_EQUALITY_FILTER)).toList(); - - if (erroneousDateFilters.size() > 0) - throw new RuntimeException( - "Search Bundle contains date search params not starting with 'eq' - [" + erroneousDateFilters - .stream().map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); - - List> erroneousDateValues = dateParams.stream() - .filter(e -> !YEAR_ONLY.matcher(e.getValue().replace(DATE_EQUALITY_FILTER, "")).matches()).toList(); - - if (erroneousDateValues.size() > 0) - throw new RuntimeException( - "Search Bundle contains date search params not limited to a year - [" + erroneousDateValues.stream() - .map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); - } - - private void testContainsValidTokenSearchParams(List uriComponents) - { - uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) - .forEach(this::testSearchParamTokenValues); - } - - private void testSearchParamTokenValues(UriComponents uriComponents) - { - List> codeParams = uriComponents.getQueryParams().entrySet().stream() - .filter(e -> TOKEN_SEARCH_PARAMS.contains(MODIFIERS.matcher(e.getKey()).replaceAll(""))) - .flatMap(e -> e.getValue().stream().map(v -> Map.entry(e.getKey(), v))).toList(); - - // Exemption for Encounter.type token params - List> erroneousCodeValues = codeParams.stream() - .filter(e -> !e.getValue().endsWith("|")) - .filter(e -> !isEncounterType(uriComponents.getPath(), e.getKey())).toList(); - - if (erroneousCodeValues.size() > 0) - throw new RuntimeException( - "Search Bundle contains code search params not limited to system - [" + erroneousCodeValues.stream() - .map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); - } - - private boolean isEncounterType(String path, String paramName) - { - return TYPE_SEARCH_PARAM.equals(paramName) && ResourceType.Encounter.name().equals(path); - } } diff --git a/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java b/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java index 6af8a90..ce4e997 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java @@ -23,6 +23,7 @@ import de.medizininformatik_initiative.process.report.service.SetTimer; import de.medizininformatik_initiative.process.report.service.StoreReceipt; import de.medizininformatik_initiative.process.report.util.ReportStatusGenerator; +import de.medizininformatik_initiative.process.report.util.SearchQueryCheckService; import dev.dsf.bpe.v1.ProcessPluginApi; import dev.dsf.bpe.v1.ProcessPluginDeploymentStateListener; import dev.dsf.bpe.v1.documentation.ProcessDocumentation; @@ -94,7 +95,14 @@ public DownloadSearchBundle downloadSearchBundle() @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE) public CheckSearchBundle checkSearchBundle() { - return new CheckSearchBundle(api); + return new CheckSearchBundle(api, searchQueryCheckService()); + } + + @Bean + @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE) + public SearchQueryCheckService searchQueryCheckService() + { + return new SearchQueryCheckService(); } @Bean diff --git a/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java b/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java new file mode 100644 index 0000000..5df211b --- /dev/null +++ b/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java @@ -0,0 +1,205 @@ +package de.medizininformatik_initiative.process.report.util; + +import java.util.EnumSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.ResourceType; +import org.springframework.util.MultiValueMap; +import org.springframework.web.util.UriComponents; +import org.springframework.web.util.UriComponentsBuilder; + +public class SearchQueryCheckService +{ + private static final Pattern MODIFIERS = Pattern.compile(":.*"); + private static final Pattern YEAR_ONLY = Pattern.compile("\\b20\\d{2}(?!\\S)"); + private static final String DATE_EQUALITY_FILTER = "eq"; + + private static final String CAPABILITY_STATEMENT_PATH = "metadata"; + private static final String SUMMARY_SEARCH_PARAM = "_summary"; + private static final String SUMMARY_SEARCH_PARAM_VALUE_COUNT = "count"; + private static final String TYPE_SEARCH_PARAM = "type"; + + private static final Set ALL_RESOURCE_TYPES = EnumSet.allOf(ResourceType.class).stream() + .map(ResourceType::name).collect(Collectors.toSet()); + + private static final List DATE_SEARCH_PARAMS = List.of("date", "recorded-date", "onset-date", "effective", + "effective-time", "authored", "collected", "issued", "period", "location-period", "occurrence"); + private static final List TOKEN_SEARCH_PARAMS = List.of("code", "ingredient-code", "type"); + private static final List OTHER_SEARCH_PARAMS = List.of("_profile", "_summary"); + private static final List VALID_SEARCH_PARAMS = Stream + .of(DATE_SEARCH_PARAMS.stream(), TOKEN_SEARCH_PARAMS.stream(), OTHER_SEARCH_PARAMS.stream()).flatMap(s -> s) + .toList(); + + + public void checkBundle(Bundle bundle) + { + List searches = bundle.getEntry(); + + testNoResources(searches); + testRequestMethod(searches); + testRequestUrls(searches); + } + + public List getValidSearchParams() + { + return VALID_SEARCH_PARAMS; + } + + private void testNoResources(List searches) + { + if (searches.stream().map(Bundle.BundleEntryComponent::getResource).anyMatch(Objects::nonNull)) + throw new RuntimeException("Search Bundle contains resources"); + } + + private void testRequestMethod(List searches) + { + long searchesCount = searches.size(); + long httpGetCount = searches.stream().filter(Bundle.BundleEntryComponent::hasRequest) + .map(Bundle.BundleEntryComponent::getRequest).filter(Bundle.BundleEntryRequestComponent::hasMethod) + .map(Bundle.BundleEntryRequestComponent::getMethod).filter(Bundle.HTTPVerb.GET::equals).count(); + + if (searchesCount != httpGetCount) + throw new RuntimeException("Search Bundle contains HTTP method other then GET"); + } + + private void testRequestUrls(List searches) + { + int searchesCount = searches.size(); + List requests = searches.stream() + .filter(Bundle.BundleEntryComponent::hasRequest).map(Bundle.BundleEntryComponent::getRequest) + .filter(Bundle.BundleEntryRequestComponent::hasUrl).toList(); + int requestCount = requests.size(); + + if (searchesCount != requestCount) + throw new RuntimeException("Search Bundle contains request without url"); + + List uriComponents = requests.stream() + .map(r -> UriComponentsBuilder.fromUriString(r.getUrl()).build()).toList(); + + testContainsOnlyResourcePath(uriComponents); + testContainsValidSummaryCount(uriComponents); + testContainsValidSearchParams(uriComponents); + testContainsValidDateSearchParams(uriComponents); + testContainsValidTokenSearchParams(uriComponents); + } + + private void testContainsOnlyResourcePath(List uriComponents) + { + uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())).forEach(this::testPath); + } + + private void testPath(UriComponents uriComponents) + { + if (!ALL_RESOURCE_TYPES.contains(uriComponents.getPath())) + { + throw new RuntimeException( + "Search Bundle contains request url with forbidden path - [" + uriComponents.getPath() + "]"); + } + } + + private void testContainsValidSummaryCount(List uriComponents) + { + uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) + .map(UriComponents::getQueryParams).forEach(this::testSummaryCount); + } + + private void testSummaryCount(MultiValueMap queryParams) + { + List summaryParams = queryParams.get(SUMMARY_SEARCH_PARAM); + + if (summaryParams == null || summaryParams.isEmpty()) + { + throw new RuntimeException("Search Bundle contains request url without _summary parameter"); + } + + if (summaryParams.size() > 1) + { + throw new RuntimeException("Search Bundle contains request url with more than one _summary parameter"); + } + + if (!SUMMARY_SEARCH_PARAM_VALUE_COUNT.equals(summaryParams.get(0))) + { + throw new RuntimeException( + "Search Bundle contains request url with unexpected _summary parameter value (expected: count, actual: " + + summaryParams.get(0) + ")"); + } + } + + private void testContainsValidSearchParams(List uriComponents) + { + uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) + .map(UriComponents::getQueryParams).forEach(this::testSearchParamNames); + } + + private void testSearchParamNames(MultiValueMap queryParams) + { + if (queryParams.keySet().stream().map(s -> MODIFIERS.matcher(s).replaceAll("")) + .anyMatch(s -> !VALID_SEARCH_PARAMS.contains(s))) + throw new RuntimeException("Search Bundle contains invalid search params, only allowed search params are " + + VALID_SEARCH_PARAMS); + } + + private void testContainsValidDateSearchParams(List uriComponents) + { + uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) + .map(UriComponents::getQueryParams).forEach(this::testSearchParamDateValues); + } + + private void testSearchParamDateValues(MultiValueMap queryParams) + { + List> dateParams = queryParams.entrySet().stream() + .filter(e -> DATE_SEARCH_PARAMS.contains(MODIFIERS.matcher(e.getKey()).replaceAll(""))) + .flatMap(e -> e.getValue().stream().map(v -> Map.entry(e.getKey(), v))).toList(); + + List> erroneousDateFilters = dateParams.stream() + .filter(e -> !e.getValue().startsWith(DATE_EQUALITY_FILTER)).toList(); + + if (erroneousDateFilters.size() > 0) + throw new RuntimeException( + "Search Bundle contains date search params not starting with 'eq' - [" + erroneousDateFilters + .stream().map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); + + List> erroneousDateValues = dateParams.stream() + .filter(e -> !YEAR_ONLY.matcher(e.getValue().replace(DATE_EQUALITY_FILTER, "")).matches()).toList(); + + if (erroneousDateValues.size() > 0) + throw new RuntimeException( + "Search Bundle contains date search params not limited to a year - [" + erroneousDateValues.stream() + .map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); + } + + private void testContainsValidTokenSearchParams(List uriComponents) + { + uriComponents.stream().filter(u -> !CAPABILITY_STATEMENT_PATH.equals(u.getPath())) + .forEach(this::testSearchParamTokenValues); + } + + private void testSearchParamTokenValues(UriComponents uriComponents) + { + List> codeParams = uriComponents.getQueryParams().entrySet().stream() + .filter(e -> TOKEN_SEARCH_PARAMS.contains(MODIFIERS.matcher(e.getKey()).replaceAll(""))) + .flatMap(e -> e.getValue().stream().map(v -> Map.entry(e.getKey(), v))).toList(); + + // Exemption for Encounter.type token params + List> erroneousCodeValues = codeParams.stream() + .filter(e -> !e.getValue().endsWith("|")) + .filter(e -> !isEncounterType(uriComponents.getPath(), e.getKey())).toList(); + + if (erroneousCodeValues.size() > 0) + throw new RuntimeException( + "Search Bundle contains code search params not limited to system - [" + erroneousCodeValues.stream() + .map(e -> e.getKey() + ":" + e.getValue()).collect(Collectors.joining(",")) + "]"); + } + + private boolean isEncounterType(String path, String paramName) + { + return TYPE_SEARCH_PARAM.equals(paramName) && ResourceType.Encounter.name().equals(path); + } +} diff --git a/src/test/java/de/medizininformatik_initiative/process/report/bpe/CheckSearchBundleServiceTest.java b/src/test/java/de/medizininformatik_initiative/process/report/bpe/CheckSearchBundleServiceTest.java deleted file mode 100644 index 28b674d..0000000 --- a/src/test/java/de/medizininformatik_initiative/process/report/bpe/CheckSearchBundleServiceTest.java +++ /dev/null @@ -1,257 +0,0 @@ -package de.medizininformatik_initiative.process.report.bpe; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -import java.io.InputStream; -import java.util.List; -import java.util.UUID; - -import org.camunda.bpm.engine.ProcessEngine; -import org.camunda.bpm.engine.RuntimeService; -import org.camunda.bpm.engine.delegate.DelegateExecution; -import org.hl7.fhir.r4.model.Bundle; -import org.hl7.fhir.r4.model.IdType; -import org.hl7.fhir.r4.model.StringType; -import org.hl7.fhir.r4.model.Task; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -import ca.uhn.fhir.context.FhirContext; -import de.medizininformatik_initiative.process.report.ConstantsReport; -import de.medizininformatik_initiative.process.report.service.CheckSearchBundle; -import dev.dsf.bpe.v1.ProcessPluginApi; -import dev.dsf.bpe.v1.constants.CodeSystems; -import dev.dsf.bpe.v1.service.FhirWebserviceClientProvider; -import dev.dsf.bpe.v1.service.TaskHelper; -import dev.dsf.bpe.v1.variables.Target; -import dev.dsf.bpe.v1.variables.Variables; -import dev.dsf.fhir.client.FhirWebserviceClient; -import dev.dsf.fhir.client.PreferReturnMinimalWithRetry; - -@RunWith(MockitoJUnitRunner.class) -public class CheckSearchBundleServiceTest -{ - @Mock - private DelegateExecution execution; - - @Mock - private ProcessPluginApi api; - - @Mock - private Variables variables; - - @Mock - private Target target; - - @Mock - private Task task; - - @Mock - private TaskHelper taskHelper; - - @Mock - FhirWebserviceClientProvider clientProvider; - - @Mock - private FhirWebserviceClient webserviceClient; - - @Mock - private PreferReturnMinimalWithRetry preferReturnMinimalWithRetry; - - @Mock - private ProcessEngine processEngine; - - @Mock - private RuntimeService runtimeService; - - @Captor - ArgumentCaptor output; - - @InjectMocks - private CheckSearchBundle service; - - @Test - public void testValid() - { - testValid("/fhir/Bundle/search-bundle.xml"); - } - - @Test - public void testInvalidResource() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-resource.xml", "resources"); - } - - @Test - public void testInvalidRequestMethod() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-request-method.xml", "GET"); - } - - @Test - public void testInvalidResourceId() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-resource-id.xml", "request url with forbidden path"); - } - - @Test - public void testInvalidResourceIdDouble() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-resource-id-double.xml", "request url with forbidden path"); - } - - @Test - public void testInvalidNoSummary() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-summary-not-exists.xml", "without _summary parameter"); - } - - @Test - public void testInvalidDoubleSummary() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-summary-double.xml", "more than one _summary parameter"); - } - - @Test - public void testInvalidSummaryUrlEncoded() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-summary-url-encoded.xml", "invalid search params"); - } - - @Test - public void testInvalidSummaryUrlEncodedFull() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-summary-url-encoded-full.xml", "invalid search params"); - } - - @Test - public void testInvalidUnexpectedSummary() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-summary-not-allowed.xml", "unexpected _summary parameter"); - } - - @Test - public void testInvalidParam() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-param.xml", "invalid search params"); - } - - @Test - public void testInvalidDateFilter() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-date-filter.xml", "not starting with 'eq'"); - } - - @Test - public void testInvalidDateValue() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-date-value-single.xml", "not limited to a year"); - } - - @Test - public void testInvalidDoubleDateValue() - { - testInvalid("/fhir/Bundle/search-bundle-invalid-date-value-double.xml", "not limited to a year"); - } - - @Test - public void testValidV1_1() - { - testValid("/fhir/Bundle/search-bundle-v1.1.xml"); - } - - @Test - public void testValidV1_1EncounterType() - { - testValid("/fhir/Bundle/search-bundle-v1.1-valid-encounter-type.xml"); - } - - @Test - public void testInvalidV1_1SingleCode() - { - testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-single.xml", "not limited to system"); - } - - @Test - public void testInvalidV1_1DoubleCode() - { - testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-double.xml", "not limited to system"); - } - - @Test - public void testInvalidV1_1CodeIngredient() - { - testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-ingredient.xml", "not limited to system"); - } - - private void testValid(String pathToBundle) - { - try (InputStream in = getClass().getResourceAsStream(pathToBundle)) - { - Bundle bundle = FhirContext.forR4().newXmlParser().parseResource(Bundle.class, in); - Mockito.when(api.getVariables(execution)).thenReturn(variables); - Mockito.when(variables.getStartTask()).thenReturn(task); - Mockito.when(variables.getTarget()).thenReturn(target); - Mockito.when(target.getOrganizationIdentifierValue()).thenReturn("Test_DIC1"); - Mockito.when(variables.getResource(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_SEARCH_BUNDLE)) - .thenReturn(bundle); - - service.execute(execution); - } - catch (Exception exception) - { - fail(); - } - } - - private void testInvalid(String pathToBundle, String errorContains) - { - try (InputStream in = getClass().getResourceAsStream(pathToBundle)) - { - Bundle bundle = FhirContext.forR4().newXmlParser().parseResource(Bundle.class, in); - Mockito.when(api.getVariables(execution)).thenReturn(variables); - Mockito.when(variables.getStartTask()).thenReturn(task); - Mockito.when(variables.getTarget()).thenReturn(target); - Mockito.when(target.getOrganizationIdentifierValue()).thenReturn("Test_DIC1"); - Mockito.when(variables.getResource(ConstantsReport.BPMN_EXECUTION_VARIABLE_REPORT_SEARCH_BUNDLE)) - .thenReturn(bundle); - Mockito.when(execution.getProcessDefinitionId()).thenReturn("processDefinitionId"); - Mockito.when(execution.getActivityInstanceId()).thenReturn("activityInstanceId"); - Mockito.when(api.getTaskHelper()).thenReturn(taskHelper); - Mockito.when(taskHelper.getLocalVersionlessAbsoluteUrl(task)).thenReturn("http://foo.bar/fhir/Task/id"); - Mockito.when(variables.getTasks()).thenReturn(List.of(task)); - Mockito.when(task.getStatus()).thenReturn(Task.TaskStatus.INPROGRESS); - - Mockito.when(api.getFhirWebserviceClientProvider()).thenReturn(clientProvider); - Mockito.when(clientProvider.getLocalWebserviceClient()).thenReturn(webserviceClient); - Mockito.when(webserviceClient.withMinimalReturn()).thenReturn(preferReturnMinimalWithRetry); - Mockito.when(preferReturnMinimalWithRetry.update(task)) - .thenReturn(new IdType(UUID.randomUUID().toString())); - Mockito.when(execution.getProcessEngine()).thenReturn(processEngine); - Mockito.when(processEngine.getRuntimeService()).thenReturn(runtimeService); - - service.execute(execution); - Mockito.verify(task).addOutput(output.capture()); - - Task.TaskOutputComponent value = output.getValue(); - assertEquals(1, - value.getType().getCoding().stream().filter(c -> CodeSystems.BpmnMessage.URL.equals(c.getSystem()) - && CodeSystems.BpmnMessage.error().getCode().equals(c.getCode())).count()); - - assertTrue(value.getValue() instanceof StringType); - assertTrue(((StringType) value.getValue()).getValue().contains(errorContains)); - } - catch (Exception exception) - { - fail(); - } - } -} diff --git a/src/test/java/de/medizininformatik_initiative/process/report/bpe/SearchBundleCheckServiceTest.java b/src/test/java/de/medizininformatik_initiative/process/report/bpe/SearchBundleCheckServiceTest.java new file mode 100644 index 0000000..f0db77b --- /dev/null +++ b/src/test/java/de/medizininformatik_initiative/process/report/bpe/SearchBundleCheckServiceTest.java @@ -0,0 +1,158 @@ +package de.medizininformatik_initiative.process.report.bpe; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.InputStream; + +import org.hl7.fhir.r4.model.Bundle; +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import de.medizininformatik_initiative.process.report.util.SearchQueryCheckService; + +public class SearchBundleCheckServiceTest +{ + @Test + public void testValid() + { + testValid("/fhir/Bundle/search-bundle.xml"); + } + + @Test + public void testInvalidResource() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-resource.xml", "resources"); + } + + @Test + public void testInvalidRequestMethod() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-request-method.xml", "GET"); + } + + @Test + public void testInvalidResourceId() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-resource-id.xml", "request url with forbidden path"); + } + + @Test + public void testInvalidResourceIdDouble() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-resource-id-double.xml", "request url with forbidden path"); + } + + @Test + public void testInvalidNoSummary() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-summary-not-exists.xml", "without _summary parameter"); + } + + @Test + public void testInvalidDoubleSummary() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-summary-double.xml", "more than one _summary parameter"); + } + + @Test + public void testInvalidSummaryUrlEncoded() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-summary-url-encoded.xml", "invalid search params"); + } + + @Test + public void testInvalidSummaryUrlEncodedFull() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-summary-url-encoded-full.xml", "invalid search params"); + } + + @Test + public void testInvalidUnexpectedSummary() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-summary-not-allowed.xml", "unexpected _summary parameter"); + } + + @Test + public void testInvalidParam() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-param.xml", "invalid search params"); + } + + @Test + public void testInvalidDateFilter() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-date-filter.xml", "not starting with 'eq'"); + } + + @Test + public void testInvalidDateValue() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-date-value-single.xml", "not limited to a year"); + } + + @Test + public void testInvalidDoubleDateValue() + { + testInvalid("/fhir/Bundle/search-bundle-invalid-date-value-double.xml", "not limited to a year"); + } + + @Test + public void testValidV1_1() + { + testValid("/fhir/Bundle/search-bundle-v1.1.xml"); + } + + @Test + public void testValidV1_1EncounterType() + { + testValid("/fhir/Bundle/search-bundle-v1.1-valid-encounter-type.xml"); + } + + @Test + public void testInvalidV1_1SingleCode() + { + testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-single.xml", "not limited to system"); + } + + @Test + public void testInvalidV1_1DoubleCode() + { + testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-double.xml", "not limited to system"); + } + + @Test + public void testInvalidV1_1CodeIngredient() + { + testInvalid("/fhir/Bundle/search-bundle-v1.1-invalid-code-ingredient.xml", "not limited to system"); + } + + private void testValid(String pathToBundle) + { + try (InputStream in = getClass().getResourceAsStream(pathToBundle)) + { + Bundle bundle = FhirContext.forR4().newXmlParser().parseResource(Bundle.class, in); + new SearchQueryCheckService().checkBundle(bundle); + } + catch (Exception exception) + { + fail(); + } + } + + private void testInvalid(String pathToBundle, String errorContains) + { + try (InputStream in = getClass().getResourceAsStream(pathToBundle)) + { + Bundle bundle = FhirContext.forR4().newXmlParser().parseResource(Bundle.class, in); + new SearchQueryCheckService().checkBundle(bundle); + + // test expected to throw error + fail(); + } + catch (Exception exception) + { + assertTrue(exception.getMessage().contains(errorContains)); + } + } +} From b72664366a0b71bf85d886b303618e96ee5760e2 Mon Sep 17 00:00:00 2001 From: Reto Wettstein Date: Wed, 18 Dec 2024 13:00:28 +0100 Subject: [PATCH 3/3] use singleton scope for service class without state, remove empty line --- .../process/report/spring/config/ReportConfig.java | 2 +- .../process/report/util/SearchQueryCheckService.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java b/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java index ce4e997..a68dc94 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/spring/config/ReportConfig.java @@ -99,7 +99,7 @@ public CheckSearchBundle checkSearchBundle() } @Bean - @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE) + @Scope(ConfigurableBeanFactory.SCOPE_SINGLETON) public SearchQueryCheckService searchQueryCheckService() { return new SearchQueryCheckService(); diff --git a/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java b/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java index 5df211b..b77ddfa 100644 --- a/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java +++ b/src/main/java/de/medizininformatik_initiative/process/report/util/SearchQueryCheckService.java @@ -37,7 +37,6 @@ public class SearchQueryCheckService .of(DATE_SEARCH_PARAMS.stream(), TOKEN_SEARCH_PARAMS.stream(), OTHER_SEARCH_PARAMS.stream()).flatMap(s -> s) .toList(); - public void checkBundle(Bundle bundle) { List searches = bundle.getEntry();