changeset 999:d26f2fbaaf0a

Reduce URLConnection knowledge in CacheEntry The CacheEntry class is doing a bit too much. The knowledge of URLConnection is especially troubling, since the metadata for a CacheEntry doesn't really match the URL in the presence of compression. Reduce how much CacheEntry knows about URLConnection and rely on the caller to supply appropriate information. 2014-05-12 Omair Majid <omajid@redhat.com> * netx/net/sourceforge/jnlp/cache/CacheEntry.java: Use constants for strings. (initialize): Remove. (getRemoteContentLength, setRemoteContentLength, (getLastModified, setLastModified, getLongKey, setLongKey): New method. * tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java: New file. * netx/net/sourceforge/jnlp/cache/ResourceTracker.java (initializeResource, downloadResource): Use CacheEntry.setRemoteContentLength and CacheEntry.setLastModified instead of CacheEntry.initialize.
author Omair Majid <omajid@redhat.com>
date Mon, 12 May 2014 11:30:47 -0400
parents c3fb4b493d78
children 461840384bee
files ChangeLog netx/net/sourceforge/jnlp/cache/CacheEntry.java netx/net/sourceforge/jnlp/cache/ResourceTracker.java tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java
diffstat 4 files changed, 251 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Mon May 12 17:24:34 2014 +0200
+++ b/ChangeLog	Mon May 12 11:30:47 2014 -0400
@@ -1,3 +1,17 @@
+2014-05-12  Omair Majid  <omajid@redhat.com>
+
+	* netx/net/sourceforge/jnlp/cache/CacheEntry.java: Use
+	constants for strings.
+	(initialize): Remove.
+	(getRemoteContentLength, setRemoteContentLength, getLastModified)
+	(setLastModified, getLongKey, setLongKey): New method.
+	* tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java: New
+	file.
+	* netx/net/sourceforge/jnlp/cache/ResourceTracker.java
+	(initializeResource, downloadResource): Use
+	CacheEntry.setRemoteContentLength and CacheEntry.setLastModified instead
+	of CacheEntry.initialize.
+
 2014-05-12  Jiri Vanek  <jvanek@redhat.com>
 
 	ALACA is now able to remember decisions.
--- a/netx/net/sourceforge/jnlp/cache/CacheEntry.java	Mon May 12 17:24:34 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/CacheEntry.java	Mon May 12 11:30:47 2014 -0400
@@ -23,7 +23,6 @@
 import java.net.*;
 
 import net.sourceforge.jnlp.*;
