SearchParameter validation was not being skipped on updates, even if … (hapifhir#6667)

* SearchParameter validation was not being skipped on updates, even if requested. This has been fixed

* spotless fixes

* removed unecessary code

* reverting the code back

* added external JIRA ref

Author: BulandMalik

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6615-fixing-validation-on-sp.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6615
+jira: SMILE-9569
+title: "SearchParameter validation was not being skipped on updates, even if requested. This has been fixed"

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java

@@ -7,6 +7,7 @@
 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 +20,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;
@@ -62,6 +64,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,16 +174,21 @@
 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;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
+import ch. qos. logback. classic.Level;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.test.util.AopTestUtils;
 import org.springframework.transaction.TransactionStatus;
 import org.springframework.transaction.support.TransactionCallbackWithoutResult;
 import org.springframework.transaction.support.TransactionTemplate;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
+import ca.uhn.fhir.rest.client.apache.ResourceEntity;
+
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -222,6 +231,9 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 	@Autowired
 	private ISearchDao mySearchEntityDao;
 
+	@RegisterExtension
+	public LogbackTestExtension myLogbackTestExtension = new LogbackTestExtension(SearchParamValidatingInterceptor.class, Level.WARN);
+
 	@Override
 	@AfterEach
 	public void after() throws Exception {
@@ -264,7 +276,31 @@ 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);
+
+		LogbackTestExtensionAssert.assertThat(myLogbackTestExtension).hasWarnMessage("Skipping validation of submitted SearchParameter because " + SearchParamValidatingInterceptor.SKIP_VALIDATION + " flag is true");
+	}
 
 	@Test
 	public void testParameterWithNoValueThrowsError_InvalidChainOnCustomSearch() throws IOException {

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/searchparam/submit/interceptor/SearchParamValidatingInterceptor.java

@@ -40,6 +40,8 @@
 import jakarta.annotation.Nullable;
 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 +56,8 @@
 @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,19 @@ 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;