opendev/gerrit
Code Issues Proposed changes

Merge changes If77b630b,I3bbeb623,I14ab15f1,Ia7777ac5,Iffae33bd

Browse Source 
* changes:
  Add field for watched projects to account index
  Add project watches to AccountState
  Remove suggest.fullTextSearch option
  Use account index for reviewer suggestion
  Fix highlighting in suggestions with full text search
This commit is contained in:
David Pursehouse
2016-07-08 00:23:21 +00:00
committed by Gerrit Code Review
parent 17699c8bf0 56c1349c55
commit a5b9e43029
24 changed files with 261 additions and 368 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
Documentation/config-gerrit.txt
View File

@@ -3636,12 +3636,6 @@ The maximum numbers of reviewers suggested.
+
By default 10.
[[suggest.fullTextSearch]]suggest.fullTextSearch::
+
If `true` the reviewer completion suggestions will be based on a full text search.
+
By default `false`.
[[suggest.from]]suggest.from::
+
The number of characters that a user must have typed before suggestions
@@ -3649,18 +3643,6 @@ are provided. If set to 0, suggestions are always provided.
+
By default 0.
[[suggest.fullTextSearchMaxMatches]]suggest.fullTextSearchMaxMatches::
+
The maximum number of matches evaluated for change access when using full text search.
+
By default 100.
[[suggest.fullTextSearchRefresh]]suggest.fullTextSearchRefresh::
+
Refresh interval for the in-memory account search index.
+
By default 1 hour.
[[theme]]
=== Section theme

8
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
View File

