/hg/icedtea-web: Reduce URLConnection knowledge in CacheEntry
omajid at icedtea.classpath.org
omajid at icedtea.classpath.org
Mon May 12 15:36:58 UTC 2014
changeset d26f2fbaaf0a in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=d26f2fbaaf0a
author: Omair Majid <omajid at redhat.com>
date: Mon May 12 11:30:47 2014 -0400
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 at 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.
diffstat:
ChangeLog | 14 +
netx/net/sourceforge/jnlp/cache/CacheEntry.java | 67 ++-
netx/net/sourceforge/jnlp/cache/ResourceTracker.java | 17 +-
tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java | 179 ++++++++++
4 files changed, 251 insertions(+), 26 deletions(-)
diffs (392 lines):
diff -r c3fb4b493d78 -r d26f2fbaaf0a ChangeLog
--- 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 at 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 at redhat.com>
ALACA is now able to remember decisions.
diff -r c3fb4b493d78 -r d26f2fbaaf0a netx/net/sourceforge/jnlp/cache/CacheEntry.java
--- 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);
}
+
}
diff -r c3fb4b493d78 -r d26f2fbaaf0a netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- 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();
diff -r c3fb4b493d78 -r d26f2fbaaf0a tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java
--- /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;
+ }
+
+}
More information about the distro-pkg-dev
mailing list