Remove suggest.fullTextSearch option

This is no longer needed since the reviewer suggestion is now based on
the account index which supports full text search as well.

Change-Id: I14ab15f143a4a77b3d1bdae125e3c34007355033
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-07-04 14:01:42 +02:00
parent e2de9ccdc2
commit 22ca9734e7
5 changed files with 2 additions and 273 deletions

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

View File

@ -168,17 +168,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
}
@Test
@GerritConfig(name = "suggest.fullTextSearch", value = "true")
public void suggestReviewersFullTextSearch() throws Exception {
testSuggestReviewersFullTextSearch();
}
@Test
public void suggestReviewersFullTextSearchWithAccountIndex() throws Exception {
testSuggestReviewersFullTextSearch();
}
private void testSuggestReviewersFullTextSearch() throws Exception {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
@ -229,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();

View File

@ -41,7 +41,6 @@ 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;
@ -61,7 +60,6 @@ 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;
@ -88,7 +86,6 @@ public class ReviewersUtil {
private final AccountIndexCollection indexes;
private final AccountQueryBuilder queryBuilder;
private final AccountQueryProcessor queryProcessor;
private final ReviewerSuggestionCache reviewerSuggestionCache;
private final AccountControl accountControl;
private final Provider<ReviewDb> dbProvider;
private final GroupBackend groupBackend;
@ -101,7 +98,6 @@ public class ReviewersUtil {
AccountIndexCollection indexes,
AccountQueryBuilder queryBuilder,
AccountQueryProcessor queryProcessor,
ReviewerSuggestionCache reviewerSuggestionCache,
AccountControl.Factory accountControlFactory,
Provider<ReviewDb> dbProvider,
GroupBackend groupBackend,
@ -114,7 +110,6 @@ public class ReviewersUtil {
this.indexes = indexes;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
this.reviewerSuggestionCache = reviewerSuggestionCache;
this.accountControl = accountControlFactory.get();
this.dbProvider = dbProvider;
this.groupBackend = groupBackend;
@ -133,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)) {
@ -144,13 +138,8 @@ public class ReviewersUtil {
return Collections.emptyList();
}
Collection<AccountInfo> suggestedAccounts;
if (useFullTextSearch) {
suggestedAccounts =
suggestAccountFullTextSearch(suggestReviewers, visibilityControl);
} else {
suggestedAccounts = suggestAccounts(suggestReviewers, visibilityControl);
}
Collection<AccountInfo> suggestedAccounts =
suggestAccounts(suggestReviewers, visibilityControl);
List<SuggestedReviewerInfo> reviewer = new ArrayList<>();
for (AccountInfo a : suggestedAccounts) {
@ -178,24 +167,6 @@ 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 Collection<AccountInfo> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {

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);
}
}

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);
}