diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java index 3dc08ef5d1c..fc5003111e2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/NumberParam.java @@ -56,7 +56,7 @@ public class NumberParam extends BaseParamWithPrefix implements IQu * Constructor * * @param theValue - * A string value, e.g. ">5.0" + * A string value, e.g. "gt5.0" */ public NumberParam(String theValue) { setValueAsQueryToken(null, null, null, theValue); diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/2023-force-sp-cache-flush-sooner.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/2023-force-sp-cache-flush-sooner.yaml new file mode 100644 index 00000000000..fc5968f2043 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/2023-force-sp-cache-flush-sooner.yaml @@ -0,0 +1,7 @@ +--- +type: add +issue: 2023 +title: "The JPA server maintains a cache of active SearchParameeter resources that can cause misleading results + if a SearchParameter is changed and other resources that would be indexed by the changed SearchParameter are updated + before the cache refreshes. A new interceptor has been added that should force a refresh sooner, especially on + non-clustered systems." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoSearchParameterDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoSearchParameterDstu2.java index 4672d60f996..c0ae4a899aa 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoSearchParameterDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoSearchParameterDstu2.java @@ -51,6 +51,7 @@ public class FhirResourceDaoSearchParameterDstu2 extends BaseHapiFhirResourceDao protected void postPersist(ResourceTable theEntity, SearchParameter theResource) { super.postPersist(theEntity, theResource); markAffectedResources(theResource); + } @Override diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchCustomSearchParamTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchCustomSearchParamTest.java index bdeb2e803ca..8687c414f01 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchCustomSearchParamTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2SearchCustomSearchParamTest.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao.dstu2; +import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken; @@ -28,6 +29,8 @@ import ca.uhn.fhir.model.primitive.DecimalDt; import ca.uhn.fhir.model.primitive.IntegerDt; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.NumberParam; import ca.uhn.fhir.rest.param.ReferenceParam; @@ -232,6 +235,47 @@ public class FhirResourceDaoDstu2SearchCustomSearchParamTest extends BaseJpaDstu } + /** + * See #2023 + */ + @Test + public void testNumberSearchParam() { + SearchParameter numberParameter = new ca.uhn.fhir.model.dstu2.resource.SearchParameter(); + numberParameter.setId("future-appointment-count"); + numberParameter.setName("Future Appointment Count"); + numberParameter.setCode("future-appointment-count"); + numberParameter.setDescription("Count of future appointments for the patient"); + numberParameter.setUrl("http://integer"); + numberParameter.setStatus(ca.uhn.fhir.model.dstu2.valueset.ConformanceResourceStatusEnum.ACTIVE); + numberParameter.setBase(ca.uhn.fhir.model.dstu2.valueset.ResourceTypeEnum.PATIENT); + numberParameter.setType(ca.uhn.fhir.model.dstu2.valueset.SearchParamTypeEnum.NUMBER); + numberParameter.setXpathUsage(XPathUsageTypeEnum.NORMAL); + numberParameter.setXpath("Patient.extension('http://integer')"); + mySearchParameterDao.update(numberParameter); + + // This fires every 10 seconds + mySearchParamRegistry.refreshCacheIfNecessary(); + + Patient patient = new Patient(); + patient.setId("future-appointment-count-pt"); + patient.setActive(true); + patient.addUndeclaredExtension(false, "http://integer", new IntegerDt(1)); + myPatientDao.update(patient); + + IBundleProvider search; + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam(1))); + assertEquals(1, search.size()); + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam("gt0"))); + assertEquals(1, search.size()); + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam("lt0"))); + assertEquals(0, search.size()); + + } + + @Test public void testIncludeExtensionReferenceAsRecurse() { SearchParameter attendingSp = new SearchParameter(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java index 7b935e91508..398f6aae40f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; import ca.uhn.fhir.interceptor.api.Pointcut; @@ -9,7 +10,11 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken; import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.model.dstu2.valueset.XPathUsageTypeEnum; +import ca.uhn.fhir.model.primitive.IntegerDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.NumberParam; import ca.uhn.fhir.rest.param.ReferenceOrListParam; @@ -112,6 +117,46 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test mySearchParamRegistry.forceRefresh(); } + /** + * See #2023 + */ + @Test + public void testNumberSearchParam() { + SearchParameter numberParameter = new SearchParameter(); + numberParameter.setId("future-appointment-count"); + numberParameter.setName("Future Appointment Count"); + numberParameter.setCode("future-appointment-count"); + numberParameter.setDescription("Count of future appointments for the patient"); + numberParameter.setUrl("http://integer"); + numberParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); + numberParameter.addBase("Patient"); + numberParameter.setType(Enumerations.SearchParamType.NUMBER); + numberParameter.setExpression("Patient.extension('http://integer')"); + mySearchParameterDao.update(numberParameter); + + // This fires every 10 seconds + mySearchParamRegistry.refreshCacheIfNecessary(); + + Patient patient = new Patient(); + patient.setId("future-appointment-count-pt"); + patient.setActive(true); + patient.addExtension( "http://integer", new IntegerType(1)); + myPatientDao.update(patient); + + IBundleProvider search; + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam(1))); + assertEquals(1, search.size()); + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam("gt0"))); + assertEquals(1, search.size()); + + search = myPatientDao.search(SearchParameterMap.newSynchronous("future-appointment-count", new NumberParam("lt0"))); + assertEquals(0, search.size()); + + } + + /** * Draft search parameters should be ok even if they aren't completely valid */ diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java index 46b41128f83..54844451975 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java @@ -24,8 +24,10 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.context.phonetic.IPhoneticEncoder; +import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.HookParams; -import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.interceptor.api.IInterceptorService; +import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.sched.HapiJob; @@ -38,8 +40,6 @@ import ca.uhn.fhir.jpa.searchparam.retry.Retrier; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.DatatypeUtil; -import ca.uhn.fhir.util.HapiExtensions; import ca.uhn.fhir.util.SearchParameterUtil; import ca.uhn.fhir.util.StopWatch; import org.apache.commons.lang3.StringUtils; @@ -51,6 +51,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -89,7 +90,8 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry { private volatile long myLastRefresh; @Autowired - private IInterceptorBroadcaster myInterceptorBroadcaster; + private IInterceptorService myInterceptorBroadcaster; + private RefreshSearchParameterCacheOnUpdate myInterceptor; @Override public RuntimeSearchParam getActiveSearchParam(String theResourceName, String theParamName) { @@ -236,8 +238,16 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry { } @PostConstruct - public void postConstruct() { + public void start() { myBuiltInSearchParams = createBuiltInSearchParamMap(myFhirContext); + + myInterceptor = new RefreshSearchParameterCacheOnUpdate(); + myInterceptorBroadcaster.registerInterceptor(myInterceptor); + } + + @PreDestroy + public void stop() { + myInterceptorBroadcaster.unregisterInterceptor(myInterceptor); } public int doRefresh(long theRefreshInterval) { @@ -376,16 +386,6 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry { mySchedulerService.scheduleLocalJob(10 * DateUtils.MILLIS_PER_SECOND, jobDetail); } - public static class Job implements HapiJob { - @Autowired - private ISearchParamRegistry myTarget; - - @Override - public void execute(JobExecutionContext theContext) { - myTarget.refreshCacheIfNecessary(); - } - } - @Override public boolean refreshCacheIfNecessary() { if (myActiveSearchParams == null || System.currentTimeMillis() - REFRESH_INTERVAL > myLastRefresh) { @@ -402,30 +402,12 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry { return Collections.unmodifiableMap(myActiveSearchParams); } - public static Map> createBuiltInSearchParamMap(FhirContext theFhirContext) { - Map> resourceNameToSearchParams = new HashMap<>(); - - Set resourceNames = theFhirContext.getResourceTypes(); - - for (String resourceName : resourceNames) { - RuntimeResourceDefinition nextResDef = theFhirContext.getResourceDefinition(resourceName); - String nextResourceName = nextResDef.getName(); - HashMap nameToParam = new HashMap<>(); - resourceNameToSearchParams.put(nextResourceName, nameToParam); - - for (RuntimeSearchParam nextSp : nextResDef.getSearchParams()) { - nameToParam.put(nextSp.getName(), nextSp); - } - } - return Collections.unmodifiableMap(resourceNameToSearchParams); - } - /** * All SearchParameters with the name "phonetic" encode the normalized index value using this phonetic encoder. * * @since 5.1.0 */ - + @Override public void setPhoneticEncoder(IPhoneticEncoder thePhoneticEncoder) { myPhoneticEncoder = thePhoneticEncoder; @@ -446,4 +428,58 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry { searchParam.setPhoneticEncoder(myPhoneticEncoder); } } + + @Interceptor + public class RefreshSearchParameterCacheOnUpdate { + + @Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED) + public void created(IBaseResource theResource) { + handle(theResource); + } + + @Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED) + public void deleted(IBaseResource theResource) { + handle(theResource); + } + + @Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED) + public void updated(IBaseResource theResource) { + handle(theResource); + } + + private void handle(IBaseResource theResource) { + if (theResource != null && myFhirContext.getResourceType(theResource).equals("SearchParameter")) { + requestRefresh(); + } + } + + } + + public static class Job implements HapiJob { + @Autowired + private ISearchParamRegistry myTarget; + + @Override + public void execute(JobExecutionContext theContext) { + myTarget.refreshCacheIfNecessary(); + } + } + + public static Map> createBuiltInSearchParamMap(FhirContext theFhirContext) { + Map> resourceNameToSearchParams = new HashMap<>(); + + Set resourceNames = theFhirContext.getResourceTypes(); + + for (String resourceName : resourceNames) { + RuntimeResourceDefinition nextResDef = theFhirContext.getResourceDefinition(resourceName); + String nextResourceName = nextResDef.getName(); + HashMap nameToParam = new HashMap<>(); + resourceNameToSearchParams.put(nextResourceName, nameToParam); + + for (RuntimeSearchParam nextSp : nextResDef.getSearchParams()) { + nameToParam.put(nextSp.getName(), nextSp); + } + } + return Collections.unmodifiableMap(resourceNameToSearchParams); + } } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParameterCanonicalizer.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParameterCanonicalizer.java index 0ce4a925e79..87f8ec0bc7d 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParameterCanonicalizer.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParameterCanonicalizer.java @@ -96,31 +96,33 @@ public class SearchParameterCanonicalizer { String path = theNextSp.getXpath(); RestSearchParameterTypeEnum paramType = null; RuntimeSearchParam.RuntimeSearchParamStatusEnum status = null; - switch (theNextSp.getTypeElement().getValueAsEnum()) { - case COMPOSITE: - paramType = RestSearchParameterTypeEnum.COMPOSITE; - break; - case DATE_DATETIME: - paramType = RestSearchParameterTypeEnum.DATE; - break; - case NUMBER: - paramType = RestSearchParameterTypeEnum.NUMBER; - break; - case QUANTITY: - paramType = RestSearchParameterTypeEnum.QUANTITY; - break; - case REFERENCE: - paramType = RestSearchParameterTypeEnum.REFERENCE; - break; - case STRING: - paramType = RestSearchParameterTypeEnum.STRING; - break; - case TOKEN: - paramType = RestSearchParameterTypeEnum.TOKEN; - break; - case URI: - paramType = RestSearchParameterTypeEnum.URI; - break; + if (theNextSp.getTypeElement().getValueAsEnum() != null) { + switch (theNextSp.getTypeElement().getValueAsEnum()) { + case COMPOSITE: + paramType = RestSearchParameterTypeEnum.COMPOSITE; + break; + case DATE_DATETIME: + paramType = RestSearchParameterTypeEnum.DATE; + break; + case NUMBER: + paramType = RestSearchParameterTypeEnum.NUMBER; + break; + case QUANTITY: + paramType = RestSearchParameterTypeEnum.QUANTITY; + break; + case REFERENCE: + paramType = RestSearchParameterTypeEnum.REFERENCE; + break; + case STRING: + paramType = RestSearchParameterTypeEnum.STRING; + break; + case TOKEN: + paramType = RestSearchParameterTypeEnum.TOKEN; + break; + case URI: + paramType = RestSearchParameterTypeEnum.URI; + break; + } } if (theNextSp.getStatus() != null) { switch (theNextSp.getStatusElement().getValueAsEnum()) { diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java index 43eab94aa59..789c6716570 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImplTest.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.searchparam.registry; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.sched.ISchedulerService; import ca.uhn.fhir.rest.server.SimpleBundleProvider; @@ -46,7 +47,7 @@ public class SearchParamRegistryImplTest { @MockBean private ModelConfig myModelConfig; @MockBean - private IInterceptorBroadcaster myInterceptorBroadcaster; + private IInterceptorService myInterceptorBroadcaster; @Configuration static class SpringConfig {