Issue 5004 make bundleutil also accept name prev for previous links (#5042)

* Add test

* Fix

* Changelog

* Small details to reset build pipe

* Reformat

* Make definition animal-sniffer (check-android-26-compliance) compliant

---------

Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>
This commit is contained in:
jmarchionatto 2023-06-28 15:46:54 -04:00 committed by GitHub
parent ce04e897cf
commit ed04dd2498
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 251 additions and 101 deletions

View File

@ -27,18 +27,22 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.rest.api.PatchTypeEnum;
import ca.uhn.fhir.rest.api.RequestTypeEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.util.bundle.BundleEntryMutator;
import ca.uhn.fhir.util.bundle.BundleEntryParts;
import ca.uhn.fhir.util.bundle.EntryListAccumulator;
import ca.uhn.fhir.util.bundle.ModifiableBundleEntry;
import ca.uhn.fhir.util.bundle.SearchBundleEntryParts;
import com.google.common.collect.Sets;
import org.apache.commons.lang3.tuple.Pair;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBinary;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.HashMap;
@ -51,29 +55,41 @@ import java.util.function.Consumer;
import java.util.stream.Collectors;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.hl7.fhir.instance.model.api.IBaseBundle.LINK_PREV;
/**
* Fetch resources from a bundle
*/
public class BundleUtil {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BundleUtil.class);
private static final Logger ourLog = LoggerFactory.getLogger(BundleUtil.class);
private static final String PREVIOUS = LINK_PREV;
private static final String PREV = "prev";
private static final Set<String> previousOrPrev = Sets.newHashSet(PREVIOUS, PREV);
public static final String DIFFERENT_LINK_ERROR_MSG = "Mismatching 'previous' and 'prev' links exist. 'previous' " +
"is: '$PREVIOUS' and 'prev' is: '$PREV'.";
/**
* @return Returns <code>null</code> if the link isn't found or has no value
*/
public static String getLinkUrlOfType(FhirContext theContext, IBaseBundle theBundle, String theLinkRelation) {
return getLinkUrlOfType(theContext, theBundle, theLinkRelation, true);
}
private static String getLinkUrlOfType(FhirContext theContext, IBaseBundle theBundle, String theLinkRelation, boolean isPreviousCheck) {
RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
BaseRuntimeChildDefinition entryChild = def.getChildByName("link");
List<IBase> links = entryChild.getAccessor().getValues(theBundle);
for (IBase nextLink : links) {
boolean isRightRel = false;
BaseRuntimeElementCompositeDefinition relDef = (BaseRuntimeElementCompositeDefinition) theContext.getElementDefinition(nextLink.getClass());
BaseRuntimeElementCompositeDefinition<?> relDef = (BaseRuntimeElementCompositeDefinition<?>) theContext.getElementDefinition(nextLink.getClass());
BaseRuntimeChildDefinition relChild = relDef.getChildByName("relation");
List<IBase> relValues = relChild.getAccessor().getValues(nextLink);
for (IBase next : relValues) {
IPrimitiveType<?> nextValue = (IPrimitiveType<?>) next;
if (theLinkRelation.equals(nextValue.getValueAsString())) {
if (isRelationMatch(theContext, theBundle,theLinkRelation, nextValue.getValueAsString(), isPreviousCheck)) {
isRightRel = true;
}
}
@ -82,7 +98,7 @@ public class BundleUtil {
continue;
}
BaseRuntimeElementCompositeDefinition linkDef = (BaseRuntimeElementCompositeDefinition) theContext.getElementDefinition(nextLink.getClass());
BaseRuntimeElementCompositeDefinition<?> linkDef = (BaseRuntimeElementCompositeDefinition<?>) theContext.getElementDefinition(nextLink.getClass());
BaseRuntimeChildDefinition urlChild = linkDef.getChildByName("url");
List<IBase> values = urlChild.getAccessor().getValues(nextLink);
for (IBase nextUrl : values) {
@ -91,12 +107,40 @@ public class BundleUtil {
return nextValue.getValueAsString();
}
}
}
return null;
}
private static boolean isRelationMatch(FhirContext theContext, IBaseBundle theBundle, String value, String matching, boolean theIsPreviousCheck) {
if ( ! theIsPreviousCheck) {
return value.equals(matching);
}
if ( previousOrPrev.contains(value) ) {
validateUniqueOrMatchingPreviousValues(theContext, theBundle);
if ( previousOrPrev.contains(matching) ) {
return true;
}
}
return (value.equals(matching));
}
private static void validateUniqueOrMatchingPreviousValues(FhirContext theContext, IBaseBundle theBundle) {
String previousLink = getLinkNoCheck(theContext, theBundle, PREVIOUS);
String prevLink = getLinkNoCheck(theContext, theBundle, PREV);
if (prevLink != null && previousLink != null) {
if ( ! previousLink.equals(prevLink)) {
String msg = DIFFERENT_LINK_ERROR_MSG.replace("$PREVIOUS", previousLink).replace("$PREV", prevLink);
throw new InternalErrorException(Msg.code(2368) + msg);
}
}
}
private static String getLinkNoCheck(FhirContext theContext, IBaseBundle theBundle, String theLinkRelation) {
return getLinkUrlOfType(theContext, theBundle, theLinkRelation, false);
}
@SuppressWarnings("unchecked")
public static List<Pair<String, IBaseResource>> getBundleEntryUrlsAndResources(FhirContext theContext, IBaseBundle theBundle) {
RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
@ -160,6 +204,7 @@ public class BundleUtil {
BaseRuntimeChildDefinition entryChild = def.getChildByName("total");
List<IBase> entries = entryChild.getAccessor().getValues(theBundle);
if (entries.size() > 0) {
@SuppressWarnings("unchecked")
IPrimitiveType<Number> typeElement = (IPrimitiveType<Number>) entries.get(0);
if (typeElement != null && typeElement.getValue() != null) {
return typeElement.getValue().intValue();
@ -171,6 +216,7 @@ public class BundleUtil {
public static void setTotal(FhirContext theContext, IBaseBundle theBundle, Integer theTotal) {
RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
BaseRuntimeChildDefinition entryChild = def.getChildByName("total");
@SuppressWarnings("unchecked")
IPrimitiveType<Integer> value = (IPrimitiveType<Integer>) entryChild.getChildByName("total").newInstance();
value.setValue(theTotal);
entryChild.getMutator().setValue(theBundle, value);
@ -194,11 +240,11 @@ public class BundleUtil {
* 1. Deletes
* 2. Creates
* 3. Updates
*
* <p>
* Furthermore, within these operation types, the entries will be sorted based on the order in which they should be processed
* e.g. if you have 2 CREATEs, one for a Patient, and one for an Observation which has this Patient as its Subject,
* the patient will come first, then the observation.
*
* <p>
* In cases of there being a cyclic dependency (e.g. Organization/1 is partOf Organization/2 and Organization/2 is partOf Organization/1)
* this function will throw an IllegalStateException.
*
@ -207,12 +253,11 @@ public class BundleUtil {
*/
public static void sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle) throws IllegalStateException {
Map<BundleEntryParts, IBase> partsToIBaseMap = getPartsToIBaseMap(theContext, theBundle);
LinkedHashSet<IBase> retVal = new LinkedHashSet<>();
//Get all deletions.
LinkedHashSet<IBase> deleteParts = sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.DELETE, partsToIBaseMap);
validatePartsNotNull(deleteParts);
retVal.addAll(deleteParts);
LinkedHashSet<IBase> retVal = new LinkedHashSet<>(deleteParts);
//Get all Creations
LinkedHashSet<IBase> createParts= sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.POST, partsToIBaseMap);
@ -408,8 +453,7 @@ public class BundleUtil {
}
}
SearchBundleEntryParts parts = new SearchBundleEntryParts(fullUrl, resource, matchMode);
return parts;
return new SearchBundleEntryParts(fullUrl, resource, matchMode);
}
/**
@ -485,8 +529,7 @@ public class BundleUtil {
}
}
}
BundleEntryParts parts = new BundleEntryParts(fullUrl, requestType, url, resource, conditionalUrl);
return parts;
return new BundleEntryParts(fullUrl, requestType, url, resource, conditionalUrl);
}
/**

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 5004
title: "Previously when requesting `previous` or `prev` link using `BundleUtil.getLinkUrlOfType`, only the literal requested
link was returned. Now when requesting either `previous` or `prev` `previous` or `prev` link is returned.
If both exist, equality is validated and exception is thrown on failure."

View File

@ -1,11 +1,14 @@
package ca.uhn.fhir.util.bundle;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.model.valueset.BundleEntrySearchModeEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.util.BundleBuilder;
import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.TestUtil;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.ExplanationOfBenefit;
import org.hl7.fhir.r4.model.Medication;
@ -16,12 +19,14 @@ import org.hl7.fhir.r4.model.Quantity;
import org.hl7.fhir.r4.model.Reference;
import org.hl7.fhir.r4.model.UriType;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import java.util.Collections;
import java.util.List;
import java.util.function.Consumer;
import static ca.uhn.fhir.util.BundleUtil.DIFFERENT_LINK_ERROR_MSG;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
@ -33,15 +38,22 @@ import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.GET;
import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.POST;
import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
public class BundleUtilTest {
private static FhirContext ourCtx = FhirContext.forR4();
private static final FhirContext ourCtx = FhirContext.forR4Cached();
@Nested
public class TestGetLinkUrlOfType {
@Test
public void testGetLink() {
public void testGetNextExists() {
Bundle b = new Bundle();
b.getLinkOrCreate("prev").setUrl("http://bar");
b.getLinkOrCreate("next").setUrl("http://foo");
@ -49,23 +61,112 @@ public class BundleUtilTest {
}
@Test
public void testGetLinkDoesntExist() {
public void testGetNextDoesntExist() {
Bundle b = new Bundle();
b.getLinkOrCreate("prev").setUrl("http://bar");
assertEquals(null, BundleUtil.getLinkUrlOfType(ourCtx, b, "next"));
assertNull(BundleUtil.getLinkUrlOfType(ourCtx, b, "next"));
}
@Nested
public class TestGetPreviousLink {
@Test
public void testGetPrevious_exists_and_no_prev() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, IBaseBundle.LINK_PREV));
}
@Test
public void testGetPrevious_exists_and_prev_also_and_equals() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
b.getLinkOrCreate("prev").setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, IBaseBundle.LINK_PREV));
}
@Test
public void testGetPrevious_exists_and_prev_also_and_different_must_throw() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
b.getLinkOrCreate("prev").setUrl("http://foo");
InternalErrorException thrown = assertThrows(InternalErrorException.class,
() -> BundleUtil.getLinkUrlOfType(ourCtx, b, IBaseBundle.LINK_PREV));
String expectedExceptionMsg = Msg.code(2368) + DIFFERENT_LINK_ERROR_MSG
.replace("$PREVIOUS", "http://bar").replace("$PREV", "http://foo");
assertEquals(expectedExceptionMsg, thrown.getMessage());
}
@Test
public void testGetPrevious_doesnt_exist_neither_prev() {
Bundle b = new Bundle();
assertNull(BundleUtil.getLinkUrlOfType(ourCtx, b, IBaseBundle.LINK_PREV));
}
@Test
public void testGetPrevious_doesnt_exist_but_prev_does() {
Bundle b = new Bundle();
b.getLinkOrCreate("prev").setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, IBaseBundle.LINK_PREV));
}
@Test
public void testGetPrev_exists_and_no_previous() {
Bundle b = new Bundle();
b.getLinkOrCreate("prev").setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, "prev"));
}
@Test
public void testGetPrev_exists_and_previous_also_and_equals() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
b.getLinkOrCreate("prev").setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, "prev"));
}
@Test
public void testGetPrev_exists_and_previous_also_and_different_must_throw() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
b.getLinkOrCreate("prev").setUrl("http://foo");
InternalErrorException thrown = assertThrows(InternalErrorException.class,
() -> BundleUtil.getLinkUrlOfType(ourCtx, b, "prev"));
String expectedExceptionMsg = Msg.code(2368) + DIFFERENT_LINK_ERROR_MSG
.replace("$PREVIOUS", "http://bar").replace("$PREV", "http://foo");
assertEquals(expectedExceptionMsg, thrown.getMessage());
}
@Test
public void testGetPrev_doesnt_exist_neither_previous() {
Bundle b = new Bundle();
assertNull(BundleUtil.getLinkUrlOfType(ourCtx, b, "prev"));
}
@Test
public void testGetPrev_doesnt_exist_but_previous_does() {
Bundle b = new Bundle();
b.getLinkOrCreate(IBaseBundle.LINK_PREV).setUrl("http://bar");
assertEquals("http://bar", BundleUtil.getLinkUrlOfType(ourCtx, b, "prev"));
}
}
}
@Test
public void testGetTotal() {
Bundle b = new Bundle();
b.setTotal(999);
assertEquals(999, BundleUtil.getTotal(ourCtx, b).intValue());
Integer total = BundleUtil.getTotal(ourCtx, b);
assertNotNull(total);
assertEquals(999, total.intValue());
}
@Test
public void testGetTotalNull() {
Bundle b = new Bundle();
assertEquals(null, BundleUtil.getTotal(ourCtx, b));
assertNull(BundleUtil.getTotal(ourCtx, b));
}
@Test
@ -148,7 +249,6 @@ public class BundleUtilTest {
assertThat(b.getEntry(), hasSize(4));
List<Bundle.BundleEntryComponent> entry = b.getEntry();
int observationIndex = getIndexOfEntryWithId("Observation/O1", b);
int patientIndex = getIndexOfEntryWithId("Patient/P1", b);
int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b);
@ -182,7 +282,7 @@ public class BundleUtilTest {
try {
BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, b);
fail();
} catch (IllegalStateException e ) {
} catch (IllegalStateException ignored) {
}
}
@ -274,71 +374,72 @@ public class BundleUtilTest {
@Test
public void testBundleToSearchBundleEntryParts() {
//Given
String bundleString = "{\n" +
" \"resourceType\": \"Bundle\",\n" +
" \"id\": \"bd194b7f-ac1e-429a-a206-ee2c470f23b5\",\n" +
" \"meta\": {\n" +
" \"lastUpdated\": \"2021-10-18T16:25:55.330-07:00\"\n" +
" },\n" +
" \"type\": \"searchset\",\n" +
" \"total\": 1,\n" +
" \"link\": [\n" +
" {\n" +
" \"relation\": \"self\",\n" +
" \"url\": \"http://localhost:8000/Patient?_count=1&_id=pata&_revinclude=Condition%3Asubject%3APatient\"\n" +
" }\n" +
" ],\n" +
" \"entry\": [\n" +
" {\n" +
" \"fullUrl\": \"http://localhost:8000/Patient/pata\",\n" +
" \"resource\": {\n" +
" \"resourceType\": \"Patient\",\n" +
" \"id\": \"pata\",\n" +
" \"meta\": {\n" +
" \"versionId\": \"1\",\n" +
" \"lastUpdated\": \"2021-10-18T16:25:48.954-07:00\",\n" +
" \"source\": \"#rnEjIucr8LR6Ze3x\"\n" +
" },\n" +
" \"name\": [\n" +
" {\n" +
" \"family\": \"Simpson\",\n" +
" \"given\": [\n" +
" \"Homer\",\n" +
" \"J\"\n" +
" ]\n" +
" }\n" +
" ]\n" +
" },\n" +
" \"search\": {\n" +
" \"mode\": \"match\"\n" +
" }\n" +
" },\n" +
" {\n" +
" \"fullUrl\": \"http://localhost:8000/Condition/1626\",\n" +
" \"resource\": {\n" +
" \"resourceType\": \"Condition\",\n" +
" \"id\": \"1626\",\n" +
" \"meta\": {\n" +
" \"versionId\": \"1\",\n" +
" \"lastUpdated\": \"2021-10-18T16:25:51.672-07:00\",\n" +
" \"source\": \"#gSOcGAdA3acaaNq1\"\n" +
" },\n" +
" \"identifier\": [\n" +
" {\n" +
" \"system\": \"urn:hssc:musc:conditionid\",\n" +
" \"value\": \"1064115000.1.5\"\n" +
" }\n" +
" ],\n" +
" \"subject\": {\n" +
" \"reference\": \"Patient/pata\"\n" +
" }\n" +
" },\n" +
" \"search\": {\n" +
" \"mode\": \"include\"\n" +
" }\n" +
" }\n" +
" ]\n" +
"}";
String bundleString = """
{
"resourceType": "Bundle",
"id": "bd194b7f-ac1e-429a-a206-ee2c470f23b5",
"meta": {
"lastUpdated": "2021-10-18T16:25:55.330-07:00"
},
"type": "searchset",
"total": 1,
"link": [
{
"relation": "self",
"url": "http://localhost:8000/Patient?_count=1&_id=pata&_revinclude=Condition%3Asubject%3APatient"
}
],
"entry": [
{
"fullUrl": "http://localhost:8000/Patient/pata",
"resource": {
"resourceType": "Patient",
"id": "pata",
"meta": {
"versionId": "1",
"lastUpdated": "2021-10-18T16:25:48.954-07:00",
"source": "#rnEjIucr8LR6Ze3x"
},
"name": [
{
"family": "Simpson",
"given": [
"Homer",
"J"
]
}
]
},
"search": {
"mode": "match"
}
},
{
"fullUrl": "http://localhost:8000/Condition/1626",
"resource": {
"resourceType": "Condition",
"id": "1626",
"meta": {
"versionId": "1",
"lastUpdated": "2021-10-18T16:25:51.672-07:00",
"source": "#gSOcGAdA3acaaNq1"
},
"identifier": [
{
"system": "urn:hssc:musc:conditionid",
"value": "1064115000.1.5"
}
],
"subject": {
"reference": "Patient/pata"
}
},
"search": {
"mode": "include"
}
}
]
}""";
Bundle bundle = ourCtx.newJsonParser().parseResource(Bundle.class, bundleString);