[JENKINS-29603] Fix notifyCommit for branches that contain slashes

src/main/java/hudson/plugins/git/BranchSpec.java

@@ -4,17 +4,22 @@
 import hudson.Extension;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.StringTokenizer;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+
 /**
  * A specification of branches to build. Rather like a refspec.
  *
@@ -80,6 +85,21 @@ public boolean matches(String ref, EnvVars env) {
         return getPattern(env).matcher(ref).matches();
     }
 
+    /**
+     * Compare the configured pattern to a git branch defined by the repository name and branch name.
+     * @param repositoryName git repository name
+     * @param branchName git branch name
+     * @return true if repositoryName/branchName matches this BranchSpec
+     */
+    public boolean matchesRepositoryBranch(String repositoryName, String branchName) {
+        if (branchName == null) {
+            return false;
+        }
+        Pattern pattern = getPattern(new EnvVars(), repositoryName);
+        String branchWithoutRefs = cutRefs(branchName);
+        return pattern.matcher(branchWithoutRefs).matches() || pattern.matcher(join(repositoryName, branchWithoutRefs)).matches();
+    }
+
     /**
      * @deprecated use {@link #filterMatching(Collection, EnvVars)}
      * @param branches source branch list to be filtered by configured branch specification using a newly constructed EnvVars
@@ -124,14 +144,26 @@ private String getExpandedName(EnvVars env) {
         }
         return expandedName;
     }
-    
+
     private Pattern getPattern(EnvVars env) {
+        return getPattern(env, null);
+    }
+
+    private Pattern getPattern(EnvVars env, String repositoryName) {
         String expandedName = getExpandedName(env);
         // use regex syntax directly if name starts with colon
         if (expandedName.startsWith(":") && expandedName.length() > 1) {
             String regexSubstring = expandedName.substring(1, expandedName.length());
             return Pattern.compile(regexSubstring);
         }
+        if (repositoryName != null) {
+            // remove the "refs/.../" stuff from the branch-spec if necessary
+            String pattern = cutRefs(expandedName)
+                    // remove a leading "remotes/" from the branch spec
+                    .replaceAll("^remotes/", "");
+            pattern = convertWildcardStringToRegex(pattern);
+            return Pattern.compile(pattern);
+        }
 
         // build a pattern into this builder
         StringBuilder builder = new StringBuilder();
@@ -148,7 +180,13 @@ private Pattern getPattern(EnvVars env) {
             builder.append("|refs/remotes/|remotes/");
         }
         builder.append(")?");
-
+        builder.append(convertWildcardStringToRegex(expandedName));
+        return Pattern.compile(builder.toString());
+    }
+
+    private String convertWildcardStringToRegex(String expandedName) {
+        StringBuilder builder = new StringBuilder();
+
         // was the last token a wildcard?
         boolean foundWildcard = false;
 
@@ -188,8 +226,16 @@ private Pattern getPattern(EnvVars env) {
         if (foundWildcard) {
             builder.append("[^/]*");
         }
-
-        return Pattern.compile(builder.toString());
+        return builder.toString();
+    }
+
+    private String cutRefs(@NonNull String name) {
+        Matcher matcher = GitSCM.GIT_REF.matcher(name);
+        return matcher.matches() ? matcher.group(2) : name;
+    }
+
+    private String join(String repositoryName, String branchWithoutRefs) {
+        return StringUtils.join(Arrays.asList(repositoryName, branchWithoutRefs), "/");
     }
 
     @Extension

src/main/java/hudson/plugins/git/GitSCM.java

@@ -660,7 +660,7 @@ public PollingResult compareRemoteRevisionWith(Job<?, ?> project, Launcher launc
         }
     }
 
-    public static final Pattern GIT_REF = Pattern.compile("(refs/[^/]+)/.*");
+    public static final Pattern GIT_REF = Pattern.compile("(refs/[^/]+)/(.*)");
 
     private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher launcher, FilePath workspace, final @NonNull TaskListener listener) throws IOException, InterruptedException {
         // Poll for changes. Are there any unbuilt revisions that Hudson ought to build ?

src/main/java/hudson/plugins/git/GitStatus.java

@@ -401,7 +401,7 @@ public List<ResponseContributor> onNotifyCommit(String origin, URIish uri, Strin
                                         parametrizedBranchSpec = true;
                                     } else {
                                         for (String branch : branches) {
-                                            if (branchSpec.matches(repository.getName() + "/" + branch)) {
+                                            if (branchSpec.matchesRepositoryBranch(repository.getName(), branch)) {
                                                 if (LOGGER.isLoggable(Level.FINE)) {
                                                     LOGGER.log(Level.FINE, "Branch Spec {0} matches modified branch {1} for {2}", new Object[]{branchSpec, branch, project.getFullDisplayName()});
                                                 }

src/test/java/hudson/plugins/git/GitStatusTest.java

@@ -1,8 +1,11 @@
 package hudson.plugins.git;
 
+import hudson.model.Action;
 import hudson.model.Cause;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
+import hudson.model.ParameterValue;
+import hudson.model.ParametersAction;
 import hudson.model.ParametersDefinitionProperty;
 import hudson.model.StringParameterDefinition;
 import hudson.plugins.git.extensions.GitSCMExtension;
@@ -14,7 +17,6 @@
 import java.io.PrintWriter;
 import java.net.URISyntaxException;
 import java.util.*;
-
 import org.eclipse.jgit.transport.URIish;
 import org.mockito.Mockito;
 import static org.mockito.Mockito.mock;
@@ -25,13 +27,19 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.FromDataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.experimental.theories.Theory;
+import org.junit.runner.RunWith;
 import org.jvnet.hudson.test.WithoutJenkins;
 
 import javax.servlet.http.HttpServletRequest;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
+@RunWith(Theories.class)
 public class GitStatusTest extends AbstractGitProject {
 
     private GitStatus gitStatus;
@@ -283,6 +291,68 @@ private void doNotifyCommitWithTwoBranchesAndAdditionalParameter(final boolean a
         assertEquals(expected, this.gitStatus.toString());
     }
 
+    @DataPoints("branchSpecPrefixes")
+    public static final String[] BRANCH_SPEC_PREFIXES = new String[] {
+            "",
+            "refs/remotes/",
+            "refs/heads/",
+            "origin/",
+            "remotes/origin/"
+    };
+
+    @Theory
+    public void testDoNotifyCommitBranchWithSlash(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception {
+        SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "feature/awesome-feature", false);
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null);
+
+        Mockito.verify(trigger).run();
+    }
+
+    @Theory
+    public void testDoNotifyCommitBranchWithoutSlash(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception {
+        SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "awesome-feature", false);
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "awesome-feature", null);
+
+        Mockito.verify(trigger).run();
+    }
+
+    @Theory
+    public void testDoNotifyCommitBranchByBranchRef(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception {
+        SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "awesome-feature", false);
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "refs/heads/awesome-feature", null);
+
+        Mockito.verify(trigger).run();
+    }
+
+    @Test
+    public void testDoNotifyCommitBranchWithRegex() throws Exception {
+        SCMTrigger trigger = setupProjectWithTrigger("remote", ":[^/]*/awesome-feature", false);
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null);
+
+        Mockito.verify(trigger).run();
+    }
+
+    @Test
+    public void testDoNotifyCommitBranchWithWildcard() throws Exception {
+        SCMTrigger trigger = setupProjectWithTrigger("remote", "origin/feature/*", false);
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null);
+
+        Mockito.verify(trigger).run();
+    }
+
+    private void assertAdditionalParameters(Collection<? extends Action> actions) {
+        for (Action action: actions) {
+            if (action instanceof ParametersAction) {
+                final List<ParameterValue> parameters = ((ParametersAction) action).getParameters();
+                assertEquals(2, parameters.size());
+                for (ParameterValue value : parameters) {
+                    assertTrue((value.getName().equals("paramKey1") && value.getValue().equals("paramValue1"))
+                            || (value.getName().equals("paramKey2") && value.getValue().equals("paramValue2")));
+                }
+            }
+        }
+    }
+
     private SCMTrigger setupProjectWithTrigger(String url, String branchString, boolean ignoreNotifyCommit) throws Exception {
         SCMTrigger trigger = Mockito.mock(SCMTrigger.class);
         Mockito.doReturn(ignoreNotifyCommit).when(trigger).isIgnorePostCommitHooks();