opendev/gerrit
Code Issues Proposed changes

Merge "Merge branch 'stable-2.12'"

Browse Source
This commit is contained in:
Edwin Kempin
2016-03-10 15:57:24 +00:00
committed by Gerrit Code Review
parent 4f1e0f4f9a 4ea3568ff3
commit 29a8e43633
20 changed files with 815 additions and 343 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
Documentation/user-review-ui.txt
View File

@@ -186,10 +186,6 @@ change is merged into the destination branch.
The `Submit` button is available if the change is submittable and
the link:access-control.html#category_submit[Submit] access right is
assigned.
+
It is also possible to submit changes that have merge conflicts. This
allows to do the conflict resolution for a change series in a single
merge commit and submit the changes in reverse order.
** [[abandon]]`Abandon`:
+

6
Documentation/user-search.txt
View File

@@ -299,7 +299,7 @@ is:closed::
+
True if the change is either merged or abandoned.
is:submitted, is:merged, is:abandoned::
is:merged, is:abandoned::
+
Same as <<status,status:'STATE'>>.
@@ -320,10 +320,6 @@ Same as 'is:reviewed', matches if any user has commented on the change
more recently than the last update (comment or patch set) from the
change owner.
status:submitted::
+
Change has been submitted, but is waiting for a dependency.
status:closed::
+
True if the change is either 'merged' or 'abandoned'.

43
ReleaseNotes/ReleaseNotes-2.11.8.txt Normal file
View File

@@ -0,0 +1,43 @@
Release notes for Gerrit 2.11.8
===============================
Gerrit 2.11.8 is now available:
link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.8.war[
https://gerrit-releases.storage.googleapis.com/gerrit-2.11.8.war]
There are no schema changes from link:ReleaseNotes-2.11.7.html[2.11.7].
Bug Fixes
---------
* Upgrade Apache commons-collections to version 3.2.2.
+
Includes a fix for a link:https://issues.apache.org/jira/browse/COLLECTIONS-580[
remote code execution exploit].
* link:https://code.google.com/p/gerrit/issues/detail?id=1207[Issue 1207]:
Fix keyboard shortcuts for non-US keyboards on side-by-side diff screen.
+
The forward/backward navigation keys `[` and `]` only worked on keyboards where
these characters could be typed without using any modifier key (like CTRL, ALT,
etc.).
+
Note that the problem still exists on the unified diff screen.
* link:https://code.google.com/p/gerrit/issues/detail?id=3919[Issue 3919]:
Explicitly set parent project to 'All-Projects' when a project is created
without giving the parent.
* Don't add message twice on abandon or restore via ssh review command.
+
When abandoning or reviewing a change via the ssh `review` command, and
providing a message with the `--message` option, the message was added to
the change twice.
* Clear the input box after cancelling add reviewer action.
+
When the action was cancelled, the content of the input box was still
there when opening it again.
* Fix internal server error when aborting ssh command.

109
ReleaseNotes/ReleaseNotes-2.12.1.txt
View File

