Only show languages for selected book category

Previously displayed all languages, period. Also includes some testing updates to make sure everything is still covered.

app-test/build.gradle

@@ -28,15 +28,16 @@ def firstVariant = androidModule.android.applicationVariants.toList().first()
 
 // TODO: Not yet including Spek tests, fix that.
 def testIncludes = [
-        '**/*Test.class'
+        '**/*Test.class',
+        '**/*Spek.class'
 ]
 def jacocoExcludes = [
         'android/**',
         'com/todddavies/**',
-		'com/cmwmobile/**',
+        'com/cmwmobile/**',
-		'org/bspeice/minimalbible/R*',
+        'org/bspeice/minimalbible/R*',
         '**/BookAdapter$ChapterInfo*',
-		'**/*$$*' // Dagger/Butterknife
+        '**/*$$*' // Dagger/Butterknife
 ]
 
 dependencies {
@@ -54,24 +55,24 @@ dependencies {
 }
 
 def buildExcludeTree(path, excludes) {
-	def tree = fileTree(path).exclude(excludes)
+    def tree = fileTree(path).exclude(excludes)
-	tree
+    tree
 }
 
 jacocoTestReport {
     doFirst {
-		// First we build a list of our base directories
+        // First we build a list of our base directories
-		def fileList = new ArrayList<String>()
+        def fileList = new ArrayList<String>()
         def outputsList = firstVariant.javaCompile.outputs.files
-		outputsList.each { fileList.add(it.absolutePath.toString()) }
+        outputsList.each { fileList.add(it.absolutePath.toString()) }
 
-		// And build a fileTree from those
+        // And build a fileTree from those
-		def outputTree = fileList.inject { tree1, tree2 ->
+        def outputTree = fileList.inject { tree1, tree2 ->
-			buildExcludeTree(tree1, jacocoExcludes) +
+            buildExcludeTree(tree1, jacocoExcludes) +
-			buildExcludeTree(tree2, jacocoExcludes)
+                    buildExcludeTree(tree2, jacocoExcludes)
-		}
+        }
-		
+
-		// And finally tell Jacoco to only include said files in the report
+        // And finally tell Jacoco to only include said files in the report
         // For whatever reason, firstVariant.javaCompile.exclude(jacocoExcludes) doesn't work...
         classDirectories = outputTree
         sourceDirectories = files(androidModule.android.sourceSets.main.java.srcDirs)

app-test/src/test/java/org/bspeice/minimalbible/test/activity/downloader/manager/LocaleManagerTest.java

@@ -1,46 +0,0 @@
-package org.bspeice.minimalbible.test.activity.downloader.manager;
-
-import org.bspeice.minimalbible.activity.downloader.manager.LocaleManager;
-import org.crosswire.common.util.Language;
-import org.junit.Test;
-
-import java.util.List;
-
-import rx.Observable;
-
-import static org.junit.Assert.assertTrue;
-
-/**
- * Test cases for the Locale Manager
- */
-public class LocaleManagerTest {
-
-    @Test
-    public void testSortedLanguagesList() {
-        Language english = new Language("en");
-        Language russian = new Language("ru");
-        Language french = new Language("fr");
-        Language german = new Language("de");
-        Language hebrew = new Language("he");
-        Language afrikaans = new Language("af");
-
-        Observable<Language> languages = Observable.just(english, russian, french,
-                german, hebrew, afrikaans);
-
-        LocaleManager.Core core = LocaleManager.Core.INSTANCE$;
-
-        //noinspection ConstantConditions
-        List<Language> sortedLanguages = core.sortedLanguagesList(languages, english)
-                .toBlocking().first();
-
-        // First language should be the 'current' (note this is an identity compare)
-        assertTrue(sortedLanguages.get(0) == english);
-        // Second language should be 'less than' third
-        assertTrue(sortedLanguages.toString(),
-                sortedLanguages.get(1).toString().compareTo(
-                        sortedLanguages.get(2).toString()) < 0);
-        // Fifth language should be greater than the fourth
-        assertTrue(sortedLanguages.toString(), sortedLanguages.get(4).toString().compareTo(
-                sortedLanguages.get(3).toString()) > 0);
-    }
-}

app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/DLProgressEventSpek.kt

@@ -8,7 +8,7 @@ import org.mockito.Mockito.mock
 import org.jetbrains.spek.api.Spek
 import kotlin.test.assertEquals
 
-class DLProgressEventSpecs : Spek() {{
+class DLProgressEventSpek : Spek() {{
 
     given("a DLProgressEvent created with 50% progress and a mock book") {
         val mockBook = mock(javaClass<Book>())

app-test/src/test/kotlin/org/bspeice/minimalbible/activity/downloader/manager/LocaleManagerSpek.kt

@@ -0,0 +1,66 @@
+package org.bspeice.minimalbible.activity.downloader.manager
+
+import org.jetbrains.spek.api.Spek
+import org.crosswire.common.util.Language
+
+/**
+ * Created by bspeice on 12/14/14.
+ */
+
+class LocaleManagerSpek() : Spek() {{
+
+    given("some example language objects") {
+        val english = Language("en")
+        val russian = Language("ru")
+        val french = Language("fr");
+
+        on("sorting between english and russian with current as english") {
+            val result = LocaleManager.compareLanguages(english, russian, english)
+
+            it("should prioritize english") {
+                assert(result < 0)
+            }
+        }
+
+        on("sorting between russian and english with current as english") {
+            val result = LocaleManager.compareLanguages(russian, english, english)
+
+            it("should prioritize english") {
+                assert(result > 0)
+            }
+        }
+
+        on("sorting between russian and english with current as french") {
+            val result = LocaleManager.compareLanguages(russian, english, french)
+
+            it("should inform us that russian is greater") {
+                assert(result > 0)
+            }
+        }
+
+        on("sorting between english and russian with current as french") {
+            val result = LocaleManager.compareLanguages(english, russian, french)
+
+            it("should inform us that english is lesser") {
+                assert(result < 0)
+            }
+        }
+
+        on("comparing the same languages with current language as the language being compared") {
+            val result = LocaleManager.compareLanguages(english, english, english)
+
+            it("should report that the languages are duplicate") {
+                assert(result == 0)
+            }
+        }
+
+        on("comparing the same languages with current language as something different") {
+            val result = LocaleManager.compareLanguages(english, english, russian)
+
+            it("should report that the languages are duplicate") {
+                assert(result == 0)
+            }
+        }
+    }
+}
+}

app/src/debug/res/values/strings.xml

@@ -1,5 +1,3 @@
 <resources>
     <string name="title_activity_fragment_test">FragmentTestActivity</string>
-    <string name="hello_world">Hello world!</string>
-    <string name="action_settings">Settings</string>
 </resources>

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

@@ -15,6 +15,7 @@ import android.widget.Toast;
 import org.bspeice.minimalbible.Injector;
 import org.bspeice.minimalbible.R;
 import org.bspeice.minimalbible.activity.BaseFragment;
+import org.bspeice.minimalbible.activity.downloader.manager.LocaleManager;
 import org.bspeice.minimalbible.activity.downloader.manager.RefreshManager;
 import org.crosswire.common.util.Language;
 import org.crosswire.jsword.book.Book;
@@ -36,18 +37,21 @@ import rx.functions.Func1;
 import rx.functions.Func2;
 
 /**
- * A placeholder fragment containing a simple view.
+ * A fragment to list out the books available for downloading.
+ * Each fragment will be responsible for a single category,
+ * another fragment will be created if a second category is needed.
  */
 
 public class BookListFragment extends BaseFragment {
     protected static final String ARG_BOOK_CATEGORY = "book_category";
+    protected BookCategory bookCategory;
 
     @Inject
     DownloadPrefs downloadPrefs;
     @Inject
     RefreshManager refreshManager;
     @Inject
-    List<Language> availableLanguages;
+    LocaleManager localeManager;
 
     @InjectView(R.id.lst_download_available)
     ListView downloadsAvailable;
@@ -56,6 +60,9 @@ public class BookListFragment extends BaseFragment {
 
     LayoutInflater inflater;
 
+    // A cache of the languages currently available for this category
+    List<Language> availableLanguages;
+
     /**
      * Returns a new instance of this fragment for the given section number.
      * TODO: Switch to AutoFactory/@Provides rather than inline creation.
@@ -72,6 +79,9 @@ public class BookListFragment extends BaseFragment {
     public void onCreate(Bundle state) {
         super.onCreate(state);
         ((Injector)getActivity()).inject(this);
+
+        bookCategory = BookCategory.fromString(getArguments().getString(ARG_BOOK_CATEGORY));
+        availableLanguages = localeManager.sortedLanguagesForCategory(bookCategory);
     }
 
     @Override
@@ -122,7 +132,8 @@ public class BookListFragment extends BaseFragment {
     void displayLanguageSpinner() {
         ArrayAdapter<Object> adapter = new ArrayAdapter<>(this.getActivity(),
                 android.R.layout.simple_spinner_item,
-                availableLanguages.toArray());
+                availableLanguages.toArray()
+        );
         adapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
         languagesSpinner.setAdapter(adapter);
 
@@ -138,7 +149,7 @@ public class BookListFragment extends BaseFragment {
     public void onClick(final int position) {
         booksByLanguage(refreshManager.getFlatModules(),
                 availableLanguages.get(position),
-                BookCategory.fromString(getArguments().getString(ARG_BOOK_CATEGORY)))
+                bookCategory)
                 // Repack all the books
                 .toSortedList(new Func2<Book, Book, Integer>() {
                     @Override
@@ -157,6 +168,7 @@ public class BookListFragment extends BaseFragment {
                 });
     }
 
+    // TODO: Refactor out, this information should come from LocaleManager
     protected Observable<Book> booksByLanguage(Observable<Book> books, final Language language,
                                                final BookCategory category) {
         return books

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

@@ -8,7 +8,6 @@ import org.bspeice.minimalbible.MinimalBibleModules;
 import org.bspeice.minimalbible.activity.downloader.manager.BookManager;
 import org.bspeice.minimalbible.activity.downloader.manager.LocaleManager;
 import org.bspeice.minimalbible.activity.downloader.manager.RefreshManager;
-import org.crosswire.common.util.Language;
 import org.crosswire.jsword.book.Book;
 import org.crosswire.jsword.book.BookCategory;
 import org.crosswire.jsword.book.Books;
@@ -49,17 +48,20 @@ public class DownloadActivityModules {
         this.activity = activity;
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     DownloadPrefs provideDownloadPrefs() {
         return Esperandro.getPreferences(DownloadPrefs.class, activity);
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     DownloadActivity provideDownloadActivity() {
         return activity;
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     Injector provideActivityInjector() {
         return activity;
     }
@@ -67,19 +69,24 @@ public class DownloadActivityModules {
     /**
      * Provide the context for the DownloadActivity. We name it so that we don't have to
      * \@Provides a specific class, but can keep track of what exactly we mean by "Context"
+     *
      * @return The DownloadActivity Context
      */
-    @Provides @Singleton @Named("DownloadActivityContext")
+    @Provides
+    @Singleton
+    @Named("DownloadActivityContext")
     Context provideActivityContext() {
         return activity;
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     BookManager provideBookDownloadManager(Books installedBooks, RefreshManager rm) {
         return new BookManager(installedBooks, rm);
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     @Named("ValidCategories")
     List<BookCategory> provideValidCategories() {
         return new ArrayList<BookCategory>() {{
@@ -91,7 +98,8 @@ public class DownloadActivityModules {
     }
 
     //TODO: Move this to a true async
-    @Provides @Singleton
+    @Provides
+    @Singleton
     Books provideInstalledBooks() {
         return Books.installed();
     }
@@ -101,12 +109,14 @@ public class DownloadActivityModules {
         return b.getBooks();
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     Collection<Installer> provideInstallers() {
         return new InstallManager().getInstallers().values();
     }
 
-    @Provides @Singleton
+    @Provides
+    @Singleton
     RefreshManager provideRefreshManager(Collection<Installer> installers, DownloadPrefs prefs,
                                          @Named("DownloadActivityContext") Context context) {
         return new RefreshManager(installers, prefs,
@@ -117,9 +127,4 @@ public class DownloadActivityModules {
     LocaleManager provideLocaleManager(RefreshManager refreshManager) {
         return new LocaleManager(refreshManager);
     }
-
-    @Provides
-    List<Language> availableLanguages(LocaleManager localeManager) {
-        return localeManager.getSortedLanguagesList();
-    }
 }

app/src/main/kotlin/org/bspeice/minimalbible/activity/downloader/manager/LocaleManager.kt

@@ -2,43 +2,52 @@ package org.bspeice.minimalbible.activity.downloader.manager
 
 import org.crosswire.common.util.Language
 import rx.Observable
-import rx.observables.GroupedObservable
+import org.crosswire.jsword.book.BookCategory
+import kotlin.platform.platformStatic
 
+/**
+ * Took me a significant amount of time, but this is an implementation I can live with.
+ * An ideal solution would be able to group by the category first, then language, with all
+ * modules underneath, so something like Map<BookCategory, Map<Language, List<Book>>>.
+ * That said, trying to build said map is a bit ridiculous. The way I wrote it requires
+ * using functions instead of cached values, but I'll get over it.
+ */
 class LocaleManager(val rM: RefreshManager) {
 
     val currentLanguage = Language.DEFAULT_LANG
 
-    private val languageModuleMap = rM.flatModules
+    // Get all modules grouped by language first
-            // Language doesn't have hashCode(), so we actually group by its String
+    val modulesByCategory = rM.flatModules.groupBy { it.getBookCategory() }
-            .groupBy { FixedLanguage(it.getLanguage()) }
 
-    // I would suppress the warning here if I could figure out how...
+    fun languagesForCategory(cat: BookCategory): Observable<Language> = modulesByCategory
-    val modulesByLanguage = languageModuleMap
+            // Then filter according to the requested language
-            .map { GroupedObservable.from(it.getKey(): Language, it) }
+            .filter { it.getKey() == cat }
+            // Then map the GroupedObservable Book element to its actual language
+            .flatMap { it.map { it.getLanguage() } }
+            // Making sure to discard anything with a null language
+            .filter { it != null }
+            // And remove duplicates. The flatMap above means that we will have one entry
+            // for each book, so we need to remove duplicate entries of
+            // languages with more than one book to them
+            .distinct()
 
-    // Cast back to the original Language implementation
+    fun sortedLanguagesForCategory(cat: BookCategory): List<Language> =
-    val availableLanguages: Observable<Language> = languageModuleMap.map { it.getKey() }
+            languagesForCategory(cat)
-    val sortedLanguagesList =
+                    // Finally, sort all languages, prioritizing the current
-            Core.sortedLanguagesList(availableLanguages, currentLanguage).toBlocking().first()
+                    .toSortedList { left, right -> compareLanguages(left, right, currentLanguage) }
+                    // And flatten this into the actual List needed
+                    .toBlocking().first()
 
-    object Core {
+    class object {
-        fun sortedLanguagesList(availableLanguages: Observable<Language>,
+        platformStatic
-                                currentLanguage: Language) =
+        fun compareLanguages(left: Language, right: Language, current: Language) =
-            availableLanguages.toSortedList {(left, right) ->
+                if (left == right)
-                // Prioritize our current language first
+                    0
-                if (left.getName() == currentLanguage.getName())
+                else if (left.getName() == current.getName())
                     -1
-                else if (right.getName() == currentLanguage.getName())
+                else if (right.getName() == current.getName())
                     1
                 else
                     left.getName() compareTo right.getName()
-            }
-    }
-
-    // TODO: Fix the actual Language implementation - Pull Request?
-    // Can't use a data class because we need to get the name of the language
-    private class FixedLanguage(language: Language?) :
-            Language(language?.getCode() ?: Language.UNKNOWN_LANG_CODE) {
-        override fun hashCode() = this.getName().hashCode()
     }
 }