Remove unnecessary abstraction of AddReviewers
Abstract class AddReviewers had only one implementation,
AddReviewersByConfiguration. This implementation wasn't configuration
specific, and all this implemetation did was to provide a Factory for
and a Set<Account.Id> of reviewers to the abstract class.
Getting the Set of reviewers from an abstract method has no benefits
to having them provided in the Constructor.
Feature: Issue 12610
Change-Id: I61d24b622f948fa71f85d24b76c315ea5cf477e8
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
index 43266a2..7fe9920 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
@@ -25,30 +25,38 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
import java.util.ArrayList;
import java.util.Set;
-abstract class AddReviewers implements Runnable {
+class AddReviewers implements Runnable {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ThreadLocalRequestContext tl;
- protected final GerritApi gApi;
- protected final IdentifiedUser.GenericFactory identifiedUserFactory;
- protected final ChangeInfo changeInfo;
+ private final GerritApi gApi;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
+ private final ChangeInfo changeInfo;
+ private final Set<Account.Id> reviewers;
+ interface Factory {
+ AddReviewers create(ChangeInfo changeInfo, Set<Account.Id> reviewers);
+ }
+
+ @Inject
AddReviewers(
ThreadLocalRequestContext tl,
GerritApi gApi,
IdentifiedUser.GenericFactory identifiedUserFactory,
- ChangeInfo changeInfo) {
+ @Assisted ChangeInfo changeInfo,
+ @Assisted Set<Account.Id> reviewers) {
this.tl = tl;
this.gApi = gApi;
this.identifiedUserFactory = identifiedUserFactory;
this.changeInfo = changeInfo;
+ this.reviewers = reviewers;
}
- abstract Set<Account.Id> getReviewers();
-
@Override
public void run() {
RequestContext old =
@@ -61,13 +69,13 @@
}
});
try {
- addReviewers(getReviewers(), changeInfo);
+ addReviewers();
} finally {
tl.setContext(old);
}
}
- private void addReviewers(Set<Account.Id> reviewers, ChangeInfo changeInfo) {
+ private void addReviewers() {
try {
// TODO(davido): Switch back to using changes API again,
// when it supports batch mode for adding reviewers
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java
deleted file mode 100644
index 788629c..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java
+++ /dev/null
@@ -1,48 +0,0 @@
-// Copyright (C) 2013 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.googlesource.gerrit.plugins.reviewers;
-
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.extensions.api.GerritApi;
-import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-import java.util.Set;
-
-class AddReviewersByConfiguration extends AddReviewers {
- private final Set<Account.Id> reviewers;
-
- interface Factory {
- AddReviewersByConfiguration create(ChangeInfo changeInfo, Set<Account.Id> reviewers);
- }
-
- @Inject
- AddReviewersByConfiguration(
- ThreadLocalRequestContext tl,
- GerritApi gApi,
- IdentifiedUser.GenericFactory identifiedUserFactory,
- @Assisted ChangeInfo changeInfo,
- @Assisted Set<Account.Id> reviewers) {
- super(tl, gApi, identifiedUserFactory, changeInfo);
- this.reviewers = reviewers;
- }
-
- @Override
- Set<Account.Id> getReviewers() {
- return reviewers;
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
index efc3614..5f5b8ae 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -67,7 +67,7 @@
DynamicSet.bind(binder(), PrivateStateChangedListener.class).to(Reviewers.class);
}
- factory(AddReviewersByConfiguration.Factory.class);
+ factory(AddReviewers.Factory.class);
if (enableREST) {
install(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index fcf4790..3873eb1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -55,7 +55,7 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ReviewersResolver resolver;
- private final AddReviewersByConfiguration.Factory byConfigFactory;
+ private final AddReviewers.Factory addReviewersFactory;
private final WorkQueue workQueue;
private final ReviewersConfig config;
private final Provider<CurrentUser> user;
@@ -65,14 +65,14 @@
@Inject
Reviewers(
ReviewersResolver resolver,
- AddReviewersByConfiguration.Factory byConfigFactory,
+ AddReviewers.Factory addReviewersFactory,
WorkQueue workQueue,
ReviewersConfig config,
Provider<CurrentUser> user,
ChangeQueryBuilder queryBuilder,
Provider<InternalChangeQuery> queryProvider) {
this.resolver = resolver;
- this.byConfigFactory = byConfigFactory;
+ this.addReviewersFactory = addReviewersFactory;
this.workQueue = workQueue;
this.config = config;
this.user = user;
@@ -155,12 +155,12 @@
if (reviewers.isEmpty()) {
return;
}
- final Runnable task =
- byConfigFactory.create(
+ final AddReviewers addReviewers =
+ addReviewersFactory.create(
c, resolver.resolve(reviewers, projectName, changeNumber, uploader));
@SuppressWarnings("unused")
- Future<?> ignored = workQueue.getDefaultQueue().submit(task);
+ Future<?> ignored = workQueue.getDefaultQueue().submit(addReviewers);
} catch (QueryParseException e) {
logger.atWarning().log(
"Could not add default reviewers for change %d of project %s, filter is invalid: %s",
