diff options
10 files changed, 235 insertions, 157 deletions
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java index 8ac346641b..85ef39e76e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java @@ -344,7 +344,9 @@ public class ChangeTable extends NavigationTable<ChangeInfo> { table.clearCell(row, col); } else { - haveReview = true; + if (!ca.getCategoryId().equals(ApprovalCategory.SANITY_REVIEW)) { + haveReview = true; + } final ApprovalCategoryValue acv = type.getValue(ca); final AccountInfo ai = aic.get(ca.getAccountId()); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java index af10932023..4c57e0e2cc 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java @@ -43,6 +43,7 @@ import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.FormPanel; import com.google.gwt.user.client.ui.FormPanel.SubmitEvent; +import com.google.gwt.user.client.ui.DisclosurePanel; import com.google.gwt.user.client.ui.Panel; import com.google.gwt.user.client.ui.RadioButton; import com.google.gwt.user.client.ui.VerticalPanel; @@ -62,7 +63,9 @@ public class PublishCommentScreen extends AccountScreen implements ClickHandler, CommentEditorContainer { private static SavedState lastState; - private enum Action { NOOP, SUBMIT, STAGING }; + private enum Action { + NOOP, SUBMIT, STAGING + }; private final PatchSet.Id patchSetId; private Collection<ValueRadioButton> approvalButtons; @@ -177,7 +180,7 @@ public class PublishCommentScreen extends AccountScreen implements } else if (cancel == sender) { saveStateOnUnload = false; goChange(); - } else if(staging == sender) { + } else if (staging == sender) { onSend(Action.STAGING); } } @@ -233,7 +236,8 @@ public class PublishCommentScreen extends AccountScreen implements ApprovalTypes types = Gerrit.getConfig().getApprovalTypes(); for (ApprovalType type : types.getApprovalTypes()) { - String permission = Permission.forLabel(type.getCategory().getLabelName()); + String permission = + Permission.forLabel(type.getCategory().getLabelName()); PermissionRange range = r.getRange(permission); if (range != null && !range.isEmpty()) { initApprovalType(r, body, type, range); @@ -249,7 +253,6 @@ public class PublishCommentScreen extends AccountScreen implements private void initApprovalType(final PatchSetPublishDetail r, final Panel body, final ApprovalType ct, final PermissionRange range) { - body.add(new SmallHeading(ct.getCategory().getName() + ":")); final VerticalPanel vp = new VerticalPanel(); vp.setStyleName(Gerrit.RESOURCES.css().approvalCategoryList()); @@ -280,7 +283,11 @@ public class PublishCommentScreen extends AccountScreen implements approvalButtons.add(b); vp.add(b); } - body.add(vp); + DisclosurePanel atp = new DisclosurePanel(ct.getCategory().getName()); + atp.setContent(vp); + atp.setOpen(!ApprovalCategory.SANITY_REVIEW.equals(ct + .getCategory().getId())); + body.add(atp); } private void display(final PatchSetPublishDetail r) { @@ -365,7 +372,7 @@ public class PublishCommentScreen extends AccountScreen implements new HashSet<ApprovalCategoryValue.Id>(values.values()), new GerritCallback<VoidResult>() { public void onSuccess(final VoidResult result) { - if(action == Action.SUBMIT) { + if (action == Action.SUBMIT) { submit(); } else if (action == Action.STAGING) { staging(); @@ -384,19 +391,18 @@ public class PublishCommentScreen extends AccountScreen implements } private void submit() { - Util.MANAGE_SVC.submit(patchSetId, - new GerritCallback<ChangeDetail>() { - public void onSuccess(ChangeDetail result) { - saveStateOnUnload = false; - goChange(); - } + Util.MANAGE_SVC.submit(patchSetId, new GerritCallback<ChangeDetail>() { + public void onSuccess(ChangeDetail result) { + saveStateOnUnload = false; + goChange(); + } - @Override - public void onFailure(Throwable caught) { - goChange(); - super.onFailure(caught); - } - }); + @Override + public void onFailure(Throwable caught) { + goChange(); + super.onFailure(caught); + } + }); } private void staging() { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishTopicCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishTopicCommentScreen.java index d4bfa9090b..4b1974dac9 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishTopicCommentScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishTopicCommentScreen.java @@ -36,6 +36,7 @@ import com.google.gerrit.reviewdb.Topic; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.user.client.ui.Button; +import com.google.gwt.user.client.ui.DisclosurePanel; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.FormPanel; import com.google.gwt.user.client.ui.FormPanel.SubmitEvent; @@ -213,8 +214,6 @@ public class PublishTopicCommentScreen extends AccountScreen implements private void initApprovalType(final ChangeSetPublishDetail r, final Panel body, final ApprovalType ct, final PermissionRange range) { - body.add(new SmallHeading(ct.getCategory().getName() + ":")); - final VerticalPanel vp = new VerticalPanel(); vp.setStyleName(Gerrit.RESOURCES.css().approvalCategoryList()); final List<ApprovalCategoryValue> lst = @@ -244,7 +243,11 @@ public class PublishTopicCommentScreen extends AccountScreen implements approvalButtons.add(b); vp.add(b); } - body.add(vp); + + DisclosurePanel atp = new DisclosurePanel(ct.getCategory().getName()); + atp.setContent(vp); + atp.setOpen(!ApprovalCategory.SANITY_REVIEW.equals(ct.getCategory().getId())); + body.add(atp); } private void display(final ChangeSetPublishDetail r) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css index a067c1dc8a..d175d91a85 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css @@ -1245,6 +1245,18 @@ a:hover.downloadLink { display: none; } +.publishCommentsScreen .gwt-DisclosurePanel .header td { + font-weight: bold; + white-space: nowrap; +} + +.publishCommentsScreen .gwt-DisclosurePanel .complexHeader { + white-space: nowrap; +} +.publishCommentsScreen .gwt-DisclosurePanel .complexHeader span { + white-space: nowrap; +} + /** AbandonChangeDialog **/ diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Approvals.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Approvals.java index d8a348dade..41fad4890b 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Approvals.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Approvals.java @@ -31,6 +31,7 @@ import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.DisclosurePanel; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.Panel; import com.google.gwt.user.client.ui.RadioButton; @@ -80,7 +81,7 @@ public class Approvals extends Composite { private static SavedState lastState; private boolean saveState = true; - private final VerticalPanel body; + private final Panel body; private final PatchSet.Id patchSetId; private Collection<ValueRadioButton> approvalButtons; private Message message; @@ -90,7 +91,7 @@ public class Approvals extends Composite { public Approvals(final PatchSet.Id patchSetId) { this.patchSetId = patchSetId; - body = new VerticalPanel(); + body = new FlowPanel(); approvalButtons = new ArrayList<ValueRadioButton>(); message = new Message(patchSetId); @@ -155,8 +156,6 @@ public class Approvals extends Composite { private void initApprovalType(final PatchSetPublishDetail r, final Panel body, final ApprovalType ct, final PermissionRange range) { - body.add(new SmallHeading(ct.getCategory().getName() + ":")); - final VerticalPanel vp = new VerticalPanel(); vp.setStyleName(Gerrit.RESOURCES.css().approvalCategoryList()); final List<ApprovalCategoryValue> lst = @@ -186,7 +185,11 @@ public class Approvals extends Composite { approvalButtons.add(b); vp.add(b); } - body.add(vp); + DisclosurePanel atp = new DisclosurePanel(ct.getCategory().getName()); + atp.setContent(vp); + atp.setOpen(!ApprovalCategory.SANITY_REVIEW.equals(ct + .getCategory().getId())); + body.add(atp); } private void populateActions(final PatchSetPublishDetail result) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TopicLink.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TopicLink.java index a9cda57e6b..7933b0dcfc 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TopicLink.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TopicLink.java @@ -25,7 +25,7 @@ public class TopicLink extends InlineHyperlink { private final Topic.Id topicId; public static String permalink(final Topic.Id t) { - return GWT.getHostPageBaseURL() + "/topic," + t.get() + "/"; + return GWT.getHostPageBaseURL() + "#/t/" + t.get() + "/"; } public TopicLink(String topic, Topic.Id topicId) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 65c1e88146..f1e2b08454 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -683,7 +683,12 @@ public class ChangeUtil { final Change.Id changeId = patchSetId.getParentKey(); AtomicUpdate<Change> atomicUpdate = getUpdateToState(Change.Status.INTEGRATING, Change.Status.MERGED); - db.changes().atomicUpdate(changeId, atomicUpdate); + Change change = db.changes().atomicUpdate(changeId, atomicUpdate); + List<PatchSetApproval> approvals = db.patchSetApprovals().byChange(changeId).toList(); + for (PatchSetApproval a : approvals) { + a.cache(change); + } + db.patchSetApprovals().update(approvals); } /** diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index 7f0b74a77f..36eddef87f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -198,10 +198,14 @@ public class ProjectConfig extends VersionedMetaData { p.setSubmitType(rc.getEnum(SUBMIT, null, KEY_ACTION, defaultSubmitAction)); p.setUseContentMerge(rc.getBoolean(SUBMIT, null, KEY_MERGE_CONTENT, false)); - + p.setIncludeOnlyMaxApproval(rc.getBoolean(CHERRY_PICK, INCLUDE_ONLY_MAX_APPROVALS, false)); p.setHideReviewedOn(rc.getBoolean(CHERRY_PICK, HIDE_REVIEWED_ON, false)); + for (String category : rc.getNames(CHERRY_PICK, CATEGORY_FOOTERS)) { + final boolean value = rc.getBoolean(CATEGORY_FOOTERS, category, true); + p.addHiddenFooter(category, value); + } accessSections = new HashMap<String, AccessSection>(); for (String refName : rc.getSubsections(ACCESS)) { if (isAccessSection(refName)) { @@ -214,16 +218,9 @@ public class ProjectConfig extends VersionedMetaData { } } } - for (String varName : rc.getNames(ACCESS, refName)) { if (isPermission(varName)) { Permission perm = as.getPermission(varName, true); - for (String category : rc.getNames(CHERRY_PICK, CATEGORY_FOOTERS)) { - final boolean value = rc.getBoolean(CATEGORY_FOOTERS, category, true); - p.addHiddenFooter(category, value); - } - - boolean useRange = perm.isLabel(); for (String ruleString : rc.getStringList(ACCESS, refName, varName)) { PermissionRule rule; 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 bd6f0da7b7..629a160e09 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 @@ -112,12 +112,12 @@ import javax.annotation.Nullable; /** Receives change upload using the Git receive-pack protocol. */ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { - private static final Logger log = - LoggerFactory.getLogger(ReceiveCommits.class); + private static final Logger log = LoggerFactory + .getLogger(ReceiveCommits.class); public static final String NEW_CHANGE = "refs/for/"; - private static final Pattern NEW_PATCHSET = - Pattern.compile("^refs/changes/(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$"); + private static final Pattern NEW_PATCHSET = Pattern + .compile("^refs/changes/(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$"); private static final FooterKey REVIEWED_BY = new FooterKey("Reviewed-by"); private static final FooterKey TESTED_BY = new FooterKey("Tested-by"); @@ -193,15 +193,12 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final ReplacePatchSetSender.Factory replacePatchSetFactory, final ReplicationQueue replication, final PatchSetInfoFactory patchSetInfoFactory, - final ChangeHookRunner hooks, - final ProjectCache projectCache, - final GitRepositoryManager repoManager, - final GroupCache groupCache, + final ChangeHookRunner hooks, final ProjectCache projectCache, + final GitRepositoryManager repoManager, final GroupCache groupCache, @CanonicalWebUrl @Nullable final String canonicalWebUrl, @GerritPersonIdent final PersonIdent gerritIdent, final TrackingFooters trackingFooters, - final MergeOp.Factory mergeFactory, - final MergeQueue merger, + final MergeOp.Factory mergeFactory, final MergeQueue merger, @Assisted final ProjectControl projectControl, @Assisted final Repository repo) throws IOException { @@ -267,8 +264,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // Advertise some recent open changes, in case a commit is based one. try { Set<PatchSet.Id> toGet = new HashSet<PatchSet.Id>(); - for (Change change : db.changes() - .byProjectOpenNext(project.getNameKey(), "z", 32)) { + for (Change change : db.changes().byProjectOpenNext(project.getNameKey(), + "z", 32)) { PatchSet.Id id = change.currentPatchSetId(); if (id != null) { toGet.add(id); @@ -328,7 +325,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } } } catch (IOException err) { - log.error("Error trying to advertise history on " + project.getNameKey(), err); + log.error("Error trying to advertise history on " + project.getNameKey(), + err); } rw.reset(); rp.getAdvertisedObjects().addAll(toInclude); @@ -340,6 +338,10 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { String reqName = project.getName(); return new Capable("Upload denied for project '" + reqName + "'"); } + if (project.getName() != null && project.getName().endsWith("/")) { + log.warn("Invalid project name '" + project.getName() + "'"); + return new Capable("Project name cannot end with '/'"); + } // Don't permit receive-pack to be executed if a refs/for/branch_name // reference exists in the destination repository. These block the @@ -417,8 +419,10 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // Change refs are scheduled when they are created. // replication.scheduleUpdate(project.getNameKey(), c.getRefName()); - Branch.NameKey destBranch = new Branch.NameKey(project.getNameKey(), c.getRefName()); - hooks.doRefUpdatedHook(destBranch, c.getOldId(), c.getNewId(), currentUser.getAccount()); + Branch.NameKey destBranch = + new Branch.NameKey(project.getNameKey(), c.getRefName()); + hooks.doRefUpdatedHook(destBranch, c.getOldId(), c.getNewId(), + currentUser.getAccount()); } } } @@ -615,7 +619,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { continue; } - if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED){ + if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) { continue; } @@ -657,8 +661,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { try { obj = rp.getRevWalk().parseAny(cmd.getNewId()); } catch (IOException err) { - log.error("Invalid object " + cmd.getNewId().name() + " for " - + cmd.getRefName() + " creation", err); + log.error( + "Invalid object " + cmd.getNewId().name() + " for " + + cmd.getRefName() + " creation", err); reject(cmd, "invalid object"); return; } @@ -695,8 +700,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { try { obj = rp.getRevWalk().parseAny(cmd.getNewId()); } catch (IOException err) { - log.error("Invalid object " + cmd.getNewId().name() + " for " - + cmd.getRefName(), err); + log.error( + "Invalid object " + cmd.getNewId().name() + " for " + + cmd.getRefName(), err); reject(cmd, "invalid object"); return false; } @@ -725,8 +731,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } catch (IncorrectObjectTypeException notCommit) { newObject = null; } catch (IOException err) { - log.error("Invalid object " + cmd.getNewId().name() + " for " - + cmd.getRefName() + " forced update", err); + log.error( + "Invalid object " + cmd.getNewId().name() + " for " + + cmd.getRefName() + " forced update", err); reject(cmd, "invalid object"); return; } @@ -769,7 +776,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { return; } - // Split the destination branch by branch and topic. The topic + // Split the destination branch by branch and topic. The topic // suffix is entirely optional, so it might not even exist. // int split = destBranchName.length(); @@ -919,11 +926,13 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { return; } if (changeEnt.getTopicId() != null) { - reject(cmd, "change " + changeId + " belongs to the topic " + changeEnt.getTopicId().get()); + reject(cmd, "change " + changeId + " belongs to the topic " + + changeEnt.getTopicId().get()); return; } if (!project.getNameKey().equals(changeEnt.getProject())) { - reject(cmd, "change " + changeId + " does not belong to project " + project.getName()); + reject(cmd, "change " + changeId + " does not belong to project " + + project.getName()); return; } @@ -931,14 +940,13 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } private boolean requestReplace(final ReceiveCommand cmd, final Change change, - final RevCommit newCommit, final Topic topic, - final int pos) { + final RevCommit newCommit, final Topic topic, final int pos) { if (change.getStatus().equals(AbstractEntity.Status.MERGED)) { reject(cmd, "change " + change.getId() + " closed"); return false; } - if (change.getStatus().equals(AbstractEntity.Status.ABANDONED) && - topic == null) { + if (change.getStatus().equals(AbstractEntity.Status.ABANDONED) + && topic == null) { reject(cmd, "change " + change.getId() + " closed"); return false; } @@ -947,8 +955,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { ChangeSet.Id csId = topic != null ? topic.currentChangeSetId() : null; final ReplaceRequest req = - new ReplaceRequest(change.getId(), newCommit, cmd, - topicId, csId, pos); + new ReplaceRequest(change.getId(), newCommit, cmd, topicId, csId, pos); if (replaceByChange.containsKey(req.ontoChange)) { reject(cmd, "duplicate request"); return false; @@ -970,8 +977,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { reject(cmd, "change " + change.getId() + " closed"); return false; } - if (change.getStatus().equals(AbstractEntity.Status.ABANDONED) && - !topic) { + if (change.getStatus().equals(AbstractEntity.Status.ABANDONED) && !topic) { reject(cmd, "change " + change.getId() + " closed"); return false; } @@ -997,14 +1003,17 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } private void createNewChanges() { - final String abandonMessage = "This Change doesn't belong any more to the topic "; + final String abandonMessage = + "This Change doesn't belong any more to the topic "; final List<RevCommit> toCreate = new ArrayList<RevCommit>(); final Map<Change, RevCommit> toReplace = new HashMap<Change, RevCommit>(); final Map<Change.Id, Change> byChangeId = new HashMap<Change.Id, Change>(); final RevWalk walk = rp.getRevWalk(); final ObjectId lastIncluded = newChange.getNewId(); final RevCommit firstIncluded; - final boolean topicSetting = project.isAllowTopicReview() && project.isRequireChangeID() && (destTopicName != null); + final boolean topicSetting = + project.isAllowTopicReview() && project.isRequireChangeID() + && (destTopicName != null); List<Change> toAbandon = new ArrayList<Change>(); List<Change> toUpdate = new ArrayList<Change>(); List<RevCommit> cOrder = new ArrayList<RevCommit>(); @@ -1072,7 +1081,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { if (changes.size() == 1) { toReplace.put(changes.get(0), c); byChangeId.put(changes.get(0).getId(), changes.get(0)); - if (requestReplaceCheck(newChange, changes.get(0), c, toReplace, topicSetting)) { + if (requestReplaceCheck(newChange, changes.get(0), c, toReplace, + topicSetting)) { continue; } else { return; @@ -1122,32 +1132,38 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final String message = ""; final ChangeSet.Id previousCs; - List<ChangeSetElement> currentChangeSetElements = new ArrayList<ChangeSetElement>(); + List<ChangeSetElement> currentChangeSetElements = + new ArrayList<ChangeSetElement>(); List<Change> currentChanges = new ArrayList<Change>(); Set<String> revIdAncestors = new HashSet<String>(); - if (cOrder.isEmpty()) firstIncluded = null; + if (cOrder.isEmpty()) + firstIncluded = null; else { firstIncluded = cOrder.get(0); for (RevCommit r : firstIncluded.getParents()) revIdAncestors.add(r.getName()); } - t = TopicUtil.findActiveTopic(destTopicName, db, destBranch, - project.getNameKey()); + t = + TopicUtil.findActiveTopic(destTopicName, db, destBranch, + project.getNameKey()); if (t != null) { if (t.getStatus().equals(AbstractEntity.Status.SUBMITTED)) { - reject(newChange, "There is a topic with the same topic name in SUBMITTED status"); + reject(newChange, + "There is a topic with the same topic name in SUBMITTED status"); return; } topicId = t.getId(); previousCs = new ChangeSet.Id(topicId, t.currChangeSetId().get()); - currentChangeSetElements = db.changeSetElements().byChangeSet(previousCs).toList(); + currentChangeSetElements = + db.changeSetElements().byChangeSet(previousCs).toList(); } // Check if the replacement Changes belongs to this topic // if (!checkSameTopic(toReplace, topicId)) { - reject(newChange, "The modified changes doesn't belong to the same topic."); + reject(newChange, + "The modified changes doesn't belong to the same topic."); return; } @@ -1167,14 +1183,17 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // Identify deleted elements // if (!currentChanges.isEmpty()) { - for (Change c : currentChanges.subList(toUpdate.size(), currentChanges.size())) { + for (Change c : currentChanges.subList(toUpdate.size(), + currentChanges.size())) { if (!byChangeId.containsKey(c.getId())) toAbandon.add(c); } } if (t == null) { // This is the first push to this topic, we need to create it // - t = TopicUtil.createTopic(currentUser.getAccountId(), db, destTopicName, destBranch, message); + t = + TopicUtil.createTopic(currentUser.getAccountId(), db, + destTopicName, destBranch, message); topicId = t.getId(); // Show info in the command line @@ -1186,9 +1205,11 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // and currentChanges == toAbandon, is the user referring to // the same topic ? - Don't allow to do that, maybe he is wrong // - if ((toCreate.size() == cOrder.size()) && (toAbandon.equals(currentChanges))) { - reject(newChange, "This new push has nothing in common with the previous one. " + - "Please, abandon this topic or create a new one"); + if ((toCreate.size() == cOrder.size()) + && (toAbandon.equals(currentChanges))) { + reject(newChange, + "This new push has nothing in common with the previous one. " + + "Please, abandon this topic or create a new one"); return; } t = TopicUtil.setUpTopic(t, db, currentUser.getAccountId()); @@ -1196,7 +1217,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // Show info in the command line // rp.sendMessage(""); - rp.sendMessage("New ChangeSet (" + t.currentChangeSetId().get() + ") in topic " + topicId); + rp.sendMessage("New ChangeSet (" + t.currentChangeSetId().get() + + ") in topic " + topicId); } } else { // That means that there were no additions or replacements @@ -1204,11 +1226,16 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { // Collections.reverse(currentChanges); for (Change c : currentChanges) { - final RevId currentRevId = db.patchSets().get(c.currentPatchSetId()).getRevision(); + final RevId currentRevId = + db.patchSets().get(c.currentPatchSetId()).getRevision(); final RevId last = new RevId(lastIncluded.name()); if (currentRevId.get().equals(last.get())) { - toAbandon = new ArrayList<Change>(currentChanges.subList(0, currentChanges.indexOf(c))); - toUpdate = new ArrayList<Change>(currentChanges.subList(currentChanges.indexOf(c), currentChanges.size())); + toAbandon = + new ArrayList<Change>(currentChanges.subList(0, + currentChanges.indexOf(c))); + toUpdate = + new ArrayList<Change>(currentChanges.subList( + currentChanges.indexOf(c), currentChanges.size())); break; } } @@ -1225,7 +1252,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { if (!toUpdate.isEmpty()) updateChangeSetId(toUpdate, t.currentChangeSetId()); } catch (OrmException e) { - log.error("Error creating Topic " + destTopicName + " for commit set.", e); + log.error("Error creating Topic " + destTopicName + " for commit set.", + e); reject(newChange, "database error"); return; } @@ -1264,14 +1292,14 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { if (!toAbandon.isEmpty()) rp.sendMessage(""); for (final Change c : toAbandon) { try { - ChangeUtil.abandon(c.currentPatchSetId(), currentUser, - abandonMessage + topicId.get(), db, null, hooks, false); + ChangeUtil.abandon(c.currentPatchSetId(), currentUser, abandonMessage + + topicId.get(), db, null, hooks, false); // Show info in the command line // - rp.sendMessage("Change " + c.getId().get() + - " (belonging to the changeset " + t.currentChangeSetId().get() + - ") in topic " + topicId + " abandoned"); + rp.sendMessage("Change " + c.getId().get() + + " (belonging to the changeset " + t.currentChangeSetId().get() + + ") in topic " + topicId + " abandoned"); } catch (NoSuchChangeException e) { reject(newChange, "Problem when abandoning change " + c.getId()); restoreOnTopicError(t); @@ -1326,30 +1354,36 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { replaceByCommit.clear(); try { - final List<ChangeSetElement> toDelete = db.changeSetElements().byChangeSet(csi).toList(); + final List<ChangeSetElement> toDelete = + db.changeSetElements().byChangeSet(csi).toList(); db.changeSetElements().delete(toDelete); } catch (OrmException e) { - log.error("Cannot delete ChangeSet elements when recovering from an error " + e); + log.error("Cannot delete ChangeSet elements when recovering from an error " + + e); return; } if (t.currentChangeSetId().get() > 1) { - final ChangeSet.Id previousCSId = new ChangeSet.Id(t.getId(), csi.get() - 1); + final ChangeSet.Id previousCSId = + new ChangeSet.Id(t.getId(), csi.get() - 1); final ChangeSetInfo csInfo = new ChangeSetInfo(previousCSId); t.setCurrentChangeSet(csInfo); try { final ChangeSet toDeleteCS = db.changeSets().get(csi); db.topics().update(Collections.singleton(t)); - if (toDeleteCS != null) db.changeSets().delete(Collections.singleton(toDeleteCS)); + if (toDeleteCS != null) + db.changeSets().delete(Collections.singleton(toDeleteCS)); } catch (OrmException e) { - log.error("Cannot recover previous state of a topic when recovering from an error " + e); + log.error("Cannot recover previous state of a topic when recovering from an error " + + e); return; } } else { try { final ChangeSet toDeleteCS = db.changeSets().get(csi); db.topics().delete(Collections.singleton(t)); - if (toDeleteCS != null) db.changeSets().delete(Collections.singleton(toDeleteCS)); + if (toDeleteCS != null) + db.changeSets().delete(Collections.singleton(toDeleteCS)); } catch (OrmException e) { log.error("Cannot delete topic when recovering from an error " + e); return; @@ -1362,7 +1396,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { for (Change c : currentChanges) { try { ChangeSetElement.Key cseKey = new ChangeSetElement.Key(c.getId(), csId); - ChangeSetElement cse = new ChangeSetElement(cseKey, currentChanges.indexOf(c)); + ChangeSetElement cse = + new ChangeSetElement(cseKey, currentChanges.indexOf(c)); db.changeSetElements().insert(Collections.singleton(cse)); } catch (OrmException e) { @@ -1377,8 +1412,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } private void createChange(final RevWalk walk, final RevCommit c, - final Topic.Id topicId, final int pos) - throws OrmException, IOException { + final Topic.Id topicId, final int pos) throws OrmException, IOException { walk.parseBody(c); warnMalformedMessage(c); @@ -1415,7 +1449,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { ChangeSet.Id currentChangeSetId = topic.currentChangeSetId(); change.setTopicId(topicId); - final ChangeSetElement.Key cseKey = new ChangeSetElement.Key(change.getId(), currentChangeSetId); + final ChangeSetElement.Key cseKey = + new ChangeSetElement.Key(change.getId(), currentChangeSetId); final ChangeSetElement cse = new ChangeSetElement(cseKey, pos); db.changeSetElements().insert(Collections.singleton(cse)); } @@ -1450,23 +1485,24 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { if (authorId != null && haveApprovals.add(authorId)) { insertDummyApproval(change, ps.getId(), authorId, catId, db); if (topicId != null) { - insertDummyApproval(db.topics().get(topicId), db.topics().get(topicId).currentChangeSetId(), - authorId, catId, db); + insertDummyApproval(db.topics().get(topicId), db.topics() + .get(topicId).currentChangeSetId(), authorId, catId, db); } } if (committerId != null && haveApprovals.add(committerId)) { insertDummyApproval(change, ps.getId(), committerId, catId, db); if (topicId != null) { - insertDummyApproval(db.topics().get(topicId), db.topics().get(topicId).currentChangeSetId(), - committerId, catId, db); + insertDummyApproval(db.topics().get(topicId), db.topics() + .get(topicId).currentChangeSetId(), committerId, catId, db); } } for (final Account.Id reviewer : reviewers) { if (haveApprovals.add(reviewer)) { insertDummyApproval(change, ps.getId(), reviewer, catId, db); if (topicId != null) { - insertDummyApproval(db.topics().get(topicId), db.topics().get(topicId).currentChangeSetId(), - reviewer, catId, db); + insertDummyApproval(db.topics().get(topicId), + db.topics().get(topicId).currentChangeSetId(), reviewer, catId, + db); } } } @@ -1531,7 +1567,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { private PatchSet.Id doReplace(final ReplaceRequest request) throws IOException, OrmException { - final String restoreMessage = "This change has been restored to be reviewed in topic "; + final String restoreMessage = + "This change has been restored to be reviewed in topic "; final RevCommit c = request.newCommit; final boolean fromTopic = request.csId != null; rp.getRevWalk().parseBody(c); @@ -1570,8 +1607,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { return null; } if (change.getStatus() == Change.Status.INTEGRATING) { - reject(request.cmd, "change " + request.ontoChange + " is already INTEGRATING"); - return null; + reject(request.cmd, "change " + request.ontoChange + + " is already INTEGRATING"); + return null; } if (change.getStatus().equals(AbstractEntity.Status.ABANDONED)) { // It is needed a special behavior in case we are working with topics @@ -1587,7 +1625,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { reject(request.cmd, "change " + request.ontoChange + " not found"); return null; } catch (InvalidChangeOperationException e) { - reject(request.cmd, "error when restoring the abandoned change " + request.ontoChange); + reject(request.cmd, "error when restoring the abandoned change " + + request.ontoChange); return null; } catch (EmailException e) { // This should never happen, as it will never send any email @@ -1741,10 +1780,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } // ApprovalCategory.SUBMIT is still in db but not relevant in git-store - if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId()) && - !ApprovalCategory.STAGING.equals(a.getCategoryId())) { - final ApprovalType type = - approvalTypes.byId(a.getCategoryId()); + if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId()) + && !ApprovalCategory.STAGING.equals(a.getCategoryId())) { + final ApprovalType type = approvalTypes.byId(a.getCategoryId()); if (a.getPatchSetId().equals(priorPatchSet) && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) { // If there was a negative vote on the prior patch set, carry it @@ -1765,15 +1803,15 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } final ChangeMessage msg = - new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil - .messageUUID(db)), me, ps.getCreatedOn()); + new ChangeMessage(new ChangeMessage.Key(change.getId(), + ChangeUtil.messageUUID(db)), me, ps.getCreatedOn()); msg.setMessage("Uploaded patch set " + ps.getPatchSetId() + "."); db.changeMessages().insert(Collections.singleton(msg)); result.msg = msg; // Check staging status before change status is updated. - boolean inStaging = (change.getStatus() == Change.Status.STAGED - || change.getStatus() == Change.Status.STAGING); + boolean inStaging = + (change.getStatus() == Change.Status.STAGED || change.getStatus() == Change.Status.STAGING); if (result.mergedIntoRef != null) { // Change was already submitted to a branch, close it. @@ -1804,9 +1842,11 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { db.changeMessages().delete(Collections.singleton(msg)); reject(request.cmd, "change is closed"); return null; - } else if (request.csId != null) { - final ChangeSetElement.Key cseKey = new ChangeSetElement.Key(change.getId(), request.csId); - final ChangeSetElement cse = new ChangeSetElement(cseKey, request.position); + } else if (request.csId != null) { + final ChangeSetElement.Key cseKey = + new ChangeSetElement.Key(change.getId(), request.csId); + final ChangeSetElement cse = + new ChangeSetElement(cseKey, request.position); db.changeSetElements().insert(Collections.singleton(cse)); } } @@ -1858,8 +1898,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { sendMergedEmail(result); if (inStaging) { try { - ChangeUtil.rebuildStaging(change.getDest(), currentUser, db, - repo, mergeFactory, merger, hooks); + ChangeUtil.rebuildStaging(change.getDest(), currentUser, db, repo, + mergeFactory, merger, hooks); } catch (NoSuchRefException e) { // Destination branch not available. log.error("Could not rebuild staging branch. No destination branch.", e); @@ -1925,8 +1965,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final Account.Id forAccount, final ApprovalCategory.Id catId, final ReviewDb db) throws OrmException { final ChangeSetApproval ca = - new ChangeSetApproval(new ChangeSetApproval.Key(csId, forAccount, catId), - (short) 0); + new ChangeSetApproval( + new ChangeSetApproval.Key(csId, forAccount, catId), (short) 0); ca.cache(topic); db.changeSetApprovals().insert(Collections.singleton(ca)); } @@ -1967,9 +2007,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final int position; ReplaceRequest(final Change.Id toChange, final RevCommit newCommit, - final ReceiveCommand cmd, - final Topic.Id topicId, final ChangeSet.Id csId, - final int position) { + final ReceiveCommand cmd, final Topic.Id topicId, + final ChangeSet.Id csId, final int position) { this.ontoChange = toChange; this.newCommit = newCommit; this.cmd = cmd; @@ -2089,8 +2128,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final List<String> idList = c.getFooterLines(CHANGE_ID); if (idList.isEmpty()) { - if (project.isRequireChangeID() && (cmd.getRefName().startsWith(NEW_CHANGE) - || NEW_PATCHSET.matcher(cmd.getRefName()).matches())) { + if (project.isRequireChangeID() + && (cmd.getRefName().startsWith(NEW_CHANGE) || NEW_PATCHSET.matcher( + cmd.getRefName()).matches())) { String errMsg = "missing Change-Id in commit message"; reject(cmd, errMsg); rp.sendMessage(getFixedCommitMsgWithChangeId(errMsg, c)); @@ -2102,7 +2142,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } else { final String v = idList.get(idList.size() - 1).trim(); if (!v.matches("^I[0-9a-f]{8,}.*$")) { - final String errMsg = "missing or invalid Change-Id line format in commit message"; + final String errMsg = + "missing or invalid Change-Id line format in commit message"; reject(cmd, errMsg); rp.sendMessage(getFixedCommitMsgWithChangeId(errMsg, c)); return false; @@ -2121,15 +2162,17 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { private String getFixedCommitMsgWithChangeId(String errMsg, RevCommit c) { // We handle 3 cases: // 1. No change id in the commit message at all. - // 2. change id last in the commit message but missing empty line to create the footer. - // 3. there is a change-id somewhere in the commit message, but we ignore it. + // 2. change id last in the commit message but missing empty line to create + // the footer. + // 3. there is a change-id somewhere in the commit message, but we ignore + // it. final String changeId = "Change-Id:"; StringBuilder sb = new StringBuilder(); sb.append("ERROR: ").append(errMsg); sb.append("\n"); sb.append("Suggestion for commit message:\n"); - if (c.getFullMessage().indexOf(changeId)==-1) { + if (c.getFullMessage().indexOf(changeId) == -1) { sb.append(c.getFullMessage()); sb.append("\n"); sb.append(changeId).append(" I").append(c.name()); @@ -2137,7 +2180,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { String lines[] = c.getFullMessage().trim().split("\n"); String lastLine = lines.length > 0 ? lines[lines.length - 1] : ""; - if (lastLine.indexOf(changeId)==0) { + if (lastLine.indexOf(changeId) == 0) { for (int i = 0; i < lines.length - 1; i++) { sb.append(lines[i]); sb.append("\n"); @@ -2160,7 +2203,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { StringBuilder sb = new StringBuilder(); sb.append("\n"); sb.append("ERROR: In commit " + c.name() + "\n"); - sb.append("ERROR: " + type + " email address " + who.getEmailAddress() + "\n"); + sb.append("ERROR: " + type + " email address " + who.getEmailAddress() + + "\n"); sb.append("ERROR: does not match your user account.\n"); sb.append("ERROR:\n"); if (currentUser.getEmailAddresses().isEmpty()) { @@ -2174,7 +2218,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { sb.append("ERROR:\n"); if (canonicalWebUrl != null) { sb.append("ERROR: To register an email address, please visit:\n"); - sb.append("ERROR: " + canonicalWebUrl + "#" + PageLinks.SETTINGS_CONTACT + "\n"); + sb.append("ERROR: " + canonicalWebUrl + "#" + PageLinks.SETTINGS_CONTACT + + "\n"); } sb.append("\n"); getReceivePack().sendMessage(sb.toString()); @@ -2225,8 +2270,9 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } final Map<ObjectId, Ref> byCommit = changeRefsById(); - final Map<Change.Key, Change.Id> byKey = openChangesByKey( - new Branch.NameKey(project.getNameKey(), cmd.getRefName())); + final Map<Change.Key, Change.Id> byKey = + openChangesByKey(new Branch.NameKey(project.getNameKey(), + cmd.getRefName())); final List<ReplaceRequest> toClose = new ArrayList<ReplaceRequest>(); RevCommit c; while ((c = rw.next()) != null) { @@ -2340,8 +2386,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } msgBuf.append("."); final ChangeMessage msg = - new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil - .messageUUID(db)), currentUser.getAccountId()); + new ChangeMessage(new ChangeMessage.Key(change.getId(), + ChangeUtil.messageUUID(db)), currentUser.getAccountId()); msg.setMessage(msgBuf.toString()); db.changeMessages().insert(Collections.singleton(msg)); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java index e3d984601b..9cc6fe3bd8 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java @@ -232,9 +232,6 @@ public class StagingApprove extends BaseCommand { throw e; } - for (PatchSet patch : toApprove) { - ChangeUtil.submit(patch.getId(), currentUser, db, opFactory, merger); - } // Rebuild staging branch. ChangeUtil.rebuildStaging(destination, currentUser, db, git, opFactory, merger, hooks); @@ -313,15 +310,22 @@ public class StagingApprove extends BaseCommand { // TopicChangeControl. final TopicControl topicControl = topicControlFactory.validateFor(topicId); - List<ChangeSet> changeSets = db.changeSets().byTopic(topicId).toList(); - for (ChangeSet changeSet : changeSets) { - CanSubmitResult result = - topicControl.canSubmit(db, changeSet.getId(), approvalTypes, - topicFunctionStateFactory); - if (result != CanSubmitResult.OK) { - throw new UnloggedFailure(1, result.getMessage()); - } + Topic topic = db.topics().get(topicId); + // Only validate most current change set of topic + ChangeSet changeSet = db.changeSets().get(topic.currentChangeSetId()); + CanSubmitResult result = topicControl.canSubmit(db, changeSet.getId(), approvalTypes, topicFunctionStateFactory); + if (result != CanSubmitResult.OK) { + throw new UnloggedFailure(1, result.getMessage()); } +// List<ChangeSet> changeSets = db.changeSets().byTopic(topicId).toList(); +// for (ChangeSet changeSet : changeSets) { +// CanSubmitResult result = +// topicControl.canSubmit(db, changeSet.getId(), approvalTypes, +// topicFunctionStateFactory); +// if (result != CanSubmitResult.OK) { +// throw new UnloggedFailure(1, result.getMessage()); +// } +// } } else { // Change is not part of a topic. Validate it with ChangeControl. final ChangeControl changeControl = |