Move static search ranking from DatabaseResultLoader to Search Adapter.

This will avoid unnecessary static ranking if smart ranking is used.

Since loader does not need to provided sorted collection of results,
the loading data type has changed from List<> to Set<>. This will also
faster lookup in the Adapter.

Fixes: 38447799
Test: make RunSettingsRoboTests

Change-Id: I448b29bd4e8700c8ec4b5766cbeab4b3087ae39a

src/com/android/settings/search/DatabaseResultLoader.java

@@ -25,10 +25,7 @@ import android.text.TextUtils;
 import com.android.settings.dashboard.SiteMapManager;
 import com.android.settings.utils.AsyncLoader;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 import static com.android.settings.search.IndexDatabaseHelper.IndexColumns;
@@ -37,7 +34,7 @@ import static com.android.settings.search.IndexDatabaseHelper.Tables.TABLE_PREFS
 /**
  * AsyncTask to retrieve Settings, First party app and any intent based results.
  */
-public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResult>> {
+public class DatabaseResultLoader extends AsyncLoader<Set<? extends SearchResult>> {
     private static final String LOG = "DatabaseResultLoader";
 
     /* These indices are used to match the columns of the this loader's SELECT statement.
@@ -114,25 +111,22 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
     }
 
     @Override
-    protected void onDiscardResult(List<? extends SearchResult> result) {
+    protected void onDiscardResult(Set<? extends SearchResult> result) {
         // TODO Search
     }
 
     @Override
-    public List<? extends SearchResult> loadInBackground() {
+    public Set<? extends SearchResult> loadInBackground() {
         if (mQueryText == null || mQueryText.isEmpty()) {
             return null;
         }
 
-        final Set<SearchResult> resultSet = new HashSet<>();
+        final Set<SearchResult> results = new HashSet<>();
 
-        resultSet.addAll(firstWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[0]));
+        results.addAll(firstWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[0]));
-        resultSet.addAll(secondaryWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[1]));
+        results.addAll(secondaryWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[1]));
-        resultSet.addAll(anyWordQuery(MATCH_COLUMNS_SECONDARY, BASE_RANKS[2]));
+        results.addAll(anyWordQuery(MATCH_COLUMNS_SECONDARY, BASE_RANKS[2]));
-        resultSet.addAll(anyWordQuery(MATCH_COLUMNS_TERTIARY, BASE_RANKS[3]));
+        results.addAll(anyWordQuery(MATCH_COLUMNS_TERTIARY, BASE_RANKS[3]));
-
-        final List<SearchResult> results = new ArrayList<>(resultSet);
-        Collections.sort(results);
         return results;
     }
 
@@ -159,7 +153,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
      *
      * @param matchColumns The columns to match on
      * @param baseRank The highest rank achievable by these results
-     * @return A list of the matching results.
+     * @return A set of the matching results.
      */
     private Set<SearchResult> firstWordQuery(String[] matchColumns, int baseRank) {
         final String whereClause = buildSingleWordWhereClause(matchColumns);
@@ -175,7 +169,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
      *
      * @param matchColumns The columns to match on
      * @param baseRank The highest rank achievable by these results
-     * @return A list of the matching results.
+     * @return A set of the matching results.
      */
     private Set<SearchResult> secondaryWordQuery(String[] matchColumns, int baseRank) {
         final String whereClause = buildSingleWordWhereClause(matchColumns);
@@ -190,7 +184,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
      *
      * @param matchColumns The columns to match on
      * @param baseRank The highest rank achievable by these results
-     * @return A list of the matching results.
+     * @return A set of the matching results.
      */
     private Set<SearchResult> anyWordQuery(String[] matchColumns, int baseRank) {
         final String whereClause = buildTwoWordWhereClause(matchColumns);
@@ -205,7 +199,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
      * @param whereClause Where clause for the SQL query which uses bindings.
      * @param selection List of the transformed query to match each bind in the whereClause
      * @param baseRank The highest rank achievable by these results.
-     * @return A list of the matching results.
+     * @return A set of the matching results.
      */
     private Set<SearchResult> query(String whereClause, String[] selection, int baseRank) {
         SQLiteDatabase database = IndexDatabaseHelper.getInstance(mContext).getReadableDatabase();

src/com/android/settings/search/InstalledAppResultLoader.java

@@ -37,14 +37,14 @@ import com.android.settings.applications.PackageManagerWrapper;
 import com.android.settings.dashboard.SiteMapManager;
 import com.android.settings.utils.AsyncLoader;
 
-import java.util.ArrayList;
+import java.util.HashSet;
-import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 /**
  * Search loader for installed apps.
  */
-public class InstalledAppResultLoader extends AsyncLoader<List<? extends SearchResult>> {
+public class InstalledAppResultLoader extends AsyncLoader<Set<? extends SearchResult>> {
 
     private static final int NAME_NO_MATCH = -1;
     private static final Intent LAUNCHER_PROBE = new Intent(Intent.ACTION_MAIN)
@@ -67,8 +67,8 @@ public class InstalledAppResultLoader extends AsyncLoader<List<? extends SearchR
     }
 
     @Override
-    public List<? extends SearchResult> loadInBackground() {
+    public Set<? extends SearchResult> loadInBackground() {
-        final List<AppSearchResult> results = new ArrayList<>();
+        final Set<AppSearchResult> results = new HashSet<>();
         final PackageManager pm = mPackageManager.getPackageManager();
 
         for (UserInfo user : getUsersToCount()) {
@@ -103,7 +103,6 @@ public class InstalledAppResultLoader extends AsyncLoader<List<? extends SearchR
                 results.add(builder.build());
             }
         }
-        Collections.sort(results);
         return results;
     }
 
@@ -124,7 +123,7 @@ public class InstalledAppResultLoader extends AsyncLoader<List<? extends SearchR
     }
 
     @Override
-    protected void onDiscardResult(List<? extends SearchResult> result) {
+    protected void onDiscardResult(Set<? extends SearchResult> result) {
 
     }
 

src/com/android/settings/search/SearchFragment.java

@@ -44,6 +44,7 @@ import com.android.settings.core.instrumentation.MetricsFeatureProvider;
 import com.android.settings.overlay.FeatureFactory;
 
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
@@ -57,7 +58,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  * the query if the user has entered text.
  */
 public class SearchFragment extends InstrumentedFragment implements SearchView.OnQueryTextListener,
-        LoaderManager.LoaderCallbacks<List<? extends SearchResult>>, IndexingCallback {
+        LoaderManager.LoaderCallbacks<Set<? extends SearchResult>>, IndexingCallback {
     private static final String TAG = "SearchFragment";
 
     @VisibleForTesting
@@ -251,7 +252,7 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
     }
 
     @Override
-    public Loader<List<? extends SearchResult>> onCreateLoader(int id, Bundle args) {
+    public Loader<Set<? extends SearchResult>> onCreateLoader(int id, Bundle args) {
         final Activity activity = getActivity();
 
         switch (id) {
@@ -265,8 +266,8 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
     }
 
     @Override
-    public void onLoadFinished(Loader<List<? extends SearchResult>> loader,
+    public void onLoadFinished(Loader<Set<? extends SearchResult>> loader,
-            List<? extends SearchResult> data) {
+            Set<? extends SearchResult> data) {
         mSearchAdapter.addSearchResults(data, loader.getClass().getName());
         if (mUnfinishedLoadersCount.decrementAndGet() != 0) {
             return;
@@ -284,7 +285,7 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
     }
 
     @Override
-    public void onLoaderReset(Loader<List<? extends SearchResult>> loader) {
+    public void onLoaderReset(Loader<Set<? extends SearchResult>> loader) {
     }
 
     /**

src/com/android/settings/search/SearchResultsAdapter.java

@@ -30,15 +30,17 @@ import android.view.ViewGroup;
 import com.android.settings.R;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder> {
 
     private final SearchFragment mFragment;
 
     private List<SearchResult> mSearchResults;
-    private Map<String, List<? extends SearchResult>> mResultsMap;
+    private Map<String, Set<? extends SearchResult>> mResultsMap;
     private final SearchFeatureProvider mSearchFeatureProvider;
 
     public SearchResultsAdapter(SearchFragment fragment,
@@ -98,7 +100,7 @@ public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder>
      * @param loaderClassName class name of the loader.
      */
     @MainThread
-    public void addSearchResults(List<? extends SearchResult> results, String loaderClassName) {
+    public void addSearchResults(Set<? extends SearchResult> results, String loaderClassName) {
         if (results == null) {
             return;
         }
@@ -125,12 +127,22 @@ public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder>
      * @return Number of matched results
      */
     public int displaySearchResults(String query) {
-        final List<? extends SearchResult> databaseResults = mResultsMap
+        List<? extends SearchResult> databaseResults = null;
-                .get(DatabaseResultLoader.class.getName());
+        List<? extends SearchResult> installedAppResults = null;
-        final List<? extends SearchResult> installedAppResults = mResultsMap
+        final String dbLoaderKey = DatabaseResultLoader.class.getName();
-                .get(InstalledAppResultLoader.class.getName());
+        final String appLoaderKey = InstalledAppResultLoader.class.getName();
-        final int dbSize = (databaseResults != null) ? databaseResults.size() : 0;
+        int dbSize = 0;
-        final int appSize = (installedAppResults != null) ? installedAppResults.size() : 0;
+        int appSize = 0;
+        if (mResultsMap.containsKey(dbLoaderKey)) {
+            databaseResults = new ArrayList<>(mResultsMap.get(dbLoaderKey));
+            dbSize = databaseResults.size();
+            Collections.sort(databaseResults);
+        }
+        if (mResultsMap.containsKey(appLoaderKey)) {
+            installedAppResults = new ArrayList<>(mResultsMap.get(appLoaderKey));
+            appSize = installedAppResults.size();
+            Collections.sort(installedAppResults);
+        }
         final List<SearchResult> newResults = new ArrayList<>(dbSize + appSize);
 
         int dbIndex = 0;

tests/robotests/src/com/android/settings/search/DatabaseResultLoaderTest.java

@@ -45,7 +45,10 @@ import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.Matchers.anyString;
@@ -224,17 +227,19 @@ public class DatabaseResultLoaderTest {
     }
 
     @Test
-    public void testSpecialCaseTwoWords_firstWordMatches_ranksHigher() {
+    public void testSpecialCaseTwoWords_multipleResults() {
         final String caseOne = "Apple pear";
         final String caseTwo = "Banana apple";
         insertSpecialCase(caseOne);
         insertSpecialCase(caseTwo);
         DatabaseResultLoader loader = new DatabaseResultLoader(mContext, "App", null);
-        List<? extends SearchResult> results = loader.loadInBackground();
+        Set<? extends SearchResult> results = loader.loadInBackground();
-
+        Set<CharSequence> expectedTitles = new HashSet<>(Arrays.asList(caseOne, caseTwo));
-        assertThat(results.get(0).title).isEqualTo(caseOne);
+        Set<CharSequence> actualTitles = new HashSet<>();
-        assertThat(results.get(1).title).isEqualTo(caseTwo);
+        for (SearchResult result : results) {
-        assertThat(results.get(0).rank).isLessThan(results.get(1).rank);
+            actualTitles.add(result.title);
+        }
+        assertThat(actualTitles).isEqualTo(expectedTitles);
     }
 
     private void insertSpecialCase(String specialCase) {

tests/robotests/src/com/android/settings/search/InstalledAppResultLoaderTest.java

@@ -43,7 +43,9 @@ import org.robolectric.annotation.Config;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import static android.content.pm.ApplicationInfo.FLAG_SYSTEM;
 import static android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP;
@@ -185,18 +187,19 @@ public class InstalledAppResultLoaderTest {
     }
 
     @Test
-    public void query_matchingQuery_shouldRankBasedOnSimilarity() {
+    public void query_matchingQuery_multipleResults() {
         final String query = "app";
 
         mLoader = new InstalledAppResultLoader(mContext, mPackageManagerWrapper, query,
                 mSiteMapManager);
-        final List<? extends SearchResult> results = mLoader.loadInBackground();
+        final Set<? extends SearchResult> results = mLoader.loadInBackground();
 
-        // List is sorted by rank
+        Set<CharSequence> expectedTitles = new HashSet<>(Arrays.asList("app4", "app", "appBuffer"));
-        assertThat(results.get(0).rank).isAtMost(results.get(1).rank);
+        Set<CharSequence> actualTitles = new HashSet<>();
-        assertThat(results.get(0).title).isEqualTo("app4");
+        for (SearchResult result : results) {
-        assertThat(results.get(1).title).isEqualTo("app");
+            actualTitles.add(result.title);
-        assertThat(results.get(2).title).isEqualTo("appBuffer");
+        }
+        assertThat(actualTitles).isEqualTo(expectedTitles);
     }
 
     @Test

tests/robotests/src/com/android/settings/search/MockAppLoader.java

@@ -21,8 +21,8 @@ import android.content.Context;
 import com.android.settings.search.InstalledAppResultLoader;
 import com.android.settings.search.SearchResult;
 
-import java.util.ArrayList;
+import java.util.HashSet;
-import java.util.List;
+import java.util.Set;
 
 /**
  * Mock loader to subvert the requirements of returning data while also driving the Loader
@@ -35,12 +35,12 @@ class MockAppLoader extends InstalledAppResultLoader {
     }
 
     @Override
-    public List<? extends SearchResult> loadInBackground() {
+    public Set<? extends SearchResult> loadInBackground() {
-        return new ArrayList<>();
+        return new HashSet<>();
     }
 
     @Override
-    protected void onDiscardResult(List<? extends SearchResult> result) {
+    protected void onDiscardResult(Set<? extends SearchResult> result) {
 
     }
 }

tests/robotests/src/com/android/settings/search/MockDBLoader.java

@@ -21,8 +21,8 @@ import android.content.Context;
 import com.android.settings.search.DatabaseResultLoader;
 import com.android.settings.search.SearchResult;
 
-import java.util.ArrayList;
+import java.util.HashSet;
-import java.util.List;
+import java.util.Set;
 
 /**
  * Mock loader to subvert the requirements of returning data while also driving the Loader
@@ -35,12 +35,12 @@ class MockDBLoader extends DatabaseResultLoader {
     }
 
     @Override
-    public List<? extends SearchResult> loadInBackground() {
+    public Set<? extends SearchResult> loadInBackground() {
-        return new ArrayList<>();
+        return new HashSet<>();
     }
 
     @Override
-    protected void onDiscardResult(List<? extends SearchResult> result) {
+    protected void onDiscardResult(Set<? extends SearchResult> result) {
 
     }
 }

tests/robotests/src/com/android/settings/search/SearchFragmentTest.java

@@ -41,7 +41,7 @@ import org.robolectric.annotation.Config;
 import org.robolectric.util.ActivityController;
 import org.robolectric.util.ReflectionHelpers;
 
-import java.util.List;
+import java.util.Set;
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.Matchers.any;
@@ -249,7 +249,7 @@ public class SearchFragmentTest {
 
         Robolectric.flushForegroundThreadScheduler();
 
-        verify(fragment, times(2)).onLoadFinished(any(Loader.class), any(List.class));
+        verify(fragment, times(2)).onLoadFinished(any(Loader.class), any(Set.class));
     }
 
     @Test

tests/robotests/src/com/android/settings/search/SearchResultsAdapterTest.java

@@ -46,8 +46,10 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 
 import static com.google.common.truth.Truth.assertThat;
 
@@ -85,7 +87,7 @@ public class SearchResultsAdapterTest {
 
     @Test
     public void testSingleSourceMerge_exactCopyReturned() {
-        ArrayList<SearchResult> intentResults = getIntentSampleResults();
+        Set<SearchResult> intentResults = getIntentSampleResults();
         mAdapter.addSearchResults(intentResults, mLoaderClassName);
         mAdapter.displaySearchResults("");
 
@@ -111,8 +113,10 @@ public class SearchResultsAdapterTest {
 
     @Test
     public void testEndToEndSearch_properResultsMerged_correctOrder() {
-        mAdapter.addSearchResults(getDummyAppResults(), InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<SearchResult>(getDummyAppResults()),
-        mAdapter.addSearchResults(getDummyDbResults(), DatabaseResultLoader.class.getName());
+                InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<SearchResult>(getDummyDbResults()),
+                DatabaseResultLoader.class.getName());
         int count = mAdapter.displaySearchResults("");
 
         List<SearchResult> results = mAdapter.getSearchResults();
@@ -130,13 +134,16 @@ public class SearchResultsAdapterTest {
         List<AppSearchResult> appResults = getDummyAppResults();
         List<SearchResult> dbResults = getDummyDbResults();
         // Add two individual items
-        mAdapter.addSearchResults(appResults.subList(0,1),
+        mAdapter.addSearchResults(new HashSet<SearchResult>(appResults.subList(0, 1)),
                 InstalledAppResultLoader.class.getName());
-        mAdapter.addSearchResults(dbResults.subList(0,1), DatabaseResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<SearchResult>(dbResults.subList(0, 1)),
+                DatabaseResultLoader.class.getName());
         mAdapter.displaySearchResults("");
         // Add super-set of items
-        mAdapter.addSearchResults(appResults, InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(
-        mAdapter.addSearchResults(dbResults, DatabaseResultLoader.class.getName());
+                new HashSet<SearchResult>(appResults), InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(
+                new HashSet<SearchResult>(dbResults), DatabaseResultLoader.class.getName());
         int count = mAdapter.displaySearchResults("");
 
         List<SearchResult> results = mAdapter.getSearchResults();
@@ -170,13 +177,16 @@ public class SearchResultsAdapterTest {
         List<AppSearchResult> appResults = getDummyAppResults();
         List<SearchResult> dbResults = getDummyDbResults();
         // Add list of items
-        mAdapter.addSearchResults(appResults, InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<SearchResult>(appResults),
-        mAdapter.addSearchResults(dbResults, DatabaseResultLoader.class.getName());
+                InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<SearchResult>(dbResults),
+                DatabaseResultLoader.class.getName());
         mAdapter.displaySearchResults("");
         // Add subset of items
-        mAdapter.addSearchResults(appResults.subList(0,1),
+        mAdapter.addSearchResults(new HashSet<SearchResult>(appResults.subList(0, 1)),
                 InstalledAppResultLoader.class.getName());
-        mAdapter.addSearchResults(dbResults.subList(0,1), DatabaseResultLoader.class.getName());
+        mAdapter.addSearchResults(new HashSet<>(dbResults.subList(0, 1)),
+                DatabaseResultLoader.class.getName());
         int count = mAdapter.displaySearchResults("");
 
         List<SearchResult> results = mAdapter.getSearchResults();
@@ -231,8 +241,8 @@ public class SearchResultsAdapterTest {
         return results;
     }
 
-    private ArrayList<SearchResult> getIntentSampleResults() {
+    private Set<SearchResult> getIntentSampleResults() {
-        ArrayList<SearchResult> sampleResults = new ArrayList<>();
+        Set<SearchResult> sampleResults = new HashSet<>();
         ArrayList<String> breadcrumbs = new ArrayList<>();
         final Drawable icon = mContext.getDrawable(R.drawable.ic_search_history);
         final ResultPayload payload = new ResultPayload(null);