Remove duplicate indexes on update (#5567)

* Remove duplicate indexes on update

* Add changelog

* Remove fixmes

* Update changelog
This commit is contained in:
James Agnew 2023-12-21 20:30:57 -05:00 committed by GitHub
parent 836b755ad3
commit 15509bc321
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 523 additions and 95 deletions

View File

@ -0,0 +1,9 @@
---
type: fix
issue: 5567
jira: SMILE-7819
title: "When updating or reindexing a resource in the JPA server, if duplicate rows are
present in the indexing tables, the duplicate rows may fail to be removed. Duplicates
are not typically present in these tables, but can happen if an indexing job fails in
some circumstances. The impact of these rows not being cleaned up is that resources
may appear in search results that they should no longer appear in."

View File

@ -1083,7 +1083,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
() -> new IdentityHashMap<>());
existingParams = existingSearchParams.get(entity);
if (existingParams == null) {
existingParams = new ResourceIndexedSearchParams(entity);
existingParams = ResourceIndexedSearchParams.withLists(entity);
/*
* If we have lots of resource links, this proactively fetches the targets so
* that we don't look them up one-by-one when comparing the new set to the
@ -1105,7 +1105,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
// TODO: is this IF statement always true? Try removing it
if (thePerformIndexing || theEntity.getVersion() == 1) {
newParams = new ResourceIndexedSearchParams();
newParams = ResourceIndexedSearchParams.withSets();
RequestPartitionId requestPartitionId;
if (!myPartitionSettings.isPartitioningEnabled()) {

View File

@ -34,7 +34,9 @@ import org.springframework.stereotype.Service;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
@Service
public class DaoSearchParamSynchronizer {
@ -82,6 +84,28 @@ public class DaoSearchParamSynchronizer {
next.calculateHashes();
}
/*
* It's technically possible that the existing index collection
* contains duplicates. Duplicates don't actually cause any
* issues for searching since we always deduplicate the PIDs we
* get back from the search, but they are wasteful. We don't
* enforce uniqueness in the DB for the index tables for
* performance reasons (no sense adding a constraint that slows
* down writes when dupes don't actually hurt anything other than
* a bit of wasted space).
*
* So we check if there are any dupes, and if we find any we
* remove them.
*/
Set<T> existingParamsAsSet = new HashSet<>(theExistingParams.size());
for (Iterator<T> iterator = theExistingParams.iterator(); iterator.hasNext(); ) {
T next = iterator.next();
if (!existingParamsAsSet.add(next)) {
iterator.remove();
myEntityManager.remove(next);
}
}
/*
* HashCodes may have changed as a result of setting the partition ID, so
* create a new set that will reflect the new hashcodes

View File

@ -28,7 +28,6 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.jpa.model.entity.BaseResourceIndexedSearchParam;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedComboStringUnique;
import ca.uhn.fhir.jpa.model.entity.ResourceLink;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamWithInlineReferencesExtractor;
import ca.uhn.fhir.jpa.searchparam.extractor.ISearchParamExtractor;
@ -50,7 +49,6 @@ import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;
import java.util.Collection;
import java.util.Iterator;
import java.util.stream.Collectors;
@Service
@ -117,19 +115,6 @@ public class SearchParamWithInlineReferencesExtractor extends BaseSearchParamWit
theTransactionDetails,
thePerformIndexing,
ISearchParamExtractor.ALL_PARAMS);
/*
* If the existing resource already has links and those match links we still want, use them instead of removing them and re adding them
*/
for (Iterator<ResourceLink> existingLinkIter =
theExistingParams.getResourceLinks().iterator();
existingLinkIter.hasNext(); ) {
ResourceLink nextExisting = existingLinkIter.next();
if (theParams.myLinks.remove(nextExisting)) {
existingLinkIter.remove();
theParams.myLinks.add(nextExisting);
}
}
}
@Nullable

View File

