mirror of
https://github.com/commons-app/apps-android-commons.git
synced 2025-10-26 20:33:53 +01:00
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 afdeaae075.
See further disucssion in the issue thread of #3179 starting
from [2].
[1]: https://www.mediawiki.org/wiki/Manual:Page_title
[2]: https://github.com/commons-app/apps-android-commons/issues/3179#issuecomment-605462140
This commit is contained in:
parent
cf73e28623
commit
c961099013
5 changed files with 39 additions and 33 deletions
|
|
@ -123,9 +123,8 @@ public class CategoriesModel{
|
||||||
}
|
}
|
||||||
|
|
||||||
//otherwise, search API for matching categories
|
//otherwise, search API for matching categories
|
||||||
//term passed as lower case to make search case-insensitive(taking only lower case for everything)
|
|
||||||
return categoryClient
|
return categoryClient
|
||||||
.searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT)
|
.searchCategoriesForPrefix(term, SEARCH_CATS_LIMIT)
|
||||||
.map(name -> new CategoryItem(name, false));
|
.map(name -> new CategoryItem(name, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -184,12 +183,11 @@ public class CategoriesModel{
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return category for single title
|
* Return category for single title
|
||||||
* title is converted to lower case to make search case-insensitive
|
|
||||||
* @param title
|
* @param title
|
||||||
* @return
|
* @return
|
||||||
*/
|
*/
|
||||||
private Observable<CategoryItem> getTitleCategories(String title) {
|
private Observable<CategoryItem> getTitleCategories(String title) {
|
||||||
return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT)
|
return categoryClient.searchCategories(title, SEARCH_CATS_LIMIT)
|
||||||
.map(name -> new CategoryItem(name, false));
|
.map(name -> new CategoryItem(name, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,7 @@ public class CategoryClient {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Searches for categories starting with the specified string.
|
* Searches for categories starting with the specified string.
|
||||||
*
|
*
|
||||||
* @param prefix The prefix to be searched
|
* @param prefix The prefix to be searched
|
||||||
* @param itemLimit How many results are returned
|
* @param itemLimit How many results are returned
|
||||||
* @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result
|
* @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,10 @@ package fr.free.nrw.commons.category;
|
||||||
import android.os.Parcel;
|
import android.os.Parcel;
|
||||||
import android.os.Parcelable;
|
import android.os.Parcelable;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Represents a Category Item.
|
||||||
|
* Implemented as Parcelable so that its object could be parsed between activity components.
|
||||||
|
*/
|
||||||
public class CategoryItem implements Parcelable {
|
public class CategoryItem implements Parcelable {
|
||||||
private final String name;
|
private final String name;
|
||||||
private boolean selected;
|
private boolean selected;
|
||||||
|
|
@ -24,28 +28,53 @@ public class CategoryItem implements Parcelable {
|
||||||
this.selected = selected;
|
this.selected = selected;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reads from the received Parcel
|
||||||
|
* @param in
|
||||||
|
*/
|
||||||
private CategoryItem(Parcel in) {
|
private CategoryItem(Parcel in) {
|
||||||
name = in.readString();
|
name = in.readString();
|
||||||
selected = in.readInt() == 1;
|
selected = in.readInt() == 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Gets Name
|
||||||
|
* @return
|
||||||
|
*/
|
||||||
public String getName() {
|
public String getName() {
|
||||||
return name;
|
return name;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Checks if that Category Item has been selected.
|
||||||
|
* @return
|
||||||
|
*/
|
||||||
public boolean isSelected() {
|
public boolean isSelected() {
|
||||||
return selected;
|
return selected;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Selects the Category Item.
|
||||||
|
* @param selected
|
||||||
|
*/
|
||||||
public void setSelected(boolean selected) {
|
public void setSelected(boolean selected) {
|
||||||
this.selected = selected;
|
this.selected = selected;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Used by Parcelable
|
||||||
|
* @return
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
public int describeContents() {
|
public int describeContents() {
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Writes to the received Parcel
|
||||||
|
* @param parcel
|
||||||
|
* @param flags
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
public void writeToParcel(Parcel parcel, int flags) {
|
public void writeToParcel(Parcel parcel, int flags) {
|
||||||
parcel.writeString(name);
|
parcel.writeString(name);
|
||||||
|
|
@ -67,13 +96,19 @@ public class CategoryItem implements Parcelable {
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns hash code for current object
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
public int hashCode() {
|
public int hashCode() {
|
||||||
return name.hashCode();
|
return name.hashCode();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return String form of current object
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
public String toString() {
|
public String toString() {
|
||||||
return "CategoryItem: '" + name + '\'';
|
return "CategoryItem: '" + name + '\'';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -17,9 +17,6 @@ class CategoriesModelTest {
|
||||||
@Mock
|
@Mock
|
||||||
internal var categoryInterface: CategoryInterface? = null
|
internal var categoryInterface: CategoryInterface? = null
|
||||||
|
|
||||||
@Mock
|
|
||||||
internal var categoryItem: CategoryItem? = null
|
|
||||||
|
|
||||||
@Spy
|
@Spy
|
||||||
internal lateinit var gson: Gson
|
internal lateinit var gson: Gson
|
||||||
|
|
||||||
|
|
@ -43,28 +40,6 @@ class CategoriesModelTest {
|
||||||
MockitoAnnotations.initMocks(this)
|
MockitoAnnotations.initMocks(this)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Test Case for verifying that Categories search (MW api calls) are case-insensitive
|
|
||||||
@Test
|
|
||||||
fun searchAllFoundCaseTest() {
|
|
||||||
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
|
|
||||||
Mockito.`when`(mwQueryPage.title()).thenReturn("Category:Test")
|
|
||||||
val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
|
|
||||||
Mockito.`when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage))
|
|
||||||
val mockResponse = Mockito.mock(MwQueryResponse::class.java)
|
|
||||||
Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
|
|
||||||
val categoriesModel: CategoriesModel = CategoriesModel(categoryClient,null,null)
|
|
||||||
|
|
||||||
Mockito.`when`(categoryInterface!!.searchCategoriesForPrefix(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()))
|
|
||||||
.thenReturn(Observable.just(mockResponse))
|
|
||||||
|
|
||||||
// Checking if both return "Test"
|
|
||||||
val actualCategoryName = categoriesModel!!.searchAll("tes",null).blockingFirst()
|
|
||||||
assertEquals("Test", actualCategoryName.getName())
|
|
||||||
|
|
||||||
val actualCategoryNameCaps = categoriesModel!!.searchAll("Tes",null).blockingFirst()
|
|
||||||
assertEquals("Test", actualCategoryNameCaps.getName())
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* For testing the substring search algorithm for Categories search
|
* For testing the substring search algorithm for Categories search
|
||||||
* To be more precise it tests the In Between substring( ex: searching `atte`
|
* To be more precise it tests the In Between substring( ex: searching `atte`
|
||||||
|
|
|
||||||
|
|
@ -58,7 +58,6 @@ class CategoryClientTest {
|
||||||
{ fail("SearchCategories returned element when it shouldn't have.") },
|
{ fail("SearchCategories returned element when it shouldn't have.") },
|
||||||
{ s -> throw s })
|
{ s -> throw s })
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun searchCategoriesForPrefixFound() {
|
fun searchCategoriesForPrefixFound() {
|
||||||
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
|
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
|
||||||
|
|
@ -93,7 +92,6 @@ class CategoryClientTest {
|
||||||
{ fail("SearchCategories returned element when it shouldn't have.") },
|
{ fail("SearchCategories returned element when it shouldn't have.") },
|
||||||
{ s -> throw s })
|
{ s -> throw s })
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun getParentCategoryListFound() {
|
fun getParentCategoryListFound() {
|
||||||
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
|
val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue