Bug 1364644 - Pre: Move change tracking responsibilities into repositories r=rnewman

As part of moving toward versioned syncing, we need to start decoupling change tracking
concepts from parts of the system that facilitate flow of records. This allows us to track
what changed differently for different data types, while maintaining a consistent and predictable API.

A move toward that is to let repositories own determinining that a record has been modified.
Repositories are now asked to provide modified records, instead of a very specific "records modified since".

This patch does not change behaviour of the system: every repository still uses timestamp-based
change tracking to actually provide modified records to the caller. A changeover to version
tracking will come later in this series for bookmarks, and as part of Bug 1383894 for other repositories.

MozReview-Commit-ID: LQuWYdlNHpt

--HG--
extra : rebase_source : 5552d74d4a967ce85af09aaa57ca438fe5b949f3

mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/BufferingMiddlewareRepositorySession.java

@@ -17,8 +17,6 @@ import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionStoreDeleg
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
 import java.util.Collection;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 
 /**
  * Buffering middleware which is intended to wrap local RepositorySessions.
@@ -43,8 +41,8 @@ import java.util.concurrent.Executors;
     }
 
     @Override
-    public void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) {
-        this.inner.fetchSince(timestamp, delegate);
+    public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+        this.inner.fetchModified(delegate);
     }
 
     @Override

mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/Crypto5MiddlewareRepositorySession.java

@@ -135,9 +135,8 @@ public class Crypto5MiddlewareRepositorySession extends MiddlewareRepositorySess
   }
 
   @Override
-  public void fetchSince(long timestamp,
-                         RepositorySessionFetchRecordsDelegate delegate) {
-    inner.fetchSince(timestamp, makeUnwrappingDelegate(delegate));
+  public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+    inner.fetchModified(makeUnwrappingDelegate(delegate));
   }
 
   @Override

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java

@@ -27,7 +27,7 @@ import org.mozilla.gecko.sync.repositories.domain.Record;
  * <li>Populate with saved information by calling {@link #unbundle(RepositorySessionBundle)}.</li>
  * <li>Begin a sync by calling {@link #begin(RepositorySessionBeginDelegate)}. <code>begin()</code>
  *   is an appropriate place to initialize expensive resources.</li>
- * <li>Perform operations such as {@link #fetchSince(long, RepositorySessionFetchRecordsDelegate)} and
+ * <li>Perform operations such as {@link #fetchModified(RepositorySessionFetchRecordsDelegate)} and
  *   {@link #store(Record)}.</li>
  * <li>Finish by calling {@link #finish(RepositorySessionFinishDelegate)}, retrieving and storing
  *   the current bundle.</li>
@@ -83,7 +83,12 @@ public abstract class RepositorySession {
     this.repository = repository;
   }
 
-  public abstract void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate);
+  /**
+   * Fetch modified records, letting repositories define what "modified" means to them.
+   *
+   * @param delegate
+   */
+  public abstract void fetchModified(RepositorySessionFetchRecordsDelegate delegate);
   public abstract void fetch(String[] guids, RepositorySessionFetchRecordsDelegate delegate) throws InactiveSessionException;
   public abstract void fetchAll(RepositorySessionFetchRecordsDelegate delegate);
 

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15RepositorySession.java

@@ -46,19 +46,22 @@ public class Server15RepositorySession extends RepositorySession {
             serverRepository.authHeaderProvider);
   }
 
-  @Override
-  public void fetchSince(long sinceTimestamp,
-                         RepositorySessionFetchRecordsDelegate delegate) {
+  private void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) {
     BatchingDownloaderController.resumeFetchSinceIfPossible(
             this.downloader,
             this.serverRepository.stateProvider,
             delegate,
-            sinceTimestamp,
+            timestamp,
             serverRepository.getBatchLimit(),
             serverRepository.getSortOrder()
     );
   }
 
+  @Override
+  public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(getLastSyncTimestamp(), delegate);
+  }
+
   @Override
   public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
     this.fetchSince(-1, delegate);

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserRepositorySession.java

@@ -273,9 +273,7 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos
     }
   }
 
-  @Override
-  public void fetchSince(long timestamp,
-                         RepositorySessionFetchRecordsDelegate delegate) {
+  private void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) {
     if (this.storeTracker == null) {
       throw new IllegalStateException("Store tracker not yet initialized!");
     }
@@ -285,6 +283,16 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos
     delegateQueue.execute(command);
   }
 
+  @Override
+  public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(getLastSyncTimestamp(), delegate);
+  }
+
+  @Override
+  public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(-1, delegate);
+  }
+
   class FetchSinceRunnable extends FetchingRunnable {
     private final long since;
     private final long end;
@@ -317,11 +325,6 @@ public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepos
     }
   }
 
