From 67d3d4fbd1586d8b95293c3ca7439cca6a27345b Mon Sep 17 00:00:00 2001 From: veronikaslc Date: Wed, 13 Mar 2019 20:48:50 -0400 Subject: [PATCH 1/2] RM-129: RM should be updated after back-end API changes were introduced in PN-401 and PN-449 Done --- .../DefaultRemotePatientMatchResource.java | 25 +++++++++++-------- .../RemoteMatchedPatientClusterView.java | 8 ++++-- .../RemoteMatchedPatientClusterViewTest.java | 20 +++++++-------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/core/rest/src/main/java/org/phenotips/remote/rest/internal/DefaultRemotePatientMatchResource.java b/core/rest/src/main/java/org/phenotips/remote/rest/internal/DefaultRemotePatientMatchResource.java index 339763e..d2a1b29 100644 --- a/core/rest/src/main/java/org/phenotips/remote/rest/internal/DefaultRemotePatientMatchResource.java +++ b/core/rest/src/main/java/org/phenotips/remote/rest/internal/DefaultRemotePatientMatchResource.java @@ -20,19 +20,21 @@ import org.phenotips.data.Patient; import org.phenotips.data.PatientRepository; import org.phenotips.data.similarity.MatchedPatientClusterView; +import org.phenotips.data.similarity.PatientSimilarityView; +import org.phenotips.matchingnotification.match.PatientMatch; import org.phenotips.matchingnotification.storage.MatchStorageManager; import org.phenotips.remote.api.ApiConfiguration; import org.phenotips.remote.api.OutgoingMatchRequest; import org.phenotips.remote.client.RemoteMatchingService; import org.phenotips.remote.common.internal.RemotePatientSimilarityView; import org.phenotips.remote.rest.RemotePatientMatchResource; - import org.xwiki.component.annotation.Component; import org.xwiki.container.Container; import org.xwiki.container.Request; import org.xwiki.rest.XWikiResource; import java.util.List; +import java.util.Map; import javax.annotation.Nonnull; import javax.inject.Inject; @@ -176,7 +178,7 @@ private Response buildResponse( return Response.status(Response.Status.NOT_ACCEPTABLE).build(); } - return buildMatches(patient, remoteResponse, offset, limit, reqNo, newRequest); + return buildMatches(patient, remoteResponse, offset, limit, reqNo); } catch (final SecurityException e) { this.slf4Jlogger.error("Failed to retrieve patient with ID [{}]: {}", patientId, e.getMessage()); return Response.status(Response.Status.UNAUTHORIZED).build(); @@ -197,8 +199,6 @@ private Response buildResponse( * @param offset the offset for the returned matches * @param limit the maximum number of matches to return after the offset * @param reqNo the current request number - * @param saveMatchesToNotificationTable when true, the matches parsed from the mmeMatchRequest will - * be saved into the matching notification database * @return a {@link Response} containing the matched patients data */ private Response buildMatches( @@ -206,18 +206,21 @@ private Response buildMatches( @Nonnull final OutgoingMatchRequest mmeMatchRequest, final int offset, final int limit, - final int reqNo, - final boolean saveMatchesToNotificationTable) + final int reqNo) { final List matches = this.matchingService.getSimilarityResults(mmeMatchRequest); - if (saveMatchesToNotificationTable) { - this.matchStorageManager.saveRemoteMatches(matches, patient.getId(), - mmeMatchRequest.getRemoteServerId(), false); - } + // FIXME saveRemoteMatches() is the most reliable way to get PatientMatch ids corresponding to + // SimilarityViews. However this is inefficient (though with latest Pn code supposedly nothing will + // be written to the DB if matches do not change), and in the future we need to combine + // SimilarityView and PatientMatch and operate on a single entity, which would be obtained from + // a single table. + + final Map matchMapping = this.matchStorageManager.saveRemoteMatches( + matches, patient.getId(), mmeMatchRequest.getRemoteServerId(), false); final MatchedPatientClusterView matchedCluster = - new RemoteMatchedPatientClusterView(patient, mmeMatchRequest, matches); + new RemoteMatchedPatientClusterView(patient, mmeMatchRequest, matches, matchMapping); final JSONObject matchesJson = matchedCluster.toJSON(offset - 1, limit); matchesJson.put(REQ_NO, reqNo); diff --git a/core/rest/src/main/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterView.java b/core/rest/src/main/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterView.java index a5bd767..9df9a88 100644 --- a/core/rest/src/main/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterView.java +++ b/core/rest/src/main/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterView.java @@ -21,10 +21,12 @@ import org.phenotips.data.similarity.MatchedPatientClusterView; import org.phenotips.data.similarity.PatientSimilarityView; import org.phenotips.data.similarity.internal.DefaultMatchedPatientClusterView; +import org.phenotips.matchingnotification.match.PatientMatch; import org.phenotips.remote.api.OutgoingMatchRequest; import org.phenotips.remote.common.internal.RemotePatientSimilarityView; import java.util.List; +import java.util.Map; import java.util.Objects; import javax.annotation.Nonnull; @@ -52,13 +54,15 @@ public class RemoteMatchedPatientClusterView extends DefaultMatchedPatientCluste * @param patient the local reference {@code patient} * @param response the response from the remote server, as {@link OutgoingMatchRequest} * @param remoteMatches a list of {@link PatientSimilarityView} objects representing remote matching patients + * @param matchesIds a map of {@link PatientSimilarityView} objects to the IDs of corresponding matches saved in DB */ public RemoteMatchedPatientClusterView( @Nonnull final Patient patient, @Nonnull final OutgoingMatchRequest mmeMatchRequest, - @Nullable final List remoteMatches) + @Nullable final List remoteMatches, + @Nullable final Map matchesIds) { - super(patient, remoteMatches); + super(patient, remoteMatches, matchesIds); Validate.notNull(mmeMatchRequest, "The remote response must not be null."); this.mmeMatchRequest = mmeMatchRequest; diff --git a/core/rest/src/test/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterViewTest.java b/core/rest/src/test/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterViewTest.java index 1e06bbd..21a98b6 100644 --- a/core/rest/src/test/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterViewTest.java +++ b/core/rest/src/test/java/org/phenotips/remote/rest/internal/RemoteMatchedPatientClusterViewTest.java @@ -147,19 +147,19 @@ public void setUp() when(this.response.getResponseJSON()).thenReturn(new JSONObject().put(RESPONSE_JSON, RESPONSE_JSON)); this.matchList = Arrays.asList(this.patient1, this.patient2, this.patient3, this.patient4, this.patient5); - this.matches = new RemoteMatchedPatientClusterView(this.reference, this.response, this.matchList); + this.matches = new RemoteMatchedPatientClusterView(this.reference, this.response, this.matchList, null); } @Test(expected = NullPointerException.class) public void instantiatingClassWithNullPatientThrowsException() { - new RemoteMatchedPatientClusterView(null, this.response, this.matchList); + new RemoteMatchedPatientClusterView(null, this.response, this.matchList, null); } @Test(expected = NullPointerException.class) public void instantiatingClassWithNullResponseThrowsException() { - new RemoteMatchedPatientClusterView(this.reference, null, this.matchList); + new RemoteMatchedPatientClusterView(this.reference, null, this.matchList, null); } @Test @@ -172,7 +172,7 @@ public void getReferenceReturnsTheReferenceThatWasSet() public void getMatchesReturnsEmptyListIfNoMatchesSet() { final MatchedPatientClusterView matches = new RemoteMatchedPatientClusterView(this.reference, this.response, - Collections.emptyList()); + Collections.emptyList(), null); Assert.assertTrue(matches.getMatches().isEmpty()); } @@ -192,7 +192,7 @@ public void getMatchesReturnedMatchesCannotBeModified() public void sizeIsZeroIfMatchesIsEmpty() { final MatchedPatientClusterView matches = new RemoteMatchedPatientClusterView(this.reference, this.response, - Collections.emptyList()); + Collections.emptyList(), null); Assert.assertEquals(0, matches.size()); } @@ -266,7 +266,7 @@ public void toJSONGetsAllDataIfNoIndicesProvided() public void equalsReturnsTrueForTwoDifferentObjectsWithSameData() { final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, this.response, - this.matchList); + this.matchList, null); Assert.assertTrue(v2.equals(this.matches)); } @@ -281,7 +281,7 @@ public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentResponses() { final OutgoingMatchRequest response2 = mock(OutgoingMatchRequest.class); final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, response2, - this.matchList); + this.matchList, null); Assert.assertFalse(v2.equals(this.matches)); } @@ -289,7 +289,7 @@ public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentResponses() public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentReference() { final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(mock(Patient.class), this.response, - this.matchList); + this.matchList, null); Assert.assertFalse(v2.equals(this.matches)); } @@ -297,7 +297,7 @@ public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentReference() public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentMatchList() { final List m2 = Arrays.asList(this.patient1, this.patient2, this.patient3); - final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, this.response, m2); + final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, this.response, m2, null); Assert.assertFalse(v2.equals(this.matches)); } @@ -305,7 +305,7 @@ public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentMatchList() public void hashCodeIsTheSameForTwoObjectsWithSameData() { final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, this.response, - this.matchList); + this.matchList, null); Assert.assertEquals(v2.hashCode(), this.matches.hashCode()); } } From 8bc642767fcb577c3a875b66270abd6b88873d73 Mon Sep 17 00:00:00 2001 From: Andrew Misyura Date: Tue, 30 Apr 2019 15:07:48 -0400 Subject: [PATCH 2/2] RM-129: RM should be updated after back-end API changes were introduced in PN-401 and PN-449 Updated code after a class was renamed for PN-453 (part of PN-401) --- .../phenotips/remote/client/internal/RemoteMatchFinder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/client/src/main/java/org/phenotips/remote/client/internal/RemoteMatchFinder.java b/core/client/src/main/java/org/phenotips/remote/client/internal/RemoteMatchFinder.java index ced4005..45a5d0f 100644 --- a/core/client/src/main/java/org/phenotips/remote/client/internal/RemoteMatchFinder.java +++ b/core/client/src/main/java/org/phenotips/remote/client/internal/RemoteMatchFinder.java @@ -22,7 +22,7 @@ import org.phenotips.matchingnotification.finder.MatchFinder; import org.phenotips.matchingnotification.finder.internal.AbstractMatchFinder; import org.phenotips.matchingnotification.match.PatientMatch; -import org.phenotips.matchingnotification.match.internal.DefaultPatientMatch; +import org.phenotips.matchingnotification.match.internal.CurrentPatientMatch; import org.phenotips.remote.api.OutgoingMatchRequest; import org.phenotips.remote.client.RemoteMatchingService; import org.phenotips.remote.common.ApplicationConfiguration; @@ -108,7 +108,7 @@ protected MatchRunStatus specificFindMatches(Patient patient, String remoteId, L List parsedResults = this.matchingService.getSimilarityResults(request); for (RemotePatientSimilarityView result : parsedResults) { - PatientMatch match = new DefaultPatientMatch(result, null, remoteId); + PatientMatch match = new CurrentPatientMatch(result, null, remoteId); matchesList.add(match); } return MatchRunStatus.OK;