Bug 723235, Bug 720304, Bug 723240 - Request bookmarks and history items sorted by sortindex/frecency, with limits.

• Bug 720304 - Limit number of processed history records, with a hard limit of 500 history items per request.
• Bug 723235 - Request history items sorted by sortindex/frecency.
• Bug 723240 - Request bookmark items sorted by sortindex/frecency.

And imposes a sanity limit of 5,000 bookmarks per fetch.

(Note that batching is outside the scope of these bugs.)

mobile/android/base/sync/repositories/ConstrainedServer11Repository.java

@@ -0,0 +1,38 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.sync.repositories;
+
+import java.net.URISyntaxException;
+
+import org.mozilla.gecko.sync.CredentialsSource;
+
+/**
+ * A kind of Server11Repository that supports explicit setting of limit and sort on operations.
+ *
+ * @author rnewman
+ *
+ */
+public class ConstrainedServer11Repository extends Server11Repository {
+
+  private String sort = null;
+  private long limit  = -1;
+
+  public ConstrainedServer11Repository(String serverURI, String username, String collection, CredentialsSource credentialsSource, long limit, String sort) throws URISyntaxException {
+    super(serverURI, username, collection, credentialsSource);
+
+    this.limit = limit;
+    this.sort  = sort;
+  }
+
+  @Override
+  protected String getDefaultSort() {
+    return sort;
+  }
+
+  @Override
+  protected long getDefaultFetchLimit() {
+    return limit;
+  }
+}

mobile/android/base/sync/repositories/Server11Repository.java

@@ -39,6 +39,7 @@ package org.mozilla.gecko.sync.repositories;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.ArrayList;
 
 import org.mozilla.gecko.sync.CredentialsSource;
 import org.mozilla.gecko.sync.Utils;
@@ -92,32 +93,50 @@ public class Server11Repository extends Repository {
     return this.collectionPathURI;
   }
 