-import net.sourceforge.jnlp.runtime.*;
 import net.sourceforge.jnlp.util.*;
 
 /**
@@ -34,6 +33,10 @@
  */
 public class CacheEntry {
 
+    private static final String KEY_CONTENT_LENGTH = "content-length";
+    private static final String KEY_LAST_MODIFIED = "last-modified";
+    private static final String KEY_LAST_UPDATED = "last-updated";
+
     /** the remote resource location */
     private URL location;
 
@@ -61,18 +64,6 @@
     }
 
     /**
-     * Initialize the cache entry data from a connection to the
-     * remote resource (does not store data).
-     */
-    void initialize(URLConnection connection) {
-        long modified = connection.getLastModified();
-        long length = connection.getContentLength(); // an int
-
-        properties.setProperty("content-length", Long.toString(length));
-        properties.setProperty("last-modified", Long.toString(modified));
-    }
-
-    /**
      * Returns the remote location this entry caches.
      */
     public URL getLocation() {
@@ -85,11 +76,7 @@
      * @return 
      */
     public long getLastUpdated() {
-        try {
-            return Long.parseLong(properties.getProperty("last-updated"));
-        } catch (Exception ex) {
-            return 0;
-        }
+        return getLongKey(KEY_LAST_UPDATED);
     }
 
     /**
@@ -98,7 +85,35 @@
      * @param updatedTime
      */
     public void setLastUpdated(long updatedTime) {
-        properties.setProperty("last-updated", Long.toString(updatedTime));
+        setLongKey(KEY_LAST_UPDATED, updatedTime);
+    }
+
+    public long getRemoteContentLength() {
+        return getLongKey(KEY_CONTENT_LENGTH);
+    }
+
+    public void setRemoteContentLength(long length) {
+        setLongKey(KEY_CONTENT_LENGTH, length);
+    }
+
+    public long getLastModified() {
+        return getLongKey(KEY_LAST_MODIFIED);
+    }
+
+    public void setLastModified(long modifyTime) {
+        setLongKey(KEY_LAST_MODIFIED, modifyTime);
+    }
+
+    private long getLongKey(String key) {
+        try {
+            return Long.parseLong(properties.getProperty(key));
+        } catch (Exception ex) {
+            return 0;
+        }
+    }
+
+    private void setLongKey(String key, long value) {
+        properties.setProperty(key, Long.toString(value));
     }
 
     /**
@@ -117,7 +132,7 @@
 
         try {
             long remoteModified = lastModified;
-            long cachedModified = Long.parseLong(properties.getProperty("last-modified"));
+            long cachedModified = Long.parseLong(properties.getProperty(KEY_LAST_MODIFIED));
 
             if (remoteModified > 0 && remoteModified <= cachedModified)
                 return true;
@@ -137,13 +152,13 @@
      * @return true if the resource is in the cache
      */
     public boolean isCached() {
-        File localFile = CacheUtil.getCacheFile(location, version);
+        File localFile = getCacheFile();
         if (!localFile.exists())
             return false;
 
         try {
             long cachedLength = localFile.length();
-            long remoteLength = Long.parseLong(properties.getProperty("content-length", "-1"));
+            long remoteLength = Long.parseLong(properties.getProperty(KEY_CONTENT_LENGTH, "-1"));
 
             if (remoteLength >= 0 && cachedLength != remoteLength)
                 return false;
@@ -157,6 +172,13 @@
     }
 
     /**
+     * Seam for testing
+     */
+    File getCacheFile() {
+        return CacheUtil.getCacheFile(location, version);
+    }
+
+    /**
      * Save the current information for the cache entry.
      */
     protected void store() {
@@ -183,4 +205,5 @@
     protected void unlock() {
         CacheUtil.unlockFile(properties);
     }
+
 }
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Mon May 12 17:24:34 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Mon May 12 11:30:47 2014 -0400
@@ -695,6 +695,9 @@
                 byte buf[] = new byte[1024];
                 int rlen;
 
+                long contentLength = con.getContentLengthLong();
+                long lastModified = con.getLastModified();
+
                 InputStream in = new BufferedInputStream(con.getInputStream());
                 OutputStream out = CacheUtil.getOutputStream(downloadLocation, resource.downloadVersion);
 
@@ -710,11 +713,16 @@
                 if (con instanceof HttpURLConnection)
                     ((HttpURLConnection) con).disconnect();
 
+                if (packgz || gzip) {
+                    // TODO why not set this otherwise?
+                    downloadEntry.setRemoteContentLength(contentLength);
+                    downloadEntry.setLastModified(lastModified);
+                }
+
                 /*
                  * If the file was compressed, uncompress it.
                  */
                 if (packgz) {
-                    downloadEntry.initialize(con);
                     GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(CacheUtil
                             .getCacheFile(downloadLocation, resource.downloadVersion)));
                     InputStream inputStream = new BufferedInputStream(gzInputStream);
@@ -729,7 +737,6 @@
                     inputStream.close();
                     gzInputStream.close();
                 } else if (gzip) {
-                    downloadEntry.initialize(con);
                     GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(CacheUtil
                             .getCacheFile(downloadLocation, resource.downloadVersion)));
                     InputStream inputStream = new BufferedInputStream(gzInputStream);
@@ -828,8 +835,10 @@
             }
 
             // update cache entry
-            if (!current)
-                entry.initialize(connection);
+            if (!current) {
+                entry.setRemoteContentLength(connection.getContentLengthLong());
+                entry.setLastModified(connection.getLastModified());
+            }
 
             entry.setLastUpdated(System.currentTimeMillis());
             entry.store();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java	Mon May 12 11:30:47 2014 -0400
