Refactoring to extract GpsCategoryModel and ensure single-responsibility-principle is maintained in CategoryApi.

app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt

@@ -8,7 +8,6 @@ import com.nhaarman.mockito_kotlin.mock
 import com.squareup.leakcanary.RefWatcher
 import fr.free.nrw.commons.auth.AccountUtil
 import fr.free.nrw.commons.auth.SessionManager
-import fr.free.nrw.commons.caching.CacheController
 import fr.free.nrw.commons.data.DBOpenHelper
 import fr.free.nrw.commons.di.CommonsApplicationComponent
 import fr.free.nrw.commons.di.CommonsApplicationModule
@@ -42,7 +41,6 @@ class MockCommonsApplicationModule(appContext: Context) : CommonsApplicationModu
     val uploadController: UploadController = mock()
     val mockSessionManager: SessionManager = mock()
     val locationServiceManager: LocationServiceManager = mock()
-    val cacheController: CacheController = mock()
     val mockDbOpenHelper: DBOpenHelper = mock()
     val nearbyPlaces: NearbyPlaces = mock()
     val lruCache: LruCache<String, String> = mock()
@@ -73,8 +71,6 @@ class MockCommonsApplicationModule(appContext: Context) : CommonsApplicationModu
 
     override fun provideLocationServiceManager(context: Context): LocationServiceManager = locationServiceManager
 
-    override fun provideCacheController(): CacheController = cacheController
-
     override fun provideDBOpenHelper(context: Context): DBOpenHelper = mockDbOpenHelper
 
     override fun provideNearbyPlaces(): NearbyPlaces = nearbyPlaces

app/src/test/kotlin/fr/free/nrw/commons/mwapi/CategoryApiTest.kt

@@ -0,0 +1,178 @@
+package fr.free.nrw.commons.mwapi
+
+import com.google.gson.Gson
+import fr.free.nrw.commons.mwapi.model.Page
+import fr.free.nrw.commons.mwapi.model.PageCategory
+import okhttp3.HttpUrl
+import okhttp3.OkHttpClient
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+class CategoryApiTest {
+    private lateinit var server: MockWebServer
+    private lateinit var url: String
+    private lateinit var categoryApi: CategoryApi
+
+    @Before
+    fun setUp() {
+        server = MockWebServer()
+        url = "http://${server.hostName}:${server.port}/"
+        categoryApi = CategoryApi(OkHttpClient.Builder().build(), Gson(), HttpUrl.parse(url))
+    }
+
+    @After
+    fun teardown() {
+        server.shutdown()
+    }
+
+    @Test
+    fun apiReturnsEmptyListWhenError() {
+        server.enqueue(MockResponse().setResponseCode(400).setBody(""))
+
+        assertTrue(categoryApi.request("foo").blockingGet().isEmpty())
+    }
+
+    @Test
+    fun apiReturnsEmptyWhenTheresNoQuery() {
+        server.success(emptyMap())
+
+        assertTrue(categoryApi.request("foo").blockingGet().isEmpty())
+    }
+
+    @Test
+    fun apiReturnsEmptyWhenQueryHasNoPages() {
+        server.success(mapOf("query" to emptyMap<String, Any>()))
+
+        assertTrue(categoryApi.request("foo").blockingGet().isEmpty())
+    }
+
+    @Test
+    fun apiReturnsEmptyWhenQueryHasPagesButTheyreEmpty() {
+        server.success(mapOf("query" to
+                mapOf("pages" to emptyList<String>())))
+
+        assertTrue(categoryApi.request("foo").blockingGet().isEmpty())
+    }
+
+    @Test
+    fun singlePageSingleCategory() {
+        server.success(mapOf("query" to
+                mapOf("pages" to listOf(
+                        page(listOf("one"))
+                ))))
+
+        val response = categoryApi.request("foo").blockingGet()
+
+        assertEquals(1, response.size)
+        assertEquals("one", response[0])
+    }
+
+    @Test
+    fun multiplePagesSingleCategory() {
+        server.success(mapOf("query" to
+                mapOf("pages" to listOf(
+                        page(listOf("one")),
+                        page(listOf("two"))
+                ))))
+
+        val response = categoryApi.request("foo").blockingGet()
+
+        assertEquals(2, response.size)
+        assertEquals("one", response[0])
+        assertEquals("two", response[1])
+    }
+
+    @Test
+    fun singlePageMultipleCategories() {
+        server.success(mapOf("query" to
+                mapOf("pages" to listOf(
+                        page(listOf("one", "two"))
+                ))))
+
+        val response = categoryApi.request("foo").blockingGet()
+
+        assertEquals(2, response.size)
+        assertEquals("one", response[0])
+        assertEquals("two", response[1])
+    }
+
+    @Test
+    fun multiplePagesMultipleCategories() {
+        server.success(mapOf("query" to
+                mapOf("pages" to listOf(
+                        page(listOf("one", "two")),
+                        page(listOf("three", "four"))
+                ))))
+
+        val response = categoryApi.request("foo").blockingGet()
+
+        assertEquals(4, response.size)
+        assertEquals("one", response[0])
+        assertEquals("two", response[1])
+        assertEquals("three", response[2])
+        assertEquals("four", response[3])
+    }
+
+    @Test
+    fun multiplePagesMultipleCategories_duplicatesRemoved() {
+        server.success(mapOf("query" to
+                mapOf("pages" to listOf(
+                        page(listOf("one", "two", "three")),
+                        page(listOf("three", "four", "one"))
+                ))))
+
+        val response = categoryApi.request("foo").blockingGet()
+
+        assertEquals(4, response.size)
+        assertEquals("one", response[0])
+        assertEquals("two", response[1])
+        assertEquals("three", response[2])
+        assertEquals("four", response[3])
+    }
+
+    @Test
+    fun requestSendsWhatWeExpect() {
+        server.success(mapOf("query" to mapOf("pages" to emptyList<String>())))
+
+        val coords = "foo,bar"
+        categoryApi.request(coords).blockingGet()
+
+        server.takeRequest().let { request ->
+            assertEquals("GET", request.method)
+            assertEquals("/w/api.php", request.requestUrl.encodedPath())
+            request.requestUrl.let { url ->
+                assertEquals("query", url.queryParameter("action"))
+                assertEquals("categories|coordinates|pageprops", url.queryParameter("prop"))
+                assertEquals("json", url.queryParameter("format"))
+                assertEquals("!hidden", url.queryParameter("clshow"))
+                assertEquals("type|name|dim|country|region|globe", url.queryParameter("coprop"))
+                assertEquals(coords, url.queryParameter("codistancefrompoint"))
+                assertEquals("geosearch", url.queryParameter("generator"))
+                assertEquals(coords, url.queryParameter("ggscoord"))
+                assertEquals("10000", url.queryParameter("ggsradius"))
+                assertEquals("10", url.queryParameter("ggslimit"))
+                assertEquals("6", url.queryParameter("ggsnamespace"))
+                assertEquals("type|name|dim|country|region|globe", url.queryParameter("ggsprop"))
+                assertEquals("all", url.queryParameter("ggsprimary"))
+                assertEquals("2", url.queryParameter("formatversion"))
+            }
+        }
+    }
+
+    private fun page(catList: List<String>) = Page().apply {
+        categories = catList.map {
+            PageCategory().apply {
+                title = "Category:$it"
+            }
+        }.toTypedArray()
+    }
+}
+
+fun MockWebServer.success(response: Map<String, Any>) {
+    enqueue(MockResponse().setResponseCode(200).setBody(Gson().toJson(response)))
+}

