fixing bug in findcandidatebyexamplesvc (#5101)

* fixing build

* cleanup

* review fix

* review changes

---------

Co-authored-by: leif stawnyczy <leifstawnyczy@leifs-MacBook-Pro.local>
This commit is contained in:
TipzCM 2023-07-19 08:26:05 -04:00 committed by GitHub
parent d6fae7c673
commit cf8c70994f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 522 additions and 165 deletions

View File

@ -0,0 +1,7 @@
---
type: fix
issue: 5100
title: "Fixed a bug in FindCandidateByExampleSvc that resulted in
class cast exceptions for IResourcePersistentIds that are not
based on Long value ids.
"

View File

@ -93,7 +93,7 @@ public class FindCandidateByExampleSvc<P extends IResourcePersistentId> extends
// we'll track the added ids so we don't add the same resources twice // we'll track the added ids so we don't add the same resources twice
// note, all these resources are the same type, so we only need the Long value // note, all these resources are the same type, so we only need the Long value
Set<Long> currentIds = new HashSet<>(); Set<String> currentIds = new HashSet<>();
for (MatchedTarget match : matchedCandidates) { for (MatchedTarget match : matchedCandidates) {
Optional<? extends IMdmLink> optionalMdmLink = myMdmLinkDaoSvc.getMatchedLinkForSourcePid( Optional<? extends IMdmLink> optionalMdmLink = myMdmLinkDaoSvc.getMatchedLinkForSourcePid(
myIdHelperService.getPidOrNull(RequestPartitionId.allPartitions(), match.getTarget())); myIdHelperService.getPidOrNull(RequestPartitionId.allPartitions(), match.getTarget()));
@ -125,7 +125,11 @@ public class FindCandidateByExampleSvc<P extends IResourcePersistentId> extends
} }
// only add if it's not already in the list // only add if it's not already in the list
if (currentIds.add((Long) candidate.getCandidateGoldenResourcePid().getId())) { // NB: we cannot use hash of IResourcePersistentId because
// BaseResourcePersistentId overrides this (and so is the same
// for any class with the same version) :(
if (currentIds.add(
String.valueOf(candidate.getCandidateGoldenResourcePid().getId()))) {
retval.add(candidate); retval.add(candidate);
} }
} }

View File

@ -0,0 +1,26 @@
package ca.uhn.fhir.jpa.mdm.helper.testmodels;
import ca.uhn.fhir.rest.api.server.storage.BaseResourcePersistentId;
/**
* A test ResourceId with a non-long implementation of the id
*/
public class StringResourceId extends BaseResourcePersistentId<String> {
private final String myId;
public StringResourceId(String theId) {
super(null);
myId = theId;
}
public StringResourceId(Long theVersion, String theId) {
super(theVersion, null);
myId = theId;
}
@Override
public String getId() {
return myId;
}
}

View File

@ -0,0 +1,184 @@
package ca.uhn.fhir.jpa.mdm.helper.testmodels;
import ca.uhn.fhir.jpa.model.entity.PartitionablePartitionId;
import ca.uhn.fhir.mdm.api.IMdmLink;
import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
import java.util.Date;
public class TestMdmLink implements IMdmLink<StringResourceId> {
private StringResourceId myId;
private StringResourceId myGoldenResourceId;
private StringResourceId mySourceResourceId;
private MdmMatchResultEnum myMatchResultEnum;
private MdmLinkSourceEnum myLinkSourceEnum;
@Override
public StringResourceId getId() {
return myId;
}
@Override
public IMdmLink<StringResourceId> setId(StringResourceId theId) {
myId = theId;
return this;
}
@Override
public StringResourceId getGoldenResourcePersistenceId() {
return myGoldenResourceId;
}
@Override
public IMdmLink<StringResourceId> setGoldenResourcePersistenceId(StringResourceId theGoldenResourcePid) {
myGoldenResourceId = theGoldenResourcePid;
return this;
}
@Override
public StringResourceId getSourcePersistenceId() {
return mySourceResourceId;
}
@Override
public IMdmLink<StringResourceId> setSourcePersistenceId(StringResourceId theSourcePid) {
mySourceResourceId = theSourcePid;
return this;
}
@Override
public MdmMatchResultEnum getMatchResult() {
return myMatchResultEnum;
}
@Override
public IMdmLink<StringResourceId> setMatchResult(MdmMatchResultEnum theMatchResult) {
myMatchResultEnum = theMatchResult;
return this;
}
@Override
public MdmLinkSourceEnum getLinkSource() {
return myLinkSourceEnum;
}
@Override
public IMdmLink<StringResourceId> setLinkSource(MdmLinkSourceEnum theLinkSource) {
myLinkSourceEnum = theLinkSource;
return this;
}
@Override
public Date getCreated() {
return new Date();
}
@Override
public IMdmLink<StringResourceId> setCreated(Date theCreated) {
// unneeded
return this;
}
@Override
public Date getUpdated() {
return new Date();
}
@Override
public IMdmLink<StringResourceId> setUpdated(Date theUpdated) {
// unneeded
return this;
}
@Override
public String getVersion() {
return null;
}
@Override
public IMdmLink<StringResourceId> setVersion(String theVersion) {
// unneeded
return this;
}
@Override
public Boolean getEidMatch() {
return null;
}
@Override
public Boolean isEidMatchPresent() {
return null;
}
@Override
public IMdmLink<StringResourceId> setEidMatch(Boolean theEidMatch) {
return this;
}
@Override
public Boolean getHadToCreateNewGoldenResource() {
return null;
}
@Override
public IMdmLink<StringResourceId> setHadToCreateNewGoldenResource(Boolean theHadToCreateNewGoldenResource) {
return this;
}
@Override
public Long getVector() {
return null;
}
@Override
public IMdmLink<StringResourceId> setVector(Long theVector) {
return this;
}
@Override
public Double getScore() {
return null;
}
@Override
public IMdmLink<StringResourceId> setScore(Double theScore) {
return this;
}
@Override
public Long getRuleCount() {
return null;
}
@Override
public IMdmLink<StringResourceId> setRuleCount(Long theRuleCount) {
return this;
}
@Override
public String getMdmSourceType() {
return null;
}
@Override
public IMdmLink<StringResourceId> setMdmSourceType(String theMdmSourceType) {
return this;
}
@Override
public void setPartitionId(PartitionablePartitionId thePartitionablePartitionId) {
}
@Override
public PartitionablePartitionId getPartitionId() {
return null;
}
}

