Fix sort in hsearch - use full millis (#3787)

Fix sort in hsearch - use full millis
This commit is contained in:
michaelabuckley 2022-07-13 13:32:40 -04:00 committed by GitHub
parent 1785c07283
commit 94a1cda512
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 45 additions and 28 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 3787
title: "FHIR queries using date sort were ignoring seconds and milliseconds when using lucene/elasticsearch SearchParameter indexing.
This has been corrected."

View File

@ -59,7 +59,7 @@ public class HSearchSortHelperImpl implements IHSearchSortHelper {
String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", "token", "system"), String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", "token", "system"),
String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", "token", "code") ), String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", "token", "code") ),
RestSearchParameterTypeEnum.REFERENCE, List.of(SEARCH_PARAM_ROOT + ".*.reference.value"), RestSearchParameterTypeEnum.REFERENCE, List.of(SEARCH_PARAM_ROOT + ".*.reference.value"),
RestSearchParameterTypeEnum.DATE, List.of(SEARCH_PARAM_ROOT + ".*.dt.lower-ord"), RestSearchParameterTypeEnum.DATE, List.of(SEARCH_PARAM_ROOT + ".*.dt.lower"),
RestSearchParameterTypeEnum.QUANTITY, List.of( RestSearchParameterTypeEnum.QUANTITY, List.of(
String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", QTY_PARAM_NAME, QTY_VALUE_NORM), String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", QTY_PARAM_NAME, QTY_VALUE_NORM),
String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", QTY_PARAM_NAME, QTY_VALUE) ), String.join(".", NESTED_SEARCH_PARAM_ROOT, "*", QTY_PARAM_NAME, QTY_VALUE) ),

View File

@ -145,7 +145,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
DependencyInjectionTestExecutionListener.class DependencyInjectionTestExecutionListener.class
,FhirResourceDaoR4SearchWithElasticSearchIT.TestDirtiesContextTestExecutionListener.class ,FhirResourceDaoR4SearchWithElasticSearchIT.TestDirtiesContextTestExecutionListener.class
}) })
public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest implements ITestDataBuilder {
public static final String URL_MY_CODE_SYSTEM = "http://example.com/my_code_system"; public static final String URL_MY_CODE_SYSTEM = "http://example.com/my_code_system";
public static final String URL_MY_VALUE_SET = "http://example.com/my_value_set"; public static final String URL_MY_VALUE_SET = "http://example.com/my_value_set";
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchWithElasticSearchIT.class); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchWithElasticSearchIT.class);
@ -229,7 +229,17 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest {
} }
@Override @Override
protected FhirContext getFhirContext() { public IIdType doCreateResource(IBaseResource theResource) {
return myTestDataBuilder.doCreateResource(theResource);
}
@Override
public IIdType doUpdateResource(IBaseResource theResource) {
return myTestDataBuilder.doUpdateResource(theResource);
}
@Override
public FhirContext getFhirContext() {
return myFhirCtx; return myFhirCtx;
} }
@ -1944,19 +1954,19 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest {
@Test @Test
public void byDate() { public void byDate() {
String id1 = myTestDataBuilder.createObservation(List.of( // check milli level precision
myTestDataBuilder.withEffectiveDate("2017-01-20T03:21:47") String id1 = createObservation(withId("20-000"), withEffectiveDate("2017-01-20T03:21:47.000")).getIdPart();
)).getIdPart(); String id2 = createObservation(withId("24-002"), withEffectiveDate("2017-01-24T03:21:47.002")).getIdPart();
String id2 = myTestDataBuilder.createObservation(List.of( String id3 = createObservation(withId("24-001"), withEffectiveDate("2017-01-24T03:21:47.001")).getIdPart();
myTestDataBuilder.withEffectiveDate("2017-01-24T03:21:47") String id4 = createObservation(withId("20-002"), withEffectiveDate("2017-01-20T03:21:47.002")).getIdPart();
)).getIdPart();
myCaptureQueriesListener.clear(); myCaptureQueriesListener.clear();
IBundleProvider result = myTestDaoSearch.searchForBundleProvider("/Observation?_sort=-date"); List<String> result = myTestDaoSearch.searchForIds("/Observation?_sort=-date");
assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "we build the bundle with no sql"); assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "we build the bundle with no sql");
// requesteddate descending so order should be id2, id1 ourLog.info("byDate sort {}", result);
assertThat(getResultIds(result), contains(id2, id1)); // date descending - order should be id2, id1
assertThat(result, contains(id2, id3, id4, id1));
} }
@Test @Test

