Bug 1274029 - Part 3: Update aggregates during syncing r=sebastian
MozReview-Commit-ID: 9WyBE0Lk3Tp
--- 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,28 +34,24 @@ 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()));
+ cv.put(BrowserContract.History.REMOTE_VISITS, Long.toString(visits.size()));
}
return cv;
}
@Override
protected String[] getAllColumns() {
return BrowserContractHelpers.HistoryColumns;
}
@@ -135,21 +131,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;
+ }
}