@@ -11,31 +11,48 @@ link:ReleaseNotes-2.11.6.html[Gerrit 2.11.6] and
link:ReleaseNotes-2.11.7.html[Gerrit 2.11.7]. These bug fixes are *not*
listed in these release notes.
Schema Upgrade
--------------
*WARNING:* This version includes a manual schema upgrade when upgrading
from 2.12.
+
If you have already upgraded to 2.12, you need to issue this SQL statement
manually (e.g. using the `gerrit gsql` SSH command or the `gqsl` site
program):
+
When upgrading a site that is already running version 2.12, the `patch_sets`
table must be manually migrated using the `gerrit gsql` SSH command or the
`gqsl` site program.
For the default H2 database, execute the command:
----
alter table patch_sets modify push_certficate clob;
+
Or with this command if the site is configured to use PostgreSQL:
+
----
For MySQL, execute the command:
----
alter table patch_sets modify push_certficate text;
----
For PostgreSQL, execute the command:
----
alter table patch_sets alter column push_certficate type text;
+
----
For other database types, execute the appropriate equivalent command.
Note that the misspelled `push_certficate` is the actual name of the
column.
+
If you are upgrading from a version earlier than 2.12, this manual step is
not necessary and should be omitted.
When upgrading from a version earlier than 2.12, this manual step is not
necessary and should be omitted.
Bug Fixes
---------
General
^^^^^^^
~~~~~~~
* Fix column type for signed push certificates.
+
@@ -69,7 +86,7 @@ in other tables, a null change was returned which would then lead to an
'Internal Server Error' that was difficult to track down. Now an error is
raised earlier which will help administrators to find the root cause.
* https://code.google.com/p/gerrit/issues/detail?id=3743[Issue 3743]:
* link:https://code.google.com/p/gerrit/issues/detail?id=3743[Issue 3743]:
Use submitter identity as committer when using 'Rebase if Necessary' merge
strategy.
+
@@ -125,8 +142,29 @@ is reduced from 'no limit' to 1024.
Explicitly set parent project to 'All-Projects' when a project is created
without giving the parent.
* link:https://code.google.com/p/gerrit/issues/detail?id=3948[Issue 3948]:
Fix submit of project parent updates on `refs/meta/config`.
+
When submitting a change on `refs/meta/config` to update a project's parent,
the error 'The change must be submitted by a Gerrit administrator' was being
displayed even when the submitter was an admin. The submit was successful
when clicking 'Submit' a second time.
* link:https://code.google.com/p/gerrit/issues/detail?id=3811[Issue 3811]:
Fix submittability of merge commits that resolve merge conflicts.
+
If a series of changes contained a change that conflicted with the destination
branch, but the conflict was solved by a merge commit at the tip of the
series, the series was not submittable.
* link:https://code.google.com/p/gerrit/issues/detail?id=3883[Issue 3883]:
Respect the `core.commentchar` setting from `.gitconfig` in `commit-msg` hook.
UI
^^
~~
* link:https://code.google.com/p/gerrit/issues/detail?id=3894[Issue 3894]:
Fix display of 'Related changes' after change is rebased in web UI:
* link:https://code.google.com/p/gerrit/issues/detail?id=3071[Issue 3071]:
Fix display of submodule differences in side-by-side view.
@@ -153,22 +191,11 @@ etc..).
+
Note that the problem still exists on the unified diff screen.
* Improve tooltip on 'Submit' button when 'Submit whole topic' is enabled
and the topic can't be submitted due to some changes not being ready.
Plugins
^^^^^^^
* Allow plugins to get the caller in merge validation requests.
+
Plugins that implement the `MergeValidationListener` interface now get the
caller (the user who initiated the merge) in the `onPreMerge` method.
+
Existing plugins that implement this interface must be adapted to the new
method signature.
* link:https://code.google.com/p/gerrit/issues/detail?id=3741[Issue 3741]:
Fix handling of merge validation exceptions emitted by plugins.
+
If a plugin raised an exception, it was reported to the user as 'Change is
new', rather than 'Missing dependency'.
~~~~~~~
* link:https://code.google.com/p/gerrit/issues/detail?id=3821[Issue 3821]:
Fix repeated reloading of plugins when running on OpenJDK 8.
@@ -178,11 +205,31 @@ are POSIX 2008 compatible. This leads to precision incompatibility when
comparing the plugin's JAR file timestamp, resulting in the plugin being
reloaded every minute.
* link:https://code.google.com/p/gerrit/issues/detail?id=3741[Issue 3741]:
Fix handling of merge validation exceptions emitted by plugins.
+
If a plugin raised an exception, it was reported to the user as 'Change is
new', rather than 'Missing dependency'.
* Allow plugins to get the caller in merge validation requests.
+
Plugins that implement the `MergeValidationListener` interface now get the
caller (the user who initiated the merge) in the `onPreMerge` method.
+
Existing plugins that implement this interface must be adapted to the new
method signature.
* link:https://code.google.com/p/gerrit/issues/detail?id=3892[Issue 3892]:
Allow plugins to suggest reviewers based on either change or project
resources.
Documentation
^^^^^^^^^^^^^
~~~~~~~~~~~~~
* Update documentation of `commentlink` to reflect changed search URL.
* Add missing documentation of valid `database.type` values.
Upgrades
--------

1
ReleaseNotes/index.txt
View File

@@ -15,6 +15,7 @@ Version 2.12.x
[[2_11]]
Version 2.11.x
--------------
* link:ReleaseNotes-2.11.8.html[2.11.8]
* link:ReleaseNotes-2.11.7.html[2.11.7]
* link:ReleaseNotes-2.11.6.html[2.11.6]
* link:ReleaseNotes-2.11.5.html[2.11.5]

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
View File

@@ -103,7 +103,7 @@ public class ActionsIT extends AbstractDaemonTest {
assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST");
assertThat(info.title).isEqualTo(
"See the \"Submitted Together\" tab for problems");
"See the \"Submitted Together\" tab for problems, specially see: 2");
} else {
noSubmitWholeTopicAssertions(actions, 1);
}

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

