Close all BitbucketApi client and reduce the number of client instance in SCMSource when process repositories

Author: nfalco79

docs/USER_GUIDE.adoc

@@ -175,7 +175,7 @@ Bitbucket https://community.atlassian.com/t5/Bitbucket-articles/Announcement-Bit
 
 The plugin can make use of an app password instead of the standard username/password.
 
-First, create a new _app password_ in Bitbucket as instructed in the https://support.atlassian.com/bitbucket-cloud/docs/app-passwords/[Bitbucket App Passwords Documentation]. At least allow _read_ access for repositories. Also, you may need to allow _read_ and _write_ access for webhooks depending on your pipeline's triggers.
+First, create a new _app password_ in Bitbucket as instructed in the https://support.atlassian.com/bitbucket-cloud/docs/app-passwords/[Bitbucket App Passwords Documentation]. At least allow _read_ access for repositories and pull requests. Also, you may need to allow _read_ and _write_ access for webhooks depending on your pipeline's triggers.
 
 Then create a new _Username with password credentials_ in Jenkins, enter the Bitbucket username (not the email) in the _Username_ field and the created app password in the _Password_ field.
 

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

@@ -25,12 +25,13 @@
 
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketApi;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketBranch;
+import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketCommit;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketPullRequest;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.Util;
 import hudson.model.TaskListener;
-import java.io.Closeable;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -39,7 +40,11 @@
 import java.util.Map;
 import java.util.Set;
 import jenkins.scm.api.SCMHead;
+import jenkins.scm.api.SCMHeadObserver;
 import jenkins.scm.api.SCMHeadOrigin;
+import jenkins.scm.api.SCMRevision;
+import jenkins.scm.api.SCMSourceCriteria;
+import jenkins.scm.api.SCMSourceCriteria.Probe;
 import jenkins.scm.api.mixin.ChangeRequestCheckoutStrategy;
 import jenkins.scm.api.trait.SCMSourceRequest;
 
