Update Settings search result unique ids

- SearchResult stableIds are now DocIds from the database
- DocIds are data reference key's hash, when the key is not
empty or null
- Otherwise, DocIds are a hashcode from a set of fields.

Change-Id: Id36f7bf4ceaaa3a2bd326ecafbfe97fd0b247df2
Fixes: 37327194
Test: make RunSettingsRoboTest
This commit is contained in:
Matthew Fritze
2017-05-12 15:59:19 -07:00
parent 7f2d779e15
commit 6efea1e624
12 changed files with 347 additions and 499 deletions

View File

@@ -28,9 +28,7 @@ import android.util.Log;
import com.android.settings.dashboard.SiteMapManager;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -64,12 +62,8 @@ class CursorToSearchResultConverter {
private final String TAG = "CursorConverter";
private final String mQueryText;
private final Context mContext;
private final Set<String> mKeys;
private final int LONG_TITLE_LENGTH = 20;
private static final String[] whiteList = {
@@ -87,19 +81,17 @@ class CursorToSearchResultConverter {
private static final Set<String> prioritySettings = new HashSet(Arrays.asList(whiteList));
public CursorToSearchResultConverter(Context context, String queryText) {
public CursorToSearchResultConverter(Context context) {
mContext = context;
mKeys = new HashSet<>();
mQueryText = queryText;
}
public List<SearchResult> convertCursor(SiteMapManager sitemapManager,
public Set<SearchResult> convertCursor(SiteMapManager sitemapManager,
Cursor cursorResults, int baseRank) {
if (cursorResults == null) {
return null;
}
final Map<String, Context> contextMap = new HashMap<>();
final List<SearchResult> results = new ArrayList<>();
final Set<SearchResult> results = new HashSet<>();
while (cursorResults.moveToNext()) {
SearchResult result = buildSingleSearchResultFromCursor(sitemapManager,
@@ -108,22 +100,12 @@ class CursorToSearchResultConverter {
results.add(result);
}
}
Collections.sort(results);
return results;
}
private SearchResult buildSingleSearchResultFromCursor(SiteMapManager sitemapManager,
Map<String, Context> contextMap, Cursor cursor, int baseRank) {
final String docId = cursor.getString(COLUMN_INDEX_ID);
/* Make sure that this result has not yet been added as a result. Checking the docID
covers the case of multiple queries matching the same row, but we need to also to check
for potentially the same named or slightly varied names pointing to the same page.
*/
if (mKeys.contains(docId)) {
return null;
}
mKeys.add(docId);
final int docId = cursor.getInt(COLUMN_INDEX_ID);
final String pkgName = cursor.getString(COLUMN_INDEX_INTENT_ACTION_TARGET_PACKAGE);
final String title = cursor.getString(COLUMN_INDEX_TITLE);
final String summaryOn = cursor.getString(COLUMN_INDEX_SUMMARY_ON);
@@ -135,15 +117,16 @@ class CursorToSearchResultConverter {
final ResultPayload payload = getUnmarshalledPayload(marshalledPayload, payloadType);
final List<String> breadcrumbs = getBreadcrumbs(sitemapManager, cursor);
final int rank = getRank(title, breadcrumbs, baseRank, key);
final int rank = getRank(title, baseRank, key);
final SearchResult.Builder builder = new SearchResult.Builder();
builder.addTitle(title)
.addSummary(summaryOn)
final SearchResult.Builder builder = new SearchResult.Builder()
.setStableId(docId)
.setTitle(title)
.setSummary(summaryOn)
.addBreadcrumbs(breadcrumbs)
.addRank(rank)
.addIcon(getIconForPackage(contextMap, pkgName, className, iconResStr))
.addPayload(payload);
.setRank(rank)
.setIcon(getIconForPackage(contextMap, pkgName, className, iconResStr))
.setPayload(payload);
return builder.build();
}
@@ -206,27 +189,23 @@ class CursorToSearchResultConverter {
* There are three checks
* A) If the result is prioritized and the highest base level
* B) If the query matches the highest level menu title
* C) If the query matches a subsequent menu title
* D) Is the title longer than 20
* C) If the query is longer than 20
*
* If the query matches A, set it to TOP_RANK
* If the query matches B and C, the offset is 0.
* If the query matches C only, the offset is 1.
* If the query matches neither B nor C, the offset is 2.
* If the query matches D, the offset is 2
* If the query matches B, the offset is 0.
* If the query matches C, the offset is 1
* @param title of the result.
* @param crumbs from the Information Architecture
* @param baseRank of the result. Lower if it's a better result.
* @return
*/
private int getRank(String title, List<String> crumbs, int baseRank, String key) {
private int getRank(String title, int baseRank, String key) {
// The result can only be prioritized if it is a top ranked result.
if (prioritySettings.contains(key) && baseRank < BASE_RANKS[1]) {
return TOP_RANK;
}
if (title.length() > LONG_TITLE_LENGTH) {
return baseRank + 2;
return baseRank + 1;
}
return baseRank;
}

View File

@@ -1061,10 +1061,11 @@ public class DatabaseIndexingManager {
* Returns the doc id for this row.
*/
public int getDocId() {
// The DocID should contains more than the title string itself (you may have two
// settings with the same title). So we need to use a combination of multiple
// attributes from this row.
return Objects.hash(updatedTitle, screenTitle, key, payloadType);
// Eventually we want all DocIds to be the data_reference key. For settings values,
// this will be preference keys, and for non-settings they should be unique.
return TextUtils.isEmpty(key)
? Objects.hash(updatedTitle, className, screenTitle, intentTargetClass)
: key.hashCode();
}
public static class Builder {

View File

@@ -21,13 +21,15 @@ import android.content.Context;
import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;
import android.support.annotation.VisibleForTesting;
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;
import static com.android.settings.search.IndexDatabaseHelper.Tables.TABLE_PREFS_INDEX;
@@ -108,7 +110,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
mSiteMapManager = mapManager;
mContext = context;
mQueryText = cleanQuery(queryText);
mConverter = new CursorToSearchResultConverter(context, mQueryText);
mConverter = new CursorToSearchResultConverter(context);
}
@Override
@@ -122,28 +124,16 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
return null;
}
final List<SearchResult> primaryFirstWordResults;
final List<SearchResult> primaryMidWordResults;
final List<SearchResult> secondaryResults;
final List<SearchResult> tertiaryResults;
final Set<SearchResult> resultSet = new HashSet<>();
primaryFirstWordResults = firstWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[0]);
primaryMidWordResults = secondaryWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[1]);
secondaryResults = anyWordQuery(MATCH_COLUMNS_SECONDARY, BASE_RANKS[2]);
tertiaryResults = anyWordQuery(MATCH_COLUMNS_TERTIARY, BASE_RANKS[3]);
resultSet.addAll(firstWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[0]));
resultSet.addAll(secondaryWordQuery(MATCH_COLUMNS_PRIMARY, BASE_RANKS[1]));
resultSet.addAll(anyWordQuery(MATCH_COLUMNS_SECONDARY, BASE_RANKS[2]));
resultSet.addAll(anyWordQuery(MATCH_COLUMNS_TERTIARY, BASE_RANKS[3]));
final List<SearchResult> results = new ArrayList<>(
primaryFirstWordResults.size()
+ primaryMidWordResults.size()
+ secondaryResults.size()
+ tertiaryResults.size());
results.addAll(primaryFirstWordResults);
results.addAll(primaryMidWordResults);
results.addAll(secondaryResults);
results.addAll(tertiaryResults);
return removeDuplicates(results);
final List<SearchResult> results = new ArrayList<>(resultSet);
Collections.sort(results);
return results;
}
@Override
@@ -171,7 +161,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
* @param baseRank The highest rank achievable by these results
* @return A list of the matching results.
*/
private List<SearchResult> firstWordQuery(String[] matchColumns, int baseRank) {
private Set<SearchResult> firstWordQuery(String[] matchColumns, int baseRank) {
final String whereClause = buildSingleWordWhereClause(matchColumns);
final String query = mQueryText + "%";
final String[] selection = buildSingleWordSelection(query, matchColumns.length);
@@ -187,7 +177,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
* @param baseRank The highest rank achievable by these results
* @return A list of the matching results.
*/
private List<SearchResult> secondaryWordQuery(String[] matchColumns, int baseRank) {
private Set<SearchResult> secondaryWordQuery(String[] matchColumns, int baseRank) {
final String whereClause = buildSingleWordWhereClause(matchColumns);
final String query = "% " + mQueryText + "%";
final String[] selection = buildSingleWordSelection(query, matchColumns.length);
@@ -202,7 +192,7 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
* @param baseRank The highest rank achievable by these results
* @return A list of the matching results.
*/
private List<SearchResult> anyWordQuery(String[] matchColumns, int baseRank) {
private Set<SearchResult> anyWordQuery(String[] matchColumns, int baseRank) {
final String whereClause = buildTwoWordWhereClause(matchColumns);
final String[] selection = buildAnyWordSelection(matchColumns.length * 2);
@@ -217,9 +207,8 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
* @param baseRank The highest rank achievable by these results.
* @return A list of the matching results.
*/
private List<SearchResult> query(String whereClause, String[] selection, int baseRank) {
final SQLiteDatabase database = IndexDatabaseHelper.getInstance(mContext)
.getReadableDatabase();
private Set<SearchResult> query(String whereClause, String[] selection, int baseRank) {
SQLiteDatabase database = IndexDatabaseHelper.getInstance(mContext).getReadableDatabase();
final Cursor resultCursor = database.query(TABLE_PREFS_INDEX, SELECT_COLUMNS, whereClause,
selection, null, null, null);
return mConverter.convertCursor(mSiteMapManager, resultCursor, baseRank);
@@ -299,55 +288,4 @@ public class DatabaseResultLoader extends AsyncLoader<List<? extends SearchResul
}
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);
}
}

View File

@@ -95,10 +95,11 @@ public class InstalledAppResultLoader extends AsyncLoader<List<? extends SearchR
final AppSearchResult.Builder builder = new AppSearchResult.Builder();
builder.setAppInfo(info)
.addTitle(info.loadLabel(pm))
.addRank(getRank(wordDiff))
.setStableId(info.packageName.hashCode())
.setTitle(info.loadLabel(pm))
.setRank(getRank(wordDiff))
.addBreadcrumbs(getBreadCrumb())
.addPayload(new ResultPayload(intent));
.setPayload(new ResultPayload(intent));
results.add(builder.build());
}
}

View File

@@ -68,8 +68,9 @@ public class SavedQueryLoader extends AsyncLoader<List<? extends SearchResult>>
final SavedQueryPayload payload = new SavedQueryPayload(
cursor.getString(cursor.getColumnIndex(SavedQueriesColumns.QUERY)));
results.add(new SearchResult.Builder()
.addTitle(payload.query)
.addPayload(payload)
.setStableId(payload.hashCode())
.setTitle(payload.query)
.setPayload(payload)
.build());
}
return results;

View File

@@ -18,15 +18,18 @@
package com.android.settings.search;
import android.graphics.drawable.Drawable;
import android.text.TextUtils;
import android.util.Log;
import java.util.List;
import java.util.Objects;
/**
* Data class as an interface for all Search Results.
*/
public class SearchResult implements Comparable<SearchResult> {
private static final String TAG = "SearchResult";
/**
* Defines the lowest rank for a search result to be considered as ranked. Results with ranks
* higher than this have no guarantee for sorting order.
@@ -84,9 +87,10 @@ public class SearchResult implements Comparable<SearchResult> {
/**
* Stable id for this object.
*/
public final long stableId;
public final int stableId;
protected SearchResult(Builder builder) {
stableId = builder.mStableId;
title = builder.mTitle;
summary = builder.mSummary;
breadcrumbs = builder.mBreadcrumbs;
@@ -94,7 +98,6 @@ public class SearchResult implements Comparable<SearchResult> {
icon = builder.mIcon;
payload = builder.mResultPayload;
viewType = payload.getType();
stableId = Objects.hash(title, summary, breadcrumbs, rank, viewType);
}
@Override
@@ -118,7 +121,7 @@ public class SearchResult implements Comparable<SearchResult> {
@Override
public int hashCode() {
return (int) stableId;
return stableId;
}
public static class Builder {
@@ -128,13 +131,14 @@ public class SearchResult implements Comparable<SearchResult> {
protected int mRank = 42;
protected ResultPayload mResultPayload;
protected Drawable mIcon;
protected int mStableId;
public Builder addTitle(CharSequence title) {
public Builder setTitle(CharSequence title) {
mTitle = title;
return this;
}
public Builder addSummary(CharSequence summary) {
public Builder setSummary(CharSequence summary) {
mSummary = summary;
return this;
}
@@ -144,29 +148,37 @@ public class SearchResult implements Comparable<SearchResult> {
return this;
}
public Builder addRank(int rank) {
public Builder setRank(int rank) {
if (rank >= 0 && rank <= 9) {
mRank = rank;
}
return this;
}
public Builder addIcon(Drawable icon) {
public Builder setIcon(Drawable icon) {
mIcon = icon;
return this;
}
public Builder addPayload(ResultPayload payload) {
public Builder setPayload(ResultPayload payload) {
mResultPayload = payload;
return this;
}
public Builder setStableId(int stableId) {
mStableId = stableId;
return this;
}
public SearchResult build() {
// Check that all of the mandatory fields are set.
if (mTitle == null) {
throw new IllegalArgumentException("SearchResult missing title argument");
if (TextUtils.isEmpty(mTitle)) {
throw new IllegalStateException("SearchResult missing title argument");
} else if (mStableId == 0) {
Log.v(TAG, "No stable ID on SearchResult with title: " + mTitle);
throw new IllegalStateException("SearchResult missing stableId argument");
} else if (mResultPayload == null) {
throw new IllegalArgumentException("SearchResult missing Payload argument");
throw new IllegalStateException("SearchResult missing Payload argument");
}
return new SearchResult(this);
}