Fix #147 - no duplicates in 'everything' operation
This commit is contained in:
parent
461fdb50ce
commit
8c37973a78
|
@ -101,7 +101,6 @@ import ca.uhn.fhir.rest.method.DeleteMethodBinding;
|
|||
import ca.uhn.fhir.rest.method.HistoryMethodBinding;
|
||||
import ca.uhn.fhir.rest.method.HttpDeleteClientInvocation;
|
||||
import ca.uhn.fhir.rest.method.HttpGetClientInvocation;
|
||||
import ca.uhn.fhir.rest.method.HttpPostClientInvocation;
|
||||
import ca.uhn.fhir.rest.method.HttpSimpleGetClientInvocation;
|
||||
import ca.uhn.fhir.rest.method.IClientResponseHandler;
|
||||
import ca.uhn.fhir.rest.method.MethodUtil;
|
||||
|
@ -1538,6 +1537,7 @@ public class GenericClient extends BaseClient implements IGenericClient {
|
|||
|
||||
private Bundle myBundle;
|
||||
private List<IResource> myResources;
|
||||
private IBaseBundle myBaseBundle;
|
||||
|
||||
public TransactionExecutable(Bundle theResources) {
|
||||
myBundle = theResources;
|
||||
|
@ -1547,18 +1547,25 @@ public class GenericClient extends BaseClient implements IGenericClient {
|
|||
myResources = theResources;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
public TransactionExecutable(IBaseBundle theBundle) {
|
||||
myBaseBundle = theBundle;
|
||||
}
|
||||
|
||||
@SuppressWarnings({ "unchecked", "rawtypes" })
|
||||
@Override
|
||||
public T execute() {
|
||||
Map<String, List<String>> params = new HashMap<String, List<String>>();
|
||||
if (myResources != null) {
|
||||
ResourceListResponseHandler binding = new ResourceListResponseHandler(null);
|
||||
BaseHttpClientInvocation invocation = TransactionMethodBinding.createTransactionInvocation(myResources, myContext);
|
||||
Map<String, List<String>> params = new HashMap<String, List<String>>();
|
||||
return (T) invoke(params, binding, invocation);
|
||||
} else if (myBaseBundle != null) {
|
||||
ResourceResponseHandler binding = new ResourceResponseHandler(myBaseBundle.getClass(),null);
|
||||
BaseHttpClientInvocation invocation = TransactionMethodBinding.createTransactionInvocation(myBaseBundle, myContext);
|
||||
return (T) invoke(params, binding, invocation);
|
||||
} else {
|
||||
BundleResponseHandler binding = new BundleResponseHandler(null);
|
||||
BaseHttpClientInvocation invocation = TransactionMethodBinding.createTransactionInvocation(myBundle, myContext);
|
||||
Map<String, List<String>> params = new HashMap<String, List<String>>();
|
||||
return (T) invoke(params, binding, invocation);
|
||||
}
|
||||
}
|
||||
|
@ -1568,15 +1575,23 @@ public class GenericClient extends BaseClient implements IGenericClient {
|
|||
private final class TransactionInternal implements ITransaction {
|
||||
|
||||
@Override
|
||||
public ITransactionTyped<Bundle> withBundle(Bundle theResources) {
|
||||
return new TransactionExecutable<Bundle>(theResources);
|
||||
public ITransactionTyped<Bundle> withBundle(Bundle theBundle) {
|
||||
Validate.notNull(theBundle, "theBundle must not be null");
|
||||
return new TransactionExecutable<Bundle>(theBundle);
|
||||
}
|
||||
|
||||
@Override
|
||||
public ITransactionTyped<List<IResource>> withResources(List<IResource> theResources) {
|
||||
Validate.notNull(theResources, "theResources must not be null");
|
||||
return new TransactionExecutable<List<IResource>>(theResources);
|
||||
}
|
||||
|
||||
@Override
|
||||
public <T extends IBaseBundle> ITransactionTyped<T> withBundle(T theBundle) {
|
||||
Validate.notNull(theBundle, "theBundle must not be null");
|
||||
return new TransactionExecutable<T>(theBundle);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private class UpdateInternal extends BaseClientExecutable<IUpdateExecutable, MethodOutcome> implements IUpdate, IUpdateTyped, IUpdateExecutable, IUpdateWithQuery, IUpdateWithQueryTyped {
|
||||
|
|
|
@ -22,14 +22,27 @@ package ca.uhn.fhir.rest.gclient;
|
|||
|
||||
import java.util.List;
|
||||
|
||||
import org.hl7.fhir.instance.model.api.IBaseBundle;
|
||||
|
||||
import ca.uhn.fhir.model.api.Bundle;
|
||||
import ca.uhn.fhir.model.api.IResource;
|
||||
|
||||
public interface ITransaction {
|
||||
|
||||
/**
|
||||
* Use a list of resources as the transaction input
|
||||
*/
|
||||
ITransactionTyped<List<IResource>> withResources(List<IResource> theResources);
|
||||
|
||||
ITransactionTyped<Bundle> withBundle(Bundle theResources);
|
||||
/**
|
||||
* Use a DSTU1 Bundle (Atom) as the transaction input
|
||||
*/
|
||||
ITransactionTyped<Bundle> withBundle(Bundle theBundle);
|
||||
|
||||
/**
|
||||
* Use a DSTU2+ Bundle resource as the transaction input
|
||||
*/
|
||||
<T extends IBaseBundle> ITransactionTyped<T> withBundle(T theBundleResource);
|
||||
|
||||
// *****
|
||||
// TODO: add withString version
|
||||
|
|
|
@ -40,6 +40,10 @@ public class HttpPostClientInvocation extends BaseHttpClientInvocationWithConten
|
|||
super(theContext, theResource, theUrlExtension);
|
||||
}
|
||||
|
||||
public HttpPostClientInvocation(FhirContext theContext, IBaseResource theResource) {
|
||||
super(theContext, theResource, null);
|
||||
}
|
||||
|
||||
public HttpPostClientInvocation(FhirContext theContext, TagList theTagList, String... theUrlExtension) {
|
||||
super(theContext, theTagList, theUrlExtension);
|
||||
}
|
||||
|
|
|
@ -27,6 +27,8 @@ import java.lang.reflect.Method;
|
|||
import java.util.IdentityHashMap;
|
||||
import java.util.List;
|
||||
|
||||
import org.hl7.fhir.instance.model.api.IBaseBundle;
|
||||
|
||||
import ca.uhn.fhir.context.ConfigurationException;
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.model.api.Bundle;
|
||||
|
@ -73,6 +75,21 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding
|
|||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public RestfulOperationTypeEnum getResourceOperationType() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected BundleTypeEnum getResponseBundleType() {
|
||||
return BundleTypeEnum.TRANSACTION_RESPONSE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public ReturnTypeEnum getReturnType() {
|
||||
return ReturnTypeEnum.BUNDLE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public RestfulOperationSystemEnum getSystemOperationType() {
|
||||
return RestfulOperationSystemEnum.TRANSACTION;
|
||||
|
@ -93,26 +110,33 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding
|
|||
}
|
||||
|
||||
@Override
|
||||
public ReturnTypeEnum getReturnType() {
|
||||
return ReturnTypeEnum.BUNDLE;
|
||||
public BaseHttpClientInvocation invokeClient(Object[] theArgs) throws InternalErrorException {
|
||||
FhirContext context = getContext();
|
||||
if (theArgs[myTransactionParamIndex] instanceof Bundle) {
|
||||
Bundle bundle = (Bundle) theArgs[myTransactionParamIndex];
|
||||
return createTransactionInvocation(bundle, context);
|
||||
} else {
|
||||
@SuppressWarnings("unchecked")
|
||||
List<IResource> resources = (List<IResource>) theArgs[myTransactionParamIndex];
|
||||
return createTransactionInvocation(resources, context);
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Override
|
||||
public Object invokeServer(RequestDetails theRequest, Object[] theMethodParams) throws InvalidRequestException, InternalErrorException {
|
||||
|
||||
|
||||
/*
|
||||
* The design of HAPI's transaction method for DSTU1 support assumed that a
|
||||
* transaction was just an update on a bunch of resources (because that's what
|
||||
* it was), but in DSTU2 transaction has become much more broad, so we no longer
|
||||
* hold the user's hand much here.
|
||||
* The design of HAPI's transaction method for DSTU1 support assumed that a transaction was just an update on a
|
||||
* bunch of resources (because that's what it was), but in DSTU2 transaction has become much more broad, so we
|
||||
* no longer hold the user's hand much here.
|
||||
*/
|
||||
if (myTransactionParamStyle == ParamStyle.RESOURCE_BUNDLE) {
|
||||
// This is the DSTU2 style
|
||||
Object response = invokeServerMethod(theMethodParams);
|
||||
return response;
|
||||
}
|
||||
|
||||
|
||||
// Grab the IDs of all of the resources in the transaction
|
||||
List<IResource> resources;
|
||||
if (theMethodParams[myTransactionParamIndex] instanceof Bundle) {
|
||||
|
@ -162,35 +186,16 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding
|
|||
return null; // This is parsed in TransactionParamBinder
|
||||
}
|
||||
|
||||
@Override
|
||||
public RestfulOperationTypeEnum getResourceOperationType() {
|
||||
return null;
|
||||
public static BaseHttpClientInvocation createTransactionInvocation(Bundle theBundle, FhirContext theContext) {
|
||||
return new HttpPostClientInvocation(theContext, theBundle);
|
||||
}
|
||||
|
||||
@Override
|
||||
public BaseHttpClientInvocation invokeClient(Object[] theArgs) throws InternalErrorException {
|
||||
FhirContext context = getContext();
|
||||
if (theArgs[myTransactionParamIndex] instanceof Bundle) {
|
||||
Bundle bundle = (Bundle) theArgs[myTransactionParamIndex];
|
||||
return createTransactionInvocation(bundle, context);
|
||||
} else {
|
||||
@SuppressWarnings("unchecked")
|
||||
List<IResource> resources = (List<IResource>) theArgs[myTransactionParamIndex];
|
||||
return createTransactionInvocation(resources, context);
|
||||
}
|
||||
public static BaseHttpClientInvocation createTransactionInvocation(IBaseBundle theBundle, FhirContext theContext) {
|
||||
return new HttpPostClientInvocation(theContext, theBundle);
|
||||
}
|
||||
|
||||
public static BaseHttpClientInvocation createTransactionInvocation(List<IResource> theResources, FhirContext theContext) {
|
||||
return new HttpPostClientInvocation(theContext, theResources, BundleTypeEnum.TRANSACTION);
|
||||
}
|
||||
|
||||
public static BaseHttpClientInvocation createTransactionInvocation(Bundle theBundle, FhirContext theContext) {
|
||||
return new HttpPostClientInvocation(theContext, theBundle);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected BundleTypeEnum getResponseBundleType() {
|
||||
return BundleTypeEnum.TRANSACTION_RESPONSE;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -1366,6 +1366,9 @@ public abstract class BaseFhirResourceDao<T extends IResource> extends BaseFhirD
|
|||
*/
|
||||
if (theParams.getIncludes() != null && theParams.getIncludes().isEmpty() == false) {
|
||||
Set<IdDt> previouslyLoadedPids = new HashSet<IdDt>();
|
||||
for (IResource next : retVal) {
|
||||
previouslyLoadedPids.add(next.getId().toUnqualifiedVersionless());
|
||||
}
|
||||
|
||||
Set<IdDt> includePids = new HashSet<IdDt>();
|
||||
List<IResource> resources = retVal;
|
||||
|
|
|
@ -487,8 +487,9 @@ public class FhirSystemDaoDstu2 extends BaseFhirSystemDao<Bundle> {
|
|||
|
||||
private static void handleTransactionCreateOrUpdateOutcome(Map<IdDt, IdDt> idSubstitutions, Map<IdDt, DaoMethodOutcome> idToPersistedOutcome, IdDt nextResourceId, DaoMethodOutcome outcome, Entry newEntry) {
|
||||
IdDt newId = outcome.getId().toUnqualifiedVersionless();
|
||||
if (newId.equals(nextResourceId) == false) {
|
||||
idSubstitutions.put(nextResourceId, newId);
|
||||
IdDt resourceId = nextResourceId.toUnqualifiedVersionless();
|
||||
if (newId.equals(resourceId) == false) {
|
||||
idSubstitutions.put(resourceId, newId);
|
||||
}
|
||||
idToPersistedOutcome.put(newId, outcome);
|
||||
if (outcome.getCreated().booleanValue()) {
|
||||
|
|
|
@ -488,12 +488,12 @@ public class FhirSystemDaoDstu2Test {
|
|||
p = new Patient();
|
||||
p.addIdentifier().setSystem("urn:system").setValue(methodName);
|
||||
p.addName().addFamily("Hello");
|
||||
p.setId(id);
|
||||
p.setId("urn:"+methodName);
|
||||
request.addEntry().setResource(p).getTransaction().setMethod(HTTPVerbEnum.PUT).setUrl("Patient?identifier=urn%3Asystem%7C" + methodName);
|
||||
|
||||
Observation o = new Observation();
|
||||
o.getCode().setText("Some Observation");
|
||||
o.getSubject().setReference(id);
|
||||
o.getSubject().setReference("Patient/urn:"+methodName);
|
||||
request.addEntry().setResource(o).getTransaction().setMethod(HTTPVerbEnum.POST);
|
||||
|
||||
Bundle resp = ourSystemDao.transaction(request);
|
||||
|
@ -501,18 +501,16 @@ public class FhirSystemDaoDstu2Test {
|
|||
|
||||
Entry nextEntry = resp.getEntry().get(1);
|
||||
assertEquals(Constants.STATUS_HTTP_201_CREATED + "", nextEntry.getTransactionResponse().getStatus());
|
||||
IdDt patientId = new IdDt(nextEntry.getTransactionResponse().getLocation());
|
||||
|
||||
assertThat(nextEntry.getTransactionResponse().getLocation(), not(containsString("test")));
|
||||
assertNotEquals(id.toVersionless(), new IdDt(nextEntry.getTransactionResponse().getLocation()).toVersionless());
|
||||
assertNotEquals(id.toVersionless(), patientId.toVersionless());
|
||||
|
||||
assertThat(nextEntry.getTransactionResponse().getLocation(), endsWith("/_history/1"));
|
||||
|
||||
nextEntry = resp.getEntry().get(1);
|
||||
assertEquals("" + Constants.STATUS_HTTP_201_CREATED, nextEntry.getTransactionResponse().getStatus());
|
||||
assertThat(patientId.getValue(), endsWith("/_history/1"));
|
||||
|
||||
nextEntry = resp.getEntry().get(2);
|
||||
o = ourObservationDao.read(new IdDt(nextEntry.getTransactionResponse().getLocation()));
|
||||
assertEquals(id.toVersionless(), o.getSubject().getReference());
|
||||
assertEquals(patientId.toVersionless(), o.getSubject().getReference());
|
||||
|
||||
}
|
||||
|
||||
|
|
|
@ -9,6 +9,7 @@ import static org.hamcrest.Matchers.startsWith;
|
|||
import static org.junit.Assert.*;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InputStreamReader;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Date;
|
||||
import java.util.HashSet;
|
||||
|
@ -35,6 +36,7 @@ import org.junit.BeforeClass;
|
|||
import org.junit.Test;
|
||||
import org.springframework.context.support.ClassPathXmlApplicationContext;
|
||||
|
||||
import ch.qos.logback.core.util.FileUtil;
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.jpa.dao.DaoConfig;
|
||||
import ca.uhn.fhir.jpa.dao.IFhirResourceDao;
|
||||
|
@ -345,14 +347,45 @@ public class ResourceProviderDstu2Test {
|
|||
ids.add(next.getResource().getId());
|
||||
}
|
||||
|
||||
ourLog.info(ids.toString());
|
||||
|
||||
assertThat(ids, containsInAnyOrder(patientId, devId, obsId, encId, orgId1, orgId2));
|
||||
|
||||
// _revinclude's are counted but not _include's
|
||||
assertEquals(3, b.getTotal().intValue());
|
||||
|
||||
ourLog.info(ids.toString());
|
||||
}
|
||||
|
||||
/**
|
||||
* See #147
|
||||
*/
|
||||
@Test
|
||||
public void testEverythingDoesnRepeatPatient() throws Exception {
|
||||
ca.uhn.fhir.model.dstu2.resource.Bundle b;
|
||||
b = ourFhirCtx.newJsonParser().parseResource(ca.uhn.fhir.model.dstu2.resource.Bundle.class, new InputStreamReader(ResourceProviderDstu2Test.class.getResourceAsStream("/bug147-bundle.json")));
|
||||
|
||||
ca.uhn.fhir.model.dstu2.resource.Bundle resp = ourClient.transaction().withBundle(b).execute();
|
||||
|
||||
ourLog.info(ourFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(resp));
|
||||
|
||||
IdDt patientId = new IdDt(resp.getEntry().get(1).getTransactionResponse().getLocation());
|
||||
assertEquals("Patient", patientId.getResourceType());
|
||||
|
||||
Parameters output = ourClient.operation().onInstance(patientId).named("everything").withNoParameters(Parameters.class).execute();
|
||||
b = (ca.uhn.fhir.model.dstu2.resource.Bundle) output.getParameterFirstRep().getResource();
|
||||
|
||||
List<IdDt> ids = new ArrayList<IdDt>();
|
||||
boolean dupes = false;
|
||||
for (Entry next : b.getEntry()) {
|
||||
IdDt toAdd = next.getResource().getId().toUnqualifiedVersionless();
|
||||
dupes = dupes | ids.contains(toAdd);
|
||||
ids.add(toAdd);
|
||||
}
|
||||
|
||||
ourLog.info(ids.toString());
|
||||
|
||||
assertFalse(ids.toString(), dupes);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCountParam() throws Exception {
|
||||
|
|
File diff suppressed because one or more lines are too long
|
@ -614,6 +614,47 @@ public class GenericClientTestDstu2 {
|
|||
// assertEquals("PATIENT2", p2.getName().get(0).getFamily().get(0).getValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTransactionWithTransactionResource() throws Exception {
|
||||
|
||||
ca.uhn.fhir.model.dstu2.resource.Bundle resp = new ca.uhn.fhir.model.dstu2.resource.Bundle();
|
||||
resp.addEntry().getTransactionResponse().setLocation("Patient/1/_history/1");
|
||||
resp.addEntry().getTransactionResponse().setLocation("Patient/2/_history/2");
|
||||
String respString = ourCtx.newJsonParser().encodeResourceToString(resp);
|
||||
|
||||
ArgumentCaptor<HttpUriRequest> 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"));
|
||||
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_JSON + "; charset=UTF-8"));
|
||||
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(respString), Charset.forName("UTF-8")));
|
||||
|
||||
IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir");
|
||||
|
||||
ca.uhn.fhir.model.dstu2.resource.Bundle input = new ca.uhn.fhir.model.dstu2.resource.Bundle();
|
||||
|
||||
Patient p1 = new Patient(); // No ID
|
||||
p1.addName().addFamily("PATIENT1");
|
||||
input.addEntry().setResource(p1);
|
||||
|
||||
Patient p2 = new Patient(); // Yes ID
|
||||
p2.addName().addFamily("PATIENT2");
|
||||
p2.setId("Patient/2");
|
||||
input.addEntry().setResource(p2);
|
||||
|
||||
//@formatter:off
|
||||
ca.uhn.fhir.model.dstu2.resource.Bundle response = client.transaction()
|
||||
.withBundle(input)
|
||||
.encodedJson()
|
||||
.execute();
|
||||
//@formatter:on
|
||||
|
||||
assertEquals("http://example.com/fhir?_format=json", capt.getValue().getURI().toString());
|
||||
assertEquals(2, response.getEntry().size());
|
||||
|
||||
assertEquals("Patient/1/_history/1", response.getEntry().get(0).getTransactionResponse().getLocation());
|
||||
assertEquals("Patient/2/_history/2", response.getEntry().get(1).getTransactionResponse().getLocation());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDeleteConditional() throws Exception {
|
||||
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
|
||||
|
|
|
@ -114,6 +114,15 @@
|
|||
for all of their help in tracking this issue down and developing
|
||||
useful unit tests to demonstrate it.
|
||||
</action>
|
||||
<action type="add">
|
||||
Client now supports invoking transcation using a DSTU2-style
|
||||
Bundle resource as the input.
|
||||
</action>
|
||||
<action type="fix" issue="147">
|
||||
JPA Server $everything operation could sometimes include a duplicate copy of
|
||||
the main focus resource if it was referred to in a deep chain. Thanks
|
||||
to David Hay for reporting!
|
||||
</action>
|
||||
</release>
|
||||
<release version="0.9" date="2015-Mar-14">
|
||||
<action type="add">
|
||||
|
|
Loading…
Reference in New Issue