Bug 926430 - Part 4: Fix pinning and other DB use to correctly handle update-or-insert. r=bnicholson, a=bajaj
authorRichard Newman <rnewman@mozilla.com>
Thu, 31 Oct 2013 10:35:17 -0700
changeset 167336 f8f0cd453b9539f9a764166c6d7cc5bbe218f4c6
parent 167335 a5dbb1214ea908f89b2ab24e59e15b14827061a3
child 167337 c866ce03a2a61a579fdda76ca719633110ec0555
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbnicholson, bajaj
bugs926430
milestone27.0a2
Bug 926430 - Part 4: Fix pinning and other DB use to correctly handle update-or-insert. r=bnicholson, a=bajaj
mobile/android/base/db/BrowserProvider.java
mobile/android/base/db/LocalBrowserDB.java
mobile/android/base/tests/testBrowserProvider.java.in
--- a/mobile/android/base/db/BrowserProvider.java
+++ b/mobile/android/base/db/BrowserProvider.java
@@ -2846,19 +2846,22 @@ public class BrowserProvider extends Con
 
 
     int updateOrInsertBookmark(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         int updated = updateBookmarks(uri, values, selection, selectionArgs);
         if (updated > 0)
             return updated;
 
-        insertBookmark(uri, values);
-
-        // Return 0 if we added a new row
+        if (0 <= insertBookmark(uri, values)) {
+            // We 'updated' one row.
+            return 1;
+        }
+
+        // If something went wrong, then we updated zero rows.
         return 0;
     }
 
     int updateBookmarks(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         trace("Updating bookmarks on URI: " + uri);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
@@ -2925,19 +2928,20 @@ public class BrowserProvider extends Con
             return updated;
 
         // Insert a new entry if necessary
         if (!values.containsKey(History.VISITS))
             values.put(History.VISITS, 1);
         if (!values.containsKey(History.TITLE))
             values.put(History.TITLE, values.getAsString(History.URL));
 
-        insertHistory(uri, values);
-
-        // Return 0 if we added a new row
+        if (0 <= insertHistory(uri, values)) {
+            return 1;
+        }
+
         return 0;
     }
 
     int updateHistory(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         trace("Updating history on URI: " + uri);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
--- a/mobile/android/base/db/LocalBrowserDB.java
+++ b/mobile/android/base/db/LocalBrowserDB.java
@@ -595,34 +595,34 @@ public class LocalBrowserDB implements B
         } finally {
             if (c != null)
                 c.close();
         }
 
         // Restore deleted record if possible
         values.put(Bookmarks.IS_DELETED, 0);
 
-        int updated = cr.update(mBookmarksUriWithProfile,
-                                values,
-                                Bookmarks.URL + " = ? AND " +
-                                Bookmarks.PARENT + " = ?",
-                                new String[] { uri, String.valueOf(folderId) });
-
-        if (updated == 0)
-            cr.insert(mBookmarksUriWithProfile, values);
+        final Uri bookmarksWithInsert = mBookmarksUriWithProfile.buildUpon()
+                                          .appendQueryParameter(BrowserContract.PARAM_INSERT_IF_NEEDED, "true")
+                                          .build();
+        cr.update(bookmarksWithInsert,
+                  values,
+                  Bookmarks.URL + " = ? AND " +
+                  Bookmarks.PARENT + " = " + folderId,
+                  new String[] { uri });
 
         // Bump parent modified time using its ID.
         debug("Bumping parent modified time for addition to: " + folderId);
         final String where  = Bookmarks._ID + " = ?";
         final String[] args = new String[] { String.valueOf(folderId) };
 
         ContentValues bumped = new ContentValues();
         bumped.put(Bookmarks.DATE_MODIFIED, now);
 
-        updated = cr.update(mBookmarksUriWithProfile, bumped, where, args);
+        final int updated = cr.update(mBookmarksUriWithProfile, bumped, where, args);
         debug("Updated " + updated + " rows to new modified time.");
     }
 
     @Override
     public void addBookmark(ContentResolver cr, String title, String uri) {
         long folderId = getFolderIdFromGuid(cr, Bookmarks.MOBILE_FOLDER_GUID);
         addBookmarkItem(cr, title, uri, folderId);
     }
