Fix for `userSelected` and `version`. (#5310)

Fix `userSelected` and `version` in tags and security labels to DSTU2 and security labels in JPA storage.
This commit is contained in:
michaelabuckley 2023-09-13 20:27:37 -04:00 committed by GitHub
parent 0c42229985
commit e20382c4c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 248 additions and 51 deletions

View File

@ -24,6 +24,7 @@ import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.model.api.BaseIdentifiableElement;
import ca.uhn.fhir.model.api.ICompositeDatatype;
import ca.uhn.fhir.model.api.IQueryParameterType;
import ca.uhn.fhir.model.primitive.BooleanDt;
import ca.uhn.fhir.model.primitive.CodeDt;
import ca.uhn.fhir.model.primitive.StringDt;
import ca.uhn.fhir.model.primitive.UriDt;
@ -58,6 +59,10 @@ public abstract class BaseCodingDt extends BaseIdentifiableElement implements IC
*/
public abstract UriDt getSystemElement();
public abstract StringDt getVersionElement();
public abstract BooleanDt getUserSelectedElement();
/**
* Gets the value(s) for <b>display</b> (Representation defined by the system).
* creating it if it does
@ -72,13 +77,6 @@ public abstract class BaseCodingDt extends BaseIdentifiableElement implements IC
public abstract BaseCodingDt setDisplay(String theString);
/*
todo: handle version
public abstract StringDt getVersion();
public abstract BaseCodingDt setVersion ( String theString);
*/
/**
* {@inheritDoc}
*/
@ -181,7 +179,7 @@ public abstract class BaseCodingDt extends BaseIdentifiableElement implements IC
* @deprecated get/setMissing is not supported in StringDt. Use {@link TokenParam} instead if you
* need this functionality
*/
@Deprecated
@Deprecated(since = "6.0.0")
@Override
public Boolean getMissing() {
return null;
@ -193,7 +191,7 @@ public abstract class BaseCodingDt extends BaseIdentifiableElement implements IC
* @deprecated get/setMissing is not supported in StringDt. Use {@link TokenParam} instead if you
* need this functionality
*/
@Deprecated
@Deprecated(since = "6.0.0")
@Override
public IQueryParameterType setMissing(Boolean theMissing) {
throw new UnsupportedOperationException(

View File

@ -1033,7 +1033,8 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
writeOptionalTagWithTextNode(theEventWriter, "system", tag.getScheme());
writeOptionalTagWithTextNode(theEventWriter, "code", tag.getTerm());
writeOptionalTagWithTextNode(theEventWriter, "display", tag.getLabel());
// wipmb should we be writing the new properties here? There must be another path.
writeOptionalTagWithTextNode(theEventWriter, "version", tag.getVersion());
write(theEventWriter, "userSelected", tag.getUserSelectedBoolean());
theEventWriter.endObject();
}
theEventWriter.endArray();

View File

@ -636,7 +636,7 @@ class ParserState<T> {
BaseRuntimeElementDefinition<?> target = child.getChildByName(theChildName);
if (target == null) {
// This is a bug with the structures and shouldn't happen..
// This is a bug with the structures and shouldn't happen.
throw new DataFormatException(
Msg.code(1809) + "Found unexpected element '" + theChildName + "' in parent element '"
+ myDefinition.getName() + "'. Valid names are: " + child.getValidChildNames());
@ -1584,16 +1584,20 @@ class ParserState<T> {
private class TagState extends BaseState {
private static final int LABEL = 2;
private static final int NONE = 0;
private static final int TERM = 1;
private static final int LABEL = 2;
private static final int SCHEME = 3;
private static final int TERM = 1;
private static final int VERSION = 4;
private static final int USER_SELECTED = 5;
private String myLabel;
private String myScheme;
private int mySubState = 0;
private TagList myTagList;
private String myTerm;
private String myVersion;
private Boolean myUserSelected;
public TagState(TagList theTagList) {
super(null);
@ -1614,6 +1618,12 @@ class ParserState<T> {
case SCHEME:
myScheme = (value);
break;
case VERSION:
myVersion = (value);
break;
case USER_SELECTED:
myUserSelected = Boolean.valueOf(value);
break;
case NONE:
// This handles JSON encoding, which is a bit weird
enteringNewElement(null, theName);
@ -1629,7 +1639,9 @@ class ParserState<T> {
mySubState = NONE;
} else {
if (isNotEmpty(myScheme) || isNotBlank(myTerm) || isNotBlank(myLabel)) {
myTagList.addTag(myScheme, myTerm, myLabel);
Tag tag = myTagList.addTag(myScheme, myTerm, myLabel);
tag.setUserSelectedBoolean(myUserSelected);
tag.setVersion(myVersion);
}
pop();
}
@ -1646,6 +1658,10 @@ class ParserState<T> {
mySubState = SCHEME;
} else if (Tag.ATTR_LABEL.equals(theLocalPart) || "display".equals(theLocalPart)) {
mySubState = LABEL;
} else if ("userSelected".equals(theLocalPart)) {
mySubState = USER_SELECTED;
} else if ("version".equals(theLocalPart)) {
mySubState = VERSION;
} else {
throw new DataFormatException(Msg.code(1818) + "Unexpected element: " + theLocalPart);
}

View File

@ -187,7 +187,8 @@ public class XmlParser extends BaseParser {
String namespaceURI = elem.getName().getNamespaceURI();
if ("extension".equals(elem.getName().getLocalPart())) {
String localPart = elem.getName().getLocalPart();
if ("extension".equals(localPart)) {
Attribute urlAttr = elem.getAttributeByName(new QName("url"));
String url;
if (urlAttr == null || isBlank(urlAttr.getValue())) {
@ -199,7 +200,7 @@ public class XmlParser extends BaseParser {
url = urlAttr.getValue();
}
parserState.enteringNewElementExtension(elem, url, false, getServerBaseUrl());
} else if ("modifierExtension".equals(elem.getName().getLocalPart())) {
} else if ("modifierExtension".equals(localPart)) {
Attribute urlAttr = elem.getAttributeByName(new QName("url"));
String url;
if (urlAttr == null || isBlank(urlAttr.getValue())) {
@ -213,8 +214,7 @@ public class XmlParser extends BaseParser {
}
parserState.enteringNewElementExtension(elem, url, true, getServerBaseUrl());
} else {
String elementName = elem.getName().getLocalPart();
parserState.enteringNewElement(namespaceURI, elementName);
parserState.enteringNewElement(namespaceURI, localPart);
}
if (!heldComments.isEmpty()) {
@ -768,6 +768,11 @@ public class XmlParser extends BaseParser {
writeOptionalTagWithValue(theEventWriter, "system", tag.getScheme());
writeOptionalTagWithValue(theEventWriter, "code", tag.getTerm());
writeOptionalTagWithValue(theEventWriter, "display", tag.getLabel());
writeOptionalTagWithValue(theEventWriter, "version", tag.getVersion());
Boolean userSelected = tag.getUserSelectedBoolean();
if (userSelected != null) {
writeOptionalTagWithValue(theEventWriter, "userSelected", userSelected.toString());
}
theEventWriter.writeEndElement();
}
}

View File

@ -151,13 +151,27 @@ public class InternalCodingDt extends BaseCodingDt implements ICompositeDatatype
* is consistent across versions. However this cannot consistently be assured. and When the meaning is not guaranteed to be consistent, the version SHOULD be exchanged
* </p>
*/
public StringDt getVersion() {
@Override
public StringDt getVersionElement() {
if (myVersion == null) {
myVersion = new StringDt();
}
return myVersion;
}
@Override
public BooleanDt getUserSelectedElement() {
return new BooleanDt();
}
/**
* Legacy name for {@link #getVersionElement()}
*/
@Deprecated(since = "7.0.0")
public StringDt getVersion() {
return getVersionElement();
}
/**
* Sets the value(s) for <b>version</b> (Version of the system - if relevant)
*

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 5310
title: "Update DSTU2 tags and security labels with support for `userSelected` and `version` elements.
Also fix them on security labels in JPA storage."

View File

@ -32,7 +32,6 @@ import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.interceptor.model.RequestPartitionId;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IDao;
import ca.uhn.fhir.jpa.api.dao.IJpaDao;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
@ -137,6 +136,7 @@ import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.transaction.support.TransactionTemplate;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@ -227,9 +227,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
@Autowired
protected IInterceptorBroadcaster myInterceptorBroadcaster;
@Autowired
protected DaoRegistry myDaoRegistry;
@Autowired
protected InMemoryResourceMatcher myInMemoryResourceMatcher;
@ -336,8 +333,8 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
next.getSystemElement().getValue(),
next.getCodeElement().getValue(),
next.getDisplayElement().getValue(),
null,
null);
next.getVersionElement().getValue(),
next.getUserSelectedElement().getValue());
if (def != null) {
ResourceTag tag = theEntity.addTag(def);
allDefs.add(tag);
@ -566,11 +563,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
}
});
} catch (Exception ex) {
// transaction template can fail if connections to db are exhausted
// and/or timeout
ourLog.warn("Transaction failed with: "
+ ex.getMessage() + ". "
+ "Transaction will rollback and be reattempted.");
// transaction template can fail if connections to db are exhausted and/or timeout
ourLog.warn(
"Transaction failed with: {}. Transaction will rollback and be reattempted.", ex.getMessage());
retVal = null;
}
count++;
@ -700,7 +695,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
}
String hashSha256 = hashCode.toString();
if (hashSha256.equals(theEntity.getHashSha256()) == false) {
if (!hashSha256.equals(theEntity.getHashSha256())) {
changed = true;
}
theEntity.setHashSha256(hashSha256);
@ -784,7 +779,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
byte[] resourceBinary;
switch (encoding) {
case JSON:
resourceBinary = encodedResource.getBytes(Charsets.UTF_8);
resourceBinary = encodedResource.getBytes(StandardCharsets.UTF_8);
break;
case JSONC:
resourceBinary = GZipUtil.compress(encodedResource);
@ -1592,7 +1587,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
if (!thePerformIndexing
&& !savedEntity.isUnchangedInCurrentOperation()
&& !ourDisableIncrementOnUpdateForUnitTest) {
if (theResourceId.hasVersionIdPart() == false) {
if (!theResourceId.hasVersionIdPart()) {
theResourceId = theResourceId.withVersion(Long.toString(savedEntity.getVersion()));
}
incrementId(theResource, savedEntity, theResourceId);
@ -1673,11 +1668,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
protected void addPidToResource(IResourceLookup<JpaPid> theEntity, IBaseResource theResource) {
if (theResource instanceof IAnyResource) {
IDao.RESOURCE_PID.put(
(IAnyResource) theResource, theEntity.getPersistentId().getId());
IDao.RESOURCE_PID.put(theResource, theEntity.getPersistentId().getId());
} else if (theResource instanceof IResource) {
IDao.RESOURCE_PID.put(
(IResource) theResource, theEntity.getPersistentId().getId());
IDao.RESOURCE_PID.put(theResource, theEntity.getPersistentId().getId());
}
}

View File

@ -401,11 +401,14 @@ public class JpaStorageResourceParser implements IJpaStorageResourceParser {
secLabel.setSystem(nextTag.getSystem());
secLabel.setCode(nextTag.getCode());
secLabel.setDisplay(nextTag.getDisplay());
// wipmb these technically support userSelected and version
secLabel.setVersion(nextTag.getVersion());
Boolean userSelected = nextTag.getUserSelected();
if (userSelected != null) {
secLabel.setUserSelected(userSelected);
}
securityLabels.add(secLabel);
break;
case TAG:
// wipmb check xml, etc.
Tag e = new Tag(nextTag.getSystem(), nextTag.getCode(), nextTag.getDisplay());
e.setVersion(nextTag.getVersion());
// careful! These are Boolean, not boolean.

View File

@ -0,0 +1,70 @@
package ca.uhn.fhir.jpa.dao.dstu2;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
import ca.uhn.fhir.model.dstu2.resource.Patient;
import org.hl7.fhir.instance.model.api.IBaseCoding;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class FhirResourceDaoDstu2ParseTest extends BaseJpaDstu2Test {
@Autowired
DaoRegistry myDaoRegistry;
@Test
void testTagRoundTrip() {
// given
Patient resource = new Patient();
IBaseCoding tag = resource.getMeta().addTag();
tag.setCode("code");
tag.setDisplay("display");
tag.setSystem("oid:123");
tag.setVersion("v1");
tag.setUserSelected(true);
// when
DaoMethodOutcome daoMethodOutcome = myPatientDao.create(resource, mySrd);
Patient resourceOut = myPatientDao.read(daoMethodOutcome.getId(), mySrd);
// then
List<? extends IBaseCoding> tags = resourceOut.getMeta().getTag();
assertEquals(1, tags.size(), "tag is present");
IBaseCoding tagOut = tags.get(0);
assertEquals("code", tagOut.getCode());
assertEquals("display", tagOut.getDisplay());
assertEquals("oid:123", tagOut.getSystem());
assertEquals("v1", tagOut.getVersion());
assertEquals(true, tagOut.getUserSelected());
}
@Test
void testSecurityRoundTrip() {
// given
Patient resource = new Patient();
IBaseCoding coding = resource.getMeta().addSecurity();
coding.setCode("code");
coding.setDisplay("display");
coding.setSystem("oid:123");
coding.setVersion("v1");
coding.setUserSelected(true);
// when
DaoMethodOutcome daoMethodOutcome = myPatientDao.create(resource, mySrd);
Patient resourceOut = myPatientDao.read(daoMethodOutcome.getId(), mySrd);
// then
List<? extends IBaseCoding> tags = resourceOut.getMeta().getSecurity();
assertEquals(1, tags.size(), "coding is present");
IBaseCoding codingOut = tags.get(0);
assertEquals("code", codingOut.getCode());
assertEquals("display", codingOut.getDisplay());
assertEquals("oid:123", codingOut.getSystem());
assertEquals("v1", codingOut.getVersion());
assertEquals(true, codingOut.getUserSelected());
}
}

View File

@ -22,6 +22,7 @@ package ca.uhn.fhir.storage.test;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.test.utilities.ITestDataBuilder;
import com.google.common.collect.HashMultimap;
@ -47,10 +48,10 @@ public class DaoTestDataBuilder implements ITestDataBuilder.WithSupport, ITestDa
final FhirContext myFhirCtx;
final DaoRegistry myDaoRegistry;
SystemRequestDetails mySrd;
RequestDetails mySrd;
final SetMultimap<String, IIdType> myIds = HashMultimap.create();
public DaoTestDataBuilder(FhirContext theFhirCtx, DaoRegistry theDaoRegistry, SystemRequestDetails theSrd) {
public DaoTestDataBuilder(FhirContext theFhirCtx, DaoRegistry theDaoRegistry, RequestDetails theSrd) {
myFhirCtx = theFhirCtx;
myDaoRegistry = theDaoRegistry;
mySrd = theSrd;

View File

@ -134,7 +134,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
public IBaseCoding addSecurity() {
List<BaseCodingDt> tagList = ResourceMetadataKeyEnum.SECURITY_LABELS.get(BaseResource.this);
if (tagList == null) {
tagList = new ArrayList<BaseCodingDt>();
tagList = new ArrayList<>();
ResourceMetadataKeyEnum.SECURITY_LABELS.put(BaseResource.this, tagList);
}
CodingDt tag = new CodingDt();
@ -185,7 +185,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
@Override
public List<? extends IPrimitiveType<String>> getProfile() {
ArrayList<IPrimitiveType<String>> retVal = new ArrayList<IPrimitiveType<String>>();
ArrayList<IPrimitiveType<String>> retVal = new ArrayList<>();
List<IdDt> profilesList = ResourceMetadataKeyEnum.PROFILES.get(BaseResource.this);
if (profilesList == null) {
return Collections.emptyList();
@ -198,16 +198,19 @@ public abstract class BaseResource extends BaseElement implements IResource {
@Override
public List<? extends IBaseCoding> getSecurity() {
ArrayList<CodingDt> retVal = new ArrayList<CodingDt>();
ArrayList<CodingDt> retVal = new ArrayList<>();
List<BaseCodingDt> labelsList = ResourceMetadataKeyEnum.SECURITY_LABELS.get(BaseResource.this);
if (labelsList == null) {
return Collections.emptyList();
}
for (BaseCodingDt next : labelsList) {
retVal.add(new CodingDt(
CodingDt c = new CodingDt(
next.getSystemElement().getValue(),
next.getCodeElement().getValue())
.setDisplay(next.getDisplayElement().getValue()));
next.getCodeElement().getValue());
c.setDisplay(next.getDisplayElement().getValue());
c.setUserSelected(next.getUserSelectedElement());
c.setVersion(next.getVersionElement());
retVal.add(c);
}
return retVal;
}
@ -224,7 +227,7 @@ public abstract class BaseResource extends BaseElement implements IResource {
@Override
public List<? extends IBaseCoding> getTag() {
ArrayList<IBaseCoding> retVal = new ArrayList<IBaseCoding>();
ArrayList<IBaseCoding> retVal = new ArrayList<>();
TagList tagList = ResourceMetadataKeyEnum.TAG_LIST.get(BaseResource.this);
if (tagList == null) {
return Collections.emptyList();

View File

@ -0,0 +1,84 @@
package ca.uhn.fhir.model.dstu2;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.dstu2.resource.Bundle;
import ca.uhn.fhir.model.dstu2.resource.Patient;
import ca.uhn.fhir.parser.IParser;
import org.hl7.fhir.instance.model.api.IBaseCoding;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class ModelParseTest {
static final Logger ourLog = LoggerFactory.getLogger(ModelParseTest.class);
static FhirContext ourFhirContext = FhirContext.forDstu2Cached();
@ParameterizedTest
@MethodSource("getParsers")
void testTagRoundTrip(IParser theParser) {
// given
Patient resource = new Patient();
IBaseCoding tag = resource.getMeta().addTag();
tag.setCode("code");
tag.setDisplay("display");
tag.setSystem("oid:123");
tag.setVersion("v1");
tag.setUserSelected(true);
// when
String string = theParser.encodeResourceToString(resource);
ourLog.info("encoded: {}", string);
Patient bundleOut = theParser.parseResource(Patient.class, string);
// then
List<? extends IBaseCoding> tags = bundleOut.getMeta().getTag();
assertEquals(1, tags.size(), "tag is present");
IBaseCoding tagOut = tags.get(0);
assertEquals("code", tagOut.getCode());
assertEquals("display", tagOut.getDisplay());
assertEquals("oid:123", tagOut.getSystem());
assertEquals("v1", tagOut.getVersion());
assertEquals(true, tagOut.getUserSelected());
}
@ParameterizedTest
@MethodSource("getParsers")
void testSecurityRoundTrip(IParser theParser) {
// given
Patient resource = new Patient();
IBaseCoding coding = resource.getMeta().addSecurity();
coding.setCode("code");
coding.setDisplay("display");
coding.setSystem("oid:123");
coding.setVersion("v1");
coding.setUserSelected(true);
// when
String string = theParser.encodeResourceToString(resource);
ourLog.info("encoded: {}", string);
Patient bundleOut = theParser.parseResource(Patient.class, string);
// then
List<? extends IBaseCoding> labels = bundleOut.getMeta().getSecurity();
assertEquals(1, labels.size(), "security is present");
IBaseCoding codingOut = labels.get(0);
assertEquals("code", codingOut.getCode());
assertEquals("display", codingOut.getDisplay());
assertEquals("oid:123", codingOut.getSystem());
assertEquals("v1", codingOut.getVersion());
assertEquals(true, codingOut.getUserSelected());
}
public static List<IParser> getParsers() {
return List.of(
ourFhirContext.newJsonParser(),
// ourFhirContext.newRDFParser(), dstu2 doesn't support RDF
ourFhirContext.newXmlParser()
);
}
}

View File

@ -27,6 +27,7 @@ import ca.uhn.fhir.util.FhirTerser;
import ca.uhn.fhir.util.MetaUtil;
import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseCoding;
import org.hl7.fhir.instance.model.api.IBaseReference;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.ICompositeType;
@ -182,8 +183,11 @@ public interface ITestDataBuilder {
return t -> ((IBaseResource)t).setId(theId.toUnqualifiedVersionless());
}
default ICreationArgument withTag(String theSystem, String theCode) {
return t -> ((IBaseResource)t).getMeta().addTag().setSystem(theSystem).setCode(theCode);
default ICreationArgument withTag(String theSystem, String theCode, Consumer<IBaseCoding>... theModifiers) {
return t -> {
IBaseCoding coding = ((IBaseResource) t).getMeta().addTag().setSystem(theSystem).setCode(theCode);
applyElementModifiers(coding, theModifiers);
};
}
default ICreationArgument withSecurity(String theSystem, String theCode) {