From 73924199a9a169aa79d0d889acdcf05e5a883f84 Mon Sep 17 00:00:00 2001 From: James Date: Sun, 5 Feb 2017 20:34:12 -0500 Subject: [PATCH 01/10] Work on custom search params --- .../fhir/jpa/dao/BaseHapiFhirSystemDao.java | 22 ++- .../dstu3/FhirResourceDaoCodeSystemDstu3.java | 24 ++- .../fhir/jpa/term/BaseHapiTerminologySvc.java | 174 +++++++++--------- .../FhirResourceDaoDstu3CodeSystemTest.java | 44 +++++ .../FhirResourceDaoDstu3ContainedTest.java | 6 - .../resources/dstu3_codesystem_complete.json | 27 +++ 6 files changed, 200 insertions(+), 97 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3CodeSystemTest.java create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/dstu3_codesystem_complete.json diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java index 1eca2daf5b5..c1770e5cb03 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import javax.persistence.Query; import javax.persistence.Tuple; @@ -64,15 +65,17 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao extends BaseHapiFhirDao extends BaseHapiFhirDao theSetToPopulate, TermConcept theConcept) { boolean retVal = theSetToPopulate.add(theConcept); @@ -108,6 +113,25 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { return retVal; } + private int ensureParentsSaved(Collection theParents) { + ourLog.trace("Checking {} parents", theParents.size()); + int retVal = 0; + + for (TermConceptParentChildLink nextLink : theParents) { + if (nextLink.getRelationshipType() == RelationshipTypeEnum.ISA) { + TermConcept nextParent = nextLink.getParent(); + retVal += ensureParentsSaved(nextParent.getParents()); + if (nextParent.getId() == null) { + myConceptDao.saveAndFlush(nextParent); + retVal++; + ourLog.debug("Saved parent code {} and got id {}", nextParent.getCode(), nextParent.getId()); + } + } + } + + return retVal; + } + private void fetchChildren(TermConcept theConcept, Set theSetToPopulate) { for (TermConceptParentChildLink nextChildLink : theConcept.getChildren()) { TermConcept nextChild = nextChildLink.getChild(); @@ -175,6 +199,15 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { return retVal; } + /** + * Subclasses may override + * @param theSystem The code system + * @param theCode The code + */ + protected List findCodesAboveUsingBuiltInSystems(String theSystem, String theCode) { + return Collections.emptyList(); + } + @Transactional(propagation = Propagation.REQUIRED) @Override public Set findCodesBelow(Long theCodeSystemResourcePid, Long theCodeSystemVersionPid, String theCode) { @@ -206,7 +239,6 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { ArrayList retVal = toVersionIndependentConcepts(theSystem, codes); return retVal; } - /** * Subclasses may override * @param theSystem The code system @@ -215,16 +247,7 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { protected List findCodesBelowUsingBuiltInSystems(String theSystem, String theCode) { return Collections.emptyList(); } - - /** - * Subclasses may override - * @param theSystem The code system - * @param theCode The code - */ - protected List findCodesAboveUsingBuiltInSystems(String theSystem, String theCode) { - return Collections.emptyList(); - } - + private TermCodeSystemVersion findCurrentCodeSystemVersionForSystem(String theCodeSystem) { TermCodeSystem cs = getCodeSystem(theCodeSystem); if (cs == null || cs.getCurrentVersion() == null) { @@ -233,11 +256,12 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { TermCodeSystemVersion csv = cs.getCurrentVersion(); return csv; } + private TermCodeSystem getCodeSystem(String theSystem) { TermCodeSystem cs = myCodeSystemDao.findByCodeSystemUri(theSystem); return cs; } - + private void persistChildren(TermConcept theConcept, TermCodeSystemVersion theCodeSystem, IdentityHashMap theConceptsStack, int theTotalConcepts) { if (theConceptsStack.put(theConcept, PLACEHOLDER_OBJECT) != null) { return; @@ -271,10 +295,47 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { } - private void saveConceptLink(TermConceptParentChildLink next) { - if (next.getId() == null) { - myConceptParentChildLinkDao.save(next); + private void populateVersion(TermConcept theNext, TermCodeSystemVersion theCodeSystemVersion) { + if (theNext.getCodeSystem() != null) { + return; } + theNext.setCodeSystem(theCodeSystemVersion); + for (TermConceptParentChildLink next : theNext.getChildren()) { + populateVersion(next.getChild(), theCodeSystemVersion); + } + } + + private void processReindexing() { + if (System.currentTimeMillis() < myNextReindexPass && !ourForceSaveDeferredAlwaysForUnitTest) { + return; + } + + TransactionTemplate tt = new TransactionTemplate(myTransactionMgr); + tt.setPropagationBehavior(TransactionTemplate.PROPAGATION_REQUIRES_NEW); + tt.execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theArg0) { + int maxResult = 1000; + Page resources = myConceptDao.findResourcesRequiringReindexing(new PageRequest(0, maxResult)); + if (resources.hasContent() == false) { + myNextReindexPass = System.currentTimeMillis() + DateUtils.MILLIS_PER_MINUTE; + return; + } + + ourLog.info("Indexing {} / {} concepts", resources.getContent().size(), resources.getTotalElements()); + + int count = 0; + StopWatch stopwatch = new StopWatch(); + + for (TermConcept resourceTable : resources) { + saveConcept(resourceTable); + count++; + } + + ourLog.info("Indexed {} / {} concepts in {}ms - Avg {}ms / resource", new Object[] { count, resources.getContent().size(), stopwatch.getMillis(), stopwatch.getMillisPerOperation(count) }); + } + }); + } private int saveConcept(TermConcept theConcept) { @@ -289,33 +350,10 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { ourLog.trace("Saved {} and got PID {}", theConcept.getCode(), theConcept.getId()); return retVal; } - - private int ensureParentsSaved(Collection theParents) { - ourLog.trace("Checking {} parents", theParents.size()); - int retVal = 0; - - for (TermConceptParentChildLink nextLink : theParents) { - if (nextLink.getRelationshipType() == RelationshipTypeEnum.ISA) { - TermConcept nextParent = nextLink.getParent(); - retVal += ensureParentsSaved(nextParent.getParents()); - if (nextParent.getId() == null) { - myConceptDao.saveAndFlush(nextParent); - retVal++; - ourLog.debug("Saved parent code {} and got id {}", nextParent.getCode(), nextParent.getId()); - } - } - } - - return retVal; - } - - private void populateVersion(TermConcept theNext, TermCodeSystemVersion theCodeSystemVersion) { - if (theNext.getCodeSystem() != null) { - return; - } - theNext.setCodeSystem(theCodeSystemVersion); - for (TermConceptParentChildLink next : theNext.getChildren()) { - populateVersion(next.getChild(), theCodeSystemVersion); + + private void saveConceptLink(TermConceptParentChildLink next) { + if (next.getId() == null) { + myConceptParentChildLinkDao.save(next); } } @@ -368,43 +406,7 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { ourLog.info("All deferred concepts and relationships have now been synchronized to the database"); } } - - @Autowired - private PlatformTransactionManager myTransactionMgr; - private void processReindexing() { - if (System.currentTimeMillis() < myNextReindexPass) { - return; - } - - TransactionTemplate tt = new TransactionTemplate(myTransactionMgr); - tt.setPropagationBehavior(TransactionTemplate.PROPAGATION_REQUIRES_NEW); - tt.execute(new TransactionCallbackWithoutResult() { - @Override - protected void doInTransactionWithoutResult(TransactionStatus theArg0) { - int maxResult = 1000; - Page resources = myConceptDao.findResourcesRequiringReindexing(new PageRequest(0, maxResult)); - if (resources.hasContent() == false) { - myNextReindexPass = System.currentTimeMillis() + DateUtils.MILLIS_PER_MINUTE; - return; - } - - ourLog.info("Indexing {} / {} concepts", resources.getContent().size(), resources.getTotalElements()); - - int count = 0; - StopWatch stopwatch = new StopWatch(); - - for (TermConcept resourceTable : resources) { - saveConcept(resourceTable); - count++; - } - - ourLog.info("Indexed {} / {} concepts in {}ms - Avg {}ms / resource", new Object[] { count, resources.getContent().size(), stopwatch.getMillis(), stopwatch.getMillisPerOperation(count) }); - } - }); - - } - @Override public void setProcessDeferred(boolean theProcessDeferred) { myProcessDeferred = theProcessDeferred; @@ -470,7 +472,7 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { totalCodeCount += validateConceptForStorage(next, theCodeSystemVersion, conceptsStack, allConcepts); } - ourLog.info("Saving version"); + ourLog.info("Saving version containing {} concepts", totalCodeCount); TermCodeSystemVersion codeSystemVersion = myCodeSystemVersionDao.saveAndFlush(theCodeSystemVersion); @@ -503,13 +505,13 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { ourLog.info("Note that some concept saving was deferred - still have {} concepts and {} relationships", myConceptsToSaveLater.size(), myConceptLinksToSaveLater.size()); } } - + @Override public boolean supportsSystem(String theSystem) { TermCodeSystem cs = getCodeSystem(theSystem); return cs != null; } - + private ArrayList toVersionIndependentConcepts(String theSystem, Set codes) { ArrayList retVal = new ArrayList(codes.size()); for (TermConcept next : codes) { @@ -547,4 +549,12 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { return retVal; } + /** + * This method is present only for unit tests, do not call from client code + */ + @VisibleForTesting + public static void setForceSaveDeferredAlwaysForUnitTest(boolean theForceSaveDeferredAlwaysForUnitTest) { + ourForceSaveDeferredAlwaysForUnitTest = theForceSaveDeferredAlwaysForUnitTest; + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3CodeSystemTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3CodeSystemTest.java new file mode 100644 index 00000000000..f86a509728a --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3CodeSystemTest.java @@ -0,0 +1,44 @@ +package ca.uhn.fhir.jpa.dao.dstu3; + +import static org.junit.Assert.assertNotEquals; + +import java.nio.charset.StandardCharsets; + +import org.apache.commons.io.IOUtils; +import org.hl7.fhir.dstu3.model.CodeSystem; +import org.junit.AfterClass; +import org.junit.Test; + +import ca.uhn.fhir.jpa.term.BaseHapiTerminologySvc; +import ca.uhn.fhir.util.TestUtil; + +public class FhirResourceDaoDstu3CodeSystemTest extends BaseJpaDstu3Test { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoDstu3CodeSystemTest.class); + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + BaseHapiTerminologySvc.setForceSaveDeferredAlwaysForUnitTest(false); + } + + + @Test + public void testIndexContained() throws Exception { + BaseHapiTerminologySvc.setForceSaveDeferredAlwaysForUnitTest(true); + + String input = IOUtils.toString(getClass().getResource("/dstu3_codesystem_complete.json"), StandardCharsets.UTF_8); + CodeSystem cs = myFhirCtx.newJsonParser().parseResource(CodeSystem.class, input); + myCodeSystemDao.create(cs, mySrd); + + + mySystemDao.markAllResourcesForReindexing(); + + int outcome = mySystemDao.performReindexingPass(100); + assertNotEquals(-1, outcome); // -1 means there was a failure + + myTermSvc.saveDeferred(); + + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3ContainedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3ContainedTest.java index ce164bf8e04..cd2c9c8fcc7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3ContainedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3ContainedTest.java @@ -1,8 +1,5 @@ package ca.uhn.fhir.jpa.dao.dstu3; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.junit.Assert.assertThat; - import org.hl7.fhir.dstu3.model.Observation; import org.hl7.fhir.dstu3.model.Patient; import org.hl7.fhir.dstu3.model.Reference; @@ -11,9 +8,6 @@ import org.junit.AfterClass; import org.junit.Test; import ca.uhn.fhir.jpa.dao.SearchParameterMap; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.rest.param.TokenParamModifier; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.TestUtil; public class FhirResourceDaoDstu3ContainedTest extends BaseJpaDstu3Test { diff --git a/hapi-fhir-jpaserver-base/src/test/resources/dstu3_codesystem_complete.json b/hapi-fhir-jpaserver-base/src/test/resources/dstu3_codesystem_complete.json new file mode 100644 index 00000000000..003d30b7bb3 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/dstu3_codesystem_complete.json @@ -0,0 +1,27 @@ +{ + "resourceType": "CodeSystem", + "id": "22472", + "meta": { + "versionId": "3", + "lastUpdated": "2017-01-27T10:53:39.457-05:00" + }, + "url": "xvalue://dedalus.eu/mci/CodeSystem/AddressUse", + "name": "AddressUse", + "status": "active", + "publisher": "x1v1-mci", + "date": "2016-04-07", + "description": "AddressUse", + "caseSensitive": true, + "compositional": false, + "versionNeeded": false, + "content": "complete", + "concept": { + "code": "work", + "display": "Work", + "definition": "An office address. First choice for business related contacts during business hours.", + "designation": { + "language": "IT", + "value": "Lavoro" + } + } +} \ No newline at end of file From 30df29c083849fedff6c2c53085a88b221639f81 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 5 Feb 2017 21:04:19 -0500 Subject: [PATCH 02/10] Work on custom params --- .../FhirResourceDaoSearchParameterDstu3.java | 24 +++++++++---------- .../dao/dstu3/SearchParamRegistryDstu3.java | 2 +- ...ceDaoDstu3SearchCustomSearchParamTest.java | 22 ++++++++--------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java index 82fea85443d..a383770a7ae 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java @@ -45,9 +45,9 @@ public class FhirResourceDaoSearchParameterDstu3 extends FhirResourceDaoDstu3 mySystemDao; private void markAffectedResources(SearchParameter theResource) { - String xpath = theResource.getXpath(); - String resourceType = xpath.substring(0, xpath.indexOf('.')); - ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", xpath); + String expression = theResource.getExpression(); + String resourceType = expression.substring(0, expression.indexOf('.')); + ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", expression); int updatedCount = myResourceTableDao.markResourcesOfTypeAsRequiringReindexing(resourceType); ourLog.info("Marked {} resources for reindexing", updatedCount); } @@ -92,34 +92,34 @@ public class FhirResourceDaoSearchParameterDstu3 extends FhirResourceDaoDstu3 Date: Sun, 5 Feb 2017 21:27:07 -0500 Subject: [PATCH 03/10] Work on custom params --- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 131 +++++++----------- .../FhirResourceDaoSearchParameterDstu3.java | 35 +++-- ...ceDaoDstu3SearchCustomSearchParamTest.java | 15 +- ...rceProviderCustomSearchParamDstu3Test.java | 19 +-- 4 files changed, 102 insertions(+), 98 deletions(-) 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 337e0ad8321..9f504f5d11b 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 @@ -22,29 +22,15 @@ package ca.uhn.fhir.jpa.dao; import static org.apache.commons.lang3.StringUtils.isNotBlank; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashSet; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Set; -import java.util.TreeSet; import javax.annotation.PostConstruct; import javax.persistence.NoResultException; import javax.persistence.TypedQuery; import org.hl7.fhir.dstu3.model.IdType; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBaseCoding; -import org.hl7.fhir.instance.model.api.IBaseMetaType; -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.IPrimitiveType; +import org.hl7.fhir.instance.model.api.*; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Required; import org.springframework.transaction.PlatformTransactionManager; @@ -59,42 +45,21 @@ import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.dao.data.ISearchResultDao; -import ca.uhn.fhir.jpa.entity.BaseHasResource; -import ca.uhn.fhir.jpa.entity.BaseTag; -import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; -import ca.uhn.fhir.jpa.entity.ResourceLink; -import ca.uhn.fhir.jpa.entity.ResourceTable; -import ca.uhn.fhir.jpa.entity.TagDefinition; -import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.interceptor.IJpaServerInterceptor; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.jpa.util.StopWatch; import ca.uhn.fhir.jpa.util.jsonpatch.JsonPatchUtils; import ca.uhn.fhir.jpa.util.xmlpatch.XmlPatchUtils; -import ca.uhn.fhir.model.api.IQueryParameterAnd; -import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.model.api.IResource; -import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; -import ca.uhn.fhir.model.api.TagList; +import ca.uhn.fhir.model.api.*; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.PatchTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; -import ca.uhn.fhir.rest.method.MethodUtil; -import ca.uhn.fhir.rest.method.QualifiedParamList; -import ca.uhn.fhir.rest.method.RequestDetails; -import ca.uhn.fhir.rest.method.RestSearchParameterTypeEnum; -import ca.uhn.fhir.rest.method.SearchMethodBinding; +import ca.uhn.fhir.rest.method.*; import ca.uhn.fhir.rest.method.SearchMethodBinding.QualifierDetails; import ca.uhn.fhir.rest.server.IBundleProvider; -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.ResourceNotFoundException; -import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; -import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.util.FhirTerser; @@ -110,14 +75,14 @@ public abstract class BaseHapiFhirResourceDao extends B @Autowired private DaoConfig myDaoConfig; @Autowired - protected IResourceTableDao myResourceTableDao; - @Autowired protected PlatformTransactionManager myPlatformTransactionManager; @Autowired private IResourceHistoryTableDao myResourceHistoryTableDao; @Autowired() protected IResourceIndexedSearchParamUriDao myResourceIndexedSearchParamUriDao; private String myResourceName; + @Autowired + protected IResourceTableDao myResourceTableDao; private Class myResourceType; @Autowired(required = false) protected IFulltextSearchSvc mySearchDao; @@ -226,7 +191,9 @@ public abstract class BaseHapiFhirResourceDao extends B T resourceToDelete = toResource(myResourceType, entity, false); validateOkToDelete(deleteConflicts, entity); - + + preDelete(resourceToDelete); + // Notify interceptors if (theRequestDetails != null) { ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getContext(), theId.getResourceType(), theId); @@ -262,7 +229,7 @@ public abstract class BaseHapiFhirResourceDao extends B ourLog.info("Processed delete on {} in {}ms", theId.getValue(), w.getMillisAndRestart()); return toMethodOutcome(savedEntity, null); } - + @Override public List deleteByUrl(String theUrl, List deleteConflicts, RequestDetails theRequestDetails) { Set resource = processMatchUrl(theUrl, myResourceType); @@ -301,7 +268,7 @@ public abstract class BaseHapiFhirResourceDao extends B return retVal; } - + @Override public DaoMethodOutcome deleteByUrl(String theUrl, RequestDetails theRequestDetails) { StopWatch w = new StopWatch(); @@ -553,6 +520,38 @@ public abstract class BaseHapiFhirResourceDao extends B return retVal; } + @Override + public MT metaDeleteOperation(IIdType theResourceId, MT theMetaDel, RequestDetails theRequestDetails) { + // Notify interceptors + ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), theResourceId); + notifyInterceptors(RestOperationTypeEnum.META_DELETE, requestDetails); + + StopWatch w = new StopWatch(); + BaseHasResource entity = readEntity(theResourceId); + if (entity == null) { + throw new ResourceNotFoundException(theResourceId); + } + + ResourceTable latestVersion = readEntityLatestVersion(theResourceId); + if (latestVersion.getVersion() != entity.getVersion()) { + doMetaDelete(theMetaDel, entity); + } else { + doMetaDelete(theMetaDel, latestVersion); + + // Also update history entry + ResourceHistoryTable history = myResourceHistoryTableDao.findForIdAndVersion(entity.getId(), entity.getVersion()); + doMetaDelete(theMetaDel, history); + } + + myEntityManager.flush(); + + ourLog.info("Processed metaDeleteOperation on {} in {}ms", new Object[] { theResourceId.getValue(), w.getMillisAndRestart() }); + + @SuppressWarnings("unchecked") + MT retVal = (MT) metaGetOperation(theMetaDel.getClass(), theResourceId, theRequestDetails); + return retVal; + } + // @Override // public IBundleProvider everything(IIdType theId) { // Search search = new Search(); @@ -635,38 +634,6 @@ public abstract class BaseHapiFhirResourceDao extends B // }; // } - @Override - public MT metaDeleteOperation(IIdType theResourceId, MT theMetaDel, RequestDetails theRequestDetails) { - // Notify interceptors - ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, getResourceName(), theResourceId); - notifyInterceptors(RestOperationTypeEnum.META_DELETE, requestDetails); - - StopWatch w = new StopWatch(); - BaseHasResource entity = readEntity(theResourceId); - if (entity == null) { - throw new ResourceNotFoundException(theResourceId); - } - - ResourceTable latestVersion = readEntityLatestVersion(theResourceId); - if (latestVersion.getVersion() != entity.getVersion()) { - doMetaDelete(theMetaDel, entity); - } else { - doMetaDelete(theMetaDel, latestVersion); - - // Also update history entry - ResourceHistoryTable history = myResourceHistoryTableDao.findForIdAndVersion(entity.getId(), entity.getVersion()); - doMetaDelete(theMetaDel, history); - } - - myEntityManager.flush(); - - ourLog.info("Processed metaDeleteOperation on {} in {}ms", new Object[] { theResourceId.getValue(), w.getMillisAndRestart() }); - - @SuppressWarnings("unchecked") - MT retVal = (MT) metaGetOperation(theMetaDel.getClass(), theResourceId, theRequestDetails); - return retVal; - } - @Override public MT metaGetOperation(Class theType, IIdType theId, RequestDetails theRequestDetails) { // Notify interceptors @@ -743,6 +710,14 @@ public abstract class BaseHapiFhirResourceDao extends B } + /** + * Subclasses may override to provide behaviour. Invoked within a delete + * transaction with the resource that is about to be deleted. + */ + protected void preDelete(T theResourceToDelete) { + // nothing by default + } + /** * May be overridden by subclasses to validate resources prior to storage * diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java index a383770a7ae..f816fe2f6d4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSearchParameterDstu3.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.jpa.dao.dstu3; import static org.apache.commons.lang3.StringUtils.isBlank; +import java.util.List; + /* * #%L * HAPI FHIR JPA Server @@ -33,6 +35,7 @@ import ca.uhn.fhir.jpa.dao.BaseSearchParamExtractor; import ca.uhn.fhir.jpa.dao.IFhirResourceDaoSearchParameter; import ca.uhn.fhir.jpa.dao.IFhirSystemDao; import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; @@ -45,13 +48,16 @@ public class FhirResourceDaoSearchParameterDstu3 extends FhirResourceDaoDstu3 mySystemDao; private void markAffectedResources(SearchParameter theResource) { - String expression = theResource.getExpression(); - String resourceType = expression.substring(0, expression.indexOf('.')); - ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", expression); - int updatedCount = myResourceTableDao.markResourcesOfTypeAsRequiringReindexing(resourceType); - ourLog.info("Marked {} resources for reindexing", updatedCount); + if (theResource != null) { + String expression = theResource.getExpression(); + String resourceType = expression.substring(0, expression.indexOf('.')); + ourLog.info("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", expression); + int updatedCount = myResourceTableDao.markResourcesOfTypeAsRequiringReindexing(resourceType); + ourLog.info("Marked {} resources for reindexing", updatedCount); + } } + /** * This method is called once per minute to perform any required re-indexing. During most passes this will * just check and find that there are no resources requiring re-indexing. In that case the method just returns @@ -77,13 +83,22 @@ public class FhirResourceDaoSearchParameterDstu3 extends FhirResourceDaoDstu3 Date: Mon, 6 Feb 2017 11:48:19 -0500 Subject: [PATCH 04/10] Add test for #551 --- .../uhn/fhir/parser/XmlParserDstu3Test.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java index 32921ead1ca..35db996195d 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java @@ -25,7 +25,9 @@ import java.nio.charset.StandardCharsets; import java.text.SimpleDateFormat; import java.util.*; +import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.StringUtils; import org.custommonkey.xmlunit.Diff; import org.custommonkey.xmlunit.XMLUnit; import org.hamcrest.collection.IsEmptyCollection; @@ -76,6 +78,24 @@ public class XmlParserDstu3Test { ourCtx.setNarrativeGenerator(null); } + /** + * See #551 + */ + @Test + public void testXmlLargeAttribute() { + String largeString = StringUtils.leftPad("", (int) FileUtils.ONE_MB, 'A'); + + Patient p = new Patient(); + p.addName().setFamily(largeString); + + String encoded = ourCtx.newXmlParser().encodeResourceToString(p); + + p = ourCtx.newXmlParser().parseResource(Patient.class, encoded); + + assertEquals(largeString, p.getNameFirstRep().getFamily()); + } + + /** * See #544 */ From 116cb1c8f76e609c3167ed2fd13626b5ae8a73e3 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Feb 2017 21:57:23 -0500 Subject: [PATCH 05/10] Work on repopulating --- .../java/ca/uhn/fhir/util/ParametersUtil.java | 2 + .../uhn/fhir/jpa/config/BaseDstu2Config.java | 1 + .../data/ITermConceptParentChildLinkDao.java | 5 + .../ca/uhn/fhir/jpa/entity/TermConcept.java | 18 ++- .../entity/TermConceptParentChildLink.java | 130 ++++++++++-------- .../fhir/jpa/term/BaseHapiTerminologySvc.java | 66 ++++++++- .../jpa/term/HapiTerminologySvcDstu3.java | 65 +++++---- .../FhirResourceDaoDstu3TerminologyTest.java | 71 ++++++++++ .../jpa/provider/SystemProviderDstu2Test.java | 40 ++++-- .../dstu3/SystemProviderDstu3Test.java | 34 ++--- 10 files changed, 302 insertions(+), 130 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ParametersUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ParametersUtil.java index 339dbe18c83..68e7ae16614 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ParametersUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ParametersUtil.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.util; import java.util.Collection; +import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseDatatype; import org.hl7.fhir.instance.model.api.IBaseParameters; @@ -84,6 +85,7 @@ public class ParametersUtil { } public static IBaseParameters newInstance(FhirContext theContext) { + Validate.notNull(theContext, "theContext must not be null"); return (IBaseParameters) theContext.getResourceDefinition("Parameters").newInstance(); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseDstu2Config.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseDstu2Config.java index 32a5021cd45..20f694e62b1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseDstu2Config.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseDstu2Config.java @@ -101,6 +101,7 @@ public class BaseDstu2Config extends BaseConfig { public ca.uhn.fhir.jpa.provider.JpaSystemProviderDstu2 systemProviderDstu2() { ca.uhn.fhir.jpa.provider.JpaSystemProviderDstu2 retVal = new ca.uhn.fhir.jpa.provider.JpaSystemProviderDstu2(); retVal.setDao(systemDaoDstu2()); + retVal.setContext(fhirContextDstu2()); return retVal; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptParentChildLinkDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptParentChildLinkDao.java index b663163edcd..d4d315a3c4f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptParentChildLinkDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermConceptParentChildLinkDao.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.jpa.dao.data; +import java.util.Collection; + /* * #%L * HAPI FHIR JPA Server @@ -32,5 +34,8 @@ public interface ITermConceptParentChildLinkDao extends JpaRepository findAllWithChild(@Param("child_pid") Long theConceptPid); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java index 4c5182d0ffa..5ae32fdd272 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConcept.java @@ -233,12 +233,14 @@ public class TermConcept implements Serializable { @PreUpdate @PrePersist public void prePersist() { - Set parentPids = new HashSet(); - TermConcept entity = this; - parentPids(entity, parentPids); - entity.setParentPids(parentPids); - - ourLog.trace("Code {}/{} has parents {}", entity.getId(), entity.getCode(), entity.getParentPidsAsString()); + if (myParentPids == null) { + Set parentPids = new HashSet(); + TermConcept entity = this; + parentPids(entity, parentPids); + entity.setParentPids(parentPids); + + ourLog.trace("Code {}/{} has parents {}", entity.getId(), entity.getCode(), entity.getParentPidsAsString()); + } } public void setCode(String theCode) { @@ -280,6 +282,10 @@ public class TermConcept implements Serializable { myParentPids = b.toString(); } + public void setParentPids(String theParentPids) { + myParentPids = theParentPids; + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE).append("code", myCode).append("display", myDisplay).build(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptParentChildLink.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptParentChildLink.java index 09887b012a3..f7ddb31c580 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptParentChildLink.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptParentChildLink.java @@ -10,7 +10,7 @@ package ca.uhn.fhir.jpa.entity; * 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 + * 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, @@ -36,77 +36,38 @@ import javax.persistence.SequenceGenerator; import javax.persistence.Table; @Entity -@Table(name="TRM_CONCEPT_PC_LINK") +@Table(name = "TRM_CONCEPT_PC_LINK") public class TermConceptParentChildLink implements Serializable { private static final long serialVersionUID = 1L; @ManyToOne() - @JoinColumn(name="CHILD_PID", nullable=false, referencedColumnName="PID", foreignKey=@ForeignKey(name="FK_TERM_CONCEPTPC_CHILD")) + @JoinColumn(name = "CHILD_PID", nullable = false, referencedColumnName = "PID", foreignKey = @ForeignKey(name = "FK_TERM_CONCEPTPC_CHILD")) private TermConcept myChild; + @Column(name = "CHILD_PID", insertable = false, updatable = false) + private Long myChildPid; + @ManyToOne() - @JoinColumn(name="CODESYSTEM_PID", nullable=false, foreignKey=@ForeignKey(name="FK_TERM_CONCEPTPC_CS")) + @JoinColumn(name = "CODESYSTEM_PID", nullable = false, foreignKey = @ForeignKey(name = "FK_TERM_CONCEPTPC_CS")) private TermCodeSystemVersion myCodeSystem; - @ManyToOne(cascade= {}) - @JoinColumn(name="PARENT_PID", nullable=false, referencedColumnName="PID", foreignKey=@ForeignKey(name="FK_TERM_CONCEPTPC_PARENT")) + @ManyToOne(cascade = {}) + @JoinColumn(name = "PARENT_PID", nullable = false, referencedColumnName = "PID", foreignKey = @ForeignKey(name = "FK_TERM_CONCEPTPC_PARENT")) private TermConcept myParent; + @Column(name = "PARENT_PID", insertable = false, updatable = false) + private Long myParentPid; + @Id() - @SequenceGenerator(name="SEQ_CONCEPT_PC_PID", sequenceName="SEQ_CONCEPT_PC_PID") - @GeneratedValue(strategy=GenerationType.AUTO, generator="SEQ_CONCEPT_PC_PID") - @Column(name="PID") + @SequenceGenerator(name = "SEQ_CONCEPT_PC_PID", sequenceName = "SEQ_CONCEPT_PC_PID") + @GeneratedValue(strategy = GenerationType.AUTO, generator = "SEQ_CONCEPT_PC_PID") + @Column(name = "PID") private Long myPid; @Enumerated(EnumType.ORDINAL) - @Column(name="REL_TYPE", length=5, nullable=true) + @Column(name = "REL_TYPE", length = 5, nullable = true) private RelationshipTypeEnum myRelationshipType; - public TermConcept getChild() { - return myChild; - } - - public RelationshipTypeEnum getRelationshipType() { - return myRelationshipType; - } - - public TermCodeSystemVersion getCodeSystem() { - return myCodeSystem; - } - - public TermConcept getParent() { - return myParent; - } - - public void setChild(TermConcept theChild) { - myChild = theChild; - } - - public void setCodeSystem(TermCodeSystemVersion theCodeSystem) { - myCodeSystem = theCodeSystem; - } - - public void setParent(TermConcept theParent) { - myParent = theParent; - } - - - public void setRelationshipType(RelationshipTypeEnum theRelationshipType) { - myRelationshipType = theRelationshipType; - } - - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((myChild == null) ? 0 : myChild.hashCode()); - result = prime * result + ((myCodeSystem == null) ? 0 : myCodeSystem.hashCode()); - result = prime * result + ((myParent == null) ? 0 : myParent.hashCode()); - result = prime * result + ((myRelationshipType == null) ? 0 : myRelationshipType.hashCode()); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) @@ -136,15 +97,64 @@ public class TermConceptParentChildLink implements Serializable { return true; } - - public enum RelationshipTypeEnum{ - // ******************************************** - // IF YOU ADD HERE MAKE SURE ORDER IS PRESERVED - ISA + public TermConcept getChild() { + return myChild; } + public Long getChildPid() { + return myChildPid; + } + + public TermCodeSystemVersion getCodeSystem() { + return myCodeSystem; + } public Long getId() { return myPid; } + + public TermConcept getParent() { + return myParent; + } + + public Long getParentPid() { + return myParentPid; + } + + public RelationshipTypeEnum getRelationshipType() { + return myRelationshipType; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((myChild == null) ? 0 : myChild.hashCode()); + result = prime * result + ((myCodeSystem == null) ? 0 : myCodeSystem.hashCode()); + result = prime * result + ((myParent == null) ? 0 : myParent.hashCode()); + result = prime * result + ((myRelationshipType == null) ? 0 : myRelationshipType.hashCode()); + return result; + } + + public void setChild(TermConcept theChild) { + myChild = theChild; + } + + public void setCodeSystem(TermCodeSystemVersion theCodeSystem) { + myCodeSystem = theCodeSystem; + } + + public void setParent(TermConcept theParent) { + myParent = theParent; + } + + public void setRelationshipType(RelationshipTypeEnum theRelationshipType) { + myRelationshipType = theRelationshipType; + } + + public enum RelationshipTypeEnum { + // ******************************************** + // IF YOU ADD HERE MAKE SURE ORDER IS PRESERVED + ISA + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvc.java index f8bbb377f5d..765e09cf340 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvc.java @@ -23,9 +23,11 @@ package ca.uhn.fhir.jpa.term; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -33,6 +35,7 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; +import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.time.DateUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; @@ -47,6 +50,8 @@ import org.springframework.transaction.support.TransactionTemplate; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimaps; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; @@ -305,6 +310,8 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { } } + private ArrayListMultimap myChildToParentPidCache; + private void processReindexing() { if (System.currentTimeMillis() < myNextReindexPass && !ourForceSaveDeferredAlwaysForUnitTest) { return; @@ -316,23 +323,60 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { @Override protected void doInTransactionWithoutResult(TransactionStatus theArg0) { int maxResult = 1000; - Page resources = myConceptDao.findResourcesRequiringReindexing(new PageRequest(0, maxResult)); - if (resources.hasContent() == false) { + Page concepts = myConceptDao.findResourcesRequiringReindexing(new PageRequest(0, maxResult)); + if (concepts.hasContent() == false) { myNextReindexPass = System.currentTimeMillis() + DateUtils.MILLIS_PER_MINUTE; + myChildToParentPidCache = null; return; } - ourLog.info("Indexing {} / {} concepts", resources.getContent().size(), resources.getTotalElements()); + if (myChildToParentPidCache == null) { + myChildToParentPidCache = ArrayListMultimap.create(); + } + + ourLog.info("Indexing {} / {} concepts", concepts.getContent().size(), concepts.getTotalElements()); int count = 0; StopWatch stopwatch = new StopWatch(); - for (TermConcept resourceTable : resources) { - saveConcept(resourceTable); + for (TermConcept nextConcept : concepts) { + + StringBuilder parentsBuilder = new StringBuilder(); + createParentsString(parentsBuilder, nextConcept.getId()); + nextConcept.setParentPids(parentsBuilder.toString()); + + saveConcept(nextConcept); count++; } - ourLog.info("Indexed {} / {} concepts in {}ms - Avg {}ms / resource", new Object[] { count, resources.getContent().size(), stopwatch.getMillis(), stopwatch.getMillisPerOperation(count) }); + ourLog.info("Indexed {} / {} concepts in {}ms - Avg {}ms / resource", new Object[] { count, concepts.getContent().size(), stopwatch.getMillis(), stopwatch.getMillisPerOperation(count) }); + } + + private void createParentsString(StringBuilder theParentsBuilder, Long theConceptPid) { + Validate.notNull(theConceptPid, "theConceptPid must not be null"); + List parents = myChildToParentPidCache.get(theConceptPid); + if (parents.contains(-1L)) { + return; + } else if (parents.isEmpty()) { + Collection parentLinks = myConceptParentChildLinkDao.findAllWithChild(theConceptPid); + if (parentLinks.isEmpty()) { + myChildToParentPidCache.put(theConceptPid, -1L); + return; + } else { + for (TermConceptParentChildLink next : parentLinks) { + myChildToParentPidCache.put(theConceptPid, next.getParentPid()); + } + } + } + + + for (Long nextParent : parents) { + if (theParentsBuilder.length() > 0) { + theParentsBuilder.append(' '); + } + theParentsBuilder.append(nextParent); + createParentsString(theParentsBuilder, nextParent); + } } }); @@ -340,7 +384,15 @@ public abstract class BaseHapiTerminologySvc implements IHapiTerminologySvc { private int saveConcept(TermConcept theConcept) { int retVal = 0; - retVal += ensureParentsSaved(theConcept.getParents()); + + /* + * If the concept has an ID, we're reindexing, so there's no need to + * save parent concepts first (it's way too slow to do that) + */ + if (theConcept.getId() == null) { + retVal += ensureParentsSaved(theConcept.getParents()); + } + if (theConcept.getId() == null || theConcept.getIndexStatus() == null) { retVal++; theConcept.setIndexStatus(BaseHapiFhirDao.INDEX_STATUS_INDEXED); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu3.java index add64802014..96a2191bfa6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcDstu3.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.term; +import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; /* @@ -12,7 +13,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * 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 + * 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, @@ -64,6 +65,7 @@ import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TermCodeSystem; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; import ca.uhn.fhir.jpa.entity.TermConcept; +import ca.uhn.fhir.jpa.util.StopWatch; import ca.uhn.fhir.rest.method.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; @@ -79,7 +81,7 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I @Autowired private IValidationSupport myValidationSupport; - + private void addCodeIfNotAlreadyAdded(String system, ValueSetExpansionComponent retVal, Set addedCodes, TermConcept nextConcept) { if (addedCodes.add(nextConcept.getCode())) { ValueSetExpansionContainsComponent contains = retVal.addContains(); @@ -126,12 +128,12 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I for (ConceptDefinitionComponent nextChild : theNext.getConcept()) { foundCodeInChild |= addTreeIfItContainsCode(theSystemString, nextChild, theCode, theListToPopulate); } - + if (theCode.equals(theNext.getCode()) || foundCodeInChild) { theListToPopulate.add(new VersionIndependentConcept(theSystemString, theNext.getCode())); return true; } - + return false; } @@ -159,17 +161,15 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I } private void addDisplayFilterInexact(QueryBuilder qb, BooleanJunction bool, ConceptSetFilterComponent nextFilter) { - //@formatter:off Query textQuery = qb .phrase() .withSlop(2) .onField("myDisplay").boostedTo(4.0f) .andField("myDisplayEdgeNGram").boostedTo(2.0f) - //.andField("myDisplayNGram").boostedTo(1.0f) - //.andField("myDisplayPhonetic").boostedTo(0.5f) + // .andField("myDisplayNGram").boostedTo(1.0f) + // .andField("myDisplayPhonetic").boostedTo(0.5f) .sentence(nextFilter.getValue().toLowerCase()).createQuery(); bool.must(textQuery); - //@formatter:on } @Override @@ -216,26 +216,33 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I bool.must(qb.keyword().onField("myCodeSystemVersionPid").matching(csv.getPid()).createQuery()); for (ConceptSetFilterComponent nextFilter : theInclude.getFilter()) { - if (isNotBlank(nextFilter.getValue())) { - if (nextFilter.getProperty().equals("display:exact") && nextFilter.getOp() == FilterOperator.EQUAL) { - addDisplayFilterExact(qb, bool, nextFilter); - } else if (nextFilter.getProperty().equals("display") && nextFilter.getOp() == FilterOperator.EQUAL) { - if (nextFilter.getValue().trim().contains(" ")) { - addDisplayFilterExact(qb, bool, nextFilter); - } else { - addDisplayFilterInexact(qb, bool, nextFilter); - } - } else if ((nextFilter.getProperty().equals("concept") || nextFilter.getProperty().equals("code")) && nextFilter.getOp() == FilterOperator.ISA) { - TermConcept code = super.findCode(system, nextFilter.getValue()); - if (code == null) { - throw new InvalidRequestException("Invalid filter criteria - code does not exist: {" + system + "}" + nextFilter.getValue()); - } + if (isBlank(nextFilter.getValue()) && nextFilter.getOp() == null && isBlank(nextFilter.getProperty())) { + continue; + } - ourLog.info(" * Filtering on codes with a parent of {}/{}/{}", code.getId(), code.getCode(), code.getDisplay()); - bool.must(qb.keyword().onField("myParentPids").matching("" + code.getId()).createQuery()); + if (isBlank(nextFilter.getValue()) || nextFilter.getOp() == null || isBlank(nextFilter.getProperty())) { + throw new InvalidRequestException("Invalid filter, must have fields populated: property op value"); + } + + + if (nextFilter.getProperty().equals("display:exact") && nextFilter.getOp() == FilterOperator.EQUAL) { + addDisplayFilterExact(qb, bool, nextFilter); + } else if ("display".equals(nextFilter.getProperty()) && nextFilter.getOp() == FilterOperator.EQUAL) { + if (nextFilter.getValue().trim().contains(" ")) { + addDisplayFilterExact(qb, bool, nextFilter); } else { - throw new InvalidRequestException("Unknown filter property[" + nextFilter + "] + op[" + nextFilter.getOpElement().getValueAsString() + "]"); + addDisplayFilterInexact(qb, bool, nextFilter); } + } else if ((nextFilter.getProperty().equals("concept") || nextFilter.getProperty().equals("code")) && nextFilter.getOp() == FilterOperator.ISA) { + TermConcept code = super.findCode(system, nextFilter.getValue()); + if (code == null) { + throw new InvalidRequestException("Invalid filter criteria - code does not exist: {" + system + "}" + nextFilter.getValue()); + } + + ourLog.info(" * Filtering on codes with a parent of {}/{}/{}", code.getId(), code.getCode(), code.getDisplay()); + bool.must(qb.keyword().onField("myParentPids").matching("" + code.getId()).createQuery()); + } else { + throw new InvalidRequestException("Unknown filter property[" + nextFilter + "] + op[" + nextFilter.getOpElement().getValueAsString() + "]"); } } @@ -243,12 +250,17 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I FullTextQuery jpaQuery = em.createFullTextQuery(luceneQuery, TermConcept.class); jpaQuery.setMaxResults(1000); + StopWatch sw = new StopWatch(); + @SuppressWarnings("unchecked") List result = jpaQuery.getResultList(); + + ourLog.info("Expansion completed in {}ms", sw.getMillis()); + for (TermConcept nextConcept : result) { addCodeIfNotAlreadyAdded(system, retVal, addedCodes, nextConcept); } - + retVal.setTotal(jpaQuery.getResultSize()); } @@ -259,7 +271,6 @@ public class HapiTerminologySvcDstu3 extends BaseHapiTerminologySvc implements I } } - return retVal; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java index 2efb85d863e..414622d3a30 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java @@ -26,7 +26,9 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import ca.uhn.fhir.jpa.dao.BaseHapiFhirSystemDao; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.IFhirResourceDaoCodeSystem.LookupCodeResult; import ca.uhn.fhir.jpa.dao.SearchParameterMap; @@ -34,6 +36,8 @@ import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; +import ca.uhn.fhir.jpa.term.BaseHapiTerminologySvc; +import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.param.TokenParamModifier; @@ -52,6 +56,8 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { @After public void after() { myDaoConfig.setDeferIndexingForCodesystemsOfSize(new DaoConfig().getDeferIndexingForCodesystemsOfSize()); + + BaseHapiTerminologySvc.setForceSaveDeferredAlwaysForUnitTest(false); } @Before @@ -318,6 +324,71 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { } } + @Test + public void testExpandWithIsAInExternalValueSet() { + createExternalCsAndLocalVs(); + + ValueSet vs = new ValueSet(); + ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(URL_MY_CODE_SYSTEM); + include.addFilter().setOp(FilterOperator.ISA).setValue("childAA").setProperty("concept"); + + ValueSet result = myValueSetDao.expand(vs, null); + logAndValidateValueSet(result); + + ArrayList codes = toCodesContains(result.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + + } + + @Autowired + private IHapiTerminologySvc myHapiTerminologySvc; + + @Test + public void testExpandWithIsAInExternalValueSetReindex() { + BaseHapiTerminologySvc.setForceSaveDeferredAlwaysForUnitTest(true); + + createExternalCsAndLocalVs(); + + mySystemDao.markAllResourcesForReindexing(); + + mySystemDao.performReindexingPass(100); + mySystemDao.performReindexingPass(100); + myHapiTerminologySvc.saveDeferred(); + myHapiTerminologySvc.saveDeferred(); + myHapiTerminologySvc.saveDeferred(); + + ValueSet vs = new ValueSet(); + ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(URL_MY_CODE_SYSTEM); + include.addFilter().setOp(FilterOperator.ISA).setValue("childAA").setProperty("concept"); + + ValueSet result = myValueSetDao.expand(vs, null); + logAndValidateValueSet(result); + + ArrayList codes = toCodesContains(result.getExpansion().getContains()); + assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + + } + + @Test + public void testExpandInvalid() { + createExternalCsAndLocalVs(); + + ValueSet vs = new ValueSet(); + ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(URL_MY_CODE_SYSTEM); + include.addFilter(); + include.addFilter().setOp(FilterOperator.ISA).setValue("childAA"); + + try { + myValueSetDao.expand(vs, null); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Invalid filter, must have fields populated: property op value", e.getMessage()); + } + } + @Test public void testExpandWithSystemAndCodesInExternalValueSet() { createExternalCsAndLocalVs(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderDstu2Test.java index 4fd7a995c19..3977ff8992e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/SystemProviderDstu2Test.java @@ -10,6 +10,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; import org.apache.commons.io.IOUtils; @@ -139,7 +140,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { get.addHeader("Accept", "application/xml, text/html"); CloseableHttpResponse http = ourHttpClient.execute(get); try { - String response = IOUtils.toString(http.getEntity().getContent()); + String response = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(response); assertThat(response, (containsString("_format=json"))); assertEquals(200, http.getStatusLine().getStatusCode()); @@ -167,7 +168,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { get.addHeader("Accept", "application/xml+fhir"); CloseableHttpResponse http = ourHttpClient.execute(get); try { - String response = IOUtils.toString(http.getEntity().getContent()); + String response = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(response); assertThat(response, not(containsString("_format"))); assertEquals(200, http.getStatusLine().getStatusCode()); @@ -216,7 +217,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { CloseableHttpResponse http = ourHttpClient.execute(get); try { assertEquals(200, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); Parameters parameters = ourCtx.newXmlParser().parseResource(Parameters.class, output); @@ -246,7 +247,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { CloseableHttpResponse http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'context' must be provided")); } finally { @@ -257,7 +258,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'searchParam' must be provided")); } finally { @@ -268,7 +269,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'text' must be provided")); } finally { @@ -286,7 +287,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { @Test public void testTransactionFromBundle() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/transaction_link_patient_eve.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); } @@ -295,7 +296,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { public void testTransactionFromBundle2() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/transaction_link_patient_eve_temp.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); @@ -311,7 +312,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { */ bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/transaction_link_patient_eve_temp.xml"); - bundle = IOUtils.toString(bundleRes); + bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); @@ -335,7 +336,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { public void testTransactionFromBundle3() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/grahame-transaction.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); } @@ -343,7 +344,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { @Test public void testTransactionFromBundle4() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/simone_bundle.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); Bundle bundleResp = ourCtx.newXmlParser().parseResource(Bundle.class, response); @@ -358,7 +359,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { @Test public void testTransactionFromBundle5() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/simone_bundle2.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); try { ourClient.transaction().withBundle(bundle).prettyPrint().execute(); fail(); @@ -372,7 +373,7 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { @Test public void testTransactionFromBundle6() throws Exception { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/simone_bundle3.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); ourClient.transaction().withBundle(bundle).prettyPrint().execute(); // try { // fail(); @@ -427,5 +428,18 @@ public class SystemProviderDstu2Test extends BaseJpaDstu2Test { assertEquals(0, respSub.getEntry().size()); } + @Test + public void testMarkResourcesForReindexing() throws Exception { + HttpGet get = new HttpGet(ourServerBase + "/$mark-all-resources-for-reindexing"); + CloseableHttpResponse http = ourHttpClient.execute(get); + try { + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(output); + assertEquals(200, http.getStatusLine().getStatusCode()); + } finally { + IOUtils.closeQuietly(http);; + } + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java index c78abae2d65..e4c70769d94 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java @@ -129,7 +129,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { CloseableHttpResponse resp = ourHttpClient.execute(req); try { - String encoded = IOUtils.toString(resp.getEntity().getContent()); + String encoded = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(encoded); assertThat(encoded, containsString("transaction-response")); @@ -162,7 +162,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { CloseableHttpResponse resp = ourHttpClient.execute(req); try { - String encoded = IOUtils.toString(resp.getEntity().getContent()); + String encoded = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(encoded); assertThat(encoded, containsString("transaction-response")); @@ -271,14 +271,14 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { try { InputStream bundleRes = SystemProviderDstu2Test.class.getResourceAsStream("/questionnaire-sdc-profile-example-ussg-fht.xml"); - String bundleStr = IOUtils.toString(bundleRes); + String bundleStr = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); HttpPost req = new HttpPost(ourServerBase); req.setEntity(new StringEntity(bundleStr, ContentType.parse(Constants.CT_FHIR_XML + "; charset=utf-8"))); CloseableHttpResponse resp = ourHttpClient.execute(req); try { - String encoded = IOUtils.toString(resp.getEntity().getContent()); + String encoded = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(encoded); //@formatter:off @@ -314,7 +314,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { CloseableHttpResponse http = ourHttpClient.execute(get); try { - String response = IOUtils.toString(http.getEntity().getContent()); + String response = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(response); assertThat(response, containsString("_format=json")); assertEquals(200, http.getStatusLine().getStatusCode()); @@ -342,7 +342,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { get.addHeader("Accept", "application/xml+fhir"); CloseableHttpResponse http = ourHttpClient.execute(get); try { - String response = IOUtils.toString(http.getEntity().getContent()); + String response = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(response); assertThat(response, not(containsString("_format"))); assertEquals(200, http.getStatusLine().getStatusCode()); @@ -373,7 +373,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { HttpGet get = new HttpGet(ourServerBase + "/$mark-all-resources-for-reindexing"); CloseableHttpResponse http = ourHttpClient.execute(get); try { - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertEquals(200, http.getStatusLine().getStatusCode()); } finally { @@ -404,7 +404,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { CloseableHttpResponse http = ourHttpClient.execute(get); try { assertEquals(200, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); Parameters parameters = ourCtx.newXmlParser().parseResource(Parameters.class, output); @@ -434,7 +434,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { CloseableHttpResponse http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'context' must be provided")); } finally { @@ -445,7 +445,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'searchParam' must be provided")); } finally { @@ -456,7 +456,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { http = ourHttpClient.execute(get); try { assertEquals(400, http.getStatusLine().getStatusCode()); - String output = IOUtils.toString(http.getEntity().getContent()); + String output = IOUtils.toString(http.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(output); assertThat(output, containsString("Parameter 'text' must be provided")); } finally { @@ -500,7 +500,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { public void testTransactionFromBundle2() throws Exception { InputStream bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/transaction_link_patient_eve_temp.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); @@ -516,7 +516,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { */ bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/transaction_link_patient_eve_temp.xml"); - bundle = IOUtils.toString(bundleRes); + bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); @@ -540,7 +540,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { public void testTransactionFromBundle3() throws Exception { InputStream bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/grahame-transaction.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); } @@ -548,7 +548,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { @Test public void testTransactionFromBundle4() throws Exception { InputStream bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/simone_bundle.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); String response = ourClient.transaction().withBundle(bundle).prettyPrint().execute(); ourLog.info(response); Bundle bundleResp = ourCtx.newXmlParser().parseResource(Bundle.class, response); @@ -563,7 +563,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { @Test public void testTransactionFromBundle5() throws Exception { InputStream bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/simone_bundle2.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); try { ourClient.transaction().withBundle(bundle).prettyPrint().execute(); fail(); @@ -577,7 +577,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { @Test public void testTransactionFromBundle6() throws Exception { InputStream bundleRes = SystemProviderDstu3Test.class.getResourceAsStream("/simone_bundle3.xml"); - String bundle = IOUtils.toString(bundleRes); + String bundle = IOUtils.toString(bundleRes, StandardCharsets.UTF_8); ourClient.transaction().withBundle(bundle).prettyPrint().execute(); // try { // fail(); From b7f165019d9009e8a70f9ccc1f33d89d1f4d8891 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 7 Feb 2017 07:00:51 -0500 Subject: [PATCH 06/10] Tweak re-indexing --- .../uhn/fhir/rest/client/GenericClient.java | 6 +- .../ca/uhn/fhir/rest/server/Constants.java | 13 +- .../rest/client/GenericClientDstu3Test.java | 155 ++++++++++-------- src/changes/changes.xml | 4 + 4 files changed, 103 insertions(+), 75 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java index 94067c7824a..511ff147aad 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java @@ -1889,7 +1889,11 @@ public class GenericClient extends BaseClient implements IGenericClient { } for (Include next : myRevInclude) { - addParam(params, Constants.PARAM_REVINCLUDE, next.getValue()); + if (next.isRecurse()) { + addParam(params, Constants.PARAM_REVINCLUDE_RECURSE, next.getValue()); + } else { + addParam(params, Constants.PARAM_REVINCLUDE, next.getValue()); + } } if (myContext.getVersion().getVersion().isNewerThan(FhirVersionEnum.DSTU2)) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java index 2589b6c088a..2080cf3342d 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java @@ -40,11 +40,13 @@ public class Constants { public static final String CT_HTML = "text/html"; public static final String CT_HTML_WITH_UTF8 = "text/html" + CHARSET_UTF8_CTSUFFIX; public static final String CT_JSON = "application/json"; + public static final String CT_JSON_PATCH = "application/json-patch+json"; public static final String CT_OCTET_STREAM = "application/octet-stream"; public static final String CT_TEXT = "text/plain"; public static final String CT_TEXT_WITH_UTF8 = CT_TEXT + CHARSET_UTF8_CTSUFFIX; public static final String CT_X_FORM_URLENCODED = "application/x-www-form-urlencoded"; public static final String CT_XML = "application/xml"; + public static final String CT_XML_PATCH = "application/xml-patch+xml"; public static final String ENCODING_GZIP = "gzip"; public static final String EXTOP_VALIDATE = "$validate"; public static final String EXTOP_VALIDATE_MODE = "mode"; @@ -61,10 +63,10 @@ public class Constants { public static final String FORMATS_HTML_XML = "html/xml"; public static final String HEADER_ACCEPT = "Accept"; public static final String HEADER_ACCEPT_ENCODING = "Accept-Encoding"; + public static final String HEADER_ACCEPT_VALUE_JSON_NON_LEGACY = CT_FHIR_JSON_NEW + ";q=1.0, " + CT_FHIR_JSON + ";q=0.9"; + public static final String HEADER_ACCEPT_VALUE_XML_NON_LEGACY = CT_FHIR_XML_NEW + ";q=1.0, " + CT_FHIR_XML + ";q=0.9"; public static final String HEADER_ACCEPT_VALUE_XML_OR_JSON_LEGACY = CT_FHIR_XML + ";q=1.0, " + CT_FHIR_JSON + ";q=1.0"; public static final String HEADER_ACCEPT_VALUE_XML_OR_JSON_NON_LEGACY = CT_FHIR_XML_NEW + ";q=1.0, " + CT_FHIR_JSON_NEW + ";q=1.0, " + HEADER_ACCEPT_VALUE_XML_OR_JSON_LEGACY.replace("1.0", "0.9"); - public static final String HEADER_ACCEPT_VALUE_XML_NON_LEGACY = CT_FHIR_XML_NEW + ";q=1.0, " + CT_FHIR_XML + ";q=0.9"; - public static final String HEADER_ACCEPT_VALUE_JSON_NON_LEGACY = CT_FHIR_JSON_NEW + ";q=1.0, " + CT_FHIR_JSON + ";q=0.9"; public static final String HEADER_ALLOW = "Allow"; public static final String HEADER_AUTHORIZATION = "Authorization"; public static final String HEADER_AUTHORIZATION_VALPREFIX_BASIC = "Basic "; @@ -127,10 +129,12 @@ public class Constants { public static final String PARAM_PAGINGACTION = "_getpages"; public static final String PARAM_PAGINGOFFSET = "_getpagesoffset"; public static final String PARAM_PRETTY = "_pretty"; + public static final String PARAM_PRETTY_VALUE_FALSE = "false"; public static final String PARAM_PRETTY_VALUE_TRUE = "true"; public static final String PARAM_PROFILE = "_profile"; public static final String PARAM_QUERY = "_query"; public static final String PARAM_REVINCLUDE = "_revinclude"; + public static final String PARAM_REVINCLUDE_RECURSE = PARAM_REVINCLUDE+PARAM_INCLUDE_QUALIFIER_RECURSE; public static final String PARAM_SEARCH = "_search"; public static final String PARAM_SECURITY = "_security"; public static final String PARAM_SINCE = "_since"; @@ -156,11 +160,11 @@ public class Constants { public static final int STATUS_HTTP_401_CLIENT_UNAUTHORIZED = 401; public static final int STATUS_HTTP_403_FORBIDDEN = 403; public static final int STATUS_HTTP_404_NOT_FOUND = 404; + public static final int STATUS_HTTP_405_METHOD_NOT_ALLOWED = 405; public static final int STATUS_HTTP_409_CONFLICT = 409; public static final int STATUS_HTTP_410_GONE = 410; public static final int STATUS_HTTP_412_PRECONDITION_FAILED = 412; - public static final int STATUS_HTTP_422_UNPROCESSABLE_ENTITY = 422; public static final int STATUS_HTTP_500_INTERNAL_ERROR = 500; public static final int STATUS_HTTP_501_NOT_IMPLEMENTED = 501; @@ -168,9 +172,6 @@ public class Constants { public static final String TAG_SUBSETTED_SYSTEM = "http://hl7.org/fhir/v3/ObservationValue"; public static final String URL_TOKEN_HISTORY = "_history"; public static final String URL_TOKEN_METADATA = "metadata"; - public static final String CT_JSON_PATCH = "application/json-patch+json"; - public static final String CT_XML_PATCH = "application/xml-patch+xml"; - public static final String PARAM_PRETTY_VALUE_FALSE = "false"; static { CHARSET_UTF8 = Charset.forName(CHARSET_NAME_UTF8); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java index 81702080fb3..35ef9907a45 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java @@ -98,7 +98,7 @@ public class GenericClientDstu3Test { byte[] body = IOUtils.toByteArray(((HttpEntityEnclosingRequestBase) capt.getAllValues().get(0)).getEntity().getContent()); return body; } - + private String extractBodyAsString(ArgumentCaptor capt) throws IOException { String body = IOUtils.toString(((HttpEntityEnclosingRequestBase) capt.getAllValues().get(0)).getEntity().getContent(), "UTF-8"); return body; @@ -120,6 +120,26 @@ public class GenericClientDstu3Test { return capt; } + @Test + public void testRevIncludeRecursive() throws ClientProtocolException, IOException { + ArgumentCaptor capt = prepareClientForSearchResponse(); + + IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); + int idx = 0; + + client.search() + .forResource(EpisodeOfCare.class) + .where(EpisodeOfCare.PATIENT.hasId("123")) + .revInclude(Encounter.INCLUDE_EPISODEOFCARE) + .revInclude(Observation.INCLUDE_ENCOUNTER.asRecursive()) + .returnBundle(Bundle.class) + .execute(); + + assertEquals("http://example.com/fhir/EpisodeOfCare?patient=123&_revinclude=Encounter%3Aepisodeofcare&_revinclude%3Arecurse=Observation%3Aencounter", capt.getAllValues().get(idx).getURI().toString()); + idx++; + + } + @Test public void testPatchJsonByIdString() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -143,20 +163,21 @@ public class GenericClientDstu3Test { String patch = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .withId("Patient/123") - .execute(); + .patch() + .withBody(patch) + .withId("Patient/123") + .execute(); assertEquals("http://example.com/fhir/Patient/123", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_JSON_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + @Test public void testPatchJsonByIdType() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -180,20 +201,21 @@ public class GenericClientDstu3Test { String patch = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .withId(new IdType("http://localhost/fhir/Patient/123/_history/234")) - .execute(); + .patch() + .withBody(patch) + .withId(new IdType("http://localhost/fhir/Patient/123/_history/234")) + .execute(); assertEquals("http://example.com/fhir/Patient/123", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_JSON_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + @Test public void testPatchJsonByConditionalString() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -217,20 +239,21 @@ public class GenericClientDstu3Test { String patch = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .conditionalByUrl("Patient?foo=bar") - .execute(); + .patch() + .withBody(patch) + .conditionalByUrl("Patient?foo=bar") + .execute(); assertEquals("http://example.com/fhir/Patient?foo=bar", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_JSON_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + @Test public void testPatchJsonByConditionalParam() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -254,21 +277,22 @@ public class GenericClientDstu3Test { String patch = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .conditional("Patient").where(Patient.NAME.matches().value("TEST")) - .and(Patient.FAMILY.matches().value("TEST2")) - .execute(); + .patch() + .withBody(patch) + .conditional("Patient").where(Patient.NAME.matches().value("TEST")) + .and(Patient.FAMILY.matches().value("TEST2")) + .execute(); assertEquals("http://example.com/fhir/Patient?name=TEST&family=TEST2", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_JSON_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + @Test public void testPatchJsonByConditionalParamResourceType() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -292,21 +316,22 @@ public class GenericClientDstu3Test { String patch = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .conditional(Patient.class).where(Patient.NAME.matches().value("TEST")) - .and(Patient.FAMILY.matches().value("TEST2")) - .execute(); + .patch() + .withBody(patch) + .conditional(Patient.class).where(Patient.NAME.matches().value("TEST")) + .and(Patient.FAMILY.matches().value("TEST2")) + .execute(); assertEquals("http://example.com/fhir/Patient?name=TEST&family=TEST2", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_JSON_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + @Test public void testPatchXmlByIdString() throws Exception { OperationOutcome conf = new OperationOutcome(); @@ -330,31 +355,31 @@ public class GenericClientDstu3Test { String patch = "false"; MethodOutcome outcome = client - .patch() - .withBody(patch) - .withId("Patient/123") - .execute(); + .patch() + .withBody(patch) + .withId("Patient/123") + .execute(); assertEquals("http://example.com/fhir/Patient/123", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("PATCH", capt.getAllValues().get(0).getRequestLine().getMethod()); assertEquals(patch, extractBodyAsString(capt)); assertEquals(Constants.CT_XML_PATCH, capt.getAllValues().get(idx).getFirstHeader("Content-Type").getValue().replaceAll(";.*", "")); idx++; - + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } - + @Test public void testPatchInvalid() throws Exception { IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); try { client - .patch() - .withBody("AA") - .withId("Patient/123") - .execute(); + .patch() + .withBody("AA") + .withId("Patient/123") + .execute(); } catch (IllegalArgumentException e) { assertEquals("Unable to determine encoding of patch", e.getMessage()); } @@ -388,7 +413,7 @@ public class GenericClientDstu3Test { assertEquals("http://example.com/fhir/Device?_format=json", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("application/fhir+json;q=1.0, application/json+fhir;q=0.9", capt.getAllValues().get(idx).getFirstHeader(Constants.HEADER_ACCEPT).getValue()); idx++; - + //@formatter:off client.setEncoding(EncodingEnum.XML); client.search() @@ -399,7 +424,7 @@ public class GenericClientDstu3Test { assertEquals("http://example.com/fhir/Device?_format=xml", UrlUtil.unescape(capt.getAllValues().get(idx).getURI().toString())); assertEquals("application/fhir+xml;q=1.0, application/xml+fhir;q=0.9", capt.getAllValues().get(idx).getFirstHeader(Constants.HEADER_ACCEPT).getValue()); idx++; - + } @Test @@ -480,9 +505,7 @@ public class GenericClientDstu3Test { assertArrayEquals(new byte[] { 0, 1, 2, 3, 4 }, ourCtx.newXmlParser().parseResource(Binary.class, extractBodyAsString(capt)).getContent()); } - - - + @SuppressWarnings("unchecked") @Test public void testClientFailures() throws Exception { @@ -592,7 +615,7 @@ public class GenericClientDstu3Test { assertEquals(myAnswerCount, capt.getAllValues().size()); assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(0).getURI().toASCIIString()); assertEquals(Constants.CT_FHIR_XML_NEW, capt.getAllValues().get(0).getFirstHeader("content-type").getValue().replaceAll(";.*", "")); - + assertEquals("http://foo.com/base/Patient/222/_history/3", capt.getAllValues().get(1).getURI().toASCIIString()); } @@ -1027,7 +1050,7 @@ public class GenericClientDstu3Test { assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(idx).getURI().toString()); idx++; - + //@formatter:off client .search() @@ -1053,22 +1076,22 @@ public class GenericClientDstu3Test { idx++; } - + @Test public void testPutDoesntForceAllIdsJson() throws Exception { IParser p = ourCtx.newJsonParser(); - + Patient patient = new Patient(); patient.setId("PATIENT1"); patient.addName().setFamily("PATIENT1"); - + Bundle bundle = new Bundle(); bundle.setId("BUNDLE1"); bundle.addEntry().setResource(patient); - + final String encoded = p.encodeResourceToString(bundle); assertEquals("{\"resourceType\":\"Bundle\",\"id\":\"BUNDLE1\",\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"id\":\"PATIENT1\",\"name\":[{\"family\":\"PATIENT1\"}]}}]}", encoded); - + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); @@ -1251,7 +1274,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate=gt"+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=gt" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1261,7 +1284,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate=gt"+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=gt" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1271,7 +1294,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate=ge"+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=ge" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1281,7 +1304,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate=lt"+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=lt" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1291,7 +1314,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate=le"+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=le" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1301,7 +1324,7 @@ public class GenericClientDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - assertEquals("http://example.com/fhir/Patient?birthdate="+dateString, capt.getAllValues().get(idx).getURI().toString()); + assertEquals("http://example.com/fhir/Patient?birthdate=" + dateString, capt.getAllValues().get(idx).getURI().toString()); idx++; //@formatter:off @@ -1639,7 +1662,7 @@ public class GenericClientDstu3Test { int idx = 0; Collection values = Arrays.asList("VAL1", "VAL2", "VAL3A,B"); - + //@formatter:off client.search() .forResource("Patient") @@ -1743,7 +1766,6 @@ public class GenericClientDstu3Test { assertEquals("Unable to determing encoding of request (body does not appear to be valid XML or JSON)", e.getMessage()); } - } @Test @@ -2012,7 +2034,7 @@ public class GenericClientDstu3Test { MyPatientWithExtensions patient = new MyPatientWithExtensions(); patient.setId("123"); patient.getText().setDivAsString("OK!"); - + final String respString = p.encodeResourceToString(patient); ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); @@ -2028,7 +2050,7 @@ public class GenericClientDstu3Test { IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); MyPatientWithExtensions read = client.read().resource(MyPatientWithExtensions.class).withId(new IdType("1")).execute(); assertEquals("
OK!
", read.getText().getDivAsString()); - + // Ensure that we haven't overridden the default type for the name assertFalse(MyPatientWithExtensions.class.isAssignableFrom(Patient.class)); assertFalse(Patient.class.isAssignableFrom(MyPatientWithExtensions.class)); @@ -2037,7 +2059,7 @@ public class GenericClientDstu3Test { IParser parser = ourCtx.newXmlParser(); String encoded = parser.encodeResourceToString(pt); pt = (Patient) parser.parseResource(encoded); - + } @Test @@ -2045,13 +2067,12 @@ public class GenericClientDstu3Test { IParser p = ourCtx.newXmlParser(); Bundle b = new Bundle(); - + MyPatientWithExtensions patient = new MyPatientWithExtensions(); patient.setId("123"); patient.getText().setDivAsString("OK!"); b.addEntry().setResource(patient); - - + final String respString = p.encodeResourceToString(b); ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); @@ -2066,7 +2087,7 @@ public class GenericClientDstu3Test { IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); Bundle bundle = client.search().forResource(MyPatientWithExtensions.class).returnBundle(Bundle.class).execute(); - + assertEquals(1, bundle.getEntry().size()); assertEquals(MyPatientWithExtensions.class, bundle.getEntry().get(0).getResource().getClass()); } @@ -2082,8 +2103,6 @@ public class GenericClientDstu3Test { assertEquals(expectedUserAgent(), capt.getAllValues().get(0).getHeaders("User-Agent")[0].getValue()); } - - @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); @@ -2093,5 +2112,5 @@ public class GenericClientDstu3Test { public static void beforeClass() { ourCtx = FhirContext.forDstu3(); } - + } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7b478705ada..65791e5e565 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -123,6 +123,10 @@ Clean up dependencies and remove Eclipse project files from git. Thanks to @sekaijin for the pull request! + + Client revincludes did not include the :recurse modifier. Thanks to + Jenny Meinsma for pointing this out on Zulip! + From d5fbcf8e82af95d93a4a0997476646974eee675d Mon Sep 17 00:00:00 2001 From: James Date: Tue, 7 Feb 2017 07:44:57 -0500 Subject: [PATCH 07/10] Return an OperationOutcome in the response for a delete in JPA --- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 12 +- .../dstu3/ResourceProviderDstu3Test.java | 116 ++++++++++++------ src/changes/changes.xml | 4 + 3 files changed, 90 insertions(+), 42 deletions(-) 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 9f504f5d11b..446534ad694 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 @@ -226,8 +226,16 @@ public abstract class BaseHapiFhirResourceDao extends B validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(getContext()); + String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", 1, w.getMillis()); + String severity = "information"; + String code = "informational"; + OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); + ourLog.info("Processed delete on {} in {}ms", theId.getValue(), w.getMillisAndRestart()); - return toMethodOutcome(savedEntity, null); + DaoMethodOutcome retVal = toMethodOutcome(savedEntity, null); + retVal.setOperationOutcome(oo); + return retVal; } @Override @@ -287,7 +295,7 @@ public abstract class BaseHapiFhirResourceDao extends B OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); } else { oo = OperationOutcomeUtil.newInstance(getContext()); - String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", theUrl, deletedResources.size(), w.getMillis()); + String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", deletedResources.size(), w.getMillis()); String severity = "information"; String code = "informational"; OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); 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 341adb09a2d..8a6eebe4060 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 @@ -149,7 +149,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { @After public void after() throws Exception { super.after(); - + myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); } @@ -160,15 +160,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { myDaoConfig.setAllowMultipleDelete(true); } - - + private void checkParamMissing(String paramName) throws IOException, ClientProtocolException { HttpGet get = new HttpGet(ourServerBase + "/Observation?" + paramName + ":missing=false"); CloseableHttpResponse resp = ourHttpClient.execute(get); IOUtils.closeQuietly(resp.getEntity().getContent()); assertEquals(200, resp.getStatusLine().getStatusCode()); } - + private ArrayList genResourcesOfType(Bundle theRes, Class theClass) { ArrayList retVal = new ArrayList(); for (BundleEntryComponent next : theRes.getEntry()) { @@ -197,7 +196,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { List exts = basic.getExtensionsByUrl("http://localhost:1080/hapi-fhir-jpaserver-example/baseDstu2/StructureDefinition/DateID"); assertEquals(1, exts.size()); } - + private List searchAndReturnUnqualifiedIdValues(String uri) throws IOException, ClientProtocolException { List ids; HttpGet get = new HttpGet(uri); @@ -339,9 +338,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { Binary binary = new Binary(); binary.setContent(arr); binary.setContentType("dansk"); - + IIdType resource = ourClient.create().resource(binary).execute().getId(); - + Binary fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("1", fromDB.getIdElement().getVersionIdPart()); @@ -355,7 +354,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("2", fromDB.getIdElement().getVersionIdPart()); @@ -370,12 +369,12 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("3", fromDB.getIdElement().getVersionIdPart()); // Now an update with the wrong ID in the body - + arr[0] = 4; binary.setId(""); encoded = myFhirCtx.newJsonParser().encodeResourceToString(binary); @@ -387,7 +386,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("3", fromDB.getIdElement().getVersionIdPart()); @@ -726,7 +725,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { * Test for #345 */ @Test - public void testDeleteNormal() throws IOException { + public void testDeleteNormal() { Patient p = new Patient(); p.addName().setFamily("FAM"); IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); @@ -743,6 +742,17 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } + @Test + public void testDeleteReturnsOperationOutcome() { + Patient p = new Patient(); + p.addName().setFamily("FAM"); + IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + IBaseOperationOutcome resp = ourClient.delete().resourceById(id).execute(); + OperationOutcome oo = (OperationOutcome) resp; + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in ")); + } + @Test public void testDeleteResourceConditional1() throws IOException { String methodName = "testDeleteResourceConditional1"; @@ -768,6 +778,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { response = ourHttpClient.execute(delete); try { assertEquals(200, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in ")); } finally { response.close(); } @@ -777,6 +791,24 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { try { ourLog.info(response.toString()); assertEquals(Constants.STATUS_HTTP_410_GONE, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Resource was deleted at")); + } finally { + response.close(); + } + + // Delete should now have no matches + + delete = new HttpDelete(ourServerBase + "/Patient?name=" + methodName); + response = ourHttpClient.execute(delete); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Unable to find resource matching URL \"Patient?name=testDeleteResourceConditional1\". Deletion failed.")); } finally { response.close(); } @@ -1781,7 +1813,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info("Response: {}", respString); assertEquals(400, response.getStatusLine().getStatusCode()); OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, respString); - assertEquals("Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"AAA\" does not match URL ID of \"" + id.getIdPart() + "\"", oo.getIssue().get(0).getDiagnostics()); + assertEquals( + "Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"AAA\" does not match URL ID of \"" + + id.getIdPart() + "\"", + oo.getIssue().get(0).getDiagnostics()); } finally { response.close(); } @@ -1809,12 +1844,11 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testIncludeWithExternalReferences() { myDaoConfig.setAllowExternalReferences(true); - + Patient p = new Patient(); p.getManagingOrganization().setReference("http://example.com/Organization/123"); ourClient.create().resource(p).execute(); - - + Bundle b = ourClient.search().forResource("Patient").include(Patient.INCLUDE_ORGANIZATION).returnBundle(Bundle.class).execute(); assertEquals(1, b.getEntry().size()); } @@ -1834,7 +1868,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } - @Test public void testMetadataSuperParamsAreIncluded() throws IOException { StructureDefinition p = new StructureDefinition(); @@ -1843,12 +1876,12 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); Bundle resp = ourClient - .search() - .forResource(StructureDefinition.class) - .where(StructureDefinition.URL.matches().value("http://example.com/foo")) - .returnBundle(Bundle.class) - .execute(); - + .search() + .forResource(StructureDefinition.class) + .where(StructureDefinition.URL.matches().value("http://example.com/foo")) + .returnBundle(Bundle.class) + .execute(); + assertEquals(1, resp.getTotal()); } @@ -2101,9 +2134,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { String input = IOUtils.toString(getClass().getResourceAsStream("/two_questionnaires.json"), StandardCharsets.UTF_8); String respString = ourClient.transaction().withBundle(input).prettyPrint().execute(); ourLog.info(respString); - - ourHttpClient.execute(new HttpGet("http://localhost:" + ourPort + "/QuestionnaireResponse?patient=QR3295&questionnaire=profile&_sort:desc=authored&_count=5&_include=QuestionnaireResponse:questionnaire&_include=QuestionnaireResponse:subject")); -// Bundle bundle = + + ourHttpClient.execute(new HttpGet("http://localhost:" + ourPort + + "/QuestionnaireResponse?patient=QR3295&questionnaire=profile&_sort:desc=authored&_count=5&_include=QuestionnaireResponse:questionnaire&_include=QuestionnaireResponse:subject")); + // Bundle bundle = } @Test @@ -2284,14 +2318,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } Bundle found = ourClient - .search() - .forResource(Patient.class) - .where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart())) - .returnBundle(Bundle.class) - .execute(); + .search() + .forResource(Patient.class) + .where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart())) + .returnBundle(Bundle.class) + .execute(); assertThat(toUnqualifiedVersionlessIds(found), empty()); - + found = ourClient .search() .forResource(Patient.class) @@ -2319,14 +2353,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .execute(); assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1)); - + found = ourClient - .search() - .forResource(Patient.class) - .where(BaseResource.RES_ID.exactly().codes(Arrays.asList(id1.getIdPart(), id2.getIdPart(), "FOOOOO"))) - .and(BaseResource.RES_ID.exactly().code(id1.getIdPart())) - .returnBundle(Bundle.class) - .execute(); + .search() + .forResource(Patient.class) + .where(BaseResource.RES_ID.exactly().codes(Arrays.asList(id1.getIdPart(), id2.getIdPart(), "FOOOOO"))) + .and(BaseResource.RES_ID.exactly().code(id1.getIdPart())) + .returnBundle(Bundle.class) + .execute(); assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1)); @@ -3445,7 +3479,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(responseString); assertEquals(400, response.getStatusLine().getStatusCode()); OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, responseString); - assertEquals("Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"A2\"", oo.getIssue().get(0).getDiagnostics()); + assertEquals( + "Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"A2\"", + oo.getIssue().get(0).getDiagnostics()); } finally { response.close(); } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 65791e5e565..722289db81a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -127,6 +127,10 @@ Client revincludes did not include the :recurse modifier. Thanks to Jenny Meinsma for pointing this out on Zulip! + + JPA server did not return an OperationOutcome in the response for + a normal delete operation. + From 21ea5a070fd9e097feef099d7f0285e3b71dc08c Mon Sep 17 00:00:00 2001 From: James Date: Tue, 7 Feb 2017 08:46:22 -0500 Subject: [PATCH 08/10] Fix test --- .../uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8a6eebe4060..2d7d97f7ddf 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 @@ -673,7 +673,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { String encoded = myFhirCtx.newXmlParser().encodeResourceToString(response); ourLog.info(encoded); assertThat(encoded, containsString( - "")); + " Date: Tue, 7 Feb 2017 11:36:24 -0500 Subject: [PATCH 09/10] Fix conditional creates updating resource body --- .../uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java | 4 +- .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 7 +++- .../jpa/dao/dstu2/FhirSystemDaoDstu2Test.java | 39 +++++++++++++++++++ .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 36 +++++++++++++++++ .../src/test/resources/dstu3-post1.xml | 22 +++++++++++ .../src/test/resources/dstu3-post2.xml | 22 +++++++++++ src/changes/changes.xml | 7 ++++ 7 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/dstu3-post1.xml create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/dstu3-post2.xml diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java index abbbd8b2214..2af6fbc3f04 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java @@ -492,7 +492,9 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { InstantDt deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get(nextResource); Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; boolean shouldUpdate = !nonUpdatedEntities.contains(nextOutcome.getEntity()); - updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, shouldUpdate, updateTime); + if (shouldUpdate) { + updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, true, updateTime); + } } myEntityManager.flush(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java index 6a38958f9d1..51b3caa17f1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java @@ -383,7 +383,8 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); res.setId((String) null); DaoMethodOutcome outcome; - outcome = resourceDao.create(res, nextReqEntry.getRequest().getIfNoneExist(), false, theRequestDetails); + String matchUrl = nextReqEntry.getRequest().getIfNoneExist(); + outcome = resourceDao.create(res, matchUrl, false, theRequestDetails); handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res); entriesToProcess.put(nextRespEntry, outcome.getEntity()); if (outcome.getCreated() == false) { @@ -489,7 +490,9 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { IPrimitiveType deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource); Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; boolean shouldUpdate = !nonUpdatedEntities.contains(nextOutcome.getEntity()); - updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, shouldUpdate, shouldUpdate, updateTime); + if (shouldUpdate) { + updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, shouldUpdate, true, updateTime); + } } myEntityManager.flush(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java index 113709d8c6a..bd77c162878 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java @@ -19,10 +19,12 @@ import static org.mockito.Mockito.verify; import java.io.InputStream; import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import org.apache.commons.io.IOUtils; +import org.hl7.fhir.dstu3.model.IdType; import org.hl7.fhir.instance.model.api.IIdType; import org.junit.AfterClass; import org.junit.Test; @@ -73,6 +75,43 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoDstu2Test.class); + /** + * Per a message on the mailing list + */ + @Test + public void testTransactionWithPostDoesntUpdate() throws Exception { + + // First bundle (name is Joshua) + + String input = IOUtils.toString(getClass().getResource("/dstu3-post1.xml"), StandardCharsets.UTF_8); + Bundle request = myFhirCtx.newXmlParser().parseResource(Bundle.class, input); + Bundle response = mySystemDao.transaction(mySrd, request); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); + + assertEquals(1, response.getEntry().size()); + assertEquals("201 Created", response.getEntry().get(0).getResponse().getStatus()); + assertEquals("1", response.getEntry().get(0).getResponse().getEtag()); + String id = response.getEntry().get(0).getResponse().getLocation(); + + // Now the second (name is Adam, shouldn't get used) + + input = IOUtils.toString(getClass().getResource("/dstu3-post2.xml"), StandardCharsets.UTF_8); + request = myFhirCtx.newXmlParser().parseResource(Bundle.class, input); + response = mySystemDao.transaction(mySrd, request); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); + + assertEquals(1, response.getEntry().size()); + assertEquals("200 OK", response.getEntry().get(0).getResponse().getStatus()); + assertEquals("1", response.getEntry().get(0).getResponse().getEtag()); + String id2 = response.getEntry().get(0).getResponse().getLocation(); + assertEquals(id, id2); + + Patient patient = myPatientDao.read(new IdType(id), mySrd); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(patient)); + assertEquals("Joshua", patient.getNameFirstRep().getGivenAsSingleString()); + } + + @Test public void testReindexing() { Patient p = new Patient(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index fe3db761a81..fe4555ea0a3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -382,6 +382,42 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { } } + /** + * Per a message on the mailing list + */ + @Test + public void testTransactionWithPostDoesntUpdate() throws Exception { + + // First bundle (name is Joshua) + + String input = IOUtils.toString(getClass().getResource("/dstu3-post1.xml"), StandardCharsets.UTF_8); + Bundle request = myFhirCtx.newXmlParser().parseResource(Bundle.class, input); + Bundle response = mySystemDao.transaction(mySrd, request); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); + + assertEquals(1, response.getEntry().size()); + assertEquals("201 Created", response.getEntry().get(0).getResponse().getStatus()); + assertEquals("1", response.getEntry().get(0).getResponse().getEtag()); + String id = response.getEntry().get(0).getResponse().getLocation(); + + // Now the second (name is Adam, shouldn't get used) + + input = IOUtils.toString(getClass().getResource("/dstu3-post2.xml"), StandardCharsets.UTF_8); + request = myFhirCtx.newXmlParser().parseResource(Bundle.class, input); + response = mySystemDao.transaction(mySrd, request); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); + + assertEquals(1, response.getEntry().size()); + assertEquals("200 OK", response.getEntry().get(0).getResponse().getStatus()); + assertEquals("1", response.getEntry().get(0).getResponse().getEtag()); + String id2 = response.getEntry().get(0).getResponse().getLocation(); + assertEquals(id, id2); + + Patient patient = myPatientDao.read(new IdType(id), mySrd); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(patient)); + assertEquals("Joshua", patient.getNameFirstRep().getGivenAsSingleString()); + } + @Test public void testTransactionCreateInlineMatchUrlWithOneMatch() { String methodName = "testTransactionCreateInlineMatchUrlWithOneMatch"; diff --git a/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post1.xml b/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post1.xml new file mode 100644 index 00000000000..745a89effb7 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post1.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post2.xml b/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post2.xml new file mode 100644 index 00000000000..c6a60f0258a --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/dstu3-post2.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7b478705ada..f21c82a4178 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -123,6 +123,13 @@ Clean up dependencies and remove Eclipse project files from git. Thanks to @sekaijin for the pull request! + + When performing a conditional create in a transaction in JPA server, + if a resource already existed matching the conditional expression, the + server did not change the version of the resource but did update the body + with the passed in body. Thanks to Artem Sopin for reporting and providing a test + case for this! + From cb8492d353d7079f5f75c38772f3f3cd42da3b5a Mon Sep 17 00:00:00 2001 From: James Date: Tue, 7 Feb 2017 20:09:37 -0500 Subject: [PATCH 10/10] Fix occasionally failing test --- .../uhn/fhir/rest/server/SearchDefaultMethodDstu3Test.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchDefaultMethodDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchDefaultMethodDstu3Test.java index 7b50cd0a1be..e6f79e9f0a0 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchDefaultMethodDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchDefaultMethodDstu3Test.java @@ -1,8 +1,7 @@ package ca.uhn.fhir.rest.server; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.oneOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -65,7 +64,7 @@ public class SearchDefaultMethodDstu3Test { ourLog.info(responseContent); assertEquals(200, status.getStatusLine().getStatusCode()); - assertThat(ourLastMethod, either(equalTo("search01")).or(equalTo("search02"))); + assertThat(ourLastMethod, oneOf("search01", "search02", "search03")); assertEquals(null, ourLastParam1); assertEquals(null, ourLastParam2); assertEquals(null, ourLastAdditionalParams);