From 260fa5979966f25c01aab1842639c819ea859cd1 Mon Sep 17 00:00:00 2001 From: inorichi Date: Fri, 4 Dec 2015 14:35:39 +0100 Subject: [PATCH] Better error handling for downloads --- .../data/download/DownloadManager.java | 69 ++++++++++++------- .../mangafeed/data/source/base/Source.java | 6 ++ .../mangafeed/ui/reader/ReaderPresenter.java | 6 +- .../eu/kanade/mangafeed/util/DiskUtils.java | 3 +- 4 files changed, 55 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/eu/kanade/mangafeed/data/download/DownloadManager.java b/app/src/main/java/eu/kanade/mangafeed/data/download/DownloadManager.java index e8c7092536..db8f445ef5 100644 --- a/app/src/main/java/eu/kanade/mangafeed/data/download/DownloadManager.java +++ b/app/src/main/java/eu/kanade/mangafeed/data/download/DownloadManager.java @@ -168,7 +168,7 @@ public class DownloadManager { try { DiskUtils.createDirectory(download.directory); } catch (IOException e) { - Timber.e(e.getMessage()); + return Observable.error(e); } Observable> pageListObservable = download.pages == null ? @@ -182,34 +182,35 @@ public class DownloadManager { return pageListObservable .subscribeOn(Schedulers.io()) - .doOnNext(pages -> download.downloadedImages = 0) + .doOnError(error -> download.setStatus(Download.ERROR)) .doOnNext(pages -> download.setStatus(Download.DOWNLOADING)) + .doOnNext(pages -> download.downloadedImages = 0) // Get all the URLs to the source images, fetch pages if necessary - .flatMap(pageList -> Observable.from(pageList) - .filter(page -> page.getImageUrl() != null) - .mergeWith(download.source.getRemainingImageUrlsFromPageList(pageList))) + .flatMap(download.source::getAllImageUrlsFromPageList) // Start downloading images, consider we can have downloaded images already - .concatMap(page -> getDownloadedImage(page, download.source, download.directory)) - .doOnNext(p -> download.downloadedImages++) + .concatMap(page -> getOrDownloadImage(page, download)) // Do after download completes .doOnCompleted(() -> onDownloadCompleted(download)) .toList() - .flatMap(pages -> Observable.just(areAllDownloadsFinished())); + .flatMap(pages -> Observable.just(download)) + // If the page list threw, it will resume here + .onErrorResumeNext(error -> Observable.just(download)) + .map(d -> areAllDownloadsFinished()); } - // Get downloaded image if exists, otherwise download it with the method below - public Observable getDownloadedImage(final Page page, Source source, File chapterDir) { - Observable pageObservable = Observable.just(page); + // Get the image from the filesystem if it exists or download from network + private Observable getOrDownloadImage(final Page page, Download download) { + // If the image URL is empty, do nothing if (page.getImageUrl() == null) - return pageObservable; + return Observable.just(page); - String imageFilename = getImageFilename(page); - File imagePath = new File(chapterDir, imageFilename); + String filename = getImageFilename(page); + File imagePath = new File(download.directory, filename); - if (!isImageDownloaded(imagePath)) { - page.setStatus(Page.DOWNLOAD_IMAGE); - pageObservable = downloadImage(page, source, chapterDir, imageFilename); - } + // If the image is already downloaded, do nothing. Otherwise download from network + Observable pageObservable = isImageDownloaded(imagePath) ? + Observable.just(page) : + downloadImage(page, download.source, download.directory, filename); return pageObservable // When the image is ready, set image path, progress (just in case) and status @@ -217,6 +218,7 @@ public class DownloadManager { p.setImagePath(imagePath.getAbsolutePath()); p.setProgress(100); p.setStatus(Page.READY); + download.downloadedImages++; }) // If the download fails, mark this page as error .doOnError(e -> page.setStatus(Page.ERROR)) @@ -224,20 +226,39 @@ public class DownloadManager { .onErrorResumeNext(e -> Observable.just(page)); } - // Download the image and save it to the filesystem - private Observable downloadImage(final Page page, Source source, File chapterDir, String imageFilename) { + private Observable downloadImage(Page page, Source source, File directory, String filename) { + page.setStatus(Page.DOWNLOAD_IMAGE); return source.getImageProgressResponse(page) .flatMap(resp -> { try { - DiskUtils.saveBufferedSourceToDirectory(resp.body().source(), chapterDir, imageFilename); - } catch (IOException e) { - Timber.e(e.fillInStackTrace(), e.getMessage()); - throw new IllegalStateException("Unable to save image"); + DiskUtils.saveBufferedSourceToDirectory(resp.body().source(), directory, filename); + } catch (Exception e) { + return Observable.error(e); } return Observable.just(page); }); } + // Public method to get the image from the filesystem. It does NOT provide any way to download the iamge + public Observable getDownloadedImage(final Page page, File chapterDir) { + if (page.getImageUrl() == null) { + page.setStatus(Page.ERROR); + return Observable.just(page); + } + + File imagePath = new File(chapterDir, getImageFilename(page)); + + // When the image is ready, set image path, progress (just in case) and status + if (isImageDownloaded(imagePath)) { + page.setImagePath(imagePath.getAbsolutePath()); + page.setProgress(100); + page.setStatus(Page.READY); + } else { + page.setStatus(Page.ERROR); + } + return Observable.just(page); + } + // Get the filename for an image given the page private String getImageFilename(Page page) { String url; diff --git a/app/src/main/java/eu/kanade/mangafeed/data/source/base/Source.java b/app/src/main/java/eu/kanade/mangafeed/data/source/base/Source.java index 67597cc042..6608698012 100644 --- a/app/src/main/java/eu/kanade/mangafeed/data/source/base/Source.java +++ b/app/src/main/java/eu/kanade/mangafeed/data/source/base/Source.java @@ -97,6 +97,12 @@ public abstract class Source extends BaseSource { }); } + public Observable getAllImageUrlsFromPageList(final List pages) { + return Observable.from(pages) + .filter(page -> page.getImageUrl() != null) + .mergeWith(getRemainingImageUrlsFromPageList(pages)); + } + // Get the URLs of the images of a chapter public Observable getRemainingImageUrlsFromPageList(final List pages) { return Observable.from(pages) diff --git a/app/src/main/java/eu/kanade/mangafeed/ui/reader/ReaderPresenter.java b/app/src/main/java/eu/kanade/mangafeed/ui/reader/ReaderPresenter.java index 851c7613e0..b367b028dc 100644 --- a/app/src/main/java/eu/kanade/mangafeed/ui/reader/ReaderPresenter.java +++ b/app/src/main/java/eu/kanade/mangafeed/ui/reader/ReaderPresenter.java @@ -149,14 +149,12 @@ public class ReaderPresenter extends BasePresenter { Observable pageObservable; if (!isDownloaded) { - pageObservable = Observable.from(pageList) - .filter(page -> page.getImageUrl() != null) - .mergeWith(source.getRemainingImageUrlsFromPageList(pageList)) + pageObservable = source.getAllImageUrlsFromPageList(pageList) .flatMap(source::getCachedImage, 3); } else { File chapterDir = downloadManager.getAbsoluteChapterDirectory(source, manga, chapter); pageObservable = Observable.from(pageList) - .flatMap(page -> downloadManager.getDownloadedImage(page, source, chapterDir)); + .flatMap(page -> downloadManager.getDownloadedImage(page, chapterDir)); } return pageObservable .subscribeOn(Schedulers.io()) diff --git a/app/src/main/java/eu/kanade/mangafeed/util/DiskUtils.java b/app/src/main/java/eu/kanade/mangafeed/util/DiskUtils.java index 600d05b902..c89edfea4e 100644 --- a/app/src/main/java/eu/kanade/mangafeed/util/DiskUtils.java +++ b/app/src/main/java/eu/kanade/mangafeed/util/DiskUtils.java @@ -134,8 +134,9 @@ public final class DiskUtils { } catch (Exception e) { if (bufferedSink != null) { bufferedSink.close(); - writeFile.delete(); } + writeFile.delete(); + throw new IOException("Failed saving image"); } return writeFile;