@@ -0,0 +1,154 @@
// Copyright (C) 2016 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.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.fail;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.Util;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.RefSpec;
import org.junit.Before;
import org.junit.Test;
public class ConfigChangeIT extends AbstractDaemonTest {
@Before
public void setUp() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
Util.allow(cfg, Permission.OWNER, REGISTERED_USERS, "refs/*");
Util.allow(
cfg, Permission.PUSH, REGISTERED_USERS, "refs/for/refs/meta/config");
Util.allow(cfg, Permission.SUBMIT, REGISTERED_USERS, "refs/meta/config");
saveProjectConfig(project, cfg);
setApiUser(user);
fetchRefsMetaConfig();
}
@Test
@TestProjectInput(cloneAs = "user")
public void updateProjectConfig() throws Exception {
Config cfg = readProjectConfig();
assertThat(cfg.getString("project", null, "description")).isNull();
String desc = "new project description";
cfg.setString("project", null, "description", desc);
PushOneCommit.Result r = createConfigChange(cfg);
String id = r.getChangeId();
gApi.changes().id(id).current().review(ReviewInput.approve());
gApi.changes().id(id).current().submit();
assertThat(gApi.changes().id(id).info().status)
.isEqualTo(ChangeStatus.MERGED);
assertThat(gApi.projects().name(project.get()).get().description)
.isEqualTo(desc);
fetchRefsMetaConfig();
assertThat(readProjectConfig().getString("project", null, "description"))
.isEqualTo(desc);
}
@Test
@TestProjectInput(cloneAs = "user")
public void onlyAdminMayUpdateProjectParent() throws Exception {
setApiUser(admin);
ProjectInput parent = new ProjectInput();
parent.name = name("parent");
parent.permissionsOnly = true;
gApi.projects().create(parent);
setApiUser(user);
Config cfg = readProjectConfig();
assertThat(cfg.getString("access", null, "inheritFrom"))
.isAnyOf(null, allProjects.get());
cfg.setString("access", null, "inheritFrom", parent.name);
PushOneCommit.Result r = createConfigChange(cfg);
String id = r.getChangeId();
gApi.changes().id(id).current().review(ReviewInput.approve());
try {
gApi.changes().id(id).current().submit();
fail("expected submit to fail");
} catch (ResourceConflictException e) {
int n = gApi.changes().id(id).info()._number;
assertThat(e).hasMessage(
"Failed to submit 1 change due to the following problems:\n"
+ "Change " + n + ": Change contains a project configuration that"
+" changes the parent project.\n"
+ "The change must be submitted by a Gerrit administrator.");
}
assertThat(gApi.projects().name(project.get()).get().parent)
.isEqualTo(allProjects.get());
fetchRefsMetaConfig();
assertThat(readProjectConfig().getString("access", null, "inheritFrom"))
.isAnyOf(null, allProjects.get());
setApiUser(admin);
gApi.changes().id(id).current().submit();
assertThat(gApi.changes().id(id).info().status)
.isEqualTo(ChangeStatus.MERGED);
assertThat(gApi.projects().name(project.get()).get().parent)
.isEqualTo(parent.name);
fetchRefsMetaConfig();
assertThat(readProjectConfig().getString("access", null, "inheritFrom"))
.isEqualTo(parent.name);
}
private void fetchRefsMetaConfig() throws Exception {
git().fetch().setRefSpecs(new RefSpec("refs/meta/config:refs/meta/config"))
.call();
testRepo.reset("refs/meta/config");
}
private Config readProjectConfig() throws Exception {
RevWalk rw = testRepo.getRevWalk();
RevTree tree = rw.parseTree(testRepo.getRepository().resolve("HEAD"));
RevObject obj = rw.parseAny(testRepo.get(tree, "project.config"));
ObjectLoader loader = rw.getObjectReader().open(obj);
String text = new String(loader.getCachedBytes(), UTF_8);
Config cfg = new Config();
cfg.fromText(text);
return cfg;
}
private PushOneCommit.Result createConfigChange(Config cfg) throws Exception {
PushOneCommit.Result r = pushFactory.create(
db, user.getIdent(), testRepo,
"Update project config",
"project.config",
cfg.toText())
.to("refs/for/refs/meta/config");
r.assertOkStatus();
return r;
}
}

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
View File

