Bug 1213688 : Update password store logic for deleted local records r=nalexander,rnewman
authorvivek <vivekb.balakrishnan@gmail.com>
Tue, 20 Oct 2015 20:03:55 +0300
changeset 303843 a0b901ae875a273a8cbb090291bc388c7a65f5be
parent 303842 1df2049352ff2ea1ab272e15f318da30aecd1e98
child 303844 2e1cf6a3b3f5a7907220b841fb973eada38fbde0
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander, rnewman
bugs1213688
milestone44.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 1213688 : Update password store logic for deleted local records r=nalexander,rnewman
mobile/android/base/sync/repositories/android/PasswordsRepositorySession.java
mobile/android/tests/background/junit3/background_junit3_sources.mozbuild
mobile/android/tests/background/junit3/src/db/TestFormHistoryRepositorySession.java
mobile/android/tests/background/junit3/src/db/TestPasswordsRepository.java
mobile/android/tests/background/junit3/src/sync/helpers/ExpectNoStoreDelegate.java
--- a/mobile/android/base/sync/repositories/android/PasswordsRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/PasswordsRepositorySession.java
@@ -360,18 +360,23 @@ public class PasswordsRepositorySession 
           trackRecord(inserted);
           delegate.onRecordStoreSucceeded(inserted.guid);
           return;
         }
 
         // We found a local dupe.
         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.");
           return;
         }
 
         // TODO: pass in timestamps?
         Logger.debug(LOG_TAG, "Replacing " + existingRecord.guid + " with record " + toStore.guid);
         Record replaced = null;