-  @Override
-  public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
-    this.fetchSince(0, delegate);
-  }
-
   protected int storeCount = 0;
 
   @Override

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FennecTabsRepository.java

@@ -118,9 +118,7 @@ public class FennecTabsRepository extends Repository {
       return null;
     }
 
-    @Override
-    public void fetchSince(final long timestamp,
-                           final RepositorySessionFetchRecordsDelegate delegate) {
+    private void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
       if (tabsProvider == null) {
         throw new IllegalArgumentException("tabsProvider was null.");
       }
@@ -140,8 +138,8 @@ public class FennecTabsRepository extends Repository {
           // but only process the record if the timestamp is sufficiently
           // recent, or if the client data has been modified.
           try {
-            final Cursor cursor = tabsHelper.safeQuery(tabsProvider, ".fetchSince()", null,
-                localClientSelection, localClientSelectionArgs, positionAscending);
+            final Cursor cursor = tabsHelper.safeQuery(tabsProvider, ".fetchModified()", null,
+                    localClientSelection, localClientSelectionArgs, positionAscending);
             try {
               final String localClientGuid = clientsDataDelegate.getAccountGUID();
               final String localClientName = clientsDataDelegate.getClientName();
@@ -151,7 +149,7 @@ public class FennecTabsRepository extends Repository {
               final TabsRecord tabsRecord = FennecTabsRepository.tabsRecordFromCursor(cursor, localClientGuid, localClientName, localClientLastModified);
 
               if (tabsRecord.lastModified >= timestamp ||
-                  clientsDataDelegate.getLastModifiedTimestamp() >= timestamp) {
+                      clientsDataDelegate.getLastModifiedTimestamp() >= timestamp) {
                 delegate.onFetchedRecord(tabsRecord);
               }
             } finally {
@@ -168,6 +166,16 @@ public class FennecTabsRepository extends Repository {
       delegateQueue.execute(command);
     }
 
+    @Override
+    public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
+      this.fetchSince(getLastSyncTimestamp(), delegate);
+    }
+
+    @Override
+    public void fetchAll(final RepositorySessionFetchRecordsDelegate delegate) {
+      this.fetchSince(-1, delegate);
+    }
+
     private long getLocalClientLastModified() {
       final String localClientSelection = Clients.GUID + " IS NULL";
       final String[] localClientSelectionArgs = null;
@@ -200,11 +208,6 @@ public class FennecTabsRepository extends Repository {
       });
     }
 
-    @Override
-    public void fetchAll(final RepositorySessionFetchRecordsDelegate delegate) {
-      fetchSince(0, delegate);
-    }
-
     private static final String TABS_CLIENT_GUID_IS = BrowserContract.Tabs.CLIENT_GUID + " = ?";
     private static final String CLIENT_GUID_IS = BrowserContract.Clients.GUID + " = ?";
 

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FormHistoryRepositorySession.java

@@ -230,9 +230,8 @@ public class FormHistoryRepositorySession extends
            DeletedFormHistory.TIME_DELETED + " <= " + Long.toString(end); // Milliseconds.
   }
 
-  @Override
-  public void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
-    Logger.trace(LOG_TAG, "Running fetchSince(" + timestamp + ").");
+  private void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
+    Logger.trace(LOG_TAG, "Running fetchSince(" + timestamp + ")");
 
     /*
      * We need to be careful about the timestamp we complete the fetch with. If
@@ -245,14 +244,14 @@ public class FormHistoryRepositorySession extends
     Callable<Cursor> regularCallable = new Callable<Cursor>() {
       @Override
       public Cursor call() throws Exception {
-        return regularHelper.safeQuery(formsProvider, ".fetchSince(regular)", null, regularBetween(timestamp, sharedEnd), null, null);
+        return regularHelper.safeQuery(formsProvider, ".fetchModified(regular)", null, regularBetween(timestamp, sharedEnd), null, null);
       }
     };
 
     Callable<Cursor> deletedCallable = new Callable<Cursor>() {
       @Override
       public Cursor call() throws Exception {
-        return deletedHelper.safeQuery(formsProvider, ".fetchSince(deleted)", null, deletedBetween(timestamp, sharedEnd), null, null);
+        return deletedHelper.safeQuery(formsProvider, ".fetchModified(deleted)", null, deletedBetween(timestamp, sharedEnd), null, null);
       }
     };
 
@@ -262,10 +261,15 @@ public class FormHistoryRepositorySession extends
     fetchHelper(delegate, sharedEnd, callableCursors);
   }
 
+  @Override
+  public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(getLastSyncTimestamp(), delegate);
+  }
+
   @Override
   public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
     Logger.trace(LOG_TAG, "Running fetchAll.");
-    fetchSince(0, delegate);
+    this.fetchSince(-1, delegate);
   }
 
   @Override

mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java

@@ -67,8 +67,7 @@ public class PasswordsRepositorySession extends
   private static final String WHERE_GUID_IS = Passwords.GUID + " = ?";
   private static final String WHERE_DELETED_GUID_IS = DeletedPasswords.GUID + " = ?";
 
-  @Override
-  public void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
+  private void fetchSince(final long timestamp, final RepositorySessionFetchRecordsDelegate delegate) {
     final RecordFilter filter = this.storeTracker.getFilter();
     final Runnable fetchSinceRunnable = new Runnable() {
       @Override
@@ -81,19 +80,19 @@ public class PasswordsRepositorySession extends
         final long end = now();
         try {
           // Fetch from data table.
-          Cursor data = passwordsHelper.safeQuery(passwordsProvider, ".fetchSince",
-                                                  getAllColumns(),
-                                                  dateModifiedWhere(timestamp),
-                                                  null, null);
+          Cursor data = passwordsHelper.safeQuery(passwordsProvider, ".fetchModified",
+                  getAllColumns(),
+                  dateModifiedWhere(timestamp),
+                  null, null);
           if (!fetchAndCloseCursorDeleted(data, false, filter, delegate)) {
             return;
           }
 
           // Fetch from deleted table.
-          Cursor deleted = deletedPasswordsHelper.safeQuery(passwordsProvider, ".fetchSince",
-                                                            getAllDeletedColumns(),
-                                                            dateModifiedWhereDeleted(timestamp),
-                                                            null, null);
+          Cursor deleted = deletedPasswordsHelper.safeQuery(passwordsProvider, ".fetchModified",
+                  getAllDeletedColumns(),
+                  dateModifiedWhereDeleted(timestamp),
+                  null, null);
           if (!fetchAndCloseCursorDeleted(deleted, true, filter, delegate)) {
             return;
           }
@@ -116,6 +115,16 @@ public class PasswordsRepositorySession extends
     delegateQueue.execute(fetchSinceRunnable);
   }
 
+  @Override
+  public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(getLastSyncTimestamp(), delegate);
+  }
+
+  @Override
+  public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
+    this.fetchSince(-1, delegate);
+  }
+
   @Override
   public void fetch(final String[] guids, final RepositorySessionFetchRecordsDelegate delegate) {
     if (guids == null || guids.length < 1) {
@@ -173,11 +182,6 @@ public class PasswordsRepositorySession extends
     delegateQueue.execute(fetchRunnable);
   }
 
-  @Override
-  public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
-    fetchSince(0, delegate);
-  }
-
   @Override
   public void store(final Record record) throws NoStoreDelegateException {
     if (storeDelegate == null) {

mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/RecordsChannel.java

@@ -6,7 +6,6 @@ package org.mozilla.gecko.sync.synchronizer;
 
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
-import android.support.annotation.VisibleForTesting;
 
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutorService;
@@ -194,7 +193,7 @@ public class RecordsChannel implements
     this.consumer = new ConcurrentRecordConsumer(this);
     ThreadPool.run(this.consumer);
     waitingForQueueDone = true;
-    source.fetchSince(source.getLastSyncTimestamp(), this);
+    source.fetchModified(this);
   }
 
   /**

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/AndroidBrowserRepositoryTestCase.java

@@ -196,7 +196,7 @@ public abstract class AndroidBrowserRepositoryTestCase extends AndroidSyncTestCa
     return new Runnable() {
       @Override
       public void run() {
-        session.fetchSince(timestamp, preparedExpectFetchSinceDelegate(timestamp, expected));
+        session.fetchModified(preparedExpectFetchSinceDelegate(timestamp, expected));
       }
     };
   }
@@ -311,7 +311,7 @@ public abstract class AndroidBrowserRepositoryTestCase extends AndroidSyncTestCa
   }
 
   /*
-   * Tests for fetchSince
+   * Tests for fetchModified
    */
   protected void fetchSinceOneRecord(Record record0, Record record1) {
     RepositorySession session = createAndBeginSession();

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/TestBookmarks.java

@@ -671,7 +671,7 @@ public class TestBookmarks extends AndroidSyncTestCase {
 
         }
       };
-      session.fetchSince(0, fetchDelegate);
+      session.fetchModified(fetchDelegate);
     }
   }
 

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/TestFennecTabsRepositorySession.java

