Add error checker for duplicate codesystem codes (#6014)
* Add error checker * Add changelog * Address review comment
This commit is contained in:
parent
8947706af3
commit
ec0021cd40
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
type: add
|
||||||
|
issue: 6014
|
||||||
|
title: "When uploading an invalid CodeSystem to the JPA server containing
|
||||||
|
duplicate codes, the server responded with an unhelpful error message
|
||||||
|
referring to a database constraint error. This has been fixed so that
|
||||||
|
a more informative error message is returned."
|
|
@ -50,8 +50,10 @@ import ca.uhn.fhir.rest.api.Constants;
|
||||||
import ca.uhn.fhir.rest.api.server.RequestDetails;
|
import ca.uhn.fhir.rest.api.server.RequestDetails;
|
||||||
import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;
|
import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;
|
||||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||||
|
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
|
||||||
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
|
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
|
||||||
import ca.uhn.fhir.util.ObjectUtil;
|
import ca.uhn.fhir.util.ObjectUtil;
|
||||||
|
import ca.uhn.fhir.util.UrlUtil;
|
||||||
import ca.uhn.fhir.util.ValidateUtil;
|
import ca.uhn.fhir.util.ValidateUtil;
|
||||||
import jakarta.annotation.Nonnull;
|
import jakarta.annotation.Nonnull;
|
||||||
import jakarta.persistence.EntityManager;
|
import jakarta.persistence.EntityManager;
|
||||||
|
@ -294,6 +296,8 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc {
|
||||||
theResourceEntity.getIdDt().getValue(),
|
theResourceEntity.getIdDt().getValue(),
|
||||||
theCodeSystem.getContentElement().getValueAsString());
|
theCodeSystem.getContentElement().getValueAsString());
|
||||||
|
|
||||||
|
detectDuplicatesInCodeSystem(theCodeSystem);
|
||||||
|
|
||||||
Long pid = (Long) theCodeSystem.getUserData(RESOURCE_PID_KEY);
|
Long pid = (Long) theCodeSystem.getUserData(RESOURCE_PID_KEY);
|
||||||
assert pid != null;
|
assert pid != null;
|
||||||
JpaPid codeSystemResourcePid = JpaPid.fromId(pid);
|
JpaPid codeSystemResourcePid = JpaPid.fromId(pid);
|
||||||
|
@ -339,6 +343,30 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static void detectDuplicatesInCodeSystem(CodeSystem theCodeSystem) {
|
||||||
|
detectDuplicatesInCodeSystem(theCodeSystem.getConcept(), new HashSet<>());
|
||||||
|
}
|
||||||
|
|
||||||
|
private static void detectDuplicatesInCodeSystem(
|
||||||
|
List<CodeSystem.ConceptDefinitionComponent> theCodeList, Set<String> theFoundCodesBuffer) {
|
||||||
|
for (var next : theCodeList) {
|
||||||
|
if (isNotBlank(next.getCode())) {
|
||||||
|
if (!theFoundCodesBuffer.add(next.getCode())) {
|
||||||
|
/*
|
||||||
|
* Note: We could possibly modify this behaviour to be forgiving, and just
|
||||||
|
* ignore duplicates. The only issue is that concepts can have properties,
|
||||||
|
* designations, etc. and it could be dangerous to just pick one and ignore the
|
||||||
|
* other. So the safer thing seems to be to just throw an error.
|
||||||
|
*/
|
||||||
|
throw new PreconditionFailedException(Msg.code(2528) + "Duplicate concept detected in CodeSystem: "
|
||||||
|
+ UrlUtil.sanitizeUrlPart(next.getCode()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Test child concepts within the parent concept
|
||||||
|
detectDuplicatesInCodeSystem(next.getConcept(), theFoundCodesBuffer);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@Transactional
|
@Transactional
|
||||||
public IIdType storeNewCodeSystemVersion(
|
public IIdType storeNewCodeSystemVersion(
|
||||||
|
|
|
@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable;
|
||||||
import ca.uhn.fhir.jpa.term.TermReindexingSvcImpl;
|
import ca.uhn.fhir.jpa.term.TermReindexingSvcImpl;
|
||||||
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
|
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
|
||||||
import ca.uhn.fhir.jpa.test.Batch2JobHelper;
|
import ca.uhn.fhir.jpa.test.Batch2JobHelper;
|
||||||
|
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
|
||||||
import org.apache.commons.io.IOUtils;
|
import org.apache.commons.io.IOUtils;
|
||||||
import org.hl7.fhir.instance.model.api.IIdType;
|
import org.hl7.fhir.instance.model.api.IIdType;
|
||||||
import org.hl7.fhir.r4.model.CodeSystem;
|
import org.hl7.fhir.r4.model.CodeSystem;
|
||||||
|
@ -20,6 +21,7 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||||
|
import static org.junit.jupiter.api.Assertions.fail;
|
||||||
|
|
||||||
public class FhirResourceDaoR4CodeSystemTest extends BaseJpaR4Test {
|
public class FhirResourceDaoR4CodeSystemTest extends BaseJpaR4Test {
|
||||||
|
|
||||||
|
@ -191,6 +193,46 @@ public class FhirResourceDaoR4CodeSystemTest extends BaseJpaR4Test {
|
||||||
return id;
|
return id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCodeSystemWithDuplicateCode() {
|
||||||
|
CodeSystem cs = new CodeSystem();
|
||||||
|
cs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE);
|
||||||
|
cs.setUrl("http://foo");
|
||||||
|
cs.setVersion("1.0");
|
||||||
|
cs.addConcept().setCode("CODE0").setDisplay("Code0");
|
||||||
|
cs.addConcept().setCode("CODE1").setDisplay("Code1");
|
||||||
|
cs.addConcept().setCode("CODE1").setDisplay("Code1");
|
||||||
|
cs.addConcept().setCode("CODE2").setDisplay("Code2");
|
||||||
|
|
||||||
|
try {
|
||||||
|
myCodeSystemDao.create(cs, mySrd);
|
||||||
|
fail();
|
||||||
|
} catch (PreconditionFailedException e) {
|
||||||
|
assertThat(e.getMessage()).contains("Duplicate concept detected in CodeSystem: CODE1");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCodeSystemWithDuplicateCodeInChild() {
|
||||||
|
CodeSystem cs = new CodeSystem();
|
||||||
|
cs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE);
|
||||||
|
cs.setUrl("http://foo");
|
||||||
|
cs.setVersion("1.0");
|
||||||
|
|
||||||
|
CodeSystem.ConceptDefinitionComponent parent = cs.addConcept().setCode("CODE0").setDisplay("Code0");
|
||||||
|
parent.addConcept().setCode("CODE1").setDisplay("Code1");
|
||||||
|
parent.addConcept().setCode("CODE1").setDisplay("Code1");
|
||||||
|
cs.addConcept().setCode("CODE2").setDisplay("Code2");
|
||||||
|
|
||||||
|
try {
|
||||||
|
myCodeSystemDao.create(cs, mySrd);
|
||||||
|
fail();
|
||||||
|
} catch (PreconditionFailedException e) {
|
||||||
|
assertThat(e.getMessage()).contains("Duplicate concept detected in CodeSystem: CODE1");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@AfterAll
|
@AfterAll
|
||||||
public static void afterClassClearContext() {
|
public static void afterClassClearContext() {
|
||||||
TermReindexingSvcImpl.setForceSaveDeferredAlwaysForUnitTest(false);
|
TermReindexingSvcImpl.setForceSaveDeferredAlwaysForUnitTest(false);
|
||||||
|
|
Loading…
Reference in New Issue