@@ -27,6 +27,7 @@ import com.google.gerrit.server.account.AccountByEmailCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
@@ -52,6 +53,7 @@ public class AccountCreator {
private final SshKeyCache sshKeyCache;
private final AccountCache accountCache;
private final AccountByEmailCache byEmailCache;
private final AccountIndexer indexer;
@Inject
AccountCreator(SchemaFactory<ReviewDb> schema,
@@ -59,7 +61,8 @@ public class AccountCreator {
GroupCache groupCache,
SshKeyCache sshKeyCache,
AccountCache accountCache,
AccountByEmailCache byEmailCache) {
AccountByEmailCache byEmailCache,
AccountIndexer indexer) {
accounts = new HashMap<>();
reviewDbProvider = schema;
this.authorizedKeys = authorizedKeys;
@@ -67,6 +70,7 @@ public class AccountCreator {
this.sshKeyCache = sshKeyCache;
this.accountCache = accountCache;
this.byEmailCache = byEmailCache;
this.indexer = indexer;
}
public synchronized TestAccount create(String username, String email,
@@ -113,6 +117,8 @@ public class AccountCreator {
accountCache.evictByUsername(username);
byEmailCache.evict(email);
indexer.index(id);
account =
new TestAccount(id, username, email, fullName, sshKey, httpPass);
accounts.put(username, account);

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

@@ -168,7 +168,6 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
}
@Test
@GerritConfig(name = "suggest.fullTextSearch", value = "true")
public void suggestReviewersFullTextSearch() throws Exception {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
@@ -220,18 +219,6 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
assertThat(reviewers.get(0).account.email).isEqualTo(user4.email);
}
@Test
@GerritConfigs(
{@GerritConfig(name = "suggest.fulltextsearch", value = "true"),
@GerritConfig(name = "suggest.fullTextSearchMaxMatches", value = "2")
})
public void suggestReviewersFullTextSearchLimitMaxMatches() throws Exception {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(changeId, name("user"), 2);
assertThat(reviewers).hasSize(2);
}
@Test
public void suggestReviewersWithoutLimitOptionSpecified() throws Exception {
String changeId = createChange().getChangeId();

52
gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/HighlightSuggestOracle.java
View File

@@ -17,6 +17,10 @@ package com.google.gwtexpui.safehtml.client;
import com.google.gwt.user.client.ui.SuggestOracle;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
/**
* A suggestion oracle that tries to highlight the matched text.
@@ -56,7 +60,7 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
}
protected String getQueryPattern(final String query) {
return "(" + escape(query) + ")";
return query;
}
/**
@@ -84,19 +88,51 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
ds = escape(ds);
}
// We now surround qstr by <strong>. But the chosen approach is not too
// smooth, if qstr is small (e.g.: "t") and this small qstr may occur in
// escapes (e.g.: "Tim &lt;email@example.org&gt;"). Those escapes will
// get <strong>-ed as well (e.g.: "&lt;" -> "&<strong>l</strong>t;"). But
// as repairing those mangled escapes is easier than not mangling them in
// the first place, we repair them afterwards.
ds = sgi(ds, qstr, "<strong>$1</strong>");
for (String qterm : splitQuery(qstr)) {
qterm = "(" + escape(qterm) + ")";
// We now surround qstr by <strong>. But the chosen approach is not too
// smooth, if qstr is small (e.g.: "t") and this small qstr may occur in
// escapes (e.g.: "Tim &lt;email@example.org&gt;"). Those escapes will
// get <strong>-ed as well (e.g.: "&lt;" -> "&<strong>l</strong>t;"). But
// as repairing those mangled escapes is easier than not mangling them in
// the first place, we repair them afterwards.
ds = sgi(ds, qterm, "<strong>$1</strong>");
}
// Repairing <strong>-ed escapes.
ds = sgi(ds, "(&[a-z]*)<strong>([a-z]*)</strong>([a-z]*;)", "$1$2$3");
displayString = ds;
}
/**
* Split the query by whitespace and filter out query terms which are
* substrings of other query terms.
*/
private static List<String> splitQuery(String query) {
List<String> queryTerms = Arrays.asList(query.split("\\s+"));
Collections.sort(queryTerms, new Comparator<String>() {
@Override
public int compare(String s1, String s2) {
return Integer.compare(s2.length(), s1.length());
}});
List<String> result = new ArrayList<>();
for (String s : queryTerms) {
boolean add = true;
for (String queryTerm : result) {
if (queryTerm.toLowerCase().contains(s.toLowerCase())) {
add = false;
break;
}
}
if (add) {
result.add(s);
}
}
return result;
}
private static native String sgi(String inString, String pat, String newHtml)
/*-{ return inString.replace(RegExp(pat, 'gi'), newHtml); }-*/;

32
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java
View File

@@ -14,13 +14,13 @@
package com.google.gerrit.client.change;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.admin.Util;
import com.google.gerrit.client.changes.ChangeApi;
import com.google.gerrit.client.groups.GroupBaseInfo;
import com.google.gerrit.client.info.AccountInfo;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.client.ui.AccountSuggestOracle;
import com.google.gerrit.client.ui.SuggestAfterTypingNCharsOracle;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwt.core.client.JavaScriptObject;
@@ -42,7 +42,7 @@ public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
public void onSuccess(JsArray<SuggestReviewerInfo> result) {
List<RestReviewerSuggestion> r = new ArrayList<>(result.length());
for (SuggestReviewerInfo reviewer : Natives.asList(result)) {
r.add(new RestReviewerSuggestion(reviewer));
r.add(new RestReviewerSuggestion(reviewer, req.getQuery()));
}
cb.onSuggestionsReady(req, new Response(r));
}
@@ -60,29 +60,29 @@ public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
}
private static class RestReviewerSuggestion implements Suggestion {
private final SuggestReviewerInfo reviewer;
private final String displayString;
private final String replacementString;
RestReviewerSuggestion(final SuggestReviewerInfo reviewer) {
this.reviewer = reviewer;
RestReviewerSuggestion(SuggestReviewerInfo reviewer, String query) {
if (reviewer.account() != null) {
this.replacementString = AccountSuggestOracle.AccountSuggestion
.format(reviewer.account(), query);
this.displayString = replacementString;
} else {
this.replacementString = reviewer.group().name();
this.displayString =
replacementString + " (" + Util.C.suggestedGroupLabel() + ")";
}
}
@Override
public String getDisplayString() {
if (reviewer.account() != null) {
return FormatUtil.nameEmail(reviewer.account());
}
return reviewer.group().name()
+ " ("
+ Util.C.suggestedGroupLabel()
+ ")";
return displayString;
}
@Override
public String getReplacementString() {
if (reviewer.account() != null) {
return FormatUtil.nameEmail(reviewer.account());
}
return reviewer.group().name();
return replacementString;
}
}

43
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java
View File

@@ -42,25 +42,11 @@ public class AccountSuggestOracle extends SuggestAfterTypingNCharsOracle {
});
}
private static class AccountSuggestion implements SuggestOracle.Suggestion {
public static class AccountSuggestion implements SuggestOracle.Suggestion {
private final String suggestion;
AccountSuggestion(AccountInfo info, String query) {
String s = FormatUtil.nameEmail(info);
if (!s.toLowerCase().contains(query.toLowerCase())
&& info.secondaryEmails() != null) {
for (String email : Natives.asList(info.secondaryEmails())) {
AccountInfo info2 = AccountInfo.create(info._accountId(), info.name(),
email, info.username());
String s2 = FormatUtil.nameEmail(info2);
if (s2.toLowerCase().contains(query.toLowerCase())) {
s = s2;
break;
}
}
}
this.suggestion = s;
this.suggestion = format(info, query);
}
@Override
@@ -72,5 +58,30 @@ public class AccountSuggestOracle extends SuggestAfterTypingNCharsOracle {
public String getReplacementString() {
return suggestion;
}
public static String format(AccountInfo info, String query) {
String s = FormatUtil.nameEmail(info);
if (!containsQuery(s, query) && info.secondaryEmails() != null) {
for (String email : Natives.asList(info.secondaryEmails())) {
AccountInfo info2 = AccountInfo.create(info._accountId(), info.name(),
email, info.username());
String s2 = FormatUtil.nameEmail(info2);
if (containsQuery(s2, query)) {
s = s2;
break;
}
}
}
return s;
}
private static boolean containsQuery(String s, String query) {
for (String qterm : query.split("\\s+")) {
if (!s.toLowerCase().contains(qterm.toLowerCase())) {
return false;
}
}
return true;
}
}
}

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

@@ -17,6 +17,7 @@ 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.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
@@ -35,22 +36,30 @@ 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.AccountState;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
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.index.account.AccountIndex;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.QueryResult;
import com.google.gerrit.server.query.account.AccountQueryBuilder;
import com.google.gerrit.server.query.account.AccountQueryProcessor;
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.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -74,7 +83,9 @@ public class ReviewersUtil {
});
private final AccountLoader accountLoader;
private final AccountCache accountCache;
private final ReviewerSuggestionCache reviewerSuggestionCache;
private final AccountIndexCollection indexes;
private final AccountQueryBuilder queryBuilder;
private final AccountQueryProcessor queryProcessor;
private final AccountControl accountControl;
private final Provider<ReviewDb> dbProvider;
private final GroupBackend groupBackend;
@@ -84,15 +95,21 @@ public class ReviewersUtil {
@Inject
ReviewersUtil(AccountLoader.Factory accountLoaderFactory,
AccountCache accountCache,
ReviewerSuggestionCache reviewerSuggestionCache,
AccountIndexCollection indexes,
AccountQueryBuilder queryBuilder,
AccountQueryProcessor queryProcessor,
AccountControl.Factory accountControlFactory,
Provider<ReviewDb> dbProvider,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser) {
this.accountLoader = accountLoaderFactory.create(true);
Set<FillOptions> fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
this.accountLoader = accountLoaderFactory.create(fillOptions);
this.accountCache = accountCache;
this.reviewerSuggestionCache = reviewerSuggestionCache;
this.indexes = indexes;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
this.accountControl = accountControlFactory.get();
this.dbProvider = dbProvider;
this.groupBackend = groupBackend;
@@ -111,7 +128,6 @@ public class ReviewersUtil {
String query = suggestReviewers.getQuery();
boolean suggestAccounts = suggestReviewers.getSuggestAccounts();
int suggestFrom = suggestReviewers.getSuggestFrom();
boolean useFullTextSearch = suggestReviewers.getUseFullTextSearch();
int limit = suggestReviewers.getLimit();
if (Strings.isNullOrEmpty(query)) {
@@ -122,12 +138,8 @@ public class ReviewersUtil {
return Collections.emptyList();
}
List<AccountInfo> suggestedAccounts;
if (useFullTextSearch) {
suggestedAccounts = suggestAccountFullTextSearch(suggestReviewers, visibilityControl);
} else {
suggestedAccounts = suggestAccount(suggestReviewers, visibilityControl);
}
Collection<AccountInfo> suggestedAccounts =
suggestAccounts(suggestReviewers, visibilityControl);
List<SuggestedReviewerInfo> reviewer = new ArrayList<>();
for (AccountInfo a : suggestedAccounts) {
@@ -155,27 +167,39 @@ public class ReviewersUtil {
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,
private Collection<AccountInfo> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
AccountIndex searchIndex = indexes.getSearchIndex();
if (searchIndex != null) {
return suggestAccountsFromIndex(suggestReviewers);
}
return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
private Collection<AccountInfo> suggestAccountsFromIndex(
SuggestReviewers suggestReviewers) throws OrmException {
try {
Map<Account.Id, AccountInfo> matches = new LinkedHashMap<>();
QueryResult<AccountState> result = queryProcessor
.setLimit(suggestReviewers.getLimit())
.query(queryBuilder.defaultQuery(suggestReviewers.getQuery()));
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
matches.put(id, accountLoader.get(id));
}
accountLoader.fill();
return matches.values();
} catch (QueryParseException e) {
return ImmutableList.of();
}
}
private Collection<AccountInfo> suggestAccountsFromDb(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
String query = suggestReviewers.getQuery();
int limit = suggestReviewers.getLimit();

20
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
View File

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.index.account.AccountIndexer;
@@ -132,8 +133,9 @@ public class AccountCacheImpl implements AccountCache {
Account account = new Account(accountId, TimeUtil.nowTs());
account.setActive(false);
Collection<AccountExternalId> ids = Collections.emptySet();
Collection<AccountProjectWatch> projectWatches = Collections.emptySet();
Set<AccountGroup.UUID> anon = ImmutableSet.of();
return new AccountState(account, anon, ids);
return new AccountState(account, anon, ids, projectWatches);
}
static class ByIdLoader extends CacheLoader<Account.Id, AccountState> {
@@ -168,16 +170,15 @@ public class AccountCacheImpl implements AccountCache {
private AccountState load(final ReviewDb db, final Account.Id who)
throws OrmException {
final Account account = db.accounts().get(who);
Account account = db.accounts().get(who);
if (account == null) {
// Account no longer exists? They are anonymous.
//
return missing(who);
}
final Collection<AccountExternalId> externalIds =
Collections.unmodifiableCollection(db.accountExternalIds().byAccount(
who).toList());
Collection<AccountExternalId> externalIds =
Collections.unmodifiableCollection(
db.accountExternalIds().byAccount(who).toList());
Set<AccountGroup.UUID> internalGroups = new HashSet<>();
for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) {
@@ -197,7 +198,12 @@ public class AccountCacheImpl implements AccountCache {
account.setGeneralPreferences(GeneralPreferencesInfo.defaults());
}
return new AccountState(account, internalGroups, externalIds);
Collection<AccountProjectWatch> projectWatches =
Collections.unmodifiableCollection(
db.accountProjectWatches().byAccount(who).toList());
return new AccountState(account, internalGroups, externalIds,
projectWatches);
}
}

15
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.CurrentUser.PropertyKey;
import com.google.gerrit.server.IdentifiedUser;
@@ -34,14 +35,17 @@ public class AccountState {
private final Account account;
private final Set<AccountGroup.UUID> internalGroups;
private final Collection<AccountExternalId> externalIds;
private final Collection<AccountProjectWatch> projectWatches;
private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
public AccountState(final Account account,
final Set<AccountGroup.UUID> actualGroups,
final Collection<AccountExternalId> externalIds) {
public AccountState(Account account,
Set<AccountGroup.UUID> actualGroups,
Collection<AccountExternalId> externalIds,
Collection<AccountProjectWatch> projectWatches) {
this.account = account;
this.internalGroups = actualGroups;
this.externalIds = externalIds;
this.projectWatches = projectWatches;
this.account.setUserName(getUserName(externalIds));
}
@@ -76,6 +80,11 @@ public class AccountState {
return externalIds;
}
/** The project watches of the account. */
public Collection<AccountProjectWatch> getProjectWatches() {
return projectWatches;
}
/** The set of groups maintained directly within the Gerrit database. */
public Set<AccountGroup.UUID> getInternalGroups() {
return internalGroups;

26
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -29,6 +30,7 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
@@ -39,26 +41,29 @@ public class DeleteWatchedProjects
private final Provider<ReviewDb> dbProvider;
private final Provider<IdentifiedUser> self;
private final AccountCache accountCache;
@Inject
DeleteWatchedProjects(Provider<ReviewDb> dbProvider,
Provider<IdentifiedUser> self) {
Provider<IdentifiedUser> self,
AccountCache accountCache) {
this.dbProvider = dbProvider;
this.self = self;
this.accountCache = accountCache;
}
@Override
public Response<?> apply(
AccountResource rsrc, List<ProjectWatchInfo> input)
throws UnprocessableEntityException, OrmException, AuthException {
if (self.get() != rsrc.getUser()
public Response<?> apply(AccountResource rsrc, List<ProjectWatchInfo> input)
throws AuthException, UnprocessableEntityException, OrmException,
IOException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("It is not allowed to edit project watches "
+ "of other users");
}
Account.Id accountId = rsrc.getUser().getAccountId();
ResultSet<AccountProjectWatch> watchedProjects =
dbProvider.get().accountProjectWatches()
.byAccount(rsrc.getUser().getAccountId());
dbProvider.get().accountProjectWatches().byAccount(accountId);
HashMap<AccountProjectWatch.Key, AccountProjectWatch>
watchedProjectsMap = new HashMap<>();
for (AccountProjectWatch watchedProject : watchedProjects) {
@@ -68,10 +73,8 @@ public class DeleteWatchedProjects
if (input != null) {
List<AccountProjectWatch> watchesToDelete = new LinkedList<>();
for (ProjectWatchInfo projectInfo : input) {
AccountProjectWatch.Key key = new AccountProjectWatch.Key(
rsrc.getUser().getAccountId(),
new Project.NameKey(projectInfo.project),
projectInfo.filter);
AccountProjectWatch.Key key = new AccountProjectWatch.Key(accountId,
new Project.NameKey(projectInfo.project), projectInfo.filter);
if (!watchedProjectsMap.containsKey(key)) {
throw new UnprocessableEntityException(projectInfo.project
+ " is not currently watched by this user.");
@@ -79,6 +82,7 @@ public class DeleteWatchedProjects
watchesToDelete.add(watchedProjectsMap.get(key));
}
dbProvider.get().accountProjectWatches().delete(watchesToDelete);
accountCache.evict(accountId);
}
return Response.none();
}

19
gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java
View File

@@ -38,20 +38,23 @@ import java.util.List;
@Singleton
public class PostWatchedProjects
implements RestModifyView<AccountResource, List<ProjectWatchInfo>> {
private final Provider<ReviewDb> dbProvider;
private final Provider<IdentifiedUser> self;
private final GetWatchedProjects getWatchedProjects;
private final Provider<ReviewDb> dbProvider;
private final ProjectsCollection projectsCollection;
private final AccountCache accountCache;
@Inject
public PostWatchedProjects(GetWatchedProjects getWatchedProjects,
Provider<ReviewDb> dbProvider,
public PostWatchedProjects(Provider<ReviewDb> dbProvider,
Provider<IdentifiedUser> self,
GetWatchedProjects getWatchedProjects,
ProjectsCollection projectsCollection,
Provider<IdentifiedUser> self) {
this.getWatchedProjects = getWatchedProjects;
AccountCache accountCache) {
this.dbProvider = dbProvider;
this.projectsCollection = projectsCollection;
this.self = self;
this.getWatchedProjects = getWatchedProjects;
this.projectsCollection = projectsCollection;
this.accountCache = accountCache;
}
@Override
@@ -62,9 +65,11 @@ public class PostWatchedProjects
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to edit project watches");
}
Account.Id accountId = rsrc.getUser().getAccountId();
List<AccountProjectWatch> accountProjectWatchList =
getAccountProjectWatchList(input, rsrc.getUser().getAccountId());
getAccountProjectWatchList(input, accountId);
dbProvider.get().accountProjectWatches().upsert(accountProjectWatchList);
accountCache.evict(accountId);
return getWatchedProjects.apply(rsrc);
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
View File

@@ -197,7 +197,7 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
try {
Predicate<AccountState> queryPred;
if (suggest) {
queryPred = queryBuilder.defaultField(query);
queryPred = queryBuilder.defaultQuery(query);
queryProcessor.setLimit(suggestLimit);
} else {
queryPred = queryBuilder.parse(query);

2
gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
View File

@@ -241,7 +241,7 @@ public class AccountApiImpl implements AccountApi {
throws RestApiException {
try {
deleteWatchedProjects.apply(account, in);
} catch (OrmException e) {
} catch (OrmException | IOException e) {
throw new RestApiException("Cannot delete watched projects", e);
}
}

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

@@ -1,187 +0,0 @@
// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Splitter;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.AccountExternalIdAccess;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.analysis.util.CharArraySet;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field.Store;
import org.apache.lucene.document.IntField;
import org.apache.lucene.document.StringField;
import org.apache.lucene.document.TextField;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.index.IndexWriterConfig.OpenMode;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.PrefixQuery;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.TopDocs;
import org.apache.lucene.store.RAMDirectory;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
/**
* The suggest oracle may be called many times in rapid succession during the
* course of one operation.
* It would be easy to have a simple {@code Cache<Boolean, List<Account>>}
* with a short expiration time of 30s.
* Cache only has a single key we're just using Cache for the expiration behavior.
*/
@Singleton
public class ReviewerSuggestionCache {
private static final Logger log = LoggerFactory
.getLogger(ReviewerSuggestionCache.class);
private static final String ID = "id";
private static final String NAME = "name";
private static final String EMAIL = "email";
private static final String USERNAME = "username";
private static final String[] ALL = {ID, NAME, EMAIL, USERNAME};
private final LoadingCache<Boolean, IndexSearcher> cache;
private final Provider<ReviewDb> db;
@Inject
ReviewerSuggestionCache(Provider<ReviewDb> db,
@GerritServerConfig Config cfg) {
this.db = db;
long expiration = ConfigUtil.getTimeUnit(cfg,
"suggest", null, "fullTextSearchRefresh",
TimeUnit.HOURS.toMillis(1),
TimeUnit.MILLISECONDS);
this.cache =
CacheBuilder.newBuilder().maximumSize(1)
.refreshAfterWrite(expiration, TimeUnit.MILLISECONDS)
.build(new CacheLoader<Boolean, IndexSearcher>() {
@Override
public IndexSearcher load(Boolean key) throws Exception {
return index();
}
});
}
public List<AccountInfo> search(String query, int n) throws IOException {
IndexSearcher searcher = get();
if (searcher == null) {
return Collections.emptyList();
}
List<String> segments = Splitter.on(' ').omitEmptyStrings().splitToList(
query.toLowerCase());
BooleanQuery.Builder q = new BooleanQuery.Builder();
for (String field : ALL) {
BooleanQuery.Builder and = new BooleanQuery.Builder();
for (String s : segments) {
and.add(new PrefixQuery(new Term(field, s)), Occur.MUST);
}
q.add(and.build(), Occur.SHOULD);
}
TopDocs results = searcher.search(q.build(), n);
ScoreDoc[] hits = results.scoreDocs;
List<AccountInfo> result = new LinkedList<>();
for (ScoreDoc h : hits) {
Document doc = searcher.doc(h.doc);
IndexableField idField = checkNotNull(doc.getField(ID));
AccountInfo info = new AccountInfo(idField.numericValue().intValue());
info.name = doc.get(NAME);
info.email = doc.get(EMAIL);
info.username = doc.get(USERNAME);
result.add(info);
}
return result;
}
private IndexSearcher get() {
try {
return cache.get(true);
} catch (ExecutionException e) {
log.warn("Cannot fetch reviewers from cache", e);
return null;
}
}
private IndexSearcher index() throws IOException, OrmException {
RAMDirectory idx = new RAMDirectory();
IndexWriterConfig config = new IndexWriterConfig(
new StandardAnalyzer(CharArraySet.EMPTY_SET));
config.setOpenMode(OpenMode.CREATE);
try (IndexWriter writer = new IndexWriter(idx, config)) {
for (Account a : db.get().accounts().all()) {
if (a.isActive()) {
addAccount(writer, a);
}
}
}
return new IndexSearcher(DirectoryReader.open(idx));
}
private void addAccount(IndexWriter writer, Account a)
throws IOException, OrmException {
Document doc = new Document();
doc.add(new IntField(ID, a.getId().get(), Store.YES));
if (a.getFullName() != null) {
doc.add(new TextField(NAME, a.getFullName(), Store.YES));
}
if (a.getPreferredEmail() != null) {
doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES));
doc.add(new StringField(EMAIL, a.getPreferredEmail().toLowerCase(),
Store.YES));
}
AccountExternalIdAccess extIdAccess = db.get().accountExternalIds();
String username = AccountState.getUserName(
extIdAccess.byAccount(a.getId()).toList());
if (username != null) {
doc.add(new StringField(USERNAME, username, Store.YES));
}
writer.addDocument(doc);
}
}

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

@@ -27,7 +27,6 @@ import org.kohsuke.args4j.Option;
public class SuggestReviewers {
private static final int DEFAULT_MAX_SUGGESTED = 10;
private static final int DEFAULT_MAX_MATCHES = 100;
protected final Provider<ReviewDb> dbProvider;
protected final IdentifiedUser.GenericFactory identifiedUserFactory;
@@ -38,8 +37,6 @@ public class SuggestReviewers {
private final int maxAllowed;
protected int limit;
protected String query;
private boolean useFullTextSearch;
private final int fullTextMaxMatches;
protected final int maxSuggestedReviewers;
@Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT",
@@ -68,14 +65,6 @@ public class SuggestReviewers {
return suggestFrom;
}
public boolean getUseFullTextSearch() {
return useFullTextSearch;
}
public int getFullTextMaxMatches() {
return fullTextMaxMatches;
}
public int getLimit() {
return limit;
}
@@ -96,15 +85,11 @@ public class SuggestReviewers {
this.maxSuggestedReviewers =
cfg.getInt("suggest", "maxSuggestedReviewers", DEFAULT_MAX_SUGGESTED);
this.limit = this.maxSuggestedReviewers;
this.fullTextMaxMatches =
cfg.getInt("suggest", "fullTextSearchMaxMatches",
DEFAULT_MAX_MATCHES);
String suggest = cfg.getString("suggest", null, "accounts");
if ("OFF".equalsIgnoreCase(suggest)
|| "false".equalsIgnoreCase(suggest)) {
this.suggestAccounts = false;
} else {
this.useFullTextSearch = cfg.getBoolean("suggest", "fullTextSearch", false);
this.suggestAccounts = (av != AccountVisibility.NONE);
}

16
gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java
View File

@@ -20,6 +20,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.FieldType;
@@ -137,6 +138,21 @@ public class AccountField {
}
};
public static final FieldDef<AccountState, Iterable<String>> WATCHED_PROJECT =
new FieldDef.Repeatable<AccountState, String>(
"watchedproject", FieldType.EXACT, false) {
@Override
public Iterable<String> get(AccountState input, FillArgs args) {
return FluentIterable.from(input.getProjectWatches())
.transform(new Function<AccountProjectWatch, String>() {
@Override
public String apply(AccountProjectWatch in) {
return in.getProjectNameKey().get();
}
}).toSet();
}
};
private AccountField() {
}
}

3
gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
View File

@@ -30,6 +30,9 @@ public class AccountSchemaDefinitions extends SchemaDefinitions<AccountState> {
AccountField.REGISTERED,
AccountField.USERNAME);
static final Schema<AccountState> V2 =
schema(V1, AccountField.WATCHED_PROJECT);
public static final AccountSchemaDefinitions INSTANCE =
new AccountSchemaDefinitions();

15
gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.query.account;
import com.google.common.base.Function;
import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.errors.NotSignedInException;
@@ -122,8 +124,19 @@ public class AccountQueryBuilder extends QueryBuilder<AccountState> {
return AccountPredicates.username(username);
}
public Predicate<AccountState> defaultQuery(String query) {
return Predicate.and(
Lists.transform(Splitter.on(' ').omitEmptyStrings().splitToList(query),
new Function<String, Predicate<AccountState>>() {
@Override
public Predicate<AccountState> apply(String s) {
return defaultField(s);
}
}));
}
@Override
public Predicate<AccountState> defaultField(String query) {
protected Predicate<AccountState> defaultField(String query) {
// Adapt the capacity of this list when adding more default predicates.
List<Predicate<AccountState>> preds = Lists.newArrayListWithCapacity(4);
if ("self".equalsIgnoreCase(query)) {

9
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -38,6 +38,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend;
@@ -184,6 +185,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
final IndexConfig indexConfig;
final Provider<ListMembers> listMembers;
final StarredChangesUtil starredChangesUtil;
final AccountCache accountCache;
final boolean allowsDrafts;
private final Provider<CurrentUser> self;
@@ -217,6 +219,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
IndexConfig indexConfig,
Provider<ListMembers> listMembers,
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
@GerritServerConfig Config cfg) {
this(db, queryProvider, rewriter, opFactories, userFactory, self,
capabilityControlFactory, changeControlGenericFactory, notesFactory,
@@ -224,7 +227,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
allProjectsName, allUsersName, patchListCache, repoManager,
projectCache, listChildProjects, submitDryRun, conflictsCache,
trackingFooters, indexes != null ? indexes.getSearchIndex() : null,
indexConfig, listMembers, starredChangesUtil,
indexConfig, listMembers, starredChangesUtil, accountCache,
cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true));
}
@@ -256,6 +259,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
IndexConfig indexConfig,
Provider<ListMembers> listMembers,
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
boolean allowsDrafts) {
this.db = db;
this.queryProvider = queryProvider;
@@ -284,6 +288,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.indexConfig = indexConfig;
this.listMembers = listMembers;
this.starredChangesUtil = starredChangesUtil;
this.accountCache = accountCache;
this.allowsDrafts = allowsDrafts;
}
@@ -295,7 +300,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
allProjectsName, allUsersName, patchListCache, repoManager,
projectCache, listChildProjects, submitDryRun,
conflictsCache, trackingFooters, index, indexConfig, listMembers,
starredChangesUtil, allowsDrafts);
starredChangesUtil, accountCache, allowsDrafts);
}
Arguments asUser(Account.Id otherId) {

30
gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.query.change;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.CurrentUser;
@@ -22,22 +21,13 @@ import com.google.gerrit.server.query.AndPredicate;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryBuilder;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gwtorm.server.OrmException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
class IsWatchedByPredicate extends AndPredicate<ChangeData> {
private static final Logger log =
LoggerFactory.getLogger(IsWatchedByPredicate.class);
private static final CurrentUser.PropertyKey<List<AccountProjectWatch>> PROJECT_WATCHES =
CurrentUser.PropertyKey.create();
private static String describe(CurrentUser user) {
if (user.isIdentifiedUser()) {
return user.getAccountId().toString();
@@ -101,22 +91,14 @@ class IsWatchedByPredicate extends AndPredicate<ChangeData> {
}
}
private static List<AccountProjectWatch> getWatches(
private static Collection<AccountProjectWatch> getWatches(
ChangeQueryBuilder.Arguments args) throws QueryParseException {
CurrentUser user = args.getUser();
List<AccountProjectWatch> watches = user.get(PROJECT_WATCHES);
if (watches == null && user.isIdentifiedUser()) {
try {
watches = args.db.get().accountProjectWatches()
.byAccount(user.asIdentifiedUser().getAccountId()).toList();
user.put(PROJECT_WATCHES, watches);
} catch (OrmException e) {
log.warn("Cannot load accountProjectWatches", e);
}
if (user.isIdentifiedUser()) {
return args.accountCache.get(args.getUser().getAccountId())
.getProjectWatches();
}
return MoreObjects.firstNonNull(
watches,
Collections.<AccountProjectWatch> emptyList());
return Collections.<AccountProjectWatch> emptySet();
}
private static List<Predicate<ChangeData>> none() {

3
gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
View File

@@ -29,7 +29,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
null, null, null, null, null, null, null, null, null, null, null,
null, null, null, indexes, null, null, null, null, null, null, null));
null, null, null, indexes, null, null, null, null, null, null, null,
null));
}
@Operator

4
gerrit-server/src/test/java/com/google/gerrit/server/mail/FromAddressGeneratorProviderTest.java
View File

@@ -25,6 +25,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
@@ -298,6 +299,7 @@ public class FromAddressGeneratorProviderTest {
account.setFullName(name);
account.setPreferredEmail(email);
return new AccountState(account, Collections.<AccountGroup.UUID> emptySet(),
Collections.<AccountExternalId> emptySet());
Collections.<AccountExternalId> emptySet(),
Collections.<AccountProjectWatch> emptySet());
}
}

2
gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
View File

@@ -255,6 +255,8 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
assertQuery("Doe", user1);
assertQuery("doe", user1);
assertQuery("DOE", user1);
assertQuery("Jo Do", user1);
assertQuery("jo do", user1);
assertQuery("self", currentUserInfo, user3);
assertQuery("name:John", user1);
assertQuery("name:john", user1);

7
gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
@@ -73,8 +74,8 @@ public class FakeAccountCache implements AccountCache {
}
private static AccountState newState(Account account) {
return new AccountState(
account, ImmutableSet.<AccountGroup.UUID> of(),
ImmutableSet.<AccountExternalId> of());
return new AccountState(account, ImmutableSet.<AccountGroup.UUID> of(),
ImmutableSet.<AccountExternalId> of(),
ImmutableSet.<AccountProjectWatch> of());
}
}