From 7e27c8ce2ef6987ddc26e50bcfbd5b6cc91935d5 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Thu, 6 Jan 2022 15:04:01 +0100 Subject: [PATCH] Refresh local feeds in DownloadService This allows displaying the refresh state. Also, it is faster because multiple local feeds can be refreshed in parallel. --- .../service/download/DownloadRequest.java | 21 +++++++--- .../download/DownloadRequestCreator.java | 4 +- .../service/download/DownloadService.java | 40 ++++++++++++++++--- .../download/LocalFeedStubDownloader.java | 17 ++++++++ .../antennapod/core/storage/DBTasks.java | 23 +++-------- .../service/download/DownloadRequestTest.java | 33 +++------------ 6 files changed, 80 insertions(+), 58 deletions(-) create mode 100644 core/src/main/java/de/danoeh/antennapod/core/service/download/LocalFeedStubDownloader.java 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 5dfbdf85a..789cde714 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 @@ -8,8 +8,9 @@ import android.text.TextUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import de.danoeh.antennapod.model.feed.FeedFile; +import de.danoeh.antennapod.model.feed.Feed; import de.danoeh.antennapod.core.util.URLChecker; +import de.danoeh.antennapod.model.feed.FeedMedia; public class DownloadRequest implements Parcelable { public static final String REQUEST_ARG_PAGE_NR = "page"; @@ -273,12 +274,20 @@ public class DownloadRequest implements Parcelable { private Bundle arguments = new Bundle(); private boolean initiatedByUser = true; - public Builder(@NonNull String destination, @NonNull FeedFile item) { + public Builder(@NonNull String destination, @NonNull FeedMedia media) { this.destination = destination; - this.source = URLChecker.prepareURL(item.getDownload_url()); - this.title = item.getHumanReadableIdentifier(); - this.feedfileId = item.getId(); - this.feedfileType = item.getTypeAsInt(); + this.source = URLChecker.prepareURL(media.getDownload_url()); + this.title = media.getHumanReadableIdentifier(); + this.feedfileId = media.getId(); + this.feedfileType = media.getTypeAsInt(); + } + + public Builder(@NonNull String destination, @NonNull Feed feed) { + this.destination = destination; + this.source = feed.isLocalFeed() ? feed.getDownload_url() : URLChecker.prepareURL(feed.getDownload_url()); + this.title = feed.getHumanReadableIdentifier(); + this.feedfileId = feed.getId(); + this.feedfileType = feed.getTypeAsInt(); } public void setInitiatedByUser(boolean initiatedByUser) { diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java index 80ce4620a..c33e6b4d6 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java @@ -14,13 +14,13 @@ import java.io.File; * Creates download requests that can be sent to the DownloadService. */ public class DownloadRequestCreator { - private static final String TAG = "DownloadRequester"; + private static final String TAG = "DownloadRequestCreat"; private static final String FEED_DOWNLOADPATH = "cache/"; private static final String MEDIA_DOWNLOADPATH = "media/"; public static DownloadRequest.Builder create(Feed feed) { File dest = new File(getFeedfilePath(), getFeedfileName(feed)); - if (!isFilenameAvailable(dest.toString())) { + if (!isFilenameAvailable(dest.toString()) && !feed.isLocalFeed()) { dest = findUnusedFile(dest); } Log.d(TAG, "Requesting download of url " + feed.getDownload_url()); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java index 9fb624bc2..dd445258e 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java @@ -20,6 +20,7 @@ import androidx.core.app.ServiceCompat; import androidx.core.content.ContextCompat; import de.danoeh.antennapod.core.R; +import de.danoeh.antennapod.core.feed.LocalFeedUpdater; import org.apache.commons.io.FileUtils; import org.greenrobot.eventbus.EventBus; @@ -274,6 +275,10 @@ public class DownloadService extends Service { DBTasks.autodownloadUndownloadedItems(getApplicationContext()); } + /** + * This method MUST NOT, in any case, throw an exception. + * Otherwise, it hangs up the refresh thread pool. + */ private void performDownload(Downloader downloader) { try { downloader.call(); @@ -295,6 +300,24 @@ public class DownloadService extends Service { }); } + /** + * This method MUST NOT, in any case, throw an exception. + * Otherwise, it hangs up the refresh thread pool. + */ + private void performLocalFeedRefresh(Downloader downloader, DownloadRequest request) { + try { + Feed feed = DBReader.getFeed(request.getFeedfileId()); + LocalFeedUpdater.updateFeed(feed, DownloadService.this); + } catch (Exception e) { + e.printStackTrace(); + } + downloadEnqueueExecutor.submit(() -> { + downloads.remove(downloader); + stopServiceIfEverythingDone(); + }); + } + + private void handleSuccessfulDownload(Downloader downloader) { DownloadRequest request = downloader.getDownloadRequest(); DownloadStatus status = downloader.getResult(); @@ -479,7 +502,7 @@ public class DownloadService extends Service { boolean initiatedByUser = intent.getBooleanExtra(EXTRA_INITIATED_BY_USER, false); List feeds = DBReader.getFeedList(); for (Feed feed : feeds) { - if (feed.getPreferences().getKeepUpdated() && !feed.isLocalFeed()) { + if (feed.getPreferences().getKeepUpdated()) { DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); builder.setInitiatedByUser(initiatedByUser); addNewRequest(builder.build()); @@ -494,11 +517,18 @@ public class DownloadService extends Service { Log.d(TAG, "Skipped enqueueing request. Already running."); return; } - writeFileUrl(request); - Downloader downloader = downloaderFactory.create(request); - if (downloader != null) { + Log.d(TAG, "Add new request: " + request.getSource()); + if (request.getSource().startsWith(Feed.PREFIX_LOCAL_FOLDER)) { + Downloader downloader = new LocalFeedStubDownloader(request); downloads.add(downloader); - downloadHandleExecutor.submit(() -> performDownload(downloader)); + downloadHandleExecutor.submit(() -> performLocalFeedRefresh(downloader, request)); + } else { + writeFileUrl(request); + Downloader downloader = downloaderFactory.create(request); + if (downloader != null) { + downloads.add(downloader); + downloadHandleExecutor.submit(() -> performDownload(downloader)); + } } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/LocalFeedStubDownloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/LocalFeedStubDownloader.java new file mode 100644 index 000000000..feb5fc6be --- /dev/null +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/LocalFeedStubDownloader.java @@ -0,0 +1,17 @@ +package de.danoeh.antennapod.core.service.download; + +import androidx.annotation.NonNull; + +/** + * This does not actually download, but it keeps track of a local feed's refresh state. + */ +public class LocalFeedStubDownloader extends Downloader { + + public LocalFeedStubDownloader(@NonNull DownloadRequest request) { + super(request); + } + + @Override + protected void download() { + } +} \ No newline at end of file diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java index b5bd6fca4..d1c041723 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java +++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java @@ -31,7 +31,6 @@ import de.danoeh.antennapod.core.R; import de.danoeh.antennapod.event.FeedItemEvent; import de.danoeh.antennapod.event.FeedListUpdateEvent; import de.danoeh.antennapod.event.MessageEvent; -import de.danoeh.antennapod.core.feed.LocalFeedUpdater; import de.danoeh.antennapod.core.preferences.UserPreferences; import de.danoeh.antennapod.core.service.download.DownloadStatus; import de.danoeh.antennapod.core.storage.mapper.FeedCursorMapper; @@ -114,14 +113,6 @@ public final class DBTasks { * @param initiatedByUser a boolean indicating if the refresh was triggered by user action. */ public static void refreshAllFeeds(final Context context, boolean initiatedByUser) { - new Thread(() -> { - List feeds = DBReader.getFeedList(); - for (Feed feed : feeds) { - if (feed.isLocalFeed() && feed.getPreferences().getKeepUpdated()) { - LocalFeedUpdater.updateFeed(feed, context); - } - } - }).start(); DownloadService.refreshAllFeeds(context, initiatedByUser); SharedPreferences prefs = context.getSharedPreferences(PREF_NAME, MODE_PRIVATE); @@ -169,15 +160,11 @@ public final class DBTasks { } private static void forceRefreshFeed(Context context, Feed feed, boolean loadAllPages, boolean initiatedByUser) { - if (feed.isLocalFeed()) { - new Thread(() -> LocalFeedUpdater.updateFeed(feed, context)).start(); - } else { - DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); - builder.setInitiatedByUser(initiatedByUser); - builder.setForce(true); - builder.loadAllPages(loadAllPages); - DownloadService.download(context, false, builder.build()); - } + DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); + builder.setInitiatedByUser(initiatedByUser); + builder.setForce(true); + builder.loadAllPages(loadAllPages); + DownloadService.download(context, false, builder.build()); } /** diff --git a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java index a4c5b99aa..d1bea221a 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java @@ -3,14 +3,13 @@ package de.danoeh.antennapod.core.service.download; import android.os.Bundle; import android.os.Parcel; +import de.danoeh.antennapod.model.feed.FeedMedia; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import java.util.ArrayList; -import de.danoeh.antennapod.model.feed.FeedFile; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -40,7 +39,7 @@ public class DownloadRequestTest { String destStr = "file://location/media.mp3"; String username = "testUser"; String password = "testPassword"; - FeedFile item = createFeedItem(1); + FeedMedia item = createFeedItem(1); DownloadRequest request1 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication(username, password) @@ -68,12 +67,12 @@ public class DownloadRequestTest { ArrayList toParcel; { // test DownloadRequests to parcel String destStr = "file://location/media.mp3"; - FeedFile item1 = createFeedItem(1); + FeedMedia item1 = createFeedItem(1); DownloadRequest request1 = new DownloadRequest.Builder(destStr, item1) .withAuthentication(username1, password1) .build(); - FeedFile item2 = createFeedItem(2); + FeedMedia item2 = createFeedItem(2); DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2) .withAuthentication(username2, password2) .build(); @@ -118,28 +117,8 @@ public class DownloadRequestTest { return sb.toString(); } - private FeedFile createFeedItem(final int id) { + private FeedMedia createFeedItem(final int id) { // Use mockito would be less verbose, but it'll take extra 1 second for this tiny test - return new FeedFile() { - @Override - public long getId() { - return id; - } - - @Override - public String getDownload_url() { - return "http://example.com/episode" + id; - } - - @Override - public int getTypeAsInt() { - return 0; - } - - @Override - public String getHumanReadableIdentifier() { - return "human-id-" + id; - } - }; + return new FeedMedia(id, null, 0, 0, 0, "", "", "http://example.com/episode" + id, false, null, 0, 0); } }