Merge "First pass at removing duplicate results"
This commit is contained in:
committed by
Android (Google) Code Review
commit
a563f1b57b
@@ -144,7 +144,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
|
|||||||
results.addAll(secondaryResults);
|
results.addAll(secondaryResults);
|
||||||
results.addAll(tertiaryResults);
|
results.addAll(tertiaryResults);
|
||||||
|
|
||||||
return results;
|
return removeDuplicates(results);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -300,4 +300,55 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
|
|||||||
}
|
}
|
||||||
return selection;
|
return selection;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Goes through the list of search results and verifies that none of the results are duplicates.
|
||||||
|
* A duplicate is quantified by a result with the same Title and the same non-empty Summary.
|
||||||
|
*
|
||||||
|
* The method walks through the results starting with the highest priority result. It removes
|
||||||
|
* the duplicates by doing the first rule that applies below:
|
||||||
|
* - If a result is inline, remove the intent result.
|
||||||
|
* - Remove the lower rank item.
|
||||||
|
* @param results A list of results with potential duplicates
|
||||||
|
* @return The list of results with duplicates removed.
|
||||||
|
*/
|
||||||
|
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
|
||||||
|
List<SearchResult> removeDuplicates(List<SearchResult> results) {
|
||||||
|
SearchResult primaryResult, secondaryResult;
|
||||||
|
|
||||||
|
// We accept the O(n^2) solution because the number of results is small.
|
||||||
|
for (int i = results.size() - 1; i >= 0; i--) {
|
||||||
|
secondaryResult = results.get(i);
|
||||||
|
|
||||||
|
for (int j = i - 1; j >= 0; j--) {
|
||||||
|
primaryResult = results.get(j);
|
||||||
|
if (areDuplicateResults(primaryResult, secondaryResult)) {
|
||||||
|
|
||||||
|
if (primaryResult.viewType != ResultPayload.PayloadType.INTENT) {
|
||||||
|
// Case where both payloads are inline
|
||||||
|
results.remove(i);
|
||||||
|
break;
|
||||||
|
} else if (secondaryResult.viewType != ResultPayload.PayloadType.INTENT) {
|
||||||
|
// Case where only second result is inline
|
||||||
|
results.remove(j);
|
||||||
|
i--; // shift the top index to reflect the lower element being removed
|
||||||
|
} else {
|
||||||
|
// Case where both payloads are intent
|
||||||
|
results.remove(i);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return results;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return True when the two {@link SearchResult SearchResults} have the same title, and the same
|
||||||
|
* non-empty summary.
|
||||||
|
*/
|
||||||
|
private boolean areDuplicateResults(SearchResult primary, SearchResult secondary) {
|
||||||
|
return TextUtils.equals(primary.title, secondary.title)
|
||||||
|
&& TextUtils.equals(primary.summary, secondary.summary)
|
||||||
|
&& !TextUtils.isEmpty(primary.summary);
|
||||||
|
}
|
||||||
}
|
}
|
@@ -19,6 +19,7 @@ package com.android.settings.search2;
|
|||||||
|
|
||||||
import android.content.ContentValues;
|
import android.content.ContentValues;
|
||||||
import android.content.Context;
|
import android.content.Context;
|
||||||
|
import android.content.Intent;
|
||||||
import android.database.sqlite.SQLiteDatabase;
|
import android.database.sqlite.SQLiteDatabase;
|
||||||
|
|
||||||
import com.android.settings.SettingsRobolectricTestRunner;
|
import com.android.settings.SettingsRobolectricTestRunner;
|
||||||
@@ -38,6 +39,7 @@ import org.mockito.MockitoAnnotations;
|
|||||||
import org.robolectric.RuntimeEnvironment;
|
import org.robolectric.RuntimeEnvironment;
|
||||||
import org.robolectric.annotation.Config;
|
import org.robolectric.annotation.Config;
|
||||||
|
|
||||||
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
@@ -58,6 +60,15 @@ public class DatabaseResultLoaderTest {
|
|||||||
private Context mContext;
|
private Context mContext;
|
||||||
private DatabaseResultLoader loader;
|
private DatabaseResultLoader loader;
|
||||||
|
|
||||||
|
private final String titleOne = "titleOne";
|
||||||
|
private final String titleTwo = "titleTwo";
|
||||||
|
private final String titleThree = "titleThree";
|
||||||
|
private final String titleFour = "titleFour";
|
||||||
|
private final String summaryOne = "summaryOne";
|
||||||
|
private final String summaryTwo = "summaryTwo";
|
||||||
|
private final String summaryThree = "summaryThree";
|
||||||
|
private final String summaryFour = "summaryFour";
|
||||||
|
|
||||||
SQLiteDatabase mDb;
|
SQLiteDatabase mDb;
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
@@ -104,49 +115,49 @@ public class DatabaseResultLoaderTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseWord_MatchesNonPrefix() {
|
public void testSpecialCaseWord_matchesNonPrefix() {
|
||||||
insertSpecialCase("Data usage");
|
insertSpecialCase("Data usage");
|
||||||
loader = new DatabaseResultLoader(mContext, "usage", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "usage", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseSpace_Matches() {
|
public void testSpecialCaseSpace_matches() {
|
||||||
insertSpecialCase("space");
|
insertSpecialCase("space");
|
||||||
loader = new DatabaseResultLoader(mContext, " space ", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, " space ", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseDash_MatchesWordNoDash() {
|
public void testSpecialCaseDash_matchesWordNoDash() {
|
||||||
insertSpecialCase("wi-fi calling");
|
insertSpecialCase("wi-fi calling");
|
||||||
loader = new DatabaseResultLoader(mContext, "wifi", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "wifi", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseDash_MatchesWordWithDash() {
|
public void testSpecialCaseDash_matchesWordWithDash() {
|
||||||
insertSpecialCase("priorités seulment");
|
insertSpecialCase("priorités seulment");
|
||||||
loader = new DatabaseResultLoader(mContext, "priorités", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "priorités", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseDash_MatchesWordWithoutDash() {
|
public void testSpecialCaseDash_matchesWordWithoutDash() {
|
||||||
insertSpecialCase("priorités seulment");
|
insertSpecialCase("priorités seulment");
|
||||||
loader = new DatabaseResultLoader(mContext, "priorites", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "priorites", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseDash_MatchesEntireQueryWithoutDash() {
|
public void testSpecialCaseDash_matchesEntireQueryWithoutDash() {
|
||||||
insertSpecialCase("wi-fi calling");
|
insertSpecialCase("wi-fi calling");
|
||||||
loader = new DatabaseResultLoader(mContext, "wifi calling", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "wifi calling", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCasePrefix_MatchesPrefixOfEntry() {
|
public void testSpecialCasePrefix_matchesPrefixOfEntry() {
|
||||||
insertSpecialCase("Photos");
|
insertSpecialCase("Photos");
|
||||||
loader = new DatabaseResultLoader(mContext, "pho", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "pho", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
@@ -160,14 +171,14 @@ public class DatabaseResultLoaderTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseMultiWordPrefix_MatchesPrefixOfEntry() {
|
public void testSpecialCaseMultiWordPrefix_matchesPrefixOfEntry() {
|
||||||
insertSpecialCase("Apps Notifications");
|
insertSpecialCase("Apps Notifications");
|
||||||
loader = new DatabaseResultLoader(mContext, "Apps", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "Apps", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseMultiWordPrefix_MatchesSecondWordPrefixOfEntry() {
|
public void testSpecialCaseMultiWordPrefix_matchesSecondWordPrefixOfEntry() {
|
||||||
insertSpecialCase("Apps Notifications");
|
insertSpecialCase("Apps Notifications");
|
||||||
loader = new DatabaseResultLoader(mContext, "Not", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "Not", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
@@ -188,21 +199,188 @@ public class DatabaseResultLoaderTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseMultiWordPrefixWithSpecial_MatchesPrefixOfEntry() {
|
public void testSpecialCaseMultiWordPrefixWithSpecial_matchesPrefixOfEntry() {
|
||||||
insertSpecialCase("Apps & Notifications");
|
insertSpecialCase("Apps & Notifications");
|
||||||
loader = new DatabaseResultLoader(mContext, "App", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "App", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseMultiWordPrefixWithSpecial_MatchesPrefixOfSecondEntry() {
|
public void testSpecialCaseMultiWordPrefixWithSpecial_matchesPrefixOfSecondEntry() {
|
||||||
insertSpecialCase("Apps & Notifications");
|
insertSpecialCase("Apps & Notifications");
|
||||||
loader = new DatabaseResultLoader(mContext, "No", mSiteMapManager);
|
loader = new DatabaseResultLoader(mContext, "No", mSiteMapManager);
|
||||||
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
assertThat(loader.loadInBackground().size()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testSpecialCaseTwoWords_FirstWordMatches_RanksHigher() {
|
public void testDeDupe_noDuplicates_originalListReturn() {
|
||||||
|
// Three elements with unique titles and summaries
|
||||||
|
List<SearchResult> results = new ArrayList();
|
||||||
|
IntentPayload intentPayload = new IntentPayload(new Intent());
|
||||||
|
|
||||||
|
SearchResult.Builder builder = new SearchResult.Builder();
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultOne = builder.build();
|
||||||
|
results.add(resultOne);
|
||||||
|
|
||||||
|
builder.addTitle(titleTwo)
|
||||||
|
.addSummary(summaryTwo);
|
||||||
|
SearchResult resultTwo = builder.build();
|
||||||
|
results.add(resultTwo);
|
||||||
|
|
||||||
|
builder.addTitle(titleThree)
|
||||||
|
.addSummary(summaryThree);
|
||||||
|
SearchResult resultThree = builder.build();
|
||||||
|
results.add(resultThree);
|
||||||
|
|
||||||
|
loader = new DatabaseResultLoader(mContext, "", null);
|
||||||
|
loader.removeDuplicates(results);
|
||||||
|
assertThat(results.size()).isEqualTo(3);
|
||||||
|
assertThat(results.get(0)).isEqualTo(resultOne);
|
||||||
|
assertThat(results.get(1)).isEqualTo(resultTwo);
|
||||||
|
assertThat(results.get(2)).isEqualTo(resultThree);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testDeDupe_oneDuplicate_duplicateRemoved() {
|
||||||
|
List<SearchResult> results = new ArrayList();
|
||||||
|
IntentPayload intentPayload = new IntentPayload(new Intent());
|
||||||
|
|
||||||
|
SearchResult.Builder builder = new SearchResult.Builder();
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addRank(0)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultOne = builder.build();
|
||||||
|
results.add(resultOne);
|
||||||
|
|
||||||
|
// Duplicate of the first element
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addRank(1);
|
||||||
|
SearchResult resultTwo = builder.build();
|
||||||
|
results.add(resultTwo);
|
||||||
|
|
||||||
|
// Unique
|
||||||
|
builder.addTitle(titleThree)
|
||||||
|
.addSummary(summaryThree);
|
||||||
|
SearchResult resultThree = builder.build();
|
||||||
|
results.add(resultThree);
|
||||||
|
|
||||||
|
loader = new DatabaseResultLoader(mContext, "", null);
|
||||||
|
loader.removeDuplicates(results);
|
||||||
|
assertThat(results.size()).isEqualTo(2);
|
||||||
|
assertThat(results.get(0)).isEqualTo(resultOne);
|
||||||
|
assertThat(results.get(1)).isEqualTo(resultThree);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testDeDupe_firstDupeInline_secondDuplicateRemoved() {
|
||||||
|
List<SearchResult> results = new ArrayList();
|
||||||
|
InlineSwitchPayload inlinePayload = new InlineSwitchPayload("", 0,
|
||||||
|
null);
|
||||||
|
IntentPayload intentPayload = new IntentPayload(new Intent());
|
||||||
|
|
||||||
|
SearchResult.Builder builder = new SearchResult.Builder();
|
||||||
|
// Inline result
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addRank(0)
|
||||||
|
.addPayload(inlinePayload);
|
||||||
|
SearchResult resultOne = builder.build();
|
||||||
|
results.add(resultOne);
|
||||||
|
|
||||||
|
// Duplicate of first result, but Intent Result. Should be removed.
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addRank(1)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultTwo = builder.build();
|
||||||
|
results.add(resultTwo);
|
||||||
|
|
||||||
|
// Unique
|
||||||
|
builder.addTitle(titleThree)
|
||||||
|
.addSummary(summaryThree);
|
||||||
|
SearchResult resultThree = builder.build();
|
||||||
|
results.add(resultThree);
|
||||||
|
|
||||||
|
loader = new DatabaseResultLoader(mContext, "", null);
|
||||||
|
loader.removeDuplicates(results);
|
||||||
|
assertThat(results.size()).isEqualTo(2);
|
||||||
|
assertThat(results.get(0)).isEqualTo(resultOne);
|
||||||
|
assertThat(results.get(1)).isEqualTo(resultThree);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testDeDupe_secondDupeInline_firstDuplicateRemoved() {
|
||||||
|
/*
|
||||||
|
* Create a list as follows:
|
||||||
|
* (5) Intent Four
|
||||||
|
* (4) Inline Two
|
||||||
|
* (3) Intent Three
|
||||||
|
* (2) Intent Two
|
||||||
|
* (1) Intent One
|
||||||
|
*
|
||||||
|
* After removing duplicates:
|
||||||
|
* (4) Intent Four
|
||||||
|
* (3) Inline Two
|
||||||
|
* (2) Intent Three
|
||||||
|
* (1) Intent One
|
||||||
|
*/
|
||||||
|
List<SearchResult> results = new ArrayList();
|
||||||
|
InlineSwitchPayload inlinePayload = new InlineSwitchPayload("", 0,
|
||||||
|
null);
|
||||||
|
IntentPayload intentPayload = new IntentPayload(new Intent());
|
||||||
|
|
||||||
|
|
||||||
|
SearchResult.Builder builder = new SearchResult.Builder();
|
||||||
|
// Intent One
|
||||||
|
builder.addTitle(titleOne)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultOne = builder.build();
|
||||||
|
results.add(resultOne);
|
||||||
|
|
||||||
|
// Intent Two
|
||||||
|
builder.addTitle(titleTwo)
|
||||||
|
.addSummary(summaryTwo)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultTwo = builder.build();
|
||||||
|
results.add(resultTwo);
|
||||||
|
|
||||||
|
// Intent Three
|
||||||
|
builder.addTitle(titleThree)
|
||||||
|
.addSummary(summaryThree);
|
||||||
|
SearchResult resultThree = builder.build();
|
||||||
|
results.add(resultThree);
|
||||||
|
|
||||||
|
// Inline Two
|
||||||
|
builder.addTitle(titleTwo)
|
||||||
|
.addSummary(summaryTwo)
|
||||||
|
.addPayload(inlinePayload);
|
||||||
|
SearchResult resultFour = builder.build();
|
||||||
|
results.add(resultFour);
|
||||||
|
|
||||||
|
// Intent Four
|
||||||
|
builder.addTitle(titleFour)
|
||||||
|
.addSummary(summaryOne)
|
||||||
|
.addPayload(intentPayload);
|
||||||
|
SearchResult resultFive = builder.build();
|
||||||
|
results.add(resultFive);
|
||||||
|
|
||||||
|
loader = new DatabaseResultLoader(mContext, "", null);
|
||||||
|
loader.removeDuplicates(results);
|
||||||
|
assertThat(results.size()).isEqualTo(4);
|
||||||
|
assertThat(results.get(0)).isEqualTo(resultOne);
|
||||||
|
assertThat(results.get(1)).isEqualTo(resultThree);
|
||||||
|
assertThat(results.get(2)).isEqualTo(resultFour);
|
||||||
|
assertThat(results.get(3)).isEqualTo(resultFive);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testSpecialCaseTwoWords_firstWordMatches_ranksHigher() {
|
||||||
final String caseOne = "Apple pear";
|
final String caseOne = "Apple pear";
|
||||||
final String caseTwo = "Banana apple";
|
final String caseTwo = "Banana apple";
|
||||||
insertSpecialCase(caseOne);
|
insertSpecialCase(caseOne);
|
||||||
|
Reference in New Issue
Block a user