diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5183-us-core-ig-ingestion-fails-value-set-version-id-conflict.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5183-us-core-ig-ingestion-fails-value-set-version-id-conflict.yaml new file mode 100644 index 000000000000..6c143123bc6c --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5183-us-core-ig-ingestion-fails-value-set-version-id-conflict.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 5183 +title: "The latest US Core IG includes two ValueSets with different contents, but the same FHIR Id and OID via two different included IGs (i.e. `2.16.840.1.113762.1.4.1010.9` via us.cdc.phinvads and us.nlm.vsac). Ingesting these duplicates in US Core failed with a 500 error. This has been resolved by logging the error and allowing the rest of the ingestion to proceed." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java index 78e2707a23c9..723450da4d0e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java @@ -44,6 +44,7 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.param.UriParam; +import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.util.FhirTerser; import ca.uhn.fhir.util.SearchParameterUtil; import com.google.common.annotations.VisibleForTesting; @@ -53,6 +54,7 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.Identifier; +import org.hl7.fhir.r4.model.MetadataResource; import org.hl7.fhir.utilities.json.model.JsonObject; import org.hl7.fhir.utilities.npm.IPackageCacheManager; import org.hl7.fhir.utilities.npm.NpmPackage; @@ -342,7 +344,8 @@ protected void assertFhirVersionsAreCompatible(String fhirVersion, String curren /** * ============================= Utility methods =============================== */ - private void create( + @VisibleForTesting + void create( IBaseResource theResource, PackageInstallationSpec theInstallationSpec, PackageInstallOutcomeJson theOutcome) { @@ -365,8 +368,30 @@ private void create( String newIdPart = "npm-" + id.getIdPart(); id.setParts(id.getBaseUrl(), id.getResourceType(), newIdPart, id.getVersionIdPart()); } - updateResource(dao, theResource); - ourLog.info("Created resource with existing id"); + + try { + updateResource(dao, theResource); + + ourLog.info("Created resource with existing id"); + } catch (ResourceVersionConflictException exception) { + final Optional optResource = readResourceById(dao, id); + + final String existingResourceUrlOrNull = optResource + .filter(MetadataResource.class::isInstance) + .map(MetadataResource.class::cast) + .map(MetadataResource::getUrl) + .orElse(null); + final String newResourceUrlOrNull = (theResource instanceof MetadataResource) + ? ((MetadataResource) theResource).getUrl() + : null; + + ourLog.error( + "Version conflict error: This is possibly due to a collision between ValueSets from different IGs that are coincidentally using the same resource ID: [{}] and new resource URL: [{}], with the exisitng resource having URL: [{}]. Ignoring this update and continuing: The first IG wins. ", + id.getIdPart(), + newResourceUrlOrNull, + existingResourceUrlOrNull, + exception); + } } } else { if (theInstallationSpec.isReloadExisting()) { @@ -394,6 +419,18 @@ private void create( } } + private Optional readResourceById(IFhirResourceDao dao, IIdType id) { + try { + return Optional.ofNullable(dao.read(id.toUnqualifiedVersionless(), newSystemRequestDetails())); + + } catch (Exception exception) { + // ignore because we're running this query to help build the log + ourLog.warn("Exception when trying to read resource with ID: {}, message: {}", id, exception.getMessage()); + } + + return Optional.empty(); + } + private IBundleProvider searchResource(IFhirResourceDao theDao, SearchParameterMap theMap) { return theDao.search(theMap, newSystemRequestDetails()); } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplCreateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplCreateTest.java new file mode 100644 index 000000000000..da9b8ee62e05 --- /dev/null +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplCreateTest.java @@ -0,0 +1,247 @@ +package ca.uhn.fhir.jpa.packages; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.jpa.dao.data.ITermValueSetDao; +import ca.uhn.fhir.jpa.entity.TermValueSet; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.test.BaseJpaR4Test; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.NamingSystem; +import org.hl7.fhir.r4.model.ValueSet; +import org.hl7.fhir.utilities.npm.NpmPackage; +import org.hl7.fhir.utilities.npm.PackageGenerator; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import javax.annotation.Nonnull; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class PackageInstallerSvcImplCreateTest extends BaseJpaR4Test { + private static final String PACKAGE_ID_1 = "package1"; + private static final String PACKAGE_VERSION = "1.0"; + private static final String VALUE_SET_OID_FIRST = "2.16.840.1.113762.1.4.1010.9"; + private static final String VALUE_SET_OID_SECOND = "2.16.840.1.113762.1.4.1010.10"; + private static final String IG_FIRST = "first.ig.gov"; + private static final String IG_SECOND = "second.ig.gov"; + private static final String FIRST_IG_URL_FIRST_OID = String.format("http://%s/fhir/ValueSet/%s", IG_FIRST, VALUE_SET_OID_FIRST); + private static final String SECOND_IG_URL_FIRST_OID = String.format("http://%s/fhir/ValueSet/%s", IG_SECOND, VALUE_SET_OID_FIRST); + private static final String SECOND_IG_URL_SECOND_OID = String.format("http://%s/fhir/ValueSet/%s", IG_SECOND, VALUE_SET_OID_SECOND); + private static final FhirContext ourCtx = FhirContext.forR4Cached(); + private static final CodeSystem CODE_SYSTEM = createCodeSystem(); + private static final NpmPackage PACKAGE = createPackage(); + private static final SystemRequestDetails REQUEST_DETAILS = new SystemRequestDetails(); + + @Autowired + private ITermValueSetDao myTermValueSetDao; + + @Autowired + private PackageInstallerSvcImpl mySvc; + @Test + void createNamingSystem() throws IOException { + final NamingSystem namingSystem = new NamingSystem(); + namingSystem.getUniqueId().add(new NamingSystem.NamingSystemUniqueIdComponent().setValue("123")); + + create(namingSystem); + + assertEquals(1, myNamingSystemDao.search(SearchParameterMap.newSynchronous(), REQUEST_DETAILS).getAllResources().size()); + } + + @Test + void createWithNoExistingResourcesNoIdOnValueSet() throws IOException { + final String version1 = "abc"; + final String copyright1 = "first"; + + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, null, version1, FIRST_IG_URL_FIRST_OID, copyright1); + + final ValueSet actualValueSet1 = getFirstValueSet(); + + assertEquals("ValueSet/" + VALUE_SET_OID_FIRST, actualValueSet1.getIdElement().toUnqualifiedVersionless().getValue()); + assertEquals(FIRST_IG_URL_FIRST_OID, actualValueSet1.getUrl()); + assertEquals(version1, actualValueSet1.getVersion()); + assertEquals(copyright1, actualValueSet1.getCopyright()); + } + + @Test + void createWithNoExistingResourcesIdOnValueSet() throws IOException { + final String version1 = "abc"; + final String copyright1 = "first"; + + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, null, version1, FIRST_IG_URL_FIRST_OID, copyright1); + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, "43", version1, SECOND_IG_URL_FIRST_OID, copyright1); + + final TermValueSet termValueSet = getFirstTermValueSet(); + + assertEquals(FIRST_IG_URL_FIRST_OID, termValueSet.getUrl()); + + final ValueSet actualValueSet1 = getFirstValueSet(); + + assertEquals("ValueSet/" + VALUE_SET_OID_FIRST, actualValueSet1.getIdElement().toUnqualifiedVersionless().getValue()); + assertEquals(FIRST_IG_URL_FIRST_OID, actualValueSet1.getUrl()); + assertEquals(version1, actualValueSet1.getVersion()); + assertEquals(copyright1, actualValueSet1.getCopyright()); + } + + @Test + void createValueSetThenUpdateSameUrl() throws IOException { + final String version1 = "abc"; + final String version2 = "def"; + final String copyright1 = "first"; + final String copyright2 = "second"; + + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, null, version1, FIRST_IG_URL_FIRST_OID, copyright1); + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, "43", version2, FIRST_IG_URL_FIRST_OID, copyright2); + + final ValueSet actualValueSet1 = getFirstValueSet(); + + assertEquals("ValueSet/" + VALUE_SET_OID_FIRST, actualValueSet1.getIdElement().toUnqualifiedVersionless().getValue()); + assertEquals(FIRST_IG_URL_FIRST_OID, actualValueSet1.getUrl()); + assertEquals(version2, actualValueSet1.getVersion()); + assertEquals(copyright2, actualValueSet1.getCopyright()); + } + + @Test + void createTwoDifferentValueSets() throws IOException { + final String version1 = "abc"; + final String version2 = "def"; + final String copyright1 = "first"; + final String copyright2 = "second"; + + createValueSetAndCallCreate(VALUE_SET_OID_FIRST, null, version1, FIRST_IG_URL_FIRST_OID, copyright1); + createValueSetAndCallCreate(VALUE_SET_OID_SECOND, "43", version2, SECOND_IG_URL_SECOND_OID, copyright2); + + final List all2 = myTermValueSetDao.findAll(); + + assertEquals(2, all2.size()); + + final TermValueSet termValueSet1 = all2.get(0); + final TermValueSet termValueSet2 = all2.get(1); + + assertEquals(FIRST_IG_URL_FIRST_OID, termValueSet1.getUrl()); + assertEquals(SECOND_IG_URL_SECOND_OID, termValueSet2.getUrl()); + + final List allValueSets = getAllValueSets(); + + assertEquals(2, allValueSets.size()); + + final ValueSet actualValueSet1 = allValueSets.get(0); + + assertEquals("ValueSet/" + VALUE_SET_OID_FIRST, actualValueSet1.getIdElement().toUnqualifiedVersionless().getValue()); + assertEquals(FIRST_IG_URL_FIRST_OID, actualValueSet1.getUrl()); + assertEquals(version1, actualValueSet1.getVersion()); + assertEquals(copyright1, actualValueSet1.getCopyright()); + + final ValueSet actualValueSet2 = allValueSets.get(1); + + assertEquals("ValueSet/" + VALUE_SET_OID_SECOND, actualValueSet2.getIdElement().toUnqualifiedVersionless().getValue()); + assertEquals(SECOND_IG_URL_SECOND_OID, actualValueSet2.getUrl()); + assertEquals(version2, actualValueSet2.getVersion()); + assertEquals(copyright2, actualValueSet2.getCopyright()); + } + + @Nonnull + private List getAllValueSets() { + final List allResources = myValueSetDao.search(SearchParameterMap.newSynchronous(), REQUEST_DETAILS).getAllResources(); + + assertFalse(allResources.isEmpty()); + assertTrue(allResources.get(0) instanceof ValueSet); + + return allResources.stream() + .map(ValueSet.class::cast) + .toList(); + } + + @Nonnull + private ValueSet getFirstValueSet() { + final List allResources = myValueSetDao.search(SearchParameterMap.newSynchronous(), REQUEST_DETAILS).getAllResources(); + + assertEquals(1, allResources.size()); + + final IBaseResource resource1 = allResources.get(0); + assertTrue(resource1 instanceof ValueSet); + + return (ValueSet) resource1; + } + + @Nonnull + private TermValueSet getFirstTermValueSet() { + final List all2 = myTermValueSetDao.findAll(); + + assertEquals(1, all2.size()); + + return all2.get(0); + } + + private void createValueSetAndCallCreate(String theOid, String theResourceVersion, String theValueSetVersion, String theUrl, String theCopyright) throws IOException { + create(createValueSet(theOid, theResourceVersion, theValueSetVersion, theUrl, theCopyright)); + } + + @Nonnull + private static ValueSet createValueSet(String theOid, String theResourceVersion, String theValueSetVersion, String theUrl, String theCopyright) { + final ValueSet valueSetFromFirstIg = new ValueSet(); + + valueSetFromFirstIg.setUrl(theUrl); + valueSetFromFirstIg.setId(new IdDt(null, "ValueSet", theOid, theResourceVersion)); + valueSetFromFirstIg.setVersion(theValueSetVersion); + valueSetFromFirstIg.setCopyright(theCopyright); + + return valueSetFromFirstIg; + } + + private void create(IBaseResource theResource) throws IOException { + mySvc.create(theResource, createInstallationSpec(packageToBytes()), new PackageInstallOutcomeJson()); + } + + @Nonnull + private static CodeSystem createCodeSystem() { + final CodeSystem cs = new CodeSystem(); + cs.setId("CodeSystem/mycs"); + cs.setUrl("http://my-code-system"); + cs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE); + return cs; + } + + @Nonnull + private static NpmPackage createPackage() { + PackageGenerator manifestGenerator = new PackageGenerator(); + manifestGenerator.name(PackageInstallerSvcImplCreateTest.PACKAGE_ID_1); + manifestGenerator.version(PACKAGE_VERSION); + manifestGenerator.description("a package"); + manifestGenerator.fhirVersions(List.of(FhirVersionEnum.R4.getFhirVersionString())); + + NpmPackage pkg = NpmPackage.empty(manifestGenerator); + + String csString = ourCtx.newJsonParser().encodeResourceToString(CODE_SYSTEM); + pkg.addFile("package", "cs.json", csString.getBytes(StandardCharsets.UTF_8), "CodeSystem"); + + return pkg; + } + + @Nonnull + private static PackageInstallationSpec createInstallationSpec(byte[] thePackageContents) { + final PackageInstallationSpec spec = new PackageInstallationSpec(); + spec.setName(PACKAGE_ID_1); + spec.setVersion(PACKAGE_VERSION); + spec.setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL); + spec.setPackageContents(thePackageContents); + return spec; + } + + @Nonnull + private static byte[] packageToBytes() throws IOException { + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + PackageInstallerSvcImplCreateTest.PACKAGE.save(stream); + return stream.toByteArray(); + } +}