diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index 0f9b12a8d94..b996327a717 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -14,6 +14,10 @@ import ca.uhn.fhir.jpa.provider.TerminologyUploaderProvider; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.IStaleSearchDeletingSvc; import ca.uhn.fhir.jpa.search.StaleSearchDeletingSvcImpl; +import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl; +import ca.uhn.fhir.jpa.search.cache.DatabaseSearchResultCacheSvcImpl; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; +import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.search.reindex.ResourceReindexingSvcImpl; import ca.uhn.fhir.jpa.subscription.dbmatcher.CompositeInMemoryDaoSubscriptionMatcher; @@ -143,6 +147,16 @@ public abstract class BaseConfig implements SchedulingConfigurer { return new BinaryStorageInterceptor(); } + @Bean + public ISearchCacheSvc searchCacheSvc() { + return new DatabaseSearchCacheSvcImpl(); + } + + @Bean + public ISearchResultCacheSvc searchResultCacheSvc() { + return new DatabaseSearchResultCacheSvcImpl(); + } + @Bean public TaskScheduler taskScheduler() { ConcurrentTaskScheduler retVal = new ConcurrentTaskScheduler(); 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 55d9e12d766..1863d6917f0 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 @@ -19,6 +19,8 @@ import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage; import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; +import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.searchparam.ResourceMetaParams; import ca.uhn.fhir.jpa.searchparam.extractor.LogicalReferenceHelper; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; @@ -156,7 +158,9 @@ public abstract class BaseHapiFhirDao implements IDao, @Autowired private PlatformTransactionManager myPlatformTransactionManager; @Autowired - private ISearchDao mySearchDao; + private ISearchCacheSvc mySearchCacheSvc; + @Autowired + private ISearchResultCacheSvc mySearchResultCacheSvc; @Autowired private ISearchParamPresenceSvc mySearchParamPresenceSvc; @Autowired @@ -463,7 +467,7 @@ public abstract class BaseHapiFhirDao implements IDao, } } - search = mySearchDao.save(search); + search = mySearchCacheSvc.save(search); return new PersistedJpaBundleProvider(theRequest, search.getUuid(), this); } @@ -489,7 +493,7 @@ public abstract class BaseHapiFhirDao implements IDao, theProvider.setContext(getContext()); theProvider.setEntityManager(myEntityManager); theProvider.setPlatformTransactionManager(myPlatformTransactionManager); - theProvider.setSearchDao(mySearchDao); + theProvider.setSearchCacheSvc(mySearchCacheSvc); theProvider.setSearchCoordinatorSvc(mySearchCoordinatorSvc); theProvider.setInterceptorBroadcaster(myInterceptorBroadcaster); } 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 f638c26be59..82994c5e145 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 @@ -54,7 +54,6 @@ import ca.uhn.fhir.validation.*; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.*; import org.hl7.fhir.r4.model.InstantType; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -72,6 +71,7 @@ import javax.annotation.PostConstruct; import javax.persistence.NoResultException; import javax.persistence.TypedQuery; import javax.servlet.http.HttpServletResponse; +import javax.validation.constraints.NotNull; import java.io.IOException; import java.util.*; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java index 04291100c31..88cbd75d8ee 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java @@ -44,7 +44,10 @@ import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import javax.servlet.http.HttpServletResponse; -import java.util.*; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Set; /** * Note that this interface is not considered a stable interface. While it is possible to build applications diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirSystemDao.java index 365d8fd8ebc..91213180336 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirSystemDao.java @@ -24,7 +24,6 @@ import ca.uhn.fhir.jpa.util.ExpungeOptions; import ca.uhn.fhir.jpa.util.ExpungeOutcome; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; -import com.google.common.annotations.Beta; import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java index e3716f54999..3acc1a39d04 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java @@ -10,6 +10,7 @@ import org.springframework.data.repository.query.Param; import java.util.Collection; import java.util.Date; +import java.util.Optional; /* * #%L @@ -33,15 +34,12 @@ import java.util.Date; public interface ISearchDao extends JpaRepository { - @Query("SELECT s FROM Search s WHERE s.myUuid = :uuid") - Search findByUuid(@Param("uuid") String theUuid); + @Query("SELECT s FROM Search s LEFT OUTER JOIN FETCH s.myIncludes WHERE s.myUuid = :uuid") + Optional findByUuidAndFetchIncludes(@Param("uuid") String theUuid); @Query("SELECT s.myId FROM Search s WHERE s.mySearchLastReturned < :cutoff") Slice findWhereLastReturnedBefore(@Param("cutoff") Date theCutoff, Pageable thePage); -// @SqlQuery("SELECT s FROM Search s WHERE s.myCreated < :cutoff") -// public Collection findWhereCreatedBefore(@Param("cutoff") Date theCutoff); - @Query("SELECT s FROM Search s WHERE s.myResourceType = :type AND mySearchQueryStringHash = :hash AND s.myCreated > :cutoff AND s.myDeleted = false") Collection find(@Param("type") String theResourceType, @Param("hash") int theHashCode, @Param("cutoff") Date theCreatedCutoff); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchIncludeDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchIncludeDao.java index 3307bb8c2be..0dc0681ba9a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchIncludeDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchIncludeDao.java @@ -20,14 +20,13 @@ package ca.uhn.fhir.jpa.dao.data; * #L% */ +import ca.uhn.fhir.jpa.entity.SearchInclude; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; -import ca.uhn.fhir.jpa.entity.SearchInclude; - -public interface ISearchIncludeDao extends JpaRepository { +public interface ISearchIncludeDao extends JpaRepository { @Modifying @Query(value="DELETE FROM SearchInclude r WHERE r.mySearchPid = :search") diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java index 58c5c0eae06..107537ed3c9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java @@ -1,8 +1,6 @@ package ca.uhn.fhir.jpa.dao.data; -import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.entity.SearchResult; -import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.jpa.repository.JpaRepository; @@ -10,9 +8,7 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; -import java.util.Iterator; import java.util.List; -import java.util.Set; /* * #%L @@ -34,21 +30,17 @@ import java.util.Set; * #L% */ -public interface ISearchResultDao extends JpaRepository { +public interface ISearchResultDao extends JpaRepository { - @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearch = :search ORDER BY r.myOrder ASC") - Page findWithSearchUuid(@Param("search") Search theSearch, Pageable thePage); + @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearchPid = :search ORDER BY r.myOrder ASC") + Slice findWithSearchPid(@Param("search") Long theSearchPid, Pageable thePage); - @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearch = :search") - List findWithSearchUuidOrderIndependent(@Param("search") Search theSearch); + @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearchPid = :search") + List findWithSearchPidOrderIndependent(@Param("search") Long theSearchPid); @Query(value="SELECT r.myId FROM SearchResult r WHERE r.mySearchPid = :search") Slice findForSearch(Pageable thePage, @Param("search") Long theSearchPid); - @Modifying - @Query("DELETE FROM SearchResult s WHERE s.myResourcePid IN :ids") - void deleteByResourceIds(@Param("ids") List theContent); - @Modifying @Query("DELETE FROM SearchResult s WHERE s.myId IN :ids") void deleteByIds(@Param("ids") List theContent); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeOperation.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeOperation.java index f092b6d73e4..95aee1da8b7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeOperation.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ExpungeOperation.java @@ -84,7 +84,6 @@ public class ExpungeOperation implements Callable { private void expungeDeletedResources() { Slice resourceIds = findHistoricalVersionsOfDeletedResources(); - deleteSearchResultCacheEntries(resourceIds); deleteHistoricalVersions(resourceIds); if (expungeLimitReached()) { return; @@ -123,10 +122,6 @@ public class ExpungeOperation implements Callable { myPartitionRunner.runInPartitionedThreads(theResourceIds, partition -> myExpungeDaoService.expungeHistoricalVersionsOfIds(myRequestDetails, partition, myRemainingCount)); } - private void deleteSearchResultCacheEntries(Slice theResourceIds) { - myPartitionRunner.runInPartitionedThreads(theResourceIds, partition -> myExpungeDaoService.deleteByResourceIdPartitions(partition)); - } - private ExpungeOutcome expungeOutcome() { return new ExpungeOutcome().setDeletedCount(myExpungeOptions.getLimit() - myRemainingCount.get()); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/IResourceExpungeService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/IResourceExpungeService.java index a8f1a52aafc..5beaf5fa548 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/IResourceExpungeService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/IResourceExpungeService.java @@ -37,7 +37,5 @@ public interface IResourceExpungeService { void expungeHistoricalVersionsOfIds(RequestDetails theRequestDetails, List thePartition, AtomicInteger theRemainingCount); - void deleteByResourceIdPartitions(List thePartition); - void deleteAllSearchParams(Long theResourceId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java index 5f5203a055f..ffc69bf92df 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java @@ -58,8 +58,6 @@ class ResourceExpungeService implements IResourceExpungeService { @Autowired private IResourceTableDao myResourceTableDao; @Autowired - private ISearchResultDao mySearchResultDao; - @Autowired private IResourceHistoryTableDao myResourceHistoryTableDao; @Autowired private IResourceIndexedSearchParamUriDao myResourceIndexedSearchParamUriDao; @@ -250,12 +248,6 @@ class ResourceExpungeService implements IResourceExpungeService { } } - @Override - @Transactional - public void deleteByResourceIdPartitions(List theResourceIds) { - mySearchResultDao.deleteByResourceIds(theResourceIds); - } - private Slice toSlice(ResourceHistoryTable myVersion) { Validate.notNull(myVersion); return new SliceImpl<>(Collections.singletonList(myVersion.getId())); 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 ee52ab794e1..10908546890 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 @@ -1,10 +1,10 @@ package ca.uhn.fhir.jpa.entity; -import ca.uhn.fhir.rest.server.util.ICachedSearchDetails; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.rest.server.util.ICachedSearchDetails; import org.apache.commons.lang3.SerializationUtils; import org.hibernate.annotations.OptimisticLock; @@ -80,8 +80,6 @@ public class Search implements ICachedSearchDetails, Serializable { private Long myResourceId; @Column(name = "RESOURCE_TYPE", length = 200, nullable = true) private String myResourceType; - @OneToMany(mappedBy = "mySearch", fetch = FetchType.LAZY) - private Collection myResults; @NotNull @Temporal(TemporalType.TIMESTAMP) @Column(name = "SEARCH_LAST_RETURNED", nullable = false, updatable = false) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/SearchResult.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/SearchResult.java index 62cb95309bb..62536ccb5d7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/SearchResult.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/SearchResult.java @@ -20,7 +20,7 @@ package ca.uhn.fhir.jpa.entity; * #L% */ -import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.ToStringBuilder; import javax.persistence.*; @@ -39,17 +39,11 @@ public class SearchResult implements Serializable { @Id @Column(name = "PID") private Long myId; - @Column(name = "SEARCH_ORDER", nullable = false) + @Column(name = "SEARCH_ORDER", nullable = false, insertable = true, updatable = false) private int myOrder; - @ManyToOne - @JoinColumn(name = "RESOURCE_PID", referencedColumnName = "RES_ID", foreignKey = @ForeignKey(name = "FK_SEARCHRES_RES"), insertable = false, updatable = false, nullable = false) - private ResourceTable myResource; @Column(name = "RESOURCE_PID", insertable = true, updatable = false, nullable = false) private Long myResourcePid; - @ManyToOne - @JoinColumn(name = "SEARCH_PID", referencedColumnName = "PID", foreignKey = @ForeignKey(name = "FK_SEARCHRES_SEARCH")) - private Search mySearch; - @Column(name = "SEARCH_PID", insertable = false, updatable = false, nullable = false) + @Column(name = "SEARCH_PID", insertable = true, updatable = false, nullable = false) private Long mySearchPid; /** @@ -63,7 +57,8 @@ public class SearchResult implements Serializable { * Constructor */ public SearchResult(Search theSearch) { - mySearch = theSearch; + Validate.notNull(theSearch.getId()); + mySearchPid = theSearch.getId(); } @Override diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/.editorconfig b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/.editorconfig deleted file mode 100644 index e69de29bb2d..00000000000 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 5f61a46a253..8d3f97c2912 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 @@ -26,11 +26,11 @@ import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.dao.IDao; import ca.uhn.fhir.jpa.dao.ISearchBuilder; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.entity.SearchTypeEnum; import ca.uhn.fhir.jpa.model.entity.BaseHasResource; import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.rest.api.server.*; @@ -46,7 +46,6 @@ import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; import javax.persistence.EntityManager; -import javax.persistence.NoResultException; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; @@ -64,7 +63,7 @@ public class PersistedJpaBundleProvider implements IBundleProvider { private EntityManager myEntityManager; private PlatformTransactionManager myPlatformTransactionManager; private ISearchCoordinatorSvc mySearchCoordinatorSvc; - private ISearchDao mySearchDao; + private ISearchCacheSvc mySearchCacheSvc; private Search mySearchEntity; private String myUuid; private boolean myCacheHit; @@ -192,27 +191,16 @@ public class PersistedJpaBundleProvider implements IBundleProvider { if (mySearchEntity == null) { ensureDependenciesInjected(); - TransactionTemplate txTemplate = new TransactionTemplate(myPlatformTransactionManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); - txTemplate.setIsolationLevel(TransactionDefinition.ISOLATION_READ_COMMITTED); - return txTemplate.execute(s -> { - try { - setSearchEntity(mySearchDao.findByUuid(myUuid)); + Optional search = mySearchCacheSvc.fetchByUuid(myUuid); + if (!search.isPresent()) { + return false; + } - if (mySearchEntity == null) { - return false; - } + setSearchEntity(search.get()); - ourLog.trace("Retrieved search with version {} and total {}", mySearchEntity.getVersion(), mySearchEntity.getTotalCount()); + ourLog.trace("Retrieved search with version {} and total {}", mySearchEntity.getVersion(), mySearchEntity.getTotalCount()); - // Load the includes now so that they are available outside of this transaction - mySearchEntity.getIncludes().size(); - - return true; - } catch (NoResultException e) { - return false; - } - }); + return true; } return true; } @@ -292,10 +280,6 @@ public class PersistedJpaBundleProvider implements IBundleProvider { mySearchCoordinatorSvc = theSearchCoordinatorSvc; } - public void setSearchDao(ISearchDao theSearchDao) { - mySearchDao = theSearchDao; - } - // Note: Leave as protected, HSPC depends on this @SuppressWarnings("WeakerAccess") protected void setSearchEntity(Search theSearchEntity) { @@ -353,4 +337,8 @@ public class PersistedJpaBundleProvider implements IBundleProvider { public void setInterceptorBroadcaster(IInterceptorBroadcaster theInterceptorBroadcaster) { myInterceptorBroadcaster = theInterceptorBroadcaster; } + + public void setSearchCacheSvc(ISearchCacheSvc theSearchCacheSvc) { + mySearchCacheSvc = theSearchCacheSvc; + } } 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 a6402100d4d..763e1f18251 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 @@ -25,16 +25,14 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.dao.*; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; -import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.entity.SearchInclude; -import ca.uhn.fhir.jpa.entity.SearchResult; import ca.uhn.fhir.jpa.entity.SearchTypeEnum; import ca.uhn.fhir.jpa.interceptor.JpaPreResourceAccessDetails; import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; +import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster; import ca.uhn.fhir.model.api.Include; @@ -56,15 +54,12 @@ import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.util.ICachedSearchDetails; import ca.uhn.fhir.util.StopWatch; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Lists; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.instance.model.api.IBaseResource; -import org.jetbrains.annotations.NotNull; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.AbstractPageRequest; -import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.orm.jpa.JpaDialect; @@ -83,7 +78,10 @@ import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nullable; import javax.annotation.PostConstruct; import javax.persistence.EntityManager; +import javax.validation.constraints.NotNull; import java.io.IOException; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.*; import java.util.concurrent.*; @@ -106,16 +104,14 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { private long myMaxMillisToWaitForRemoteResults = DateUtils.MILLIS_PER_MINUTE; private boolean myNeverUseLocalSearchForUnitTests; @Autowired - private ISearchDao mySearchDao; - @Autowired - private ISearchIncludeDao mySearchIncludeDao; - @Autowired - private ISearchResultDao mySearchResultDao; - @Autowired private IInterceptorBroadcaster myInterceptorBroadcaster; @Autowired private PlatformTransactionManager myManagedTxManager; @Autowired + private ISearchCacheSvc mySearchCacheSvc; + @Autowired + private ISearchResultCacheSvc mySearchResultCacheSvc; + @Autowired private DaoRegistry myDaoRegistry; @Autowired private IPagingProvider myPagingProvider; @@ -134,6 +130,12 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { myExecutor = Executors.newCachedThreadPool(threadFactory); } + @VisibleForTesting + public void setSearchCacheServicesForUnitTest(ISearchCacheSvc theSearchCacheSvc, ISearchResultCacheSvc theSearchResultCacheSvc) { + mySearchCacheSvc = theSearchCacheSvc; + mySearchResultCacheSvc = theSearchResultCacheSvc; + } + @PostConstruct public void start() { if (myManagedTxManager instanceof JpaTransactionManager) { @@ -184,13 +186,13 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } } - search = txTemplate.execute(t -> mySearchDao.findByUuid(theUuid)); - - if (search == null) { - ourLog.debug("Client requested unknown paging ID[{}]", theUuid); - String msg = myContext.getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", theUuid); - throw new ResourceGoneException(msg); - } + search = mySearchCacheSvc + .fetchByUuid(theUuid) + .orElseThrow(() -> { + ourLog.debug("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) { @@ -210,7 +212,7 @@ 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) { - Optional newSearch = tryToMarkSearchAsInProgress(search); + Optional newSearch = mySearchCacheSvc.tryToMarkSearchAsInProgress(search); if (newSearch.isPresent()) { search = newSearch.get(); String resourceType = search.getResourceType(); @@ -229,60 +231,22 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } } - final Pageable page = toPage(theFrom, theTo); - if (page == null) { - return Collections.emptyList(); - } - final Search foundSearch = search; - - ourLog.trace("Loading stored search"); - List retVal = txTemplate.execute(theStatus -> { - final List resultPids = new ArrayList<>(); - Page searchResultPids = mySearchResultDao.findWithSearchUuid(foundSearch, page); - for (Long next : searchResultPids) { - resultPids.add(next); - } - return resultPids; - }); - return retVal; + return mySearchResultCacheSvc.fetchResultPids(search, theFrom, theTo); } - private Optional tryToMarkSearchAsInProgress(Search theSearch) { - ourLog.trace("Going to try to change search status from {} to {}", theSearch.getStatus(), SearchStatusEnum.LOADING); - try { - TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); - txTemplate.afterPropertiesSet(); - return txTemplate.execute(t -> { - Search search = mySearchDao.findById(theSearch.getId()).orElse(theSearch); - - if (search.getStatus() != SearchStatusEnum.PASSCMPLET) { - throw new IllegalStateException("Can't change to LOADING because state is " + theSearch.getStatus()); - } - search.setStatus(SearchStatusEnum.LOADING); - Search newSearch = mySearchDao.save(search); - return Optional.of(newSearch); - }); - } catch (Exception e) { - ourLog.warn("Failed to activate search: {}", e.toString()); - ourLog.trace("Failed to activate search", e); - return Optional.empty(); - } - } private void populateBundleProvider(PersistedJpaBundleProvider theRetVal) { theRetVal.setContext(myContext); theRetVal.setEntityManager(myEntityManager); theRetVal.setPlatformTransactionManager(myManagedTxManager); - theRetVal.setSearchDao(mySearchDao); + theRetVal.setSearchCacheSvc(mySearchCacheSvc); theRetVal.setSearchCoordinatorSvc(this); theRetVal.setInterceptorBroadcaster(myInterceptorBroadcaster); } @Override public IBundleProvider registerSearch(final IDao theCallingDao, final SearchParameterMap theParams, String theResourceType, CacheControlDirective theCacheControlDirective, RequestDetails theRequestDetails) { - StopWatch w = new StopWatch(); final String searchUuid = UUID.randomUUID().toString(); ourLog.debug("Registering new search {}", searchUuid); @@ -292,6 +256,185 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { sb.setType(resourceTypeClass, theResourceType); sb.setFetchSize(mySyncSize); + final Integer loadSynchronousUpTo = getLoadSynchronousUpToOrNull(theCacheControlDirective); + + if (theParams.isLoadSynchronous() || loadSynchronousUpTo != null) { + ourLog.debug("Search {} is loading in synchronous mode", searchUuid); + return executeQuery(theParams, theRequestDetails, searchUuid, sb, loadSynchronousUpTo); + } + + /* + * See if there are any cached searches whose results we can return + * instead + */ + boolean useCache = true; + if (theCacheControlDirective != null && theCacheControlDirective.isNoCache() == true) { + useCache = false; + } + + final String queryString = theParams.toNormalizedQueryString(myContext); + if (theParams.getEverythingMode() == null) { + if (myDaoConfig.getReuseCachedSearchResultsForMillis() != null && useCache) { + IBundleProvider foundSearchProvider = findCachedQuery(theCallingDao, theParams, theResourceType, theRequestDetails, queryString); + if (foundSearchProvider != null) { + return foundSearchProvider; + } + } + } + + return submitSearch(theCallingDao, theParams, theResourceType, theRequestDetails, searchUuid, sb, queryString); + + } + + @NotNull + private IBundleProvider submitSearch(IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequestDetails, String theSearchUuid, ISearchBuilder theSb, String theQueryString) { + StopWatch w = new StopWatch(); + Search search = new Search(); + populateSearchEntity(theParams, theResourceType, theSearchUuid, theQueryString, search); + + // Interceptor call: STORAGE_PRESEARCH_REGISTERED + HookParams params = new HookParams() + .add(ICachedSearchDetails.class, search) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); + JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESEARCH_REGISTERED, params); + + SearchTask task = new SearchTask(search, theCallingDao, theParams, theResourceType, theRequestDetails); + myIdToSearchTask.put(search.getUuid(), task); + myExecutor.submit(task); + + PersistedJpaSearchFirstPageBundleProvider retVal = new PersistedJpaSearchFirstPageBundleProvider(search, theCallingDao, task, theSb, myManagedTxManager, theRequestDetails); + populateBundleProvider(retVal); + + ourLog.debug("Search initial phase completed in {}ms", w.getMillis()); + return retVal; + } + + @org.jetbrains.annotations.Nullable + private IBundleProvider findCachedQuery(IDao theCallingDao, SearchParameterMap theParams, String theResourceType, RequestDetails theRequestDetails, String theQueryString) { + TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); + PersistedJpaBundleProvider foundSearchProvider = txTemplate.execute(t -> { + + // Interceptor call: STORAGE_PRECHECK_FOR_CACHED_SEARCH + HookParams params = new HookParams() + .add(SearchParameterMap.class, theParams) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); + Object outcome = JpaInterceptorBroadcaster.doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRECHECK_FOR_CACHED_SEARCH, params); + if (Boolean.FALSE.equals(outcome)) { + return null; + } + + // Check for a search matching the given hash + Search searchToUse = findSearchToUseOrNull(theQueryString, theResourceType); + if (searchToUse == null) { + return null; + } + + ourLog.debug("Reusing search {} from cache", searchToUse.getUuid()); + // Interceptor call: JPA_PERFTRACE_SEARCH_REUSING_CACHED + params = new HookParams() + .add(SearchParameterMap.class, theParams) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); + JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.JPA_PERFTRACE_SEARCH_REUSING_CACHED, params); + + mySearchCacheSvc.updateSearchLastReturned(searchToUse, new Date()); + + PersistedJpaBundleProvider retVal = new PersistedJpaBundleProvider(theRequestDetails, searchToUse.getUuid(), theCallingDao); + retVal.setCacheHit(true); + populateBundleProvider(retVal); + + return retVal; + }); + + if (foundSearchProvider != null) { + return foundSearchProvider; + } + return null; + } + + @Nullable + private Search findSearchToUseOrNull(String theQueryString, String theResourceType) { + Search searchToUse = null; + + // createdCutoff is in recent past + final Instant createdCutoff = Instant.now().minus(myDaoConfig.getReuseCachedSearchResultsForMillis(), ChronoUnit.MILLIS); + Collection candidates = mySearchCacheSvc.findCandidatesForReuse(theResourceType, theQueryString, theQueryString.hashCode(), Date.from(createdCutoff)); + + for (Search nextCandidateSearch : candidates) { + // We should only reuse our search if it was created within the permitted window + // Date.after() is unreliable. Instant.isAfter() always works. + if (theQueryString.equals(nextCandidateSearch.getSearchQueryString()) && nextCandidateSearch.getCreated().toInstant().isAfter(createdCutoff)) { + searchToUse = nextCandidateSearch; + break; + } + } + return searchToUse; + } + + private IBundleProvider executeQuery(SearchParameterMap theParams, RequestDetails theRequestDetails, String theSearchUuid, ISearchBuilder theSb, Integer theLoadSynchronousUpTo) { + SearchRuntimeDetails searchRuntimeDetails = new SearchRuntimeDetails(theRequestDetails, theSearchUuid); + searchRuntimeDetails.setLoadSynchronous(true); + + // Execute the query and make sure we return distinct results + TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); + return txTemplate.execute(t -> { + + // Load the results synchronously + final List pids = new ArrayList<>(); + + try (IResultIterator resultIter = theSb.createQuery(theParams, searchRuntimeDetails, theRequestDetails)) { + while (resultIter.hasNext()) { + pids.add(resultIter.next()); + if (theLoadSynchronousUpTo != null && pids.size() >= theLoadSynchronousUpTo) { + break; + } + if (theParams.getLoadSynchronousUpTo() != null && pids.size() >= theParams.getLoadSynchronousUpTo()) { + break; + } + } + } catch (IOException e) { + ourLog.error("IO failure during database access", e); + throw new InternalErrorException(e); + } + + JpaPreResourceAccessDetails accessDetails = new JpaPreResourceAccessDetails(pids, () -> theSb); + HookParams params = new HookParams() + .add(IPreResourceAccessDetails.class, accessDetails) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); + JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PREACCESS_RESOURCES, params); + + for (int i = pids.size() - 1; i >= 0; i--) { + if (accessDetails.isDontReturnResourceAtIndex(i)) { + pids.remove(i); + } + } + + /* + * For synchronous queries, we load all the includes right away + * since we're returning a static bundle with all the results + * pre-loaded. This is ok because syncronous requests are not + * expected to be paged + * + * On the other hand for async queries we load includes/revincludes + * individually for pages as we return them to clients + */ + final Set includedPids = new HashSet<>(); + includedPids.addAll(theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getRevIncludes(), true, theParams.getLastUpdated(), "(synchronous)", theRequestDetails)); + includedPids.addAll(theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails)); + List includedPidsList = new ArrayList<>(includedPids); + + List resources = new ArrayList<>(); + theSb.loadResourcesByPid(pids, includedPidsList, resources, false, theRequestDetails); + return new SimpleBundleProvider(resources); + }); + } + + @org.jetbrains.annotations.Nullable + private Integer getLoadSynchronousUpToOrNull(CacheControlDirective theCacheControlDirective) { final Integer loadSynchronousUpTo; if (theCacheControlDirective != null && theCacheControlDirective.isNoStore()) { if (theCacheControlDirective.getMaxResults() != null) { @@ -305,158 +448,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } else { loadSynchronousUpTo = null; } - - if (theParams.isLoadSynchronous() || loadSynchronousUpTo != null) { - - ourLog.debug("Search {} is loading in synchronous mode", searchUuid); - SearchRuntimeDetails searchRuntimeDetails = new SearchRuntimeDetails(theRequestDetails, searchUuid); - searchRuntimeDetails.setLoadSynchronous(true); - - // Execute the query and make sure we return distinct results - TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); - return txTemplate.execute(t -> { - - // Load the results synchronously - final List pids = new ArrayList<>(); - - try (IResultIterator resultIter = sb.createQuery(theParams, searchRuntimeDetails, theRequestDetails)) { - while (resultIter.hasNext()) { - pids.add(resultIter.next()); - if (loadSynchronousUpTo != null && pids.size() >= loadSynchronousUpTo) { - break; - } - if (theParams.getLoadSynchronousUpTo() != null && pids.size() >= theParams.getLoadSynchronousUpTo()) { - break; - } - } - } catch (IOException e) { - ourLog.error("IO failure during database access", e); - throw new InternalErrorException(e); - } - - JpaPreResourceAccessDetails accessDetails = new JpaPreResourceAccessDetails(pids, () -> sb); - HookParams params = new HookParams() - .add(IPreResourceAccessDetails.class, accessDetails) - .add(RequestDetails.class, theRequestDetails) - .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); - JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PREACCESS_RESOURCES, params); - - for (int i = pids.size() - 1; i >= 0; i--) { - if (accessDetails.isDontReturnResourceAtIndex(i)) { - pids.remove(i); - } - } - - /* - * For synchronous queries, we load all the includes right away - * since we're returning a static bundle with all the results - * pre-loaded. This is ok because syncronous requests are not - * expected to be paged - * - * On the other hand for async queries we load includes/revincludes - * individually for pages as we return them to clients - */ - final Set includedPids = new HashSet<>(); - includedPids.addAll(sb.loadIncludes(myContext, myEntityManager, pids, theParams.getRevIncludes(), true, theParams.getLastUpdated(), "(synchronous)", theRequestDetails)); - includedPids.addAll(sb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails)); - List includedPidsList = new ArrayList<>(includedPids); - - List resources = new ArrayList<>(); - sb.loadResourcesByPid(pids, includedPidsList, resources, false, theRequestDetails); - return new SimpleBundleProvider(resources); - }); - } - - /* - * See if there are any cached searches whose results we can return - * instead - */ - boolean useCache = true; - if (theCacheControlDirective != null && theCacheControlDirective.isNoCache() == true) { - useCache = false; - } - final String queryString = theParams.toNormalizedQueryString(myContext); - if (theParams.getEverythingMode() == null) { - if (myDaoConfig.getReuseCachedSearchResultsForMillis() != null && useCache) { - - final Date createdCutoff = new Date(System.currentTimeMillis() - myDaoConfig.getReuseCachedSearchResultsForMillis()); - final String resourceType = theResourceType; - - TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); - PersistedJpaBundleProvider foundSearchProvider = txTemplate.execute(t -> { - Search searchToUse = null; - - // Interceptor call: STORAGE_PRECHECK_FOR_CACHED_SEARCH - HookParams params = new HookParams() - .add(SearchParameterMap.class, theParams) - .add(RequestDetails.class, theRequestDetails) - .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); - Object outcome = JpaInterceptorBroadcaster.doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRECHECK_FOR_CACHED_SEARCH, params); - if (Boolean.FALSE.equals(outcome)) { - return null; - } - - // Check for a search matching the given hash - int hashCode = queryString.hashCode(); - Collection candidates = mySearchDao.find(resourceType, hashCode, createdCutoff); - for (Search nextCandidateSearch : candidates) { - if (queryString.equals(nextCandidateSearch.getSearchQueryString())) { - searchToUse = nextCandidateSearch; - break; - } - } - - PersistedJpaBundleProvider retVal = null; - if (searchToUse != null) { - ourLog.debug("Reusing search {} from cache", searchToUse.getUuid()); - - // Interceptor call: JPA_PERFTRACE_SEARCH_REUSING_CACHED - params = new HookParams() - .add(SearchParameterMap.class, theParams) - .add(RequestDetails.class, theRequestDetails) - .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); - JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.JPA_PERFTRACE_SEARCH_REUSING_CACHED, params); - - searchToUse.setSearchLastReturned(new Date()); - mySearchDao.updateSearchLastReturned(searchToUse.getId(), new Date()); - - retVal = new PersistedJpaBundleProvider(theRequestDetails, searchToUse.getUuid(), theCallingDao); - retVal.setCacheHit(true); - - populateBundleProvider(retVal); - } - - return retVal; - }); - - if (foundSearchProvider != null) { - return foundSearchProvider; - } - - } - } - - Search search = new Search(); - populateSearchEntity(theParams, theResourceType, searchUuid, queryString, search); - - // Interceptor call: STORAGE_PRESEARCH_REGISTERED - HookParams params = new HookParams() - .add(ICachedSearchDetails.class, search) - .add(RequestDetails.class, theRequestDetails) - .addIfMatchesType(ServletRequestDetails.class, theRequestDetails); - JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESEARCH_REGISTERED, params); - - SearchTask task = new SearchTask(search, theCallingDao, theParams, theResourceType, theRequestDetails); - myIdToSearchTask.put(search.getUuid(), task); - myExecutor.submit(task); - - PersistedJpaSearchFirstPageBundleProvider retVal = new PersistedJpaSearchFirstPageBundleProvider(search, theCallingDao, task, sb, myManagedTxManager, theRequestDetails); - populateBundleProvider(retVal); - - ourLog.debug("Search initial phase completed in {}ms", w.getMillis()); - return retVal; - + return loadSynchronousUpTo; } private void callInterceptorStoragePreAccessResources(IInterceptorBroadcaster theInterceptorBroadcaster, RequestDetails theRequestDetails, ISearchBuilder theSb, List thePids) { @@ -500,21 +492,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { myNeverUseLocalSearchForUnitTests = theNeverUseLocalSearchForUnitTests; } - @VisibleForTesting - void setSearchDaoForUnitTest(ISearchDao theSearchDao) { - mySearchDao = theSearchDao; - } - - @VisibleForTesting - void setSearchDaoIncludeForUnitTest(ISearchIncludeDao theSearchIncludeDao) { - mySearchIncludeDao = theSearchIncludeDao; - } - - @VisibleForTesting - void setSearchDaoResultForUnitTest(ISearchResultDao theSearchResultDao) { - mySearchResultDao = theSearchResultDao; - } - @VisibleForTesting public void setSyncSizeForUnitTests(int theSyncSize) { mySyncSize = theSyncSize; @@ -696,20 +673,10 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } } - List resultsToSave = Lists.newArrayList(); - for (Long nextPid : unsyncedPids) { - SearchResult nextResult = new SearchResult(mySearch); - nextResult.setResourcePid(nextPid); - nextResult.setOrder(myCountSavedTotal); - resultsToSave.add(nextResult); - int order = nextResult.getOrder(); - ourLog.trace("Saving ORDER[{}] Resource {}", order, nextResult.getResourcePid()); - - myCountSavedTotal++; - myCountSavedThisPass++; - } - - mySearchResultDao.saveAll(resultsToSave); + // Actually store the results in the query cache storage + myCountSavedTotal += unsyncedPids.size(); + myCountSavedThisPass += unsyncedPids.size(); + mySearchResultCacheSvc.storeResults(mySearch, mySyncedPids, unsyncedPids); synchronized (mySyncedPids) { int numSyncedThisPass = unsyncedPids.size(); @@ -877,15 +844,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { private void doSaveSearch() { - Search newSearch; - if (mySearch.getId() == null) { - newSearch = mySearchDao.save(mySearch); - for (SearchInclude next : mySearch.getIncludes()) { - mySearchIncludeDao.save(next); - } - } else { - newSearch = mySearchDao.save(mySearch); - } + Search newSearch = mySearchCacheSvc.save(mySearch); // mySearchDao.save is not supposed to return null, but in unit tests // it can if the mock search dao isn't set up to handle that @@ -928,7 +887,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { mySearch.setStatus(SearchStatusEnum.FINISHED); } doSaveSearch(); - mySearchDao.flush(); } }); if (wantOnlyCount) { @@ -1058,7 +1016,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { TransactionTemplate txTemplate = new TransactionTemplate(myManagedTxManager); txTemplate.afterPropertiesSet(); txTemplate.execute(t -> { - List previouslyAddedResourcePids = mySearchResultDao.findWithSearchUuidOrderIndependent(getSearch()); + List previouslyAddedResourcePids = mySearchResultCacheSvc.fetchAllResultPids(getSearch()); ourLog.debug("Have {} previously added IDs in search: {}", previouslyAddedResourcePids.size(), getSearch().getUuid()); setPreviouslyAddedResourcePids(previouslyAddedResourcePids); return null; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java index b470e574e9d..5f41ca45230 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/StaleSearchDeletingSvcImpl.java @@ -21,24 +21,13 @@ package ca.uhn.fhir.jpa.search; */ import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; -import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Lists; -import org.apache.commons.lang3.time.DateUtils; -import org.hl7.fhir.dstu3.model.InstantType; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.domain.PageRequest; -import org.springframework.data.domain.Slice; import org.springframework.scheduling.annotation.Scheduled; -import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionTemplate; -import java.util.Date; -import java.util.List; +import static ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.DEFAULT_CUTOFF_SLACK; /** * Deletes old searches @@ -49,111 +38,16 @@ import java.util.List; // in Smile. // public class StaleSearchDeletingSvcImpl implements IStaleSearchDeletingSvc { - public static final long DEFAULT_CUTOFF_SLACK = 10 * DateUtils.MILLIS_PER_SECOND; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(StaleSearchDeletingSvcImpl.class); - /* - * Be careful increasing this number! We use the number of params here in a - * DELETE FROM foo WHERE params IN (aaaa) - * type query and this can fail if we have 1000s of params - */ - public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT = 500; - public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS = 20000; - private static int ourMaximumResultsToDeleteInOneStatement = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT; - private static int ourMaximumResultsToDeleteInOnePass = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS; - private static Long ourNowForUnitTests; - /* - * We give a bit of extra leeway just to avoid race conditions where a query result - * is being reused (because a new client request came in with the same params) right before - * the result is to be deleted - */ - private long myCutoffSlack = DEFAULT_CUTOFF_SLACK; @Autowired private DaoConfig myDaoConfig; @Autowired - private ISearchDao mySearchDao; - @Autowired - private ISearchIncludeDao mySearchIncludeDao; - @Autowired - private ISearchResultDao mySearchResultDao; - @Autowired - private PlatformTransactionManager myTransactionManager; - - private void deleteSearch(final Long theSearchPid) { - mySearchDao.findById(theSearchPid).ifPresent(searchToDelete -> { - mySearchIncludeDao.deleteForSearch(searchToDelete.getId()); - - /* - * Note, we're only deleting up to 500 results in an individual search here. This - * is to prevent really long running transactions in cases where there are - * huge searches with tons of results in them. By the time we've gotten here - * we have marked the parent Search entity as deleted, so it's not such a - * huge deal to be only partially deleting search results. They'll get deleted - * eventually - */ - int max = ourMaximumResultsToDeleteInOnePass; - Slice resultPids = mySearchResultDao.findForSearch(PageRequest.of(0, max), searchToDelete.getId()); - if (resultPids.hasContent()) { - List> partitions = Lists.partition(resultPids.getContent(), ourMaximumResultsToDeleteInOneStatement); - for (List nextPartition : partitions) { - mySearchResultDao.deleteByIds(nextPartition); - } - - } - - // Only delete if we don't have results left in this search - if (resultPids.getNumberOfElements() < max) { - ourLog.debug("Deleting search {}/{} - Created[{}] -- Last returned[{}]", searchToDelete.getId(), searchToDelete.getUuid(), new InstantType(searchToDelete.getCreated()), new InstantType(searchToDelete.getSearchLastReturned())); - mySearchDao.deleteByPid(searchToDelete.getId()); - } else { - ourLog.debug("Purged {} search results for deleted search {}/{}", resultPids.getSize(), searchToDelete.getId(), searchToDelete.getUuid()); - } - }); - } + private ISearchCacheSvc mySearchCacheSvc; @Override @Transactional(propagation = Propagation.NEVER) public void pollForStaleSearchesAndDeleteThem() { - if (!myDaoConfig.isExpireSearchResults()) { - return; - } - - long cutoffMillis = myDaoConfig.getExpireSearchResultsAfterMillis(); - if (myDaoConfig.getReuseCachedSearchResultsForMillis() != null) { - cutoffMillis = Math.max(cutoffMillis, myDaoConfig.getReuseCachedSearchResultsForMillis()); - } - final Date cutoff = new Date((now() - cutoffMillis) - myCutoffSlack); - - if (ourNowForUnitTests != null) { - ourLog.info("Searching for searches which are before {} - now is {}", new InstantType(cutoff), new InstantType(new Date(now()))); - } - - ourLog.debug("Searching for searches which are before {}", cutoff); - - TransactionTemplate tt = new TransactionTemplate(myTransactionManager); - final Slice toDelete = tt.execute(theStatus -> - mySearchDao.findWhereLastReturnedBefore(cutoff, PageRequest.of(0, 2000)) - ); - for (final Long nextSearchToDelete : toDelete) { - ourLog.debug("Deleting search with PID {}", nextSearchToDelete); - tt.execute(t -> { - mySearchDao.updateDeleted(nextSearchToDelete, true); - return null; - }); - - tt.execute(t -> { - deleteSearch(nextSearchToDelete); - return null; - }); - } - - int count = toDelete.getContent().size(); - if (count > 0) { - if (ourLog.isDebugEnabled()) { - long total = tt.execute(t -> mySearchDao.count()); - ourLog.debug("Deleted {} searches, {} remaining", count, total); - } - } - + mySearchCacheSvc.pollForStaleSearchesAndDeleteThem(); } @Scheduled(fixedDelay = DEFAULT_CUTOFF_SLACK) @@ -164,35 +58,4 @@ public class StaleSearchDeletingSvcImpl implements IStaleSearchDeletingSvc { pollForStaleSearchesAndDeleteThem(); } } - - @VisibleForTesting - public void setCutoffSlackForUnitTest(long theCutoffSlack) { - myCutoffSlack = theCutoffSlack; - } - - @VisibleForTesting - public static void setMaximumResultsToDeleteInOnePassForUnitTest(int theMaximumResultsToDeleteInOnePass) { - ourMaximumResultsToDeleteInOnePass = theMaximumResultsToDeleteInOnePass; - } - - @VisibleForTesting - public static void setMaximumResultsToDeleteForUnitTest(int theMaximumResultsToDelete) { - ourMaximumResultsToDeleteInOneStatement = theMaximumResultsToDelete; - } - - private static long now() { - if (ourNowForUnitTests != null) { - return ourNowForUnitTests; - } - return System.currentTimeMillis(); - } - - /** - * This is for unit tests only, do not call otherwise - */ - @VisibleForTesting - public static void setNowForUnitTests(Long theNowForUnitTests) { - ourNowForUnitTests = theNowForUnitTests; - } - } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/BaseSearchCacheSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/BaseSearchCacheSvcImpl.java new file mode 100644 index 00000000000..86899d2b5ba --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/BaseSearchCacheSvcImpl.java @@ -0,0 +1,44 @@ +package ca.uhn.fhir.jpa.search.cache; + +import ca.uhn.fhir.jpa.entity.Search; +import org.apache.commons.lang3.time.DateUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import java.util.Date; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +public abstract class BaseSearchCacheSvcImpl implements ISearchCacheSvc { + + @Autowired + private PlatformTransactionManager myTxManager; + + private ConcurrentHashMap myUnsyncedLastUpdated = new ConcurrentHashMap<>(); + + @Override + public void updateSearchLastReturned(Search theSearch, Date theDate) { + myUnsyncedLastUpdated.put(theSearch.getId(), theDate); + } + + @Override + @Scheduled(fixedDelay = 10 * DateUtils.MILLIS_PER_SECOND) + public void flushLastUpdated() { + TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); + txTemplate.execute(t->{ + for (Iterator> iter = myUnsyncedLastUpdated.entrySet().iterator(); iter.hasNext(); ) { + Map.Entry next = iter.next(); + flushLastUpdated(next.getKey(), next.getValue()); + iter.remove(); + } + return null; + }); + } + + protected abstract void flushLastUpdated(Long theSearchId, Date theLastUpdated); + + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchCacheSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchCacheSvcImpl.java new file mode 100644 index 00000000000..30e1616c4ea --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchCacheSvcImpl.java @@ -0,0 +1,241 @@ +package ca.uhn.fhir.jpa.search.cache; + +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; +import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.jpa.entity.SearchInclude; +import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; +import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.time.DateUtils; +import org.hl7.fhir.dstu3.model.InstantType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Slice; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; + +import javax.transaction.Transactional; +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Optional; + +public class DatabaseSearchCacheSvcImpl extends BaseSearchCacheSvcImpl { + private static final Logger ourLog = LoggerFactory.getLogger(DatabaseSearchCacheSvcImpl.class); + + /* + * Be careful increasing this number! We use the number of params here in a + * DELETE FROM foo WHERE params IN (aaaa) + * type query and this can fail if we have 1000s of params + */ + public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT = 500; + public static final int DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS = 20000; + public static final long DEFAULT_CUTOFF_SLACK = 10 * DateUtils.MILLIS_PER_SECOND; + + private static int ourMaximumResultsToDeleteInOneStatement = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT; + private static int ourMaximumResultsToDeleteInOnePass = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS; + private static Long ourNowForUnitTests; + /* + * We give a bit of extra leeway just to avoid race conditions where a query result + * is being reused (because a new client request came in with the same params) right before + * the result is to be deleted + */ + private long myCutoffSlack = DEFAULT_CUTOFF_SLACK; + + @Autowired + private ISearchDao mySearchDao; + @Autowired + private ISearchResultDao mySearchResultDao; + @Autowired + private ISearchIncludeDao mySearchIncludeDao; + @Autowired + private PlatformTransactionManager myTxManager; + @Autowired + private DaoConfig myDaoConfig; + + @VisibleForTesting + public void setCutoffSlackForUnitTest(long theCutoffSlack) { + myCutoffSlack = theCutoffSlack; + } + + @Transactional(Transactional.TxType.REQUIRED) + @Override + public Search save(Search theSearch) { + Search newSearch; + if (theSearch.getId() == null) { + newSearch = mySearchDao.save(theSearch); + for (SearchInclude next : theSearch.getIncludes()) { + mySearchIncludeDao.save(next); + } + } else { + newSearch = mySearchDao.save(theSearch); + } + return newSearch; + } + + @Override + @Transactional(Transactional.TxType.REQUIRED) + public Optional fetchByUuid(String theUuid) { + Validate.notBlank(theUuid); + return mySearchDao.findByUuidAndFetchIncludes(theUuid); + } + + + @Override + @Transactional(Transactional.TxType.NEVER) + public Optional tryToMarkSearchAsInProgress(Search theSearch) { + ourLog.trace("Going to try to change search status from {} to {}", theSearch.getStatus(), SearchStatusEnum.LOADING); + try { + TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + txTemplate.afterPropertiesSet(); + return txTemplate.execute(t -> { + Search search = mySearchDao.findById(theSearch.getId()).orElse(theSearch); + + if (search.getStatus() != SearchStatusEnum.PASSCMPLET) { + throw new IllegalStateException("Can't change to LOADING because state is " + theSearch.getStatus()); + } + search.setStatus(SearchStatusEnum.LOADING); + Search newSearch = mySearchDao.save(search); + return Optional.of(newSearch); + }); + } catch (Exception e) { + ourLog.warn("Failed to activate search: {}", e.toString()); + ourLog.trace("Failed to activate search", e); + return Optional.empty(); + } + } + + @Override + public Collection findCandidatesForReuse(String theResourceType, String theQueryString, int theQueryStringHash, Date theCreatedAfter) { + int hashCode = theQueryString.hashCode(); + return mySearchDao.find(theResourceType, hashCode, theCreatedAfter); + + } + + @Override + protected void flushLastUpdated(Long theSearchId, Date theLastUpdated) { + mySearchDao.updateSearchLastReturned(theSearchId, theLastUpdated); + } + + @Transactional(Transactional.TxType.NEVER) + @Override + public void pollForStaleSearchesAndDeleteThem() { + if (!myDaoConfig.isExpireSearchResults()) { + return; + } + + long cutoffMillis = myDaoConfig.getExpireSearchResultsAfterMillis(); + if (myDaoConfig.getReuseCachedSearchResultsForMillis() != null) { + cutoffMillis = Math.max(cutoffMillis, myDaoConfig.getReuseCachedSearchResultsForMillis()); + } + final Date cutoff = new Date((now() - cutoffMillis) - myCutoffSlack); + + if (ourNowForUnitTests != null) { + ourLog.info("Searching for searches which are before {} - now is {}", new InstantType(cutoff), new InstantType(new Date(now()))); + } + + ourLog.debug("Searching for searches which are before {}", cutoff); + + TransactionTemplate tt = new TransactionTemplate(myTxManager); + final Slice toDelete = tt.execute(theStatus -> + mySearchDao.findWhereLastReturnedBefore(cutoff, PageRequest.of(0, 2000)) + ); + for (final Long nextSearchToDelete : toDelete) { + ourLog.debug("Deleting search with PID {}", nextSearchToDelete); + tt.execute(t -> { + mySearchDao.updateDeleted(nextSearchToDelete, true); + return null; + }); + + tt.execute(t -> { + deleteSearch(nextSearchToDelete); + return null; + }); + } + + int count = toDelete.getContent().size(); + if (count > 0) { + if (ourLog.isDebugEnabled()) { + long total = tt.execute(t -> mySearchDao.count()); + ourLog.debug("Deleted {} searches, {} remaining", count, total); + } + } + } + + + @VisibleForTesting + void setSearchDaoForUnitTest(ISearchDao theSearchDao) { + mySearchDao = theSearchDao; + } + + @VisibleForTesting + void setSearchDaoIncludeForUnitTest(ISearchIncludeDao theSearchIncludeDao) { + mySearchIncludeDao = theSearchIncludeDao; + } + + private void deleteSearch(final Long theSearchPid) { + mySearchDao.findById(theSearchPid).ifPresent(searchToDelete -> { + mySearchIncludeDao.deleteForSearch(searchToDelete.getId()); + + /* + * Note, we're only deleting up to 500 results in an individual search here. This + * is to prevent really long running transactions in cases where there are + * huge searches with tons of results in them. By the time we've gotten here + * we have marked the parent Search entity as deleted, so it's not such a + * huge deal to be only partially deleting search results. They'll get deleted + * eventually + */ + int max = ourMaximumResultsToDeleteInOnePass; + Slice resultPids = mySearchResultDao.findForSearch(PageRequest.of(0, max), searchToDelete.getId()); + if (resultPids.hasContent()) { + List> partitions = Lists.partition(resultPids.getContent(), ourMaximumResultsToDeleteInOneStatement); + for (List nextPartition : partitions) { + mySearchResultDao.deleteByIds(nextPartition); + } + + } + + // Only delete if we don't have results left in this search + if (resultPids.getNumberOfElements() < max) { + ourLog.debug("Deleting search {}/{} - Created[{}] -- Last returned[{}]", searchToDelete.getId(), searchToDelete.getUuid(), new InstantType(searchToDelete.getCreated()), new InstantType(searchToDelete.getSearchLastReturned())); + mySearchDao.deleteByPid(searchToDelete.getId()); + } else { + ourLog.debug("Purged {} search results for deleted search {}/{}", resultPids.getSize(), searchToDelete.getId(), searchToDelete.getUuid()); + } + }); + } + + @VisibleForTesting + public static void setMaximumResultsToDeleteInOnePassForUnitTest(int theMaximumResultsToDeleteInOnePass) { + ourMaximumResultsToDeleteInOnePass = theMaximumResultsToDeleteInOnePass; + } + + @VisibleForTesting + public static void setMaximumResultsToDeleteForUnitTest(int theMaximumResultsToDelete) { + ourMaximumResultsToDeleteInOneStatement = theMaximumResultsToDelete; + } + + /** + * This is for unit tests only, do not call otherwise + */ + @VisibleForTesting + public static void setNowForUnitTests(Long theNowForUnitTests) { + ourNowForUnitTests = theNowForUnitTests; + } + + private static long now() { + if (ourNowForUnitTests != null) { + return ourNowForUnitTests; + } + return System.currentTimeMillis(); + } + +} 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 new file mode 100644 index 00000000000..572416dc952 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/DatabaseSearchResultCacheSvcImpl.java @@ -0,0 +1,78 @@ +package ca.uhn.fhir.jpa.search.cache; + +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; +import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.jpa.entity.SearchResult; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Pageable; + +import javax.transaction.Transactional; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl.toPage; + +public class DatabaseSearchResultCacheSvcImpl implements ISearchResultCacheSvc { + private static final Logger ourLog = LoggerFactory.getLogger(DatabaseSearchResultCacheSvcImpl.class); + + @Autowired + private ISearchResultDao mySearchResultDao; + + @Override + @Transactional(Transactional.TxType.REQUIRED) + public List fetchResultPids(Search theSearch, int theFrom, int theTo) { + final Pageable page = toPage(theFrom, theTo); + if (page == null) { + return Collections.emptyList(); + } + + List retVal = mySearchResultDao + .findWithSearchPid(theSearch.getId(), page) + .getContent(); + + ourLog.trace("fetchResultPids for range {}-{} returned {} pids", theFrom, theTo, retVal.size()); + + return new ArrayList<>(retVal); + } + + @Override + @Transactional(Transactional.TxType.REQUIRED) + public List fetchAllResultPids(Search theSearch) { + List retVal = mySearchResultDao.findWithSearchPidOrderIndependent(theSearch.getId()); + ourLog.trace("fetchAllResultPids returned {} pids", retVal.size()); + return retVal; + } + + @Override + @Transactional(Transactional.TxType.REQUIRED) + public void storeResults(Search theSearch, List thePreviouslyStoredResourcePids, List theNewResourcePids) { + List resultsToSave = Lists.newArrayList(); + + ourLog.trace("Storing {} results with {} previous for search", theNewResourcePids.size(), thePreviouslyStoredResourcePids.size()); + + int order = thePreviouslyStoredResourcePids.size(); + for (Long nextPid : theNewResourcePids) { + SearchResult nextResult = new SearchResult(theSearch); + nextResult.setResourcePid(nextPid); + nextResult.setOrder(order); + resultsToSave.add(nextResult); + ourLog.trace("Saving ORDER[{}] Resource {}", order, nextResult.getResourcePid()); + + order++; + } + + mySearchResultDao.saveAll(resultsToSave); + } + + @VisibleForTesting + void setSearchDaoResultForUnitTest(ISearchResultDao theSearchResultDao) { + mySearchResultDao = theSearchResultDao; + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchCacheSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchCacheSvc.java new file mode 100644 index 00000000000..74b84fc58a3 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchCacheSvc.java @@ -0,0 +1,81 @@ +package ca.uhn.fhir.jpa.search.cache; + +import ca.uhn.fhir.jpa.entity.Search; + +import java.util.Collection; +import java.util.Date; +import java.util.Optional; + +public interface ISearchCacheSvc { + + /** + * Places a new search of some sort in the cache, or updates an existing search. The search passed in is guaranteed to have + * a {@link Search#getUuid() UUID} so that is a good candidate for consistent identification. + * + * @param theSearch The search to store + * @return Returns a copy of the search as it was saved. Callers should use the returned Search object for any further processing. + */ + Search save(Search theSearch); + + /** + * Fetch a search using its UUID. The search should be fully loaded when it is returned (i.e. includes are fetched, so that access to its + * fields will not cause database errors if the current tranaction scope ends. + * + * @param theUuid The search UUID + * @return The search if it exists + */ + Optional fetchByUuid(String theUuid); + + /** + * TODO: this is perhaps an inappropriate responsibility for this service + * + *

+ * This method marks a search as in progress, but should only allow exactly one call to do so across the cluster. This + * is done so that if two client threads request the next page at the exact same time (which is unlikely but not + * impossible) only one will actually proceed to load the next results and the other will just wait for them + * to arrive. + * + * @param theSearch The search to mark + * @return This method should return an empty optional if the search was not marked (meaning that another thread + * succeeded in marking it). If the search doesn't exist or some other error occurred, an exception will be thrown + * instead of {@link Optional#empty()} + */ + Optional tryToMarkSearchAsInProgress(Search theSearch); + + /** + * Look for any existing searches matching the given resource type and query string. + *

+ * This method is allowed to perofrm a "best effort" return, so it can return searches that don't match the query string exactly, or + * which have a created timestamp before theCreatedAfter date. The caller is responsible for removing + * any inappropriate Searches and picking the most relevant one. + *

+ * + * @param theResourceType The resource type of the search. Results MUST match this type + * @param theQueryString The query string. Results SHOULD match this type + * @param theQueryStringHash The query string hash. Results SHOULD match this type + * @param theCreatedAfter Results SHOULD not include any searches created before this cutoff timestamp + * @return A collection of candidate searches + */ + Collection findCandidatesForReuse(String theResourceType, String theQueryString, int theQueryStringHash, Date theCreatedAfter); + + /** + * Mark a search as having been "last used" at the given time. This method may (and probably should) be implemented + * to work asynchronously in order to avoid hammering the database if the search gets reused many times. + * + * @param theSearch The search + * @param theDate The "last returned" timestamp + */ + void updateSearchLastReturned(Search theSearch, Date theDate); + + /** + * This is mostly public for unit tests + */ + void flushLastUpdated(); + + /** + * This method will be called periodically to delete stale searches. Implementations are not required to do anything + * if they have some other mechanism for expiring stale results other than manually looking for them + * and deleting them. + */ + void pollForStaleSearchesAndDeleteThem(); +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchResultCacheSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchResultCacheSvc.java new file mode 100644 index 00000000000..13d2f255724 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/ISearchResultCacheSvc.java @@ -0,0 +1,31 @@ +package ca.uhn.fhir.jpa.search.cache; + +import ca.uhn.fhir.jpa.entity.Search; + +import java.util.List; + +public interface ISearchResultCacheSvc { + /** + * @param theSearch The search - This method is not required to persist any chances to the Search object, it is only provided here for identification + * @param thePreviouslyStoredResourcePids A list of resource PIDs that have previously been saved to this search + * @param theNewResourcePids A list of new resoure PIDs to add to this search (these ones have not been previously saved) + */ + void storeResults(Search theSearch, List thePreviouslyStoredResourcePids, List theNewResourcePids); + + /** + * Fetch a sunset of the search result IDs from the cache + * + * @param theSearch The search to fetch IDs for + * @param theFrom The starting index (inclusive) + * @param theTo The ending index (exclusive) + * @return A list of resource PIDs + */ + List fetchResultPids(Search theSearch, int theFrom, int theTo); + + /** + * Fetch all result PIDs for a given search with no particular order required + * @param theSearch + * @return + */ + List fetchAllResultPids(Search theSearch); +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java index cdba0d04b2d..2511af14770 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java @@ -61,7 +61,6 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.codesystems.ConceptSubsumptionOutcome; -import org.jetbrains.annotations.NotNull; import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; @@ -87,6 +86,7 @@ import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; import javax.persistence.TypedQuery; import javax.persistence.criteria.*; +import javax.validation.constraints.NotNull; import java.util.*; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu2.java index 1dc8a94b4a6..33f7d792b7d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu2.java @@ -26,7 +26,9 @@ import org.hl7.fhir.instance.hapi.validation.IValidationSupport; import org.hl7.fhir.instance.model.api.IBaseDatatype; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.*; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.ConceptMap; +import org.hl7.fhir.r4.model.ValueSet; import org.springframework.beans.factory.annotation.Autowired; import java.util.ArrayList; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/IHapiTerminologySvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/IHapiTerminologySvc.java index f6b06b8b4e7..0b6c3572f6b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/IHapiTerminologySvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/IHapiTerminologySvc.java @@ -6,7 +6,9 @@ import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.rest.api.server.RequestDetails; import org.hl7.fhir.instance.model.api.*; -import org.hl7.fhir.r4.model.*; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.ConceptMap; +import org.hl7.fhir.r4.model.ValueSet; import javax.annotation.Nullable; import java.util.List; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TerminologyLoaderSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TerminologyLoaderSvcImpl.java index f719c98b37d..31bff96384a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TerminologyLoaderSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TerminologyLoaderSvcImpl.java @@ -34,9 +34,9 @@ import org.hl7.fhir.r4.model.CodeSystem; import org.hl7.fhir.r4.model.ConceptMap; import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.ValueSet; -import org.jetbrains.annotations.NotNull; import org.springframework.beans.factory.annotation.Autowired; +import javax.validation.constraints.NotNull; import java.io.*; import java.util.*; import java.util.Map.Entry; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java index 597a012ab4c..248c7cd7435 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java @@ -5,23 +5,25 @@ import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.interceptor.executor.InterceptorService; import ca.uhn.fhir.jpa.entity.TermConcept; +import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; +import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.jpa.util.ExpungeOptions; -import ca.uhn.fhir.jpa.model.util.JpaConstants; -import ca.uhn.fhir.test.utilities.LoggingRule; import ca.uhn.fhir.model.dstu2.resource.Bundle; import ca.uhn.fhir.model.dstu2.resource.Bundle.Entry; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.test.utilities.LoggingRule; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.TestUtil; @@ -91,6 +93,10 @@ public abstract class BaseJpaTest { protected IInterceptorService myInterceptorRegistry; @Autowired protected CircularQueueCaptureQueriesListener myCaptureQueriesListener; + @Autowired + protected ISearchResultCacheSvc mySearchResultCacheSvc; + @Autowired + protected ISearchCacheSvc mySearchCacheSvc; @After public void afterPerformCleanup() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index edd63ceb41a..a21515375f6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -206,8 +206,6 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired(required = false) protected IFulltextSearchSvc mySearchDao; @Autowired - protected ISearchDao mySearchEntityDao; - @Autowired @Qualifier("mySearchParameterDaoDstu3") protected IFhirResourceDao mySearchParameterDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchPageExpiryTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchPageExpiryTest.java deleted file mode 100644 index 779e8175506..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchPageExpiryTest.java +++ /dev/null @@ -1,401 +0,0 @@ -package ca.uhn.fhir.jpa.dao.dstu3; - -import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.r4.FhirResourceDaoR4SearchPageExpiryTest; -import ca.uhn.fhir.jpa.entity.Search; -import ca.uhn.fhir.jpa.search.StaleSearchDeletingSvcImpl; -import ca.uhn.fhir.jpa.util.TestUtil; -import ca.uhn.fhir.util.StopWatch; -import ca.uhn.fhir.rest.api.server.IBundleProvider; -import ca.uhn.fhir.rest.param.StringParam; -import org.apache.commons.lang3.Validate; -import org.apache.commons.lang3.time.DateUtils; -import org.hl7.fhir.dstu3.model.InstantType; -import org.hl7.fhir.dstu3.model.Patient; -import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.lang.Nullable; -import org.springframework.test.util.AopTestUtils; -import org.springframework.transaction.TransactionStatus; -import org.springframework.transaction.support.TransactionCallback; -import org.springframework.transaction.support.TransactionCallbackWithoutResult; -import org.springframework.transaction.support.TransactionTemplate; - -import java.util.Date; -import java.util.concurrent.atomic.AtomicLong; - -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.junit.Assert.*; - -public class FhirResourceDaoDstu3SearchPageExpiryTest extends BaseJpaDstu3Test { - private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoDstu3SearchPageExpiryTest.class); - - @After() - public void after() { - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_CUTOFF_SLACK); - StaleSearchDeletingSvcImpl.setNowForUnitTests(null); - - myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); - myDaoConfig.setExpireSearchResults(new DaoConfig().isExpireSearchResults()); - } - - @Before - public void before() { - myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); - myDaoConfig.setExpireSearchResults(new DaoConfig().isExpireSearchResults()); - myDaoConfig.setCountSearchResultsUpTo(null); - - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(0); - } - - @Before - public void beforeDisableResultReuse() { - myDaoConfig.setReuseCachedSearchResultsForMillis(null); - } - - @Test - public void testExpirePagesAfterReuse() throws Exception { - IIdType pid1; - IIdType pid2; - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid1 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid2 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - - myDaoConfig.setExpireSearchResultsAfterMillis(1000L); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); - long start = System.currentTimeMillis(); - - final String searchUuid1; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid1 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid1); - } - - waitForSearchToSave(searchUuid1); - - final String searchUuid2; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid2 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid2); - } - assertEquals(searchUuid1, searchUuid2); - - TestUtil.sleepAtLeast(501); - // We're now past 500ms so we shouldn't reuse the search - - final String searchUuid3; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid3 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid3); - } - assertNotEquals(searchUuid1, searchUuid3); - - // Search just got used so it shouldn't be deleted - - StaleSearchDeletingSvcImpl.setNowForUnitTests(start + 500); - final AtomicLong search3timestamp = new AtomicLong(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search3 = mySearchEntityDao.findByUuid(searchUuid3); - assertNotNull(search3); - Search search2 = mySearchEntityDao.findByUuid(searchUuid2); - assertNotNull(search2); - search3timestamp.set(search2.getSearchLastReturned().getTime()); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid3)); - } - }); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid1)); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNotNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); - } - }); - - } - - @Test - public void testExpirePagesAfterSingleUse() throws Exception { - IIdType pid1; - IIdType pid2; - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid1 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid2 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - - final StopWatch sw = new StopWatch(); - - SearchParameterMap params; - params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - - waitForSearchToSave(bundleProvider.getUuid()); - final AtomicLong start = new AtomicLong(); - - TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); - txTemplate.execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); - assertNotNull("Failed after " + sw.toString(), search); - start.set(search.getCreated().getTime()); - ourLog.info("Search was created: {}", new InstantType(new Date(start.get()))); - } - }); - - myDaoConfig.setExpireSearchResultsAfterMillis(500); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); - StaleSearchDeletingSvcImpl.setNowForUnitTests(start.get() + 499); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - txTemplate.execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(bundleProvider.getUuid())); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(start.get() + 600); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - txTemplate.execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull(mySearchEntityDao.findByUuid(bundleProvider.getUuid())); - } - }); - } - - @Test - public void testExpirePagesAfterSingleUse2() throws Exception { - IIdType pid1; - IIdType pid2; - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid1 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - pid2 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - Thread.sleep(10); - - myDaoConfig.setExpireSearchResultsAfterMillis(1000L); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); - long start = System.currentTimeMillis(); - - final String searchUuid1; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid1 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid1); - } - - String searchUuid2; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid2 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid2); - } - assertEquals(searchUuid1, searchUuid2); - - TestUtil.sleepAtLeast(501); - - // We're now past 500ms so we shouldn't reuse the search - - final String searchUuid3; - { - SearchParameterMap params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - final IBundleProvider bundleProvider = myPatientDao.search(params); - assertThat(toUnqualifiedVersionlessIds(bundleProvider), containsInAnyOrder(pid1, pid2)); - searchUuid3 = bundleProvider.getUuid(); - Validate.notBlank(searchUuid3); - } - assertNotEquals(searchUuid1, searchUuid3); - - // Search just got used so it shouldn't be deleted - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - final AtomicLong search3timestamp = new AtomicLong(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search3 = mySearchEntityDao.findByUuid(searchUuid3); - assertNotNull(search3); - search3timestamp.set(search3.getCreated().getTime()); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid3)); - } - }); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull(mySearchEntityDao.findByUuid(searchUuid1)); - } - }); - - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); - } - }); - - } - - @Test - public void testSearchPagesExpiryDisabled() throws Exception { - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - { - Patient patient = new Patient(); - patient.addName().setFamily("EXPIRE"); - myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); - } - - SearchParameterMap params; - params = new SearchParameterMap(); - params.add(Patient.SP_FAMILY, new StringParam("EXPIRE")); - params.setCount(1); - final IBundleProvider bundleProvider = myPatientDao.search(params); - - Search search = null; - for (int i = 0; i < 100 && search == null; i++) { - search = newTxTemplate().execute(new TransactionCallback() { - @Nullable - @Override - public Search doInTransaction(TransactionStatus status) { - return mySearchEntityDao.findByUuid(bundleProvider.getUuid()); - } - }); - if (search == null) { - TestUtil.sleepAtLeast(100); - } - } - assertNotNull("Search " + bundleProvider.getUuid() + " not found on disk after 10 seconds", search); - - - myDaoConfig.setExpireSearchResults(false); - StaleSearchDeletingSvcImpl.setNowForUnitTests(System.currentTimeMillis() + DateUtils.MILLIS_PER_DAY); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); - assertNotNull(search); - } - }); - - myDaoConfig.setExpireSearchResults(true); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); - assertNull(search); - } - }); - - } - - private void waitForSearchToSave(final String theUuid) { - final ISearchDao searchEntityDao = mySearchEntityDao; - TransactionTemplate txTemplate = newTxTemplate(); - FhirResourceDaoR4SearchPageExpiryTest.waitForSearchToSave(theUuid, searchEntityDao, txTemplate); - } -} 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 d7d26c21eb3..d872b64fc6e 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 @@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.interceptor.api.IInterceptorService; +import ca.uhn.fhir.jpa.binstore.BinaryAccessProvider; import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.*; @@ -11,7 +12,6 @@ import ca.uhn.fhir.jpa.interceptor.PerformanceTracingLoggingInterceptor; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.model.entity.ResourceTable; -import ca.uhn.fhir.jpa.binstore.BinaryAccessProvider; import ca.uhn.fhir.jpa.provider.r4.JpaSystemProviderR4; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; @@ -21,7 +21,6 @@ import ca.uhn.fhir.jpa.search.warm.ICacheWarmingSvc; import ca.uhn.fhir.jpa.searchparam.registry.BaseSearchParamRegistry; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionRegistry; import ca.uhn.fhir.jpa.term.BaseHapiTerminologySvcImpl; -import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.term.IHapiTerminologySvcR4; import ca.uhn.fhir.jpa.util.ResourceCountCache; import ca.uhn.fhir.jpa.util.ResourceProviderFactory; @@ -57,7 +56,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.PlatformTransactionManager; @@ -251,12 +249,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { @Autowired(required = false) protected IFulltextSearchSvc mySearchDao; @Autowired - protected ISearchDao mySearchEntityDao; - @Autowired - protected ISearchResultDao mySearchResultDao; - @Autowired - protected ISearchIncludeDao mySearchIncludeDao; - @Autowired protected IResourceReindexJobDao myResourceReindexJobDao; @Autowired @Qualifier("mySearchParameterDaoR4") diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 6c9c0cd571a..7c18e672068 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; @@ -55,6 +56,8 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { @Autowired MatchUrlService myMatchUrlService; + @Autowired + private ISearchDao mySearchEntityDao; @After public void afterResetSearchSize() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java index f7257a82f63..d38c66269b5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoHashesTest.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.dao.r4; -import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap.EverythingModeEnum; @@ -30,6 +30,7 @@ import org.hl7.fhir.r4.model.Observation.ObservationStatus; import org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType; import org.hl7.fhir.r4.model.Subscription.SubscriptionStatus; import org.junit.*; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionCallbackWithoutResult; @@ -49,6 +50,8 @@ import static org.mockito.Mockito.mock; @SuppressWarnings({"unchecked", "Duplicates"}) public class FhirResourceDaoR4SearchNoHashesTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchNoHashesTest.class); + @Autowired + private ISearchDao mySearchEntityDao; @After public void afterResetSearchSize() { 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 de55c65a11b..6d0931d484d 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 @@ -1,6 +1,8 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; @@ -14,6 +16,7 @@ import ca.uhn.fhir.rest.param.ReferenceOrListParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.TestUtil; import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.IAnyResource; @@ -24,6 +27,7 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.springframework.aop.framework.AopProxyUtils; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.scheduling.concurrent.ThreadPoolExecutorFactoryBean; import org.springframework.test.context.TestPropertySource; @@ -48,6 +52,10 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4SearchOptimizedTest.class); private SearchCoordinatorSvcImpl mySearchCoordinatorSvcImpl; + @Autowired + private ISearchDao mySearchEntityDao; + @Autowired + private ISearchResultDao mySearchResultDao; @Before public void before() { @@ -288,7 +296,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(200, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertNull(search.getTotalCount()); @@ -307,7 +315,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search gets incremented twice as a part of loading the next batch */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(SearchStatusEnum.FINISHED, search.getStatus()); assertEquals(200, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); @@ -343,7 +351,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(20, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertNull(search.getTotalCount()); @@ -367,7 +375,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search should be untouched */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(1, search.getVersion().intValue()); }); @@ -383,7 +391,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search gets incremented twice as a part of loading the next batch */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(SearchStatusEnum.PASSCMPLET, search.getStatus()); assertEquals(50, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); @@ -407,7 +415,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search should be untouched */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(3, search.getVersion().intValue()); }); @@ -423,7 +431,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search gets incremented twice as a part of loading the next batch */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(190, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertEquals(190, search.getTotalCount().intValue()); @@ -470,7 +478,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(50, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertEquals(null, search.getTotalCount()); @@ -505,7 +513,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(20, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertNull(search.getTotalCount()); @@ -529,7 +537,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * Search should be untouched */ runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(200, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertEquals(200, search.getTotalCount().intValue()); @@ -562,9 +570,9 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { * 20 should be prefetched since that's the initial page size */ - waitForSize(20, () -> runInTransaction(() -> mySearchEntityDao.findByUuid(uuid).getNumFound())); + waitForSize(20, () -> runInTransaction(() -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")).getNumFound())); runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(20, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); assertNull(search.getTotalCount()); @@ -625,7 +633,7 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test { assertEquals(1, ids.size()); runInTransaction(() -> { - Search search = mySearchEntityDao.findByUuid(uuid); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(uuid).orElseThrow(() -> new InternalErrorException("")); assertEquals(SearchStatusEnum.FINISHED, search.getStatus()); assertEquals(1, search.getNumFound()); assertEquals(search.getNumFound(), mySearchResultDao.count()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java index 17b3bf93c4a..c14e457e9d6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchPageExpiryTest.java @@ -1,15 +1,16 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; -import ca.uhn.fhir.jpa.search.StaleSearchDeletingSvcImpl; +import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.TestUtil; -import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.util.StopWatch; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.instance.model.api.IIdType; @@ -20,6 +21,7 @@ import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; @@ -30,6 +32,7 @@ import javax.annotation.Nullable; import java.util.Date; import java.util.concurrent.atomic.AtomicLong; +import static ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.DEFAULT_CUTOFF_SLACK; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.*; @@ -37,16 +40,19 @@ import static org.junit.Assert.*; public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4SearchPageExpiryTest.class); + @Autowired + private ISearchDao mySearchEntityDao; + @After() public void after() { - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_CUTOFF_SLACK); - StaleSearchDeletingSvcImpl.setNowForUnitTests(null); + DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); + staleSearchDeletingSvc.setCutoffSlackForUnitTest(DEFAULT_CUTOFF_SLACK); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(null); } @Before public void before() { - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); + DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); staleSearchDeletingSvc.setCutoffSlackForUnitTest(0); myDaoConfig.setCountSearchResultsUpTo(new DaoConfig().getCountSearchResultsUpTo()); } @@ -77,7 +83,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { myDaoConfig.setExpireSearchResultsAfterMillis(1000L); myDaoConfig.setReuseCachedSearchResultsForMillis(500L); long start = System.currentTimeMillis(); - StaleSearchDeletingSvcImpl.setNowForUnitTests(start); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start); final String searchUuid1; { @@ -117,53 +123,53 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { // Search just got used so it shouldn't be deleted - StaleSearchDeletingSvcImpl.setNowForUnitTests(start + 500); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start + 500); final AtomicLong search3timestamp = new AtomicLong(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search3 = mySearchEntityDao.findByUuid(searchUuid3); + Search search3 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search3); - Search search2 = mySearchEntityDao.findByUuid(searchUuid2); + Search search2 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid2).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search2); search3timestamp.set(search2.getSearchLastReturned().getTime()); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid3)); + assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3)); } }); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid1)); + assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1)); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNotNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); + assertFalse("Search 1 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); + assertTrue("Search 3 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent()); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); + assertFalse("Search 1 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); + assertFalse("Search 3 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent()); } }); @@ -202,7 +208,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { txTemplate.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid()).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull("Failed after " + sw.toString(), search); start.set(search.getCreated().getTime()); ourLog.info("Search was created: {}", new InstantType(new Date(start.get()))); @@ -211,21 +217,21 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { myDaoConfig.setExpireSearchResultsAfterMillis(500); myDaoConfig.setReuseCachedSearchResultsForMillis(500L); - StaleSearchDeletingSvcImpl.setNowForUnitTests(start.get() + 499); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + 499); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); txTemplate.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(bundleProvider.getUuid())); + assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid())); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(start.get() + 600); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + 600); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); txTemplate.execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull(mySearchEntityDao.findByUuid(bundleProvider.getUuid())); + assertFalse(mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid()).isPresent()); } }); } @@ -296,36 +302,36 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search3 = mySearchEntityDao.findByUuid(searchUuid3); + Search search3 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search3); search3timestamp.set(search3.getCreated().getTime()); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuid(searchUuid3)); + assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3)); } }); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull(mySearchEntityDao.findByUuid(searchUuid1)); + assertFalse(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); } }); - StaleSearchDeletingSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNull("Search 1 still exists", mySearchEntityDao.findByUuid(searchUuid1)); - assertNull("Search 3 still exists", mySearchEntityDao.findByUuid(searchUuid3)); + assertFalse("Search 1 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); + assertFalse("Search 3 still exists", mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent()); } }); @@ -356,7 +362,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { @Nullable @Override public Search doInTransaction(TransactionStatus status) { - return mySearchEntityDao.findByUuid(bundleProvider.getUuid()); + return mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid()).orElse(null); } }); if (search == null) { @@ -367,13 +373,13 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { myDaoConfig.setExpireSearchResults(false); - StaleSearchDeletingSvcImpl.setNowForUnitTests(System.currentTimeMillis() + DateUtils.MILLIS_PER_DAY); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(System.currentTimeMillis() + DateUtils.MILLIS_PER_DAY); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid()).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search); } }); @@ -386,7 +392,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search = mySearchEntityDao.findByUuid(bundleProvider.getUuid()); + Search search = mySearchEntityDao.findByUuidAndFetchIncludes(bundleProvider.getUuid()).orElse(null); assertNull(search); } }); @@ -405,7 +411,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { protected void doInTransactionWithoutResult(TransactionStatus theArg0) { Search search = null; for (int i = 0; i < 20 && search == null; i++) { - search = theSearchEntityDao.findByUuid(theUuid); + search = theSearchEntityDao.findByUuidAndFetchIncludes(theUuid).orElse(null); if (search == null || search.getStatus() == SearchStatusEnum.LOADING) { TestUtil.sleepAtLeast(100); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java index f709df0beb8..64e942ed78f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java @@ -3860,7 +3860,7 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test { search.setStatus(SearchStatusEnum.FAILED); search.setFailureCode(500); search.setFailureMessage("FOO"); - mySearchEntityDao.save(search); + mySearchCacheSvc.save(search); }); IBundleProvider results = myEncounterDao.search(map); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java index b4c44fa3eb7..15c56df0035 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java @@ -249,12 +249,6 @@ public abstract class BaseJpaR5Test extends BaseJpaTest { @Autowired(required = false) protected IFulltextSearchSvc mySearchDao; @Autowired - protected ISearchDao mySearchEntityDao; - @Autowired - protected ISearchResultDao mySearchResultDao; - @Autowired - protected ISearchIncludeDao mySearchIncludeDao; - @Autowired protected IResourceReindexJobDao myResourceReindexJobDao; @Autowired @Qualifier("mySearchParameterDaoR5") diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java index e386a885eac..93ce55c988d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/BaseResourceProviderDstu2Test.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.test.utilities.JettyUtil; import ca.uhn.fhir.util.TestUtil; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -36,8 +37,6 @@ import java.util.concurrent.TimeUnit; import static org.apache.commons.lang3.StringUtils.isNotBlank; -import ca.uhn.fhir.test.utilities.JettyUtil; - public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { protected static IGenericClient ourClient; @@ -61,12 +60,12 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { myFhirCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.ONCE); } - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({"unchecked", "rawtypes"}) @Before public void before() throws Exception { myFhirCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); myFhirCtx.getRestfulClientFactory().setSocketTimeout(1200 * 1000); - + if (ourServer == null) { ourRestServer = new RestfulServer(myFhirCtx); ourRestServer.registerProviders(myResourceProviders.createProviders()); @@ -82,7 +81,7 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { ourConnectionPoolSize = myAppCtx.getBean("maxDatabaseThreadsForTest", Integer.class); ourRestServer.setPagingProvider(ourPagingProvider); - Server server = new Server(ourPort); + Server server = new Server(0); ServletContextHandler proxyHandler = new ServletContextHandler(); proxyHandler.setContextPath("/"); @@ -103,16 +102,14 @@ public abstract class BaseResourceProviderDstu2Test extends BaseJpaDstu2Test { dispatcherServlet.setContextClass(AnnotationConfigWebApplicationContext.class); ServletHolder subsServletHolder = new ServletHolder(); subsServletHolder.setServlet(dispatcherServlet); - subsServletHolder.setInitParameter( - ContextLoader.CONFIG_LOCATION_PARAM, - WebsocketDispatcherConfig.class.getName()); + subsServletHolder.setInitParameter(ContextLoader.CONFIG_LOCATION_PARAM, WebsocketDispatcherConfig.class.getName()); proxyHandler.addServlet(subsServletHolder, "/*"); server.setHandler(proxyHandler); JettyUtil.startServer(server); - ourPort = JettyUtil.getPortForStartedServer(server); - ourServerBase = "http://localhost:" + ourPort + "/fhir/context"; + ourPort = JettyUtil.getPortForStartedServer(server); + ourServerBase = "http://localhost:" + ourPort + "/fhir/context"; ourClient = myFhirCtx.newRestfulGenericClient(ourServerBase); ourClient.registerInterceptor(new LoggingInterceptor()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java index a0a0cdfa278..891be247054 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/BaseResourceProviderDstu3Test.java @@ -1,7 +1,6 @@ package ca.uhn.fhir.jpa.provider.dstu3; import ca.uhn.fhir.jpa.config.WebsocketDispatcherConfig; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.dao.dstu3.BaseJpaDstu3Test; import ca.uhn.fhir.jpa.provider.GraphQLProvider; import ca.uhn.fhir.jpa.provider.SubscriptionTriggeringProvider; @@ -18,6 +17,7 @@ import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.interceptor.CorsInterceptor; +import ca.uhn.fhir.test.utilities.JettyUtil; import ca.uhn.fhir.util.TestUtil; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -48,8 +48,6 @@ import java.util.concurrent.TimeUnit; import static org.apache.commons.lang3.StringUtils.isNotBlank; -import ca.uhn.fhir.test.utilities.JettyUtil; - public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { @@ -62,7 +60,6 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { protected static GenericWebApplicationContext ourWebApplicationContext; protected static SearchParamRegistryDstu3 ourSearchParamRegistry; protected static DatabaseBackedPagingProvider ourPagingProvider; - protected static ISearchDao mySearchEntityDao; protected static ISearchCoordinatorSvc ourSearchCoordinatorSvc; private static Server ourServer; protected static SubscriptionTriggeringProvider ourSubscriptionTriggeringProvider; @@ -156,7 +153,6 @@ public abstract class BaseResourceProviderDstu3Test extends BaseJpaDstu3Test { WebApplicationContext wac = WebApplicationContextUtils.getWebApplicationContext(subsServletHolder.getServlet().getServletConfig().getServletContext()); myValidationSupport = wac.getBean(JpaValidationSupportChainDstu3.class); ourSearchCoordinatorSvc = wac.getBean(ISearchCoordinatorSvc.class); - mySearchEntityDao = wac.getBean(ISearchDao.class); ourSearchParamRegistry = wac.getBean(SearchParamRegistryDstu3.class); ourSubscriptionTriggeringProvider = wac.getBean(SubscriptionTriggeringProvider.class); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 0b6ea5d58b7..91d6fa55695 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.provider.dstu3; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; @@ -18,10 +19,7 @@ import ca.uhn.fhir.rest.client.api.IHttpRequest; import ca.uhn.fhir.rest.client.api.IHttpResponse; import ca.uhn.fhir.rest.gclient.StringClientParam; import ca.uhn.fhir.rest.param.*; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; -import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; -import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.util.UrlUtil; @@ -51,7 +49,11 @@ import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; 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.junit.*; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Ignore; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; @@ -74,6 +76,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderDstu3Test.class); private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; + @Autowired + private ISearchDao mySearchEntityDao; @Override @After @@ -2969,12 +2973,13 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .count(5) .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); Search search1 = newTxTemplate().execute(new TransactionCallback() { @Override public Search doInTransaction(TransactionStatus theStatus) { - return mySearchEntityDao.findByUuid(uuid1); + return mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException("")); } }); Date lastReturned1 = search1.getSearchLastReturned(); @@ -2986,12 +2991,13 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .count(5) .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); Search search2 = newTxTemplate().execute(new TransactionCallback() { @Override public Search doInTransaction(TransactionStatus theStatus) { - return mySearchEntityDao.findByUuid(uuid2); + return mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException("")); } }); Date lastReturned2 = search2.getSearchLastReturned(); @@ -3031,14 +3037,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); - Search search1 = newTxTemplate().execute(new TransactionCallback() { - @Override - public Search doInTransaction(TransactionStatus theStatus) { - return mySearchEntityDao.findByUuid(uuid1); - } - }); + Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException(""))); Date lastReturned1 = search1.getSearchLastReturned(); Bundle result2 = ourClient @@ -3046,14 +3048,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); - Search search2 = newTxTemplate().execute(new TransactionCallback() { - @Override - public Search doInTransaction(TransactionStatus theStatus) { - return mySearchEntityDao.findByUuid(uuid2); - } - }); + Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException(""))); Date lastReturned2 = search2.getSearchLastReturned(); assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/StaleSearchDeletingSvcDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/StaleSearchDeletingSvcDstu3Test.java deleted file mode 100644 index 2561e0841af..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/StaleSearchDeletingSvcDstu3Test.java +++ /dev/null @@ -1,97 +0,0 @@ -package ca.uhn.fhir.jpa.provider.dstu3; - -import static org.hamcrest.Matchers.blankOrNullString; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; - -import org.hl7.fhir.dstu3.model.Bundle; -import org.hl7.fhir.dstu3.model.Bundle.BundleLinkComponent; -import org.hl7.fhir.dstu3.model.Patient; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Test; -import org.springframework.test.util.AopTestUtils; - -import ca.uhn.fhir.jpa.search.StaleSearchDeletingSvcImpl; -import ca.uhn.fhir.rest.gclient.IClientExecutable; -import ca.uhn.fhir.rest.gclient.IQuery; -import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; -import ca.uhn.fhir.util.TestUtil; - -public class StaleSearchDeletingSvcDstu3Test extends BaseResourceProviderDstu3Test { - - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(StaleSearchDeletingSvcDstu3Test.class); - - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - - - @After() - public void after() throws Exception { - super.after(); - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_CUTOFF_SLACK); - } - - @Before - public void before() throws Exception { - super.before(); - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(0); - } - - @Test - public void testEverythingInstanceWithContentFilter() throws Exception { - - for (int i = 0; i < 20; i++) { - Patient pt1 = new Patient(); - pt1.addName().setFamily("Everything").addGiven("Arthur"); - myPatientDao.create(pt1, mySrd).getId().toUnqualifiedVersionless(); - } - - //@formatter:off - IClientExecutable, Bundle> search = ourClient - .search() - .forResource(Patient.class) - .where(Patient.NAME.matches().value("Everything")) - .returnBundle(Bundle.class); - //@formatter:on - - Bundle resp1 = search.execute(); - - for (int i = 0; i < 20; i++) { - search.execute(); - } - - BundleLinkComponent nextLink = resp1.getLink("next"); - assertNotNull(nextLink); - String nextLinkUrl = nextLink.getUrl(); - assertThat(nextLinkUrl, not(blankOrNullString())); - - Bundle resp2 = ourClient.search().byUrl(nextLinkUrl).returnBundle(Bundle.class).execute(); - ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp2)); - - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - - ourClient.search().byUrl(nextLinkUrl).returnBundle(Bundle.class).execute(); - - Thread.sleep(20); - myDaoConfig.setExpireSearchResultsAfterMillis(10); - myDaoConfig.setReuseCachedSearchResultsForMillis(null); - myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - - try { - ourClient.search().byUrl(nextLinkUrl).returnBundle(Bundle.class).execute(); - fail(); - } catch (ResourceGoneException e) { - assertThat(e.getMessage(), containsString("does not exist and may have expired")); - } - } - -} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java index abdd8dffb15..7cdf48c75c6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java @@ -2,7 +2,6 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.jpa.config.WebsocketDispatcherConfig; import ca.uhn.fhir.jpa.dao.DaoRegistry; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test; import ca.uhn.fhir.jpa.provider.GraphQLProvider; import ca.uhn.fhir.jpa.provider.TerminologyUploaderProvider; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ExpungeR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ExpungeR4Test.java index 9bc035957ab..182b39366d5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ExpungeR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ExpungeR4Test.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.search.PersistedJpaSearchFirstPageBundleProvider; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.ExpungeOptions; @@ -21,6 +23,7 @@ import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import java.util.List; @@ -38,6 +41,10 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test { private IIdType myOneVersionObservationId; private IIdType myTwoVersionObservationId; private IIdType myDeletedObservationId; + @Autowired + private ISearchDao mySearchEntityDao; + @Autowired + private ISearchResultDao mySearchResultDao; @After public void afterDisableExpunge() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java index dd4eb173b2f..ca08514295b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.util.TestUtil; import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.rest.api.CacheControlDirective; @@ -13,9 +13,8 @@ import org.hl7.fhir.r4.model.Patient; import org.junit.After; import org.junit.AfterClass; import org.junit.Test; -import org.springframework.test.util.AopTestUtils; +import org.springframework.beans.factory.annotation.Autowired; -import java.io.IOException; import java.util.Date; import static org.hamcrest.Matchers.*; @@ -24,9 +23,9 @@ import static org.junit.Assert.*; public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderR4CacheTest.class); - private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; private CapturingInterceptor myCapturingInterceptor; + @Autowired + private ISearchDao mySearchEntityDao; @Override @After @@ -42,14 +41,13 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { public void before() throws Exception { super.before(); myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); - mySearchCoordinatorSvcRaw = AopTestUtils.getTargetObject(mySearchCoordinatorSvc); myCapturingInterceptor = new CapturingInterceptor(); ourClient.registerInterceptor(myCapturingInterceptor); } @Test - public void testCacheNoStore() throws IOException { + public void testCacheNoStore() { Patient pt1 = new Patient(); pt1.addName().setFamily("FAM"); @@ -84,7 +82,7 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { } @Test - public void testCacheNoStoreMaxResults() throws IOException { + public void testCacheNoStoreMaxResults() { for (int i = 0; i < 10; i++) { Patient pt1 = new Patient(); @@ -106,7 +104,7 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { } @Test - public void testCacheNoStoreMaxResultsWithIllegalValue() throws IOException { + public void testCacheNoStoreMaxResultsWithIllegalValue() { myDaoConfig.setCacheControlNoStoreMaxResultsUpperLimit(123); try { ourClient @@ -123,7 +121,7 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { } @Test - public void testCacheSuppressed() throws IOException { + public void testCacheSuppressed() { Patient pt1 = new Patient(); pt1.addName().setFamily("FAM"); @@ -152,7 +150,7 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { } @Test - public void testCacheUsedNormally() throws IOException { + public void testCacheUsedNormally() { Patient pt1 = new Patient(); pt1.addName().setFamily("FAM"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index 06b8eca19b9..44da70e8c45 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java @@ -2,10 +2,11 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable; -import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.jpa.model.util.JpaConstants; +import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.jpa.util.TestUtil; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.primitive.InstantDt; @@ -21,10 +22,7 @@ import ca.uhn.fhir.rest.client.api.IHttpResponse; import ca.uhn.fhir.rest.client.interceptor.CapturingInterceptor; import ca.uhn.fhir.rest.gclient.StringClientParam; import ca.uhn.fhir.rest.param.*; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; -import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; -import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.UrlUtil; @@ -54,6 +52,7 @@ import org.hl7.fhir.r4.model.Questionnaire.QuestionnaireItemType; import org.hl7.fhir.r4.model.Subscription.SubscriptionChannelType; import org.hl7.fhir.r4.model.Subscription.SubscriptionStatus; import org.junit.*; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallbackWithoutResult; @@ -82,6 +81,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { public static final int LARGE_NUMBER = 77; private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; private CapturingInterceptor myCapturingInterceptor = new CapturingInterceptor(); + @Autowired + private ISearchDao mySearchEntityDao; @Override @After @@ -3937,9 +3938,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .count(5) .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); - Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuid(uuid1)); + Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(()->new InternalErrorException(""))); Date lastReturned1 = search1.getSearchLastReturned(); Bundle result2 = ourClient @@ -3949,9 +3951,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .count(5) .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); - Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuid(uuid2)); + Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(()->new InternalErrorException(""))); Date lastReturned2 = search2.getSearchLastReturned(); assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); @@ -3965,6 +3968,7 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .count(5) .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); String uuid3 = toSearchUuidFromLinkNext(result3); @@ -3989,9 +3993,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); - Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuid(uuid1)); + Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(()->new InternalErrorException(""))); Date lastReturned1 = search1.getSearchLastReturned(); sleepOneClick(); @@ -4001,9 +4006,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); + mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); - Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuid(uuid2)); + Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(()->new InternalErrorException(""))); Date lastReturned2 = search2.getSearchLastReturned(); assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/StaleSearchDeletingSvcR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/StaleSearchDeletingSvcR4Test.java index 3a2441bb556..f2429ccf050 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/StaleSearchDeletingSvcR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/StaleSearchDeletingSvcR4Test.java @@ -1,9 +1,15 @@ package ca.uhn.fhir.jpa.provider.r4; -import ca.uhn.fhir.jpa.entity.*; +import ca.uhn.fhir.jpa.dao.data.ISearchDao; +import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; +import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; +import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.jpa.entity.SearchInclude; +import ca.uhn.fhir.jpa.entity.SearchResult; +import ca.uhn.fhir.jpa.entity.SearchTypeEnum; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; -import ca.uhn.fhir.jpa.search.StaleSearchDeletingSvcImpl; +import ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl; import ca.uhn.fhir.rest.gclient.IClientExecutable; import ca.uhn.fhir.rest.gclient.IQuery; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; @@ -16,6 +22,7 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.util.AopTestUtils; import java.util.Date; @@ -27,22 +34,28 @@ import static org.junit.Assert.*; public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(StaleSearchDeletingSvcR4Test.class); + @Autowired + private ISearchDao mySearchEntityDao; + @Autowired + private ISearchResultDao mySearchResultDao; + @Autowired + private ISearchIncludeDao mySearchIncludeDao; @Override @After() public void after() throws Exception { super.after(); - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_CUTOFF_SLACK); - StaleSearchDeletingSvcImpl.setMaximumResultsToDeleteForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT); - StaleSearchDeletingSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(StaleSearchDeletingSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS); + DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); + staleSearchDeletingSvc.setCutoffSlackForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_CUTOFF_SLACK); + DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT); + DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS); } @Override @Before public void before() throws Exception { super.before(); - StaleSearchDeletingSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(myStaleSearchDeletingSvc); + DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); staleSearchDeletingSvc.setCutoffSlackForUnitTest(0); } @@ -96,8 +109,8 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { @Test public void testDeleteVeryLargeSearch() { - StaleSearchDeletingSvcImpl.setMaximumResultsToDeleteForUnitTest(10); - StaleSearchDeletingSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(10); + DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(10); + DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(10); runInTransaction(() -> { Search search = new Search(); @@ -139,7 +152,7 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { @Test public void testDeleteVerySmallSearch() { - StaleSearchDeletingSvcImpl.setMaximumResultsToDeleteForUnitTest(10); + DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(10); runInTransaction(() -> { Search search = new Search(); @@ -149,8 +162,7 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { search.setSearchType(SearchTypeEnum.SEARCH); search.setResourceType("Patient"); search.setSearchLastReturned(DateUtils.addDays(new Date(), -10000)); - search = mySearchEntityDao.save(search); - + mySearchEntityDao.save(search); }); // It should take one pass to delete the search fully diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/BaseResourceProviderR5Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/BaseResourceProviderR5Test.java index 5ddb411c440..534899c45cb 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/BaseResourceProviderR5Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/BaseResourceProviderR5Test.java @@ -2,7 +2,6 @@ package ca.uhn.fhir.jpa.provider.r5; import ca.uhn.fhir.jpa.config.WebsocketDispatcherConfig; import ca.uhn.fhir.jpa.dao.DaoRegistry; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.dao.r5.BaseJpaR5Test; import ca.uhn.fhir.jpa.provider.GraphQLProvider; import ca.uhn.fhir.jpa.provider.TerminologyUploaderProvider; @@ -64,7 +63,6 @@ public abstract class BaseResourceProviderR5Test extends BaseJpaR5Test { protected static String ourServerBase; protected static SearchParamRegistryR5 ourSearchParamRegistry; private static DatabaseBackedPagingProvider ourPagingProvider; - protected static ISearchDao mySearchEntityDao; protected static ISearchCoordinatorSvc mySearchCoordinatorSvc; private static GenericWebApplicationContext ourWebApplicationContext; private static SubscriptionMatcherInterceptor ourSubscriptionMatcherInterceptor; @@ -166,7 +164,6 @@ public abstract class BaseResourceProviderR5Test extends BaseJpaR5Test { WebApplicationContext wac = WebApplicationContextUtils.getWebApplicationContext(subsServletHolder.getServlet().getServletConfig().getServletContext()); myValidationSupport = wac.getBean(JpaValidationSupportChainR5.class); mySearchCoordinatorSvc = wac.getBean(ISearchCoordinatorSvc.class); - mySearchEntityDao = wac.getBean(ISearchDao.class); ourSearchParamRegistry = wac.getBean(SearchParamRegistryR5.class); ourSubscriptionMatcherInterceptor = wac.getBean(SubscriptionMatcherInterceptor.class); ourSubscriptionMatcherInterceptor.start(); 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 3ae92ea1da6..02d94fba0ae 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 @@ -3,13 +3,11 @@ package ca.uhn.fhir.jpa.search; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.jpa.dao.*; -import ca.uhn.fhir.jpa.dao.data.ISearchDao; -import ca.uhn.fhir.jpa.dao.data.ISearchIncludeDao; -import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; import ca.uhn.fhir.jpa.entity.Search; -import ca.uhn.fhir.jpa.entity.SearchResult; import ca.uhn.fhir.jpa.entity.SearchTypeEnum; import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; +import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; +import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.BaseIterator; import ca.uhn.fhir.model.dstu2.resource.Patient; @@ -19,7 +17,6 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.TestUtil; -import com.google.common.collect.Lists; import org.hl7.fhir.instance.model.api.IBaseResource; import org.junit.After; import org.junit.AfterClass; @@ -34,7 +31,6 @@ import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionStatus; @@ -53,20 +49,18 @@ public class SearchCoordinatorSvcImplTest { private static final Logger ourLog = LoggerFactory.getLogger(SearchCoordinatorSvcImplTest.class); private static FhirContext ourCtx = FhirContext.forDstu3(); @Captor - ArgumentCaptor> mySearchResultIterCaptor; + ArgumentCaptor> mySearchResultIterCaptor; @Mock private IFhirResourceDao myCallingDao; @Mock private EntityManager myEntityManager; private int myExpectedNumberOfSearchBuildersCreated = 2; @Mock - private ISearchBuilder mySearchBuider; + private ISearchBuilder mySearchBuilder; @Mock - private ISearchDao mySearchDao; + private ISearchCacheSvc mySearchCacheSvc; @Mock - private ISearchIncludeDao mySearchIncludeDao; - @Mock - private ISearchResultDao mySearchResultDao; + private ISearchResultCacheSvc mySearchResultCacheSvc; private SearchCoordinatorSvcImpl mySvc; @Mock private PlatformTransactionManager myTxManager; @@ -90,16 +84,14 @@ public class SearchCoordinatorSvcImplTest { mySvc.setEntityManagerForUnitTest(myEntityManager); mySvc.setTransactionManagerForUnitTest(myTxManager); mySvc.setContextForUnitTest(ourCtx); - mySvc.setSearchDaoForUnitTest(mySearchDao); - mySvc.setSearchDaoIncludeForUnitTest(mySearchIncludeDao); - mySvc.setSearchDaoResultForUnitTest(mySearchResultDao); + mySvc.setSearchCacheServicesForUnitTest(mySearchCacheSvc, mySearchResultCacheSvc); mySvc.setDaoRegistryForUnitTest(myDaoRegistry); mySvc.setInterceptorBroadcasterForUnitTest(myInterceptorBroadcaster); myDaoConfig = new DaoConfig(); mySvc.setDaoConfigForUnitTest(myDaoConfig); - when(myCallingDao.newSearchBuilder()).thenReturn(mySearchBuider); + when(myCallingDao.newSearchBuilder()).thenReturn(mySearchBuilder); when(myTxManager.getTransaction(any())).thenReturn(mock(TransactionStatus.class)); @@ -107,7 +99,7 @@ public class SearchCoordinatorSvcImplTest { PersistedJpaBundleProvider provider = (PersistedJpaBundleProvider) theInvocation.getArguments()[0]; provider.setSearchCoordinatorSvc(mySvc); provider.setPlatformTransactionManager(myTxManager); - provider.setSearchDao(mySearchDao); + provider.setSearchCacheSvc(mySearchCacheSvc); provider.setEntityManager(myEntityManager); provider.setContext(ourCtx); provider.setInterceptorBroadcaster(myInterceptorBroadcaster); @@ -143,7 +135,7 @@ public class SearchCoordinatorSvcImplTest { List pids = createPidSequence(10, 800); IResultIterator iter = new FailAfterNIterator(new SlowIterator(pids.iterator(), 2), 300); - when(mySearchBuider.createQuery(Mockito.same(params), any(), any())).thenReturn(iter); + when(mySearchBuilder.createQuery(Mockito.same(params), any(), any())).thenReturn(iter); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNotNull(result.getUuid()); @@ -159,23 +151,42 @@ public class SearchCoordinatorSvcImplTest { @Test public void testAsyncSearchLargeResultSetBigCountSameCoordinator() { + List allResults = new ArrayList<>(); + doAnswer(t->{ + List oldResults = t.getArgument(1, List.class); + List newResults = t.getArgument(2, List.class); + ourLog.info("Saving {} new results - have {} old results", newResults.size(), oldResults.size()); + assertEquals(allResults.size(), oldResults.size()); + allResults.addAll(newResults); + return null; + }).when(mySearchResultCacheSvc).storeResults(any(),anyList(),anyList()); + + SearchParameterMap params = new SearchParameterMap(); params.add("name", new StringParam("ANAME")); List pids = createPidSequence(10, 800); SlowIterator iter = new SlowIterator(pids.iterator(), 1); - when(mySearchBuider.createQuery(any(), any(), any())).thenReturn(iter); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + when(mySearchBuilder.createQuery(any(), any(), any())).thenReturn(iter); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); - when(mySearchResultDao.findWithSearchUuid(any(), any())).thenAnswer(t -> { + when(mySearchResultCacheSvc.fetchResultPids(any(), anyInt(), anyInt())).thenAnswer(t -> { List returnedValues = iter.getReturnedValues(); - Pageable page = (Pageable) t.getArguments()[1]; - int offset = (int) page.getOffset(); - int end = (int) (page.getOffset() + page.getPageSize()); + int offset = t.getArgument(1, Integer.class); + int end = t.getArgument(2, Integer.class); end = Math.min(end, returnedValues.size()); offset = Math.min(offset, returnedValues.size()); ourLog.info("findWithSearchUuid {} - {} out of {} values", offset, end, returnedValues.size()); - return new PageImpl<>(returnedValues.subList(offset, end)); + return returnedValues.subList(offset, end); + }); + + when(mySearchResultCacheSvc.fetchAllResultPids(any())).thenReturn(allResults); + + when(mySearchCacheSvc.tryToMarkSearchAsInProgress(any())).thenAnswer(t->{ + Search search = t.getArgument(0, Search.class); + assertEquals(SearchStatusEnum.PASSCMPLET, search.getStatus()); + search.setStatus(SearchStatusEnum.LOADING); + return Optional.of(search); }); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); @@ -184,12 +195,14 @@ public class SearchCoordinatorSvcImplTest { List resources; - when(mySearchDao.save(any())).thenAnswer(t -> { + when(mySearchCacheSvc.save(any())).thenAnswer(t -> { Search search = (Search) t.getArguments()[0]; myCurrentSearch = search; return search; }); - when(mySearchDao.findByUuid(any())).thenAnswer(t -> myCurrentSearch); + when(mySearchCacheSvc.fetchByUuid(any())).thenAnswer(t -> { + return Optional.ofNullable(myCurrentSearch); + }); IFhirResourceDao dao = myCallingDao; when(myDaoRegistry.getResourceDao(any(String.class))).thenReturn(dao); @@ -199,17 +212,11 @@ public class SearchCoordinatorSvcImplTest { assertEquals("799", resources.get(789).getIdElement().getValueAsString()); ArgumentCaptor searchCaptor = ArgumentCaptor.forClass(Search.class); - verify(mySearchDao, atLeastOnce()).save(searchCaptor.capture()); - - verify(mySearchResultDao, atLeastOnce()).saveAll(mySearchResultIterCaptor.capture()); - List allResults = new ArrayList<>(); - for (Iterable next : mySearchResultIterCaptor.getAllValues()) { - allResults.addAll(Lists.newArrayList(next)); - } + verify(mySearchCacheSvc, atLeastOnce()).save(searchCaptor.capture()); assertEquals(790, allResults.size()); - assertEquals(10, allResults.get(0).getResourcePid().longValue()); - assertEquals(799, allResults.get(789).getResourcePid().longValue()); + assertEquals(10, allResults.get(0).longValue()); + assertEquals(799, allResults.get(789).longValue()); myExpectedNumberOfSearchBuildersCreated = 4; } @@ -221,9 +228,9 @@ public class SearchCoordinatorSvcImplTest { List pids = createPidSequence(10, 800); SlowIterator iter = new SlowIterator(pids.iterator(), 2); - when(mySearchBuider.createQuery(same(params), any(), any())).thenReturn(iter); + when(mySearchBuilder.createQuery(same(params), any(), any())).thenReturn(iter); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNotNull(result.getUuid()); @@ -249,16 +256,16 @@ public class SearchCoordinatorSvcImplTest { List pids = createPidSequence(10, 800); IResultIterator iter = new SlowIterator(pids.iterator(), 2); - when(mySearchBuider.createQuery(same(params), any(), any())).thenReturn(iter); - when(mySearchDao.save(any())).thenAnswer(t -> t.getArguments()[0]); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + when(mySearchBuilder.createQuery(same(params), any(), any())).thenReturn(iter); + when(mySearchCacheSvc.save(any())).thenAnswer(t -> t.getArguments()[0]); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNotNull(result.getUuid()); assertEquals(null, result.size()); ArgumentCaptor searchCaptor = ArgumentCaptor.forClass(Search.class); - verify(mySearchDao, atLeast(1)).save(searchCaptor.capture()); + verify(mySearchCacheSvc, atLeast(1)).save(searchCaptor.capture()); Search search = searchCaptor.getValue(); assertEquals(SearchTypeEnum.SEARCH, search.getSearchType()); @@ -271,7 +278,7 @@ public class SearchCoordinatorSvcImplTest { assertEquals("10", resources.get(0).getIdElement().getValueAsString()); assertEquals("19", resources.get(9).getIdElement().getValueAsString()); - when(mySearchDao.findByUuid(eq(result.getUuid()))).thenReturn(search); + when(mySearchCacheSvc.fetchByUuid(eq(result.getUuid()))).thenReturn(Optional.of(search)); /* * Now call from a new bundle provider. This simulates a separate HTTP @@ -293,9 +300,9 @@ public class SearchCoordinatorSvcImplTest { List pids = createPidSequence(10, 100); SlowIterator iter = new SlowIterator(pids.iterator(), 2); - when(mySearchBuider.createQuery(same(params), any(), any())).thenReturn(iter); + when(mySearchBuilder.createQuery(same(params), any(), any())).thenReturn(iter); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNotNull(result.getUuid()); @@ -324,8 +331,8 @@ public class SearchCoordinatorSvcImplTest { search.setSearchType(SearchTypeEnum.SEARCH); search.setResourceType("Patient"); - when(mySearchDao.findByUuid(eq(uuid))).thenReturn(search); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + when(mySearchCacheSvc.fetchByUuid(eq(uuid))).thenReturn(Optional.of(search)); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); PersistedJpaBundleProvider provider; List resources; @@ -337,16 +344,13 @@ public class SearchCoordinatorSvcImplTest { // ignore } - when(mySearchResultDao.findWithSearchUuid(any(Search.class), any(Pageable.class))).thenAnswer(theInvocation -> { - Pageable page = (Pageable) theInvocation.getArguments()[1]; - + when(mySearchResultCacheSvc.fetchResultPids(any(Search.class), anyInt(), anyInt())).thenAnswer(theInvocation -> { ArrayList results = new ArrayList<>(); - int max = (page.getPageNumber() * page.getPageSize()) + page.getPageSize(); - for (long i = page.getOffset(); i < max; i++) { - results.add(i + 10L); - } + for (long i = theInvocation.getArgument(1, Integer.class); i < theInvocation.getArgument(2, Integer.class); i++) { + results.add(i + 10L); + } - return new PageImpl<>(results); + return results; }); search.setStatus(SearchStatusEnum.FINISHED); }).start(); @@ -377,9 +381,9 @@ public class SearchCoordinatorSvcImplTest { params.add("name", new StringParam("ANAME")); List pids = createPidSequence(10, 800); - when(mySearchBuider.createQuery(same(params), any(), any())).thenReturn(new ResultIterator(pids.iterator())); + when(mySearchBuilder.createQuery(same(params), any(), any())).thenReturn(new ResultIterator(pids.iterator())); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(any(Collection.class), any(Collection.class), any(List.class), anyBoolean(), any()); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNull(result.getUuid()); @@ -398,10 +402,10 @@ public class SearchCoordinatorSvcImplTest { params.add("name", new StringParam("ANAME")); List pids = createPidSequence(10, 800); - when(mySearchBuider.createQuery(Mockito.same(params), any(), nullable(RequestDetails.class))).thenReturn(new ResultIterator(pids.iterator())); + when(mySearchBuilder.createQuery(Mockito.same(params), any(), nullable(RequestDetails.class))).thenReturn(new ResultIterator(pids.iterator())); pids = createPidSequence(10, 110); - doAnswer(loadPids()).when(mySearchBuider).loadResourcesByPid(eq(pids), any(Collection.class), any(List.class), anyBoolean(), nullable(RequestDetails.class)); + doAnswer(loadPids()).when(mySearchBuilder).loadResourcesByPid(eq(pids), any(Collection.class), any(List.class), anyBoolean(), nullable(RequestDetails.class)); IBundleProvider result = mySvc.registerSearch(myCallingDao, params, "Patient", new CacheControlDirective(), null); assertNull(result.getUuid()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java index ffe45928795..cf9591cf8cb 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java @@ -20,16 +20,17 @@ import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.test.utilities.JettyUtil; import com.google.common.collect.Lists; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.jetbrains.annotations.NotNull; import org.junit.*; import org.springframework.beans.factory.annotation.Autowired; +import javax.validation.constraints.NotNull; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -38,8 +39,6 @@ import static ca.uhn.fhir.jpa.subscription.resthook.RestHookTestDstu3Test.logAll import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.*; -import ca.uhn.fhir.test.utilities.JettyUtil; - /** * Test the rest-hook subscriptions */ diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu3Test.java index 07b6b479d42..02fde230ef3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu3Test.java @@ -17,6 +17,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.test.utilities.JettyUtil; import com.google.common.collect.Lists; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; @@ -24,11 +25,11 @@ import org.eclipse.jetty.servlet.ServletHolder; import org.hl7.fhir.dstu3.model.*; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.jetbrains.annotations.NotNull; import org.junit.*; import org.springframework.beans.factory.annotation.Autowired; import javax.servlet.http.HttpServletRequest; +import javax.validation.constraints.NotNull; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -40,8 +41,6 @@ import static org.awaitility.Awaitility.await; import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.*; -import ca.uhn.fhir.test.utilities.JettyUtil; - /** * Test the rest-hook subscriptions */ diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java index 014de578bd4..32810675705 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java @@ -185,7 +185,11 @@ public class JdbcUtils { DatabaseMetaData metadata; try { metadata = connection.getMetaData(); - ResultSet indexes = metadata.getCrossReference(connection.getCatalog(), connection.getSchema(), massageIdentifier(metadata, theTableName), connection.getCatalog(), connection.getSchema(), massageIdentifier(metadata, theForeignTable)); + String catalog = connection.getCatalog(); + String schema = connection.getSchema(); + String parentTable = massageIdentifier(metadata, theTableName); + String foreignTable = massageIdentifier(metadata, theForeignTable); + ResultSet indexes = metadata.getCrossReference(catalog, schema, parentTable, catalog, schema, foreignTable); Set columnNames = new HashSet<>(); while (indexes.next()) { diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java index 3f68e64de3a..e53908edf9a 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java @@ -21,7 +21,6 @@ package ca.uhn.fhir.jpa.migrate.taskdef; */ import ca.uhn.fhir.jpa.migrate.JdbcUtils; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java new file mode 100644 index 00000000000..bea3f581096 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java @@ -0,0 +1,91 @@ +package ca.uhn.fhir.jpa.migrate.taskdef; + +/*- + * #%L + * HAPI FHIR JPA Server - Migration + * %% + * Copyright (C) 2014 - 2019 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.jpa.migrate.JdbcUtils; +import org.apache.commons.lang3.Validate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.sql.SQLException; +import java.util.Set; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; + +public class DropForeignKeyTask extends BaseTableTask { + + private static final Logger ourLog = LoggerFactory.getLogger(DropForeignKeyTask.class); + private String myConstraintName; + private String myForeignTableName; + + public void setConstraintName(String theConstraintName) { + myConstraintName = theConstraintName; + } + + public void setForeignTableName(String theForeignTableName) { + myForeignTableName = theForeignTableName; + } + + @Override + public void validate() { + super.validate(); + + Validate.isTrue(isNotBlank(myConstraintName)); + Validate.isTrue(isNotBlank(myForeignTableName)); + } + + @Override + public void execute() throws SQLException { + + Set existing = JdbcUtils.getForeignKeys(getConnectionProperties(), getTableName(), myForeignTableName); + if (!existing.contains(myConstraintName)) { + ourLog.info("Don't have constraint named {} - No action performed", myConstraintName); + return; + } + + String sql = null; + String sql2 = null; + switch (getDriverType()) { + case MYSQL_5_7: + // Lousy MYQL.... + sql = "alter table " + getTableName() + " drop constraint " + myConstraintName; + sql2 = "alter table " + getTableName() + " drop index " + myConstraintName; + break; + case MARIADB_10_1: + case POSTGRES_9_4: + case DERBY_EMBEDDED: + case H2_EMBEDDED: + case ORACLE_12C: + case MSSQL_2012: + sql = "alter table " + getTableName() + " drop constraint " + myConstraintName; + break; + default: + throw new IllegalStateException(); + } + + executeSql(getTableName(), sql); + if (isNotBlank(sql2)) { + executeSql(getTableName(), sql2); + } + + } + +} diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 851fbac8694..3f33971b378 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -87,6 +87,9 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { .addForeignKey("FK_TRM_VSCD_VS_PID") .toColumn("VALUESET_PID") .references("TRM_VALUESET", "PID"); + // Drop HFJ_SEARCH_RESULT foreign keys + version.onTable("HFJ_SEARCH_RESULT").dropForeignKey("FK_SEARCHRES_RES", "HFJ_RESOURCE"); + version.onTable("HFJ_SEARCH_RESULT").dropForeignKey("FK_SEARCHRES_SEARCH", "HFJ_SEARCH"); // TermValueSet version.startSectionWithMessage("Processing table: TRM_VALUESET"); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/BaseMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/BaseMigrationTasks.java index 6681a208cea..06faaa96c44 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/BaseMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/BaseMigrationTasks.java @@ -246,6 +246,19 @@ public class BaseMigrationTasks { return this; } + /** + * + * @param theFkName the name of the foreign key + * @param theForeignTableName the name of the table that imports the foreign key (I know it's a confusing name, but that's what java.sql.DatabaseMetaData calls it) + */ + public void dropForeignKey(String theFkName, String theForeignTableName) { + DropForeignKeyTask task = new DropForeignKeyTask(); + task.setConstraintName(theFkName); + task.setForeignTableName(theForeignTableName); + task.setTableName(getTableName()); + addTask(task); + } + public class BuilderAddIndexWithName { private final String myIndexName; diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java index 6e21947a2e7..0f78c9e1ad7 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIndexTest.java @@ -1,12 +1,12 @@ package ca.uhn.fhir.jpa.migrate.taskdef; import ca.uhn.fhir.jpa.migrate.JdbcUtils; -import com.google.common.collect.Lists; import org.junit.Test; import java.sql.SQLException; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.assertThat; public class AddIndexTest extends BaseTest { diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTaskTest.java new file mode 100644 index 00000000000..ae6d3b17f02 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTaskTest.java @@ -0,0 +1,38 @@ +package ca.uhn.fhir.jpa.migrate.taskdef; + +import ca.uhn.fhir.jpa.migrate.JdbcUtils; +import org.junit.Test; + +import java.sql.SQLException; + +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.junit.Assert.assertThat; + +public class DropForeignKeyTaskTest extends BaseTest { + + @Test + public void testDropForeignKey() throws SQLException { + executeSql("create table PARENT (PID bigint not null, TEXTCOL varchar(255), primary key (PID))"); + executeSql("create table CHILD (PID bigint not null, PARENTREF bigint)"); + executeSql("alter table CHILD add constraint FK_MOM foreign key (PARENTREF) references PARENT(PID)"); + + assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1)); + + DropForeignKeyTask task = new DropForeignKeyTask(); + task.setTableName("PARENT"); + task.setForeignTableName("CHILD"); + task.setConstraintName("FK_MOM"); + getMigrator().addTask(task); + + getMigrator().migrate(); + + assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), empty()); + + // Make sure additional calls don't crash + getMigrator().migrate(); + getMigrator().migrate(); + } + + +} diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorR5.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorR5.java index 8c665557624..c50383d8dd7 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorR5.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorR5.java @@ -21,12 +21,10 @@ package ca.uhn.fhir.jpa.searchparam.extractor; */ import ca.uhn.fhir.context.ConfigurationException; -import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.model.entity.*; import ca.uhn.fhir.jpa.model.util.StringNormalizer; import ca.uhn.fhir.jpa.searchparam.SearchParamConstants; -import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import com.google.common.annotations.VisibleForTesting; diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java index 4d6b44bef62..28936db1428 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java @@ -18,7 +18,10 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.messaging.*; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; +import org.springframework.messaging.MessageHandler; +import org.springframework.messaging.MessagingException; import org.springframework.stereotype.Service; import java.util.Collection; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f87492746f4..af872a8c8df 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -73,9 +73,13 @@ The informational message returned in an OperationOutcome when a delete failed due to cascades not being enabled contained an incorrect example. This has been corrected. + + Two foreign keys have been dropped from the HFJ_SEARCH_RESULT table used by the FHIR search query cache. These + constraints did not add value and caused unneccessary contention when used under high load. + An inefficient regex expression in UrlUtil was replaced with a much more efficient hand-written - checker. This regex was causing a noticable performance drop when feeding large numbers of transactions + checker. This regex was causing a noticable performance drop when feeding large numbers of transactions into the JPA server at the same time (i.e. when loading Synthea data).