opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.13'

Browse Source 
* stable-2.13:
  Implement reviewers visibility check for suggestions
  SuggestReviewersIT: Avoid side-effects between tests

Change-Id: Ic622c769ecd9e949d344b98cb61c48cabab4e0c3
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-12-20 11:34:32 +01:00
parent 709627f8f1 9b51298d05
commit 0038c300dc
2 changed files with 42 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
@@ -148,6 +149,18 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
.isEqualTo(user2.fullName);
}
@Test
public void suggestReviewsPrivateProjectVisibility() throws Exception {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
setApiUser(user3);
block("read", ANONYMOUS_USERS, "refs/*");
allow("read", group1.getGroupUUID(), "refs/*");
reviewers = suggestReviewers(changeId, user2.username, 2);
assertThat(reviewers).isEmpty();
}
@Test
@GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
public void suggestReviewersViewAllAccounts() throws Exception {
@@ -181,49 +194,49 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
reviewers = suggestReviewers(changeId, "first", 4);
reviewers = suggestReviewers(changeId, "first");
assertThat(reviewers).hasSize(3);
reviewers = suggestReviewers(changeId, "first1", 2);
reviewers = suggestReviewers(changeId, "first1");
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "last", 4);
reviewers = suggestReviewers(changeId, "last");
assertThat(reviewers).hasSize(3);
reviewers = suggestReviewers(changeId, "last1", 2);
reviewers = suggestReviewers(changeId, "last1");
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "fi la", 4);
reviewers = suggestReviewers(changeId, "fi la");
assertThat(reviewers).hasSize(3);
reviewers = suggestReviewers(changeId, "la fi", 4);
reviewers = suggestReviewers(changeId, "la fi");
assertThat(reviewers).hasSize(3);
reviewers = suggestReviewers(changeId, "first1 la", 2);
reviewers = suggestReviewers(changeId, "first1 la");
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "fi last1", 2);
reviewers = suggestReviewers(changeId, "fi last1");
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "first1 last2", 1);
reviewers = suggestReviewers(changeId, "first1 last2");
assertThat(reviewers).hasSize(0);
reviewers = suggestReviewers(changeId, name("user"), 7);
reviewers = suggestReviewers(changeId, name("user"));
assertThat(reviewers).hasSize(6);
reviewers = suggestReviewers(changeId, user1.username, 2);
reviewers = suggestReviewers(changeId, user1.username);
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "example.com", 7);
reviewers = suggestReviewers(changeId, "example.com");
assertThat(reviewers).hasSize(5);
reviewers = suggestReviewers(changeId, user1.email, 2);
reviewers = suggestReviewers(changeId, user1.email);
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, user1.username + " example", 2);
reviewers = suggestReviewers(changeId, user1.username + " example");
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, user4.email.toLowerCase(), 2);
reviewers = suggestReviewers(changeId, user4.email.toLowerCase());
assertThat(reviewers).hasSize(1);
assertThat(reviewers.get(0).account.email).isEqualTo(user4.email);
}
@@ -428,6 +441,14 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
.inOrder();
}
private List<SuggestedReviewerInfo> suggestReviewers(String changeId,
String query) throws Exception {
return gApi.changes()
.id(changeId)
.suggestReviewers(query)
.get();
}
private List<SuggestedReviewerInfo> suggestReviewers(String changeId,
String query, int n) throws Exception {
return gApi.changes()

9
gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
View File

@@ -194,14 +194,15 @@ public class ReviewersUtil {
try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
AccountIndex searchIndex = accountIndexes.getSearchIndex();
if (searchIndex != null) {
return suggestAccountsFromIndex(suggestReviewers);
return suggestAccountsFromIndex(suggestReviewers, visibilityControl);
}
return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
}
private List<Account.Id> suggestAccountsFromIndex(
SuggestReviewers suggestReviewers) throws OrmException {
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
try {
Set<Account.Id> matches = new HashSet<>();
QueryResult<AccountState> result = accountQueryProcessor
@@ -209,7 +210,9 @@ public class ReviewersUtil {
.query(accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
matches.add(id);
if (visibilityControl.isVisibleTo(id)) {
matches.add(id);
}
}
return new ArrayList<>(matches);
} catch (QueryParseException e) {