Merge changes from topic "commit-validators-cleanup" into stable-2.15
* changes:
CommitValidators: trim "ERROR" shouting from "forge committer" check
ReceiveCommits: uniformize commit validation error messages.
Inline "Change-Id" string into error messages
Always end the "Change-Id missing" error message with \n.
Print only one hint about Change-Ids at a time
Clean up Change-Id hint text
CommitValidators: Replace indexOf calls with String#contains
CommitValidators: Prefer using Splitter to String.split
diff --git a/.mailmap b/.mailmap
index 0c691cf..42b713c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -11,6 +11,7 @@
Bruce Zu <bruce.zu.run10@gmail.com> <bruce.zu@sonymobile.com>
Carlos Eduardo Baldacin <carloseduardo.baldacin@sonyericsson.com> carloseduardo.baldacin <carloseduardo.baldacin@sonyericsson.com>
Dariusz Luksza <dluksza@collab.net> <dariusz@luksza.org>
+Darrien Glasser <darrien@arista.com> darrien <darrien@arista.com>
David Ostrovsky <david@ostrovsky.org> <d.ostrovsky@gmx.de>
David Ostrovsky <david@ostrovsky.org> <david.ostrovsky@gmail.com>
David Pursehouse <dpursehouse@collab.net> <david.pursehouse@sonymobile.com>
diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt
index a407856..605fc1f 100644
--- a/Documentation/cmd-review.txt
+++ b/Documentation/cmd-review.txt
@@ -21,7 +21,7 @@
[--verified <N>] [--code-review <N>]
[--label Label-Name=<N>]
[--tag TAG]
- {COMMIT | CHANGEID,PATCHSET}...
+ {COMMIT | CHANGENUMBER,PATCHSET}...
--
== DESCRIPTION
@@ -147,16 +147,21 @@
$ ssh -p 29418 review.example.com gerrit review --verified +1 c0ff33
----
+Approve the change with change number 8242 and patch set 2 as "Code-Review +2"
+----
+ $ ssh -p 29418 review.example.com gerrit review --code-review +2 8242,2
+----
+
Vote on the project specific label "mylabel":
----
- $ ssh -p 29418 review.example.com gerrit review --label mylabel=+1 c0ff33
+ $ ssh -p 29418 review.example.com gerrit review --label mylabel=+1 8242,2
----
Append the message "Build Successful". Notice two levels of quoting is
required, one for the local shell, and another for the argument parser
inside the Gerrit server:
----
- $ ssh -p 29418 review.example.com gerrit review -m '"Build Successful"' c0ff33
+ $ ssh -p 29418 review.example.com gerrit review -m '"Build Successful"' 8242,2
----
Mark the unmerged commits both "Verified +1" and "Code-Review +2" and
@@ -172,7 +177,7 @@
Abandon an active change:
----
- $ ssh -p 29418 review.example.com gerrit review --abandon c0ff33
+ $ ssh -p 29418 review.example.com gerrit review --abandon 8242,2
----
== SEE ALSO
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 3d81179..477ff1e 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -119,6 +119,7 @@
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Module;
import com.google.inject.Provider;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@@ -247,6 +248,7 @@
protected String resourcePrefix;
protected Description description;
protected boolean testRequiresSsh;
+ protected Module testSysModule;
@Inject private ChangeIndexCollection changeIndexes;
@Inject private EventRecorder.Factory eventRecorderFactory;
@@ -323,11 +325,11 @@
baseConfig.setInt("receive", null, "changeUpdateThreads", 4);
if (classDesc.equals(methodDesc) && !classDesc.sandboxed() && !methodDesc.sandboxed()) {
if (commonServer == null) {
- commonServer = GerritServer.initAndStart(classDesc, baseConfig);
+ commonServer = GerritServer.initAndStart(classDesc, baseConfig, testSysModule);
}
server = commonServer;
} else {
- server = GerritServer.initAndStart(methodDesc, baseConfig);
+ server = GerritServer.initAndStart(methodDesc, baseConfig, testSysModule);
}
server.getTestInjector().injectMembers(this);
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
index d60463f..eafffca 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -33,7 +33,6 @@
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.server.util.SocketUtil;
import com.google.gerrit.server.util.SystemLog;
-import com.google.gerrit.testutil.FakeAuditService;
import com.google.gerrit.testutil.FakeEmailSender;
import com.google.gerrit.testutil.NoteDbChecker;
import com.google.gerrit.testutil.NoteDbMode;
@@ -252,13 +251,14 @@
* @return started server.
* @throws Exception
*/
- public static GerritServer initAndStart(Description desc, Config baseConfig) throws Exception {
+ public static GerritServer initAndStart(
+ Description desc, Config baseConfig, @Nullable Module testSysModule) throws Exception {
Path site = TempFileUtil.createTempDirectory().toPath();
try {
if (!desc.memory()) {
init(desc, baseConfig, site);
}
- return start(desc, baseConfig, site, null);
+ return start(desc, baseConfig, site, testSysModule);
} catch (Exception e) {
TempFileUtil.recursivelyDelete(site.toFile());
throw e;
@@ -302,7 +302,6 @@
},
site);
daemon.setEmailModuleForTesting(new FakeEmailSender.Module());
- daemon.setAuditEventModuleForTesting(new FakeAuditService.Module());
daemon.setAdditionalSysModuleForTesting(testSysModule);
daemon.setEnableSshd(desc.useSsh());
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 0c47793..41f6a67 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -35,6 +35,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
+import com.google.common.collect.ListMultimap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
@@ -1332,6 +1333,47 @@
.containsExactlyElementsIn(ImmutableSet.of(admin.getId(), user.getId()));
}
+ @Test
+ public void listVotesByRevision() throws Exception {
+ // Create patch set 1 and vote on it
+ String changeId = createChange().getChangeId();
+ ListMultimap<String, ApprovalInfo> votes = gApi.changes().id(changeId).current().votes();
+ assertThat(votes).isEmpty();
+ recommend(changeId);
+ votes = gApi.changes().id(changeId).current().votes();
+ assertThat(votes.keySet()).containsExactly("Code-Review");
+ List<ApprovalInfo> approvals = votes.get("Code-Review");
+ assertThat(approvals).hasSize(1);
+ ApprovalInfo approval = approvals.get(0);
+ assertThat(approval._accountId).isEqualTo(admin.id.get());
+ assertThat(approval.email).isEqualTo(admin.email);
+ assertThat(approval.username).isEqualTo(admin.username);
+
+ // Also vote on it with another user
+ setApiUser(user);
+ gApi.changes().id(changeId).current().review(ReviewInput.dislike());
+
+ // Patch set 1 has 2 votes on Code-Review
+ setApiUser(admin);
+ votes = gApi.changes().id(changeId).current().votes();
+ assertThat(votes.keySet()).containsExactly("Code-Review");
+ approvals = votes.get("Code-Review");
+ assertThat(approvals).hasSize(2);
+ assertThat(approvals.stream().map(a -> a._accountId))
+ .containsExactlyElementsIn(ImmutableList.of(admin.id.get(), user.id.get()));
+
+ // Create a new patch set which does not have any votes
+ amendChange(changeId);
+ votes = gApi.changes().id(changeId).current().votes();
+ assertThat(votes).isEmpty();
+
+ // Votes are still returned for ps 1
+ votes = gApi.changes().id(changeId).revision(1).votes();
+ assertThat(votes.keySet()).containsExactly("Code-Review");
+ approvals = votes.get("Code-Review");
+ assertThat(approvals).hasSize(2);
+ }
+
private static void assertCherryPickResult(
ChangeInfo changeInfo, CherryPickInput input, String srcChangeId) throws Exception {
assertThat(changeInfo.changeId).isEqualTo(srcChangeId);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
index f6d4277..3d6d16a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
@@ -17,15 +17,31 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.audit.AuditEvent;
+import com.google.gerrit.audit.AuditService;
+import com.google.gerrit.testutil.FakeAuditService;
+import com.google.inject.AbstractModule;
import java.util.Collections;
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.Description;
public class GitOverHttpServletIT extends AbstractPushForReview {
+ @Override
+ protected void beforeTest(Description description) throws Exception {
+ testSysModule =
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(AuditService.class).to(FakeAuditService.class);
+ }
+ };
+ super.beforeTest(description);
+ }
+
@Before
public void beforeEach() throws Exception {
CredentialsProvider.setDefault(
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 38d1ec7..2c2b3d1 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -14,8 +14,10 @@
package com.google.gerrit.extensions.api.changes;
+import com.google.common.collect.ListMultimap;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
+import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
@@ -129,6 +131,9 @@
RelatedChangesInfo related() throws RestApiException;
+ /** Returns votes on the revision. */
+ ListMultimap<String, ApprovalInfo> votes() throws RestApiException;
+
abstract class MergeListRequest {
private boolean addLinks;
private int uninterestingParent = 1;
@@ -362,6 +367,11 @@
}
@Override
+ public ListMultimap<String, ApprovalInfo> votes() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void description(String description) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index 0fb1f63..497039d 100644
--- a/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -32,12 +32,11 @@
To use PolyGerrit, please enable JavaScript in your browser settings, and then refresh this page.
</noscript>
- {if $canonicalPath != '' or $versionInfo}
- <script>
- {if $canonicalPath != ''}window.CANONICAL_PATH = '{$canonicalPath}';{/if}
- {if $versionInfo}window.VERSION_INFO = '{$versionInfo}';{/if}
- </script>{\n}
- {/if}
+ <script>
+ window.CLOSURE_NO_DEPS = true;
+ {if $canonicalPath != ''}window.CANONICAL_PATH = '{$canonicalPath}';{/if}
+ {if $versionInfo}window.VERSION_INFO = '{$versionInfo}';{/if}
+ </script>{\n}
<link rel="icon" type="image/x-icon" href="{$canonicalPath}/favicon.ico">{\n}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index f818d68..4f30e43 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -195,7 +195,6 @@
private AbstractModule luceneModule;
private Module emailModule;
private Module testSysModule;
- private Module auditEventModule;
private Runnable serverStarted;
private IndexType indexType;
@@ -313,11 +312,6 @@
}
@VisibleForTesting
- public void setAuditEventModuleForTesting(Module module) {
- auditEventModule = module;
- }
-
- @VisibleForTesting
public void setLuceneModule(LuceneIndexModule m) {
luceneModule = m;
inMemoryTest = true;
@@ -416,11 +410,7 @@
} else {
modules.add(new SmtpEmailSender.Module());
}
- if (auditEventModule != null) {
- modules.add(auditEventModule);
- } else {
- modules.add(new AuditModule());
- }
+ modules.add(new AuditModule());
modules.add(new SignedTokenEmailTokenVerifier.Module());
modules.add(new PluginModule());
if (VersionManager.getOnlineUpgrade(config)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditModule.java b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditModule.java
index d5bd8d8..404b8d3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditModule.java
@@ -23,6 +23,5 @@
protected void configure() {
DynamicSet.setOf(binder(), AuditListener.class);
DynamicSet.setOf(binder(), GroupMemberAuditListener.class);
- bind(AuditService.class).to(AuditServiceImpl.class);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
index 171ff45..0c62d11 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
@@ -17,8 +17,10 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
+import com.google.inject.ImplementedBy;
import java.util.Collection;
+@ImplementedBy(AuditServiceImpl.class)
public interface AuditService {
void dispatch(AuditEvent action);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index f07d4d0..2eb55a5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -18,6 +18,8 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.MultimapBuilder.ListMultimapBuilder;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
@@ -35,6 +37,7 @@
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
+import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
@@ -46,7 +49,13 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.account.AccountDirectory.FillOptions;
+import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.ApplyFix;
+import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.CherryPick;
import com.google.gerrit.server.change.Comments;
import com.google.gerrit.server.change.CreateDraftComment;
@@ -79,6 +88,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
+import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -128,6 +138,9 @@
private final GetRelated getRelated;
private final PutDescription putDescription;
private final GetDescription getDescription;
+ private final ApprovalsUtil approvalsUtil;
+ private final Provider<ReviewDb> db;
+ private final AccountLoader.Factory accountLoaderFactory;
@Inject
RevisionApiImpl(
@@ -168,6 +181,9 @@
GetRelated getRelated,
PutDescription putDescription,
GetDescription getDescription,
+ ApprovalsUtil approvalsUtil,
+ Provider<ReviewDb> db,
+ AccountLoader.Factory accountLoaderFactory,
@Assisted RevisionResource r) {
this.repoManager = repoManager;
this.changes = changes;
@@ -206,6 +222,9 @@
this.getRelated = getRelated;
this.putDescription = putDescription;
this.getDescription = getDescription;
+ this.approvalsUtil = approvalsUtil;
+ this.db = db;
+ this.accountLoaderFactory = accountLoaderFactory;
this.revision = r;
}
@@ -578,6 +597,42 @@
}
@Override
+ public ListMultimap<String, ApprovalInfo> votes() throws RestApiException {
+ ListMultimap<String, ApprovalInfo> result =
+ ListMultimapBuilder.treeKeys().arrayListValues().build();
+ try {
+ Iterable<PatchSetApproval> approvals =
+ approvalsUtil.byPatchSet(
+ db.get(),
+ revision.getNotes(),
+ revision.getChangeResource().getUser(),
+ revision.getPatchSet().getId(),
+ null,
+ null);
+ AccountLoader accountLoader =
+ accountLoaderFactory.create(
+ EnumSet.of(
+ FillOptions.ID, FillOptions.NAME, FillOptions.EMAIL, FillOptions.USERNAME));
+ for (PatchSetApproval approval : approvals) {
+ String label = approval.getLabel();
+ ApprovalInfo info =
+ ChangeJson.getApprovalInfo(
+ approval.getAccountId(),
+ Integer.valueOf(approval.getValue()),
+ null,
+ approval.getTag(),
+ approval.getGranted());
+ accountLoader.put(info);
+ result.get(label).add(info);
+ }
+ accountLoader.fill();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot get votes", e);
+ }
+ return result;
+ }
+
+ @Override
public void description(String description) throws RestApiException {
PutDescription.Input in = new PutDescription.Input();
in.description = description;
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 7769dbb..142e35e 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
@@ -994,8 +994,8 @@
public static ApprovalInfo getApprovalInfo(
Account.Id id,
Integer value,
- VotingRangeInfo permittedVotingRange,
- String tag,
+ @Nullable VotingRangeInfo permittedVotingRange,
+ @Nullable String tag,
Timestamp date) {
ApprovalInfo ai = new ApprovalInfo(id.get());
ai.value = value;