app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/ApiResponseTest.kt

@@ -0,0 +1,28 @@
+package fr.free.nrw.commons.mwapi.model
+
+import org.junit.Assert.*
+import org.junit.Test
+
+class ApiResponseTest {
+    @Test
+    fun hasPages_whenQueryIsNull() {
+        val response = ApiResponse()
+        assertFalse(response.hasPages())
+    }
+
+    @Test
+    fun hasPages_whenPagesIsNull() {
+        val response = ApiResponse()
+        response.query = Query()
+        response.query.pages = null
+        assertFalse(response.hasPages())
+    }
+
+    @Test
+    fun hasPages_defaultsToSafeValue() {
+        val response = ApiResponse()
+        response.query = Query()
+        assertNotNull(response.query.pages)
+        assertTrue(response.hasPages())
+    }
+}

app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageCategoryTest.kt

@@ -0,0 +1,20 @@
+package fr.free.nrw.commons.mwapi.model
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+class PageCategoryTest {
+    @Test
+    fun stripPrefix_whenPresent() {
+        val testObject = PageCategory()
+        testObject.title = "Category:Foo"
+        assertEquals("Foo", testObject.withoutPrefix())
+    }
+
+    @Test
+    fun stripPrefix_prefixAbsent() {
+        val testObject = PageCategory()
+        testObject.title = "Foo_Bar"
+        assertEquals("Foo_Bar", testObject.withoutPrefix())
+    }
+}

app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageTest.kt

@@ -0,0 +1,12 @@
+package fr.free.nrw.commons.mwapi.model
+
+import org.junit.Assert.assertNotNull
+import org.junit.Test
+
+class PageTest {
+    @Test
+    fun categoriesDefaultToSafeValue() {
+        val page = Page()
+        assertNotNull(page.getCategories())
+    }
+}

app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt

@@ -0,0 +1,77 @@
+package fr.free.nrw.commons.upload
+
+import org.junit.Assert.*
+import org.junit.Before
+import org.junit.Test
+
+class GpsCategoryModelTest {
+
+    private lateinit var testObject : GpsCategoryModel
+
+    @Before
+    fun setUp() {
+        testObject = GpsCategoryModel()
+    }
+
+    @Test
+    fun initiallyTheModelIsEmpty() {
+        assertFalse(testObject.gpsCatExists)
+        assertTrue(testObject.categoryList.isEmpty())
+    }
+
+    @Test
+    fun addingCategoriesToTheModel() {
+        testObject.add("one")
+        assertTrue(testObject.gpsCatExists)
+        assertFalse(testObject.categoryList.isEmpty())
+        assertEquals(listOf("one"), testObject.categoryList)
+    }
+
+    @Test
+    fun duplicatesAreIgnored() {
+        testObject.add("one")
+        testObject.add("one")
+        assertEquals(listOf("one"), testObject.categoryList)
+    }
+
+    @Test
+    fun modelProtectsAgainstExternalModification() {
+        testObject.add("one")
+
+        val list = testObject.categoryList
+        list.add("two")
+
+        assertEquals(listOf("one"), testObject.categoryList)
+    }
+
+    @Test
+    fun clearingTheModel() {
+        testObject.add("one")
+
+        testObject.clear()
+        assertFalse(testObject.gpsCatExists)
+        assertTrue(testObject.categoryList.isEmpty())
+
+        testObject.add("two")
+        assertEquals(listOf("two"), testObject.categoryList)
+    }
+
+    @Test
+    fun settingTheListHandlesNull() {
+        testObject.add("one")
+
+        testObject.categoryList = null
+
+        assertFalse(testObject.gpsCatExists)
+        assertTrue(testObject.categoryList.isEmpty())
+    }
+
+    @Test
+    fun setttingTheListOverwritesExistingValues() {
+        testObject.add("one")
+
+        testObject.categoryList = listOf("two")
+
+        assertEquals(listOf("two"), testObject.categoryList)
+    }
+}