From bd6c93a78bdea6ba00dc95a54bcb588e9c126ba6 Mon Sep 17 00:00:00 2001 From: Martin Fietz Date: Tue, 10 Mar 2015 18:52:46 +0100 Subject: [PATCH 1/2] Download service submits If-Modified-Since header for feeds --- .../service/download/DownloadRequest.java | 80 +++++++++++++++ .../core/service/download/HttpDownloader.java | 17 +++- .../core/storage/DownloadRequester.java | 97 +++++++++---------- 3 files changed, 144 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java index 75d6570b2..41bbd5ba6 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java @@ -6,6 +6,9 @@ import android.os.Parcelable; import org.apache.commons.lang3.Validate; +import de.danoeh.antennapod.core.feed.FeedFile; +import de.danoeh.antennapod.core.util.URLChecker; + public class DownloadRequest implements Parcelable { private final String destination; @@ -13,6 +16,7 @@ public class DownloadRequest implements Parcelable { private final String title; private String username; private String password; + private long ifModifiedSince; private boolean deleteOnFailure; private final long feedfileId; private final int feedfileType; @@ -45,12 +49,26 @@ public class DownloadRequest implements Parcelable { this(destination, source, title, feedfileId, feedfileType, null, null, true, null); } + public DownloadRequest(Builder builder) { + this.destination = builder.destination; + this.source = builder.source; + this.title = builder.title; + this.feedfileId = builder.feedfileId; + this.feedfileType = builder.feedfileType; + this.username = builder.username; + this.password = builder.password; + this.ifModifiedSince = builder.ifModifiedSince; + this.deleteOnFailure = builder.deleteOnFailure; + this.arguments = (builder.arguments != null) ? builder.arguments : new Bundle(); + } + private DownloadRequest(Parcel in) { destination = in.readString(); source = in.readString(); title = in.readString(); feedfileId = in.readLong(); feedfileType = in.readInt(); + ifModifiedSince = in.readLong(); deleteOnFailure = (in.readByte() > 0); arguments = in.readBundle(); if (in.dataAvail() > 0) { @@ -77,6 +95,7 @@ public class DownloadRequest implements Parcelable { dest.writeString(title); dest.writeLong(feedfileId); dest.writeInt(feedfileType); + dest.writeLong(ifModifiedSince); dest.writeByte((deleteOnFailure) ? (byte) 1 : 0); dest.writeBundle(arguments); if (username != null) { @@ -105,6 +124,7 @@ public class DownloadRequest implements Parcelable { DownloadRequest that = (DownloadRequest) o; + if (ifModifiedSince != that.ifModifiedSince) return false; if (deleteOnFailure != that.deleteOnFailure) return false; if (feedfileId != that.feedfileId) return false; if (feedfileType != that.feedfileType) return false; @@ -131,6 +151,7 @@ public class DownloadRequest implements Parcelable { result = 31 * result + (title != null ? title.hashCode() : 0); result = 31 * result + (username != null ? username.hashCode() : 0); result = 31 * result + (password != null ? password.hashCode() : 0); + result = 31 * result + (int)ifModifiedSince; result = 31 * result + (deleteOnFailure ? 1 : 0); result = 31 * result + (int) (feedfileId ^ (feedfileId >>> 32)); result = 31 * result + feedfileType; @@ -210,6 +231,15 @@ public class DownloadRequest implements Parcelable { this.password = password; } + public DownloadRequest setIfModifiedSince(long time) { + this.ifModifiedSince = time; + return this; + } + + public long getIfModifiedSince() { + return this.ifModifiedSince; + } + public boolean isDeleteOnFailure() { return deleteOnFailure; } @@ -217,4 +247,54 @@ public class DownloadRequest implements Parcelable { public Bundle getArguments() { return arguments; } + + public static class Builder { + private String destination; + private String source; + private String title; + private String username; + private String password; + private long ifModifiedSince; + private boolean deleteOnFailure = false; + private long feedfileId; + private int feedfileType; + private Bundle arguments; + + public Builder(String destination, FeedFile item) { + this.destination = destination; + this.source = URLChecker.prepareURL(item.getDownload_url()); + this.title = item.getHumanReadableIdentifier(); + this.feedfileId = item.getId(); + this.feedfileType = item.getTypeAsInt(); + } + + public Builder deleteOnFailure(boolean deleteOnFailure) { + this.deleteOnFailure = deleteOnFailure; + return this; + } + + public Builder ifModifiedSince(long time) { + this.ifModifiedSince = time; + return this; + } + + public Builder withAuthentication(String username, String password) { + this.username = username; + this.password = password; + return this; + } + + public DownloadRequest build() { + Validate.notNull(destination); + Validate.notNull(source); + Validate.notNull(title); + return new DownloadRequest(this); + } + + public Builder withArguments(Bundle args) { + this.arguments = args; + return this; + } + + } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java index 6bfd9b4a0..4cacb1465 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java @@ -7,6 +7,7 @@ import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; import com.squareup.okhttp.ResponseBody; +import com.squareup.okhttp.internal.http.HttpDate; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; @@ -21,6 +22,8 @@ import java.net.HttpURLConnection; import java.net.SocketTimeoutException; import java.net.URI; import java.net.UnknownHostException; +import java.text.SimpleDateFormat; +import java.util.Date; import de.danoeh.antennapod.core.BuildConfig; import de.danoeh.antennapod.core.ClientConfig; @@ -64,6 +67,12 @@ public class HttpDownloader extends Downloader { final URI uri = URIUtil.getURIFromRequestUrl(request.getSource()); Request.Builder httpReq = new Request.Builder().url(uri.toURL()) .header("User-Agent", ClientConfig.USER_AGENT); + if(request.getIfModifiedSince() > 0) { + Date date = new Date(request.getIfModifiedSince()); + Log.d(TAG, "Header If-Modified-Since: " + + new SimpleDateFormat("EEE, d MMM yyyy HH:mm:ss Z").format(date)); + httpReq.addHeader("If-Modified-Since", HttpDate.format(date)); + } // add authentication information String userInfo = uri.getUserInfo(); @@ -83,7 +92,7 @@ public class HttpDownloader extends Downloader { request.setSoFar(destination.length()); httpReq.addHeader("Range", "bytes=" + request.getSoFar() + "-"); - if (BuildConfig.DEBUG) Log.d(TAG, "Adding range header: " + request.getSoFar()); + Log.d(TAG, "Adding range header: " + request.getSoFar()); } Response response = httpClient.newCall(httpReq.build()).execute(); @@ -96,6 +105,12 @@ public class HttpDownloader extends Downloader { if (BuildConfig.DEBUG) Log.d(TAG, "Response code is " + response.code()); + if(!response.isSuccessful() && response.code() == HttpURLConnection.HTTP_NOT_MODIFIED) { + Log.d(TAG, "Feed not modified since last update, Download canceled"); + onCancelled(); + return; + } + if (!response.isSuccessful() || response.body() == null) { final DownloadError error; final String details; diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java index d0cdad649..7182cfe99 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java +++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java @@ -11,6 +11,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import java.io.File; +import java.util.Date; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -91,57 +92,53 @@ public class DownloadRequester { } private void download(Context context, FeedFile item, FeedFile container, File dest, - boolean overwriteIfExists, String username, String password, boolean deleteOnFailure, Bundle arguments) { + boolean overwriteIfExists, String username, String password, + long ifModifiedSince, boolean deleteOnFailure, Bundle arguments) { final boolean partiallyDownloadedFileExists = item.getFile_url() != null; - if (!isDownloadingFile(item)) { - if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) { - if (BuildConfig.DEBUG) - Log.d(TAG, "Filename already used."); - if (isFilenameAvailable(dest.toString()) && overwriteIfExists) { - boolean result = dest.delete(); - if (BuildConfig.DEBUG) - Log.d(TAG, "Deleting file. Result: " + result); - } else { - // find different name - File newDest = null; - for (int i = 1; i < Integer.MAX_VALUE; i++) { - String newName = FilenameUtils.getBaseName(dest - .getName()) - + "-" - + i - + FilenameUtils.EXTENSION_SEPARATOR - + FilenameUtils.getExtension(dest.getName()); - if (BuildConfig.DEBUG) - Log.d(TAG, "Testing filename " + newName); - newDest = new File(dest.getParent(), newName); - if (!newDest.exists() - && isFilenameAvailable(newDest.toString())) { - if (BuildConfig.DEBUG) - Log.d(TAG, "File doesn't exist yet. Using " - + newName); - break; - } - } - if (newDest != null) { - dest = newDest; + if (isDownloadingFile(item)) { + Log.e(TAG, "URL " + item.getDownload_url() + + " is already being downloaded"); + return; + } + if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) { + Log.d(TAG, "Filename already used."); + if (isFilenameAvailable(dest.toString()) && overwriteIfExists) { + boolean result = dest.delete(); + Log.d(TAG, "Deleting file. Result: " + result); + } else { + // find different name + File newDest = null; + for (int i = 1; i < Integer.MAX_VALUE; i++) { + String newName = FilenameUtils.getBaseName(dest + .getName()) + + "-" + + i + + FilenameUtils.EXTENSION_SEPARATOR + + FilenameUtils.getExtension(dest.getName()); + Log.d(TAG, "Testing filename " + newName); + newDest = new File(dest.getParent(), newName); + if (!newDest.exists() + && isFilenameAvailable(newDest.toString())) { + Log.d(TAG, "File doesn't exist yet. Using " + newName); + break; } } + if (newDest != null) { + dest = newDest; + } } - if (BuildConfig.DEBUG) - Log.d(TAG, - "Requesting download of url " + item.getDownload_url()); - String baseUrl = (container != null) ? container.getDownload_url() : null; - item.setDownload_url(URLChecker.prepareURL(item.getDownload_url(), baseUrl)); - - DownloadRequest request = new DownloadRequest(dest.toString(), - URLChecker.prepareURL(item.getDownload_url()), item.getHumanReadableIdentifier(), - item.getId(), item.getTypeAsInt(), username, password, deleteOnFailure, arguments); - - download(context, request); - } else { - Log.e(TAG, "URL " + item.getDownload_url() - + " is already being downloaded"); } + Log.d(TAG, "Requesting download of url " + item.getDownload_url()); + String baseUrl = (container != null) ? container.getDownload_url() : null; + item.setDownload_url(URLChecker.prepareURL(item.getDownload_url(), baseUrl)); + + DownloadRequest.Builder builder = new DownloadRequest.Builder(dest.toString(), item) + .withAuthentication(username, password) + .ifModifiedSince(ifModifiedSince) + .deleteOnFailure(deleteOnFailure) + .withArguments(arguments); + DownloadRequest request = builder.build(); + download(context, request); } /** @@ -168,13 +165,15 @@ public class DownloadRequester { if (feedFileValid(feed)) { String username = (feed.getPreferences() != null) ? feed.getPreferences().getUsername() : null; String password = (feed.getPreferences() != null) ? feed.getPreferences().getPassword() : null; + Date lastUpdate = feed.getLastUpdate(); + long ifModifiedSince = lastUpdate.getTime() - 30; // account for some processing time Bundle args = new Bundle(); args.putInt(REQUEST_ARG_PAGE_NR, feed.getPageNr()); args.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, loadAllPages); download(context, feed, null, new File(getFeedfilePath(context), - getFeedfileName(feed)), true, username, password, true, args); + getFeedfileName(feed)), true, username, password, ifModifiedSince, true, args); } } @@ -187,7 +186,7 @@ public class DownloadRequester { if (feedFileValid(image)) { FeedFile container = (image.getOwner() instanceof FeedFile) ? (FeedFile) image.getOwner() : null; download(context, image, container, new File(getImagefilePath(context), - getImagefileName(image)), false, null, null, false, null); + getImagefileName(image)), false, null, null, 0, false, null); } } @@ -213,7 +212,7 @@ public class DownloadRequester { getMediafilename(feedmedia)); } download(context, feedmedia, feed, - dest, false, username, password, false, null); + dest, false, username, password, 0, false, null); } } From ae138e2b1cb769d352bd1bb76b5c0e6af928ecb1 Mon Sep 17 00:00:00 2001 From: Martin Fietz Date: Wed, 11 Mar 2015 13:45:12 +0100 Subject: [PATCH 2/2] New feed last update is never, not now --- .../DefaultOnlineFeedViewActivity.java | 2 +- .../activity/OnlineFeedViewActivity.java | 27 ++++++++++--------- .../antennapod/asynctask/OpmlFeedQueuer.java | 11 ++++---- .../antennapod/receiver/SPAReceiver.java | 12 +++++---- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/de/danoeh/antennapod/activity/DefaultOnlineFeedViewActivity.java b/app/src/main/java/de/danoeh/antennapod/activity/DefaultOnlineFeedViewActivity.java index 8401b41a7..287ae3568 100644 --- a/app/src/main/java/de/danoeh/antennapod/activity/DefaultOnlineFeedViewActivity.java +++ b/app/src/main/java/de/danoeh/antennapod/activity/DefaultOnlineFeedViewActivity.java @@ -138,7 +138,7 @@ public class DefaultOnlineFeedViewActivity extends OnlineFeedViewActivity { @Override public void onClick(View v) { try { - Feed f = new Feed(selectedDownloadUrl, new Date(), feed.getTitle()); + Feed f = new Feed(selectedDownloadUrl, new Date(0), feed.getTitle()); f.setPreferences(feed.getPreferences()); DefaultOnlineFeedViewActivity.this.feed = f; diff --git a/app/src/main/java/de/danoeh/antennapod/activity/OnlineFeedViewActivity.java b/app/src/main/java/de/danoeh/antennapod/activity/OnlineFeedViewActivity.java index 9f028000e..3b03ed2db 100644 --- a/app/src/main/java/de/danoeh/antennapod/activity/OnlineFeedViewActivity.java +++ b/app/src/main/java/de/danoeh/antennapod/activity/OnlineFeedViewActivity.java @@ -13,9 +13,21 @@ import android.widget.ArrayAdapter; import android.widget.LinearLayout; import android.widget.ProgressBar; import android.widget.RelativeLayout; + +import org.apache.commons.lang3.StringUtils; +import org.xml.sax.SAXException; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.Map; + +import javax.xml.parsers.ParserConfigurationException; + import de.danoeh.antennapod.BuildConfig; import de.danoeh.antennapod.R; -import de.danoeh.antennapod.dialog.AuthenticationDialog; import de.danoeh.antennapod.core.feed.Feed; import de.danoeh.antennapod.core.feed.FeedPreferences; import de.danoeh.antennapod.core.preferences.UserPreferences; @@ -31,16 +43,7 @@ import de.danoeh.antennapod.core.util.FileNameGenerator; import de.danoeh.antennapod.core.util.StorageUtils; import de.danoeh.antennapod.core.util.URLChecker; import de.danoeh.antennapod.core.util.syndication.FeedDiscoverer; -import org.apache.commons.lang3.StringUtils; -import org.xml.sax.SAXException; - -import javax.xml.parsers.ParserConfigurationException; -import java.io.File; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; -import java.util.Map; +import de.danoeh.antennapod.dialog.AuthenticationDialog; /** * Downloads a feed from a feed URL and parses it. Subclasses can display the @@ -181,7 +184,7 @@ public abstract class OnlineFeedViewActivity extends ActionBarActivity { if (BuildConfig.DEBUG) Log.d(TAG, "Starting feed download"); url = URLChecker.prepareURL(url); - feed = new Feed(url, new Date()); + feed = new Feed(url, new Date(0)); if (username != null && password != null) { feed.setPreferences(new FeedPreferences(0, false, username, password)); } diff --git a/app/src/main/java/de/danoeh/antennapod/asynctask/OpmlFeedQueuer.java b/app/src/main/java/de/danoeh/antennapod/asynctask/OpmlFeedQueuer.java index cb9197b8e..00327bce0 100644 --- a/app/src/main/java/de/danoeh/antennapod/asynctask/OpmlFeedQueuer.java +++ b/app/src/main/java/de/danoeh/antennapod/asynctask/OpmlFeedQueuer.java @@ -4,16 +4,17 @@ import android.annotation.SuppressLint; import android.app.ProgressDialog; import android.content.Context; import android.os.AsyncTask; -import de.danoeh.antennapod.core.R; + +import java.util.Arrays; +import java.util.Date; + import de.danoeh.antennapod.activity.OpmlImportHolder; +import de.danoeh.antennapod.core.R; import de.danoeh.antennapod.core.feed.Feed; import de.danoeh.antennapod.core.opml.OpmlElement; import de.danoeh.antennapod.core.storage.DownloadRequestException; import de.danoeh.antennapod.core.storage.DownloadRequester; -import java.util.Arrays; -import java.util.Date; - /** Queues items for download in the background. */ public class OpmlFeedQueuer extends AsyncTask { private Context context; @@ -46,7 +47,7 @@ public class OpmlFeedQueuer extends AsyncTask { for (int idx = 0; idx < selection.length; idx++) { OpmlElement element = OpmlImportHolder.getReadElements().get( selection[idx]); - Feed feed = new Feed(element.getXmlUrl(), new Date(), + Feed feed = new Feed(element.getXmlUrl(), new Date(0), element.getText()); try { requester.downloadFeed(context.getApplicationContext(), feed); diff --git a/app/src/main/java/de/danoeh/antennapod/receiver/SPAReceiver.java b/app/src/main/java/de/danoeh/antennapod/receiver/SPAReceiver.java index 359a546f6..d15108bfe 100644 --- a/app/src/main/java/de/danoeh/antennapod/receiver/SPAReceiver.java +++ b/app/src/main/java/de/danoeh/antennapod/receiver/SPAReceiver.java @@ -5,15 +5,17 @@ import android.content.Context; import android.content.Intent; import android.util.Log; import android.widget.Toast; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Arrays; +import java.util.Date; + import de.danoeh.antennapod.BuildConfig; import de.danoeh.antennapod.R; import de.danoeh.antennapod.core.feed.Feed; import de.danoeh.antennapod.core.storage.DownloadRequestException; import de.danoeh.antennapod.core.storage.DownloadRequester; -import org.apache.commons.lang3.StringUtils; - -import java.util.Arrays; -import java.util.Date; /** * Receives intents from AntennaPod Single Purpose apps @@ -34,7 +36,7 @@ public class SPAReceiver extends BroadcastReceiver{ if (feedUrls != null) { if (BuildConfig.DEBUG) Log.d(TAG, "Received feeds list: " + Arrays.toString(feedUrls)); for (String url : feedUrls) { - Feed f = new Feed(url, new Date()); + Feed f = new Feed(url, new Date(0)); try { DownloadRequester.getInstance().downloadFeed(context, f); } catch (DownloadRequestException e) {