[JENKINS-16215] Avoiding unnecessary downloads in FilePath.installIfN…

…ecessaryFrom. The updated version uses the locally cached timestamp, '.timestamp', to set the 'If-Modified-Since' header. 304 Not Modified responses are dealt with by aborting the installation.
jenkinsci · Dec 28, 2012 · 35f7653 · 35f7653
1 parent d43b978
commit 35f7653
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -77,6 +77,7 @@
 import java.io.Writer;
 import java.io.OutputStreamWriter;
 import java.lang.reflect.Field;
+import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URL;
 import java.net.URLConnection;
@@ -682,9 +683,13 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
      */
     public boolean installIfNecessaryFrom(URL archive, TaskListener listener, String message) throws IOException, InterruptedException {
         try {
+            FilePath timestamp = this.child(".timestamp");
             URLConnection con;
             try {
                 con = ProxyConfiguration.open(archive);
+                if (timestamp.exists()) {
+                    con.setIfModifiedSince(timestamp.lastModified());
+                }
                 con.connect();
             } catch (IOException x) {
                 if (this.exists()) {
@@ -697,8 +702,13 @@ public boolean installIfNecessaryFrom(URL archive, TaskListener listener, String
                     throw x;
                 }
             }
+
+            if (con instanceof HttpURLConnection
+                    && ((HttpURLConnection)con).getResponseCode() == HttpURLConnection.HTTP_NOT_MODIFIED) {
+                return false;
+            }
+
             long sourceTimestamp = con.getLastModified();
-            FilePath timestamp = this.child(".timestamp");
 
             if(this.exists()) {
                 if(timestamp.exists() && sourceTimestamp ==timestamp.lastModified())

diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java
@@ -23,17 +23,25 @@
  */
 package hudson;
 
+import static org.mockito.Mockito.*;
 import hudson.FilePath.TarCompression;
 import hudson.model.TaskListener;
 import hudson.remoting.LocalChannel;
 import hudson.remoting.VirtualChannel;
 import hudson.util.IOException2;
 import hudson.util.NullStream;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -43,6 +51,8 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.output.NullOutputStream;
@@ -502,4 +512,69 @@ public void testDeleteLongPathOnWindows() throws Exception {
             Util.deleteRecursive(tmp);
         }
     }
+
+    @Bug(16215)
+    public void testInstallIfNecessaryAvoidsExcessiveDownloadsByUsingIfModifiedSince() throws Exception {
+        final File tmp = Util.createTempDir();
+        try {
+            final FilePath d = new FilePath(tmp);
+
+            d.child(".timestamp").touch(123000);
+
+            final HttpURLConnection con = mock(HttpURLConnection.class);
+            final URL url = someUrlToZipFile(con);
+
+            when(con.getResponseCode())
+                .thenReturn(HttpURLConnection.HTTP_NOT_MODIFIED);
+
+            assertFalse(d.installIfNecessaryFrom(url, null, null));
+
+            verify(con).setIfModifiedSince(123000);
+        } finally {
+            Util.deleteRecursive(tmp);
+        }
+    }
+
+    @Bug(16215)
+    public void testInstallIfNecessaryPerformsInstallation() throws Exception {
+        final File tmp = Util.createTempDir();
+        try {
+            final FilePath d = new FilePath(tmp);
+
+            final HttpURLConnection con = mock(HttpURLConnection.class);
+            final URL url = someUrlToZipFile(con);
+
+            when(con.getResponseCode())
+              .thenReturn(HttpURLConnection.HTTP_OK);
+
+            when(con.getInputStream())
+              .thenReturn(someZippedContent());
+
+            assertTrue(d.installIfNecessaryFrom(url, null, null));
+        } finally {
+          Util.deleteRecursive(tmp);
+        }
+    }
+
+    private URL someUrlToZipFile(final URLConnection con) throws IOException {
+
+        final URLStreamHandler urlHandler = new URLStreamHandler() {
+            @Override protected URLConnection openConnection(URL u) throws IOException {
+                return con;
+            }
+        };
+
+        return new URL("http", "some-host", 0, "/some-path.zip", urlHandler);
+    }
+
+    private InputStream someZippedContent() throws IOException {
+        final ByteArrayOutputStream buf = new ByteArrayOutputStream();
+        final ZipOutputStream zip = new ZipOutputStream(buf);
+
+        zip.putNextEntry(new ZipEntry("abc"));
+        zip.write("abc".getBytes());
+        zip.close();
+
+        return new ByteArrayInputStream(buf.toByteArray());
+    }
 }