From f58274f2350fc91118aa79c5fa0263c018e43198 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Mon, 19 Jun 2017 10:25:28 -0400 Subject: [PATCH 01/11] for what it's worth ... --- .idea/codeStyleSettings.xml | 15 ---- .idea/encodings.xml | 3 - github-pullrequest-plugin/pom.xml | 2 +- .../branch/GitHubBranchRepository.java | 6 ++ .../branch/GitHubBranchTrigger.java | 70 ++++++++++++++++--- ...pullrequest.GitHubPRRepository.runtime.xml | 2 + 6 files changed, 68 insertions(+), 30 deletions(-) delete mode 100644 .idea/codeStyleSettings.xml diff --git a/.idea/codeStyleSettings.xml b/.idea/codeStyleSettings.xml deleted file mode 100644 index 0d89db59..00000000 --- a/.idea/codeStyleSettings.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/encodings.xml b/.idea/encodings.xml index 16c3507e..f85b9fc4 100644 --- a/.idea/encodings.xml +++ b/.idea/encodings.xml @@ -2,9 +2,6 @@ - - - diff --git a/github-pullrequest-plugin/pom.xml b/github-pullrequest-plugin/pom.xml index b4e6db4d..f96f6c15 100644 --- a/github-pullrequest-plugin/pom.xml +++ b/github-pullrequest-plugin/pom.xml @@ -10,7 +10,7 @@ github-pullrequest - 0.1.0-SNAPSHOT + 0.1.2-SNAPSHOT hpi GitHub Integration Plugin 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 dcb0cdd5..e5f428c5 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; @@ -29,12 +30,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.io.IOException; -import java.util.ArrayList; -import java.util.Date; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Objects; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import static com.github.kostyasha.github.integration.branch.trigger.check.BranchToCauseConverter.toGitHubBranchCause; @@ -161,9 +157,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; @@ -224,7 +224,7 @@ private List readyToBuildCauses(GitHubBranchRepository localR GHRepository remoteRepo = getRemoteRepository(); Set remoteBranches = branchesToCheck(branch, 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 @@ -253,7 +253,7 @@ private Set branchesToCheck(String branch, @Nonnull GHRepository remot 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); @@ -265,8 +265,10 @@ private Set branchesToCheck(String branch, @Nonnull GHRepository remot 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)) @@ -275,6 +277,52 @@ 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; } diff --git a/github-pullrequest-plugin/src/test/resources/org.jenkinsci.plugins.github.pullrequest.GitHubPRRepository.runtime.xml b/github-pullrequest-plugin/src/test/resources/org.jenkinsci.plugins.github.pullrequest.GitHubPRRepository.runtime.xml index f459f1ab..de1183f2 100644 --- a/github-pullrequest-plugin/src/test/resources/org.jenkinsci.plugins.github.pullrequest.GitHubPRRepository.runtime.xml +++ b/github-pullrequest-plugin/src/test/resources/org.jenkinsci.plugins.github.pullrequest.GitHubPRRepository.runtime.xml @@ -22,6 +22,7 @@ 2015-01-02 13:11:21.0 UTC user + false @@ -42,6 +43,7 @@ 2015-01-31 19:21:01.0 UTC user + false From 19ee5b4c26b5e7e5a346cab72ce1e1d43a534ab9 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Wed, 29 Nov 2017 00:50:01 -0500 Subject: [PATCH 02/11] Update GitHubBranchTrigger.java fix the line to long errors caught by the travis-ci.org unit testing --- .../github/integration/branch/GitHubBranchTrigger.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 8c94772f..ca533c05 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 @@ -285,7 +285,8 @@ private List checkBranches(String branchName, Set r .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 ... + // 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; @@ -298,7 +299,8 @@ private List checkBranches(String branchName, Set r 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 ... + // 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); } From 1557008a0f7d2ff37da6a0c859fdfa5a91d8c84d Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 16:04:07 -0500 Subject: [PATCH 03/11] Update GitHubBranchRepository.java --- .../github/integration/branch/GitHubBranchRepository.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 ef5b8c88..b0ebd602 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,10 +82,8 @@ public String getUrlName() { } - public synchronized void removeBranch(String branchName) { - if (nonNull(branches)) { - branches.remove(branchName); - } + public synchronized void removeBranch(@NonNull String branchName) { + getBranches().remove(branchName) } /** From e72eefa0af5967fd861af8fb3f4918ce255d1413 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 16:04:58 -0500 Subject: [PATCH 04/11] Update pom.xml --- github-pullrequest-plugin/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-pullrequest-plugin/pom.xml b/github-pullrequest-plugin/pom.xml index f96f6c15..b4e6db4d 100644 --- a/github-pullrequest-plugin/pom.xml +++ b/github-pullrequest-plugin/pom.xml @@ -10,7 +10,7 @@ github-pullrequest - 0.1.2-SNAPSHOT + 0.1.0-SNAPSHOT hpi GitHub Integration Plugin From ea6b0d885c7c392e05c8f96a49f919272f6ac4c6 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 16:12:29 -0500 Subject: [PATCH 05/11] Create codeStyleSettings.xml ahhh I don't know how this got removed ... did my idea nuke it and then i committed ? I apologize. --- .idea/codeStyleSettings.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .idea/codeStyleSettings.xml diff --git a/.idea/codeStyleSettings.xml b/.idea/codeStyleSettings.xml new file mode 100644 index 00000000..bf63aad2 --- /dev/null +++ b/.idea/codeStyleSettings.xml @@ -0,0 +1,14 @@ + + + + + From 3de4489587452b9c45416b37cc76e78a63643a57 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 22:07:40 -0500 Subject: [PATCH 06/11] Update GitHubBranchRepository.java --- .../github/integration/branch/GitHubBranchRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b0ebd602..d9f6c872 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 @@ -83,7 +83,7 @@ public String getUrlName() { public synchronized void removeBranch(@NonNull String branchName) { - getBranches().remove(branchName) + getBranches().remove(branchName); } /** From 4231019c4efcb695cde3d57dfb19b64299eab920 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 22:33:46 -0500 Subject: [PATCH 07/11] Update GitHubBranchRepository.java typo on Nonnull --- .../github/integration/branch/GitHubBranchRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d9f6c872..52e5c73c 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,7 +82,7 @@ public String getUrlName() { } - public synchronized void removeBranch(@NonNull String branchName) { + public synchronized void removeBranch(@Nonnull String branchName) { getBranches().remove(branchName); } From d101cebe7fc543e209d4837da3a25b9d39e96292 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Thu, 30 Nov 2017 23:52:45 -0500 Subject: [PATCH 08/11] Update GitHubBranchTrigger.java --- .../github/integration/branch/GitHubBranchTrigger.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ca533c05..0f30698c 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 @@ -273,7 +273,8 @@ private Set branchesToCheck(String branch, @Nonnull GHRepository remot return ghBranches; } - private List checkBranches(String branchName, Set remoteBranches, @Nonnull GHRepository remoteRepo, + // i think this is what I have to do to get the complication to pass ? + private List checkBranches(@Nullable String branchName, Set remoteBranches, @Nonnull GHRepository remoteRepo, GitHubBranchRepository localRepository, LoggingTaskListenerWrapper listener) throws IOException { From aef29bae6480896573f6015eaafc50ea3fa3854e Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Fri, 1 Dec 2017 00:18:59 -0500 Subject: [PATCH 09/11] method call to use requestedBranch variable --- .../github/integration/branch/GitHubBranchTrigger.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b6956852..8835bcec 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 @@ -233,7 +233,7 @@ private List readyToBuildCauses(GitHubBranchRepository localR GHRepository remoteRepo = getRemoteRepository(); Set remoteBranches = branchesToCheck(requestedBranch, remoteRepo, localRepository); - List causes = checkBranches(branch, remoteBranches, remoteRepo, localRepository, listener); + List causes = checkBranches(requestedBranch, remoteBranches, remoteRepo, localRepository, listener); /* * update details about the local repo after the causes are determined as they expect From 25ab1fbca5cf8eb4834254b88315b2576e5b7da0 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Fri, 1 Dec 2017 00:49:29 -0500 Subject: [PATCH 10/11] fixed line length CHeckStyle error --- .../github/integration/branch/GitHubBranchTrigger.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 8835bcec..438765a3 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 @@ -275,8 +275,11 @@ private Set branchesToCheck(@CheckForNull String branch, @Nonnull GHRe } // i think this is what I have to do to get the complication to pass ? - private List checkBranches(@Nullable String branchName, Set remoteBranches, @Nonnull GHRepository remoteRepo, - GitHubBranchRepository localRepository, LoggingTaskListenerWrapper listener) + private List checkBranches(@Nullable String branchName, + Set remoteBranches, + @Nonnull GHRepository remoteRepo, + GitHubBranchRepository localRepository, + LoggingTaskListenerWrapper listener) throws IOException { List causes = remoteBranches.stream() From 1a298137697a67ddc7bf576e213ab98bc453426f Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Fri, 1 Dec 2017 01:13:51 -0500 Subject: [PATCH 11/11] fixed line length CHeckStyle error --- .../github/integration/branch/GitHubBranchTrigger.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 438765a3..9624b4b2 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 @@ -276,10 +276,10 @@ private Set branchesToCheck(@CheckForNull String branch, @Nonnull GHRe // i think this is what I have to do to get the complication to pass ? private List checkBranches(@Nullable String branchName, - Set remoteBranches, - @Nonnull GHRepository remoteRepo, + Set remoteBranches, + @Nonnull GHRepository remoteRepo, GitHubBranchRepository localRepository, - LoggingTaskListenerWrapper listener) + LoggingTaskListenerWrapper listener) throws IOException { List causes = remoteBranches.stream()