@@ -0,0 +1,179 @@
+/* CacheEntryTest -- unit test for CacheEntry
+   Copyright (C) 2014 Red Hat, Inc.
+
+This file is part of IcedTea.
+
+IcedTea is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License as published by
+the Free Software Foundation, version 2.
+
+IcedTea is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with IcedTea; see the file COPYING.  If not, write to
+the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version.
+ */
+
+package net.sourceforge.jnlp.cache;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.nio.file.Files;
+
+import net.sourceforge.jnlp.Version;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class CacheEntryTest {
+
+    /** A custom subclass that allows supplying a predefined cache file */
+    static class TestCacheEntry extends CacheEntry {
+        private File cacheFile;
+        public TestCacheEntry(URL location, Version version, File cacheFile) {
+            super(location, version);
+            this.cacheFile = cacheFile;
+        }
+        @Override
+        protected File getCacheFile() {
+            return cacheFile;
+        }
+    }
+
+    private URL url;
+    private Version version;
+
+    @Before
+    public void setUp() throws MalformedURLException {
+        url = new URL("http://example.com/example.jar");
+        version = new Version("1.0");
+    }
+
+    @Test
+    public void verifyLocationIsSame() {
+        CacheEntry entry = new TestCacheEntry(url, version, null);
+        assertEquals(url, entry.getLocation());
+    }
+
+    @Test
+    public void verifyLastModifiedIsSetCorrectly() {
+        long LAST_MODIFIED = 1000;
+
+        CacheEntry entry = new TestCacheEntry(url, version, null);
+        entry.setLastModified(LAST_MODIFIED);
+
+        assertEquals(LAST_MODIFIED, entry.getLastModified());
+    }
+
+    @Test
+    public void verifyLastUpdatedIsSetCorrectly() {
+        long LAST_UPDATED = 1000;
+
+        CacheEntry entry = new TestCacheEntry(url, version, null);
+        entry.setLastUpdated(LAST_UPDATED);
+
+        assertEquals(LAST_UPDATED, entry.getLastUpdated());
+    }
+
+    @Test
+    public void verifyContentLengthIsSetCorrectly() {
+        long CONTENT_LENGTH = 1000;
+
+        CacheEntry entry = new TestCacheEntry(url, version, null);
+        entry.setRemoteContentLength(CONTENT_LENGTH);
+
+        assertEquals(CONTENT_LENGTH, entry.getRemoteContentLength());
+    }
+
+    @Test
+    public void verifyNotCachedIfFileIsAbsent() {
+        File doesNotExist = new File("/foo/bar/baz/spam/eggs");
+
+        CacheEntry entry = new TestCacheEntry(url, version, doesNotExist);
+
+        assertFalse(entry.isCached());
+    }
+
+    @Test
+    public void verifyNotCachedIfContentLengthsDiffer() throws IOException {
+        File cachedFile = createCacheFile("Foo");
+
+        CacheEntry entry = new TestCacheEntry(url, version, cachedFile);
+        entry.setRemoteContentLength(10000);
+
+        assertFalse(entry.isCached());
+    }
+
+    @Test
+    public void verifyCachedIfContentLengthsAreSame() throws IOException {
+        String contents = "Foo";
+        File cachedFile = createCacheFile(contents);
+
+        CacheEntry entry = new TestCacheEntry(url, version, cachedFile);
+        entry.setRemoteContentLength(contents.length());
+
+        assertTrue(entry.isCached());
+    }
+
+    @Test
+    public void verifyCurrentWhenCacheEntryHasSameTimeStamp() throws IOException {
+        long lastModified = 10;
+        String contents = "Foo";
+        File cachedFile = createCacheFile(contents);
+
+        CacheEntry entry = new TestCacheEntry(url, version, cachedFile);
+        entry.setRemoteContentLength(contents.length());
+        entry.setLastModified(lastModified);
+
+        assertTrue(entry.isCurrent(lastModified));
+    }
+
+    @Test
+    public void verifyNotCurrentWhenRemoteContentIsNewer() throws IOException {
+        long oldTimeStamp = 10;
+        long newTimeStamp = 100;
+        String contents = "Foo";
+        File cachedFile = createCacheFile(contents);
+
+        CacheEntry entry = new TestCacheEntry(url, version, cachedFile);
+        entry.setRemoteContentLength(contents.length());
+        entry.setLastModified(oldTimeStamp);
+
+        assertFalse(entry.isCurrent(newTimeStamp));
+    }
+
+    protected File createCacheFile(String contents) throws IOException {
+        File cachedFile = File.createTempFile("CacheEntryTest", null);
+        Files.write(cachedFile.toPath(), contents.getBytes());
+        cachedFile.deleteOnExit();
+        return cachedFile;
+    }
+
+}