From 9d6f3e32ee378c0298ccf5e68efcf9d172f9dc51 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 22 Sep 2010 21:36:01 +0000 Subject: [PATCH] Track the reported size of each resource and its download progress individually. Cap the actual download size at the reported size to ensure that we don't end up borking our progress calculations if a web server returns a "clever" value for content-size like 1 or 0, but then sends back real data. Also sanitize the returned content-size in case the web server says something awesome like -42. No one can be trusted on the Interwebs, least of all web servers. --- .../threerings/getdown/launcher/Getdown.java | 23 +++--- .../threerings/getdown/net/Downloader.java | 57 +++++++++++---- .../getdown/net/HTTPDownloader.java | 70 ++++++++----------- 3 files changed, 89 insertions(+), 61 deletions(-) diff --git a/src/main/java/com/threerings/getdown/launcher/Getdown.java b/src/main/java/com/threerings/getdown/launcher/Getdown.java index ea23942..0477a0a 100644 --- a/src/main/java/com/threerings/getdown/launcher/Getdown.java +++ b/src/main/java/com/threerings/getdown/launcher/Getdown.java @@ -47,6 +47,7 @@ import java.io.PrintStream; import java.net.HttpURLConnection; import java.net.URL; +import java.net.URLConnection; import java.util.ArrayList; import java.util.HashMap; @@ -300,14 +301,20 @@ public abstract class Getdown extends Thread URL rurl = _app.getConfigResource().getRemote(); try { // try to make a HEAD request for this URL - HttpURLConnection ucon = (HttpURLConnection)rurl.openConnection(); - ucon.setRequestMethod("HEAD"); - ucon.connect(); - - // make sure we got a satisfactory response code - if (ucon.getResponseCode() != HttpURLConnection.HTTP_OK) { - log.warning("Got a non-200 response but assuming we're OK because we got " + - "something... [url=" + rurl + ", rsp=" + ucon.getResponseCode() + "]."); + URLConnection conn = rurl.openConnection(); + if (conn instanceof HttpURLConnection) { + HttpURLConnection hcon = (HttpURLConnection)conn; + try { + hcon.setRequestMethod("HEAD"); + hcon.connect(); + // make sure we got a satisfactory response code + if (hcon.getResponseCode() != HttpURLConnection.HTTP_OK) { + log.warning("Got a non-200 response but assuming we're OK because we got " + + "something...", "url", rurl, "rsp", hcon.getResponseCode()); + } + } finally { + hcon.disconnect(); + } } // we got through, so we appear not to require a proxy; make a blank proxy config and diff --git a/src/main/java/com/threerings/getdown/net/Downloader.java b/src/main/java/com/threerings/getdown/net/Downloader.java index 9f8a1ad..c8a556b 100644 --- a/src/main/java/com/threerings/getdown/net/Downloader.java +++ b/src/main/java/com/threerings/getdown/net/Downloader.java @@ -27,7 +27,9 @@ package com.threerings.getdown.net; import java.io.File; import java.io.IOException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import com.threerings.getdown.data.Resource; @@ -117,7 +119,8 @@ public abstract class Downloader extends Thread discoverSize(resource); } - log.info("Downloading " + _totalSize + " bytes..."); + long totalSize = sum(_sizes.values()); + log.info("Downloading " + totalSize + " bytes..."); // make a note of the time at which we started the download _start = System.currentTimeMillis(); @@ -154,8 +157,7 @@ public abstract class Downloader extends Thread protected void discoverSize (Resource rsrc) throws IOException { - // add this resource's size to our total download size - _totalSize += checkSize(rsrc); + _sizes.put(rsrc, Math.max(checkSize(rsrc), 0L)); } /** @@ -181,25 +183,40 @@ public abstract class Downloader extends Thread } /** - * Periodically called by the protocol-specific downloaders to update their progress. + * Periodically called by the protocol-specific downloaders to update their progress. This + * should be called at least once for each resource to be downloaded, with the total downloaded + * size for that resource. It can also be called periodically along the way for each resource + * to communicate incremental progress. + * + * @param rsrc the resource currently being downloaded. + * @param currentSize the number of bytes currently downloaded for said resource. */ - protected void updateObserver () + protected void updateObserver (Resource rsrc, long currentSize) throws IOException { + // update the current downloaded size for said resource; don't allow the downloaded bytes + // to exceed the original claimed size of the resource, otherwise our progress will get + // booched and we'll end up back on the Daily WTF: http://tinyurl.com/29wt4oq + _downloaded.put(rsrc, Math.min(_sizes.get(rsrc), currentSize)); + // notify the observer if it's been sufficiently long since our last notification long now = System.currentTimeMillis(); if ((now - _lastUpdate) >= UPDATE_DELAY) { _lastUpdate = now; + // total up our current and total bytes + long downloaded = sum(_downloaded.values()); + long totalSize = sum(_sizes.values()); + // compute our bytes per second long secs = (now - _start) / 1000L; - long bps = (secs == 0) ? 0 : (_currentSize / secs); + long bps = (secs == 0) ? 0 : (downloaded / secs); // compute our percentage completion - int pctdone = (_totalSize == 0) ? 0 : (int)((_currentSize * 100f) / _totalSize); + int pctdone = (totalSize == 0) ? 0 : (int)((downloaded * 100f) / totalSize); // estimate our time remaining - long remaining = (bps <= 0 || _totalSize == 0) ? -1 : (_totalSize - _currentSize) / bps; + long remaining = (bps <= 0 || totalSize == 0) ? -1 : (totalSize - downloaded) / bps; // make sure we only report 100% exactly once if (pctdone < 100 || !_complete) { @@ -211,6 +228,18 @@ public abstract class Downloader extends Thread } } + /** + * Sums the supplied values. + */ + protected static long sum (Iterable values) + { + long acc = 0L; + for (Long value : values) { + acc += value; + } + return acc; + } + /** * Accomplishes the copying of the resource from remote location to local location using * protocol-specific code @@ -220,18 +249,18 @@ public abstract class Downloader extends Thread /** The list of resources to be downloaded. */ protected List _resources; + /** The reported sizes of our resources. */ + protected Map _sizes = new HashMap(); + + /** The bytes downloaded for each resource. */ + protected Map _downloaded = new HashMap(); + /** The observer with whom we are communicating. */ protected Observer _obs; /** Used while downloading. */ protected byte[] _buffer = new byte[4096]; - /** The total file size in bytes to be transferred. */ - protected long _totalSize; - - /** The file size in bytes transferred thus far. */ - protected long _currentSize; - /** The time at which the file transfer began. */ protected long _start; diff --git a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java index df7c204..fa1e4a9 100644 --- a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java +++ b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java @@ -28,6 +28,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.URLConnection; import java.util.List; import com.samskivert.io.StreamUtil; @@ -46,62 +47,54 @@ public class HTTPDownloader extends Downloader super(resources, obs); } - /** - * A method of instantiating a downloader to take over the job partway. - */ - public HTTPDownloader (List resources, Observer obs, - long totalSize) - { - super(resources, obs); - _totalSize = totalSize; - _start = System.currentTimeMillis(); - } - - /** - * Issues a HEAD request for the specified resource and notes the - * amount of data we will be downloading to account for it. - */ @Override protected long checkSize (Resource rsrc) throws IOException { - // read the file information via an HTTP HEAD request - HttpURLConnection ucon = (HttpURLConnection) - rsrc.getRemote().openConnection(); - ucon.setRequestMethod("HEAD"); - ucon.connect(); + URLConnection conn = rsrc.getRemote().openConnection(); + try { + // if we're accessing our data via HTTP, we only need a HEAD request + if (conn instanceof HttpURLConnection) { + HttpURLConnection hcon = (HttpURLConnection)conn; + hcon.setRequestMethod("HEAD"); + hcon.connect(); + // make sure we got a satisfactory response code + if (hcon.getResponseCode() != HttpURLConnection.HTTP_OK) { + throw new IOException("Unable to check up-to-date for " + + rsrc.getRemote() + ": " + hcon.getResponseCode()); + } + } + return conn.getContentLength(); - // make sure we got a satisfactory response code - if (ucon.getResponseCode() != HttpURLConnection.HTTP_OK) { - String errmsg = "Unable to check up-to-date for " + - rsrc.getRemote() + ": " + ucon.getResponseCode(); - throw new IOException(errmsg); + } finally { + // let it be known that we're done with this connection + conn.getInputStream().close(); } - - return ucon.getContentLength(); } - // documentation inherited @Override protected void doDownload (Resource rsrc) throws IOException { // download the resource from the specified URL - HttpURLConnection ucon = (HttpURLConnection)rsrc.getRemote().openConnection(); - ucon.connect(); + URLConnection conn = rsrc.getRemote().openConnection(); + conn.connect(); // make sure we got a satisfactory response code - if (ucon.getResponseCode() != HttpURLConnection.HTTP_OK) { - String errmsg = "Unable to download resource " + - rsrc.getRemote() + ": " + ucon.getResponseCode(); - throw new IOException(errmsg); + if (conn instanceof HttpURLConnection) { + HttpURLConnection hcon = (HttpURLConnection)rsrc.getRemote().openConnection(); + if (hcon.getResponseCode() != HttpURLConnection.HTTP_OK) { + throw new IOException("Unable to download resource " + rsrc.getRemote() + ": " + + hcon.getResponseCode()); + } } log.info("Downloading resource [url=" + rsrc.getRemote() + "]."); InputStream in = null; FileOutputStream out = null; + long currentSize = 0L; try { - in = ucon.getInputStream(); + in = conn.getInputStream(); out = new FileOutputStream(rsrc.getLocal()); int read; @@ -114,15 +107,14 @@ public class HTTPDownloader extends Downloader // write it out to our local copy out.write(_buffer, 0, read); - // if we have no observer, then don't bother computing - // download statistics + // if we have no observer, then don't bother computing download statistics if (_obs == null) { continue; } // note that we've downloaded some data - _currentSize += read; - updateObserver(); + currentSize += read; + updateObserver(rsrc, currentSize); } } finally { StreamUtil.close(in);