diff --git a/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchRepository.java b/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchRepository.java index 44895650..ef5b8c88 100644 --- a/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchRepository.java +++ b/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchRepository.java @@ -82,6 +82,12 @@ public String getUrlName() { } + public synchronized void removeBranch(String branchName) { + if (nonNull(branches)) { + branches.remove(branchName); + } + } + /** * Searches for all builds performed in the runs of current job. * diff --git a/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchTrigger.java b/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchTrigger.java index 51da1ed5..9ec48c01 100644 --- a/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchTrigger.java +++ b/github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/GitHubBranchTrigger.java @@ -3,6 +3,7 @@ import antlr.ANTLRException; import com.github.kostyasha.github.integration.branch.events.GitHubBranchEvent; import com.github.kostyasha.github.integration.branch.events.GitHubBranchEventDescriptor; +import com.github.kostyasha.github.integration.branch.events.impl.GitHubBranchDeletedEvent; import com.github.kostyasha.github.integration.branch.trigger.JobRunnerForBranchCause; import com.github.kostyasha.github.integration.generic.GitHubTrigger; import com.github.kostyasha.github.integration.generic.GitHubTriggerDescriptor; @@ -164,9 +165,13 @@ public void queueRun(Job job, final String branch) { /** * Runs check * + * synchronizing a method is bad ... we really should just focus on synchronizing the variable localreposittory + * so that it is cleaned up in case multiple events come in ... so we dont' prematurely fire delete events on + * local repo that maybe processing a delete ... + * * @param branch - branch for check, if null - then all PRs */ - public void doRun(String branch) { + public synchronized void doRun(String branch) { if (not(isBuildable()).apply(job)) { LOG.debug("Job {} is disabled, but trigger run!", isNull(job) ? "" : job.getFullName()); return; @@ -227,7 +232,7 @@ private List readyToBuildCauses(GitHubBranchRepository localR GHRepository remoteRepo = getRemoteRepository(); Set remoteBranches = branchesToCheck(requestedBranch, remoteRepo, localRepository); - List causes = checkBranches(remoteBranches, localRepository, listener); + List causes = checkBranches(branch, remoteBranches, remoteRepo, localRepository, listener); /* * update details about the local repo after the causes are determined as they expect @@ -256,7 +261,7 @@ private Set branchesToCheck(@CheckForNull String branch, @Nonnull GHRe throws IOException { final LinkedHashSet ghBranches = new LinkedHashSet<>(); - if (branch != null) { + if (branch != null) { // What about DELETED event ? the remote branch is already gone ... final GHBranch ghBranch = remoteRepo.getBranches().get(branch); if (ghBranch != null) { ghBranches.add(ghBranch); @@ -268,8 +273,10 @@ private Set branchesToCheck(@CheckForNull String branch, @Nonnull GHRe return ghBranches; } - private List checkBranches(Set remoteBranches, - GitHubBranchRepository localRepository, LoggingTaskListenerWrapper listener) { + private List checkBranches(String branchName, Set remoteBranches, @Nonnull GHRepository remoteRepo, + GitHubBranchRepository localRepository, LoggingTaskListenerWrapper listener) + throws IOException { + List causes = remoteBranches.stream() // TODO: update user whitelist filter .filter(ifSkippedFirstRun(listener, skipFirstRun)) @@ -278,6 +285,54 @@ private List checkBranches(Set remoteBranches, .filter(Objects::nonNull) .collect(Collectors.toList()); + // DELETE BRANCH is a special case since the remote branch exists for all the other events + // and there is probably a more elegant solution ... + //boolean processDelete = false; + for (GitHubBranchEvent event : events) { + //processDelete = (event instanceof GitHubBranchDeletedEvent) ? true : false; + //} + //if (processDelete) { + if (event instanceof GitHubBranchDeletedEvent) { + Map localBranches = localRepository.getBranches(); + GitHubBranch localBranch = localBranches.get(branchName); + if (localBranch != null) { + Map remoteRepoBranches = remoteRepo.getBranches(); + if (remoteRepoBranches.get(branchName) == null) { + causes.add(new GitHubBranchCause(localBranch, localRepository, "Branch Deleted", false)); + // we probably want to take the localBranch out of the localRepository ... + // cause that also operates on a empty "Set" stream ... + localRepository.removeBranch(branchName); // so that we don't process a delete on this again ... + LOG.error("Adding cause to trigger delete event for [{}] : {}", localRepository.getFullName(), branchName); + } + } + break; // we only care about delete in the loop ... + } + } +/* + // DELETE BRANCH is a special case since the remote branch exists for all the other events + // and there is probably a more elegant solution ... + boolean processDelete = false; + for (GitHubBranchEvent event : events) { + if (event instanceof GitHubBranchDeletedEvent) { + processDelete = true; + } + } + + if (processDelete) { + synchronized (localRepository) { + Map localBranches = localRepository.getBranches(); + Map remoteRepoBranches = remoteRepo.getBranches(); + localBranches.forEach((localBranchName, localBranch) -> { + if (remoteRepoBranches.get(localBranchName) == null) { + causes.add(new GitHubBranchCause(localBranch, localRepository, "Branch Deleted", false)); + LOG.error("MG Adding cause to trigger delete event for [{}] : {}", localRepository.getFullName(), localBranchName); + localRepository.removeBranch(localBranchName); + } + }); + } + } + */ + LOG.debug("Build trigger count for [{}] : {}", localRepository.getFullName(), causes.size()); return causes; }