Bug 1274029 - Part 3: Update aggregates during syncing r=sebastian
authorGrigory Kruglov <gkruglov@mozilla.com>
Wed, 01 Jun 2016 15:02:46 -0700
changeset 338899 4b6a54c20fff6cc1205a7098e403b2b27bf82c2a
parent 338898 ce62fcd4df4d3c7dd31c92dd7d73d736a043cb0e
child 338900 ea45ad32acf03ea698f60f9c9346e9ce5b43e183
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1274029
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1274029 - Part 3: Update aggregates during syncing r=sebastian MozReview-Commit-ID: 9WyBE0Lk3Tp
mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -45,16 +45,17 @@ public class BrowserContract {
     public static final String PARAM_PROFILE_PATH = "profilePath";
     public static final String PARAM_LIMIT = "limit";
     public static final String PARAM_SUGGESTEDSITES_LIMIT = "suggestedsites_limit";
     public static final String PARAM_IS_SYNC = "sync";
     public static final String PARAM_SHOW_DELETED = "show_deleted";
     public static final String PARAM_IS_TEST = "test";
     public static final String PARAM_INSERT_IF_NEEDED = "insert_if_needed";
     public static final String PARAM_INCREMENT_VISITS = "increment_visits";
+    public static final String PARAM_INCREMENT_REMOTE_AGGREGATES = "increment_remote_aggregates";
     public static final String PARAM_EXPIRE_PRIORITY = "priority";
     public static final String PARAM_DATASET_ID = "dataset_id";
     public static final String PARAM_GROUP_BY = "group_by";
 
     static public enum ExpirePriority {
         NORMAL,
         AGGRESSIVE
     }
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -378,16 +378,21 @@ public class BrowserProvider extends Sha
         db.execSQL(sql);
     }
 
     private boolean shouldIncrementVisits(Uri uri) {
         String incrementVisits = uri.getQueryParameter(BrowserContract.PARAM_INCREMENT_VISITS);
         return Boolean.parseBoolean(incrementVisits);
     }
 