@@ -770,24 +770,20 @@ public class LocalBrowserDB implements B
             Log.w(LOGTAG, "Favicon compression failed.");
         }
         values.put(Favicons.DATA, data);
 
         // Update or insert
         Uri faviconsUri = getAllFaviconsUri().buildUpon().
                 appendQueryParameter(BrowserContract.PARAM_INSERT_IF_NEEDED, "true").build();
 
-        int updated = cr.update(faviconsUri,
-                                values,
-                                Favicons.URL + " = ?",
-                                new String[] { faviconUri });
-
-        if (updated == 0) {
-            cr.insert(mFaviconsUriWithProfile, values);
-        }
+        cr.update(faviconsUri,
+                  values,
+                  Favicons.URL + " = ?",
+                  new String[] { faviconUri });
     }
 
     @Override
     public void updateThumbnailForUrl(ContentResolver cr, String uri,
             BitmapDrawable thumbnail) {
         Bitmap bitmap = thumbnail.getBitmap();
 
         byte[] data = null;
@@ -797,23 +793,22 @@ public class LocalBrowserDB implements B
         } else {
             Log.w(LOGTAG, "Favicon compression failed.");
         }
 
         ContentValues values = new ContentValues();
         values.put(Thumbnails.DATA, data);
         values.put(Thumbnails.URL, uri);
 
