Skip to content

Commit 1e64acd

Browse files
savschsavsch
authored
If depicted Wikidata item has no associated Commons category property, then suggest categories from its P18 (commons-app#6130)
* Fix NPE with UploadMediaDetails.captionText * Store P18 instead of processed image url in DepictedItem * Add routes for fetching category info from titles * Consider depict's P18 when suggesting categories * Add tests * Corrected DepictedItem constructor arguments * Add test for DepictedItem::primaryImage
1 parent 1f33926 commit 1e64acd

File tree

7 files changed

+212
-28
lines changed

7 files changed

+212
-28
lines changed

‎app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.kt

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -127,30 +127,64 @@ class CategoriesModel
127127
/**
128128
* Fetches details of every category associated with selected depictions, converts them into
129129
* CategoryItem and returns them in a list.
130+
* If a selected depiction has no categories, the categories in which its P18 belongs are
131+
* returned in the list.
130132
*
131133
* @param selectedDepictions selected DepictItems
132134
* @return List of CategoryItem associated with selected depictions
133135
*/
134-
private fun categoriesFromDepiction(selectedDepictions: List<DepictedItem>): Observable<MutableList<CategoryItem>>? =
135-
Observable
136-
.fromIterable(
137-
selectedDepictions.map { it.commonsCategories }.flatten(),
138-
).map { categoryItem ->
139-
categoryClient
140-
.getCategoriesByName(
141-
categoryItem.name,
142-
categoryItem.name,
143-
SEARCH_CATS_LIMIT,
144-
).map {
145-
CategoryItem(
146-
it[0].name,
147-
it[0].description,
148-
it[0].thumbnail,
149-
it[0].isSelected,
150-
)
151-
}.blockingGet()
152-
}.toList()
153-
.toObservable()
136+
private fun categoriesFromDepiction(selectedDepictions: List<DepictedItem>): Observable<MutableList<CategoryItem>>? {
137+
val observables = selectedDepictions.map { depictedItem ->
138+
if (depictedItem.commonsCategories.isEmpty()) {
139+
if (depictedItem.primaryImage == null) {
140+
return@map Observable.just(emptyList<CategoryItem>())
141+
}
142+
Observable.just(
143+
depictedItem.primaryImage
144+
).map { image ->
145+
categoryClient
146+
.getCategoriesOfImage(
147+
image,
148+
SEARCH_CATS_LIMIT,
149+
).map {
150+
it.map { category ->
151+
CategoryItem(
152+
category.name,
153+
category.description,
154+
category.thumbnail,
155+
category.isSelected,
156+
)
157+
}
158+
}.blockingGet()
159+
}.flatMapIterable { it }.toList()
160+
.toObservable()
161+
} else {
162+
Observable
163+
.fromIterable(
164+
depictedItem.commonsCategories,
165+
).map { categoryItem ->
166+
categoryClient
167+
.getCategoriesByName(
168+
categoryItem.name,
169+
categoryItem.name,
170+
SEARCH_CATS_LIMIT,
171+
).map {
172+
CategoryItem(
173+
it[0].name,
174+
it[0].description,
175+
it[0].thumbnail,
176+
it[0].isSelected,
177+
)
178+
}.blockingGet()
179+
}.toList()
180+
.toObservable()
181+
}
182+
}
183+
return Observable.concat(observables)
184+
.scan(mutableListOf<CategoryItem>()) { accumulator, currentList ->
185+
accumulator.apply { addAll(currentList) }
186+
}
187+
}
154188

155189
/**
156190
* Fetches details of every category by their name, converts them into

‎app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,24 @@ class CategoryClient
7878
),
7979
)
8080

81+
/**
82+
* Fetches categories belonging to an image (P18 of some wikidata entity).
83+
*
84+
* @param image P18 of some wikidata entity
85+
* @param itemLimit How many categories to return
86+
* @return Single Observable emitting the list of categories
87+
*/
88+
fun getCategoriesOfImage(
89+
image: String,
90+
itemLimit: Int,
91+
): Single<List<CategoryItem>> =
92+
responseMapper(
93+
categoryInterface.getCategoriesByTitles(
94+
"File:${image}",
95+
itemLimit,
96+
),
97+
)
98+
8199
/**
82100
* The method takes categoryName as input and returns a List of Subcategories
83101
* It uses the generator query API to get the subcategories in a category, 500 at a time.

‎app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ interface CategoryInterface {
6161
@Query("gacoffset") offset: Int,
6262
): Single<MwQueryResponse>
6363

64+
/**
65+
* Fetches non-hidden categories by titles.
66+
*
67+
* @param titles titles to fetch categories for (e.g. File:<P18 of a wikidata entity>)
68+
* @param itemLimit How many categories to return
69+
* @return MwQueryResponse
70+
*/
71+
@GET(
72+
"w/api.php?action=query&format=json&formatversion=2&generator=categories&prop=categoryinfo|description|pageimages&piprop=thumbnail&pithumbsize=70&gclshow=!hidden",
73+
)
74+
fun getCategoriesByTitles(
75+
@Query("titles") titles: String?,
76+
@Query("gcllimit") itemLimit: Int,
77+
): Single<MwQueryResponse>
78+
6479
@GET("w/api.php?action=query&format=json&formatversion=2&generator=categorymembers&gcmtype=subcat&prop=info&gcmlimit=50")
6580
fun getSubCategoryList(
6681
@Query("gcmtitle") categoryName: String,

‎app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictedItem.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ data class DepictedItem constructor(
6868
entity.id(),
6969
)
7070

71+
val primaryImage: String?
72+
get() = imageUrl?.split('-')?.lastOrNull()
73+
7174
override fun equals(other: Any?) =
7275
when {
7376
this === other -> true

‎app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package fr.free.nrw.commons.category
22

33
import categoryItem
4+
import com.nhaarman.mockitokotlin2.any
45
import com.nhaarman.mockitokotlin2.mock
6+
import com.nhaarman.mockitokotlin2.times
57
import com.nhaarman.mockitokotlin2.verify
68
import com.nhaarman.mockitokotlin2.whenever
79
import depictedItem
@@ -90,14 +92,18 @@ class CategoriesModelTest {
9092
val depictedItem =
9193
depictedItem(
9294
commonsCategories =
93-
listOf(
94-
CategoryItem(
95-
"depictionCategory",
96-
"",
97-
"",
98-
false,
99-
),
95+
listOf(
96+
CategoryItem(
97+
"depictionCategory",
98+
"",
99+
"",
100+
false,
100101
),
102+
),
103+
)
104+
val depictedItemWithoutCategories =
105+
depictedItem(
106+
imageUrl = "testUrl"
101107
)
102108

103109
whenever(gpsCategoryModel.categoriesFromLocation)
@@ -159,6 +165,23 @@ class CategoriesModelTest {
159165
),
160166
),
161167
)
168+
whenever(
169+
categoryClient.getCategoriesOfImage(
170+
"testUrl",
171+
25,
172+
),
173+
).thenReturn(
174+
Single.just(
175+
listOf(
176+
CategoryItem(
177+
"categoriesOfP18",
178+
"",
179+
"",
180+
false,
181+
),
182+
),
183+
),
184+
)
162185
val imageTitleList = listOf("Test")
163186
CategoriesModel(categoryClient, categoryDao, gpsCategoryModel)
164187
.searchAll("", imageTitleList, listOf(depictedItem))
@@ -171,8 +194,21 @@ class CategoriesModelTest {
171194
categoryItem("recentCategories"),
172195
),
173196
)
197+
CategoriesModel(categoryClient, categoryDao, gpsCategoryModel)
198+
.searchAll("", imageTitleList, listOf(depictedItemWithoutCategories))
199+
.test()
200+
.assertValue(
201+
listOf(
202+
categoryItem("categoriesOfP18"),
203+
categoryItem("gpsCategory"),
204+
categoryItem("titleSearch"),
205+
categoryItem("recentCategories"),
206+
),
207+
)
174208
imageTitleList.forEach {
175-
verify(categoryClient).searchCategories(it, CategoriesModel.SEARCH_CATS_LIMIT)
209+
verify(categoryClient, times(2)).searchCategories(it, CategoriesModel.SEARCH_CATS_LIMIT)
210+
verify(categoryClient).getCategoriesByName(any(), any(), any(), any())
211+
verify(categoryClient).getCategoriesOfImage(any(), any())
176212
}
177213
}
178214

‎app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,45 @@ class CategoryClientTest {
132132
)
133133
}
134134

