Bug 1392449 - introduce shouldReconcileRecords so reconcileRecords never returns null. r?grisha
MozReview-Commit-ID: L0rsLmzNTRr
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
@@ -298,31 +298,59 @@ public abstract class RepositorySession
this.status = to;
return;
}
Logger.warn(LOG_TAG, "Wanted to transition from " + from + " but in state " + this.status);
throw new InvalidSessionTransitionException(null);
}
/**
+ * Indicate if the specified records should be reconciled.
+ *
+ * @param remoteRecord
+ * The record retrieved from upstream, already adjusted for clock skew.
+ * @param localRecord
+ * The record retrieved from local storage.
+ *
+ * @return
+ * true if the records should be passed to shouldReconcileRecords for
+ * the actual resolution, false otherwise.
+ */
+ public boolean shouldReconcileRecords(final Record remoteRecord,
+ final Record localRecord) {
+ Logger.debug(LOG_TAG, "Checking if we should reconcile remote " + remoteRecord.guid + " against local " + localRecord.guid);
+ if (localRecord.equalPayloads(remoteRecord)) {
+ if (remoteRecord.lastModified > localRecord.lastModified) {
+ Logger.debug(LOG_TAG, "Records are equal (remote is newer). No record application needed.");
+ return false;
+ }
+ // Local wins.
+ Logger.debug(LOG_TAG, "Records are equal (local is newer). No record application needed.");
+ return false;
+ }
+ return true;
+ }
+
+ /**
* Produce a record that is some combination of the remote and local records
* provided.
*
* The returned record must be produced without mutating either remoteRecord
* or localRecord. It is acceptable to return either remoteRecord or localRecord
* if no modifications are to be propagated.
*
* The returned record *should* have the local androidID and the remote GUID,
* and some optional merge of data from the two records.
*
- * This method can be called with records that are identical, or differ in
- * any regard.
+ * This method should only be called when shouldReconcileRecords returns true
+ * for the specified records.
*
* This method will not be called if:
*
+ * * shouldReconcileRecords() returns false
* * either record is marked as deleted, or
* * there is no local mapping for a new remote record.
*
* Otherwise, it will be called precisely once.
*
* Side-effects (e.g., for transactional storage) can be hooked in here.
*
* @param remoteRecord
@@ -330,34 +358,24 @@ public abstract class RepositorySession
* @param localRecord
* The record retrieved from local storage.
* @param lastRemoteRetrieval
* The timestamp of the last retrieved set of remote records, adjusted for
* clock skew.
* @param lastLocalRetrieval
* The timestamp of the last retrieved set of local records.
* @return
- * A Record instance to apply, or null to apply nothing.
+ * A Record instance to apply.
*/
public Record reconcileRecords(final Record remoteRecord,
- final Record localRecord,
- final long lastRemoteRetrieval,
- final long lastLocalRetrieval) {
+ final Record localRecord,
+ final long lastRemoteRetrieval,
+ final long lastLocalRetrieval) {
Logger.debug(LOG_TAG, "Reconciling remote " + remoteRecord.guid + " against local " + localRecord.guid);
- if (localRecord.equalPayloads(remoteRecord)) {
- if (remoteRecord.lastModified > localRecord.lastModified) {
- Logger.debug(LOG_TAG, "Records are equal. No record application needed.");
- return null;
- }
-
- // Local wins.
- return null;
- }
-
// TODO: Decide what to do based on:
// * Which of the two records is modified;
// * Whether they are equal or congruent;
// * The modified times of each record (interpreted through the lens of clock skew);
// * Whether localVersion==syncVersion or localVersion>syncVersion for localRecord
// * ...
boolean localIsMoreRecent = localRecord.lastModified > remoteRecord.lastModified;
Logger.debug(LOG_TAG, "Local record is more recent? " + localIsMoreRecent);
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java
@@ -314,22 +314,23 @@ public class PasswordsRepositorySession
trace("Incoming record " + remoteRecord.guid + " dupes to local record " + existingRecord.guid);
Logger.debug(LOG_TAG, "remote " + remoteRecord.guid + " dupes to " + existingRecord.guid);
if (existingRecord.deleted && existingRecord.lastModified > remoteRecord.lastModified) {
Logger.debug(LOG_TAG, "Local deletion is newer, not storing remote record.");
return;
}
- Record toStore = reconcileRecords(remoteRecord, existingRecord, lastRemoteRetrieval, lastLocalRetrieval);
- if (toStore == null) {
- Logger.debug(LOG_TAG, "Reconciling returned null. Not inserting a record.");
+ if (!shouldReconcileRecords(remoteRecord, existingRecord)) {
+ Logger.debug(LOG_TAG, "shouldReconcileRecords returned false. Not inserting a record.");
return;
}
+ Record toStore = reconcileRecords(remoteRecord, existingRecord, lastRemoteRetrieval, lastLocalRetrieval);
+
// TODO: pass in timestamps?
Logger.debug(LOG_TAG, "Replacing " + existingRecord.guid + " with record " + toStore.guid);
Record replaced = null;
try {
replaced = replace(existingRecord, toStore);
} catch (RemoteException e) {
Logger.debug(LOG_TAG, "Record replace caused a RemoteException.");
storeDelegate.onRecordStoreFailed(e, record.guid);
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/SessionHelper.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/SessionHelper.java
@@ -173,16 +173,21 @@ import org.mozilla.gecko.sync.repositori
}
}
void finish() {
// TODO are we paranoid that a reference might persist for too long once we're done with it?
recordToGuid = null;
}
+ private boolean shouldReconcileRecords(final Record remoteRecord,
+ final Record localRecord) {
+ return session.shouldReconcileRecords(remoteRecord, localRecord);
+ }
+
private void putRecordToGuidMap(String recordString, String guid)
throws NoGuidForIdException, NullCursorException, ParentNotFoundException {
if (recordString == null) {
return;
}
if (recordToGuid == null) {
createRecordToGuidMap();
@@ -292,24 +297,22 @@ import org.mozilla.gecko.sync.repositori
}
// We found a local dupe.
trace("Incoming record " + record.guid + " dupes to local record " + existingRecord.guid);
// Populate more expensive fields prior to reconciling.
existingRecord = transformRecord(existingRecord);
+ if (!shouldReconcileRecords(record, existingRecord)) {
+ Logger.debug(LOG_TAG, "shouldReconcileRecords returned false. Not processing a record.");
+ break;
+ }
// Implementation is expected to decide if it's necessary to "track" the record.
Record toStore = reconcileRecords(record, existingRecord, lastRemoteRetrieval, lastLocalRetrieval);
-
- if (toStore == null) {
- Logger.debug(LOG_TAG, "Reconciling returned null. Not inserting a record.");
- break;
- }
-
Logger.debug(LOG_TAG, "Reconcile attempt #" + reconcileAttempt);
// This section of code will only run if the incoming record is not
// marked as deleted, so we never want to just drop ours from the database:
// we need to upload it later.
// Implementations are expected to ensure this happens.
boolean replaceSuccessful = doReplaceRecord(toStore, existingRecord, storeDelegate);