honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use integers for date ordinals (#3346) 

* Use integers for date ordinals

* Add changelog

* Adjust changelog
This commit is contained in:
James Agnew 2022-02-19 18:50:18 -05:00 committed by GitHub
parent c6122bcb1b
commit 058e53616f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 106 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateUtils.java
View File

@ -242,27 +242,27 @@ public final class DateUtils {
public static Pair<String, String> getCompletedDate(String theIncompleteDateStr) {
if (StringUtils.isBlank(theIncompleteDateStr))
return new ImmutablePair<String, String>(null, null);
return new ImmutablePair<>(null, null);
String lbStr, upStr;
// YYYY only, return the last day of the year
if (theIncompleteDateStr.length() == 4) {
lbStr = theIncompleteDateStr + "-01-01"; // first day of the year
upStr = theIncompleteDateStr + "-12-31"; // last day of the year
return new ImmutablePair<String, String>(lbStr, upStr);
return new ImmutablePair<>(lbStr, upStr);
}
// Not YYYY-MM, no change
if (theIncompleteDateStr.length() != 7)
return new ImmutablePair<String, String>(theIncompleteDateStr, theIncompleteDateStr);
return new ImmutablePair<>(theIncompleteDateStr, theIncompleteDateStr);
// YYYY-MM Only
Date lb=null;
Date lb;
try {
// first day of the month
lb = new SimpleDateFormat("yyyy-MM-dd").parse(theIncompleteDateStr+"-01");
} catch (ParseException e) {
return new ImmutablePair<String, String>(theIncompleteDateStr, theIncompleteDateStr);
return new ImmutablePair<>(theIncompleteDateStr, theIncompleteDateStr);
}
// last day of the month
@ -278,7 +278,7 @@ public final class DateUtils {
lbStr = new SimpleDateFormat("yyyy-MM-dd").format(lb);
upStr = new SimpleDateFormat("yyyy-MM-dd").format(ub);
return new ImmutablePair<String, String>(lbStr, upStr);
return new ImmutablePair<>(lbStr, upStr);
}
public static Date getEndOfDay(Date theDate) {

5
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3346-always-use-integers-for-date-ordinals.yaml Normal file
View File

@ -0,0 +1,5 @@
---
type: fix
issue: 3346
title: "When searching for date search parameters on Postgres, ordinals could sometimes be
represented as strings, causing a search failure. This has been corrected."

2
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java
View File

@ -404,7 +404,7 @@ public class QueryStack {
List<Condition> codePredicates = new ArrayList<>();
for (IQueryParameterType nextOr : theList) {
Condition p = predicateBuilder.createPredicateDateWithoutIdentityPredicate(nextOr, predicateBuilder, theOperation);
Condition p = predicateBuilder.createPredicateDateWithoutIdentityPredicate(nextOr, theOperation);
codePredicates.add(p);
}

51
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/DatePredicateBuilder.java
View File

@ -31,7 +31,7 @@ import ca.uhn.fhir.rest.param.DateRangeParam;
import ca.uhn.fhir.rest.param.ParamPrefixEnum;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.util.DateUtils;
import com.google.common.annotations.VisibleForTesting;
import com.healthmarketscience.sqlbuilder.ComboCondition;
import com.healthmarketscience.sqlbuilder.Condition;
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn;
@ -48,7 +48,6 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
private final DbColumn myColumnValueLow;
private final DbColumn myColumnValueLowDateOrdinal;
private final DbColumn myColumnValueHighDateOrdinal;
@Autowired
private DaoConfig myDaoConfig;
@ -64,9 +63,12 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
myColumnValueHighDateOrdinal = getTable().addColumn("SP_VALUE_HIGH_DATE_ORDINAL");
}
@VisibleForTesting
public void setDaoConfigForUnitTest(DaoConfig theDaoConfig) {
myDaoConfig = theDaoConfig;
}
public Condition createPredicateDateWithoutIdentityPredicate(IQueryParameterType theParam,
DatePredicateBuilder theFrom,
SearchFilterParser.CompareOperation theOperation) {
Condition p;
@ -77,17 +79,14 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
date = new DateParam(ParamPrefixEnum.EQUAL, date.getValueAsString());
}
DateRangeParam range = new DateRangeParam(date);
p = createPredicateDateFromRange(theFrom, range, theOperation);
p = createPredicateDateFromRange(range, theOperation);
} else {
// TODO: handle missing date param?
p = null;
}
} else if (theParam instanceof DateRangeParam) {
DateRangeParam range = (DateRangeParam) theParam;
p = createPredicateDateFromRange(
theFrom,
range,
theOperation);
p = createPredicateDateFromRange(range, theOperation);
} else {
throw new IllegalArgumentException(Msg.code(1251) + "Invalid token type: " + theParam.getClass());
}
@ -95,8 +94,7 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
return p;
}
private Condition createPredicateDateFromRange(DatePredicateBuilder theFrom,
DateRangeParam theRange,
private Condition createPredicateDateFromRange(DateRangeParam theRange,
SearchFilterParser.CompareOperation theOperation) {
@ -129,8 +127,7 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
genericLowerBound = lowerBoundAsOrdinal;
genericUpperBound = upperBoundAsOrdinal;
if (upperBound != null && upperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.MONTH.ordinal()) {
genericUpperBound = DateUtils.getCompletedDate(upperBound.getValueAsString()).getRight().replace("-", "");
genericUpperBound = Integer.parseInt(DateUtils.getCompletedDate(upperBound.getValueAsString()).getRight().replace("-", ""));
}
} else {
lowValueField = DatePredicateBuilder.ColumnEnum.LOW;
@ -138,7 +135,7 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
genericLowerBound = lowerBoundInstant;
genericUpperBound = upperBoundInstant;
if (upperBound != null && upperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.MONTH.ordinal()) {
String theCompleteDateStr = DateUtils.getCompletedDate(upperBound.getValueAsString()).getRight().replace("-", "");
String theCompleteDateStr = DateUtils.getCompletedDate(upperBound.getValueAsString()).getRight().replace("-", "");
genericUpperBound = DateUtils.parseDate(theCompleteDateStr);
}
}
@ -146,14 +143,14 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
if (theOperation == SearchFilterParser.CompareOperation.lt || theOperation == SearchFilterParser.CompareOperation.le) {
// use lower bound first
if (lowerBoundInstant != null) {
lb = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericLowerBound);
lb = this.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericLowerBound);
if (myDaoConfig.isAccountForDateIndexNulls()) {
lb = ComboCondition.or(lb, theFrom.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericLowerBound));
lb = ComboCondition.or(lb, this.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericLowerBound));
}
} else if (upperBoundInstant != null) {
ub = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
ub = this.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
if (myDaoConfig.isAccountForDateIndexNulls()) {
ub = ComboCondition.or(ub, theFrom.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound));
ub = ComboCondition.or(ub, this.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound));
}
} else {
throw new InvalidRequestException(Msg.code(1252) + "lowerBound and upperBound value not correctly specified for comparing " + theOperation);
@ -161,14 +158,14 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
} else if (theOperation == SearchFilterParser.CompareOperation.gt || theOperation == SearchFilterParser.CompareOperation.ge) {
// use upper bound first, e.g value between 6 and 10
if (upperBoundInstant != null) {
ub = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericUpperBound);
ub = this.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericUpperBound);
if (myDaoConfig.isAccountForDateIndexNulls()) {
ub = ComboCondition.or(ub, theFrom.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericUpperBound));
ub = ComboCondition.or(ub, this.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericUpperBound));
}
} else if (lowerBoundInstant != null) {
lb = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
lb = this.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
if (myDaoConfig.isAccountForDateIndexNulls()) {
lb = ComboCondition.or(lb, theFrom.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound));
lb = ComboCondition.or(lb, this.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound));
}
} else {
throw new InvalidRequestException(Msg.code(1253) + "upperBound and lowerBound value not correctly specified for compare theOperation");
@ -178,16 +175,16 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
(upperBoundInstant == null)) {
throw new InvalidRequestException(Msg.code(1254) + "lowerBound and/or upperBound value not correctly specified for compare theOperation");
}
lt = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN, genericLowerBound);
gt = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN, genericUpperBound);
lt = this.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN, genericLowerBound);
gt = this.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN, genericUpperBound);
lb = ComboCondition.or(lt, gt);
} else if ((theOperation == SearchFilterParser.CompareOperation.eq)
|| (theOperation == SearchFilterParser.CompareOperation.sa)
|| (theOperation == SearchFilterParser.CompareOperation.eb)
|| (theOperation == null)) {
if (lowerBoundInstant != null) {
gt = theFrom.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
lt = theFrom.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
gt = this.createPredicate(lowValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
lt = this.createPredicate(highValueField, ParamPrefixEnum.GREATERTHAN_OR_EQUALS, genericLowerBound);
if (lowerBound.getPrefix() == ParamPrefixEnum.STARTS_AFTER || lowerBound.getPrefix() == ParamPrefixEnum.EQUAL) {
lb = gt;
@ -197,8 +194,8 @@ public class DatePredicateBuilder extends BaseSearchParamPredicateBuilder {
}
if (upperBoundInstant != null) {
gt = theFrom.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
lt = theFrom.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
gt = this.createPredicate(lowValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
lt = this.createPredicate(highValueField, ParamPrefixEnum.LESSTHAN_OR_EQUALS, genericUpperBound);
if (theRange.getUpperBound().getPrefix() == ParamPrefixEnum.ENDS_BEFORE || theRange.getUpperBound().getPrefix() == ParamPrefixEnum.EQUAL) {

70
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilderDialectPostgresTest.java Normal file
View File

@ -0,0 +1,70 @@
package ca.uhn.fhir.jpa.search.builder.sql;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.dao.predicate.SearchFilterParser;
import ca.uhn.fhir.jpa.search.builder.predicate.BaseJoiningPredicateBuilder;
import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder;
import ca.uhn.fhir.jpa.search.builder.predicate.ResourceTablePredicateBuilder;
import ca.uhn.fhir.rest.param.DateParam;
import ca.uhn.fhir.rest.param.DateRangeParam;
import com.healthmarketscience.sqlbuilder.Condition;
import org.apache.commons.lang3.StringUtils;
import org.hibernate.dialect.Dialect;
import org.hibernate.dialect.PostgreSQL10Dialect;
import org.hibernate.dialect.PostgreSQL95Dialect;
import org.hibernate.dialect.SQLServer2012Dialect;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import javax.annotation.Nonnull;
import java.util.Locale;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class SearchQueryBuilderDialectPostgresTest extends BaseSearchQueryBuilderDialectTest {
/**
* Make sure we're using integers and not strings as bind variables
* for ordinals
*/
@Test
public void testOrdinalSearchesUseIntegerParameters() {
DaoConfig daoConfig = new DaoConfig();
daoConfig.getModelConfig().setUseOrdinalDatesForDayPrecisionSearches(true);
SearchQueryBuilder searchQueryBuilder = createSearchQueryBuilder();
when(mySqlObjectFactory.dateIndexTable(any())).thenReturn(new DatePredicateBuilder(searchQueryBuilder));
DatePredicateBuilder datePredicateBuilder = searchQueryBuilder.addDatePredicateBuilder(null);
datePredicateBuilder.setDaoConfigForUnitTest(daoConfig);
Condition datePredicate = datePredicateBuilder.createPredicateDateWithoutIdentityPredicate(new DateParam("2022"), SearchFilterParser.CompareOperation.eq);
Condition comboPredicate = datePredicateBuilder.combineWithHashIdentityPredicate("Observation", "date", datePredicate);
searchQueryBuilder.addPredicate(comboPredicate);
GeneratedSql generatedSql = searchQueryBuilder.generate(0, 500);
logSql(generatedSql);
String sql = generatedSql.getSql();
assertEquals("SELECT t0.RES_ID FROM HFJ_SPIDX_DATE t0 WHERE ((t0.HASH_IDENTITY = ?) AND ((t0.SP_VALUE_LOW_DATE_ORDINAL >= ?) AND (t0.SP_VALUE_HIGH_DATE_ORDINAL <= ?))) limit ?", sql);
assertEquals(4, StringUtils.countMatches(sql, "?"));
assertEquals(4, generatedSql.getBindVariables().size());
assertEquals(123682819940570799L, generatedSql.getBindVariables().get(0));
assertEquals(20220101, generatedSql.getBindVariables().get(1));
assertEquals(20221231, generatedSql.getBindVariables().get(2));
assertEquals(500, generatedSql.getBindVariables().get(3));
}
@Nonnull
@Override
protected Dialect createDialect() {
return new PostgreSQL10Dialect();
}
}