Bug 1153973 - Don't blindly apply deletions as insertions. r=nalexander, a=sledru
authorRichard Newman <rnewman@mozilla.com>
Mon, 13 Apr 2015 12:37:30 -0700
changeset 258507 f9d36adcdf51
parent 258506 7a10ff7fd9e4
child 258508 8fd05ce16a5f
push id4682
push userryanvm@gmail.com
push date2015-04-16 18:48 +0000
treeherdermozilla-beta@45a5eaa7813b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander, sledru
bugs1153973
milestone38.0
Bug 1153973 - Don't blindly apply deletions as insertions. r=nalexander, a=sledru
mobile/android/base/db/ReadingListProvider.java
mobile/android/base/reading/LocalReadingListStorage.java
mobile/android/base/reading/ReadingListRecord.java
--- a/mobile/android/base/db/ReadingListProvider.java
+++ b/mobile/android/base/db/ReadingListProvider.java
@@ -3,25 +3,29 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.db;
 
 import android.content.ContentUris;
 import android.content.ContentValues;
 import android.content.UriMatcher;
 import android.database.Cursor;
+import android.database.SQLException;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteQueryBuilder;
 import android.net.Uri;
 import android.text.TextUtils;
+import android.util.Log;
 import org.mozilla.gecko.db.DBUtils.UpdateOperation;
 
 import static org.mozilla.gecko.db.BrowserContract.ReadingListItems.*;
 
 public class ReadingListProvider extends SharedBrowserDatabaseProvider {
+    private static final String LOGTAG = "GeckoRLProvider";
+
     static final String TABLE_READING_LIST = TABLE_NAME;
 
     static final int ITEMS = 101;
     static final int ITEMS_ID = 102;
     static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH);
 
     public static final String PLACEHOLDER_THIS_DEVICE = "$local";
 
@@ -157,17 +161,22 @@ public class ReadingListProvider extends
             }
             if (!values.containsKey(ADDED_BY)) {
                 values.put(ADDED_BY, PLACEHOLDER_THIS_DEVICE);
             }
         }
 
         final String url = values.getAsString(URL);
         debug("Inserting item in database with URL: " + url);
-        return getWritableDatabase(uri).insertOrThrow(TABLE_READING_LIST, null, values);
+        try {
+            return getWritableDatabase(uri).insertOrThrow(TABLE_READING_LIST, null, values);
+        } catch (SQLException e) {
+            Log.e(LOGTAG, "Insert failed.", e);
+            throw e;
+        }
     }
 
     private static final ContentValues DELETED_VALUES;
     static {
         final ContentValues values = new ContentValues();
         values.put(IS_DELETED, 1);
 
         values.put(URL, "");             // Non-null column.
@@ -318,25 +327,28 @@ public class ReadingListProvider extends
 
         switch (match) {
             case ITEMS:
                 trace("Insert on ITEMS: " + uri);
                 id = insertItem(uri, values);
                 break;
 
             default:
+                // Log here because we typically insert in a batch, and that will muffle.
+                Log.e(LOGTAG, "Unknown insert URI " + uri);
                 throw new UnsupportedOperationException("Unknown insert URI " + uri);
         }
 
         debug("Inserted ID in database: " + id);
 
         if (id >= 0) {
             return ContentUris.withAppendedId(uri, id);
         }
 
+        Log.e(LOGTAG, "Got to end of insertInTransaction without returning an id!");
         return null;
     }
 
     @Override
     public Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder) {
         String groupBy = null;
         SQLiteDatabase db = getReadableDatabase(uri);
         SQLiteQueryBuilder qb = new SQLiteQueryBuilder();
--- a/mobile/android/base/reading/LocalReadingListStorage.java
+++ b/mobile/android/base/reading/LocalReadingListStorage.java
@@ -230,17 +230,22 @@ public class LocalReadingListStorage imp
     @Override
     public void addUploadedRecord(ClientReadingListRecord up,
                                   ServerReadingListRecord down) {
       // TODO
     }
 
     @Override
     public void addDownloadedRecord(ServerReadingListRecord down) {
-      additionsOrChanges.add(down);
+      final Boolean deleted = down.fields.getBoolean("deleted");
+      if (deleted != null && deleted.booleanValue()) {
+        addDeletion(down.getGUID());
+      } else {
+        additionsOrChanges.add(down);
+      }
     }
   }
 
   private final ContentProviderClient client;
   private final Uri URI_WITHOUT_DELETED = BrowserContract.READING_LIST_AUTHORITY_URI
       .buildUpon()
       .appendPath("items")
       .appendQueryParameter(BrowserContract.PARAM_IS_SYNC, "1")
--- a/mobile/android/base/reading/ReadingListRecord.java
+++ b/mobile/android/base/reading/ReadingListRecord.java
@@ -18,17 +18,17 @@ public abstract class ReadingListRecord 
       this.guid = guid;
       this.lastModified = lastModified;
     }
 
     /**
      * From server record.
      */
     public ServerMetadata(ExtendedJSONObject obj) {
-      this(obj.getString("id"), obj.getLong("last_modified"));
+      this(obj.getString("id"), obj.containsKey("last_modified") ? obj.getLong("last_modified") : -1L);
     }
   }
 
   public final ServerMetadata serverMetadata;
 
   public String getGUID() {
     if (serverMetadata == null) {
       return null;