Skip to content

Commit

Permalink
Improve caching by leveraging atlassian-cache
Browse files Browse the repository at this point in the history 
This will enable caching in a clustered environment, such as Jira Data Center.
  • Loading branch information
@jhansche
jhansche committed Jul 11, 2019
1 parent 1c7da89 commit 252f47f
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 82 deletions.
84 changes: 84 additions & 0 deletions src/main/java/com/meetme/plugins/jira/gerrit/data/IssueReviewsCacheLoader.java
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
101 changes: 41 additions & 60 deletions src/main/java/com/meetme/plugins/jira/gerrit/data/IssueReviewsImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/atlassian-plugin.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
51 changes: 29 additions & 22 deletions src/test/java/com/meetme/plugins/jira/gerrit/data/IssueReviewsManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,27 @@

import com.meetme.plugins.jira.gerrit.data.dto.GerritChange;

import com.atlassian.cache.Cache;
import com.atlassian.cache.CacheLoader;
import com.atlassian.cache.CacheManager;
import com.atlassian.jira.issue.IssueManager;
import com.atlassian.jira.issue.MutableIssue;
import com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritQueryException;

import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
Expand Down Expand Up @@ -50,6 +54,12 @@ public class IssueReviewsManagerTest {
@Mock
private IssueManager mockJiraIssueManager;

@Mock
private CacheManager mockCacheManager;

@Mock
private Cache<String, List<GerritChange>> mockCache;

private IssueReviewsManager issueReviewsManager;

@Before
Expand All @@ -71,26 +81,23 @@ public void setUp() {
when(mockJiraIssueManager.getAllIssueKeys(mockIssue.getId())).thenReturn(allIssueKeys);

// mock gerrit review retrieval
issueReviewsManager = new IssueReviewsImpl(configuration, mockJiraIssueManager) {
@Override
protected List<GerritChange> getReviewsFromGerrit(String searchQuery) throws GerritQueryException {
List<GerritChange> reviews = new ArrayList<>();

if (searchQuery.contains(ISSUE_KEY_OLD)) {
GerritChange oldChangeMock = mock(GerritChange.class);
when(oldChangeMock.getSubject()).thenReturn(ISSUE_KEY_OLD);
reviews.add(oldChangeMock);
}

if (searchQuery.contains(ISSUE_KEY_NEW)) {
GerritChange newChangeMock = mock(GerritChange.class);
when(newChangeMock.getSubject()).thenReturn(ISSUE_KEY_NEW);
reviews.add(newChangeMock);
}

return reviews;
}
};
when(mockCacheManager.getCache(
eq("com.meetme.plugins.jira.gerrit.data.IssueReviewsManager.issueChanges.cache"),
Mockito.<CacheLoader<String, List<GerritChange>>>any(),
any()
)).thenReturn(mockCache);
issueReviewsManager = new IssueReviewsImpl(configuration, mockJiraIssueManager, mockCacheManager, null);

GerritChange oldChange = createMockChange(ISSUE_KEY_OLD);
GerritChange newChange = createMockChange(ISSUE_KEY_NEW);
when(mockCache.get(eq(ISSUE_KEY_OLD))).thenReturn(Collections.singletonList(oldChange));
when(mockCache.get(eq(ISSUE_KEY_NEW))).thenReturn(Collections.singletonList(newChange));
}

private GerritChange createMockChange(String key) {
GerritChange change = mock(GerritChange.class);
when(change.getSubject()).thenReturn(key);
return change;
}

@Test
Expand Down

0 comments on commit 252f47f

Please sign in to comment.