From 18c9df404ebadbee5dbaa022630d4f810e2a9034 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Wed, 12 Nov 2014 01:01:34 -0500 Subject: [PATCH] Books are now deleted correctly Still need to handle all the network related errors... --- .../downloader/manager/BookManagerTest.java | 5 +++-- .../activity/downloader/BookItemHolder.java | 2 +- .../downloader/manager/BookManager.kt | 20 +++++++++++-------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java b/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java index be93771..a4ee1cc 100644 --- a/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java +++ b/app/src/androidTest/java/org/bspeice/minimalbible/test/activity/downloader/manager/BookManagerTest.java @@ -135,6 +135,7 @@ public class BookManagerTest extends MBTestCase implements Injector { BookDriver driver = mock(BookDriver.class); Book mockBook = mock(Book.class); + Book secondMockBook = mock(Book.class); when(mockBook.getDriver()).thenReturn(driver); BooksEvent event = mock(BooksEvent.class); @@ -142,9 +143,9 @@ public class BookManagerTest extends MBTestCase implements Injector { bookManager.getInstalledBooksList().add(mockBook); assertTrue(bookManager.getInstalledBooksList().contains(mockBook)); - bookManager.removeBook(mockBook, driver); + bookManager.removeBook(mockBook, secondMockBook); assertFalse(bookManager.getInstalledBooksList().contains(mockBook)); - verify(driver, times(1)).delete(mockBook); + verify(driver, times(1)).delete(secondMockBook); } /** 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 8d939bf..e1871fb 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 @@ -90,7 +90,7 @@ public class BookItemHolder { public void onDownloadItem(View v) { if (bookManager.isInstalled(b)) { // Remove the book - boolean didRemove = bookManager.removeBook(b, bookManager.getRealDriver(b)); + boolean didRemove = bookManager.removeBook(b, bookManager.getRealBook(b)); if (didRemove) { isDownloaded.setImageResource(R.drawable.ic_action_download); } else { 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 9495815..9e99039 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 @@ -14,7 +14,6 @@ import rx.Observable; import rx.schedulers.Schedulers; import rx.subjects.PublishSubject; import org.crosswire.jsword.book.BookException -import org.crosswire.jsword.book.BookDriver /** * Single point of authority for what is being downloaded and its progress @@ -73,28 +72,33 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : /** * For whatever reason, not just any old "book" reference will do. We need to actually - * get a reference corresponding to a physically installed book to get the proper driver. + * get a reference corresponding to a physically installed book for the driver to remove. * Plus, it makes the removeBook method easier to test. * @param b The book to find the actual driver for * @return The driver corresponding to the physical book */ - fun getRealDriver(b: Book): BookDriver = (installedBooks getBook b.getInitials()).getDriver() + fun getRealBook(b: Book): Book = installedBooks getBook b.getInitials() /** * Remove a book from being installed. * Currently only supports books that have been installed outside the current application run. * Not quite sure why this is, but And-Bible exhibits the same behavior. + * Also, I'll document it for the future: It seems like a book is only remove if you return + * true from this method. Which is incredibly strange, because this method should have no + * effect on the actual process of deleting a book. Even so, things work when + * I return true here. * @param b The book to remove * @return Whether the book was removed. */ - fun removeBook(b: Book, driver: BookDriver): Boolean { + fun removeBook(b: Book, realBook: Book = getRealBook(b)): Boolean { try { - driver delete b - return installedBooksList remove b + b.getDriver() delete realBook + installedBooksList remove b + return true } catch (e: BookException) { Log.e("InstalledManager", - "Unable to remove book (already uninstalled?): ${e.getDetailedMessage()}"); - return false; + "Unable to remove book (already uninstalled?): ${e.getDetailedMessage()}") + return false } } /**