Fix injection problems for Settings provider
Added tests from injection
Change-Id: I4be1ef0ca8557688bfc1e925110b2d493dc07077
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>
diff --git a/pom.xml b/pom.xml
index a1af88d..f0c1710 100644
--- a/pom.xml
+++ b/pom.xml
@@ -6,7 +6,7 @@
<groupId>io.fd.gerrit</groupId>
<artifactId>maintainer-plugin</artifactId>
- <version>2.13-SR3</version>
+ <version>2.13-SR4-SNAPSHOT</version>
<packaging>jar</packaging>
<properties>
@@ -80,5 +80,11 @@
<version>1.10.19</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.google.inject.extensions</groupId>
+ <artifactId>guice-testlib</artifactId>
+ <version>4.1.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
diff --git a/src/main/java/io/fd/maintainer/plugin/service/MaintainersProvider.java b/src/main/java/io/fd/maintainer/plugin/service/MaintainersProvider.java
index ca6d23c..b402bcd 100644
--- a/src/main/java/io/fd/maintainer/plugin/service/MaintainersProvider.java
+++ b/src/main/java/io/fd/maintainer/plugin/service/MaintainersProvider.java
@@ -16,9 +16,6 @@
package io.fd.maintainer.plugin.service;
-import static java.lang.String.format;
-import static java.util.Objects.nonNull;
-
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -33,11 +30,6 @@
import io.fd.maintainer.plugin.service.dto.PluginBranchSpecificSettings;
import io.fd.maintainer.plugin.util.ClosestMatch;
import io.fd.maintainer.plugin.util.PatchListProcessing;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.util.List;
-import java.util.Optional;
-import javax.annotation.Nonnull;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.Ref;
@@ -49,6 +41,16 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.annotation.Nonnull;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.lang.String.format;
+import static java.util.Objects.nonNull;
+
@Singleton
public class MaintainersProvider implements ClosestMatch, PatchListProcessing {
@@ -68,7 +70,6 @@
@Nonnull
public List<ComponentInfo> getMaintainersInfo(@Nonnull final String branchName, final int changeNumber,
@Nonnull final Project.NameKey projectKey) {
-
// get configuration for branch of change
final PluginBranchSpecificSettings settings =
settingsProvider.getBranchSpecificSettings(branchName, projectKey);
@@ -92,15 +93,14 @@
if (nonNull(maintainersFileContent)) {
return maintainersParser.parseMaintainers(maintainersFileContent);
} else {
+ LOG.info("Unable to find file {} in branch {}", settings.getLocalFilePath(), fullFileRef);
throw new IllegalStateException(
format("Unable to find file %s in branch %s", settings.getLocalFilePath(),
fullFileRef));
}
- } catch (IOException | MaintainerMismatchException e) {
- throw new IllegalStateException(e);
}
-
- } catch (OrmException e) {
+ } catch (OrmException | IOException | MaintainerMismatchException e) {
+ LOG.error("Error while searching maintainers file", e);
throw new IllegalStateException(e);
}
}
@@ -113,6 +113,7 @@
final RevCommit headCommit) {
LOG.info("Starting search at {}", headCommit);
+ checkState(headCommit.getParentCount() > 0, "Bottom of the branch has been reached and maintainers file hasnt been found");
final RevCommit parent = getRevCommit(revWalk, headCommit.getParent(0).getId());
LOG.info("Finding most recent maintainers file in {}", parent);
diff --git a/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java b/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java
index 7c69e0a..01034bc 100644
--- a/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java
+++ b/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java
@@ -16,7 +16,7 @@
package io.fd.maintainer.plugin.service;
-import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -37,7 +37,7 @@
import static java.lang.String.format;
@Singleton
-public final class SettingsProvider implements ClosestMatch {
+public class SettingsProvider implements ClosestMatch {
private static final Logger LOG = LoggerFactory.getLogger(SettingsProvider.class);
@@ -65,11 +65,11 @@
private static final String DISLIKE_WARNINGS = "dislikewarnings";
private static final boolean DEFAULT_DISLIKE_WARNINGS = false;
- @Inject
- private PluginConfigFactory cfg;
- @VisibleForTesting
- SettingsProvider(PluginConfigFactory cfg) {
+ private final PluginConfigFactory cfg;
+
+ @Inject
+ public SettingsProvider(PluginConfigFactory cfg) {
this.cfg = cfg;
}
@@ -156,12 +156,16 @@
final String alternativeBranch,
final Project.NameKey projectKey) {
final Config config = projectSpecificPluginConfig(projectKey);
- return Optional.ofNullable(config.getString(BRANCH_SECTION, branch, PLUGIN_USER))
- .orElse(Optional.ofNullable(config.getString(BRANCH_SECTION, alternativeBranch, PLUGIN_USER))
- .orElseThrow(() -> {
- LOG.error("Plugin user not specified for branch {}", branch);
- return new IllegalStateException(format("Plugin user not specified for branch %s", branch));
- }));
+
+ final String user = Optional.ofNullable(config.getString(BRANCH_SECTION, branch, PLUGIN_USER))
+ .orElse(config.getString(BRANCH_SECTION, alternativeBranch, PLUGIN_USER));
+
+ if (Strings.isNullOrEmpty(user)) {
+ LOG.error("Plugin user not specified for branch {}", branch);
+ throw new IllegalStateException(format("Plugin user not specified for branch %s", branch));
+ } else {
+ return user;
+ }
}
private <T> T getKey(final Project.NameKey projectKey,
diff --git a/src/main/java/io/fd/maintainer/plugin/service/push/ReviewerPusher.java b/src/main/java/io/fd/maintainer/plugin/service/push/ReviewerPusher.java
index 4354df3..00d958c 100644
--- a/src/main/java/io/fd/maintainer/plugin/service/push/ReviewerPusher.java
+++ b/src/main/java/io/fd/maintainer/plugin/service/push/ReviewerPusher.java
@@ -41,6 +41,7 @@
import io.fd.maintainer.plugin.util.WarningGenerator;
import java.util.Collection;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
@@ -93,6 +94,7 @@
.flatMap(Collection::stream)
.map(Maintainer::getName)
.map(accountIndex::get)
+ .filter(Objects::nonNull)
.collect(Collectors.toSet());
LOG.info("Adding reviewers for change {}", change.getId());
diff --git a/src/test/java/io/fd/maintainer/plugin/MaintainerPluginModuleTest.java b/src/test/java/io/fd/maintainer/plugin/MaintainerPluginModuleTest.java
new file mode 100644
index 0000000..222074c
--- /dev/null
+++ b/src/test/java/io/fd/maintainer/plugin/MaintainerPluginModuleTest.java
@@ -0,0 +1,79 @@
+package io.fd.maintainer.plugin;
+
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountByEmailCache;
+import com.google.gerrit.server.change.*;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Guice;
+import com.google.inject.testing.fieldbinder.Bind;
+import com.google.inject.testing.fieldbinder.BoundFieldModule;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+public class MaintainerPluginModuleTest {
+
+ @Mock
+ @Bind
+ private PluginConfigFactory pluginConfigFactory;
+
+ @Mock
+ @Bind
+ private GitRepositoryManager repoManager;
+
+ @Mock
+ @Bind
+ private SchemaFactory<ReviewDb> reviewDbSchemaFactory;
+
+ @Mock
+ @Bind
+ private PatchListCache patchListCache;
+
+ @Mock
+ @Bind
+ private ReviewDb reviewDb;
+
+ @Mock
+ @Bind
+ private CurrentUser currentUser;
+
+ @Mock
+ @Bind
+ private PostReview postReview;
+
+ @Mock
+ @Bind
+ private PostReviewers postReviewers;
+
+ @Mock
+ @Bind
+ private ChangesCollection changesCollection;
+
+ @Mock
+ @Bind
+ private Revisions revisions;
+
+ @Mock
+ @Bind
+ private Submit submitPusher;
+
+ @Mock
+ @Bind
+ private AccountByEmailCache accountByEmailCache;
+
+ @Before
+ public void init() {
+ MockitoAnnotations.initMocks(this);
+ }
+
+ @Test
+ public void configure() throws Exception {
+ Guice.createInjector(new MaintainerPluginModule(), BoundFieldModule.of(this)).injectMembers(this);
+ }
+
+}
\ No newline at end of file
