Bug 1311480 - Add support to query client with order by name alphabetically, r=grisha
MozReview-Commit-ID: 7bN7Vh3TqHU
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -426,17 +426,17 @@ public class BrowserContract {
public static final String LAST_USED = "last_used";
// Position of the tab. 0 represents foreground.
public static final String POSITION = "position";
}
public static final class Clients implements CommonColumns {
private Clients() {}
- public static final Uri CONTENT_RECENCY_URI = Uri.withAppendedPath(TABS_AUTHORITY_URI, "clients_recency");
+ public static final Uri CONTENT_NO_STALE_SORTED_URI = Uri.withAppendedPath(TABS_AUTHORITY_URI, "clients_no_stale_sorted");
public static final Uri CONTENT_URI = Uri.withAppendedPath(TABS_AUTHORITY_URI, "clients");
public static final String CONTENT_TYPE = "vnd.android.cursor.dir/client";
public static final String CONTENT_ITEM_TYPE = "vnd.android.cursor.item/client";
// Client-provided name string. Could conceivably be null.
public static final String NAME = "name";
// Sync-assigned GUID for client device. NULL for local tabs.
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalTabsAccessor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalTabsAccessor.java
@@ -59,32 +59,32 @@ public class LocalTabsAccessor implement
BrowserContract.Clients.GUID + " DESC, " +
// Within a single client, most recently used tabs first.
BrowserContract.Tabs.LAST_USED + " DESC";
private static final String LOCAL_CLIENT_SELECTION = BrowserContract.Clients.GUID + " IS NULL";
private static final Pattern FILTERED_URL_PATTERN = Pattern.compile("^(about|chrome|wyciwyg|file):");
- private final Uri clientsRecencyUriWithProfile;
+ private final Uri clientsNoStaleSortedUriWithProfile;
private final Uri tabsUriWithProfile;
private final Uri clientsUriWithProfile;
public LocalTabsAccessor(String profileName) {
tabsUriWithProfile = DBUtils.appendProfileWithDefault(profileName, BrowserContract.Tabs.CONTENT_URI);
clientsUriWithProfile = DBUtils.appendProfileWithDefault(profileName, BrowserContract.Clients.CONTENT_URI);
- clientsRecencyUriWithProfile = DBUtils.appendProfileWithDefault(profileName, BrowserContract.Clients.CONTENT_RECENCY_URI);
+ clientsNoStaleSortedUriWithProfile = DBUtils.appendProfileWithDefault(profileName, BrowserContract.Clients.CONTENT_NO_STALE_SORTED_URI);
}
/**
* Extracts a List of just RemoteClients from a cursor.
- * The supplied cursor should be grouped by guid and sorted by most recently used.
+ * The supplied cursor should be grouped by guid and sorted by name alphabetically.
*/
@Override
- public List<RemoteClient> getClientsWithoutTabsByRecencyFromCursor(Cursor cursor) {
+ public List<RemoteClient> getClientsWithoutTabsNoStaleSortedFromCursor(Cursor cursor) {
final ArrayList<RemoteClient> clients = new ArrayList<>(cursor.getCount());
final int originalPosition = cursor.getPosition();
try {
if (!cursor.moveToFirst()) {
return clients;
}
@@ -163,18 +163,18 @@ public class LocalTabsAccessor implement
} finally {
cursor.moveToPosition(originalPosition);
}
return clients;
}
@Override
- public Cursor getRemoteClientsByRecencyCursor(Context context) {
- final Uri uri = clientsRecencyUriWithProfile;
+ public Cursor getRemoteClientsNoStaleSorted(Context context) {
+ final Uri uri = clientsNoStaleSortedUriWithProfile;
return context.getContentResolver().query(uri, CLIENTS_PROJECTION_COLUMNS,
REMOTE_CLIENTS_SELECTION, null, null);
}
@Override
public Cursor getRemoteTabsCursor(Context context) {
return getRemoteTabsCursor(context, -1);
}
--- a/mobile/android/base/java/org/mozilla/gecko/db/TabsAccessor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/TabsAccessor.java
@@ -12,17 +12,17 @@ import org.mozilla.gecko.Tab;
import java.util.List;
public interface TabsAccessor {
public interface OnQueryTabsCompleteListener {
public void onQueryTabsComplete(List<RemoteClient> clients);
}
- public Cursor getRemoteClientsByRecencyCursor(Context context);
+ public Cursor getRemoteClientsNoStaleSorted(Context context);
public Cursor getRemoteTabsCursor(Context context);
public Cursor getRemoteTabsCursor(Context context, int limit);
- public List<RemoteClient> getClientsWithoutTabsByRecencyFromCursor(final Cursor cursor);
+ public List<RemoteClient> getClientsWithoutTabsNoStaleSortedFromCursor(final Cursor cursor);
public List<RemoteClient> getClientsFromCursor(final Cursor cursor);
public void getTabs(final Context context, final OnQueryTabsCompleteListener listener);
public void getTabs(final Context context, final int limit, final OnQueryTabsCompleteListener listener);
public void persistLocalTabs(final ContentResolver cr, final Iterable<Tab> tabs);
}
--- a/mobile/android/base/java/org/mozilla/gecko/db/TabsProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/TabsProvider.java
@@ -27,17 +27,17 @@ public class TabsProvider extends Shared
static final String TABLE_TABS = "tabs";
static final String TABLE_CLIENTS = "clients";
static final int TABS = 600;
static final int TABS_ID = 601;
static final int CLIENTS = 602;
static final int CLIENTS_ID = 603;
- static final int CLIENTS_RECENCY = 604;
+ static final int CLIENTS_NO_STALE_SORTED = 604;
// Exclude clients that are more than three weeks old and also any duplicates that are older than one week old.
static final String EXCLUDE_STALE_CLIENTS_SUBQUERY =
"(SELECT " + Clients.GUID +
", " + Clients.NAME +
", " + Clients.LAST_MODIFIED +
", " + Clients.DEVICE_TYPE +
" FROM " + TABLE_CLIENTS +
@@ -56,33 +56,33 @@ public class TabsProvider extends Shared
" WHERE (" + Clients.LAST_MODIFIED + " < %1$s" + " AND " + Clients.LAST_MODIFIED + " > %2$s) AND " +
Clients.NAME + " NOT IN " + "( SELECT " + Clients.NAME + " FROM " + TABLE_CLIENTS + " WHERE " + Clients.LAST_MODIFIED + " > %1$s)" +
" GROUP BY " + Clients.NAME +
") AS c2" +
" ON c." + Clients.GUID + " = c2." + Clients.GUID + ")";
static final String DEFAULT_TABS_SORT_ORDER = Clients.LAST_MODIFIED + " DESC, " + Tabs.LAST_USED + " DESC";
static final String DEFAULT_CLIENTS_SORT_ORDER = Clients.LAST_MODIFIED + " DESC";
- static final String DEFAULT_CLIENTS_RECENCY_SORT_ORDER = "COALESCE(MAX(" + Tabs.LAST_USED + "), " + Clients.LAST_MODIFIED + ") DESC";
+ static final String DEFAULT_CLIENTS_NAME_ORDER = Clients.NAME + " COLLATE NOCASE ASC";
static final String INDEX_TABS_GUID = "tabs_guid_index";
static final String INDEX_TABS_POSITION = "tabs_position_index";
static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH);
static final Map<String, String> TABS_PROJECTION_MAP;
static final Map<String, String> CLIENTS_PROJECTION_MAP;
static final Map<String, String> CLIENTS_RECENCY_PROJECTION_MAP;
static {
URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "tabs", TABS);
URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "tabs/#", TABS_ID);
URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "clients", CLIENTS);
URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "clients/#", CLIENTS_ID);
- URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "clients_recency", CLIENTS_RECENCY);
+ URI_MATCHER.addURI(BrowserContract.TABS_AUTHORITY, "clients_no_stale_sorted", CLIENTS_NO_STALE_SORTED);
HashMap<String, String> map;
map = new HashMap<String, String>();
map.put(Tabs._ID, Tabs._ID);
map.put(Tabs.TITLE, Tabs.TITLE);
map.put(Tabs.URL, Tabs.URL);
map.put(Tabs.HISTORY, Tabs.HISTORY);
@@ -305,20 +305,20 @@ public class TabsProvider extends Shared
} else {
debug("Using sort order " + sortOrder + ".");
}
qb.setProjectionMap(CLIENTS_PROJECTION_MAP);
qb.setTables(TABLE_CLIENTS);
break;
- case CLIENTS_RECENCY:
- trace("Query is on CLIENTS_RECENCY: " + uri);
+ case CLIENTS_NO_STALE_SORTED:
+ trace("Query is on CLIENTS_NO_STALE_SORTED: " + uri);
if (TextUtils.isEmpty(sortOrder)) {
- sortOrder = DEFAULT_CLIENTS_RECENCY_SORT_ORDER;
+ sortOrder = DEFAULT_CLIENTS_NAME_ORDER;
} else {
debug("Using sort order " + sortOrder + ".");
}
final long oneWeekAgo = System.currentTimeMillis() - ONE_WEEK_IN_MILLISECONDS;
final long threeWeeksAgo = System.currentTimeMillis() - THREE_WEEKS_IN_MILLISECONDS;
final String excludeStaleClientsTable = String.format(EXCLUDE_STALE_CLIENTS_SUBQUERY, oneWeekAgo, threeWeeksAgo);
--- a/mobile/android/base/java/org/mozilla/gecko/overlays/service/sharemethods/SendTab.java
+++ b/mobile/android/base/java/org/mozilla/gecko/overlays/service/sharemethods/SendTab.java
@@ -225,32 +225,32 @@ public class SendTab extends ShareMethod
@Override
public void executeCommand(final GlobalSession session, List<String> args) {
CommandProcessor.displayURI(args, session.getContext());
}
});
}
/**
- * @return A collection of unique remote clients sorted by most recently used.
+ * @return A collection of unique remote clients sorted by name alphabetically.
*/
protected Collection<RemoteClient> getOtherClients(final TabSender sender) {
if (sender == null) {
Log.w(LOGTAG, "No tab sender when fetching other client IDs.");
return Collections.emptyList();
}
final BrowserDB browserDB = BrowserDB.from(context);
final TabsAccessor tabsAccessor = browserDB.getTabsAccessor();
- final Cursor remoteTabsCursor = tabsAccessor.getRemoteClientsByRecencyCursor(context);
+ final Cursor remoteTabsCursor = tabsAccessor.getRemoteClientsNoStaleSorted(context);
try {
if (remoteTabsCursor.getCount() == 0) {
return Collections.emptyList();
}
- return tabsAccessor.getClientsWithoutTabsByRecencyFromCursor(remoteTabsCursor);
+ return tabsAccessor.getClientsWithoutTabsNoStaleSortedFromCursor(remoteTabsCursor);
} finally {
remoteTabsCursor.close();
}
}
/**
* Inteface for interacting with Sync accounts. Used to hide the difference in implementation
* between FXA and "old sync" accounts when sending tabs.
--- a/mobile/android/base/java/org/mozilla/gecko/widget/ActivityChooserModel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/widget/ActivityChooserModel.java
@@ -1311,17 +1311,17 @@ public class ActivityChooserModel extend
// checking if we have accounts set up - if not, we can't have any clients.
if (!FirefoxAccounts.firefoxAccountsExist(mContext)) {
return false;
}
final BrowserDB browserDB = BrowserDB.from(mContext);
final TabsAccessor tabsAccessor = browserDB.getTabsAccessor();
final Cursor remoteClientsCursor = tabsAccessor
- .getRemoteClientsByRecencyCursor(mContext);
+ .getRemoteClientsNoStaleSorted(mContext);
if (remoteClientsCursor == null) {
return false;
}
try {
return remoteClientsCursor.getCount() > 0;
} finally {
remoteClientsCursor.close();
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/db/TestTabsProviderRemoteTabs.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/db/TestTabsProviderRemoteTabs.java
@@ -29,16 +29,20 @@ import org.robolectric.shadows.ShadowCon
import java.util.List;
@RunWith(TestRunner.class)
public class TestTabsProviderRemoteTabs {
private static final long ONE_DAY_IN_MILLISECONDS = 1000 * 60 * 60 * 24;
private static final long ONE_WEEK_IN_MILLISECONDS = 7 * ONE_DAY_IN_MILLISECONDS;
private static final long THREE_WEEKS_IN_MILLISECONDS = 3 * ONE_WEEK_IN_MILLISECONDS;
+ private static final String CLIENT_LOCAL_NAME = "our local";
+ private static final String CLIENT_REMOTE1_NAME = "The remote1";
+ private static final String CLIENT_REMOTE2_NAME = "another remote2";
+
protected TabsProvider provider;
@Before
public void setUp() {
provider = new TabsProvider();
provider.onCreate();
ShadowContentResolver.registerProvider(BrowserContract.TABS_AUTHORITY, new DelegatingTestContentProvider(provider));
}
@@ -50,17 +54,17 @@ public class TestTabsProviderRemoteTabs
}
protected ContentProviderClient getClientsClient() {
final ShadowContentResolver cr = new ShadowContentResolver();
return cr.acquireContentProviderClient(BrowserContractHelpers.CLIENTS_CONTENT_URI);
}
@Test
- public void testGetClientsWithoutTabsByRecencyFromCursor() throws Exception {
+ public void testGetClientsWithoutTabsNoStaleSortedFromCursor() throws Exception {
final Uri uri = BrowserContractHelpers.CLIENTS_CONTENT_URI;
final ContentProviderClient cpc = getClientsClient();
final LocalTabsAccessor accessor = new LocalTabsAccessor("test"); // The profile name given doesn't matter.
try {
// Delete all tabs to begin with.
cpc.delete(uri, null, null);
Cursor allClients = cpc.query(uri, null, null, null, null);
@@ -69,49 +73,49 @@ public class TestTabsProviderRemoteTabs
} finally {
allClients.close();
}
// Insert a local and remote1 client record, neither with tabs.
final long now = System.currentTimeMillis();
// Local client has GUID = null.
final ContentValues local = new ContentValues();
- local.put(BrowserContract.Clients.NAME, "local");
+ local.put(BrowserContract.Clients.NAME, CLIENT_LOCAL_NAME);
local.put(BrowserContract.Clients.LAST_MODIFIED, now + 1);
// Remote clients have GUID != null.
final ContentValues remote1 = new ContentValues();
remote1.put(BrowserContract.Clients.GUID, "guid1");
- remote1.put(BrowserContract.Clients.NAME, "remote1");
+ remote1.put(BrowserContract.Clients.NAME, CLIENT_REMOTE1_NAME);
remote1.put(BrowserContract.Clients.LAST_MODIFIED, now + 2);
final ContentValues remote2 = new ContentValues();
remote2.put(BrowserContract.Clients.GUID, "guid2");
- remote2.put(BrowserContract.Clients.NAME, "remote2");
+ remote2.put(BrowserContract.Clients.NAME, CLIENT_REMOTE2_NAME);
remote2.put(BrowserContract.Clients.LAST_MODIFIED, now + 3);
ContentValues[] values = new ContentValues[]{local, remote1, remote2};
int inserted = cpc.bulkInsert(uri, values);
Assert.assertEquals(3, inserted);
- allClients = cpc.query(BrowserContract.Clients.CONTENT_RECENCY_URI, null, null, null, null);
+ allClients = cpc.query(BrowserContract.Clients.CONTENT_NO_STALE_SORTED_URI, null, null, null, null);
try {
CursorDumper.dumpCursor(allClients);
// The local client is not ignored.
Assert.assertEquals(3, allClients.getCount());
- final List<RemoteClient> clients = accessor.getClientsWithoutTabsByRecencyFromCursor(allClients);
+ final List<RemoteClient> clients = accessor.getClientsWithoutTabsNoStaleSortedFromCursor(allClients);
Assert.assertEquals(3, clients.size());
for (RemoteClient client : clients) {
// Each client should not have any tabs.
Assert.assertNotNull(client.tabs);
Assert.assertEquals(0, client.tabs.size());
}
- // Since there are no tabs, the order should be based on last_modified.
- Assert.assertEquals("guid2", clients.get(0).guid);
- Assert.assertEquals("guid1", clients.get(1).guid);
- Assert.assertEquals(null, clients.get(2).guid);
+ // Client should sorted by name alphabetically.
+ Assert.assertEquals(CLIENT_REMOTE2_NAME, clients.get(0).name);
+ Assert.assertEquals(CLIENT_LOCAL_NAME, clients.get(1).name);
+ Assert.assertEquals(CLIENT_REMOTE1_NAME, clients.get(2).name);
} finally {
allClients.close();
}
// Now let's add a few tabs to one client. The times are chosen so that one tab's
// last used is not relevant, and the other tab is the most recent used.
final ContentValues remoteTab1 = new ContentValues();
remoteTab1.put(BrowserContract.Tabs.CLIENT_GUID, "guid1");
@@ -128,33 +132,32 @@ public class TestTabsProviderRemoteTabs
remoteTab2.put(BrowserContract.Tabs.HISTORY, "[\"http://test.com/test2\"]");
remoteTab2.put(BrowserContract.Tabs.LAST_USED, now + 5);
remoteTab2.put(BrowserContract.Tabs.POSITION, 1);
values = new ContentValues[]{remoteTab1, remoteTab2};
inserted = cpc.bulkInsert(BrowserContract.Tabs.CONTENT_URI, values);
Assert.assertEquals(2, inserted);
- allClients = cpc.query(BrowserContract.Clients.CONTENT_RECENCY_URI, null, BrowserContract.Clients.GUID + " IS NOT NULL", null, null);
+ allClients = cpc.query(BrowserContract.Clients.CONTENT_NO_STALE_SORTED_URI, null, BrowserContract.Clients.GUID + " IS NOT NULL", null, null);
try {
CursorDumper.dumpCursor(allClients);
// The local client is ignored.
Assert.assertEquals(2, allClients.getCount());
- final List<RemoteClient> clients = accessor.getClientsWithoutTabsByRecencyFromCursor(allClients);
+ final List<RemoteClient> clients = accessor.getClientsWithoutTabsNoStaleSortedFromCursor(allClients);
Assert.assertEquals(2, clients.size());
for (RemoteClient client : clients) {
// Each client should be remote and should not have any tabs.
Assert.assertNotNull(client.guid);
Assert.assertNotNull(client.tabs);
Assert.assertEquals(0, client.tabs.size());
}
- // Since now there is a tab attached to the remote2 client more recent than the
- // remote1 client modified time, it should be first.
- Assert.assertEquals("guid1", clients.get(0).guid);
- Assert.assertEquals("guid2", clients.get(1).guid);
+ // Client should sorted by name alphabetically.
+ Assert.assertEquals(CLIENT_REMOTE2_NAME, clients.get(0).name);
+ Assert.assertEquals(CLIENT_REMOTE1_NAME, clients.get(1).name);
} finally {
allClients.close();
}
} finally {
cpc.release();
}
}
@@ -211,27 +214,27 @@ public class TestTabsProviderRemoteTabs
remote5.put(BrowserContract.Clients.NAME, "remote3");
remote5.put(BrowserContract.Clients.LAST_MODIFIED, now - ONE_WEEK_IN_MILLISECONDS);
ContentValues[] values = new ContentValues[]{local, remote1, remote2, remote3, remote4, remote5};
int inserted = cpc.bulkInsert(uri, values);
Assert.assertEquals(values.length, inserted);
final Cursor remoteClients =
- accessor.getRemoteClientsByRecencyCursor(context);
+ accessor.getRemoteClientsNoStaleSorted(context);
try {
CursorDumper.dumpCursor(remoteClients);
// Local client is not included.
// (remote1, guid1), (remote2, guid2), (remote3, guid3) are expected.
Assert.assertEquals(3, remoteClients.getCount());
// Check the inner data, according to recency.
List<RemoteClient> recentRemoteClientsList =
- accessor.getClientsWithoutTabsByRecencyFromCursor(remoteClients);
+ accessor.getClientsWithoutTabsNoStaleSortedFromCursor(remoteClients);
Assert.assertEquals(3, recentRemoteClientsList.size());
Assert.assertEquals("remote1", recentRemoteClientsList.get(0).name);
Assert.assertEquals("guid1", recentRemoteClientsList.get(0).guid);
Assert.assertEquals("remote2", recentRemoteClientsList.get(1).name);
Assert.assertEquals("guid2", recentRemoteClientsList.get(1).guid);
Assert.assertEquals("remote3", recentRemoteClientsList.get(2).name);
Assert.assertEquals("guid3", recentRemoteClientsList.get(2).guid);
} finally {