@ -160,7 +160,7 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService {
myInterceptorService, theRequestDetails, theResourceId, resource);
BaseHapiFhirResourceDao.invokeStoragePreShowResources(myInterceptorService, theRequestDetails, resource);
ResourceIndexedSearchParams existingParamsToPopulate = new ResourceIndexedSearchParams(entity);
ResourceIndexedSearchParams existingParamsToPopulate = ResourceIndexedSearchParams.withLists(entity);
existingParamsToPopulate.mySearchParamPresentEntities.addAll(entity.getSearchParamPresents());
List<String> messages = new ArrayList<>();
@ -173,7 +173,7 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService {
messages.add("WARNING: " + next);
}
ResourceIndexedSearchParams newParamsToPopulate = new ResourceIndexedSearchParams(entity);
ResourceIndexedSearchParams newParamsToPopulate = ResourceIndexedSearchParams.withLists(entity);
newParamsToPopulate.mySearchParamPresentEntities.addAll(entity.getSearchParamPresents());
return buildIndexResponse(existingParamsToPopulate, newParamsToPopulate, true, messages);
@ -205,12 +205,12 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService {
.collect(Collectors.toSet());
}
ResourceIndexedSearchParams newParamsToPopulate = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams newParamsToPopulate = ResourceIndexedSearchParams.withSets();
mySearchParamExtractorService.extractFromResource(
theRequestPartitionId,
theRequestDetails,
newParamsToPopulate,
new ResourceIndexedSearchParams(),
ResourceIndexedSearchParams.empty(),
entity,
resource,
theTransactionDetails,
@ -220,13 +220,13 @@ public class InstanceReindexServiceImpl implements IInstanceReindexService {
ResourceIndexedSearchParams existingParamsToPopulate;
boolean showAction;
if (theParameters == null) {
existingParamsToPopulate = new ResourceIndexedSearchParams(entity);
existingParamsToPopulate = ResourceIndexedSearchParams.withLists(entity);
existingParamsToPopulate.mySearchParamPresentEntities.addAll(entity.getSearchParamPresents());
fillInParamNames(
entity, existingParamsToPopulate.mySearchParamPresentEntities, theResourceId.getResourceType());
showAction = true;
} else {
existingParamsToPopulate = new ResourceIndexedSearchParams();
existingParamsToPopulate = ResourceIndexedSearchParams.withSets();
showAction = false;
}

View File

@ -53,8 +53,8 @@ public class DaoSearchParamSynchronizerTest {
when(existingEntity.isParamsNumberPopulated()).thenReturn(true);
when(existingEntity.getParamsNumber()).thenReturn(List.of(EXISTING_SEARCH_PARAM_NUMBER));
theParams = new ResourceIndexedSearchParams(theEntity);
existingParams = new ResourceIndexedSearchParams(existingEntity);
theParams = ResourceIndexedSearchParams.withLists(theEntity);
existingParams = ResourceIndexedSearchParams.withLists(existingEntity);
final ResourceTable resourceTable = new ResourceTable();
resourceTable.setId(1L);

View File

@ -21,8 +21,22 @@ package ca.uhn.fhir.jpa.searchparam.extractor;
import ca.uhn.fhir.context.RuntimeSearchParam;
import ca.uhn.fhir.jpa.model.config.PartitionSettings;
import ca.uhn.fhir.jpa.model.entity.BaseResourceIndexedSearchParam;
import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedComboStringUnique;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedComboTokenNonUnique;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamCoords;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamNumber;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantity;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantityNormalized;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamUri;
import ca.uhn.fhir.jpa.model.entity.ResourceLink;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.model.entity.SearchParamPresentEntity;
import ca.uhn.fhir.jpa.model.entity.StorageSettings;
import ca.uhn.fhir.jpa.model.entity.*;
import ca.uhn.fhir.jpa.model.util.UcumServiceUtil;
import ca.uhn.fhir.jpa.searchparam.util.RuntimeSearchParamHelper;
import ca.uhn.fhir.model.api.IQueryParameterType;
@ -47,28 +61,50 @@ import static org.apache.commons.lang3.StringUtils.compare;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
public final class ResourceIndexedSearchParams {
public final Collection<ResourceIndexedSearchParamString> myStringParams = new ArrayList<>();
public final Collection<ResourceIndexedSearchParamToken> myTokenParams = new HashSet<>();
public final Collection<ResourceIndexedSearchParamNumber> myNumberParams = new ArrayList<>();
public final Collection<ResourceIndexedSearchParamQuantity> myQuantityParams = new ArrayList<>();
public final Collection<ResourceIndexedSearchParamQuantityNormalized> myQuantityNormalizedParams =
new ArrayList<>();
public final Collection<ResourceIndexedSearchParamDate> myDateParams = new ArrayList<>();
public final Collection<ResourceIndexedSearchParamUri> myUriParams = new ArrayList<>();
public final Collection<ResourceIndexedSearchParamCoords> myCoordsParams = new ArrayList<>();
public final Collection<ResourceIndexedComboStringUnique> myComboStringUniques = new HashSet<>();
public final Collection<ResourceIndexedComboTokenNonUnique> myComboTokenNonUnique = new HashSet<>();
public final Collection<ResourceLink> myLinks = new HashSet<>();
public final Set<String> myPopulatedResourceLinkParameters = new HashSet<>();
public final Collection<SearchParamPresentEntity> mySearchParamPresentEntities = new HashSet<>();
public final Collection<ResourceIndexedSearchParamComposite> myCompositeParams = new HashSet<>();
private static final Set<String> myIgnoredParams = Set.of(Constants.PARAM_TEXT, Constants.PARAM_CONTENT);
public final Collection<ResourceIndexedSearchParamString> myStringParams;
public final Collection<ResourceIndexedSearchParamToken> myTokenParams;
public final Collection<ResourceIndexedSearchParamNumber> myNumberParams;
public final Collection<ResourceIndexedSearchParamQuantity> myQuantityParams;
public final Collection<ResourceIndexedSearchParamQuantityNormalized> myQuantityNormalizedParams;
public final Collection<ResourceIndexedSearchParamDate> myDateParams;
public final Collection<ResourceIndexedSearchParamUri> myUriParams;
public final Collection<ResourceIndexedSearchParamCoords> myCoordsParams;
public final Collection<ResourceIndexedComboStringUnique> myComboStringUniques;
public final Collection<ResourceIndexedComboTokenNonUnique> myComboTokenNonUnique;
public final Collection<ResourceLink> myLinks;
public final Collection<SearchParamPresentEntity> mySearchParamPresentEntities;
public final Collection<ResourceIndexedSearchParamComposite> myCompositeParams;
public final Set<String> myPopulatedResourceLinkParameters = new HashSet<>();
public ResourceIndexedSearchParams() {}
/**
* TODO: Remove this - Currently used by CDR though
*
* @deprecated Use a factory constructor instead
*/
@Deprecated
public ResourceIndexedSearchParams() {
this(Mode.SET);
}
public ResourceIndexedSearchParams(ResourceTable theEntity) {
private ResourceIndexedSearchParams(Mode theMode) {
myStringParams = theMode.newCollection();
myTokenParams = theMode.newCollection();
myNumberParams = theMode.newCollection();
myQuantityParams = theMode.newCollection();
myQuantityNormalizedParams = theMode.newCollection();
myDateParams = theMode.newCollection();
myUriParams = theMode.newCollection();
myCoordsParams = theMode.newCollection();
myComboStringUniques = theMode.newCollection();
myComboTokenNonUnique = theMode.newCollection();
myLinks = theMode.newCollection();
mySearchParamPresentEntities = theMode.newCollection();
myCompositeParams = theMode.newCollection();
}
private ResourceIndexedSearchParams(ResourceTable theEntity, Mode theMode) {
this(theMode);
if (theEntity.isParamsStringPopulated()) {
myStringParams.addAll(theEntity.getParamsString());
}
@ -575,4 +611,52 @@ public final class ResourceIndexedSearchParams {
}
}
}
/**
* Create a new instance that uses Sets as the internal collection
* type in order to defend against duplicates. This should be used
* when calculating the set of indexes for a resource that is
* about to be stored.
*/
public static ResourceIndexedSearchParams withSets() {
return new ResourceIndexedSearchParams(Mode.SET);
}
/**
* Create an empty and immutable structure.
*/
public static ResourceIndexedSearchParams empty() {
return new ResourceIndexedSearchParams(Mode.EMPTY);
}
/**
* Create a new instance that holds all the existing indexes
* in lists so that any duplicates are preserved.
*/
public static ResourceIndexedSearchParams withLists(ResourceTable theResourceTable) {
return new ResourceIndexedSearchParams(theResourceTable, Mode.LIST);
}
private enum Mode {
LIST {
@Override
public <T> Collection<T> newCollection() {
return new ArrayList<>();
}
},
SET {
@Override
public <T> Collection<T> newCollection() {
return new HashSet<>();
}
},
EMPTY {
@Override
public <T> Collection<T> newCollection() {
return List.of();
}
};
public abstract <T> Collection<T> newCollection();
}
}

View File

@ -124,7 +124,7 @@ public class SearchParamExtractorService {
theRequestPartitionId,
theRequestDetails,
theParams,
new ResourceIndexedSearchParams(),
ResourceIndexedSearchParams.withSets(),
theEntity,
theResource,
theTransactionDetails,
@ -149,7 +149,7 @@ public class SearchParamExtractorService {
boolean theFailOnInvalidReference,
@Nonnull ISearchParamExtractor.ISearchParamFilter theSearchParamFilter) {
// All search parameter types except Reference
ResourceIndexedSearchParams normalParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams normalParams = ResourceIndexedSearchParams.withSets();
extractSearchIndexParameters(theRequestDetails, normalParams, theResource, theSearchParamFilter);
mergeParams(normalParams, theNewParams);
@ -159,14 +159,14 @@ public class SearchParamExtractorService {
SearchParamExtractorService.handleWarnings(theRequestDetails, myInterceptorBroadcaster, indexedReferences);
if (indexOnContainedResources) {
ResourceIndexedSearchParams containedParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams containedParams = ResourceIndexedSearchParams.withSets();
extractSearchIndexParametersForContainedResources(
theRequestDetails, containedParams, theResource, theEntity, indexedReferences);
mergeParams(containedParams, theNewParams);
}
if (myStorageSettings.isIndexOnUpliftedRefchains()) {
ResourceIndexedSearchParams containedParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams containedParams = ResourceIndexedSearchParams.withSets();
extractSearchIndexParametersForUpliftedRefchains(
theRequestDetails,
containedParams,
@ -427,7 +427,7 @@ public class SearchParamExtractorService {
continue;
}
ResourceIndexedSearchParams currParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams currParams = ResourceIndexedSearchParams.withSets();
// 3.3 create indexes for the current contained resource
extractSearchIndexParameters(theRequestDetails, currParams, targetResource, searchParamsToIndex);
@ -599,7 +599,7 @@ public class SearchParamExtractorService {
ISearchParamExtractor.SearchParamSet<PathAndRef> theIndexedReferences) {
extractResourceLinks(
theRequestPartitionId,
new ResourceIndexedSearchParams(),
ResourceIndexedSearchParams.withSets(),
theParams,
theEntity,
theResource,
@ -937,7 +937,7 @@ public class SearchParamExtractorService {
continue;
}
currParams = new ResourceIndexedSearchParams();
currParams = ResourceIndexedSearchParams.withSets();
// 3.3 create indexes for the current contained resource
ISearchParamExtractor.SearchParamSet<PathAndRef> indexedReferences =

View File

@ -50,12 +50,12 @@ public class IndexedSearchParamExtractor {
TransactionDetails transactionDetails = new TransactionDetails();
String resourceType = myContext.getResourceType(theResource);
entity.setResourceType(resourceType);
ResourceIndexedSearchParams resourceIndexedSearchParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams resourceIndexedSearchParams = ResourceIndexedSearchParams.withSets();
mySearchParamExtractorService.extractFromResource(
null,
theRequest,
resourceIndexedSearchParams,
new ResourceIndexedSearchParams(),
ResourceIndexedSearchParams.empty(),
entity,
theResource,
transactionDetails,

View File

@ -31,7 +31,7 @@ public class ResourceIndexedSearchParamsTest {
mySource = new ResourceTable();
mySource.setResourceType("Patient");
myParams = new ResourceIndexedSearchParams(mySource);
myParams = ResourceIndexedSearchParams.withLists(mySource);
}
@Test

View File

@ -103,7 +103,7 @@ public class InMemoryResourceMatcherConfigurationR5Test {
}
private ResourceIndexedSearchParams extractSearchParams(Observation theObservation) {
ResourceIndexedSearchParams retval = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams retval = ResourceIndexedSearchParams.withSets();
retval.myTokenParams.add(extractCodeTokenParam(theObservation));
return retval;
}

View File

@ -166,7 +166,7 @@ public class InMemoryResourceMatcherR5Test {
public void testMatch_sourceWithModifiers_matchesSuccessfully(String theSourceValue, String theSearchCriteria, boolean theShouldMatch) {
myObservation.getMeta().setSource(theSourceValue);
ResourceIndexedSearchParams searchParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams searchParams = ResourceIndexedSearchParams.withSets();
searchParams.myUriParams.add(extractSourceUriParam(myObservation));
InMemoryMatchResult resultInsidePeriod = myInMemoryResourceMatcher.match(theSearchCriteria, myObservation, searchParams, newRequest());
@ -408,7 +408,7 @@ public class InMemoryResourceMatcherR5Test {
private ResourceIndexedSearchParams extractSearchParams(Observation theObservation) {
ResourceIndexedSearchParams retval = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams retval = ResourceIndexedSearchParams.withSets();
retval.myDateParams.add(extractEffectiveDateParam(theObservation));
retval.myTokenParams.add(extractCodeTokenParam(theObservation));
return retval;

View File

@ -259,9 +259,12 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
*/
@Test
public void testUpdateWithNoChanges() {
IIdType orgId = createOrganization(withName("MY ORG"));
IIdType id = runInTransaction(() -> {
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue("2");
p.setManagingOrganization(new Reference(orgId));
return myPatientDao.create(p).getId().toUnqualified();
});
@ -270,10 +273,11 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
Patient p = new Patient();
p.setId(id.getIdPart());
p.addIdentifier().setSystem("urn:system").setValue("2");
p.setManagingOrganization(new Reference(orgId));
myPatientDao.update(p);
});
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
assertEquals(3, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
assertEquals(5, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
myCaptureQueriesListener.logUpdateQueriesForCurrentThread();
assertEquals(0, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size());
assertThat(myCaptureQueriesListener.getInsertQueriesForCurrentThread(), empty());
@ -286,9 +290,13 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
*/
@Test
public void testUpdateWithChanges() {
IIdType orgId = createOrganization(withName("MY ORG"));
IIdType orgId2 = createOrganization(withName("MY ORG 2"));
IIdType id = runInTransaction(() -> {
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue("2");
p.setManagingOrganization(new Reference(orgId));
return myPatientDao.create(p).getId().toUnqualified();
});
@ -297,12 +305,13 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
Patient p = new Patient();
p.setId(id.getIdPart());
p.addIdentifier().setSystem("urn:system").setValue("3");
p.setManagingOrganization(new Reference(orgId2));
myPatientDao.update(p).getResource();
});
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
assertEquals(3, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
assertEquals(6, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
myCaptureQueriesListener.logUpdateQueriesForCurrentThread();
assertEquals(2, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size());
assertEquals(3, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size());
myCaptureQueriesListener.logInsertQueriesForCurrentThread();
assertEquals(1, myCaptureQueriesListener.getInsertQueriesForCurrentThread().size());
myCaptureQueriesListener.logDeleteQueriesForCurrentThread();
@ -1042,11 +1051,13 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
ResourceIdListWorkChunkJson data = new ResourceIdListWorkChunkJson();
IIdType patientId = createPatient(withActiveTrue());
IIdType orgId = createOrganization(withName("MY ORG"));
for (int i = 0; i < 10; i++) {
Patient p = new Patient();
p.setId(patientId.toUnqualifiedVersionless());
p.setActive(true);
p.addIdentifier().setValue("" + i);
p.setManagingOrganization(new Reference(orgId));
myPatientDao.update(p, mySrd);
}
data.addTypedPid("Patient", patientId.getIdPartAsLong());
@ -1055,7 +1066,6 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
data.addTypedPid("Patient", nextPatientId.getIdPartAsLong());
}
myStorageSettings.setInlineResourceTextBelowSize(10000);
ReindexJobParameters params = new ReindexJobParameters()
.setOptimizeStorage(theOptimizeStorageModeEnum)
.setReindexSearchParameters(ReindexParameters.ReindexSearchParametersEnum.NONE)

View File

@ -8,6 +8,7 @@ import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.model.entity.TagDefinition;
import ca.uhn.fhir.jpa.model.entity.TagTypeEnum;

View File

@ -1,5 +1,6 @@
package ca.uhn.fhir.jpa.dao.r5;
import ca.uhn.fhir.batch2.api.IJobCoordinator;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.support.IValidationSupport;
import ca.uhn.fhir.interceptor.api.IInterceptorService;
@ -144,6 +145,8 @@ import static org.mockito.Mockito.mock;
@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = {TestR5Config.class})
public abstract class BaseJpaR5Test extends BaseJpaTest implements ITestDataBuilder {
@Autowired
protected IJobCoordinator myJobCoordinator;
@Autowired
protected PartitionSettings myPartitionSettings;
@Autowired

View File

@ -0,0 +1,312 @@
package ca.uhn.fhir.jpa.dao.r5;
import ca.uhn.fhir.batch2.jobs.reindex.ReindexAppCtx;
import ca.uhn.fhir.batch2.jobs.reindex.ReindexJobParameters;
import ca.uhn.fhir.batch2.model.JobInstanceStartRequest;
import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedComboTokenNonUnique;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString;
import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken;
import ca.uhn.fhir.jpa.model.entity.ResourceLink;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.util.HapiExtensions;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r5.model.BooleanType;
import org.hl7.fhir.r5.model.Enumerations;
import org.hl7.fhir.r5.model.SearchParameter;
import org.hl7.fhir.r5.model.Patient;
import org.hl7.fhir.r5.model.Reference;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class DuplicateIndexR5Test extends BaseJpaR5Test {
@Test
public void testDuplicateTokensClearedOnUpdate() {
// Setup
IIdType id = createPatientWithDuplicateTokens();
assertEquals(3, runInTransaction(()->myResourceIndexedSearchParamTokenDao.findAll().stream().filter(t->t.getParamName().equals("identifier")).count()));
// Test
Patient pt = new Patient();
pt.setId(id);
pt.addIdentifier().setSystem("http://foo").setValue("bar22");
myPatientDao.update(pt, mySrd);
// Verify
logAllTokenIndexes();
assertEquals(1, runInTransaction(()->myResourceIndexedSearchParamTokenDao.findAll().stream().filter(t->t.getParamName().equals("identifier")).count()));
}
@Test
public void testDuplicateTokensClearedOnReindex() {
// Setup
createPatientWithDuplicateTokens();
// Test
reindexAllPatients();
// Verify
logAllTokenIndexes();
assertEquals(1, runInTransaction(()->myResourceIndexedSearchParamTokenDao.findAll().stream().filter(t->t.getParamName().equals("identifier")).count()));
}
@Test
public void testDuplicateStringsClearedOnUpdate() {
// Setup
IIdType id = createPatientWithDuplicateStrings();
assertEquals(3, runInTransaction(()->myResourceIndexedSearchParamStringDao.findAll().stream().filter(t->t.getParamName().equals("family")).count()));
// Test
Patient pt = new Patient();
pt.setId(id);
pt.getNameFirstRep().setFamily("FAMILY2");
myPatientDao.update(pt, mySrd);
// Verify
logAllTokenIndexes();
assertEquals(1, runInTransaction(()->myResourceIndexedSearchParamStringDao.findAll().stream().filter(t->t.getParamName().equals("family")).count()));
}
@Test
public void testDuplicateStringsClearedOnReindex() {
// Setup
createPatientWithDuplicateStrings();
assertEquals(3, runInTransaction(()->myResourceIndexedSearchParamStringDao.findAll().stream().filter(t->t.getParamName().equals("family")).count()));
// Test
reindexAllPatients();
// Verify
logAllTokenIndexes();
assertEquals(1, runInTransaction(()->myResourceIndexedSearchParamStringDao.findAll().stream().filter(t->t.getParamName().equals("family")).count()));
}
@Test
public void testDuplicateResourceLinksClearedOnUpdate() {
// Setup
IIdType id = createPatientWithDuplicateResourceLinks();
assertEquals(3, runInTransaction(()->myResourceLinkDao.findAll().stream().filter(t->t.getSourcePath().equals("Patient.managingOrganization")).count()));
// Test
IIdType orgId = createOrganization(withName("MY ORG 2"));
Patient pt = new Patient();
pt.setId(id);
pt.setManagingOrganization(new Reference(orgId));
myPatientDao.update(pt, mySrd);
// Verify
assertEquals(1, runInTransaction(()->myResourceLinkDao.findAll().stream().filter(t->t.getSourcePath().equals("Patient.managingOrganization")).count()));
}
@Test
public void testDuplicateResourceLinksClearedOnReindex() {
// Setup
createPatientWithDuplicateResourceLinks();
// Test
reindexAllPatients();
// Verify
assertEquals(1, runInTransaction(()->myResourceLinkDao.findAll().stream().filter(t->t.getSourcePath().equals("Patient.managingOrganization")).count()));
}
@Test
public void testDuplicateComboParamsClearedOnUpdate() {
// Setup
IIdType id = createPatientWithDuplicateNonUniqueComboParams();
assertEquals(3, runInTransaction(()->myResourceIndexedComboTokensNonUniqueDao.count()));
// Test
Patient pt = new Patient();
pt.setId(id);
pt.getNameFirstRep().setFamily("FAMILY2").addGiven("GIVEN");
pt.setGender(Enumerations.AdministrativeGender.MALE);
myPatientDao.update(pt, mySrd);
// Verify
assertEquals(1, runInTransaction(()->myResourceIndexedComboTokensNonUniqueDao.count()));
}
@Test
public void testDuplicateComboParamsClearedOnReindex() {
// Setup
createPatientWithDuplicateNonUniqueComboParams();
assertEquals(3, runInTransaction(()->myResourceIndexedComboTokensNonUniqueDao.count()));
// Test
reindexAllPatients();
// Verify
assertEquals(1, runInTransaction(()->myResourceIndexedComboTokensNonUniqueDao.count()));
}
private void reindexAllPatients() {
ReindexJobParameters parameters = new ReindexJobParameters();
parameters.addUrl("Patient?");
JobInstanceStartRequest startRequest = new JobInstanceStartRequest();
startRequest.setJobDefinitionId(ReindexAppCtx.JOB_REINDEX);
startRequest.setParameters(parameters);
Batch2JobStartResponse res = myJobCoordinator.startInstance(mySrd, startRequest);
myBatch2JobHelper.awaitJobCompletion(res.getInstanceId());
}
private IIdType createPatientWithDuplicateNonUniqueComboParams() {
createNamesAndGenderSp();
IIdType id1 = createPatient(withFamily("FAMILY"), withGiven("GIVEN"), withGender("male"));
runInTransaction(()->{
assertEquals(5, myResourceTableDao.count());
ResourceTable table = myResourceTableDao.findAll().stream().filter(t1 -> t1.getResourceType().equals("Patient")).findFirst().orElseThrow();
assertEquals(1, table.getmyParamsComboTokensNonUnique().size());
ResourceIndexedComboTokenNonUnique param = table.getmyParamsComboTokensNonUnique().iterator().next();
// Create a dupe
ResourceIndexedComboTokenNonUnique dupe0 = new ResourceIndexedComboTokenNonUnique();
dupe0.setPartitionSettings(param.getPartitionSettings());
dupe0.setResource(param.getResource());
dupe0.setHashComplete(param.getHashComplete());
dupe0.setIndexString(param.getIndexString());
dupe0.setSearchParameterId(param.getSearchParameterId());
dupe0.calculateHashes();
myResourceIndexedComboTokensNonUniqueDao.save(dupe0);
// Create a second dupe
ResourceIndexedComboTokenNonUnique dupe1 = new ResourceIndexedComboTokenNonUnique();
dupe1.setPartitionSettings(param.getPartitionSettings());
dupe1.setResource(param.getResource());
dupe1.setHashComplete(param.getHashComplete());
dupe1.setIndexString(param.getIndexString());
dupe1.setSearchParameterId(param.getSearchParameterId());
dupe1.calculateHashes();
myResourceIndexedComboTokensNonUniqueDao.save(dupe1);
});
return id1;
}
private IIdType createPatientWithDuplicateResourceLinks() {
IIdType orgId = createOrganization(withName("MY ORG"));
IIdType id1 = createPatient(withOrganization(orgId));
runInTransaction(()->{
assertEquals(2, myResourceTableDao.count());
ResourceTable table = myResourceTableDao.findAll().stream().filter(t->t.getResourceType().equals("Patient")).findFirst().orElseThrow();
assertEquals(1, table.getResourceLinks().size());
ResourceLink existingLink = table.getResourceLinks().iterator().next();
// Create a dupe
ResourceLink dupe0 = new ResourceLink();
dupe0.setSourceResource(existingLink.getSourceResource());
dupe0.setUpdated(existingLink.getUpdated());
dupe0.setSourcePath(existingLink.getSourcePath());
dupe0.setTargetResource(existingLink.getTargetResourceType(), existingLink.getTargetResourcePid(), existingLink.getTargetResourceId());
dupe0.setTargetResourceVersion(existingLink.getTargetResourceVersion());
dupe0.calculateHashes();
myResourceLinkDao.save(dupe0);
// Create a second dupe
ResourceLink dupe1 = new ResourceLink();
dupe1.setSourceResource(existingLink.getSourceResource());
dupe1.setUpdated(existingLink.getUpdated());
dupe1.setSourcePath(existingLink.getSourcePath());
dupe1.setTargetResource(existingLink.getTargetResourceType(), existingLink.getTargetResourcePid(), existingLink.getTargetResourceId());
dupe1.setTargetResourceVersion(existingLink.getTargetResourceVersion());
dupe1.calculateHashes();
myResourceLinkDao.save(dupe1);
});
return id1;
}
private IIdType createPatientWithDuplicateStrings() {
IIdType id1 = createPatient(withFamily("FAMILY"));
runInTransaction(()->{
assertEquals(1, myResourceTableDao.count());
ResourceTable table = myResourceTableDao.findAll().get(0);
// Create a dupe
ResourceIndexedSearchParamString dupe0 = new ResourceIndexedSearchParamString(myPartitionSettings, myStorageSettings, "Patient", "family", "FAMILY", "FAMILY");
dupe0.setResource(table);
dupe0.calculateHashes();
myResourceIndexedSearchParamStringDao.save(dupe0);
// Create a second dupe
ResourceIndexedSearchParamString dupe1 = new ResourceIndexedSearchParamString(myPartitionSettings, myStorageSettings, "Patient", "family", "FAMILY", "FAMILY");
dupe1.setResource(table);
dupe1.calculateHashes();
myResourceIndexedSearchParamStringDao.save(dupe1);
});
return id1;
}
private IIdType createPatientWithDuplicateTokens() {
IIdType id = createPatient(withIdentifier("http://foo", "bar"));
runInTransaction(()->{
assertEquals(1, myResourceTableDao.count());
ResourceTable table = myResourceTableDao.findAll().get(0);
// Create a dupe
ResourceIndexedSearchParamToken dupe0 = new ResourceIndexedSearchParamToken(myPartitionSettings, "Patient", "identifier", "http://foo", "bar");
dupe0.setResource(table);
dupe0.calculateHashes();
myResourceIndexedSearchParamTokenDao.save(dupe0);
// Create a second dupe
ResourceIndexedSearchParamToken dupe1 = new ResourceIndexedSearchParamToken(myPartitionSettings, "Patient", "identifier", "http://foo", "bar");
dupe1.setResource(table);
dupe1.calculateHashes();
myResourceIndexedSearchParamTokenDao.save(dupe1);
});
return id;
}
private void createNamesAndGenderSp() {
SearchParameter sp = new SearchParameter();
sp.setId("SearchParameter/patient-family");
sp.setType(Enumerations.SearchParamType.STRING);
sp.setCode("family");
sp.setExpression("Patient.name.family + '|'");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.addBase(Enumerations.VersionIndependentResourceTypesAll.PATIENT);
mySearchParameterDao.update(sp, mySrd);
sp = new SearchParameter();
sp.setId("SearchParameter/patient-given");
sp.setType(Enumerations.SearchParamType.STRING);
sp.setCode("given");
sp.setExpression("Patient.name.given");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.addBase(Enumerations.VersionIndependentResourceTypesAll.PATIENT);
mySearchParameterDao.update(sp, mySrd);
sp = new SearchParameter();
sp.setId("SearchParameter/patient-gender");
sp.setType(Enumerations.SearchParamType.TOKEN);
sp.setCode("gender");
sp.setExpression("Patient.gender");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.addBase(Enumerations.VersionIndependentResourceTypesAll.PATIENT);
mySearchParameterDao.update(sp, mySrd);
sp = new SearchParameter();
sp.setId("SearchParameter/patient-names-and-gender");
sp.setType(Enumerations.SearchParamType.COMPOSITE);
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.addBase(Enumerations.VersionIndependentResourceTypesAll.PATIENT);
sp.addComponent()
.setExpression("Patient")
.setDefinition("SearchParameter/patient-family");
sp.addComponent()
.setExpression("Patient")
.setDefinition("SearchParameter/patient-given");
sp.addComponent()
.setExpression("Patient")
.setDefinition("SearchParameter/patient-gender");
sp.addExtension()
.setUrl(HapiExtensions.EXT_SP_UNIQUE)
.setValue(new BooleanType(false));
mySearchParameterDao.update(sp, mySrd);
mySearchParamRegistry.forceRefresh();
}
}

View File

@ -307,7 +307,7 @@ public class InstanceReindexServiceImplNarrativeR5Test {
@Nonnull
private static ResourceIndexedSearchParams newParams() {
return new ResourceIndexedSearchParams();
return ResourceIndexedSearchParams.withSets();
}
}

View File

@ -70,23 +70,23 @@ public class Batch2JobHelper {
return awaitJobHasStatusWithoutMaintenancePass(theBatchJobId, StatusEnum.COMPLETED);
}
public JobInstance awaitJobCancelled(String theBatchJobId) {
return awaitJobHasStatus(theBatchJobId, StatusEnum.CANCELLED);
public JobInstance awaitJobCancelled(String theInstanceId) {
return awaitJobHasStatus(theInstanceId, StatusEnum.CANCELLED);
}
public JobInstance awaitJobCompletion(String theBatchJobId, int theSecondsToWait) {
return awaitJobHasStatus(theBatchJobId, theSecondsToWait, StatusEnum.COMPLETED);
public JobInstance awaitJobCompletion(String theInstanceId, int theSecondsToWait) {
return awaitJobHasStatus(theInstanceId, theSecondsToWait, StatusEnum.COMPLETED);
}
public JobInstance awaitJobHasStatus(String theBatchJobId, StatusEnum... theExpectedStatus) {
return awaitJobHasStatus(theBatchJobId, 10, theExpectedStatus);
public JobInstance awaitJobHasStatus(String theInstanceId, StatusEnum... theExpectedStatus) {
return awaitJobHasStatus(theInstanceId, 10, theExpectedStatus);
}
public JobInstance awaitJobHasStatusWithoutMaintenancePass(String theBatchJobId, StatusEnum... theExpectedStatus) {
return awaitJobawaitJobHasStatusWithoutMaintenancePass(theBatchJobId, 10, theExpectedStatus);
public JobInstance awaitJobHasStatusWithoutMaintenancePass(String theInstanceId, StatusEnum... theExpectedStatus) {
return awaitJobawaitJobHasStatusWithoutMaintenancePass(theInstanceId, 10, theExpectedStatus);
}
public JobInstance awaitJobHasStatus(String theBatchJobId, int theSecondsToWait, StatusEnum... theExpectedStatus) {
public JobInstance awaitJobHasStatus(String theInstanceId, int theSecondsToWait, StatusEnum... theExpectedStatus) {
assert !TransactionSynchronizationManager.isActualTransactionActive();
try {
@ -95,12 +95,12 @@ public class Batch2JobHelper {
.until(() -> {
boolean inFinalStatus = false;
if (ArrayUtils.contains(theExpectedStatus, StatusEnum.COMPLETED) && !ArrayUtils.contains(theExpectedStatus, StatusEnum.FAILED)) {
inFinalStatus = hasStatus(theBatchJobId, StatusEnum.FAILED);
inFinalStatus = hasStatus(theInstanceId, StatusEnum.FAILED);
}
if (ArrayUtils.contains(theExpectedStatus, StatusEnum.FAILED) && !ArrayUtils.contains(theExpectedStatus, StatusEnum.COMPLETED)) {
inFinalStatus = hasStatus(theBatchJobId, StatusEnum.COMPLETED);
inFinalStatus = hasStatus(theInstanceId, StatusEnum.COMPLETED);
}
boolean retVal = checkStatusWithMaintenancePass(theBatchJobId, theExpectedStatus);
boolean retVal = checkStatusWithMaintenancePass(theInstanceId, theExpectedStatus);
if (!retVal && inFinalStatus) {
// Fail fast - If we hit one of these statuses and it's not the one we want, abort
throw new ConditionTimeoutException("Already in failed/completed status");
@ -112,10 +112,10 @@ public class Batch2JobHelper {
.stream()
.map(t -> t.getInstanceId() + " " + t.getJobDefinitionId() + "/" + t.getStatus().name())
.collect(Collectors.joining("\n"));
String currentStatus = myJobCoordinator.getInstance(theBatchJobId).getStatus().name();
fail("Job " + theBatchJobId + " still has status " + currentStatus + " - All statuses:\n" + statuses);
String currentStatus = myJobCoordinator.getInstance(theInstanceId).getStatus().name();
fail("Job " + theInstanceId + " still has status " + currentStatus + " - All statuses:\n" + statuses);
}
return myJobCoordinator.getInstance(theBatchJobId);
return myJobCoordinator.getInstance(theInstanceId);
}
public JobInstance awaitJobawaitJobHasStatusWithoutMaintenancePass(String theBatchJobId, int theSecondsToWait, StatusEnum... theExpectedStatus) {
@ -136,34 +136,34 @@ public class Batch2JobHelper {
return myJobCoordinator.getInstance(theBatchJobId);
}
private boolean checkStatusWithMaintenancePass(String theBatchJobId, StatusEnum... theExpectedStatuses) throws InterruptedException {
if (hasStatus(theBatchJobId, theExpectedStatuses)) {
private boolean checkStatusWithMaintenancePass(String theInstanceId, StatusEnum... theExpectedStatuses) throws InterruptedException {
if (hasStatus(theInstanceId, theExpectedStatuses)) {
return true;
}
myJobMaintenanceService.runMaintenancePass();
return hasStatus(theBatchJobId, theExpectedStatuses);
return hasStatus(theInstanceId, theExpectedStatuses);
}
private boolean hasStatus(String theBatchJobId, StatusEnum... theExpectedStatuses) {
StatusEnum status = getStatus(theBatchJobId);
ourLog.debug("Checking status of {} in {}: is {}", theBatchJobId, theExpectedStatuses, status);
private boolean hasStatus(String theInstanceId, StatusEnum... theExpectedStatuses) {
StatusEnum status = getStatus(theInstanceId);
ourLog.debug("Checking status of {} in {}: is {}", theInstanceId, theExpectedStatuses, status);
return ArrayUtils.contains(theExpectedStatuses, status);
}
private StatusEnum getStatus(String theBatchJobId) {
return myJobCoordinator.getInstance(theBatchJobId).getStatus();
private StatusEnum getStatus(String theInstanceId) {
return myJobCoordinator.getInstance(theInstanceId).getStatus();
}
public JobInstance awaitJobFailure(Batch2JobStartResponse theStartResponse) {
return awaitJobFailure(theStartResponse.getInstanceId());
}
public JobInstance awaitJobFailure(String theJobId) {
return awaitJobHasStatus(theJobId, StatusEnum.ERRORED, StatusEnum.FAILED);
public JobInstance awaitJobFailure(String theInstanceId) {
return awaitJobHasStatus(theInstanceId, StatusEnum.ERRORED, StatusEnum.FAILED);
}
public void awaitJobInProgress(String theBatchJobId) {
await().until(() -> checkStatusWithMaintenancePass(theBatchJobId, StatusEnum.IN_PROGRESS));
public void awaitJobInProgress(String theInstanceId) {
await().until(() -> checkStatusWithMaintenancePass(theInstanceId, StatusEnum.IN_PROGRESS));
}
public void assertNotFastTracking(String theInstanceId) {
@ -178,8 +178,8 @@ public class Batch2JobHelper {
await().until(() -> theExpectedGatedStepId.equals(myJobCoordinator.getInstance(theInstanceId).getCurrentGatedStepId()));
}
public long getCombinedRecordsProcessed(String theJobId) {
JobInstance job = myJobCoordinator.getInstance(theJobId);
public long getCombinedRecordsProcessed(String theInstanceId) {
JobInstance job = myJobCoordinator.getInstance(theInstanceId);
return job.getCombinedRecordsProcessed();
}

View File

@ -48,7 +48,7 @@ class ExtendedHSearchIndexExtractorTest implements ITestDataBuilder.WithSupport
valueParams.add(new ResourceIndexedSearchParamToken(new PartitionSettings(), "Observation", "component-value-concept", "https://example.com", "some_other_value"));
composite.addComponentIndexedSearchParams("component-value-concept", RestSearchParameterTypeEnum.TOKEN, valueParams);
ResourceIndexedSearchParams extractedParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams extractedParams = ResourceIndexedSearchParams.withSets();
extractedParams.myCompositeParams.add(composite);
// run: now translate to HSearch
@ -65,7 +65,7 @@ class ExtendedHSearchIndexExtractorTest implements ITestDataBuilder.WithSupport
@Test
void testExtract_withParamMarkedAsMissing_willBeIgnored() {
//setup
ResourceIndexedSearchParams searchParams = new ResourceIndexedSearchParams();
ResourceIndexedSearchParams searchParams = ResourceIndexedSearchParams.withSets();
ResourceIndexedSearchParamDate searchParamDate = new ResourceIndexedSearchParamDate(new PartitionSettings(), "SearchParameter", "Date", null, null, null, null, null);
searchParamDate.setMissing(true);
searchParams.myDateParams.add(searchParamDate);

View File

@ -1836,7 +1836,7 @@ public abstract class BaseTransactionProcessor {
.filter(t -> t.getEntity().getDeleted() == null)
.filter(t -> t.getResource() != null)
.forEach(t -> resourceToIndexedParams.put(
t.getResource(), new ResourceIndexedSearchParams((ResourceTable) t.getEntity())));
t.getResource(), ResourceIndexedSearchParams.withLists((ResourceTable) t.getEntity())));
for (Map.Entry<String, Class<? extends IBaseResource>> nextEntry : conditionalRequestUrls.entrySet()) {
String matchUrl = nextEntry.getKey();