Skip to content

Commit

Permalink
Correctly update versions on transactipn update
Browse files Browse the repository at this point in the history
  • Loading branch information
@jamesagnew
jamesagnew committed Aug 29, 2016
1 parent ddc0abe commit 333aa0a
Show file tree
Hide file tree
Showing 16 changed files with 639 additions and 22 deletions.
3 changes: 2 additions & 1 deletion hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,13 @@ private int parseInt(String theValue, String theSubstring, int theLowerBound, in
*
* @throws DataFormatException
*/
public void setPrecision(TemporalPrecisionEnum thePrecision) throws DataFormatException {
public BaseDateTimeDt setPrecision(TemporalPrecisionEnum thePrecision) throws DataFormatException {
if (thePrecision == null) {
throw new NullPointerException("Precision may not be null");
}
myPrecision = thePrecision;
updateStringValue();
return this;
}

private BaseDateTimeDt setTimeZone(String theWholeValue, String theValue) {
Expand Down
4 changes: 2 additions & 2 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private DaoMethodOutcome doCreate(T theResource, String theIfNoneExist, boolean
}

// Perform actual DB update
updateEntity(theResource, entity, null, thePerformIndexing, true, theUpdateTime);
updateEntity(theResource, entity, null, thePerformIndexing, thePerformIndexing, theUpdateTime);
theResource.setId(entity.getIdDt());

// Notify JPA interceptors
Expand Down Expand Up @@ -1080,7 +1080,7 @@ public DaoMethodOutcome update(T theResource, String theMatchUrl, boolean thePer
}

// Perform update
ResourceTable savedEntity = updateEntity(theResource, entity, null, thePerformIndexing, true, new Date());
ResourceTable savedEntity = updateEntity(theResource, entity, null, thePerformIndexing, thePerformIndexing, new Date());

// Notify interceptors
if (theRequestDetails != null) {
Expand Down
10 changes: 6 additions & 4 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,12 @@ protected boolean hasValue(InstantDt theInstantDt) {

@Override
public IBundleProvider history(Date theSince, Date theUntil, RequestDetails theRequestDetails) {
// Notify interceptors
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails);
notifyInterceptors(RestOperationTypeEnum.HISTORY_SYSTEM, requestDetails);

if (theRequestDetails != null) {
// Notify interceptors
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails);
notifyInterceptors(RestOperationTypeEnum.HISTORY_SYSTEM, requestDetails);
}

StopWatch w = new StopWatch();
IBundleProvider retVal = super.history(null, null, theSince, theUntil);
ourLog.info("Processed global history in {}ms", w.getMillisAndRestart());
Expand Down
21 changes: 17 additions & 4 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
Expand All @@ -39,6 +40,7 @@
import javax.persistence.TypedQuery;

import org.apache.http.NameValuePair;
import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -315,7 +317,9 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe

List<IIdType> deletedResources = new ArrayList<IIdType>();
List<DeleteConflict> deleteConflicts = new ArrayList<DeleteConflict>();

Map<Entry, ResourceTable> entriesToProcess = new IdentityHashMap<Entry, ResourceTable>();
Set<ResourceTable> nonUpdatedEntities = new HashSet<ResourceTable>();

/*
* Loop through the request and process any entries of type
* PUT, POST or DELETE
Expand Down Expand Up @@ -381,6 +385,10 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
DaoMethodOutcome outcome;
outcome = resourceDao.create(res, nextReqEntry.getRequest().getIfNoneExist(), false, theRequestDetails);
handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res);
entriesToProcess.put(nextRespEntry, outcome.getEntity());
if (outcome.getCreated() == false) {
nonUpdatedEntities.add(outcome.getEntity());
}
break;
}
case DELETE: {
Expand Down Expand Up @@ -426,6 +434,7 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
}

handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res);
entriesToProcess.put(nextRespEntry, outcome.getEntity());
break;
}
}
Expand Down Expand Up @@ -474,7 +483,8 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe

InstantDt deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get(nextResource);
Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null;
updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, updateTime);
boolean shouldUpdate = !nonUpdatedEntities.contains(nextOutcome.getEntity());
updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, shouldUpdate, updateTime);
}

myEntityManager.flush();
Expand Down Expand Up @@ -578,6 +588,11 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
ourLog.info("Flushing context after {}", theActionName);
myEntityManager.flush();

for (java.util.Map.Entry<Entry, ResourceTable> nextEntry : entriesToProcess.entrySet()) {
nextEntry.getKey().getResponse().setLocation(nextEntry.getValue().getIdDt().toUnqualified().getValue());
nextEntry.getKey().getResponse().setEtag(nextEntry.getValue().getIdDt().getVersionIdPart());
}

long delay = System.currentTimeMillis() - start;
int numEntries = theRequest.getEntry().size();
long delayPer = delay / numEntries;
Expand Down Expand Up @@ -606,8 +621,6 @@ private static void handleTransactionCreateOrUpdateOutcome(Map<IdDt, IdDt> idSub
} else {
newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK));
}
newEntry.getResponse().setLocation(outcome.getId().toUnqualified().getValue());
newEntry.getResponse().setEtag(outcome.getId().getVersionIdPart());
newEntry.getResponse().setLastModified(ResourceMetadataKeyEnum.UPDATED.get(theRes));
}

Expand Down
31 changes: 24 additions & 7 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

import javax.persistence.TypedQuery;
Expand Down Expand Up @@ -264,9 +265,11 @@ private ca.uhn.fhir.jpa.dao.IFhirResourceDao<? extends IBaseResource> toDao(UrlP
@Transactional(propagation = Propagation.REQUIRED)
@Override
public Bundle transaction(RequestDetails theRequestDetails, Bundle theRequest) {
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null);
notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails);

if (theRequestDetails != null) {
ActionRequestDetails requestDetails = new ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null);
notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails);
}

String actionName = "Transaction";
return transaction((ServletRequestDetails) theRequestDetails, theRequest, actionName);
}
Expand Down Expand Up @@ -329,6 +332,8 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe

Set<String> deletedResources = new HashSet<String>();
List<DeleteConflict> deleteConflicts = new ArrayList<DeleteConflict>();
Map<BundleEntryComponent, ResourceTable> entriesToProcess = new IdentityHashMap<BundleEntryComponent, ResourceTable>();
Set<ResourceTable> nonUpdatedEntities = new HashSet<ResourceTable>();

/*
* Loop through the request and process any entries of type
Expand Down Expand Up @@ -392,6 +397,11 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
DaoMethodOutcome outcome;
outcome = resourceDao.create(res, nextReqEntry.getRequest().getIfNoneExist(), false, theRequestDetails);
handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res);
entriesToProcess.put(nextRespEntry, outcome.getEntity());
if (outcome.getCreated() == false) {
nonUpdatedEntities.add(outcome.getEntity());
}

break;
}
case DELETE: {
Expand Down Expand Up @@ -426,10 +436,10 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
@SuppressWarnings("rawtypes")
IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass());

DaoMethodOutcome outcome;

String url = extractTransactionUrlOrThrowException(nextReqEntry, verb);

DaoMethodOutcome outcome;
UrlParts parts = UrlUtil.parseUrl(url);
if (isNotBlank(parts.getResourceId())) {
res.setId(new IdType(parts.getResourceType(), parts.getResourceId()));
Expand All @@ -440,6 +450,7 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
}

handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res);
entriesToProcess.put(nextRespEntry, outcome.getEntity());
break;
}
}
Expand Down Expand Up @@ -481,14 +492,17 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe
IdType newId = idSubstitutions.get(nextId);
ourLog.info(" * Replacing resource ref {} with {}", nextId, newId);
nextRef.setReference(newId.getValue());
} else if (nextId.getValue().startsWith("urn:")) {
throw new InvalidRequestException("Unable to satisfy placeholder ID: " + nextId.getValue());
} else {
ourLog.debug(" * Reference [{}] does not exist in bundle", nextId);
}
}

IPrimitiveType<Date> deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) nextResource);
Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null;
updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, updateTime);
boolean shouldUpdate = !nonUpdatedEntities.contains(nextOutcome.getEntity());
updateEntity(nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, shouldUpdate, shouldUpdate, updateTime);
}

myEntityManager.flush();
Expand Down Expand Up @@ -589,6 +603,11 @@ private Bundle transaction(ServletRequestDetails theRequestDetails, Bundle theRe

}

for (Entry<BundleEntryComponent, ResourceTable> nextEntry : entriesToProcess.entrySet()) {
nextEntry.getKey().getResponse().setLocation(nextEntry.getValue().getIdDt().toUnqualified().getValue());
nextEntry.getKey().getResponse().setEtag(nextEntry.getValue().getIdDt().getVersionIdPart());
}

long delay = System.currentTimeMillis() - start;
ourLog.info(theActionName + " completed in {}ms", new Object[] { delay });

Expand All @@ -615,8 +634,6 @@ private static void handleTransactionCreateOrUpdateOutcome(Map<IdType, IdType> i
} else {
newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK));
}
newEntry.getResponse().setLocation(outcome.getId().toUnqualified().getValue());
newEntry.getResponse().setEtag(outcome.getId().getVersionIdPart());
newEntry.getResponse().setLastModified(((Resource)theRes).getMeta().getLastUpdated());
}

Expand Down
5 changes: 5 additions & 0 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ResourceHistoryTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public class ResourceHistoryTable extends BaseHasResource implements Serializabl
@OneToMany(mappedBy = "myResourceHistory", cascade = CascadeType.ALL, fetch = FetchType.LAZY, orphanRemoval = true)
private Collection<ResourceHistoryTag> myTags;

public ResourceHistoryTable() {
super();
}


public void addTag(ResourceHistoryTag theTag) {
for (ResourceHistoryTag next : getTags()) {
if (next.getTag().equals(theTag)) {
Expand Down
9 changes: 9 additions & 0 deletions hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.apache.commons.io.IOUtils;
import org.hibernate.search.jpa.Search;
import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
import org.hl7.fhir.dstu3.model.Resource;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
Expand Down Expand Up @@ -78,6 +79,14 @@ protected List toList(IBundleProvider theSearch) {
return theSearch.getResources(0, theSearch.size());
}

protected org.hl7.fhir.dstu3.model.Bundle toBundle(IBundleProvider theSearch) {
org.hl7.fhir.dstu3.model.Bundle bundle = new org.hl7.fhir.dstu3.model.Bundle();
for (IBaseResource next : theSearch.getResources(0, theSearch.size())) {
bundle.addEntry().setResource((Resource) next);
}
return bundle;
}

protected abstract FhirContext getContext();

protected List<String> toUnqualifiedVersionlessIdValues(IBaseBundle theFound) {
Expand Down
4 changes: 3 additions & 1 deletion hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1262,13 +1262,15 @@ public void testTransactionUpdateMatchUrlWithOneMatch() {

Bundle resp = mySystemDao.transaction(mySrd, request);
assertEquals(2, resp.getEntry().size());

ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp));

Entry nextEntry = resp.getEntry().get(0);
assertEquals("200 OK", nextEntry.getResponse().getStatus());
assertThat(nextEntry.getResponse().getLocation(), not(containsString("test")));
assertEquals(id.toVersionless(), p.getId().toVersionless());
assertNotEquals(id, p.getId());
assertThat(p.getId().toString(), endsWith("/_history/2"));
assertNotEquals(id, p.getId());

nextEntry = resp.getEntry().get(0);
assertEquals(Constants.STATUS_HTTP_200_OK + " OK", nextEntry.getResponse().getStatus());
Expand Down
54 changes: 54 additions & 0 deletions hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.nio.charset.StandardCharsets;
import java.util.List;

import javax.mail.Quota.Resource;

import org.apache.commons.io.IOUtils;
import org.hl7.fhir.dstu3.model.Appointment;
import org.hl7.fhir.dstu3.model.Bundle;
Expand Down Expand Up @@ -98,7 +100,59 @@ public void testContainedArePreservedForBug410() throws IOException {
ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(mo));
}

@Test
public void testTransactionDoesNotAllowDanglingTemporaryIds() throws Exception {
String input = IOUtils.toString(getClass().getResourceAsStream("/cdr-bundle.json"), StandardCharsets.UTF_8);
Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, input);

BundleEntryComponent entry = bundle.addEntry();
Patient p = new Patient();
p.getManagingOrganization().setReference("urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5");
entry.setResource(p);
entry.getRequest().setMethod(HTTPVerb.POST);
entry.getRequest().setUrl("Patient");

try {
mySystemDao.transaction(mySrd, bundle);
fail();
} catch (InvalidRequestException e) {
assertEquals("Unable to satisfy placeholder ID: urn:uuid:30ce60cf-f7cb-4196-961f-cadafa8b7ff5", e.getMessage());
}
}

@Test
public void testTransactionDoesNotLeavePlaceholderIds() throws Exception {
String input = IOUtils.toString(getClass().getResourceAsStream("/cdr-bundle.json"), StandardCharsets.UTF_8);
Bundle bundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, input);
mySystemDao.transaction(mySrd, bundle);

IBundleProvider history = mySystemDao.history(null, null, null);
Bundle list = toBundle(history);
ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(list));

assertEquals(6, list.getEntry().size());

Patient p = find(list, Patient.class, 0);
assertTrue(p.getIdElement().isIdPartValidLong());
assertTrue(p.getGeneralPractitionerFirstRep().getReferenceElement().isIdPartValidLong());
}

@SuppressWarnings("unchecked")
private <T extends org.hl7.fhir.dstu3.model.Resource> T find(Bundle theBundle, Class<T> theType, int theIndex) {
int count = 0;
for (BundleEntryComponent nextEntry : theBundle.getEntry()) {
if (nextEntry.getResource() != null && theType.isAssignableFrom(nextEntry.getResource().getClass())) {
if (count == theIndex) {
T t = (T) nextEntry.getResource();
return t;
}
count++;
}
}
fail();
return null;
}

@Test
public void testTransactionFromBundle2() throws Exception {
String input = IOUtils.toString(getClass().getResourceAsStream("/transaction-bundle.xml"), StandardCharsets.UTF_8);
Expand Down
Loading

0 comments on commit 333aa0a

Please sign in to comment.