diff --git a/app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java b/app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java index 667bc05..766ed39 100644 --- a/app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java +++ b/app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java @@ -2,7 +2,6 @@ package org.bspeice.minimalbible.test.activity.downloader.manager; import android.net.ConnectivityManager; import android.net.NetworkInfo; -import android.util.Log; import org.bspeice.minimalbible.Injector; import org.bspeice.minimalbible.activity.downloader.DownloadPrefs; @@ -40,6 +39,7 @@ import dagger.Provides; import rx.Observable; import rx.functions.Action1; import rx.functions.Func1; +import rx.subjects.PublishSubject; import static com.jayway.awaitility.Awaitility.await; import static org.junit.Assert.assertFalse; @@ -82,38 +82,42 @@ public class BookManagerTest implements Injector { }); } - @Ignore + // TODO: Why doesn't this work? + @Ignore("Should be working, but isn't...") + @Test public void testInstallBook() throws Exception { final Book toInstall = installableBooks().toBlocking().first(); - bookManager.installBook(toInstall); - final AtomicBoolean signal = new AtomicBoolean(false); bookManager.getDownloadEvents() .subscribe(new Action1() { @Override public void call(DLProgressEvent dlProgressEvent) { + System.out.println(dlProgressEvent.getAverageProgress()); if (dlProgressEvent.getB().getInitials().equals(toInstall.getInitials()) - && dlProgressEvent.getProgress() == DLProgressEvent.PROGRESS_COMPLETE) { + && dlProgressEvent.getAverageProgress() == DLProgressEvent.PROGRESS_COMPLETE) { signal.set(true); } } }); - await().atMost(60, TimeUnit.SECONDS) + bookManager.downloadBook(toInstall); + + await().atMost(30, TimeUnit.SECONDS) .untilTrue(signal); } - @Ignore + // TODO: Why doesn't this work? + @Ignore("Should be working, but isn't...") + @Test public void testJobIdMatch() { final Book toInstall = installableBooks().toBlocking().first(); - final String jobName = bookManager.getJobId(toInstall); + final String jobName = bookManager.getJobNames(toInstall).get(0); final AtomicBoolean jobNameMatch = new AtomicBoolean(false); JobManager.addWorkListener(new WorkListener() { @Override public void workProgressed(WorkEvent ev) { - Log.d("testJobIdMatch", ev.getJob().getJobID() + " " + jobName); if (ev.getJob().getJobID().equals(jobName)) { jobNameMatch.set(true); } @@ -124,8 +128,8 @@ public class BookManagerTest implements Injector { } }); - bookManager.installBook(toInstall); - await().atMost(5, TimeUnit.SECONDS) + bookManager.downloadBook(toInstall); + await().atMost(10, TimeUnit.SECONDS) .untilTrue(jobNameMatch); } @@ -169,20 +173,19 @@ public class BookManagerTest implements Injector { public void testWorkProgressedCorrectProgress() { Book mockBook = mock(Book.class); when(mockBook.getInitials()).thenReturn("mockBook"); - String bookJobName = bookManager.getJobId(mockBook); - bookManager.getBookMappings().put(bookJobName, mockBook); + String bookJobName = bookManager.getJobNames(mockBook).get(0); + bookManager.getInProgressJobNames().put(bookJobName, mockBook); // Percent to degrees - int workProgress = 1; - int totalWork = 2; - final int circularProgress = 180; + final int workDone = 50; // 50% + // There are two jobs, each comprising 180 degrees. + // Since we are simulating one job being 50% complete, that's 90 degrees + final int circularProgress = 90; WorkEvent ev = mock(WorkEvent.class); Progress p = mock(Progress.class); when(p.getJobID()).thenReturn(bookJobName); - when(p.getWorkDone()).thenReturn(workProgress); - when(p.getTotalWork()).thenReturn(totalWork); - + when(p.getWork()).thenReturn(workDone); when(ev.getJob()).thenReturn(p); final AtomicBoolean progressCorrect = new AtomicBoolean(false); @@ -259,8 +262,15 @@ public class BookManagerTest implements Injector { @Provides @Singleton - BookManager bookDownloadManager(Books installed, RefreshManager rm) { - return new BookManager(installed, rm); + PublishSubject dlProgressEventPublisher() { + return PublishSubject.create(); + } + + @Provides + @Singleton + BookManager bookDownloadManager(Books installed, RefreshManager rm, + PublishSubject eventPublisher) { + return new BookManager(installed, rm, eventPublisher); } } } \ No newline at end of file diff --git a/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEventSpek.kt b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEventSpek.kt index 8cfc34c..b57d79b 100644 --- a/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEventSpek.kt +++ b/app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEventSpek.kt @@ -12,7 +12,7 @@ class DLProgressEventSpek : Spek() {{ given("a DLProgressEvent created with 50% progress and a mock book") { val mockBook = mock(javaClass()) - val dlEvent = DLProgressEvent(50, mockBook) + val dlEvent = DLProgressEvent(50, 50, mockBook) on("getting the progress in degrees") { val progressDegrees = dlEvent.toCircular() 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 4f894a3..5b520ea 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 @@ -25,6 +25,7 @@ import rx.Subscription; import rx.android.schedulers.AndroidSchedulers; import rx.functions.Action1; import rx.functions.Func1; +import rx.subjects.PublishSubject; /** * Created by bspeice on 5/20/14. @@ -47,6 +48,9 @@ public class BookItemHolder { BookManager bookManager; @Inject @Named("DownloadActivityContext") Context ctx; + @Inject + PublishSubject downloadProgressEvents; + private Subscription subscription; // TODO: Factory style? @@ -61,12 +65,13 @@ public class BookItemHolder { itemName.setText(b.getName()); DLProgressEvent dlProgressEvent = bookManager.getDownloadProgress(b); if (dlProgressEvent != null) { - displayProgress((int) dlProgressEvent.toCircular()); + displayProgress(dlProgressEvent.toCircular()); } else if (bookManager.isInstalled(b)) { displayInstalled(); } + //TODO: Refactor - subscription = bookManager.getDownloadEvents() + subscription = downloadProgressEvents .observeOn(AndroidSchedulers.mainThread()) .filter(new Func1() { @Override @@ -98,7 +103,7 @@ public class BookItemHolder { , Toast.LENGTH_SHORT).show(); } } else { - bookManager.installBook(this.b); + bookManager.downloadBook(this.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 55b8c1e..d0a9eda 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 @@ -6,6 +6,7 @@ import android.net.ConnectivityManager; import org.bspeice.minimalbible.Injector; import org.bspeice.minimalbible.MinimalBibleModules; import org.bspeice.minimalbible.activity.downloader.manager.BookManager; +import org.bspeice.minimalbible.activity.downloader.manager.DLProgressEvent; import org.bspeice.minimalbible.activity.downloader.manager.LocaleManager; import org.bspeice.minimalbible.activity.downloader.manager.RefreshManager; import org.crosswire.jsword.book.BookCategory; @@ -23,6 +24,7 @@ import javax.inject.Singleton; import dagger.Module; import dagger.Provides; import de.devland.esperandro.Esperandro; +import rx.subjects.PublishSubject; /** * Module mappings for the classes under the Download Activity @@ -79,8 +81,15 @@ public class DownloadActivityModules { @Provides @Singleton - BookManager provideBookDownloadManager(Books installedBooks, RefreshManager rm) { - return new BookManager(installedBooks, rm); + PublishSubject dlProgressEventPublisher() { + return PublishSubject.create(); + } + + @Provides + @Singleton + BookManager provideBookDownloadManager(Books installedBooks, RefreshManager rm, + PublishSubject progressEvents) { + return new BookManager(installedBooks, rm, progressEvents); } @Provides diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookManager.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookManager.kt index 6a31f69..c0c02ac 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookManager.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/BookManager.kt @@ -12,33 +12,41 @@ import rx.Observable; import rx.schedulers.Schedulers; import rx.subjects.PublishSubject; import org.crosswire.jsword.book.BookException +import org.crosswire.jsword.util.IndexDownloader +import org.crosswire.common.progress.Progress /** * Single point of authority for what is being downloaded and its progress * Please note that you should never be modifying installedBooks, * only operate on installedBooksList */ -//TODO: Install indexes for Bibles -//TODO: Figure out how to get Robolectric to mock the Log, rather than removing the calls -class BookManager(private val installedBooks: Books, val rM: RefreshManager) : +class BookManager(private val installedBooks: Books, + val rM: RefreshManager, + val downloadEvents: PublishSubject) : WorkListener, BooksListener { + private val bookJobNamePrefix = Progress.INSTALL_BOOK.substringBeforeLast("%s") + private val indexJobNamePrefix = Progress.DOWNLOAD_SEARCH_INDEX.substringBeforeLast("%s") + + /** + * List of jobs currently active by their job name + */ + val inProgressJobNames: MutableMap = hashMapOf() + /** * Cached copy of downloads in progress so views displaying this info can get it quickly. */ - // TODO: Combine to one map - val bookMappings: MutableMap = hashMapOf() val inProgressDownloads: MutableMap = hashMapOf() /** * A list of books that is locally maintained - installedBooks isn't always up-to-date */ - val installedBooksList: MutableList = installedBooks.getBooks() ?: linkedListOf() - val downloadEvents: PublishSubject = PublishSubject.create(); + val installedBooksList: MutableList = installedBooks.getBooks() ?: linkedListOf(); { JobManager.addWorkListener(this) installedBooks.addBooksListener(this) + downloadEvents.subscribe { this.inProgressDownloads[it.b] = it } } /** @@ -48,25 +56,35 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : * @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 getJobNames(b: Book) = listOf("${bookJobNamePrefix}${b.getInitials()}", + "${indexJobNamePrefix}${b.getInitials()}") fun downloadBook(b: Book) { // First, look up where the Book came from - Observable.just(rM installerFromBook b) - .observeOn(Schedulers.io()) - .subscribe { it subscribe { it install b } } + val installerObs = Observable.just(rM installerFromBook b) - downloadEvents onNext DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b) + // And subscribe on two different threads for the download + // Not sure why we need two threads, guessing it's because the + // thread is closed when the install event is done + installerObs + .observeOn(Schedulers.newThread()) + .subscribe { + // Download the actual book + it subscribe { it install b } + } + + installerObs + .observeOn(Schedulers.newThread()) + .subscribe { + // Download the book index + it subscribe { IndexDownloader.downloadIndex(b, it) } + } + + // Then notify everyone that we're starting + downloadEvents onNext DLProgressEvent.beginningEvent(b) + + // Finally register the jobs in progress + getJobNames(b).forEach { this.inProgressJobNames[it] = b } } /** @@ -100,6 +118,7 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : return false } } + /** * Check the status of a book download in progress. * @param b The book to get the current progress of @@ -112,20 +131,24 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : // 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 { - // We multiply by 100 first to avoid integer truncation - // Also avoids roundoff error. Neat trick, but I'm spending just as much time - // documenting it as implementing the floating point would take - val event = DLProgressEvent(job.getWorkDone() * 100 / job.getTotalWork(), it.getValue()) - downloadEvents onNext event - if (job.getWorkDone() == job.getTotalWork()) { - inProgressDownloads remove bookMappings[job.getJobID()] - bookMappings remove job.getJobID() - } else - inProgressDownloads.put(it.getValue(), event) - } + val book = inProgressJobNames[job.getJobID()] as Book + val oldEvent = inProgressDownloads[book] ?: DLProgressEvent.beginningEvent(book) + + var newEvent: DLProgressEvent + if (job.getJobID().contains(bookJobNamePrefix)) + newEvent = oldEvent.copy(bookProgress = job.getWork()) + else + newEvent = oldEvent.copy(indexProgress = job.getWork()) + + downloadEvents onNext newEvent + + if (newEvent.averageProgress == DLProgressEvent.PROGRESS_COMPLETE) { + inProgressDownloads remove inProgressJobNames[job.getJobID()] + inProgressJobNames remove job.getJobID() + } else + inProgressDownloads.put(book, newEvent) + } override fun workStateChanged(ev: WorkEvent) { @@ -133,18 +156,8 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : } 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) - - // And update the locally available list - installedBooksList add b + // Update the local list of available books + installedBooksList add booksEvent.getBook() } override fun bookRemoved(booksEvent: BooksEvent) { diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEvent.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEvent.kt index 5bfd1dd..7da47d2 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEvent.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEvent.kt @@ -5,12 +5,23 @@ import org.crosswire.jsword.book.Book /** * Created by bspeice on 11/11/14. */ - -class DLProgressEvent(val progress: Int, val b: Book) { +data class DLProgressEvent(val bookProgress: Int, + val indexProgress: Int, + val b: Book) { class object { val PROGRESS_COMPLETE = 100 val PROGRESS_BEGINNING = 0 + + /** + * Build a DLProgressEvent that is just beginning + * Mostly just a nice shorthand + */ + fun beginningEvent(b: Book) = DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, + DLProgressEvent.PROGRESS_BEGINNING, b) } - fun toCircular() = (progress.toFloat() * 360 / 100).toInt() + val averageProgress: Int + get() = (bookProgress + indexProgress) / 2 + + fun toCircular() = (averageProgress.toFloat() * 360 / 100).toInt() }