Mercurial > hg > release > icedtea-web-1.6
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; + } + +}