Skip to content

Commit

Permalink
Googlified the code
Browse files Browse the repository at this point in the history
Change-Id: I2fdf025704273036a4f36f60bdd2f21042c49bb5
  • Loading branch information
nllptr committed Jan 13, 2015
1 parent 8b38afe commit 659670f
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ public interface AdviceCache {

/**
* Caches the provided calculation.
*
* @param calculation the calculation object to be cached
*/
public void storeCalculation(Calculation calculation);

/**
* Returns the calculation object with the matching RevisionResource.
*
* @param resource the RevisionResource to fetch calculation from cache
* @return a Calculation object if one is found, null otherwise
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,16 @@ public class AdviceCacheImpl implements AdviceCache {
private GerritApi gApi;
private PluginConfigFactory cfg;

@Inject
AdviceCacheImpl(@PluginData File dir, GerritApi gApi, PluginConfigFactory cfg) {
@Inject AdviceCacheImpl(@PluginData File dir, GerritApi gApi, PluginConfigFactory cfg) {
this.dir = dir;
this.gApi = gApi;
this.cfg = cfg;
}

@Override
public void storeCalculation(Calculation calculation) {
File file = new File(dir, calculation.commitId.substring(0, 2)
+ File.separator + calculation.commitId.substring(2));
@Override public void storeCalculation(Calculation calculation) {
File file = new File(dir,
calculation.commitId.substring(0, 2) + File.separator + calculation.commitId
.substring(2));
log.debug("Writing calculation to {}", file);
file.getParentFile().mkdirs();
try (BufferedWriter writer = Files
Expand All @@ -62,10 +61,10 @@ public void storeCalculation(Calculation calculation) {
}
}

@Override
public Calculation fetchCalculation(RevisionResource resource) {
File file = new File(dir, resource.getPatchSet().getRevision().get().substring(0, 2)
+ File.separator+ resource.getPatchSet().getRevision().get().substring(2));
@Override public Calculation fetchCalculation(RevisionResource resource) {
File file = new File(dir,
resource.getPatchSet().getRevision().get().substring(0, 2) + File.separator + resource
.getPatchSet().getRevision().get().substring(2));
Calculation calculation = null;
log.debug("Loading calculation from {}", file);
try (BufferedReader reader = Files
Expand All @@ -85,8 +84,9 @@ public Calculation fetchCalculation(RevisionResource resource) {
try {
ChangeApi cApi = gApi.changes().id(resource.getChange().getChangeId());
ChangeInfo info = cApi.get();
double reviewTimeModifier = cfg.getProjectPluginConfigWithInheritance(resource.getChange()
.getProject(), "reviewassistant").getInt("time", "reviewTimeModifier", 100);
double reviewTimeModifier =
cfg.getProjectPluginConfigWithInheritance(resource.getChange().getProject(),
"reviewassistant").getInt("time", "reviewTimeModifier", 100);
calculation = ReviewAssistant.calculate(info, reviewTimeModifier / 100);
storeCalculation(calculation);
} catch (RestApiException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,20 @@ class ChangeEventListener implements ChangeListener {

private static final Logger log = LoggerFactory.getLogger(ChangeEventListener.class);
private final ReviewAssistant.Factory reviewAssistantFactory;
private WorkQueue workQueue;
private GitRepositoryManager repoManager;
private SchemaFactory<ReviewDb> schemaFactory;
private final ThreadLocalRequestContext tl;
private ReviewDb db;
private final PluginUser pluginUser;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final PluginConfigFactory cfg;
private WorkQueue workQueue;
private GitRepositoryManager repoManager;
private SchemaFactory<ReviewDb> schemaFactory;
private ReviewDb db;

@Inject ChangeEventListener(final ReviewAssistant.Factory reviewAssistantFactory,
final WorkQueue workQueue, final GitRepositoryManager repoManager,
final SchemaFactory<ReviewDb> schemaFactory,
final ThreadLocalRequestContext tl, final PluginUser pluginUser,
final IdentifiedUser.GenericFactory identifiedUserFactory, final PluginConfigFactory cfg) {
final SchemaFactory<ReviewDb> schemaFactory, final ThreadLocalRequestContext tl,
final PluginUser pluginUser, final IdentifiedUser.GenericFactory identifiedUserFactory,
final PluginConfigFactory cfg) {
this.workQueue = workQueue;
this.reviewAssistantFactory = reviewAssistantFactory;
this.repoManager = repoManager;
Expand All @@ -61,8 +61,7 @@ class ChangeEventListener implements ChangeListener {
this.cfg = cfg;
}

@Override
public void onChangeEvent(ChangeEvent changeEvent) {
@Override public void onChangeEvent(ChangeEvent changeEvent) {
if (!(changeEvent instanceof PatchSetCreatedEvent)) {
return;
}
Expand Down Expand Up @@ -118,24 +117,20 @@ public void onChangeEvent(ChangeEvent changeEvent) {
final Runnable task =
reviewAssistantFactory.create(commit, change, ps, repo, projectName);
workQueue.getDefaultQueue().submit(new Runnable() {
@Override
public void run() {
@Override public void run() {
RequestContext old = tl.setContext(new RequestContext() {

@Override
public CurrentUser getCurrentUser() {
@Override public CurrentUser getCurrentUser() {
if (!ReviewAssistant.realUser) {
return pluginUser;
} else {
return identifiedUserFactory.create(change.getOwner());
}
}

@Override
public Provider<ReviewDb> getReviewDbProvider() {
@Override public Provider<ReviewDb> getReviewDbProvider() {
return new Provider<ReviewDb>() {
@Override
public ReviewDb get() {
@Override public ReviewDb get() {
if (db == null) {
try {
db = schemaFactory.open();
Expand All @@ -161,7 +156,8 @@ public ReviewDb get() {
}
});
} catch (IOException e) {
log.error("Could not get commit for revision {}: {}", event.patchSet.revision, e);
log.error("Could not get commit for revision {}: {}", event.patchSet.revision,
e);
} finally {
reviewDb.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import com.google.gerrit.extensions.webui.WebUiPlugin;
import com.google.gerrit.httpd.plugins.HttpPluginModule;

public class HttpModule extends HttpPluginModule{
@Override
protected void configureServlets() {
DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new JavaScriptPlugin("reviewassistant.js"));
public class HttpModule extends HttpPluginModule {
@Override protected void configureServlets() {
DynamicSet.bind(binder(), WebUiPlugin.class)
.toInstance(new JavaScriptPlugin("reviewassistant.js"));
}
}
10 changes: 4 additions & 6 deletions src/main/java/com/github/nexception/reviewassistant/Module.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package com.github.nexception.reviewassistant;

import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;

import com.github.nexception.reviewassistant.rest.GetAdvice;
import com.google.gerrit.common.ChangeListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.server.config.FactoryModule;

import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;

public class Module extends FactoryModule {

@Override
protected void configure() {
@Override protected void configure() {
DynamicSet.bind(binder(), ChangeListener.class).to(ChangeEventListener.class);
bind(AdviceCache.class).to(AdviceCacheImpl.class);
factory(ReviewAssistant.Factory.class);

install(new RestApiModule() {
@Override
protected void configure() {
@Override protected void configure() {
get(REVISION_KIND, "advice").to(GetAdvice.class);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ enum AddReason {PLUS_TWO, EXPERIENCE}
*/
public class ReviewAssistant implements Runnable {

private static final Logger log = LoggerFactory.getLogger(ReviewAssistant.class);
public static boolean realUser;
private final AccountByEmailCache emailCache;
private final AccountCache accountCache;
Expand All @@ -60,7 +61,6 @@ public class ReviewAssistant implements Runnable {
private final PatchSet ps;
private final Repository repo;
private final RevCommit commit;
private static final Logger log = LoggerFactory.getLogger(ReviewAssistant.class);
private final Project.NameKey projectName;
private final GerritApi gApi;
private final int maxReviewers;
Expand All @@ -69,16 +69,14 @@ public class ReviewAssistant implements Runnable {
private final int plusTwoLimit;
private final boolean plusTwoRequired;


public interface Factory {
ReviewAssistant create(RevCommit commit, Change change, PatchSet ps, Repository repo,
Project.NameKey projectName);
}

@Inject
public ReviewAssistant(final PatchListCache patchListCache, final AccountCache accountCache,
final GerritApi gApi, final AccountByEmailCache emailCache,
final PluginConfigFactory cfg,
final GerritApi gApi, final AccountByEmailCache emailCache, final PluginConfigFactory cfg,
@Assisted final RevCommit commit, @Assisted final Change change,
@Assisted final PatchSet ps, @Assisted final Repository repo,
@Assisted final Project.NameKey projectName) {
Expand Down Expand Up @@ -190,14 +188,12 @@ private static int calculateReviewSessions(int minutes) {
private List<Entry<Account, Integer>> getApprovalAccounts() {
Map<Account, Integer> reviewersApproved = new HashMap<>();
try {
List<ChangeInfo> infoList =
gApi.changes().query(
"status:merged -age:" + plusTwoAge + "weeks limit:" + plusTwoLimit
+ " -label:Code-Review=2," + change.getOwner().get()
+ " label:Code-Review=2 project:" +
projectName.toString())
.withOptions(ListChangesOption.LABELS, ListChangesOption.DETAILED_ACCOUNTS)
.get();
List<ChangeInfo> infoList = gApi.changes().query(
"status:merged -age:" + plusTwoAge + "weeks limit:" + plusTwoLimit
+ " -label:Code-Review=2," + change.getOwner().get()
+ " label:Code-Review=2 project:" +
projectName.toString())
.withOptions(ListChangesOption.LABELS, ListChangesOption.DETAILED_ACCOUNTS).get();
for (ChangeInfo info : infoList) {
//TODO Check if this is good enough
try {
Expand Down Expand Up @@ -294,8 +290,7 @@ private List<Entry<Account, Integer>> getReviewers(List<Edit> edits, BlameResult

List<Entry<Account, Integer>> topReviewers =
Ordering.from(new Comparator<Entry<Account, Integer>>() {
@Override
public int compare(Entry<Account, Integer> itemOne,
@Override public int compare(Entry<Account, Integer> itemOne,
Entry<Account, Integer> itemTwo) {
return itemOne.getValue() - itemTwo.getValue();
}
Expand Down Expand Up @@ -364,8 +359,7 @@ public int compare(Entry<Account, Integer> o1, Entry<Account, Integer> o2) {
return modifiableList;
}

@Override
public void run() {
@Override public void run() {
log.info(
"CONFIG: maxReviewers: " + maxReviewers + ", enableLoadBalancing: " + loadBalancing +
", plusTwoAge: " + plusTwoAge + ", plusTwoLimit: " + plusTwoLimit
Expand Down Expand Up @@ -417,16 +411,16 @@ public void run() {

for (Entry<Account, Integer> e : mergeCandidates) {
log.debug("Merge candidate: {}, score: {}", e.getKey().getPreferredEmail(),
e.getValue());
e.getValue());
}

for (Entry<Account, Integer> e : blameCandidates) {
log.debug("Blame candidate: {}, score: {}", e.getKey().getPreferredEmail(),
e.getValue());
e.getValue());
}

Map<Account, AddReason> finalMap = new HashMap<>();
if(blameCandidates.size() < maxReviewers) {
if (blameCandidates.size() < maxReviewers) {
Iterator<Entry<Account, Integer>> mergeItr = mergeCandidates.iterator();
for (Entry<Account, Integer> e : blameCandidates) {
finalMap.put(e.getKey(), AddReason.EXPERIENCE);
Expand All @@ -444,13 +438,14 @@ public void run() {
if (!plusTwoAdded && plusTwoRequired) {
finalMap.put(mergeCandidates.get(0).getKey(), AddReason.PLUS_TWO);
log.debug("Changed reason for {} to {}",
mergeCandidates.get(0).getKey().getPreferredEmail(), AddReason.PLUS_TWO);
mergeCandidates.get(0).getKey().getPreferredEmail(), AddReason.PLUS_TWO);
}
} else {
Iterator<Entry<Account, Integer>> blameItr = blameCandidates.iterator();
if (!mergeCandidates.isEmpty()) {
finalMap.put(mergeCandidates.get(0).getKey(), AddReason.PLUS_TWO);
log.debug("Added {} ({})", mergeCandidates.get(0).getKey().getPreferredEmail(), AddReason.PLUS_TWO);
log.debug("Added {} ({})", mergeCandidates.get(0).getKey().getPreferredEmail(),
AddReason.PLUS_TWO);
}
while (finalMap.size() < maxReviewers && blameItr.hasNext()) {
Account account = blameItr.next().getKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,15 @@
* This rest view fetches a calculation and returns it. It is used by the front-end to
* present the review suggestions to the users.
*/
@Singleton
public class GetAdvice implements RestReadView<RevisionResource> {
@Singleton public class GetAdvice implements RestReadView<RevisionResource> {

private AdviceCache adviceCache;

@Inject
public GetAdvice(AdviceCache adviceCache) {
@Inject public GetAdvice(AdviceCache adviceCache) {
this.adviceCache = adviceCache;
}

@Override
public Object apply(RevisionResource resource)
@Override public Object apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceConflictException {
Calculation calculation = adviceCache.fetchCalculation(resource);
String advice =
Expand Down

0 comments on commit 659670f

Please sign in to comment.