From 542d321acc7a40a4adbdfdf39f731b92520de1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20Mac=20Gillicuddy?= Date: Fri, 26 Jun 2020 08:54:26 +0100 Subject: [PATCH] #3841 Unit Test DepictsClient (#3842) * #3468 Switch from RvRenderer to AdapterDelegates - replace SearchDepictionsRenderer * #3468 Switch from RvRenderer to AdapterDelegates - replace UploadCategoryDepictionsRenderer * #3468 Switch from RvRenderer to AdapterDelegates - update BaseAdapter to be easier to use * #3468 Switch from RvRenderer to AdapterDelegates - replace SearchImagesRenderer * #3468 Switch from RvRenderer to AdapterDelegates - replace SearchCategoriesRenderer * #3468 Switch from RvRenderer to AdapterDelegates - replace NotificationRenderer * #3468 Switch from RvRenderer to AdapterDelegates - replace UploadDepictsRenderer * #3468 Switch from RvRenderer to AdapterDelegates - replace PlaceRenderer * #3756 Convert SearchDepictionsFragment to use Pagination - convert SearchDepictionsFragment * #3756 Convert SearchDepictionsFragment to use Pagination - fix presenter unit tests now that view is not nullable - fix Category prefix imports * #3756 Convert SearchDepictionsFragment to use Pagination - test DataSource related classes * #3756 Convert SearchDepictionsFragment to use Pagination - reset rx scheduler - ignore failing test * #3760 Convert SearchCategoriesFragment to use Pagination - extract functionality of pagination to base classes - add category pagination * #3772 Convert SearchImagesFragment to use Pagination - convert SearchImagesFragment - tidy up showing the empty view - make search fragments show snackbar with appropriate text * #3772 Convert SearchImagesFragment to use Pagination - allow viewpager to load more data * #3760 remove test that got re-added by merge * #3760 remove duplicate dependency * #3772 fix compilation * #3780 Create media using a combination of Entities & MwQueryResult - construct media with an entity - move fields from media down to contribution - move dynamic fields outside of media - remove unused constructors - remove all unnecessary fetching of captions/descriptions - bump database version * #3808 Construct media objects that depict an item id correctly - use generator to construct media for DepictedImages * #3810 Convert DepictedImagesFragment to use Pagination - extract common media paging methods - convert to DepictedImages to use pagination * #3810 Convert DepictedImagesFragment to use Pagination - rename base classes to better reflect usage * #3810 Convert DepictedImagesFragment to use Pagination - map to empty result with no pages * #3810 Convert DepictedImagesFragment to use Pagination - align test with returned values * #3780 Create media using a combination of Entities & MwQueryResult - update wikicode to align with expected behaviour * #3780 Create media using a combination of Entities & MwQueryResult - replace old site of thumbnail title with most relevant caption * #3818 Convert SubDepictionListFragment to use Pagination - replace SubDepictionList with Child and Parent Fragments - replace contracts with simple presenter declarations - move classes to appropriate packages - delete unused network models - delete duplicated paging classes * #3820 Convert CategoryImagesListFragment to use Pagination - replace CategoryImagesListFragment with CategoriesMediaFragment - disallow the construction of media objects without imageinfo * #3822 Convert SubCategoryImagesListFragment to use Pagination - convert subcategories - add continuation support in category client - rely on interfaces for callbacks of PageableMediaFragments * #3822 Convert SubCategoryImagesListFragment to use Pagination - convert parent categories - delete list fragment - creat base class to support continuation requests in clients * #3822 Convert SubCategoryImagesListFragment to use Pagination - add tests for ParentCategoriesDataSource * #3822 Convert SubCategoryImagesListFragment to use Pagination - remove no longer applicable test * #3841 Unit Test DepictsClient - add tests * #3841 Unit Test DepictsClient - fix return types --- .../explore/depictions/DepictsClient.kt | 26 ++++--- .../PageableChildDepictionsDataSource.kt | 2 +- .../PageableParentDepictionsDataSource.kt | 2 +- .../commons/mwapi/OkHttpJsonApiClient.java | 8 +-- app/src/test/kotlin/ModelFunctions.kt | 9 +++ .../explore/depictions/DepictsClientTest.kt | 69 +++++++++++++++++++ .../PageableChildDepictionsDataSourceTest.kt | 5 +- .../PageableParentDepictionsDataSourceTest.kt | 4 +- 8 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/DepictsClientTest.kt diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt index 14e674744..13b7d1cd3 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/DepictsClient.kt @@ -1,9 +1,10 @@ package fr.free.nrw.commons.explore.depictions +import fr.free.nrw.commons.mwapi.Binding import fr.free.nrw.commons.mwapi.SparqlResponse import fr.free.nrw.commons.upload.depicts.DepictsInterface import fr.free.nrw.commons.upload.structure.depictions.DepictedItem -import io.reactivex.Observable +import fr.free.nrw.commons.wikidata.model.DepictSearchItem import io.reactivex.Single import org.wikipedia.wikidata.Entities import java.util.* @@ -14,9 +15,7 @@ import javax.inject.Singleton * Depicts Client to handle custom calls to Commons Wikibase APIs */ @Singleton -class DepictsClient @Inject constructor( - private val depictsInterface: DepictsInterface -) { +class DepictsClient @Inject constructor(private val depictsInterface: DepictsInterface) { /** * Search for depictions using the search item @@ -25,22 +24,21 @@ class DepictsClient @Inject constructor( fun searchForDepictions(query: String?, limit: Int, offset: Int): Single> { val language = Locale.getDefault().language return depictsInterface.searchForDepicts(query, "$limit", language, language, "$offset") - .map { it.search.joinToString("|") { searchItem -> searchItem.id } } - .flatMap(::getEntities) - .map { it.entities().values.map(::DepictedItem) } + .map { it.search.joinToString("|", transform = DepictSearchItem::id) } + .mapToDepictions() } fun getEntities(ids: String): Single { return depictsInterface.getEntities(ids) } - fun toDepictions(sparqlResponse: Observable): Observable> { + fun toDepictions(sparqlResponse: Single): Single> { return sparqlResponse.map { - it.results.bindings.joinToString("|") { binding -> - binding.id - } - } - .flatMap { getEntities(it).toObservable() } - .map { it.entities().values.map(::DepictedItem) } + it.results.bindings.joinToString("|", transform = Binding::id) + }.mapToDepictions() } + + private fun Single.mapToDepictions() = + flatMap(::getEntities) + .map { it.entities().values.map(::DepictedItem) } } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSource.kt b/app/src/main/java/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSource.kt index 2fb549f28..c189a2e0a 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSource.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSource.kt @@ -11,7 +11,7 @@ class PageableChildDepictionsDataSource @Inject constructor( private val okHttpJsonApiClient: OkHttpJsonApiClient ) : PageableBaseDataSource(liveDataConverter) { override val loadFunction = { limit: Int, startPosition: Int -> - okHttpJsonApiClient.getChildDepictions(query, startPosition, limit).blockingFirst() + okHttpJsonApiClient.getChildDepictions(query, startPosition, limit).blockingGet() } } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSource.kt b/app/src/main/java/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSource.kt index e2459bf66..8771adbfa 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSource.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSource.kt @@ -11,7 +11,7 @@ class PageableParentDepictionsDataSource @Inject constructor( private val okHttpJsonApiClient: OkHttpJsonApiClient ) : PageableBaseDataSource(liveDataConverter) { override val loadFunction = { limit: Int, startPosition: Int -> - okHttpJsonApiClient.getParentDepictions(query, startPosition, limit).blockingFirst() + okHttpJsonApiClient.getParentDepictions(query, startPosition, limit).blockingGet() } } diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java b/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java index 1d010e00a..21435116c 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java @@ -209,7 +209,7 @@ public class OkHttpJsonApiClient { * Get the QIDs of all Wikidata items that are subclasses of the given Wikidata item. Example: * bridge -> suspended bridge, aqueduct, etc */ - public Observable> getChildDepictions(String qid, int startPosition, + public Single> getChildDepictions(String qid, int startPosition, int limit) throws IOException { return depictedItemsFrom(sparqlQuery(qid, startPosition, limit,"/queries/subclasses_query.rq")); } @@ -218,14 +218,14 @@ public class OkHttpJsonApiClient { * Get the QIDs of all Wikidata items that are subclasses of the given Wikidata item. Example: * bridge -> suspended bridge, aqueduct, etc */ - public Observable> getParentDepictions(String qid, int startPosition, + public Single> getParentDepictions(String qid, int startPosition, int limit) throws IOException { return depictedItemsFrom(sparqlQuery(qid, startPosition, limit, "/queries/parentclasses_query.rq")); } - private Observable> depictedItemsFrom(Request request) { - return depictsClient.toDepictions(Observable.fromCallable(() -> { + private Single> depictedItemsFrom(Request request) { + return depictsClient.toDepictions(Single.fromCallable(() -> { try (ResponseBody body = okHttpClient.newCall(request).execute().body()) { return gson.fromJson(body.string(), SparqlResponse.class); } diff --git a/app/src/test/kotlin/ModelFunctions.kt b/app/src/test/kotlin/ModelFunctions.kt index 684cab3ac..3e39ead37 100644 --- a/app/src/test/kotlin/ModelFunctions.kt +++ b/app/src/test/kotlin/ModelFunctions.kt @@ -7,6 +7,7 @@ import fr.free.nrw.commons.nearby.Label import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.nearby.Sitelinks import fr.free.nrw.commons.upload.structure.depictions.DepictedItem +import fr.free.nrw.commons.wikidata.model.DepictSearchItem import org.wikipedia.wikidata.* import java.util.* @@ -63,6 +64,14 @@ fun media( depictionIds ) +fun depictSearchItem( + id: String = "id", + pageId: String = "pageid", + url: String = "url", + label: String = "label", + description: String = "description" +) = DepictSearchItem(id, pageId, url, label, description) + fun place( name: String = "name", label: Label? = null, diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/DepictsClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/DepictsClientTest.kt new file mode 100644 index 000000000..237fb49f4 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/DepictsClientTest.kt @@ -0,0 +1,69 @@ +package fr.free.nrw.commons.explore.depictions + +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import depictSearchItem +import fr.free.nrw.commons.mwapi.Binding +import fr.free.nrw.commons.mwapi.Result +import fr.free.nrw.commons.mwapi.SparqlResponse +import fr.free.nrw.commons.upload.depicts.DepictsInterface +import fr.free.nrw.commons.wikidata.model.DepictSearchResponse +import io.reactivex.Single +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.wikipedia.wikidata.Entities + +class DepictsClientTest { + + @Mock + private lateinit var depictsInterface: DepictsInterface + private lateinit var depictsClient: DepictsClient + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + depictsClient = DepictsClient(depictsInterface) + } + + @Test + fun searchForDepictions() { + val depictSearchResponse = mock() + whenever(depictsInterface.searchForDepicts("query", "1", "en", "en", "0")) + .thenReturn(Single.just(depictSearchResponse)) + whenever(depictSearchResponse.search).thenReturn(listOf(depictSearchItem("1"),depictSearchItem("2"))) + val entities = mock() + whenever(depictsInterface.getEntities("1|2")).thenReturn(Single.just(entities)) + whenever(entities.entities()).thenReturn(emptyMap()) + depictsClient.searchForDepictions("query", 1, 0) + .test() + .assertValue(emptyList()) + } + + + @Test + fun getEntities() { + val entities = mock() + whenever(depictsInterface.getEntities("ids")).thenReturn(Single.just(entities)) + depictsClient.getEntities("ids").test().assertValue(entities) + } + + @Test + fun toDepictions() { + val sparqlResponse = mock() + val result = mock() + whenever(sparqlResponse.results).thenReturn(result) + val binding1 = mock() + val binding2 = mock() + whenever(result.bindings).thenReturn(listOf(binding1, binding2)) + whenever(binding1.id).thenReturn("1") + whenever(binding2.id).thenReturn("2") + val entities = mock() + whenever(depictsInterface.getEntities("1|2")).thenReturn(Single.just(entities)) + whenever(entities.entities()).thenReturn(emptyMap()) + depictsClient.toDepictions(Single.just(sparqlResponse)) + .test() + .assertValue(emptyList()) + } +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSourceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSourceTest.kt index 9300bb9fa..6230c1620 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSourceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/child/PageableChildDepictionsDataSourceTest.kt @@ -1,11 +1,10 @@ package fr.free.nrw.commons.explore.depictions.child -import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever import depictedItem import fr.free.nrw.commons.explore.paging.LiveDataConverter import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient -import io.reactivex.Observable +import io.reactivex.Single import org.hamcrest.CoreMatchers.`is` import org.hamcrest.MatcherAssert.assertThat import org.junit.Before @@ -30,7 +29,7 @@ class PageableChildDepictionsDataSourceTest { PageableChildDepictionsDataSource(liveDataConverter, okHttpJsonApiClient) dataSource.onQueryUpdated("test") whenever(okHttpJsonApiClient.getChildDepictions("test", 0, 1)) - .thenReturn(Observable.just(listOf(depictedItem()))) + .thenReturn(Single.just(listOf(depictedItem()))) assertThat(dataSource.loadFunction(1, 0), `is`(listOf(depictedItem()))) } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSourceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSourceTest.kt index 7e1346854..16972c1dd 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSourceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/depictions/parent/PageableParentDepictionsDataSourceTest.kt @@ -4,7 +4,7 @@ import com.nhaarman.mockitokotlin2.whenever import depictedItem import fr.free.nrw.commons.explore.paging.LiveDataConverter import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient -import io.reactivex.Observable +import io.reactivex.Single import org.hamcrest.CoreMatchers.`is` import org.hamcrest.MatcherAssert.assertThat import org.junit.Before @@ -30,7 +30,7 @@ class PageableParentDepictionsDataSourceTest { PageableParentDepictionsDataSource(liveDataConverter, okHttpJsonApiClient) dataSource.onQueryUpdated("test") whenever(okHttpJsonApiClient.getParentDepictions("test", 0, 1)) - .thenReturn(Observable.just(listOf(depictedItem()))) + .thenReturn(Single.just(listOf(depictedItem()))) assertThat(dataSource.loadFunction(1, 0), `is`(listOf(depictedItem()))) } }