-        int updated = cr.update(mThumbnailsUriWithProfile,
-                                values,
-                                Thumbnails.URL + " = ?",
-                                new String[] { uri });
-
-        if (updated == 0)
-            cr.insert(mThumbnailsUriWithProfile, values);
+        Uri thumbnailsUri = mThumbnailsUriWithProfile.buildUpon().
+                appendQueryParameter(BrowserContract.PARAM_INSERT_IF_NEEDED, "true").build();
+        cr.update(thumbnailsUri,
+                  values,
+                  Thumbnails.URL + " = ?",
+                  new String[] { uri });
     }
 
     @Override
     public byte[] getThumbnailForUrl(ContentResolver cr, String uri) {
         Cursor c = null;
         byte[] b = null;
         try {
             c = cr.query(mThumbnailsUriWithProfile,
--- a/mobile/android/base/tests/testBrowserProvider.java.in
+++ b/mobile/android/base/tests/testBrowserProvider.java.in
@@ -1268,47 +1268,58 @@ public class testBrowserProvider extends
             long id = c.getLong(0);
             c.close();
 
             return id;
         }
 
         @Override
         public void test() throws Exception {
-            Uri updateHistoryUriWithProfile = mHistoryUri.buildUpon().
+            Uri updateHistoryUri = mHistoryUri.buildUpon().
+                appendQueryParameter("increment_visits", "true").build();
+            Uri updateOrInsertHistoryUri = mHistoryUri.buildUpon().
                 appendQueryParameter("insert_if_needed", "true").
                 appendQueryParameter("increment_visits", "true").build();
 
             // Update a non-existent history entry, without specifying visits or title
             ContentValues values = new ContentValues();
             values.put(mHistoryUrlCol, TEST_URL_1);
 
-            int updated = mProvider.update(updateHistoryUriWithProfile, values,
+            int updated = mProvider.update(updateHistoryUri, values,
                                            mHistoryUrlCol + " = ?",
                                            new String[] { TEST_URL_1 });
-            mAsserter.is((updated == 0), true, "History entry was inserted, not updated");
+            mAsserter.is((updated == 0), true, "History entry was not updated");
+            Cursor c = mProvider.query(mHistoryUri, null, null, null, null);
+            mAsserter.is(c.moveToFirst(), false, "History entry was not inserted");
+            c.close();
+
+            // Now let's try with update-or-insert.
+            updated = mProvider.update(updateOrInsertHistoryUri, values,
+                                           mHistoryUrlCol + " = ?",
+                                           new String[] { TEST_URL_1 });
+            mAsserter.is((updated == 1), true, "History entry was inserted");
 
             long id = getHistoryEntryIdByUrl(TEST_URL_1);
-            Cursor c = getHistoryEntryById(id);
+            c = getHistoryEntryById(id);
             mAsserter.is(c.moveToFirst(), true, "History entry was inserted");
 
             long dateCreated = c.getLong(c.getColumnIndex(mHistoryDateCreatedCol));
             long dateModified = c.getLong(c.getColumnIndex(mHistoryDateModifiedCol));
 
             mAsserter.is(new Long(c.getLong(c.getColumnIndex(mHistoryVisitsCol))), new Long(1),
                          "Inserted history entry has correct default number of visits");
             mAsserter.is(c.getString(c.getColumnIndex(mHistoryTitleCol)), TEST_URL_1,
                          "Inserted history entry has correct default title");
 
             // Update the history entry, without specifying an additional visit count
             values = new ContentValues();
             values.put(mHistoryLastVisitedCol, System.currentTimeMillis());
             values.put(mHistoryTitleCol, TEST_TITLE);
 
-            updated = mProvider.update(updateHistoryUriWithProfile, values,
+            updated = mProvider.update(updateOrInsertHistoryUri, values,
                                        mHistoryIdCol + " = ?",
                                        new String[] { String.valueOf(id) });
             mAsserter.is((updated == 1), true, "Inserted history entry was updated");
 
             c = getHistoryEntryById(id);
             mAsserter.is(c.moveToFirst(), true, "Updated history entry found");
 
             mAsserter.is(c.getString(c.getColumnIndex(mHistoryTitleCol)), TEST_TITLE,
@@ -1321,20 +1332,20 @@ public class testBrowserProvider extends
                             "Updated history entry has new modification date");
 
             // Create a new history entry, specifying visits and history
             values = new ContentValues();
             values.put(mHistoryUrlCol, TEST_URL_2);
             values.put(mHistoryTitleCol, TEST_TITLE);
             values.put(mHistoryVisitsCol, 10);
 
-            updated = mProvider.update(updateHistoryUriWithProfile, values,
+            updated = mProvider.update(updateOrInsertHistoryUri, values,
                                            mHistoryUrlCol + " = ?",
                                            new String[] { values.getAsString(mHistoryUrlCol) });
-            mAsserter.is((updated == 0), true, "History entry was inserted, not updated");
+            mAsserter.is((updated == 1), true, "History entry was inserted");
 
             id = getHistoryEntryIdByUrl(TEST_URL_2);
             c = getHistoryEntryById(id);
             mAsserter.is(c.moveToFirst(), true, "History entry was inserted");
 
             dateCreated = c.getLong(c.getColumnIndex(mHistoryDateCreatedCol));
             dateModified = c.getLong(c.getColumnIndex(mHistoryDateModifiedCol));
 
@@ -1342,17 +1353,17 @@ public class testBrowserProvider extends
                          "Inserted history entry has correct specified number of visits");
             mAsserter.is(c.getString(c.getColumnIndex(mHistoryTitleCol)), TEST_TITLE,
                          "Inserted history entry has correct specified title");
 
             // Update the history entry, specifying additional visit count
             values = new ContentValues();
             values.put(mHistoryVisitsCol, 10);
 
-            updated = mProvider.update(updateHistoryUriWithProfile, values,
+            updated = mProvider.update(updateOrInsertHistoryUri, values,
                                        mHistoryIdCol + " = ?",
                                        new String[] { String.valueOf(id) });
             mAsserter.is((updated == 1), true, "Inserted history entry was updated");
 
             c = getHistoryEntryById(id);
             mAsserter.is(c.moveToFirst(), true, "Updated history entry found");
 
             mAsserter.is(c.getString(c.getColumnIndex(mHistoryTitleCol)), TEST_TITLE,