From 8186b29499e83d9eebc9b810ab73a2d1720f109c Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Mon, 19 Jun 2017 10:25:28 -0400 Subject: [PATCH 1/3] 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 | 63 +++++++++++++++++-- ...pullrequest.GitHubPRRepository.runtime.xml | 2 + 6 files changed, 67 insertions(+), 24 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 51da1ed5..ba65a417 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,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 abd3d89d4401e3e9afcac7b5021e0dbcc75559a5 Mon Sep 17 00:00:00 2001 From: MichaelGreco Date: Wed, 29 Nov 2017 00:50:01 -0500 Subject: [PATCH 2/3] 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 ba65a417..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 @@ -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 c079a2d7b93bdd7c91b379a5433bb9aaceb3e56a Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Thu, 30 Nov 2017 23:58:17 +0300 Subject: [PATCH 3/3] restore some files --- .idea/codeStyleSettings.xml | 15 +++++++++++++++ .idea/encodings.xml | 3 +++ github-pullrequest-plugin/pom.xml | 2 +- ...hub.pullrequest.GitHubPRRepository.runtime.xml | 2 -- 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 .idea/codeStyleSettings.xml diff --git a/.idea/codeStyleSettings.xml b/.idea/codeStyleSettings.xml new file mode 100644 index 00000000..0d89db59 --- /dev/null +++ b/.idea/codeStyleSettings.xml @@ -0,0 +1,15 @@ + + + + + + \ No newline at end of file diff --git a/.idea/encodings.xml b/.idea/encodings.xml index f85b9fc4..16c3507e 100644 --- a/.idea/encodings.xml +++ b/.idea/encodings.xml @@ -2,6 +2,9 @@ + + + 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 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 de1183f2..f459f1ab 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,7 +22,6 @@ 2015-01-02 13:11:21.0 UTC user - false @@ -43,7 +42,6 @@ 2015-01-31 19:21:01.0 UTC user - false