Merge pull request #139 from markus-mnm/125-Use_reponame_for_branch_caching

125 use reponame for branch caching

src/main/java/com/dabsquared/gitlabjenkins/GitLabProjectBranchesService.java

@@ -0,0 +1,166 @@
+package com.dabsquared.gitlabjenkins;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.gitlab.api.models.GitlabBranch;
+import org.gitlab.api.models.GitlabProject;
+
+public class GitLabProjectBranchesService {
+
+    private static final Logger LOGGER = Logger.getLogger(GitLabProjectBranchesService.class.getName());
+
+    /**
+     * A map of git projects' branches; this is cached for
+     * BRANCH_CACHE_TIME_IN_MILLISECONDS ms
+     */
+    private final Map<String, BranchListEntry> projectBranchCache = new HashMap<String, BranchListEntry>();
+
+    /**
+     * length of time a git project's branch list is kept in the
+     * projectBranchCache for a particular source Repository
+     */
+    protected static final long BRANCH_CACHE_TIME_IN_MILLISECONDS = 5000;
+
+    /**
+     * a map of git projects; this is cached for
+     * PROJECT_LIST_CACHE_TIME_IN_MILLISECONDS ms
+     */
+    private HashMap<String, GitlabProject> projectMapCache = new HashMap<String, GitlabProject>();
+
+    /**
+     * length of time the list of git project is kept without being refreshed
+     * the map is also refreshed when a key hasnt been found, so we can leave
+     * the cache time high e.g. 1 day:
+     */
+    protected static final long PROJECT_MAP_CACHE_TIME_IN_MILLISECONDS = 24 * 3600 * 1000;
+
+    /**
+     * time (epoch) the project cache will have expired
+     */
+    private long projectCacheExpiry;
+
+    private final TimeUtility timeUtility;
+
+    private static transient GitLabProjectBranchesService gitLabProjectBranchesService;
+
+    public static GitLabProjectBranchesService instance() {
+        if (gitLabProjectBranchesService == null) {
+            gitLabProjectBranchesService = new GitLabProjectBranchesService(new TimeUtility());
+        }
+        return gitLabProjectBranchesService;
+    }
+
+    protected GitLabProjectBranchesService(TimeUtility timeUtility) {
+        this.timeUtility = timeUtility;
+    }
+
+    public List<String> getBranches(GitLab gitLab, String sourceRepositoryString) throws IOException {
+
+        synchronized (projectBranchCache) {
+            BranchListEntry branchListEntry = projectBranchCache.get(sourceRepositoryString);
+            if (branchListEntry != null && !branchListEntry.hasExpired()) {
+                if (LOGGER.isLoggable(Level.FINEST)) {
+                    LOGGER.log(Level.FINEST, "found branches in cache for {0}", sourceRepositoryString);
+                }
+                return branchListEntry.branchNames;
+            }
+
+            final List<String> branchNames = new ArrayList<String>();
+
+            try {
+                GitlabProject gitlabProject = findGitlabProjectForRepositoryUrl(gitLab, sourceRepositoryString);
+                if (gitlabProject != null) {
+                    final List<GitlabBranch> branches = gitLab.instance().getBranches(gitlabProject);
+                    for (final GitlabBranch branch : branches) {
+                        branchNames.add(branch.getName());
+                    }
+                    projectBranchCache.put(sourceRepositoryString, new BranchListEntry(branchNames));
+
+                    if (LOGGER.isLoggable(Level.FINEST)) {
+                        LOGGER.log(Level.FINEST, "found these branches for repo {0} : {1}",
+                                new Object[] { sourceRepositoryString, branchNames.toString() });
+                    }
+                }
+            } catch (final Error error) {
+                /* WTF WTF WTF */
+                final Throwable cause = error.getCause();
+                if (cause instanceof IOException) {
+                    throw (IOException) cause;
+                } else {
+                    throw error;
+                }
+            }
+            return branchNames;
+        }
+    }
+
+    public GitlabProject findGitlabProjectForRepositoryUrl(GitLab gitLab, String sourceRepositoryString)
+            throws IOException {
+        synchronized (projectMapCache) {
+            String repositoryUrl = sourceRepositoryString.toLowerCase();
+            if (projectCacheExpiry < timeUtility.getCurrentTimeInMillis()
+                    || !projectMapCache.containsKey(repositoryUrl)) {
+
+                if (LOGGER.isLoggable(Level.FINEST)) {
+                    LOGGER.log(Level.FINEST,
+                            "refreshing repo map for {0} because expired : {1} or missing Key {2} expiry:{3} TS:{4}",
+                            new Object[] { sourceRepositoryString,
+                                    (Boolean) (projectCacheExpiry < timeUtility.getCurrentTimeInMillis()),
+                                    (Boolean) projectMapCache.containsKey(repositoryUrl), projectCacheExpiry,
+                                    timeUtility.getCurrentTimeInMillis() });
+                }
+                refreshGitLabProjectMap(gitLab);
+            }
+            return projectMapCache.get(repositoryUrl);
+        }
+    }
+
+    public Map<String, GitlabProject> refreshGitLabProjectMap(GitLab gitLab) throws IOException {
+        synchronized (projectMapCache) {
+            try {
+                projectMapCache.clear();
+                List<GitlabProject> projects = gitLab.instance().getProjects();
+                for (GitlabProject gitlabProject : projects) {
+                    projectMapCache.put(gitlabProject.getSshUrl().toLowerCase(), gitlabProject);
+                    projectMapCache.put(gitlabProject.getHttpUrl().toLowerCase(), gitlabProject);
+                }
+                projectCacheExpiry = timeUtility.getCurrentTimeInMillis() + PROJECT_MAP_CACHE_TIME_IN_MILLISECONDS;
+            } catch (final Error error) {
+                final Throwable cause = error.getCause();
+                if (cause instanceof IOException) {
+                    throw (IOException) cause;
+                } else {
+                    throw error;
+                }
+            }
+            return projectMapCache;
+        }
+    }
+
+    public class BranchListEntry {
+        long expireTimestamp;
+        List<String> branchNames;
+
+        public BranchListEntry(List<String> branchNames) {
+            this.branchNames = branchNames;
+            this.expireTimestamp = timeUtility.getCurrentTimeInMillis() + BRANCH_CACHE_TIME_IN_MILLISECONDS;
+        }
+
+        boolean hasExpired() {
+            return expireTimestamp < timeUtility.getCurrentTimeInMillis();
+        }
+    }
+
+    public static class TimeUtility {
+        public long getCurrentTimeInMillis() {
+            return System.currentTimeMillis();
+        }
+    }
+
+}

