From 6031b05397c91f022a2eaeb96e930049f83567ad Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 25 Jan 2019 13:57:14 +0900 Subject: [PATCH] RevisionApi: Add method to list votes per revision The current API allows to get votes either: - By revision, but only the votes from a specific reviewer - By change, but only all votes on the current patch set Add a method that allows to get all the votes for a specific revision. The result is returned as a multimap of the label name to a list of approval infos for that label. The approval infos contain the Id, name, username and email fields. Change-Id: I35cde09b3ee059e8b03aff1c4ee2747944c3d37c --- .../acceptance/api/revision/RevisionIT.java | 42 ++++++++++++++ .../extensions/api/changes/RevisionApi.java | 10 ++++ .../server/api/changes/RevisionApiImpl.java | 55 +++++++++++++++++++ 3 files changed, 107 insertions(+) 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 0c477933f9..41f6a671f1 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.ImmutableMap; 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 @@ public class RevisionIT extends AbstractDaemonTest { .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 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 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-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 38d1ec7758..2c2b3d1caa 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 @@ public interface RevisionApi { RelatedChangesInfo related() throws RestApiException; + /** Returns votes on the revision. */ + ListMultimap votes() throws RestApiException; + abstract class MergeListRequest { private boolean addLinks; private int uninterestingParent = 1; @@ -361,6 +366,11 @@ public interface RevisionApi { throw new NotImplementedException(); } + @Override + public ListMultimap votes() throws RestApiException { + throw new NotImplementedException(); + } + @Override public void description(String description) throws RestApiException { throw new NotImplementedException(); 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 f07d4d0f05..2eb55a5e12 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.common.base.Preconditions.checkArgument; 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.RobotCommentApi; 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.BinaryResult; 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.gerrit.server.git.GitRepositoryManager; 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 @@ class RevisionApiImpl implements RevisionApi { private final GetRelated getRelated; private final PutDescription putDescription; private final GetDescription getDescription; + private final ApprovalsUtil approvalsUtil; + private final Provider db; + private final AccountLoader.Factory accountLoaderFactory; @Inject RevisionApiImpl( @@ -168,6 +181,9 @@ class RevisionApiImpl implements RevisionApi { GetRelated getRelated, PutDescription putDescription, GetDescription getDescription, + ApprovalsUtil approvalsUtil, + Provider db, + AccountLoader.Factory accountLoaderFactory, @Assisted RevisionResource r) { this.repoManager = repoManager; this.changes = changes; @@ -206,6 +222,9 @@ class RevisionApiImpl implements RevisionApi { this.getRelated = getRelated; this.putDescription = putDescription; this.getDescription = getDescription; + this.approvalsUtil = approvalsUtil; + this.db = db; + this.accountLoaderFactory = accountLoaderFactory; this.revision = r; } @@ -577,6 +596,42 @@ class RevisionApiImpl implements RevisionApi { } } + @Override + public ListMultimap votes() throws RestApiException { + ListMultimap result = + ListMultimapBuilder.treeKeys().arrayListValues().build(); + try { + Iterable 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();