View File

@ -0,0 +1,139 @@
package ca.uhn.fhir.jpa.mdm.svc.candidate;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.interceptor.model.RequestPartitionId;
import ca.uhn.fhir.jpa.api.svc.IIdHelperService;
import ca.uhn.fhir.jpa.mdm.dao.MdmLinkDaoSvc;
import ca.uhn.fhir.jpa.mdm.helper.testmodels.StringResourceId;
import ca.uhn.fhir.jpa.mdm.helper.testmodels.TestMdmLink;
import ca.uhn.fhir.mdm.api.IMdmLink;
import ca.uhn.fhir.mdm.api.IMdmMatchFinderSvc;
import ca.uhn.fhir.mdm.api.MatchedTarget;
import ca.uhn.fhir.mdm.api.MdmMatchOutcome;
import ca.uhn.fhir.mdm.util.MdmPartitionHelper;
import org.hl7.fhir.r4.model.Patient;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class FindCandidateByExampleSvcTest {
@Spy
private FhirContext myFhirContext = FhirContext.forR4Cached();
@Mock
private IIdHelperService<StringResourceId> myIdHelperService;
@Mock
private MdmLinkDaoSvc<StringResourceId, IMdmLink<StringResourceId>> myMdmLinkDaoSvc;
@Mock
private MdmPartitionHelper myMdmPartitionHelper;
@Mock
private IMdmMatchFinderSvc myMdmMatchFinderSvc;
@InjectMocks
private FindCandidateByExampleSvc<StringResourceId> myFindCandidateByExampleSvc;
@Test
public void findMatchGoldenResourceCandidates_withNonLongIds_returnsList() {
// setup
int total = 3;
Patient patient = new Patient();
patient.setActive(true);
patient.addName()
.setFamily("Simpson")
.addGiven("Homer");
List<MatchedTarget> matchCandidatesList = new ArrayList<>();
Map<String, Patient> targets = new HashMap<>();
for (int i = 0; i < total*2; i++) {
String id = "Test-" + (i % 3);
if (!targets.containsKey(id)) {
// the different names don't matter for the test
// but makes debugging easier
String name;
if (i == 0) {
name = "Bart";
} else if (i == 1) {
name = "Lisa";
} else {
name = "Maggie";
}
Patient p = new Patient();
p.setActive(true);
p.addName()
.setFamily("Simpson")
.addGiven(name);
targets.put(id, p);
}
matchCandidatesList.add(
new MatchedTarget(targets.get(id), MdmMatchOutcome.POSSIBLE_MATCH)
);
}
// when
when(myMdmPartitionHelper.getRequestPartitionIdFromResourceForSearch(any()))
.thenReturn(RequestPartitionId.allPartitions());
when(myIdHelperService.getPidOrNull(any(), any()))
.thenReturn(new StringResourceId("omit"));
when(myMdmLinkDaoSvc.getMdmLinksBySourcePidAndMatchResult(any(), any()))
.thenReturn(new ArrayList<>());
when(myMdmMatchFinderSvc.getMatchedTargets(anyString(), any(Patient.class), any()))
.thenReturn(matchCandidatesList);
// we don't care about the id we return here
// because we are going to mock the return value anyways
when(myIdHelperService.getPidOrNull(any(), any(Patient.class)))
.thenReturn(new StringResourceId("test"));
int[] count = new int[] { 0 };
when(myMdmLinkDaoSvc.getMatchedLinkForSourcePid(any(StringResourceId.class)))
.thenAnswer(args -> {
if (count[0] < total) {
TestMdmLink link = new TestMdmLink();
link.setSourcePersistenceId(args.getArgument(0));
if (count[0] % 2 == 0) {
link.setGoldenResourcePersistenceId(new StringResourceId("even"));
} else {
link.setGoldenResourcePersistenceId(new StringResourceId("odd"));
}
count[0]++;
return Optional.of(link);
}
return Optional.empty();
});
// test
List<MatchedGoldenResourceCandidate> goldenResourceCanddiates = myFindCandidateByExampleSvc.findMatchGoldenResourceCandidates(patient);
// verify
assertNotNull(goldenResourceCanddiates);
assertEquals(2, goldenResourceCanddiates.size());
Set<String> ids = new HashSet<>();
for (MatchedGoldenResourceCandidate r : goldenResourceCanddiates) {
// we know these are strings
assertTrue(ids.add((String)r.getCandidateGoldenResourcePid().getId()));
}
}
}