+    private boolean shouldIncrementRemoteAggregates(Uri uri) {
+        final String incrementRemoteAggregates = uri.getQueryParameter(BrowserContract.PARAM_INCREMENT_REMOTE_AGGREGATES);
+        return Boolean.parseBoolean(incrementRemoteAggregates);
+    }
+
     @Override
     public String getType(Uri uri) {
         final int match = URI_MATCHER.match(uri);
 
         trace("Getting URI type: " + uri);
 
         switch (match) {
             case BOOKMARKS:
@@ -1418,34 +1423,50 @@ public class BrowserProvider extends Sha
 
         final SQLiteDatabase db = getWritableDatabase(uri);
 
         if (!values.containsKey(History.DATE_MODIFIED)) {
             values.put(History.DATE_MODIFIED, System.currentTimeMillis());
         }
 
         // Use the simple code path for easy updates.
-        if (!shouldIncrementVisits(uri)) {
+        if (!shouldIncrementVisits(uri) && !shouldIncrementRemoteAggregates(uri)) {
             trace("Updating history meta data only");
             return db.update(TABLE_HISTORY, values, selection, selectionArgs);
         }
 
         trace("Updating history meta data and incrementing visits");
 
         if (values.containsKey(History.DATE_LAST_VISITED)) {
             values.put(History.LOCAL_DATE_LAST_VISITED, values.getAsLong(History.DATE_LAST_VISITED));
         }
 
-        // Update data and increment visits by 1.
-        final long incVisits = 1;
-
         // Create a separate set of values that will be updated as an expression.
         final ContentValues visits = new ContentValues();
-        visits.put(History.VISITS, History.VISITS + " + " + incVisits);
-        visits.put(History.LOCAL_VISITS, History.LOCAL_VISITS + " + " + incVisits);
+        if (shouldIncrementVisits(uri)) {
+            // Update data and increment visits by 1.
+            final long incVisits = 1;
+
+            visits.put(History.VISITS, History.VISITS + " + " + incVisits);
+            visits.put(History.LOCAL_VISITS, History.LOCAL_VISITS + " + " + incVisits);
+        }
+
+        if (shouldIncrementRemoteAggregates(uri)) {
+            // Let's fail loudly instead of trying to assume what users of this API meant to do.
+            if (!values.containsKey(History.REMOTE_VISITS)) {
+                throw new IllegalArgumentException(
+                        "Tried incrementing History.REMOTE_VISITS by unknown value");
+            }
+            visits.put(
+                    History.REMOTE_VISITS,
+                    History.REMOTE_VISITS + " + " + values.getAsInteger(History.REMOTE_VISITS)
+            );
+            // Need to remove passed in value, so that we increment REMOTE_VISITS, and not just set it.
+            values.remove(History.REMOTE_VISITS);
+        }
 
         final ContentValues[] valuesAndVisits = { values,  visits };
         final UpdateOperation[] ops = { UpdateOperation.ASSIGN, UpdateOperation.EXPRESSION };
 
         return DBUtils.updateArrays(db, TABLE_HISTORY, valuesAndVisits, ops, selection, selectionArgs);
     }
 
     private long insertVisitForHistory(Uri uri, ContentValues values, String selection, String[] selectionArgs) {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java
@@ -34,27 +34,22 @@ public class AndroidBrowserHistoryDataAc
   protected ContentValues getContentValues(Record record) {
     ContentValues cv = new ContentValues();
     HistoryRecord rec = (HistoryRecord) record;
     cv.put(BrowserContract.History.GUID, rec.guid);
     cv.put(BrowserContract.History.TITLE, rec.title);
     cv.put(BrowserContract.History.URL, rec.histURI);
     if (rec.visits != null) {
       JSONArray visits = rec.visits;
-      long mostRecent = 0;
-      for (int i = 0; i < visits.size(); i++) {
-        JSONObject visit = (JSONObject) visits.get(i);
-        long visitDate = (Long) visit.get(VisitsHelper.SYNC_DATE_KEY);
-        if (visitDate > mostRecent) {
-          mostRecent = visitDate;
-        }
-      }
+      long mostRecent = getLastVisited(visits);
+
       // Fennec stores history timestamps in milliseconds, and visit timestamps in microseconds.
       // The rest of Sync works in microseconds. This is the conversion point for records coming form Sync.
       cv.put(BrowserContract.History.DATE_LAST_VISITED, mostRecent / 1000);
+      cv.put(BrowserContract.History.REMOTE_DATE_LAST_VISITED, mostRecent / 1000);
       cv.put(BrowserContract.History.VISITS, Long.toString(visits.size()));
     }
     return cv;
   }
 
   @Override
   protected String[] getAllColumns() {
     return BrowserContractHelpers.HistoryColumns;
@@ -135,21 +130,59 @@ public class AndroidBrowserHistoryDataAc
     if (inserted == size) {
       Logger.debug(LOG_TAG, "Inserted " + inserted + " records, as expected.");
     } else {
       Logger.debug(LOG_TAG, "Inserted " +
                    inserted + " records but expected " +
                    size     + " records; continuing to update visits.");
     }
 
+    final ContentValues remoteVisitAggregateValues = new ContentValues();
+    final Uri historyIncrementRemoteAggregateUri = getUri().buildUpon()
+            .appendQueryParameter(BrowserContract.PARAM_INCREMENT_REMOTE_AGGREGATES, "true")
+            .build();
     for (Record record : records) {
       HistoryRecord rec = (HistoryRecord) record;
       if (rec.visits != null && rec.visits.size() != 0) {
-        context.getContentResolver().bulkInsert(
+        int remoteVisitsInserted = context.getContentResolver().bulkInsert(
                 BrowserContract.Visits.CONTENT_URI,
                 VisitsHelper.getVisitsContentValues(rec.guid, rec.visits)
         );
+
+        // If we just inserted any visits, update remote visit aggregate values.
+        // While inserting visits, we might not insert all of rec.visits - if we already have a local
+        // visit record with matching (guid,date), we will skip that visit.
+        // Remote visits aggregate value will be incremented by number of visits inserted.
+        // Note that we don't need to set REMOTE_DATE_LAST_VISITED, because it already gets set above.
+        if (remoteVisitsInserted > 0) {
+          // Note that REMOTE_VISITS must be set before calling cr.update(...) with a URI
+          // that has PARAM_INCREMENT_REMOTE_AGGREGATES=true.
+          remoteVisitAggregateValues.put(BrowserContract.History.REMOTE_VISITS, remoteVisitsInserted);
+          context.getContentResolver().update(
+                  historyIncrementRemoteAggregateUri,
+                  remoteVisitAggregateValues,
+                  BrowserContract.History.GUID + " = ?", new String[] {rec.guid}
+          );
+        }
       }
     }
 
     return inserted;
   }
+
+  /**
+   * Helper method used to find largest <code>VisitsHelper.SYNC_DATE_KEY</code> value in a provided JSONArray.
+   *
+   * @param visits Array of objects which will be searched.
+   * @return largest value of <code>VisitsHelper.SYNC_DATE_KEY</code>.
+     */
+  private long getLastVisited(JSONArray visits) {
+    long mostRecent = 0;
+    for (int i = 0; i < visits.size(); i++) {
+      final JSONObject visit = (JSONObject) visits.get(i);
+      long visitDate = (Long) visit.get(VisitsHelper.SYNC_DATE_KEY);
+      if (visitDate > mostRecent) {
+        mostRecent = visitDate;
+      }
+    }
+    return mostRecent;
+  }
 }