Merge pull request #95 from phenotips/RM-129

RM-129: RM should be updated after back-end API changes were introduced in PN-401 and PN-449

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<RemotePatientSimilarityView> 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;

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,27 +199,28 @@ 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(
         @Nonnull final Patient patient,
         @Nonnull final OutgoingMatchRequest mmeMatchRequest,
         final int offset,
         final int limit,
-        final int reqNo,
-        final boolean saveMatchesToNotificationTable)
+        final int reqNo)
     {
         final List<RemotePatientSimilarityView> 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<PatientSimilarityView, PatientMatch> 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);

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<RemotePatientSimilarityView> remoteMatches)
+        @Nullable final List<RemotePatientSimilarityView> remoteMatches,
+        @Nullable final Map<PatientSimilarityView, PatientMatch> matchesIds)
     {
-        super(patient, remoteMatches);
+        super(patient, remoteMatches, matchesIds);
 
         Validate.notNull(mmeMatchRequest, "The remote response must not be null.");
         this.mmeMatchRequest = mmeMatchRequest;

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.<RemotePatientSimilarityView>emptyList());
+            Collections.<RemotePatientSimilarityView>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.<RemotePatientSimilarityView>emptyList());
+            Collections.<RemotePatientSimilarityView>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,31 +281,31 @@ 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));
     }
 
     @Test
     public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentReference()
     {
         final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(mock(Patient.class), this.response,
-            this.matchList);
+            this.matchList, null);
         Assert.assertFalse(v2.equals(this.matches));
     }
 
     @Test
     public void equalsReturnsFalseForTwoDifferentObjectsWithDifferentMatchList()
     {
         final List<RemotePatientSimilarityView> 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));
     }
 
     @Test
     public void hashCodeIsTheSameForTwoObjectsWithSameData()
     {
         final MatchedPatientClusterView v2 = new RemoteMatchedPatientClusterView(this.reference, this.response,
-            this.matchList);
+            this.matchList, null);
         Assert.assertEquals(v2.hashCode(), this.matches.hashCode());
     }
 }