Fix closest match for branches in settings, to match only branches
with common root.
Example
stable/* vs stable/1707 = Valid
stable/1704 vs stable/1707 = Valid
master vs stable/1707 = Invalid
master vs stable/* = Invalid
Change-Id: I82775d90379ed28e579d59b04d08f51e707ecf52
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>
diff --git a/pom.xml b/pom.xml
index edd010e..34afdc3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -73,5 +73,12 @@
<version>${Gerrit-ApiVersion}</version>
<scope>test</scope>
</dependency>
+ <!-- https://mvnrepository.com/artifact/org.mockito/mockito-all -->
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-all</artifactId>
+ <version>1.10.19</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
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 8dc9ca0..7c69e0a 100644
--- a/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java
+++ b/src/main/java/io/fd/maintainer/plugin/service/SettingsProvider.java
@@ -16,8 +16,7 @@
package io.fd.maintainer.plugin.service;
-import static java.lang.String.format;
-
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -26,13 +25,17 @@
import com.google.inject.Singleton;
import io.fd.maintainer.plugin.service.dto.PluginBranchSpecificSettings;
import io.fd.maintainer.plugin.util.ClosestMatch;
-import java.util.Optional;
-import java.util.function.Function;
-import javax.annotation.Nonnull;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.annotation.Nonnull;
+import java.util.Optional;
+import java.util.function.Function;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static java.lang.String.format;
+
@Singleton
public final class SettingsProvider implements ClosestMatch {
@@ -42,6 +45,7 @@
private static final String BRANCH_SECTION = "branch";
private static final String PLUGIN_USER = "pluginuser";
+ private static final String DEFAULT_PLUGIN_USER = "non-existing-user";
private static final String MAINTAINERS_FILE_PATH_REF = "maintainerfileref";
private static final String DEFAULT_MAINTAINERS_FILE_PATH_REF = "master/HEAD";
@@ -64,6 +68,11 @@
@Inject
private PluginConfigFactory cfg;
+ @VisibleForTesting
+ SettingsProvider(PluginConfigFactory cfg) {
+ this.cfg = cfg;
+ }
+
public PluginBranchSpecificSettings getBranchSpecificSettings(@Nonnull final String branchName,
@Nonnull final Project.NameKey projectKey) {
@@ -72,7 +81,24 @@
: RefNames.REFS_HEADS.concat(branchName);
LOG.info("Reading configuration for branch {}", fullBranchName);
- return getSettingsForBranch(fullBranchName, closesBranchMatch(fullBranchName, projectKey), projectKey);
+ final Optional<String> closestBranch = closesBranchMatch(fullBranchName, projectKey);
+
+ if (closestBranch.isPresent()) {
+ // either current branch or some similar has config
+ return getSettingsForBranch(fullBranchName, closestBranch.get(), projectKey);
+ }
+
+ //not current nor similar branch has config, therefore return default
+ return new PluginBranchSpecificSettings.PluginSettingsBuilder()
+ .setAllowMaintainersSubmit(DEFAULT_ALLOW_SUBMIT)
+ .setAutoAddReviewers(DEFAULT_AUTO_ADD_REVIEWERS)
+ .setAutoSubmit(DEFAULT_AUTO_SUBMIT)
+ .setDislikeWarnings(DEFAULT_DISLIKE_WARNINGS)
+ .setBranch(fullBranchName)
+ .setFileRef(DEFAULT_MAINTAINERS_FILE_REF)
+ .setLocalFilePath(DEFAULT_MAINTAINERS_FILE_PATH_REF)
+ .setPluginUserName(DEFAULT_PLUGIN_USER)
+ .createPluginSettings();
}
private PluginBranchSpecificSettings getSettingsForBranch(final String branchName, final String closestBranch,
@@ -162,10 +188,60 @@
}
// match by the number of changes needed to change one String into another
- private String closesBranchMatch(final String branchName, final Project.NameKey projectKey) {
+ private Optional<String> closesBranchMatch(final String branchName, final Project.NameKey projectKey) {
+ final BranchInfo currentBranchInfo = new BranchInfo(branchName);
return projectSpecificPluginConfig(projectKey).getSubsections(BRANCH_SECTION).stream()
- .reduce((branchOne, branchTwo) -> closestMatch(branchName, branchOne, branchTwo))
- // if non use default
- .orElse(RefNames.REFS_HEADS);
+ .map(BranchInfo::new)
+ .filter(branchInfo -> branchInfo.isAlternativeFor(currentBranchInfo))
+ .map(BranchInfo::getBranchPart)
+ .reduce((branchOne, branchTwo) -> closestMatch(branchName, branchOne, branchTwo));
+ }
+
+ static class BranchInfo {
+
+ private final boolean isWildcarded;
+ private final boolean isGerritReviewBranch;
+ private final String fullBranchName;
+ private final String branchPart;
+
+ BranchInfo(@Nonnull final String input) {
+ checkNotNull(input, "Input for %s cannot be null", this.getClass().getName());
+ final String[] parts = input.split("\\/");
+ isWildcarded = parts[parts.length - 1].trim().equals("*");
+ isGerritReviewBranch = input.trim().startsWith(RefNames.REFS_HEADS);
+ fullBranchName = input.trim();
+ if (isGerritReviewBranch) {
+ branchPart = fullBranchName.replace(RefNames.REFS_HEADS, "").replace("/*", "");
+ } else {
+ branchPart = fullBranchName;
+ }
+ }
+
+ public boolean isAlternativeFor(final BranchInfo other) {
+ if (this.isGerritReviewBranch && other.isGerritReviewBranch) {
+ // both branches are standard review branches like /refs/heads/master for ex.
+ final String[] thisBranchParts = this.branchPart.split("\\/");
+ final String[] otherBranchParts = other.branchPart.split("\\/");
+
+ return thisBranchParts[0].equals(otherBranchParts[0]);
+ }
+ return fullBranchName.equals(other.fullBranchName);
+ }
+
+ public boolean isWildcarded() {
+ return isWildcarded;
+ }
+
+ public boolean isGerritReviewBranch() {
+ return isGerritReviewBranch;
+ }
+
+ public String getFullBranchName() {
+ return fullBranchName;
+ }
+
+ public String getBranchPart() {
+ return branchPart;
+ }
}
}
diff --git a/src/test/java/io/fd/maintainer/plugin/service/BranchInfoTest.java b/src/test/java/io/fd/maintainer/plugin/service/BranchInfoTest.java
new file mode 100644
index 0000000..68e1666
--- /dev/null
+++ b/src/test/java/io/fd/maintainer/plugin/service/BranchInfoTest.java
@@ -0,0 +1,79 @@
+package io.fd.maintainer.plugin.service;
+
+import io.fd.maintainer.plugin.service.SettingsProvider.BranchInfo;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class BranchInfoTest {
+
+ private BranchInfo stableWildcardedInfo;
+ private BranchInfo masterInfo;
+ private BranchInfo stableInfo;
+ private BranchInfo nonReviewBranch;
+
+ @Before
+ public void init() {
+ this.masterInfo = new BranchInfo("refs/heads/master");
+ this.stableInfo = new BranchInfo("refs/heads/stable/1707");
+ this.stableWildcardedInfo = new BranchInfo("refs/heads/stable/*");
+ this.nonReviewBranch = new BranchInfo("non/review/branch");
+ }
+
+ @Test
+ public void testNonReview() {
+ assertEquals("non/review/branch", nonReviewBranch.getBranchPart());
+ assertEquals("non/review/branch", nonReviewBranch.getFullBranchName());
+ assertFalse(nonReviewBranch.isGerritReviewBranch());
+ assertFalse(nonReviewBranch.isWildcarded());
+ }
+
+ @Test
+ public void testMasterProcessing() {
+ assertEquals("master", masterInfo.getBranchPart());
+ assertEquals("refs/heads/master", masterInfo.getFullBranchName());
+ assertFalse(masterInfo.isWildcarded());
+ assertTrue(masterInfo.isGerritReviewBranch());
+ }
+
+ @Test
+ public void testStableProcessing() {
+ assertEquals("stable/1707", stableInfo.getBranchPart());
+ assertEquals("refs/heads/stable/1707", stableInfo.getFullBranchName());
+ assertFalse(stableInfo.isWildcarded());
+ assertTrue(stableInfo.isGerritReviewBranch());
+ }
+
+ @Test
+ public void testWildcaded() {
+ assertEquals("stable", stableWildcardedInfo.getBranchPart());
+ assertEquals("refs/heads/stable/*", stableWildcardedInfo.getFullBranchName());
+ assertTrue(stableWildcardedInfo.isWildcarded());
+ assertTrue(stableWildcardedInfo.isGerritReviewBranch());
+ }
+
+ @Test
+ public void testCompare() {
+ assertFalse(masterInfo.isAlternativeFor(stableInfo));
+ assertFalse(masterInfo.isAlternativeFor(stableWildcardedInfo));
+ assertTrue(masterInfo.isAlternativeFor(masterInfo));
+ assertFalse(masterInfo.isAlternativeFor(nonReviewBranch));
+
+ assertFalse(stableInfo.isAlternativeFor(masterInfo));
+ assertTrue(stableInfo.isAlternativeFor(stableWildcardedInfo));
+ assertTrue(stableInfo.isAlternativeFor(stableInfo));
+ assertFalse(stableInfo.isAlternativeFor(nonReviewBranch));
+
+ assertFalse(stableWildcardedInfo.isAlternativeFor(masterInfo));
+ assertTrue(stableWildcardedInfo.isAlternativeFor(stableInfo));
+ assertTrue(stableWildcardedInfo.isAlternativeFor(stableWildcardedInfo));
+ assertFalse(stableWildcardedInfo.isAlternativeFor(nonReviewBranch));
+
+ assertFalse(nonReviewBranch.isAlternativeFor(masterInfo));
+ assertFalse(nonReviewBranch.isAlternativeFor(stableInfo));
+ assertFalse(nonReviewBranch.isAlternativeFor(stableWildcardedInfo));
+ assertTrue(nonReviewBranch.isAlternativeFor(nonReviewBranch));
+ }
+
+}
\ No newline at end of file
diff --git a/src/test/java/io/fd/maintainer/plugin/service/SettingsProviderTest.java b/src/test/java/io/fd/maintainer/plugin/service/SettingsProviderTest.java
new file mode 100644
index 0000000..6acf11f
--- /dev/null
+++ b/src/test/java/io/fd/maintainer/plugin/service/SettingsProviderTest.java
@@ -0,0 +1,57 @@
+package io.fd.maintainer.plugin.service;
+
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import io.fd.maintainer.plugin.service.dto.PluginBranchSpecificSettings;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import static org.junit.Assert.assertFalse;
+import static org.mockito.Mockito.when;
+
+public class SettingsProviderTest {
+
+ @Mock
+ private PluginConfigFactory cfg;
+
+ private SettingsProvider provider;
+
+ @Before
+ public void setUp() throws Exception {
+ MockitoAnnotations.initMocks(this);
+ /*
+ * [branch "refs/heads/master"]
+ pluginuser = vppmaintainerplugin
+ maintainerfileref = master
+ maintainerfile = MAINTAINER
+ autoaddreviewers = true
+ allowmaintainersubmit = false
+ autosubmit = false
+ dislikewarnings = false
+ * */
+ Config config = new Config();
+ config.setString("branch", "refs/heads/master", "pluginuser", "vppmaintainerplugin");
+ config.setString("branch", "refs/heads/master", "maintainerfileref", "master");
+ config.setString("branch", "refs/heads/master", "maintainerfile", "MAINTAINER");
+ config.setString("branch", "refs/heads/master", "autoaddreviewers", "true");
+ config.setString("branch", "refs/heads/master", "allowmaintainersubmit", "false");
+ config.setString("branch", "refs/heads/master", "autosubmit", "false");
+ config.setString("branch", "refs/heads/master", "dislikewarnings", "false");
+
+ when(cfg.getProjectPluginConfig(new Project.NameKey("vpp"), "maintainer")).thenReturn(config);
+ provider = new SettingsProvider(cfg);
+ }
+
+ @Test
+ public void getBranchSpecificSettings() throws Exception {
+ PluginBranchSpecificSettings settings = provider.getBranchSpecificSettings("refs/for/stable/1707", new Project.NameKey("vpp"));
+ assertFalse(settings.isAutoAddReviewers());
+ assertFalse(settings.isAllowMaintainersSubmit());
+ assertFalse(settings.isAutoSubmit());
+ assertFalse(settings.isDislikeWarnings());
+ }
+
+}
\ No newline at end of file
