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