Merge pull request #948 from MarkEWaite/no-empty-dirs-on-cache-lookup

[JENKINS-63541] Don't create empty dirs in chooser
jenkinsci · Aug 28, 2020 · 3121391 · 3121391
2 parents 6745835 + 2cf7ef7
commit 3121391
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 4 deletions.
diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java
@@ -1219,15 +1219,23 @@ protected String getCacheEntry() {
     }
 
     protected static File getCacheDir(String cacheEntry) {
+        return getCacheDir(cacheEntry, true);
+    }
+
+    protected static File getCacheDir(String cacheEntry, boolean createDirectory) {
         Jenkins jenkins = Jenkins.getInstanceOrNull();
         if (jenkins == null) {
             return null;
         }
         File cacheDir = new File(new File(jenkins.getRootDir(), "caches"), cacheEntry);
         if (!cacheDir.isDirectory()) {
-            boolean ok = cacheDir.mkdirs();
-            if (!ok) {
-                LOGGER.log(Level.WARNING, "Failed mkdirs of {0}", cacheDir);
+            if (createDirectory) {
+                boolean ok = cacheDir.mkdirs();
+                if (!ok) {
+                    LOGGER.log(Level.WARNING, "Failed mkdirs of {0}", cacheDir);
+                }
+            } else {
+                cacheDir = null;
             }
         }
         return cacheDir;

diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java
@@ -75,7 +75,7 @@ public GitToolChooser(String remoteName, Item projectContext, String credentials
     private boolean decideAndUseCache(String remoteName) throws IOException, InterruptedException {
         boolean useCache = false;
         String cacheEntry = AbstractGitSCMSource.getCacheEntry(remoteName);
-        File cacheDir = AbstractGitSCMSource.getCacheDir(cacheEntry);
+        File cacheDir = AbstractGitSCMSource.getCacheDir(cacheEntry, false);
         if (cacheDir != null) {
             Git git = Git.with(TaskListener.NULL, new EnvVars(EnvVars.masterEnvVars)).in(cacheDir).using("git");
             GitClient client = git.getClient();

diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java
@@ -21,14 +21,17 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.TestExtension;
 import org.mockito.Mockito;
 
+import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
+import static org.hamcrest.io.FileMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.assertEquals;
@@ -420,6 +423,29 @@ public void testGitToolChooserWithAllTools_2() throws Exception {
         GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true);
         assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git"));
     }
+
+    @Test
+    @Issue("JENKINS-63541")
+    public void getCacheDirCreatesNoDirectory() throws Exception {
+        // Generate a unique repository name and compute expected cache directory
+        String remoteName = "https://github.com/jenkinsci/git-plugin-" + java.util.UUID.randomUUID().toString() + ".git";
+        String cacheEntry = AbstractGitSCMSource.getCacheEntry(remoteName);
+        File expectedCacheDir = new File(new File(jenkins.jenkins.getRootDir(), "caches"), cacheEntry);
+
+        // Directory should not exist
+        assertThat(expectedCacheDir, is(not(anExistingFileOrDirectory())));
+
+        // Getting the cache directory will not create an empty directory
+        File nullCacheDir = AbstractGitSCMSource.getCacheDir(cacheEntry, false);
+        assertThat(nullCacheDir, is(nullValue()));
+        assertThat(expectedCacheDir, is(not(anExistingFileOrDirectory())));
+
+        // Getting the cache directory will create an empty directory
+        File cacheDir = AbstractGitSCMSource.getCacheDir(cacheEntry, true);
+        assertThat(cacheDir, is(anExistingDirectory()));
+        assertThat(expectedCacheDir, is(anExistingDirectory()));
+    }
+
     /*
     A test extension implemented to clone the behavior of a plugin extending the capability of providing the size of
     repo from a remote URL of "Github".