src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java

@@ -45,7 +45,6 @@ import net.sf.json.JSONObject;
 import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.transport.RemoteConfig;
 import org.eclipse.jgit.transport.URIish;
-import org.gitlab.api.models.GitlabBranch;
 import org.gitlab.api.models.GitlabProject;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -226,11 +225,11 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
             getDescriptor().queue.execute(new Runnable() {
 
                 public void run() {
-            		LOGGER.log(Level.INFO, "{0} triggered for push.", job.getName());
+                    LOGGER.log(Level.INFO, "{0} triggered for push.", job.getFullName());
 
             		String name = " #" + job.getNextBuildNumber();
             		GitLabPushCause cause = createGitLabPushCause(req);
-            		Action[] actions = createActions(req);
+                    Action[] actions = createActions(req);
 
                     boolean scheduled;
 
@@ -242,11 +241,13 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                         scheduled = scheduledJob.scheduleBuild(cause);
                     }
 
-            		if (scheduled) {
+                    if (scheduled) {
-            			LOGGER.log(Level.INFO, "GitLab Push Request detected in {0}. Triggering {1}", new String[]{job.getName(), name});
+                        LOGGER.log(Level.INFO, "GitLab Push Request detected in {0}. Triggering {1}",
-            		} else {
+                                new String[] { job.getFullName(), name });
-            			LOGGER.log(Level.INFO, "GitLab Push Request detected in {0}. Job is already in the queue.", job.getName());
+                    } else {
-            		}
+                        LOGGER.log(Level.INFO, "GitLab Push Request detected in {0}. Job is already in the queue.",
+                                job.getFullName());
+                    }
 
                     if(addCiMessage) {
                         req.createCommitStatus(getDescriptor().getGitlab().instance(), "pending", Jenkins.getInstance().getRootUrl() + job.getUrl());
@@ -282,7 +283,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                     values.put("gitlabMergeRequestId", new StringParameterValue("gitlabMergeRequestId", ""));
                     values.put("gitlabMergeRequestAssignee", new StringParameterValue("gitlabMergeRequestAssignee", ""));
 
-                    LOGGER.log(Level.INFO, "Trying to get name and URL for job: {0}", job.getName());
+                    LOGGER.log(Level.INFO, "Trying to get name and URL for job: {0}", job.getFullName());
                     String sourceRepoName = getDesc().getSourceRepoNameDefault(job);
                     String sourceRepoURL = getDesc().getSourceRepoURLDefault(job).toString();
 
@@ -315,7 +316,6 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
 
                     return actionsArray;
                 }
-
             });
         }
     }
