diff --git a/README.md b/README.md index 84afd8716b6..9a7ce3ff074 100644 --- a/README.md +++ b/README.md @@ -17,5 +17,3 @@ http://hapi.fhir.org/ This project is Open Source, licensed under the Apache Software License 2.0. Please see [this wiki page](https://github.com/jamesagnew/hapi-fhir/wiki/Getting-Help) for information on where to get help with HAPI FHIR. Please see [Smile CDR](https://smilecdr.com) for information on commercial support. - ---- diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 386f1f3ae08..02ca50f6ccd 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,4 @@ -# Starter pipeline -# Start with a minimal pipeline that you can customize to build and deploy your code. -# Add steps that build, run tests, deploy, and more: -# https://aka.ms/yaml +# HAPI FHIR Build Pipeline variables: MAVEN_CACHE_FOLDER: $(Pipeline.Workspace)/.m2/repository @@ -13,7 +10,6 @@ trigger: pool: vmImage: 'ubuntu-latest' - jobs: - job: Build timeoutInMinutes: 360 @@ -23,37 +19,37 @@ jobs: key: maven path: $(MAVEN_CACHE_FOLDER) displayName: Cache Maven local repo - - task: Maven@3 inputs: - #mavenPomFile: 'pom.xml' - goals: 'clean install' # Optional - options: '-P ALLMODULES,JACOCO' - #publishJUnitResults: true - #testResultsFiles: '**/surefire-reports/TEST-*.xml' # Required when publishJUnitResults == True - #testRunTitle: # Optional - #codeCoverageToolOption: 'None' # Optional. Options: none, cobertura, jaCoCo. Enabling code coverage inserts the `clean` goal into the Maven goals list when Maven runs. - #codeCoverageClassFilter: # Optional. Comma-separated list of filters to include or exclude classes from collecting code coverage. For example: +:com.*,+:org.*,-:my.app*.* - #codeCoverageClassFilesDirectories: # Optional - #codeCoverageSourceDirectories: # Optional - #codeCoverageFailIfEmpty: false # Optional - #javaHomeOption: 'JDKVersion' # Options: jDKVersion, path - #jdkVersionOption: 'default' # Optional. Options: default, 1.11, 1.10, 1.9, 1.8, 1.7, 1.6 - #jdkDirectory: # Required when javaHomeOption == Path - #jdkArchitectureOption: 'x64' # Optional. Options: x86, x64 - #mavenVersionOption: 'Default' # Options: default, path - #mavenDirectory: # Required when mavenVersionOption == Path - #mavenSetM2Home: false # Required when mavenVersionOption == Path - mavenOptions: '-Xmx2048m $(MAVEN_OPTS) -Dorg.slf4j.simpleLogger.showDateTime=true -Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss,SSS' # Optional - #mavenAuthenticateFeed: false - #effectivePomSkip: false - #sonarQubeRunAnalysis: false - #sqMavenPluginVersionChoice: 'latest' # Required when sonarQubeRunAnalysis == True# Options: latest, pom - #checkStyleRunAnalysis: false # Optional - #pmdRunAnalysis: false # Optional - #findBugsRunAnalysis: false # Optional - + goals: 'clean install' + # These are Maven CLI options (and show up in the build logs) - "-nsu"=Don't update snapshots. We can remove this when Maven OSS is more healthy + options: '-P ALLMODULES,JACOCO -nsu' + # These are JVM options (and don't show up in the build logs) + mavenOptions: '-Xmx2048m $(MAVEN_OPTS) -Dorg.slf4j.simpleLogger.showDateTime=true -Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss,SSS -Duser.timezone=America/Toronto' - script: bash <(curl https://codecov.io/bash) -t $(CODECOV_TOKEN) displayName: 'codecov' +# Potential Additional Maven3 Options: +#publishJUnitResults: true +#testResultsFiles: '**/surefire-reports/TEST-*.xml' # Required when publishJUnitResults == True +#testRunTitle: # Optional +#codeCoverageToolOption: 'None' # Optional. Options: none, cobertura, jaCoCo. Enabling code coverage inserts the `clean` goal into the Maven goals list when Maven runs. +#codeCoverageClassFilter: # Optional. Comma-separated list of filters to include or exclude classes from collecting code coverage. For example: +:com.*,+:org.*,-:my.app*.* +#codeCoverageClassFilesDirectories: # Optional +#codeCoverageSourceDirectories: # Optional +#codeCoverageFailIfEmpty: false # Optional +#javaHomeOption: 'JDKVersion' # Options: jDKVersion, path +#jdkVersionOption: 'default' # Optional. Options: default, 1.11, 1.10, 1.9, 1.8, 1.7, 1.6 +#jdkDirectory: # Required when javaHomeOption == Path +#jdkArchitectureOption: 'x64' # Optional. Options: x86, x64 +#mavenVersionOption: 'Default' # Options: default, path +#mavenDirectory: # Required when mavenVersionOption == Path +#mavenSetM2Home: false # Required when mavenVersionOption == Path +#mavenAuthenticateFeed: false +#effectivePomSkip: false +#sonarQubeRunAnalysis: false +#sqMavenPluginVersionChoice: 'latest' # Required when sonarQubeRunAnalysis == True# Options: latest, pom +#checkStyleRunAnalysis: false # Optional +#pmdRunAnalysis: false # Optional +#findBugsRunAnalysis: false # Optional diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/json/GsonStructure.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/json/GsonStructure.java index 30aa96b7a57..a439becc441 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/json/GsonStructure.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/json/GsonStructure.java @@ -52,15 +52,6 @@ public class GsonStructure implements JsonLikeStructure { super(); } - public GsonStructure (JsonObject json) { - super(); - setNativeObject(json); - } - public GsonStructure (JsonArray json) { - super(); - setNativeArray(json); - } - public void setNativeObject (JsonObject json) { this.rootType = ROOT_TYPE.OBJECT; this.nativeRoot = json; @@ -111,8 +102,7 @@ public class GsonStructure implements JsonLikeStructure { if (nextInt == '{') { JsonObject root = gson.fromJson(pbr, JsonObject.class); setNativeObject(root); - } else - if (nextInt == '[') { + } else if (nextInt == '[') { JsonArray root = gson.fromJson(pbr, JsonArray.class); setNativeArray(root); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/OperationOutcomeUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/OperationOutcomeUtil.java index c2bf6973a37..f88262c43c6 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/OperationOutcomeUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/OperationOutcomeUtil.java @@ -108,9 +108,13 @@ public class OperationOutcomeUtil { if (theOutcome == null) { return false; } + return getIssueCount(theCtx, theOutcome) > 0; + } + + public static int getIssueCount(FhirContext theCtx, IBaseOperationOutcome theOutcome) { RuntimeResourceDefinition ooDef = theCtx.getResourceDefinition(theOutcome); BaseRuntimeChildDefinition issueChild = ooDef.getChildByName("issue"); - return issueChild.getAccessor().getValues(theOutcome).size() > 0; + return issueChild.getAccessor().getValues(theOutcome).size(); } public static IBaseOperationOutcome newInstance(FhirContext theCtx) { @@ -152,5 +156,4 @@ public class OperationOutcomeUtil { locationChild.getMutator().addValue(theIssue, locationElem); } } - } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java index add97e858a8..ba6f05098c7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java @@ -53,21 +53,6 @@ public class Search implements ICachedSearchDetails, Serializable { private static final int FAILURE_MESSAGE_LENGTH = 500; private static final long serialVersionUID = 1L; private static final Logger ourLog = LoggerFactory.getLogger(Search.class); - - @Override - public String toString() { - return new ToStringBuilder(this) - .append("myLastUpdatedHigh", myLastUpdatedHigh) - .append("myLastUpdatedLow", myLastUpdatedLow) - .append("myNumFound", myNumFound) - .append("myNumBlocked", myNumBlocked) - .append("myStatus", myStatus) - .append("myTotalCount", myTotalCount) - .append("myUuid", myUuid) - .append("myVersion", myVersion) - .toString(); - } - @Temporal(TemporalType.TIMESTAMP) @Column(name = "CREATED", nullable = false, updatable = false) private Date myCreated; @@ -139,6 +124,20 @@ public class Search implements ICachedSearchDetails, Serializable { super(); } + @Override + public String toString() { + return new ToStringBuilder(this) + .append("myLastUpdatedHigh", myLastUpdatedHigh) + .append("myLastUpdatedLow", myLastUpdatedLow) + .append("myNumFound", myNumFound) + .append("myNumBlocked", myNumBlocked) + .append("myStatus", myStatus) + .append("myTotalCount", myTotalCount) + .append("myUuid", myUuid) + .append("myVersion", myVersion) + .toString(); + } + public int getNumBlocked() { return myNumBlocked != null ? myNumBlocked : 0; } @@ -351,8 +350,8 @@ public class Search implements ICachedSearchDetails, Serializable { return myVersion; } - public SearchParameterMap getSearchParameterMap() { - return SerializationUtils.deserialize(mySearchParameterMap); + public Optional getSearchParameterMap() { + return Optional.ofNullable(mySearchParameterMap).map(t -> SerializationUtils.deserialize(mySearchParameterMap)); } public void setSearchParameterMap(SearchParameterMap theSearchParameterMap) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java index f0effc0e5c6..25a74c21dc1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermCodeSystemVersion.java @@ -173,4 +173,11 @@ public class TermCodeSystemVersion implements Serializable { "Version ID exceeds maximum length (" + MAX_VERSION_LENGTH + "): " + length(theCodeSystemDisplayName)); myCodeSystemDisplayName = theCodeSystemDisplayName; } + + public TermConcept addConcept() { + TermConcept concept = new TermConcept(); + concept.setCodeSystemVersion(this); + getConcepts().add(concept); + return concept; + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ISearchCoordinatorSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ISearchCoordinatorSvc.java index bce9b2e1c64..eed92afe78a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ISearchCoordinatorSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/ISearchCoordinatorSvc.java @@ -28,6 +28,7 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import javax.annotation.Nullable; import java.util.List; +import java.util.Optional; public interface ISearchCoordinatorSvc { @@ -37,4 +38,10 @@ public interface ISearchCoordinatorSvc { IBundleProvider registerSearch(IDao theCallingDao, SearchParameterMap theParams, String theResourceType, CacheControlDirective theCacheControlDirective, @Nullable RequestDetails theRequestDetails); + /** + * Fetch the total number of search results for the given currently executing search, if one is currently executing and + * the total is known. Will return empty otherwise + */ + Optional getSearchTotal(String theUuid); + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java index 8d3f97c2912..9d7707213f7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java @@ -191,12 +191,12 @@ public class PersistedJpaBundleProvider implements IBundleProvider { if (mySearchEntity == null) { ensureDependenciesInjected(); - Optional search = mySearchCacheSvc.fetchByUuid(myUuid); - if (!search.isPresent()) { + Optional searchOpt = mySearchCacheSvc.fetchByUuid(myUuid); + if (!searchOpt.isPresent()) { return false; } - setSearchEntity(search.get()); + setSearchEntity(searchOpt.get()); ourLog.trace("Retrieved search with version {} and total {}", mySearchEntity.getVersion(), mySearchEntity.getTotalCount()); @@ -292,10 +292,16 @@ public class PersistedJpaBundleProvider implements IBundleProvider { SearchCoordinatorSvcImpl.verifySearchHasntFailedOrThrowInternalErrorException(mySearchEntity); Integer size = mySearchEntity.getTotalCount(); - if (size == null) { - return null; + if (size != null) { + return Math.max(0, size); } - return Math.max(0, size); + + if (mySearchEntity.getSearchType() == SearchTypeEnum.HISTORY) { + return null; + } else { + return mySearchCoordinatorSvc.getSearchTotal(myUuid).orElse(null); + } + } // Note: Leave as protected, HSPC depends on this diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java index 06936e4991d..3ae64722b4c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaSearchFirstPageBundleProvider.java @@ -62,6 +62,8 @@ public class PersistedJpaSearchFirstPageBundleProvider extends PersistedJpaBundl public List getResources(int theFromIndex, int theToIndex) { SearchCoordinatorSvcImpl.verifySearchHasntFailedOrThrowInternalErrorException(mySearch); + mySearchTask.awaitInitialSync(); + ourLog.trace("Fetching search resource PIDs from task: {}", mySearchTask.getClass()); final List pids = mySearchTask.getResourcePids(theFromIndex, theToIndex); ourLog.trace("Done fetching search resource PIDs"); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index f5b56b83943..f8dc6a474ca 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -92,7 +92,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { public static final int DEFAULT_SYNC_SIZE = 250; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchCoordinatorSvcImpl.class); - private final ConcurrentHashMap myIdToSearchTask = new ConcurrentHashMap<>(); + private final ConcurrentHashMap myIdToSearchTask = new ConcurrentHashMap<>(); @Autowired private FhirContext myContext; @Autowired @@ -151,7 +151,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { @Override public void cancelAllActiveSearches() { - for (BaseTask next : myIdToSearchTask.values()) { + for (SearchTask next : myIdToSearchTask.values()) { next.requestImmediateAbort(); try { next.getCompletionLatch().await(30, TimeUnit.SECONDS); @@ -171,17 +171,35 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); + // If we're actively searching right now, don't try to do anything until at least one batch has been + // persisted in the DB + SearchTask searchTask = myIdToSearchTask.get(theUuid); + if (searchTask != null) { + searchTask.awaitInitialSync(); + } + + ourLog.trace("About to start looking for resources {}-{}", theFrom, theTo); + Search search; StopWatch sw = new StopWatch(); while (true) { if (myNeverUseLocalSearchForUnitTests == false) { - BaseTask task = myIdToSearchTask.get(theUuid); - if (task != null) { + if (searchTask != null) { ourLog.trace("Local search found"); - List resourcePids = task.getResourcePids(theFrom, theTo); + List resourcePids = searchTask.getResourcePids(theFrom, theTo); if (resourcePids != null) { - return resourcePids; + ourLog.trace("Local search returned {} pids, wanted {}-{} - Search: {}", resourcePids.size(), theFrom, theTo, searchTask.getSearch()); + + /* + * Generally, if a search task is open, the fastest possible thing is to just return its results. This + * will work most of the time, but can fail if the task hit a search threshold and the client is requesting + * results beyond that threashold. In that case, we'll keep going below, since that will trigger another + * task. + */ + if ((searchTask.getSearch().getNumFound() - searchTask.getSearch().getNumBlocked()) >= theTo || resourcePids.size() == (theTo - theFrom)) { + return resourcePids; + } } } } @@ -189,18 +207,18 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { search = mySearchCacheSvc .fetchByUuid(theUuid) .orElseThrow(() -> { - ourLog.debug("Client requested unknown paging ID[{}]", theUuid); + ourLog.trace("Client requested unknown paging ID[{}]", theUuid); String msg = myContext.getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", theUuid); return new ResourceGoneException(msg); }); verifySearchHasntFailedOrThrowInternalErrorException(search); if (search.getStatus() == SearchStatusEnum.FINISHED) { - ourLog.debug("Search entity marked as finished with {} results", search.getNumFound()); + ourLog.trace("Search entity marked as finished with {} results", search.getNumFound()); break; } if (search.getNumFound() >= theTo) { - ourLog.debug("Search entity has {} results so far", search.getNumFound()); + ourLog.trace("Search entity has {} results so far", search.getNumFound()); break; } @@ -212,11 +230,12 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { // If the search was saved in "pass complete mode" it's probably time to // start a new pass if (search.getStatus() == SearchStatusEnum.PASSCMPLET) { + ourLog.trace("Going to try to start next search"); Optional newSearch = mySearchCacheSvc.tryToMarkSearchAsInProgress(search); if (newSearch.isPresent()) { search = newSearch.get(); String resourceType = search.getResourceType(); - SearchParameterMap params = search.getSearchParameterMap(); + SearchParameterMap params = search.getSearchParameterMap().orElseThrow(() -> new IllegalStateException("No map in PASSCOMPLET search")); IFhirResourceDao resourceDao = myDaoRegistry.getResourceDao(resourceType); SearchContinuationTask task = new SearchContinuationTask(search, resourceDao, params, resourceType, theRequestDetails); myIdToSearchTask.put(search.getUuid(), task); @@ -231,10 +250,11 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } } + ourLog.trace("Finished looping"); List pids = mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo); - + ourLog.trace("Fetched {} results", pids.size()); return pids; } @@ -290,6 +310,38 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } + @Override + public Optional getSearchTotal(String theUuid) { + SearchTask task = myIdToSearchTask.get(theUuid); + if (task != null) { + return Optional.ofNullable(task.awaitInitialSync()); + } + + /* + * In case there is no running search, if the total is listed as accurate we know one is coming + * so let's wait a bit for it to show up + */ + TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); + Optional search = mySearchCacheSvc.fetchByUuid(theUuid); + if (search.isPresent()) { + Optional searchParameterMap = search.get().getSearchParameterMap(); + if (searchParameterMap.isPresent() && searchParameterMap.get().getSearchTotalMode() == SearchTotalModeEnum.ACCURATE) { + for (int i = 0; i < 10; i++) { + if (search.isPresent()) { + verifySearchHasntFailedOrThrowInternalErrorException(search.get()); + if (search.get().getTotalCount() != null) { + return Optional.of(search.get().getTotalCount()); + } + } + search = mySearchCacheSvc.fetchByUuid(theUuid); + } + } + } + + return Optional.empty(); + } + @NotNull private IBundleProvider submitSearch(IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequestDetails, String theSearchUuid, ISearchBuilder theSb, String theQueryString) { StopWatch w = new StopWatch(); @@ -516,7 +568,20 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { myInterceptorBroadcaster = theInterceptorBroadcaster; } - public abstract class BaseTask implements Callable { + /** + * A search task is a Callable task that runs in + * a thread pool to handle an individual search. One instance + * is created for any requested search and runs from the + * beginning to the end of the search. + *

