Remove old patchSetPublishDetail RPC

Change-Id: I776761163f747117fb94e4c3fd57dde77de2d31e

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

@@ -15,7 +15,6 @@
 package com.google.gerrit.common.data;
 
 import com.google.gerrit.common.audit.Audit;
-import com.google.gerrit.common.auth.SignInRequired;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwtjsonrpc.common.AsyncCallback;
@@ -31,8 +30,4 @@ public interface ChangeDetailService extends RemoteJsonService {
   @Audit
   void patchSetDetail2(PatchSet.Id baseId, PatchSet.Id key,
       AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback);
-
-  @SignInRequired
-  void patchSetPublishDetail(PatchSet.Id key,
-      AsyncCallback<PatchSetPublishDetail> callback);
 }

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

@@ -23,6 +23,8 @@ import com.google.gerrit.client.patches.CommentEditorPanel;
 import com.google.gerrit.client.projects.ConfigInfoCache;
 import com.google.gerrit.client.rpc.CallbackGroup;
 import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.rpc.NativeMap;
+import com.google.gerrit.client.rpc.NativeString;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
@@ -31,12 +33,16 @@ import com.google.gerrit.client.ui.CommentLinkProcessor;
 import com.google.gerrit.client.ui.PatchLink;
 import com.google.gerrit.client.ui.SmallHeading;
 import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.common.changes.ListChangesOption;
+import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.common.data.ChangeDetail;
-import com.google.gerrit.common.data.PatchSetPublishDetail;
+import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gwt.core.client.JsArray;
 import com.google.gwt.core.client.JsArrayString;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
@@ -57,6 +63,7 @@ import com.google.gwtjsonrpc.common.VoidResult;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -66,7 +73,6 @@ public class PublishCommentScreen extends AccountScreen implements
   private static SavedState lastState;
 
   private final PatchSet.Id patchSetId;
-  private String revision;
   private Collection<ValueRadioButton> approvalButtons;
   private ChangeDescriptionBlock descBlock;
   private ApprovalTable approvals;
@@ -79,6 +85,9 @@ public class PublishCommentScreen extends AccountScreen implements
   private boolean saveStateOnUnload = true;
   private List<CommentEditorPanel> commentEditors;
   private ChangeInfo change;
