Support diff between patch sets

Pass through arguments for diff between patch sets

Modify internal APIs to pass through the old, new and preferences
needed to compute the differences between two patch sets for the
UI.

Bug: issue 194
Change-Id: I98827bf88227e912860769f22cd90f5c35b784b0

gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.common.data;
 
 import com.google.gerrit.common.auth.SignInRequired;
+import com.google.gerrit.reviewdb.AccountDiffPreference;
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gwt.user.client.rpc.AsyncCallback;
@@ -28,7 +29,8 @@ public interface ChangeDetailService extends RemoteJsonService {
 
   void includedInDetail(Change.Id id, AsyncCallback<IncludedInDetail> callback);
 
-  void patchSetDetail(PatchSet.Id key, AsyncCallback<PatchSetDetail> callback);
+  void patchSetDetail(PatchSet.Id keyA, PatchSet.Id keyB,
+      AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback);
 
   @SignInRequired
   void patchSetPublishDetail(PatchSet.Id key,

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java

@@ -503,7 +503,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
   @Override
   public void onOpen(final OpenEvent<DisclosurePanel> event) {
     if (infoTable == null) {
-      Util.DETAIL_SVC.patchSetDetail(patchSet.getId(),
+      Util.DETAIL_SVC.patchSetDetail(patchSet.getId(), null, null,
           new GerritCallback<PatchSetDetail>() {
             public void onSuccess(final PatchSetDetail result) {
               ensureLoaded(result);

gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java

@@ -324,7 +324,7 @@ public abstract class PatchScreen extends Screen implements
   protected void onLoad() {
     super.onLoad();
     if (patchSetDetail == null) {
-      Util.DETAIL_SVC.patchSetDetail(idSideB,
+      Util.DETAIL_SVC.patchSetDetail(idSideB, null, null,
           new GerritCallback<PatchSetDetail>() {
             @Override
             public void onSuccess(PatchSetDetail result) {
@@ -407,7 +407,7 @@ public abstract class PatchScreen extends Screen implements
       commitMessageBlock.display(patchSetDetail.getInfo().getMessage());
     } else {
       commitMessageBlock.setVisible(false);
-      Util.DETAIL_SVC.patchSetDetail(idSideB,
+      Util.DETAIL_SVC.patchSetDetail(idSideB, null, null,
           new GerritCallback<PatchSetDetail>() {
             @Override
             public void onSuccess(PatchSetDetail result) {
@@ -501,7 +501,7 @@ public abstract class PatchScreen extends Screen implements
         final PatchSet.Id psid = patchKey.getParentKey();
         fileList = new PatchTable(prefs);
         fileList.setSavePointerId("PatchTable " + psid);
-        Util.DETAIL_SVC.patchSetDetail(psid,
+        Util.DETAIL_SVC.patchSetDetail(psid, null, null,
             new GerritCallback<PatchSetDetail>() {
               public void onSuccess(final PatchSetDetail result) {
                 fileList.display(result);

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java

@@ -190,7 +190,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
       NoSuchEntityException, PatchSetInfoNotAvailableException,
       NoSuchChangeException {
     final PatchSet.Id psId = detail.getChange().currentPatchSetId();
-    final PatchSetDetailFactory loader = patchSetDetail.create(psId);
+    final PatchSetDetailFactory loader = patchSetDetail.create(psId, null, null);
     loader.patchSet = detail.getCurrentPatchSet();
     loader.control = control;
     detail.setCurrentPatchSetDetail(loader.call());

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java

@@ -19,6 +19,7 @@ import com.google.gerrit.common.data.ChangeDetailService;
 import com.google.gerrit.common.data.IncludedInDetail;
 import com.google.gerrit.common.data.PatchSetDetail;
 import com.google.gerrit.common.data.PatchSetPublishDetail;
+import com.google.gerrit.reviewdb.AccountDiffPreference;
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gwt.user.client.rpc.AsyncCallback;
@@ -51,9 +52,10 @@ class ChangeDetailServiceImpl implements ChangeDetailService {
     includedInDetail.create(id).to(callback);
   }
 
-  public void patchSetDetail(final PatchSet.Id id,
+  public void patchSetDetail(final PatchSet.Id idA, final PatchSet.Id idB,
+      final AccountDiffPreference diffPrefs,
       final AsyncCallback<PatchSetDetail> callback) {
-    patchSetDetail.create(id).to(callback);
+    patchSetDetail.create(idA, idB, diffPrefs).to(callback);
   }
 
   public void patchSetPublishDetail(final PatchSet.Id id,

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java

@@ -18,15 +18,20 @@ import com.google.gerrit.common.data.PatchSetDetail;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.reviewdb.AccountDiffPreference;
 import com.google.gerrit.reviewdb.AccountPatchReview;
 import com.google.gerrit.reviewdb.Patch;
 import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
@@ -35,54 +40,92 @@ import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.Nullable;
+
 /** Creates a {@link PatchSetDetail} from a {@link PatchSet}. */
 class PatchSetDetailFactory extends Handler<PatchSetDetail> {
+
+  private static final Logger log =
+    LoggerFactory.getLogger(PatchSetDetailFactory.class);
+
   interface Factory {
-    PatchSetDetailFactory create(PatchSet.Id id);
+    PatchSetDetailFactory create(@Assisted("psIdNew") PatchSet.Id psIdA,
+        @Assisted("psIdOld") PatchSet.Id psIdB, AccountDiffPreference diffPrefs);
   }
 
+  private final GitRepositoryManager repoManager;
+
   private final PatchSetInfoFactory infoFactory;
   private final ReviewDb db;
   private final PatchListCache patchListCache;
   private final ChangeControl.Factory changeControlFactory;
 
-  private final PatchSet.Id psId;
+  private Project.NameKey projectKey;
+  private final PatchSet.Id psIdNew;
+  private final PatchSet.Id psIdOld;
+  private final AccountDiffPreference diffPrefs;
+  private ObjectId oldId;
+  private ObjectId newId;
 
   private PatchSetDetail detail;
   ChangeControl control;
   PatchSet patchSet;
 
   @Inject
-  PatchSetDetailFactory(final PatchSetInfoFactory psif, final ReviewDb db,
+  PatchSetDetailFactory(final GitRepositoryManager grm,
+      final PatchSetInfoFactory psif, final ReviewDb db,
       final PatchListCache patchListCache,
       final ChangeControl.Factory changeControlFactory,
-      @Assisted final PatchSet.Id id) {
+      @Assisted("psIdNew") final PatchSet.Id psIdNew,
+      @Assisted("psIdOld") @Nullable final PatchSet.Id psIdOld,
+      @Assisted @Nullable final AccountDiffPreference diffPrefs) {
+    this.repoManager = grm;
     this.infoFactory = psif;
     this.db = db;
     this.patchListCache = patchListCache;
     this.changeControlFactory = changeControlFactory;
 
-    this.psId = id;
+    this.psIdNew = psIdNew;
+    this.psIdOld = psIdOld;
+    this.diffPrefs = diffPrefs;
   }
 
+  @SuppressWarnings("deprecation")
   @Override
   public PatchSetDetail call() throws OrmException, NoSuchEntityException,
       PatchSetInfoNotAvailableException, NoSuchChangeException {
     if (control == null || patchSet == null) {
-      control = changeControlFactory.validateFor(psId.getParentKey());
+      control = changeControlFactory.validateFor(psIdNew.getParentKey());
-      patchSet = db.patchSets().get(psId);
+      patchSet = db.patchSets().get(psIdNew);
       if (patchSet == null) {
         throw new NoSuchEntityException();
       }
     }
 
-    final PatchList list = patchListCache.get(control.getChange(), patchSet);
+    final PatchList list;
-    if (list == null) {
+
-      throw new NoSuchEntityException();
+    if (psIdOld != null) {
+      newId = toObjectId(psIdNew);
+      oldId = psIdOld != null ? toObjectId(psIdOld) : null;
+
+      projectKey = control.getProject().getNameKey();
+
+      list = listFor(keyFor(diffPrefs.getIgnoreWhitespace()));
+    } else { // OK, means use base to compare
+      list = patchListCache.get(control.getChange(), patchSet);
+      if (list == null) {
+        throw new NoSuchEntityException();
+      }
     }
 
     final List<Patch> patches = list.toPatchList(patchSet.getId());
@@ -91,7 +134,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
       byKey.put(p.getKey(), p);
     }
 
-    for (final PatchLineComment c : db.patchComments().published(psId)) {
+    for (final PatchLineComment c : db.patchComments().published(psIdNew)) {
       final Patch p = byKey.get(c.getKey().getParentKey());
       if (p != null) {
         p.setCommentCount(p.getCommentCount() + 1);
@@ -101,7 +144,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
     detail = new PatchSetDetail();
     detail.setPatchSet(patchSet);
 
-    detail.setInfo(infoFactory.get(psId));
+    detail.setInfo(infoFactory.get(psIdNew));
     detail.setPatches(patches);
 
     final CurrentUser user = control.getCurrentUser();
@@ -111,14 +154,14 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
       // quickly locate where they have pending drafts, and review them.
       //
       final Account.Id me = ((IdentifiedUser) user).getAccountId();
-      for (final PatchLineComment c : db.patchComments().draft(psId, me)) {
+      for (final PatchLineComment c : db.patchComments().draft(psIdNew, me)) {
         final Patch p = byKey.get(c.getKey().getParentKey());
         if (p != null) {
           p.setDraftCount(p.getDraftCount() + 1);
         }
       }
 
-      for (AccountPatchReview r : db.accountPatchReviews().byReviewer(me, psId)) {
+      for (AccountPatchReview r : db.accountPatchReviews().byReviewer(me, psIdNew)) {
         final Patch p = byKey.get(r.getKey().getPatchKey());
         if (p != null) {
           p.setReviewedByCurrentUser(true);
@@ -128,4 +171,27 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
 
     return detail;
   }
+
+  private ObjectId toObjectId(final PatchSet.Id psId) throws OrmException,
+      NoSuchEntityException {
+    final PatchSet ps = db.patchSets().get(psId);
+    if (ps == null) {
+      throw new NoSuchEntityException();
+    }
+
+    try {
+      return ObjectId.fromString(ps.getRevision().get());
+    } catch (IllegalArgumentException e) {
+      log.error("Patch set " + psId + " has invalid revision");
+      throw new NoSuchEntityException();
+    }
+  }
+
+  private PatchListKey keyFor(final Whitespace whitespace) {
+    return new PatchListKey(projectKey, oldId, newId, whitespace);
+  }
+
+  private PatchList listFor(final PatchListKey key) {
+    return patchListCache.get(key);
+  }
 }