+ * Understand: + * This class executes in its own thread separate from the + * web server client thread that made the request. We do that + * so that we can return to the client as soon as possible, + * but keep the search going in the background (and have + * the next page of results ready to go when the client asks). + */ + public class SearchTask implements Callable { private final SearchParameterMap myParams; private final IDao myCallingDao; private final String myResourceType; @@ -538,7 +603,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { /** * Constructor */ - protected BaseTask(Search theSearch, IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequest) { + protected SearchTask(Search theSearch, IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequest) { mySearch = theSearch; myCallingDao = theCallingDao; myParams = theParams; @@ -549,6 +614,29 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { myRequest = theRequest; } + /** + * This method is called by the server HTTP thread, and + * will block until at least one page of results have been + * fetched from the DB, and will never block after that. + */ + Integer awaitInitialSync() { + ourLog.trace("Awaiting initial sync"); + do { + try { + if (getInitialCollectionLatch().await(250, TimeUnit.MILLISECONDS)) { + break; + } + } catch (InterruptedException e) { + // Shouldn't happen + Thread.currentThread().interrupt(); + throw new InternalErrorException(e); + } + } while (getSearch().getStatus() == SearchStatusEnum.LOADING); + ourLog.trace("Initial sync completed"); + + return getSearch().getTotalCount(); + } + protected Search getSearch() { return mySearch; } @@ -1010,7 +1098,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } - public class SearchContinuationTask extends BaseTask { + public class SearchContinuationTask extends SearchTask { public SearchContinuationTask(Search theSearch, IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequest) { super(theSearch, theCallingDao, theParams, theResourceType, theRequest); @@ -1041,51 +1129,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } - /** - * A search task is a Callable task that runs in - * a thread pool to handle an individual search. One instance - * is created for any requested search and runs from the - * beginning to the end of the search. - *

- * Understand: - * This class executes in its own thread separate from the - * web server client thread that made the request. We do that - * so that we can return to the client as soon as possible, - * but keep the search going in the background (and have - * the next page of results ready to go when the client asks). - */ - class SearchTask extends BaseTask { - - /** - * Constructor - */ - SearchTask(Search theSearch, IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequestDetails) { - super(theSearch, theCallingDao, theParams, theResourceType, theRequestDetails); - } - - /** - * This method is called by the server HTTP thread, and - * will block until at least one page of results have been - * fetched from the DB, and will never block after that. - */ - Integer awaitInitialSync() { - ourLog.trace("Awaiting initial sync"); - do { - try { - if (getInitialCollectionLatch().await(250, TimeUnit.MILLISECONDS)) { - break; - } - } catch (InterruptedException e) { - // Shouldn't happen - throw new InternalErrorException(e); - } - } while (getSearch().getStatus() == SearchStatusEnum.LOADING); - ourLog.trace("Initial sync completed"); - - return getSearch().getTotalCount(); - } - - } public static void populateSearchEntity(SearchParameterMap theParams, String theResourceType, String theSearchUuid, String theQueryString, Search theSearch) { theSearch.setDeleted(false); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java index d3b371cb30a..b1fb0258642 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java @@ -58,9 +58,6 @@ public class DatabaseSearchResultCacheSvcImpl implements ISearchResultCacheSvc { ourLog.debug("fetchResultPids for range {}-{} returned {} pids", theFrom, theTo, retVal.size()); - // FIXME: should we remove the blocked number from this message? - Validate.isTrue((theSearch.getNumFound() - theSearch.getNumBlocked()) < theTo || retVal.size() == (theTo - theFrom), "Failed to find results in cache, requested %d - %d and got %d with total found=%d and blocked %s", theFrom, theTo, retVal.size(), theSearch.getNumFound(), theSearch.getNumBlocked()); - return new ArrayList<>(retVal); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR5.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR5.java index 457094eee71..9c2cdb059af 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR5.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR5.java @@ -270,7 +270,7 @@ public class HapiTerminologySvcR5 extends BaseHapiTerminologySvcImpl implements public IValidationSupport.CodeValidationResult validateCode(FhirContext theContext, String theCodeSystem, String theCode, String theDisplay) { TransactionTemplate txTemplate = new TransactionTemplate(myTransactionManager); txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); - return txTemplate.execute(t-> { + return txTemplate.execute(t -> { Optional codeOpt = findCode(theCodeSystem, theCode); if (codeOpt.isPresent()) { TermConcept code = codeOpt.get(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 11be87537f9..3e5201d5b3e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -367,6 +367,11 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { myInterceptorRegistry.registerInterceptor(myPerformanceTracingLoggingInterceptor); } + @Before + public void beforeUnregisterAllSubscriptions() { + mySubscriptionRegistry.unregisterAllSubscriptions(); + } + @Before public void beforeFlushFT() { runInTransaction(() -> { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java index 5a04bac0169..b6e3fc8eaa9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java @@ -120,20 +120,24 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { myPatientDao.update(p); myDaoConfig.setSearchPreFetchThresholds(Arrays.asList(20, 50, 190)); + SearchParameterMap params; + IBundleProvider results; + String uuid; + List ids; // Search with count only - SearchParameterMap params = new SearchParameterMap(); + params = new SearchParameterMap(); params.add(Patient.SP_NAME, new StringParam("FAM")); params.setSummaryMode((SummaryEnum.COUNT)); - IBundleProvider results = myPatientDao.search(params); - String uuid = results.getUuid(); + results = myPatientDao.search(params); + uuid = results.getUuid(); ourLog.info("** Search returned UUID: {}", uuid); assertEquals(201, results.size().intValue()); - List ids = toUnqualifiedVersionlessIdValues(results, 0, 10, true); + ids = toUnqualifiedVersionlessIdValues(results, 0, 10, true); assertThat(ids, empty()); assertEquals(201, myDatabaseBackedPagingProvider.retrieveResultList(null, uuid).size().intValue()); - // Seach with total explicitly requested + // Search with total explicitly requested params = new SearchParameterMap(); params.add(Patient.SP_NAME, new StringParam("FAM")); params.setSearchTotalMode(SearchTotalModeEnum.ACCURATE); @@ -213,7 +217,6 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { assertEquals("Patient/PT00000", ids.get(0)); assertEquals("Patient/PT00009", ids.get(9)); - await().until(() -> myDatabaseBackedPagingProvider.retrieveResultList(null, uuid).size() != null); results = myDatabaseBackedPagingProvider.retrieveResultList(null, uuid); Integer resultsSize = results.size(); assertEquals(200, resultsSize.intValue()); @@ -354,6 +357,15 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * 20 should be prefetched since that's the initial page size */ + await().until(()->{ + return runInTransaction(()->{ + return mySearchEntityDao + .findByUuidAndFetchIncludes(uuid) + .orElseThrow(() -> new InternalErrorException("")) + .getStatus() == SearchStatusEnum.PASSCMPLET; + }); + }); + runInTransaction(() -> { Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(20, search.getNumFound()); @@ -636,6 +648,10 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { assertEquals("Patient/PT00000", ids.get(0)); assertEquals(1, ids.size()); + await().until(()-> runInTransaction(()-> mySearchEntityDao + .findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")) + .getStatus() == SearchStatusEnum.FINISHED)); + runInTransaction(() -> { Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(SearchStatusEnum.FINISHED, search.getStatus()); @@ -778,6 +794,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { search.getResources(0, 20); ourLog.info("** Done retrieving resources"); + await().until(()->myCaptureQueriesListener.countSelectQueries() == 4); myCaptureQueriesListener.logSelectQueriesForCurrentThread(); assertEquals(4, myCaptureQueriesListener.countSelectQueries()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java index fbad2885d84..639529df3ac 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java @@ -1,6 +1,8 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; +import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.ValidationModeEnum; @@ -8,10 +10,12 @@ import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.util.OperationOutcomeUtil; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.validation.IValidatorModule; import org.apache.commons.io.IOUtils; +import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator; @@ -28,16 +32,18 @@ import org.springframework.test.util.AopTestUtils; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Collections; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; public class FhirResourceDaoR4ValidateTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4ValidateTest.class); @Autowired private IValidatorModule myValidatorModule; + @Autowired + private IHapiTerminologySvc myTerminologySvc; @Test public void testValidateStructureDefinition() throws Exception { @@ -106,6 +112,43 @@ public class FhirResourceDaoR4ValidateTest extends BaseJpaR4Test { assertThat(ooString, containsString("Element '/f:Observation.device': minimum required = 1, but only found 0")); } + @Test + public void testValidateUsingExternallyDefinedCode() { + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl("http://foo"); + codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT); + IIdType csId = myCodeSystemDao.create(codeSystem).getId(); + + TermCodeSystemVersion csv = new TermCodeSystemVersion(); + csv.addConcept().setCode("bar").setDisplay("Bar Code"); + myTerminologySvc.storeNewCodeSystemVersion(codeSystem, csv, mySrd, Collections.emptyList(), Collections.emptyList()); + + // Validate a resource containing this codesystem in a field with an extendable binding + Patient patient = new Patient(); + patient.getText().setStatus(Narrative.NarrativeStatus.GENERATED).setDivAsString("

hello
"); + patient + .addIdentifier() + .setSystem("http://example.com") + .setValue("12345") + .getType() + .addCoding() + .setSystem("http://foo") + .setCode("bar"); + MethodOutcome outcome = myPatientDao.validate(patient, null, encode(patient), EncodingEnum.JSON, ValidationModeEnum.CREATE, null, mySrd); + IBaseOperationOutcome oo = outcome.getOperationOutcome(); + ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(oo)); + + // It would be ok for this to produce 0 issues, or just an information message too + assertEquals(1, OperationOutcomeUtil.getIssueCount(myFhirCtx, oo)); + assertEquals("None of the codes provided are in the value set http://hl7.org/fhir/ValueSet/identifier-type (http://hl7.org/fhir/ValueSet/identifier-type, and a code should come from this value set unless it has no suitable code) (codes = http://foo#bar)", OperationOutcomeUtil.getFirstIssueDetails(myFhirCtx, oo)); + + } + + private String encode(Patient thePatient) { + return myFhirCtx.newJsonParser().encodeResourceToString(thePatient); + } + + private OperationOutcome doTestValidateResourceContainingProfileDeclaration(String methodName, EncodingEnum enc) throws IOException { Bundle vss = loadResourceFromClasspath(Bundle.class, "/org/hl7/fhir/r4/model/valueset/valuesets.xml"); myValueSetDao.update((ValueSet) findResourceByIdInBundle(vss, "observation-status"), mySrd); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java index 02d94fba0ae..be44fcf4e3c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java @@ -293,6 +293,7 @@ public class SearchCoordinatorSvcImplTest { myExpectedNumberOfSearchBuildersCreated = 4; } + @Test public void testAsyncSearchSmallResultSetSameCoordinator() { SearchParameterMap params = new SearchParameterMap(); diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/dstu3/hapi/validation/ValidationSupportChain.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/dstu3/hapi/validation/ValidationSupportChain.java index d465be2dba7..a399211d5f4 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/dstu3/hapi/validation/ValidationSupportChain.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/dstu3/hapi/validation/ValidationSupportChain.java @@ -98,11 +98,9 @@ public class ValidationSupportChain implements IValidationSupport { @Override public CodeSystem fetchCodeSystem(FhirContext theCtx, String theSystem) { for (IValidationSupport next : myChain) { - if (next.isCodeSystemSupported(theCtx, theSystem)) { - CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); - if (retVal != null) { - return retVal; - } + CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); + if (retVal != null) { + return retVal; } } return null; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/r4/hapi/validation/ValidationSupportChain.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/r4/hapi/validation/ValidationSupportChain.java index 1f80bcfb382..4567802b638 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/r4/hapi/validation/ValidationSupportChain.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/r4/hapi/validation/ValidationSupportChain.java @@ -77,11 +77,9 @@ public class ValidationSupportChain implements IValidationSupport { @Override public CodeSystem fetchCodeSystem(FhirContext theCtx, String theSystem) { for (IValidationSupport next : myChain) { - if (next.isCodeSystemSupported(theCtx, theSystem)) { - CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); - if (retVal != null) { - return retVal; - } + CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); + if (retVal != null) { + return retVal; } } return null; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/r5/hapi/validation/ValidationSupportChain.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/r5/hapi/validation/ValidationSupportChain.java index 2ddde0adf08..cdbb001bd5b 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/r5/hapi/validation/ValidationSupportChain.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/r5/hapi/validation/ValidationSupportChain.java @@ -75,11 +75,9 @@ public class ValidationSupportChain implements IValidationSupport { @Override public CodeSystem fetchCodeSystem(FhirContext theCtx, String theSystem) { for (IValidationSupport next : myChain) { - if (next.isCodeSystemSupported(theCtx, theSystem)) { - CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); - if (retVal != null) { - return retVal; - } + CodeSystem retVal = next.fetchCodeSystem(theCtx, theSystem); + if (retVal != null) { + return retVal; } } return null; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7eac542c8e7..d93e1387c28 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -248,6 +248,10 @@ =]]> or in]]> operators. At present, the ancestor]]> filter can only be used with the =]]> operator. + + The JPA server failed to find codes defined in not-present codesystems in some cases, and reported + that the CodeSystem did not exist. This has been corrected. +