From 8137c4795a85d0da73f7e1ae199e88516661080a Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Wed, 12 Nov 2014 00:34:31 -0500 Subject: [PATCH] Fix issues the OsisParser regenerating VerseContent BookManager is still currently unable to delete books --- .../downloader/manager/BookManagerTest.java | 2 +- .../format/osisparser/OsisParserTest.java | 23 +++++++++++++++++++ .../activity/downloader/BookItemHolder.java | 2 +- .../downloader/manager/BookManager.kt | 17 ++++++++++---- .../service/format/osisparser/OsisParser.kt | 4 ++-- 5 files changed, 39 insertions(+), 9 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 b1346c3..be93771 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 @@ -142,7 +142,7 @@ public class BookManagerTest extends MBTestCase implements Injector { bookManager.getInstalledBooksList().add(mockBook); assertTrue(bookManager.getInstalledBooksList().contains(mockBook)); - bookManager.removeBook(mockBook); + bookManager.removeBook(mockBook, driver); assertFalse(bookManager.getInstalledBooksList().contains(mockBook)); verify(driver, times(1)).delete(mockBook); } diff --git a/app/src/androidTest/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java b/app/src/androidTest/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java index 4a98d36..b739001 100644 --- a/app/src/androidTest/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java +++ b/app/src/androidTest/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java @@ -5,6 +5,7 @@ import android.annotation.SuppressLint; import org.bspeice.minimalbible.MBTestCase; import org.bspeice.minimalbible.service.format.osisparser.OsisParser; import org.crosswire.jsword.book.OSISUtil; +import org.crosswire.jsword.passage.Verse; import org.xml.sax.Attributes; import static org.mockito.Mockito.mock; @@ -48,4 +49,26 @@ public class OsisParserTest extends MBTestCase { parser.endElement("", "", ""); assertTrue(parser.getDoWrite().isEmpty()); } + + // During initial development, I accidentally set up the verseContent + // as a value computed every time - so you'd get a new "content" every time + // you tried to update it. Thus, if you updated the content only once, you're fine. + // Try and update multiple times, and things would start going crazy. + @SuppressLint("NewApi") + @SuppressWarnings("unused") + public void ignoreTestVerseContentConsistent() { + String string1 = "1"; + String string2 = "2"; + + // Yes, I need intimate knowledge of the state machine to run this test + // Also, Verse is final, so I can't mock that, which is why this test is ignored. + Verse mockVerse = mock(Verse.class); + parser.setVerse(mockVerse); + parser.getDoWrite().push(true); + parser.characters(string1.toCharArray(), 0, string1.length()); + assertEquals(parser.getVerseContent().getContent(), string1); + + parser.characters(string2.toCharArray(), 0, string2.length()); + assertEquals(parser.getVerseContent().getContent(), string1 + string2); + } } 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 8bbaf25..8d939bf 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); + boolean didRemove = bookManager.removeBook(b, bookManager.getRealDriver(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 cc6b763..9495815 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,6 +14,7 @@ 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 @@ -70,19 +71,25 @@ class BookManager(private val installedBooks: Books, val rM: RefreshManager) : downloadEvents onNext DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b) } + /** + * 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. + * 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() + /** * 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, make sure to manually test if you change this implementation. The `drivers` are - * a bit persnickety. * @param b The book to remove * @return Whether the book was removed. */ - fun removeBook(b: Book): Boolean { + fun removeBook(b: Book, driver: BookDriver): Boolean { try { - val realBook = installedBooks getBook b.getInitials() - realBook.getDriver().delete(b) + driver delete b return installedBooksList remove b } catch (e: BookException) { Log.e("InstalledManager", diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt b/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt index 655d5e3..a826670 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt @@ -6,6 +6,7 @@ import java.util.ArrayDeque import org.xml.sax.Attributes import org.crosswire.jsword.book.OSISUtil import org.bspeice.minimalbible.SafeValDelegate +import kotlin.properties.Delegates /** * Created by bspeice on 9/10/14. @@ -16,8 +17,7 @@ class OsisParser() : DefaultHandler() { // Don't pass a verse as part of the constructor, but still guarantee // that it will exist public var verse: Verse by SafeValDelegate() - val verseContent: VerseContent - get() = VerseContent(verse) + val verseContent: VerseContent by Delegates.lazy { VerseContent(verse) } // TODO: Implement a stack to keep min API 8 val doWrite = ArrayDeque()