-  public URI collectionURI(boolean full, long newer, String ids) throws URISyntaxException {
+  public URI collectionURI(boolean full, long newer, long limit, String sort, String ids) throws URISyntaxException {
-    // Do it this way to make it easier to add more params later.
+    ArrayList<String> params = new ArrayList<String>();
-    // It's pretty ugly, I'll grant.
+    if (full) {
-    // I can't believe Java doesn't have a good way to do this.
+      params.add("full=1");
-    boolean anyParams = full;
-    String  uriParams = "";
-    if (anyParams) {
-      StringBuilder params = new StringBuilder("?");
-      if (full) {
-        params.append("full=1");
-      }
-      if (newer >= 0) {
-        // Translate local millisecond timestamps into server decimal seconds.
-        String newerString = Utils.millisecondsToDecimalSecondsString(newer);
-        params.append((full ? "&newer=" : "newer=") + newerString);
-      }
-      if (ids != null) {
-        params.append(((full || newer >= 0) ? "&ids=" : "ids=") + ids);
-      }
-      uriParams = params.toString();
     }
-    String uri = this.collectionPath + uriParams;
+    if (newer >= 0) {
+      // Translate local millisecond timestamps into server decimal seconds.
+      String newerString = Utils.millisecondsToDecimalSecondsString(newer);
+      params.add("newer=" + newerString);
+    }
+    if (limit > 0) {
+      params.add("limit=" + limit);
+    }
+    if (sort != null) {
+      params.add("sort=" + sort);       // We trust these values.
+    }
+    if (ids != null) {
+      params.add("ids=" + ids);         // We trust these values.
+    }
+
+    if (params.size() == 0) {
+      return this.collectionPathURI;
+    }
+
+    StringBuilder out = new StringBuilder();
+    char indicator = '?';
+    for (String param : params) {
+      out.append(indicator);
+      indicator = '&';
+      out.append(param);
+    }
+    String uri = this.collectionPath + out.toString();
     return new URI(uri);
   }
 
   public URI wboURI(String id) throws URISyntaxException {
     return new URI(this.collectionPath + "/" + id);
   }
+
+  // Override these.
+  protected long getDefaultFetchLimit() {
+    return -1;
+  }
+  protected String getDefaultSort() {
+    return null;
+  }
 }

mobile/android/base/sync/repositories/Server11RepositorySession.java

@@ -214,21 +214,34 @@ public class Server11RepositorySession extends RepositorySession {
   }
 
   protected void fetchWithParameters(long newer,
+                                     long limit,
                                      boolean full,
+                                     String sort,
                                      String ids,
-                                     SyncStorageRequestDelegate delegate) throws URISyntaxException {
+                                     SyncStorageRequestDelegate delegate)
+                                         throws URISyntaxException {
 
-    URI collectionURI = serverRepository.collectionURI(full, newer, ids);
+    URI collectionURI = serverRepository.collectionURI(full, newer, limit, sort, ids);
     SyncStorageCollectionRequest request = new SyncStorageCollectionRequest(collectionURI);
     request.delegate = delegate;
     request.get();
   }
 
+  public void fetchSince(long timestamp, long limit, String sort, RepositorySessionFetchRecordsDelegate delegate) {
+    try {
+      this.fetchWithParameters(timestamp, limit, true, sort, null, new RequestFetchDelegateAdapter(delegate));
+    } catch (URISyntaxException e) {
+      delegate.onFetchFailed(e, null);
+    }
+  }
+
   @Override
   public void fetchSince(long timestamp,
                          RepositorySessionFetchRecordsDelegate delegate) {
     try {
-      this.fetchWithParameters(timestamp, true, null, new RequestFetchDelegateAdapter(delegate));
+      long limit = serverRepository.getDefaultFetchLimit();
+      String sort = serverRepository.getDefaultSort();
+      this.fetchWithParameters(timestamp, limit, true, sort, null, new RequestFetchDelegateAdapter(delegate));
     } catch (URISyntaxException e) {
       delegate.onFetchFailed(e, null);
     }
@@ -245,7 +258,7 @@ public class Server11RepositorySession extends RepositorySession {
     // TODO: watch out for URL length limits!
     try {
       String ids = flattenIDs(guids);
-      this.fetchWithParameters(-1, true, ids, new RequestFetchDelegateAdapter(delegate));
+      this.fetchWithParameters(-1, -1, true, "index", ids, new RequestFetchDelegateAdapter(delegate));
     } catch (URISyntaxException e) {
       delegate.onFetchFailed(e, null);
     }

mobile/android/base/sync/stage/AndroidBrowserBookmarksServerSyncStage.java

@@ -37,12 +37,21 @@
 
 package org.mozilla.gecko.sync.stage;
 
+import java.net.URISyntaxException;
+
+import org.mozilla.gecko.sync.repositories.ConstrainedServer11Repository;
 import org.mozilla.gecko.sync.repositories.RecordFactory;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.android.AndroidBrowserBookmarksRepository;
 import org.mozilla.gecko.sync.repositories.domain.BookmarkRecordFactory;
 
 public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage {
+
+  // Eventually this kind of sync stage will be data-driven,
+  // and all this hard-coding can go away.
+  private static final String BOOKMARKS_SORT          = "index";
+  private static final long   BOOKMARKS_REQUEST_LIMIT = 5000;         // Sanity limit.
+
   @Override
   public void execute(org.mozilla.gecko.sync.GlobalSession session) throws NoSuchStageException {
     super.execute(session);
@@ -57,6 +66,16 @@ public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage {
     return "bookmarks";
   }
 
+  @Override
+  protected Repository getRemoteRepository() throws URISyntaxException {
+    return new ConstrainedServer11Repository(session.config.getClusterURLString(),
+                                             session.config.username,
+                                             getCollection(),
+                                             session,
+                                             BOOKMARKS_REQUEST_LIMIT,
+                                             BOOKMARKS_SORT);
+  }
+
   @Override
   protected Repository getLocalRepository() {
     return new AndroidBrowserBookmarksRepository();

mobile/android/base/sync/stage/AndroidBrowserHistoryServerSyncStage.java

@@ -37,12 +37,21 @@
 
 package org.mozilla.gecko.sync.stage;
 
+import java.net.URISyntaxException;
+
+import org.mozilla.gecko.sync.repositories.ConstrainedServer11Repository;
 import org.mozilla.gecko.sync.repositories.RecordFactory;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.android.AndroidBrowserHistoryRepository;
 import org.mozilla.gecko.sync.repositories.domain.HistoryRecordFactory;
 
 public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage {
+
+  // Eventually this kind of sync stage will be data-driven,
+  // and all this hard-coding can go away.
+  private static final String HISTORY_SORT          = "index";
+  private static final long   HISTORY_REQUEST_LIMIT = 500;
+
   @Override
   public void execute(org.mozilla.gecko.sync.GlobalSession session) throws NoSuchStageException {
     super.execute(session);
@@ -62,6 +71,16 @@ public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage {
     return new AndroidBrowserHistoryRepository();
   }
 
+  @Override
+  protected Repository getRemoteRepository() throws URISyntaxException {
+    return new ConstrainedServer11Repository(session.config.getClusterURLString(),
+                                             session.config.username,
+                                             getCollection(),
+                                             session,
+                                             HISTORY_REQUEST_LIMIT,
+                                             HISTORY_SORT);
+  }
+
   @Override
   protected RecordFactory getRecordFactory() {
     return new HistoryRecordFactory();

mobile/android/base/sync/stage/ServerSyncStage.java

@@ -84,6 +84,14 @@ public abstract class ServerSyncStage implements
   protected abstract Repository getLocalRepository();
   protected abstract RecordFactory getRecordFactory();
 
+  // Override this in subclasses.
+  protected Repository getRemoteRepository() throws URISyntaxException {
+    return new Server11Repository(session.config.getClusterURLString(),
+                                  session.config.username,
+                                  getCollection(),
+                                  session);
+  }
+
   /**
    * Return a Crypto5Middleware-wrapped Server11Repository.
    *
@@ -97,11 +105,7 @@ public abstract class ServerSyncStage implements
   protected Repository wrappedServerRepo() throws NoCollectionKeysSetException, URISyntaxException {
     String collection = this.getCollection();
     KeyBundle collectionKey = session.keyForCollection(collection);
-    Server11Repository serverRepo = new Server11Repository(session.config.getClusterURLString(),
+    Crypto5MiddlewareRepository cryptoRepo = new Crypto5MiddlewareRepository(getRemoteRepository(), collectionKey);
-                                                           session.config.username,
-                                                           collection,
-                                                           session);
-    Crypto5MiddlewareRepository cryptoRepo = new Crypto5MiddlewareRepository(serverRepo, collectionKey);
     cryptoRepo.recordFactory = getRecordFactory();
     return cryptoRepo;
   }