Skip to content

Commit e546803

Browse files
Revert "Fixes: commons-app#3179 Make category search non case-sensitive (commons-app#3326)"
Simply lower casing the name of the category sent to the server doesn't result in the server doing a case insensitive category search. In fact, it reduces the category search space as only categories that has a lower case character is searched even if the search text contains upper case characters. The test case did not catch this issue as the first character of the title is case insensitive[1]. So, revert the changes done in commit afdeaae. See further disucssion in the issue thread of commons-app#3179 starting from [2]. [1]: https://www.mediawiki.org/wiki/Manual:Page_title [2]: commons-app#3179 (comment)
1 parent cf73e28 commit e546803

File tree

5 files changed

+39
-33
lines changed

5 files changed

+39
-33
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,8 @@ public Observable<CategoryItem> searchAll(String term, List<String> imageTitleLi
123123
}
124124

125125
//otherwise, search API for matching categories
126-
//term passed as lower case to make search case-insensitive(taking only lower case for everything)
127126
return categoryClient
128-
.searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT)
127+
.searchCategoriesForPrefix(term, SEARCH_CATS_LIMIT)
129128
.map(name -> new CategoryItem(name, false));
130129
}
131130

@@ -184,12 +183,11 @@ private Observable<CategoryItem> titleCategories(List<String> titleList) {
184183

185184
/**
186185
* Return category for single title
187-
* title is converted to lower case to make search case-insensitive
188186
* @param title
189187
* @return
190188
*/
191189
private Observable<CategoryItem> getTitleCategories(String title) {
192-
return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT)
190+
return categoryClient.searchCategories(title, SEARCH_CATS_LIMIT)
193191
.map(name -> new CategoryItem(name, false));
194192
}
195193

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public Observable<String> searchCategories(String filter, int itemLimit) {
5555

5656
/**
5757
* Searches for categories starting with the specified string.
58-
*
58+
*
5959
* @param prefix The prefix to be searched
6060
* @param itemLimit How many results are returned
6161
* @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result

app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
import android.os.Parcel;
44
import android.os.Parcelable;
55

6+
/**
7+
* Represents a Category Item.
8+
* Implemented as Parcelable so that its object could be parsed between activity components.
9+
*/
610
public class CategoryItem implements Parcelable {
711
private final String name;
812
private boolean selected;
@@ -24,28 +28,53 @@ public CategoryItem(String name, boolean selected) {
2428
this.selected = selected;
2529
}
2630

31+
/**
32+
* Reads from the received Parcel
33+
* @param in
34+
*/
2735
private CategoryItem(Parcel in) {
2836
name = in.readString();
2937
selected = in.readInt() == 1;
3038
}
3139

40+
/**
41+
* Gets Name
42+
* @return
43+
*/
3244
public String getName() {
3345
return name;
3446
}
3547

48+
/**
49+
* Checks if that Category Item has been selected.
50+
* @return
51+
*/
3652
public boolean isSelected() {
3753
return selected;
3854
}
3955

56+
/**
57+
* Selects the Category Item.
58+
* @param selected
59+
*/
4060
public void setSelected(boolean selected) {
4161
this.selected = selected;
4262
}
4363

64+
/**
65+
* Used by Parcelable
66+
* @return
67+
*/
4468
@Override
4569
public int describeContents() {
4670
return 0;
4771
}
4872

73+
/**
74+
* Writes to the received Parcel
75+
* @param parcel
76+
* @param flags
77+
*/
4978
@Override
5079
public void writeToParcel(Parcel parcel, int flags) {
5180
parcel.writeString(name);
@@ -67,13 +96,19 @@ public boolean equals(Object o) {
6796

6897
}
6998

99+
/**
100+
* Returns hash code for current object
101+
*/
70102
@Override
71103
public int hashCode() {
72104
return name.hashCode();
73105
}
74106

107+
/**
108+
* Return String form of current object
109+
*/
75110
@Override
76111
public String toString() {
77112
return "CategoryItem: '" + name + '\'';
78113
}
79-
}
114+
}

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

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ class CategoriesModelTest {
1717
@Mock
1818
internal var categoryInterface: CategoryInterface? = null
1919

20-
@Mock
21-
internal var categoryItem: CategoryItem? = null
22-
2320
@Spy
2421
internal lateinit var gson: Gson
2522

@@ -43,28 +40,6 @@ class CategoriesModelTest {
4340
MockitoAnnotations.initMocks(this)
4441
}
4542

46-
// Test Case for verifying that Categories search (MW api calls) are case-insensitive
47-
@Test
48-
fun searchAllFoundCaseTest() {
49-
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
50-
Mockito.`when`(mwQueryPage.title()).thenReturn("Category:Test")
51-
val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
52-
Mockito.`when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage))
53-
val mockResponse = Mockito.mock(MwQueryResponse::class.java)
54-
Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
55-
val categoriesModel: CategoriesModel = CategoriesModel(categoryClient,null,null)
56-
57-
Mockito.`when`(categoryInterface!!.searchCategoriesForPrefix(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()))
58-
.thenReturn(Observable.just(mockResponse))
59-
60-
// Checking if both return "Test"
61-
val actualCategoryName = categoriesModel!!.searchAll("tes",null).blockingFirst()
62-
assertEquals("Test", actualCategoryName.getName())
63-
64-
val actualCategoryNameCaps = categoriesModel!!.searchAll("Tes",null).blockingFirst()
65-
assertEquals("Test", actualCategoryNameCaps.getName())
66-
}
67-
6843
/**
6944
* For testing the substring search algorithm for Categories search
7045
* To be more precise it tests the In Between substring( ex: searching `atte`

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ class CategoryClientTest {
5858
{ fail("SearchCategories returned element when it shouldn't have.") },
5959
{ s -> throw s })
6060
}
61-
6261
@Test
6362
fun searchCategoriesForPrefixFound() {
6463
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
@@ -93,7 +92,6 @@ class CategoryClientTest {
9392
{ fail("SearchCategories returned element when it shouldn't have.") },
9493
{ s -> throw s })
9594
}
96-
9795
@Test
9896
fun getParentCategoryListFound() {
9997
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)

0 commit comments

Comments
 (0)