@@ -49,6 +54,131 @@
  * @since 2.2.0
  */
 public class BitbucketSCMSourceRequest extends SCMSourceRequest {
+
+    private class BitbucketProbeFactory<I> implements SCMSourceRequest.ProbeLambda<SCMHead, I> {
+        private final BitbucketApi client;
+
+        public BitbucketProbeFactory(BitbucketApi client) {
+            this.client = client;
+        }
+
+        @NonNull
+        @Override
+        public Probe create(@NonNull final SCMHead head, @CheckForNull final I revisionInfo) throws IOException, InterruptedException {
+            final String hash = (revisionInfo instanceof BitbucketCommit bbRevision) //
+                    ? bbRevision.getHash() //
+                    : (String) revisionInfo;
+
+            return new SCMSourceCriteria.Probe() {
+                private static final long serialVersionUID = 1L;
+
+                @Override
+                public String name() {
+                    return head.getName();
+                }
+
+                @Override
+                public long lastModified() {
+                    try {
+                        BitbucketCommit commit = null;
+                        if (hash != null) {
+                            commit = (revisionInfo instanceof BitbucketCommit bbRevision) //
+                                    ? bbRevision //
+                                    : client.resolveCommit(hash);
+                        }
+
+                        if (commit == null) {
+                            listener().getLogger().format("Can not resolve commit by hash [%s] on repository %s/%s%n", //
+                                    hash, client.getOwner(), client.getRepositoryName());
+                            return 0;
+                        }
+                        return commit.getDateMillis();
+                    } catch (InterruptedException | IOException e) {
+                        listener().getLogger().format("Can not resolve commit by hash [%s] on repository %s/%s%n", //
+                                hash, client.getOwner(), client.getRepositoryName());
+                        return 0;
+                    }
+                }
+
+                @Override
+                public boolean exists(@NonNull String path) throws IOException {
+                    if (hash == null) {
+                        listener().getLogger() //
+                                .format("Can not resolve path for hash [%s] on repository %s/%s%n", //
+                                        hash, client.getOwner(), client.getRepositoryName());
+                        return false;
+                    }
+
+                    try {
+                        return client.checkPathExists(hash, path);
+                    } catch (InterruptedException e) {
+                        throw new IOException("Interrupted", e);
+                    }
+                }
+            };
+        }
+    }
+
+    public static class BitbucketRevisionFactory<I> implements SCMSourceRequest.LazyRevisionLambda<SCMHead, SCMRevision, I> {
+        private final BitbucketApi client;
+
+        public BitbucketRevisionFactory(BitbucketApi client) {
+            this.client = client;
+        }
+
+        @NonNull
+        @Override
+        public SCMRevision create(@NonNull SCMHead head, @Nullable I input) throws IOException, InterruptedException {
+            return create(head, input, null);
+        }
+
+        @NonNull
+        public SCMRevision create(@NonNull SCMHead head,
+                                  @Nullable I sourceInput,
+                                  @Nullable I targetInput) throws IOException, InterruptedException {
+            BitbucketCommit sourceCommit = asCommit(sourceInput);
+            BitbucketCommit targetCommit = asCommit(targetInput);
+
+            SCMRevision revision;
+            if (head instanceof PullRequestSCMHead prHead) {
+                SCMHead targetHead = prHead.getTarget();
+
+                return new PullRequestSCMRevision( //
+                        prHead, //
+                        new BitbucketGitSCMRevision(targetHead, targetCommit), //
+                        new BitbucketGitSCMRevision(prHead, sourceCommit));
+            } else {
+                revision = new BitbucketGitSCMRevision(head, sourceCommit);
+            }
+            return revision;
+        }
+
+        private BitbucketCommit asCommit(I input) throws IOException, InterruptedException {
+            if (input instanceof String value) {
+                return client.resolveCommit(value);
+            } else if (input instanceof BitbucketCommit commit) {
+                return commit;
+            }
+            return null;
+        }
+    }
+
+    private class CriteriaWitness implements SCMSourceRequest.Witness {
+        @Override
+        public void record(@NonNull SCMHead scmHead, SCMRevision revision, boolean isMatch) {
+            if (revision == null) {
+                listener().getLogger().println("    Skipped");
+            } else {
+                if (isMatch) {
+                    listener().getLogger().println("    Met criteria");
+                } else {
+                    listener().getLogger().println("    Does not meet criteria");
+                }
+
+            }
+        }
+    }
+
     /**
      * {@code true} if branch details need to be fetched.
      */
@@ -355,9 +485,14 @@ public final void setPullRequests(@CheckForNull Iterable<BitbucketPullRequest> p
      * or if the pull request details have not been provided by {@link #setPullRequests(Iterable)} yet.
      *
      * @return the pull request details (may be empty)
+     * @throws InterruptedException
+     * @throws IOException
      */
     @NonNull
-    public final Iterable<BitbucketPullRequest> getPullRequests() {
+    public final Iterable<BitbucketPullRequest> getPullRequests() throws IOException, InterruptedException {
+        if (pullRequests == null) {
+            pullRequests = (Iterable<BitbucketPullRequest>) getBitbucketApiClient().getPullRequests();
+        }
         return Util.fixNull(pullRequests);
     }
 
@@ -399,9 +534,14 @@ public final void setBranches(@CheckForNull Iterable<BitbucketBranch> branches)
      * or if the branch details have not been provided by {@link #setBranches(Iterable)} yet.
      *
      * @return the branch details (may be empty)
+     * @throws IOException if there was a network communications error.
+     * @throws InterruptedException if interrupted while waiting on remote communications.
      */
     @NonNull
-    public final Iterable<BitbucketBranch> getBranches() {
+    public final Iterable<BitbucketBranch> getBranches() throws IOException, InterruptedException {
+        if (branches == null) {
+            branches = (Iterable<BitbucketBranch>) getBitbucketApiClient().getBranches();
+        }
         return Util.fixNull(branches);
     }
 
@@ -419,9 +559,14 @@ public final void setTags(@CheckForNull Iterable<BitbucketBranch> tags) {
      * or if the tag details have not been provided by {@link #setTags(Iterable)} yet.
      *
      * @return the tag details (may be empty)
+     * @throws IOException if there was a network communications error.
+     * @throws InterruptedException if interrupted while waiting on remote communications.
      */
     @NonNull
-    public final Iterable<BitbucketBranch> getTags() {
+    public final Iterable<BitbucketBranch> getTags() throws IOException, InterruptedException {
+        if (tags == null) {
+            tags = (Iterable<BitbucketBranch>) getBitbucketApiClient().getTags();
+        }
         return Util.fixNull(tags);
     }
 
@@ -430,12 +575,54 @@ public final Iterable<BitbucketBranch> getTags() {
      */
     @Override
     public void close() throws IOException {
-        if (pullRequests instanceof Closeable closable) {
-            closable.close();
-        }
-        if (branches instanceof Closeable closable) {
-            closable.close();
+        if (api != null) {
+            api.close();
         }
         super.close();
     }
+
+    /**
+     * Processes a head in the context of the current request where an intermediary operation is required before
+     * the {@link SCMRevision} can be instantiated.
+     *
+     * @param head                the {@link SCMHead} to process.
+     * @param intermediateFactory factory method that provides the seed information for both the {@link ProbeLambda}
+     *                            and the {@link LazyRevisionLambda}.
+     * @param <H>                 the type of {@link SCMHead}.
+     * @param <I>                 the type of the intermediary operation result.
+     * @param <R>                 the type of {@link SCMRevision}.
+     * @return {@code true} if the {@link SCMHeadObserver} for this request has completed observing, {@code false} to
+     * continue processing.
+     * @throws IOException          if there was an I/O error.
+     * @throws InterruptedException if the processing was interrupted.
+     */
+    public final <H extends SCMHead, I, R extends SCMRevision> boolean process(@NonNull H head,
+                                                                               @CheckForNull IntermediateLambda<I> intermediateFactory)
+                                                                               throws IOException, InterruptedException {
+        return super.process(head, //
+                       intermediateFactory, //
+                       defaultProbeLamda(), //
+                       defaultRevisionLamda(), //
+                       new CriteriaWitness());
+    }
+
+    @NonNull
+    <I> ProbeLambda<SCMHead, I> defaultProbeLamda() {
+        return this.new BitbucketProbeFactory<>(getBitbucketApiClient());
+    }
+
+    @NonNull
+    <I> ProbeLambda<SCMHead, I> buildProbeLamda(@NonNull BitbucketApi client) {
+        return this.new BitbucketProbeFactory<>(client);
+    }
+
+    @NonNull
+    <I> LazyRevisionLambda<SCMHead, SCMRevision, I> defaultRevisionLamda() {
+        return new BitbucketRevisionFactory<>(getBitbucketApiClient());
+    }
+
+    @NonNull
+    Witness defaultWitness() {
+        return this.new CriteriaWitness();
+    }
 }

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSourceRequest.java

@@ -28,6 +28,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.util.ListBoxModel;
+import java.io.IOException;
 import jenkins.scm.api.SCMHead;
 import jenkins.scm.api.SCMHeadCategory;
 import jenkins.scm.api.SCMHeadOrigin;
@@ -242,14 +243,19 @@ public boolean isExcluded(@NonNull SCMSourceRequest request, @NonNull SCMHead he
             if (head instanceof BranchSCMHead && request instanceof BitbucketSCMSourceRequest) {
                 BitbucketSCMSourceRequest req = (BitbucketSCMSourceRequest) request;
                 String fullName = req.getRepoOwner() + "/" + req.getRepository();
-                for (BitbucketPullRequest pullRequest : req.getPullRequests()) {
-                    BitbucketRepository source = pullRequest.getSource().getRepository();
-                    if (StringUtils.equalsIgnoreCase(fullName, source.getFullName())
-                            && pullRequest.getSource().getBranch().getName().equals(head.getName())) {
-                        request.listener().getLogger().println("Discard branch " + head.getName()
-                                + " because current strategy excludes branches that are also filed as a pull request");
-                        return true;
+                try {
+                    for (BitbucketPullRequest pullRequest : req.getPullRequests()) {
+                        BitbucketRepository source = pullRequest.getSource().getRepository();
+                        if (StringUtils.equalsIgnoreCase(fullName, source.getFullName())
+                                && pullRequest.getSource().getBranch().getName().equals(head.getName())) {
+                            request.listener().getLogger().println("Discard branch " + head.getName()
+                                    + " because current strategy excludes branches that are also filed as a pull request");
+                            return true;
+                        }
                     }
+                } catch (IOException | InterruptedException e) {
+                    // should never happens because data in the requests has been already initialised
+                    e.printStackTrace(request.listener().getLogger());
                 }
             }
             return false;
@@ -268,16 +274,21 @@ public boolean isExcluded(@NonNull SCMSourceRequest request, @NonNull SCMHead he
             if (head instanceof BranchSCMHead && request instanceof BitbucketSCMSourceRequest) {
                 BitbucketSCMSourceRequest req = (BitbucketSCMSourceRequest) request;
                 String fullName = req.getRepoOwner() + "/" + req.getRepository();
-                for (BitbucketPullRequest pullRequest : req.getPullRequests()) {
-                    BitbucketRepository source = pullRequest.getSource().getRepository();
-                    if (fullName.equalsIgnoreCase(source.getFullName())
-                            && pullRequest.getSource().getBranch().getName().equals(head.getName())) {
-                        return false;
+                try {
+                    for (BitbucketPullRequest pullRequest : req.getPullRequests()) {
+                        BitbucketRepository source = pullRequest.getSource().getRepository();
+                        if (fullName.equalsIgnoreCase(source.getFullName())
+                                && pullRequest.getSource().getBranch().getName().equals(head.getName())) {
+                            return false;
+                        }
                     }
+                    request.listener().getLogger().println("Discard branch " + head.getName()
+                            + " because current strategy excludes branches that are not also filed as a pull request");
+                    return true;
+                } catch (IOException | InterruptedException e) {
+                    // should never happens because data in the requests has been already initialised
+                    e.printStackTrace(request.listener().getLogger());
                 }
-                request.listener().getLogger().println("Discard branch " + head.getName()
-                        + " because current strategy excludes branches that are not also filed as a pull request");
-                return true;
             }
             return false;
         }

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BranchDiscoveryTrait.java

@@ -31,6 +31,7 @@
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketException;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketPullRequest;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRepository;
+import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRequestException;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketTeam;
 import com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketWebHook;
 import com.cloudbees.jenkins.plugins.bitbucket.client.branch.BitbucketCloudBranch;
@@ -668,6 +669,8 @@ public AvatarImage getAvatar(@CheckForNull String url) throws IOException {
                 return new AvatarImage(avatar, System.currentTimeMillis());
             } catch (FileNotFoundException e) {
                 logger.log(Level.FINE, "Failed to get avatar from URL {0}", url);
+            } catch (BitbucketRequestException e) {
+                throw e;
             } catch (IOException e) {
                 throw new IOException("I/O error when parsing response from URL: " + url, e);
             }

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java

@@ -263,16 +263,12 @@ private Map<String, BitbucketServerPullRequest> getPullRequests(BitbucketSCMSour
         return pullRequests;
     }
 
-    private Map<String, BitbucketServerPullRequest> loadPullRequests(BitbucketSCMSource src,
-        NativeServerChange change) throws InterruptedException {
-
+    private Map<String, BitbucketServerPullRequest> loadPullRequests(BitbucketSCMSource src, NativeServerChange change) throws InterruptedException {
         final BitbucketServerRepository eventRepo = repository;
-        final BitbucketServerAPIClient api = (BitbucketServerAPIClient) src
-            .buildBitbucketClient(eventRepo.getOwnerName(), eventRepo.getRepositoryName());
-
         final Map<String, BitbucketServerPullRequest> pullRequests = new HashMap<>();
 
-        try {
+        try (BitbucketServerAPIClient api = (BitbucketServerAPIClient) src
+                .buildBitbucketClient(eventRepo.getOwnerName(), eventRepo.getRepositoryName())) {
             try {
                 for (final BitbucketServerPullRequest pullRequest : api.getOutgoingOpenPullRequests(change.getRefId())) {
                     pullRequests.put(pullRequest.getId(), pullRequest);
@@ -294,6 +290,8 @@ private Map<String, BitbucketServerPullRequest> loadPullRequests(BitbucketSCMSou
             }
         } catch (FileNotFoundException e) {
             LOGGER.log(Level.INFO, "No such Repository on Bitbucket: {0}", e.getMessage());
+        } catch (IOException e1) {
+            LOGGER.log(Level.INFO, "Comunication fail with server", e1);
         }
 
         return pullRequests;