From abf76a778f790bc953ee979a50979d649a22c775 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 8 Jan 2018 07:15:31 -0500 Subject: [PATCH] Add some tests for unique search params --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 32 ++- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 27 +- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 102 ++++--- .../ResourceIndexedCompositeStringUnique.java | 34 ++- ...hirResourceDaoR4UniqueSearchParamTest.java | 269 ++++++++++++++++++ .../src/test/resources/logback-test.xml | 1 - 6 files changed, 379 insertions(+), 86 deletions(-) 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 84eec7e4daf..bfc0d827e7d 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 @@ -181,12 +181,6 @@ public abstract class BaseHapiFhirDao implements IDao { } } - InstantDt createHistoryToTimestamp() { - // final InstantDt end = new InstantDt(DateUtils.addSeconds(DateUtils.truncate(new Date(), Calendar.SECOND), - // -1)); - return InstantDt.withCurrentTime(); - } - private Set extractCompositeStringUniques(ResourceTable theEntity, Set theStringParams, Set theTokenParams, Set theNumberParams, Set theQuantityParams, Set theDateParams, Set theUriParams, Set theLinks) { Set compositeStringUniques; compositeStringUniques = new HashSet<>(); @@ -198,6 +192,7 @@ public abstract class BaseHapiFhirDao implements IDao { for (RuntimeSearchParam nextCompositeOf : next.getCompositeOf()) { Set paramsListForCompositePart = null; Set linksForCompositePart = null; + Set linksForCompositePartWantPaths = null; switch (nextCompositeOf.getParamType()) { case NUMBER: paramsListForCompositePart = theNumberParams; @@ -213,6 +208,8 @@ public abstract class BaseHapiFhirDao implements IDao { break; case REFERENCE: linksForCompositePart = theLinks; + linksForCompositePartWantPaths = new HashSet<>(); + linksForCompositePartWantPaths.addAll(nextCompositeOf.getPathsSplit()); break; case QUANTITY: paramsListForCompositePart = theQuantityParams; @@ -243,10 +240,12 @@ public abstract class BaseHapiFhirDao implements IDao { } if (linksForCompositePart != null) { for (ResourceLink nextLink : linksForCompositePart) { - String value = nextLink.getTargetResource().getIdDt().toUnqualifiedVersionless().getValue(); - if (isNotBlank(value)) { - value = UrlUtil.escapeUrlParam(value); - nextChoicesList.add(key + "=" + value); + if (linksForCompositePartWantPaths.contains(nextLink.getSourcePath())) { + String value = nextLink.getTargetResource().getIdDt().toUnqualifiedVersionless().getValue(); + if (isNotBlank(value)) { + value = UrlUtil.escapeUrlParam(value); + nextChoicesList.add(key + "=" + value); + } } } } @@ -942,7 +941,7 @@ public abstract class BaseHapiFhirDao implements IDao { break; } - ourLog.info("Encoded {} chars of resource body as {} bytes", encoded.length(), bytes.length); + ourLog.debug("Encoded {} chars of resource body as {} bytes", encoded.length(), bytes.length); boolean changed = false; @@ -1615,7 +1614,13 @@ public abstract class BaseHapiFhirDao implements IDao { * Create history entry */ if (theCreateNewHistoryEntry) { - final ResourceHistoryTable historyEntry = theEntity.toHistory(null); + + ResourceHistoryTable existing = null; +// if (theEntity.getVersion() > 1) { +// existing = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion()); +// ourLog.warn("Reusing existing history entry entity {}", theEntity.getIdDt().getValue()); +// } + final ResourceHistoryTable historyEntry = theEntity.toHistory(existing); ourLog.info("Saving history entry {}", historyEntry.getIdDt()); myResourceHistoryTableDao.save(historyEntry); @@ -1692,6 +1697,7 @@ public abstract class BaseHapiFhirDao implements IDao { if (getConfig().isUniqueIndexesEnabled()) { for (ResourceIndexedCompositeStringUnique next : existingCompositeStringUniques) { if (!compositeStringUniques.contains(next)) { + ourLog.info("Removing unique index: {}", next); myEntityManager.remove(next); } } @@ -1703,7 +1709,7 @@ public abstract class BaseHapiFhirDao implements IDao { throw new PreconditionFailedException("Can not create resource of type " + theEntity.getResourceType() + " as it would create a duplicate index matching query: " + next.getIndexString() + " (existing index belongs to " + existing.getResource().getIdDt().toUnqualifiedVersionless().getValue() + ")"); } } - ourLog.debug("Persisting unique index: {}", next); + ourLog.info("Persisting unique index: {}", next); myEntityManager.persist(next); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 31acf109abe..4d679e369c8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -593,22 +593,23 @@ public abstract class BaseHapiFhirResourceDao extends B } protected void markResourcesMatchingExpressionAsNeedingReindexing(String theExpression) { - if (isNotBlank(theExpression)) { - final String resourceType = theExpression.substring(0, theExpression.indexOf('.')); - ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", resourceType, theExpression); + if (myDaoConfig.isMarkResourcesForReindexingUponSearchParameterChange()) { + if (isNotBlank(theExpression)) { + final String resourceType = theExpression.substring(0, theExpression.indexOf('.')); + ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", resourceType, theExpression); - TransactionTemplate txTemplate = new TransactionTemplate(myPlatformTransactionManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); - int updatedCount = txTemplate.execute(new TransactionCallback() { - @Override - public Integer doInTransaction(TransactionStatus theStatus) { - return myResourceTableDao.markResourcesOfTypeAsRequiringReindexing(resourceType); - } - }); + TransactionTemplate txTemplate = new TransactionTemplate(myPlatformTransactionManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED); + int updatedCount = txTemplate.execute(new TransactionCallback() { + @Override + public Integer doInTransaction(TransactionStatus theStatus) { + return myResourceTableDao.markResourcesOfTypeAsRequiringReindexing(resourceType); + } + }); - ourLog.info("Marked {} resources for reindexing", updatedCount); + ourLog.info("Marked {} resources for reindexing", updatedCount); + } } - mySearchParamRegistry.forceRefresh(); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 06800ab8dde..03d0318e49b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -17,9 +17,9 @@ import java.util.*; * 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. @@ -109,6 +109,7 @@ public class DaoConfig { private Integer myCacheControlNoStoreMaxResultsUpperLimit = 1000; private Integer myCountSearchResultsUpTo = null; private IdStrategyEnum myResourceServerIdStrategy = IdStrategyEnum.SEQUENTIAL_NUMERIC; + private boolean myMarkResourcesForReindexingUponSearchParameterChange; /** * Constructor @@ -117,6 +118,7 @@ public class DaoConfig { setSubscriptionEnabled(true); setSubscriptionPollDelay(0); setSubscriptionPurgeInactiveAfterMillis(Long.MAX_VALUE); + setMarkResourcesForReindexingUponSearchParameterChange(true); } /** @@ -340,18 +342,6 @@ public class DaoConfig { myHardTagListLimit = theHardTagListLimit; } - /** - * This is the maximum number of resources that will be added to a single page of returned resources. Because of - * includes with wildcards and other possibilities it is possible for a client to make requests that include very - * large amounts of data, so this hard limit can be imposed to prevent runaway requests. - * - * @deprecated Deprecated in HAPI FHIR 3.2.0 as this method doesn't actually do anything - */ - @Deprecated - public void setIncludeLimit(@SuppressWarnings("unused") int theIncludeLimit) { - // nothing - } - /** * If set to {@link IndexEnabledEnum#DISABLED} (default is {@link IndexEnabledEnum#DISABLED}) * the server will not create search indexes for search parameters with no values in resources. @@ -404,11 +394,8 @@ public class DaoConfig { /** * This may be used to optionally register server interceptors directly against the DAOs. */ - public void setInterceptors(IServerInterceptor... theInterceptor) { - setInterceptors(new ArrayList()); - if (theInterceptor != null && theInterceptor.length != 0) { - getInterceptors().addAll(Arrays.asList(theInterceptor)); - } + public void setInterceptors(List theInterceptors) { + myInterceptors = theInterceptors; } /** @@ -465,25 +452,6 @@ public class DaoConfig { myResourceEncoding = theResourceEncoding; } - /** - * This setting configures the strategy to use in generating IDs for newly - * created resources on the server. The default is {@link IdStrategyEnum#SEQUENTIAL_NUMERIC}. - */ - public IdStrategyEnum getResourceServerIdStrategy() { - return myResourceServerIdStrategy; - } - - /** - * This setting configures the strategy to use in generating IDs for newly - * created resources on the server. The default is {@link IdStrategyEnum#SEQUENTIAL_NUMERIC}. - * - * @param theResourceIdStrategy The strategy. Must not be null. - */ - public void setResourceServerIdStrategy(IdStrategyEnum theResourceIdStrategy) { - Validate.notNull(theResourceIdStrategy, "theResourceIdStrategy must not be null"); - myResourceServerIdStrategy = theResourceIdStrategy; - } - /** * If set, an individual resource will not be allowed to have more than the * given number of tags, profiles, and security labels (the limit is for the combined @@ -514,6 +482,25 @@ public class DaoConfig { myResourceMetaCountHardLimit = theResourceMetaCountHardLimit; } + /** + * This setting configures the strategy to use in generating IDs for newly + * created resources on the server. The default is {@link IdStrategyEnum#SEQUENTIAL_NUMERIC}. + */ + public IdStrategyEnum getResourceServerIdStrategy() { + return myResourceServerIdStrategy; + } + + /** + * This setting configures the strategy to use in generating IDs for newly + * created resources on the server. The default is {@link IdStrategyEnum#SEQUENTIAL_NUMERIC}. + * + * @param theResourceIdStrategy The strategy. Must not be null. + */ + public void setResourceServerIdStrategy(IdStrategyEnum theResourceIdStrategy) { + Validate.notNull(theResourceIdStrategy, "theResourceIdStrategy must not be null"); + myResourceServerIdStrategy = theResourceIdStrategy; + } + /** * If set to a non {@literal null} value (default is {@link #DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS non null}) * if an identical search is requested multiple times within this window, the same results will be returned @@ -922,6 +909,24 @@ public class DaoConfig { myIndexContainedResources = theIndexContainedResources; } + /** + * Should resources be marked as needing reindexing when a + * SearchParameter resource is added or changed. This should generally + * be true (which is the default) + */ + public boolean isMarkResourcesForReindexingUponSearchParameterChange() { + return myMarkResourcesForReindexingUponSearchParameterChange; + } + + /** + * Should resources be marked as needing reindexing when a + * SearchParameter resource is added or changed. This should generally + * be true (which is the default) + */ + public void setMarkResourcesForReindexingUponSearchParameterChange(boolean theMarkResourcesForReindexingUponSearchParameterChange) { + myMarkResourcesForReindexingUponSearchParameterChange = theMarkResourcesForReindexingUponSearchParameterChange; + } + public boolean isSchedulingDisabled() { return mySchedulingDisabled; } @@ -979,7 +984,7 @@ public class DaoConfig { * a new one. *

* This causes friendlier error messages to be generated, but adds an - * extra round-trip to the database for eavh save so it can cause + * extra round-trip to the database for each save so it can cause * a small performance hit. *

*/ @@ -1022,11 +1027,26 @@ public class DaoConfig { // this method does nothing } + /** + * This is the maximum number of resources that will be added to a single page of returned resources. Because of + * includes with wildcards and other possibilities it is possible for a client to make requests that include very + * large amounts of data, so this hard limit can be imposed to prevent runaway requests. + * + * @deprecated Deprecated in HAPI FHIR 3.2.0 as this method doesn't actually do anything + */ + @Deprecated + public void setIncludeLimit(@SuppressWarnings("unused") int theIncludeLimit) { + // nothing + } + /** * This may be used to optionally register server interceptors directly against the DAOs. */ - public void setInterceptors(List theInterceptors) { - myInterceptors = theInterceptors; + public void setInterceptors(IServerInterceptor... theInterceptor) { + setInterceptors(new ArrayList()); + if (theInterceptor != null && theInterceptor.length != 0) { + getInterceptors().addAll(Arrays.asList(theInterceptor)); + } } /** diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceIndexedCompositeStringUnique.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceIndexedCompositeStringUnique.java index edb2a049a63..228a7765baa 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceIndexedCompositeStringUnique.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceIndexedCompositeStringUnique.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.entity; * 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. @@ -22,7 +22,6 @@ package ca.uhn.fhir.jpa.entity; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.*; -import org.hl7.fhir.r4.model.Resource; import javax.persistence.*; @@ -44,20 +43,10 @@ public class ResourceIndexedCompositeStringUnique implements Comparable all = myResourceIndexedCompositeStringUniqueDao.findAll(); + assertEquals(2, all.size()); + } + }); + + } + + @Test + public void testObservationSubject() { + createUniqueIndexObservationSubject(); + + Patient pt = new Patient(); + pt.addIdentifier().setSystem("urn").setValue("111"); + pt.addIdentifier().setSystem("urn").setValue("222"); + IIdType ptid = myPatientDao.create(pt).getId().toUnqualifiedVersionless(); + + Encounter enc = new Encounter(); + enc.setSubject(new Reference(ptid)); + IIdType encid = myEncounterDao.create(enc).getId().toUnqualifiedVersionless(); + + Observation obs = new Observation(); + obs.setSubject(new Reference(ptid)); + obs.setContext(new Reference(encid)); + myObservationDao.create(obs); + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) { + List all = myResourceIndexedCompositeStringUniqueDao.findAll(); + assertEquals(all.toString(), 1, all.size()); + } + }); + + }er + + @Test + public void testIndexFirstMatchOnly() { + createUniqueIndexPatientIdentifierCount1(); + + Patient pt = new Patient(); + pt.addIdentifier().setSystem("urn").setValue("111"); + pt.addIdentifier().setSystem("urn").setValue("222"); + myPatientDao.create(pt); + + pt = new Patient(); + pt.addIdentifier().setSystem("urn").setValue("111"); + pt.addIdentifier().setSystem("urn").setValue("222"); + try { + myPatientDao.create(pt); + fail(); + } catch (PreconditionFailedException e) { + // good + } + + pt = new Patient(); + pt.addIdentifier().setSystem("urn").setValue("333"); + pt.addIdentifier().setSystem("urn").setValue("222"); + myPatientDao.create(pt); + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) { + List all = myResourceIndexedCompositeStringUniqueDao.findAll(); + assertEquals(2, all.size()); + } + }); + + } + + + @Test + public void testDoubleMatchingPut() { + createUniqueIndexPatientIdentifier(); + + Patient pt = new Patient(); + pt.addIdentifier().setSystem("urn").setValue("111"); + pt.addIdentifier().setSystem("urn").setValue("222"); + + Bundle input = new Bundle(); + input.setType(Bundle.BundleType.TRANSACTION); + input.addEntry() + .setResource(pt) + .setFullUrl("Patient") + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Patient?identifier=urn|111,urn|222"); + mySystemDao.transaction(mySrd, input); + + myEntityManager.clear(); + + pt = new Patient(); + pt.setActive(true); + pt.addIdentifier().setSystem("urn").setValue("111"); + pt.addIdentifier().setSystem("urn").setValue("222"); + + input = new Bundle(); + input.setType(Bundle.BundleType.TRANSACTION); + input.addEntry() + .setResource(pt) + .setFullUrl("Patient") + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Patient?identifier=urn|111,urn|222"); + mySystemDao.transaction(mySrd, input); + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus status) { + List all = myResourceIndexedCompositeStringUniqueDao.findAll(); + assertEquals(2, all.size()); + } + }); + + } + @Test public void testIndexTransactionWithMatchUrl2() { createUniqueIndexCoverageBeneficiary(); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index d5b2da01e7d..91f8a74d3e2 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -1,6 +1,5 @@ - INFO