From 7cfe273cb611510b0e729382788770ca9cca51f6 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Tue, 11 Nov 2014 22:55:31 -0500 Subject: [PATCH] Refactor BookDownloadManager to Kotlin I don't like that I had to make one static method a class method, but I like how much cleaner everything else is! --- .../manager/BookDownloadManagerTest.java | 14 +- .../activity/downloader/BookItemHolder.java | 6 +- .../downloader/DownloadActivityModules.java | 6 +- .../manager/BookDownloadManager.java | 163 ------------------ .../downloader/manager/BookDownloadManager.kt | 108 ++++++++++++ 5 files changed, 120 insertions(+), 177 deletions(-) delete mode 100644 app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.java create mode 100644 app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.kt diff --git a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookDownloadManagerTest.java b/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookDownloadManagerTest.java index 462a1b6..bb6d5a0 100644 --- a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookDownloadManagerTest.java +++ b/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookDownloadManagerTest.java @@ -91,7 +91,7 @@ public class BookDownloadManagerTest extends MBTestCase implements Injector { public void testJobIdMatch() { final Book toInstall = installableBooks().toBlocking().first(); - final String jobName = BookDownloadManager.getJobId(toInstall); + final String jobName = bookDownloadManager.getJobId(toInstall); final AtomicBoolean jobNameMatch = new AtomicBoolean(false); JobManager.addWorkListener(new WorkListener() { @@ -138,12 +138,6 @@ public class BookDownloadManagerTest extends MBTestCase implements Injector { prefs = mock(DownloadPrefs.class); } - @Provides - @Singleton - Injector provideInjector() { - return i; - } - @Provides @Singleton Books provideBooks() { @@ -170,5 +164,11 @@ public class BookDownloadManagerTest extends MBTestCase implements Injector { return new RefreshManager(installers, prefs, manager); } + + @Provides + @Singleton + BookDownloadManager bookDownloadManager(Books installed, RefreshManager rm) { + return new BookDownloadManager(installed, rm); + } } } \ No newline at end of file diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookItemHolder.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookItemHolder.java index 2ac34f5..49167b3 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookItemHolder.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/BookItemHolder.java @@ -35,6 +35,7 @@ public class BookItemHolder { // TODO: The holder should register and unregister itself for DownloadProgress events // so that we can display live updates. + private final Book b; @InjectView(R.id.download_txt_item_acronym) TextView acronym; @InjectView(R.id.txt_download_item_name) @@ -43,15 +44,12 @@ public class BookItemHolder { ImageButton isDownloaded; @InjectView(R.id.download_prg_download) ProgressWheel downloadProgress; - @Inject BookDownloadManager bookDownloadManager; @Inject InstalledManager installedManager; @Inject @Named("DownloadActivityContext") Context ctx; - - private final Book b; private Subscription subscription; // TODO: Factory style? @@ -64,7 +62,7 @@ public class BookItemHolder { public void bindHolder() { acronym.setText(b.getInitials()); itemName.setText(b.getName()); - DLProgressEvent dlProgressEvent = bookDownloadManager.getInProgressDownloadProgress(b); + DLProgressEvent dlProgressEvent = bookDownloadManager.getDownloadProgress(b); if (dlProgressEvent != null) { displayProgress((int) dlProgressEvent.toCircular()); } else if (installedManager.isInstalled(b)) { diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/DownloadActivityModules.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/DownloadActivityModules.java index 68664ed..2c6775d 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/DownloadActivityModules.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/DownloadActivityModules.java @@ -77,8 +77,8 @@ public class DownloadActivityModules { } @Provides @Singleton - BookDownloadManager provideBookDownloadManager() { - return new BookDownloadManager(activity); + BookDownloadManager provideBookDownloadManager(Books installedBooks, RefreshManager rm) { + return new BookDownloadManager(installedBooks, rm); } @Provides @Singleton @@ -94,7 +94,7 @@ public class DownloadActivityModules { //TODO: Move this to a true async @Provides @Singleton - Books provideInstalledBooksManager() { + Books provideInstalledBooks() { return Books.installed(); } diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.java deleted file mode 100644 index 164456f..0000000 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.java +++ /dev/null @@ -1,163 +0,0 @@ -package org.bspeice.minimalbible.activity.downloader.manager; - -import android.util.Log; - -import org.bspeice.minimalbible.Injector; -import org.crosswire.common.progress.JobManager; -import org.crosswire.common.progress.Progress; -import org.crosswire.common.progress.WorkEvent; -import org.crosswire.common.progress.WorkListener; -import org.crosswire.jsword.book.Book; -import org.crosswire.jsword.book.Books; -import org.crosswire.jsword.book.BooksEvent; -import org.crosswire.jsword.book.BooksListener; -import org.crosswire.jsword.book.install.InstallException; -import org.crosswire.jsword.book.install.Installer; - -import java.util.HashMap; -import java.util.Map; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import rx.Observable; -import rx.functions.Action1; -import rx.schedulers.Schedulers; -import rx.subjects.PublishSubject; - -/** - * Wrapper to convert JSword progress events to MinimalBible EventBus-based - */ -//TODO: Make sure that jobs have the correct name -//TODO: Install indexes for Bibles -@Singleton -public class BookDownloadManager implements WorkListener, BooksListener { - /** - * Mapping of Job ID to the EventBus we should trigger progress on - */ - private final Map bookMappings; - /** - * Cached copy of downloads in progress so views displaying this info can get it quickly. - */ - private final Map inProgressDownloads; - private final PublishSubject downloadEvents = PublishSubject.create(); - @Inject Books installedBooks; - @Inject RefreshManager refreshManager; - - @Inject - public BookDownloadManager(Injector injector) { - bookMappings = new HashMap(); - inProgressDownloads = new HashMap(); - JobManager.addWorkListener(this); - injector.inject(this); - installedBooks.addBooksListener(this); - } - - /** - * Build what the installer creates the job name as. - * Likely prone to be brittle. - * - * @param b The book to predict the download job name of - * @return The name of the job that will/is download/ing this book - */ - - public static String getJobId(Book b) { - return "INSTALL_BOOK-" + b.getInitials(); - } - - public void installBook(Book b) { - downloadBook(b); - addJob(getJobId(b), b); - downloadEvents.onNext(new DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b)); - } - - public void addJob(String jobId, Book b) { - bookMappings.put(jobId, b); - } - - public void downloadBook(final Book b) { - // So, the JobManager can't be injected, but we'll make do - - // First, look up where the Book came from - Observable.just(refreshManager.installerFromBook(b)) - .subscribeOn(Schedulers.io()) - .subscribe(new Action1>() { - @Override - public void call(Observable installerObservable) { - try { - installerObservable.toBlocking().first().install(b); - } catch (InstallException e) { - e.printStackTrace(); - } - } - }); - - getDownloadEvents() - .onNext(new DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b)); - } - - @Override - public void workProgressed(WorkEvent ev) { - Progress job = ev.getJob(); - Log.d("BookDownloadManager", "Download in progress: " + job.getJobID() + " - " + job.getJobName() + " " + job.getWorkDone() + "/" + job.getTotalWork()); - if (bookMappings.containsKey(job.getJobID())) { - Book b = bookMappings.get(job.getJobID()); - - if (job.getWorkDone() == job.getTotalWork()) { - // Download is complete - inProgressDownloads.remove(bookMappings.get(job.getJobID())); - bookMappings.remove(job.getJobID()); - downloadEvents.onNext(new DLProgressEvent(DLProgressEvent.PROGRESS_COMPLETE, b)); - } else { - // Track the ongoing download - DLProgressEvent event = new DLProgressEvent( - (job.getWorkDone() / job.getTotalWork()) * 100, - b); - inProgressDownloads.put(b, event); - downloadEvents.onNext(event); - } - } - } - - /** - * Check the status of a book download in progress. - * @param b The book to get the current progress of - * @return The most recent DownloadProgressEvent for the book, or null if not downloading - */ - public DLProgressEvent getInProgressDownloadProgress(Book b) { - if (inProgressDownloads.containsKey(b)) { - return inProgressDownloads.get(b); - } else { - return null; - } - } - - public PublishSubject getDownloadEvents() { - return downloadEvents; - } - - @Override - public void workStateChanged(WorkEvent ev) { - Log.d("BookDownloadManager", ev.toString()); - } - - @Override - public void bookAdded(BooksEvent booksEvent) { - // It's possible the install finished before we received a progress event for it, - // we handle that case here. - Book b = booksEvent.getBook(); - Log.d("BookDownloadManager", "Book added: " + b.getName()); - if (inProgressDownloads.containsKey(b)) { - inProgressDownloads.remove(b); - } - // Not sure why, but the inProgressDownloads might not have our book, - // so we always trigger the PROGRESS_COMPLETE event. - // TODO: Make sure all books get to the inProgressDownloads - downloadEvents.onNext(new DLProgressEvent(DLProgressEvent.PROGRESS_COMPLETE, b)); - } - - @Override - public void bookRemoved(BooksEvent booksEvent) { - // Not too worried about this just yet. - } -} diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.kt new file mode 100644 index 0000000..bcbda3b --- /dev/null +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.kt @@ -0,0 +1,108 @@ +package org.bspeice.minimalbible.activity.downloader.manager + +import android.util.Log; + +import org.crosswire.common.progress.JobManager; +import org.crosswire.common.progress.WorkEvent; +import org.crosswire.common.progress.WorkListener; +import org.crosswire.jsword.book.Book; +import org.crosswire.jsword.book.Books; +import org.crosswire.jsword.book.BooksEvent; +import org.crosswire.jsword.book.BooksListener; + +import rx.Observable; +import rx.schedulers.Schedulers; +import rx.subjects.PublishSubject; + +/** + * Single point of authority for what is being downloaded and its progress + */ +//TODO: Install indexes for Bibles +class BookDownloadManager(val installedBooks: Books, val rM: RefreshManager) : + WorkListener, BooksListener { + + /** + * Cached copy of downloads in progress so views displaying this info can get it quickly. + */ + // TODO: Combine to one map + var bookMappings: MutableMap = hashMapOf() + var inProgressDownloads: MutableMap = hashMapOf() + val downloadEvents: PublishSubject = PublishSubject.create(); + + { + JobManager.addWorkListener(this) + installedBooks.addBooksListener(this) + } + + /** + * Build what the installer creates the job name as. + * This technically could be static, but Kotlin won't have none of that. + * + * @param b The book to predict the download job name of + * @return The name of the job that will/is download/ing this book + */ + fun getJobId(b: Book) = "INSTALL_BOOK-" + b.getInitials() + + fun installBook(b: Book) { + downloadBook(b) + addJob(getJobId(b), b) + downloadEvents onNext DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b) + } + + fun addJob(jobId: String, b: Book) { + bookMappings.put(jobId, b) + } + + fun downloadBook(b: Book) { + // First, look up where the Book came from + Observable.just(rM installerFromBook b) + .subscribeOn(Schedulers.io()) + .subscribe { it.toBlocking().first().install(b) } + + downloadEvents onNext DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b) + } + + // TODO: I have a strange feeling I can simplify this further... + override fun workProgressed(ev: WorkEvent) { + val job = ev.getJob() + bookMappings.filter { it.getKey() == job.getJobID() } + .map { + val event = DLProgressEvent(job.getWorkDone() / job.getTotalWork() * 100, + it.getValue()) + downloadEvents onNext event + + if (job.getWorkDone() == job.getTotalWork()) { + inProgressDownloads remove bookMappings.get(job.getJobID()) + bookMappings remove job.getJobID() + } else + inProgressDownloads.put(it.getValue(), event) + } + } + + /** + * Check the status of a book download in progress. + * @param b The book to get the current progress of + * @return The most recent DownloadProgressEvent for the book, or null if not downloading + */ + fun getDownloadProgress(b: Book) = inProgressDownloads.get(b) + + override fun workStateChanged(ev: WorkEvent) { + Log.d("BookDownloadManager", ev.toString()) + } + + override fun bookAdded(booksEvent: BooksEvent) { + // It's possible the install finished before we received a progress event for it, + // we handle that case here. + val b = booksEvent.getBook() + Log.d("BookDownloadManager", "Book added: " + b.getName()) + inProgressDownloads remove b + + // Not sure why, but the inProgressDownloads might not have our book, + // so we always trigger the PROGRESS_COMPLETE event. + downloadEvents onNext DLProgressEvent(DLProgressEvent.PROGRESS_COMPLETE, b) + } + + override fun bookRemoved(booksEvent: BooksEvent) { + } +} +