diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java
index 456b0edbd0f..b0173ecc487 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Tag.java
@@ -19,13 +19,14 @@
*/
package ca.uhn.fhir.model.api;
-import java.net.URI;
-
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.hl7.fhir.instance.model.api.IBaseCoding;
+import java.net.URI;
+import java.util.Objects;
+
/**
* A single tag
*
@@ -58,7 +59,7 @@ public class Tag extends BaseElement implements IElement, IBaseCoding {
private String myScheme;
private String myTerm;
private String myVersion;
- private boolean myUserSelected;
+ private Boolean myUserSelected;
public Tag() {
}
@@ -114,37 +115,22 @@ public class Tag extends BaseElement implements IElement, IBaseCoding {
if (getClass() != obj.getClass())
return false;
Tag other = (Tag) obj;
- if (myScheme == null) {
- if (other.myScheme != null)
- return false;
- } else if (!myScheme.equals(other.myScheme))
- return false;
- if (myTerm == null) {
- if (other.myTerm != null)
- return false;
- } else if (!myTerm.equals(other.myTerm))
- return false;
- if (myVersion == null) {
- if (other.getVersion() != null)
- return false;
- } else if (!myVersion.equals(other.getVersion()))
- return false;
-
- if (myUserSelected != other.getUserSelected())
- return false;
-
- return true;
+ return
+ Objects.equals(myScheme, other.myScheme) &&
+ Objects.equals(myTerm, other.myTerm) &&
+ Objects.equals(myVersion, other.myVersion) &&
+ Objects.equals(myUserSelected, other.myUserSelected);
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
- result = prime * result + ((myScheme == null) ? 0 : myScheme.hashCode());
- result = prime * result + ((myTerm == null) ? 0 : myTerm.hashCode());
- result = prime * result + ((myVersion == null) ? 0 : myVersion.hashCode());
- result = prime * result + Boolean.hashCode(myUserSelected);
+ result = prime * result + Objects.hashCode(myScheme);
+ result = prime * result + Objects.hashCode(myTerm);
+ result = prime * result + Objects.hashCode(myVersion);
+ result = prime * result + Objects.hashCode(myUserSelected);
return result;
}
@@ -234,7 +220,9 @@ public class Tag extends BaseElement implements IElement, IBaseCoding {
}
@Override
- public boolean getUserSelected() { return myUserSelected; }
+ public boolean getUserSelected() { return myUserSelected != null && myUserSelected; }
+
+ public Boolean getUserSelectedBoolean() { return myUserSelected; }
@Override
public IBaseCoding setUserSelected(boolean theUserSelected) {
@@ -242,4 +230,8 @@ public class Tag extends BaseElement implements IElement, IBaseCoding {
return this;
}
+ public void setUserSelectedBoolean(Boolean theUserSelected) {
+ myUserSelected = theUserSelected;
+ }
+
}
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
index ae56cee0b73..43d29557289 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
@@ -724,73 +724,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
}
if (theResource instanceof IResource) {
- IResource resource = (IResource) theResource;
- // Object securityLabelRawObj =
-
- List securityLabels = extractMetadataListNotNull(resource, ResourceMetadataKeyEnum.SECURITY_LABELS);
- List extends IIdType> profiles = extractMetadataListNotNull(resource, ResourceMetadataKeyEnum.PROFILES);
- profiles = super.getProfileTagsForEncoding(resource, profiles);
-
- TagList tags = getMetaTagsForEncoding(resource, theEncodeContext);
- InstantDt updated = (InstantDt) resource.getResourceMetadata().get(ResourceMetadataKeyEnum.UPDATED);
- IdDt resourceId = resource.getId();
- String versionIdPart = resourceId.getVersionIdPart();
- if (isBlank(versionIdPart)) {
- versionIdPart = ResourceMetadataKeyEnum.VERSION.get(resource);
- }
- List, Object>> extensionMetadataKeys = getExtensionMetadataKeys(resource);
-
- if (super.shouldEncodeResourceMeta(resource) && (ElementUtil.isEmpty(versionIdPart, updated, securityLabels, tags, profiles) == false) || !extensionMetadataKeys.isEmpty()) {
- beginObject(theEventWriter, "meta");
-
- if (shouldEncodePath(resource, "meta.versionId")) {
- writeOptionalTagWithTextNode(theEventWriter, "versionId", versionIdPart);
- }
- if (shouldEncodePath(resource, "meta.lastUpdated")) {
- writeOptionalTagWithTextNode(theEventWriter, "lastUpdated", updated);
- }
-
- if (profiles != null && profiles.isEmpty() == false) {
- beginArray(theEventWriter, "profile");
- for (IIdType profile : profiles) {
- if (profile != null && isNotBlank(profile.getValue())) {
- theEventWriter.write(profile.getValue());
- }
- }
- theEventWriter.endArray();
- }
-
- if (securityLabels.isEmpty() == false) {
- beginArray(theEventWriter, "security");
- for (BaseCodingDt securityLabel : securityLabels) {
- theEventWriter.beginObject();
- theEncodeContext.pushPath("security", false);
- encodeCompositeElementChildrenToStreamWriter(resDef, resource, securityLabel, theEventWriter, theContainedResource, null, theEncodeContext);
- theEncodeContext.popPath();
- theEventWriter.endObject();
- }
- theEventWriter.endArray();
- }
-
- if (tags != null && tags.isEmpty() == false) {
- beginArray(theEventWriter, "tag");
- for (Tag tag : tags) {
- if (tag.isEmpty()) {
- continue;
- }
- theEventWriter.beginObject();
- writeOptionalTagWithTextNode(theEventWriter, "system", tag.getScheme());
- writeOptionalTagWithTextNode(theEventWriter, "code", tag.getTerm());
- writeOptionalTagWithTextNode(theEventWriter, "display", tag.getLabel());
- theEventWriter.endObject();
- }
- theEventWriter.endArray();
- }
-
- addExtensionMetadata(theResDef, theResource, theContainedResource, extensionMetadataKeys, resDef, theEventWriter, theEncodeContext);
-
- theEventWriter.endObject(); // end meta
- }
+ parseMetaForDSTU2(theResDef, theResource, theEventWriter, theContainedResource, theEncodeContext, resDef);
}
encodeCompositeElementToStreamWriter(theResDef, theResource, theResource, theEventWriter, theContainedResource, new CompositeChildElement(resDef, theEncodeContext), theEncodeContext);
@@ -798,6 +732,77 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
theEventWriter.endObject();
}
+ private void parseMetaForDSTU2(RuntimeResourceDefinition theResDef, IBaseResource theResource, BaseJsonLikeWriter theEventWriter, boolean theContainedResource, EncodeContext theEncodeContext, RuntimeResourceDefinition resDef) throws IOException {
+ IResource resource = (IResource) theResource;
+ // Object securityLabelRawObj =
+
+ List securityLabels = extractMetadataListNotNull(resource, ResourceMetadataKeyEnum.SECURITY_LABELS);
+ List extends IIdType> profiles = extractMetadataListNotNull(resource, ResourceMetadataKeyEnum.PROFILES);
+ profiles = super.getProfileTagsForEncoding(resource, profiles);
+
+ TagList tags = getMetaTagsForEncoding(resource, theEncodeContext);
+ InstantDt updated = (InstantDt) resource.getResourceMetadata().get(ResourceMetadataKeyEnum.UPDATED);
+ IdDt resourceId = resource.getId();
+ String versionIdPart = resourceId.getVersionIdPart();
+ if (isBlank(versionIdPart)) {
+ versionIdPart = ResourceMetadataKeyEnum.VERSION.get(resource);
+ }
+ List, Object>> extensionMetadataKeys = getExtensionMetadataKeys(resource);
+
+ if (super.shouldEncodeResourceMeta(resource) && (ElementUtil.isEmpty(versionIdPart, updated, securityLabels, tags, profiles) == false) || !extensionMetadataKeys.isEmpty()) {
+ beginObject(theEventWriter, "meta");
+
+ if (shouldEncodePath(resource, "meta.versionId")) {
+ writeOptionalTagWithTextNode(theEventWriter, "versionId", versionIdPart);
+ }
+ if (shouldEncodePath(resource, "meta.lastUpdated")) {
+ writeOptionalTagWithTextNode(theEventWriter, "lastUpdated", updated);
+ }
+
+ if (profiles != null && profiles.isEmpty() == false) {
+ beginArray(theEventWriter, "profile");
+ for (IIdType profile : profiles) {
+ if (profile != null && isNotBlank(profile.getValue())) {
+ theEventWriter.write(profile.getValue());
+ }
+ }
+ theEventWriter.endArray();
+ }
+
+ if (securityLabels.isEmpty() == false) {
+ beginArray(theEventWriter, "security");
+ for (BaseCodingDt securityLabel : securityLabels) {
+ theEventWriter.beginObject();
+ theEncodeContext.pushPath("security", false);
+ encodeCompositeElementChildrenToStreamWriter(resDef, resource, securityLabel, theEventWriter, theContainedResource, null, theEncodeContext);
+ theEncodeContext.popPath();
+ theEventWriter.endObject();
+ }
+ theEventWriter.endArray();
+ }
+
+ if (tags != null && tags.isEmpty() == false) {
+ beginArray(theEventWriter, "tag");
+ for (Tag tag : tags) {
+ if (tag.isEmpty()) {
+ continue;
+ }
+ theEventWriter.beginObject();
+ 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.
+ theEventWriter.endObject();
+ }
+ theEventWriter.endArray();
+ }
+
+ addExtensionMetadata(theResDef, theResource, theContainedResource, extensionMetadataKeys, resDef, theEventWriter, theEncodeContext);
+
+ theEventWriter.endObject(); // end meta
+ }
+ }
+
private void addExtensionMetadata(RuntimeResourceDefinition theResDef, IBaseResource theResource,
boolean theContainedResource,
diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagTest.java
index 02230007192..ab747cf2923 100644
--- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagTest.java
+++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/model/api/TagTest.java
@@ -27,7 +27,7 @@ public class TagTest {
@Test
public void testHashCode() {
Tag tag1 = new Tag().setScheme("scheme").setTerm("term").setLabel("label");
- assertEquals(-1029266947, tag1.hashCode());
+ assertEquals(-1029268184, tag1.hashCode());
}
@Test
diff --git a/hapi-fhir-converter/src/main/java/ca/uhn/hapi/converters/canonical/VersionCanonicalizer.java b/hapi-fhir-converter/src/main/java/ca/uhn/hapi/converters/canonical/VersionCanonicalizer.java
index f6922cb196b..2aff63449b0 100644
--- a/hapi-fhir-converter/src/main/java/ca/uhn/hapi/converters/canonical/VersionCanonicalizer.java
+++ b/hapi-fhir-converter/src/main/java/ca/uhn/hapi/converters/canonical/VersionCanonicalizer.java
@@ -364,7 +364,9 @@ public class VersionCanonicalizer {
retVal.setSystem(coding.getSystem());
retVal.setDisplay(coding.getDisplay());
retVal.setVersion(coding.getVersion());
- retVal.setUserSelected(!coding.getUserSelectedElement().isEmpty() && coding.getUserSelected());
+ if (!coding.getUserSelectedElement().isEmpty()) {
+ retVal.setUserSelected( coding.getUserSelected() );
+ }
return retVal;
}
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4819-tag-userSelected-null.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4819-tag-userSelected-null.yaml
new file mode 100644
index 00000000000..bf5bfd26918
--- /dev/null
+++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4819-tag-userSelected-null.yaml
@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 4819
+title: "Tags no longer provide a default `false` value for `userSelected`."
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4813-fix-tag-nonunique.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4813-fix-tag-nonunique.yaml
new file mode 100644
index 00000000000..c7aa497b7d6
--- /dev/null
+++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4813-fix-tag-nonunique.yaml
@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 4813
+title: "Under heavy concurrency, a bug resulted in identical tag definitions being rejected with a `NonUniqueResultException` some of the time. This has been corrected."
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 4c5c0af9930..938b297a2e7 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
@@ -247,6 +247,8 @@ public abstract class BaseHapiFhirDao extends BaseStora
@Autowired
private PlatformTransactionManager myTransactionManager;
+ protected final CodingSpy myCodingSpy = new CodingSpy();
+
@VisibleForTesting
public void setExternallyStoredResourceServiceRegistryForUnitTest(ExternallyStoredResourceServiceRegistry theExternallyStoredResourceServiceRegistry) {
myExternallyStoredResourceServiceRegistry = theExternallyStoredResourceServiceRegistry;
@@ -283,7 +285,7 @@ public abstract class BaseHapiFhirDao extends BaseStora
if (tagList != null) {
for (Tag next : tagList) {
TagDefinition def = getTagOrNull(theTransactionDetails, TagTypeEnum.TAG, next.getScheme(), next.getTerm(),
- next.getLabel(), next.getVersion(), next.getUserSelected());
+ next.getLabel(), next.getVersion(), myCodingSpy.getBooleanObject(next));
if (def != null) {
ResourceTag tag = theEntity.addTag(def);
allDefs.add(tag);
@@ -323,7 +325,7 @@ public abstract class BaseHapiFhirDao extends BaseStora
if (tagList != null) {
for (IBaseCoding next : tagList) {
TagDefinition def = getTagOrNull(theTransactionDetails, TagTypeEnum.TAG, next.getSystem(), next.getCode(),
- next.getDisplay(), next.getVersion(), next.getUserSelected());
+ next.getDisplay(), next.getVersion(), myCodingSpy.getBooleanObject(next));
if (def != null) {
ResourceTag tag = theEntity.addTag(def);
theAllTags.add(tag);
@@ -335,7 +337,7 @@ public abstract class BaseHapiFhirDao extends BaseStora
List extends IBaseCoding> securityLabels = theResource.getMeta().getSecurity();
if (securityLabels != null) {
for (IBaseCoding next : securityLabels) {
- TagDefinition def = getTagOrNull(theTransactionDetails, TagTypeEnum.SECURITY_LABEL, next.getSystem(), next.getCode(), next.getDisplay(), null, null);
+ TagDefinition def = getTagOrNull(theTransactionDetails, TagTypeEnum.SECURITY_LABEL, next.getSystem(), next.getCode(), next.getDisplay(), next.getVersion(), myCodingSpy.getBooleanObject(next));
if (def != null) {
ResourceTag tag = theEntity.addTag(def);
theAllTags.add(tag);
@@ -394,7 +396,6 @@ public abstract class BaseHapiFhirDao extends BaseStora
MemoryCacheService.TagDefinitionCacheKey key = toTagDefinitionMemoryCacheKey(theTagType, theScheme, theTerm, theVersion, theUserSelected);
TagDefinition retVal = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.TAG_DEFINITION, key);
-
if (retVal == null) {
HashMap resolvedTagDefinitions = theTransactionDetails
.getOrCreateUserData(HapiTransactionService.XACT_USERDATA_KEY_RESOLVED_TAG_DEFINITIONS, HashMap::new);
@@ -425,6 +426,7 @@ public abstract class BaseHapiFhirDao extends BaseStora
String theVersion, Boolean theUserSelected) {
TypedQuery q = buildTagQuery(theTagType, theScheme, theTerm, theVersion, theUserSelected);
+ q.setMaxResults(1);
TransactionTemplate template = new TransactionTemplate(myTransactionManager);
template.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
@@ -525,9 +527,9 @@ public abstract class BaseHapiFhirDao extends BaseStora
? builder.isNull(from.get("myVersion"))
: builder.equal(from.get("myVersion"), theVersion));
- predicates.add( isNull(theUserSelected) || isFalse(theUserSelected)
- ? builder.isFalse(from.get("myUserSelected"))
- : builder.isTrue(from.get("myUserSelected")));
+ predicates.add( isNull(theUserSelected)
+ ? builder.isNull(from.get("myUserSelected"))
+ : builder.equal(from.get("myUserSelected"), theUserSelected));
cq.where(predicates.toArray(new Predicate[0]));
return myEntityManager.createQuery(cq);
@@ -817,12 +819,14 @@ public abstract class BaseHapiFhirDao extends BaseStora
// Update the resource to contain the old tags
allTagsOld.forEach(tag -> {
- theResource.getMeta()
+ IBaseCoding iBaseCoding = theResource.getMeta()
.addTag()
.setCode(tag.getTag().getCode())
.setSystem(tag.getTag().getSystem())
- .setVersion(tag.getTag().getVersion())
- .setUserSelected(tag.getTag().getUserSelected());
+ .setVersion(tag.getTag().getVersion());
+ if (tag.getTag().getUserSelected() != null) {
+ iBaseCoding.setUserSelected(tag.getTag().getUserSelected());
+ }
});
theEntity.setHasTags(!allTagsNew.isEmpty());
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 2f990493b33..d48ba0719fe 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
@@ -834,6 +834,7 @@ public abstract class BaseHapiFhirResourceDao extends B
}
}
+
if (!hasTag) {
theEntity.setHasTags(true);
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/CodingSpy.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/CodingSpy.java
new file mode 100644
index 00000000000..d9dfede8480
--- /dev/null
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/CodingSpy.java
@@ -0,0 +1,61 @@
+package ca.uhn.fhir.jpa.dao;
+
+import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.model.primitive.BooleanDt;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.hl7.fhir.instance.model.api.IBaseBooleanDatatype;
+import org.hl7.fhir.instance.model.api.IBaseCoding;
+
+import java.lang.reflect.Field;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * We are trying to preserve null behaviour despite IBaseCoding using primitive boolean for userSelected.
+ */
+public class CodingSpy {
+ final Map mySpies = new HashMap<>();
+
+ /**
+ * Reach into the Coding and pull out the Boolean instead of the boolean.
+ * @param theValue the Coding, or CodingDt
+ * @return the Boolean
+ */
+ public Boolean getBooleanObject(IBaseCoding theValue) {
+ Field spy = getSpy(theValue);
+ try {
+ Object o = spy.get(theValue);
+ if (o == null) {
+ return null;
+ }
+ if (o instanceof BooleanDt) {
+ BooleanDt booleanDt = (BooleanDt) o;
+ return booleanDt.getValue();
+ }
+ if (o instanceof IBaseBooleanDatatype) {
+ IBaseBooleanDatatype booleanValue = (IBaseBooleanDatatype) o;
+ return booleanValue.getValue();
+ }
+ if (o instanceof Boolean) {
+ return (Boolean) o;
+ }
+ throw new RuntimeException(Msg.code(2342) + "unsupported type :" + theValue.getClass().getName());
+ } catch (IllegalAccessException theException) {
+ // should never happen - all Coding models have this field.
+ throw new RuntimeException(Msg.code(2343) + "illegal access during reflection", theException);
+ }
+ }
+
+ private Field getSpy(IBaseCoding theValue) {
+ return mySpies.computeIfAbsent(theValue.getClass(), k -> getFieldHandle(k));
+ }
+
+ private static Field getFieldHandle(Class k) {
+ Field result = FieldUtils.getField(k, "userSelected", true);
+ if (result == null) {
+ result = FieldUtils.getField(k, "myUserSelected", true);
+ }
+ return result;
+ }
+
+}
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
index 71185b1c9a0..776a9b6bc4b 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
@@ -39,6 +39,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.model.entity.ResourceTag;
+import ca.uhn.fhir.jpa.model.entity.TagDefinition;
import ca.uhn.fhir.jpa.model.entity.TagTypeEnum;
import ca.uhn.fhir.jpa.partition.IPartitionLookupSvc;
import ca.uhn.fhir.model.api.IResource;
@@ -72,7 +73,7 @@ import java.util.Date;
import java.util.List;
import static ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.decodeResource;
-import static org.apache.commons.lang3.StringUtils.defaultString;
+import static java.util.Objects.nonNull;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
public class JpaStorageResourceParser implements IJpaStorageResourceParser {
@@ -350,19 +351,26 @@ public class JpaStorageResourceParser implements IJpaStorageResourceParser {
List securityLabels = new ArrayList<>();
List profiles = new ArrayList<>();
for (BaseTag next : theTagList) {
- switch (next.getTag().getTagType()) {
+ TagDefinition nextTag = next.getTag();
+ switch (nextTag.getTagType()) {
case PROFILE:
- profiles.add(new IdDt(next.getTag().getCode()));
+ profiles.add(new IdDt(nextTag.getCode()));
break;
case SECURITY_LABEL:
IBaseCoding secLabel = (IBaseCoding) myFhirContext.getVersion().newCodingDt();
- secLabel.setSystem(next.getTag().getSystem());
- secLabel.setCode(next.getTag().getCode());
- secLabel.setDisplay(next.getTag().getDisplay());
+ secLabel.setSystem(nextTag.getSystem());
+ secLabel.setCode(nextTag.getCode());
+ secLabel.setDisplay(nextTag.getDisplay());
+ // wipmb these technically support userSelected and version
securityLabels.add(secLabel);
break;
case TAG:
- tagList.add(new Tag(next.getTag().getSystem(), next.getTag().getCode(), next.getTag().getDisplay()));
+ // wipmb check xml, etc.
+ Tag e = new Tag(nextTag.getSystem(), nextTag.getCode(), nextTag.getDisplay());
+ e.setVersion(nextTag.getVersion());
+ // careful! These are Boolean, not boolean.
+ e.setUserSelectedBoolean(nextTag.getUserSelected());
+ tagList.add(e);
break;
}
}
@@ -434,7 +442,12 @@ public class JpaStorageResourceParser implements IJpaStorageResourceParser {
tag.setCode(next.getTag().getCode());
tag.setDisplay(next.getTag().getDisplay());
tag.setVersion(next.getTag().getVersion());
- tag.setUserSelected(next.getTag().getUserSelected());
+ Boolean userSelected = next.getTag().getUserSelected();
+ // the tag is created with a null userSelected, but the api is primitive boolean.
+ // Only update if we are non-null.
+ if (nonNull(userSelected)) {
+ tag.setUserSelected(userSelected);
+ }
break;
}
}
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java
index 3b259a94513..18db9408637 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java
@@ -101,13 +101,26 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks {
protected void init680() {
Builder version = forVersion(VersionEnum.V6_8_0);
- // HAPI-FHIR #4801 - Add New Index On HFJ_RESOURCE
- Builder.BuilderWithTableName resourceTable = version.onTable("HFJ_RESOURCE");
- resourceTable
- .addIndex("20230502.1", "IDX_RES_RESID_UPDATED")
+ // HAPI-FHIR #4801 - Add New Index On HFJ_RESOURCE
+ Builder.BuilderWithTableName resourceTable = version.onTable("HFJ_RESOURCE");
+ resourceTable
+ .addIndex("20230502.1", "IDX_RES_RESID_UPDATED")
+ .unique(false)
+ .online(true)
+ .withColumns("RES_ID", "RES_UPDATED", "PARTITION_ID");
+
+ Builder.BuilderWithTableName tagDefTable = version.onTable("HFJ_TAG_DEF");
+ tagDefTable.dropIndex("20230505.1", "IDX_TAGDEF_TYPESYSCODEVERUS");
+
+ tagDefTable.dropIndex("20230505.2", "IDX_TAG_DEF_TP_CD_SYS");
+ tagDefTable
+ .addIndex("20230505.3", "IDX_TAG_DEF_TP_CD_SYS")
.unique(false)
- .online(true)
- .withColumns("RES_ID", "RES_UPDATED", "PARTITION_ID");
+ .online(false)
+ .withColumns("TAG_TYPE", "TAG_CODE", "TAG_SYSTEM", "TAG_ID", "TAG_VERSION", "TAG_USER_SELECTED");
+
+
+
}
protected void init660() {
@@ -362,8 +375,14 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks {
.online(true)
.withColumns("SEARCH_PID")
.onlyAppliesToPlatforms(NON_AUTOMATIC_FK_INDEX_PLATFORMS);
+
+ { //We added this constraint when userSelected and Version were added. It is no longer necessary.
+ Builder.BuilderWithTableName tagDefTable = version.onTable("HFJ_TAG_DEF");
+ tagDefTable.dropIndex("20230503.1", "IDX_TAGDEF_TYPESYSCODEVERUS");
+ }
}
+
private void init620() {
Builder version = forVersion(VersionEnum.V6_2_0);
diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/CodingSpyTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/CodingSpyTest.java
new file mode 100644
index 00000000000..f321c205e02
--- /dev/null
+++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/CodingSpyTest.java
@@ -0,0 +1,83 @@
+package ca.uhn.fhir.jpa.dao;
+
+import com.google.common.collect.Lists;
+import org.hl7.fhir.instance.model.api.IBaseCoding;
+import org.hl7.fhir.r4.model.BooleanType;
+import org.hl7.fhir.r4.model.Coding;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import javax.annotation.Nonnull;
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.blankOrNullString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+class CodingSpyTest {
+
+ /**
+ * Ensure we can read the default null value of userSelected on Coding
+ */
+
+ @ParameterizedTest
+ @MethodSource("getCases")
+ void canReadValueUserSelected(IBaseCoding theObject, Boolean theValue) {
+ IBaseCoding value = theObject.setSystem("http://example.com").setCode("value");
+ if (theValue != null) {
+ theObject.setUserSelected(theValue);
+ }
+
+ Boolean result = new CodingSpy().getBooleanObject(theObject);
+
+ assertEquals(theValue, result);
+ }
+
+ @Test
+ void canReadNullUserSelected() {
+ Coding value = new Coding().setSystem("http://example.com").setCode("value");
+
+ Boolean result = new CodingSpy().getBooleanObject(value);
+
+ assertNull(result);
+ }
+
+ static List getCases() {
+ var classes = List.of(
+ org.hl7.fhir.r4.model.Coding.class,
+ org.hl7.fhir.r5.model.Coding.class,
+ ca.uhn.fhir.model.api.Tag.class
+ );
+ var values = Lists.newArrayList(true, false, null);
+ return classes.stream()
+ .flatMap(k-> values.stream()
+ .map(v-> Arguments.of(getNewInstance(k), v)))
+ .toList();
+ }
+
+ @Nonnull
+ private static Object getNewInstance(Class extends IBaseCoding> k) {
+ try {
+ return k.getDeclaredConstructor().newInstance();
+ } catch (Exception theE) {
+ throw new RuntimeException(theE);
+ }
+ }
+
+
+ @Test
+ void booleanNulls() {
+ // given
+ BooleanType b = new BooleanType();
+
+ // when
+ var s = b.asStringValue();
+
+ assertThat(s, blankOrNullString());
+ }
+
+
+}
diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/TagDefinition.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/TagDefinition.java
index 72ccb2a14d5..c3e260f8fde 100644
--- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/TagDefinition.java
+++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/TagDefinition.java
@@ -46,11 +46,7 @@ import java.util.Collection;
@Table(
name = "HFJ_TAG_DEF",
indexes = {
- @Index(name = "IDX_TAG_DEF_TP_CD_SYS", columnList = "TAG_TYPE, TAG_CODE, TAG_SYSTEM, TAG_ID"),
- },
- uniqueConstraints = {
- @UniqueConstraint(name = "IDX_TAGDEF_TYPESYSCODEVERUS",
- columnNames = {"TAG_TYPE", "TAG_SYSTEM", "TAG_CODE", "TAG_VERSION", "TAG_USER_SELECTED"})
+ @Index(name = "IDX_TAG_DEF_TP_CD_SYS", columnList = "TAG_TYPE, TAG_CODE, TAG_SYSTEM, TAG_ID, TAG_VERSION, TAG_USER_SELECTED"),
}
)
public class TagDefinition implements Serializable {
@@ -161,14 +157,16 @@ public class TagDefinition implements Serializable {
}
}
+ /**
+ * Warning - this is nullable, while IBaseCoding getUserSelected isn't.
+ * wipmb maybe rename?
+ */
public Boolean getUserSelected() {
- // TODO: LD: this is not ideal as we are implicitly assuming null is false.
- // Ideally we should fix IBaseCoding to return wrapper Boolean but that will involve another core/hapi release
- return myUserSelected != null ? myUserSelected : false;
+ return myUserSelected;
}
public void setUserSelected(Boolean theUserSelected) {
- myUserSelected = theUserSelected != null && theUserSelected;
+ myUserSelected = theUserSelected;
}
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4MetaTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4MetaTest.java
index fa89ec23990..8584d4be187 100644
--- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4MetaTest.java
+++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4MetaTest.java
@@ -9,6 +9,7 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.param.TokenParam;
import ca.uhn.fhir.system.HapiTestSystemProperties;
+import ch.qos.logback.classic.Level;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Coding;
@@ -41,6 +42,8 @@ import static org.hamcrest.Matchers.empty;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -153,6 +156,30 @@ public class FhirResourceDaoR4MetaTest extends BaseJpaR4Test {
assertEquals("bar", patient2.getMeta().getSecurityFirstRep().getCode());
}
+ /**
+ * Make sure round-trips with tags don't add a userSelected property.
+ * wipmb tag userSelected test
+ * Verify https://github.com/hapifhir/hapi-fhir/issues/4819
+ */
+ @Test
+ void testAddTag_userSelectedStaysNull() {
+ // given
+ Patient patient1 = new Patient();
+ patient1.getMeta().addTag().setSystem("http://foo").setCode("bar");
+ patient1.setActive(true);
+ IIdType pid1 = myPatientDao.create(patient1).getId();
+
+ // when
+ var patientReadback = myPatientDao.read(pid1);
+ ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(patientReadback));
+
+ // then
+ var tag = patientReadback.getMeta().getTag().get(0);
+ assertNotNull(tag);
+ assertNull(tag.getUserSelectedElement().asStringValue());
+ }
+
+
@Nested
public class TestTagWithVersionAndUserSelected {
@@ -204,6 +231,7 @@ public class FhirResourceDaoR4MetaTest extends BaseJpaR4Test {
.setSystem(expectedSystem1)
.setCode(expectedCode1)
.setDisplay(expectedDisplay1);
+ assertNull(newTag.getUserSelectedElement().asStringValue());
assertFalse(newTag.getUserSelected());
newTag.setVersion(expectedVersion1)
.setUserSelected(expectedUserSelected1);
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java
index 61cf03ba0f3..07552eeb139 100644
--- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java
+++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java
@@ -5,6 +5,8 @@ import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
+import ca.uhn.fhir.jpa.model.entity.TagDefinition;
+import ca.uhn.fhir.jpa.model.entity.TagTypeEnum;
import ca.uhn.fhir.jpa.model.util.JpaConstants;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
@@ -96,6 +98,33 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
}
+ @Test
+ public void testTagCollision() {
+ TagDefinition def = new TagDefinition();
+ def.setTagType(TagTypeEnum.TAG);
+ def.setSystem("system");
+ def.setCode("coding");
+ def.setDisplay("display");
+ def.setUserSelected(null);
+
+
+ Patient p = new Patient();
+ p.getMeta().addTag("system", "coding", "display");
+
+ myMemoryCacheService.invalidateAllCaches();
+ myPatientDao.create(p, new SystemRequestDetails());
+ //inject conflicting.
+ myTagDefinitionDao.saveAndFlush(def);
+ myMemoryCacheService.invalidateAllCaches();
+
+ myPatientDao.create(p, new SystemRequestDetails());
+ myMemoryCacheService.invalidateAllCaches();
+
+ myPatientDao.create(p, new SystemRequestDetails());
+
+ }
+
+
@Test
public void testCreateAndUpdateWithoutRequest() {
String methodName = "testUpdateByUrl";