135+
@Test
136+
fun getCategoriesByTitlesFound() {
137+
val mockResponse = withMockResponse("Category:Test")
138+
whenever(
139+
categoryInterface.getCategoriesByTitles(
140+
anyString(),
141+
anyInt(),
142+
),
143+
).thenReturn(Single.just(mockResponse))
144+
categoryClient
145+
.getCategoriesOfImage("tes", 10)
146+
.test()
147+
.assertValues(
148+
listOf(
149+
CategoryItem(
150+
"Test",
151+
"",
152+
"",
153+
false,
154+
),
155+
),
156+
)
157+
categoryClient
158+
.getCategoriesOfImage(
159+
"tes",
160+
10,
161+
).test()
162+
.assertValues(
163+
listOf(
164+
CategoryItem(
165+
"Test",
166+
"",
167+
"",
168+
false,
169+
),
170+
),
171+
)
172+
}
173+
135174
@Test
136175
fun getCategoriesByNameNull() {
137176
val mockResponse = withNullPages()
@@ -160,6 +199,29 @@ class CategoryClientTest {
160199
.assertValues(emptyList())
161200
}
162201

202+
@Test
203+
fun getCategoriesByTitlesNull() {
204+
val mockResponse = withNullPages()
205+
whenever(
206+
categoryInterface.getCategoriesByTitles(
207+
anyString(),
208+
anyInt(),
209+
),
210+
).thenReturn(Single.just(mockResponse))
211+
categoryClient
212+
.getCategoriesOfImage(
213+
"tes",
214+
10,
215+
).test()
216+
.assertValues(emptyList())
217+
categoryClient
218+
.getCategoriesOfImage(
219+
"tes",
220+
10,
221+
).test()
222+
.assertValues(emptyList())
223+
}
224+
163225
@Test
164226
fun getParentCategoryListFound() {
165227
val mockResponse = withMockResponse("Category:Test")

‎app/src/test/kotlin/fr/free/nrw/commons/upload/structure/depictions/DepictedItemTest.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,20 @@ class DepictedItemTest {
181181
fun `hashCode returns different values for objects with different name`() {
182182
Assert.assertNotEquals(depictedItem(name = "a").hashCode(), depictedItem(name = "b").hashCode())
183183
}
184+
185+
@Test
186+
fun `primaryImage is derived correctly from imageUrl`() {
187+
Assert.assertEquals(
188+
DepictedItem(
189+
entity(
190+
statements = mapOf(
191+
WikidataProperties.IMAGE.propertyName to listOf(
192+
statement(snak(dataValue = valueString("prefix: example_image name"))),
193+
),
194+
),
195+
),
196+
).primaryImage,
197+
"_example_image_name",
198+
)
199+
}
184200
}

0 commit comments

Comments
 (0)