From 05defb58c596128a34ff7f308621cbbbee537734 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 15 May 2023 08:28:55 -0400 Subject: [PATCH] Server fixes from Connectathon 32 (#4838) * NPM test cleanup * fix imports * Ongoing cleanup * Rework memory queue * Reindex cleanup * Cleanup * Clean up * Test fixes * Test fixes * Test fixes * Test fix * Test fix --- azure-pipelines.yml | 8 +- .../uhn/fhir/rest/param/DateRangeParam.java | 41 ++- .../java/ca/uhn/fhir/util/DateRangeUtil.java | 20 +- .../ca/uhn/fhir/util/DateRangeUtilTest.java | 23 +- ...8-delete-expunge-with-lucene-disabled.yaml | 8 + ...fix-blocking-queue-in-balp-queue-sink.yaml | 6 + .../4838-reindex-delete-search-indexes.yaml | 7 + ...index-deleted-resources-dont-validate.yaml | 5 + ...838-reindex-return-status-information.yaml | 6 + .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 7 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 49 ++-- .../fhir/jpa/dao/data/IResourceTableDao.java | 4 + .../batch2/DeleteExpungeSqlBuilder.java | 3 - .../delete/batch2/DeleteExpungeSvcImpl.java | 2 +- .../jpa/packages/PackageInstallerSvcImpl.java | 46 ++-- .../reindex/InstanceReindexServiceImpl.java | 29 +- .../jpa/search/reindex/reindex-outcome.html | 10 + .../packages/PackageInstallerSvcImplTest.java | 255 +++++++++++++++++- .../fhir/jpa/model/entity/ResourceTable.java | 17 ++ .../r4/FhirResourceDaoR4QueryCountTest.java | 40 +++ .../packages/PackageInstallerSvcImplTest.java | 132 --------- .../uhn/fhir/jpa/reindex/ReindexStepTest.java | 1 + .../reindex/ResourceReindexSvcImplTest.java | 5 +- .../ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java | 3 + .../dao/r5/FhirResourceDaoR5ReindexTest.java | 58 ++++ ...anceReindexServiceImplNarrativeR5Test.java | 25 +- hapi-fhir-jpaserver-uhnfhirtest/pom.xml | 59 ++-- .../java/ca/uhn/fhirtest/MySqlServer.java | 20 -- .../ca/uhn/fhirtest/TestRestfulServer.java | 2 +- .../CommonJpaStorageSettingsConfigurer.java | 2 + .../config/OldAuditEventPurgeService.java | 65 +++++ .../uhn/fhirtest/config/TestAuditConfig.java | 20 +- .../fhir/jpa/api/dao/IFhirResourceDao.java | 2 + ...ncMemoryQueueBackedFhirClientBalpSink.java | 114 ++++---- .../interceptor/balp/FhirClientBalpSink.java | 2 +- ...moryQueueBackedFhirClientBalpSinkTest.java | 70 ++++- .../fhir/rest/param/DateRangeParamR4Test.java | 7 +- .../ca/uhn/fhir/rest/server/SearchR4Test.java | 15 +- .../ResponseHighlightingInterceptorTest.java | 26 +- pom.xml | 4 +- .../pom.xml | 54 +++- 41 files changed, 877 insertions(+), 395 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-delete-expunge-with-lucene-disabled.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-fix-blocking-queue-in-balp-queue-sink.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-delete-search-indexes.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-deleted-resources-dont-validate.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-return-status-information.yaml delete mode 100644 hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java create mode 100644 hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5ReindexTest.java delete mode 100644 hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/MySqlServer.java create mode 100644 hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/OldAuditEventPurgeService.java diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c185e76d221..0d85e48bba4 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -70,8 +70,8 @@ stages: # Put to top, but kept in order here # - name: hapi_fhir_jpaserver_test_utilities # module: hapi-fhir-jpaserver-test-utilities -# - name: hapi_fhir_jpaserver_uhnfhirtest -# module: hapi-fhir-jpaserver-uhnfhirtest + - name: hapi_fhir_jpaserver_uhnfhirtest + module: hapi-fhir-jpaserver-uhnfhirtest - name: hapi_fhir_server module: hapi-fhir-server - name: hapi_fhir_server_mdm @@ -129,8 +129,8 @@ stages: module: hapi-tinder-plugin - name: hapi_tinder_test module: hapi-tinder-test -# - name: tests_hapi_fhir_base_test_jaxrsserver_kotlin -# module: tests/hapi-fhir-base-test-jaxrsserver-kotlin + - name: tests_hapi_fhir_base_test_jaxrsserver_kotlin + module: tests/hapi-fhir-base-test-jaxrsserver-kotlin - name: tests_hapi_fhir_base_test_mindeps_client module: tests/hapi-fhir-base-test-mindeps-client - name: tests_hapi_fhir_base_test_mindeps_server diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java index beadf2f65b6..9f882665232 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateRangeParam.java @@ -30,6 +30,8 @@ import ca.uhn.fhir.util.DateUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IPrimitiveType; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -355,17 +357,21 @@ public class DateRangeParam implements IQueryParameterAnd { if (myLowerBound == null || myLowerBound.getValue() == null) { return null; } - Date retVal = myLowerBound.getValue(); + return getLowerBoundAsInstant(myLowerBound); + } - if (myLowerBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { + @Nonnull + private static Date getLowerBoundAsInstant(@Nonnull DateParam theLowerBound) { + Date retVal = theLowerBound.getValue(); + if (theLowerBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { retVal = DateUtils.getLowestInstantFromDate(retVal); } - if (myLowerBound.getPrefix() != null) { - switch (myLowerBound.getPrefix()) { + if (theLowerBound.getPrefix() != null) { + switch (theLowerBound.getPrefix()) { case GREATERTHAN: case STARTS_AFTER: - retVal = myLowerBound.getPrecision().add(retVal, 1); + retVal = theLowerBound.getPrecision().add(retVal, 1); break; case EQUAL: case NOT_EQUAL: @@ -375,7 +381,7 @@ public class DateRangeParam implements IQueryParameterAnd { case APPROXIMATE: case LESSTHAN_OR_EQUALS: case ENDS_BEFORE: - throw new IllegalStateException(Msg.code(1928) + "Invalid lower bound comparator: " + myLowerBound.getPrefix()); + throw new IllegalStateException(Msg.code(1928) + "Invalid lower bound comparator: " + theLowerBound.getPrefix()); } } return retVal; @@ -417,14 +423,19 @@ public class DateRangeParam implements IQueryParameterAnd { return null; } - Date retVal = myUpperBound.getValue(); + return getUpperBoundAsInstant(myUpperBound); + } - if (myUpperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { + @Nonnull + private static Date getUpperBoundAsInstant(@Nonnull DateParam theUpperBound) { + Date retVal = theUpperBound.getValue(); + + if (theUpperBound.getPrecision().ordinal() <= TemporalPrecisionEnum.DAY.ordinal()) { retVal = DateUtils.getHighestInstantFromDate(retVal); } - if (myUpperBound.getPrefix() != null) { - switch (myUpperBound.getPrefix()) { + if (theUpperBound.getPrefix() != null) { + switch (theUpperBound.getPrefix()) { case LESSTHAN: case ENDS_BEFORE: retVal = new Date(retVal.getTime() - 1L); @@ -432,14 +443,14 @@ public class DateRangeParam implements IQueryParameterAnd { case EQUAL: case NOT_EQUAL: case LESSTHAN_OR_EQUALS: - retVal = myUpperBound.getPrecision().add(retVal, 1); + retVal = theUpperBound.getPrecision().add(retVal, 1); retVal = new Date(retVal.getTime() - 1L); break; case GREATERTHAN_OR_EQUALS: case GREATERTHAN: case APPROXIMATE: case STARTS_AFTER: - throw new IllegalStateException(Msg.code(1929) + "Invalid upper bound comparator: " + myUpperBound.getPrefix()); + throw new IllegalStateException(Msg.code(1929) + "Invalid upper bound comparator: " + theUpperBound.getPrefix()); } } return retVal; @@ -626,12 +637,14 @@ public class DateRangeParam implements IQueryParameterAnd { * are the same value. As such, even though the prefixes for the lower and * upper bounds default to ge and le respectively, * the resulting prefix is effectively eq where only a single - * date is provided - as required by the FHIR specificiation (i.e. "If no + * date is provided - as required by the FHIR specification (i.e. "If no * prefix is present, the prefix eq is assumed"). */ private void validateAndSet(DateParam lowerBound, DateParam upperBound) { if (hasBound(lowerBound) && hasBound(upperBound)) { - if (lowerBound.getValue().getTime() > upperBound.getValue().getTime()) { + Date lowerBoundAsInstant = getLowerBoundAsInstant(lowerBound); + Date upperBoundAsInstant = getUpperBoundAsInstant(upperBound); + if (lowerBoundAsInstant.after(upperBoundAsInstant)) { throw new DataFormatException(Msg.code(1932) + format( "Lower bound of %s is after upper bound of %s", lowerBound.getValueAsString(), upperBound.getValueAsString())); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateRangeUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateRangeUtil.java index b2f0b148876..e4f717439c7 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateRangeUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/DateRangeUtil.java @@ -29,9 +29,10 @@ public class DateRangeUtil { /** * Narrow the DateRange to be within theStartInclusive, and theEndExclusive, if provided. + * * @param theDateRangeParam the initial range, null for unconstrained * @param theStartInclusive a lower bound to apply, or null for unchanged. - * @param theEndExclusive an upper bound to apply, or null for unchanged. + * @param theEndExclusive an upper bound to apply, or null for unchanged. * @return a DateRange within the original range, and between theStartInclusive and theEnd */ @Nonnull @@ -39,16 +40,23 @@ public class DateRangeUtil { if (theStartInclusive == null && theEndExclusive == null) { return theDateRangeParam; } - DateRangeParam result = theDateRangeParam==null?new DateRangeParam():new DateRangeParam(theDateRangeParam); + DateRangeParam result = theDateRangeParam == null ? new DateRangeParam() : new DateRangeParam(theDateRangeParam); - if (theStartInclusive != null) { + Date startInclusive = theStartInclusive; + if (startInclusive != null) { Date inputStart = result.getLowerBoundAsInstant(); - if (theDateRangeParam == null || inputStart == null || inputStart.before(theStartInclusive)) { - result.setLowerBoundInclusive(theStartInclusive); + + Date upperBound = result.getUpperBoundAsInstant(); + if (upperBound != null && upperBound.before(startInclusive)) { + startInclusive = upperBound; + } + + if (theDateRangeParam == null || inputStart == null || inputStart.before(startInclusive)) { + result.setLowerBoundInclusive(startInclusive); } } if (theEndExclusive != null) { - Date inputEnd = result.getUpperBound() == null? null : result.getUpperBound().getValue(); + Date inputEnd = result.getUpperBound() == null ? null : result.getUpperBound().getValue(); if (theDateRangeParam == null || inputEnd == null || inputEnd.after(theEndExclusive)) { result.setUpperBoundExclusive(theEndExclusive); } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/DateRangeUtilTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/DateRangeUtilTest.java index b0d8d3499fc..cce99a49e8b 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/DateRangeUtilTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/DateRangeUtilTest.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ParamPrefixEnum; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -59,6 +60,11 @@ class DateRangeUtilTest { new DateParam(theResultStartPrefix, theResultStart), new DateParam(theResultEndPrefix, theResultEnd)); } + static NarrowCase from(String theMessage, DateRangeParam theRange, Date theNarrowStart, Date theNarrowEnd, + DateParam theResultStart, DateParam theResultEnd) { + return new NarrowCase(theMessage, theRange, theNarrowStart, theNarrowEnd, theResultStart, theResultEnd); + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SIMPLE_STYLE) @@ -89,8 +95,23 @@ class DateRangeUtilTest { // half-open cases NarrowCase.from("end inside open end", new DateRangeParam(dateTwo, null), null, dateFour, dateTwo, dateFour), NarrowCase.from("start inside open start", new DateRangeParam(null, dateFour), dateTwo, null, GREATERTHAN_OR_EQUALS, dateTwo, LESSTHAN_OR_EQUALS, dateFour), - NarrowCase.from("gt case preserved", new DateRangeParam(new DateParam(GREATERTHAN, dateTwo), null), null, dateFour, GREATERTHAN, dateTwo, LESSTHAN, dateFour) + NarrowCase.from("gt case preserved", new DateRangeParam(new DateParam(GREATERTHAN, dateTwo), null), null, dateFour, GREATERTHAN, dateTwo, LESSTHAN, dateFour), + NarrowCase.from("lt date level precision date, narrow from is inside date", + new DateRangeParam(new DateParam(LESSTHAN, "2023-05-06")), + Date.from(Instant.parse("2023-05-06T10:00:20.512+00:00")), + Date.from(Instant.parse("2023-05-10T00:00:00.000+00:00")), + new DateParam(GREATERTHAN_OR_EQUALS, Date.from(Instant.parse("2023-05-06T10:00:20.512+00:00"))), + new DateParam(LESSTHAN, "2023-05-06") + ), + + NarrowCase.from("gt date level precision date, narrow to is inside date", + new DateRangeParam(new DateParam(GREATERTHAN_OR_EQUALS, "2023-05-06")), + Date.from(Instant.parse("2023-05-01T00:00:00.000+00:00")), + Date.from(Instant.parse("2023-05-06T10:00:20.512+00:00")), + new DateParam(GREATERTHAN_OR_EQUALS, "2023-05-06"), + new DateParam(LESSTHAN, Date.from(Instant.parse("2023-05-06T10:00:20.512+00:00"))) + ) ); } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-delete-expunge-with-lucene-disabled.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-delete-expunge-with-lucene-disabled.yaml new file mode 100644 index 00000000000..c5e437696de --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-delete-expunge-with-lucene-disabled.yaml @@ -0,0 +1,8 @@ +--- +type: fix +issue: 4838 +title: "Two failures in the $delete-expunge operation were fixed: +
    +
  • Jobs could fail if hibernate search was loaded but not enabled.
    • +
  • Jobs could fail if the criteria included a _lastUpdated=lt[date] clause
    • +
" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-fix-blocking-queue-in-balp-queue-sink.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-fix-blocking-queue-in-balp-queue-sink.yaml new file mode 100644 index 00000000000..224f2a4436f --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-fix-blocking-queue-in-balp-queue-sink.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 4838 +title: "The BALP AsyncMemoryQueueBackedFhirClientBalpSink incorrectly used a non-blocking method + to add events to the blocking queue, resulting in race conditions on a heavily loaded + server." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-delete-search-indexes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-delete-search-indexes.yaml new file mode 100644 index 00000000000..1129f28e33a --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-delete-search-indexes.yaml @@ -0,0 +1,7 @@ +--- +type: add +issue: 4838 +title: "When performing a resource reindex on a deleted resource, any search index rows will now + be deleted. Deleting a resource should generally not leave any such rows behind, but they can + be left if the resource is manually deleted using SQL directly against the database and in this + case the reindex job will now clean up these unwanted rows." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-deleted-resources-dont-validate.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-deleted-resources-dont-validate.yaml new file mode 100644 index 00000000000..14212b59a22 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-deleted-resources-dont-validate.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 4838 +title: "When reindexing resources, deleted resources could incorrectly fail validation rules and + cause the reindex job to not complete correctly. This has been corrected." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-return-status-information.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-return-status-information.yaml new file mode 100644 index 00000000000..bf7160d7750 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4838-reindex-return-status-information.yaml @@ -0,0 +1,6 @@ +--- +type: add +issue: 4838 +title: "When invoking the instance level `$reindex` and `$reindex-dryrun` operations, the resulting + status message and any warnings are now included in the response Parameters object as well as in + the generated response HTML narrative." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index ef74fee1df0..d89407a83ee 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -961,7 +961,7 @@ public abstract class BaseHapiFhirDao extends BaseStora * This should be the very first thing.. */ if (theResource != null) { - if (thePerformIndexing) { + if (thePerformIndexing && theDeletedTimestampOrNull == null) { if (!ourValidationDisabledForUnitTest) { validateResourceForStorage((T) theResource, entity); } @@ -1057,7 +1057,9 @@ public abstract class BaseHapiFhirDao extends BaseStora verifyMatchUrlForConditionalCreate(theResource, entity.getCreatedByMatchUrl(), newParams, theRequest); } - entity.setUpdated(theTransactionDetails.getTransactionDate()); + if (CURRENTLY_REINDEXING.get(theResource) != Boolean.TRUE) { + entity.setUpdated(theTransactionDetails.getTransactionDate()); + } newParams.populateResourceTableSearchParamsPresentFlags(entity); entity.setIndexStatus(INDEX_STATUS_INDEXED); } @@ -1151,6 +1153,7 @@ public abstract class BaseHapiFhirDao extends BaseStora if (thePerformIndexing) { if (newParams == null) { myExpungeService.deleteAllSearchParams(JpaPid.fromId(entity.getId())); + entity.clearAllParamsPopulated(); } else { // Synchronize search param indexes diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 932eeeaca37..e7ea644dc87 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1387,7 +1387,7 @@ public abstract class BaseHapiFhirResourceDao extends B } if (theReindexParameters.getReindexSearchParameters() == ReindexParameters.ReindexSearchParametersEnum.ALL) { - reindexSearchParameters(entity, retVal); + reindexSearchParameters(entity, retVal, theTransactionDetails); } if (theReindexParameters.getOptimizeStorage() != ReindexParameters.OptimizeStorageModeEnum.NONE) { reindexOptimizeStorage(entity, theReindexParameters.getOptimizeStorage()); @@ -1397,16 +1397,42 @@ public abstract class BaseHapiFhirResourceDao extends B } @SuppressWarnings("unchecked") - private void reindexSearchParameters(ResourceTable entity, ReindexOutcome theReindexOutcome) { + private void reindexSearchParameters(ResourceTable entity, ReindexOutcome theReindexOutcome, TransactionDetails theTransactionDetails) { try { T resource = (T) myJpaStorageResourceParser.toResource(entity, false); - reindex(resource, entity); + reindexSearchParameters(resource, entity, theTransactionDetails); } catch (Exception e) { theReindexOutcome.addWarning("Failed to reindex resource " + entity.getIdDt() + ": " + e); myResourceTableDao.updateIndexStatus(entity.getId(), INDEX_STATUS_INDEXING_FAILED); } } + /** + * @deprecated Use {@link #reindex(IResourcePersistentId, ReindexParameters, RequestDetails, TransactionDetails)} + */ + @Deprecated + @Override + public void reindex(T theResource, IBasePersistedResource theEntity) { + assert TransactionSynchronizationManager.isActualTransactionActive(); + ResourceTable entity = (ResourceTable) theEntity; + TransactionDetails transactionDetails = new TransactionDetails(entity.getUpdatedDate()); + + reindexSearchParameters(theResource, theEntity, transactionDetails); + } + + private void reindexSearchParameters(T theResource, IBasePersistedResource theEntity, TransactionDetails transactionDetails) { + ourLog.debug("Indexing resource {} - PID {}", theEntity.getIdDt().getValue(), theEntity.getPersistentId()); + if (theResource != null) { + CURRENTLY_REINDEXING.put(theResource, Boolean.TRUE); + } + + ResourceTable resourceTable = updateEntity(null, theResource, theEntity, theEntity.getDeleted(), true, false, transactionDetails, true, false); + if (theResource != null) { + CURRENTLY_REINDEXING.put(theResource, null); + } + } + + private void reindexOptimizeStorage(ResourceTable entity, ReindexParameters.OptimizeStorageModeEnum theOptimizeStorageMode) { ResourceHistoryTable historyEntity = entity.getCurrentVersionEntity(); if (historyEntity != null) { @@ -1546,23 +1572,6 @@ public abstract class BaseHapiFhirResourceDao extends B return entity; } - @Override - public void reindex(T theResource, IBasePersistedResource theEntity) { - assert TransactionSynchronizationManager.isActualTransactionActive(); - - ourLog.debug("Indexing resource {} - PID {}", theEntity.getIdDt().getValue(), theEntity.getPersistentId()); - if (theResource != null) { - CURRENTLY_REINDEXING.put(theResource, Boolean.TRUE); - } - - ResourceTable entity = (ResourceTable) theEntity; - TransactionDetails transactionDetails = new TransactionDetails(entity.getUpdatedDate()); - ResourceTable resourceTable = updateEntity(null, theResource, theEntity, theEntity.getDeleted(), true, false, transactionDetails, true, false); - if (theResource != null) { - CURRENTLY_REINDEXING.put(theResource, null); - } - } - @Transactional @Override public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java index 5bd2d8e04c5..8b1d3121a1f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java @@ -85,6 +85,10 @@ public interface IResourceTableDao extends JpaRepository, I @Query("UPDATE ResourceTable t SET t.myIndexStatus = :status WHERE t.myId = :id") void updateIndexStatus(@Param("id") Long theId, @Param("status") Long theIndexStatus); + @Modifying + @Query("UPDATE ResourceTable t SET t.myUpdated = :updated WHERE t.myId = :id") + void updateLastUpdated(@Param("id") Long theId, @Param("updated") Date theUpdated); + @Modifying @Query("DELETE FROM ResourceTable t WHERE t.myId = :pid") void deleteByPid(@Param("pid") Long theId); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSqlBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSqlBuilder.java index f8d84d390fa..3c01d772cf0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSqlBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSqlBuilder.java @@ -39,9 +39,6 @@ import java.util.stream.Collectors; public class DeleteExpungeSqlBuilder { private static final Logger ourLog = LoggerFactory.getLogger(DeleteExpungeSqlBuilder.class); - public static final String PROCESS_NAME = "Delete Expunging"; - public static final String THREAD_PREFIX = "delete-expunge"; - private final ResourceTableFKProvider myResourceTableFKProvider; private final JpaStorageSettings myStorageSettings; private final IIdHelperService myIdHelper; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSvcImpl.java index 72e3d0cdf53..4f2664d10f5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/batch2/DeleteExpungeSvcImpl.java @@ -69,7 +69,7 @@ public class DeleteExpungeSvcImpl implements IDeleteExpungeSvc { * This method clears the Hibernate Search index for the given resources. */ private void clearHibernateSearchIndex(List thePersistentIds) { - if (myFullTextSearchSvc != null) { + if (myFullTextSearchSvc != null && !myFullTextSearchSvc.isDisabled()) { List objectIds = thePersistentIds.stream().map(JpaPid::getId).collect(Collectors.toList()); myFullTextSearchSvc.deleteIndexedDocumentsByTypeAndId(ResourceTable.class, objectIds); ourLog.info("Cleared Hibernate Search indexes."); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java index 5d30c22a073..1b4033b5f95 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java @@ -32,6 +32,7 @@ import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao; +import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionEntity; import ca.uhn.fhir.jpa.packages.loader.PackageResourceParsingSvc; @@ -58,8 +59,6 @@ import org.hl7.fhir.utilities.npm.NpmPackage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; import javax.annotation.PostConstruct; @@ -90,7 +89,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { @Autowired private IHapiPackageCacheManager myPackageCacheManager; @Autowired - private PlatformTransactionManager myTxManager; + private IHapiTransactionService myTxService; @Autowired private INpmPackageVersionDao myPackageVersionDao; @Autowired @@ -128,9 +127,10 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { } } - public PackageDeleteOutcomeJson uninstall(PackageInstallationSpec theInstallationSpec) { - return myPackageCacheManager.uninstallPackage(theInstallationSpec.getName(), theInstallationSpec.getVersion()); - } + @Override + public PackageDeleteOutcomeJson uninstall(PackageInstallationSpec theInstallationSpec) { + return myPackageCacheManager.uninstallPackage(theInstallationSpec.getName(), theInstallationSpec.getVersion()); + } /** * Loads and installs an IG from a file on disk or the Simplifier repo using @@ -152,12 +152,12 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { if (enabled) { try { - boolean exists = new TransactionTemplate(myTxManager).execute(tx -> { + boolean exists = myTxService.withSystemRequest().withRequestPartitionId(RequestPartitionId.defaultPartition()).execute(() -> { Optional existing = myPackageVersionDao.findByPackageIdAndVersion(theInstallationSpec.getName(), theInstallationSpec.getVersion()); return existing.isPresent(); }); if (exists) { - ourLog.info("Package {}#{} is already installed", theInstallationSpec.getName(), theInstallationSpec.getVersion()); + ourLog.info("Package {}#{} is already installed", theInstallationSpec.getName(), theInstallationSpec.getVersion()); } NpmPackage npmPackage = myPackageCacheManager.installPackage(theInstallationSpec); @@ -267,8 +267,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { } } catch (IOException e) { - throw new ImplementationGuideInstallationException(Msg.code(1287) + String.format( - "Cannot resolve dependency %s#%s", id, ver), e); + throw new ImplementationGuideInstallationException(Msg.code(1287) + String.format("Cannot resolve dependency %s#%s", id, ver), e); } } } @@ -278,8 +277,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { * Asserts if package FHIR version is compatible with current FHIR version * by using semantic versioning rules. */ - protected void assertFhirVersionsAreCompatible(String fhirVersion, String currentFhirVersion) - throws ImplementationGuideInstallationException { + protected void assertFhirVersionsAreCompatible(String fhirVersion, String currentFhirVersion) throws ImplementationGuideInstallationException { FhirVersionEnum fhirVersionEnum = FhirVersionEnum.forVersionString(fhirVersion); FhirVersionEnum currentFhirVersionEnum = FhirVersionEnum.forVersionString(currentFhirVersion); @@ -290,9 +288,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { compatible = true; } if (!compatible) { - throw new ImplementationGuideInstallationException(Msg.code(1288) + String.format( - "Cannot install implementation guide: FHIR versions mismatch (expected <=%s, package uses %s)", - currentFhirVersion, fhirVersion)); + throw new ImplementationGuideInstallationException(Msg.code(1288) + String.format("Cannot install implementation guide: FHIR versions mismatch (expected <=%s, package uses %s)", currentFhirVersion, fhirVersion)); } } @@ -336,26 +332,18 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { ourLog.info("Skipping update of existing resource matching {}", map.toNormalizedQueryString(myFhirContext)); } } - } - else{ + } else { ourLog.warn("Failed to upload resource of type {} with ID {} - Error: Resource failed validation", theResource.fhirType(), theResource.getIdElement().getValue()); } } private IBundleProvider searchResource(IFhirResourceDao theDao, SearchParameterMap theMap) { - if (myPartitionSettings.isPartitioningEnabled()) { - SystemRequestDetails requestDetails = newSystemRequestDetails(); - return theDao.search(theMap, requestDetails); - } else { - return theDao.search(theMap); - } + return theDao.search(theMap, newSystemRequestDetails()); } @Nonnull private SystemRequestDetails newSystemRequestDetails() { - return - new SystemRequestDetails() - .setRequestPartitionId(RequestPartitionId.defaultPartition()); + return new SystemRequestDetails().setRequestPartitionId(RequestPartitionId.defaultPartition()); } private void createResource(IFhirResourceDao theDao, IBaseResource theResource) { @@ -400,8 +388,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { } if (!isValidResourceStatusForPackageUpload(theResource)) { - ourLog.warn("Failed to validate resource of type {} with ID {} - Error: Resource status not accepted value.", - theResource.fhirType(), theResource.getIdElement().getValue()); + ourLog.warn("Failed to validate resource of type {} with ID {} - Error: Resource status not accepted value.", theResource.fhirType(), theResource.getIdElement().getValue()); return false; } @@ -458,8 +445,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { try { return validationSupport.generateSnapshot(new ValidationSupportContext(validationSupport), sd, null, null, null); } catch (Exception e) { - throw new ImplementationGuideInstallationException(Msg.code(1290) + String.format( - "Failure when generating snapshot of StructureDefinition: %s", sd.getIdElement()), e); + throw new ImplementationGuideInstallationException(Msg.code(1290) + String.format("Failure when generating snapshot of StructureDefinition: %s", sd.getIdElement()), e); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImpl.java index c37aabd25b1..3ed59e6f82a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImpl.java @@ -26,10 +26,13 @@ import ca.uhn.fhir.interceptor.model.ReadPartitionIdRequestDetails; import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.api.dao.ReindexOutcome; +import ca.uhn.fhir.jpa.api.dao.ReindexParameters; import ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao; import ca.uhn.fhir.jpa.dao.IJpaStorageResourceParser; import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.partition.BaseRequestPartitionHelperSvc; import ca.uhn.fhir.jpa.searchparam.extractor.ISearchParamExtractor; @@ -40,6 +43,7 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.rest.server.util.ResourceSearchParams; +import ca.uhn.fhir.util.StopWatch; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import com.google.common.annotations.VisibleForTesting; import org.hl7.fhir.instance.model.api.IBaseParameters; @@ -59,6 +63,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -132,6 +137,7 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService { @SuppressWarnings({"unchecked", "rawtypes"}) @Nonnull private Parameters reindexInTransaction(RequestDetails theRequestDetails, IIdType theResourceId) { + StopWatch sw = new StopWatch(); IFhirResourceDao dao = myDaoRegistry.getResourceDao(theResourceId.getResourceType()); ResourceTable entity = (ResourceTable) dao.readEntity(theResourceId, theRequestDetails); IBaseResource resource = myJpaStorageResourceParser.toResource(entity, false); @@ -144,16 +150,26 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService { ResourceIndexedSearchParams existingParamsToPopulate = new ResourceIndexedSearchParams(entity); existingParamsToPopulate.mySearchParamPresentEntities.addAll(entity.getSearchParamPresents()); - dao.reindex(resource, entity); + List messages = new ArrayList<>(); + + JpaPid pid = JpaPid.fromId(entity.getId()); + ReindexOutcome outcome = dao.reindex(pid, new ReindexParameters(), theRequestDetails, new TransactionDetails()); + messages.add("Reindex completed in " + sw); + + for (String next : outcome.getWarnings()) { + messages.add("WARNING: " + next); + } ResourceIndexedSearchParams newParamsToPopulate = new ResourceIndexedSearchParams(entity); newParamsToPopulate.mySearchParamPresentEntities.addAll(entity.getSearchParamPresents()); - return buildIndexResponse(existingParamsToPopulate, newParamsToPopulate, true); + return buildIndexResponse(existingParamsToPopulate, newParamsToPopulate, true, messages); } @Nonnull private Parameters reindexDryRunInTransaction(RequestDetails theRequestDetails, IIdType theResourceId, RequestPartitionId theRequestPartitionId, TransactionDetails theTransactionDetails, Set theParameters) { + StopWatch sw = new StopWatch(); + IFhirResourceDao dao = myDaoRegistry.getResourceDao(theResourceId.getResourceType()); ResourceTable entity = (ResourceTable) dao.readEntity(theResourceId, theRequestDetails); IBaseResource resource = myJpaStorageResourceParser.toResource(entity, false); @@ -186,7 +202,8 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService { showAction = false; } - return buildIndexResponse(existingParamsToPopulate, newParamsToPopulate, showAction); + String message = "Reindex dry-run completed in " + sw + ". No changes were committed to any stored data."; + return buildIndexResponse(existingParamsToPopulate, newParamsToPopulate, showAction, List.of(message)); } @Nonnull @@ -197,12 +214,16 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService { @Nonnull @VisibleForTesting - Parameters buildIndexResponse(ResourceIndexedSearchParams theExistingParams, ResourceIndexedSearchParams theNewParams, boolean theShowAction) { + Parameters buildIndexResponse(ResourceIndexedSearchParams theExistingParams, ResourceIndexedSearchParams theNewParams, boolean theShowAction, List theMessages) { Parameters parameters = new Parameters(); Parameters.ParametersParameterComponent narrativeParameter = parameters.addParameter(); narrativeParameter.setName("Narrative"); + for (String next : theMessages) { + parameters.addParameter("Message", new StringType(next)); + } + // Normal indexes addParamsNonMissing(parameters, "CoordinateIndexes", "Coords", theExistingParams.myCoordsParams, theNewParams.myCoordsParams, new CoordsParamPopulator(), theShowAction); addParamsNonMissing(parameters, "DateIndexes", "Date", theExistingParams.myDateParams, theNewParams.myDateParams, new DateParamPopulator(), theShowAction); diff --git a/hapi-fhir-jpaserver-base/src/main/resources/ca/uhn/fhir/jpa/search/reindex/reindex-outcome.html b/hapi-fhir-jpaserver-base/src/main/resources/ca/uhn/fhir/jpa/search/reindex/reindex-outcome.html index f57cb949dfe..124ac158ada 100644 --- a/hapi-fhir-jpaserver-base/src/main/resources/ca/uhn/fhir/jpa/search/reindex/reindex-outcome.html +++ b/hapi-fhir-jpaserver-base/src/main/resources/ca/uhn/fhir/jpa/search/reindex/reindex-outcome.html @@ -1,5 +1,15 @@
+ +
+

Outcome

+
    +
  • + [[${part.getValue().getValue()}]] +
    • +
+
+

Number Indexes

diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java index 1b9e7f8a679..52e4b33b7f9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java @@ -1,12 +1,263 @@ package ca.uhn.fhir.jpa.packages; -import org.elasticsearch.common.inject.Inject; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao; +import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; +import ca.uhn.fhir.jpa.dao.tx.NonTransactionalHapiTransactionService; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.packages.loader.PackageResourceParsingSvc; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistryController; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.SimpleBundleProvider; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.Communication; +import org.hl7.fhir.r4.model.DocumentReference; +import org.hl7.fhir.r4.model.Enumerations; +import org.hl7.fhir.r4.model.SearchParameter; +import org.hl7.fhir.r4.model.Subscription; +import org.hl7.fhir.utilities.npm.NpmPackage; +import org.hl7.fhir.utilities.npm.PackageGenerator; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.jupiter.MockitoExtension; +import javax.annotation.Nonnull; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) public class PackageInstallerSvcImplTest { + public static final String PACKAGE_VERSION = "1.0"; + public static final String PACKAGE_ID_1 = "package1"; + + @Mock + private INpmPackageVersionDao myPackageVersionDao; + @Mock + private IHapiPackageCacheManager myPackageCacheManager; + @Mock + private ISearchParamRegistryController mySearchParamRegistryController; + @Mock + private DaoRegistry myDaoRegistry; + @Mock + private IFhirResourceDao myCodeSystemDao; + @Spy + private FhirContext myCtx = FhirContext.forR4Cached(); + @Spy + private IHapiTransactionService myTxService = new NonTransactionalHapiTransactionService(); + @Spy + private PackageResourceParsingSvc myPackageResourceParsingSvc = new PackageResourceParsingSvc(myCtx); + @Spy + private PartitionSettings myPartitionSettings = new PartitionSettings(); + @InjectMocks + private PackageInstallerSvcImpl mySvc; + @Test public void testPackageCompatibility() { - new PackageInstallerSvcImpl().assertFhirVersionsAreCompatible("R4", "R4B"); + mySvc.assertFhirVersionsAreCompatible("R4", "R4B"); } + + @Test + public void testValidForUpload_SearchParameterWithMetaParam() { + SearchParameter sp = new SearchParameter(); + sp.setCode("_id"); + assertFalse(mySvc.validForUpload(sp)); + } + + @Test + public void testValidForUpload_SearchParameterWithNoBase() { + SearchParameter sp = new SearchParameter(); + sp.setCode("name"); + sp.setExpression("Patient.name"); + sp.setStatus(Enumerations.PublicationStatus.ACTIVE); + assertFalse(mySvc.validForUpload(sp)); + } + + @Test + public void testValidForUpload_SearchParameterWithNoExpression() { + SearchParameter sp = new SearchParameter(); + sp.setCode("name"); + sp.addBase("Patient"); + sp.setStatus(Enumerations.PublicationStatus.ACTIVE); + assertFalse(mySvc.validForUpload(sp)); + } + + + @Test + public void testValidForUpload_GoodSearchParameter() { + SearchParameter sp = new SearchParameter(); + sp.setCode("name"); + sp.addBase("Patient"); + sp.setExpression("Patient.name"); + sp.setStatus(Enumerations.PublicationStatus.ACTIVE); + assertTrue(mySvc.validForUpload(sp)); + } + + @Test + public void testValidForUpload_RequestedSubscription() { + Subscription.SubscriptionChannelComponent subscriptionChannelComponent = + new Subscription.SubscriptionChannelComponent() + .setType(Subscription.SubscriptionChannelType.RESTHOOK) + .setEndpoint("https://tinyurl.com/2p95e27r"); + Subscription subscription = new Subscription(); + subscription.setCriteria("Patient?name=smith"); + subscription.setChannel(subscriptionChannelComponent); + subscription.setStatus(Subscription.SubscriptionStatus.REQUESTED); + assertTrue(mySvc.validForUpload(subscription)); + } + + @Test + public void testValidForUpload_ErrorSubscription() { + Subscription.SubscriptionChannelComponent subscriptionChannelComponent = + new Subscription.SubscriptionChannelComponent() + .setType(Subscription.SubscriptionChannelType.RESTHOOK) + .setEndpoint("https://tinyurl.com/2p95e27r"); + Subscription subscription = new Subscription(); + subscription.setCriteria("Patient?name=smith"); + subscription.setChannel(subscriptionChannelComponent); + subscription.setStatus(Subscription.SubscriptionStatus.ERROR); + assertFalse(mySvc.validForUpload(subscription)); + } + + @Test + public void testValidForUpload_ActiveSubscription() { + Subscription.SubscriptionChannelComponent subscriptionChannelComponent = + new Subscription.SubscriptionChannelComponent() + .setType(Subscription.SubscriptionChannelType.RESTHOOK) + .setEndpoint("https://tinyurl.com/2p95e27r"); + Subscription subscription = new Subscription(); + subscription.setCriteria("Patient?name=smith"); + subscription.setChannel(subscriptionChannelComponent); + subscription.setStatus(Subscription.SubscriptionStatus.ACTIVE); + assertFalse(mySvc.validForUpload(subscription)); + } + + @Test + public void testValidForUpload_DocumentRefStatusValuePresent() { + DocumentReference documentReference = new DocumentReference(); + documentReference.setStatus(Enumerations.DocumentReferenceStatus.ENTEREDINERROR); + assertTrue(mySvc.validForUpload(documentReference)); + } + + @Test + public void testValidForUpload_DocumentRefStatusValueNull() { + DocumentReference documentReference = new DocumentReference(); + documentReference.setStatus(Enumerations.DocumentReferenceStatus.NULL); + assertFalse(mySvc.validForUpload(documentReference)); + documentReference.setStatus(null); + assertFalse(mySvc.validForUpload(documentReference)); + } + + @Test + public void testValidForUpload_CommunicationStatusValuePresent() { + Communication communication = new Communication(); + communication.setStatus(Communication.CommunicationStatus.NOTDONE); + assertTrue(mySvc.validForUpload(communication)); + } + + @Test + public void testValidForUpload_CommunicationStatusValueNull() { + Communication communication = new Communication(); + communication.setStatus(Communication.CommunicationStatus.NULL); + assertFalse(mySvc.validForUpload(communication)); + communication.setStatus(null); + assertFalse(mySvc.validForUpload(communication)); + } + + @Test + public void testDontTryToInstallDuplicateCodeSystem_CodeSystemAlreadyExistsWithDifferentId() throws IOException { + // Setup + + // The CodeSystem that is already saved in the repository + CodeSystem existingCs = new CodeSystem(); + existingCs.setId("CodeSystem/existingcs"); + existingCs.setUrl("http://my-code-system"); + existingCs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE); + + // A new code system in a package we're installing that has the + // same URL as the previously saved one, but a different ID. + CodeSystem cs = new CodeSystem(); + cs.setId("CodeSystem/mycs"); + cs.setUrl("http://my-code-system"); + cs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE); + + NpmPackage pkg = createPackage(cs, PACKAGE_ID_1); + + when(myPackageVersionDao.findByPackageIdAndVersion(any(), any())).thenReturn(Optional.empty()); + when(myPackageCacheManager.installPackage(any())).thenReturn(pkg); + when(myDaoRegistry.getResourceDao(CodeSystem.class)).thenReturn(myCodeSystemDao); + when(myCodeSystemDao.search(any(), any())).thenReturn(new SimpleBundleProvider(existingCs)); + when(myCodeSystemDao.update(any(),any(RequestDetails.class))).thenReturn(new DaoMethodOutcome()); + + PackageInstallationSpec spec = new PackageInstallationSpec(); + spec.setName(PACKAGE_ID_1); + spec.setVersion(PACKAGE_VERSION); + spec.setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL); + spec.setPackageContents(packageToBytes(pkg)); + + // Test + mySvc.install(spec); + + // Verify + verify(myCodeSystemDao, times(1)).search(mySearchParameterMapCaptor.capture(), any()); + SearchParameterMap map = mySearchParameterMapCaptor.getValue(); + assertEquals("?url=http%3A%2F%2Fmy-code-system", map.toNormalizedQueryString(myCtx)); + + verify(myCodeSystemDao, times(1)).update(myCodeSystemCaptor.capture(), any(RequestDetails.class)); + CodeSystem codeSystem = myCodeSystemCaptor.getValue(); + assertEquals("existingcs", codeSystem.getIdPart()); + } + + @Nonnull + private static byte[] packageToBytes(NpmPackage pkg) throws IOException { + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + pkg.save(stream); + byte[] bytes = stream.toByteArray(); + return bytes; + } + + @Captor + private ArgumentCaptor mySearchParameterMapCaptor; + @Captor + private ArgumentCaptor myCodeSystemCaptor; + + @Nonnull + private NpmPackage createPackage(CodeSystem cs, String packageId) throws IOException { + PackageGenerator manifestGenerator = new PackageGenerator(); + manifestGenerator.name(packageId); + manifestGenerator.version(PACKAGE_VERSION); + manifestGenerator.description("a package"); + manifestGenerator.fhirVersions(List.of(FhirVersionEnum.R4.getFhirVersionString())); + + NpmPackage pkg = NpmPackage.empty(manifestGenerator); + + String csString = myCtx.newJsonParser().encodeResourceToString(cs); + pkg.addFile("package", "cs.json", csString.getBytes(StandardCharsets.UTF_8), "CodeSystem"); + + return pkg; + } + + } 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 f468ad39d83..05b5f875680 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 @@ -620,6 +620,23 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas myHasLinks = theHasLinks; } + /** + * Clears all the index population flags, e.g. {@link #isParamsStringPopulated()} + * + * @since 6.8.0 + */ + public void clearAllParamsPopulated() { + myParamsTokenPopulated = false; + myParamsCoordsPopulated = false; + myParamsDatePopulated = false; + myParamsNumberPopulated = false; + myParamsStringPopulated = false; + myParamsQuantityPopulated = false; + myParamsQuantityNormalizedPopulated = false; + myParamsUriPopulated = false; + myHasLinks = false; + } + public boolean isParamsComboStringUniquePresent() { if (myParamsComboStringUniquePresent == null) { return false; diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java index b214b1c94f0..7ae7d12a870 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java @@ -85,6 +85,7 @@ import org.springframework.data.domain.Slice; import org.springframework.util.comparator.ComparableComparator; import javax.annotation.Nonnull; +import javax.persistence.Id; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -100,6 +101,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.eq; @@ -3342,6 +3344,34 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test } + /** + * See the class javadoc before changing the counts in this test! + */ + @Test + public void testDeleteResource_WithOutgoingReference() { + // Setup + createOrganization(withId("A")); + IIdType patientId = createPatient(withOrganization(new IdType("Organization/A")), withActiveTrue()); + + // Test + myCaptureQueriesListener.clear(); + myPatientDao.delete(patientId, mySrd); + + // Verify + assertEquals(4, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(1, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); + assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); + assertEquals(3, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); + runInTransaction(()->{ + ResourceTable version = myResourceTableDao.findById(patientId.getIdPartAsLong()).orElseThrow(); + assertFalse(version.isParamsTokenPopulated()); + assertFalse(version.isHasLinks()); + assertEquals(0, myResourceIndexedSearchParamTokenDao.count()); + assertEquals(0, myResourceLinkDao.count()); + }); + + } + /** * See the class javadoc before changing the counts in this test! */ @@ -3353,6 +3383,11 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test Observation observation = new Observation().setStatus(Observation.ObservationStatus.FINAL).addCategory(new CodeableConcept().addCoding(new Coding("http://category-type", "12345", null))).setCode(new CodeableConcept().addCoding(new Coding("http://coverage-type", "12345", null))); IIdType idDt = myObservationDao.create(observation, mySrd).getEntity().getIdDt(); + runInTransaction(()->{ + assertEquals(4, myResourceIndexedSearchParamTokenDao.count()); + ResourceTable version = myResourceTableDao.findById(idDt.getIdPartAsLong()).orElseThrow(); + assertTrue(version.isParamsTokenPopulated()); + }); // when myCaptureQueriesListener.clear(); @@ -3360,6 +3395,11 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test // then assertQueryCount(3, 1, 1, 2); + runInTransaction(()->{ + assertEquals(0, myResourceIndexedSearchParamTokenDao.count()); + ResourceTable version = myResourceTableDao.findById(idDt.getIdPartAsLong()).orElseThrow(); + assertFalse(version.isParamsTokenPopulated()); + }); } private void assertQueryCount(int theExpectedSelectCount, int theExpectedUpdateCount, int theExpectedInsertCount, int theExpectedDeleteCount) { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java deleted file mode 100644 index b97a30c1a6d..00000000000 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java +++ /dev/null @@ -1,132 +0,0 @@ -package ca.uhn.fhir.jpa.packages; - -import ca.uhn.fhir.context.FhirContext; -import org.hl7.fhir.r4.model.Communication; -import org.hl7.fhir.r4.model.DocumentReference; -import org.hl7.fhir.r4.model.Enumerations; -import org.hl7.fhir.r4.model.SearchParameter; -import org.hl7.fhir.r4.model.Subscription; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class PackageInstallerSvcImplTest { - private final FhirContext myCtx = FhirContext.forR4Cached(); - private PackageInstallerSvcImpl mySvc; - - - @BeforeEach - public void before() { - mySvc = new PackageInstallerSvcImpl(); - mySvc.setFhirContextForUnitTest(myCtx); - } - - @Test - public void testValidForUpload_SearchParameterWithMetaParam() { - SearchParameter sp = new SearchParameter(); - sp.setCode("_id"); - assertFalse(mySvc.validForUpload(sp)); - } - - @Test - public void testValidForUpload_SearchParameterWithNoBase() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.setExpression("Patient.name"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertFalse(mySvc.validForUpload(sp)); - } - - @Test - public void testValidForUpload_SearchParameterWithNoExpression() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.addBase("Patient"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertFalse(mySvc.validForUpload(sp)); - } - - - @Test - public void testValidForUpload_GoodSearchParameter() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.addBase("Patient"); - sp.setExpression("Patient.name"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertTrue(mySvc.validForUpload(sp)); - } - - @Test - public void testValidForUpload_RequestedSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.REQUESTED); - assertTrue(mySvc.validForUpload(subscription)); - } - - @Test - public void testValidForUpload_ErrorSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.ERROR); - assertFalse(mySvc.validForUpload(subscription)); - } - - @Test - public void testValidForUpload_ActiveSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.ACTIVE); - assertFalse(mySvc.validForUpload(subscription)); - } - - @Test - public void testValidForUpload_DocumentRefStatusValuePresent() { - DocumentReference documentReference = new DocumentReference(); - documentReference.setStatus(Enumerations.DocumentReferenceStatus.ENTEREDINERROR); - assertTrue(mySvc.validForUpload(documentReference)); - } - - @Test - public void testValidForUpload_DocumentRefStatusValueNull() { - DocumentReference documentReference = new DocumentReference(); - documentReference.setStatus(Enumerations.DocumentReferenceStatus.NULL); - assertFalse(mySvc.validForUpload(documentReference)); - documentReference.setStatus(null); - assertFalse(mySvc.validForUpload(documentReference)); - } - - @Test - public void testValidForUpload_CommunicationStatusValuePresent() { - Communication communication = new Communication(); - communication.setStatus(Communication.CommunicationStatus.NOTDONE); - assertTrue(mySvc.validForUpload(communication)); - } - - @Test - public void testValidForUpload_CommunicationStatusValueNull() { - Communication communication = new Communication(); - communication.setStatus(Communication.CommunicationStatus.NULL); - assertFalse(mySvc.validForUpload(communication)); - communication.setStatus(null); - assertFalse(mySvc.validForUpload(communication)); - } -} diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ReindexStepTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ReindexStepTest.java index 5ef7ec3d4b2..81ad6c198c7 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ReindexStepTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ReindexStepTest.java @@ -66,6 +66,7 @@ public class ReindexStepTest extends BaseJpaR4Test { assertEquals(2, outcome.getRecordsProcessed()); assertEquals(6, myCaptureQueriesListener.logSelectQueries().size()); assertEquals(0, myCaptureQueriesListener.countInsertQueries()); + myCaptureQueriesListener.logUpdateQueries(); assertEquals(0, myCaptureQueriesListener.countUpdateQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); assertEquals(1, myCaptureQueriesListener.getCommitCount()); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ResourceReindexSvcImplTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ResourceReindexSvcImplTest.java index b2e6706e7e3..01b2afeeadf 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ResourceReindexSvcImplTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/reindex/ResourceReindexSvcImplTest.java @@ -3,12 +3,16 @@ package ca.uhn.fhir.jpa.reindex; import ca.uhn.fhir.jpa.api.pid.IResourcePidList; import ca.uhn.fhir.jpa.api.pid.TypedResourcePid; import ca.uhn.fhir.jpa.api.svc.IBatch2DaoSvc; +import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; +import org.hl7.fhir.r4.model.DateType; +import org.hl7.fhir.r4.model.InstantType; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; import org.springframework.beans.factory.annotation.Autowired; +import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -151,5 +155,4 @@ public class ResourceReindexSvcImplTest extends BaseJpaR4Test { } - } diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java index 971e6f95451..f4cf47553c3 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java @@ -49,6 +49,7 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.provider.JpaSystemProvider; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.IStaleSearchDeletingSvc; +import ca.uhn.fhir.jpa.search.reindex.IInstanceReindexService; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.search.warm.ICacheWarmingSvc; import ca.uhn.fhir.jpa.searchparam.registry.SearchParamRegistryImpl; @@ -144,6 +145,8 @@ public abstract class BaseJpaR5Test extends BaseJpaTest implements ITestDataBuil @Autowired protected PartitionSettings myPartitionSettings; @Autowired + protected IInstanceReindexService myInstanceReindexService; + @Autowired protected ITermCodeSystemStorageSvc myTermCodeSystemStorageSvc; @Autowired protected IFhirResourceDao myClinicalUseDefinitionDao; diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5ReindexTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5ReindexTest.java new file mode 100644 index 00000000000..43e1ff2134d --- /dev/null +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5ReindexTest.java @@ -0,0 +1,58 @@ +package ca.uhn.fhir.jpa.dao.r5; + +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamUri; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import ca.uhn.fhir.jpa.test.config.TestHSearchAddInConfig; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r5.model.Enumerations; +import org.hl7.fhir.r5.model.Parameters; +import org.hl7.fhir.r5.model.SearchParameter; +import org.junit.jupiter.api.Test; +import org.springframework.test.context.ContextConfiguration; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.*; + +@ContextConfiguration(classes = TestHSearchAddInConfig.NoFT.class) +@SuppressWarnings({"Duplicates"}) +public class FhirResourceDaoR5ReindexTest extends BaseJpaR5Test { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR5ReindexTest.class); + + + @Test + public void testReindexDeletedSearchParameter() { + + SearchParameter sp = new SearchParameter(); + sp.setCode("foo"); + sp.addBase(Enumerations.VersionIndependentResourceTypesAll.PATIENT); + sp.setStatus(Enumerations.PublicationStatus.ACTIVE); + sp.setExpression("Patient.name"); + sp.setType(Enumerations.SearchParamType.STRING); + IIdType id = mySearchParameterDao.create(sp, mySrd).getId().toUnqualifiedVersionless(); + + mySearchParameterDao.delete(id, mySrd); + + runInTransaction(() -> { + assertEquals(1, myEntityManager.createNativeQuery("UPDATE HFJ_RESOURCE set sp_uri_present = true").executeUpdate()); + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(); + assertTrue(table.isParamsUriPopulated()); + ResourceIndexedSearchParamUri uri = new ResourceIndexedSearchParamUri(new PartitionSettings(), "SearchParameter", "url", "http://foo"); + uri.setResource(table); + uri.calculateHashes(); + myResourceIndexedSearchParamUriDao.save(uri); + }); + + Parameters outcome = (Parameters) myInstanceReindexService.reindex(mySrd, id); + ourLog.info("Outcome: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); + assertThat(outcome.getParameter("Narrative").getValueStringType().getValue(), containsString("Reindex completed in")); + assertEquals("REMOVE", outcome.getParameter("UriIndexes").getPartFirstRep().getPartFirstRep().getValueCodeType().getValue()); + + runInTransaction(() -> { + ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(); + assertFalse(table.isParamsUriPopulated()); + }); + } + +} diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImplNarrativeR5Test.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImplNarrativeR5Test.java index 94c7d260602..f523b6e2243 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImplNarrativeR5Test.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/search/reindex/InstanceReindexServiceImplNarrativeR5Test.java @@ -28,6 +28,7 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; import java.io.IOException; import java.math.BigDecimal; +import java.util.Collections; import java.util.Date; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -53,7 +54,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myComboTokenNonUnique.add(new ResourceIndexedComboTokenNonUnique(myPartitionSettings, myEntity, "Patient?identifier=123")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -71,7 +72,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myComboStringUniques.add(new ResourceIndexedComboStringUnique(myEntity, "Patient?identifier=123", new IdType("Parameter/foo"))); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -94,7 +95,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.mySearchParamPresentEntities.add(subject); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -117,7 +118,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myNumberParams.add(new ResourceIndexedSearchParamNumber(myPartitionSettings, "Immunization", "dose", BigDecimal.ONE)); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -136,7 +137,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myLinks.add(ResourceLink.forLocalReference("Observation.subject", myEntity, "Patient", 123L, "123", new Date(), 555L)); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -156,7 +157,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myLinks.add(ResourceLink.forLogicalReference("Observation.subject", myEntity, "http://foo/base/Patient/456", new Date())); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -177,7 +178,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myLinks.add(ResourceLink.forAbsoluteReference("Observation.subject", myEntity, new IdType("http://foo/base/Patient/123"), new Date())); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -198,7 +199,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myQuantityParams.add(new ResourceIndexedSearchParamQuantity(myPartitionSettings, "Observation", "value-quantity", BigDecimal.valueOf(123), "http://unitsofmeasure.org", "kg")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -219,7 +220,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myQuantityNormalizedParams.add(new ResourceIndexedSearchParamQuantityNormalized(myPartitionSettings, "Observation", "value-quantity", 123.0, "http://unitsofmeasure.org", "kg")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -240,7 +241,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myStringParams.add(new ResourceIndexedSearchParamString(myPartitionSettings, myStorageSettings, "Patient", "family", "Simpson", "SIMPSON")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -260,7 +261,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myTokenParams.add(new ResourceIndexedSearchParamToken(myPartitionSettings, "Observation", "identifier", "http://id-system", "id-value")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify @@ -280,7 +281,7 @@ public class InstanceReindexServiceImplNarrativeR5Test { newParams.myUriParams.add(new ResourceIndexedSearchParamUri(myPartitionSettings, "CodeSystem", "uri", "http://some-codesystem")); // Test - Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true); + Parameters outcome = mySvc.buildIndexResponse(newParams(), newParams, true, Collections.emptyList()); ourLog.info("Output:\n{}", myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); // Verify diff --git a/hapi-fhir-jpaserver-uhnfhirtest/pom.xml b/hapi-fhir-jpaserver-uhnfhirtest/pom.xml index f09d66b8b44..c7ea3c54e50 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/pom.xml +++ b/hapi-fhir-jpaserver-uhnfhirtest/pom.xml @@ -11,7 +11,7 @@ hapi-fhir-jpaserver-uhnfhirtest - war + jar HAPI FHIR - fhirtest.uhn.ca Deployable WAR @@ -230,26 +230,6 @@ - - org.apache.maven.plugins - maven-war-plugin - - - - ${maven.build.timestamp} - - - - - ca.uhn.hapi.fhir - hapi-fhir-testpage-overlay - - WEB-INF/classes/**/*.class - - - - - org.apache.maven.plugins maven-deploy-plugin @@ -261,4 +241,41 @@ hapi-fhir-jpaserver + + + DIST + + + + org.apache.maven.plugins + maven-war-plugin + + + + war + + + + + + + ${maven.build.timestamp} + + + + + ca.uhn.hapi.fhir + hapi-fhir-testpage-overlay + + WEB-INF/classes/**/*.class + + + + + + + + + + diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/MySqlServer.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/MySqlServer.java deleted file mode 100644 index 31610b254cd..00000000000 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/MySqlServer.java +++ /dev/null @@ -1,20 +0,0 @@ -package ca.uhn.fhirtest; - -import org.springframework.beans.factory.DisposableBean; -import org.springframework.beans.factory.InitializingBean; - -/** - * Created by mochaholic on 18/02/2015. - */ -public class MySqlServer implements InitializingBean, DisposableBean { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(MySqlServer.class); - - @Override - public void destroy() throws Exception { - } - - @Override - public void afterPropertiesSet() throws Exception { - } - -} \ No newline at end of file diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/TestRestfulServer.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/TestRestfulServer.java index 29a5833ae82..2c01e75503b 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/TestRestfulServer.java +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/TestRestfulServer.java @@ -333,7 +333,7 @@ public class TestRestfulServer extends RestfulServer { } /** - * The public server is deployed to http://fhirtest.uhn.ca and the JEE webserver + * The public server is deployed to hapi.fhir.org and the JEE webserver * where this FHIR server is deployed is actually fronted by an Apache HTTPd instance, * so we use an address strategy to let the server know how it should address itself. */ diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/CommonJpaStorageSettingsConfigurer.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/CommonJpaStorageSettingsConfigurer.java index 7e8bc19273f..0c5353bee68 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/CommonJpaStorageSettingsConfigurer.java +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/CommonJpaStorageSettingsConfigurer.java @@ -5,5 +5,7 @@ import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; public class CommonJpaStorageSettingsConfigurer { public CommonJpaStorageSettingsConfigurer(JpaStorageSettings theStorageSettings) { theStorageSettings.setIndexOnUpliftedRefchains(true); + theStorageSettings.setMarkResourcesForReindexingUponSearchParameterChange(false); + } } diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/OldAuditEventPurgeService.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/OldAuditEventPurgeService.java new file mode 100644 index 00000000000..bfc5b40f09c --- /dev/null +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/OldAuditEventPurgeService.java @@ -0,0 +1,65 @@ +package ca.uhn.fhirtest.config; + +import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; +import ca.uhn.fhir.jpa.model.sched.HapiJob; +import ca.uhn.fhir.jpa.model.sched.ISchedulerService; +import ca.uhn.fhir.jpa.model.sched.ScheduledJobDefinition; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.rest.api.server.storage.IDeleteExpungeJobSubmitter; +import org.apache.commons.lang3.time.DateUtils; +import org.hl7.fhir.r4.model.DateType; +import org.quartz.JobExecutionContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.context.event.EventListener; + +import javax.annotation.PostConstruct; +import java.util.Date; +import java.util.List; + +public class OldAuditEventPurgeService { + private static final Logger ourLog = LoggerFactory.getLogger(OldAuditEventPurgeService.class); + + @Autowired + private ISchedulerService mySchedulerSvc; + @Autowired + private IDeleteExpungeJobSubmitter myDeleteExpungeSubmitter; + @Autowired + private JpaStorageSettings myStorageSettings; + + @EventListener(ContextRefreshedEvent.class) + public void start() { + myStorageSettings.setAllowMultipleDelete(true); + myStorageSettings.setExpungeEnabled(true); + myStorageSettings.setDeleteExpungeEnabled(true); + + ScheduledJobDefinition jobDetail = new ScheduledJobDefinition(); + jobDetail.setId(OldAuditEventPurgeServiceJob.class.getName()); + jobDetail.setJobClass(OldAuditEventPurgeServiceJob.class); + mySchedulerSvc.scheduleLocalJob(DateUtils.MILLIS_PER_DAY, jobDetail); + } + + private void doPass() { + Date cutoff = DateUtils.addDays(new Date(), -7); + String cutoffString = new DateType(cutoff).getValueAsString(); + String url = "AuditEvent?_lastUpdated=lt" + cutoffString; + + ourLog.info("Submitting an AuditEvent purge job with URL: {}", url); + + myDeleteExpungeSubmitter.submitJob(1000, List.of(url), new SystemRequestDetails()); + } + + public static class OldAuditEventPurgeServiceJob implements HapiJob { + + @Autowired + private OldAuditEventPurgeService mySvc; + + @Override + public void execute(JobExecutionContext context) { + mySvc.doPass(); + } + } + +} diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestAuditConfig.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestAuditConfig.java index 9a0488b22d7..973bf616e86 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestAuditConfig.java +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestAuditConfig.java @@ -6,34 +6,20 @@ import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.config.HapiJpaConfig; import ca.uhn.fhir.jpa.config.r4.JpaR4Config; import ca.uhn.fhir.jpa.config.util.HapiEntityManagerFactoryUtil; -import ca.uhn.fhir.jpa.ips.api.IIpsGenerationStrategy; -import ca.uhn.fhir.jpa.ips.generator.IIpsGeneratorSvc; -import ca.uhn.fhir.jpa.ips.generator.IpsGeneratorSvcImpl; -import ca.uhn.fhir.jpa.ips.provider.IpsOperationProvider; -import ca.uhn.fhir.jpa.ips.strategy.DefaultIpsGenerationStrategy; import ca.uhn.fhir.jpa.model.dialect.HapiFhirH2Dialect; import ca.uhn.fhir.jpa.model.dialect.HapiFhirPostgres94Dialect; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; -import ca.uhn.fhir.jpa.search.HapiHSearchAnalysisConfigurers; import ca.uhn.fhir.jpa.util.CurrentThreadCaptureQueriesListener; import ca.uhn.fhir.jpa.validation.ValidationSettings; -import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; -import ca.uhn.fhir.validation.IInstanceValidatorModule; -import ca.uhn.fhir.validation.ResultSeverityEnum; import ca.uhn.fhirtest.interceptor.PublicSecurityInterceptor; import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; import org.apache.commons.dbcp2.BasicDataSource; -import org.hibernate.search.backend.lucene.cfg.LuceneBackendSettings; -import org.hibernate.search.backend.lucene.cfg.LuceneIndexSettings; -import org.hibernate.search.engine.cfg.BackendSettings; -import org.hl7.fhir.dstu2.model.Subscription; import org.hl7.fhir.r5.utils.validation.constants.ReferenceValidationPolicy; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; -import org.springframework.context.annotation.Lazy; import org.springframework.context.annotation.Primary; import org.springframework.context.support.PropertySourcesPlaceholderConfigurer; import org.springframework.orm.jpa.JpaTransactionManager; @@ -167,6 +153,11 @@ public class TestAuditConfig { } + @Bean + public OldAuditEventPurgeService oldEventPurgeService() { + return new OldAuditEventPurgeService(); + } + @Bean public DaoRegistryConfigurer daoRegistryConfigurer() { return new DaoRegistryConfigurer(); @@ -177,7 +168,6 @@ public class TestAuditConfig { @Autowired private DaoRegistry myDaoRegistry; - @PostConstruct public void start() { myDaoRegistry.setSupportedResourceTypes("AuditEvent", "SearchParameter", "Subscription"); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index e4a0840e512..de5d967a315 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -261,7 +261,9 @@ public interface IFhirResourceDao extends IDao { * * @param theResource The FHIR resource object corresponding to the entity to reindex * @param theEntity The storage entity to reindex + * @deprecated Use {@link #reindex(IResourcePersistentId, ReindexParameters, RequestDetails, TransactionDetails)} */ + @Deprecated void reindex(T theResource, IBasePersistedResource theEntity); /** diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSink.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSink.java index 62112a7496a..423c5fe3a8c 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSink.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSink.java @@ -21,17 +21,21 @@ package ca.uhn.fhir.storage.interceptor.balp; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.util.BundleBuilder; +import ca.uhn.fhir.util.ThreadPoolUtil; +import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; /** @@ -41,16 +45,17 @@ import java.util.concurrent.atomic.AtomicLong; * events will be converted automatically prior to sending. *

* This sink transmits events asynchronously using an in-memory queue. This means that - * in the event of a server shutdown data could be lost. + * in the event of a server shutdown or unavailability of the target server data could be lost. *

*/ public class AsyncMemoryQueueBackedFhirClientBalpSink extends FhirClientBalpSink implements IBalpAuditEventSink { - private static final Logger ourLog = LoggerFactory.getLogger(AsyncMemoryQueueBackedFhirClientBalpSink.class); - private final ArrayBlockingQueue myQueue; + public static final IBaseResource[] EMPTY_RESOURCE_ARRAY = new IBaseResource[0]; private static final AtomicLong ourNextThreadId = new AtomicLong(0); - private boolean myRunning; - private TransmitterThread myThread; + private static final Logger ourLog = LoggerFactory.getLogger(AsyncMemoryQueueBackedFhirClientBalpSink.class); + private final List myQueue = new ArrayList<>(100); + private final ThreadPoolTaskExecutor myThreadPool; + private final Runnable myTransmitterTask = new TransmitterTask(); /** * Sets the FhirContext to use when initiating outgoing connections @@ -66,6 +71,7 @@ public class AsyncMemoryQueueBackedFhirClientBalpSink extends FhirClientBalpSink this(theFhirContext, theTargetBaseUrl, null); } + /** * Sets the FhirContext to use when initiating outgoing connections * @@ -82,7 +88,6 @@ public class AsyncMemoryQueueBackedFhirClientBalpSink extends FhirClientBalpSink this(createClient(theFhirContext, theTargetBaseUrl, theClientInterceptors)); } - /** * Constructor * @@ -90,76 +95,61 @@ public class AsyncMemoryQueueBackedFhirClientBalpSink extends FhirClientBalpSink */ public AsyncMemoryQueueBackedFhirClientBalpSink(IGenericClient theClient) { super(theClient); - myQueue = new ArrayBlockingQueue<>(100); + myThreadPool = ThreadPoolUtil.newThreadPool(1, 1, "BalpClientSink-" + ourNextThreadId.getAndIncrement() + "-", 100); } @Override protected void recordAuditEvent(IBaseResource theAuditEvent) { - myQueue.add(theAuditEvent); - } - - @PostConstruct - public void start() { - if (!myRunning) { - myRunning = true; - myThread = new TransmitterThread(); - myThread.start(); + synchronized (myQueue) { + myQueue.add(theAuditEvent); } + myThreadPool.submit(myTransmitterTask); } @PreDestroy public void stop() { - if (myRunning) { - myRunning = false; - myThread.interrupt(); - } + myThreadPool.shutdown(); } - public boolean isRunning() { - return myThread != null && myThread.isRunning(); - } - - private class TransmitterThread extends Thread { - - private boolean myThreadRunning; - - public TransmitterThread() { - setName("BalpClientSink-" + ourNextThreadId.getAndIncrement()); - } + private class TransmitterTask implements Runnable { @Override public void run() { - ourLog.info("Starting BALP Client Sink Transmitter"); - myThreadRunning = true; - while (myRunning) { - IBaseResource next = null; - try { - next = myQueue.poll(10, TimeUnit.SECONDS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + IBaseResource[] queue; + synchronized (myQueue) { + if (myQueue.isEmpty()) { + queue = EMPTY_RESOURCE_ARRAY; + } else { + queue = myQueue.toArray(EMPTY_RESOURCE_ARRAY); + myQueue.clear(); } - - // TODO: Currently we transmit events one by one, but a nice optimization - // would be to batch them into FHIR transaction Bundles. If we do this, we - // would get better performance, but we'd also want to have some retry - // logic that submits events individually if a transaction fails. - - if (next != null) { - try { - transmitEventToClient(next); - } catch (Exception e) { - ourLog.warn("Failed to transmit AuditEvent to sink: {}", e.toString()); - myQueue.add(next); - } - } - } - ourLog.info("Stopping BALP Client Sink Transmitter"); - myThreadRunning = false; - } - public boolean isRunning() { - return myThreadRunning; + if (queue.length == 0) { + return; + } + + BundleBuilder bundleBuilder = new BundleBuilder(myClient.getFhirContext()); + for (IBaseResource next : queue) { + bundleBuilder.addTransactionCreateEntry(next); + } + + IBaseBundle transactionBundle = bundleBuilder.getBundle(); + try { + myClient.transaction().withBundle(transactionBundle).execute(); + return; + } catch (BaseServerResponseException e) { + ourLog.error("Failed to transmit AuditEvent items to target. Will re-attempt {} failed events once. Error: {}", queue.length, e.toString()); + } + + // Retry once then give up + for (IBaseResource next : queue) { + try { + myClient.create().resource(next).execute(); + } catch (BaseServerResponseException e) { + ourLog.error("Second failure uploading AuditEvent. Error: {}", e.toString()); + } + } } } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/FhirClientBalpSink.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/FhirClientBalpSink.java index 3aee733d877..e7df5fdbde3 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/FhirClientBalpSink.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/interceptor/balp/FhirClientBalpSink.java @@ -32,7 +32,7 @@ import java.util.List; public class FhirClientBalpSink implements IBalpAuditEventSink { - private final IGenericClient myClient; + protected final IGenericClient myClient; private final VersionCanonicalizer myVersionCanonicalizer; /** diff --git a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSinkTest.java b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSinkTest.java index 701b928b002..a5bec2fb570 100644 --- a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSinkTest.java +++ b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/storage/interceptor/balp/AsyncMemoryQueueBackedFhirClientBalpSinkTest.java @@ -5,12 +5,15 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; import ca.uhn.fhir.interceptor.api.IPointcut; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.annotation.Transaction; +import ca.uhn.fhir.rest.annotation.TransactionParam; +import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.storage.interceptor.balp.AsyncMemoryQueueBackedFhirClientBalpSink; import ca.uhn.fhir.test.utilities.server.HashMapResourceProviderExtension; import ca.uhn.fhir.test.utilities.server.RestfulServerExtension; import org.hl7.fhir.dstu3.model.AuditEvent; -import org.hl7.fhir.r4.model.Reference; +import org.hl7.fhir.dstu3.model.Bundle; +import org.hl7.fhir.instance.model.api.IIdType; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -23,26 +26,48 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.lessThan; +import static org.junit.jupiter.api.Assertions.assertEquals; public class AsyncMemoryQueueBackedFhirClientBalpSinkTest { @RegisterExtension @Order(0) - private RestfulServerExtension myServer = new RestfulServerExtension(FhirVersionEnum.DSTU3); + private RestfulServerExtension myServer = new RestfulServerExtension(FhirVersionEnum.DSTU3) + .withServer(t->t.registerProvider(new MySystemProvider())); @RegisterExtension @Order(1) private HashMapResourceProviderExtension myAuditEventProvider = new HashMapResourceProviderExtension<>(myServer, AuditEvent.class); + + @Test + public void testStressTest() { + AsyncMemoryQueueBackedFhirClientBalpSink sink = new AsyncMemoryQueueBackedFhirClientBalpSink(myServer.getFhirContext(), myServer.getBaseUrl()); + try { + for (int i = 0; i < 10; i++) { + for (int j = 0; j < 100; j++) { + AuditEvent auditEvent = new AuditEvent(); + auditEvent.setAction(AuditEvent.AuditEventAction.C); + sink.recordAuditEvent(auditEvent); + } + } + + await().until(() -> myAuditEventProvider.getStoredResources().size(), equalTo(1000)); + + } finally { + sink.stop(); + } + } + + @Test public void recordAuditEvent() { // Setup AsyncMemoryQueueBackedFhirClientBalpSink sink = new AsyncMemoryQueueBackedFhirClientBalpSink(myServer.getFhirContext(), myServer.getBaseUrl()); - sink.start(); try { org.hl7.fhir.r4.model.AuditEvent auditEvent1 = new org.hl7.fhir.r4.model.AuditEvent(); - auditEvent1.addEntity().setWhat(new Reference("Patient/123")); + auditEvent1.addEntity().setWhat(new org.hl7.fhir.r4.model.Reference("Patient/123")); org.hl7.fhir.r4.model.AuditEvent auditEvent2 = new org.hl7.fhir.r4.model.AuditEvent(); - auditEvent2.addEntity().setWhat(new Reference("Patient/456")); + auditEvent2.addEntity().setWhat(new org.hl7.fhir.r4.model.Reference("Patient/456")); // Test sink.recordAuditEvent(auditEvent1); @@ -58,7 +83,6 @@ public class AsyncMemoryQueueBackedFhirClientBalpSinkTest { assertThat(whats, containsInAnyOrder("Patient/123", "Patient/456")); } finally { sink.stop(); - await().until(sink::isRunning, equalTo(false)); } } @@ -66,18 +90,17 @@ public class AsyncMemoryQueueBackedFhirClientBalpSinkTest { public void recordAuditEvent_AutoRetry() { // Setup AsyncMemoryQueueBackedFhirClientBalpSink sink = new AsyncMemoryQueueBackedFhirClientBalpSink(myServer.getFhirContext(), myServer.getBaseUrl()); - sink.start(); try { org.hl7.fhir.r4.model.AuditEvent auditEvent1 = new org.hl7.fhir.r4.model.AuditEvent(); - auditEvent1.addEntity().setWhat(new Reference("Patient/123")); + auditEvent1.addEntity().setWhat(new org.hl7.fhir.r4.model.Reference("Patient/123")); org.hl7.fhir.r4.model.AuditEvent auditEvent2 = new org.hl7.fhir.r4.model.AuditEvent(); - auditEvent2.addEntity().setWhat(new Reference("Patient/456")); + auditEvent2.addEntity().setWhat(new org.hl7.fhir.r4.model.Reference("Patient/456")); - AtomicInteger counter = new AtomicInteger(10); + AtomicInteger counter = new AtomicInteger(1); myServer.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, new IAnonymousInterceptor() { @Override public void invoke(IPointcut thePointcut, HookParams theArgs) { - if (counter.decrementAndGet() > 0) { + if (counter.getAndDecrement() > 0) { throw new InternalErrorException("Intentional error for unit test"); } } @@ -98,9 +121,30 @@ public class AsyncMemoryQueueBackedFhirClientBalpSinkTest { assertThat(whats.toString(), whats, containsInAnyOrder("Patient/123", "Patient/456")); } finally { sink.stop(); - await().until(sink::isRunning, equalTo(false)); } } + public class MySystemProvider { + + @Transaction + public Bundle transaction(@TransactionParam Bundle theInput, RequestDetails theRequestDetails) { + Bundle retVal = new Bundle(); + retVal.setType(Bundle.BundleType.TRANSACTIONRESPONSE); + + for (Bundle.BundleEntryComponent next : theInput.getEntry()) { + assertEquals(Bundle.HTTPVerb.POST, next.getRequest().getMethod()); + assertEquals("AuditEvent", next.getRequest().getUrl()); + AuditEvent resource = (AuditEvent) next.getResource(); + IIdType outcome = myAuditEventProvider.create(resource, theRequestDetails).getId(); + + Bundle.BundleEntryComponent respEntry = retVal.addEntry(); + respEntry.getResponse().setStatus("201 Created"); + respEntry.getResponse().setLocation(outcome.getValue()); + } + + return retVal; + } + + } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/DateRangeParamR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/DateRangeParamR4Test.java index 48e20056279..aedeb3c7f84 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/DateRangeParamR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/param/DateRangeParamR4Test.java @@ -21,6 +21,7 @@ import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.jena.base.Sys; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; @@ -59,7 +60,7 @@ public class DateRangeParamR4Test { private static final SimpleDateFormat ourFmtUpperForTime; private static final Logger ourLog = LoggerFactory.getLogger(DateRangeParamR4Test.class); private static CloseableHttpClient ourClient; - private static FhirContext ourCtx = FhirContext.forR4(); + private static FhirContext ourCtx = FhirContext.forR4Cached(); private static DateRangeParam ourLastDateRange; private static int ourPort; private static Server ourServer; @@ -315,8 +316,8 @@ public class DateRangeParamR4Test { @Test public void testSetBoundsWithDatesExclusive() { DateRangeParam range = new DateRangeParam(); - range.setLowerBoundExclusive(new Date()); - range.setUpperBoundExclusive(new Date()); + range.setLowerBoundExclusive(new Date(System.currentTimeMillis())); + range.setUpperBoundExclusive(new Date(System.currentTimeMillis() + 1000)); assertEquals(ParamPrefixEnum.GREATERTHAN, range.getLowerBound().getPrefix()); assertEquals(ParamPrefixEnum.LESSTHAN, range.getUpperBound().getPrefix()); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java index 55d7ffd480f..198285c87e5 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.valueset.BundleEntrySearchModeEnum; +import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; import ca.uhn.fhir.rest.annotation.IncludeParam; import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.annotation.Search; @@ -77,6 +78,7 @@ public class SearchR4Test { ourLastMethod = null; ourIdentifiers = null; myPort = myRestfulServerExtension.getPort(); + myCtx.setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator()); } private Bundle executeSearchAndValidateHasLinkNext(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException { @@ -139,13 +141,13 @@ public class SearchR4Test { String linkSelf; // Initial search - httpGet = new HttpGet("http://localhost:" + myPort + "/Patient?identifier=foo%7Cbar&_elements=name&_elements:exclude=birthDate,active"); + httpGet = new HttpGet("http://localhost:" + myPort + "/Patient?identifier=foo%7Cbar&_elements=identifier,name&_elements:exclude=birthDate,active"); bundle = executeSearchAndValidateHasLinkNext(httpGet, EncodingEnum.JSON); assertThat(toJson(bundle), not(containsString("\"active\""))); linkSelf = bundle.getLink(Constants.LINK_SELF).getUrl(); - assertThat(linkSelf, containsString("_elements=name")); + assertThat(linkSelf, containsString("_elements=identifier%2Cname")); linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); - assertThat(linkNext, containsString("_elements=name")); + assertThat(linkNext, containsString("_elements=identifier,name")); ourLog.info(toJson(bundle)); @@ -154,7 +156,7 @@ public class SearchR4Test { bundle = executeSearchAndValidateHasLinkNext(httpGet, EncodingEnum.JSON); assertThat(toJson(bundle), not(containsString("\"active\""))); linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); - assertThat(linkNext, containsString("_elements=name")); + assertThat(linkNext, containsString("_elements=identifier,name")); assertThat(linkNext, containsString("_elements:exclude=active,birthDate")); // Fetch the next page @@ -162,7 +164,7 @@ public class SearchR4Test { bundle = executeSearchAndValidateHasLinkNext(httpGet, EncodingEnum.JSON); assertThat(toJson(bundle), not(containsString("\"active\""))); linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); - assertThat(linkNext, containsString("_elements=name")); + assertThat(linkNext, containsString("_elements=identifier,name")); assertThat(linkNext, containsString("_elements:exclude=active,birthDate")); // Fetch the next page @@ -170,7 +172,7 @@ public class SearchR4Test { bundle = executeSearchAndValidateHasLinkNext(httpGet, EncodingEnum.JSON); assertThat(toJson(bundle), not(containsString("\"active\""))); linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); - assertThat(linkNext, containsString("_elements=name")); + assertThat(linkNext, containsString("_elements=identifier,name")); assertThat(linkNext, containsString("_elements:exclude=active,birthDate")); } @@ -519,6 +521,7 @@ public class SearchR4Test { patient.getIdElement().setValue("Patient/" + i + "/_history/222"); ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.put(patient, BundleEntrySearchModeEnum.INCLUDE); patient.addName(new HumanName().setFamily("FAMILY")); + patient.addIdentifier().setSystem("http://foo").setValue("bar"); patient.setActive(true); retVal.add(patient); } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java index 4360af1dc80..468d7a91400 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.context.api.BundleInclusionRule; import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; import ca.uhn.fhir.rest.annotation.GraphQL; import ca.uhn.fhir.rest.annotation.GraphQLQueryUrl; import ca.uhn.fhir.rest.annotation.IdParam; @@ -105,6 +106,7 @@ public class ResponseHighlightingInterceptorTest { ourInterceptor.setShowRequestHeaders(defaults.isShowRequestHeaders()); ourInterceptor.setShowResponseHeaders(defaults.isShowResponseHeaders()); ourInterceptor.setShowNarrative(defaults.isShowNarrative()); + ourCtx.setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator()); } /** @@ -310,7 +312,7 @@ public class ResponseHighlightingInterceptorTest { status.close(); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals(Constants.CT_FHIR_JSON_NEW + ";charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); - assertThat(responseContent, not(containsString("html"))); + assertThat(responseContent, not(containsString("{<")); - assertThat(responseContent, not(containsString("<"))); assertThat(responseContent, containsString(Constants.HEADER_REQUEST_ID)); } @@ -410,7 +411,7 @@ public class ResponseHighlightingInterceptorTest { assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("text/html;charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); - assertThat(responseContent, containsString("html")); + assertThat(responseContent, containsString(""urn:hapitest:mrns"")); assertThat(responseContent, containsString(Constants.HEADER_REQUEST_ID)); @@ -426,9 +427,8 @@ public class ResponseHighlightingInterceptorTest { status.close(); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("text/html;charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); - assertThat(responseContent, containsString("html")); + assertThat(responseContent, containsString("{<")); - assertThat(responseContent, not(containsString("<"))); ourLog.info(responseContent); } @@ -443,7 +443,7 @@ public class ResponseHighlightingInterceptorTest { status.close(); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("text/html;charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); - assertThat(responseContent, containsString("html")); + assertThat(responseContent, containsString("{<"))); assertThat(responseContent, containsString("<")); } @@ -458,7 +458,7 @@ public class ResponseHighlightingInterceptorTest { status.close(); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals(Constants.CT_FHIR_JSON_NEW + ";charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); - assertThat(responseContent, not(containsString("html"))); + assertThat(responseContent, not(containsString("hapi-fhir-jpaserver-ips hapi-fhir-jpaserver-mdm hapi-fhir-testpage-overlay - + hapi-fhir-jpaserver-uhnfhirtest hapi-fhir-client-okhttp hapi-fhir-android hapi-fhir-cli hapi-fhir-dist - + tests/hapi-fhir-base-test-jaxrsserver-kotlin tests/hapi-fhir-base-test-mindeps-client tests/hapi-fhir-base-test-mindeps-server hapi-fhir-spring-boot diff --git a/tests/hapi-fhir-base-test-jaxrsserver-kotlin/pom.xml b/tests/hapi-fhir-base-test-jaxrsserver-kotlin/pom.xml index 24a7c1a11f4..846c5774b5e 100644 --- a/tests/hapi-fhir-base-test-jaxrsserver-kotlin/pom.xml +++ b/tests/hapi-fhir-base-test-jaxrsserver-kotlin/pom.xml @@ -1,4 +1,5 @@ - + 4.0.0 1.6.21