opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'reviewers-by-email'

Browse Source 
* changes:
  Add reviewers by email to REST API and Email sender
  Add reviewer.enableByEmail to ProjectConfig
  Add reviewers by email to NoteDb
This commit is contained in:
Dave Borowitz
2017-03-24 22:27:57 +00:00
committed by Gerrit Code Review
parent 38055ae647 11873ef74e
commit f34af258cd
39 changed files with 1355 additions and 289 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
Documentation/config-project-config.txt
View File

@@ -291,6 +291,18 @@ Branches not listed in this section will not be included in the mergeability
check. If the `branchOrder` section is not defined then the mergeability of a
change into other branches will not be done.
[[reviewer-section]]
=== reviewer section
Defines config options to adjust a project's reviewer workflow such as enabling
reviewers and CCs by email.
[[reviewer.enableByEmail]]reviewer.enableByEmail::
+
A boolean indicating if reviewers and CCs that do not currently have a Gerrit
account can be added to a change by providing their email address.
Defaults to `false`.
[[file-groups]]
== The file +groups+

39
Documentation/rest-api-changes.txt
View File

@@ -2805,6 +2805,41 @@ To confirm the addition of the reviewers, resend the request with the
}
----
If link:config-project-config.html#reviewer.enableByEmail[reviewer.enableByEmail] is set
for the project, reviewers and CCs are not required to have a Gerrit account. If you POST
an email address of a reviewer or CC then, they will be added to the change even if they
don't have a Gerrit account.
If this option is disabled, the request would fail with `400 Bad Request` if the email
address can't be resolved to an active Gerrit account.
Note that the name is optional so both "un.registered@reviewer.com" and
"John Doe <un.registered@reviewer.com>" are valid inputs.
Reviewers without Gerrit accounts can only be added on changes visible to anonymous users.
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"reviewer": "John Doe <un.registered@reviewer.com>"
}
----
.Response
----
HTTP/1.1 200 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]}'
{
"input": "John Doe <un.registered@reviewer.com>"
}
----
[[delete-reviewer]]
=== Delete Reviewer
--
@@ -6183,6 +6218,10 @@ In addition `ReviewerInfo` has the following fields:
|`approvals` |
The approvals of the reviewer as a map that maps the label names to the
approval values ("`-2`", "`-1`", "`0`", "`+1`", "`+2`").
|`_account_id` |
This field is inherited from `AccountInfo` but is optional here if an
unregistered reviewer was added by email. See
link:rest-api-changes.html#add-reviewer[add-reviewer] for details.
|==========================
[[reviewer-input]]

17
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -88,6 +88,7 @@ import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.send.EmailHeader;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -1200,21 +1201,29 @@ public abstract class AbstractDaemonTest {
}
protected void assertNotifyTo(TestAccount expected) {
assertNotifyTo(expected.emailAddress);
}
protected void assertNotifyTo(Address expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.emailAddress);
assertThat(m.rcpt()).containsExactly(expected);
assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
.containsExactly(expected.emailAddress);
.containsExactly(expected);
assertThat(m.headers().get("CC").isEmpty()).isTrue();
}
protected void assertNotifyCc(TestAccount expected) {
assertNotifyCc(expected.emailAddress);
}
protected void assertNotifyCc(Address expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.emailAddress);
assertThat(m.rcpt()).containsExactly(expected);
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(((EmailHeader.AddressList) m.headers().get("CC")).getAddressList())
.containsExactly(expected.emailAddress);
.containsExactly(expected);
}
protected void assertNotifyBcc(TestAccount expected) {

245
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java Normal file
View File

@@ -0,0 +1,245 @@
// Copyright (C) 2017 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.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.testutil.FakeEmailSender.Message;
import java.util.EnumSet;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
@Before
public void setUp() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(true);
saveProjectConfig(project, cfg);
}
@Test
public void addByEmail() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
PushOneCommit.Result r = createChange();
AddReviewerInput input = new AddReviewerInput();
input.reviewer = toRfcAddressString(acc);
input.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(input);
ChangeInfo info =
gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc)));
// All reviewers added by email should be removable
assertThat(info.removableReviewers).isEqualTo(ImmutableList.of(acc));
}
}
@Test
public void removeByEmail() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
PushOneCommit.Result r = createChange();
AddReviewerInput addInput = new AddReviewerInput();
addInput.reviewer = toRfcAddressString(acc);
addInput.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(addInput);
gApi.changes().id(r.getChangeId()).reviewer(acc.email).remove();
ChangeInfo info =
gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
assertThat(info.reviewers).isEmpty();
}
}
@Test
public void convertFromCCToReviewer() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
PushOneCommit.Result r = createChange();
AddReviewerInput addInput = new AddReviewerInput();
addInput.reviewer = toRfcAddressString(acc);
addInput.state = ReviewerState.CC;
gApi.changes().id(r.getChangeId()).addReviewer(addInput);
AddReviewerInput modifyInput = new AddReviewerInput();
modifyInput.reviewer = addInput.reviewer;
modifyInput.state = ReviewerState.REVIEWER;
gApi.changes().id(r.getChangeId()).addReviewer(modifyInput);
ChangeInfo info =
gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
assertThat(info.reviewers)
.isEqualTo(ImmutableMap.of(ReviewerState.REVIEWER, ImmutableList.of(acc)));
}
@Test
public void addedReviewersGetNotified() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
PushOneCommit.Result r = createChange();
AddReviewerInput input = new AddReviewerInput();
input.reviewer = toRfcAddressString(acc);
input.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(input);
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt()).containsExactly(Address.parse(input.reviewer));
sender.clear();
}
}
@Test
public void removingReviewerTriggersNotification() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
PushOneCommit.Result r = createChange();
AddReviewerInput addInput = new AddReviewerInput();
addInput.reviewer = toRfcAddressString(acc);
addInput.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(addInput);
// Review change as user
ReviewInput reviewInput = new ReviewInput();
reviewInput.message = "I have a comment";
setApiUser(user);
revision(r).review(reviewInput);
setApiUser(admin);
sender.clear();
// Delete as admin
gApi.changes().id(r.getChangeId()).reviewer(addInput.reviewer).remove();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt())
.containsExactly(Address.parse(addInput.reviewer), user.emailAddress);
sender.clear();
}
}
@Test
public void reviewerAndCCReceiveRegularNotification() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
PushOneCommit.Result r = createChange();
AddReviewerInput input = new AddReviewerInput();
input.reviewer = toRfcAddressString(acc);
input.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(input);
sender.clear();
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
.review(ReviewInput.approve());
if (state == ReviewerState.CC) {
assertNotifyCc(Address.parse(input.reviewer));
} else {
assertNotifyTo(Address.parse(input.reviewer));
}
}
}
@Test
public void rejectMissingEmail() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
PushOneCommit.Result r = createChange();
exception.expect(UnprocessableEntityException.class);
exception.expectMessage("email invalid");
gApi.changes().id(r.getChangeId()).addReviewer("");
}
@Test
public void rejectMalformedEmail() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
PushOneCommit.Result r = createChange();
exception.expect(UnprocessableEntityException.class);
exception.expectMessage("email invalid");
gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@");
}
@Test
public void rejectOnNonPublicChange() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
PushOneCommit.Result r = createDraftChange();
exception.expect(BadRequestException.class);
exception.expectMessage("change is not publicly visible");
gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@gerritcodereview.com>");
}
@Test
public void rejectWhenFeatureIsDisabled() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(false);
saveProjectConfig(project, cfg);
PushOneCommit.Result r = createChange();
exception.expect(UnprocessableEntityException.class);
exception.expectMessage(
"Foo Bar <foo.bar@gerritcodereview.com> does not identify a registered user or group");
gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@gerritcodereview.com>");
}
private static String toRfcAddressString(AccountInfo info) {
return (new Address(info.name, info.email)).toString();
}
}

9
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerInfo.java
View File

