diff options
8 files changed, 97 insertions, 197 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index adba1aad1a..8f04d7cb40 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -844,30 +844,6 @@ abbreviated commit SHA-1 (`c9c0edb`). + Default is "Submit patch set ${patchSet} into ${branch}". -[[change.submitWholeTopic]]change.submitWholeTopic:: -+ -Determines if the submit button submits the whole topic instead of -just the current change. -+ -Default is false. - -[[change.submitTopicLabel]]change.submitTopicLabel:: -+ -If `change.submitWholeTopic` is set and a change has a topic, -the label name for the submit button is given here instead of -the configuration `change.submitLabel`. -+ -Defaults to "Submit whole topic" - -[[change.submitTopicTooltip]]change.submitTopicTooltip:: -+ -If `change.submitWholeTopic` is configuerd to true and a change has a -topic, this configuration determines the tooltip for the submit button -instead of `change.submitTooltip`. The variable `${topicSize}` is available -for the number of changes to be submitted. -+ -Defaults to "Submit all ${topicSize} changes within the topic". - [[change.replyLabel]]change.replyLabel:: + Label name for the reply button. In the user interface an ellipsis (…) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java index 37b90c4060..525684d44d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java @@ -65,6 +65,7 @@ class Actions extends Composite { private String message; private String branch; private String key; + private boolean canSubmit; Actions() { initWidget(uiBinder.createAndBindUi(this)); @@ -86,12 +87,7 @@ class Actions extends Composite { changeInfo = info; initChangeActions(info, hasUser); - - NativeMap<ActionInfo> actionMap = revInfo.has_actions() - ? revInfo.actions() - : NativeMap.<ActionInfo> create(); - actionMap.copyKeysIntoChildren("id"); - reloadRevisionActions(actionMap); + initRevisionActions(info, revInfo, hasUser); } private void initChangeActions(ChangeInfo info, boolean hasUser) { @@ -111,29 +107,30 @@ class Actions extends Composite { } } - 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); - - a2b(actions, "cherrypick", cherrypick); - a2b(actions, "rebase", rebase); + private void initRevisionActions(ChangeInfo info, RevisionInfo revInfo, + boolean hasUser) { + NativeMap<ActionInfo> actions = revInfo.has_actions() + ? revInfo.actions() + : NativeMap.<ActionInfo> create(); + actions.copyKeysIntoChildren("id"); - RevisionInfo revInfo = changeInfo.revision(revision); - for (String id : filterNonCore(actions)) { - add(new ActionButton(changeInfo, revInfo, actions.get(id))); + 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))); + } } } @@ -149,6 +146,10 @@ class Actions extends Composite { return ids; } + void setSubmitEnabled() { + submit.setVisible(canSubmit); + } + @UiHandler("followUp") void onFollowUp(@SuppressWarnings("unused") ClickEvent e) { if (followUpAction == null) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java index bae181c647..8a27fd049a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java @@ -248,7 +248,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.CHANGE_ACTIONS, + ListChangesOption.CURRENT_ACTIONS, ListChangesOption.ALL_REVISIONS)); if (!fg) { call.background(); @@ -256,18 +256,6 @@ 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) { @@ -415,8 +403,7 @@ public class ChangeScreen extends Screen { } } - private void initRevisionsAction(ChangeInfo info, String revision, - NativeMap<ActionInfo> actions) { + private void initRevisionsAction(ChangeInfo info, String revision) { int currentPatchSet; if (info.current_revision() != null && info.revisions().containsKey(info.current_revision())) { @@ -444,6 +431,11 @@ 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()); @@ -814,7 +806,6 @@ public class ChangeScreen extends Screen { commentLinkProcessor = result.getCommentLinkProcessor(); setTheme(result.getTheme()); renderChangeInfo(info); - loadRevisionInfo(); } })); } @@ -942,6 +933,7 @@ 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()); } @@ -1051,7 +1043,19 @@ 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); @@ -1060,6 +1064,7 @@ public class ChangeScreen extends Screen { initReplyButton(info, revision); initIncludedInAction(info); initChangeAction(info); + initRevisionsAction(info, revision); initDownloadAction(info, revision); initProjectLinks(info); initBranchLink(info); @@ -1079,37 +1084,6 @@ 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); @@ -1131,7 +1105,14 @@ public class ChangeScreen extends Screen { quickApprove.setVisible(false); setVisible(strategy, false); } - actions.reloadRevisionActions(actionMap); + + 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()); } private void renderOwner(ChangeInfo info) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index 1135491d4d..98595e7aff 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java @@ -95,13 +95,6 @@ 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); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java index 1555cdd842..16472e35cf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java @@ -57,15 +57,17 @@ public class ChangeResource implements RestResource, HasETag { return getControl().getNotes(); } - - // This includes all information relevant for ETag computation - // unrelated to the UI. - public void prepareETag(Hasher h, CurrentUser user) { - h.putLong(getChange().getLastUpdatedOn().getTime()) + @Override + public String getETag() { + CurrentUser user = control.getCurrentUser(); + Hasher h = Hashing.md5().newHasher() + .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 { @@ -80,14 +82,6 @@ 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(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java index b21bee227b..d58c8d2a5b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java @@ -14,59 +14,22 @@ 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.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.gerrit.extensions.restapi.RestReadView; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; -import org.eclipse.jgit.lib.Config; - @Singleton -public class GetRevisionActions implements ETagView<RevisionResource> { +public class GetRevisionActions implements RestReadView<RevisionResource> { private final ActionJson delegate; - private final Provider<InternalChangeQuery> queryProvider; - private final Config config; + @Inject - GetRevisionActions( - ActionJson delegate, - Provider<InternalChangeQuery> queryProvider, - @GerritServerConfig Config config) { + GetRevisionActions(ActionJson delegate) { 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(); - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index 03bc7f1c72..41151ae8da 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/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 = wholeTopicEnabled(cfg); + submitWholeTopic = cfg.getBoolean("change", null, "submitWholeTopic" , false); this.submitTopicLabel = MoreObjects.firstNonNull( Strings.emptyToNull(cfg.getString("change", null, "submitTopicLabel")), "Submit whole topic"); @@ -206,19 +206,17 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>, rsrc.getPatchSet().getRevision().get())); } - List<Change> submittedChanges = submit(rsrc, caller, false); + change = submit(rsrc, caller, false); + if (change == null) { + throw new ResourceConflictException("change is " + + status(dbProvider.get().changes().get(rsrc.getChange().getId()))); + } if (input.waitForMerge) { - 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()); - } + mergeQueue.merge(change.getDest()); change = dbProvider.get().changes().get(change.getId()); } else { - for (Change c : submittedChanges) { - mergeQueue.schedule(c.getDest()); - } + mergeQueue.schedule(change.getDest()); } if (change == null) { @@ -347,10 +345,9 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>, .orNull(); } - private Change submitToDatabase(final ReviewDb db, final Change.Id changeId, - final Timestamp timestamp) throws OrmException, - ResourceConflictException { - Change ret = db.changes().atomicUpdate(changeId, + private Change submitToDatabase(ReviewDb db, Change.Id changeId, + final Timestamp timestamp) throws OrmException { + return db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() { @Override public Change update(Change change) { @@ -362,12 +359,6 @@ 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, @@ -389,6 +380,9 @@ 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(); @@ -397,7 +391,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>, return change; } - private List<Change> submitWholeTopic(RevisionResource rsrc, IdentifiedUser caller, + private Change submitWholeTopic(RevisionResource rsrc, IdentifiedUser caller, boolean force, String topic) throws ResourceConflictException, OrmException, IOException { Preconditions.checkNotNull(topic); @@ -426,31 +420,30 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>, batch.write(update, new CommitBuilder()); for (ChangeData c : changesByTopic) { - submitToDatabase(db, c.getId(), timestamp); + if (submitToDatabase(db, c.getId(), timestamp) == null) { + return null; + } } 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 ret; + return change; } - public List<Change> submit(RevisionResource rsrc, IdentifiedUser caller, + public 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 Arrays.asList(submitThisChange(rsrc, caller, force)); + return submitThisChange(rsrc, caller, force); } } @@ -661,10 +654,6 @@ 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; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index cf63e78bb5..cc2919d58b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1726,15 +1726,18 @@ public class ReceiveCommits { throws OrmException, IOException { Submit submit = submitProvider.get(); RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps); - List<Change> changes; + Change c; try { // Force submit even if submit rule evaluation fails. - changes = submit.submit(rsrc, currentUser, true); + c = submit.submit(rsrc, currentUser, true); } catch (ResourceConflictException e) { throw new IOException(e); } - addMessage(""); - for (Change c : changes) { + if (c == null) { + addError("Submitting change " + changeCtl.getChange().getChangeId() + + " failed."); + } else { + addMessage(""); mergeQueue.merge(c.getDest()); c = db.changes().get(c.getId()); switch (c.getStatus()) { |