View File

@ -0,0 +1,157 @@
package ca.uhn.fhir.jpa.mdm.svc.candidate;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
import ca.uhn.fhir.jpa.entity.MdmLink;
import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
import ca.uhn.fhir.jpa.mdm.dao.MdmLinkDaoSvc;
import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import org.hl7.fhir.r4.model.Patient;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.TestPropertySource;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(MockitoExtension.class)
@TestPropertySource(properties = {
"module.mdm.config.script.file=classpath:mdm/mdm-rules-john-doe.json"
})
class MdmGoldenResourceFindingSvcIT extends BaseMdmR4Test {
@Autowired
MdmGoldenResourceFindingSvc myMdmGoldenResourceFindingSvc = new MdmGoldenResourceFindingSvc();
@Autowired
MdmLinkDaoSvc myMdmLinkDaoSvc;
@Test
public void testNoMatchCandidatesSkipped() {
// setup
Patient jane = createPatientAndUpdateLinks(addExternalEID(buildJanePatient(), EID_1));
// hack the link into a NO_MATCH
List<MdmLink> links = (List<MdmLink>) myMdmLinkDaoSvc.findMdmLinksBySourceResource(jane);
assertThat(links, hasSize(1));
MdmLink link = links.get(0);
link.setMatchResult(MdmMatchResultEnum.NO_MATCH);
link.setLinkSource(MdmLinkSourceEnum.MANUAL);
myMdmLinkDaoSvc.save(link);
// the NO_MATCH golden resource should not be a candidate
CandidateList candidateList = myMdmGoldenResourceFindingSvc.findGoldenResourceCandidates(jane);
assertEquals(0, candidateList.size());
}
@Test
public void findMdmLinksBySourceResource_withMatchingResources_doesNotReturnDuplicates() {
// setup
// create a bunch of patients that match
// (according to the rules in mdm-rules-john-doe.json)
// patient 1
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jho",
"given": [ "Doe"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 2
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhyee",
"given": [ "Deeon"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 3
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhoye",
"given": [ "Deo"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 4
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhee",
"given": [ "Deo"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 5
Patient candidate;
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhee",
"given": [ "Doe"]
} ],
"birthDate": "1974-12-25"
}
""";
candidate = createPatientFromJsonString(patientStr, true);
}
// test
CandidateList candidateList = myMdmGoldenResourceFindingSvc.findGoldenResourceCandidates(candidate);
// verify
assertNotNull(candidateList);
Set<Long> ids = new HashSet<>();
for (MatchedGoldenResourceCandidate c : candidateList.getCandidates()) {
assertTrue(ids.add((Long) c.getCandidateGoldenResourcePid().getId()));
}
}
private Patient createPatientFromJsonString(String theStr, boolean theCreateGolden) {
Patient patient = (Patient) myFhirContext.newJsonParser().parseResource(theStr);
DaoMethodOutcome daoOutcome = myPatientDao.create(
patient,
new SystemRequestDetails()
);
if (theCreateGolden) {
myMdmMatchLinkSvc.updateMdmLinksForMdmSource(patient, createContextForCreate("Patient"));
}
return (Patient) daoOutcome.getResource();
}
}

View File

@ -1,163 +0,0 @@
package ca.uhn.fhir.jpa.mdm.svc.candidate;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
import ca.uhn.fhir.jpa.entity.MdmLink;
import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test;
import ca.uhn.fhir.jpa.mdm.dao.MdmLinkDaoSvc;
import ca.uhn.fhir.mdm.api.IMdmSettings;
import ca.uhn.fhir.mdm.api.MdmLinkSourceEnum;
import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
import ca.uhn.fhir.mdm.rules.json.MdmRulesJson;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.test.utilities.UnregisterScheduledProcessor;
import org.hl7.fhir.r4.model.Patient;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestPropertySource;
import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(MockitoExtension.class)
@TestPropertySource(properties = {
"module.mdm.config.script.file=classpath:mdm/mdm-rules-john-doe.json"
})
class MdmGoldenResourceFindingSvcTest extends BaseMdmR4Test {
@Autowired
MdmGoldenResourceFindingSvc myMdmGoldenResourceFindingSvc = new MdmGoldenResourceFindingSvc();
@Autowired
MdmLinkDaoSvc myMdmLinkDaoSvc;
@Test
public void testNoMatchCandidatesSkipped() {
// setup
Patient jane = createPatientAndUpdateLinks(addExternalEID(buildJanePatient(), EID_1));
// hack the link into a NO_MATCH
List<MdmLink> links = (List<MdmLink>) myMdmLinkDaoSvc.findMdmLinksBySourceResource(jane);
assertThat(links, hasSize(1));
MdmLink link = links.get(0);
link.setMatchResult(MdmMatchResultEnum.NO_MATCH);
link.setLinkSource(MdmLinkSourceEnum.MANUAL);
myMdmLinkDaoSvc.save(link);
// the NO_MATCH golden resource should not be a candidate
CandidateList candidateList = myMdmGoldenResourceFindingSvc.findGoldenResourceCandidates(jane);
assertEquals(0, candidateList.size());
}
@Test
public void findMdmLinksBySourceResource_withMatchingResources_doesNotReturnDuplicates() throws IOException {
// setup
// create a bunch of patients that match
// (according to the rules in mdm-rules-john-doe.json)
// patient 1
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jho",
"given": [ "Doe"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 2
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhyee",
"given": [ "Deeon"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 3
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhoye",
"given": [ "Deo"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 4
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhee",
"given": [ "Deo"]
} ],
"birthDate": "1974-12-25"
}
""";
createPatientFromJsonString(patientStr, true);
}
// patient 5
Patient candidate;
{
String patientStr = """
{
"resourceType": "Patient",
"name": [ {
"family": "Jhee",
"given": [ "Doe"]
} ],
"birthDate": "1974-12-25"
}
""";
candidate = createPatientFromJsonString(patientStr, true);
}
// test
CandidateList candidateList = myMdmGoldenResourceFindingSvc.findGoldenResourceCandidates(candidate);
// verify
assertNotNull(candidateList);
Set<Long> ids = new HashSet<>();
for (MatchedGoldenResourceCandidate c : candidateList.getCandidates()) {
assertTrue(ids.add((Long)c.getCandidateGoldenResourcePid().getId()));
}
}
private Patient createPatientFromJsonString(String theStr, boolean theCreateGolden) {
Patient patient = (Patient) myFhirContext.newJsonParser().parseResource(theStr);
DaoMethodOutcome daoOutcome = myPatientDao.create(patient, new SystemRequestDetails());
if (theCreateGolden) {
myMdmMatchLinkSvc.updateMdmLinksForMdmSource(patient, createContextForCreate("Patient"));
}
return (Patient) daoOutcome.getResource();
}
}

View File

@ -66,6 +66,9 @@ public class PointcutLatchSession {
List<HookParams> awaitExpectedWithTimeout(int theTimeoutSecond) throws InterruptedException { List<HookParams> awaitExpectedWithTimeout(int theTimeoutSecond) throws InterruptedException {
if (!myCountdownLatch.await(theTimeoutSecond, TimeUnit.SECONDS)) { if (!myCountdownLatch.await(theTimeoutSecond, TimeUnit.SECONDS)) {
if (!myFailures.isEmpty()) {
ourLog.error(String.join(",", myFailures));
}
throw new LatchTimedOutError(Msg.code(1483) + myName + " timed out waiting " + theTimeoutSecond + " seconds for latch to countdown from " + myInitialCount + " to 0. Is " + myCountdownLatch.getCount() + "."); throw new LatchTimedOutError(Msg.code(1483) + myName + " timed out waiting " + theTimeoutSecond + " seconds for latch to countdown from " + myInitialCount + " to 0. Is " + myCountdownLatch.getCount() + ".");
} }