diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 46791583c9..0b853c014f 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -2264,6 +2264,55 @@ Deletes a reviewer from a change. HTTP/1.1 204 No Content ---- +[[list-votes]] +=== List Votes +-- +'GET /changes/link:#change-id[\{change-id\}]/reviewers/link:rest-api-accounts.html#account-id[\{account-id\}]/votes/' +-- + +Lists the votes for a specific reviewer of the change. + +.Request +---- + GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/files/ HTTP/1.0 +---- + +As result a map is returned that maps the label name to the label value. +The entries in the map are sorted by label name. + +.Response +---- + HTTP/1.1 200 OK + Content-Disposition: attachment + Content-Type: application/json;charset=UTF-8 + + )]}' + { + "Code-Review": -1, + "Verified": 1 + "Work-In-Progress": 1, + } +---- + +[[delete-vote]] +=== Delete Vote +-- +'DELETE /changes/link:#change-id[\{change-id\}]/reviewers/link:rest-api-accounts.html#account-id[\{account-id\}]/votes/link:#label-id[\{label-id\}]' +-- + +Deletes a single vote from a change. Note, that even when the last vote of +a reviewer is removed the reviewer itself is still listed on the change. + +.Request +---- + DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/votes/Code-Review HTTP/1.0 +---- + +.Response +---- + HTTP/1.1 204 No Content +---- + [[revision-endpoints]] == Revision Endpoints @@ -3747,6 +3796,10 @@ UUID of a published comment. === \{draft-id\} UUID of a draft comment. +[[label-id]] +=== \{label-id\} +The name of the label. + [[file-id]] \{file-id\} ~~~~~~~~~~~~ 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 159877461f..9788a5fd24 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 @@ -26,6 +26,8 @@ import static com.google.gerrit.server.project.Util.category; import static com.google.gerrit.server.project.Util.value; import static com.google.gerrit.testutil.GerritServerTests.isNoteDbTestEnabled; +import com.google.common.base.Function; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -46,7 +48,9 @@ import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.GitPerson; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.extensions.common.RevisionInfo; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; @@ -63,6 +67,7 @@ import java.util.Collection; import java.util.EnumSet; import java.util.Iterator; import java.util.List; +import java.util.Map; @NoHttpd public class ChangeIT extends AbstractDaemonTest { @@ -378,6 +383,115 @@ public class ChangeIT extends AbstractDaemonTest { } } + @Test + public void listVotes() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.approve()); + + Map m = gApi.changes() + .id(r.getChangeId()) + .reviewer(admin.getId().toString()) + .votes(); + + assertThat(m).hasSize(1); + assertThat(m).containsEntry("Code-Review", new Short((short)2)); + + setApiUser(user); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.dislike()); + + m = gApi.changes() + .id(r.getChangeId()) + .reviewer(user.getId().toString()) + .votes(); + + assertThat(m).hasSize(1); + assertThat(m).containsEntry("Code-Review", new Short((short)-1)); + } + + @Test + public void deleteVote() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.approve()); + + setApiUser(user); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.recommend()); + + setApiUser(admin); + gApi.changes() + .id(r.getChangeId()) + .reviewer(admin.getId().toString()) + .deleteVote("Code-Review"); + + Map m = gApi.changes() + .id(r.getChangeId()) + .reviewer(admin.getId().toString()) + .votes(); + + if (isNoteDbTestEnabled()) { + // When notedb is enabled each reviewer is explicitly recorded in the + // notedb and this record stays even when all votes of that user have been + // deleted, hence there is no dummy 0 approval left when a vote is + // deleted. + assertThat(m).isEmpty(); + } else { + // When notedb is disabled there is a dummy 0 approval on the change so + // that the user is still returned as CC when all votes of that user have + // been deleted. + assertThat(m).containsEntry("Code-Review", new Short((short)0)); + } + + ChangeInfo c = gApi.changes() + .id(r.getChangeId()) + .get(); + + assertThat(Iterables.getLast(c.messages).message).isEqualTo( + "Removed Code-Review+2 by Administrator \n"); + if (isNoteDbTestEnabled()) { + // When notedb is enabled each reviewer is explicitly recorded in the + // notedb and this record stays even when all votes of that user have been + // deleted. + assertThat(getReviewers(c.reviewers.get(REVIEWER))) + .containsExactlyElementsIn( + ImmutableSet.of(admin.getId(), user.getId())); + } else { + // When notedb is disabled users that have only dummy 0 approvals on the + // change are returned as CC and not as REVIEWER. + assertThat(getReviewers(c.reviewers.get(REVIEWER))) + .containsExactlyElementsIn(ImmutableSet.of(user.getId())); + assertThat(getReviewers(c.reviewers.get(CC))) + .containsExactlyElementsIn(ImmutableSet.of(admin.getId())); + } + } + + @Test + public void deleteVoteNotPermitted() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .review(ReviewInput.approve()); + + setApiUser(user); + exception.expect(AuthException.class); + exception.expectMessage("delete not permitted"); + gApi.changes() + .id(r.getChangeId()) + .reviewer(admin.getId().toString()) + .deleteVote("Code-Review"); + } + @Test public void createEmptyChange() throws Exception { ChangeInfo in = new ChangeInfo(); @@ -677,4 +791,15 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(rev2.pushCertificate.certificate).isNull(); assertThat(rev2.pushCertificate.key).isNull(); } + + + private static Iterable getReviewers( + Collection r) { + return Iterables.transform(r, new Function() { + @Override + public Account.Id apply(AccountInfo account) { + return new Account.Id(account._accountId); + } + }); + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index ce070984bb..d927231b83 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -58,6 +58,19 @@ public interface ChangeApi { */ RevisionApi revision(String id) throws RestApiException; + /** + * Look up the reviewer of the change. + *

+ * @param id ID of the account, can be a string of the format + * "Full Name ", just the email address, a full name + * if it is unique, an account ID, a user name or 'self' for the + * calling user. + * @return API for accessing the reviewer. + * @throws RestApiException if id is not account ID or is a user that isn't + * known to be a reviewer for this change. + */ + ReviewerApi reviewer(String id) throws RestApiException; + void abandon() throws RestApiException; void abandon(AbandonInput in) throws RestApiException; @@ -176,6 +189,11 @@ public interface ChangeApi { throw new NotImplementedException(); } + @Override + public ReviewerApi reviewer(String id) throws RestApiException { + throw new NotImplementedException(); + } + @Override public RevisionApi revision(String id) throws RestApiException { throw new NotImplementedException(); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerApi.java new file mode 100644 index 0000000000..11b670da43 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerApi.java @@ -0,0 +1,25 @@ +// Copyright (C) 2014 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.extensions.api.changes; + +import com.google.gerrit.extensions.restapi.RestApiException; + +import java.util.Map; + +public interface ReviewerApi { + + Map votes() throws RestApiException; + void deleteVote(String label) throws RestApiException; +} diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java index cace7ad1ff..b1dbcd6406 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java @@ -31,6 +31,7 @@ import java.sql.Timestamp; import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.HashSet; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -84,6 +85,16 @@ public class ChangeInfo extends JavaScriptObject { return allLabels().keySet(); } + public final Set removableReviewerIds() { + Set removable = new HashSet<>(); + if (removableReviewers() != null) { + for (AccountInfo a : Natives.asList(removableReviewers())) { + removable.add(a._accountId()); + } + } + return removable; + } + public final native String id() /*-{ return this.id; }-*/; public final native String project() /*-{ return this.project; }-*/; public final native String branch() /*-{ return this.branch; }-*/; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Labels.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Labels.java index 4139348cf0..bba63c9f06 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Labels.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Labels.java @@ -48,20 +48,26 @@ import java.util.Set; /** Displays a table of label and reviewer scores. */ class Labels extends Grid { private static final String DATA_ID = "data-id"; - private static final String REMOVE; + private static final String DATA_VOTE = "data-vote"; + private static final String REMOVE_REVIEWER; + private static final String REMOVE_VOTE; static { - REMOVE = DOM.createUniqueId().replace('-', '_'); - init(REMOVE); + REMOVE_REVIEWER = DOM.createUniqueId().replace('-', '_'); + REMOVE_VOTE = DOM.createUniqueId().replace('-', '_'); + init(REMOVE_REVIEWER, REMOVE_VOTE); } - private static final native void init(String r) /*-{ + private static final native void init(String r, String v) /*-{ $wnd[r] = $entry(function(e) { - @com.google.gerrit.client.change.Labels::onRemove(Lcom/google/gwt/dom/client/NativeEvent;)(e) + @com.google.gerrit.client.change.Labels::onRemoveReviewer(Lcom/google/gwt/dom/client/NativeEvent;)(e) + }); + $wnd[v] = $entry(function(e) { + @com.google.gerrit.client.change.Labels::onRemoveVote(Lcom/google/gwt/dom/client/NativeEvent;)(e) }); }-*/; - private static void onRemove(NativeEvent event) { + private static void onRemoveReviewer(NativeEvent event) { Integer user = getDataId(event); if (user != null) { final ChangeScreen screen = ChangeScreen.get(event); @@ -77,6 +83,23 @@ class Labels extends Grid { } } + private static void onRemoveVote(NativeEvent event) { + Integer user = getDataId(event); + String vote = getVoteId(event); + if (user != null && vote != null) { + final ChangeScreen screen = ChangeScreen.get(event); + ChangeApi.vote(screen.getChangeId().get(), user, vote).delete( + new GerritCallback() { + @Override + public void onSuccess(JavaScriptObject result) { + if (screen.isCurrentView()) { + Gerrit.display(PageLinks.toChange(screen.getChangeId())); + } + } + }); + } + } + private static Integer getDataId(NativeEvent event) { Element e = event.getEventTarget().cast(); while (e != null) { @@ -89,6 +112,18 @@ class Labels extends Grid { return null; } + private static String getVoteId(NativeEvent event) { + Element e = event.getEventTarget().cast(); + while (e != null) { + String v = e.getAttribute(DATA_VOTE); + if (!v.isEmpty()) { + return v; + } + e = e.getParentElement(); + } + return null; + } + private ChangeScreen.Style style; void init(ChangeScreen.Style style) { @@ -97,6 +132,7 @@ class Labels extends Grid { void set(ChangeInfo info) { List names = new ArrayList<>(info.labels()); + Set removable = info.removableReviewerIds(); Collections.sort(names); resize(names.size(), 2); @@ -106,14 +142,14 @@ class Labels extends Grid { LabelInfo label = info.label(name); setText(row, 0, name); if (label.all() != null) { - setWidget(row, 1, renderUsers(label)); + setWidget(row, 1, renderUsers(label, removable)); } getCellFormatter().setStyleName(row, 0, style.labelName()); getCellFormatter().addStyleName(row, 0, getStyleForLabel(label)); } } - private Widget renderUsers(LabelInfo label) { + private Widget renderUsers(LabelInfo label, Set removable) { Map> m = new HashMap<>(4); int approved = 0; int rejected = 0; @@ -150,8 +186,8 @@ class Labels extends Grid { html.setStyleName(style.label_reject()); } html.append(val).append(" "); - html.append(formatUserList(style, m.get(v), - Collections. emptySet(), null)); + html.append(formatUserList(style, m.get(v), removable, + label.name(), null)); html.closeSpan(); } return html.toBlockWidget(); @@ -198,6 +234,7 @@ class Labels extends Grid { static SafeHtml formatUserList(ChangeScreen.Style style, Collection in, Set removable, + String label, Map votable) { List users = new ArrayList<>(in); Collections.sort(users, new Comparator() { @@ -257,6 +294,9 @@ class Labels extends Grid { .setAttribute(DATA_ID, ai._accountId()) .setAttribute("title", getTitle(ai, votableCategories)) .setStyleName(style.label_user()); + if (label != null) { + html.setAttribute(DATA_VOTE, label); + } if (img != null) { html.openElement("img") .setStyleName(style.avatar()) @@ -271,10 +311,15 @@ class Labels extends Grid { } html.append(name); if (removable.contains(ai._accountId())) { - html.openElement("button") - .setAttribute("title", Util.M.removeReviewer(name)) - .setAttribute("onclick", REMOVE + "(event)") - .append("×") + html.openElement("button"); + if (label != null) { + html.setAttribute("title", Util.M.removeVote(label)) + .setAttribute("onclick", REMOVE_VOTE + "(event)"); + } else { + html.setAttribute("title", Util.M.removeReviewer(name)) + .setAttribute("onclick", REMOVE_REVIEWER + "(event)"); + } + html.append("×") .closeElement("button"); } html.closeSpan(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java index 7d2415cfc0..ae648dc91e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java @@ -51,7 +51,6 @@ import com.google.gwtexpui.safehtml.client.SafeHtml; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -214,20 +213,13 @@ public class Reviewers extends Composite { cc.remove(i); } cc.remove(info.owner()._accountId()); - - Set removable = new HashSet<>(); - if (info.removableReviewers() != null) { - for (AccountInfo a : Natives.asList(info.removableReviewers())) { - removable.add(a._accountId()); - } - } - + Set removable = info.removableReviewerIds(); Map votable = votable(info); SafeHtml rHtml = Labels.formatUserList(style, - r.values(), removable, votable); + r.values(), removable, null, votable); SafeHtml ccHtml = Labels.formatUserList(style, - cc.values(), removable, votable); + cc.values(), removable, null, votable); reviewersText.setInnerSafeHtml(rHtml); ccText.setInnerSafeHtml(ccHtml); 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 9cafeec2f9..8314e3efc2 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 @@ -155,6 +155,10 @@ public class ChangeApi { .addParameter("n", n); } + public static RestApi vote(int id, int reviewer, String vote) { + return reviewer(id, reviewer).view("votes").id(vote); + } + public static RestApi reviewer(int id, int reviewer) { return change(id).view("reviewers").id(reviewer); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java index 95f2824d49..eb096571a7 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java @@ -43,6 +43,7 @@ public interface ChangeMessages extends Messages { String removeHashtag(String name); String removeReviewer(String fullName); + String removeVote(String label); String messageWrittenOn(String date); String renamedFrom(String sourcePath); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties index 9e58c9faf9..b3c398054f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties @@ -24,6 +24,7 @@ patchTableSize_Lines = {0} lines removeHashtag = Remove hashtag {0} removeReviewer = Remove reviewer {0} +removeVote = Remove vote {0} messageWrittenOn = on {0} renamedFrom = renamed from {0} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index b7617a29df..d7f8cde437 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.RestoreInput; import com.google.gerrit.extensions.api.changes.RevertInput; +import com.google.gerrit.extensions.api.changes.ReviewerApi; import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; @@ -46,6 +47,7 @@ import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.change.PutTopic; import com.google.gerrit.server.change.Restore; import com.google.gerrit.server.change.Revert; +import com.google.gerrit.server.change.Reviewers; import com.google.gerrit.server.change.Revisions; import com.google.gerrit.server.change.SubmittedTogether; import com.google.gerrit.server.change.SuggestReviewers; @@ -68,7 +70,9 @@ class ChangeApiImpl implements ChangeApi { private final Provider user; private final Changes changeApi; + private final Reviewers reviewers; private final Revisions revisions; + private final ReviewerApiImpl.Factory reviewerApi; private final RevisionApiImpl.Factory revisionApi; private final Provider suggestReviewers; private final ChangeResource change; @@ -90,7 +94,9 @@ class ChangeApiImpl implements ChangeApi { @Inject ChangeApiImpl(Provider user, Changes changeApi, + Reviewers reviewers, Revisions revisions, + ReviewerApiImpl.Factory reviewerApi, RevisionApiImpl.Factory revisionApi, Provider suggestReviewers, Abandon abandon, @@ -111,7 +117,9 @@ class ChangeApiImpl implements ChangeApi { this.user = user; this.changeApi = changeApi; this.revert = revert; + this.reviewers = reviewers; this.revisions = revisions; + this.reviewerApi = reviewerApi; this.revisionApi = revisionApi; this.suggestReviewers = suggestReviewers; this.abandon = abandon; @@ -155,6 +163,16 @@ class ChangeApiImpl implements ChangeApi { } } + @Override + public ReviewerApi reviewer(String id) throws RestApiException { + try { + return reviewerApi.create( + reviewers.parse(change, IdString.fromDecoded(id))); + } catch (OrmException e) { + throw new RestApiException("Cannot parse reviewer", e); + } + } + @Override public void abandon() throws RestApiException { abandon(new AbandonInput()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/Module.java index a5e584e8eb..228dad6072 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/Module.java @@ -27,5 +27,6 @@ public class Module extends FactoryModule { factory(DraftApiImpl.Factory.class); factory(RevisionApiImpl.Factory.class); factory(FileApiImpl.Factory.class); + factory(ReviewerApiImpl.Factory.class); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ReviewerApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ReviewerApiImpl.java new file mode 100644 index 0000000000..49b343248b --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ReviewerApiImpl.java @@ -0,0 +1,65 @@ +// Copyright (C) 2014 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.api.changes; + +import com.google.gerrit.extensions.api.changes.ReviewerApi; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.server.change.DeleteVote; +import com.google.gerrit.server.change.ReviewerResource; +import com.google.gerrit.server.change.VoteResource; +import com.google.gerrit.server.change.Votes; +import com.google.gerrit.server.git.UpdateException; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +import java.util.Map; + +public class ReviewerApiImpl implements ReviewerApi { + interface Factory { + ReviewerApiImpl create(ReviewerResource r); + } + + private final ReviewerResource reviewer; + private final Votes.List listVotes; + private final DeleteVote deleteVote; + + @Inject + ReviewerApiImpl(Votes.List listVotes, + DeleteVote deleteVote, + @Assisted ReviewerResource reviewer) { + this.listVotes = listVotes; + this.deleteVote = deleteVote; + this.reviewer = reviewer; + } + + @Override + public Map votes() throws RestApiException { + try { + return listVotes.apply(reviewer); + } catch (OrmException e) { + throw new RestApiException("Cannot list votes", e); + } + } + + @Override + public void deleteVote(String label) throws RestApiException { + try { + deleteVote.apply(new VoteResource(reviewer, label), null); + } catch (UpdateException e) { + throw new RestApiException("Cannot delete vote", e); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java new file mode 100644 index 0000000000..b3e59b9672 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java @@ -0,0 +1,151 @@ +// Copyright (C) 2014 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.change; + +import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.ChangeMessage; +import com.google.gerrit.reviewdb.client.PatchSet; +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.ChangeMessagesUtil; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.change.DeleteVote.Input; +import com.google.gerrit.server.git.BatchUpdate; +import com.google.gerrit.server.git.BatchUpdate.ChangeContext; +import com.google.gerrit.server.git.UpdateException; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.util.Collections; + +@Singleton +public class DeleteVote implements RestModifyView { + public static class Input { + } + + private final Provider db; + private final BatchUpdate.Factory batchUpdateFactory; + private final ApprovalsUtil approvalsUtil; + private final ChangeMessagesUtil cmUtil; + private final IdentifiedUser.GenericFactory userFactory; + + @Inject + DeleteVote(Provider db, + BatchUpdate.Factory batchUpdateFactory, + ApprovalsUtil approvalsUtil, + ChangeMessagesUtil cmUtil, + IdentifiedUser.GenericFactory userFactory) { + this.db = db; + this.batchUpdateFactory = batchUpdateFactory; + this.approvalsUtil = approvalsUtil; + this.cmUtil = cmUtil; + this.userFactory = userFactory; + } + + @Override + public Response apply(VoteResource rsrc, Input input) + throws RestApiException, UpdateException { + ReviewerResource r = rsrc.getReviewer(); + ChangeControl ctl = r.getControl(); + Change change = r.getChange(); + try (BatchUpdate bu = batchUpdateFactory.create(db.get(), + change.getProject(), ctl.getUser().asIdentifiedUser(), + TimeUtil.nowTs())) { + bu.addOp(change.getId(), + new Op(r.getUser().getAccountId(), rsrc.getLabel())); + bu.execute(); + } + + return Response.none(); + } + + private class Op extends BatchUpdate.Op { + private final Account.Id accountId; + private final String label; + + private Op(Account.Id accountId, String label) { + this.accountId = accountId; + this.label = label; + } + + @Override + public void updateChange(ChangeContext ctx) + throws OrmException, AuthException, ResourceNotFoundException { + IdentifiedUser user = ctx.getUser().asIdentifiedUser(); + Change change = ctx.getChange(); + ChangeControl ctl = ctx.getChangeControl(); + PatchSet.Id psId = change.currentPatchSetId(); + + PatchSetApproval psa = null; + StringBuilder msg = new StringBuilder(); + for (PatchSetApproval a : approvalsUtil.byPatchSetUser( + ctx.getDb(), ctl, psId, accountId)) { + if (ctl.canRemoveReviewer(a)) { + if (a.getLabel().equals(label)) { + msg.append("Removed ") + .append(a.getLabel()).append(formatLabelValue(a.getValue())) + .append(" by ").append(userFactory.create(user.getAccountId()) + .getNameEmail()) + .append("\n"); + psa = a; + a.setValue((short)0); + ctx.getChangeUpdate().setPatchSetId(psId); + ctx.getChangeUpdate().removeApproval(label); + break; + } + } else { + throw new AuthException("delete not permitted"); + } + } + if (psa == null) { + throw new ResourceNotFoundException(); + } + ChangeUtil.bumpRowVersionNotLastUpdatedOn(change.getId(), ctx.getDb()); + ctx.getDb().patchSetApprovals().update(Collections.singleton(psa)); + + if (msg.length() > 0) { + ChangeMessage changeMessage = + new ChangeMessage(new ChangeMessage.Key(change.getId(), + ChangeUtil.messageUUID(ctx.getDb())), + user.getAccountId(), + ctx.getWhen(), + change.currentPatchSetId()); + changeMessage.setMessage(msg.toString()); + cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(), + changeMessage); + } + } + } + + private static String formatLabelValue(short value) { + if (value > 0) { + return "+" + value; + } else { + return Short.toString(value); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index 9c588ed769..0ef8b514c6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -21,6 +21,7 @@ import static com.google.gerrit.server.change.DraftCommentResource.DRAFT_COMMENT import static com.google.gerrit.server.change.FileResource.FILE_KIND; import static com.google.gerrit.server.change.ReviewerResource.REVIEWER_KIND; import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND; +import static com.google.gerrit.server.change.VoteResource.VOTE_KIND; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.RestApiModule; @@ -37,6 +38,7 @@ public class Module extends RestApiModule { bind(DraftComments.class); bind(Comments.class); bind(Files.class); + bind(Votes.class); DynamicMap.mapOf(binder(), CHANGE_KIND); DynamicMap.mapOf(binder(), COMMENT_KIND); @@ -45,6 +47,7 @@ public class Module extends RestApiModule { DynamicMap.mapOf(binder(), REVIEWER_KIND); DynamicMap.mapOf(binder(), REVISION_KIND); DynamicMap.mapOf(binder(), CHANGE_EDIT_KIND); + DynamicMap.mapOf(binder(), VOTE_KIND); get(CHANGE_KIND).to(GetChange.class); get(CHANGE_KIND, "detail").to(GetDetail.class); @@ -73,6 +76,8 @@ public class Module extends RestApiModule { child(CHANGE_KIND, "reviewers").to(Reviewers.class); get(REVIEWER_KIND).to(GetReviewer.class); delete(REVIEWER_KIND).to(DeleteReviewer.class); + child(REVIEWER_KIND, "votes").to(Votes.class); + delete(VOTE_KIND).to(DeleteVote.class); child(CHANGE_KIND, "revisions").to(Revisions.class); get(REVISION_KIND, "actions").to(GetRevisionActions.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/VoteResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/VoteResource.java new file mode 100644 index 0000000000..4dfaff001d --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/VoteResource.java @@ -0,0 +1,40 @@ +// Copyright (C) 2014 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.change; + +import com.google.gerrit.extensions.restapi.RestResource; +import com.google.gerrit.extensions.restapi.RestView; +import com.google.inject.TypeLiteral; + +public class VoteResource implements RestResource { + public static final TypeLiteral> VOTE_KIND = + new TypeLiteral>() {}; + + private final ReviewerResource reviewer; + private final String label; + + public VoteResource(ReviewerResource reviewer, String label) { + this.reviewer = reviewer; + this.label = label; + } + + public ReviewerResource getReviewer() { + return reviewer; + } + + public String getLabel() { + return label; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Votes.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Votes.java new file mode 100644 index 0000000000..a86ce7f87f --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Votes.java @@ -0,0 +1,89 @@ +// Copyright (C) 2014 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.change; + +import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.extensions.restapi.RestView; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ApprovalsUtil; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.util.Map; +import java.util.TreeMap; + +@Singleton +public class Votes implements ChildCollection { + private final DynamicMap> views; + private final List list; + + @Inject + Votes(DynamicMap> views, + List list) { + this.views = views; + this.list = list; + } + + @Override + public DynamicMap> views() { + return views; + } + + @Override + public RestView list() throws AuthException { + return list; + } + + @Override + public VoteResource parse(ReviewerResource reviewer, IdString id) + throws ResourceNotFoundException, OrmException, AuthException { + return new VoteResource(reviewer, id.get()); + } + + @Singleton + public static class List implements RestReadView { + private final Provider db; + private final ApprovalsUtil approvalsUtil; + + @Inject + List(Provider db, + ApprovalsUtil approvalsUtil) { + this.db = db; + this.approvalsUtil = approvalsUtil; + } + + @Override + public Map apply(ReviewerResource rsrc) throws OrmException { + Map votes = new TreeMap<>(); + Iterable byPatchSetUser = approvalsUtil.byPatchSetUser( + db.get(), + rsrc.getControl(), + rsrc.getChange().currentPatchSetId(), + rsrc.getUser().getAccountId()); + for (PatchSetApproval psa : byPatchSetUser) { + votes.put(psa.getLabel(), psa.getValue()); + } + return votes; + } + } +}