Branch expression matching before triggering the build

[grantsunny]

The current logic on triggering build from Bitbucket Server pushing is tricky, and I tried several times to make the wildcard branch spec working, but not very successful. After debugging the plugin I am not 100% sure, but it seems that logic under following has space to be improved.

  public boolean shouldTriggerBuild(BitBucketPPRAction bitbucketAction) {
    if (bitbucketAction.getType().equalsIgnoreCase("BRANCH")
        || bitbucketAction.getType().equalsIgnoreCase("UPDATE") || this.triggerAlsoIfTagPush) {

/** REVIEW - Grant: Forgive me if I am wrong. 
Are we matching the branch name with spec, or matching spec with branch name?  
May the BranchSpec object here  contain the regular expression and being enhanced 
by EnvVars later, but the current logic seems actually put the branch name which shall
 be matched by the expression to the BranchSpec? 

As a result, only a complete match will trigger the build. This happens ti work if 
we configure the spec as master though...  
*/

      return matches(new BranchSpec(bitbucketAction.getBranchName()), this.allowedBranches);
    }

    return false;
  }

  protected boolean matches(BranchSpec branchSpec, String allowedBranches) {
    LOGGER.info("Should trigger build?");
    if (this.allowedBranches.isEmpty()) {
      LOGGER.info("allowed branches are not set.");
      return true;
    }

    LOGGER.info("Allowed branches are set.");
    String[] branches = this.allowedBranches.split(",");
    Arrays.stream(branches).map(String::trim).toArray(unused -> branches);

    for (String branchPattern : branches) {
      if (branchSpec.matches(branchPattern)) {
        return true;
      }
    }

    return false;
  }

And I tried to update the code as follows, it seems works fine with my Jenkins. So if any guys could confirm my finding and update I am planning to create a PR for this. Thanks and could you please share your thinking?

	public boolean shouldTriggerBuild(BitBucketPPRAction bitbucketAction) {
		if (bitbucketAction.getType().equalsIgnoreCase("BRANCH") || bitbucketAction.getType().equalsIgnoreCase("UPDATE")
				|| this.triggerAlsoIfTagPush) {

			String branchName = bitbucketAction.getBranchName();
			LOGGER.info("The branchName in action is: " + branchName);
			
			String[] branchSpecs = this.allowedBranches.split(",");
			for (String branchSpec: branchSpecs) {
				LOGGER.info("Matching branch: " + branchName + " with branchSpec: " + branchSpec);
				if (new BranchSpec(branchSpec.trim()).matches(branchName)) 
					return true;
			}
		}
		return false;
	}

[macghriogair]

Hello @grantsunny ,
thank you for your review and comment! I think, indeed the pattern matching is done wrong way around here and was introduced with #18
Reading through the BranchSpec source, it should instead be instantiated with the parts of allowedBranches, as you propose. Feel free to provide a PR! :)

[cdelmonte-zg]

Hi @grantsunny
added in the release 1.6.3
Thanks!

[grantsunny]

Hi @cdelmonte-zg and @macghriogair

I were very happy for this change so I downloaded this upgrade but it seems my build is not being triggered. After checking the content of the commit I found this PR seems just for Bitbucket Cloud, but may forget to make changes for Bitbucket server which is supposed to be taken care of in BitBucketPPRServerRepositoryPushActionFilter?

Anyway, thank you so much for your attention and appreciate your great help!
Regards,
Grant.