diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index c678e68331e..61d29393b95 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -53,6 +53,7 @@ import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; +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; @@ -93,6 +94,7 @@ import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.ResourceTag; import ca.uhn.fhir.jpa.entity.TagDefinition; import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.jpa.util.StopWatch; import ca.uhn.fhir.model.api.IQueryParameterAnd; import ca.uhn.fhir.model.api.IQueryParameterType; @@ -126,14 +128,20 @@ 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.ResourceNotFoundException; +import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.util.FhirTerser; +import ca.uhn.fhir.util.OperationOutcomeUtil; import net.sourceforge.cobertura.CoverageIgnore; public abstract class BaseHapiFhirDao implements IDao { + static final String OO_SEVERITY_ERROR = "error"; + static final String OO_SEVERITY_INFO = "information"; + static final String OO_SEVERITY_WARN = "warning"; + /** * These are parameters which are supported by {@link BaseHapiFhirResourceDao#searchForIds(Map)} */ @@ -160,6 +168,7 @@ public abstract class BaseHapiFhirDao implements IDao { RESOURCE_META_AND_PARAMS = Collections.unmodifiableMap(resourceMetaAndParams); } + public static final long INDEX_STATUS_INDEXED = Long.valueOf(1L); public static final long INDEX_STATUS_INDEXING_FAILED = Long.valueOf(2L); public static final String NS_JPA_PROFILE = "https://github.com/jamesagnew/hapi-fhir/ns/jpa/profile"; @@ -1562,6 +1571,20 @@ public abstract class BaseHapiFhirDao implements IDao { return retVal.toString(); } + protected void validateDeleteConflictsEmptyOrThrowException(List theDeleteConflicts) { + if (theDeleteConflicts.isEmpty()) { + return; + } + + IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(getContext()); + for (DeleteConflict next : theDeleteConflicts) { + String msg = "Unable to delete " + next.getTargetId().toUnqualifiedVersionless().getValue() + " because at least one resource has a reference to this resource. First reference found was resource " + next.getTargetId().toUnqualifiedVersionless().getValue() + " in path " + next.getSourcePath(); + OperationOutcomeUtil.addIssue(getContext(), oo, OO_SEVERITY_ERROR, msg, null, "processing"); + } + + throw new ResourceVersionConflictException("Delete failed because of constraint failure", oo); + } + public BaseHasResource readEntity(IIdType theValueId) { throw new NotImplementedException(""); } 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 092837c59d9..35e19d384fa 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 @@ -62,6 +62,7 @@ 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.interceptor.IJpaServerInterceptor; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.jpa.util.StopWatch; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.model.api.IResource; @@ -81,7 +82,6 @@ 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.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; @@ -91,31 +91,27 @@ import ca.uhn.fhir.util.ObjectUtil; @Transactional(propagation = Propagation.REQUIRED) public abstract class BaseHapiFhirResourceDao extends BaseHapiFhirDao implements IFhirResourceDao { - static final String OO_SEVERITY_ERROR = "error"; - static final String OO_SEVERITY_INFO = "information"; - static final String OO_SEVERITY_WARN = "warning"; - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseHapiFhirResourceDao.class); + @Autowired + private DaoConfig myDaoConfig; + @PersistenceContext(type = PersistenceContextType.TRANSACTION) protected EntityManager myEntityManager; @Autowired protected PlatformTransactionManager myPlatformTransactionManager; - @Autowired - private DaoConfig myDaoConfig; + private String myResourceName; + private Class myResourceType; @Autowired(required = false) protected ISearchDao mySearchDao; - - private String myResourceName; - private Class myResourceType; - private String mySecondaryPrimaryKeyParamName; - @Autowired() protected ISearchResultDao mySearchResultDao; + private String mySecondaryPrimaryKeyParamName; + @Override public void addTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm, String theLabel) { StopWatch w = new StopWatch(); @@ -184,16 +180,28 @@ public abstract class BaseHapiFhirResourceDao extends BaseH @Override public DaoMethodOutcome delete(IIdType theId) { + List deleteConflicts = new ArrayList(); + StopWatch w = new StopWatch(); + + ResourceTable savedEntity = delete(theId, deleteConflicts); + + validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + + ourLog.info("Processed delete on {} in {}ms", theId.getValue(), w.getMillisAndRestart()); + return toMethodOutcome(savedEntity, null); + } + + @Override + public ResourceTable delete(IIdType theId, List deleteConflicts) { if (theId == null || !theId.hasIdPart()) { throw new InvalidRequestException("Can not perform delete, no ID provided"); } - StopWatch w = new StopWatch(); final ResourceTable entity = readEntityLatestVersion(theId); if (theId.hasVersionIdPart() && Long.parseLong(theId.getVersionIdPart()) != entity.getVersion()) { throw new InvalidRequestException("Trying to delete " + theId + " but this is not the current version"); } - validateOkToDeleteOrThrowResourceVersionConflictException(entity); + validateOkToDelete(deleteConflicts, entity); // Notify interceptors ActionRequestDetails requestDetails = new ActionRequestDetails(theId, theId.getResourceType()); @@ -208,37 +216,41 @@ public abstract class BaseHapiFhirResourceDao extends BaseH ((IJpaServerInterceptor) next).resourceDeleted(requestDetails, entity); } } - - ourLog.info("Processed delete on {} in {}ms", theId.getValue(), w.getMillisAndRestart()); - return toMethodOutcome(savedEntity, null); + return savedEntity; } @Override public DaoMethodOutcome deleteByUrl(String theUrl) { - return deleteByUrl(theUrl, false); + StopWatch w = new StopWatch(); + List deleteConflicts = new ArrayList(); + + List deletedResources = deleteByUrl(theUrl, deleteConflicts); + + validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + + if (deletedResources.isEmpty()) { + throw new ResourceNotFoundException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "unableToDeleteNotFound", theUrl)); + } + + ourLog.info("Processed delete on {} (matched {} resource(s)) in {}ms", new Object[] { theUrl, deletedResources.size(), w.getMillisAndRestart() }); + return new DaoMethodOutcome(); } @Override - public DaoMethodOutcome deleteByUrl(String theUrl, boolean theInTransaction) { - StopWatch w = new StopWatch(); - + public List deleteByUrl(String theUrl, List deleteConflicts) { Set resource = processMatchUrl(theUrl, myResourceType); - if (resource.isEmpty()) { - if (!theInTransaction) { - throw new ResourceNotFoundException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "unableToDeleteNotFound", theUrl)); - } else { - return new DaoMethodOutcome(); - } - } else if (resource.size() > 1) { + if (resource.size() > 1) { if (myDaoConfig.isAllowMultipleDelete() == false) { throw new PreconditionFailedException(getContext().getLocalizer().getMessage(BaseHapiFhirDao.class, "transactionOperationWithMultipleMatchFailure", "DELETE", theUrl, resource.size())); } } + List retVal = new ArrayList(); for (Long pid : resource) { ResourceTable entity = myEntityManager.find(ResourceTable.class, pid); - - validateOkToDeleteOrThrowResourceVersionConflictException(entity); + retVal.add(entity); + + validateOkToDelete(deleteConflicts, entity); // Notify interceptors IdDt idToDelete = entity.getIdDt(); @@ -257,10 +269,8 @@ public abstract class BaseHapiFhirResourceDao extends BaseH } } - - ourLog.info("Processed delete on {} (matched {} resource(s)) in {}ms", new Object[] { theUrl, resource.size(), w.getMillisAndRestart() }); - - return new DaoMethodOutcome(); + + return retVal; } private DaoMethodOutcome doCreate(T theResource, String theIfNoneExist, boolean thePerformIndexing, Date theUpdateTime) { @@ -491,6 +501,49 @@ public abstract class BaseHapiFhirResourceDao extends BaseH return retVal; } + @Override + public MetaDt metaAddOperation(IIdType theResourceId, MetaDt theMetaAdd) { + // Notify interceptors + ActionRequestDetails requestDetails = new ActionRequestDetails(theResourceId, getResourceName()); + notifyInterceptors(RestOperationTypeEnum.META_ADD, requestDetails); + + StopWatch w = new StopWatch(); + BaseHasResource entity = readEntity(theResourceId); + if (entity == null) { + throw new ResourceNotFoundException(theResourceId); + } + + List tags = toTagList(theMetaAdd); + + //@formatter:off + for (TagDefinition nextDef : tags) { + + boolean hasTag = false; + for (BaseTag next : new ArrayList(entity.getTags())) { + if (ObjectUtil.equals(next.getTag().getTagType(), nextDef.getTagType()) && + ObjectUtil.equals(next.getTag().getSystem(), nextDef.getSystem()) && + ObjectUtil.equals(next.getTag().getCode(), nextDef.getCode())) { + hasTag = true; + break; + } + } + + if (!hasTag) { + entity.setHasTags(true); + + TagDefinition def = getTag(nextDef.getTagType(), nextDef.getSystem(), nextDef.getCode(), nextDef.getDisplay()); + BaseTag newEntity = entity.addTag(def); + myEntityManager.persist(newEntity); + } + } + //@formatter:on + + myEntityManager.merge(entity); + ourLog.info("Processed metaAddOperation on {} in {}ms", new Object[] { theResourceId, w.getMillisAndRestart() }); + + return metaGetOperation(theResourceId); + } + // @Override // public IBundleProvider everything(IIdType theId) { // Search search = new Search(); @@ -573,49 +626,6 @@ public abstract class BaseHapiFhirResourceDao extends BaseH // }; // } - @Override - public MetaDt metaAddOperation(IIdType theResourceId, MetaDt theMetaAdd) { - // Notify interceptors - ActionRequestDetails requestDetails = new ActionRequestDetails(theResourceId, getResourceName()); - notifyInterceptors(RestOperationTypeEnum.META_ADD, requestDetails); - - StopWatch w = new StopWatch(); - BaseHasResource entity = readEntity(theResourceId); - if (entity == null) { - throw new ResourceNotFoundException(theResourceId); - } - - List tags = toTagList(theMetaAdd); - - //@formatter:off - for (TagDefinition nextDef : tags) { - - boolean hasTag = false; - for (BaseTag next : new ArrayList(entity.getTags())) { - if (ObjectUtil.equals(next.getTag().getTagType(), nextDef.getTagType()) && - ObjectUtil.equals(next.getTag().getSystem(), nextDef.getSystem()) && - ObjectUtil.equals(next.getTag().getCode(), nextDef.getCode())) { - hasTag = true; - break; - } - } - - if (!hasTag) { - entity.setHasTags(true); - - TagDefinition def = getTag(nextDef.getTagType(), nextDef.getSystem(), nextDef.getCode(), nextDef.getDisplay()); - BaseTag newEntity = entity.addTag(def); - myEntityManager.persist(newEntity); - } - } - //@formatter:on - - myEntityManager.merge(entity); - ourLog.info("Processed metaAddOperation on {} in {}ms", new Object[] { theResourceId, w.getMillisAndRestart() }); - - return metaGetOperation(theResourceId); - } - @Override public MetaDt metaDeleteOperation(IIdType theResourceId, MetaDt theMetaDel) { // Notify interceptors @@ -810,6 +820,11 @@ public abstract class BaseHapiFhirResourceDao extends BaseH return entity; } + @Override + public void reindex(T theResource, ResourceTable theEntity) { + updateEntity(theResource, theEntity, false, null, true, false, theEntity.getUpdatedDate()); + } + @Override public void removeTag(IIdType theId, TagTypeEnum theTagType, String theScheme, String theTerm) { // Notify interceptors @@ -842,11 +857,6 @@ public abstract class BaseHapiFhirResourceDao extends BaseH ourLog.info("Processed remove tag {}/{} on {} in {}ms", new Object[] { theScheme, theTerm, theId.getValue(), w.getMillisAndRestart() }); } - @Override - public void reindex(T theResource, ResourceTable theEntity) { - updateEntity(theResource, theEntity, false, null, true, false, theEntity.getUpdatedDate()); - } - @Override public IBundleProvider search(Map theParams) { SearchParameterMap map = new SearchParameterMap(); @@ -855,7 +865,7 @@ public abstract class BaseHapiFhirResourceDao extends BaseH } return search(map); } - + @Override public IBundleProvider search(final SearchParameterMap theParams) { // Notify interceptors @@ -924,9 +934,6 @@ public abstract class BaseHapiFhirResourceDao extends BaseH return retVal; } - - - private ArrayList toTagList(MetaDt theMeta) { ArrayList retVal = new ArrayList(); @@ -943,6 +950,9 @@ public abstract class BaseHapiFhirResourceDao extends BaseH return retVal; } + + + @Override public DaoMethodOutcome update(T theResource) { return update(theResource, null); @@ -1032,7 +1042,7 @@ public abstract class BaseHapiFhirResourceDao extends BaseH } } - protected void validateOkToDeleteOrThrowResourceVersionConflictException(ResourceTable theEntity) { + protected void validateOkToDelete(List theDeleteConflicts, ResourceTable theEntity) { TypedQuery query = myEntityManager.createQuery("SELECT l FROM ResourceLink l WHERE l.myTargetResourcePid = :target_pid", ResourceLink.class); query.setParameter("target_pid", theEntity.getId()); query.setMaxResults(1); @@ -1042,13 +1052,15 @@ public abstract class BaseHapiFhirResourceDao extends BaseH } ResourceLink link = resultList.get(0); - String targetId = theEntity.getIdDt().toUnqualifiedVersionless().getValue(); - String sourceId = link.getSourceResource().getIdDt().toUnqualifiedVersionless().getValue(); + IdDt targetId = theEntity.getIdDt(); + IdDt sourceId = link.getSourceResource().getIdDt(); String sourcePath = link.getSourcePath(); - throw new ResourceVersionConflictException("Unable to delete " + targetId + " because at least one resource has a reference to this resource. First reference found was resource " + sourceId + " in path " + sourcePath); + theDeleteConflicts.add(new DeleteConflict(sourceId, sourcePath, targetId)); } + + private void validateResourceType(BaseHasResource entity) { validateResourceType(entity, myResourceName); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2.java index 8dbd991e792..1571e749fb1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2.java @@ -37,6 +37,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.model.api.Bundle; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.Include; @@ -108,14 +109,15 @@ public class FhirResourceDaoDstu2 extends BaseHapiFhirResou throw new InvalidRequestException("No ID supplied. ID is required when validating with mode=DELETE"); } final ResourceTable entity = readEntityLatestVersion(theId); + + // Validate that there are no resources pointing to the candidate that + // would prevent deletion + List deleteConflicts = new ArrayList(); + validateOkToDelete(deleteConflicts, entity); + validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + OperationOutcome oo = new OperationOutcome(); - try { - validateOkToDeleteOrThrowResourceVersionConflictException(entity); - oo.addIssue().setSeverity(IssueSeverityEnum.INFORMATION).setDiagnostics("Ok to delete"); - } catch (ResourceVersionConflictException e) { - oo.addIssue().setSeverity(IssueSeverityEnum.ERROR).setDiagnostics(e.getMessage()); - throw new ResourceVersionConflictException(e.getMessage(), oo); - } + oo.addIssue().setSeverity(IssueSeverityEnum.INFORMATION).setDiagnostics("Ok to delete"); return new MethodOutcome(new IdDt(theId.getValue()), oo); } 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 a3decc92d96..dfdeb3dcad8 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 @@ -23,9 +23,14 @@ import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -35,6 +40,7 @@ import javax.persistence.TypedQuery; import org.apache.http.NameValuePair; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; @@ -47,7 +53,9 @@ import org.springframework.transaction.support.TransactionTemplate; import com.google.common.collect.ArrayListMultimap; import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TagDefinition; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt; @@ -86,14 +94,6 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { @Autowired private PlatformTransactionManager myTxManager; - private String extractTransactionUrlOrThrowException(Entry nextEntry, HTTPVerbEnum verb) { - String url = nextEntry.getRequest().getUrl(); - if (isBlank(url)) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionMissingUrl", verb.name())); - } - return url; - } - private Bundle batch(final RequestDetails theRequestDetails, Bundle theRequest) { ourLog.info("Beginning batch with {} resources", theRequest.getEntry().size()); long start = System.currentTimeMillis(); @@ -165,6 +165,36 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { return resp; } + private String extractTransactionUrlOrThrowException(Entry nextEntry, HTTPVerbEnum verb) { + String url = nextEntry.getRequest().getUrl(); + if (isBlank(url)) { + throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionMissingUrl", verb.name())); + } + return url; + } + + /** + * This method is called for nested bundles (e.g. if we received a transaction with an entry that + * was a GET search, this method is called on the bundle for the search result, that will be placed in the + * outer bundle). This method applies the _summary and _content parameters to the output of + * that bundle. + * + * TODO: This isn't the most efficient way of doing this.. hopefully we can come up with something better in the future. + */ + private IBaseResource filterNestedBundle(RequestDetails theRequestDetails, IBaseResource theResource) { + IParser p = getContext().newJsonParser(); + RestfulServerUtils.configureResponseParser(theRequestDetails, p); + return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); + } + + private IFhirResourceDao getDaoOrThrowException(Class theClass) { + IFhirResourceDao retVal = getDao(theClass); + if (retVal == null) { + throw new InvalidRequestException("Unable to process request, this server does not know how to handle resources of type " + getContext().getResourceDefinition(theClass).getName()); + } + return retVal; + } + @Override public MetaDt metaGetOperation() { // Notify interceptors @@ -180,6 +210,31 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { return retVal; } + private ca.uhn.fhir.jpa.dao.IFhirResourceDao toDao(UrlParts theParts, String theVerb, String theUrl) { + RuntimeResourceDefinition resType; + try { + resType = getContext().getResourceDefinition(theParts.getResourceType()); + } catch (DataFormatException e) { + String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + throw new InvalidRequestException(msg); + } + IFhirResourceDao dao = null; + if (resType != null) { + dao = getDao(resType.getImplementingClass()); + } + if (dao == null) { + String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + throw new InvalidRequestException(msg); + } + + // if (theParts.getResourceId() == null && theParts.getParams() == null) { + // String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); + // throw new InvalidRequestException(msg); + // } + + return dao; + } + @Transactional(propagation = Propagation.REQUIRED) @Override public Bundle transaction(RequestDetails theRequestDetails, Bundle theRequest) { @@ -215,26 +270,51 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { Map idSubstitutions = new HashMap(); Map idToPersistedOutcome = new HashMap(); + /* + * We want to execute the transaction request bundle elements in the order + * specified by the FHIR specification (see TransactionSorter) so we save the + * original order in the request, then sort it. + * + * Entries with a type of GET are removed from the bundle so that they + * can be processed at the very end. We do this because the incoming resources + * are saved in a two-phase way in order to deal with interdependencies, and + * we want the GET processing to use the final indexing state + */ Bundle response = new Bundle(); - - // TODO: process verbs in the correct order - + List getEntries = new ArrayList(); + IdentityHashMap originalRequestOrder = new IdentityHashMap(); + for (int i = 0; i < theRequest.getEntry().size(); i++) { + originalRequestOrder.put(theRequest.getEntry().get(i), i); + response.addEntry(); + if (theRequest.getEntry().get(i).getRequest().getMethodElement().getValueAsEnum() == HTTPVerbEnum.GET) { + getEntries.add(theRequest.getEntry().get(i)); + } + } + Collections.sort(theRequest.getEntry(), new TransactionSorter()); + + List deletedResources = new ArrayList(); + List deleteConflicts = new ArrayList(); + + /* + * Loop through the request and process any entries of type + * PUT, POST or DELETE + */ for (int i = 0; i < theRequest.getEntry().size(); i++) { if (i % 100 == 0) { - ourLog.info("Processed {} entries out of {}", i, theRequest.getEntry().size()); + ourLog.info("Processed {} non-GET entries out of {}", i, theRequest.getEntry().size()); } - Entry nextEntry = theRequest.getEntry().get(i); - IResource res = nextEntry.getResource(); + Entry nextReqEntry = theRequest.getEntry().get(i); + IResource res = nextReqEntry.getResource(); IdDt nextResourceId = null; if (res != null) { nextResourceId = res.getId(); if (nextResourceId.hasIdPart() == false) { - if (isNotBlank(nextEntry.getFullUrl())) { - nextResourceId = new IdDt(nextEntry.getFullUrl()); + if (isNotBlank(nextReqEntry.getFullUrl())) { + nextResourceId = new IdDt(nextReqEntry.getFullUrl()); } } @@ -263,12 +343,13 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { } - HTTPVerbEnum verb = nextEntry.getRequest().getMethodElement().getValueAsEnum(); + HTTPVerbEnum verb = nextReqEntry.getRequest().getMethodElement().getValueAsEnum(); if (verb == null) { - throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionEntryHasInvalidVerb", nextEntry.getRequest().getMethod())); + throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionEntryHasInvalidVerb", nextReqEntry.getRequest().getMethod())); } String resourceType = res != null ? getContext().getResourceDefinition(res).getName() : null; + Entry nextRespEntry = response.getEntry().get(originalRequestOrder.get(nextReqEntry)); switch (verb) { case POST: { @@ -277,24 +358,32 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); res.setId((String) null); DaoMethodOutcome outcome; - Entry newEntry = response.addEntry(); - outcome = resourceDao.create(res, nextEntry.getRequest().getIfNoneExist(), false); - handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, newEntry, resourceType, res); + outcome = resourceDao.create(res, nextReqEntry.getRequest().getIfNoneExist(), false); + handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res); break; } case DELETE: { // DELETE - Entry newEntry = response.addEntry(); - String url = extractTransactionUrlOrThrowException(nextEntry, verb); + String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); UrlParts parts = UrlUtil.parseUrl(url); ca.uhn.fhir.jpa.dao.IFhirResourceDao dao = toDao(parts, verb.getCode(), url); + int status = Constants.STATUS_HTTP_204_NO_CONTENT; if (parts.getResourceId() != null) { - dao.delete(new IdDt(parts.getResourceType(), parts.getResourceId())); + ResourceTable deleted = dao.delete(new IdDt(parts.getResourceType(), parts.getResourceId()), deleteConflicts); + if (deleted != null) { + deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless()); + } } else { - dao.deleteByUrl(parts.getResourceType() + '?' + parts.getParams(), true); + List allDeleted = dao.deleteByUrl(parts.getResourceType() + '?' + parts.getParams(), deleteConflicts); + for (ResourceTable deleted : allDeleted) { + deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless()); + } + if (allDeleted.isEmpty()) { + status = Constants.STATUS_HTTP_404_NOT_FOUND; + } } - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_204_NO_CONTENT)); + nextRespEntry.getResponse().setStatus(toStatusString(status)); break; } case PUT: { @@ -303,9 +392,8 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); DaoMethodOutcome outcome; - Entry newEntry = response.addEntry(); - String url = extractTransactionUrlOrThrowException(nextEntry, verb); + String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); UrlParts parts = UrlUtil.parseUrl(url); if (isNotBlank(parts.getResourceId())) { @@ -316,80 +404,32 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { outcome = resourceDao.update(res, parts.getResourceType() + '?' + parts.getParams(), false); } - handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, newEntry, resourceType, res); + handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res); break; } - case GET: { - // SEARCH/READ/VREAD - RequestDetails requestDetails = new RequestDetails(); - requestDetails.setServletRequest(theRequestDetails.getServletRequest()); - requestDetails.setRequestType(RequestTypeEnum.GET); - requestDetails.setServer(theRequestDetails.getServer()); - - String url = extractTransactionUrlOrThrowException(nextEntry, verb); - - int qIndex = url.indexOf('?'); - ArrayListMultimap paramValues = ArrayListMultimap.create(); - requestDetails.setParameters(new HashMap()); - if (qIndex != -1) { - String params = url.substring(qIndex); - List parameters = translateMatchUrl(params); - for (NameValuePair next : parameters) { - paramValues.put(next.getName(), next.getValue()); - } - for (java.util.Map.Entry> nextParamEntry : paramValues.asMap().entrySet()) { - String[] nextValue = nextParamEntry.getValue().toArray(new String[nextParamEntry.getValue().size()]); - requestDetails.getParameters().put(nextParamEntry.getKey(), nextValue); - } - url = url.substring(0, qIndex); - } - - requestDetails.setRequestPath(url); - requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase()); - - theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url); - BaseMethodBinding method = theRequestDetails.getServer().determineResourceMethod(requestDetails, url); - if (method == null) { - throw new IllegalArgumentException("Unable to handle GET " + url); - } - - if (isNotBlank(nextEntry.getRequest().getIfMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_MATCH, nextEntry.getRequest().getIfMatch()); - } - if (isNotBlank(nextEntry.getRequest().getIfNoneExist())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, nextEntry.getRequest().getIfNoneExist()); - } - if (isNotBlank(nextEntry.getRequest().getIfNoneMatch())) { - requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, nextEntry.getRequest().getIfNoneMatch()); - } - - if (method instanceof BaseResourceReturningMethodBinding) { - try { - ResourceOrDstu1Bundle responseData = ((BaseResourceReturningMethodBinding) method).invokeServer(theRequestDetails.getServer(), requestDetails, new byte[0]); - Entry newEntry = response.addEntry(); - IBaseResource resource = responseData.getResource(); - if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { - resource = filterNestedBundle(requestDetails, resource); - } - newEntry.setResource((IResource) resource); - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); - } catch (NotModifiedException e) { - Entry newEntry = response.addEntry(); - newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); - } - } else { - throw new IllegalArgumentException("Unable to handle GET " + url); - } - } } } - FhirTerser terser = getContext().newTerser(); + /* + * Make sure that there are no conflicts from deletions. E.g. we can't delete something + * if something else has a reference to it.. Unless the thing that has a reference to it + * was also deleted as a part of this transaction, which is why we check this now at the + * end. + */ + + for (Iterator iter = deleteConflicts.iterator(); iter.hasNext(); ) { + DeleteConflict next = iter.next(); + if (deletedResources.contains(next.getTargetId().toVersionless())) { + iter.remove(); + } + } + validateDeleteConflictsEmptyOrThrowException(deleteConflicts); /* * Perform ID substitutions and then index each resource we have saved */ + FhirTerser terser = getContext().newTerser(); for (DaoMethodOutcome nextOutcome : idToPersistedOutcome.values()) { IResource nextResource = (IResource) nextOutcome.getResource(); if (nextResource == null) { @@ -443,6 +483,74 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { ourLog.info("Placeholder resource ID \"{}\" was replaced with permanent ID \"{}\"", next, replacement); } + /* + * Loop through the request and process any entries of type GET + */ + for (int i = 0; i < getEntries.size(); i++) { + Entry nextReqEntry = getEntries.get(i); + Integer originalOrder = originalRequestOrder.get(nextReqEntry); + Entry nextRespEntry = response.getEntry().get(originalOrder); + + RequestDetails requestDetails = new RequestDetails(); + requestDetails.setServletRequest(theRequestDetails.getServletRequest()); + requestDetails.setRequestType(RequestTypeEnum.GET); + requestDetails.setServer(theRequestDetails.getServer()); + + String url = extractTransactionUrlOrThrowException(nextReqEntry, HTTPVerbEnum.GET); + + int qIndex = url.indexOf('?'); + ArrayListMultimap paramValues = ArrayListMultimap.create(); + requestDetails.setParameters(new HashMap()); + if (qIndex != -1) { + String params = url.substring(qIndex); + List parameters = translateMatchUrl(params); + for (NameValuePair next : parameters) { + paramValues.put(next.getName(), next.getValue()); + } + for (java.util.Map.Entry> nextParamEntry : paramValues.asMap().entrySet()) { + String[] nextValue = nextParamEntry.getValue().toArray(new String[nextParamEntry.getValue().size()]); + requestDetails.getParameters().put(nextParamEntry.getKey(), nextValue); + } + url = url.substring(0, qIndex); + } + + requestDetails.setRequestPath(""); + requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase()); + + theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url); + BaseMethodBinding method = theRequestDetails.getServer().determineResourceMethod(requestDetails, url); + if (method == null) { + throw new IllegalArgumentException("Unable to handle GET " + url); + } + + if (isNotBlank(nextReqEntry.getRequest().getIfMatch())) { + requestDetails.addHeader(Constants.HEADER_IF_MATCH, nextReqEntry.getRequest().getIfMatch()); + } + if (isNotBlank(nextReqEntry.getRequest().getIfNoneExist())) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_EXIST, nextReqEntry.getRequest().getIfNoneExist()); + } + if (isNotBlank(nextReqEntry.getRequest().getIfNoneMatch())) { + requestDetails.addHeader(Constants.HEADER_IF_NONE_MATCH, nextReqEntry.getRequest().getIfNoneMatch()); + } + + if (method instanceof BaseResourceReturningMethodBinding) { + try { + ResourceOrDstu1Bundle responseData = ((BaseResourceReturningMethodBinding) method).invokeServer(theRequestDetails.getServer(), requestDetails, new byte[0]); + IBaseResource resource = responseData.getResource(); + if (paramValues.containsKey(Constants.PARAM_SUMMARY) || paramValues.containsKey(Constants.PARAM_CONTENT)) { + resource = filterNestedBundle(requestDetails, resource); + } + nextRespEntry.setResource((IResource) resource); + nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); + } catch (NotModifiedException e) { + nextRespEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_304_NOT_MODIFIED)); + } + } else { + throw new IllegalArgumentException("Unable to handle GET " + url); + } + + } + long delay = System.currentTimeMillis() - start; ourLog.info(theActionName + " completed in {}ms", new Object[] { delay }); @@ -450,53 +558,6 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { return response; } - /** - * This method is called for nested bundles (e.g. if we received a transaction with an entry that - * was a GET search, this method is called on the bundle for the search result, that will be placed in the - * outer bundle). This method applies the _summary and _content parameters to the output of - * that bundle. - * - * TODO: This isn't the most efficient way of doing this.. hopefully we can come up with something better in the future. - */ - private IBaseResource filterNestedBundle(RequestDetails theRequestDetails, IBaseResource theResource) { - IParser p = getContext().newJsonParser(); - RestfulServerUtils.configureResponseParser(theRequestDetails, p); - return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); - } - - private ca.uhn.fhir.jpa.dao.IFhirResourceDao toDao(UrlParts theParts, String theVerb, String theUrl) { - RuntimeResourceDefinition resType; - try { - resType = getContext().getResourceDefinition(theParts.getResourceType()); - } catch (DataFormatException e) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - IFhirResourceDao dao = null; - if (resType != null) { - dao = getDao(resType.getImplementingClass()); - } - if (dao == null) { - String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - throw new InvalidRequestException(msg); - } - - // if (theParts.getResourceId() == null && theParts.getParams() == null) { - // String msg = getContext().getLocalizer().getMessage(BaseHapiFhirSystemDao.class, "transactionInvalidUrl", theVerb, theUrl); - // throw new InvalidRequestException(msg); - // } - - return dao; - } - - private IFhirResourceDao getDaoOrThrowException(Class theClass) { - IFhirResourceDao retVal = getDao(theClass); - if (retVal == null) { - throw new InvalidRequestException("Unable to process request, this server does not know how to handle resources of type " + getContext().getResourceDefinition(theClass).getName()); - } - return retVal; - } - private static void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IdDt nextResourceId, DaoMethodOutcome outcome, Entry newEntry, String theResourceType, IResource theRes) { IdDt newId = (IdDt) outcome.getId().toUnqualifiedVersionless(); @@ -532,4 +593,47 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { return Integer.toString(theStatusCode) + " " + defaultString(Constants.HTTP_STATUS_NAMES.get(theStatusCode)); } + //@formatter:off + /** + * Transaction Order, per the spec: + * + * Process any DELETE interactions + * Process any POST interactions + * Process any PUT interactions + * Process any GET interactions + */ + //@formatter:off + public class TransactionSorter implements Comparator { + + @Override + public int compare(Entry theO1, Entry theO2) { + int o1 = toOrder(theO1); + int o2 = toOrder(theO2); + + return o1 - o2; + } + + private int toOrder(Entry theO1) { + int o1 = 0; + if (theO1.getRequest().getMethodElement().getValueAsEnum() != null) { + switch (theO1.getRequest().getMethodElement().getValueAsEnum()) { + case DELETE: + o1 = 1; + break; + case POST: + o1 = 2; + break; + case PUT: + o1 = 3; + break; + case GET: + o1 = 4; + break; + } + } + return o1; + } + + } + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java index 859b302b6b2..d25e52701f1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFhirResourceDao.java @@ -23,6 +23,7 @@ import java.util.Collection; */ import java.util.Date; +import java.util.List; import java.util.Map; import java.util.Set; @@ -32,6 +33,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import ca.uhn.fhir.jpa.entity.BaseHasResource; import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TagTypeEnum; +import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.model.api.TagList; import ca.uhn.fhir.model.dstu2.composite.MetaDt; @@ -57,14 +59,28 @@ public interface IFhirResourceDao extends IDao { */ DaoMethodOutcome create(T theResource, String theIfNoneExist, boolean thePerformIndexing); + /** + * This method throws an exception if there are delete conflicts + */ DaoMethodOutcome delete(IIdType theResource); + /** + * This method does not throw an exception if there are delete conflicts, but populates them + * in the provided list + */ + ResourceTable delete(IIdType theResource, List theDeleteConflictsListToPopulate); + + /** + * This method throws an exception if there are delete conflicts + */ DaoMethodOutcome deleteByUrl(String theString); /** - * @param theTransaction Is this being called in a bundle? If so, don't throw an exception if no matches + * This method does not throw an exception if there are delete conflicts, but populates them + * in the provided list + * @return */ - DaoMethodOutcome deleteByUrl(String theUrl, boolean theTransaction); + List deleteByUrl(String theUrl, List theDeleteConflictsListToPopulate); TagList getAllResourceTags(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/DeleteConflict.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/DeleteConflict.java new file mode 100644 index 00000000000..70d0ed28b54 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/DeleteConflict.java @@ -0,0 +1,29 @@ +package ca.uhn.fhir.jpa.util; + +import ca.uhn.fhir.model.primitive.IdDt; + +public class DeleteConflict { + + private final IdDt mySourceId; + private final String mySourcePath; + private final IdDt myTargetId; + + public DeleteConflict(IdDt theSourceId, String theSourcePath, IdDt theTargetId) { + mySourceId = theSourceId; + mySourcePath = theSourcePath; + myTargetId = theTargetId; + } + + public IdDt getSourceId() { + return mySourceId; + } + + public String getSourcePath() { + return mySourcePath; + } + + public IdDt getTargetId() { + return myTargetId; + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2Test.java index a5ce9b94a87..9201ff1d3d4 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2Test.java @@ -685,8 +685,7 @@ public class FhirResourceDaoDstu2Test extends BaseJpaDstu2Test { myOrganizationDao.delete(orgId); fail(); } catch (ResourceVersionConflictException e) { - assertThat(e.getMessage(), containsString("Unable to delete Organization/" + orgId.getIdPart() - + " because at least one resource has a reference to this resource. First reference found was resource Patient/" + patId.getIdPart() + " in path Patient.managingOrganization")); + assertThat(e.getMessage(), containsString("Delete failed because of constraint")); } myPatientDao.delete(patId); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2ValidateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2ValidateTest.java index 61f1511fc8c..de51c711444 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2ValidateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoDstu2ValidateTest.java @@ -215,7 +215,7 @@ public class FhirResourceDaoDstu2ValidateTest extends BaseJpaDstu2Test { String ooString = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome); ourLog.info(ooString); - assertThat(ooString, containsString("Unable to delete "+orgId.getValue()+" because at least one resource has a reference to this resource. First reference found was resource " + patId.getValue() + " in path Patient.managingOrganization")); + assertThat(ooString, containsString("Unable to delete Organization")); pat.setId(patId); pat.getManagingOrganization().setReference(""); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2Test.java index e0b78bbf2ca..edd722c6e50 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2Test.java @@ -141,7 +141,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertEquals(Long.valueOf(2), entity.getIndexStatus()); } - + @Test public void testSystemMetaOperation() { @@ -589,6 +589,124 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { } + /** + * See #253 Test that the order of deletes is version independent + */ + @Test + public void testTransactionDeleteIsOrderIndependantTargetFirst() { + String methodName = "testTransactionDeleteIsOrderIndependantTargetFirst"; + + Patient p1 = new Patient(); + p1.addIdentifier().setSystem("urn:system").setValue(methodName); + IIdType pid = myPatientDao.create(p1).getId().toUnqualifiedVersionless(); + ourLog.info("Created patient, got it: {}", pid); + + Observation o1 = new Observation(); + o1.getSubject().setReference(pid); + IIdType oid1 = myObservationDao.create(o1).getId().toUnqualifiedVersionless(); + + Observation o2 = new Observation(); + o2.addIdentifier().setValue(methodName); + o2.getSubject().setReference(pid); + IIdType oid2 = myObservationDao.create(o2).getId().toUnqualifiedVersionless(); + + myPatientDao.read(pid); + myObservationDao.read(oid1); + + // The target is Patient, so try with it first in the bundle + Bundle request = new Bundle(); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl(pid.getValue()); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl(oid1.getValue()); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Observation?identifier=" + methodName); + Bundle resp = mySystemDao.transaction(myRequestDetails, request); + + assertEquals(3, resp.getEntry().size()); + assertEquals("204 No Content", resp.getEntry().get(0).getResponse().getStatus()); + assertEquals("204 No Content", resp.getEntry().get(1).getResponse().getStatus()); + assertEquals("204 No Content", resp.getEntry().get(2).getResponse().getStatus()); + + try { + myPatientDao.read(pid); + fail(); + } catch (ResourceGoneException e) { + // good + } + + try { + myObservationDao.read(oid1); + fail(); + } catch (ResourceGoneException e) { + // good + } + + try { + myObservationDao.read(oid2); + fail(); + } catch (ResourceGoneException e) { + // good + } + + } + + /** + * See #253 Test that the order of deletes is version independent + */ + @Test + public void testTransactionDeleteIsOrderIndependantTargetLast() { + String methodName = "testTransactionDeleteIsOrderIndependantTargetFirst"; + + Patient p1 = new Patient(); + p1.addIdentifier().setSystem("urn:system").setValue(methodName); + IIdType pid = myPatientDao.create(p1).getId().toUnqualifiedVersionless(); + ourLog.info("Created patient, got it: {}", pid); + + Observation o1 = new Observation(); + o1.getSubject().setReference(pid); + IIdType oid1 = myObservationDao.create(o1).getId().toUnqualifiedVersionless(); + + Observation o2 = new Observation(); + o2.addIdentifier().setValue(methodName); + o2.getSubject().setReference(pid); + IIdType oid2 = myObservationDao.create(o2).getId().toUnqualifiedVersionless(); + + myPatientDao.read(pid); + myObservationDao.read(oid1); + + // The target is Patient, so try with it last in the bundle + Bundle request = new Bundle(); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl(oid1.getValue()); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Observation?identifier=" + methodName); + request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl(pid.getValue()); + Bundle resp = mySystemDao.transaction(myRequestDetails, request); + + assertEquals(3, resp.getEntry().size()); + assertEquals("204 No Content", resp.getEntry().get(0).getResponse().getStatus()); + assertEquals("204 No Content", resp.getEntry().get(1).getResponse().getStatus()); + assertEquals("204 No Content", resp.getEntry().get(2).getResponse().getStatus()); + + try { + myPatientDao.read(pid); + fail(); + } catch (ResourceGoneException e) { + // good + } + + try { + myObservationDao.read(oid1); + fail(); + } catch (ResourceGoneException e) { + // good + } + + try { + myObservationDao.read(oid2); + fail(); + } catch (ResourceGoneException e) { + // good + } + + } + @Test public void testTransactionDeleteMatchUrlWithOneMatch() { String methodName = "testTransactionDeleteMatchUrlWithOneMatch"; @@ -670,7 +788,10 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { request.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Patient?identifier=urn%3Asystem%7C" + methodName); // try { - mySystemDao.transaction(myRequestDetails, request); + Bundle resp = mySystemDao.transaction(myRequestDetails, request); + assertEquals(1, resp.getEntry().size()); + assertEquals("404 Not Found", resp.getEntry().get(0).getResponse().getStatus()); + // fail(); // } catch (ResourceNotFoundException e) { // assertThat(e.getMessage(), containsString("resource matching URL \"Patient?")); @@ -762,6 +883,88 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus()); } + @Test + public void testTransactionOrdering() { + String methodName = "testTransactionOrdering"; + + //@formatter:off + /* + * Transaction Order, per the spec: + * + * Process any DELETE interactions + * Process any POST interactions + * Process any PUT interactions + * Process any GET interactions + * + * This test creates a transaction bundle that includes + * these four operations in the reverse order and verifies + * that they are invoked correctly. + */ + //@formatter:off + + int pass = 0; + IdDt patientPlaceholderId = IdDt.newRandomUuid(); + + Bundle req = testTransactionOrderingCreateBundle(methodName, pass, patientPlaceholderId); + Bundle resp = mySystemDao.transaction(myRequestDetails, req); + testTransactionOrderingValidateResponse(pass, resp); + + pass = 1; + patientPlaceholderId = IdDt.newRandomUuid(); + + req = testTransactionOrderingCreateBundle(methodName, pass, patientPlaceholderId); + resp = mySystemDao.transaction(myRequestDetails, req); + testTransactionOrderingValidateResponse(pass, resp); + + } + + private void testTransactionOrderingValidateResponse(int pass, Bundle resp) { + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); + assertEquals(4, resp.getEntry().size()); + assertEquals("200 OK", resp.getEntry().get(0).getResponse().getStatus()); + if (pass == 0) { + assertEquals("201 Created", resp.getEntry().get(1).getResponse().getStatus()); + assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Observation/")); + assertThat(resp.getEntry().get(1).getResponse().getLocation(), endsWith("_history/1")); + } else { + assertEquals("200 OK", resp.getEntry().get(1).getResponse().getStatus()); + assertThat(resp.getEntry().get(1).getResponse().getLocation(), startsWith("Observation/")); + assertThat(resp.getEntry().get(1).getResponse().getLocation(), endsWith("_history/2")); + } + assertEquals("201 Created", resp.getEntry().get(2).getResponse().getStatus()); + assertThat(resp.getEntry().get(2).getResponse().getLocation(), startsWith("Patient/")); + if (pass == 0) { + assertEquals("404 Not Found", resp.getEntry().get(3).getResponse().getStatus()); + } else { + assertEquals("204 No Content", resp.getEntry().get(3).getResponse().getStatus()); + } + + + Bundle respGetBundle = (Bundle) resp.getEntry().get(0).getResource(); + assertEquals(1, respGetBundle.getEntry().size()); + assertEquals("testTransactionOrdering" + pass, ((Patient)respGetBundle.getEntry().get(0).getResource()).getNameFirstRep().getFamilyFirstRep().getValue()); + assertEquals("/Patient?identifier=testTransactionOrdering", respGetBundle.getLink("self").getUrl()); + } + + private Bundle testTransactionOrderingCreateBundle(String methodName, int pass, IdDt patientPlaceholderId) { + Bundle req = new Bundle(); + req.addEntry().getRequest().setMethod(HTTPVerbEnum.GET).setUrl("Patient?identifier=" + methodName); + + Observation obs = new Observation(); + obs.getSubject().setReference(patientPlaceholderId); + obs.addIdentifier().setValue(methodName); + obs.getCode().setText(methodName + pass); + req.addEntry().setResource(obs).getRequest().setMethod(HTTPVerbEnum.PUT).setUrl("Observation?identifier=" + methodName); + + Patient pat = new Patient(); + pat.addIdentifier().setValue(methodName); + pat.addName().addFamily(methodName + pass); + req.addEntry().setResource(pat).setFullUrl(patientPlaceholderId.getValue()).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Patient"); + + req.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Patient?identifier=" + methodName); + return req; + } + @Test public void testTransactionReadAndSearch() { String methodName = "testTransactionReadAndSearch"; @@ -829,6 +1032,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { details = detailsCapt.getValue(); assertEquals("Patient", details.getResourceType()); + } @Test diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0300084206d..452705ecbbb 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -240,6 +240,12 @@ correct, but the intention is clear so we will honour them just to be helpful. + + In the JPA server, order of transaction processing should be + DELETE, POST, PUT, GET, and the order should not matter + within entries with the same verb. Thanks to Bill de Beaubien + for reporting! +