Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing SearchParameter validation #6616

Closed
wants to merge 8 commits into from
Closed
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
type: fix
issue: 6615
title: "SearchParameter validation was not being skipped on updates, even if requested. This has been fixed"
Original file line number Diff line number Diff line change
@@ -4,9 +4,11 @@
import ca.uhn.fhir.i18n.HapiLocalizer;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.Hook;
import ca.uhn.fhir.interceptor.api.IInterceptorService;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
import ca.uhn.fhir.jpa.dao.data.ISearchDao;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.model.dao.JpaPidFk;
@@ -19,6 +21,7 @@
import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.searchparam.submit.interceptor.SearchParamValidatingInterceptor;
import ca.uhn.fhir.jpa.term.ZipCollectionBuilder;
import ca.uhn.fhir.jpa.test.config.TestR4Config;
import ca.uhn.fhir.jpa.util.MemoryCacheService;
@@ -37,6 +40,7 @@
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
import ca.uhn.fhir.rest.api.SummaryEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.client.apache.ResourceEntity;
import ca.uhn.fhir.rest.client.api.IClientInterceptor;
import ca.uhn.fhir.rest.client.api.IGenericClient;
@@ -62,6 +66,8 @@
import ca.uhn.fhir.util.StopWatch;
import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.util.UrlUtil;
import ca.uhn.test.util.LogbackTestExtension;
import ca.uhn.test.util.LogbackTestExtensionAssert;
import com.google.common.base.Charsets;
import com.google.common.collect.Lists;
import jakarta.annotation.Nonnull;
@@ -170,6 +176,7 @@
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.CsvSource;
@@ -197,6 +204,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import java.util.stream.Stream;

@@ -222,6 +230,12 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
@Autowired
private ISearchDao mySearchEntityDao;

@Autowired
private IInterceptorService myInterceptorService;

@RegisterExtension
public LogbackTestExtension myLogbackTestExtension = new LogbackTestExtension();

@Override
@AfterEach
public void after() throws Exception {
@@ -264,7 +278,35 @@ public void before() throws Exception {
myStorageSettings.setSearchPreFetchThresholds(new JpaStorageSettings().getSearchPreFetchThresholds());
}

@Test
public void testSearchParameterValidation() {
// setup
SystemRequestDetails requestDetails = new SystemRequestDetails();
requestDetails.getUserData().put(SearchParamValidatingInterceptor.SKIP_VALIDATION, true);

SearchParameter sp = new SearchParameter();
sp.setUrl("http://example.com/name");
sp.setId("name");
sp.setCode("name");
sp.setType(Enumerations.SearchParamType.STRING);
sp.setStatus(Enumerations.PublicationStatus.RETIRED);
sp.addBase("Patient");
sp.setExpression("Patient.name");

// test
DaoMethodOutcome outcome = mySearchParameterDao.update(sp, requestDetails);

myCaptureQueriesListener.clear();
sp.setId(outcome.getId());
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
mySearchParameterDao.update(sp, requestDetails);

// verify
// TODO - investigate better 'validation' options
myCaptureQueriesListener.logAllQueries();
assertEquals(8, myCaptureQueriesListener.countSelectQueries());
LogbackTestExtensionAssert.assertThat(myLogbackTestExtension).hasWarnMessage("Skipping validation of submitted SearchParameter because " + SearchParamValidatingInterceptor.SKIP_VALIDATION + " flag is true");
}

@Test
public void testParameterWithNoValueThrowsError_InvalidChainOnCustomSearch() throws IOException {
Original file line number Diff line number Diff line change
@@ -38,8 +38,11 @@
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.util.HapiExtensions;
import jakarta.annotation.Nullable;
import org.hl7.fhir.common.hapi.validation.validator.VersionSpecificWorkerContextWrapper;
import org.hl7.fhir.instance.model.api.IBaseExtension;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;

import java.util.HashSet;
@@ -54,6 +57,7 @@
@Interceptor
public class SearchParamValidatingInterceptor {

private static final Logger logger = LoggerFactory.getLogger(SearchParamValidatingInterceptor.class);
public static final String SEARCH_PARAM = "SearchParameter";
public static final String SKIP_VALIDATION = SearchParamValidatingInterceptor.class.getName() + ".SKIP_VALIDATION";

@@ -142,6 +146,16 @@ public void validateSearchParamOnUpdate(IBaseResource theResource, RequestDetail
if (isNotSearchParameterResource(theResource)) {
return;
}

// avoid a loop when loading our hard-coded core FhirContext SearchParameters
//skip Search Param validation if been set in the request
boolean isStartup = theRequestDetails != null
&& Boolean.TRUE == theRequestDetails.getUserData().get(SKIP_VALIDATION);
if (isStartup) {
logger.warn("Skipping validation of submitted SearchParameter because {} flag is {}",SKIP_VALIDATION, Boolean.TRUE);
return;
}

RuntimeSearchParam runtimeSearchParam = mySearchParameterCanonicalizer.canonicalizeSearchParameter(theResource);
if (runtimeSearchParam == null) {
return;
Original file line number Diff line number Diff line change
@@ -64,7 +64,6 @@ public void beforeEach() {
mySearchParamValidatingInterceptor.setDaoRegistry(myDaoRegistry);

myExistingSearchParameter = buildSearchParameterWithId(ID1);

}

@Test
@@ -84,7 +83,6 @@ public void whenCreatingNonOverlappingSearchParam_thenIsAllowed() {
SearchParameter newSearchParam = buildSearchParameterWithId(ID1);

mySearchParamValidatingInterceptor.resourcePreCreate(newSearchParam, myRequestDetails);

}

@Test
Loading