Improve caching by leveraging atlassian-cache

This will enable caching in a clustered environment, such as Jira Data Center.

src/main/java/com/meetme/plugins/jira/gerrit/data/IssueReviewsCacheLoader.java

@@ -0,0 +1,84 @@
+package com.meetme.plugins.jira.gerrit.data;
+
+import com.meetme.plugins.jira.gerrit.data.dto.GerritChange;
+
+import com.atlassian.cache.CacheException;
+import com.atlassian.cache.CacheLoader;
+import com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritQueryException;
+import com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritQueryHandler;
+import com.sonyericsson.hudson.plugins.gerrit.gerritevents.ssh.Authentication;
+import com.sonyericsson.hudson.plugins.gerrit.gerritevents.ssh.SshException;
+
+import net.sf.json.JSONObject;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+
+import javax.annotation.Nonnull;
+
+public class IssueReviewsCacheLoader implements CacheLoader<String, List<GerritChange>> {
+    private final Logger log = LoggerFactory.getLogger(IssueReviewsCacheLoader.class);
+    private final GerritConfiguration configuration;
+
+    public IssueReviewsCacheLoader(GerritConfiguration configuration) {
+        this.configuration = configuration;
+    }
+
+    @Nonnull
+    @Override
+    public List<GerritChange> load(@Nonnull String key) {
+        String query = String.format(Locale.US, configuration.getIssueSearchQuery(), key);
+
+        try {
+            return getReviewsFromGerrit(query);
+        } catch (GerritQueryException e) {
+            log.error("Error querying for issues", e);
+            throw new CacheException("Error querying for issues: " + e.getMessage(), e);
+        }
+    }
+
+    protected List<GerritChange> getReviewsFromGerrit(String searchQuery) throws GerritQueryException {
+        List<GerritChange> changes;
+
+        if (!configuration.isSshValid()) {
+            // return Collections.emptyList();
+            throw new GerritConfiguration.NotConfiguredException("Not configured for SSH access");
+        }
+
+        Authentication auth = new Authentication(configuration.getSshPrivateKey(), configuration.getSshUsername());
+        GerritQueryHandler query = new GerritQueryHandler(configuration.getSshHostname(), configuration.getSshPort(), null, auth);
+        List<JSONObject> reviews;
+
+        try {
+            reviews = query.queryJava(searchQuery, false, true, false);
+        } catch (SshException e) {
+            throw new GerritQueryException("An ssh error occurred while querying for reviews.", e);
+        } catch (IOException e) {
+            throw new GerritQueryException("An error occurred while querying for reviews.", e);
+        }
+
+        changes = new ArrayList<>(reviews.size());
+
+        for (JSONObject obj : reviews) {
+            if (obj.has("type") && "stats".equalsIgnoreCase(obj.getString("type"))) {
+                // The final JSON object in the query results is just a set of statistics
+                if (log.isDebugEnabled()) {
+                    log.trace("Results from QUERY: " + obj.optString("rowCount", "(unknown)") + " rows; runtime: "
+                            + obj.optString("runTimeMilliseconds", "(unknown)") + " ms");
+                }
+                continue;
+            }
+
+            changes.add(new GerritChange(obj));
+        }
+
+        Collections.sort(changes);
+        return changes;
+    }
+}

src/main/java/com/meetme/plugins/jira/gerrit/data/IssueReviewsImpl.java

@@ -15,34 +15,52 @@
 
 import com.meetme.plugins.jira.gerrit.data.dto.GerritChange;
 
+import com.atlassian.cache.Cache;
+import com.atlassian.cache.CacheException;
+import com.atlassian.cache.CacheManager;
+import com.atlassian.cache.CacheSettingsBuilder;
 import com.atlassian.core.user.preferences.Preferences;
 import com.atlassian.jira.issue.Issue;
 import com.atlassian.jira.issue.IssueManager;
 import com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritQueryException;
