diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4803-forced-id-step-2.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/4803-forced-id-step-2.yaml similarity index 100% rename from hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4803-forced-id-step-2.yaml rename to hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/4803-forced-id-step-2.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5405-use-new-fhir-id-for-sort.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5405-use-new-fhir-id-for-sort.yaml new file mode 100644 index 000000000000..85c322066d13 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5405-use-new-fhir-id-for-sort.yaml @@ -0,0 +1,4 @@ +--- +type: perf +issue: 5405 +title: "Sorting by _id now uses the FHIR_ID column on HFJ_RESOURCE and avoid joins." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialect.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialect.java index 0339ea695469..463e3a471cb6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialect.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialect.java @@ -96,7 +96,7 @@ private DataAccessException convertHibernateAccessException( + makeErrorMessage( messageToPrepend, "resourceIndexedCompositeStringUniqueConstraintFailure")); } - if (constraintName.contains(ResourceTable.IDX_RES_FHIR_ID)) { + if (constraintName.contains(ResourceTable.IDX_RES_TYPE_FHIR_ID)) { throw new ResourceVersionConflictException( Msg.code(825) + makeErrorMessage(messageToPrepend, "forcedIdConstraintFailure")); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java index 3a44a5f8ed2c..c6a27252a954 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java @@ -114,7 +114,6 @@ import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder; -import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder; @@ -613,12 +612,6 @@ public DatePredicateBuilder newDatePredicateBuilder(SearchQueryBuilder theSearch return new DatePredicateBuilder(theSearchBuilder); } - @Bean - @Scope("prototype") - public ForcedIdPredicateBuilder newForcedIdPredicateBuilder(SearchQueryBuilder theSearchBuilder) { - return new ForcedIdPredicateBuilder(theSearchBuilder); - } - @Bean @Scope("prototype") public NumberPredicateBuilder newNumberPredicateBuilder(SearchQueryBuilder theSearchBuilder) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 444549e8968c..a0f718dceb82 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -118,12 +118,19 @@ protected void init700() { Builder.BuilderWithTableName hfjResource = version.onTable("HFJ_RESOURCE"); hfjResource.modifyColumn("20231018.2", "FHIR_ID").nonNullable(); + + hfjResource.dropIndex("20231027.1", "IDX_RES_FHIR_ID"); hfjResource - .addIndex("20231018.3", "IDX_RES_FHIR_ID") + .addIndex("20231027.2", "IDX_RES_TYPE_FHIR_ID") .unique(true) .online(true) - .includeColumns("RES_ID") - .withColumns("FHIR_ID", "RES_TYPE"); + // include res_id and our deleted flag so we can satisfy Observation?_sort=_id from the index on + // platforms that support it. + .includeColumns("RES_ID, RES_DELETED_AT") + .withColumns("RES_TYPE", "FHIR_ID"); + + // For resolving references that don't supply the type. + hfjResource.addIndex("20231027.3", "IDX_RES_FHIR_ID").unique(false).withColumns("FHIR_ID"); } protected void init680() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 7359ed1136da..d867f312d64d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -43,7 +43,6 @@ import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder; -import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.ICanMakeMissingParamPredicate; import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.ParsedLocationParam; @@ -95,7 +94,6 @@ import com.healthmarketscience.sqlbuilder.Condition; import com.healthmarketscience.sqlbuilder.Expression; import com.healthmarketscience.sqlbuilder.InCondition; -import com.healthmarketscience.sqlbuilder.OrderObject; import com.healthmarketscience.sqlbuilder.SelectQuery; import com.healthmarketscience.sqlbuilder.SetOperationQuery; import com.healthmarketscience.sqlbuilder.Subquery; @@ -276,16 +274,15 @@ public void addSortOnQuantity(String theResourceName, String theParamName, boole } public void addSortOnResourceId(boolean theAscending) { + ResourceTablePredicateBuilder resourceTablePredicateBuilder; BaseJoiningPredicateBuilder firstPredicateBuilder = mySqlBuilder.getOrCreateFirstPredicateBuilder(); - ForcedIdPredicateBuilder sortPredicateBuilder = - mySqlBuilder.addForcedIdPredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); - if (!theAscending) { - mySqlBuilder.addSortString( - sortPredicateBuilder.getColumnForcedId(), false, OrderObject.NullOrder.FIRST, myUseAggregate); + if (firstPredicateBuilder instanceof ResourceTablePredicateBuilder) { + resourceTablePredicateBuilder = (ResourceTablePredicateBuilder) firstPredicateBuilder; } else { - mySqlBuilder.addSortString(sortPredicateBuilder.getColumnForcedId(), true, myUseAggregate); + resourceTablePredicateBuilder = + mySqlBuilder.addResourceTablePredicateBuilder(firstPredicateBuilder.getResourceIdColumn()); } - mySqlBuilder.addSortNumeric(firstPredicateBuilder.getResourceIdColumn(), theAscending, myUseAggregate); + mySqlBuilder.addSortString(resourceTablePredicateBuilder.getColumnFhirId(), theAscending, myUseAggregate); } public void addSortOnResourceLink( diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ForcedIdPredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ForcedIdPredicateBuilder.java deleted file mode 100644 index 6f8d1e1b66c5..000000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ForcedIdPredicateBuilder.java +++ /dev/null @@ -1,51 +0,0 @@ -/*- - * #%L - * HAPI FHIR JPA Server - * %% - * Copyright (C) 2014 - 2023 Smile CDR, Inc. - * %% - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * #L% - */ -package ca.uhn.fhir.jpa.search.builder.predicate; - -import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; -import com.healthmarketscience.sqlbuilder.dbspec.basic.DbColumn; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class ForcedIdPredicateBuilder extends BaseJoiningPredicateBuilder { - - private static final Logger ourLog = LoggerFactory.getLogger(ForcedIdPredicateBuilder.class); - private final DbColumn myColumnResourceId; - private final DbColumn myColumnForcedId; - - /** - * Constructor - */ - public ForcedIdPredicateBuilder(SearchQueryBuilder theSearchSqlBuilder) { - super(theSearchSqlBuilder, theSearchSqlBuilder.addTable("HFJ_FORCED_ID")); - - myColumnResourceId = getTable().addColumn("RESOURCE_PID"); - myColumnForcedId = getTable().addColumn("FORCED_ID"); - } - - @Override - public DbColumn getResourceIdColumn() { - return myColumnResourceId; - } - - public DbColumn getColumnForcedId() { - return myColumnForcedId; - } -} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceTablePredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceTablePredicateBuilder.java index c9fb8845dd7f..75ccddb11706 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceTablePredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceTablePredicateBuilder.java @@ -19,6 +19,7 @@ */ package ca.uhn.fhir.jpa.search.builder.predicate; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; import ca.uhn.fhir.jpa.util.QueryParameterUtils; import com.healthmarketscience.sqlbuilder.BinaryCondition; @@ -35,6 +36,7 @@ public class ResourceTablePredicateBuilder extends BaseJoiningPredicateBuilder { private final DbColumn myColumnResType; private final DbColumn myColumnLastUpdated; private final DbColumn myColumnLanguage; + private final DbColumn myColumnFhirId; /** * Constructor @@ -42,10 +44,11 @@ public class ResourceTablePredicateBuilder extends BaseJoiningPredicateBuilder { public ResourceTablePredicateBuilder(SearchQueryBuilder theSearchSqlBuilder) { super(theSearchSqlBuilder, theSearchSqlBuilder.addTable("HFJ_RESOURCE")); myColumnResId = getTable().addColumn("RES_ID"); - myColumnResType = getTable().addColumn("RES_TYPE"); + myColumnResType = getTable().addColumn(ResourceTable.RES_TYPE); myColumnResDeletedAt = getTable().addColumn("RES_DELETED_AT"); myColumnLastUpdated = getTable().addColumn("RES_UPDATED"); myColumnLanguage = getTable().addColumn("RES_LANGUAGE"); + myColumnFhirId = getTable().addColumn(ResourceTable.FHIR_ID); } @Override @@ -77,4 +80,8 @@ public Condition createLanguagePredicate(Set theValues, boolean theNegat public DbColumn getColumnLastUpdated() { return myColumnLastUpdated; } + + public DbColumn getColumnFhirId() { + return myColumnFhirId; + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java index 74091779b9ec..e840bdec9d38 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryBuilder.java @@ -32,7 +32,6 @@ import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder; -import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder; @@ -62,7 +61,6 @@ import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable; -import org.apache.commons.lang3.Validate; import org.hibernate.dialect.Dialect; import org.hibernate.dialect.SQLServerDialect; import org.hibernate.dialect.pagination.AbstractLimitHandler; @@ -222,18 +220,6 @@ public DatePredicateBuilder createDatePredicateBuilder() { return mySqlBuilderFactory.dateIndexTable(this); } - /** - * Add and return a predicate builder for selecting a forced ID. This is only intended for use with sorts so it can not - * be the root query. - */ - public ForcedIdPredicateBuilder addForcedIdPredicateBuilder(@Nonnull DbColumn theSourceJoinColumn) { - Validate.isTrue(theSourceJoinColumn != null); - - ForcedIdPredicateBuilder retVal = mySqlBuilderFactory.newForcedIdPredicateBuilder(this); - addTableForSorting(retVal, theSourceJoinColumn); - return retVal; - } - /** * Create, add and return a predicate builder (or a root query if no root query exists yet) for selecting on a NUMBER search parameter */ @@ -417,11 +403,6 @@ private void addTable(BaseJoiningPredicateBuilder thePredicateBuilder, @Nullable addTable(thePredicateBuilder, theSourceJoinColumn, SelectQuery.JoinType.INNER); } - private void addTableForSorting( - BaseJoiningPredicateBuilder thePredicateBuilder, @Nullable DbColumn theSourceJoinColumn) { - addTable(thePredicateBuilder, theSourceJoinColumn, SelectQuery.JoinType.LEFT_OUTER); - } - private void addTable( BaseJoiningPredicateBuilder thePredicateBuilder, @Nullable DbColumn theSourceJoinColumn, @@ -766,7 +747,7 @@ public void excludeResourceIdsPredicate(Set theExistingPidSetToExclude) List excludePids = JpaPid.toLongList(theExistingPidSetToExclude); - ourLog.trace("excludePids = " + excludePids); + ourLog.trace("excludePids = {}", excludePids); DbColumn resourceIdColumn = getOrCreateFirstPredicateBuilder().getResourceIdColumn(); InCondition predicate = new InCondition(resourceIdColumn, generatePlaceholders(excludePids)); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SqlObjectFactory.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SqlObjectFactory.java index 29e04527f967..621fe211185a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SqlObjectFactory.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SqlObjectFactory.java @@ -24,7 +24,6 @@ import ca.uhn.fhir.jpa.search.builder.predicate.ComboUniqueSearchParameterPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.CoordsPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.DatePredicateBuilder; -import ca.uhn.fhir.jpa.search.builder.predicate.ForcedIdPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.NumberPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityNormalizedPredicateBuilder; import ca.uhn.fhir.jpa.search.builder.predicate.QuantityPredicateBuilder; @@ -63,10 +62,6 @@ public DatePredicateBuilder dateIndexTable(SearchQueryBuilder theSearchSqlBuilde return myApplicationContext.getBean(DatePredicateBuilder.class, theSearchSqlBuilder); } - public ForcedIdPredicateBuilder newForcedIdPredicateBuilder(SearchQueryBuilder theSearchSqlBuilder) { - return myApplicationContext.getBean(ForcedIdPredicateBuilder.class, theSearchSqlBuilder); - } - public NumberPredicateBuilder numberIndexTable(SearchQueryBuilder theSearchSqlBuilder) { return myApplicationContext.getBean(NumberPredicateBuilder.class, theSearchSqlBuilder); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java index edc52f2e4a04..447e3058c4d2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java @@ -2979,7 +2979,7 @@ public Optional readCodeSystemByForcedId(String theForcedId) { if (resultList.size() > 1) throw new NonUniqueResultException(Msg.code(911) + "More than one CodeSystem is pointed by forcedId: " - + theForcedId + ". Was constraint " + ResourceTable.IDX_RES_FHIR_ID + " removed?"); + + theForcedId + ". Was constraint " + ResourceTable.IDX_RES_TYPE_FHIR_ID + " removed?"); IFhirResourceDao csDao = myDaoRegistry.getResourceDao("CodeSystem"); IBaseResource cs = myJpaStorageResourceParser.toResource(resultList.get(0), false); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java index f516209a3cdc..bf1ca3b56ac6 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java @@ -76,7 +76,7 @@ import javax.persistence.UniqueConstraint; import javax.persistence.Version; -import static ca.uhn.fhir.jpa.model.entity.ResourceTable.IDX_RES_FHIR_ID; +import static ca.uhn.fhir.jpa.model.entity.ResourceTable.IDX_RES_TYPE_FHIR_ID; @Indexed(routingBinder = @RoutingBinderRef(type = ResourceTableRoutingBinder.class)) @Entity @@ -84,12 +84,13 @@ name = ResourceTable.HFJ_RESOURCE, uniqueConstraints = { @UniqueConstraint( - name = IDX_RES_FHIR_ID, - columnNames = {"FHIR_ID", "RES_TYPE"}) + name = IDX_RES_TYPE_FHIR_ID, + columnNames = {"RES_TYPE", "FHIR_ID"}) }, indexes = { // Do not reuse previously used index name: IDX_INDEXSTATUS, IDX_RES_TYPE @Index(name = "IDX_RES_DATE", columnList = BaseHasResource.RES_UPDATED), + @Index(name = "IDX_RES_FHIR_ID", columnList = "FHIR_ID"), @Index( name = "IDX_RES_TYPE_DEL_UPDATED", columnList = "RES_TYPE,RES_DELETED_AT,RES_UPDATED,PARTITION_ID,RES_ID"), @@ -100,10 +101,11 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas public static final int RESTYPE_LEN = 40; public static final String HFJ_RESOURCE = "HFJ_RESOURCE"; public static final String RES_TYPE = "RES_TYPE"; + public static final String FHIR_ID = "FHIR_ID"; private static final int MAX_LANGUAGE_LENGTH = 20; private static final long serialVersionUID = 1L; public static final int MAX_FORCED_ID_LENGTH = 100; - public static final String IDX_RES_FHIR_ID = "IDX_RES_FHIR_ID"; + public static final String IDX_RES_TYPE_FHIR_ID = "IDX_RES_TYPE_FHIR_ID"; /** * Holds the narrative text only - Used for Fulltext searching but not directly stored in the DB @@ -381,7 +383,7 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas * Will be null during insert time until the first read. */ @Column( - name = "FHIR_ID", + name = FHIR_ID, // [A-Za-z0-9\-\.]{1,64} - https://www.hl7.org/fhir/datatypes.html#id length = 64, // we never update this after insert, and the Generator will otherwise "dirty" the object. diff --git a/hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java b/hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java index 7037f682ec5e..6fbd1c642c9b 100644 --- a/hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java @@ -2189,21 +2189,21 @@ public void testSortById() { pm.setSort(new SortSpec(BaseResource.SP_RES_ID)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual, contains(idMethodName, id1, id2, id3, id4)); + assertThat(actual, contains(id1, id2, id3, id4, idMethodName)); pm = new SearchParameterMap(); pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); pm.setSort(new SortSpec(BaseResource.SP_RES_ID).setOrder(SortOrderEnum.ASC)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual, contains(idMethodName, id1, id2, id3, id4)); + assertThat(actual, contains(id1, id2, id3, id4, idMethodName)); pm = new SearchParameterMap(); pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); pm.setSort(new SortSpec(BaseResource.SP_RES_ID).setOrder(SortOrderEnum.DESC)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual, contains(id4, id3, id2, id1, idMethodName)); + assertThat(actual, contains(idMethodName, id4, id3, id2, id1)); } @Test diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 7c2d95ac1164..db98eb767bdd 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -3323,7 +3323,7 @@ public void testSortOnId() throws Exception { map = new SearchParameterMap(); map.setSort(new SortSpec("_id", SortOrderEnum.ASC)); ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); - assertThat(ids, contains("Patient/AA", "Patient/AB", id1, id2)); + assertThat(ids, contains(id1, id2, "Patient/AA", "Patient/AB")); } diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java index 184a18fb8aa3..7b2b699d85fd 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java @@ -2788,21 +2788,21 @@ public void testSortById() { pm.setSort(new SortSpec(IAnyResource.SP_RES_ID)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual.toString(), actual, contains(idMethodName, id1, id2, id3, id4)); + assertThat(actual.toString(), actual, contains(id1, id2, id3, id4, idMethodName)); pm = new SearchParameterMap(); pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.ASC)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual.toString(), actual, contains(idMethodName, id1, id2, id3, id4)); + assertThat(actual.toString(), actual, contains(id1, id2, id3, id4, idMethodName)); pm = new SearchParameterMap(); pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.DESC)); actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); assertEquals(5, actual.size()); - assertThat(actual.toString(), actual, contains(id4, id3, id2, id1, idMethodName)); + assertThat(actual.toString(), actual, contains(idMethodName, id4, id3, id2, id1)); } @Test diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java index 2700b1b24898..e91e3421bf2c 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java @@ -138,7 +138,7 @@ protected void createUniqueCompositeSp() { protected void dropForcedIdUniqueConstraint() { runInTransaction(() -> { myEntityManager.createNativeQuery("alter table " + ForcedId.HFJ_FORCED_ID + " drop constraint " + ForcedId.IDX_FORCEDID_TYPE_FID).executeUpdate(); - myEntityManager.createNativeQuery("alter table " + ResourceTable.HFJ_RESOURCE + " drop constraint " + ResourceTable.IDX_RES_FHIR_ID).executeUpdate(); + myEntityManager.createNativeQuery("alter table " + ResourceTable.HFJ_RESOURCE + " drop constraint " + ResourceTable.IDX_RES_TYPE_FHIR_ID).executeUpdate(); }); myHaveDroppedForcedIdUniqueConstraint = true; } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SortTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SortTest.java index ea781b9e4cc1..23ab501a2274 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SortTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SortTest.java @@ -89,12 +89,12 @@ public void testSortOnId() throws Exception { map = new SearchParameterMap(); map.setSort(new SortSpec("_id", SortOrderEnum.ASC)); ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); - assertThat(ids, contains("Patient/AA", "Patient/AB", id1, id2)); + assertThat(ids, contains(id1, id2, "Patient/AA", "Patient/AB")); map = new SearchParameterMap(); map.setSort(new SortSpec("_id", SortOrderEnum.DESC)); ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); - assertThat(ids, contains(id2, id1, "Patient/AB", "Patient/AA")); + assertThat(ids, contains("Patient/AB", "Patient/AA", id2, id1)); } @Test diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java index 3236f6b60fd3..a605f0bc0ab7 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.TestDaoSearch; import ca.uhn.fhir.jpa.search.CompositeSearchParameterTestCases; +import ca.uhn.fhir.jpa.search.IIdSearchTestTemplate; import ca.uhn.fhir.jpa.search.QuantitySearchParameterTestCases; import ca.uhn.fhir.jpa.search.BaseSourceSearchParameterTestCases; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; @@ -13,6 +14,7 @@ import ca.uhn.fhir.jpa.test.config.TestR4Config; import ca.uhn.fhir.storage.test.BaseDateSearchDaoTests; import ca.uhn.fhir.storage.test.DaoTestDataBuilder; +import ca.uhn.fhir.test.utilities.ITestDataBuilder; import ca.uhn.fhir.test.utilities.ITestDataBuilder.ICreationArgument; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Observation; @@ -256,10 +258,10 @@ public void sortBySystemThenValue() { String idExM = withObservation(myDataBuilder.withObservationCode("http://example.org", "MValue")).getIdPart(); List allIds = myTestDaoSearch.searchForIds("/Observation?_sort=code"); - assertThat(allIds, hasItems(idAlphaA, idAlphaM, idAlphaZ, idExA, idExD, idExM)); + assertThat(allIds, contains(idAlphaA, idAlphaM, idAlphaZ, idExA, idExD, idExM)); allIds = myTestDaoSearch.searchForIds("/Observation?_sort=code&code=http://example.org|"); - assertThat(allIds, hasItems(idExA, idExD, idExM)); + assertThat(allIds, contains(idExA, idExD, idExM)); } } } @@ -368,7 +370,7 @@ public void sortByNumeric() { String idAlpha5 = withRiskAssessmentWithProbabilty(0.5).getIdPart(); List allIds = myTestDaoSearch.searchForIds("/RiskAssessment?_sort=probability"); - assertThat(allIds, hasItems(idAlpha2, idAlpha5, idAlpha7)); + assertThat(allIds, contains(idAlpha2, idAlpha5, idAlpha7)); } } @@ -491,12 +493,25 @@ public void sortByNumeric() { String idAlpha5 = withObservationWithValueQuantity(0.5).getIdPart(); List allIds = myTestDaoSearch.searchForIds("/Observation?_sort=value-quantity"); - assertThat(allIds, hasItems(idAlpha2, idAlpha5, idAlpha7)); + assertThat(allIds, contains(idAlpha2, idAlpha5, idAlpha7)); } } } + @Nested + public class IdSearch implements IIdSearchTestTemplate { + @Override + public TestDaoSearch getSearch() { + return myTestDaoSearch; + } + + @Override + public ITestDataBuilder getBuilder() { + return myDataBuilder; + } + } + // todo mb re-enable this. Some of these fail! @Disabled @Nested diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java index d06cba7de2b9..ab11fcd27f13 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java @@ -3352,63 +3352,6 @@ public void testSortByInvalidParameter() { } - @Test - public void testSortById() { - String methodName = "testSortBTyId"; - - Patient p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType id1 = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); - - p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType id2 = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); - - p = new Patient(); - p.setId(methodName + "1"); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType idMethodName1 = myPatientDao.update(p, mySrd).getId().toUnqualifiedVersionless(); - assertEquals(methodName + "1", idMethodName1.getIdPart()); - - p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType id3 = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); - - p = new Patient(); - p.setId(methodName + "2"); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType idMethodName2 = myPatientDao.update(p, mySrd).getId().toUnqualifiedVersionless(); - assertEquals(methodName + "2", idMethodName2.getIdPart()); - - p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - IIdType id4 = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); - - SearchParameterMap pm; - List actual; - - pm = SearchParameterMap.newSynchronous(); - pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); - pm.setSort(new SortSpec(IAnyResource.SP_RES_ID)); - actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); - assertEquals(6, actual.size()); - assertThat(actual, contains(idMethodName1, idMethodName2, id1, id2, id3, id4)); - - pm = SearchParameterMap.newSynchronous(); - pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); - pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.ASC)); - actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); - assertEquals(6, actual.size()); - assertThat(actual, contains(idMethodName1, idMethodName2, id1, id2, id3, id4)); - - pm = SearchParameterMap.newSynchronous(); - pm.add(Patient.SP_IDENTIFIER, new TokenParam("urn:system", methodName)); - pm.setSort(new SortSpec(IAnyResource.SP_RES_ID).setOrder(SortOrderEnum.DESC)); - actual = toUnqualifiedVersionlessIds(myPatientDao.search(pm)); - assertEquals(6, actual.size()); - assertThat(actual, contains(id4, id3, id2, id1, idMethodName2, idMethodName1)); - } - @ParameterizedTest @ValueSource(booleans = {true, false}) public void testSortByMissingAttribute(boolean theIndexMissingData) { diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java index 670b8e078754..e43e91c1947f 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java @@ -46,6 +46,8 @@ import java.util.stream.Collectors; import javax.annotation.Nonnull; +import static org.apache.commons.lang3.ArrayUtils.EMPTY_STRING_ARRAY; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.in; @@ -105,6 +107,10 @@ public void assertSearchFinds(String theReason, String theQueryUrl, String... th assertSearchResultIds(theQueryUrl, theReason, hasItems(theIds)); } + public void assertSearchFinds(String theReason, String theQueryUrl, List theIds) { + assertSearchFinds(theReason, theQueryUrl, theIds.toArray(EMPTY_STRING_ARRAY)); + } + /** * Assert that the FHIR search has theIds in the search results. * @param theReason junit reason message @@ -117,6 +123,16 @@ public void assertSearchFinds(String theReason, String theQueryUrl, IIdType... t assertSearchResultIds(theQueryUrl, theReason, hasItems(bareIds)); } + public void assertSearchFindsInOrder(String theReason, String theQueryUrl, String... theIds) { + List ids = searchForIds(theQueryUrl); + + MatcherAssert.assertThat(theReason, ids, contains(theIds)); + } + + public void assertSearchFindsInOrder(String theReason, String theQueryUrl, List theIds) { + assertSearchFindsInOrder(theReason, theQueryUrl, theIds.toArray(EMPTY_STRING_ARRAY)); + } + public void assertSearchResultIds(String theQueryUrl, String theReason, Matcher> matcher) { List ids = searchForIds(theQueryUrl); diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/search/IIdSearchTestTemplate.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/search/IIdSearchTestTemplate.java new file mode 100644 index 000000000000..bd122f0d4b89 --- /dev/null +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/search/IIdSearchTestTemplate.java @@ -0,0 +1,55 @@ +package ca.uhn.fhir.jpa.search; + +import ca.uhn.fhir.jpa.dao.TestDaoSearch; +import ca.uhn.fhir.test.utilities.ITestDataBuilder; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.jupiter.api.Test; + +import java.util.List; + +public interface IIdSearchTestTemplate { + TestDaoSearch getSearch(); + + ITestDataBuilder getBuilder(); + + @Test + default void testSearchByServerAssignedId_findsResource() { + IIdType id = getBuilder().createPatient(); + + getSearch().assertSearchFinds("search by server assigned id", "Patient?_id=" + id.getIdPart(), id); + } + + @Test + default void testSearchByClientAssignedId_findsResource() { + ITestDataBuilder b = getBuilder(); + b.createPatient(b.withId("client-assigned-id")); + + getSearch() + .assertSearchFinds( + "search by client assigned id", "Patient?_id=client-assigned-id", "client-assigned-id"); + } + + /** + * The _id SP is defined as token, and there is no system. + * So sorting should be string order of the value. + */ + @Test + default void testSortById_treatsIdsAsString() { + ITestDataBuilder b = getBuilder(); + b.createPatient(b.withId("client-assigned-id")); + IIdType serverId = b.createPatient(); + b.createPatient(b.withId("0-sorts-before-other-numbers")); + + getSearch() + .assertSearchFindsInOrder( + "sort by resource id", + "Patient?_sort=_id", + List.of("0-sorts-before-other-numbers", serverId.getIdPart(), "client-assigned-id")); + + getSearch() + .assertSearchFindsInOrder( + "reverse sort by resource id", + "Patient?_sort=-_id", + List.of("client-assigned-id", serverId.getIdPart(), "0-sorts-before-other-numbers")); + } +} diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialectTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialectTest.java index b0bafdcdf7fd..aa12360a2236 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialectTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/config/HapiFhirHibernateJpaDialectTest.java @@ -36,7 +36,7 @@ public void testConvertHibernateAccessException() { assertThat(outcome.getMessage(), containsString("this is a message")); try { - mySvc.convertHibernateAccessException(new ConstraintViolationException("this is a message", new SQLException("reason"), ResourceTable.IDX_RES_FHIR_ID)); + mySvc.convertHibernateAccessException(new ConstraintViolationException("this is a message", new SQLException("reason"), ResourceTable.IDX_RES_TYPE_FHIR_ID)); fail(); } catch (ResourceVersionConflictException e) { assertThat(e.getMessage(), containsString("The operation has failed with a client-assigned ID constraint failure")); @@ -67,7 +67,7 @@ public void testTranslate() { assertEquals("FOO", outcome.getMessage()); try { - PersistenceException exception = new PersistenceException("a message", new ConstraintViolationException("this is a message", new SQLException("reason"), ResourceTable.IDX_RES_FHIR_ID)); + PersistenceException exception = new PersistenceException("a message", new ConstraintViolationException("this is a message", new SQLException("reason"), ResourceTable.IDX_RES_TYPE_FHIR_ID)); mySvc.translate(exception, "a message"); fail(); } catch (ResourceVersionConflictException e) { diff --git a/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java b/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java index 625addbd3790..46ebe20b9448 100644 --- a/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java +++ b/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java @@ -75,7 +75,9 @@ public IIdType doUpdateResource(IBaseResource theResource) { //noinspection rawtypes IFhirResourceDao dao = myDaoRegistry.getResourceDao(theResource.getClass()); //noinspection unchecked - return dao.update(theResource, mySrd).getId().toUnqualifiedVersionless(); + IIdType id = dao.update(theResource, mySrd).getId().toUnqualifiedVersionless(); + myIds.put(theResource.fhirType(), id); + return id; } @Override