View File

@ -1,8 +1,5 @@
<configuration> <configuration>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender"> <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<filter class="ch.qos.logback.classic.filter.ThresholdFilter">
<level>INFO</level>
</filter>
<encoder> <encoder>
<pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] %msg%n</pattern> <pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] %msg%n</pattern>
</encoder> </encoder>
@ -27,10 +24,6 @@
<appender-ref ref="STDOUT" /> <appender-ref ref="STDOUT" />
</logger> </logger>
<logger name="org.elasticsearch.client" additivity="true" level="trace">
<appender-ref ref="STDOUT" />
</logger>
<logger name="org.eclipse" additivity="false" level="error"> <logger name="org.eclipse" additivity="false" level="error">
</logger> </logger>
@ -43,7 +36,7 @@
</logger> </logger>
<!-- set to debug to enable term expansion logs --> <!-- set to debug to enable term expansion logs -->
<logger name="ca.uhn.fhir.jpa.term" additivity="false" level="debug"> <logger name="ca.uhn.fhir.jpa.term" additivity="false" level="info">
<appender-ref ref="STDOUT" /> <appender-ref ref="STDOUT" />
</logger> </logger>
@ -56,15 +49,17 @@
<appender-ref ref="STDOUT" /> <appender-ref ref="STDOUT" />
</logger> </logger>
<logger name="org.hibernate.search.elasticsearch.request" additivity="false" level="trace">
<appender-ref ref="STDOUT" />
</logger>
<!-- <!--
<logger name="ca.uhn.fhir.jpa.model.search" additivity="false" level="debug"> <logger name="ca.uhn.fhir.jpa.model.search" additivity="false" level="debug"/>
<appender-ref ref="STDOUT" /> <logger name="org.elasticsearch.client" additivity="true" level="trace"/>
</logger> <logger name="org.hibernate.search.elasticsearch.request" additivity="false" level="trace"/>
--> <logger name="org.hibernate.search" level="TRACE"/>
<logger name="org.springframework.test.context.cache" additivity="false" level="debug"> <logger name="org.hibernate.search.query" level="TRACE"/>
<logger name="org.hibernate.search.elasticsearch.request" level="TRACE"/>
-->
<!-- See https://docs.jboss.org/hibernate/stable/search/reference/en-US/html_single/#backend-lucene-io-writer-infostream for lucene logging
<logger name="org.hibernate.search.backend.lucene.infostream" level="TRACE"/> -->
<logger name="org.springframework.test.context.cache" additivity="false" level="info">
<appender-ref ref="STDOUT" /> <appender-ref ref="STDOUT" />
</logger> </logger>

View File

@ -37,6 +37,12 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
/**
* Implements ITestDataBuilder via a live DaoRegistry.
*
* Add the inner {@link Config} to your spring context to inject this.
* For convenience, you can still implement ITestDataBuilder on your test class, and delegate the missing methods to this bean.
*/
public class DaoTestDataBuilder implements ITestDataBuilder, AfterEachCallback { public class DaoTestDataBuilder implements ITestDataBuilder, AfterEachCallback {
private static final Logger ourLog = LoggerFactory.getLogger(DaoTestDataBuilder.class); private static final Logger ourLog = LoggerFactory.getLogger(DaoTestDataBuilder.class);
@ -84,6 +90,7 @@ public class DaoTestDataBuilder implements ITestDataBuilder, AfterEachCallback {
ourLog.info("cleanup {}", myIds); ourLog.info("cleanup {}", myIds);
myIds.keySet().forEach(nextType->{ myIds.keySet().forEach(nextType->{
// todo do this in a bundle for perf.
IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(nextType); IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(nextType);
myIds.get(nextType).forEach(dao::delete); myIds.get(nextType).forEach(dao::delete);
}); });