@@ -148,7 +148,7 @@ public class TestFennecTabsRepositorySession extends AndroidSyncTestCase {
     return new Runnable() {
       @Override
       public void run() {
-        session.fetchSince(timestamp, new ExpectFetchDelegate(expectedRecords));
+        session.fetchModified(new ExpectFetchDelegate(expectedRecords));
       }
     };
   }

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/TestFormHistoryRepositorySession.java

@@ -221,7 +221,7 @@ public class TestFormHistoryRepositorySession extends AndroidSyncTestCase {
     return new Runnable() {
       @Override
       public void run() {
-        session.fetchSince(timestamp, new ExpectFetchSinceDelegate(timestamp, expectedGuids));
+        session.fetchModified(new ExpectFetchSinceDelegate(timestamp, expectedGuids));
       }
     };
   }

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/TestPasswordsRepository.java

@@ -59,7 +59,7 @@ public class TestPasswordsRepository extends AndroidSyncTestCase {
   public void testFetchSinceOneRecord() {
     RepositorySession session = createAndBeginSession();
 
-    // Passwords fetchSince checks timePasswordChanged, not insertion time.
+    // Passwords fetchModified checks timePasswordChanged, not insertion time.
     PasswordRecord record1 = PasswordHelpers.createPassword1();
     long timeModified1 = updatePassword(NEW_PASSWORD1, record1);
     performWait(storeRunnable(session, record1));
@@ -423,7 +423,7 @@ public class TestPasswordsRepository extends AndroidSyncTestCase {
     return new Runnable() {
       @Override
       public void run() {
-        session.fetchSince(timestamp, new ExpectFetchSinceDelegate(timestamp, expected));
+        session.fetchModified(new ExpectFetchSinceDelegate(timestamp, expected));
       }
     };
   }

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/sync/TestStoreTracking.java

@@ -82,7 +82,7 @@ public class TestStoreTracking extends AndroidSyncTestCase {
               Logger.debug(getName(), "Fetch completed at " + fetchEnd + ".");
 
               // But fetching by time returns nothing.
-              session.fetchSince(0, new SimpleSuccessFetchDelegate() {
+              session.fetchModified(new SimpleSuccessFetchDelegate() {
                 private AtomicBoolean fetched = new AtomicBoolean(false);
 
                 @Override
@@ -155,7 +155,7 @@ public class TestStoreTracking extends AndroidSyncTestCase {
             @Override
             public void onBeginSucceeded(final RepositorySession session) {
               // Now we get a result.
-              session.fetchSince(0, new SimpleSuccessFetchDelegate() {
+              session.fetchModified(new SimpleSuccessFetchDelegate() {
 
                 @Override
                 public void onFetchedRecord(Record record) {

mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/testhelpers/WBORepository.java

@@ -67,15 +67,15 @@ public class WBORepository extends Repository {
     }
 
     @Override
-    public void fetchSince(long timestamp,
-                           RepositorySessionFetchRecordsDelegate delegate) {
+    public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+      final long fetchSince = getLastSyncTimestamp();
       long fetchBegan  = now();
       stats.fetchBegan = fetchBegan;
       RecordFilter filter = storeTracker.getFilter();
 
       for (Entry<String, Record> entry : wbos.entrySet()) {
         Record record = entry.getValue();
-        if (record.lastModified >= timestamp) {
+        if (record.lastModified >= fetchSince) {
           if (filter != null &&
               filter.excludeRecord(record)) {
             Logger.debug(LOG_TAG, "Excluding record " + record.guid);

mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/SynchronizerHelpers.java

@@ -62,9 +62,8 @@ public class SynchronizerHelpers {
                               Context context) {
       delegate.deferredCreationDelegate().onSessionCreated(new WBORepositorySession(this) {
         @Override
-        public void fetchSince(long timestamp,
-                               final RepositorySessionFetchRecordsDelegate delegate) {
-          super.fetchSince(timestamp, new RepositorySessionFetchRecordsDelegate() {
+        public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
+          super.fetchModified(new RepositorySessionFetchRecordsDelegate() {
             @Override
             public void onFetchedRecord(Record record) {
               if (record.guid.contains(FAIL_SENTINEL)) {

mobile/android/tests/background/junit4/src/org/mozilla/gecko/background/testhelpers/WBORepository.java

@@ -66,15 +66,15 @@ public class WBORepository extends Repository {
     }
 
     @Override
-    public void fetchSince(long timestamp,
-                           RepositorySessionFetchRecordsDelegate delegate) {
+    public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
+      final long fetchSince = getLastSyncTimestamp();
       long fetchBegan  = now();
       stats.fetchBegan = fetchBegan;
       RecordFilter filter = storeTracker.getFilter();
 
       for (Entry<String, Record> entry : wbos.entrySet()) {
         Record record = entry.getValue();
-        if (record.lastModified >= timestamp) {
+        if (record.lastModified >= fetchSince) {
           if (filter != null &&
               filter.excludeRecord(record)) {
             Logger.debug(LOG_TAG, "Excluding record " + record.guid);