opendev/gerrit
Code Issues Proposed changes

Allow adding groups as reviewer

Browse Source 
On the ChangeScreen it is now possible to add a group as reviewer for
a change. When a group is added as reviewer the group is resolved and
all its members are added as reviewers to the change.

To avoid that users accidentily add groups as reviewers that have a
large amount of members, Gerrit administrators can configure a
maximum number of reviewers that can be added at once by adding a
group as reviewer. In addition Gerrit administrators can configure
that users should confirm the adding of the reviewers if the number
of reviewers that should be added is over a certain limit.

It is also possible to add the system group 'Project Owners' as
reviewer. In this case all users which own the project are added as
reviewers.

If a user and a group have the same name, only the user is added as
reviewer.

Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
Bug: issue 881
Change-Id: I78a359f89ce045a81306e0a447384c37cda07d00
This commit is contained in:
Edwin Kempin
2011-06-29 14:35:14 +02:00
parent c4af33451e
commit 49cb3e126d
25 changed files with 701 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
Documentation/config-gerrit.txt
View File

@@ -24,6 +24,29 @@ Sample `etc/gerrit.config`:
diskbuffer = 10 m
----
[[addreviewer]]Section addreviewer
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[[addreviewer.maxWithoutConfirmation]]addreviewer.maxWithoutConfirmation::
+
The maximum number of reviewers a user can add at once by adding a
group as reviewer without being asked to confirm the operation.
+
If set to 0, the user will never be asked to confirm adding a group
as reviewer.
+
Default is 10.
[[addreviewer.maxAllowed]]addreviewer.maxAllowed::
+
The maximum number of reviewers a user can add at once by adding a
group as reviewer.
+
If set to 0, there is no limit for the number of reviewers that can
be added at once by adding a group as reviewer.
+
Default is 20.
[[auth]]Section auth
~~~~~~~~~~~~~~~~~~~~

2
gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java
View File