@@ -25,6 +25,13 @@ public class ReviewerInfo extends AccountInfo {
*/
@Nullable public Map<String, String> approvals;
public static ReviewerInfo byEmail(@Nullable String name, String email) {
ReviewerInfo info = new ReviewerInfo();
info.name = name;
info.email = email;
return info;
}
public ReviewerInfo(Integer id) {
super(id);
}
@@ -33,4 +40,6 @@ public class ReviewerInfo extends AccountInfo {
public String toString() {
return username;
}
private ReviewerInfo() {}
}

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
View File

@@ -31,6 +31,7 @@ public class ConfigInfo {
public InheritedBooleanInfo enableSignedPush;
public InheritedBooleanInfo requireSignedPush;
public InheritedBooleanInfo rejectImplicitMerges;
public InheritedBooleanInfo enableReviewerByEmail;
public MaxObjectSizeLimitInfo maxObjectSizeLimit;
public SubmitType submitType;
public ProjectState state;

31
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.extensions.common;
import java.util.List;
import java.util.Objects;
public class AccountInfo {
public Integer _accountId;
@@ -29,4 +30,34 @@ public class AccountInfo {
public AccountInfo(Integer id) {
this._accountId = id;
}
/** To be used ONLY in connection with unregistered reviewers and CCs. */
public AccountInfo(String name, String email) {
this.name = name;
this.email = email;
}
@Override
public boolean equals(Object o) {
if (o instanceof AccountInfo) {
AccountInfo accountInfo = (AccountInfo) o;
return Objects.equals(_accountId, accountInfo._accountId)
&& Objects.equals(name, accountInfo.name)
&& Objects.equals(email, accountInfo.email)
&& Objects.equals(secondaryEmails, accountInfo.secondaryEmails)
&& Objects.equals(username, accountInfo.username)
&& Objects.equals(avatars, accountInfo.avatars)
&& Objects.equals(_moreAccounts, accountInfo._moreAccounts)
&& Objects.equals(status, accountInfo.status);
}
return false;
}
@Override
public int hashCode() {
return Objects.hash(
_accountId, name, email, secondaryEmails, username, avatars, _moreAccounts, status);
}
protected AccountInfo() {}
}

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java
View File

@@ -86,6 +86,7 @@ public class ProjectInfoScreen extends ProjectScreen {
private ListBox enableSignedPush;
private ListBox requireSignedPush;
private ListBox rejectImplicitMerges;
private ListBox enableReviewerByEmail;
private NpTextBox maxObjectSizeLimit;
private Label effectiveMaxObjectSizeLimit;
private Map<String, Map<String, HasEnabled>> pluginConfigWidgets;
@@ -191,6 +192,7 @@ public class ProjectInfoScreen extends ProjectScreen {
requireChangeID.setEnabled(isOwner);
rejectImplicitMerges.setEnabled(isOwner);
maxObjectSizeLimit.setEnabled(isOwner);
enableReviewerByEmail.setEnabled(isOwner);
if (pluginConfigWidgets != null) {
for (Map<String, HasEnabled> widgetMap : pluginConfigWidgets.values()) {
@@ -264,6 +266,10 @@ public class ProjectInfoScreen extends ProjectScreen {
saveEnabler.listenTo(rejectImplicitMerges);
grid.addHtml(AdminConstants.I.rejectImplicitMerges(), rejectImplicitMerges);
enableReviewerByEmail = newInheritedBooleanBox();
saveEnabler.listenTo(enableReviewerByEmail);
grid.addHtml(AdminConstants.I.rejectImplicitMerges(), enableReviewerByEmail);
maxObjectSizeLimit = new NpTextBox();
saveEnabler.listenTo(maxObjectSizeLimit);
effectiveMaxObjectSizeLimit = new Label();
@@ -395,6 +401,7 @@ public class ProjectInfoScreen extends ProjectScreen {
setBool(requireSignedPush, result.requireSignedPush());
}
setBool(rejectImplicitMerges, result.rejectImplicitMerges());
setBool(enableReviewerByEmail, result.enableReviewerByEmail());
setSubmitType(result.submitType());
setState(result.state());
maxObjectSizeLimit.setText(result.maxObjectSizeLimit().configuredValue());

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java
View File

@@ -57,6 +57,9 @@ public class ConfigInfo extends JavaScriptObject {
public final native InheritedBooleanInfo rejectImplicitMerges()
/*-{ return this.reject_implicit_merges; }-*/ ;
public final native InheritedBooleanInfo enableReviewerByEmail()
/*-{ return this.enable_reviewer_by_email; }-*/ ;
public final SubmitType submitType() {
return SubmitType.valueOf(submitTypeRaw());
}

7
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Project.java
View File

@@ -99,6 +99,8 @@ public final class Project {
protected InheritableBoolean rejectImplicitMerges;
protected InheritableBoolean enableReviewerByEmail;
protected Project() {}
public Project(Project.NameKey nameKey) {
@@ -112,6 +114,7 @@ public final class Project {
createNewChangeForAllNotInTarget = InheritableBoolean.INHERIT;
enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT;
enableReviewerByEmail = InheritableBoolean.INHERIT;
}
public Project.NameKey getNameKey() {
@@ -154,6 +157,10 @@ public final class Project {
return rejectImplicitMerges;
}
public InheritableBoolean getEnableReviewerByEmail() {
return enableReviewerByEmail;
}
public void setUseContributorAgreements(final InheritableBoolean u) {
useContributorAgreements = u;
}

79
gerrit-server/src/main/java/com/google/gerrit/server/ReviewerByEmailSet.java Normal file
View File

@@ -0,0 +1,79 @@
// Copyright (C) 2017 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;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Table;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import java.sql.Timestamp;
/**
* Set of reviewers on a change that do not have a Gerrit account and were added by email instead.
*
* <p>A given account may appear in multiple states and at different timestamps. No reviewers with
* state {@link ReviewerStateInternal#REMOVED} are ever exposed by this interface.
*/
public class ReviewerByEmailSet {
private static final ReviewerByEmailSet EMPTY = new ReviewerByEmailSet(ImmutableTable.of());
public static ReviewerByEmailSet fromTable(
Table<ReviewerStateInternal, Address, Timestamp> table) {
return new ReviewerByEmailSet(table);
}
public static ReviewerByEmailSet empty() {
return EMPTY;
}
private final ImmutableTable<ReviewerStateInternal, Address, Timestamp> table;
private ImmutableSet<Address> users;
private ReviewerByEmailSet(Table<ReviewerStateInternal, Address, Timestamp> table) {
this.table = ImmutableTable.copyOf(table);
}
public ImmutableSet<Address> all() {
if (users == null) {
// Idempotent and immutable, don't bother locking.
users = ImmutableSet.copyOf(table.columnKeySet());
}
return users;
}
public ImmutableSet<Address> byState(ReviewerStateInternal state) {
return table.row(state).keySet();
}
public ImmutableTable<ReviewerStateInternal, Address, Timestamp> asTable() {
return table;
}
@Override
public boolean equals(Object o) {
return (o instanceof ReviewerByEmailSet) && table.equals(((ReviewerByEmailSet) o).table);
}
@Override
public int hashCode() {
return table.hashCode();
}
@Override
public String toString() {
return getClass().getSimpleName() + table;
}
}

31
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -108,6 +108,7 @@ import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -531,6 +532,12 @@ public class ChangeJson {
cd.reviewers().asTable().rowMap().entrySet()) {
out.reviewers.put(e.getKey().asReviewerState(), toAccountInfo(e.getValue().keySet()));
}
// TODO(hiesel) Load from ChangeData instead after the data was added there
for (Map.Entry<ReviewerStateInternal, Map<Address, Timestamp>> e :
cd.notes().getReviewersByEmail().asTable().rowMap().entrySet()) {
out.reviewers.put(
e.getKey().asReviewerState(), toAccountInfoByEmail(e.getValue().keySet()));
}
out.removableReviewers = removableReviewers(ctl, out);
}
@@ -1075,9 +1082,11 @@ public class ChangeJson {
Collection<AccountInfo> ccs = out.reviewers.get(ReviewerState.CC);
if (ccs != null) {
for (AccountInfo ai : ccs) {
Account.Id id = new Account.Id(ai._accountId);
if (ctl.canRemoveReviewer(id, 0)) {
removable.add(id);
if (ai._accountId != null) {
Account.Id id = new Account.Id(ai._accountId);
if (ctl.canRemoveReviewer(id, 0)) {
removable.add(id);
}
}
}
}
@@ -1091,6 +1100,14 @@ public class ChangeJson {
for (Account.Id id : removable) {
result.add(accountLoader.get(id));
}
// Reviewers added by email are always removable
for (Collection<AccountInfo> infos : out.reviewers.values()) {
for (AccountInfo info : infos) {
if (info._accountId == null) {
result.add(info);
}
}
}
return result;
}
@@ -1102,6 +1119,14 @@ public class ChangeJson {
.collect(toList());
}
private Collection<AccountInfo> toAccountInfoByEmail(Collection<Address> addresses) {
return addresses
.stream()
.map(a -> new AccountInfo(a.getName(), a.getEmail()))
.sorted(AccountInfoComparator.ORDER_NULLS_FIRST)
.collect(toList());
}
@Nullable
private Repository openRepoIfNecessary(ChangeControl ctl) throws IOException {
if (has(ALL_COMMITS) || has(CURRENT_COMMIT) || has(COMMIT_FOOTERS)) {

214
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
View File

@@ -14,92 +14,38 @@
package com.google.gerrit.server.change;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
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.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.ReviewerDeleted;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.BatchUpdateReviewDb;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class DeleteReviewer implements RestModifyView<ReviewerResource, DeleteReviewerInput> {
private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
private final Provider<ReviewDb> dbProvider;
private final ApprovalsUtil approvalsUtil;
private final PatchSetUtil psUtil;
private final ChangeMessagesUtil cmUtil;
private final BatchUpdate.Factory batchUpdateFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final ReviewerDeleted reviewerDeleted;
private final Provider<IdentifiedUser> user;
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
private final NotesMigration migration;
private final NotifyUtil notifyUtil;
private final DeleteReviewerOp.Factory deleteReviewerOpFactory;
private final DeleteReviewerByEmailOp.Factory deleteReviewerByEmailOpFactory;
@Inject
DeleteReviewer(
Provider<ReviewDb> dbProvider,
ApprovalsUtil approvalsUtil,
PatchSetUtil psUtil,
ChangeMessagesUtil cmUtil,
BatchUpdate.Factory batchUpdateFactory,
IdentifiedUser.GenericFactory userFactory,
ReviewerDeleted reviewerDeleted,
Provider<IdentifiedUser> user,
DeleteReviewerSender.Factory deleteReviewerSenderFactory,
NotesMigration migration,
NotifyUtil notifyUtil) {
DeleteReviewerOp.Factory deleteReviewerOpFactory,
DeleteReviewerByEmailOp.Factory deleteReviewerByEmailOpFactory) {
this.dbProvider = dbProvider;
this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil;
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.userFactory = userFactory;
this.reviewerDeleted = reviewerDeleted;
this.user = user;
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
this.migration = migration;
this.notifyUtil = notifyUtil;
this.deleteReviewerOpFactory = deleteReviewerOpFactory;
this.deleteReviewerByEmailOpFactory = deleteReviewerByEmailOpFactory;
}
@Override
@@ -118,151 +64,15 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, DeleteRe
rsrc.getChangeResource().getProject(),
rsrc.getChangeResource().getUser(),
TimeUtil.nowTs())) {
Op op = new Op(rsrc.getReviewerUser().getAccount(), input);
BatchUpdateOp op;
if (rsrc.isByEmail()) {
op = deleteReviewerByEmailOpFactory.create(rsrc.getReviewerByEmail(), input);
} else {
op = deleteReviewerOpFactory.create(rsrc.getReviewerUser().getAccount(), input);
}
bu.addOp(rsrc.getChange().getId(), op);
bu.execute();
}
return Response.none();
}
private class Op implements BatchUpdateOp {
private final Account reviewer;
private final DeleteReviewerInput input;
ChangeMessage changeMessage;
Change currChange;
PatchSet currPs;
Map<String, Short> newApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
Op(Account reviewerAccount, DeleteReviewerInput input) {
this.reviewer = reviewerAccount;
this.input = input;
}
@Override
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException {
Account.Id reviewerId = reviewer.getId();
if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
throw new ResourceNotFoundException();
}
currChange = ctx.getChange();
currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
StringBuilder msg = new StringBuilder();
msg.append("Removed reviewer " + reviewer.getFullName());
StringBuilder removedVotesMsg = new StringBuilder();
removedVotesMsg.append(" with the following votes:\n\n");
List<PatchSetApproval> del = new ArrayList<>();
boolean votesRemoved = false;
for (PatchSetApproval a : approvals(ctx, reviewerId)) {
if (ctx.getControl().canRemoveReviewer(a)) {
del.add(a);
if (a.getPatchSetId().equals(currPs.getId()) && a.getValue() != 0) {
oldApprovals.put(a.getLabel(), a.getValue());
removedVotesMsg
.append("* ")
.append(a.getLabel())
.append(formatLabelValue(a.getValue()))
.append(" by ")
.append(userFactory.create(a.getAccountId()).getNameEmail())
.append("\n");
votesRemoved = true;
}
} else {
throw new AuthException("delete reviewer not permitted");
}
}
if (votesRemoved) {
msg.append(removedVotesMsg);
} else {
msg.append(".");
}
ctx.getDb().patchSetApprovals().delete(del);
ChangeUpdate update = ctx.getUpdate(currPs.getId());
update.removeReviewer(reviewerId);
changeMessage =
ChangeMessagesUtil.newMessage(
ctx, msg.toString(), ChangeMessagesUtil.TAG_DELETE_REVIEWER);
cmUtil.addChangeMessage(ctx.getDb(), update, changeMessage);
return true;
}
@Override
public void postUpdate(Context ctx) {
if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
emailReviewers(ctx.getProject(), currChange, changeMessage);
}
reviewerDeleted.fire(
currChange,
currPs,
reviewer,
ctx.getAccount(),
changeMessage.getMessage(),
newApprovals,
oldApprovals,
input.notify,
ctx.getWhen());
}
private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId)
throws OrmException {
Change.Id changeId = ctx.getNotes().getChangeId();
Iterable<PatchSetApproval> approvals;
PrimaryStorage r = PrimaryStorage.of(ctx.getChange());
if (migration.readChanges() && r == PrimaryStorage.REVIEW_DB) {
// Because NoteDb and ReviewDb have different semantics for zero-value
// approvals, we must fall back to ReviewDb as the source of truth here.
ReviewDb db = ctx.getDb();
if (db instanceof BatchUpdateReviewDb) {
db = ((BatchUpdateReviewDb) db).unsafeGetDelegate();
}
db = ReviewDbUtil.unwrapDb(db);
approvals = db.patchSetApprovals().byChange(changeId);
} else {
approvals = approvalsUtil.byChange(ctx.getDb(), ctx.getNotes()).values();
}
return Iterables.filter(approvals, psa -> accountId.equals(psa.getAccountId()));
}
private String formatLabelValue(short value) {
if (value > 0) {
return "+" + value;
}
return Short.toString(value);
}
private void emailReviewers(
Project.NameKey projectName, Change change, ChangeMessage changeMessage) {
Account.Id userId = user.get().getAccountId();
if (userId.equals(reviewer.getId())) {
// The user knows they removed themselves, don't bother emailing them.
return;
}
try {
DeleteReviewerSender cm = deleteReviewerSenderFactory.create(projectName, change.getId());
cm.setFrom(userId);
cm.addReviewers(Collections.singleton(reviewer.getId()));
cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
cm.setNotify(input.notify);
cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
cm.send();
} catch (Exception err) {
log.error("Cannot email update for change " + change.getId(), err);
}
}
}
}

96
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java Normal file
View File

@@ -0,0 +1,96 @@
// Copyright (C) 2017 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.api.changes.DeleteReviewerInput;
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.server.ChangeUtil;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Collections;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class DeleteReviewerByEmailOp implements BatchUpdateOp {
private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
public interface Factory {
DeleteReviewerByEmailOp create(Address reviewer, DeleteReviewerInput input);
}
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
private final NotifyUtil notifyUtil;
private final Address reviewer;
private final DeleteReviewerInput input;
private ChangeMessage changeMessage;
private Change.Id changeId;
@Inject
DeleteReviewerByEmailOp(
DeleteReviewerSender.Factory deleteReviewerSenderFactory,
NotifyUtil notifyUtil,
@Assisted Address reviewer,
@Assisted DeleteReviewerInput input) {
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
this.notifyUtil = notifyUtil;
this.reviewer = reviewer;
this.input = input;
}
@Override
public boolean updateChange(ChangeContext ctx) throws OrmException {
changeId = ctx.getChange().getId();
PatchSet.Id psId = ctx.getChange().currentPatchSetId();
String msg = "Removed reviewer " + reviewer;
changeMessage =
new ChangeMessage(
new ChangeMessage.Key(changeId, ChangeUtil.messageUuid()),
ctx.getAccountId(),
ctx.getWhen(),
psId);
changeMessage.setMessage(msg);
ctx.getUpdate(psId).setChangeMessage(msg);
ctx.getUpdate(psId).removeReviewerByEmail(reviewer);
return true;
}
@Override
public void postUpdate(Context ctx) {
if (!NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
return;
}
try {
DeleteReviewerSender cm = deleteReviewerSenderFactory.create(ctx.getProject(), changeId);
cm.setFrom(ctx.getAccountId());
cm.addReviewersByEmail(Collections.singleton(reviewer));
cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
cm.setNotify(input.notify);
cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
cm.send();
} catch (Exception err) {
log.error("Cannot email update for change " + changeId, err);
}
}
}

232
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java Normal file
View File

@@ -0,0 +1,232 @@
// Copyright (C) 2017 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.common.collect.Iterables;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
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.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.ReviewerDeleted;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.BatchUpdateReviewDb;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class DeleteReviewerOp implements BatchUpdateOp {
private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
public interface Factory {
DeleteReviewerOp create(Account reviewerAccount, DeleteReviewerInput input);
}
private final ApprovalsUtil approvalsUtil;
private final PatchSetUtil psUtil;
private final ChangeMessagesUtil cmUtil;
private final IdentifiedUser.GenericFactory userFactory;
private final ReviewerDeleted reviewerDeleted;
private final Provider<IdentifiedUser> user;
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
private final NotesMigration migration;
private final NotifyUtil notifyUtil;
private final Account reviewer;
private final DeleteReviewerInput input;
ChangeMessage changeMessage;
Change currChange;
PatchSet currPs;
Map<String, Short> newApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
@Inject
DeleteReviewerOp(
ApprovalsUtil approvalsUtil,
PatchSetUtil psUtil,
ChangeMessagesUtil cmUtil,
IdentifiedUser.GenericFactory userFactory,
ReviewerDeleted reviewerDeleted,
Provider<IdentifiedUser> user,
DeleteReviewerSender.Factory deleteReviewerSenderFactory,
NotesMigration migration,
NotifyUtil notifyUtil,
@Assisted Account reviewerAccount,
@Assisted DeleteReviewerInput input) {
this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil;
this.cmUtil = cmUtil;
this.userFactory = userFactory;
this.reviewerDeleted = reviewerDeleted;
this.user = user;
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
this.migration = migration;
this.notifyUtil = notifyUtil;
this.reviewer = reviewerAccount;
this.input = input;
}
@Override
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException {
Account.Id reviewerId = reviewer.getId();
if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
throw new ResourceNotFoundException();
}
currChange = ctx.getChange();
currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
StringBuilder msg = new StringBuilder();
msg.append("Removed reviewer " + reviewer.getFullName());
StringBuilder removedVotesMsg = new StringBuilder();
removedVotesMsg.append(" with the following votes:\n\n");
List<PatchSetApproval> del = new ArrayList<>();
boolean votesRemoved = false;
for (PatchSetApproval a : approvals(ctx, reviewerId)) {
if (ctx.getControl().canRemoveReviewer(a)) {
del.add(a);
if (a.getPatchSetId().equals(currPs.getId()) && a.getValue() != 0) {
oldApprovals.put(a.getLabel(), a.getValue());
removedVotesMsg
.append("* ")
.append(a.getLabel())
.append(formatLabelValue(a.getValue()))
.append(" by ")
.append(userFactory.create(a.getAccountId()).getNameEmail())
.append("\n");
votesRemoved = true;
}
} else {
throw new AuthException("delete reviewer not permitted");
}
}
if (votesRemoved) {
msg.append(removedVotesMsg);
} else {
msg.append(".");
}
ctx.getDb().patchSetApprovals().delete(del);
ChangeUpdate update = ctx.getUpdate(currPs.getId());
update.removeReviewer(reviewerId);
changeMessage =
ChangeMessagesUtil.newMessage(ctx, msg.toString(), ChangeMessagesUtil.TAG_DELETE_REVIEWER);
cmUtil.addChangeMessage(ctx.getDb(), update, changeMessage);
return true;
}
@Override
public void postUpdate(Context ctx) {
if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
emailReviewers(ctx.getProject(), currChange, changeMessage);
}
reviewerDeleted.fire(
currChange,
currPs,
reviewer,
ctx.getAccount(),
changeMessage.getMessage(),
newApprovals,
oldApprovals,
input.notify,
ctx.getWhen());
}
private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId)
throws OrmException {
Change.Id changeId = ctx.getNotes().getChangeId();
Iterable<PatchSetApproval> approvals;
PrimaryStorage r = PrimaryStorage.of(ctx.getChange());
if (migration.readChanges() && r == PrimaryStorage.REVIEW_DB) {
// Because NoteDb and ReviewDb have different semantics for zero-value
// approvals, we must fall back to ReviewDb as the source of truth here.
ReviewDb db = ctx.getDb();
if (db instanceof BatchUpdateReviewDb) {
db = ((BatchUpdateReviewDb) db).unsafeGetDelegate();
}
db = ReviewDbUtil.unwrapDb(db);
approvals = db.patchSetApprovals().byChange(changeId);
} else {
approvals = approvalsUtil.byChange(ctx.getDb(), ctx.getNotes()).values();
}
return Iterables.filter(approvals, psa -> accountId.equals(psa.getAccountId()));
}
private String formatLabelValue(short value) {
if (value > 0) {
return "+" + value;
}
return Short.toString(value);
}
private void emailReviewers(
Project.NameKey projectName, Change change, ChangeMessage changeMessage) {
Account.Id userId = user.get().getAccountId();
if (userId.equals(reviewer.getId())) {
// The user knows they removed themselves, don't bother emailing them.
return;
}
try {
DeleteReviewerSender cm = deleteReviewerSenderFactory.create(projectName, change.getId());
cm.setFrom(userId);
cm.addReviewers(Collections.singleton(reviewer.getId()));
cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
cm.setNotify(input.notify);
cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
cm.send();
} catch (Exception err) {
log.error("Cannot email update for change " + change.getId(), err);
}
}
}

12
gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.mail.Address;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -48,11 +49,16 @@ class ListReviewers implements RestReadView<ChangeResource> {
@Override
public List<ReviewerInfo> apply(ChangeResource rsrc) throws OrmException {
Map<Account.Id, ReviewerResource> reviewers = new LinkedHashMap<>();
Map<String, ReviewerResource> reviewers = new LinkedHashMap<>();
ReviewDb db = dbProvider.get();
for (Account.Id accountId : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
if (!reviewers.containsKey(accountId)) {
reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
if (!reviewers.containsKey(accountId.toString())) {
reviewers.put(accountId.toString(), resourceFactory.create(rsrc, accountId));
}
}
for (Address adr : rsrc.getNotes().getReviewersByEmail().all()) {
if (!reviewers.containsKey(adr.toString())) {
reviewers.put(adr.toString(), new ReviewerResource(rsrc, adr));
}
}
return json.format(reviewers.values());

12
gerrit-server/src/main/java/com/google/gerrit/server/change/ListRevisionReviewers.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.mail.Address;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -54,11 +55,16 @@ class ListRevisionReviewers implements RestReadView<RevisionResource> {
throw new MethodNotAllowedException("Cannot list reviewers on non-current patch set");
}
Map<Account.Id, ReviewerResource> reviewers = new LinkedHashMap<>();
Map<String, ReviewerResource> reviewers = new LinkedHashMap<>();
ReviewDb db = dbProvider.get();
for (Account.Id accountId : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
if (!reviewers.containsKey(accountId)) {
reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
if (!reviewers.containsKey(accountId.toString())) {
reviewers.put(accountId.toString(), resourceFactory.create(rsrc, accountId));
}
}
for (Address address : rsrc.getNotes().getReviewersByEmail().all()) {
if (!reviewers.containsKey(address.toString())) {
reviewers.put(address.toString(), new ReviewerResource(rsrc, address));
}
}
return json.format(reviewers.values());

2
gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
View File

@@ -161,5 +161,7 @@ public class Module extends RestApiModule {
factory(SetAssigneeOp.Factory.class);
factory(SetHashtagsOp.Factory.class);
factory(ChangeResource.Factory.class);
factory(DeleteReviewerOp.Factory.class);
factory(DeleteReviewerByEmailOp.Factory.class);
}
}

7
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -86,6 +86,7 @@ import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
@@ -313,14 +314,18 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
ListMultimap<RecipientType, Account.Id> accountsToNotify) {
List<Account.Id> to = new ArrayList<>();
List<Account.Id> cc = new ArrayList<>();
List<Address> toByEmail = new ArrayList<>();
List<Address> ccByEmail = new ArrayList<>();
for (PostReviewers.Addition addition : reviewerAdditions) {
if (addition.op.state == ReviewerState.REVIEWER) {
to.addAll(addition.op.reviewers.keySet());
toByEmail.addAll(addition.op.reviewersByEmail);
} else if (addition.op.state == ReviewerState.CC) {
cc.addAll(addition.op.reviewers.keySet());
ccByEmail.addAll(addition.op.reviewersByEmail);
}
}
postReviewers.emailReviewers(change, to, cc, notify, accountsToNotify);
postReviewers.emailReviewers(change, to, cc, toByEmail, ccByEmail, notify, accountsToNotify);
}
private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in)

183
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
@@ -32,6 +33,7 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -42,6 +44,7 @@ import com.google.gerrit.reviewdb.client.Change;
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.AnonymousUser;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -52,12 +55,17 @@ import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.send.AddReviewerSender;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -104,6 +112,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
private final NotesMigration migration;
private final AccountCache accountCache;
private final NotifyUtil notifyUtil;
private final ProjectCache projectCache;
private final Provider<AnonymousUser> anonymousProvider;
@Inject
PostReviewers(
@@ -124,7 +134,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
ReviewerAdded reviewerAdded,
NotesMigration migration,
AccountCache accountCache,
NotifyUtil notifyUtil) {
NotifyUtil notifyUtil,
ProjectCache projectCache,
Provider<AnonymousUser> anonymousProvider) {
this.accounts = accounts;
this.reviewerFactory = reviewerFactory;
this.approvalsUtil = approvalsUtil;
@@ -143,6 +155,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
this.migration = migration;
this.accountCache = accountCache;
this.notifyUtil = notifyUtil;
this.projectCache = projectCache;
this.anonymousProvider = anonymousProvider;
}
@Override
@@ -167,6 +181,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
return addition.result;
}
// TODO(hiesel) Refactor this as it starts to become unreadable
public Addition prepareApplication(
ChangeResource rsrc, AddReviewerInput input, boolean allowGroup)
throws OrmException, RestApiException, IOException {
@@ -174,17 +189,28 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
try {
accountId = accounts.parse(input.reviewer).getAccountId();
} catch (UnprocessableEntityException e) {
ProjectConfig projectConfig = projectCache.checkedGet(rsrc.getProject()).getConfig();
if (allowGroup) {
try {
return putGroup(rsrc, input);
} catch (UnprocessableEntityException e2) {
throw new UnprocessableEntityException(
MessageFormat.format(
ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
if (!projectConfig.getEnableReviewerByEmail()) {
throw new UnprocessableEntityException(
MessageFormat.format(
ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
}
}
}
throw new UnprocessableEntityException(
MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
if (!projectConfig.getEnableReviewerByEmail()) {
throw new UnprocessableEntityException(
MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
}
return putAccountByEmail(
input.reviewer,
rsrc,
input.state(),
input.notify,
notifyUtil.resolveAccounts(input.notifyDetails));
}
return putAccount(
input.reviewer,
@@ -199,6 +225,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
user.getUserName(),
revision.getChangeResource(),
ImmutableMap.of(user.getAccountId(), revision.getControl()),
null,
CC,
NotifyHandling.NONE,
ImmutableListMultimap.of());
@@ -218,6 +245,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
reviewer,
rsrc.getChangeResource(),
ImmutableMap.of(member.getId(), control),
null,
state,
notify,
accountsToNotify);
@@ -228,6 +256,32 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
throw new UnprocessableEntityException(String.format("Account of %s is inactive.", reviewer));
}
private Addition putAccountByEmail(
String reviewer,
ChangeResource rsrc,
ReviewerState state,
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify)
throws UnprocessableEntityException, OrmException, BadRequestException {
if (!rsrc.getControl().forUser(anonymousProvider.get()).isVisible(dbProvider.get())) {
throw new BadRequestException("change is not publicly visible");
}
if (!migration.readChanges()) {
throw new BadRequestException("feature only supported in NoteDb");
}
Address adr;
try {
adr = Address.parse(reviewer);
} catch (IllegalArgumentException e) {
throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
}
if (!OutgoingEmailValidator.isValid(adr.getEmail())) {
throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
}
return new Addition(
reviewer, rsrc, null, ImmutableList.of(adr), state, notify, accountsToNotify);
}
private Addition putGroup(ChangeResource rsrc, AddReviewerInput input)
throws RestApiException, OrmException, IOException {
GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
@@ -283,6 +337,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
input.reviewer,
rsrc,
reviewers,
null,
input.state(),
input.notify,
notifyUtil.resolveAccounts(input.notifyDetails));
@@ -314,26 +369,28 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
final Op op;
private final Map<Account.Id, ChangeControl> reviewers;
private final Collection<Address> reviewersByEmail;
protected Addition(String reviewer) {
this(reviewer, null, null, REVIEWER, null, ImmutableListMultimap.of());
this(reviewer, null, null, null, REVIEWER, null, ImmutableListMultimap.of());
}
protected Addition(
String reviewer,
ChangeResource rsrc,
Map<Account.Id, ChangeControl> reviewers,
@Nullable Map<Account.Id, ChangeControl> reviewers,
@Nullable Collection<Address> reviewersByEmail,
ReviewerState state,
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify) {
result = new AddReviewerResult(reviewer);
if (reviewers == null) {
this.reviewers = ImmutableMap.of();
this.reviewers = reviewers == null ? ImmutableMap.of() : reviewers;
this.reviewersByEmail = reviewersByEmail == null ? ImmutableList.of() : reviewersByEmail;
if (reviewers == null && reviewersByEmail == null) {
op = null;
return;
}
this.reviewers = reviewers;
op = new Op(rsrc, reviewers, state, notify, accountsToNotify);
op = new Op(rsrc, this.reviewers, this.reviewersByEmail, state, notify, accountsToNotify);
}
void gatherResults() throws OrmException {
@@ -345,6 +402,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
result.ccs.add(json.format(new ReviewerInfo(accountId.get()), reviewers.get(accountId)));
}
accountLoaderFactory.create(true).fill(result.ccs);
for (Address a : reviewersByEmail) {
result.ccs.add(new AccountInfo(a.getName(), a.getEmail()));
}
} else {
result.reviewers = Lists.newArrayListWithCapacity(op.addedReviewers.size());
for (PatchSetApproval psa : op.addedReviewers) {
@@ -356,17 +416,22 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
ImmutableList.of(psa)));
}
accountLoaderFactory.create(true).fill(result.reviewers);
for (Address a : reviewersByEmail) {
result.reviewers.add(ReviewerInfo.byEmail(a.getName(), a.getEmail()));
}
}
}
}
public class Op implements BatchUpdateOp {
final Map<Account.Id, ChangeControl> reviewers;
final Collection<Address> reviewersByEmail;
final ReviewerState state;
final NotifyHandling notify;
final ListMultimap<RecipientType, Account.Id> accountsToNotify;
List<PatchSetApproval> addedReviewers;
Collection<Account.Id> addedCCs;
List<PatchSetApproval> addedReviewers = new ArrayList<>();
Collection<Account.Id> addedCCs = new ArrayList<>();
Collection<Address> addedCCsByEmail = new ArrayList<>();
private final ChangeResource rsrc;
private PatchSet patchSet;
@@ -374,11 +439,13 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
Op(
ChangeResource rsrc,
Map<Account.Id, ChangeControl> reviewers,
Collection<Address> reviewersByEmail,
ReviewerState state,
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify) {
this.rsrc = rsrc;
this.reviewers = reviewers;
this.reviewersByEmail = reviewersByEmail;
this.state = state;
this.notify = notify;
this.accountsToNotify = checkNotNull(accountsToNotify);
@@ -387,55 +454,55 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
@Override
public boolean updateChange(ChangeContext ctx)
throws RestApiException, OrmException, IOException {
if (migration.readChanges() && state == CC) {
addedCCs =
approvalsUtil.addCcs(
ctx.getNotes(),
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
reviewers.keySet());
if (addedCCs.isEmpty()) {
return false;
}
} else {
addedReviewers =
approvalsUtil.addReviewers(
ctx.getDb(),
ctx.getNotes(),
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
rsrc.getControl().getLabelTypes(),
rsrc.getChange(),
reviewers.keySet());
if (addedReviewers.isEmpty()) {
return false;
if (!reviewers.isEmpty()) {
if (migration.readChanges() && state == CC) {
addedCCs =
approvalsUtil.addCcs(
ctx.getNotes(),
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
reviewers.keySet());
if (addedCCs.isEmpty()) {
return false;
}
} else {
addedReviewers =
approvalsUtil.addReviewers(
ctx.getDb(),
ctx.getNotes(),
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
rsrc.getControl().getLabelTypes(),
rsrc.getChange(),
reviewers.keySet());
if (addedReviewers.isEmpty()) {
return false;
}
}
}
for (Address a : reviewersByEmail) {
ctx.getUpdate(ctx.getChange().currentPatchSetId())
.putReviewerByEmail(a, ReviewerStateInternal.fromReviewerState(state));
}
patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
return true;
}
@Override
public void postUpdate(Context ctx) throws Exception {
if (addedReviewers != null || addedCCs != null) {
if (addedReviewers == null) {
addedReviewers = new ArrayList<>();
}
if (addedCCs == null) {
addedCCs = new ArrayList<>();
}
emailReviewers(
rsrc.getChange(),
Lists.transform(addedReviewers, r -> r.getAccountId()),
addedCCs,
notify,
accountsToNotify);
if (!addedReviewers.isEmpty()) {
List<Account> reviewers =
Lists.transform(
addedReviewers, psa -> accountCache.get(psa.getAccountId()).getAccount());
reviewerAdded.fire(
rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
}
emailReviewers(
rsrc.getChange(),
Lists.transform(addedReviewers, r -> r.getAccountId()),
addedCCs == null ? ImmutableList.of() : addedCCs,
reviewersByEmail,
addedCCsByEmail,
notify,
accountsToNotify);
if (!addedReviewers.isEmpty()) {
List<Account> reviewers =
Lists.transform(
addedReviewers, psa -> accountCache.get(psa.getAccountId()).getAccount());
reviewerAdded.fire(rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
}
}
}
@@ -444,9 +511,11 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
Change change,
Collection<Account.Id> added,
Collection<Account.Id> copied,
Collection<Address> addedByEmail,
Collection<Address> copiedByEmail,
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify) {
if (added.isEmpty() && copied.isEmpty()) {
if (added.isEmpty() && copied.isEmpty() && addedByEmail.isEmpty() && copiedByEmail.isEmpty()) {
return;
}
@@ -466,7 +535,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
toCopy.add(id);
}
}
if (toMail.isEmpty() && toCopy.isEmpty()) {
if (toMail.isEmpty() && toCopy.isEmpty() && addedByEmail.isEmpty() && copiedByEmail.isEmpty()) {
return;
}
@@ -478,7 +547,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
cm.setAccountsToNotify(accountsToNotify);
cm.setFrom(userId);
cm.addReviewers(toMail);
cm.addReviewersByEmail(addedByEmail);
cm.addExtraCC(toCopy);
cm.addExtraCCByEmail(copiedByEmail);
cm.send();
} catch (Exception err) {
log.error("Cannot send email to new reviewers of change " + change.getId(), err);

46
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java
View File

@@ -14,11 +14,15 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.TypeLiteral;
import com.google.inject.assistedinject.Assisted;
@@ -36,7 +40,8 @@ public class ReviewerResource implements RestResource {
private final ChangeResource change;
private final RevisionResource revision;
private final IdentifiedUser user;
@Nullable private final IdentifiedUser user;
@Nullable private final Address address;
@AssistedInject
ReviewerResource(
@@ -44,8 +49,9 @@ public class ReviewerResource implements RestResource {
@Assisted ChangeResource change,
@Assisted Account.Id id) {
this.change = change;
this.revision = null;
this.user = userFactory.create(id);
this.revision = null;
this.address = null;
}
@AssistedInject
@@ -56,6 +62,21 @@ public class ReviewerResource implements RestResource {
this.revision = revision;
this.change = revision.getChangeResource();
this.user = userFactory.create(id);
this.address = null;
}
ReviewerResource(ChangeResource change, Address address) {
this.change = change;
this.address = address;
this.revision = null;
this.user = null;
}
ReviewerResource(RevisionResource revision, Address address) {
this.revision = revision;
this.change = revision.getChangeResource();
this.address = address;
this.user = null;
}
public ChangeResource getChangeResource() {
@@ -75,10 +96,28 @@ public class ReviewerResource implements RestResource {
}
public IdentifiedUser getReviewerUser() {
checkArgument(user != null, "no user provided");
return user;
}
public Address getReviewerByEmail() {
checkArgument(address != null, "no address provided");
return address;
}
/**
* Check if this resource was constructed by email or by {@code Account.Id}.
*
* @return true if the resource was constructed by providing an {@code Address}; false if the
* resource was constructed by providing an {@code Account.Id}.
*/
public boolean isByEmail() {
return user == null;
}
/**
* Get the control for the caller's user.
*
* @return the control for the caller's user (as opposed to the reviewer's user as returned by
* {@link #getReviewerControl()}).
*/
@@ -87,10 +126,13 @@ public class ReviewerResource implements RestResource {
}
/**
* Get the control for the reviewer's user.
*
* @return the control for the reviewer's user (as opposed to the caller's user as returned by
* {@link #getControl()}).
*/
public ChangeControl getReviewerControl() {
checkArgument(user != null, "no user provided");
return change.getControl().forUser(user);
}
}

19
gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
View File

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.mail.Address;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -69,12 +70,26 @@ public class Reviewers implements ChildCollection<ChangeResource, ReviewerResour
@Override
public ReviewerResource parse(ChangeResource rsrc, IdString id)
throws OrmException, ResourceNotFoundException, AuthException {
Account.Id accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
Address address = Address.tryParse(id.get());
Account.Id accountId = null;
try {
accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
} catch (ResourceNotFoundException e) {
if (address == null) {
throw e;
}
}
// See if the id exists as a reviewer for this change
if (fetchAccountIds(rsrc).contains(accountId)) {
if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
return resourceFactory.create(rsrc, accountId);
}
// See if the address exists as a reviewer on the change
if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
return new ReviewerResource(rsrc, address);
}
throw new ResourceNotFoundException(id);
}

19
gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionReviewers.java
View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.mail.Address;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -73,14 +74,28 @@ public class RevisionReviewers implements ChildCollection<RevisionResource, Revi
if (!rsrc.isCurrent()) {
throw new MethodNotAllowedException("Cannot access on non-current patch set");
}
Address address = Address.tryParse(id.get());
Account.Id accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
Account.Id accountId = null;
try {
accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
} catch (ResourceNotFoundException e) {
if (address == null) {
throw e;
}
}
Collection<Account.Id> reviewers =
approvalsUtil.getReviewers(dbProvider.get(), rsrc.getNotes()).all();
// See if the id exists as a reviewer for this change
if (reviewers.contains(accountId)) {
return resourceFactory.create(rsrc, accountId);
}
// See if the address exists as a reviewer on the change
if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
return new ReviewerResource(rsrc, address);
}
throw new ResourceNotFoundException(id);
}
}

54
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@@ -155,6 +155,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
ImmutableSet.of(
"MaxWithBlock", "AnyWithBlock", "MaxNoBlock", "NoBlock", "NoOp", "PatchSetLock");
private static final String REVIEWER = "reviewer";
private static final String KEY_ENABLE_REVIEWER_BY_EMAIL = "enableByEmail";
private static final String LEGACY_PERMISSION_PUSH_TAG = "pushTag";
private static final String LEGACY_PERMISSION_PUSH_SIGNED_TAG = "pushSignedTag";
@@ -182,6 +185,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private boolean checkReceivedObjects;
private Set<String> sectionsWithUnknownPermissions;
private boolean hasLegacyPermissions;
private boolean enableReviewerByEmail;
public static ProjectConfig read(MetaDataUpdate update)
throws IOException, ConfigInvalidException {
@@ -435,6 +439,16 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
return checkReceivedObjects;
}
/** @return the enableReviewerByEmail for this project, default is false. */
public boolean getEnableReviewerByEmail() {
return enableReviewerByEmail;
}
/** Set enableReviewerByEmail for this project, default is false. */
public void setEnableReviewerByEmail(boolean val) {
enableReviewerByEmail = val;
}
/**
* Check all GroupReferences use current group name, repairing stale ones.
*
@@ -526,6 +540,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
mimeTypes = new ConfiguredMimeTypes(projectName.get(), rc);
loadPluginSections(rc);
loadReceiveSection(rc);
loadReviewerSection(rc);
}
private void loadAccountsSection(Config rc, Map<String, GroupReference> groupsByName) {
@@ -933,6 +948,10 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
maxObjectSizeLimit = rc.getLong(RECEIVE, null, KEY_MAX_OBJECT_SIZE_LIMIT, 0);
}
private void loadReviewerSection(Config rc) {
enableReviewerByEmail = rc.getBoolean(REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, false);
}
private void loadPluginSections(Config rc) {
pluginConfigs = new HashMap<>();
for (String plugin : rc.getSubsections(PLUGIN)) {
@@ -1067,6 +1086,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
saveAccessSections(rc, keepGroups);
saveNotifySections(rc, keepGroups);
savePluginSections(rc, keepGroups);
saveReviewerSection(rc);
groupList.retainUUIDs(keepGroups);
saveLabelSections(rc);
saveSubscribeSections(rc);
@@ -1288,40 +1308,55 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_ALLOW_POST_SUBMIT,
label.allowPostSubmit(),
LabelType.DEF_ALLOW_POST_SUBMIT);
setBooleanConfigKey(
rc, name, KEY_COPY_MIN_SCORE, label.isCopyMinScore(), LabelType.DEF_COPY_MIN_SCORE);
setBooleanConfigKey(
rc, name, KEY_COPY_MAX_SCORE, label.isCopyMaxScore(), LabelType.DEF_COPY_MAX_SCORE);
rc,
LABEL,
name,
KEY_COPY_MIN_SCORE,
label.isCopyMinScore(),
LabelType.DEF_COPY_MIN_SCORE);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_COPY_MAX_SCORE,
label.isCopyMaxScore(),
LabelType.DEF_COPY_MAX_SCORE);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE,
label.isCopyAllScoresOnTrivialRebase(),
LabelType.DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE,
label.isCopyAllScoresIfNoCodeChange(),
LabelType.DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_COPY_ALL_SCORES_IF_NO_CHANGE,
label.isCopyAllScoresIfNoChange(),
LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE,
label.isCopyAllScoresOnMergeFirstParentUpdate(),
LabelType.DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
setBooleanConfigKey(
rc, name, KEY_CAN_OVERRIDE, label.canOverride(), LabelType.DEF_CAN_OVERRIDE);
rc, LABEL, name, KEY_CAN_OVERRIDE, label.canOverride(), LabelType.DEF_CAN_OVERRIDE);
List<String> values = Lists.newArrayListWithCapacity(label.getValues().size());
for (LabelValue value : label.getValues()) {
values.add(value.format());
@@ -1335,11 +1370,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
private static void setBooleanConfigKey(
Config rc, String name, String key, boolean value, boolean defaultValue) {
Config rc, String section, String name, String key, boolean value, boolean defaultValue) {
if (value == defaultValue) {
rc.unset(LABEL, name, key);
rc.unset(section, name, key);
} else {
rc.setBoolean(LABEL, name, key, value);
rc.setBoolean(section, name, key, value);
}
}
@@ -1367,6 +1402,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
}
private void saveReviewerSection(Config rc) {
setBooleanConfigKey(
rc, REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, enableReviewerByEmail, false);
}
private void saveGroupList() throws IOException {
saveUTF8(GroupList.FILE_NAME, groupList.asText());
}

8
gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java
View File

@@ -42,6 +42,14 @@ public class Address {
throw new IllegalArgumentException("Invalid email address: " + in);
}
public static Address tryParse(String in) {
try {
return parse(in);
} catch (IllegalArgumentException e) {
return null;
}
}
final String name;
final String email;

14
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
View File

@@ -180,6 +180,20 @@ public abstract class ChangeEmail extends NotificationEmail {
setHeader("X-Gerrit-Change-Number", "" + change.getChangeId());
setChangeUrlHeader();
setCommitIdHeader();
if (notify.ordinal() >= NotifyHandling.OWNER_REVIEWERS.ordinal()) {
try {
// TODO(hiesel) Load from index instead
addByEmail(
RecipientType.CC,
changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.CC));
addByEmail(
RecipientType.TO,
changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.REVIEWER));
} catch (OrmException e) {
throw new EmailException("Failed to add unregistered CCs " + change.getChangeId(), e);
}
}
}
private void setChangeUrlHeader() {

12
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.account.WatchConfig.NotifyType;
import com.google.gerrit.server.mail.Address;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -32,6 +33,7 @@ import java.util.Set;
/** Let users know that a reviewer and possibly her review have been removed. */
public class DeleteReviewerSender extends ReplyToChangeSender {
private final Set<Account.Id> reviewers = new HashSet<>();
private final Set<Address> reviewersByEmail = new HashSet<>();
public interface Factory extends ReplyToChangeSender.Factory<DeleteReviewerSender> {
@Override
@@ -49,6 +51,10 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
reviewers.addAll(cc);
}
public void addReviewersByEmail(Collection<Address> cc) {
reviewersByEmail.addAll(cc);
}
@Override
protected void init() throws EmailException {
super.init();
@@ -58,6 +64,7 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
ccExistingReviewers();
includeWatchers(NotifyType.ALL_COMMENTS);
add(RecipientType.TO, reviewers);
addByEmail(RecipientType.TO, reviewersByEmail);
removeUsersThatIgnoredTheChange();
}
@@ -70,13 +77,16 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
}
public List<String> getReviewerNames() {
if (reviewers.isEmpty()) {
if (reviewers.isEmpty() && reviewersByEmail.isEmpty()) {
return null;
}
List<String> names = new ArrayList<>();
for (Account.Id id : reviewers) {
names.add(getNameFor(id));
}
for (Address a : reviewersByEmail) {
names.add(a.toString());
}
return names;
}

13
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import java.util.ArrayList;
@@ -28,7 +29,9 @@ import java.util.Set;
/** Sends an email alerting a user to a new change for them to review. */
public abstract class NewChangeSender extends ChangeEmail {
private final Set<Account.Id> reviewers = new HashSet<>();
private final Set<Address> reviewersByEmail = new HashSet<>();
private final Set<Account.Id> extraCC = new HashSet<>();
private final Set<Address> extraCCByEmail = new HashSet<>();
protected NewChangeSender(EmailArguments ea, ChangeData cd) throws OrmException {
super(ea, "newchange", cd);
@@ -38,10 +41,18 @@ public abstract class NewChangeSender extends ChangeEmail {
reviewers.addAll(cc);
}
public void addReviewersByEmail(final Collection<Address> cc) {
reviewersByEmail.addAll(cc);
}
public void addExtraCC(final Collection<Account.Id> cc) {
extraCC.addAll(cc);
}
public void addExtraCCByEmail(final Collection<Address> cc) {
extraCCByEmail.addAll(cc);
}
@Override
protected void init() throws EmailException {
super.init();
@@ -55,9 +66,11 @@ public abstract class NewChangeSender extends ChangeEmail {
case ALL:
default:
add(RecipientType.CC, extraCC);
extraCCByEmail.stream().forEach(cc -> add(RecipientType.CC, cc));
//$FALL-THROUGH$
case OWNER_REVIEWERS:
add(RecipientType.TO, reviewers);
addByEmail(RecipientType.TO, reviewersByEmail);
break;
}

7
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
View File

@@ -441,6 +441,13 @@ public abstract class OutgoingEmail {
}
}
/** Schedule this message for delivery to the listed address. */
protected void addByEmail(final RecipientType rt, final Collection<Address> list) {
for (final Address id : list) {
add(rt, id);
}
}
protected void add(final RecipientType rt, final UserIdentity who) {
if (who != null && who.getAccount() != null) {
add(rt, who.getAccount());

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@@ -48,6 +48,7 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.git.RefCache;
@@ -428,6 +429,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return state.reviewers();
}
/** @return reviewers that do not currently have a Gerrit account and were added by email. */
public ReviewerByEmailSet getReviewersByEmail() {
return state.reviewersByEmail();
}
public ImmutableList<ReviewerStatusUpdate> getReviewerUpdates() {
return state.reviewerUpdates();
}

33
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -63,8 +63,10 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.LabelVote;
import java.io.IOException;
@@ -128,6 +130,7 @@ class ChangeNotesParser {
// Private final but mutable members initialized in the constructor and filled
// in during the parsing process.
private final Table<Account.Id, ReviewerStateInternal, Timestamp> reviewers;
private final Table<Address, ReviewerStateInternal, Timestamp> reviewersByEmail;
private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates;
private final List<SubmitRecord> submitRecords;
@@ -174,6 +177,7 @@ class ChangeNotesParser {
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
reviewers = HashBasedTable.create();
reviewersByEmail = HashBasedTable.create();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
@@ -201,6 +205,7 @@ class ChangeNotesParser {
parseNotes();
allPastReviewers.addAll(reviewers.rowKeySet());
pruneReviewers();
pruneReviewersByEmail();
updatePatchSetStates();
checkMandatoryFooters();
@@ -234,6 +239,7 @@ class ChangeNotesParser {
patchSets,
buildApprovals(),
ReviewerSet.fromTable(Tables.transpose(reviewers)),
ReviewerByEmailSet.fromTable(Tables.transpose(reviewersByEmail)),
allPastReviewers,
buildReviewerUpdates(),
submitRecords,
@@ -374,6 +380,9 @@ class ChangeNotesParser {
for (String line : commit.getFooterLineValues(state.getFooterKey())) {
parseReviewer(ts, state, line);
}
for (String line : commit.getFooterLineValues(state.getByEmailFooterKey())) {
parseReviewerByEmail(ts, state, line);
}
// Don't update timestamp when a reviewer was added, matching RevewDb
// behavior.
}
@@ -917,6 +926,19 @@ class ChangeNotesParser {
}
}
private void parseReviewerByEmail(Timestamp ts, ReviewerStateInternal state, String line)
throws ConfigInvalidException {
Address adr;
try {
adr = Address.parse(line);
} catch (IllegalArgumentException e) {
throw invalidFooter(state.getByEmailFooterKey(), line);
}
if (!reviewersByEmail.containsRow(adr)) {
reviewersByEmail.put(adr, state, ts);
}
}
private void parseReadOnlyUntil(ChangeNotesCommit commit) throws ConfigInvalidException {
String raw = parseOneFooter(commit, FOOTER_READ_ONLY_UNTIL);
if (raw == null) {
@@ -956,6 +978,17 @@ class ChangeNotesParser {
}
}
private void pruneReviewersByEmail() {
Iterator<Table.Cell<Address, ReviewerStateInternal, Timestamp>> rit =
reviewersByEmail.cellSet().iterator();
while (rit.hasNext()) {
Table.Cell<Address, ReviewerStateInternal, Timestamp> e = rit.next();
if (e.getColumnKey() == ReviewerStateInternal.REMOVED) {
rit.remove();
}
}
}
private void updatePatchSetStates() {
Set<PatchSet.Id> missing = new TreeSet<>(ReviewDbUtil.intKeyOrdering());
for (Iterator<PatchSet> it = patchSets.values().iterator(); it.hasNext(); ) {

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@@ -34,6 +34,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
@@ -65,6 +66,7 @@ public abstract class ChangeNotesState {
ImmutableList.of(),
ImmutableList.of(),
ReviewerSet.empty(),
ReviewerByEmailSet.empty(),
ImmutableList.of(),
ImmutableList.of(),
ImmutableList.of(),
@@ -95,6 +97,7 @@ public abstract class ChangeNotesState {
Map<PatchSet.Id, PatchSet> patchSets,
ListMultimap<PatchSet.Id, PatchSetApproval> approvals,
ReviewerSet reviewers,
ReviewerByEmailSet reviewersByEmail,
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
List<SubmitRecord> submitRecords,
@@ -128,6 +131,7 @@ public abstract class ChangeNotesState {
ImmutableList.copyOf(patchSets.entrySet()),
ImmutableList.copyOf(approvals.entries()),
reviewers,
reviewersByEmail,
ImmutableList.copyOf(allPastReviewers),
ImmutableList.copyOf(reviewerUpdates),
ImmutableList.copyOf(submitRecords),
@@ -204,6 +208,8 @@ public abstract class ChangeNotesState {
abstract ReviewerSet reviewers();
abstract ReviewerByEmailSet reviewersByEmail();
abstract ImmutableList<Account.Id> allPastReviewers();
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();

16
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -61,6 +61,7 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.LabelVote;
@@ -128,6 +129,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private final Table<String, Account.Id, Optional<Short>> approvals;
private final Map<Account.Id, ReviewerStateInternal> reviewers = new LinkedHashMap<>();
private final Map<Address, ReviewerStateInternal> reviewersByEmail = new LinkedHashMap<>();
private final List<Comment> comments = new ArrayList<>();
private String commitSubject;
@@ -471,6 +473,15 @@ public class ChangeUpdate extends AbstractChangeUpdate {
reviewers.put(reviewer, ReviewerStateInternal.REMOVED);
}
public void putReviewerByEmail(Address reviewer, ReviewerStateInternal type) {
checkArgument(type != ReviewerStateInternal.REMOVED, "invalid ReviewerType");
reviewersByEmail.put(reviewer, type);
}
public void removeReviewerByEmail(Address reviewer) {
reviewersByEmail.put(reviewer, ReviewerStateInternal.REMOVED);
}
public void setPatchSetState(PatchSetState psState) {
this.psState = psState;
}
@@ -660,6 +671,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addIdent(msg, e.getKey()).append('\n');
}
for (Map.Entry<Address, ReviewerStateInternal> e : reviewersByEmail.entrySet()) {
addFooter(msg, e.getValue().getByEmailFooterKey(), e.getKey().toString());
}
for (Table.Cell<String, Account.Id, Optional<Short>> c : approvals.cellSet()) {
addFooter(msg, FOOTER_LABEL);
// Label names/values are safe to append without sanitizing.
@@ -749,6 +764,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
&& changeMessage == null
&& comments.isEmpty()
&& reviewers.isEmpty()
&& reviewersByEmail.isEmpty()
&& changeId == null
&& branch == null
&& status == null

12
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerStateInternal.java
View File

@@ -29,13 +29,17 @@ public enum ReviewerStateInternal {
/** The user was previously a reviewer on the change, but was removed. */
REMOVED(new FooterKey("Removed"), ReviewerState.REMOVED);
public static ReviewerStateInternal fromReviewerState(ReviewerState state) {
return ReviewerStateInternal.values()[state.ordinal()];
}
static {
boolean ok = true;
if (ReviewerStateInternal.values().length != ReviewerState.values().length) {
ok = false;
}
for (ReviewerStateInternal s : ReviewerStateInternal.values()) {
ok &= s.name().equals(s.state.name());
for (int i = 0; i < ReviewerStateInternal.values().length; i++) {
ok &= ReviewerState.values()[i].equals(ReviewerStateInternal.values()[i].state);
}
if (!ok) {
throw new IllegalStateException(
@@ -58,6 +62,10 @@ public enum ReviewerStateInternal {
return footerKey;
}
FooterKey getByEmailFooterKey() {
return new FooterKey(footerKey.getName() + "-email");
}
public ReviewerState asReviewerState() {
return state;
}

3
gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfoImpl.java
View File

@@ -58,6 +58,7 @@ public class ConfigInfoImpl extends ConfigInfo {
InheritedBooleanInfo enableSignedPush = new InheritedBooleanInfo();
InheritedBooleanInfo requireSignedPush = new InheritedBooleanInfo();
InheritedBooleanInfo rejectImplicitMerges = new InheritedBooleanInfo();
InheritedBooleanInfo enableReviewerByEmail = new InheritedBooleanInfo();
useContributorAgreements.value = projectState.isUseContributorAgreements();
useSignedOffBy.value = projectState.isUseSignedOffBy();
@@ -73,6 +74,7 @@ public class ConfigInfoImpl extends ConfigInfo {
enableSignedPush.configuredValue = p.getEnableSignedPush();
requireSignedPush.configuredValue = p.getRequireSignedPush();
rejectImplicitMerges.configuredValue = p.getRejectImplicitMerges();
enableReviewerByEmail.configuredValue = p.getEnableReviewerByEmail();
ProjectState parentState = Iterables.getFirst(projectState.parents(), null);
if (parentState != null) {
@@ -85,6 +87,7 @@ public class ConfigInfoImpl extends ConfigInfo {
enableSignedPush.inheritedValue = projectState.isEnableSignedPush();
requireSignedPush.inheritedValue = projectState.isRequireSignedPush();
rejectImplicitMerges.inheritedValue = projectState.isRejectImplicitMerges();
enableReviewerByEmail.inheritedValue = projectState.isEnableReviewerByEmail();
}
this.useContributorAgreements = useContributorAgreements;

4
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
View File

@@ -394,6 +394,10 @@ public class ProjectState {
return getInheritableBoolean(Project::getRejectImplicitMerges);
}
public boolean isEnableReviewerByEmail() {
return getInheritableBoolean(Project::getEnableReviewerByEmail);
}
public LabelTypes getLabelTypes() {
Map<String, LabelType> types = new LinkedHashMap<>();
for (ProjectState s : treeInOrder()) {

6
gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
View File

@@ -105,7 +105,9 @@ public class ProjectConfigTest extends LocalDiskRepositoryTestCase {
+ " accepted = group Developers\n" //
+ " accepted = group Staff\n" //
+ " autoVerify = group Developers\n" //
+ " agreementUrl = http://www.example.com/agree\n")) //
+ " agreementUrl = http://www.example.com/agree\n" //
+ "[reviewer]\n" //
+ " enableByEmail = true\n")) //
));
ProjectConfig cfg = read(rev);
@@ -132,6 +134,8 @@ public class ProjectConfigTest extends LocalDiskRepositoryTestCase {
assertThat(submit.getExclusiveGroup()).isTrue();
assertThat(read.getExclusiveGroup()).isTrue();
assertThat(push.getExclusiveGroup()).isFalse();
assertThat(cfg.getEnableReviewerByEmail()).isTrue();
}
@Test

100
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -49,6 +49,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.RequestId;
import com.google.gerrit.testutil.TestChanges;
@@ -3298,6 +3299,105 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.isPrivate()).isFalse();
}
@Test
public void defaultReviewersByEmailIsEmpty() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().all()).isEmpty();
}
@Test
public void putReviewerByEmail() throws Exception {
Address adr = new Address("Foo Bar", "foo.bar@gerritcodereview.com");
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.REVIEWER);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().all()).containsExactly(adr);
}
@Test
public void putAndRemoveReviewerByEmail() throws Exception {
Address adr = new Address("Foo Bar", "foo.bar@gerritcodereview.com");
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.REVIEWER);
update.commit();
update = newUpdate(c, changeOwner);
update.removeReviewerByEmail(adr);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().all()).isEmpty();
}
@Test
public void putRemoveAndAddBackReviewerByEmail() throws Exception {
Address adr = new Address("Foo Bar", "foo.bar@gerritcodereview.com");
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.REVIEWER);
update.commit();
update = newUpdate(c, changeOwner);
update.removeReviewerByEmail(adr);
update.commit();
update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.CC);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().all()).containsExactly(adr);
}
@Test
public void putReviewerByEmailAndCcByEmail() throws Exception {
Address adrReviewer = new Address("Foo Bar", "foo.bar@gerritcodereview.com");
Address adrCc = new Address("Foo Bor", "foo.bar.2@gerritcodereview.com");
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adrReviewer, ReviewerStateInternal.REVIEWER);
update.commit();
update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adrCc, ReviewerStateInternal.CC);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().byState(ReviewerStateInternal.REVIEWER))
.containsExactly(adrReviewer);
assertThat(notes.getReviewersByEmail().byState(ReviewerStateInternal.CC))
.containsExactly(adrCc);
assertThat(notes.getReviewersByEmail().all()).containsExactly(adrReviewer, adrCc);
}
@Test
public void putReviewerByEmailAndChangeToCc() throws Exception {
Address adr = new Address("Foo Bar", "foo.bar@gerritcodereview.com");
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.REVIEWER);
update.commit();
update = newUpdate(c, changeOwner);
update.putReviewerByEmail(adr, ReviewerStateInternal.CC);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getReviewersByEmail().byState(ReviewerStateInternal.REVIEWER)).isEmpty();
assertThat(notes.getReviewersByEmail().byState(ReviewerStateInternal.CC)).containsExactly(adr);
assertThat(notes.getReviewersByEmail().all()).containsExactly(adr);
}
private boolean testJson() {
return noteUtil.getWriteJson();
}

27
gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.util.RequestId;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.TestChanges;
@@ -382,6 +383,32 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
assertBodyEquals("Update patch set 1\n\nPatch-set: 1\nCurrent: true\n", update.getResult());
}
@Test
public void reviewerByEmail() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(
new Address("John Doe", "j.doe@gerritcodereview.com"), ReviewerStateInternal.REVIEWER);
update.commit();
assertBodyEquals(
"Update patch set 1\n\nPatch-set: 1\n"
+ "Reviewer-email: John Doe <j.doe@gerritcodereview.com>\n",
update.getResult());
}
@Test
public void ccByEmail() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putReviewerByEmail(new Address("j.doe@gerritcodereview.com"), ReviewerStateInternal.CC);
update.commit();
assertBodyEquals(
"Update patch set 1\n\nPatch-set: 1\nCC-email: j.doe@gerritcodereview.com\n",
update.getResult());
}
private RevCommit parseCommit(ObjectId id) throws Exception {
if (id instanceof RevCommit) {
return (RevCommit) id;