Build a Recommender for Reviewer Suggestion

Until now, reviewer suggestion was purely based on a search. I've built
a small recommender to improve the suggestions based on past
contributions by the individual reviewers and added an extension point
so that people can customize this feature.

The built-in recommender makes a default suggestion of reviewers before
the user types a query. These are based on people that have reviewed the
last contributions that a user made.

If the user starts typing in the box, we generate a list of candidates
using the account index and feed it into a small recommender. The
recommender ranks the list by looking at recent contributions of the
candidates made in the same project. Contributions include reviews,
owned-changes and comments at different weights.

Change-Id: I5aca23ddd2442146fd26bdc12e7c18da85de7ac1
This commit is contained in:
Patrick Hiesel
2016-05-03 18:15:08 +02:00
parent a4b637d578
commit 87880b0543
17 changed files with 723 additions and 143 deletions

View File

@@ -67,6 +67,15 @@ be added at once by adding a group as reviewer.
+
Default is 20.
[[addReviewer.baseWeight]]addReviewer.baseWeight::
+
The weight that will be applied in the default reviewer ranking algorithm.
This can be increased or decreased to give more or less influence to plugins.
If set to zero, the base ranking will not have any effect. Reviewers will then
be ordered as ranked by the plugins (if there are any).
+
By default 1.
[[auth]]
=== Section auth
@@ -3907,11 +3916,11 @@ By default 10.
[[suggest.from]]suggest.from::
+
The number of characters that a user must have typed before suggestions
are provided. If set to 0, suggestions are always provided.
are provided. If set to 0, suggestions are always provided. This is only
used for suggesting accounts when adding members to a group.
+
By default 0.
[[theme]]
=== Section theme

View File

@@ -2399,6 +2399,41 @@ new RestApi("accounts").id("self").view("password.http")
----
[[reviewer-suggestion]]
== Reviewer Suggestion Plugins
Gerrit provides an extension point that enables Plugins to rank
the list of reviewer suggestion a user receives upon clicking "Add Reviewer" on
the change screen.
Gerrit supports both a default suggestion that appears when the user has not yet
typed anything and a filtered suggestion that is shown as the user starts
typing.
Plugins receive a candidate list and can return a Set of suggested reviewers
containing the Account.Id and a score for each reviewer.
The candidate list is non-binding and plugins can choose to return reviewers not
initially contained in the candidate list.
Server administrators can configure the overall weight of each plugin using the
weight config parameter on [addreviewer "<pluginName-exportName>"].
[source, java]
----
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import java.util.Set;
public class MyPlugin implements ReviewerSuggestion {
public Set<SuggestedReviewer> suggestReviewers(
Change.Id changeId, String query, Set<Account.Id> candidates){
Set<SuggestedReviewer> suggestions = new HashSet<>();
// Implement your ranking logic here
return suggestions;
}
}
----
== SEE ALSO
* link:js-api.html[JavaScript API]

View File