@@ -50,7 +50,7 @@ public interface PatchDetailService extends RemoteJsonService {
AsyncCallback<VoidResult> callback);
@SignInRequired
void addReviewers(Change.Id id, List<String> reviewers,
void addReviewers(Change.Id id, List<String> reviewers, boolean confirmed,
AsyncCallback<ReviewerResult> callback);
@SignInRequired

58
gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerInfo.java Normal file
View File

@@ -0,0 +1,58 @@
// Copyright (C) 2011 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.common.data;
/**
* Suggested reviewer for a change. Can be a user ({@link AccountInfo}) or a
* group ({@link GroupReference}).
*/
public class ReviewerInfo implements Comparable<ReviewerInfo> {
private AccountInfo accountInfo;
private GroupReference groupReference;
protected ReviewerInfo() {
}
public ReviewerInfo(final AccountInfo accountInfo) {
this.accountInfo = accountInfo;
}
public ReviewerInfo(final GroupReference groupReference) {
this.groupReference = groupReference;
}
public AccountInfo getAccountInfo() {
return accountInfo;
}
public GroupReference getGroup() {
return groupReference;
}
@Override
public int compareTo(final ReviewerInfo o) {
return getSortValue().compareTo(o.getSortValue());
}
private String getSortValue() {
if (accountInfo != null) {
if (accountInfo.getPreferredEmail() != null) {
return accountInfo.getPreferredEmail();
}
return accountInfo.getFullName().toString();
}
return groupReference.getName();
}
}

31
gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewerResult.java
View File

@@ -24,6 +24,8 @@ import java.util.List;
public class ReviewerResult {
protected List<Error> errors;
protected ChangeDetail change;
protected int memberCount;
protected boolean askForConfirmation;
public ReviewerResult() {
errors = new ArrayList<Error>();
@@ -45,10 +47,26 @@ public class ReviewerResult {
change = d;
}
public int getMemberCount() {
return memberCount;
}
public void setMemberCount(final int memberCount) {
this.memberCount = memberCount;
}
public boolean askForConfirmation() {
return askForConfirmation;
}
public void setAskForConfirmation(final boolean askForConfirmation) {
this.askForConfirmation = askForConfirmation;
}
public static class Error {
public static enum Type {
/** Name supplied does not match to a registered account. */
ACCOUNT_NOT_FOUND,
/** Name supplied does not match to a registered account or account group. */
REVIEWER_NOT_FOUND,
/** The account is inactive. */
ACCOUNT_INACTIVE,
@@ -56,6 +74,15 @@ public class ReviewerResult {
/** The account is not permitted to see the change. */
CHANGE_NOT_VISIBLE,
/** The groups has no members. */
GROUP_EMPTY,
/** The groups has too many members. */
GROUP_HAS_TOO_MANY_MEMBERS,
/** The group is not allowed to be added as reviewer. */
GROUP_NOT_ALLOWED,
/** Could not remove this reviewer from the change due to ORMException. */
COULD_NOT_REMOVE,

12
gerrit-common/src/main/java/com/google/gerrit/common/data/SuggestService.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.common.data;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.Project;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwtjsonrpc.client.RemoteJsonService;
@@ -32,4 +33,15 @@ public interface SuggestService extends RemoteJsonService {
void suggestAccountGroup(String query, int limit,
AsyncCallback<List<GroupReference>> callback);
/**
* Suggests reviewers. A reviewer can be a user or a group. Inactive users,
* the system groups {@link AccountGroup#ANONYMOUS_USERS} and
* {@link AccountGroup#REGISTERED_USERS} and groups that have more than the
* configured <code>addReviewer.maxAllowed</code> members are not suggested as
* reviewers.
* @param project the project for which reviewers should be suggested
*/
void suggestReviewer(Project.NameKey project, String query, int limit,
AsyncCallback<List<ReviewerInfo>> callback);
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@@ -46,6 +46,7 @@ public interface AdminConstants extends Constants {
String emailOnlyAuthors();
String descriptionNotifications();
String buttonSaveGroupOptions();
String suggestedGroupLabel();
String headingOwner();
String headingParentProjectName();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@@ -23,6 +23,7 @@ requireChangeID = Require <a href="http://gerrit.googlecode.com/svn/documentatio
headingGroupOptions = Group Options
isVisibleToAll = Make group visible to all registered users.
buttonSaveGroupOptions = Save Group Options
suggestedGroupLabel = group
emailOnlyAuthors = Authors
descriptionNotifications = Send email notifications about comments and actions by users in this group only to:

78
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
View File

@@ -14,13 +14,15 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.client.ConfirmationCallback;
import com.google.gerrit.client.ConfirmationDialog;
import com.google.gerrit.client.ErrorDialog;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.ReviewerSuggestOracle;
import com.google.gerrit.client.ui.AccountDashboardLink;
import com.google.gerrit.client.ui.AccountSuggestOracle;
import com.google.gerrit.client.ui.AddMemberBox;
import com.google.gerrit.common.data.AccountInfoCache;
import com.google.gerrit.common.data.ApprovalDetail;
@@ -39,11 +41,12 @@ import com.google.gwt.user.client.Element;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.Grid;
import com.google.gwt.user.client.ui.HTML;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
import com.google.gwt.user.client.ui.Image;
import com.google.gwt.user.client.ui.Panel;
import com.google.gwt.user.client.ui.PushButton;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import java.util.ArrayList;
@@ -59,6 +62,7 @@ public class ApprovalTable extends Composite {
private final Grid table;
private final Widget missing;
private final Panel addReviewer;
private final ReviewerSuggestOracle reviewerSuggestOracle;
private final AddMemberBox addMemberBox;
private Change.Id changeId;
private AccountInfoCache accountCache = AccountInfoCache.empty();
@@ -77,10 +81,10 @@ public class ApprovalTable extends Composite {
addReviewer = new FlowPanel();
addReviewer.setStyleName(Gerrit.RESOURCES.css().addReviewer());
reviewerSuggestOracle = new ReviewerSuggestOracle();
addMemberBox =
new AddMemberBox(Util.C.approvalTableAddReviewer(),
Util.C.approvalTableAddReviewerHint(),
new AccountSuggestOracle());
Util.C.approvalTableAddReviewerHint(), reviewerSuggestOracle);
addMemberBox.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
@@ -131,6 +135,8 @@ public class ApprovalTable extends Composite {
}
void display(ChangeDetail detail) {
reviewerSuggestOracle.setProject(detail.getChange().getProject());
List<String> columns = new ArrayList<String>();
List<ApprovalDetail> rows = detail.getApprovals();
@@ -242,27 +248,38 @@ public class ApprovalTable extends Composite {
}
private void doAddReviewer() {
final String nameEmail = addMemberBox.getText();
if (nameEmail.length() == 0) {
final String reviewer = addMemberBox.getText();
if (reviewer.length() == 0) {
return;
}
addMemberBox.setEnabled(false);
final List<String> reviewers = new ArrayList<String>();
reviewers.add(nameEmail);
reviewers.add(reviewer);
PatchUtil.DETAIL_SVC.addReviewers(changeId, reviewers,
addReviewers(reviewers, false);
}
private void addReviewers(final List<String> reviewers,
final boolean confirmed) {
PatchUtil.DETAIL_SVC.addReviewers(changeId, reviewers, confirmed,
new GerritCallback<ReviewerResult>() {
public void onSuccess(final ReviewerResult result) {
addMemberBox.setEnabled(true);
addMemberBox.setText("");
final ChangeDetail changeDetail = result.getChange();
if (changeDetail != null) {
setAccountInfoCache(changeDetail.getAccounts());
display(changeDetail);
}
if (!result.getErrors().isEmpty()) {
final SafeHtmlBuilder r = new SafeHtmlBuilder();
for (final ReviewerResult.Error e : result.getErrors()) {
switch (e.getType()) {
case ACCOUNT_NOT_FOUND:
r.append(Util.M.accountNotFound(e.getName()));
case REVIEWER_NOT_FOUND:
r.append(Util.M.reviewerNotFound(e.getName()));
break;
case ACCOUNT_INACTIVE:
@@ -273,6 +290,23 @@ public class ApprovalTable extends Composite {
r.append(Util.M.changeNotVisibleTo(e.getName()));
break;
case GROUP_EMPTY:
r.append(Util.M.groupIsEmpty(e.getName()));
break;
case GROUP_HAS_TOO_MANY_MEMBERS:
if (result.askForConfirmation() && !confirmed) {
askForConfirmation(e.getName(), result.getMemberCount());
return;
} else {
r.append(Util.M.groupHasTooManyMembers(e.getName()));
}
break;
case GROUP_NOT_ALLOWED:
r.append(Util.M.groupIsNotAllowed(e.getName()));
break;
default:
r.append(e.getName());
r.append(" - ");
@@ -283,14 +317,26 @@ public class ApprovalTable extends Composite {
}
new ErrorDialog(r).center();
}
final ChangeDetail r = result.getChange();
if (r != null) {
setAccountInfoCache(r.getAccounts());
display(r);
}
}
private void askForConfirmation(final String groupName,
final int memberCount) {
final StringBuilder message = new StringBuilder();
message.append("<b>");
message.append(Util.M.groupManyMembersConfirmation(groupName,
memberCount));
message.append("</b>");
final ConfirmationDialog confirmationDialog =
new ConfirmationDialog(Util.C
.approvalTableAddManyReviewersConfirmationDialogTitle(),
new HTML(message.toString()), new ConfirmationCallback() {
@Override
public void onOk() {
addReviewers(reviewers, true);
}
});
confirmationDialog.center();
}
@Override
public void onFailure(final Throwable caught) {

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
View File

@@ -81,6 +81,7 @@ public interface ChangeConstants extends Constants {
String approvalTableRemoveNotPermitted();
String approvalTableCouldNotRemove();
String approvalTableAddReviewerHint();
String approvalTableAddManyReviewersConfirmationDialogTitle();
String changeInfoBlockOwner();
String changeInfoBlockProject();

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

@@ -57,7 +57,8 @@ approvalTableReviewer = Reviewer
approvalTableAddReviewer = Add Reviewer
approvalTableRemoveNotPermitted = Not allowed to remove reviewer
approvalTableCouldNotRemove = Could not remove reviewer
approvalTableAddReviewerHint = Username or Email
approvalTableAddReviewerHint = Name or Email or Group
approvalTableAddManyReviewersConfirmationDialogTitle = Adding many reviewers
changeInfoBlockOwner = Owner
changeInfoBlockProject = Project

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
View File

@@ -51,9 +51,13 @@ public interface ChangeMessages extends Messages {
String changeQueryWindowTitle(String query);
String changeQueryPageTitle(String query);
String accountNotFound(String who);
String reviewerNotFound(String who);
String accountInactive(String who);
String changeNotVisibleTo(String who);
String groupIsEmpty(String group);
String groupIsNotAllowed(String group);
String groupHasTooManyMembers(String group);
String groupManyMembersConfirmation(String group, int memberCount);
String anonymousDownload(String protocol);
}

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
View File

@@ -32,8 +32,12 @@ lineHeader = Line {0}:
changeQueryWindowTitle = {0}
changeQueryPageTitle = Search for {0}
accountNotFound = {0} is not a registered user.
reviewerNotFound = {0} is neither a registered user nor a group.
accountInactive = {0} is not an active user.
changeNotVisibleTo = {0} cannot access the change.
groupIsEmpty = {0} has no members.
groupIsNotAllowed = {0} cannot be added as reviewer.
groupHasTooManyMembers = {0} has too many members.
groupManyMembersConfirmation = {0} has {1} members. Do you want to add them all as reviewers?
anonymousDownload = Anonymous {0}

83
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ReviewerSuggestOracle.java Normal file
View File

@@ -0,0 +1,83 @@
// Copyright (C) 2011 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.client.ui;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.RpcStatus;
import com.google.gerrit.client.admin.Util;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.common.data.AccountInfo;
import com.google.gerrit.common.data.ReviewerInfo;
import com.google.gerrit.reviewdb.Project;
import com.google.gwt.user.client.ui.SuggestOracle;
import com.google.gwtexpui.safehtml.client.HighlightSuggestOracle;
import java.util.ArrayList;
import java.util.List;
/** Suggestion Oracle for reviewers. */
public class ReviewerSuggestOracle extends HighlightSuggestOracle {
private Project.NameKey project;
@Override
protected void onRequestSuggestions(final Request req, final Callback callback) {
RpcStatus.hide(new Runnable() {
public void run() {
SuggestUtil.SVC.suggestReviewer(project, req.getQuery(),
req.getLimit(), new GerritCallback<List<ReviewerInfo>>() {
public void onSuccess(final List<ReviewerInfo> result) {
final List<ReviewerSuggestion> r =
new ArrayList<ReviewerSuggestion>(result
.size());
for (final ReviewerInfo reviewer : result) {
r.add(new ReviewerSuggestion(reviewer));
}
callback.onSuggestionsReady(req, new Response(r));
}
});
}
});
}
public void setProject(final Project.NameKey project) {
this.project = project;
}
private static class ReviewerSuggestion implements SuggestOracle.Suggestion {
private final ReviewerInfo reviewerInfo;
ReviewerSuggestion(final ReviewerInfo reviewerInfo) {
this.reviewerInfo = reviewerInfo;
}
public String getDisplayString() {
final AccountInfo accountInfo = reviewerInfo.getAccountInfo();
if (accountInfo != null) {
return FormatUtil.nameEmail(accountInfo);
}
return reviewerInfo.getGroup().getName() + " ("
+ Util.C.suggestedGroupLabel() + ")";
}
public String getReplacementString() {
final AccountInfo accountInfo = reviewerInfo.getAccountInfo();
if (accountInfo != null) {
return FormatUtil.nameEmail(accountInfo);
}
return reviewerInfo.getGroup().getName();
}
}
}

100
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.httpd.rpc;
import com.google.gerrit.common.data.AccountInfo;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.ReviewerInfo;
import com.google.gerrit.common.data.SuggestService;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.Account;
@@ -29,7 +30,10 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupMembersFactory;
import com.google.gerrit.server.account.GroupMembersFactory.Factory;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.patch.AddReviewer;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
@@ -56,29 +60,34 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
private final ProjectCache projectCache;
private final AccountCache accountCache;
private final GroupControl.Factory groupControlFactory;
private final Factory groupMembersFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<CurrentUser> currentUser;
private final SuggestAccountsEnum suggestAccounts;
private final Config cfg;
private final GroupCache groupCache;
@Inject
SuggestServiceImpl(final Provider<ReviewDb> schema,
final ProjectControl.Factory projectControlFactory,
final ProjectCache projectCache, final AccountCache accountCache,
final GroupControl.Factory groupControlFactory,
final GroupMembersFactory.Factory groupMembersFactory,
final IdentifiedUser.GenericFactory userFactory,
final Provider<CurrentUser> currentUser,
@GerritServerConfig final Config cfg,
final GroupCache groupCache) {
@GerritServerConfig final Config cfg, final GroupCache groupCache) {
super(schema, currentUser);
this.projectControlFactory = projectControlFactory;
this.projectCache = projectCache;
this.accountCache = accountCache;
this.groupControlFactory = groupControlFactory;
this.groupMembersFactory = groupMembersFactory;
this.userFactory = userFactory;
this.currentUser = currentUser;
this.suggestAccounts =
cfg.getEnum("suggest", null, "accounts", SuggestAccountsEnum.ALL);
this.cfg = cfg;
this.groupCache = groupCache;
}
@@ -106,13 +115,20 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
public void suggestAccount(final String query, final Boolean active,
final int limit, final AsyncCallback<List<AccountInfo>> callback) {
if (suggestAccounts == SuggestAccountsEnum.OFF) {
callback.onSuccess(Collections.<AccountInfo> emptyList());
return;
}
run(callback, new Action<List<AccountInfo>>() {
public List<AccountInfo> run(final ReviewDb db) throws OrmException {
return suggestAccount(db, query, active, limit);
}
});
}
private List<AccountInfo> suggestAccount(final ReviewDb db,
final String query, final Boolean active, final int limit)
throws OrmException {
if (suggestAccounts == SuggestAccountsEnum.OFF) {
return Collections.<AccountInfo> emptyList();
}
final String a = query;
final String b = a + MAX_SUFFIX;
final int max = 10;
@@ -142,8 +158,6 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
}
return new ArrayList<AccountInfo>(r.values());
}
});
}
private void addSuggestion(Map<Account.Id, AccountInfo> map, Account account,
AccountInfo info, Boolean active) {
@@ -201,13 +215,19 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
final AsyncCallback<List<GroupReference>> callback) {
run(callback, new Action<List<GroupReference>>() {
public List<GroupReference> run(final ReviewDb db) throws OrmException {
return suggestAccountGroup(db, query, limit);
}
});
}
private List<GroupReference> suggestAccountGroup(final ReviewDb db,
final String query, final int limit) throws OrmException {
final String a = query;
final String b = a + MAX_SUFFIX;
final int max = 10;
final int n = limit <= 0 ? max : Math.min(limit, max);
List<GroupReference> r = new ArrayList<GroupReference>(n);
for (AccountGroupName group : db.accountGroupNames()
.suggestByName(a, b, n)) {
for (AccountGroupName group : db.accountGroupNames().suggestByName(a, b, n)) {
try {
if (groupControlFactory.controlFor(group.getId()).isVisible()) {
AccountGroup g = groupCache.get(group.getId());
@@ -221,6 +241,64 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
}
return r;
}
@Override
public void suggestReviewer(final Project.NameKey project,
final String query, final int limit,
final AsyncCallback<List<ReviewerInfo>> callback) {
run(callback, new Action<List<ReviewerInfo>>() {
public List<ReviewerInfo> run(final ReviewDb db) throws OrmException {
final List<AccountInfo> suggestedAccounts =
suggestAccount(db, query, Boolean.TRUE, limit);
final List<ReviewerInfo> reviewer =
new ArrayList<ReviewerInfo>(suggestedAccounts.size());
for (final AccountInfo a : suggestedAccounts) {
reviewer.add(new ReviewerInfo(a));
}
final List<GroupReference> suggestedAccountGroups =
suggestAccountGroup(db, query, limit);
for (final GroupReference g : suggestedAccountGroups) {
if (suggestGroupAsReviewer(project, g)) {
reviewer.add(new ReviewerInfo(g));
}
}
Collections.sort(reviewer);
if (reviewer.size() <= limit) {
return reviewer;
} else {
return reviewer.subList(0, limit);
}
}
});
}
private boolean suggestGroupAsReviewer(final Project.NameKey project,
final GroupReference group) throws OrmException {
if (!AddReviewer.isLegalReviewerGroup(group.getUUID())) {
return false;
}
try {
final Set<Account> members =
groupMembersFactory.create(project, group.getUUID()).call();
if (members.isEmpty()) {
return false;
}
final int maxAllowed =
cfg.getInt("addreviewer", "maxAllowed",
AddReviewer.DEFAULT_MAX_REVIEWERS);
if (maxAllowed > 0 && members.size() > maxAllowed) {
return false;
}
} catch (NoSuchGroupException e) {
return false;
} catch (NoSuchProjectException e) {
return false;
}
return true;
}
}

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java
View File

@@ -32,7 +32,7 @@ public class AccountModule extends RpcServletModule {
factory(CreateGroup.Factory.class);
factory(DeleteExternalIds.Factory.class);
factory(ExternalIdDetailFactory.Factory.class);
factory(GroupDetailFactory.Factory.class);
factory(GroupDetailHandler.Factory.class);
factory(MyGroupsFactory.Factory.class);
factory(RenameGroup.Factory.class);
}

4
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
View File

@@ -63,7 +63,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
private final CreateGroup.Factory createGroupFactory;
private final RenameGroup.Factory renameGroupFactory;
private final GroupDetailFactory.Factory groupDetailFactory;
private final GroupDetailHandler.Factory groupDetailFactory;
@Inject
GroupAdminServiceImpl(final Provider<ReviewDb> schema,
@@ -75,7 +75,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
final GroupControl.Factory groupControlFactory,
final CreateGroup.Factory createGroupFactory,
final RenameGroup.Factory renameGroupFactory,
final GroupDetailFactory.Factory groupDetailFactory) {
final GroupDetailHandler.Factory groupDetailFactory) {
super(schema, currentUser);
this.identifiedUser = currentUser;
this.accountCache = accountCache;

46
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailHandler.java Normal file
View File

@@ -0,0 +1,46 @@
// Copyright (C) 2011 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.httpd.rpc.account;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.server.account.GroupDetailFactory;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
public class GroupDetailHandler extends Handler<GroupDetail> {
public interface Factory {
GroupDetailHandler create(AccountGroup.Id groupId);
}
private final GroupDetailFactory.Factory groupDetailFactory;
private final AccountGroup.Id groupId;
@Inject
GroupDetailHandler(final GroupDetailFactory.Factory groupDetailFactory,
@Assisted final AccountGroup.Id groupId) {
this.groupDetailFactory = groupDetailFactory;
this.groupId = groupId;
}
@Override
public GroupDetail call() throws OrmException, NoSuchGroupException {
return groupDetailFactory.create(groupId).call();
}
}

1
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/RenameGroup.java
View File

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupDetailFactory;
import com.google.gerrit.server.git.RenameGroupOp;
import com.google.gwtorm.client.OrmDuplicateKeyException;
import com.google.gwtorm.client.OrmException;

12
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewerHandler.java
View File

@@ -26,7 +26,8 @@ import java.util.Collection;
class AddReviewerHandler extends Handler<ReviewerResult> {
interface Factory {
AddReviewerHandler create(Change.Id changeId, Collection<String> nameOrEmails);
AddReviewerHandler create(Change.Id changeId, Collection<String> reviewers,
boolean confirmed);
}
private final AddReviewer.Factory addReviewerFactory;
@@ -34,23 +35,26 @@ class AddReviewerHandler extends Handler<ReviewerResult> {
private final Change.Id changeId;
private final Collection<String> reviewers;
private final boolean confirmed;
@Inject
AddReviewerHandler(final AddReviewer.Factory addReviewerFactory,
final ChangeDetailFactory.Factory changeDetailFactory,
@Assisted final Change.Id changeId,
@Assisted final Collection<String> nameOrEmails) {
@Assisted final Collection<String> reviewers,
@Assisted final boolean confirmed) {
this.addReviewerFactory = addReviewerFactory;
this.changeDetailFactory = changeDetailFactory;
this.changeId = changeId;
this.reviewers = nameOrEmails;
this.reviewers = reviewers;
this.confirmed = confirmed;
}
@Override
public ReviewerResult call() throws Exception {
ReviewerResult result = addReviewerFactory.create(changeId, reviewers).call();
ReviewerResult result = addReviewerFactory.create(changeId, reviewers, confirmed).call();
result.setChange(changeDetailFactory.create(changeId).call());
return result;
}

4
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
View File

@@ -155,8 +155,8 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
}
public void addReviewers(final Change.Id id, final List<String> reviewers,
final AsyncCallback<ReviewerResult> callback) {
addReviewerHandlerFactory.create(id, reviewers).to(callback);
final boolean confirmed, final AsyncCallback<ReviewerResult> callback) {
addReviewerHandlerFactory.create(id, reviewers, confirmed).to(callback);
}
public void removeReviewer(final Change.Id id, final Account.Id reviewerId,

12
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java → gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
View File

@@ -12,20 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.httpd.rpc.account;
package com.google.gerrit.server.account;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.AccountGroupInclude;
import com.google.gerrit.reviewdb.AccountGroupMember;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.server.account.AccountInfoCacheFactory;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupInfoCacheFactory;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -34,9 +29,10 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.Callable;
class GroupDetailFactory extends Handler<GroupDetail> {
interface Factory {
public class GroupDetailFactory implements Callable<GroupDetail> {
public interface Factory {
GroupDetailFactory create(AccountGroup.Id groupId);
}

122
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembersFactory.java Normal file
View File

@@ -0,0 +1,122 @@
// Copyright (C) 2011 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.account;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.AccountGroupInclude;
import com.google.gerrit.reviewdb.AccountGroupMember;
import com.google.gerrit.reviewdb.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.Callable;
public class GroupMembersFactory implements Callable<Set<Account>> {
public interface Factory {
GroupMembersFactory create(Project.NameKey project,
AccountGroup.UUID groupUUID);
}
private GroupCache groupCache;
private final GroupDetailFactory.Factory groupDetailFactory;
private final AccountCache accountCache;
private final ProjectControl.GenericFactory projectControl;
private final IdentifiedUser currentUser;
private final Project.NameKey project;
private final AccountGroup.UUID groupUUID;
@Inject
GroupMembersFactory(final GroupCache groupCache,
final GroupDetailFactory.Factory groupDetailFactory,
final AccountCache accountCache,
final ProjectControl.GenericFactory projectControl,
final IdentifiedUser currentUser,
@Assisted final Project.NameKey project,
@Assisted final AccountGroup.UUID groupUUID) {
this.groupCache = groupCache;
this.groupDetailFactory = groupDetailFactory;
this.accountCache = accountCache;
this.projectControl = projectControl;
this.currentUser = currentUser;
this.project = project;
this.groupUUID = groupUUID;
}
@Override
public Set<Account> call() throws NoSuchGroupException,
NoSuchProjectException, OrmException {
if (AccountGroup.PROJECT_OWNERS.equals(groupUUID)) {
return getProjectOwners();
}
return getAllGroupMembers(groupCache.get(groupUUID),
new HashSet<AccountGroup.Id>());
}
private Set<Account> getProjectOwners() throws NoSuchProjectException,
NoSuchGroupException, OrmException {
if (project == null) {
return Collections.emptySet();
}
final Set<AccountGroup.UUID> ownerGroups =
projectControl.controlFor(project, currentUser).getProjectState()
.getOwners();
final HashSet<Account> projectOwners = new HashSet<Account>();
for (final AccountGroup.UUID ownerGroup : ownerGroups) {
projectOwners.addAll(getAllGroupMembers(groupCache.get(ownerGroup),
new HashSet<AccountGroup.Id>()));
}
return projectOwners;
}
private Set<Account> getAllGroupMembers(final AccountGroup group,
final Set<AccountGroup.Id> seen) throws NoSuchGroupException,
OrmException {
seen.add(group.getId());
final GroupDetail groupDetail =
groupDetailFactory.create(group.getId()).call();
final Set<Account> members = new HashSet<Account>();
if (groupDetail.members != null) {
for (final AccountGroupMember member : groupDetail.members) {
members.add(accountCache.get(member.getAccountId()).getAccount());
}
}
if (groupDetail.includes != null) {
for (AccountGroupInclude groupInclude : groupDetail.includes) {
if (!seen.contains(groupInclude.getIncludeId())) {
members.addAll(getAllGroupMembers(
groupCache.get(groupInclude.getIncludeId()), seen));
}
}
}
return members;
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
View File

@@ -22,6 +22,8 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.RequestCleanup;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupDetailFactory;
import com.google.gerrit.server.account.GroupMembersFactory;
import com.google.gerrit.server.account.PerformCreateGroup;
import com.google.gerrit.server.git.CreateCodeReviewNotes;
import com.google.gerrit.server.git.MergeOp;
@@ -87,5 +89,7 @@ public class GerritRequestModule extends FactoryModule {
factory(MergeFailSender.Factory.class);
factory(RegisterNewEmailSender.Factory.class);
factory(PerformCreateGroup.Factory.class);
factory(GroupDetailFactory.Factory.class);
factory(GroupMembersFactory.Factory.class);
}
}

100
gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
import com.google.gerrit.common.data.ReviewerResult;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.ApprovalCategory;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSet;
@@ -26,12 +27,17 @@ import com.google.gerrit.reviewdb.PatchSetApproval;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupMembersFactory;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.mail.AddReviewerSender;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.Config;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
@@ -40,42 +46,56 @@ import java.util.Set;
import java.util.concurrent.Callable;
public class AddReviewer implements Callable<ReviewerResult> {
public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
public final static int DEFAULT_MAX_REVIEWERS = 20;
public interface Factory {
AddReviewer create(Change.Id changeId, Collection<String> nameOrEmails);
AddReviewer create(Change.Id changeId,
Collection<String> userNameOrEmailOrGroupNames, boolean confirmed);
}
private final AddReviewerSender.Factory addReviewerSenderFactory;
private final AccountResolver accountResolver;
private final GroupCache groupCache;
private final GroupMembersFactory.Factory groupMembersFactory;
private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db;
private final IdentifiedUser currentUser;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ApprovalCategory.Id addReviewerCategoryId;
private final Config cfg;
private final Change.Id changeId;
private final Collection<String> reviewers;
private final boolean confirmed;
@Inject
AddReviewer(final AddReviewerSender.Factory addReviewerSenderFactory,
final AccountResolver accountResolver,
final AccountResolver accountResolver, final GroupCache groupCache,
final GroupMembersFactory.Factory groupMembersFactory,
final ChangeControl.Factory changeControlFactory, final ReviewDb db,
final IdentifiedUser.GenericFactory identifiedUserFactory,
final IdentifiedUser currentUser, final ApprovalTypes approvalTypes,
@Assisted final Change.Id changeId,
@Assisted final Collection<String> nameOrEmails) {
final @GerritServerConfig Config cfg, @Assisted final Change.Id changeId,
@Assisted final Collection<String> reviewers,
@Assisted final boolean confirmed) {
this.addReviewerSenderFactory = addReviewerSenderFactory;
this.accountResolver = accountResolver;
this.groupCache = groupCache;
this.groupMembersFactory = groupMembersFactory;
this.db = db;
this.changeControlFactory = changeControlFactory;
this.identifiedUserFactory = identifiedUserFactory;
this.currentUser = currentUser;
this.cfg = cfg;
final List<ApprovalType> allTypes = approvalTypes.getApprovalTypes();
addReviewerCategoryId =
allTypes.get(allTypes.size() - 1).getCategory().getId();
this.changeId = changeId;
this.reviewers = nameOrEmails;
this.reviewers = reviewers;
this.confirmed = confirmed;
}
@Override
@@ -84,18 +104,73 @@ public class AddReviewer implements Callable<ReviewerResult> {
final ChangeControl control = changeControlFactory.validateFor(changeId);
final ReviewerResult result = new ReviewerResult();
for (final String nameOrEmail : reviewers) {
final Account account = accountResolver.find(nameOrEmail);
for (final String reviewer : reviewers) {
final Account account = accountResolver.find(reviewer);
if (account == null) {
AccountGroup group = groupCache.get(new AccountGroup.NameKey(reviewer));
if (group == null) {
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.ACCOUNT_NOT_FOUND, nameOrEmail));
ReviewerResult.Error.Type.REVIEWER_NOT_FOUND, reviewer));
continue;
}
if (!isLegalReviewerGroup(group.getGroupUUID())) {
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.GROUP_NOT_ALLOWED, reviewer));
continue;
}
final Set<Account> members =
groupMembersFactory.create(control.getProject().getNameKey(),
group.getGroupUUID()).call();
if (members == null || members.size() == 0) {
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.GROUP_EMPTY, reviewer));
continue;
}
// if maxAllowed is set to 0, it is allowed to add any number of
// reviewers
final int maxAllowed =
cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
if (maxAllowed > 0 && members.size() > maxAllowed) {
result.setMemberCount(members.size());
result.setAskForConfirmation(false);
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer));
continue;
}
// if maxWithoutCheck is set to 0, we never ask for confirmation
final int maxWithoutConfirmation =
cfg.getInt("addreviewer", "maxWithoutConfirmation",
DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
if (!confirmed && maxWithoutConfirmation > 0
&& members.size() > maxWithoutConfirmation) {
result.setMemberCount(members.size());
result.setAskForConfirmation(true);
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer));
continue;
}
for (final Account member : members) {
if (member.isActive()) {
final IdentifiedUser user =
identifiedUserFactory.create(member.getId());
if (control.forUser(user).isVisible()) {
reviewerIds.add(member.getId());
}
}
}
continue;
}
if (!account.isActive()) {
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.ACCOUNT_INACTIVE,
formatUser(account, nameOrEmail)));
formatUser(account, reviewer)));
continue;
}
@@ -103,7 +178,7 @@ public class AddReviewer implements Callable<ReviewerResult> {
if (!control.forUser(user).isVisible()) {
result.addError(new ReviewerResult.Error(
ReviewerResult.Error.Type.CHANGE_NOT_VISIBLE,
formatUser(account, nameOrEmail)));
formatUser(account, reviewer)));
continue;
}
@@ -165,4 +240,9 @@ public class AddReviewer implements Callable<ReviewerResult> {
return new PatchSetApproval(new PatchSetApproval.Key(patchSetId,
reviewerId, addReviewerCategoryId), (short) 0);
}
public static boolean isLegalReviewerGroup(final AccountGroup.UUID groupUUID) {
return !(AccountGroup.ANONYMOUS_USERS.equals(groupUUID)
|| AccountGroup.REGISTERED_USERS.equals(groupUUID));
}
}

5
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ModifyReviewersCommand.java
View File

@@ -138,12 +138,13 @@ public class ModifyReviewersCommand extends BaseCommand {
// Add reviewers
//
result = addReviewerFactory.create(changeId, stringSet(toAdd)).call();
result =
addReviewerFactory.create(changeId, stringSet(toAdd), false).call();
ok &= result.getErrors().isEmpty();
for (ReviewerResult.Error resultError : result.getErrors()) {
String message;
switch (resultError.getType()) {
case ACCOUNT_NOT_FOUND:
case REVIEWER_NOT_FOUND:
message = "account {0} not found";
break;
case ACCOUNT_INACTIVE: