[icedtea-web] RFC: Reduce URLConnection knowledge in CacheEntry
Omair Majid
omajid at redhat.com
Fri May 2 17:23:14 UTC 2014
Hi,
I had to work on CacheEntry for a few fixes (I will be posting those
later) and the class seems to be 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.
The attached patch tries to reduce how much CacheEntry knows about
URLConnection.
Okay to push?
Thanks,
Omair
--
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
-------------- next part --------------
diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2014-05-02 Omair Majid <omajid at redhat.com>
+
+ * netx/net/sourceforge/jnlp/cache/CacheEntry.java: Use
+ constants for strings.
+ (initialize): Remove.
+ (getContentLength, setContentLength, 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.setContentLength
+ and CacheEntry.setLastModified instead of CacheEntry.initialize.
+
2014-05-02 Omair Majid <omajid at redhat.com>
* netx/net/sourceforge/jnlp/cache/package-info.java,
diff --git a/netx/net/sourceforge/jnlp/cache/CacheEntry.java b/netx/net/sourceforge/jnlp/cache/CacheEntry.java
--- a/netx/net/sourceforge/jnlp/cache/CacheEntry.java
+++ b/netx/net/sourceforge/jnlp/cache/CacheEntry.java
@@ -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() {
@@ -84,11 +75,7 @@
* most recently checked for an update.
*/
public long getLastUpdated() {
- try {
- return Long.parseLong(properties.getProperty("last-updated"));
- } catch (Exception ex) {
- return 0;
- }
+ return getLongKey(KEY_LAST_UPDATED);
}
/**
@@ -96,7 +83,35 @@
* most recently checked for an update.
*/
public void setLastUpdated(long updatedTime) {
- properties.setProperty("last-updated", Long.toString(updatedTime));
+ setLongKey(KEY_LAST_UPDATED, updatedTime);
+ }
+
+ public long getContentLength() {
+ return getLongKey(KEY_CONTENT_LENGTH);
+ }
+
+ public void setContentLength(long length) {
+ setLongKey(KEY_CONTENT_LENGTH, length);
+ }
+
+ public long getLastModified() {
+ return getLongKey(KEY_LAST_MODIFIED);
+ }
+
+ public void setLastModified(long length) {
+ setLongKey(KEY_LAST_MODIFIED, length);
+ }
+
+ 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));
}
/**
@@ -115,7 +130,7 @@
try {
long remoteModified = connection.getLastModified();
- long cachedModified = Long.parseLong(properties.getProperty("last-modified"));
+ long cachedModified = Long.parseLong(properties.getProperty(KEY_LAST_MODIFIED));
if (remoteModified > 0 && remoteModified <= cachedModified)
return true;
@@ -141,7 +156,7 @@
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;
@@ -181,4 +196,5 @@
protected void unlock() {
CacheUtil.unlockFile(properties);
}
+
}
diff --git a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
@@ -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.setContentLength(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);
@@ -823,8 +830,10 @@
}
// update cache entry
- if (!current)
- entry.initialize(connection);
+ if (!current) {
+ entry.setContentLength(connection.getContentLengthLong());
+ entry.setLastModified(connection.getLastModified());
+ }
entry.setLastUpdated(System.currentTimeMillis());
entry.store();
diff --git a/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java
new file mode 100644
--- /dev/null
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java
@@ -0,0 +1,97 @@
+/* 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 java.net.MalformedURLException;
+import java.net.URL;
+
+import net.sourceforge.jnlp.Version;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class CacheEntryTest {
+
+ 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 testLocationIsSame() {
+ CacheEntry entry = new CacheEntry(url, version);
+ assertEquals(url, entry.getLocation());
+ }
+
+ @Test
+ public void testLastModifiedIsSetCorrectly() {
+ long LAST_MODIFIED = 1000;
+
+ CacheEntry entry = new CacheEntry(url, version);
+ entry.setLastModified(LAST_MODIFIED);
+
+ assertEquals(LAST_MODIFIED, entry.getLastModified());
+ }
+
+ @Test
+ public void testLastUpdatedIsSetCorrectly() {
+ long LAST_UPDATED = 1000;
+
+ CacheEntry entry = new CacheEntry(url, version);
+ entry.setLastUpdated(LAST_UPDATED);
+
+ assertEquals(LAST_UPDATED, entry.getLastUpdated());
+ }
+
+ @Test
+ public void testContentLengthIsSetCorrectly() {
+ long CONTENT_LENGTH = 1000;
+
+ CacheEntry entry = new CacheEntry(url, version);
+ entry.setContentLength(CONTENT_LENGTH);
+
+ assertEquals(CONTENT_LENGTH, entry.getContentLength());
+ }
+
+}
More information about the distro-pkg-dev
mailing list