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!

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);
+        }
     }
 }

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)) {

app/src/main/java/org/bspeice/minimalbible/activity/downloader/DownloadActivityModules.java

@@ -77,8 +77,8 @@ public class DownloadActivityModules {
     }
 
     @Provides @Singleton
-    BookDownloadManager provideBookDownloadManager() {
+    BookDownloadManager provideBookDownloadManager(Books installedBooks, RefreshManager rm) {
-        return new BookDownloadManager(activity);
+        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();
     }
 

app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/BookDownloadManager.java

@@ -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<String, Book> bookMappings;
-    /**
-     * Cached copy of downloads in progress so views displaying this info can get it quickly.
-     */
-    private final Map<Book, DLProgressEvent> inProgressDownloads;
-    private final PublishSubject<DLProgressEvent> downloadEvents = PublishSubject.create();
-    @Inject Books installedBooks;
-    @Inject RefreshManager refreshManager;
-
-    @Inject
-    public BookDownloadManager(Injector injector) {
-        bookMappings = new HashMap<String, Book>();
-        inProgressDownloads = new HashMap<Book, DLProgressEvent>();
-        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<Observable<Installer>>() {
-                    @Override
-                    public void call(Observable<Installer> 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<DLProgressEvent> 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.
-    }
-}

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<String, Book> = hashMapOf()
+    var inProgressDownloads: MutableMap<Book, DLProgressEvent> = hashMapOf()
+    val downloadEvents: PublishSubject<DLProgressEvent> = 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) {
+    }
+}
+