From cda906d7b87659cc4384e2dac4a72fe4bd3d5e53 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 3 Feb 2020 09:44:10 -0500 Subject: [PATCH 1/3] Column removed. Time to run tests. --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 1 - .../ca/uhn/fhir/jpa/dao/data/ISearchDao.java | 7 +- .../java/ca/uhn/fhir/jpa/entity/Search.java | 15 ---- .../jpa/search/SearchCoordinatorSvcImpl.java | 3 - .../search/cache/BaseSearchCacheSvcImpl.java | 87 ------------------- .../cache/DatabaseSearchCacheSvcImpl.java | 11 +-- .../jpa/search/cache/ISearchCacheSvc.java | 14 --- ...FhirResourceDaoR4SearchPageExpiryTest.java | 2 +- .../dstu3/ResourceProviderDstu3Test.java | 16 ++-- .../provider/r4/ResourceProviderR4Test.java | 78 ++++------------- .../r4/StaleSearchDeletingSvcR4Test.java | 6 -- .../tasks/HapiFhirJpaMigrationTasks.java | 3 + 12 files changed, 29 insertions(+), 214 deletions(-) delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/BaseSearchCacheSvcImpl.java 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 785ea06a62a..fa4b228bb78 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 @@ -368,7 +368,6 @@ public abstract class BaseHapiFhirDao extends BaseStora Search search = new Search(); search.setDeleted(false); search.setCreated(new Date()); - search.setSearchLastReturned(new Date()); search.setLastUpdated(theSince, theUntil); search.setUuid(UUID.randomUUID().toString()); search.setResourceType(resourceName); 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 47c7d92cdf7..184ce87e2ee 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 @@ -1,7 +1,6 @@ package ca.uhn.fhir.jpa.dao.data; import ca.uhn.fhir.jpa.entity.Search; -import ca.uhn.fhir.jpa.model.search.SearchStatusEnum; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.jpa.repository.JpaRepository; @@ -38,16 +37,12 @@ public interface ISearchDao extends JpaRepository { @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) AND (s.myExpiryOrNull IS NULL OR s.myExpiryOrNull < :now)") + @Query("SELECT s.myId FROM Search s WHERE (s.myCreated < :cutoff) AND (s.myExpiryOrNull IS NULL OR s.myExpiryOrNull < :now)") Slice findWhereLastReturnedBefore(@Param("cutoff") Date theCutoff, @Param("now") Date theNow, Pageable thePage); @Query("SELECT s FROM Search s WHERE s.myResourceType = :type AND mySearchQueryStringHash = :hash AND (s.myCreated > :cutoff) AND s.myDeleted = false AND s.myStatus <> 'FAILED'") Collection findWithCutoffOrExpiry(@Param("type") String theResourceType, @Param("hash") int theHashCode, @Param("cutoff") Date theCreatedCutoff); - @Modifying - @Query("UPDATE Search s SET s.mySearchLastReturned = :last WHERE s.myId = :pid") - void updateSearchLastReturned(@Param("pid") long thePid, @Param("last") Date theDate); - @Modifying @Query("UPDATE Search s SET s.myDeleted = :deleted WHERE s.myId = :pid") void updateDeleted(@Param("pid") Long thePid, @Param("deleted") boolean theDeleted); 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 51aa90a45de..f121c471a85 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 @@ -13,7 +13,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.persistence.*; -import javax.validation.constraints.NotNull; import java.io.Serializable; import java.util.*; @@ -43,7 +42,6 @@ import static org.apache.commons.lang3.StringUtils.left; @Table(name = "HFJ_SEARCH", uniqueConstraints = { @UniqueConstraint(name = "IDX_SEARCH_UUID", columnNames = "SEARCH_UUID") }, indexes = { - @Index(name = "IDX_SEARCH_LASTRETURNED", columnList = "SEARCH_LAST_RETURNED"), @Index(name = "IDX_SEARCH_RESTYPE_HASHS", columnList = "RESOURCE_TYPE,SEARCH_QUERY_STRING_HASH,CREATED") }) public class Search implements ICachedSearchDetails, Serializable { @@ -90,11 +88,6 @@ public class Search implements ICachedSearchDetails, Serializable { private Long myResourceId; @Column(name = "RESOURCE_TYPE", length = 200, nullable = true) private String myResourceType; - @NotNull - @Temporal(TemporalType.TIMESTAMP) - @Column(name = "SEARCH_LAST_RETURNED", nullable = false, updatable = false) - @OptimisticLock(excluded = true) - private Date mySearchLastReturned; @Lob() @Basic(fetch = FetchType.LAZY) @Column(name = "SEARCH_QUERY_STRING", nullable = true, updatable = false, length = MAX_SEARCH_QUERY_STRING) @@ -261,14 +254,6 @@ public class Search implements ICachedSearchDetails, Serializable { myResourceType = theResourceType; } - public Date getSearchLastReturned() { - return mySearchLastReturned; - } - - public void setSearchLastReturned(Date theDate) { - mySearchLastReturned = theDate; - } - public String getSearchQueryString() { return mySearchQueryString; } 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 75927cfbb53..731f4019148 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 @@ -406,8 +406,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { .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, mySearchBuilderFactory); retVal.setCacheHit(true); populateBundleProvider(retVal); @@ -1142,7 +1140,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { theSearch.setDeleted(false); theSearch.setUuid(theSearchUuid); theSearch.setCreated(new Date()); - theSearch.setSearchLastReturned(new Date()); theSearch.setTotalCount(null); theSearch.setNumFound(0); theSearch.setPreferredPageSize(theParams.getCount()); 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 deleted file mode 100644 index 32899d6d1d3..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/BaseSearchCacheSvcImpl.java +++ /dev/null @@ -1,87 +0,0 @@ -package ca.uhn.fhir.jpa.search.cache; - -/*- - * #%L - * HAPI FHIR JPA Server - * %% - * Copyright (C) 2014 - 2020 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.entity.Search; -import ca.uhn.fhir.jpa.model.sched.HapiJob; -import ca.uhn.fhir.jpa.model.sched.ISchedulerService; -import ca.uhn.fhir.jpa.model.sched.ScheduledJobDefinition; -import org.apache.commons.lang3.time.DateUtils; -import org.quartz.JobExecutionContext; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.support.TransactionTemplate; - -import javax.annotation.PostConstruct; -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; - @Autowired - private ISchedulerService mySchedulerService; - - private ConcurrentHashMap myUnsyncedLastUpdated = new ConcurrentHashMap<>(); - - @Override - public void updateSearchLastReturned(Search theSearch, Date theDate) { - myUnsyncedLastUpdated.put(theSearch.getId(), theDate); - } - - @PostConstruct - public void scheduleJob() { - ScheduledJobDefinition jobDetail = new ScheduledJobDefinition(); - jobDetail.setId(getClass().getName()); - jobDetail.setJobClass(Job.class); - mySchedulerService.scheduleLocalJob(10 * DateUtils.MILLIS_PER_SECOND, jobDetail); - } - - @Override - 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); - - public static class Job implements HapiJob { - @Autowired - private ISearchCacheSvc myTarget; - - @Override - public void execute(JobExecutionContext theContext) { - myTarget.flushLastUpdated(); - } - } - - -} 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 index 5a008d0f61c..0888e9d19e1 100644 --- 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 @@ -47,7 +47,7 @@ import java.util.Date; import java.util.List; import java.util.Optional; -public class DatabaseSearchCacheSvcImpl extends BaseSearchCacheSvcImpl { +public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { /* * Be careful increasing this number! We use the number of params here in a * DELETE FROM foo WHERE params IN (term,term,term...) @@ -146,11 +146,6 @@ public class DatabaseSearchCacheSvcImpl extends BaseSearchCacheSvcImpl { } - @Override - protected void flushLastUpdated(Long theSearchId, Date theLastUpdated) { - mySearchDao.updateSearchLastReturned(theSearchId, theLastUpdated); - } - @Transactional(Transactional.TxType.NEVER) @Override public void pollForStaleSearchesAndDeleteThem() { @@ -160,7 +155,7 @@ public class DatabaseSearchCacheSvcImpl extends BaseSearchCacheSvcImpl { long cutoffMillis = myDaoConfig.getExpireSearchResultsAfterMillis(); if (myDaoConfig.getReuseCachedSearchResultsForMillis() != null) { - cutoffMillis = Math.max(cutoffMillis, myDaoConfig.getReuseCachedSearchResultsForMillis()); + cutoffMillis = cutoffMillis + myDaoConfig.getReuseCachedSearchResultsForMillis(); } final Date cutoff = new Date((now() - cutoffMillis) - myCutoffSlack); @@ -223,7 +218,7 @@ public class DatabaseSearchCacheSvcImpl extends BaseSearchCacheSvcImpl { // 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())); + ourLog.debug("Deleting search {}/{} - Created[{}]", searchToDelete.getId(), searchToDelete.getUuid(), new InstantType(searchToDelete.getCreated())); mySearchDao.deleteByPid(searchToDelete.getId()); } else { ourLog.debug("Purged {} search results for deleted search {}/{}", resultPids.getSize(), searchToDelete.getId(), searchToDelete.getUuid()); 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 index 90d9b1124e6..82b35d526e2 100644 --- 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 @@ -78,20 +78,6 @@ public interface ISearchCacheSvc { */ 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 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 fcd0961bd9f..e08affabf86 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 @@ -133,7 +133,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { assertNotNull(search3); Search search2 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid2).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search2); - search3timestamp.set(search2.getSearchLastReturned().getTime()); + search3timestamp.set(search2.getCreated().getTime()); } }); 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 69a03290a8a..d73e4a5ef4a 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 @@ -3003,7 +3003,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .count(5) .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); Search search1 = newTxTemplate().execute(new TransactionCallback() { @@ -3012,7 +3011,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { return mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException("")); } }); - Date lastReturned1 = search1.getSearchLastReturned(); + Date created1 = search1.getCreated(); Bundle result2 = ourClient .search() @@ -3021,7 +3020,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .count(5) .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); Search search2 = newTxTemplate().execute(new TransactionCallback() { @@ -3030,9 +3028,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { return mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException("")); } }); - Date lastReturned2 = search2.getSearchLastReturned(); + Date created2 = search2.getCreated(); - assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); + assertEquals(created2.getTime(), created1.getTime()); Thread.sleep(1500); @@ -3067,24 +3065,22 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid1 = toSearchUuidFromLinkNext(result1); Search search1 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned1 = search1.getSearchLastReturned(); + Date created1 = search1.getCreated(); Bundle result2 = ourClient .search() .forResource("Organization") .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned2 = search2.getSearchLastReturned(); + Date created2 = search2.getCreated(); - assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); + assertEquals(created2.getTime(), created1.getTime()); assertEquals(uuid1, uuid2); } 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 22a94634be4..a8f485ba05d 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 @@ -14,27 +14,15 @@ import ca.uhn.fhir.model.primitive.UriDt; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.PreferReturnEnum; -import ca.uhn.fhir.rest.api.SearchTotalModeEnum; -import ca.uhn.fhir.rest.api.SummaryEnum; +import ca.uhn.fhir.rest.api.*; import ca.uhn.fhir.rest.client.api.IClientInterceptor; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.api.IHttpRequest; 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.DateRangeParam; -import ca.uhn.fhir.rest.param.NumberParam; -import ca.uhn.fhir.rest.param.ParamPrefixEnum; -import ca.uhn.fhir.rest.param.StringAndListParam; -import ca.uhn.fhir.rest.param.StringOrListParam; -import ca.uhn.fhir.rest.param.StringParam; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -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.param.*; +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; @@ -46,29 +34,16 @@ import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.time.DateUtils; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpDelete; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPatch; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.*; import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicNameValuePair; import org.hamcrest.Matchers; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBaseBundle; -import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.instance.model.api.*; import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator; import org.hl7.fhir.r4.model.*; -import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent; -import org.hl7.fhir.r4.model.Bundle.BundleLinkComponent; -import org.hl7.fhir.r4.model.Bundle.BundleType; -import org.hl7.fhir.r4.model.Bundle.HTTPVerb; -import org.hl7.fhir.r4.model.Bundle.SearchEntryMode; +import org.hl7.fhir.r4.model.Bundle.*; import org.hl7.fhir.r4.model.Encounter.EncounterLocationComponent; import org.hl7.fhir.r4.model.Encounter.EncounterStatus; import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender; @@ -77,11 +52,7 @@ import org.hl7.fhir.r4.model.Observation.ObservationStatus; 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.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; +import org.junit.*; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.TransactionStatus; @@ -96,28 +67,14 @@ import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Date; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import java.util.stream.Collectors; import static ca.uhn.fhir.jpa.util.TestUtil.sleepAtLeast; import static ca.uhn.fhir.jpa.util.TestUtil.sleepOneClick; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; @SuppressWarnings("Duplicates") public class ResourceProviderR4Test extends BaseResourceProviderR4Test { @@ -4115,11 +4072,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.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned1 = search1.getSearchLastReturned(); + Date created1 = search1.getCreated(); Bundle result2 = ourClient .search() @@ -4128,13 +4084,12 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .count(5) .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned2 = search2.getSearchLastReturned(); + Date created2 = search2.getCreated(); - assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); + assertEquals(created2.getTime(), created1.getTime()); Thread.sleep(1500); @@ -4145,7 +4100,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .count(5) .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); String uuid3 = toSearchUuidFromLinkNext(result3); @@ -4170,11 +4124,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.findByUuidAndFetchIncludes(uuid1).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned1 = search1.getSearchLastReturned(); + Date created1 = search1.getCreated(); sleepOneClick(); @@ -4183,13 +4136,12 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { .forResource("Organization") .returnBundle(Bundle.class) .execute(); - mySearchCacheSvc.flushLastUpdated(); final String uuid2 = toSearchUuidFromLinkNext(result2); Search search2 = newTxTemplate().execute(theStatus -> mySearchEntityDao.findByUuidAndFetchIncludes(uuid2).orElseThrow(() -> new InternalErrorException(""))); - Date lastReturned2 = search2.getSearchLastReturned(); + Date created2 = search2.getCreated(); - assertTrue(lastReturned2.getTime() > lastReturned1.getTime()); + assertEquals(created2.getTime(), created1.getTime()); assertEquals(uuid1, uuid2); } 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 a760af6a195..6ed219e0f9e 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 @@ -9,12 +9,10 @@ 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; -import ca.uhn.fhir.test.utilities.UnregisterScheduledProcessor; import ca.uhn.fhir.util.TestUtil; import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.r4.model.Bundle; @@ -25,7 +23,6 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.test.context.TestPropertySource; import org.springframework.test.util.AopTestUtils; import java.util.Date; @@ -121,7 +118,6 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { search.setCreated(DateUtils.addDays(new Date(), -10000)); search.setSearchType(SearchTypeEnum.SEARCH); search.setResourceType("Patient"); - search.setSearchLastReturned(DateUtils.addDays(new Date(), -10000)); search = mySearchEntityDao.save(search); for (int i = 0; i < 15; i++) { @@ -163,7 +159,6 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { search.setCreated(DateUtils.addDays(new Date(), -10000)); search.setSearchType(SearchTypeEnum.SEARCH); search.setResourceType("Patient"); - search.setSearchLastReturned(DateUtils.addDays(new Date(), -10000)); mySearchEntityDao.save(search); }); @@ -190,7 +185,6 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { search.setCreated(DateUtils.addDays(new Date(), -10000)); search.setSearchType(SearchTypeEnum.SEARCH); search.setResourceType("Patient"); - search.setSearchLastReturned(DateUtils.addDays(new Date(), -10000)); search = mySearchEntityDao.save(search); }); 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 53489f0c607..393a28169fe 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 @@ -61,6 +61,9 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { protected void init420() { // 20191015 - present Builder version = forVersion(VersionEnum.V4_2_0); + Builder.BuilderWithTableName searchTable = version.onTable("HFJ_SEARCH"); + searchTable.dropIndex("20200203.1", "IDX_SEARCH_LASTRETURNED"); + searchTable.dropColumn("20200203.2", "SEARCH_LAST_RETURNED"); } protected void init410() { // 20190815 - 20191014 From b4325199cfb791f59deaaa846a27ce3c9e531cbe Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 3 Feb 2020 11:20:18 -0500 Subject: [PATCH 2/3] review feedback --- .../src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchDao.java | 2 +- .../src/main/java/ca/uhn/fhir/jpa/entity/Search.java | 3 ++- .../uhn/fhir/jpa/search/cache/DatabaseSearchCacheSvcImpl.java | 2 +- .../uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) 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 184ce87e2ee..cf1dbc4e7a8 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 @@ -38,7 +38,7 @@ public interface ISearchDao extends JpaRepository { Optional findByUuidAndFetchIncludes(@Param("uuid") String theUuid); @Query("SELECT s.myId FROM Search s WHERE (s.myCreated < :cutoff) AND (s.myExpiryOrNull IS NULL OR s.myExpiryOrNull < :now)") - Slice findWhereLastReturnedBefore(@Param("cutoff") Date theCutoff, @Param("now") Date theNow, Pageable thePage); + Slice findWhereCreatedBefore(@Param("cutoff") Date theCutoff, @Param("now") Date theNow, Pageable thePage); @Query("SELECT s FROM Search s WHERE s.myResourceType = :type AND mySearchQueryStringHash = :hash AND (s.myCreated > :cutoff) AND s.myDeleted = false AND s.myStatus <> 'FAILED'") Collection findWithCutoffOrExpiry(@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/entity/Search.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java index f121c471a85..d0d904c3b3d 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 @@ -42,7 +42,8 @@ import static org.apache.commons.lang3.StringUtils.left; @Table(name = "HFJ_SEARCH", uniqueConstraints = { @UniqueConstraint(name = "IDX_SEARCH_UUID", columnNames = "SEARCH_UUID") }, indexes = { - @Index(name = "IDX_SEARCH_RESTYPE_HASHS", columnList = "RESOURCE_TYPE,SEARCH_QUERY_STRING_HASH,CREATED") + @Index(name = "IDX_SEARCH_RESTYPE_HASHS", columnList = "RESOURCE_TYPE,SEARCH_QUERY_STRING_HASH,CREATED"), + @Index(name = "IDX_SEARCH_CREATED", columnList = "CREATED") }) public class Search implements ICachedSearchDetails, Serializable { 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 index 0888e9d19e1..ed699f817a0 100644 --- 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 @@ -167,7 +167,7 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { TransactionTemplate tt = new TransactionTemplate(myTxManager); final Slice toDelete = tt.execute(theStatus -> - mySearchDao.findWhereLastReturnedBefore(cutoff, new Date(), PageRequest.of(0, 2000)) + mySearchDao.findWhereCreatedBefore(cutoff, new Date(), PageRequest.of(0, 2000)) ); assert toDelete != null; 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 393a28169fe..25f1ab64bbd 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 @@ -64,6 +64,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { Builder.BuilderWithTableName searchTable = version.onTable("HFJ_SEARCH"); searchTable.dropIndex("20200203.1", "IDX_SEARCH_LASTRETURNED"); searchTable.dropColumn("20200203.2", "SEARCH_LAST_RETURNED"); + searchTable.addIndex("20200203.3", "IDX_SEARCH_CREATED").unique(false).withColumns("CREATED"); } protected void init410() { // 20190815 - 20191014 From 2e96d6bc2d5a0a8f85b27eef514c008d887e3b97 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 3 Feb 2020 15:49:37 -0500 Subject: [PATCH 3/3] fix test --- .../search/StaleSearchDeletingSvcImpl.java | 4 +- .../cache/DatabaseSearchCacheSvcImpl.java | 18 ++-- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 2 - ...FhirResourceDaoR4SearchPageExpiryTest.java | 90 ++++++++++--------- .../r4/StaleSearchDeletingSvcR4Test.java | 2 +- 5 files changed, 63 insertions(+), 53 deletions(-) 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 623fd1e4831..4f22d98f8f9 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 @@ -32,7 +32,7 @@ import org.springframework.transaction.annotation.Transactional; import javax.annotation.PostConstruct; -import static ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.DEFAULT_CUTOFF_SLACK; +import static ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.SEARCH_CLEANUP_JOB_INTERVAL_MILLIS; /** * Deletes old searches @@ -62,7 +62,7 @@ public class StaleSearchDeletingSvcImpl implements IStaleSearchDeletingSvc { ScheduledJobDefinition jobDetail = new ScheduledJobDefinition(); jobDetail.setId(getClass().getName()); jobDetail.setJobClass(Job.class); - mySchedulerService.scheduleClusteredJob(DEFAULT_CUTOFF_SLACK, jobDetail); + mySchedulerService.scheduleClusteredJob(SEARCH_CLEANUP_JOB_INTERVAL_MILLIS, jobDetail); } public static class Job implements HapiJob { 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 index ed699f817a0..dd319a635b1 100644 --- 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 @@ -55,7 +55,7 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { */ 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; + public static final long SEARCH_CLEANUP_JOB_INTERVAL_MILLIS = 10 * DateUtils.MILLIS_PER_SECOND; private static final Logger ourLog = LoggerFactory.getLogger(DatabaseSearchCacheSvcImpl.class); private static int ourMaximumResultsToDeleteInOneStatement = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT; private static int ourMaximumResultsToDeleteInOnePass = DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS; @@ -65,7 +65,7 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { * 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; + private long myCutoffSlack = SEARCH_CLEANUP_JOB_INTERVAL_MILLIS; @Autowired private ISearchDao mySearchDao; @@ -163,7 +163,8 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { 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); + // FIXME KHS + ourLog.info("Searching for searches which are before {}", cutoff); TransactionTemplate tt = new TransactionTemplate(myTxManager); final Slice toDelete = tt.execute(theStatus -> @@ -172,7 +173,9 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { assert toDelete != null; for (final Long nextSearchToDelete : toDelete) { - ourLog.debug("Deleting search with PID {}", nextSearchToDelete); + // FIXME KHS + ourLog.info("Deleting search with PID {}", nextSearchToDelete); + // FIXME KHS do this outside of loop. Make a "deleteWhereCreatedBefore" tt.execute(t -> { mySearchDao.updateDeleted(nextSearchToDelete, true); return null; @@ -186,9 +189,12 @@ public class DatabaseSearchCacheSvcImpl implements ISearchCacheSvc { int count = toDelete.getContent().size(); if (count > 0) { - if (ourLog.isDebugEnabled() || "true".equalsIgnoreCase(System.getProperty("test"))) { + // FIXME KHS +// if (ourLog.isDebugEnabled() || "true".equalsIgnoreCase(System.getProperty("test"))) { + if ( "true".equalsIgnoreCase(System.getProperty("test"))) { Long total = tt.execute(t -> mySearchDao.count()); - ourLog.debug("Deleted {} searches, {} remaining", count, total); + // FIXME KHS + ourLog.info("Deleted {} searches, {} remaining", count, total); } } } 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 7dfffc7dfb9..a3f8a136228 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 @@ -64,7 +64,6 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.util.AopTestUtils; import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.annotation.Transactional; import javax.persistence.EntityManager; import java.io.IOException; @@ -409,7 +408,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { } @Before - @Transactional() public void beforePurgeDatabase() { purgeDatabase(myDaoConfig, mySystemDao, myResourceReindexingSvc, mySearchCoordinatorSvc, mySearchParamRegistry, myBulkDataExportSvc); } 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 e08affabf86..c81ca66f935 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 @@ -32,7 +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 ca.uhn.fhir.jpa.search.cache.DatabaseSearchCacheSvcImpl.SEARCH_CLEANUP_JOB_INTERVAL_MILLIS; import static org.awaitility.Awaitility.await; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.*; @@ -47,7 +47,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { @After() public void after() { DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(DEFAULT_CUTOFF_SLACK); + staleSearchDeletingSvc.setCutoffSlackForUnitTest(SEARCH_CLEANUP_JOB_INTERVAL_MILLIS); DatabaseSearchCacheSvcImpl.setNowForUnitTests(null); } @@ -81,8 +81,11 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } Thread.sleep(10); - myDaoConfig.setExpireSearchResultsAfterMillis(1000L); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); + long reuseCachedSearchResultsForMillis = 500L; + myDaoConfig.setReuseCachedSearchResultsForMillis(reuseCachedSearchResultsForMillis); + long millisBetweenReuseAndExpire = 800L; + long expireSearchResultsAfterMillis = 1000L; + myDaoConfig.setExpireSearchResultsAfterMillis(expireSearchResultsAfterMillis); long start = System.currentTimeMillis(); DatabaseSearchCacheSvcImpl.setNowForUnitTests(start); @@ -107,9 +110,9 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } assertEquals(searchUuid1, searchUuid2); - TestUtil.sleepAtLeast(501); + TestUtil.sleepAtLeast(reuseCachedSearchResultsForMillis + 1); - // We're now past 500ms so we shouldn't reuse the search + // We're now past reuseCachedSearchResultsForMillis so we shouldn't reuse the search final String searchUuid3; { @@ -124,35 +127,36 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { // Search just got used so it shouldn't be deleted - DatabaseSearchCacheSvcImpl.setNowForUnitTests(start + 500); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start + reuseCachedSearchResultsForMillis); + final AtomicLong search1timestamp = new AtomicLong(); + final AtomicLong search2timestamp = new AtomicLong(); final AtomicLong search3timestamp = new AtomicLong(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - Search search3 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).orElseThrow(()->new InternalErrorException("Search doesn't exist")); - assertNotNull(search3); + Search search1 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).orElseThrow(()->new InternalErrorException("Search doesn't exist")); + assertNotNull(search1); + search1timestamp.set(search1.getCreated().getTime()); Search search2 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid2).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search2); - search3timestamp.set(search2.getCreated().getTime()); + search2timestamp.set(search2.getCreated().getTime()); + Search search3 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).orElseThrow(()->new InternalErrorException("Search doesn't exist")); + assertNotNull(search3); + search3timestamp.set(search3.getCreated().getTime()); } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search1timestamp.get() + millisBetweenReuseAndExpire); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3)); - } - }); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1)); + assertTrue(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent()); + assertTrue(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 1100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search1timestamp.get() + reuseCachedSearchResultsForMillis + expireSearchResultsAfterMillis + 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @@ -163,14 +167,12 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + reuseCachedSearchResultsForMillis + expireSearchResultsAfterMillis + 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); await().until(()-> newTxTemplate().execute(t -> !mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent())); await().until(()-> newTxTemplate().execute(t -> !mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent())); - - } @Test @@ -197,7 +199,6 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { 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(); @@ -213,9 +214,11 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } }); - myDaoConfig.setExpireSearchResultsAfterMillis(500); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + 499); + int expireSearchResultsAfterMillis = 700; + myDaoConfig.setExpireSearchResultsAfterMillis(expireSearchResultsAfterMillis); + long reuseCachedSearchResultsForMillis = 400L; + myDaoConfig.setReuseCachedSearchResultsForMillis(reuseCachedSearchResultsForMillis); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + expireSearchResultsAfterMillis - 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); txTemplate.execute(new TransactionCallbackWithoutResult() { @Override @@ -224,7 +227,7 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + 600); + DatabaseSearchCacheSvcImpl.setNowForUnitTests(start.get() + expireSearchResultsAfterMillis + reuseCachedSearchResultsForMillis + 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); txTemplate.execute(new TransactionCallbackWithoutResult() { @Override @@ -251,8 +254,12 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } Thread.sleep(10); - myDaoConfig.setExpireSearchResultsAfterMillis(1000L); - myDaoConfig.setReuseCachedSearchResultsForMillis(500L); + long expireSearchResultsAfterMillis = 1000L; + myDaoConfig.setExpireSearchResultsAfterMillis(expireSearchResultsAfterMillis); + + long reuseCachedSearchResultsForMillis = 500L; + myDaoConfig.setReuseCachedSearchResultsForMillis(reuseCachedSearchResultsForMillis); + long start = System.currentTimeMillis(); final String searchUuid1; @@ -278,9 +285,10 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } assertEquals(searchUuid1, searchUuid2); - TestUtil.sleepAtLeast(501); + TestUtil.sleepAtLeast(reuseCachedSearchResultsForMillis + 1); + myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - // We're now past 500ms so we shouldn't reuse the search + // We're now past reuseCachedSearchResultsForMillis so we shouldn't reuse the search final String searchUuid3; { @@ -293,37 +301,35 @@ public class FhirResourceDaoR4SearchPageExpiryTest extends BaseJpaR4Test { } assertNotEquals(searchUuid1, searchUuid3); - // Search just got used so it shouldn't be deleted + waitForSearchToSave(searchUuid3); + + // Search hasn't expired yet so it shouldn't be deleted myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); + final AtomicLong search1timestamp = new AtomicLong(); final AtomicLong search3timestamp = new AtomicLong(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { + Search search1 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).orElseThrow(()->new InternalErrorException("Search doesn't exist")); Search search3 = mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).orElseThrow(()->new InternalErrorException("Search doesn't exist")); assertNotNull(search3); + search1timestamp.set(search1.getCreated().getTime()); search3timestamp.set(search3.getCreated().getTime()); } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 800); - + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search1timestamp.get() + expireSearchResultsAfterMillis + reuseCachedSearchResultsForMillis + 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); - newTxTemplate().execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - assertNotNull(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3)); - } - }); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { assertFalse(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid1).isPresent()); + assertTrue(mySearchEntityDao.findByUuidAndFetchIncludes(searchUuid3).isPresent()); } }); - DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + 2100); - + DatabaseSearchCacheSvcImpl.setNowForUnitTests(search3timestamp.get() + expireSearchResultsAfterMillis + reuseCachedSearchResultsForMillis + 1); myStaleSearchDeletingSvc.pollForStaleSearchesAndDeleteThem(); newTxTemplate().execute(new TransactionCallbackWithoutResult() { @Override 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 6ed219e0f9e..c66c824022c 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 @@ -47,7 +47,7 @@ public class StaleSearchDeletingSvcR4Test extends BaseResourceProviderR4Test { public void after() throws Exception { super.after(); DatabaseSearchCacheSvcImpl staleSearchDeletingSvc = AopTestUtils.getTargetObject(mySearchCacheSvc); - staleSearchDeletingSvc.setCutoffSlackForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_CUTOFF_SLACK); + staleSearchDeletingSvc.setCutoffSlackForUnitTest(DatabaseSearchCacheSvcImpl.SEARCH_CLEANUP_JOB_INTERVAL_MILLIS); DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_STMT); DatabaseSearchCacheSvcImpl.setMaximumResultsToDeleteInOnePassForUnitTest(DatabaseSearchCacheSvcImpl.DEFAULT_MAX_RESULTS_TO_DELETE_IN_ONE_PAS); }