Bug 1329131 - Part 1: Implement the functionality of listing all bookmark folders except special folders. r?Grisha,ahunt draft
authorJing-wei Wu <topwu.tw@gmail.com>
Fri, 21 Apr 2017 18:31:03 +0800
changeset 572742 7befe17e1d4e18fc1a308b0bc79b1172df77c3cc
parent 572730 0b255199db9d6a6f189b89b7906f99155bde3726
child 572743 7a058d7ff4dbbdd6e118fed0a7a7a195b07d194e
push id57174
push userbmo:topwu.tw@gmail.com
push dateThu, 04 May 2017 17:39:17 +0000
reviewersGrisha, ahunt
bugs1329131
milestone55.0a1
Bug 1329131 - Part 1: Implement the functionality of listing all bookmark folders except special folders. r?Grisha,ahunt MozReview-Commit-ID: AK79E6kkXFI
mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/db/LocalBrowserDBTest.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
@@ -1,15 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.db;
 
-import java.io.File;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
 
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.db.BrowserContract.ExpirePriority;
 import org.mozilla.gecko.distribution.Distribution;
@@ -18,16 +17,17 @@ import org.mozilla.gecko.icons.decoders.
 import android.content.ContentProviderClient;
 import android.content.ContentProviderOperation;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.database.ContentObserver;
 import android.database.Cursor;
 import android.graphics.drawable.BitmapDrawable;
 import android.net.Uri;
