Bug 1392802 - Increment localVersion while reconciling a bookmark if we modified its dateAdded t.s. r=rnewman
We might decide that there's an older dateAdded timestamp present for an incoming bookmark while processing it,
in which case we need to ensure that our changes will be uploaded.
MozReview-Commit-ID: BKLh4rYBiRu
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -158,17 +158,17 @@ public class BrowserContract {
@RobocopTarget
public interface VersionColumns {
String LOCAL_VERSION = "localVersion";
String SYNC_VERSION = "syncVersion";
// This is a flag used to indicate that records inserted from sync should start as modified.
// That is, their versions will start as (2,1), instead of (1,1).
- String PARAM_INSERT_FROM_SYNC_AS_MODIFIED = "insertFromSyncAsModified";
+ String PARAM_INSERT_FROM_SYNC_AS_MODIFIED = "modifiedBySync";
}
@RobocopTarget
public interface SyncColumns extends DateSyncColumns {
public static final String GUID = "guid";
public static final String IS_DELETED = "deleted";
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksDataAccessor.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksDataAccessor.java
@@ -339,17 +339,17 @@ public class BookmarksDataAccessor exten
if (rec.tags == null) {
rec.tags = new JSONArray();
}
// We might want to indicate that this record is to be inserted as "modified". Incoming records
// might be modified as we're processing them for insertion, and so should be re-uploaded.
// Our data provider layer manages versions, so we don't pass in localVersion explicitly.
- if (rec.insertFromSyncAsModified) {
+ if (rec.modifiedBySync) {
cv.put(BrowserContract.Bookmarks.PARAM_INSERT_FROM_SYNC_AS_MODIFIED, true);
}
cv.put(BrowserContract.Bookmarks.TAGS, rec.tags.toJSONString());
cv.put(BrowserContract.Bookmarks.KEYWORD, rec.keyword);
cv.put(BrowserContract.Bookmarks.PARENT, rec.androidParentID);
cv.put(BrowserContract.Bookmarks.POSITION, rec.androidPosition);
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksSessionHelper.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksSessionHelper.java
@@ -287,26 +287,26 @@ import java.util.concurrent.ConcurrentHa
* This function is only called while inserting a record which doesn't exist locally.
*/
@Override
protected Record processBeforeInsertion(Record toProcess) {
// If incoming record is missing dateAdded, use its lastModified value instead.
if (((BookmarkRecord) toProcess).dateAdded == null) {
((BookmarkRecord) toProcess).dateAdded = toProcess.lastModified;
toProcess.lastModified = RepositorySession.now();
- toProcess.insertFromSyncAsModified = true;
+ toProcess.modifiedBySync = true;
return toProcess;
}
// If both are present, use the lowest value. We trust server's monotonously increasing timestamps
// more than clients' potentially bogus ones.
if (toProcess.lastModified < ((BookmarkRecord) toProcess).dateAdded) {
((BookmarkRecord) toProcess).dateAdded = toProcess.lastModified;
toProcess.lastModified = RepositorySession.now();
- toProcess.insertFromSyncAsModified = true;
+ toProcess.modifiedBySync = true;
return toProcess;
}
return toProcess;
}
/**
* Implement method of BookmarksInsertionManager.BookmarkInserter.
@@ -428,21 +428,22 @@ import java.util.concurrent.ConcurrentHa
Logger.error(LOG_TAG, "Error finishing up store: " + e);
}
}
};
}
@Override
/* package-private */ boolean doReplaceRecord(Record toStore, Record existingRecord, RepositorySessionStoreDelegate delegate) throws NoGuidForIdException, NullCursorException, ParentNotFoundException {
- // Request that localVersion is bumped if we're working with a record that also changed
- // locally since we last synced. We do this so that any merged changes are uploaded.
+ // Request that localVersion is bumped if an incoming record has been modified upstream, or
+ // if we're working with a record that also changed locally since we last synced.
+ // We do this so that any merged changes are uploaded.
// We simply replace and don't subsequently upload records which didn't change locally
// since our last sync.
- boolean shouldIncrementLocalVersion = existingRecord.localVersion > existingRecord.syncVersion;
+ boolean shouldIncrementLocalVersion = toStore.modifiedBySync || (existingRecord.localVersion > existingRecord.syncVersion);
Record replaced = replaceWithAssertion(
toStore,
existingRecord,
shouldIncrementLocalVersion
);
if (replaced == null) {
return false;
}
@@ -509,20 +510,22 @@ import java.util.concurrent.ConcurrentHa
// We always pick the lowest value out of what is available.
long lowest = remote.lastModified;
// During a similar operation, desktop clients consider dates before Jan 23, 1993 to be invalid.
// We do the same here out of a desire to be consistent.
final long releaseOfNCSAMosaicMillis = 727747200000L;
if (local.dateAdded != null && local.dateAdded < lowest && local.dateAdded > releaseOfNCSAMosaicMillis) {
+ reconciled.modifiedBySync = true;
lowest = local.dateAdded;
}
if (remote.dateAdded != null && remote.dateAdded < lowest && remote.dateAdded > releaseOfNCSAMosaicMillis) {
+ reconciled.modifiedBySync = true;
lowest = remote.dateAdded;
}
reconciled.dateAdded = lowest;
return reconciled;
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/domain/Record.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/domain/Record.java
@@ -72,17 +72,17 @@ public abstract class Record {
public String collection;
public long lastModified;
public boolean deleted;
public long androidID;
// Not all record types support versioning. See Bug 1383894.
@Nullable public Integer localVersion;
@Nullable public Integer syncVersion;
- public boolean insertFromSyncAsModified = false;
+ public boolean modifiedBySync = false;
/**
* An integer indicating the relative importance of this item in the collection.
* <p>
* Default is 0.
*/
public long sortIndex;
/**