Merge changes from topic 'submitWholeTopic'

* changes:
  Submit Whole Topic: trigger merge queue for all submitted changes
  ChangeScreen: explicitly load revision info
  Actions: reloadRevisionActions determines submit button visibility
  ChangeScreen: introduce renderRevisionInfo
  ChangeAPI: add call to new actions REST API call
  ETag computation needs to honor changes in other changes

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java

@@ -65,7 +65,6 @@ class Actions extends Composite {
   private String message;
   private String branch;
   private String key;
-  private boolean canSubmit;
 
   Actions() {
     initWidget(uiBinder.createAndBindUi(this));
@@ -87,7 +86,12 @@ class Actions extends Composite {
     changeInfo = info;
 
     initChangeActions(info, hasUser);
-    initRevisionActions(info, revInfo, hasUser);
+
+    NativeMap<ActionInfo> actionMap = revInfo.has_actions()
+        ? revInfo.actions()
+        : NativeMap.<ActionInfo> create();
+    actionMap.copyKeysIntoChildren("id");
+    reloadRevisionActions(actionMap);
   }
 
   private void initChangeActions(ChangeInfo info, boolean hasUser) {
@@ -107,30 +111,29 @@ class Actions extends Composite {
     }
   }
 
-  private void initRevisionActions(ChangeInfo info, RevisionInfo revInfo,
-      boolean hasUser) {
-    NativeMap<ActionInfo> actions = revInfo.has_actions()
-        ? revInfo.actions()
-        : NativeMap.<ActionInfo> create();
-    actions.copyKeysIntoChildren("id");
+  void reloadRevisionActions(NativeMap<ActionInfo> actions) {
+    if (!Gerrit.isSignedIn()) {
+      return;
+    }
+    boolean canSubmit = actions.containsKey("submit");
+    if (canSubmit) {
+      ActionInfo action = actions.get("submit");
+      submit.setTitle(action.title());
+      submit.setEnabled(action.enabled());
+      submit.setHTML(new SafeHtmlBuilder()
+          .openDiv()
+          .append(action.label())
+          .closeDiv());
+      submit.setEnabled(action.enabled());
+    }
+    submit.setVisible(canSubmit);
 
-    canSubmit = false;
-    if (hasUser) {
-      canSubmit = actions.containsKey("submit");
-      if (canSubmit) {
-        ActionInfo action = actions.get("submit");
-        submit.setTitle(action.title());
-        submit.setEnabled(action.enabled());
-        submit.setHTML(new SafeHtmlBuilder()
-            .openDiv()
-            .append(action.label())
-            .closeDiv());
-      }
-      a2b(actions, "cherrypick", cherrypick);
-      a2b(actions, "rebase", rebase);
-      for (String id : filterNonCore(actions)) {
-        add(new ActionButton(info, revInfo, actions.get(id)));
-      }
+    a2b(actions, "cherrypick", cherrypick);
+    a2b(actions, "rebase", rebase);
+
+    RevisionInfo revInfo = changeInfo.revision(revision);
+    for (String id : filterNonCore(actions)) {
+      add(new ActionButton(changeInfo, revInfo, actions.get(id)));
     }
   }
 
@@ -146,10 +149,6 @@ class Actions extends Composite {
     return ids;
   }
 
-  void setSubmitEnabled() {
-    submit.setVisible(canSubmit);
-  }
-
   @UiHandler("followUp")
   void onFollowUp(@SuppressWarnings("unused") ClickEvent e) {
     if (followUpAction == null) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java

@@ -240,7 +240,10 @@ public class ChangeScreen extends Screen {
           @Override
           public void onSuccess(ChangeInfo info) {
             info.init();
+            // Revision loading may be slower than the rest, so do it
+            // asynchronous to have the rest fast.
             loadConfigInfo(info, base);
+            loadRevisionInfo();
           }
         }));
   }
@@ -248,7 +251,7 @@ public class ChangeScreen extends Screen {
   void loadChangeInfo(boolean fg, AsyncCallback<ChangeInfo> cb) {
     RestApi call = ChangeApi.detail(changeId.get());
     ChangeList.addOptions(call, EnumSet.of(
-      ListChangesOption.CURRENT_ACTIONS,
+      ListChangesOption.CHANGE_ACTIONS,
       ListChangesOption.ALL_REVISIONS));
     if (!fg) {
       call.background();
@@ -256,6 +259,18 @@ public class ChangeScreen extends Screen {
     call.get(cb);
   }
 
+  void loadRevisionInfo() {
+    RestApi call = ChangeApi.actions(changeId.get(), revision);
+    call.background();
+    call.get(new GerritCallback<NativeMap<ActionInfo>>() {
+      @Override
+      public void onSuccess(NativeMap<ActionInfo> actionMap) {
+        actionMap.copyKeysIntoChildren("id");
+        renderRevisionInfo(changeInfo, actionMap);
+      }
+    });
+  }
+
   @Override
   protected void onUnload() {
     if (replyAction != null) {
@@ -403,7 +418,8 @@ public class ChangeScreen extends Screen {
     }
   }
 
-  private void initRevisionsAction(ChangeInfo info, String revision) {
+  private void initRevisionsAction(ChangeInfo info, String revision,
+      NativeMap<ActionInfo> actions) {
     int currentPatchSet;
     if (info.current_revision() != null
         && info.revisions().containsKey(info.current_revision())) {
@@ -431,11 +447,6 @@ public class ChangeScreen extends Screen {
 
     RevisionInfo revInfo = info.revision(revision);
     if (revInfo.draft()) {
-      NativeMap<ActionInfo> actions = revInfo.has_actions()
-          ? revInfo.actions()
-          : NativeMap.<ActionInfo> create();
-      actions.copyKeysIntoChildren("id");
-
       if (actions.containsKey("publish")) {
         publish.setVisible(true);
         publish.setTitle(actions.get("publish").title());
@@ -933,7 +944,6 @@ public class ChangeScreen extends Screen {
 
   private void loadSubmitType(final Change.Status status, final boolean canSubmit) {
     if (canSubmit) {
-      actions.setSubmitEnabled();
       if (status == Change.Status.NEW) {
         statusText.setInnerText(Util.C.readyToSubmit());
       }
@@ -1043,19 +1053,7 @@ public class ChangeScreen extends Screen {
   private void renderChangeInfo(ChangeInfo info) {
     changeInfo = info;
     lastDisplayedUpdate = info.updated();
-    RevisionInfo revisionInfo = info.revision(revision);
-    boolean current = info.status().isOpen()
-        && revision.equals(info.current_revision())
-        && !revisionInfo.is_edit();
 
-    if (revisionInfo.is_edit()) {
-      statusText.setInnerText(Util.C.changeEdit());
-    } else if (!current && info.status() == Change.Status.NEW) {
-      statusText.setInnerText(Util.C.notCurrent());
-      labels.setVisible(false);
-    } else {
-      statusText.setInnerText(Util.toLongString(info.status()));
-    }
     labels.set(info);
 
     renderOwner(info);
@@ -1064,7 +1062,6 @@ public class ChangeScreen extends Screen {
     initReplyButton(info, revision);
     initIncludedInAction(info);
     initChangeAction(info);
-    initRevisionsAction(info, revision);
     initDownloadAction(info, revision);
     initProjectLinks(info);
     initBranchLink(info);
@@ -1084,6 +1081,37 @@ public class ChangeScreen extends Screen {
       setVisible(hashtagTableRow, false);
     }
 
+    StringBuilder sb = new StringBuilder();
+    sb.append(Util.M.changeScreenTitleId(info.id_abbreviated()));
+    if (info.subject() != null) {
+      sb.append(": ");
+      sb.append(info.subject());
+    }
+    setWindowTitle(sb.toString());
+
+    // Properly render revision actions initially while waiting for
+    // the callback to populate them correctly.
+    renderRevisionInfo(changeInfo, NativeMap.<ActionInfo> create());
+  }
+
+  private void renderRevisionInfo(ChangeInfo info,
+      NativeMap<ActionInfo> actionMap) {
+    RevisionInfo revisionInfo = info.revision(revision);
+    boolean current = info.status().isOpen()
+        && revision.equals(info.current_revision())
+        && !revisionInfo.is_edit();
+
+    if (revisionInfo.is_edit()) {
+      statusText.setInnerText(Util.C.changeEdit());
+    } else if (!current && info.status() == Change.Status.NEW) {
+      statusText.setInnerText(Util.C.notCurrent());
+      labels.setVisible(false);
+    } else {
+      statusText.setInnerText(Util.toLongString(info.status()));
+    }
+
+    initRevisionsAction(info, revision, actionMap);
+
     if (Gerrit.isSignedIn()) {
       replyAction = new ReplyAction(info, revision,
           style, commentLinkProcessor, reply, quickApprove);
@@ -1105,14 +1133,7 @@ public class ChangeScreen extends Screen {
       quickApprove.setVisible(false);
       setVisible(strategy, false);
     }
-
-    StringBuilder sb = new StringBuilder();
-    sb.append(Util.M.changeScreenTitleId(info.id_abbreviated()));
-    if (info.subject() != null) {
-      sb.append(": ");
-      sb.append(info.subject());
-    }
-    setWindowTitle(sb.toString());
+    actions.reloadRevisionActions(actionMap);
   }
 
   private void renderOwner(ChangeInfo info) {

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

@@ -95,6 +95,13 @@ public class ChangeApi {
     return call(id, "detail");
   }
 
+  public static RestApi actions(int id, String revision) {
+    if (revision == null || revision.equals("")) {
+      revision = "current";
+    }
+    return call(id, revision, "actions");
+  }
+
   public static void edit(int id, AsyncCallback<EditInfo> cb) {
     edit(id).get(cb);
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java

@@ -57,17 +57,15 @@ public class ChangeResource implements RestResource, HasETag {
     return getControl().getNotes();
   }
 
-  @Override
-  public String getETag() {
-    CurrentUser user = control.getCurrentUser();
-    Hasher h = Hashing.md5().newHasher()
-      .putLong(getChange().getLastUpdatedOn().getTime())
+
+  // This includes all information relevant for ETag computation
+  // unrelated to the UI.
+  public void prepareETag(Hasher h, CurrentUser user) {
+    h.putLong(getChange().getLastUpdatedOn().getTime())
       .putInt(getChange().getRowVersion())
-      .putBoolean(user.getStarredChanges().contains(getChange().getId()))
       .putInt(user.isIdentifiedUser()
           ? ((IdentifiedUser) user).getAccountId().get()
           : 0);
-
     byte[] buf = new byte[20];
     ObjectId noteId;
     try {
@@ -82,6 +80,14 @@ public class ChangeResource implements RestResource, HasETag {
     for (ProjectState p : control.getProjectControl().getProjectState().tree()) {
       hashObjectId(h, p.getConfig().getRevision(), buf);
     }
+  }
+
+  @Override
+  public String getETag() {
+    CurrentUser user = control.getCurrentUser();
+    Hasher h = Hashing.md5().newHasher()
+        .putBoolean(user.getStarredChanges().contains(getChange().getId()));
+    prepareETag(h, user);
     return h.hash().toString();
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java

@@ -14,22 +14,59 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.common.base.Strings;
+import com.google.common.hash.Hasher;
+import com.google.common.hash.Hashing;
+import com.google.gerrit.extensions.restapi.ETagView;
 import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.OrmRuntimeException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
-@Singleton
-public class GetRevisionActions implements RestReadView<RevisionResource> {
-  private final ActionJson delegate;
+import org.eclipse.jgit.lib.Config;
 
+@Singleton
+public class GetRevisionActions implements ETagView<RevisionResource> {
+  private final ActionJson delegate;
+  private final Provider<InternalChangeQuery> queryProvider;
+  private final Config config;
   @Inject
-  GetRevisionActions(ActionJson delegate) {
+  GetRevisionActions(
+      ActionJson delegate,
+      Provider<InternalChangeQuery> queryProvider,
+      @GerritServerConfig Config config) {
     this.delegate = delegate;
+    this.queryProvider = queryProvider;
+    this.config = config;
   }
 
   @Override
   public Object apply(RevisionResource rsrc) {
     return Response.withMustRevalidate(delegate.format(rsrc));
   }
+
+  @Override
+  public String getETag(RevisionResource rsrc) {
+    String topic = rsrc.getChange().getTopic();
+    if (!Submit.wholeTopicEnabled(config)
+        || Strings.isNullOrEmpty(topic)) {
+      return rsrc.getETag();
+    }
+    Hasher h = Hashing.md5().newHasher();
+    CurrentUser user = rsrc.getControl().getCurrentUser();
+    try {
+      for (ChangeData c : queryProvider.get().byTopicOpen(topic)) {
+        new ChangeResource(c.changeControl()).prepareETag(h, user);
+      }
+    } catch (OrmException e){
+      throw new OrmRuntimeException(e);
+    }
+    return h.hash().toString();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java

@@ -169,7 +169,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     this.titlePattern = new ParameterizedString(MoreObjects.firstNonNull(
         cfg.getString("change", null, "submitTooltip"),
         DEFAULT_TOOLTIP));
-    submitWholeTopic = cfg.getBoolean("change", null, "submitWholeTopic" , false);
+    submitWholeTopic = wholeTopicEnabled(cfg);
     this.submitTopicLabel = MoreObjects.firstNonNull(
         Strings.emptyToNull(cfg.getString("change", null, "submitTopicLabel")),
         "Submit whole topic");
@@ -206,17 +206,19 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
           rsrc.getPatchSet().getRevision().get()));
     }
 
-    change = submit(rsrc, caller, false);
-    if (change == null) {
-      throw new ResourceConflictException("change is "
-          + status(dbProvider.get().changes().get(rsrc.getChange().getId())));
-    }
+    List<Change> submittedChanges = submit(rsrc, caller, false);
 
     if (input.waitForMerge) {
-      mergeQueue.merge(change.getDest());
+      for (Change c : submittedChanges) {
+        // TODO(sbeller): We should make schedule return a Future, then we
+        // could do these all in parallel and still block until they're done.
+        mergeQueue.merge(c.getDest());
+      }
       change = dbProvider.get().changes().get(change.getId());
     } else {
-      mergeQueue.schedule(change.getDest());
+      for (Change c : submittedChanges) {
+        mergeQueue.schedule(c.getDest());
+      }
     }
 
     if (change == null) {
@@ -345,9 +347,10 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
         .orNull();
   }
 
-  private Change submitToDatabase(ReviewDb db, Change.Id changeId,
-      final Timestamp timestamp) throws OrmException {
-    return db.changes().atomicUpdate(changeId,
+  private Change submitToDatabase(final ReviewDb db, final Change.Id changeId,
+      final Timestamp timestamp) throws OrmException,
+      ResourceConflictException {
+    Change ret = db.changes().atomicUpdate(changeId,
       new AtomicUpdate<Change>() {
         @Override
         public Change update(Change change) {
@@ -359,6 +362,12 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
           return null;
         }
       });
+    if (ret != null) {
+      return ret;
+    } else {
+      throw new ResourceConflictException("change " + changeId + " is "
+          + status(db.changes().get(changeId)));
+    }
   }
 
   private Change submitThisChange(RevisionResource rsrc, IdentifiedUser caller,
@@ -380,9 +389,6 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       // Write update commit after all normalized label commits.
       batch.write(update, new CommitBuilder());
       change = submitToDatabase(db, change.getId(), timestamp);
-      if (change == null) {
-        return null;
-      }
       db.commit();
     } finally {
       db.rollback();
@@ -391,7 +397,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     return change;
   }
 
-  private Change submitWholeTopic(RevisionResource rsrc, IdentifiedUser caller,
+  private List<Change> submitWholeTopic(RevisionResource rsrc, IdentifiedUser caller,
       boolean force, String topic) throws ResourceConflictException, OrmException,
       IOException {
     Preconditions.checkNotNull(topic);
@@ -420,30 +426,31 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       batch.write(update, new CommitBuilder());
 
       for (ChangeData c : changesByTopic) {
-        if (submitToDatabase(db, c.getId(), timestamp) == null) {
-          return null;
-        }
+        submitToDatabase(db, c.getId(), timestamp);
       }
       db.commit();
     } finally {
       db.rollback();
     }
     List<Change.Id> ids = new ArrayList<>(changesByTopic.size());
+    List<Change> ret = new ArrayList<>(changesByTopic.size());
     for (ChangeData c : changesByTopic) {
       ids.add(c.getId());
+      ret.add(c.change());
     }
     indexer.indexAsync(ids).checkedGet();
-    return change;
+
+    return ret;
   }
 
-  public Change submit(RevisionResource rsrc, IdentifiedUser caller,
+  public List<Change> submit(RevisionResource rsrc, IdentifiedUser caller,
       boolean force) throws ResourceConflictException, OrmException,
       IOException {
     String topic = rsrc.getChange().getTopic();
     if (submitWholeTopic && !Strings.isNullOrEmpty(topic)) {
       return submitWholeTopic(rsrc, caller, force, topic);
     } else {
-      return submitThisChange(rsrc, caller, force);
+      return Arrays.asList(submitThisChange(rsrc, caller, force));
     }
   }
 
@@ -654,6 +661,10 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     return new RevisionResource(changes.parse(target), rsrc.getPatchSet());
   }
 
+  static boolean wholeTopicEnabled(Config config) {
+    return config.getBoolean("change", null, "submitWholeTopic" , false);
+  }
+
   public static class CurrentRevision implements
       RestModifyView<ChangeResource, SubmitInput> {
     private final Provider<ReviewDb> dbProvider;

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -1726,18 +1726,15 @@ public class ReceiveCommits {
       throws OrmException, IOException {
     Submit submit = submitProvider.get();
     RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps);
-    Change c;
+    List<Change> changes;
     try {
       // Force submit even if submit rule evaluation fails.
-      c = submit.submit(rsrc, currentUser, true);
+      changes = submit.submit(rsrc, currentUser, true);
     } catch (ResourceConflictException e) {
       throw new IOException(e);
     }
-    if (c == null) {
-      addError("Submitting change " + changeCtl.getChange().getChangeId()
-          + " failed.");
-    } else {
-      addMessage("");
+    addMessage("");
+    for (Change c : changes) {
       mergeQueue.merge(c.getDest());
       c = db.changes().get(c.getId());
       switch (c.getStatus()) {