Merge "Add an 'Ack' button next to the 'Done' button"
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 41c464f..87e5031 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -482,6 +482,14 @@
Certain operations in Gerrit can be validated by plugins by
implementing the corresponding link:config-validation.html[listeners].
+[[change-message-modifier]]
+== Change Message Modifier
+
+`com.google.gerrit.server.git.ChangeMessageModifier`:
+plugins implementing this can modify commit message of the change being
+submitted by Rebase Always and Cherry Pick submit strategies as well as
+change being queried with COMMIT_FOOTERS option.
+
[[receive-pack]]
== Receive Pack Initializers
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 5a6c229..c2618f5 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -73,6 +73,8 @@
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -91,6 +93,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ChangeControl;
@@ -129,6 +132,9 @@
@Inject
private BatchUpdate.Factory updateFactory;
+ @Inject
+ private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+
@Before
public void setTimeForTesting() {
systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
@@ -1810,6 +1816,40 @@
}
@Test
+ public void customCommitFooters() throws Exception {
+ PushOneCommit.Result change = createChange();
+ RegistrationHandle handle =
+ changeMessageModifiers.add(new ChangeMessageModifier() {
+ @Override
+ public String onSubmit(String newCommitMessage, RevCommit original,
+ RevCommit mergeTip, Branch.NameKey destination) {
+ return newCommitMessage + "Custom: " + destination.get();
+ }
+ });
+ ChangeInfo actual;
+ try {
+ EnumSet<ListChangesOption> options = EnumSet.of(
+ ListChangesOption.ALL_REVISIONS, ListChangesOption.COMMIT_FOOTERS);
+ actual = gApi.changes().id(change.getChangeId()).get(options);
+ } finally {
+ handle.remove();
+ }
+ List<String> footers = new ArrayList<>(Arrays.asList(
+ actual.revisions.get(change.getCommit().getName()).commitWithFooters
+ .split("\\n")));
+ // remove subject + blank line
+ footers.remove(0);
+ footers.remove(0);
+
+ List<String> expectedFooters =
+ Arrays.asList(
+ "Change-Id: " + change.getChangeId(), "Reviewed-on: "
+ + canonicalWebUrl.get() + change.getChange().getId(),
+ "Custom: refs/heads/master");
+ assertThat(footers).containsExactlyElementsIn(expectedFooters);
+ }
+
+ @Test
public void defaultSearchDoesNotTouchDatabase() throws Exception {
setApiUser(admin);
PushOneCommit.Result r1 = createChange();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index af43373..0a3b217 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -19,17 +19,23 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.Submit.TestSubmitInput;
+import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
+import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -40,6 +46,8 @@
import java.util.List;
public class SubmitByCherryPickIT extends AbstractSubmit {
+ @Inject
+ private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Override
protected SubmitType getSubmitType() {
@@ -89,6 +97,31 @@
}
@Test
+ public void changeMessageOnSubmit() throws Exception {
+ PushOneCommit.Result change = createChange();
+ RegistrationHandle handle =
+ changeMessageModifiers.add(new ChangeMessageModifier() {
+ @Override
+ public String onSubmit(String newCommitMessage, RevCommit original,
+ RevCommit mergeTip, Branch.NameKey destination) {
+ return newCommitMessage + "Custom: " + destination.get();
+ }
+ });
+ try {
+ submit(change.getChangeId());
+ } finally {
+ handle.remove();
+ }
+ testRepo.git().fetch().setRemote("origin").call();
+ ChangeInfo info = get(change.getChangeId());
+ RevCommit c = testRepo.getRevWalk()
+ .parseCommit(ObjectId.fromString(info.currentRevision));
+ testRepo.getRevWalk().parseBody(c);
+ assertThat(c.getFooterLines("Custom")).containsExactly("refs/heads/master");
+ assertThat(c.getFooterLines(FooterConstants.REVIEWED_ON)).hasSize(1);
+ }
+
+ @Test
@TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
public void submitWithContentMerge() throws Exception {
RevCommit initialHead = getRemoteHead();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index d1c9577..0389417 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -22,12 +22,21 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.server.git.ChangeMessageModifier;
+import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
+import java.util.List;
+
public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase {
+ @Inject
+ private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Override
protected SubmitType getSubmitType() {
@@ -76,6 +85,41 @@
assertLatestRevisionHasFooters(change2);
}
+ @Test
+ @TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
+ public void changeMessageOnSubmit() throws Exception {
+ PushOneCommit.Result change1 = createChange();
+ PushOneCommit.Result change2 = createChange();
+
+ RegistrationHandle handle =
+ changeMessageModifiers.add(new ChangeMessageModifier() {
+ @Override
+ public String onSubmit(String newCommitMessage, RevCommit original,
+ RevCommit mergeTip, Branch.NameKey destination) {
+ List<String> custom = mergeTip.getFooterLines("Custom");
+ if (!custom.isEmpty()) {
+ newCommitMessage += "Custom-Parent: " + custom.get(0) + "\n";
+ }
+ return newCommitMessage + "Custom: " + destination.get();
+ }
+ });
+ try {
+ // change1 is a fast-forward, but should be rebased in cherry pick style
+ // anyway, making change2 not a fast-forward, requiring a rebase.
+ approve(change1.getChangeId());
+ submit(change2.getChangeId());
+ } finally {
+ handle.remove();
+ }
+ // ... but both changes should get custom footers.
+ assertThat(getCurrentCommit(change1).getFooterLines("Custom"))
+ .containsExactly("refs/heads/master");
+ assertThat(getCurrentCommit(change2).getFooterLines("Custom"))
+ .containsExactly("refs/heads/master");
+ assertThat(getCurrentCommit(change2).getFooterLines("Custom-Parent"))
+ .containsExactly("refs/heads/master");
+ }
+
private void assertLatestRevisionHasFooters(PushOneCommit.Result change)
throws Exception {
RevCommit c = getCurrentCommit(change);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
index f1489bb..7d6b1c3 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
@@ -113,6 +113,8 @@
if (currentAssignee != null) {
suggestBox.setText(FormatUtil.nameEmail(currentAssignee));
suggestBox.selectAll();
+ } else {
+ suggestBox.setText("");
}
}
@@ -137,7 +139,7 @@
}
private void editAssignee(final String assignee) {
- if (assignee.isEmpty()) {
+ if (assignee.trim().isEmpty()) {
ChangeApi.deleteAssignee(changeId.get(),
new GerritCallback<AccountInfo>() {
@Override
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 a008149..84c2403 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
@@ -122,7 +122,7 @@
AsyncCallback<AccountInfo> cb) {
AssigneeInput input = AssigneeInput.create();
input.assignee(user);
- change(id).view("assignee").put(user, cb);
+ change(id).view("assignee").put(input, cb);
}
public static RestApi comments(int id) {
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index faddd6c..e6d395c 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
-
import static java.util.stream.Collectors.toList;
import com.google.common.base.Throwables;
diff --git a/gerrit-patch-jgit/BUILD b/gerrit-patch-jgit/BUILD
index eff4e5a..f908d83 100644
--- a/gerrit-patch-jgit/BUILD
+++ b/gerrit-patch-jgit/BUILD
@@ -27,12 +27,12 @@
genrule2(
name = 'jgit_edit_src',
cmd = ' && '.join([
- 'unzip -qd $$TMP $(location @jgit//src) ' +
+ 'unzip -qd $$TMP $(location @jgit//jar:src) ' +
'org/eclipse/jgit/diff/Edit.java',
'cd $$TMP',
'zip -Dq $$ROOT/$@ org/eclipse/jgit/diff/Edit.java',
]),
- tools = ['@jgit//src'],
+ tools = ['@jgit//jar:src'],
outs = [ 'edit.srcjar' ],
)
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/ConsoleUI.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
index e210d5b..78ea5a0 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
@@ -87,6 +87,12 @@
/** Prompt the user for a password, returning the string; null if blank. */
public abstract String password(String fmt, Object... args);
+ /** Display an error message on the system stderr. */
+ public void error(String format, Object... args) {
+ System.err.println(String.format(format, args));
+ System.err.flush();
+ }
+
/** Prompt the user to make a choice from an enumeration's values. */
public abstract <T extends Enum<?>> T readEnum(T def, String fmt,
Object... args);
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
index 0f3d45c..fbaabc6 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
@@ -143,6 +143,9 @@
}
public static Id fromRef(String ref) {
+ if (RefNames.isRefsEdit(ref)) {
+ return fromEditRefPart(ref);
+ }
int cs = startIndex(ref);
if (cs < 0) {
return null;
@@ -156,6 +159,42 @@
return null;
}
+ public static Id fromAllUsersRef(String ref) {
+ if (ref == null) {
+ return null;
+ }
+ String prefix;
+ if (ref.startsWith(RefNames.REFS_STARRED_CHANGES)) {
+ prefix = RefNames.REFS_STARRED_CHANGES;
+ } else if (ref.startsWith(RefNames.REFS_DRAFT_COMMENTS)) {
+ prefix = RefNames.REFS_DRAFT_COMMENTS;
+ } else {
+ return null;
+ }
+ int cs = startIndex(ref, prefix);
+ if (cs < 0) {
+ return null;
+ }
+ int ce = nextNonDigit(ref, cs);
+ if (ce < ref.length() && ref.charAt(ce) == '/'
+ && isNumeric(ref, ce + 1)) {
+ return new Change.Id(Integer.parseInt(ref.substring(cs, ce)));
+ }
+ return null;
+ }
+
+ private static boolean isNumeric(String s, int off) {
+ if (off >= s.length()) {
+ return false;
+ }
+ for (int i = off; i < s.length(); i++) {
+ if (!Character.isDigit(s.charAt(i))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public static Id fromEditRefPart(String ref) {
int startChangeId = ref.indexOf(RefNames.EDIT_PREFIX) +
RefNames.EDIT_PREFIX.length();
@@ -173,12 +212,16 @@
}
static int startIndex(String ref) {
- if (ref == null || !ref.startsWith(REFS_CHANGES)) {
+ return startIndex(ref, REFS_CHANGES);
+ }
+
+ static int startIndex(String ref, String expectedPrefix) {
+ if (ref == null || !ref.startsWith(expectedPrefix)) {
return -1;
}
// Last 2 digits.
- int ls = REFS_CHANGES.length();
+ int ls = expectedPrefix.length();
int le = nextNonDigit(ref, ls);
if (le - ls != 2 || le >= ref.length() || ref.charAt(le) != '/') {
return -1;
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
index 33479d4..c7c870e 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
@@ -195,7 +195,8 @@
}
public static boolean isRefsEdit(String ref) {
- return ref.startsWith(REFS_USERS) && ref.contains(EDIT_PREFIX);
+ return ref != null && ref.startsWith(REFS_USERS)
+ && ref.contains(EDIT_PREFIX);
}
public static boolean isRefsUsers(String ref) {
diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
index cf2d289..2aa863e 100644
--- a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
+++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
@@ -64,6 +64,15 @@
}
@Test
+ public void parseEditRefNames() {
+ assertRef(5, "refs/users/34/1234/edit-5/1");
+ assertRef(5, "refs/users/34/1234/edit-5");
+ assertNotRef("refs/changes/34/1234/edit-5/1");
+ assertNotRef("refs/users/34/1234/EDIT-5/1");
+ assertNotRef("refs/users/34/1234");
+ }
+
+ @Test
public void parseChangeMetaRefNames() {
assertRef(1, "refs/changes/01/1/meta");
assertRef(1234, "refs/changes/34/1234/meta");
@@ -74,6 +83,44 @@
}
@Test
+ public void parseRobotCommentRefNames() {
+ assertRef(1, "refs/changes/01/1/robot-comments");
+ assertRef(1234, "refs/changes/34/1234/robot-comments");
+
+ assertNotRef("refs/changes/01/1/robot-comment");
+ assertNotRef("refs/changes/01/1/ROBOT-COMMENTS");
+ assertNotRef("refs/changes/01/1/1/robot-comments");
+ }
+
+ @Test
+ public void parseStarredChangesRefNames() {
+ assertAllUsersRef(1, "refs/starred-changes/01/1/1001");
+ assertAllUsersRef(1234, "refs/starred-changes/34/1234/1001");
+
+ assertNotRef("refs/starred-changes/01/1/1001");
+ assertNotAllUsersRef(null);
+ assertNotAllUsersRef("refs/starred-changes/01/1/1xx1");
+ assertNotAllUsersRef("refs/starred-changes/01/1/");
+ assertNotAllUsersRef("refs/starred-changes/01/1");
+ assertNotAllUsersRef("refs/starred-changes/35/1234/1001");
+ assertNotAllUsersRef("refs/starred-changeS/01/1/1001");
+ }
+
+ @Test
+ public void parseDraftRefNames() {
+ assertAllUsersRef(1, "refs/draft-comments/01/1/1001");
+ assertAllUsersRef(1234, "refs/draft-comments/34/1234/1001");
+
+ assertNotRef("refs/draft-comments/01/1/1001");
+ assertNotAllUsersRef(null);
+ assertNotAllUsersRef("refs/draft-comments/01/1/1xx1");
+ assertNotAllUsersRef("refs/draft-comments/01/1/");
+ assertNotAllUsersRef("refs/draft-comments/01/1");
+ assertNotAllUsersRef("refs/draft-comments/35/1234/1001");
+ assertNotAllUsersRef("refs/draft-commentS/01/1/1001");
+ }
+
+ @Test
public void toRefPrefix() {
assertThat(new Change.Id(1).toRefPrefix())
.isEqualTo("refs/changes/01/1/");
@@ -110,6 +157,15 @@
assertThat(Change.Id.fromRef(refName)).isNull();
}
+ private static void assertAllUsersRef(int changeId, String refName) {
+ assertThat(Change.Id.fromAllUsersRef(refName))
+ .isEqualTo(new Change.Id(changeId));
+ }
+
+ private static void assertNotAllUsersRef(String refName) {
+ assertThat(Change.Id.fromAllUsersRef(refName)).isNull();
+ }
+
private static void assertRefPart(int changeId, String refName) {
assertEquals(new Change.Id(changeId), Change.Id.fromRefPart(refName));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java
index c0d81cb..24a0dae 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java
@@ -16,15 +16,14 @@
import static com.google.gerrit.server.config.ConfigUtil.loadSection;
import static com.google.gerrit.server.config.ConfigUtil.skipField;
+import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE;
+import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
import static com.google.gerrit.server.git.UserConfigSections.KEY_ID;
import static com.google.gerrit.server.git.UserConfigSections.KEY_MATCH;
import static com.google.gerrit.server.git.UserConfigSections.KEY_TARGET;
import static com.google.gerrit.server.git.UserConfigSections.KEY_TOKEN;
import static com.google.gerrit.server.git.UserConfigSections.KEY_URL;
import static com.google.gerrit.server.git.UserConfigSections.URL_ALIAS;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
-
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
@@ -169,7 +168,6 @@
VersionedAccountPreferences v, VersionedAccountPreferences d) {
r.changeTable = changeTable(v);
- Config cfg = v.getConfig();
if (r.changeTable.isEmpty() && !v.isDefaults()) {
r.changeTable = changeTable(d);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
index 08b7b0b..3714cee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
@@ -15,14 +15,13 @@
package com.google.gerrit.server.account;
import static com.google.gerrit.server.config.ConfigUtil.storeSection;
+import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
import static com.google.gerrit.server.git.UserConfigSections.KEY_ID;
import static com.google.gerrit.server.git.UserConfigSections.KEY_MATCH;
import static com.google.gerrit.server.git.UserConfigSections.KEY_TARGET;
import static com.google.gerrit.server.git.UserConfigSections.KEY_TOKEN;
import static com.google.gerrit.server.git.UserConfigSections.KEY_URL;
import static com.google.gerrit.server.git.UserConfigSections.URL_ALIAS;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 3f3afa5..680234c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -121,6 +121,7 @@
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -1111,9 +1112,15 @@
out.commit = toCommit(ctl, rw, commit, has(WEB_LINKS), fillCommit);
}
if (addFooters) {
+ Ref ref = repo.exactRef(in.getRefName());
+ RevCommit mergeTip = null;
+ if (ref != null){
+ mergeTip = rw.parseCommit(ref.getObjectId());
+ rw.parseBody(mergeTip);
+ }
out.commitWithFooters = mergeUtilFactory
.create(projectCache.get(project))
- .createDetailedCommitMessage(commit, ctl, in.getId());
+ .createCommitMessageOnSubmit(commit, mergeTip, ctl, in.getId());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java
index 8236d3d..92b4150 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java
@@ -23,7 +23,8 @@
}
public String revertChangeDefaultMessage;
- public String reviewerNotFound;
+ public String reviewerNotFoundUser;
+ public String reviewerNotFoundUserOrGroup;
public String groupIsNotAllowed;
public String groupHasTooManyMembers;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index cd62e45..ae440c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -209,7 +209,7 @@
reviewerInput.notify = NotifyHandling.NONE;
PostReviewers.Addition result = postReviewers.prepareApplication(
- revision.getChangeResource(), reviewerInput);
+ revision.getChangeResource(), reviewerInput, true);
reviewerJsonResults.put(reviewerInput.reviewer, result.result);
if (result.result.error != null) {
hasError = true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index f0af5da..0cdddca 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -147,7 +147,7 @@
throw new BadRequestException("missing reviewer field");
}
- Addition addition = prepareApplication(rsrc, input);
+ Addition addition = prepareApplication(rsrc, input, true);
if (addition.op == null) {
return addition.result;
}
@@ -161,18 +161,24 @@
return addition.result;
}
- public Addition prepareApplication(ChangeResource rsrc, AddReviewerInput input)
- throws OrmException, RestApiException, IOException {
+ public Addition prepareApplication(ChangeResource rsrc,
+ AddReviewerInput input, boolean allowGroup)
+ throws OrmException, RestApiException, IOException {
Account.Id accountId;
try {
accountId = accounts.parse(input.reviewer).getAccountId();
} catch (UnprocessableEntityException e) {
- try {
- return putGroup(rsrc, input);
- } catch (UnprocessableEntityException e2) {
- throw new UnprocessableEntityException(MessageFormat
- .format(ChangeMessages.get().reviewerNotFound, input.reviewer));
+ if (allowGroup) {
+ try {
+ return putGroup(rsrc, input);
+ } catch (UnprocessableEntityException e2) {
+ throw new UnprocessableEntityException(MessageFormat.format(
+ ChangeMessages.get().reviewerNotFoundUserOrGroup,
+ input.reviewer));
+ }
}
+ throw new UnprocessableEntityException(MessageFormat
+ .format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
}
return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
input.state(), input.notify);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
index 5002436..271fd33 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
@@ -65,7 +64,7 @@
if (!rsrc.getControl().canEditAssignee()) {
throw new AuthException("Changing Assignee not permitted");
}
- if (Strings.isNullOrEmpty(input.assignee)) {
+ if (input.assignee == null || input.assignee.trim().isEmpty()) {
throw new BadRequestException("missing assignee field");
}
@@ -91,7 +90,7 @@
reviewerInput.state = ReviewerState.CC;
reviewerInput.confirmed = true;
reviewerInput.notify = NotifyHandling.NONE;
- return postReviewers.prepareApplication(rsrc, reviewerInput);
+ return postReviewers.prepareApplication(rsrc, reviewerInput, false);
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 7ece10f..9c51425 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -142,14 +142,6 @@
RevCommit original = rw.parseCommit(ObjectId.fromString(oldRev.get()));
rw.parseBody(original);
- String newCommitMessage;
- if (detailedCommitMessage) {
- newCommitMessage = newMergeUtil().createDetailedCommitMessage(original,
- ctl, originalPatchSet.getId());
- } else {
- newCommitMessage = original.getFullMessage();
- }
-
RevCommit baseCommit;
if (baseCommitish != null) {
baseCommit = rw.parseCommit(ctx.getRepository().resolve(baseCommitish));
@@ -159,6 +151,15 @@
ctx.getRepository(), ctx.getRevWalk()));
}
+ String newCommitMessage;
+ if (detailedCommitMessage) {
+ rw.parseBody(baseCommit);
+ newCommitMessage = newMergeUtil().createCommitMessageOnSubmit(original,
+ baseCommit, ctl, originalPatchSet.getId());
+ } else {
+ newCommitMessage = original.getFullMessage();
+ }
+
rebasedCommit = rebaseCommit(ctx, original, baseCommit, newCommitMessage);
RevId baseRevId = new RevId((baseCommitish != null) ? baseCommitish
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 6dd0fab..acae738 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -107,6 +107,7 @@
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.AbandonOp;
import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.EmailMerge;
import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitModules;
@@ -342,6 +343,7 @@
DynamicSet.bind(binder(), EventListener.class).to(EventsMetrics.class);
DynamicSet.setOf(binder(), UserScopedEventListener.class);
DynamicSet.setOf(binder(), CommitValidationListener.class);
+ DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
DynamicSet.setOf(binder(), MergeValidationListener.class);
DynamicSet.setOf(binder(), ProjectCreationValidationListener.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMessageModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMessageModifier.java
new file mode 100644
index 0000000..75911f3f
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMessageModifier.java
@@ -0,0 +1,53 @@
+// Copyright (C) 2016 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.server.git;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.reviewdb.client.Branch;
+
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/**
+ * Allows to modify the commit message for new commits generated by Rebase
+ * Always submit strategy.
+ *
+ * Invoked by Gerrit when all information about new commit is already known such
+ * as parent(s), tree hash, etc, but commit's message can still be modified.
+ */
+@ExtensionPoint
+public interface ChangeMessageModifier {
+
+ /**
+ * Implementation must return non-Null commit message.
+ *
+ * mergeTip and original commit are guaranteed to have their body parsed,
+ * meaning that their commit messages and footers can be accessed.
+ *
+ * @param newCommitMessage the new commit message that was result of either
+ * <ul>
+ * <li>{@link MergeUtil#createDetailedCommitMessage} called before</li>
+ * <li>other extensions or plugins implementing the same point and
+ * called before.</li>
+ * </ul>
+ * @param original the commit of the change being submitted. <b>Note that its
+ * commit message may be different than newCommitMessage argument.</b>
+ * @param mergeTip the current HEAD of the destination branch, which will be a
+ * parent of a new commit being generated
+ * @param destination the branch onto which the change is being submitted
+ * @return a new not null commit message.
+ */
+ String onSubmit(String newCommitMessage, RevCommit original,
+ RevCommit mergeTip, Branch.NameKey destination);
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 795bec8..0a4a430 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
@@ -25,6 +26,7 @@
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -33,6 +35,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
@@ -44,6 +47,7 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -98,6 +102,32 @@
*/
public class MergeUtil {
private static final Logger log = LoggerFactory.getLogger(MergeUtil.class);
+
+ static class PluggableCommitMessageGenerator {
+ private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+
+ @Inject
+ public PluggableCommitMessageGenerator(
+ DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
+ this.changeMessageModifiers = changeMessageModifiers;
+ }
+
+ public String generate(RevCommit original, RevCommit mergeTip,
+ ChangeControl ctl, String current) {
+ checkNotNull(original.getRawBuffer());
+ if (mergeTip != null) {
+ checkNotNull(mergeTip.getRawBuffer());
+ }
+ for (ChangeMessageModifier changeMessageModifier : changeMessageModifiers) {
+ current = changeMessageModifier.onSubmit(current, original,
+ mergeTip, ctl.getChange().getDest());
+ checkNotNull(current, changeMessageModifier.getClass().getName()
+ + ".OnSubmit returned null instead of new commit message");
+ }
+ return current;
+ }
+ }
+
private static final String R_HEADS_MASTER =
Constants.R_HEADS + Constants.MASTER;
@@ -123,25 +153,28 @@
private final ProjectState project;
private final boolean useContentMerge;
private final boolean useRecursiveMerge;
+ private final PluggableCommitMessageGenerator commitMessageGenerator;
@AssistedInject
MergeUtil(@GerritServerConfig Config serverConfig,
- final Provider<ReviewDb> db,
- final IdentifiedUser.GenericFactory identifiedUserFactory,
- @CanonicalWebUrl @Nullable final Provider<String> urlProvider,
- final ApprovalsUtil approvalsUtil,
- @Assisted final ProjectState project) {
+ Provider<ReviewDb> db,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ @CanonicalWebUrl @Nullable Provider<String> urlProvider,
+ ApprovalsUtil approvalsUtil,
+ PluggableCommitMessageGenerator commitMessageGenerator,
+ @Assisted ProjectState project) {
this(serverConfig, db, identifiedUserFactory, urlProvider, approvalsUtil,
- project, project.isUseContentMerge());
+ project, commitMessageGenerator, project.isUseContentMerge());
}
@AssistedInject
MergeUtil(@GerritServerConfig Config serverConfig,
- final Provider<ReviewDb> db,
- final IdentifiedUser.GenericFactory identifiedUserFactory,
- @CanonicalWebUrl @Nullable final Provider<String> urlProvider,
- final ApprovalsUtil approvalsUtil,
- @Assisted final ProjectState project,
+ Provider<ReviewDb> db,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ @CanonicalWebUrl @Nullable Provider<String> urlProvider,
+ ApprovalsUtil approvalsUtil,
+ @Assisted ProjectState project,
+ PluggableCommitMessageGenerator commitMessageGenerator,
@Assisted boolean useContentMerge) {
this.db = db;
this.identifiedUserFactory = identifiedUserFactory;
@@ -150,6 +183,7 @@
this.project = project;
this.useContentMerge = useContentMerge;
this.useRecursiveMerge = useRecursiveMerge(serverConfig);
+ this.commitMessageGenerator = commitMessageGenerator;
}
public CodeReviewCommit getFirstFastForward(
@@ -264,7 +298,7 @@
* @param psId
* @return new message
*/
- public String createDetailedCommitMessage(RevCommit n, ChangeControl ctl,
+ private String createDetailedCommitMessage(RevCommit n, ChangeControl ctl,
PatchSet.Id psId) {
Change c = ctl.getChange();
final List<FooterLine> footers = n.getFooterLines();
@@ -368,12 +402,32 @@
msgbuf.append('\n');
}
}
-
return msgbuf.toString();
}
- public String createDetailedCommitMessage(final CodeReviewCommit n) {
- return createDetailedCommitMessage(n, n.getControl(), n.getPatchsetId());
+ public String createCommitMessageOnSubmit(CodeReviewCommit n,
+ RevCommit mergeTip) {
+ return createCommitMessageOnSubmit(n, mergeTip, n.getControl(),
+ n.getPatchsetId());
+ }
+
+ /**
+ * Creates a commit message for a change, which can be customized by plugins.
+ *
+ * By default, adds footers to existing commit message based on the state of
+ * the change. Plugins implementing {@link ChangeMessageModifier} can modify
+ * the resulting commit message arbitrarily.
+ *
+ * @param n
+ * @param mergeTip
+ * @param ctl
+ * @param id
+ * @return new message
+ */
+ public String createCommitMessageOnSubmit(RevCommit n, RevCommit mergeTip,
+ ChangeControl ctl, Id id) {
+ return commitMessageGenerator.generate(n, mergeTip, ctl,
+ createDetailedCommitMessage(n, ctl, id));
}
private static boolean isCodeReview(LabelId id) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
index e9772e5..af08b63 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
@@ -33,6 +33,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.ReceiveCommand;
import java.io.IOException;
@@ -99,8 +100,10 @@
args.rw.parseBody(toMerge);
psId = ChangeUtil.nextPatchSetId(
args.repo, toMerge.change().currentPatchSetId());
+ RevCommit mergeTip = args.mergeTip.getCurrentTip();
+ args.rw.parseBody(mergeTip);
String cherryPickCmtMsg =
- args.mergeUtil.createDetailedCommitMessage(toMerge);
+ args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip);
PersonIdent committer = args.caller.newCommitterIdent(
ctx.getWhen(), args.serverIdent.getTimeZone());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
index 34d3a80..135db95 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
@@ -137,10 +137,10 @@
args.rw.parseBody(toMerge);
newPatchSetId = ChangeUtil.nextPatchSetId(
args.repo, toMerge.change().currentPatchSetId());
- // TODO(tandrii): add extension point to customize this commit message.
+ RevCommit mergeTip = args.mergeTip.getCurrentTip();
+ args.rw.parseBody(mergeTip);
String cherryPickCmtMsg =
- args.mergeUtil.createDetailedCommitMessage(toMerge);
-
+ args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip);
PersonIdent committer = args.caller.newCommitterIdent(ctx.getWhen(),
args.serverIdent.getTimeZone());
try {
@@ -162,8 +162,6 @@
// Stale read of patch set is ok; see comments in RebaseChangeOp.
PatchSet origPs = args.psUtil.get(ctx.getDb(),
toMerge.getControl().getNotes(), toMerge.getPatchsetId());
- // TODO(tandrii): add extension point to customize commit message while
- // rebasing.
rebaseOp = args.rebaseFactory.create(
toMerge.getControl(), origPs, args.mergeTip.getCurrentTip().name())
.setFireRevisionCreated(false)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index 1672eea..1d376d5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -971,7 +971,7 @@
input.editRefs().values().forEach(
r -> result.add(RefState.of(r).toByteArray(project)));
input.starRefs().values().forEach(
- r -> result.add(RefState.of(r.ref()).toByteArray(project)));
+ r -> result.add(RefState.of(r.ref()).toByteArray(args.allUsers)));
if (PrimaryStorage.of(input.change()) == PrimaryStorage.NOTE_DB) {
ChangeNotes notes = input.notes();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
index 0c3d89c..7c841c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
@@ -62,7 +62,7 @@
private static final Logger log =
LoggerFactory.getLogger(StalenessChecker.class);
- private final ImmutableSet<String> FIELDS = ImmutableSet.of(
+ public static final ImmutableSet<String> FIELDS = ImmutableSet.of(
ChangeField.CHANGE.getName(),
ChangeField.REF_STATE.getName(),
ChangeField.REF_STATE_PATTERN.getName());
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties
index f05f23b..f34c992 100644
--- a/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties
+++ b/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties
@@ -1,7 +1,8 @@
# Changes to this file should also be made in
# gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}.
-reviewerNotFound = {0} does not identify a registered user or group
+reviewerNotFoundUser = {0} does not identify a registered user
+reviewerNotFoundUserOrGroup = {0} does not identify a registered user or group
groupIsNotAllowed = The group {0} cannot be added as reviewer.
groupHasTooManyMembers = The group {0} has too many members to add them all as reviewers.
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index d22e7a8..5e5a047 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -21,6 +21,7 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toList;
import com.google.common.base.MoreObjects;
import com.google.common.collect.FluentIterable;
@@ -38,6 +39,7 @@
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -54,6 +56,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.Sequences;
@@ -66,10 +69,15 @@
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.index.IndexConfig;
+import com.google.gerrit.server.index.QueryOptions;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.index.change.IndexedChangeQuery;
+import com.google.gerrit.server.index.change.StalenessChecker;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
@@ -100,6 +108,7 @@
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
@@ -125,6 +134,7 @@
@Inject protected ChangeEditModifier changeEditModifier;
@Inject protected ChangeIndexCollection indexes;
@Inject protected ChangeIndexer indexer;
+ @Inject protected IndexConfig indexConfig;
@Inject protected InMemoryDatabase schemaFactory;
@Inject protected InMemoryRepositoryManager repoManager;
@Inject protected InternalChangeQuery internalChangeQuery;
@@ -1638,6 +1648,97 @@
assertQuery("has:edit");
}
+ @Test
+ public void refStateFields() throws Exception {
+ Account.Id user = createAccount("user");
+ Project.NameKey project = new Project.NameKey("repo");
+ TestRepository<Repo> repo = createProject(project.get());
+ String path = "file";
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one").add(path, "contents").create());
+ Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change.Id id = change.getId();
+ int c = id.get();
+ PatchSet ps = db.patchSets().get(change.currentPatchSetId());
+ requestContext.setContext(newRequestContext(user));
+
+ // Ensure one of each type of supported ref is present for the change. If
+ // any more refs are added, update this test to reflect them.
+
+ // Edit
+ assertThat(changeEditModifier.createEdit(change, ps))
+ .isEqualTo(RefUpdate.Result.NEW);
+
+ // Star
+ gApi.accounts()
+ .self()
+ .starChange(change.getId().toString());
+
+ if (notesMigration.readChanges()) {
+ // Robot comment.
+ ReviewInput rin = new ReviewInput();
+ RobotCommentInput rcin = new RobotCommentInput();
+ rcin.robotId = "happyRobot";
+ rcin.robotRunId = "1";
+ rcin.line = 1;
+ rcin.message = "nit: trailing whitespace";
+ rcin.path = path;
+ rin.robotComments = ImmutableMap.of(path, ImmutableList.of(rcin));
+ gApi.changes().id(c).current().review(rin);
+ }
+
+ // Draft.
+ DraftInput din = new DraftInput();
+ din.path = path;
+ din.line = 1;
+ din.message = "draft";
+ gApi.changes().id(c).current().createDraft(din);
+
+ if (notesMigration.readChanges()) {
+ // Force NoteDb primary.
+ change = ReviewDbUtil.unwrapDb(db).changes().get(id);
+ change.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+ ReviewDbUtil.unwrapDb(db).changes().update(Collections.singleton(change));
+ indexer.index(db, change);
+ }
+
+ QueryOptions opts = IndexedChangeQuery.createOptions(
+ indexConfig, 0, 1, StalenessChecker.FIELDS);
+ ChangeData cd = indexes.getSearchIndex().get(id, opts).get();
+
+ String cs = RefNames.shard(c);
+ int u = user.get();
+ String us = RefNames.shard(u);
+
+ List<String> expectedStates = Lists.newArrayList(
+ "repo:refs/users/" + us + "/edit-" + c + "/1",
+ "All-Users:refs/starred-changes/" + cs + "/" + u);
+ if (notesMigration.readChanges()) {
+ expectedStates.add("repo:refs/changes/" + cs + "/meta");
+ expectedStates.add("repo:refs/changes/" + cs + "/robot-comments");
+ expectedStates.add("All-Users:refs/draft-comments/" + cs + "/" + u);
+ }
+ assertThat(
+ cd.getRefStates().stream()
+ .map(String::new)
+ // Omit SHA-1, we're just concerned with the project/ref names.
+ .map(s -> s.substring(0, s.lastIndexOf(':')))
+ .collect(toList()))
+ .containsExactlyElementsIn(expectedStates);
+
+ List<String> expectedPatterns = Lists.newArrayList(
+ "repo:refs/users/*/edit-" + c + "/*");
+ if (notesMigration.readChanges()) {
+ expectedPatterns.add("All-Users:refs/starred-changes/" + cs + "/*");
+ expectedPatterns.add("All-Users:refs/draft-comments/" + cs + "/*");
+ }
+ assertThat(
+ cd.getRefStatePatterns().stream()
+ .map(String::new)
+ .collect(toList()))
+ .containsExactlyElementsIn(expectedPatterns);
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo)
throws Exception {
return newChange(repo, null, null, null, null);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
index 85b1f32..d3065df 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
@@ -44,7 +44,7 @@
try {
changeArgumentParser.addChange(token, changes, null, false);
} catch (UnloggedFailure e) {
- throw new IllegalArgumentException(e.getMessage(), e);
+ writeError("warning", e.getMessage());
} catch (OrmException e) {
throw new IllegalArgumentException("database is down", e);
}
diff --git a/lib/BUILD b/lib/BUILD
index f159a45..a4f1d51 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -32,7 +32,7 @@
java_library(
name = 'gwtjsonrpc_src',
- exports = ['@gwtjsonrpc//src'],
+ exports = ['@gwtjsonrpc//jar:src'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-Apache2.0'],
)
@@ -53,7 +53,7 @@
java_library(
name = 'gwtorm_client_src',
- exports = ['@gwtorm_client//src'],
+ exports = ['@gwtorm_client//jar:src'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-Apache2.0'],
)
diff --git a/lib/gwt/BUILD b/lib/gwt/BUILD
index 116f86c..5d826d2 100644
--- a/lib/gwt/BUILD
+++ b/lib/gwt/BUILD
@@ -32,15 +32,14 @@
java_library(
name = 'javax-validation_src',
- exports = ['@javax_validation//src'],
+ exports = ['@javax_validation//jar:src'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-Apache2.0'],
)
java_library(
name = 'jsinterop-annotations_src',
- exports = ['@jsinterop_annotations//src'],
+ exports = ['@jsinterop_annotations//jar:src'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-Apache2.0'],
)
-
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index a98b00b..c187cf2 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -393,7 +393,8 @@
ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.CURRENT_COMMIT,
ListChangesOption.DOWNLOAD_COMMANDS,
- ListChangesOption.SUBMITTABLE
+ ListChangesOption.SUBMITTABLE,
+ ListChangesOption.WEB_LINKS
);
return this._getChangeDetail(
changeNum, options, opt_errFn, opt_cancelCondition);
diff --git a/tools/bzl/gwt.bzl b/tools/bzl/gwt.bzl
index d14e96a..4866b23 100644
--- a/tools/bzl/gwt.bzl
+++ b/tools/bzl/gwt.bzl
@@ -81,7 +81,7 @@
'//gerrit-gwtexpui:CSS',
'//lib:gwtjsonrpc',
'//lib/gwt:dev',
- '@jgit//src',
+ '@jgit//jar:src',
]
USER_AGENT_XML = """<module rename-to='gerrit_ui'>
diff --git a/tools/bzl/maven_jar.bzl b/tools/bzl/maven_jar.bzl
index 41a5ef7..17d1423 100644
--- a/tools/bzl/maven_jar.bzl
+++ b/tools/bzl/maven_jar.bzl
@@ -49,24 +49,29 @@
version = version,
)
-# Provides the syntax "@jar_name//jar" for bin classifier
-# and "@jar_name//src" for sources
-def _generate_build_file(ctx, classifier, filename):
+def _generate_build_file(ctx, binjar, srcjar):
+ srcjar_attr = ""
+ if srcjar:
+ srcjar_attr = 'srcjar = "%s",' % srcjar
contents = """
# DO NOT EDIT: automatically generated BUILD file for maven_archive rule {rule_name}
+package(default_visibility = ['//visibility:public'])
java_import(
- name = '{classifier}',
- jars = ['{filename}'],
- visibility = ['//visibility:public']
+ name = 'jar',
+ {srcjar_attr}
+ jars = ['{binjar}'],
)
-filegroup(
- name = 'file',
- srcs = ['{filename}'],
- visibility = ['//visibility:public']
-)\n""".format(classifier = classifier,
+\n""".format(srcjar_attr = srcjar_attr,
rule_name = ctx.name,
- filename = filename)
- ctx.file('%s/BUILD' % ctx.path(classifier), contents, False)
+ binjar = binjar)
+ if srcjar:
+ contents += """
+java_import(
+ name = 'src',
+ jars = ['{srcjar}'],
+)
+""".format(srcjar = srcjar)
+ ctx.file('%s/BUILD' % ctx.path("jar"), contents, False)
def _maven_jar_impl(ctx):
"""rule to download a Maven archive."""
@@ -83,10 +88,6 @@
binjar_path = ctx.path('/'.join(['jar', binjar]))
binurl = url + '.jar'
- srcjar = jar + '-src.jar'
- srcjar_path = ctx.path('/'.join(['src', srcjar]))
- srcurl = url + '-sources.jar'
-
python = ctx.which("python")
script = ctx.path(ctx.attr._download_script)
@@ -100,16 +101,20 @@
if out.return_code:
fail("failed %s: %s" % (' '.join(args), out.stderr))
- _generate_build_file(ctx, "jar", binjar)
+ srcjar = None
if ctx.attr.src_sha1 or ctx.attr.attach_source:
+ srcjar = jar + '-src.jar'
+ srcurl = url + '-sources.jar'
+ srcjar_path = ctx.path('jar/' + srcjar)
args = [python, script, "-o", srcjar_path, "-u", srcurl]
if ctx.attr.src_sha1:
args.extend(['-v', ctx.attr.src_sha1])
out = ctx.execute(args)
if out.return_code:
fail("failed %s: %s" % (args, out.stderr))
- _generate_build_file(ctx, "src", srcjar)
+
+ _generate_build_file(ctx, binjar, srcjar)
maven_jar=repository_rule(
implementation=_maven_jar_impl,
diff --git a/tools/eclipse/project_bzl.py b/tools/eclipse/project_bzl.py
index 86fb527..6bc7398 100755
--- a/tools/eclipse/project_bzl.py
+++ b/tools/eclipse/project_bzl.py
@@ -210,7 +210,7 @@
if m:
prefix = m.group(1)
suffix = m.group(2)
- p = path.join(prefix, "src", "%s-src.jar" % suffix)
+ p = path.join(prefix, "jar", "%s-src.jar" % suffix)
if path.exists(p):
s = p
# TODO(davido): make plugins actually work