@@ -353,7 +353,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
 
     		getDescriptor().queue.execute(new Runnable() {
                 public void run() {
-	                LOGGER.log(Level.INFO, "{0} triggered for merge request.", job.getName());
+	                LOGGER.log(Level.INFO, "{0} triggered for merge request.", job.getFullName());
                   String name = " #" + job.getNextBuildNumber();
 
 	                GitLabMergeCause cause = createGitLabMergeCause(req);
@@ -375,9 +375,9 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                     }
 
                     if (scheduled) {
-	                    LOGGER.log(Level.INFO, "GitLab Merge Request detected in {0}. Triggering {1}", new String[]{job.getName(), name});
+	                    LOGGER.log(Level.INFO, "GitLab Merge Request detected in {0}. Triggering {1}", new String[]{job.getFullName(), name});
 	                } else {
-	                    LOGGER.log(Level.INFO, "GitLab Merge Request detected in {0}. Job is already in the queue.", job.getName());
+	                    LOGGER.log(Level.INFO, "GitLab Merge Request detected in {0}. Job is already in the queue.", job.getFullName());
 	                }
 
                     if(addCiMessage) {
@@ -413,7 +413,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                     }
 
 
-                    LOGGER.log(Level.INFO, "Trying to get name and URL for job: {0}", job.getName());
+                    LOGGER.log(Level.INFO, "Trying to get name and URL for job: {0}", job.getFullName());
                     String sourceRepoName = getDesc().getSourceRepoNameDefault(job);
                     String sourceRepoURL = getDesc().getSourceRepoURLDefault(job).toString();
 
@@ -651,8 +651,6 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
         private transient final SequentialExecutionQueue queue = new SequentialExecutionQueue(Jenkins.MasterComputer.threadPoolForRemoting);
         private transient GitLab gitlab;
 
-        private final Map<String, List<String>> projectBranches = new HashMap<String, List<String>>();
-
         public DescriptorImpl() {
         	load();
         }
@@ -714,10 +712,6 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
         }
 
         private List<String> getProjectBranches(final Job<?, ?> job) throws IOException, IllegalStateException {
-            if (projectBranches.containsKey(job.getName())){
-                return projectBranches.get(job.getName());
-            }
-
             if (!(job instanceof AbstractProject<?, ?>)) {
                 return Lists.newArrayList();
             }
@@ -728,36 +722,12 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                 throw new IllegalStateException(Messages.GitLabPushTrigger_NoSourceRepository());
             }
 
-            try {
+            if (!getGitlabHostUrl().isEmpty()) {
-                final List<String> branchNames = new ArrayList<String>();
+                return GitLabProjectBranchesService.instance().getBranches(getGitlab(), sourceRepository.toString());
-                if (!gitlabHostUrl.isEmpty()) {
+            } else {
-                    /* TODO until java-gitlab-api v1.1.5 is released,
+                LOGGER.log(Level.WARNING, "getProjectBranches: gitlabHostUrl hasn't been configured globally. Job {0}.",
-                     * cannot search projects by namespace/name
+                        job.getFullName());
-                     * For now getting project id before getting project branches */
+                return Lists.newArrayList();
-                    final List<GitlabProject> projects = getGitlab().instance().getProjects();
-                    for (final GitlabProject gitlabProject : projects) {
-                        if (gitlabProject.getSshUrl().equalsIgnoreCase(sourceRepository.toString())
-                            || gitlabProject.getHttpUrl().equalsIgnoreCase(sourceRepository.toString())) {
-                            //Get all branches of project
-                            final List<GitlabBranch> branches = getGitlab().instance().getBranches(gitlabProject);
-                            for (final GitlabBranch branch : branches) {
-                                branchNames.add(branch.getName());
-                            }
-                            break;
-                        }
-                    }
-                }
-
-                projectBranches.put(job.getName(), branchNames);
-                return branchNames;
-            } catch (final Error error) {
-                /* WTF WTF WTF */
-                final Throwable cause = error.getCause();
-                if (cause instanceof IOException) {
-                    throw (IOException) cause;
-                } else {
-                    throw error;
-                }
             }
         }
 
@@ -787,8 +757,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                 // show all suggestions for short strings
                 if (query.length() < 2){
                     values.addAll(branches);
-                }
+                } else {
-                else {
                     for (String branch : branches){
                       if (branch.toLowerCase().indexOf(query) > -1){
                         values.add(branch);
@@ -796,9 +765,9 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
                     }
                 }
             } catch (final IllegalStateException ex) {
-                /* no-op */
+                LOGGER.log(Level.FINEST, "Unexpected IllegalStateException. Please check the logs and your configuration.", ex);
             } catch (final IOException ex) {
-                /* no-op */
+                LOGGER.log(Level.FINEST, "Unexpected IllegalStateException. Please check the logs and your configuration.", ex);
             }
 
             return ac;

src/test/java/com/dabsquared/gitlabjenkins/GitLabProjectBranchesServiceTest.java

@@ -0,0 +1,206 @@
+package com.dabsquared.gitlabjenkins;
+
+import static java.util.Arrays.asList;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.gitlab.api.GitlabAPI;
+import org.gitlab.api.models.GitlabBranch;
+import org.gitlab.api.models.GitlabNamespace;
+import org.gitlab.api.models.GitlabProject;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.dabsquared.gitlabjenkins.GitLabProjectBranchesService.TimeUtility;
+
+public class GitLabProjectBranchesServiceTest {
+
+    private GitLabProjectBranchesService branchesService;
+
+    private GitlabAPI gitlabApi;
+    private GitLab gitLab;
+    private TimeUtility timeUtility;
+
+    private GitlabProject gitlabProjectA;
+    private GitlabProject gitlabProjectB;
+
+    private List<String> branchNamesProjectA;
+    private List<String> branchNamesProjectB;
+
+    @Before
+    @SuppressWarnings("unchecked")
+    public void setUp() throws IOException {
+
+        // some test data
+        gitlabProjectA = setupGitlabProject("groupOne", "A");
+        gitlabProjectB = setupGitlabProject("groupOne", "B");
+
+        branchNamesProjectA = asList("master", "A-branch-1");
+        branchNamesProjectB = asList("master", "B-branch-1", "B-branch-2");
+
+        // mock the gitlab factory
+        gitLab = mockGitlab(asList(gitlabProjectA, gitlabProjectB), asList(branchNamesProjectA, branchNamesProjectB));
+
+        // never expire cache for tests
+        timeUtility = mock(TimeUtility.class);
+        when(timeUtility.getCurrentTimeInMillis()).thenReturn(1L);
+
+        branchesService = new GitLabProjectBranchesService(timeUtility);
+    }
+
+    @Test
+    public void shouldReturnProjectFromGitlabApi() throws Exception {
+        // when
+        GitlabProject gitlabProject = branchesService.findGitlabProjectForRepositoryUrl(gitLab,
+                "git@git.example.com:groupOne/A.git");
+
+        // then
+        assertThat(gitlabProject, is(gitlabProjectA));
+    }
+
+    @Test
+    public void shouldReturnBranchNamesFromGitlabApi() throws Exception {
+        // when
+        List<String> actualBranchNames = branchesService.getBranches(gitLab, "git@git.example.com:groupOne/B.git");
+
+        // then
+        assertThat(actualBranchNames, is(branchNamesProjectB));
+    }
+
+    @Test
+    public void shouldNotCallGitlabApiGetProjectsWhenElementIsCached() throws Exception {
+        // when
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/A.git");
+        verify(gitlabApi, times(1)).getProjects();
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/B.git");
+
+        // then
+        verify(gitlabApi, times(1)).getProjects();
+    }
+
+    @Test
+    public void shouldCallGitlabApiGetProjectsWhenElementIsNotCached() throws Exception {
+        // when
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/A.git");
+        verify(gitlabApi, times(1)).getProjects();
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/DoesNotExist.git");
+
+        // then
+        verify(gitlabApi, times(2)).getProjects();
+    }
+
+    @Test
+    public void shoulNotCallGitlabApiGetBranchesWhenElementIsCached() throws Exception {
+        // when
+        branchesService.getBranches(gitLab, "git@git.example.com:groupOne/B.git");
+        verify(gitlabApi, times(1)).getBranches(gitlabProjectB);
+        branchesService.getBranches(gitLab, "git@git.example.com:groupOne/B.git");
+
+        // then
+        verify(gitlabApi, times(1)).getProjects();
+    }
+
+    @Test
+    public void shoulNotMakeUnnecessaryCallsToGitlabApiGetBranches() throws Exception {
+        // when
+        branchesService.getBranches(gitLab, "git@git.example.com:groupOne/A.git");
+
+        // then
+        verify(gitlabApi, times(1)).getBranches(gitlabProjectA);
+        verify(gitlabApi, times(0)).getBranches(gitlabProjectB);
+    }
+
+    @Test
+    public void shouldExpireBranchCacheAtSetTime() throws Exception {
+        // first call should retrieve branches from gitlabApi
+        branchesService.getBranches(gitLab, "git@git.example.com:groupOne/A.git");
+        verify(gitlabApi, times(1)).getBranches(gitlabProjectA);
+
+        long timeAfterCacheExpiry = GitLabProjectBranchesService.BRANCH_CACHE_TIME_IN_MILLISECONDS + 2;
+        when(timeUtility.getCurrentTimeInMillis()).thenReturn(timeAfterCacheExpiry);
+        branchesService.getBranches(gitLab, "git@git.example.com:groupOne/A.git");
+
+        // then
+        verify(gitlabApi, times(2)).getBranches(gitlabProjectA);
+    }
+
+    @Test
+    public void shouldExpireProjectCacheAtSetTime() throws Exception {
+        // first call should retrieve projects from gitlabApi
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/A.git");
+        verify(gitlabApi, times(1)).getProjects();
+
+        long timeAfterCacheExpiry = GitLabProjectBranchesService.PROJECT_MAP_CACHE_TIME_IN_MILLISECONDS + 2;
+        when(timeUtility.getCurrentTimeInMillis()).thenReturn(timeAfterCacheExpiry);
+        branchesService.findGitlabProjectForRepositoryUrl(gitLab, "git@git.example.com:groupOne/A.git");
+
+        // then
+        verify(gitlabApi, times(2)).getProjects();
+    }
+
+    /**
+     * mocks calls to GitLab.instance() and GitlabAPI.getProjects and GitlabAPI.getBranches(gitlabProject)
+     *
+     * projectList has to have the size as the branchNamesList list.
+     *
+     * Each branchNamesList entry is a list of strings that is used to create a list of GitlabBranch elements; that list
+     * is then returned for each gitlabProject.
+     *
+     * @param projectList
+     *            returned for GitlabAPI.getProjects
+     * @param branchNamesList
+     *            an array of lists of branch names used to mock getBranches
+     * @return a mocked gitlabAPI
+     * @throws IOException
+     */
+    private GitLab mockGitlab(List<GitlabProject> projectList, List<List<String>> branchNamesList) throws IOException {
+        // mock the actual API
+        gitlabApi = mock(GitlabAPI.class);
+
+        // mock the gitlab API factory
+        GitLab gitLab = mock(GitLab.class);
+        when(gitLab.instance()).thenReturn(gitlabApi);
+
+        when(gitlabApi.getProjects()).thenReturn(projectList);
+
+        List<GitlabBranch> branchList;
+        for (int i = 0; i < branchNamesList.size(); i++) {
+            branchList = createGitlabBranches(projectList.get(i), branchNamesList.get(1));
+            when(gitlabApi.getBranches(projectList.get(i))).thenReturn(branchList);
+        }
+        return gitLab;
+    }
+
+    private List<GitlabBranch> createGitlabBranches(GitlabProject gitlabProject, List<String> branchNames) {
+        List<GitlabBranch> branches = new ArrayList<GitlabBranch>();
+        GitlabBranch branch;
+        for (String branchName : branchNames) {
+            branch = new GitlabBranch();
+            branch.setName(branchName);
+            branches.add(branch);
+        }
+        return branches;
+    }
+
+    private GitlabProject setupGitlabProject(String namespace, String name) {
+        GitlabProject project = new GitlabProject();
+        project.setPathWithNamespace(namespace + "/" + name);
+        project.setHttpUrl("http://git.example.com/" + project.getPathWithNamespace() + ".git");
+        project.setSshUrl("git@git.example.com:" + project.getPathWithNamespace() + ".git");
+        project.setName(name);
+        GitlabNamespace gitNameSpace = new GitlabNamespace();
+        gitNameSpace.setName(namespace);
+        gitNameSpace.setPath(namespace);
+        project.setNamespace(gitNameSpace);
+        return project;
+    }
+
+}