+  private ChangeInfo detail;
+  private NativeMap<JsArray<CommentInfo>> drafts;
+  private SubmitTypeRecord submitTypeRecord;
   private CommentLinkProcessor commentLinkProcessor;
 
   public PublishCommentScreen(final PatchSet.Id psi) {
@@ -149,50 +158,63 @@ public class PublishCommentScreen extends AccountScreen implements
   protected void onLoad() {
     super.onLoad();
 
-    CallbackGroup cbs = new CallbackGroup();
+    CallbackGroup group = new CallbackGroup();
+    RestApi call = ChangeApi.detail(patchSetId.getParentKey().get());
+    ChangeList.addOptions(call, EnumSet.of(
+      ListChangesOption.CURRENT_ACTIONS,
+      ListChangesOption.ALL_REVISIONS,
+      ListChangesOption.ALL_COMMITS));
+    call.get(group.add(new GerritCallback<ChangeInfo>() {
+        @Override
+        public void onSuccess(ChangeInfo result) {
+          detail = result;
+        }
+      }));
+    ChangeApi.revision(patchSetId)
+      .view("submit_type")
+      .get(group.add(new GerritCallback<NativeString>() {
+        @Override
+        public void onSuccess(NativeString result) {
+          submitTypeRecord = SubmitTypeRecord.OK(
+              Project.SubmitType.valueOf(result.asString()));
+        }
+        public void onFailure(Throwable caught) {}
+      }));
+    ChangeApi.revision(patchSetId.getParentKey().get(), "" + patchSetId.get())
+      .view("drafts")
+      .get(group.add(new AsyncCallback<NativeMap<JsArray<CommentInfo>>>() {
+        @Override
+        public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
+          drafts = result;
+        }
+        public void onFailure(Throwable caught) {}
+      }));
     ChangeApi.revision(patchSetId).view("review")
-        .get(cbs.add(new AsyncCallback<ChangeInfo>() {
-          @Override
-          public void onSuccess(ChangeInfo result) {
-            result.init();
-            change = result;
-          }
+      .get(group.addFinal(new GerritCallback<ChangeInfo>() {
+        @Override
+        public void onSuccess(ChangeInfo result) {
+          result.init();
+          change = result;
+          preDisplay(result);
+        }
+      }));
+  }
 
+  private void preDisplay(final ChangeInfo info) {
+    ConfigInfoCache.get(info.project_name_key(),
+        new ScreenLoadCallback<ConfigInfoCache.Entry>(this) {
           @Override
-          public void onFailure(Throwable caught) {
-            // Handled by ScreenLoadCallback.onFailure().
-          }
-        }));
-    Util.DETAIL_SVC.patchSetPublishDetail(patchSetId, cbs.addFinal(
-        new ScreenLoadCallback<PatchSetPublishDetail>(this) {
-          @Override
-          protected void preDisplay(final PatchSetPublishDetail result) {
+          protected void preDisplay(ConfigInfoCache.Entry result) {
             send.setEnabled(true);
-            PublishCommentScreen.this.preDisplay(result, this);
+            commentLinkProcessor = result.getCommentLinkProcessor();
+            setTheme(result.getTheme());
+            displayScreen();
           }
 
           @Override
           protected void postDisplay() {
             message.setFocus(true);
           }
-        }));
-  }
-
-  private void preDisplay(final PatchSetPublishDetail pubDetail,
-      final ScreenLoadCallback<PatchSetPublishDetail> origCb) {
-    ConfigInfoCache.get(pubDetail.getChange().getProject(),
-        new AsyncCallback<ConfigInfoCache.Entry>() {
-          @Override
-          public void onSuccess(ConfigInfoCache.Entry result) {
-            commentLinkProcessor = result.getCommentLinkProcessor();
-            setTheme(result.getTheme());
-            display(pubDetail);
-          }
-
-          @Override
-          public void onFailure(Throwable caught) {
-            origCb.onFailure(caught);
-          }
         });
   }
 
@@ -317,14 +339,13 @@ public class PublishCommentScreen extends AccountScreen implements
     body.add(vp);
   }
 
-  private void display(final PatchSetPublishDetail r) {
-    ChangeDetail changeDetail = new ChangeDetail();
-    changeDetail.setChange(r.getChange());
+  private void displayScreen() {
+    ChangeDetail r = ChangeDetailCache.reverse(detail);
 
     setPageTitle(Util.M.publishComments(r.getChange().getKey().abbreviate(),
         patchSetId.get()));
-    descBlock.display(changeDetail, null, false, r.getPatchSetInfo(), r.getAccounts(),
-       r.getSubmitTypeRecord(), commentLinkProcessor);
+    descBlock.display(r, null, false, r.getCurrentPatchSetDetail().getInfo(),
+        r.getAccounts(), submitTypeRecord, commentLinkProcessor);
 
     if (r.getChange().getStatus().isOpen()) {
       initApprovals(approvalPanel);
@@ -339,14 +360,13 @@ public class PublishCommentScreen extends AccountScreen implements
 
     draftsPanel.clear();
     commentEditors = new ArrayList<CommentEditorPanel>();
-    revision = r.getPatchSetInfo().getRevId();
 
-    if (!r.getDrafts().isEmpty()) {
+    if (!drafts.isEmpty()) {
       draftsPanel.add(new SmallHeading(Util.C.headingPatchComments()));
 
       Panel panel = null;
       String priorFile = "";
-      for (final PatchLineComment c : r.getDrafts()) {
+      for (final PatchLineComment c : draftList()) {
         final Patch.Key patchKey = c.getKey().getParentKey();
         final String fn = patchKey.get();
         if (!fn.equals(priorFile)) {
@@ -375,7 +395,7 @@ public class PublishCommentScreen extends AccountScreen implements
       }
     }
 
-    submit.setVisible(r.canSubmit());
+    submit.setVisible(true/* TODO canSubmit? */);
   }
 
   private void onSend(final boolean submit) {
@@ -411,7 +431,7 @@ public class PublishCommentScreen extends AccountScreen implements
     enableForm(false);
     new RestApi("/changes/")
       .id(String.valueOf(patchSetId.getParentKey().get()))
-      .view("revisions").id(revision).view("review")
+      .view("revisions").id(patchSetId.get()).view("review")
       .post(data, new GerritCallback<ReviewInput>() {
           @Override
           public void onSuccess(ReviewInput result) {
@@ -432,7 +452,9 @@ public class PublishCommentScreen extends AccountScreen implements
   }
 
   private void submit() {
-    ChangeApi.submit(patchSetId.getParentKey().get(), revision,
+    ChangeApi.submit(
+      patchSetId.getParentKey().get(),
+      "" + patchSetId.get(),
       new GerritCallback<SubmitInfo>() {
           public void onSuccess(SubmitInfo result) {
             saveStateOnUnload = false;
@@ -456,6 +478,33 @@ public class PublishCommentScreen extends AccountScreen implements
     Gerrit.display(PageLinks.toChange(ck), new ChangeScreen(ck));
   }
 
+  private List<PatchLineComment> draftList() {
+    List<PatchLineComment> d = new ArrayList<PatchLineComment>();
+    List<String> paths = new ArrayList<String>(drafts.keySet());
+    Collections.sort(paths);
+    for (String path : paths) {
+      JsArray<CommentInfo> comments = drafts.get(path);
+      for (int i = 0; i < comments.length(); i++) {
+        d.add(toComment(path, comments.get(i)));
+      }
+    }
+    return d;
+  }
+
+  private PatchLineComment toComment(String path, CommentInfo i) {
+    PatchLineComment p = new PatchLineComment(
+        new PatchLineComment.Key(
+            new Patch.Key(patchSetId, path),
+            i.id()),
+        i.line(),
+        Gerrit.getUserAccount().getId(),
+        i.in_reply_to(),
+        i.updated());
+    p.setMessage(i.message());
+    p.setSide((short) (i.side() == Side.PARENT ? 0 : 1));
+    return p;
+  }
+
   private static class ValueRadioButton extends RadioButton {
     final LabelInfo label;
     final String value;

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

@@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.changedetail;
 
 import com.google.gerrit.common.data.ChangeDetailService;
 import com.google.gerrit.common.data.PatchSetDetail;
-import com.google.gerrit.common.data.PatchSetPublishDetail;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwtjsonrpc.common.AsyncCallback;
@@ -24,14 +23,11 @@ import com.google.inject.Inject;
 
 class ChangeDetailServiceImpl implements ChangeDetailService {
   private final PatchSetDetailFactory.Factory patchSetDetail;
-  private final PatchSetPublishDetailFactory.Factory patchSetPublishDetail;
 
   @Inject
   ChangeDetailServiceImpl(
-      final PatchSetDetailFactory.Factory patchSetDetail,
-      final PatchSetPublishDetailFactory.Factory patchSetPublishDetail) {
+      final PatchSetDetailFactory.Factory patchSetDetail) {
     this.patchSetDetail = patchSetDetail;
-    this.patchSetPublishDetail = patchSetPublishDetail;
   }
 
   public void patchSetDetail(PatchSet.Id id,
@@ -43,9 +39,4 @@ class ChangeDetailServiceImpl implements ChangeDetailService {
       AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback) {
     patchSetDetail.create(baseId, id, diffPrefs).to(callback);
   }
-
-  public void patchSetPublishDetail(final PatchSet.Id id,
-      final AsyncCallback<PatchSetPublishDetail> callback) {
-    patchSetPublishDetail.create(id).to(callback);
-  }
 }

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

@@ -29,7 +29,6 @@ public class ChangeModule extends RpcServletModule {
       @Override
       protected void configure() {
         factory(PatchSetDetailFactory.Factory.class);
-        factory(PatchSetPublishDetailFactory.Factory.class);
       }
     });
     rpc(ChangeDetailServiceImpl.class);

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

@@ -1,164 +0,0 @@
-// Copyright (C) 2009 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.httpd.rpc.changedetail;
-
-import com.google.gerrit.common.data.LabelType;
-import com.google.gerrit.common.data.PatchSetPublishDetail;
-import com.google.gerrit.common.data.PermissionRange;
-import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.httpd.rpc.Handler;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountInfoCacheFactory;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail> {
-  interface Factory {
-    PatchSetPublishDetailFactory create(PatchSet.Id patchSetId);
-  }
-
-  private final PatchSetInfoFactory infoFactory;
-  private final ReviewDb db;
-  private final ChangeControl.Factory changeControlFactory;
-  private final AccountInfoCacheFactory aic;
-  private final IdentifiedUser user;
-
-  private final PatchSet.Id patchSetId;
-
-  private PatchSetInfo patchSetInfo;
-  private Change change;
-  private List<PatchLineComment> drafts;
-
-  @Inject
-  PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory,
-      final ReviewDb db,
-      final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
-      final ChangeControl.Factory changeControlFactory,
-      final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
-    this.infoFactory = infoFactory;
-    this.db = db;
-    this.changeControlFactory = changeControlFactory;
-    this.aic = accountInfoCacheFactory.create();
-    this.user = user;
-
-    this.patchSetId = patchSetId;
-  }
-
-  @Override
-  public PatchSetPublishDetail call() throws OrmException,
-      PatchSetInfoNotAvailableException, NoSuchChangeException {
-    final Change.Id changeId = patchSetId.getParentKey();
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
-    change = control.getChange();
-    PatchSet patchSet = db.patchSets().get(patchSetId);
-    patchSetInfo = infoFactory.get(change, patchSet);
-    drafts = db.patchComments().draftByPatchSetAuthor(patchSetId, user.getAccountId()).toList();
-
-    aic.want(change.getOwner());
-
-    PatchSetPublishDetail detail = new PatchSetPublishDetail();
-    detail.setPatchSetInfo(patchSetInfo);
-    detail.setChange(change);
-    detail.setDrafts(drafts);
-
-    if (change.getStatus().isOpen()
-        && patchSetId.equals(change.currentPatchSetId())) {
-      // TODO Push this selection of labels down into the Prolog interpreter.
-      // Ideally we discover the labels the user can apply here based on doing
-      // a findall() over the space of labels they can apply combined against
-      // the submit rule, thereby skipping any mutually exclusive cases. However
-      // those are not common, so it might just be reasonable to take this
-      // simple approach.
-
-      Map<String, PermissionRange> rangeByName =
-          new HashMap<String, PermissionRange>();
-      for (PermissionRange r : control.getLabelRanges()) {
-        if (r.isLabel()) {
-          rangeByName.put(r.getLabel(), r);
-        }
-      }
-
-      boolean couldSubmit = false;
-      List<SubmitRecord> submitRecords = control.canSubmit(db, patchSet);
-      for (SubmitRecord rec : submitRecords) {
-        if (rec.status == SubmitRecord.Status.OK) {
-          couldSubmit = true;
-        }
-
-        if (rec.labels != null) {
-          int ok = 0;
-
-          for (SubmitRecord.Label lbl : rec.labels) {
-            aic.want(lbl.appliedBy);
-
-            boolean canMakeOk = false;
-            PermissionRange range = rangeByName.get(lbl.label);
-            if (range != null) {
-              LabelType lt = control.getLabelTypes().byLabel(lbl.label);
-              if (lt != null && lt.getMax().getValue() == range.getMax()) {
-                canMakeOk = true;
-              }
-            }
-
-            switch (lbl.status) {
-              case OK:
-              case MAY:
-                ok++;
-                break;
-
-              case NEED:
-                if (canMakeOk) {
-                  ok++;
-                }
-                break;
-
-              case IMPOSSIBLE:
-              case REJECT:
-                break;
-            }
-          }
-
-          if (rec.status == SubmitRecord.Status.NOT_READY
-              && ok == rec.labels.size()) {
-            couldSubmit = true;
-          }
-        }
-      }
-
-      if (couldSubmit && control.getRefControl().canSubmit()) {
-        detail.setCanSubmit(true);
-      }
-    }
-
-    detail.setSubmitTypeRecord(control.getSubmitTypeRecord(db, patchSet));
-    detail.setAccounts(aic.create());
-
-    return detail;
-  }
-}