Bug 1291821 - Use sync deadline to decide of batching downloader should proceed r=rnewman
MozReview-Commit-ID: IDgIj9lBt61
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java
@@ -19,25 +19,27 @@ import org.mozilla.gecko.sync.net.AuthHe
*/
public class ConstrainedServer11Repository extends Server11Repository {
private final String sortOrder;
private final long batchLimit;
private final boolean allowMultipleBatches;
public ConstrainedServer11Repository(
String collection,
+ long syncDeadline,
String storageURL,
AuthHeaderProvider authHeaderProvider,
InfoCollections infoCollections,
InfoConfiguration infoConfiguration,
long batchLimit,
String sort,
boolean allowMultipleBatches) throws URISyntaxException {
super(
collection,
+ syncDeadline,
storageURL,
authHeaderProvider,
infoCollections,
infoConfiguration
);
this.batchLimit = batchLimit;
this.sortOrder = sort;
this.allowMultipleBatches = allowMultipleBatches;
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java
@@ -20,17 +20,17 @@ import android.support.annotation.Nullab
* A Server11Repository implements fetching and storing against the Sync 1.5 API.
* It doesn't do crypto: that's the job of the middleware.
*
* @author rnewman
*/
public class Server11Repository extends Repository {
public final AuthHeaderProvider authHeaderProvider;
- /* package-local */ final long syncDeadline;
+ private final long syncDeadlineMillis;
/* package-local */ final URI collectionURI;
protected final String collection;
protected final InfoCollections infoCollections;
private final InfoConfiguration infoConfiguration;
private final static String DEFAULT_SORT_ORDER = "oldest";
private final static long DEFAULT_BATCH_LIMIT = 100;
@@ -41,30 +41,32 @@ public class Server11Repository extends
* @param collection name.
* @param storageURL full URL to storage endpoint.
* @param authHeaderProvider to use in requests; may be null.
* @param infoCollections instance; must not be null.
* @throws URISyntaxException
*/
public Server11Repository(
@NonNull String collection,
+ long syncDeadlineMillis,
@NonNull String storageURL,
AuthHeaderProvider authHeaderProvider,
@NonNull InfoCollections infoCollections,
@NonNull InfoConfiguration infoConfiguration) throws URISyntaxException {
if (collection == null) {
throw new IllegalArgumentException("collection must not be null");
}
if (storageURL == null) {
throw new IllegalArgumentException("storageURL must not be null");
}
if (infoCollections == null) {
throw new IllegalArgumentException("infoCollections must not be null");
}
this.collection = collection;
+ this.syncDeadlineMillis = syncDeadlineMillis;
this.collectionURI = new URI(storageURL + (storageURL.endsWith("/") ? collection : "/" + collection));
this.authHeaderProvider = authHeaderProvider;
this.infoCollections = infoCollections;
this.infoConfiguration = infoConfiguration;
}
@Override
public void createSession(RepositorySessionCreationDelegate delegate,
@@ -95,9 +97,18 @@ public class Server11Repository extends
public Long getBatchLimit() {
return DEFAULT_BATCH_LIMIT;
}
public boolean getAllowMultipleBatches() {
return true;
}
+
+ /**
+ * A point in time by which this repository's session must complete fetch and store operations.
+ * Particularly pertinent for batching downloads performed by the session (should we fetch
+ * another batch?) and buffered repositories (do we have enough time to merge what we've downloaded?).
+ */
+ public long getSyncDeadline() {
+ return syncDeadlineMillis;
+ }
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java
@@ -107,12 +107,13 @@ public class Server11RepositorySession e
public boolean dataAvailable() {
return serverRepository.updateNeeded(getLastSyncTimestamp());
}
protected static BatchingDownloader initializeDownloader(final Server11RepositorySession serverRepositorySession) {
return new BatchingDownloader(
serverRepositorySession.serverRepository.authHeaderProvider,
Uri.parse(serverRepositorySession.serverRepository.collectionURI().toString()),
+ serverRepositorySession.serverRepository.getSyncDeadline(),
serverRepositorySession.serverRepository.getAllowMultipleBatches(),
serverRepositorySession);
}
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java
@@ -53,34 +53,37 @@ import java.util.concurrent.TimeUnit;
*/
public class BatchingDownloader {
public static final String LOG_TAG = "BatchingDownloader";
private static final String DEFAULT_SORT_ORDER = "index";
private final RepositorySession repositorySession;
private final DelayedWorkTracker workTracker = new DelayedWorkTracker();
private final Uri baseCollectionUri;
+ private final long fetchDeadline;
private final boolean allowMultipleBatches;
/* package-local */ final AuthHeaderProvider authHeaderProvider;
// Used to track outstanding requests, so that we can abort them as needed.
@VisibleForTesting
protected final Set<SyncStorageCollectionRequest> pending = Collections.synchronizedSet(new HashSet<SyncStorageCollectionRequest>());
/* @GuardedBy("this") */ private String lastModified;
public BatchingDownloader(
AuthHeaderProvider authHeaderProvider,
Uri baseCollectionUri,
+ long fetchDeadline,
boolean allowMultipleBatches,
RepositorySession repositorySession) {
this.repositorySession = repositorySession;
this.authHeaderProvider = authHeaderProvider;
this.baseCollectionUri = baseCollectionUri;
this.allowMultipleBatches = allowMultipleBatches;
+ this.fetchDeadline = fetchDeadline;
}
@VisibleForTesting
protected static String flattenIDs(String[] guids) {
// Consider using Utils.toDelimitedString if and when the signature changes
// to Collection<String> guids.
if (guids.length == 0) {
return "";
@@ -212,16 +215,22 @@ public class BatchingDownloader {
this.workTracker.delayWorkItem(new Runnable() {
@Override
public void run() {
Logger.debug(LOG_TAG, "Running onBatchCompleted.");
fetchRecordsDelegate.onBatchCompleted();
}
});
+ // Should we proceed, however? Do we have enough time?
+ if (!mayProceedWithBatching(fetchDeadline)) {
+ this.abort(fetchRecordsDelegate, new Exception("Not enough time to complete next batch"));
+ return;
+ }
+
// Create and execute new batch request.
try {
final SyncStorageCollectionRequest newRequest = makeSyncStorageCollectionRequest(newer,
limit, full, sort, ids, offset);
this.fetchWithParameters(newer, limit, full, sort, ids, newRequest, fetchRecordsDelegate);
} catch (final URISyntaxException | UnsupportedEncodingException e) {
this.workTracker.delayWorkItem(new Runnable() {
@Override
@@ -288,16 +297,24 @@ public class BatchingDownloader {
this.workTracker.delayWorkItem(new Runnable() {
@Override
public void run() {
Logger.debug(LOG_TAG, "Delayed onFetchCompleted running.");
delegate.onFetchFailed(exception);
}
});
}
+
+ private static boolean mayProceedWithBatching(long deadline) {
+ // For simplicity, allow batching to proceed if there's at least a minute left for the sync.
+ // This should be enough to fetch and process records in the batch.
+ final long timeLeft = deadline - SystemClock.elapsedRealtime();
+ return timeLeft > TimeUnit.MINUTES.toMillis(1);
+ }
+
@VisibleForTesting
public static URI buildCollectionURI(Uri baseCollectionUri, boolean full, long newer, long limit, String sort, String ids, String offset) throws URISyntaxException {
Uri.Builder uriBuilder = baseCollectionUri.buildUpon();
if (full) {
uriBuilder.appendQueryParameter("full", "1");
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java
@@ -36,16 +36,17 @@ public class AndroidBrowserBookmarksServ
public Integer getStorageVersion() {
return VersionConstants.BOOKMARKS_ENGINE_VERSION;
}
@Override
protected Repository getRemoteRepository() throws URISyntaxException {
return new ConstrainedServer11Repository(
getCollection(),
+ session.getSyncDeadline(),
session.config.storageURL(),
session.getAuthHeaderProvider(),
session.config.infoCollections,
session.config.infoConfiguration,
BOOKMARKS_BATCH_LIMIT,
BOOKMARKS_SORT,
true /* allow multiple batches */);
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java
@@ -41,16 +41,17 @@ public class AndroidBrowserHistoryServer
protected Repository getLocalRepository() {
return new AndroidBrowserHistoryRepository();
}
@Override
protected Repository getRemoteRepository() throws URISyntaxException {
return new ConstrainedServer11Repository(
getCollection(),
+ session.getSyncDeadline(),
session.config.storageURL(),
session.getAuthHeaderProvider(),
session.config.infoCollections,
session.config.infoConfiguration,
HISTORY_BATCH_LIMIT,
HISTORY_SORT,
true /* allow multiple batches */);
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java
@@ -139,16 +139,17 @@ public abstract class ServerSyncStage ex
protected abstract String getEngineName();
protected abstract Repository getLocalRepository();
protected abstract RecordFactory getRecordFactory();
// Override this in subclasses.
protected Repository getRemoteRepository() throws URISyntaxException {
String collection = getCollection();
return new Server11Repository(collection,
+ session.getSyncDeadline(),
session.config.storageURL(),
session.getAuthHeaderProvider(),
session.config.infoCollections,
session.config.infoConfiguration);
}
/**
* Return a Crypto5Middleware-wrapped Server11Repository.