@@ -20,15 +20,20 @@ import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GerritConfigs;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.group.CreateGroup;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.inject.Inject;
@@ -38,7 +43,9 @@ import org.junit.Test;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
@Sandboxed
public class SuggestReviewersIT extends AbstractDaemonTest {
@Inject
private CreateGroup.Factory createGroupFactory;
@@ -88,15 +95,6 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
assertThat(reviewers).isEmpty();
}
@Test
@GerritConfig(name = "suggest.from", value = "2")
public void suggestReviewersNoResult3() throws Exception {
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(changeId, name("").substring(0, 1), 6);
assertThat(reviewers).isEmpty();
}
@Test
public void suggestReviewersChange() throws Exception {
String changeId = createChange().getChangeId();
@@ -204,7 +202,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
assertThat(reviewers).hasSize(1);
reviewers = suggestReviewers(changeId, "example.com", 7);
assertThat(reviewers).hasSize(6);
assertThat(reviewers).hasSize(5);
reviewers = suggestReviewers(changeId, user1.email, 2);
assertThat(reviewers).hasSize(1);
@@ -267,6 +265,145 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
assertThat(reviewer.confirm).isTrue();
}
@Test
public void defaultReviewerSuggestion() throws Exception{
TestAccount user1 = user("customuser1", "User1");
TestAccount reviewer1 = user("customuser2", "User2");
TestAccount reviewer2 = user("customuser3", "User3");
setApiUser(user1);
String changeId1 = createChangeFromApi();
setApiUser(reviewer1);
reviewChange(changeId1);
setApiUser(user1);
String changeId2 = createChangeFromApi();
setApiUser(reviewer1);
reviewChange(changeId2);
setApiUser(reviewer2);
reviewChange(changeId2);
setApiUser(user1);
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(createChangeFromApi(), null, 4);
assertThat(
reviewers.stream()
.map(r -> r.account._accountId)
.collect(Collectors.toList()))
.containsExactly(
reviewer1.id.get(),
reviewer2.id.get())
.inOrder();
}
@Test
public void defaultReviewerSuggestionOnFirstChange() throws Exception{
TestAccount user1 = user("customuser1", "User1");
setApiUser(user1);
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(createChange().getChangeId(), "", 4);
assertThat(reviewers).isEmpty();
}
@Test
@GerritConfig(name = "suggest.maxSuggestedReviewers", value = "10")
public void reviewerRanking() throws Exception{
// Assert that user are ranked by the number of times they have applied a
// a label to a change (highest), added comments (medium) or owned a
// change (low).
String fullName = "Primum Finalis";
TestAccount userWhoOwns = user("customuser1", fullName);
TestAccount reviewer1 = user("customuser2", fullName);
TestAccount reviewer2 = user("customuser3", fullName);
TestAccount userWhoComments = user("customuser4", fullName);
TestAccount userWhoLooksForSuggestions = user("customuser5", fullName);
// Create a change as userWhoOwns and add some reviews
setApiUser(userWhoOwns);
String changeId1 = createChangeFromApi();
setApiUser(reviewer1);
reviewChange(changeId1);
setApiUser(user1);
String changeId2 = createChangeFromApi();
setApiUser(reviewer1);
reviewChange(changeId2);
setApiUser(reviewer2);
reviewChange(changeId2);
// Create a comment as a different user
setApiUser(userWhoComments);
ReviewInput ri = new ReviewInput();
ri.message = "Test";
gApi.changes().id(changeId1).revision(1).review(ri);
// Create a change as a new user to assert that we receive the correct
// ranking
setApiUser(userWhoLooksForSuggestions);
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(createChangeFromApi(), "Pri", 4);
assertThat(
reviewers.stream()
.map(r -> r.account._accountId)
.collect(Collectors.toList()))
.containsExactly(
reviewer1.id.get(),
reviewer2.id.get(),
userWhoOwns.id.get(),
userWhoComments.id.get())
.inOrder();
}
@Test
public void reviewerRankingProjectIsolation() throws Exception{
// Create new project
Project.NameKey newProject = createProject("test");
// Create users who review changes in both the default and the new project
String fullName = "Primum Finalis";
TestAccount userWhoOwns = user("customuser1", fullName);
TestAccount reviewer1 = user("customuser2", fullName);
TestAccount reviewer2 = user("customuser3", fullName);
setApiUser(userWhoOwns);
String changeId1 = createChangeFromApi();
setApiUser(reviewer1);
reviewChange(changeId1);
setApiUser(userWhoOwns);
String changeId2 = createChangeFromApi(newProject);
setApiUser(reviewer2);
reviewChange(changeId2);
setApiUser(userWhoOwns);
String changeId3 = createChangeFromApi(newProject);
setApiUser(reviewer2);
reviewChange(changeId3);
setApiUser(userWhoOwns);
List<SuggestedReviewerInfo> reviewers =
suggestReviewers(createChangeFromApi(), "Prim", 4);
// Assert that reviewer1 is on top, even though reviewer2 has more reviews
// in other projects
assertThat(
reviewers.stream()
.map(r -> r.account._accountId)
.collect(Collectors.toList()))
.containsExactly(reviewer1.id.get(), reviewer2.id.get())
.inOrder();
}
private List<SuggestedReviewerInfo> suggestReviewers(String changeId,
String query, int n) throws Exception {
return gApi.changes()
@@ -296,4 +433,23 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
throws Exception {
return user(name, fullName, name, groups);
}
private void reviewChange(String changeId) throws RestApiException {
ReviewInput ri = new ReviewInput();
ri.label("Code-Review", 1);
gApi.changes().id(changeId).current().review(ri);
}
private String createChangeFromApi() throws RestApiException{
return createChangeFromApi(project);
}
private String createChangeFromApi(Project.NameKey project)
throws RestApiException{
ChangeInput ci = new ChangeInput();
ci.project = project.get();
ci.subject = "Test change at" + System.nanoTime();
ci.branch = "master";
return gApi.changes().create(ci).get().changeId;
}
}

View File