@@ -294,7 +294,7 @@ public class SubmitResolvingMergeCommitIT extends AbstractDaemonTest {
OrmException {
ChangeSet cs =
mergeSuperSet.completeChangeSet(db, change.change(), user(admin));
assertThat(submit.isPatchSetMergeable(cs)).isEqualTo(expected);
assertThat(submit.unmergeableChanges(cs).isEmpty()).isEqualTo(expected);
}
private void assertMergeable(ChangeData change, boolean expected)

89
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/AbandonRestoreIT.java Normal file
View File

@@ -0,0 +1,89 @@
// Copyright (C) 2016 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.ssh;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
@NoHttpd
public class AbandonRestoreIT extends AbstractDaemonTest {
@Test
public void withMessage() throws Exception {
Result result = createChange();
String commit = result.getCommit().name();
executeCmd(commit, "abandon", "'abandon it'");
executeCmd(commit, "restore", "'restore it'");
assertChangeMessages(result.getChangeId(), ImmutableList.of(
"Uploaded patch set 1.",
"Abandoned\n\nabandon it",
"Restored\n\nrestore it"));
}
@Test
public void withoutMessage() throws Exception {
Result result = createChange();
String commit = result.getCommit().name();
executeCmd(commit, "abandon", null);
executeCmd(commit, "restore", null);
assertChangeMessages(result.getChangeId(), ImmutableList.of(
"Uploaded patch set 1.",
"Abandoned",
"Restored"));
}
private void executeCmd(String commit, String op, String message)
throws Exception {
StringBuilder command = new StringBuilder("gerrit review ")
.append(commit)
.append(" --")
.append(op);
if (message != null) {
command.append(" --message ").append(message);
}
String response = sshSession.exec(command.toString());
assert_()
.withFailureMessage(sshSession.getError())
.that(sshSession.hasError())
.isFalse();
assertThat(response.toLowerCase(Locale.US)).doesNotContain("error");
}
private void assertChangeMessages(String changeId, List<String> expected)
throws Exception {
ChangeInfo c = get(changeId);
Iterable<ChangeMessageInfo> messages = c.messages;
assertThat(messages).isNotNull();
assertThat(messages).hasSize(expected.size());
List<String> actual = new ArrayList<>();
for (ChangeMessageInfo info : messages) {
actual.add(info.message);
}
assertThat(actual).containsExactlyElementsIn(expected);
}
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java
View File

@@ -142,6 +142,7 @@ public class Reviewers extends Composite {
addReviewerIcon.setVisible(true);
UIObject.setVisible(form, false);
suggestBox.setFocus(false);
suggestBox.setText("");
}
private void addReviewer(final String reviewer, boolean confirmed) {

286
gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java Normal file
View File

@@ -0,0 +1,286 @@
// Copyright (C) 2016 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.base.Function;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupBaseInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.change.ReviewerSuggestionCache;
import com.google.gerrit.server.change.SuggestReviewers;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
public class ReviewersUtil {
private static final String MAX_SUFFIX = "\u9fa5";
private static final Ordering<SuggestedReviewerInfo> ORDERING =
Ordering.natural().onResultOf(new Function<SuggestedReviewerInfo, String>() {
@Nullable
@Override
public String apply(@Nullable SuggestedReviewerInfo suggestedReviewerInfo) {
if (suggestedReviewerInfo == null) {
return null;
}
return suggestedReviewerInfo.account != null
? MoreObjects.firstNonNull(suggestedReviewerInfo.account.email,
Strings.nullToEmpty(suggestedReviewerInfo.account.name))
: Strings.nullToEmpty(suggestedReviewerInfo.group.name);
}
});
private final AccountLoader accountLoader;
private final AccountCache accountCache;
private final ReviewerSuggestionCache reviewerSuggestionCache;
private final AccountControl accountControl;
private final Provider<ReviewDb> dbProvider;
private final GroupBackend groupBackend;
private final GroupMembers.Factory groupMembersFactory;
private final Provider<CurrentUser> currentUser;
@Inject
ReviewersUtil(AccountLoader.Factory accountLoaderFactory,
AccountCache accountCache,
ReviewerSuggestionCache reviewerSuggestionCache,
AccountControl.Factory accountControlFactory,
Provider<ReviewDb> dbProvider,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser) {
this.accountLoader = accountLoaderFactory.create(true);
this.accountCache = accountCache;
this.reviewerSuggestionCache = reviewerSuggestionCache;
this.accountControl = accountControlFactory.get();
this.dbProvider = dbProvider;
this.groupBackend = groupBackend;
this.groupMembersFactory = groupMembersFactory;
this.currentUser = currentUser;
}
public interface VisibilityControl {
boolean isVisibleTo(Account.Id account) throws OrmException;
}
public List<SuggestedReviewerInfo> suggestReviewers(
SuggestReviewers suggestReviewers, ProjectControl projectControl,
VisibilityControl visibilityControl)
throws IOException, OrmException, BadRequestException {
String query = suggestReviewers.getQuery();
boolean suggestAccounts = suggestReviewers.getSuggestAccounts();
int suggestFrom = suggestReviewers.getSuggestFrom();
boolean useFullTextSearch = suggestReviewers.getUseFullTextSearch();
int limit = suggestReviewers.getLimit();
if (Strings.isNullOrEmpty(query)) {
throw new BadRequestException("missing query field");
}
if (!suggestAccounts || query.length() < suggestFrom) {
return Collections.emptyList();
}
List<AccountInfo> suggestedAccounts;
if (useFullTextSearch) {
suggestedAccounts = suggestAccountFullTextSearch(suggestReviewers, visibilityControl);
} else {
suggestedAccounts = suggestAccount(suggestReviewers, visibilityControl);
}
List<SuggestedReviewerInfo> reviewer = Lists.newArrayList();
for (AccountInfo a : suggestedAccounts) {
SuggestedReviewerInfo info = new SuggestedReviewerInfo();
info.account = a;
reviewer.add(info);
}
for (GroupReference g : suggestAccountGroup(suggestReviewers, projectControl)) {
if (suggestGroupAsReviewer(suggestReviewers, projectControl.getProject(),
g, visibilityControl)) {
GroupBaseInfo info = new GroupBaseInfo();
info.id = Url.encode(g.getUUID().get());
info.name = g.getName();
SuggestedReviewerInfo suggestedReviewerInfo = new SuggestedReviewerInfo();
suggestedReviewerInfo.group = info;
reviewer.add(suggestedReviewerInfo);
}
}
reviewer = ORDERING.immutableSortedCopy(reviewer);
if (reviewer.size() <= limit) {
return reviewer;
} else {
return reviewer.subList(0, limit);
}
}
private List<AccountInfo> suggestAccountFullTextSearch(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws IOException, OrmException {
List<AccountInfo> results = reviewerSuggestionCache.search(
suggestReviewers.getQuery(), suggestReviewers.getFullTextMaxMatches());
Iterator<AccountInfo> it = results.iterator();
while (it.hasNext()) {
Account.Id accountId = new Account.Id(it.next()._accountId);
if (!(visibilityControl.isVisibleTo(accountId)
&& accountControl.canSee(accountId))) {
it.remove();
}
}
return results;
}
private List<AccountInfo> suggestAccount(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
String query = suggestReviewers.getQuery();
int limit = suggestReviewers.getLimit();
String a = query;
String b = a + MAX_SUFFIX;
Map<Account.Id, AccountInfo> r = new LinkedHashMap<>();
Map<Account.Id, String> queryEmail = new HashMap<>();
for (Account p : dbProvider.get().accounts()
.suggestByFullName(a, b, limit)) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
if (r.size() < limit) {
for (Account p : dbProvider.get().accounts()
.suggestByPreferredEmail(a, b, limit - r.size())) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
}
if (r.size() < limit) {
for (AccountExternalId e : dbProvider.get().accountExternalIds()
.suggestByEmailAddress(a, b, limit - r.size())) {
if (!r.containsKey(e.getAccountId())) {
Account p = accountCache.get(e.getAccountId()).getAccount();
if (p.isActive()) {
if (addSuggestion(r, p.getId(), visibilityControl)) {
queryEmail.put(e.getAccountId(), e.getEmailAddress());
}
}
}
}
}
accountLoader.fill();
for (Map.Entry<Account.Id, String> p : queryEmail.entrySet()) {
AccountInfo info = r.get(p.getKey());
if (info != null) {
info.email = p.getValue();
}
}
return new ArrayList<>(r.values());
}
private boolean addSuggestion(Map<Account.Id, AccountInfo> map,
Account.Id account, VisibilityControl visibilityControl)
throws OrmException {
if (!map.containsKey(account)
// Can the suggestion see the change?
&& visibilityControl.isVisibleTo(account)
// Can the account see the current user?
&& accountControl.canSee(account)) {
map.put(account, accountLoader.get(account));
return true;
}
return false;
}
private List<GroupReference> suggestAccountGroup(
SuggestReviewers suggestReviewers, ProjectControl ctl) {
return Lists.newArrayList(
Iterables.limit(groupBackend.suggest(suggestReviewers.getQuery(), ctl),
suggestReviewers.getLimit()));
}
private boolean suggestGroupAsReviewer(SuggestReviewers suggestReviewers,
Project project, GroupReference group,
VisibilityControl visibilityControl) throws OrmException, IOException {
int maxAllowed = suggestReviewers.getMaxAllowed();
if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
return false;
}
try {
Set<Account> members = groupMembersFactory
.create(currentUser.get())
.listAccounts(group.getUUID(), project.getNameKey());
if (members.isEmpty()) {
return false;
}
if (maxAllowed > 0 && members.size() > maxAllowed) {
return false;
}
// require that at least one member in the group can see the change
for (Account account : members) {
if (visibilityControl.isVisibleTo(account.getId())) {
return true;
}
}
} catch (NoSuchGroupException e) {
return false;
} catch (NoSuchProjectException e) {
return false;
}
return false;
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@@ -52,7 +52,7 @@ import com.google.gerrit.server.change.Revert;
import com.google.gerrit.server.change.Reviewers;
import com.google.gerrit.server.change.Revisions;
import com.google.gerrit.server.change.SubmittedTogether;
import com.google.gerrit.server.change.SuggestReviewers;
import com.google.gerrit.server.change.SuggestChangeReviewers;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
@@ -77,7 +77,7 @@ class ChangeApiImpl implements ChangeApi {
private final Revisions revisions;
private final ReviewerApiImpl.Factory reviewerApi;
private final RevisionApiImpl.Factory revisionApi;
private final Provider<SuggestReviewers> suggestReviewers;
private final Provider<SuggestChangeReviewers> suggestReviewers;
private final ChangeResource change;
private final Abandon abandon;
private final Revert revert;
@@ -104,7 +104,7 @@ class ChangeApiImpl implements ChangeApi {
Revisions revisions,
ReviewerApiImpl.Factory reviewerApi,
RevisionApiImpl.Factory revisionApi,
Provider<SuggestReviewers> suggestReviewers,
Provider<SuggestChangeReviewers> suggestReviewers,
Abandon abandon,
Revert revert,
Restore restore,
@@ -304,7 +304,7 @@ class ChangeApiImpl implements ChangeApi {
private List<SuggestedReviewerInfo> suggestReviewers(SuggestedReviewersRequest r)
throws RestApiException {
try {
SuggestReviewers mySuggestReviewers = suggestReviewers.get();
SuggestChangeReviewers mySuggestReviewers = suggestReviewers.get();
mySuggestReviewers.setQuery(r.getQuery());
mySuggestReviewers.setLimit(r.getLimit());
return mySuggestReviewers.apply(change);

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

@@ -26,6 +26,7 @@ import static com.google.gerrit.server.change.VoteResource.VOTE_KIND;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.SuggestChangeReviewers;
import com.google.gerrit.server.change.Reviewed.DeleteReviewed;
import com.google.gerrit.server.change.Reviewed.PutReviewed;
@@ -72,7 +73,7 @@ public class Module extends RestApiModule {
post(CHANGE_KIND, "index").to(Index.class);
post(CHANGE_KIND, "reviewers").to(PostReviewers.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestReviewers.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestChangeReviewers.class);
child(CHANGE_KIND, "reviewers").to(Reviewers.class);
get(REVIEWER_KIND).to(GetReviewer.class);
delete(REVIEWER_KIND).to(DeleteReviewer.class);

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

@@ -102,7 +102,7 @@ public class ReviewerSuggestionCache {
});
}
List<AccountInfo> search(String query, int n) throws IOException {
public List<AccountInfo> search(String query, int n) throws IOException {
IndexSearcher searcher = get();
if (searcher == null) {
return Collections.emptyList();

35
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -15,11 +15,14 @@
package com.google.gerrit.server.change;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.ParameterizedString;
@@ -71,6 +74,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -96,7 +100,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private static final String CLICK_FAILURE_TOOLTIP =
"Clicking the button would fail";
private static final String CHANGES_NOT_MERGEABLE =
"See the \"Submitted Together\" tab for problems";
"See the \"Submitted Together\" tab for problems, specially see: ";
public static class Output {
transient Change change;
@@ -263,11 +267,18 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
MergeOp.checkSubmitRule(c);
}
Boolean csIsMergeable = isPatchSetMergeable(cs);
if (csIsMergeable == null) {
Collection<ChangeData> unmergeable = unmergeableChanges(cs);
if (unmergeable == null) {
return CLICK_FAILURE_TOOLTIP;
} else if (!csIsMergeable) {
return CHANGES_NOT_MERGEABLE;
} else if (!unmergeable.isEmpty()) {
return CHANGES_NOT_MERGEABLE + Joiner.on(", ").join(
Iterables.transform(unmergeable,
new Function<ChangeData, String>() {
@Override
public String apply(ChangeData cd) {
return String.valueOf(cd.getId().get());
}
}));
}
} catch (ResourceConflictException e) {
return BLOCKED_SUBMIT_TOOLTIP;
@@ -407,11 +418,11 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
return change != null ? change.getStatus().name().toLowerCase() : "deleted";
}
public Boolean isPatchSetMergeable(ChangeSet cs)
public Collection<ChangeData> unmergeableChanges(ChangeSet cs)
throws OrmException, IOException {
Map<ChangeData, Boolean> mergeabilityMap = new HashMap<>();
Set<ChangeData> mergeabilityMap = new HashSet<>();
for (ChangeData change : cs.changes()) {
mergeabilityMap.put(change, false);
mergeabilityMap.add(change);
}
Multimap<Branch.NameKey, ChangeData> cbb = cs.changesByBranch();
@@ -442,17 +453,19 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
// Skip whole check, cannot determine if mergeable
return null;
}
mergeabilityMap.put(change, mergeable);
if (mergeable) {
mergeabilityMap.remove(change);
}
if (isLastInChain && isMergeCommit && mergeable) {
for (ChangeData c : targetBranch) {
mergeabilityMap.put(c, true);
mergeabilityMap.remove(c);
}
break;
}
}
}
return !mergeabilityMap.values().contains(Boolean.FALSE);
return mergeabilityMap;
}
private HashMap<Change.Id, RevCommit> findCommits(

76
gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java Normal file
View File

@@ -0,0 +1,76 @@
// Copyright (C) 2016 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.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
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.IdentifiedUser;
import com.google.gerrit.server.ReviewersUtil;
import com.google.gerrit.server.IdentifiedUser.GenericFactory;
import com.google.gerrit.server.ReviewersUtil.VisibilityControl;
import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.Config;
import java.io.IOException;
import java.util.List;
public class SuggestChangeReviewers extends SuggestReviewers
implements RestReadView<ChangeResource> {
@Inject
SuggestChangeReviewers(AccountVisibility av,
GenericFactory identifiedUserFactory,
Provider<ReviewDb> dbProvider,
@GerritServerConfig Config cfg,
ReviewersUtil reviewersUtil) {
super(av, identifiedUserFactory, dbProvider, cfg, reviewersUtil);
}
@Override
public List<SuggestedReviewerInfo> apply(ChangeResource rsrc)
throws BadRequestException, OrmException, IOException {
return reviewersUtil.suggestReviewers(this,
rsrc.getControl().getProjectControl(), getVisibility(rsrc));
}
private VisibilityControl getVisibility(final ChangeResource rsrc) {
if (rsrc.getControl().getRefControl().isVisibleByRegisteredUsers()) {
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
return true;
}
};
} else {
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
IdentifiedUser who =
identifiedUserFactory.create(dbProvider, account);
// we can't use changeControl directly as it won't suggest reviewers
// to drafts
return rsrc.getControl().forUser(who).isRefVisible();
}
};
}
}
}

312
gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java
View File

@@ -14,89 +14,33 @@
package com.google.gerrit.server.change;
import com.google.common.base.Function;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupBaseInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.ReviewersUtil;
import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.Config;
import org.kohsuke.args4j.Option;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
public class SuggestReviewers implements RestReadView<ChangeResource> {
private static final String MAX_SUFFIX = "\u9fa5";
public class SuggestReviewers {
private static final int DEFAULT_MAX_SUGGESTED = 10;
private static final int DEFAULT_MAX_MATCHES = 100;
private static final Ordering<SuggestedReviewerInfo> ORDERING =
Ordering.natural().onResultOf(new Function<SuggestedReviewerInfo, String>() {
@Nullable
@Override
public String apply(@Nullable SuggestedReviewerInfo suggestedReviewerInfo) {
if (suggestedReviewerInfo == null) {
return null;
}
return suggestedReviewerInfo.account != null
? MoreObjects.firstNonNull(suggestedReviewerInfo.account.email,
Strings.nullToEmpty(suggestedReviewerInfo.account.name))
: Strings.nullToEmpty(suggestedReviewerInfo.group.name);
}
});
private final AccountLoader accountLoader;
private final AccountControl accountControl;
private final GroupMembers.Factory groupMembersFactory;
private final AccountCache accountCache;
private final Provider<ReviewDb> dbProvider;
private final Provider<CurrentUser> currentUser;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final GroupBackend groupBackend;
protected final Provider<ReviewDb> dbProvider;
protected final IdentifiedUser.GenericFactory identifiedUserFactory;
protected final ReviewersUtil reviewersUtil;
private final boolean suggestAccounts;
private final int suggestFrom;
private final int maxAllowed;
private int limit;
private String query;
protected int limit;
protected String query;
private boolean useFullTextSearch;
private final int fullTextMaxMatches;
private final int maxSuggestedReviewers;
private final ReviewerSuggestionCache reviewerSuggestionCache;
protected final int maxSuggestedReviewers;
@Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT",
usage = "maximum number of reviewers to list")
@@ -112,27 +56,43 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
this.query = q;
}
public String getQuery() {
return query;
}
public boolean getSuggestAccounts() {
return suggestAccounts;
}
public int getSuggestFrom() {
return suggestFrom;
}
public boolean getUseFullTextSearch() {
return useFullTextSearch;
}
public int getFullTextMaxMatches() {
return fullTextMaxMatches;
}
public int getLimit() {
return limit;
}
public int getMaxAllowed() {
return maxAllowed;
}
@Inject
SuggestReviewers(AccountVisibility av,
AccountLoader.Factory accountLoaderFactory,
AccountControl.Factory accountControlFactory,
AccountCache accountCache,
GroupMembers.Factory groupMembersFactory,
public SuggestReviewers(AccountVisibility av,
IdentifiedUser.GenericFactory identifiedUserFactory,
Provider<CurrentUser> currentUser,
Provider<ReviewDb> dbProvider,
@GerritServerConfig Config cfg,
GroupBackend groupBackend,
ReviewerSuggestionCache reviewerSuggestionCache) {
this.accountLoader = accountLoaderFactory.create(true);
this.accountControl = accountControlFactory.get();
this.accountCache = accountCache;
this.groupMembersFactory = groupMembersFactory;
ReviewersUtil reviewersUtil) {
this.dbProvider = dbProvider;
this.identifiedUserFactory = identifiedUserFactory;
this.currentUser = currentUser;
this.groupBackend = groupBackend;
this.reviewerSuggestionCache = reviewerSuggestionCache;
this.reviewersUtil = reviewersUtil;
this.maxSuggestedReviewers =
cfg.getInt("suggest", "maxSuggestedReviewers", DEFAULT_MAX_SUGGESTED);
this.limit = this.maxSuggestedReviewers;
@@ -152,196 +112,4 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
this.maxAllowed = cfg.getInt("addreviewer", "maxAllowed",
PostReviewers.DEFAULT_MAX_REVIEWERS);
}
private interface VisibilityControl {
boolean isVisibleTo(Account.Id account) throws OrmException;
}
@Override
public List<SuggestedReviewerInfo> apply(ChangeResource rsrc)
throws BadRequestException, OrmException, IOException {
if (Strings.isNullOrEmpty(query)) {
throw new BadRequestException("missing query field");
}
if (!suggestAccounts || query.length() < suggestFrom) {
return Collections.emptyList();
}
VisibilityControl visibilityControl = getVisibility(rsrc);
List<AccountInfo> suggestedAccounts;
if (useFullTextSearch) {
suggestedAccounts = suggestAccountFullTextSearch(visibilityControl);
} else {
suggestedAccounts = suggestAccount(visibilityControl);
}
List<SuggestedReviewerInfo> reviewer = Lists.newArrayList();
for (AccountInfo a : suggestedAccounts) {
SuggestedReviewerInfo info = new SuggestedReviewerInfo();
info.account = a;
reviewer.add(info);
}
Project p = rsrc.getControl().getProject();
for (GroupReference g : suggestAccountGroup(
rsrc.getControl().getProjectControl())) {
if (suggestGroupAsReviewer(p, g, visibilityControl)) {
GroupBaseInfo info = new GroupBaseInfo();
info.id = Url.encode(g.getUUID().get());
info.name = g.getName();
SuggestedReviewerInfo suggestedReviewerInfo = new SuggestedReviewerInfo();
suggestedReviewerInfo.group = info;
reviewer.add(suggestedReviewerInfo);
}
}
reviewer = ORDERING.immutableSortedCopy(reviewer);
if (reviewer.size() <= limit) {
return reviewer;
} else {
return reviewer.subList(0, limit);
}
}
private VisibilityControl getVisibility(final ChangeResource rsrc) {
if (rsrc.getControl().getRefControl().isVisibleByRegisteredUsers()) {
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
return true;
}
};
} else {
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
IdentifiedUser who =
identifiedUserFactory.create(dbProvider, account);
// we can't use changeControl directly as it won't suggest reviewers
// to drafts
return rsrc.getControl().forUser(who).isRefVisible();
}
};
}
}
private List<GroupReference> suggestAccountGroup(ProjectControl ctl) {
return Lists.newArrayList(
Iterables.limit(groupBackend.suggest(query, ctl), limit));
}
private List<AccountInfo> suggestAccount(VisibilityControl visibilityControl)
throws OrmException {
String a = query;
String b = a + MAX_SUFFIX;
Map<Account.Id, AccountInfo> r = new LinkedHashMap<>();
Map<Account.Id, String> queryEmail = new HashMap<>();
for (Account p : dbProvider.get().accounts()
.suggestByFullName(a, b, limit)) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
if (r.size() < limit) {
for (Account p : dbProvider.get().accounts()
.suggestByPreferredEmail(a, b, limit - r.size())) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
}
if (r.size() < limit) {
for (AccountExternalId e : dbProvider.get().accountExternalIds()
.suggestByEmailAddress(a, b, limit - r.size())) {
if (!r.containsKey(e.getAccountId())) {
Account p = accountCache.get(e.getAccountId()).getAccount();
if (p.isActive()) {
if (addSuggestion(r, p.getId(), visibilityControl)) {
queryEmail.put(e.getAccountId(), e.getEmailAddress());
}
}
}
}
}
accountLoader.fill();
for (Map.Entry<Account.Id, String> p : queryEmail.entrySet()) {
AccountInfo info = r.get(p.getKey());
if (info != null) {
info.email = p.getValue();
}
}
return new ArrayList<>(r.values());
}
private List<AccountInfo> suggestAccountFullTextSearch(
VisibilityControl visibilityControl) throws IOException, OrmException {
List<AccountInfo> results = reviewerSuggestionCache.search(
query, fullTextMaxMatches);
Iterator<AccountInfo> it = results.iterator();
while (it.hasNext()) {
Account.Id accountId = new Account.Id(it.next()._accountId);
if (!(visibilityControl.isVisibleTo(accountId)
&& accountControl.canSee(accountId))) {
it.remove();
}
}
return results;
}
private boolean addSuggestion(Map<Account.Id, AccountInfo> map,
Account.Id account, VisibilityControl visibilityControl)
throws OrmException {
if (!map.containsKey(account)
// Can the suggestion see the change?
&& visibilityControl.isVisibleTo(account)
// Can the account see the current user?
&& accountControl.canSee(account)) {
map.put(account, accountLoader.get(account));
return true;
}
return false;
}
private boolean suggestGroupAsReviewer(Project project,
GroupReference group, VisibilityControl visibilityControl)
throws OrmException, IOException {
if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
return false;
}
try {
Set<Account> members = groupMembersFactory
.create(currentUser.get())
.listAccounts(group.getUUID(), project.getNameKey());
if (members.isEmpty()) {
return false;
}
if (maxAllowed > 0 && members.size() > maxAllowed) {
return false;
}
// require that at least one member in the group can see the change
for (Account account : members) {
if (visibilityControl.isVisibleTo(account.getId())) {
return true;
}
}
} catch (NoSuchGroupException e) {
return false;
} catch (NoSuchProjectException e) {
return false;
}
return false;
}
}

6
gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java
View File

@@ -157,6 +157,9 @@ class SshLog implements LifecycleListener {
}
private Multimap<String, ?> extractParameters(DispatchCommand dcmd) {
if (dcmd == null) {
return ArrayListMultimap.create(0, 0);
}
String[] cmdArgs = dcmd.getArguments();
String paramName = null;
int argPos = 0;
@@ -274,6 +277,9 @@ class SshLog implements LifecycleListener {
}
private String extractWhat(DispatchCommand dcmd) {
if (dcmd == null) {
return "Command was already destroyed";
}
StringBuilder commandName = new StringBuilder(dcmd.getCommandName());
String[] args = dcmd.getArguments();
for (int i = 1; i < args.length; i++) {

15
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -262,9 +262,6 @@ public class ReviewCommand extends SshCommand {
}
private void reviewPatchSet(final PatchSet patchSet) throws Exception {
if (changeComment == null) {
changeComment = "";
}
if (notify == null) {
notify = NotifyHandling.ALL;
}
@@ -283,22 +280,20 @@ public class ReviewCommand extends SshCommand {
}
review.labels.putAll(customLabels);
// If review labels are being applied, the comment will be included
// on the review note. We don't need to add it again on the abandon
// or restore comment.
if (!review.labels.isEmpty() && (abandonChange || restoreChange)) {
changeComment = null;
// We don't need to add the review comment when abandoning/restoring.
if (abandonChange || restoreChange) {
review.message = null;
}
try {
if (abandonChange) {
AbandonInput input = new AbandonInput();
input.message = changeComment;
input.message = Strings.emptyToNull(changeComment);
applyReview(patchSet, review);
changeApi(patchSet).abandon(input);
} else if (restoreChange) {
RestoreInput input = new RestoreInput();
input.message = changeComment;
input.message = Strings.emptyToNull(changeComment);
changeApi(patchSet).restore(input);
applyReview(patchSet, review);
} else {

4
lib/commons/BUCK
View File

@@ -10,8 +10,8 @@ maven_jar(
maven_jar(
name = 'collections',
id = 'commons-collections:commons-collections:3.2.1',
sha1 = '761ea405b9b37ced573d2df0d1e3a4e0f9edc668',
id = 'commons-collections:commons-collections:3.2.2',
sha1 = '8ad72fe39fa8c91eaaf12aadb21e0c3661fe26d5',
license = 'Apache2.0',
exclude = ['META-INF/LICENSE.txt', 'META-INF/NOTICE.txt'],
attach_source = False,