honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Error when expanding task code ValueSet resource (#4974) 

* Error when expanding task-code ValueSet resource - tests

* Error when expanding task-code ValueSet resource - fix valuesets.xml for R4 and R4B

* Error when expanding task-code ValueSet resource - throw exception in case of Not found CodeSystem

* Error when expanding task-code ValueSet resource - added changelog

* Error when expanding task-code ValueSet resource - fixed changelog

* Error when expanding task-code ValueSet resource - fixes

* Error when expanding task-code ValueSet resource - fixes

* Error when expanding task-code ValueSet resource - fixes and tests

* Error when expanding task-code ValueSet resource - added tests for TermCodeSystemVersionDao methods

* Error when expanding task-code ValueSet resource - added comments to valuesets.xml

* Error when expanding task-code ValueSet resource - test fixes
This commit is contained in:
volodymyr-korzh 2023-06-19 07:31:05 -06:00 committed by GitHub
parent a544a546ec
commit 6296884583
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 144 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4971-expand-task-code-value-set-returns-null-pointer.yaml Normal file
View File

@ -0,0 +1,4 @@
---
type: fix
issue: 4971
title: "Previously, expanding a valueSet including a codeSystem that is unknown to the server would return an internal error(500) to the caller. This problem has been fixed."

4
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermCodeSystemVersionDao.java
View File

@ -40,6 +40,10 @@ public interface ITermCodeSystemVersionDao extends JpaRepository<TermCodeSystemV
@Query("SELECT cs FROM TermCodeSystemVersion cs WHERE cs.myCodeSystemPid = :codesystem_pid AND cs.myCodeSystemVersionId = :codesystem_version_id")
TermCodeSystemVersion findByCodeSystemPidAndVersion(@Param("codesystem_pid") Long theCodeSystemPid, @Param("codesystem_version_id") String theCodeSystemVersionId);
@Query("SELECT tcsv FROM TermCodeSystemVersion tcsv INNER JOIN FETCH TermCodeSystem tcs on tcs.myPid = tcsv.myCodeSystemPid " +
"WHERE tcs.myCodeSystemUri = :code_system_uri AND tcsv.myCodeSystemVersionId = :codesystem_version_id")
TermCodeSystemVersion findByCodeSystemUriAndVersion(@Param("code_system_uri") String theCodeSystemUri, @Param("codesystem_version_id") String theCodeSystemVersionId);
@Query("SELECT cs FROM TermCodeSystemVersion cs WHERE cs.myCodeSystemPid = :codesystem_pid AND cs.myCodeSystemVersionId IS NULL")
TermCodeSystemVersion findByCodeSystemPidVersionIsNull(@Param("codesystem_pid") Long theCodeSystemPid);

55
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java
View File

@ -205,6 +205,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
private static final String IDX_PROP_KEY = IDX_PROPERTIES + ".myKey";
private static final String IDX_PROP_VALUE_STRING = IDX_PROPERTIES + ".myValueString";
private static final String IDX_PROP_DISPLAY_STRING = IDX_PROPERTIES + ".myDisplayString";
private static final String OUR_PIPE_CHARACTER = "|";
private static final int SECONDS_IN_MINUTE = 60;
private static final int INDEXED_ROOTS_LOGGING_COUNT = 50_000;
private static Runnable myInvokeOnNextCallForUnitTest;
@ -296,7 +297,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
Collection<TermConceptDesignation> designations = theConcept.getDesignations();
if (StringUtils.isNotEmpty(theValueSetIncludeVersion)) {
return addCodeIfNotAlreadyAdded(theValueSetCodeAccumulator, theAddedCodes, designations, theAdd, codeSystem + "|" + theValueSetIncludeVersion, code, display, sourceConceptPid, directParentPids, codeSystemVersion);
return addCodeIfNotAlreadyAdded(theValueSetCodeAccumulator, theAddedCodes, designations, theAdd, codeSystem + OUR_PIPE_CHARACTER + theValueSetIncludeVersion, code, display, sourceConceptPid, directParentPids, codeSystemVersion);
} else {
return addCodeIfNotAlreadyAdded(theValueSetCodeAccumulator, theAddedCodes, designations, theAdd, codeSystem, code, display, sourceConceptPid, directParentPids, codeSystemVersion);
}
@ -305,23 +306,23 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
private boolean addCodeIfNotAlreadyAdded(IValueSetConceptAccumulator theValueSetCodeAccumulator, Set<String> theAddedCodes, boolean theAdd, String theCodeSystem, String theCodeSystemVersion, String theCode, String theDisplay, Long theSourceConceptPid, String theSourceConceptDirectParentPids, Collection<TermConceptDesignation> theDesignations) {
if (StringUtils.isNotEmpty(theCodeSystemVersion)) {
if (isNoneBlank(theCodeSystem, theCode)) {
if (theAdd && theAddedCodes.add(theCodeSystem + "|" + theCode)) {
theValueSetCodeAccumulator.includeConceptWithDesignations(theCodeSystem + "|" + theCodeSystemVersion, theCode, theDisplay, theDesignations, theSourceConceptPid, theSourceConceptDirectParentPids, theCodeSystemVersion);
if (theAdd && theAddedCodes.add(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.includeConceptWithDesignations(theCodeSystem + OUR_PIPE_CHARACTER + theCodeSystemVersion, theCode, theDisplay, theDesignations, theSourceConceptPid, theSourceConceptDirectParentPids, theCodeSystemVersion);
return true;
}
if (!theAdd && theAddedCodes.remove(theCodeSystem + "|" + theCode)) {
theValueSetCodeAccumulator.excludeConcept(theCodeSystem + "|" + theCodeSystemVersion, theCode);
if (!theAdd && theAddedCodes.remove(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.excludeConcept(theCodeSystem + OUR_PIPE_CHARACTER + theCodeSystemVersion, theCode);
return true;
}
}
} else {
if (theAdd && theAddedCodes.add(theCodeSystem + "|" + theCode)) {
if (theAdd && theAddedCodes.add(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.includeConceptWithDesignations(theCodeSystem, theCode, theDisplay, theDesignations, theSourceConceptPid, theSourceConceptDirectParentPids, theCodeSystemVersion);
return true;
}
if (!theAdd && theAddedCodes.remove(theCodeSystem + "|" + theCode)) {
if (!theAdd && theAddedCodes.remove(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.excludeConcept(theCodeSystem, theCode);
return true;
}
@ -332,12 +333,12 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
private boolean addCodeIfNotAlreadyAdded(IValueSetConceptAccumulator theValueSetCodeAccumulator, Set<String> theAddedCodes, Collection<TermConceptDesignation> theDesignations, boolean theAdd, String theCodeSystem, String theCode, String theDisplay, Long theSourceConceptPid, String theSourceConceptDirectParentPids, String theSystemVersion) {
if (isNoneBlank(theCodeSystem, theCode)) {
if (theAdd && theAddedCodes.add(theCodeSystem + "|" + theCode)) {
if (theAdd && theAddedCodes.add(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.includeConceptWithDesignations(theCodeSystem, theCode, theDisplay, theDesignations, theSourceConceptPid, theSourceConceptDirectParentPids, theSystemVersion);
return true;
}
if (!theAdd && theAddedCodes.remove(theCodeSystem + "|" + theCode)) {
if (!theAdd && theAddedCodes.remove(theCodeSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.excludeConcept(theCodeSystem, theCode);
return true;
}
@ -798,11 +799,11 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
ourLog.debug("Starting {} expansion around CodeSystem: {}", (theAdd ? "inclusion" : "exclusion"), system);
TermCodeSystem cs = myCodeSystemDao.findByCodeSystemUri(system);
if (cs != null) {
Optional<TermCodeSystemVersion> termCodeSystemVersion = optionalFindTermCodeSystemVersion(theIncludeOrExclude);
if (termCodeSystemVersion.isPresent()) {
expandValueSetHandleIncludeOrExcludeUsingDatabase(theExpansionOptions, theValueSetCodeAccumulator,
theAddedCodes, theIncludeOrExclude, theAdd, theExpansionFilter, system, cs);
theAddedCodes, theIncludeOrExclude, theAdd, theExpansionFilter, system, termCodeSystemVersion.get());
} else {
@ -857,6 +858,14 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
}
private Optional<TermCodeSystemVersion> optionalFindTermCodeSystemVersion(ValueSet.ConceptSetComponent theIncludeOrExclude) {
if (isEmpty(theIncludeOrExclude.getVersion())) {
return Optional.ofNullable(myCodeSystemDao.findByCodeSystemUri(theIncludeOrExclude.getSystem())).map(TermCodeSystem::getCurrentVersion);
} else {
return Optional.ofNullable(myCodeSystemVersionDao.findByCodeSystemUriAndVersion(theIncludeOrExclude.getSystem(), theIncludeOrExclude.getVersion()));
}
}
private boolean isHibernateSearchEnabled() {
return myFulltextSearchSvc != null && !ourForceDisableHibernateSearchForUnitTest;
}
@ -869,21 +878,17 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
boolean theAdd,
@Nonnull ExpansionFilter theExpansionFilter,
String theSystem,
TermCodeSystem theCs) {
TermCodeSystemVersion theTermCodeSystemVersion) {
StopWatch fullOperationSw = new StopWatch();
String includeOrExcludeVersion = theIncludeOrExclude.getVersion();
TermCodeSystemVersion termCodeSystemVersion = isEmpty(includeOrExcludeVersion)
? theCs.getCurrentVersion()
: myCodeSystemVersionDao.findByCodeSystemPidAndVersion(theCs.getPid(), includeOrExcludeVersion);
/*
* If FullText searching is not enabled, we can handle only basic expansions
* since we're going to do it without the database.
*/
if (!isHibernateSearchEnabled()) {
expandWithoutHibernateSearch(theValueSetCodeAccumulator, termCodeSystemVersion, theAddedCodes, theIncludeOrExclude, theSystem, theAdd);
expandWithoutHibernateSearch(theValueSetCodeAccumulator, theTermCodeSystemVersion, theAddedCodes, theIncludeOrExclude, theSystem, theAdd);
return;
}
@ -899,7 +904,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
}
int chunkSize = chunkSizeOpt.get();
SearchProperties searchProps = buildSearchScroll(termCodeSystemVersion, theExpansionFilter, theSystem,
SearchProperties searchProps = buildSearchScroll(theTermCodeSystemVersion, theExpansionFilter, theSystem,
theIncludeOrExclude, chunkSize, includeOrExcludeVersion);
int accumulatedBatchesSoFar = 0;
@ -1087,7 +1092,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
private String buildCodeSystemUrlAndVersion(String theSystem, String theIncludeOrExcludeVersion) {
String codeSystemUrlAndVersion;
if (theIncludeOrExcludeVersion != null) {
codeSystemUrlAndVersion = theSystem + "|" + theIncludeOrExcludeVersion;
codeSystemUrlAndVersion = theSystem + OUR_PIPE_CHARACTER + theIncludeOrExcludeVersion;
} else {
codeSystemUrlAndVersion = theSystem;
}
@ -1100,10 +1105,10 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
}
private void addOrRemoveCode(IValueSetConceptAccumulator theValueSetCodeAccumulator, Set<String> theAddedCodes, boolean theAdd, String theSystem, String theCode, String theDisplay, String theSystemVersion) {
if (theAdd && theAddedCodes.add(theSystem + "|" + theCode)) {
if (theAdd && theAddedCodes.add(theSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.includeConcept(theSystem, theCode, theDisplay, null, null, theSystemVersion);
}
if (!theAdd && theAddedCodes.remove(theSystem + "|" + theCode)) {
if (!theAdd && theAddedCodes.remove(theSystem + OUR_PIPE_CHARACTER + theCode)) {
theValueSetCodeAccumulator.excludeConcept(theSystem, theCode);
}
}
@ -1697,7 +1702,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
List<TermValueSetConcept> retVal = new ArrayList<>();
Optional<TermValueSetConcept> optionalTermValueSetConcept;
int versionIndex = theSystem.indexOf("|");
int versionIndex = theSystem.indexOf(OUR_PIPE_CHARACTER);
if (versionIndex >= 0) {
String systemUrl = theSystem.substring(0, versionIndex);
String systemVersion = theSystem.substring(versionIndex + 1);
@ -2075,7 +2080,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
String codeASystemIdentifier;
if (StringUtils.isNotEmpty(conceptA.getSystemVersion())) {
codeASystemIdentifier = conceptA.getSystem() + "|" + conceptA.getSystemVersion();
codeASystemIdentifier = conceptA.getSystem() + OUR_PIPE_CHARACTER + conceptA.getSystemVersion();
} else {
codeASystemIdentifier = conceptA.getSystem();
}
@ -2084,7 +2089,7 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs {
String codeBSystemIdentifier;
if (StringUtils.isNotEmpty(conceptB.getSystemVersion())) {
codeBSystemIdentifier = conceptB.getSystem() + "|" + conceptB.getSystemVersion();
codeBSystemIdentifier = conceptB.getSystem() + OUR_PIPE_CHARACTER + conceptB.getSystemVersion();
} else {
codeBSystemIdentifier = conceptB.getSystem();
}

44
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4Test.java
View File

@ -22,7 +22,9 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import com.google.common.collect.Lists;
import org.hamcrest.MatcherAssert;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.CodeSystem;
import org.hl7.fhir.r4.model.CodeType;
import org.hl7.fhir.r4.model.Enumerations;
@ -33,7 +35,6 @@ import org.hl7.fhir.r4.model.ValueSet;
import org.hl7.fhir.r4.model.codesystems.HttpVerb;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.slf4j.Logger;
@ -47,7 +48,6 @@ import javax.annotation.Nonnull;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import static ca.uhn.fhir.util.HapiExtensions.EXT_VALUESET_EXPANSION_MESSAGE;
@ -1020,6 +1020,46 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
MatcherAssert.assertThat(myValueSetTestUtil.toCodes(expandedValueSet), is(equalTo(expandedConceptCodes.subList(1, 23))));
}
/**
* This test intended to check that version of included CodeSystem in task-code ValueSet is correct
* There is a typo in the FHIR Specification: <a href="http://hl7.org/fhir/R4/valueset-task-code.xml.html"/>
* As agreed in FHIR-30377 included CodeSystem version for task-code ValueSet changed from 3.6.0 to 4.0.1
* <a href="https://jira.hl7.org/browse/FHIR-30377">Source resources for task-code ValueSet reference old version of CodeSystem</a>
*/
@Test
public void testExpandTaskCodeValueSet_withCorrectedCodeSystemVersion_willExpandCorrectly() throws IOException {
// load validation file
Bundle r4ValueSets = loadResourceFromClasspath(Bundle.class, "/org/hl7/fhir/r4/model/valueset/valuesets.xml");
ValueSet taskCodeVs = (ValueSet) findResourceByFullUrlInBundle(r4ValueSets, "http://hl7.org/fhir/ValueSet/task-code");
CodeSystem taskCodeCs = (CodeSystem) findResourceByFullUrlInBundle(r4ValueSets, "http://hl7.org/fhir/CodeSystem/task-code");
// check valueSet and codeSystem versions
String expectedCodeSystemVersion = "4.0.1";
assertEquals(expectedCodeSystemVersion, taskCodeCs.getVersion());
assertEquals(expectedCodeSystemVersion, taskCodeVs.getVersion());
assertEquals(expectedCodeSystemVersion, taskCodeVs.getCompose().getInclude().get(0).getVersion());
myCodeSystemDao.create(taskCodeCs);
IIdType id = myValueSetDao.create(taskCodeVs).getId();
ValueSet expandedValueSet = myValueSetDao.expand(id, new ValueSetExpansionOptions(), mySrd);
// check expansion size and include CodeSystem version
assertEquals(7, expandedValueSet.getExpansion().getContains().size());
assertEquals(1, expandedValueSet.getCompose().getInclude().size());
assertEquals(expectedCodeSystemVersion, expandedValueSet.getCompose().getInclude().get(0).getVersion());
}
private IBaseResource findResourceByFullUrlInBundle(Bundle thebundle, String theFullUrl) {
Optional<Bundle.BundleEntryComponent> bundleEntry = thebundle.getEntry().stream()
.filter(entry -> theFullUrl.equals(entry.getFullUrl()))
.findFirst();
if (bundleEntry.isEmpty()) {
fail("Can't find resource: " + theFullUrl);
}
return bundleEntry.get().getResource();
}
@Test
public void testExpandValueSetPreservesExplicitOrder() {

60
hapi-fhir-jpaserver-test-r4b/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionR4BTest.java Normal file
View File

@ -0,0 +1,60 @@
package ca.uhn.fhir.jpa.term;
import ca.uhn.fhir.context.support.ValueSetExpansionOptions;
import ca.uhn.fhir.jpa.dao.r4b.BaseJpaR4BTest;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4b.model.Bundle;
import org.hl7.fhir.r4b.model.CodeSystem;
import org.hl7.fhir.r4b.model.ValueSet;
import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
public class ValueSetExpansionR4BTest extends BaseJpaR4BTest {
/**
* This test intended to check that version of included CodeSystem in task-code ValueSet is correct
* There is a typo in the FHIR Specification <a href="http://hl7.org/fhir/R4B/valueset-task-code.xml.html"/>
* As agreed in FHIR-30377 included CodeSystem version for task-code ValueSet changed from 3.6.0 to 4.3.0
* <a href="https://jira.hl7.org/browse/FHIR-30377">Source resources for task-code ValueSet reference old version of CodeSystem</a>
*/
@Test
public void testExpandTaskCodeValueSet_withCorrectedCodeSystemVersion_willExpandCorrectly() throws IOException {
// load validation file
Bundle r4bValueSets = loadResourceFromClasspath(Bundle.class, "/org/hl7/fhir/r4b/model/valueset/valuesets.xml");
ValueSet taskCodeVs = (ValueSet) findResourceByFullUrlInBundle(r4bValueSets, "http://hl7.org/fhir/ValueSet/task-code");
CodeSystem taskCodeCs = (CodeSystem) findResourceByFullUrlInBundle(r4bValueSets, "http://hl7.org/fhir/CodeSystem/task-code");
// check valueSet and codeSystem versions
String expectedCodeSystemVersion = "4.3.0";
assertEquals(expectedCodeSystemVersion, taskCodeCs.getVersion());
assertEquals(expectedCodeSystemVersion, taskCodeVs.getVersion());
assertEquals(expectedCodeSystemVersion, taskCodeVs.getCompose().getInclude().get(0).getVersion());
myCodeSystemDao.create(taskCodeCs);
IIdType id = myValueSetDao.create(taskCodeVs).getId();
ValueSet expandedValueSet = myValueSetDao.expand(id, new ValueSetExpansionOptions(), mySrd);
// check expansion size and include CodeSystem version
assertEquals(7, expandedValueSet.getExpansion().getContains().size());
assertEquals(1, expandedValueSet.getCompose().getInclude().size());
assertEquals(expectedCodeSystemVersion, expandedValueSet.getCompose().getInclude().get(0).getVersion());
}
private IBaseResource findResourceByFullUrlInBundle(Bundle thebundle, String theFullUrl) {
Optional<Bundle.BundleEntryComponent> bundleEntry = thebundle.getEntry().stream()
.filter(entry -> theFullUrl.equals(entry.getFullUrl()))
.findFirst();
if (bundleEntry.isEmpty()) {
fail("Can't find resource: " + theFullUrl);
}
return bundleEntry.get().getResource();
}
}

3
hapi-fhir-validation-resources-r4/src/main/resources/org/hl7/fhir/r4/model/valueset/valuesets.xml
View File

@ -133510,7 +133510,8 @@
<compose>
<include>
<system value="http://hl7.org/fhir/CodeSystem/task-code"></system>
<version value="3.6.0"></version>
<!-- changed from 3.6.0 to 4.0.1 due to a typo in the FHIR Specification, as agreed in https://jira.hl7.org/browse/FHIR-30377 -->
<version value="4.0.1"></version>
</include>
</compose>
</ValueSet>

3
hapi-fhir-validation-resources-r4b/src/main/resources/org/hl7/fhir/r4b/model/valueset/valuesets.xml
View File

@ -114221,7 +114221,8 @@
<compose>
<include>
<system value="http://hl7.org/fhir/CodeSystem/task-code"></system>
<version value="3.6.0"></version>
<!-- changed from 3.6.0 to 4.3.0 due to a typo in the FHIR Specification, as agreed in https://jira.hl7.org/browse/FHIR-30377 -->
<version value="4.3.0"></version>
</include>
</compose>
</ValueSet>