@@ -43,7 +43,7 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
}
@Override
public final void requestSuggestions(final Request request, final Callback cb) {
public final void requestSuggestions(Request request, Callback cb) {
onRequestSuggestions(request, new Callback() {
@Override
public void onSuggestionsReady(final Request request,
@@ -88,6 +88,7 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
ds = escape(ds);
}
if (qstr != null && !qstr.isEmpty()) {
StringBuilder pattern = new StringBuilder();
for (String qterm : splitQuery(qstr)) {
qterm = escape(qterm);
@@ -97,7 +98,6 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
// 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.
if (pattern.length() > 0) {
pattern.append("|");
}
@@ -108,6 +108,7 @@ public abstract class HighlightSuggestOracle extends SuggestOracle {
// Repairing <strong>-ed escapes.
ds = sgi(ds, "(&[a-z]*)<strong>([a-z]*)</strong>([a-z]*;)", "$1$2$3");
}
displayString = ds;
}

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.client.ui;
import com.google.gwt.user.client.Timer;
import com.google.gwt.user.client.ui.SuggestOracle;
/**
@@ -31,6 +32,10 @@ public class RemoteSuggestOracle extends SuggestOracle {
private final SuggestOracle oracle;
private Query query;
private String last;
private Timer requestRetentionTimer;
private boolean cancelOutstandingRequest;
private boolean serveSuggestions;
public RemoteSuggestOracle(SuggestOracle src) {
oracle = src;
@@ -42,6 +47,18 @@ public class RemoteSuggestOracle extends SuggestOracle {
@Override
public void requestSuggestions(Request req, Callback cb) {
if (!serveSuggestions){
return;
}
// Use a timer for key stroke retention, such that we don't query the
// backend for each and every keystroke we receive.
if (requestRetentionTimer != null) {
requestRetentionTimer.cancel();
}
requestRetentionTimer = new Timer() {
@Override
public void run() {
Query q = new Query(req, cb);
if (query == null) {
query = q;
@@ -50,12 +67,33 @@ public class RemoteSuggestOracle extends SuggestOracle {
query = q;
}
}
};
requestRetentionTimer.schedule(200);
}
@Override
public void requestDefaultSuggestions(Request req, Callback cb) {
requestSuggestions(req, cb);
}
@Override
public boolean isDisplayStringHTML() {
return oracle.isDisplayStringHTML();
}
public void cancelOutstandingRequest() {
if (requestRetentionTimer != null) {
requestRetentionTimer.cancel();
}
if (query != null) {
cancelOutstandingRequest = true;
}
}
public void setServeSuggestions(boolean serveSuggestions) {
this.serveSuggestions = serveSuggestions;
}
private class Query implements Callback {
final Request request;
final Callback callback;
@@ -71,7 +109,11 @@ public class RemoteSuggestOracle extends SuggestOracle {
@Override
public void onSuggestionsReady(Request req, Response res) {
if (query == this) {
if (cancelOutstandingRequest || !serveSuggestions) {
// If cancelOutstandingRequest() was called, we ignore this response
cancelOutstandingRequest = false;
query = null;
} else if (query == this) {
// No new request was started while this query was running.
// Propose this request's response as the suggestions.
query = null;

View File

@@ -21,21 +21,21 @@ import com.google.gerrit.client.info.GroupBaseInfo;
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;
import com.google.gwt.core.client.JsArray;
import com.google.gwtexpui.safehtml.client.HighlightSuggestOracle;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
/** REST API based suggestion Oracle for reviewers. */
public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
public class ReviewerSuggestOracle extends HighlightSuggestOracle {
private Change.Id changeId;
@Override
protected void _onRequestSuggestions(final Request req, final Callback cb) {
protected void onRequestSuggestions(final Request req, final Callback cb) {
ChangeApi
.suggestReviewers(changeId.get(), req.getQuery(), req.getLimit(), false)
.get(new GerritCallback<JsArray<SuggestReviewerInfo>>() {
@@ -56,6 +56,11 @@ public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
});
}
@Override
public void requestDefaultSuggestions(final Request req, final Callback cb) {
requestSuggestions(req, cb);
}
public void setChange(Change.Id changeId) {
this.changeId = changeId;
}

View File

@@ -81,6 +81,7 @@ public class Reviewers extends Composite {
Reviewers() {
reviewerSuggestOracle = new ReviewerSuggestOracle();
suggestBox = new RemoteSuggestBox(reviewerSuggestOracle);
suggestBox.enableDefaultSuggestions();
suggestBox.setVisibleLength(55);
suggestBox.setHintText(Util.C.approvalTableAddReviewerHint());
suggestBox.addCloseHandler(new CloseHandler<RemoteSuggestBox>() {
@@ -123,6 +124,7 @@ public class Reviewers extends Composite {
UIObject.setVisible(form, true);
UIObject.setVisible(error, false);
addReviewerIcon.setVisible(false);
suggestBox.setServeSuggestionsOnOracle(true);
suggestBox.setFocus(true);
}
@@ -143,6 +145,7 @@ public class Reviewers extends Composite {
UIObject.setVisible(form, false);
suggestBox.setFocus(false);
suggestBox.setText("");
suggestBox.setServeSuggestionsOnOracle(false);
}
private void addReviewer(final String reviewer, boolean confirmed) {

View File

@@ -171,10 +171,13 @@ public class ChangeApi {
}
public static RestApi suggestReviewers(int id, String q, int n, boolean e) {
return change(id).view("suggest_reviewers")
.addParameter("q", q)
RestApi api = change(id).view("suggest_reviewers")
.addParameter("n", n)
.addParameter("e", e);
if (q != null) {
api.addParameter("q", q);
}
return api;
}
public static RestApi vote(int id, int reviewer, String vote) {

View File

@@ -61,7 +61,8 @@ public class AccountSuggestOracle extends SuggestAfterTypingNCharsOracle {
public static String format(AccountInfo info, String query) {
String s = FormatUtil.nameEmail(info);
if (!containsQuery(s, query) && info.secondaryEmails() != null) {
if (query != null && !containsQuery(s, query) &&
info.secondaryEmails() != null) {
for (String email : Natives.asList(info.secondaryEmails())) {
AccountInfo info2 = AccountInfo.create(info._accountId(), info.name(),
email, info.username());

View File

@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.gerrit.client.ui;
import com.google.gwt.event.dom.client.FocusEvent;
import com.google.gwt.event.dom.client.FocusHandler;
import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyDownEvent;
import com.google.gwt.event.dom.client.KeyDownHandler;
@@ -42,6 +44,7 @@ public class RemoteSuggestBox extends Composite implements Focusable, HasText,
public RemoteSuggestBox(SuggestOracle oracle) {
remoteSuggestOracle = new RemoteSuggestOracle(oracle);
remoteSuggestOracle.setServeSuggestions(true);
display = new DefaultSuggestionDisplay();
textBox = new HintTextBox();
@@ -49,7 +52,6 @@ public class RemoteSuggestBox extends Composite implements Focusable, HasText,
@Override
public void onKeyDown(KeyDownEvent e) {
submitOnSelection = false;
if (e.getNativeKeyCode() == KeyCodes.KEY_ESCAPE) {
CloseEvent.fire(RemoteSuggestBox.this, RemoteSuggestBox.this);
} else if (e.getNativeKeyCode() == KeyCodes.KEY_ENTER) {
@@ -70,10 +72,11 @@ public class RemoteSuggestBox extends Composite implements Focusable, HasText,
suggestBox.addSelectionHandler(new SelectionHandler<Suggestion>() {
@Override
public void onSelection(SelectionEvent<Suggestion> event) {
textBox.setFocus(true);
if (submitOnSelection) {
SelectionEvent.fire(RemoteSuggestBox.this, getText());
}
remoteSuggestOracle.cancelOutstandingRequest();
display.hideSuggestions();
}
});
initWidget(suggestBox);
@@ -138,4 +141,19 @@ public class RemoteSuggestBox extends Composite implements Focusable, HasText,
public void selectAll() {
suggestBox.getValueBox().selectAll();
}
public void enableDefaultSuggestions() {
textBox.addFocusHandler(new FocusHandler() {
@Override
public void onFocus(FocusEvent focusEvent) {
if (textBox.getText().equals("")) {
suggestBox.showSuggestionList();
}
}
});
}
public void setServeSuggestionsOnOracle(boolean serveSuggestions) {
remoteSuggestOracle.setServeSuggestions(serveSuggestions);
}
}

View File

@@ -0,0 +1,267 @@
// 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.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestReviewers;
import com.google.gerrit.server.change.SuggestedReviewer;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import org.apache.commons.lang.mutable.MutableDouble;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;
public class ReviewerRecommender {
private static final Logger log =
LoggerFactory.getLogger(ReviewersUtil.class);
private static final double BASE_REVIEWER_WEIGHT = 10;
private static final double BASE_OWNER_WEIGHT = 1;
private static final double BASE_COMMENT_WEIGHT = 0.5;
private static final long PLUGIN_QUERY_TIMEOUT = 500; //ms
private final ChangeQueryBuilder changeQueryBuilder;
private final Config config;
private final DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap;
private final InternalChangeQuery internalChangeQuery;
private final WorkQueue workQueue;
@Inject
ReviewerRecommender(ChangeQueryBuilder changeQueryBuilder,
DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap,
InternalChangeQuery internalChangeQuery,
WorkQueue workQueue,
@GerritServerConfig Config config) {
Set<FillOptions> fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
this.changeQueryBuilder = changeQueryBuilder;
this.config = config;
this.internalChangeQuery = internalChangeQuery;
this.reviewerSuggestionPluginMap = reviewerSuggestionPluginMap;
this.workQueue = workQueue;
}
public List<Account.Id> suggestReviewers(
ChangeNotes changeNotes,
SuggestReviewers suggestReviewers, ProjectControl projectControl,
List<Account.Id> candidateList)
throws OrmException {
String query = suggestReviewers.getQuery();
double baseWeight = config.getInt("addReviewer", "baseWeight", 1);
Map<Account.Id, MutableDouble> reviewerScores;
if (Strings.isNullOrEmpty(query)) {
reviewerScores = baseRankingForEmptyQuery(baseWeight);
} else {
reviewerScores = baseRankingForCandidateList(
candidateList, projectControl, baseWeight);
}
// Send the query along with a candidate list to all plugins and merge the
// results. Plugins don't necessarily need to use the candidates list, they
// can also return non-candidate account ids.
List<Callable<Set<SuggestedReviewer>>> tasks =
new ArrayList<>(reviewerSuggestionPluginMap.plugins().size());
List<Double> weights =
new ArrayList<>(reviewerSuggestionPluginMap.plugins().size());
for (DynamicMap.Entry<ReviewerSuggestion> plugin :
reviewerSuggestionPluginMap) {
tasks.add(() -> plugin.getProvider().get().suggestReviewers(
changeNotes.getChangeId(), query, reviewerScores.keySet()));
String pluginWeight = config.getString("addReviewer",
plugin.getPluginName() + "-" + plugin.getExportName(), "weight");
if (Strings.isNullOrEmpty(pluginWeight)) {
pluginWeight = "1";
}
try {
weights.add(Double.parseDouble(pluginWeight));
} catch (NumberFormatException e) {
log.error("Exception while parsing weight for " +
plugin.getPluginName() + "-" + plugin.getExportName(), e);
weights.add(1d);
}
}
try {
List<Future<Set<SuggestedReviewer>>> futures = workQueue
.getDefaultQueue()
.invokeAll(tasks, PLUGIN_QUERY_TIMEOUT, TimeUnit.MILLISECONDS);
Iterator<Double> weightIterator = weights.iterator();
for (Future<Set<SuggestedReviewer>> f : futures) {
double weight = weightIterator.next();
for (SuggestedReviewer s : f.get()) {
if (reviewerScores.containsKey(s.account)) {
reviewerScores.get(s.account).add(s.score * weight);
} else {
reviewerScores.put(s.account, new MutableDouble(s.score * weight));
}
}
}
} catch (ExecutionException | InterruptedException e) {
log.error("Exception while suggesting reviewers", e);
return ImmutableList.of();
}
// Remove change owner
reviewerScores.remove(changeNotes.getChange().getOwner());
// Sort results
Stream<Entry<Account.Id, MutableDouble>> sorted =
reviewerScores.entrySet().stream()
.sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
List<Account.Id> sortedSuggestions = sorted
.map(Map.Entry::getKey)
.collect(Collectors.toList());
return sortedSuggestions;
}
private Map<Account.Id, MutableDouble> baseRankingForEmptyQuery(
double baseWeight) throws OrmException{
// Get the user's last 50 changes, check approvals
try {
List<ChangeData> result = internalChangeQuery
.setLimit(50)
.setRequestedFields(ImmutableSet.of(ChangeField.REVIEWER.getName()))
.query(changeQueryBuilder.owner("self"));
Map<Account.Id, MutableDouble> suggestions = new HashMap<>();
for (ChangeData cd : result) {
for (PatchSetApproval approval : cd.currentApprovals()) {
Account.Id id = approval.getAccountId();
if (suggestions.containsKey(id)) {
suggestions.get(id).add(baseWeight);
} else {
suggestions.put(id, new MutableDouble(baseWeight));
}
}
}
return suggestions;
} catch (QueryParseException e) {
// Unhandled, because owner:self will never provoke a QueryParseException
log.error("Exception while suggesting reviewers", e);
return ImmutableMap.of();
}
}
private Map<Account.Id, MutableDouble> baseRankingForCandidateList(
List<Account.Id> candidates,
ProjectControl projectControl,
double baseWeight) throws OrmException {
// Get each reviewer's activity based on number of applied labels
// (weighted 10d), number of comments (weighted 0.5d) and number of owned
// changes (weighted 1d).
Map<Account.Id, MutableDouble> reviewers = new LinkedHashMap<>();
if (candidates.size() == 0) {
return reviewers;
}
List<Predicate<ChangeData>> predicates = new ArrayList<>();
for (Account.Id id : candidates) {
try {
Predicate<ChangeData> projectQuery =
changeQueryBuilder.project(projectControl.getProject().getName());
// Get all labels for this project and create a compound OR query to
// fetch all changes where users have applied one of these labels
List<LabelType> labelTypes =
projectControl.getLabelTypes().getLabelTypes();
List<Predicate<ChangeData>> labelPredicates =
new ArrayList<>(labelTypes.size());
for (LabelType type : labelTypes) {
labelPredicates
.add(changeQueryBuilder.label(type.getName() + ",user=" + id));
}
Predicate<ChangeData> reviewerQuery =
Predicate.and(projectQuery, Predicate.or(labelPredicates));
Predicate<ChangeData> ownerQuery = Predicate.and(projectQuery,
changeQueryBuilder.owner(id.toString()));
Predicate<ChangeData> commentedByQuery = Predicate.and(projectQuery,
changeQueryBuilder.commentby(id.toString()));
predicates.add(reviewerQuery);
predicates.add(ownerQuery);
predicates.add(commentedByQuery);
reviewers.put(id, new MutableDouble());
} catch (QueryParseException e) {
// Unhandled: If an exception is thrown, we won't increase the
// candidates's score
log.error("Exception while suggesting reviewers", e);
}
}
List<List<ChangeData>> result = internalChangeQuery
.setLimit(100 * predicates.size())
.setRequestedFields(ImmutableSet.of())
.query(predicates);
Iterator<List<ChangeData>> queryResultIterator = result.iterator();
Iterator<Account.Id> reviewersIterator = reviewers.keySet().iterator();
double[] weights = new double[]{
BASE_REVIEWER_WEIGHT, BASE_OWNER_WEIGHT, BASE_COMMENT_WEIGHT};
int i = 0;
Account.Id currentId = null;
while (queryResultIterator.hasNext()) {
List<ChangeData> currentResult = queryResultIterator.next();
if (i % weights.length == 0) {
currentId = reviewersIterator.next();
}
reviewers.get(currentId).add(weights[i % weights.length] *
baseWeight * currentResult.size());
i++;
}
return reviewers;
}
}

View File

@@ -14,20 +14,15 @@
package com.google.gerrit.server;
import static java.util.Comparator.comparing;
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;
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;
@@ -44,6 +39,7 @@ import com.google.gerrit.server.change.PostReviewers;
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.notedb.ChangeNotes;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.QueryParseException;
@@ -56,98 +52,95 @@ 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.LinkedHashMap;
import java.util.HashSet;
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.<SuggestedReviewerInfo> from(comparing(
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);
}));
// Generate a candidate list at 3x the size of what the user wants to see to
// give the ranking algorithm a good set of candidates it can work with
private static final int CANDIDATE_LIST_MULTIPLIER = 3;
private final AccountLoader accountLoader;
private final AccountCache accountCache;
private final AccountIndexCollection indexes;
private final AccountQueryBuilder queryBuilder;
private final AccountQueryProcessor queryProcessor;
private final AccountControl accountControl;
private final Provider<ReviewDb> dbProvider;
private final AccountIndexCollection accountIndexes;
private final AccountLoader accountLoader;
private final AccountQueryBuilder accountQueryBuilder;
private final AccountQueryProcessor accountQueryProcessor;
private final GroupBackend groupBackend;
private final GroupMembers.Factory groupMembersFactory;
private final Provider<CurrentUser> currentUser;
private final Provider<ReviewDb> dbProvider;
private final ReviewerRecommender reviewerRecommender;
@Inject
ReviewersUtil(AccountLoader.Factory accountLoaderFactory,
AccountCache accountCache,
AccountIndexCollection indexes,
AccountQueryBuilder queryBuilder,
AccountQueryProcessor queryProcessor,
ReviewersUtil(AccountCache accountCache,
AccountControl.Factory accountControlFactory,
Provider<ReviewDb> dbProvider,
AccountIndexCollection accountIndexes,
AccountLoader.Factory accountLoaderFactory,
AccountQueryBuilder accountQueryBuilder,
AccountQueryProcessor accountQueryProcessor,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser) {
Provider<CurrentUser> currentUser,
Provider<ReviewDb> dbProvider,
ReviewerRecommender reviewerRecommender) {
Set<FillOptions> fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
this.accountLoader = accountLoaderFactory.create(fillOptions);
this.accountCache = accountCache;
this.indexes = indexes;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
this.accountControl = accountControlFactory.get();
this.accountIndexes = accountIndexes;
this.accountLoader = accountLoaderFactory.create(fillOptions);
this.accountQueryBuilder = accountQueryBuilder;
this.accountQueryProcessor = accountQueryProcessor;
this.currentUser = currentUser;
this.dbProvider = dbProvider;
this.groupBackend = groupBackend;
this.groupMembersFactory = groupMembersFactory;
this.currentUser = currentUser;
this.reviewerRecommender = reviewerRecommender;
}
public interface VisibilityControl {
boolean isVisibleTo(Account.Id account) throws OrmException;
}
public List<SuggestedReviewerInfo> suggestReviewers(
public List<SuggestedReviewerInfo> suggestReviewers(ChangeNotes changeNotes,
SuggestReviewers suggestReviewers, ProjectControl projectControl,
VisibilityControl visibilityControl, boolean excludeGroups)
throws IOException, OrmException, BadRequestException {
throws IOException, OrmException {
String query = suggestReviewers.getQuery();
boolean suggestAccounts = suggestReviewers.getSuggestAccounts();
int suggestFrom = suggestReviewers.getSuggestFrom();
int limit = suggestReviewers.getLimit();
if (Strings.isNullOrEmpty(query)) {
throw new BadRequestException("missing query field");
}
if (!suggestAccounts || query.length() < suggestFrom) {
if (!suggestReviewers.getSuggestAccounts()) {
return Collections.emptyList();
}
Collection<AccountInfo> suggestedAccounts =
suggestAccounts(suggestReviewers, visibilityControl);
List<Account.Id> candidateList = new ArrayList<>();
if (!Strings.isNullOrEmpty(query)) {
candidateList = suggestAccounts(suggestReviewers, visibilityControl);
}
List<Account.Id> sortedRecommendations = reviewerRecommender
.suggestReviewers(changeNotes, suggestReviewers, projectControl,
candidateList);
// Populate AccountInfo
List<SuggestedReviewerInfo> reviewer = new ArrayList<>();
for (AccountInfo a : suggestedAccounts) {
for (Account.Id id : sortedRecommendations) {
AccountInfo account = accountLoader.get(id);
if (account != null) {
SuggestedReviewerInfo info = new SuggestedReviewerInfo();
info.account = a;
info.account = account;
info.count = 1;
reviewer.add(info);
}
}
accountLoader.fill();
if (!excludeGroups) {
if (!excludeGroups && !Strings.isNullOrEmpty(query)) {
for (GroupReference g : suggestAccountGroup(suggestReviewers, projectControl)) {
GroupAsReviewer result = suggestGroupAsReviewer(
suggestReviewers, projectControl.getProject(), g, visibilityControl);
@@ -161,59 +154,56 @@ public class ReviewersUtil {
if (result.allowedWithConfirmation) {
suggestedReviewerInfo.confirm = true;
}
// Always add groups at the end as individual accounts are usually
// more important
reviewer.add(suggestedReviewerInfo);
}
}
}
reviewer = ORDERING.immutableSortedCopy(reviewer);
if (reviewer.size() <= limit) {
return reviewer;
}
return reviewer.subList(0, limit);
}
private Collection<AccountInfo> suggestAccounts(SuggestReviewers suggestReviewers,
private List<Account.Id> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
AccountIndex searchIndex = indexes.getSearchIndex();
AccountIndex searchIndex = accountIndexes.getSearchIndex();
if (searchIndex != null) {
return suggestAccountsFromIndex(suggestReviewers);
}
return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
private Collection<AccountInfo> suggestAccountsFromIndex(
private List<Account.Id> 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()));
Set<Account.Id> matches = new HashSet<>();
QueryResult<AccountState> result = accountQueryProcessor
.setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
.query(accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
matches.put(id, accountLoader.get(id));
matches.add(id);
}
accountLoader.fill();
return matches.values();
return new ArrayList<>(matches);
} catch (QueryParseException e) {
return ImmutableList.of();
}
}
private Collection<AccountInfo> suggestAccountsFromDb(
private List<Account.Id> suggestAccountsFromDb(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
String query = suggestReviewers.getQuery();
int limit = suggestReviewers.getLimit();
int limit = suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER;
String a = query;
String b = a + MAX_SUFFIX;
Map<Account.Id, AccountInfo> r = new LinkedHashMap<>();
Map<Account.Id, String> queryEmail = new HashMap<>();
Set<Account.Id> r = new HashSet<>();
for (Account p : dbProvider.get().accounts()
.suggestByFullName(a, b, limit)) {
@@ -234,36 +224,26 @@ public class ReviewersUtil {
if (r.size() < limit) {
for (AccountExternalId e : dbProvider.get().accountExternalIds()
.suggestByEmailAddress(a, b, limit - r.size())) {
if (!r.containsKey(e.getAccountId())) {
if (!r.contains(e.getAccountId())) {
Account p = accountCache.get(e.getAccountId()).getAccount();
if (p.isActive()) {
if (addSuggestion(r, p.getId(), visibilityControl)) {
queryEmail.put(e.getAccountId(), e.getEmailAddress());
addSuggestion(r, p.getId(), visibilityControl);
}
}
}
}
return new ArrayList<>(r);
}
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,
private boolean addSuggestion(Set<Account.Id> map,
Account.Id account, VisibilityControl visibilityControl)
throws OrmException {
if (!map.containsKey(account)
if (!map.contains(account)
// Can the suggestion see the change?
&& visibilityControl.isVisibleTo(account)
// Can the current user see the account?
&& accountControl.canSee(account)) {
map.put(account, accountLoader.get(account));
map.add(account);
return true;
}
return false;
@@ -282,7 +262,8 @@ public class ReviewersUtil {
int size;
}
private GroupAsReviewer suggestGroupAsReviewer(SuggestReviewers suggestReviewers,
private GroupAsReviewer suggestGroupAsReviewer(
SuggestReviewers suggestReviewers,
Project project, GroupReference group,
VisibilityControl visibilityControl) throws OrmException, IOException {
GroupAsReviewer result = new GroupAsReviewer();

View File

@@ -0,0 +1,41 @@
// 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.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import java.util.Set;
/**
* Listener to provide reviewer suggestions.
* <p>
* Invoked by Gerrit a user who is searching for a reviewer to add to a change.
*/
@ExtensionPoint
public interface ReviewerSuggestion {
/**
* Reviewer suggestion.
*
* @param changeId The changeId that the suggestion is for.
* @param query The query as typed by the user. Can be an empty string.
* @param candidates A set of candidates for the ranking.
* @return Set of suggested reviewers as a tuple of account id and score.
* The account ids listed here don't have to be a part of candidates.
*/
Set<SuggestedReviewer> suggestReviewers(
Change.Id changeId, String query, Set<Account.Id> candidates);
}

View File

@@ -54,7 +54,7 @@ public class SuggestChangeReviewers extends SuggestReviewers
@Override
public List<SuggestedReviewerInfo> apply(ChangeResource rsrc)
throws BadRequestException, OrmException, IOException {
return reviewersUtil.suggestReviewers(this,
return reviewersUtil.suggestReviewers(rsrc.getNotes(), this,
rsrc.getControl().getProjectControl(), getVisibility(rsrc), excludeGroups);
}

View File

@@ -33,7 +33,6 @@ public class SuggestReviewers {
protected final ReviewersUtil reviewersUtil;
private final boolean suggestAccounts;
private final int suggestFrom;
private final int maxAllowed;
private final int maxAllowedWithoutConfirmation;
protected int limit;
@@ -62,10 +61,6 @@ public class SuggestReviewers {
return suggestAccounts;
}
public int getSuggestFrom() {
return suggestFrom;
}
public int getLimit() {
return limit;
}
@@ -98,7 +93,6 @@ public class SuggestReviewers {
this.suggestAccounts = (av != AccountVisibility.NONE);
}
this.suggestFrom = cfg.getInt("suggest", null, "from", 0);
this.maxAllowed = cfg.getInt("addreviewer", "maxAllowed",
PostReviewers.DEFAULT_MAX_REVIEWERS);
this.maxAllowedWithoutConfirmation = cfg.getInt(

View File

@@ -0,0 +1,22 @@
// 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.reviewdb.client.Account;
public class SuggestedReviewer {
public Account.Id account;
public double score;
}

View File

@@ -100,6 +100,7 @@ import com.google.gerrit.server.change.AccountPatchReviewStore;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
import com.google.gerrit.server.change.MergeabilityCacheImpl;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.events.EventsMetrics;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -352,6 +353,7 @@ public class GerritGlobalModule extends FactoryModule {
DynamicMap.mapOf(binder(), DownloadScheme.class);
DynamicMap.mapOf(binder(), DownloadCommand.class);
DynamicMap.mapOf(binder(), CloneCommand.class);
DynamicMap.mapOf(binder(), ReviewerSuggestion.class);
DynamicSet.setOf(binder(), ExternalIncludedIn.class);
DynamicMap.mapOf(binder(), ProjectConfigEntry.class);
DynamicSet.setOf(binder(), PatchSetWebLink.class);