+import android.support.annotation.Nullable;
 import android.support.v4.content.CursorLoader;
 
 /**
  * Interface for interactions with all databases. If you want an instance
  * that implements this, you should go through GeckoProfile. E.g.,
  * <code>BrowserDB.from(context)</code>.
  */
 public abstract class BrowserDB {
@@ -103,32 +103,33 @@ public abstract class BrowserDB {
     public abstract void clearHistory(ContentResolver cr, boolean clearSearchHistory);
 
 
     public abstract String getUrlForKeyword(ContentResolver cr, String keyword);
 
     public abstract boolean isBookmark(ContentResolver cr, String uri);
     public abstract boolean addBookmark(ContentResolver cr, String title, String uri);
     public abstract Uri addBookmarkFolder(ContentResolver cr, String title, long parentId);
-    public abstract Cursor getBookmarkForUrl(ContentResolver cr, String url);
-    public abstract Cursor getBookmarksForPartialUrl(ContentResolver cr, String partialUrl);
-    public abstract Cursor getBookmarkById(ContentResolver cr, long id);
+    @Nullable public abstract Cursor getBookmarkForUrl(ContentResolver cr, String url);
+    @Nullable public abstract Cursor getBookmarksForPartialUrl(ContentResolver cr, String partialUrl);
+    @Nullable public abstract Cursor getBookmarkById(ContentResolver cr, long id);
+    @Nullable public abstract Cursor getAllBookmarkFolders(ContentResolver cr);
     public abstract void removeBookmarksWithURL(ContentResolver cr, String uri);
     public abstract void removeBookmarkWithId(ContentResolver cr, long id);
     public abstract void registerBookmarkObserver(ContentResolver cr, ContentObserver observer);
     public abstract void updateBookmark(ContentResolver cr, long id, String uri, String title, String keyword);
     public abstract void updateBookmark(ContentResolver cr, long id, String uri, String title, String keyword, long newParentId, long oldParentId);
     public abstract boolean hasBookmarkWithGuid(ContentResolver cr, String guid);
 
     public abstract boolean insertPageMetadata(ContentProviderClient contentProviderClient, String pageUrl, boolean hasImage, String metadataJSON);
     public abstract int deletePageMetadata(ContentProviderClient contentProviderClient, String pageUrl);
     /**
      * Can return <code>null</code>.
      */
-    public abstract Cursor getBookmarksInFolder(ContentResolver cr, long folderId);
+    @Nullable public abstract Cursor getBookmarksInFolder(ContentResolver cr, long folderId);
 
     public abstract int getBookmarkCountForFolder(ContentResolver cr, long folderId);
 
     /**
      * Get the favicon from the database, if any, associated with the given favicon URL. (That is,
      * the URL of the actual favicon image, not the URL of the page with which the favicon is associated.)
      * @param cr The ContentResolver to use.
      * @param faviconURL The URL of the favicon to fetch from the database.
@@ -146,17 +147,17 @@ public abstract class BrowserDB {
 
     /**
      * Query for non-null thumbnails matching the provided <code>urls</code>.
      * The returned cursor will have no more than, but possibly fewer than,
      * the requested number of thumbnails.
      *
      * Returns null if the provided list of URLs is empty or null.
      */
-    public abstract Cursor getThumbnailsForUrls(ContentResolver cr,
+    @Nullable public abstract Cursor getThumbnailsForUrls(ContentResolver cr,
             List<String> urls);
 
     public abstract void removeThumbnails(ContentResolver cr);
 
     // Utility function for updating existing history using batch operations
     public abstract void updateHistoryInBatch(ContentResolver cr,
             Collection<ContentProviderOperation> operations, String url,
             String title, long date, int visits);
@@ -185,17 +186,17 @@ public abstract class BrowserDB {
     public abstract int getSuggestedBackgroundColorForUrl(String url);
 
     /**
      * Obtain a set of recently visited links to rank.
      *
      * @param contentResolver to load the cursor.
      * @param limit Maximum number of results to return.
      */
-    public abstract Cursor getHighlightCandidates(ContentResolver contentResolver, int limit);
+    @Nullable public abstract Cursor getHighlightCandidates(ContentResolver contentResolver, int limit);
 
     /**
      * Block a page from the highlights list.
      *
      * @param url The page URL. Only pages exactly matching this URL will be blocked.
      */
     public abstract void blockActivityStreamSite(ContentResolver cr, String url);
 
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -1445,16 +1445,17 @@ public class LocalBrowserDB extends Brow
     /**
      * Query for non-null thumbnails matching the provided <code>urls</code>.
      * The returned cursor will have no more than, but possibly fewer than,
      * the requested number of thumbnails.
      *
      * Returns null if the provided list of URLs is empty or null.
      */
     @Override
+    @Nullable
     public Cursor getThumbnailsForUrls(ContentResolver cr, List<String> urls) {
         final int urlCount = urls.size();
         if (urlCount == 0) {
             return null;
         }
 
         // Don't match against null thumbnails.
         final String selection = Thumbnails.DATA + " IS NOT NULL AND " +
@@ -1788,16 +1789,17 @@ public class LocalBrowserDB extends Brow
             return c.getCount() > 0;
         } finally {
             c.close();
         }
     }
 
     @Override
     @RobocopTarget
+    @Nullable
     public Cursor getBookmarkForUrl(ContentResolver cr, String url) {
         Cursor c = cr.query(bookmarksUriWithLimit(1),
                             new String[] { Bookmarks._ID,
                                            Bookmarks.URL,
                                            Bookmarks.TITLE,
                                            Bookmarks.TYPE,
                                            Bookmarks.PARENT,
                                            Bookmarks.GUID,
@@ -1810,16 +1812,17 @@ public class LocalBrowserDB extends Brow
             c.close();
             c = null;
         }
 
         return c;
     }
 
     @Override
+    @Nullable
     public Cursor getBookmarkById(ContentResolver cr, long id) {
         final Cursor c = cr.query(mBookmarksUriWithProfile,
                                   new String[] { Bookmarks._ID,
                                                  Bookmarks.URL,
                                                  Bookmarks.TITLE,
                                                  Bookmarks.TYPE,
                                                  Bookmarks.PARENT,
                                                  Bookmarks.GUID,
@@ -1832,16 +1835,40 @@ public class LocalBrowserDB extends Brow
             c.close();
             return null;
         }
 
         return c;
     }
 
     @Override
+    @Nullable
+    public Cursor getAllBookmarkFolders(ContentResolver cr) {
+        final Cursor cursor = cr.query(mBookmarksUriWithProfile,
+                                       DEFAULT_BOOKMARK_COLUMNS,
+                                       Bookmarks.TYPE + " = ? AND " +
+                                       Bookmarks.GUID + " NOT IN (?, ?, ?, ?, ?) AND " +
+                                       Bookmarks.IS_DELETED + " = 0",
+                                       new String[] { String.valueOf(Bookmarks.TYPE_FOLDER),
+                                                      Bookmarks.SCREENSHOT_FOLDER_GUID,
+                                                      Bookmarks.FAKE_READINGLIST_SMARTFOLDER_GUID,
+                                                      Bookmarks.TAGS_FOLDER_GUID,
+                                                      Bookmarks.PLACES_FOLDER_GUID,
+                                                      Bookmarks.PINNED_FOLDER_GUID },
+                                       null);
+        if (desktopBookmarksExist(cr)) {
+            final Cursor desktopCursor = getSpecialFolderCursor(Bookmarks.FAKE_DESKTOP_FOLDER_ID,
+                                                                Bookmarks.FAKE_DESKTOP_FOLDER_GUID);
+            return new MergeCursor(new Cursor[] { cursor, desktopCursor });
+        }
+        return cursor;
+    }
+
+    @Override
+    @Nullable
     public Cursor getBookmarksForPartialUrl(ContentResolver cr, String partialUrl) {
         Cursor c = cr.query(mBookmarksUriWithProfile,
                 new String[] { Bookmarks.GUID, Bookmarks._ID, Bookmarks.URL },
                 Bookmarks.URL + " LIKE '%" + partialUrl + "%'", // TODO: Escaping!
                 null,
                 null);
 
         if (c != null && c.getCount() == 0) {
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/db/LocalBrowserDBTest.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/db/LocalBrowserDBTest.java
@@ -1,41 +1,47 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 package org.mozilla.gecko.db;
 
+import android.annotation.SuppressLint;
 import android.content.ContentProviderClient;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.RemoteException;
+import android.support.annotation.Nullable;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.db.DelegatingTestContentProvider;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.mozilla.gecko.sync.repositories.android.BrowserContractHelpers;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.shadows.ShadowContentResolver;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(TestRunner.class)
 public class LocalBrowserDBTest {
     private static final long INVALID_ID = -1;
     private final String BOOKMARK_URL = "https://www.mozilla.org";
     private final String BOOKMARK_TITLE = "mozilla";
+    private final String BOOKMARK_GUID = "guid1";
 
     private final String UPDATE_URL = "https://bugzilla.mozilla.org";
     private final String UPDATE_TITLE = "bugzilla";
 
     private final String FOLDER_NAME = "folder1";
 
     private Context context;
     private BrowserProvider provider;
@@ -54,17 +60,17 @@ public class LocalBrowserDBTest {
 
     @After
     public void tearDown() {
         bookmarkClient.release();
         provider.shutdown();
     }
 
     @Test
-    public void testRemoveBookmarkWithURL() {
+    public void testRemoveBookmarkWithURL() throws Exception {
         BrowserDB db = new LocalBrowserDB("default");
         ContentResolver cr = context.getContentResolver();
 
         db.addBookmark(cr, BOOKMARK_TITLE, BOOKMARK_URL);
         Cursor cursor = db.getBookmarkForUrl(cr, BOOKMARK_URL);
         assertNotNull(cursor);
 
         long parentId = INVALID_ID;
@@ -93,17 +99,17 @@ public class LocalBrowserDBTest {
         assertNull(cursor);
 
         // Check parent's lastModified timestamp is updated
         final long lastModifiedAfterRemove = getModifiedDate(parentId);
         assertTrue(lastModifiedAfterRemove > lastModifiedBeforeRemove);
     }
 
     @Test
-    public void testRemoveBookmarkWithId() {
+    public void testRemoveBookmarkWithId() throws Exception {
         BrowserDB db = new LocalBrowserDB("default");
         ContentResolver cr = context.getContentResolver();
 
         db.addBookmark(cr, BOOKMARK_TITLE, BOOKMARK_URL);
         Cursor cursor = db.getBookmarkForUrl(cr, BOOKMARK_URL);
         assertNotNull(cursor);
 
         long bookmarkId = INVALID_ID;
@@ -223,17 +229,17 @@ public class LocalBrowserDBTest {
             assertTrue(cursor.moveToFirst());
 
             final String updatedUrl = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.URL));
             assertEquals(updatedUrl, UPDATE_URL);
 
             final String updatedTitle = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.TITLE));
             assertEquals(updatedTitle, UPDATE_TITLE);
 
-            final long parentId = cursor.getLong(cursor.getColumnIndex(BrowserContract.Bookmarks.PARENT));
+            final long parentId = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.PARENT));
             assertEquals(parentId, newParentId);
         } finally {
             cursor.close();
         }
 
         // Check parent's lastModified timestamp
         final long originalParentLastModifiedAfterUpdate = getModifiedDate(originalParentId);
         assertTrue(originalParentLastModifiedAfterUpdate > originalParentLastModifiedBeforeUpdate);
@@ -246,18 +252,16 @@ public class LocalBrowserDBTest {
     public void testAddBookmarkFolder() throws Exception {
         BrowserDB db = new LocalBrowserDB("default");
         ContentResolver cr = context.getContentResolver();
 
         // Add a bookmark folder record
         final long rootFolderId = getBookmarkIdFromGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID);
         final long lastModifiedBeforeAdd = getModifiedDate(rootFolderId);
         final Uri folderUri = db.addBookmarkFolder(cr, FOLDER_NAME, rootFolderId);
-        assertNotNull(folderUri);
-
         // Get id from Uri
         long folderId = Long.valueOf(folderUri.getLastPathSegment());
 
         final Cursor cursor = db.getBookmarkById(cr, folderId);
         assertNotNull(cursor);
         try {
             assertTrue(cursor.moveToFirst());
 
@@ -300,16 +304,63 @@ public class LocalBrowserDBTest {
             cursor.close();
         }
 
         // Check parent's lastModified timestamp is updated
         final long lastModifiedAfterAdd = getModifiedDate(rootFolderId);
         assertTrue(lastModifiedAfterAdd > lastModifiedBeforeAdd);
     }
 
+    @Test
+    public void testGetAllBookmarkFolders() throws Exception {
+        final BrowserDB db = new LocalBrowserDB("default");
+        final ContentResolver cr = context.getContentResolver();
+
+        // Get folder sparse array directly through BrowserProvider
+        final Map<Long, Folder> folderMap = getFolderMap();
+        final int folderCount = folderMap.size();
+
+        final long parentId = getBookmarkIdFromGuid(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID);
+        assertNotEquals(parentId, INVALID_ID);
+
+        // Create 2 folders into 'mobile bookmarks' and put to folderSparseArray.
+        final Uri childUri1 = db.addBookmarkFolder(cr, "child-1", parentId);
+        final Folder childFolder1 = getFolder(childUri1);
+        assertNotNull(childFolder1);
+        folderMap.put(childFolder1.id, childFolder1);
+
+        final Uri childUri2 = db.addBookmarkFolder(cr, "child-2", parentId);
+        final Folder childFolder2 = getFolder(childUri2);
+        assertNotNull(childFolder2);
+        folderMap.put(childFolder2.id, childFolder2);
+
+        // Create a bookmark, adding a bookmark should not increase folder count.
+        db.addBookmark(cr, BOOKMARK_TITLE, BOOKMARK_URL);
+
+        final Cursor cursor = db.getAllBookmarkFolders(cr);
+        assertNotNull(cursor);
+
+        try {
+            // Current folder count should equal (folderCount + 2) since we added 2 folders.
+            final int currentFolderCount = cursor.getCount();
+            assertEquals(folderCount + 2, currentFolderCount);
+
+            while (cursor.moveToNext()) {
+                final long id = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
+                final String title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.TITLE));
+                final String guid = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.GUID));
+                final Folder actual = new Folder(id, title, guid);
+                final Folder expected = folderMap.get(id);
+                assertEquals(expected, actual);
+            }
+        } finally {
+            cursor.close();
+        }
+    }
+
     private long getBookmarkIdFromGuid(String guid) throws RemoteException {
         Cursor cursor = bookmarkClient.query(BrowserContract.Bookmarks.CONTENT_URI,
                                              new String[] { BrowserContract.Bookmarks._ID },
                                              BrowserContract.Bookmarks.GUID + " = ?",
                                              new String[] { guid },
                                              null);
         assertNotNull(cursor);
 
@@ -319,27 +370,144 @@ public class LocalBrowserDBTest {
             id = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
         } finally {
             cursor.close();
         }
         assertNotEquals(id, INVALID_ID);
         return id;
     }
 
-    private long getModifiedDate(long id) {
-        Cursor cursor = provider.query(BrowserContract.Bookmarks.CONTENT_URI,
-                                       new String[] { BrowserContract.Bookmarks.DATE_MODIFIED },
-                                       BrowserContract.Bookmarks._ID + " = ?",
-                                       new String[] { String.valueOf(id) },
-                                       null);
+    private long getModifiedDate(long id) throws RemoteException {
+        Cursor cursor = bookmarkClient.query(BrowserContract.Bookmarks.CONTENT_URI,
+                                             new String[] { BrowserContract.Bookmarks.DATE_MODIFIED },
+                                             BrowserContract.Bookmarks._ID + " = ?",
+                                             new String[] { String.valueOf(id) },
+                                             null);
         assertNotNull(cursor);
 
         long modified = -1;
         try {
             assertTrue(cursor.moveToFirst());
             modified = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_MODIFIED));
         } finally {
             cursor.close();
         }
         assertNotEquals(modified, -1);
         return modified;
     }
+
+    private Map<Long, Folder> getFolderMap() throws RemoteException {
+        final String selection = BrowserContract.Bookmarks.TYPE + " = ? AND " +
+                                         BrowserContract.Bookmarks.GUID + " NOT IN (?, ?, ?, ?, ?) AND " +
+                                         BrowserContract.Bookmarks.IS_DELETED + " = 0";
+        final String[] selectionArgs = { String.valueOf(BrowserContract.Bookmarks.TYPE_FOLDER),
+                                         BrowserContract.Bookmarks.SCREENSHOT_FOLDER_GUID,
+                                         BrowserContract.Bookmarks.FAKE_READINGLIST_SMARTFOLDER_GUID,
+                                         BrowserContract.Bookmarks.TAGS_FOLDER_GUID,
+                                         BrowserContract.Bookmarks.PLACES_FOLDER_GUID,
+                                         BrowserContract.Bookmarks.PINNED_FOLDER_GUID };
+
+        final Cursor cursor = bookmarkClient.query(BrowserContract.Bookmarks.CONTENT_URI,
+                                                   new String[] { BrowserContract.Bookmarks._ID,
+                                                                  BrowserContract.Bookmarks.TITLE,
+                                                                  BrowserContract.Bookmarks.GUID },
+                                                   selection,
+                                                   selectionArgs,
+                                                   null);
+        assertNotNull(cursor);
+
+        @SuppressLint("UseSparseArrays") final Map<Long, Folder> array = new HashMap<>();
+        try {
+            while (cursor.moveToNext()) {
+                final long id = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
+                final String title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.TITLE));
+                final String guid = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.GUID));
+
+                final Folder folder = new Folder(id, title, guid);
+                array.put(id, folder);
+            }
+        } finally {
+            cursor.close();
+        }
+        return array;
+    }
+
+    @Nullable
+    private Folder getFolder(Uri uri) throws RemoteException {
+        final Cursor cursor = bookmarkClient.query(uri,
+                                                   new String[] { BrowserContract.Bookmarks._ID,
+                                                                  BrowserContract.Bookmarks.TITLE,
+                                                                  BrowserContract.Bookmarks.GUID },
+                                                   null,
+                                                   null,
+                                                   null);
+        assertNotNull(cursor);
+        try {
+            if (cursor.moveToFirst()) {
+                final long id = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
+                final String title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.TITLE));
+                final String guid = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.GUID));
+                return new Folder(id, title, guid);
+            }
+        } finally {
+            cursor.close();
+        }
+        return null;
+    }
+
+    /**
+     * A private struct to make it easier to access folder data.
+     */
+    private static class Folder {
+        private final long id;
+        private final String title;
+        private final String guid;
+
+        private Folder(long id, String title, String guid) {
+            this.id = id;
+            this.title = title;
+            this.guid = guid;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (o == null) {
+                return false;
+            }
+            if (!(o instanceof Folder)) {
+                return false;
+            }
+            if (this == o) {
+                return true;
+            }
+
+            final Folder other = (Folder) o;
+            if (this.id != other.id) {
+                return false;
+            }
+            if (this.title == null) {
+                if (other.title != null) {
+                    return false;
+                }
+            } else if (!this.title.equals(other.title)) {
+                return false;
+            }
+            if (this.guid == null) {
+                if (other.guid != null) {
+                    return false;
+                }
+            } else if (!this.guid.equals(other.guid)) {
+                return false;
+            }
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            final int prime = 31;
+            int result = 1;
+            result = prime * result + (int) id;
+            result = prime * result + ((title == null) ? 0 : title.hashCode());
+            result = prime * result + ((guid == null) ? 0 : guid.hashCode());
+            return result;
+        }
+    }
 }