-import com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritQueryHandler;
-import com.sonyericsson.hudson.plugins.gerrit.gerritevents.ssh.Authentication;
-import com.sonyericsson.hudson.plugins.gerrit.gerritevents.ssh.SshException;
-
-import net.sf.json.JSONObject;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 public class IssueReviewsImpl implements IssueReviewsManager {
     private static final Logger log = LoggerFactory.getLogger(IssueReviewsImpl.class);
-    private final Map<String, List<GerritChange>> lruCache;
+
+    private final Cache<String, List<GerritChange>> cache;
 
     private GerritConfiguration configuration;
 
     private IssueManager jiraIssueManager;
 
-    public IssueReviewsImpl(GerritConfiguration configuration, IssueManager jiraIssueManager) {
+    public IssueReviewsImpl(
+            GerritConfiguration configuration,
+            IssueManager jiraIssueManager,
+            CacheManager cacheManager,
+            IssueReviewsCacheLoader cacheLoader
+    ) {
         this.configuration = configuration;
         this.jiraIssueManager = jiraIssueManager;
-        this.lruCache = IssueReviewsCache.getCache();
+        this.cache = cacheManager.getCache(
+                IssueReviewsManager.class.getName() + ".issueChanges.cache",
+                cacheLoader, //new IssueReviewsCacheLoader(configuration),
+                new CacheSettingsBuilder()
+                        .flushable()
+                        .statisticsEnabled()
+                        .maxEntries(100)
+                        .replicateAsynchronously()
+                        .expireAfterAccess(30, TimeUnit.MINUTES)
+                        .build()
+        );
     }
 
     @Override
@@ -56,60 +74,23 @@ public List<GerritChange> getReviewsForIssue(Issue issue) throws GerritQueryExce
 
         Set<String> allIssueKeys = getIssueKeys(issue);
         for (String key : allIssueKeys) {
-            List<GerritChange> changes;
-
-            if (lruCache.containsKey(key)) {
-                log.debug("Getting issues from cache");
-                changes = lruCache.get(key);
-            } else {
-                log.debug("Getting issues from Gerrit");
-                changes = getReviewsFromGerrit(String.format(configuration.getIssueSearchQuery(), key));
-                lruCache.put(key, changes);
-            }
-
-            gerritChanges.addAll(changes);
-        }
-
-        return gerritChanges;
-    }
-
-    protected List<GerritChange> getReviewsFromGerrit(String searchQuery) throws GerritQueryException {
-        List<GerritChange> changes;
-
-        if (!configuration.isSshValid()) {
-            // return Collections.emptyList();
-            throw new GerritConfiguration.NotConfiguredException("Not configured for SSH access");
-        }
-
-        Authentication auth = new Authentication(configuration.getSshPrivateKey(), configuration.getSshUsername());
-        GerritQueryHandler query = new GerritQueryHandler(configuration.getSshHostname(), configuration.getSshPort(), null, auth);
-        List<JSONObject> reviews;
-
-        try {
-            reviews = query.queryJava(searchQuery, false, true, false);
-        } catch (SshException e) {
-            throw new GerritQueryException("An ssh error occurred while querying for reviews.", e);
-        } catch (IOException e) {
-            throw new GerritQueryException("An error occurred while querying for reviews.", e);
-        }
-
-        changes = new ArrayList<>(reviews.size());
-
-        for (JSONObject obj : reviews) {
-            if (obj.has("type") && "stats".equalsIgnoreCase(obj.getString("type"))) {
-                // The final JSON object in the query results is just a set of statistics
-                if (log.isDebugEnabled()) {
-                    log.trace("Results from QUERY: " + obj.optString("rowCount", "(unknown)") + " rows; runtime: "
-                            + obj.optString("runTimeMilliseconds", "(unknown)") + " ms");
+            try {
+                List<GerritChange> changes = cache.get(key);
+                if (changes != null) gerritChanges.addAll(changes);
+            } catch (CacheException exc) {
+                if (exc.getCause() instanceof GerritQueryException) {
+                    // TODO: is this really necessary?
+                    // If we swallow the error, then there's no indication on the UI that an error occurred.
+                    // The CacheLoader has to wrap the underlying exception in CacheException in order to throw it.
+                    throw (GerritQueryException) exc.getCause();
                 }
-                continue;
-            }
 
-            changes.add(new GerritChange(obj));
+                log.error("Error fetching from cache", exc);
+                throw exc;
+            }
         }
 
-        Collections.sort(changes);
-        return changes;
+        return gerritChanges;
     }
 
     @Override
@@ -128,7 +109,7 @@ public boolean doApprovals(Issue issue, List<GerritChange> changes, String args,
             }
 
             // Something probably changed!
-            lruCache.remove(issueKey);
+            cache.remove(issueKey);
         }
 
         return result;

src/main/resources/atlassian-plugin.xml

@@ -48,6 +48,10 @@
         <interface>com.meetme.plugins.jira.gerrit.data.IssueReviewsManager</interface>
     </component>
 
+    <component name="IssueReviews Cache Loader" class="com.meetme.plugins.jira.gerrit.data.IssueReviewsCacheLoader" key="issueReviewsCacheLoader">
+        <description>Populates the cached list of reviews per issue.</description>
+    </component>
+
     <template-context-item name="Application Properties Context Item"
         component-ref="applicationProperties"
         context-key="applicationProperties"