@@ -467,20 +472,27 @@ public class PasswordsRepositorySession 
   public Record replace(Record origRecord, Record newRecord) throws RemoteException {
     PasswordRecord newPasswordRecord = (PasswordRecord) newRecord;
     PasswordRecord origPasswordRecord = (PasswordRecord) origRecord;
     propagateTimes(newPasswordRecord, origPasswordRecord);
     ContentValues cv = getContentValues(newPasswordRecord);
 
     final String[] args = new String[] { origRecord.guid };
 
-    int updated = context.getContentResolver().update(BrowserContractHelpers.PASSWORDS_CONTENT_URI, cv, WHERE_GUID_IS, args);
-    if (updated != 1) {
-      Logger.warn(LOG_TAG, "Unexpectedly updated " + updated + " rows for guid " + origPasswordRecord.guid);
+    if (origRecord.deleted) {
+      // Purge from deleted table.
+      deleteGUID(origRecord.guid);
+      insert(newPasswordRecord);
+    } else {
+      int updated = context.getContentResolver().update(BrowserContractHelpers.PASSWORDS_CONTENT_URI, cv, WHERE_GUID_IS, args);
+      if (updated != 1) {
+        Logger.warn(LOG_TAG, "Unexpectedly updated " + updated + " rows for guid " + origPasswordRecord.guid);
+      }
     }
+
     return newRecord;
   }
 
   // When replacing a record, propagate the times.
   private static void propagateTimes(PasswordRecord toRecord, PasswordRecord fromRecord) {
     toRecord.timePasswordChanged = now();
     toRecord.timeCreated  = fromRecord.timeCreated;
     toRecord.timeLastUsed = fromRecord.timeLastUsed;
--- a/mobile/android/tests/background/junit3/background_junit3_sources.mozbuild
+++ b/mobile/android/tests/background/junit3/background_junit3_sources.mozbuild
@@ -64,16 +64,17 @@ background_junit3_sources = [
     'src/sync/helpers/ExpectFetchSinceDelegate.java',
     'src/sync/helpers/ExpectFinishDelegate.java',
     'src/sync/helpers/ExpectFinishFailDelegate.java',
     'src/sync/helpers/ExpectGuidsSinceDelegate.java',
     'src/sync/helpers/ExpectInvalidRequestFetchDelegate.java',
     'src/sync/helpers/ExpectInvalidTypeStoreDelegate.java',
     'src/sync/helpers/ExpectManyStoredDelegate.java',
     'src/sync/helpers/ExpectNoGUIDsSinceDelegate.java',
+    'src/sync/helpers/ExpectNoStoreDelegate.java',
     'src/sync/helpers/ExpectStoreCompletedDelegate.java',
     'src/sync/helpers/ExpectStoredDelegate.java',
     'src/sync/helpers/HistoryHelpers.java',
     'src/sync/helpers/PasswordHelpers.java',
     'src/sync/helpers/SessionTestHelper.java',
     'src/sync/helpers/SimpleSuccessBeginDelegate.java',
     'src/sync/helpers/SimpleSuccessCreationDelegate.java',
     'src/sync/helpers/SimpleSuccessFetchDelegate.java',
--- a/mobile/android/tests/background/junit3/src/db/TestFormHistoryRepositorySession.java
+++ b/mobile/android/tests/background/junit3/src/db/TestFormHistoryRepositorySession.java
@@ -4,17 +4,17 @@
 package org.mozilla.gecko.background.db;
 
 import java.util.concurrent.ExecutorService;
 
 import org.mozilla.gecko.background.helpers.AndroidSyncTestCase;
 import org.mozilla.gecko.background.sync.helpers.ExpectFetchDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectFetchSinceDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectGuidsSinceDelegate;
-import org.mozilla.gecko.background.sync.helpers.ExpectStoreCompletedDelegate;
+import org.mozilla.gecko.background.sync.helpers.ExpectNoStoreDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectStoredDelegate;
 import org.mozilla.gecko.background.sync.helpers.SessionTestHelper;
 import org.mozilla.gecko.background.testhelpers.WaitHelper;
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.sync.repositories.InactiveSessionException;
 import org.mozilla.gecko.sync.repositories.NoContentProviderException;
 import org.mozilla.gecko.sync.repositories.NoStoreDelegateException;
 import org.mozilla.gecko.sync.repositories.RepositorySession;
@@ -390,23 +390,16 @@ public class TestFormHistoryRepositorySe
     // remote deleted, local deleted, remote newer => should delete everything.
     rec = new FormHistoryRecord(deleted2.guid, deleted2.collection, newTimestamp, true);
     performWait(storeRunnable(session, rec, new ExpectNoStoreDelegate()));
     performWait(fetchRunnable(session, new String[] { deleted2.guid }, new Record[] { }));
 
     session.abort();
   }
 
-  public static class ExpectNoStoreDelegate extends ExpectStoreCompletedDelegate {
-    @Override
-    public void onRecordStoreSucceeded(String guid) {
-      performNotify("Should not have stored record " + guid, null);
-    }
-  }
-
   public void testStoreRemoteOlder() throws NoContentProviderException, RemoteException {
     final FormHistoryRepositorySession session = createAndBeginSession();
 
     long oldTimestamp = System.currentTimeMillis() - 100;
     insertFourRecords(session);
 
     FormHistoryRecord rec;
 
--- a/mobile/android/tests/background/junit3/src/db/TestPasswordsRepository.java
+++ b/mobile/android/tests/background/junit3/src/db/TestPasswordsRepository.java
@@ -5,34 +5,37 @@ package org.mozilla.gecko.background.db;
 
 import java.util.HashSet;
 import java.util.Set;
 
 import org.mozilla.gecko.background.helpers.AndroidSyncTestCase;
 import org.mozilla.gecko.background.sync.helpers.ExpectFetchDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectFetchSinceDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectGuidsSinceDelegate;
+import org.mozilla.gecko.background.sync.helpers.ExpectNoStoreDelegate;
 import org.mozilla.gecko.background.sync.helpers.ExpectStoredDelegate;
 import org.mozilla.gecko.background.sync.helpers.PasswordHelpers;
 import org.mozilla.gecko.background.sync.helpers.SessionTestHelper;
 import org.mozilla.gecko.background.testhelpers.WaitHelper;
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.sync.Utils;
 import org.mozilla.gecko.sync.repositories.InactiveSessionException;
 import org.mozilla.gecko.sync.repositories.NoStoreDelegateException;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.RepositorySession;
 import org.mozilla.gecko.sync.repositories.android.BrowserContractHelpers;
 import org.mozilla.gecko.sync.repositories.android.PasswordsRepositorySession;
 import org.mozilla.gecko.sync.repositories.android.RepoUtils;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate;
+import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionStoreDelegate;
 import org.mozilla.gecko.sync.repositories.domain.PasswordRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
 import android.content.ContentProviderClient;
+import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
 import android.os.RemoteException;
 
 public class TestPasswordsRepository extends AndroidSyncTestCase {
   private final String NEW_PASSWORD1 = "password";
   private final String NEW_PASSWORD2 = "drowssap";
 
@@ -171,16 +174,47 @@ public class TestPasswordsRepository ext
     // Sync a remote record version that is newer.
     PasswordRecord remote = PasswordHelpers.createPassword2();
     remote.guid = local.guid;
     updatePassword(NEW_PASSWORD2, remote);
     performWait(storeRunnable(session, remote));
 
     // Make a fetch, expecting only the newer (remote) record.
     performWait(fetchAllRunnable(session, new Record[] { remote }));
+
+    // Store an older local record.
+    PasswordRecord local2 = PasswordHelpers.createPassword3();
+    updatePassword(NEW_PASSWORD2, local2, System.currentTimeMillis() - 1000);
+    performWait(storeRunnable(session, local2));
+
+    // Sync a remote record version that is newer and is deleted.
+    PasswordRecord remote2 = PasswordHelpers.createPassword3();
+    remote2.guid = local2.guid;
+    remote2.deleted = true;
+    updatePassword(NEW_PASSWORD2, remote2);
+    performWait(storeRunnable(session, remote2));
+
+    // Make a fetch, expecting the local record to be deleted.
+    performWait(fetchRunnable(session, new String[] { remote2.guid }, new Record[] {}));
+
+    // Store an older deleted local record.
+    PasswordRecord local3 = PasswordHelpers.createPassword4();
+    updatePassword(NEW_PASSWORD2, local3, System.currentTimeMillis() - 1000);
+    local3.deleted = true;
+    storeLocalDeletedRecord(local3, System.currentTimeMillis() - 1000);
+
+    // Sync a remote record version that is newer and is deleted.
+    PasswordRecord remote3 = PasswordHelpers.createPassword5();
+    remote3.guid = local3.guid;
+    remote3.deleted = true;
+    updatePassword(NEW_PASSWORD2, remote3);
+    performWait(storeRunnable(session, remote3));
+
+    // Make a fetch, expecting the local record to be deleted.
+    performWait(fetchRunnable(session, new String[] { remote3.guid }, new Record[] {}));
     dispose(session);
   }
 
   public void testLocalNewerTimeStamp() {
     final RepositorySession session = createAndBeginSession();
     // Remote record updated before local record.
     PasswordRecord remote = PasswordHelpers.createPassword1();
     updatePassword(NEW_PASSWORD1, remote, System.currentTimeMillis() - 1000);
@@ -191,16 +225,51 @@ public class TestPasswordsRepository ext
     performWait(storeRunnable(session, local));
 
     // Sync a remote record version that is older.
     remote.guid = local.guid;
     performWait(storeRunnable(session, remote));
 
     // Make a fetch, expecting only the newer (local) record.
     performWait(fetchAllRunnable(session, new Record[] { local }));
+
+    // Remote record updated before local record.
+    PasswordRecord remote2 = PasswordHelpers.createPassword3();
+    updatePassword(NEW_PASSWORD1, remote2, System.currentTimeMillis() - 1000);
+
+    // Store updated local record that is deleted.
+    PasswordRecord local2 = PasswordHelpers.createPassword3();
+    updatePassword(NEW_PASSWORD2, local2);
+    local2.deleted = true;
+    storeLocalDeletedRecord(local2, System.currentTimeMillis());
+
+    // Sync a remote record version that is older.
+    remote2.guid = local2.guid;
+    performWait(storeRunnable(session, remote2, new ExpectNoStoreDelegate()));
+
+    // Make a fetch, expecting only the deleted newer (local) record.
+    performWait(fetchRunnable(session, new String[] { local2.guid }, new Record[] { local2 }));
+
+    // Remote record updated before local record.
+    PasswordRecord remote3 = PasswordHelpers.createPassword4();
+    updatePassword(NEW_PASSWORD1, remote3, System.currentTimeMillis() - 1000);
+
+    // Store updated local record that is deleted.
+    PasswordRecord local3 = PasswordHelpers.createPassword4();
+    updatePassword(NEW_PASSWORD2, local3);
+    local3.deleted = true;
+    storeLocalDeletedRecord(local3, System.currentTimeMillis());
+
+    // Sync a remote record version that is older and is deleted.
+    remote3.guid = local3.guid;
+    remote3.deleted = true;
+    performWait(storeRunnable(session, remote3));
+
+    // Make a fetch, expecting the local record to be deleted.
+    performWait(fetchRunnable(session, new String[] { local3.guid }, new Record[] {}));
     dispose(session);
   }
 
   /*
    * Store two records that are identical except for guid. Expect to find the
    * remote one after reconciling.
    */
   public void testStoreIdenticalExceptGuid() {
@@ -318,16 +387,27 @@ public class TestPasswordsRepository ext
   }
 
   private void wipe() {
     Context context = getApplicationContext();
     context.getContentResolver().delete(BrowserContractHelpers.PASSWORDS_CONTENT_URI, null, null);
     context.getContentResolver().delete(BrowserContractHelpers.DELETED_PASSWORDS_CONTENT_URI, null, null);
   }
 
+  private void storeLocalDeletedRecord(Record record, long time) {
+    // Wipe data-store
+    wipe();
+    // Store record in deleted table.
+    ContentValues contentValues = new ContentValues();
+    contentValues.put(BrowserContract.DeletedColumns.GUID, record.guid);
+    contentValues.put(BrowserContract.DeletedColumns.TIME_DELETED, time);
+    contentValues.put(BrowserContract.DeletedColumns.ID, record.androidID);
+    getApplicationContext().getContentResolver().insert(BrowserContractHelpers.DELETED_PASSWORDS_CONTENT_URI, contentValues);
+  }
+
   private static void dispose(RepositorySession session) {
     if (session != null) {
       session.abort();
     }
   }
 
   private static long updatePassword(String password, PasswordRecord record, long timestamp) {
     record.encryptedPassword = password;
@@ -337,20 +417,24 @@ public class TestPasswordsRepository ext
   }
 
   private static long updatePassword(String password, PasswordRecord record) {
     return updatePassword(password, record, System.currentTimeMillis());
   }
 
   // Runnable Helpers.
   private static Runnable storeRunnable(final RepositorySession session, final Record record) {
+    return storeRunnable(session, record, new ExpectStoredDelegate(record.guid));
+  }
+
+  private static Runnable storeRunnable(final RepositorySession session, final Record record, final RepositorySessionStoreDelegate delegate) {
     return new Runnable() {
       @Override
       public void run() {
-        session.setStoreDelegate(new ExpectStoredDelegate(record.guid));
+        session.setStoreDelegate(delegate);
         try {
           session.store(record);
           session.storeDone();
         } catch (NoStoreDelegateException e) {
           fail("NoStoreDelegateException should not occur.");
         }
       }
     };
new file mode 100644
--- /dev/null
+++ b/mobile/android/tests/background/junit3/src/sync/helpers/ExpectNoStoreDelegate.java
@@ -0,0 +1,11 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+package org.mozilla.gecko.background.sync.helpers;
+
+public class ExpectNoStoreDelegate extends ExpectStoreCompletedDelegate {
+    @Override
+    public void onRecordStoreSucceeded(String guid) {
+        performNotify("Should not have stored record " + guid, null);
+    }
+}
\ No newline at end of file