author | Grisha Kruglov <gkruglov@mozilla.com> |
Thu, 20 Oct 2016 13:34:06 -0700 | |
changeset 344865 | 63957df45bace6be17728d614625e6b358c0cc08 |
parent 344864 | 4ecaf3862bc7d0feb4281ae5ec24b8495446f0c7 |
child 344866 | 50608e0550054650b192bc451c3aeab6e6a0d5e6 |
push id | 37970 |
push user | gkruglov@mozilla.com |
push date | Sat, 25 Feb 2017 01:09:28 +0000 |
treeherder | autoland@bd232d46a396 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | rnewman |
bugs | 1291821 |
milestone | 54.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
|
--- a/mobile/android/base/android-services.mozbuild +++ b/mobile/android/base/android-services.mozbuild @@ -1047,17 +1047,16 @@ sync_java_files = [TOPSRCDIR + '/mobile/ 'sync/stage/FennecTabsServerSyncStage.java', 'sync/stage/FetchInfoCollectionsStage.java', 'sync/stage/FetchInfoConfigurationStage.java', 'sync/stage/FetchMetaGlobalStage.java', 'sync/stage/FormHistoryServerSyncStage.java', 'sync/stage/GlobalSyncStage.java', 'sync/stage/NoSuchStageException.java', 'sync/stage/PasswordsServerSyncStage.java', - 'sync/stage/SafeConstrainedServer11Repository.java', 'sync/stage/ServerSyncStage.java', 'sync/stage/SyncClientsEngineStage.java', 'sync/stage/UploadMetaGlobalStage.java', 'sync/Sync11Configuration.java', 'sync/SyncConfiguration.java', 'sync/SyncConfigurationException.java', 'sync/SyncConstants.java', 'sync/SyncException.java',
--- 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 @@ -6,46 +6,55 @@ package org.mozilla.gecko.sync.repositor import java.net.URISyntaxException; import org.mozilla.gecko.sync.InfoCollections; import org.mozilla.gecko.sync.InfoConfiguration; import org.mozilla.gecko.sync.net.AuthHeaderProvider; /** - * A kind of Server11Repository that supports explicit setting of total fetch limit, per-batch fetch limit, and a sort order. + * A kind of Server11Repository that supports explicit setting of per-batch fetch limit, + * batching mode (single batch vs multi-batch), and a sort order. * * @author rnewman * */ public class ConstrainedServer11Repository extends Server11Repository { - - private final String sort; + private final String sortOrder; private final long batchLimit; - private final long totalLimit; + private final boolean allowMultipleBatches; - public ConstrainedServer11Repository(String collection, String storageURL, - AuthHeaderProvider authHeaderProvider, - InfoCollections infoCollections, - InfoConfiguration infoConfiguration, - long batchLimit, long totalLimit, String sort) - throws URISyntaxException { - super(collection, storageURL, authHeaderProvider, infoCollections, infoConfiguration); + public ConstrainedServer11Repository( + String collection, + String storageURL, + AuthHeaderProvider authHeaderProvider, + InfoCollections infoCollections, + InfoConfiguration infoConfiguration, + long batchLimit, + String sort, + boolean allowMultipleBatches) throws URISyntaxException { + super( + collection, + storageURL, + authHeaderProvider, + infoCollections, + infoConfiguration + ); this.batchLimit = batchLimit; - this.totalLimit = totalLimit; - this.sort = sort; + this.sortOrder = sort; + this.allowMultipleBatches = allowMultipleBatches; } @Override - public String getDefaultSort() { - return sort; + public String getSortOrder() { + return sortOrder; } @Override - public long getDefaultBatchLimit() { + public Long getBatchLimit() { return batchLimit; } @Override - public long getDefaultTotalLimit() { - return totalLimit; + public boolean getAllowMultipleBatches() { + return 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 @@ -1,52 +1,60 @@ /* 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.URI; import java.net.URISyntaxException; -import java.util.ArrayList; import org.mozilla.gecko.sync.InfoCollections; import org.mozilla.gecko.sync.InfoConfiguration; -import org.mozilla.gecko.sync.Utils; import org.mozilla.gecko.sync.net.AuthHeaderProvider; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate; import android.content.Context; import android.support.annotation.NonNull; import android.support.annotation.Nullable; /** - * A Server11Repository implements fetching and storing against the Sync 1.1 API. + * 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 { - protected String collection; - protected URI collectionURI; - protected final AuthHeaderProvider authHeaderProvider; + public final AuthHeaderProvider authHeaderProvider; + + /* package-local */ final long syncDeadline; + /* 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; /** - * Construct a new repository that fetches and stores against the Sync 1.1. API. + * Construct a new repository that fetches and stores against the Sync 1.5 API. * * @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, @NonNull String storageURL, AuthHeaderProvider authHeaderProvider, @NonNull InfoCollections infoCollections, @NonNull InfoConfiguration infoConfiguration) throws URISyntaxException { + public Server11Repository( + @NonNull String collection, + @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"); @@ -63,82 +71,33 @@ public class Server11Repository extends Context context) { delegate.onSessionCreated(new Server11RepositorySession(this)); } public URI collectionURI() { return this.collectionURI; } - public URI collectionURI(boolean full, long newer, long limit, String sort, String ids, String offset) throws URISyntaxException { - ArrayList<String> params = new ArrayList<String>(); - if (full) { - params.add("full=1"); - } - 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 (offset != null) { - // Offset comes straight out of HTTP headers and it is the responsibility of the caller to URI-escape it. - params.add("offset=" + offset); - } - if (params.size() == 0) { - return this.collectionURI; - } - - StringBuilder out = new StringBuilder(); - char indicator = '?'; - for (String param : params) { - out.append(indicator); - indicator = '&'; - out.append(param); - } - String uri = this.collectionURI + out.toString(); - return new URI(uri); - } - - public URI wboURI(String id) throws URISyntaxException { - return new URI(this.collectionURI + "/" + id); - } - - // Override these. - @SuppressWarnings("static-method") - public long getDefaultBatchLimit() { - return -1; - } - - @SuppressWarnings("static-method") - public String getDefaultSort() { - return null; - } - - public long getDefaultTotalLimit() { - return -1; - } - - public AuthHeaderProvider getAuthHeaderProvider() { - return authHeaderProvider; - } - - public boolean updateNeeded(long lastSyncTimestamp) { + /* package-local */ boolean updateNeeded(long lastSyncTimestamp) { return infoCollections.updateNeeded(collection, lastSyncTimestamp); } @Nullable - public Long getCollectionLastModified() { + /* package-local */ Long getCollectionLastModified() { return infoCollections.getTimestamp(collection); } public InfoConfiguration getInfoConfiguration() { return infoConfiguration; } + + public String getSortOrder() { + return DEFAULT_SORT_ORDER; + } + + public Long getBatchLimit() { + return DEFAULT_BATCH_LIMIT; + } + + public boolean getAllowMultipleBatches() { + return true; + } }
--- 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 @@ -11,30 +11,26 @@ import org.mozilla.gecko.sync.repositori import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionGuidsSinceDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionStoreDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionWipeDelegate; import org.mozilla.gecko.sync.repositories.domain.Record; import org.mozilla.gecko.sync.repositories.downloaders.BatchingDownloader; import org.mozilla.gecko.sync.repositories.uploaders.BatchingUploader; public class Server11RepositorySession extends RepositorySession { - public static final String LOG_TAG = "Server11Session"; + public static final String LOG_TAG = "Server11RepositorySession"; - Server11Repository serverRepository; + protected final Server11Repository serverRepository; private BatchingUploader uploader; private final BatchingDownloader downloader; public Server11RepositorySession(Repository repository) { super(repository); - serverRepository = (Server11Repository) repository; - this.downloader = new BatchingDownloader(serverRepository, this); - } - - public Server11Repository getServerRepository() { - return serverRepository; + this.serverRepository = (Server11Repository) repository; + this.downloader = initializeDownloader(this); } @Override public void setStoreDelegate(RepositorySessionStoreDelegate storeDelegate) { super.setStoreDelegate(storeDelegate); // Now that we have the delegate, we can initialize our uploader. this.uploader = new BatchingUploader( @@ -46,19 +42,24 @@ public class Server11RepositorySession e @Override public void guidsSince(long timestamp, RepositorySessionGuidsSinceDelegate delegate) { // TODO Auto-generated method stub } @Override - public void fetchSince(long timestamp, + public void fetchSince(long sinceTimestamp, RepositorySessionFetchRecordsDelegate delegate) { - this.downloader.fetchSince(timestamp, delegate); + this.downloader.fetchSince( + delegate, + sinceTimestamp, + serverRepository.getBatchLimit(), + serverRepository.getSortOrder() + ); } @Override public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) { this.fetchSince(-1, delegate); } @Override @@ -101,9 +102,17 @@ public class Server11RepositorySession e uploader.noMoreRecordsToUpload(); } @Override 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.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 @@ -1,74 +1,86 @@ /* 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.downloaders; +import android.net.Uri; +import android.os.SystemClock; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import org.mozilla.gecko.background.common.log.Logger; import org.mozilla.gecko.sync.CryptoRecord; import org.mozilla.gecko.sync.DelayedWorkTracker; +import org.mozilla.gecko.sync.Utils; +import org.mozilla.gecko.sync.net.AuthHeaderProvider; import org.mozilla.gecko.sync.net.SyncResponse; import org.mozilla.gecko.sync.net.SyncStorageCollectionRequest; import org.mozilla.gecko.sync.net.SyncStorageResponse; -import org.mozilla.gecko.sync.repositories.Server11Repository; -import org.mozilla.gecko.sync.repositories.Server11RepositorySession; +import org.mozilla.gecko.sync.repositories.RepositorySession; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate; import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; -import java.net.URLEncoder; import java.util.Collections; +import java.util.ConcurrentModificationException; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.TimeUnit; /** - * Batching Downloader, which implements batching protocol as supported by Sync 1.5. + * Batching Downloader implements batching protocol as supported by Sync 1.5. * * Downloader's batching behaviour is configured via two parameters, obtained from the repository: * - Per-batch limit, which specified how many records may be fetched in an individual GET request. - * - Total limit, which controls number of batch GET requests we will make. - * + * - allowMultipleBatches, which determines if downloader is allowed to perform more than one fetch. * * Batching is implemented via specifying a 'limit' GET parameter, and looking for an 'offset' token * in the response. If offset token is present, this indicates that there are more records than what - * we've received so far, and we perform an additional fetch. Batching stops when either we hit a total - * limit, or offset token is no longer present (indicating that we're done). + * we've received so far, and we perform an additional fetch, if we're allowed to do so by our + * configuration. Batching stops when offset token is no longer present (indicating that we're done). * - * For unlimited repositories (such as passwords), both of these value will be -1. Downloader will not - * specify a limit parameter in this case, and the response will contain every record available and no - * offset token, thus fully completing in one go. + * If we are not allowed to perform multiple batches, we consider batching to be succesfully complete + * after fist fetch request succeeds. Similarly, a trivial case of collection having less records than + * the batch limit will also successfully complete in one fetch. * - * In between batches, we maintain a Last-Modified timestamp, based off the value return in the header + * In between batches, we maintain a Last-Modified timestamp, based off the value returned in the header * of the first response. Every response will have a Last-Modified header, indicating when the collection * was modified last. We pass along this header in our subsequent requests in a X-If-Unmodified-Since * header. Server will ensure that our collection did not change while we are batching, if it did it will - * fail our fetch with a 412 (Consequent Modification) error. Additionally, we perform the same checks - * locally. + * fail our fetch with a 412 error. Additionally, we perform the same checks locally. */ public class BatchingDownloader { public static final String LOG_TAG = "BatchingDownloader"; + private static final String DEFAULT_SORT_ORDER = "index"; - protected final Server11Repository repository; - private final Server11RepositorySession repositorySession; + private final RepositorySession repositorySession; private final DelayedWorkTracker workTracker = new DelayedWorkTracker(); + private final Uri baseCollectionUri; + 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; - /* @GuardedBy("this") */ private long numRecords = 0; - public BatchingDownloader(final Server11Repository repository, final Server11RepositorySession repositorySession) { - this.repository = repository; + public BatchingDownloader( + AuthHeaderProvider authHeaderProvider, + Uri baseCollectionUri, + boolean allowMultipleBatches, + RepositorySession repositorySession) { this.repositorySession = repositorySession; + this.authHeaderProvider = authHeaderProvider; + this.baseCollectionUri = baseCollectionUri; + this.allowMultipleBatches = allowMultipleBatches; } @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 ""; @@ -89,165 +101,141 @@ public class BatchingDownloader { protected void fetchWithParameters(long newer, long batchLimit, boolean full, String sort, String ids, SyncStorageCollectionRequest request, RepositorySessionFetchRecordsDelegate fetchRecordsDelegate) throws URISyntaxException, UnsupportedEncodingException { - if (batchLimit > repository.getDefaultTotalLimit()) { - throw new IllegalArgumentException("Batch limit should not be greater than total limit"); - } - request.delegate = new BatchingDownloaderDelegate(this, fetchRecordsDelegate, request, newer, batchLimit, full, sort, ids); this.pending.add(request); request.get(); } @VisibleForTesting - @Nullable - protected String encodeParam(String param) throws UnsupportedEncodingException { - if (param != null) { - return URLEncoder.encode(param, "UTF-8"); - } - return null; - } - - @VisibleForTesting protected SyncStorageCollectionRequest makeSyncStorageCollectionRequest(long newer, long batchLimit, boolean full, String sort, String ids, String offset) throws URISyntaxException, UnsupportedEncodingException { - URI collectionURI = repository.collectionURI(full, newer, batchLimit, sort, ids, encodeParam(offset)); + final URI collectionURI = buildCollectionURI(baseCollectionUri, full, newer, batchLimit, sort, ids, offset); Logger.debug(LOG_TAG, collectionURI.toString()); return new SyncStorageCollectionRequest(collectionURI); } - public void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate fetchRecordsDelegate) { - this.fetchSince(timestamp, null, fetchRecordsDelegate); + public void fetchSince(RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, long timestamp, long batchLimit, String sortOrder) { + this.fetchSince(fetchRecordsDelegate, timestamp, batchLimit, sortOrder, null); } - private void fetchSince(long timestamp, String offset, - RepositorySessionFetchRecordsDelegate fetchRecordsDelegate) { - long batchLimit = repository.getDefaultBatchLimit(); - String sort = repository.getDefaultSort(); - + @VisibleForTesting + public void fetchSince(RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, long timestamp, long batchLimit, String sortOrder, String offset) { try { SyncStorageCollectionRequest request = makeSyncStorageCollectionRequest(timestamp, - batchLimit, true, sort, null, offset); - this.fetchWithParameters(timestamp, batchLimit, true, sort, null, request, fetchRecordsDelegate); + batchLimit, true, sortOrder, null, offset); + this.fetchWithParameters(timestamp, batchLimit, true, sortOrder, null, request, fetchRecordsDelegate); } catch (URISyntaxException | UnsupportedEncodingException e) { fetchRecordsDelegate.onFetchFailed(e); } } public void fetch(String[] guids, RepositorySessionFetchRecordsDelegate fetchRecordsDelegate) { String ids = flattenIDs(guids); - String index = "index"; try { SyncStorageCollectionRequest request = makeSyncStorageCollectionRequest( - -1, -1, true, index, ids, null); - this.fetchWithParameters(-1, -1, true, index, ids, request, fetchRecordsDelegate); + -1, -1, true, DEFAULT_SORT_ORDER, ids, null); + this.fetchWithParameters(-1, -1, true, DEFAULT_SORT_ORDER, ids, request, fetchRecordsDelegate); } catch (URISyntaxException | UnsupportedEncodingException e) { fetchRecordsDelegate.onFetchFailed(e); } } - public Server11Repository getServerRepository() { - return this.repository; - } - public void onFetchCompleted(SyncStorageResponse response, final RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, final SyncStorageCollectionRequest request, long newer, long limit, boolean full, String sort, String ids) { removeRequestFromPending(request); - // When we process our first request, we get back a X-Last-Modified header indicating when collection was modified last. - // We pass it to the server with every subsequent request (if we need to make more) as the X-If-Unmodified-Since header, - // and server is supposed to ensure that this pre-condition is met, and fail our request with a 412 error code otherwise. - // So, if all of this happens, these checks should never fail. - // However, we also track this header in client side, and can defensively validate against it here as well. + // When we process our first request, we get back a X-Last-Modified header indicating when + // collection was modified last. We pass it to the server with every subsequent request + // (if we need to make more) as the X-If-Unmodified-Since header, and server is supposed to + // ensure that this pre-condition is met, and fail our request with a 412 error code otherwise. + // So, if all of this happens, these checks should never fail. However, we also track this + // header on the client side, and can defensively validate against it here as well. + + // This value won't be null, since we check for this in the delegate. final String currentLastModifiedTimestamp = response.lastModified(); Logger.debug(LOG_TAG, "Last modified timestamp " + currentLastModifiedTimestamp); - // Sanity check. We also did a null check in delegate before passing it into here. - if (currentLastModifiedTimestamp == null) { - this.abort(fetchRecordsDelegate, "Last modified timestamp is missing"); - return; - } - final boolean lastModifiedChanged; synchronized (this) { if (this.lastModified == null) { // First time seeing last modified timestamp. this.lastModified = currentLastModifiedTimestamp; } lastModifiedChanged = !this.lastModified.equals(currentLastModifiedTimestamp); } + // We expected server to fail our request with 412 in case of concurrent modifications, so + // this is unexpected. However, let's treat this case just as if we received a 412. if (lastModifiedChanged) { - this.abort(fetchRecordsDelegate, "Last modified timestamp has changed unexpectedly"); + this.abort( + fetchRecordsDelegate, + new ConcurrentModificationException("Last-modified timestamp has changed unexpectedly") + ); return; } - final boolean hasNotReachedLimit; - synchronized (this) { - this.numRecords += response.weaveRecords(); - hasNotReachedLimit = this.numRecords < repository.getDefaultTotalLimit(); + // If we can (or must) stop batching at this point, let the delegate know that we're all done! + final String offset = response.weaveOffset(); + if (offset == null || !allowMultipleBatches) { + final long normalizedTimestamp = response.normalizedTimestampForHeader(SyncResponse.X_LAST_MODIFIED); + Logger.debug(LOG_TAG, "Fetch completed. Timestamp is " + normalizedTimestamp); + + this.workTracker.delayWorkItem(new Runnable() { + @Override + public void run() { + Logger.debug(LOG_TAG, "Delayed onFetchCompleted running."); + fetchRecordsDelegate.onFetchCompleted(normalizedTimestamp); + } + }); + return; } - final String offset = response.weaveOffset(); - final SyncStorageCollectionRequest newRequest; + // We need to make another batching request! + // Let the delegate know that a batch fetch just completed before we proceed. + // This operation needs to run after every call to onFetchedRecord for this batch has been + // processed, hence the delayWorkItem call. + this.workTracker.delayWorkItem(new Runnable() { + @Override + public void run() { + Logger.debug(LOG_TAG, "Running onBatchCompleted."); + fetchRecordsDelegate.onBatchCompleted(); + } + }); + + // Create and execute new batch request. try { - newRequest = makeSyncStorageCollectionRequest(newer, + 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 public void run() { Logger.debug(LOG_TAG, "Delayed onFetchCompleted running."); fetchRecordsDelegate.onFetchFailed(e); } }); - return; } - - if (offset != null && hasNotReachedLimit) { - try { - this.fetchWithParameters(newer, limit, full, sort, ids, newRequest, fetchRecordsDelegate); - } catch (final URISyntaxException | UnsupportedEncodingException e) { - this.workTracker.delayWorkItem(new Runnable() { - @Override - public void run() { - Logger.debug(LOG_TAG, "Delayed onFetchCompleted running."); - fetchRecordsDelegate.onFetchFailed(e, null); - } - }); - } - return; - } - - final long normalizedTimestamp = response.normalizedTimestampForHeader(SyncResponse.X_LAST_MODIFIED); - Logger.debug(LOG_TAG, "Fetch completed. Timestamp is " + normalizedTimestamp); - - this.workTracker.delayWorkItem(new Runnable() { - @Override - public void run() { - Logger.debug(LOG_TAG, "Delayed onFetchCompleted running."); - fetchRecordsDelegate.onFetchCompleted(normalizedTimestamp); - } - }); } public void onFetchFailed(final Exception ex, final RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, final SyncStorageCollectionRequest request) { removeRequestFromPending(request); this.workTracker.delayWorkItem(new Runnable() { @Override @@ -289,22 +277,49 @@ public class BatchingDownloader { } } @Nullable protected synchronized String getLastModified() { return this.lastModified; } - private void abort(final RepositorySessionFetchRecordsDelegate delegate, final String msg) { - Logger.error(LOG_TAG, msg); + private void abort(final RepositorySessionFetchRecordsDelegate delegate, final Exception exception) { + Logger.error(LOG_TAG, exception.getMessage()); this.abortRequests(); this.workTracker.delayWorkItem(new Runnable() { @Override public void run() { Logger.debug(LOG_TAG, "Delayed onFetchCompleted running."); - delegate.onFetchFailed( - new IllegalStateException(msg), - null); + delegate.onFetchFailed(exception); } }); } + @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"); + } + + if (newer >= 0) { + // Translate local millisecond timestamps into server decimal seconds. + String newerString = Utils.millisecondsToDecimalSecondsString(newer); + uriBuilder.appendQueryParameter("newer", newerString); + } + if (limit > 0) { + uriBuilder.appendQueryParameter("limit", Long.toString(limit)); + } + if (sort != null) { + uriBuilder.appendQueryParameter("sort", sort); // We trust these values. + } + if (ids != null) { + uriBuilder.appendQueryParameter("ids", ids); // We trust these values. + } + if (offset != null) { + // Offset comes straight out of HTTP headers and it is the responsibility of the caller to URI-escape it. + uriBuilder.appendQueryParameter("offset", offset); + } + + return new URI(uriBuilder.build().toString()); + } }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderDelegate.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderDelegate.java @@ -9,31 +9,34 @@ import org.mozilla.gecko.sync.CryptoReco import org.mozilla.gecko.sync.HTTPFailureException; import org.mozilla.gecko.sync.crypto.KeyBundle; import org.mozilla.gecko.sync.net.AuthHeaderProvider; import org.mozilla.gecko.sync.net.SyncStorageCollectionRequest; import org.mozilla.gecko.sync.net.SyncStorageResponse; import org.mozilla.gecko.sync.net.WBOCollectionRequestDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate; + +import java.util.ConcurrentModificationException; + /** * Delegate that gets passed into fetch methods to handle server response from fetch. */ public class BatchingDownloaderDelegate extends WBOCollectionRequestDelegate { public static final String LOG_TAG = "BatchingDownloaderDelegate"; - private BatchingDownloader downloader; - private RepositorySessionFetchRecordsDelegate fetchRecordsDelegate; - public SyncStorageCollectionRequest request; + private final BatchingDownloader downloader; + private final RepositorySessionFetchRecordsDelegate fetchRecordsDelegate; + public final SyncStorageCollectionRequest request; // Used to pass back to BatchDownloader to start another fetch with these parameters if needed. - private long newer; - private long batchLimit; - private boolean full; - private String sort; - private String ids; + private final long newer; + private final long batchLimit; + private final boolean full; + private final String sort; + private final String ids; public BatchingDownloaderDelegate(final BatchingDownloader downloader, final RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, final SyncStorageCollectionRequest request, long newer, long batchLimit, boolean full, String sort, String ids) { this.downloader = downloader; this.fetchRecordsDelegate = fetchRecordsDelegate; this.request = request; @@ -41,17 +44,17 @@ public class BatchingDownloaderDelegate this.batchLimit = batchLimit; this.full = full; this.sort = sort; this.ids = ids; } @Override public AuthHeaderProvider getAuthHeaderProvider() { - return this.downloader.getServerRepository().getAuthHeaderProvider(); + return this.downloader.authHeaderProvider; } @Override public String ifUnmodifiedSince() { return this.downloader.getLastModified(); } @Override @@ -65,17 +68,28 @@ public class BatchingDownloaderDelegate this.downloader.onFetchFailed( new IllegalStateException("Missing last modified header from response"), this.fetchRecordsDelegate, this.request); } @Override public void handleRequestFailure(SyncStorageResponse response) { - this.handleRequestError(new HTTPFailureException(response)); + Logger.warn(LOG_TAG, "Got a non-success response."); + // Handle concurrent modification errors separately. We will need to signal upwards that + // this happened, in case stage buffer will want to clean up. + if (response.getStatusCode() == 412) { + this.downloader.onFetchFailed( + new ConcurrentModificationException(), + this.fetchRecordsDelegate, + this.request + ); + } else { + this.handleRequestError(new HTTPFailureException(response)); + } } @Override public void handleRequestError(final Exception ex) { Logger.warn(LOG_TAG, "Got request error.", ex); this.downloader.onFetchFailed(ex, this.fetchRecordsDelegate, this.request); }
--- 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 @@ -1,35 +1,31 @@ /* 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.stage; import java.net.URISyntaxException; -import org.mozilla.gecko.sync.JSONRecordFetcher; import org.mozilla.gecko.sync.MetaGlobalException; -import org.mozilla.gecko.sync.net.AuthHeaderProvider; +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; import org.mozilla.gecko.sync.repositories.domain.VersionConstants; public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage { protected static final String LOG_TAG = "BookmarksStage"; // 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"; - // Sanity limit. Batch and total limit are the same for now, and will be adjusted - // once buffer and high water mark are in place. See Bug 730142. + private static final String BOOKMARKS_SORT = "oldest"; private static final long BOOKMARKS_BATCH_LIMIT = 5000; - private static final long BOOKMARKS_TOTAL_LIMIT = 5000; @Override protected String getCollection() { return "bookmarks"; } @Override protected String getEngineName() { @@ -38,31 +34,25 @@ public class AndroidBrowserBookmarksServ @Override public Integer getStorageVersion() { return VersionConstants.BOOKMARKS_ENGINE_VERSION; } @Override protected Repository getRemoteRepository() throws URISyntaxException { - // If this is a first sync, we need to check server counts to make sure that we aren't - // going to screw up. SafeConstrainedServer11Repository does this. See Bug 814331. - AuthHeaderProvider authHeaderProvider = session.getAuthHeaderProvider(); - final JSONRecordFetcher countsFetcher = new JSONRecordFetcher(session.config.infoCollectionCountsURL(), authHeaderProvider); - String collection = getCollection(); - return new SafeConstrainedServer11Repository( - collection, - session.config.storageURL(), - session.getAuthHeaderProvider(), - session.config.infoCollections, - session.config.infoConfiguration, - BOOKMARKS_BATCH_LIMIT, - BOOKMARKS_TOTAL_LIMIT, - BOOKMARKS_SORT, - countsFetcher); + return new ConstrainedServer11Repository( + getCollection(), + session.config.storageURL(), + session.getAuthHeaderProvider(), + session.config.infoCollections, + session.config.infoConfiguration, + BOOKMARKS_BATCH_LIMIT, + BOOKMARKS_SORT, + true /* allow multiple batches */); } @Override protected Repository getLocalRepository() { return new AndroidBrowserBookmarksRepository(); } @Override
--- 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 @@ -14,21 +14,18 @@ import org.mozilla.gecko.sync.repositori import org.mozilla.gecko.sync.repositories.domain.HistoryRecordFactory; import org.mozilla.gecko.sync.repositories.domain.VersionConstants; public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage { protected static final String LOG_TAG = "HistoryStage"; // 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"; - // Sanity limit. Batch and total limit are the same for now, and will be adjusted - // once buffer and high water mark are in place. See Bug 730142. - private static final long HISTORY_BATCH_LIMIT = 250; - private static final long HISTORY_TOTAL_LIMIT = 250; + private static final String HISTORY_SORT = "oldest"; + private static final long HISTORY_BATCH_LIMIT = 500; @Override protected String getCollection() { return "history"; } @Override protected String getEngineName() { @@ -42,26 +39,25 @@ public class AndroidBrowserHistoryServer @Override protected Repository getLocalRepository() { return new AndroidBrowserHistoryRepository(); } @Override protected Repository getRemoteRepository() throws URISyntaxException { - String collection = getCollection(); return new ConstrainedServer11Repository( - collection, - session.config.storageURL(), - session.getAuthHeaderProvider(), - session.config.infoCollections, - session.config.infoConfiguration, - HISTORY_BATCH_LIMIT, - HISTORY_TOTAL_LIMIT, - HISTORY_SORT); + getCollection(), + session.config.storageURL(), + session.getAuthHeaderProvider(), + session.config.infoCollections, + session.config.infoConfiguration, + HISTORY_BATCH_LIMIT, + HISTORY_SORT, + true /* allow multiple batches */); } @Override protected RecordFactory getRecordFactory() { return new HistoryRecordFactory(); } @Override
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FormHistoryServerSyncStage.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FormHistoryServerSyncStage.java @@ -14,21 +14,18 @@ import org.mozilla.gecko.sync.repositori import org.mozilla.gecko.sync.repositories.domain.FormHistoryRecord; import org.mozilla.gecko.sync.repositories.domain.Record; import org.mozilla.gecko.sync.repositories.domain.VersionConstants; public class FormHistoryServerSyncStage extends ServerSyncStage { // Eventually this kind of sync stage will be data-driven, // and all this hard-coding can go away. - private static final String FORM_HISTORY_SORT = "index"; - // Sanity limit. Batch and total limit are the same for now, and will be adjusted - // once buffer and high water mark are in place. See Bug 730142. + private static final String FORM_HISTORY_SORT = "oldest"; private static final long FORM_HISTORY_BATCH_LIMIT = 5000; - private static final long FORM_HISTORY_TOTAL_LIMIT = 5000; @Override protected String getCollection() { return "forms"; } @Override protected String getEngineName() { @@ -39,24 +36,25 @@ public class FormHistoryServerSyncStage public Integer getStorageVersion() { return VersionConstants.FORMS_ENGINE_VERSION; } @Override protected Repository getRemoteRepository() throws URISyntaxException { String collection = getCollection(); return new ConstrainedServer11Repository( - collection, - session.config.storageURL(), - session.getAuthHeaderProvider(), - session.config.infoCollections, - session.config.infoConfiguration, - FORM_HISTORY_BATCH_LIMIT, - FORM_HISTORY_TOTAL_LIMIT, - FORM_HISTORY_SORT); + collection, + session.getSyncDeadline(), + session.config.storageURL(), + session.getAuthHeaderProvider(), + session.config.infoCollections, + session.config.infoConfiguration, + FORM_HISTORY_BATCH_LIMIT, + FORM_HISTORY_SORT, + true /* allow multiple batches */); } @Override protected Repository getLocalRepository() { return new FormHistoryRepositorySession.FormHistoryRepository(); } public class FormHistoryRecordFactory extends RecordFactory {
deleted file mode 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/SafeConstrainedServer11Repository.java +++ /dev/null @@ -1,110 +0,0 @@ -/* 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.stage; - -import java.net.URISyntaxException; - -import org.mozilla.gecko.background.common.log.Logger; -import org.mozilla.gecko.sync.InfoCollections; -import org.mozilla.gecko.sync.InfoConfiguration; -import org.mozilla.gecko.sync.InfoCounts; -import org.mozilla.gecko.sync.JSONRecordFetcher; -import org.mozilla.gecko.sync.net.AuthHeaderProvider; -import org.mozilla.gecko.sync.repositories.ConstrainedServer11Repository; -import org.mozilla.gecko.sync.repositories.Repository; -import org.mozilla.gecko.sync.repositories.Server11RepositorySession; -import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate; - -import android.content.Context; - -/** - * This is a constrained repository -- one which fetches a limited number - * of records -- that additionally refuses to sync if the limit will - * be exceeded on a first sync by the number of records on the server. - * - * You must pass an {@link InfoCounts} instance, which will be interrogated - * in the event of a first sync. - * - * "First sync" means that our sync timestamp is not greater than zero. - */ -public class SafeConstrainedServer11Repository extends ConstrainedServer11Repository { - - // This can be lazily evaluated if we need it. - private final JSONRecordFetcher countFetcher; - - public SafeConstrainedServer11Repository(String collection, - String storageURL, - AuthHeaderProvider authHeaderProvider, - InfoCollections infoCollections, - InfoConfiguration infoConfiguration, - long batchLimit, - long totalLimit, - String sort, - JSONRecordFetcher countFetcher) - throws URISyntaxException { - super(collection, storageURL, authHeaderProvider, infoCollections, infoConfiguration, - batchLimit, totalLimit, sort); - if (countFetcher == null) { - throw new IllegalArgumentException("countFetcher must not be null"); - } - this.countFetcher = countFetcher; - } - - @Override - public void createSession(RepositorySessionCreationDelegate delegate, - Context context) { - delegate.onSessionCreated(new CountCheckingServer11RepositorySession(this, this.getDefaultBatchLimit())); - } - - public class CountCheckingServer11RepositorySession extends Server11RepositorySession { - private static final String LOG_TAG = "CountCheckingServer11RepositorySession"; - - /** - * The session will report no data available if this is a first sync - * and the server has more data available than this limit. - */ - private final long fetchLimit; - - public CountCheckingServer11RepositorySession(Repository repository, long fetchLimit) { - super(repository); - this.fetchLimit = fetchLimit; - } - - @Override - public boolean shouldSkip() { - // If this is a first sync, verify that we aren't going to blow through our limit. - final long lastSyncTimestamp = getLastSyncTimestamp(); - if (lastSyncTimestamp > 0) { - Logger.info(LOG_TAG, "Collection " + collection + " has already had a first sync: " + - "timestamp is " + lastSyncTimestamp + "; " + - "ignoring any updated counts and syncing as usual."); - } else { - Logger.info(LOG_TAG, "Collection " + collection + " is starting a first sync; checking counts."); - - final InfoCounts counts; - try { - // This'll probably be the same object, but best to obey the API. - counts = new InfoCounts(countFetcher.fetchBlocking()); - } catch (Exception e) { - Logger.warn(LOG_TAG, "Skipping " + collection + " until we can fetch counts.", e); - return true; - } - - Integer c = counts.getCount(collection); - if (c == null) { - Logger.info(LOG_TAG, "Fetched counts does not include collection " + collection + "; syncing as usual."); - return false; - } - - Logger.info(LOG_TAG, "First sync for " + collection + ": " + c + " items."); - if (c > fetchLimit) { - Logger.warn(LOG_TAG, "Too many items to sync safely. Skipping."); - return true; - } - } - return super.shouldSkip(); - } - } -}
--- a/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestServer11Repository.java +++ b/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestServer11Repository.java @@ -22,27 +22,16 @@ public class TestServer11Repository { protected final InfoCollections infoCollections = new InfoCollections(); protected final InfoConfiguration infoConfiguration = new InfoConfiguration(); public static void assertQueryEquals(String expected, URI u) { Assert.assertEquals(expected, u.getRawQuery()); } - @SuppressWarnings("static-method") - @Test - public void testCollectionURIFull() throws URISyntaxException { - Server11Repository r = new Server11Repository(COLLECTION, COLLECTION_URL, null, infoCollections, infoConfiguration); - assertQueryEquals("full=1&newer=5000.000", r.collectionURI(true, 5000000L, -1, null, null, null)); - assertQueryEquals("newer=1230.000", r.collectionURI(false, 1230000L, -1, null, null, null)); - assertQueryEquals("newer=5000.000&limit=10", r.collectionURI(false, 5000000L, 10, null, null, null)); - assertQueryEquals("full=1&newer=5000.000&sort=index", r.collectionURI(true, 5000000L, 0, "index", null, null)); - assertQueryEquals("full=1&ids=123,abc", r.collectionURI(true, -1L, -1, null, "123,abc", null)); - } - @Test public void testCollectionURI() throws URISyntaxException { Server11Repository noTrailingSlash = new Server11Repository(COLLECTION, COLLECTION_URL, null, infoCollections, infoConfiguration); Server11Repository trailingSlash = new Server11Repository(COLLECTION, COLLECTION_URL + "/", null, infoCollections, infoConfiguration); Assert.assertEquals("http://foo.com/1.1/n6ec3u5bee3tixzp2asys7bs6fve4jfw/storage/bookmarks", noTrailingSlash.collectionURI().toASCIIString()); Assert.assertEquals("http://foo.com/1.1/n6ec3u5bee3tixzp2asys7bs6fve4jfw/storage/bookmarks", trailingSlash.collectionURI().toASCIIString()); } }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestServer11RepositorySession.java +++ b/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestServer11RepositorySession.java @@ -170,67 +170,9 @@ public class TestServer11RepositorySessi return; } }; Exception e = doSynchronize(server); assertNotNull(e); assertEquals(StoreFailedException.class, e.getClass()); } - - @Test - public void testConstraints() throws Exception { - MockServer server = new MockServer() { - @Override - public void handle(Request request, Response response) { - if (request.getMethod().equals("GET")) { - if (request.getPath().getPath().endsWith("/info/collection_counts")) { - this.handle(request, response, 200, "{\"bookmarks\": 5001}"); - } - } - this.handle(request, response, 400, "NOOOO"); - } - }; - final JSONRecordFetcher countsFetcher = new JSONRecordFetcher(LOCAL_COUNTS_URL, getAuthHeaderProvider()); - String collection = "bookmarks"; - final SafeConstrainedServer11Repository remote = new SafeConstrainedServer11Repository(collection, - getCollectionURL(collection), - getAuthHeaderProvider(), - infoCollections, - infoConfiguration, - 5000, 5000, "sortindex", countsFetcher); - - data.startHTTPServer(server); - final AtomicBoolean out = new AtomicBoolean(false); - - // Verify that shouldSkip returns true due to a fetch of too large counts, - // rather than due to a timeout failure waiting to fetch counts. - try { - WaitHelper.getTestWaiter().performWait( - SHORT_TIMEOUT, - new Runnable() { - @Override - public void run() { - remote.createSession(new RepositorySessionCreationDelegate() { - @Override - public void onSessionCreated(RepositorySession session) { - out.set(session.shouldSkip()); - WaitHelper.getTestWaiter().performNotify(); - } - - @Override - public void onSessionCreateFailed(Exception ex) { - WaitHelper.getTestWaiter().performNotify(ex); - } - - @Override - public RepositorySessionCreationDelegate deferredCreationDelegate() { - return this; - } - }, null); - } - }); - assertTrue(out.get()); - } finally { - data.stopHTTPServer(); - } - } }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderDelegateTest.java +++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderDelegateTest.java @@ -1,55 +1,65 @@ /* 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.downloaders; +import android.net.Uri; +import android.os.SystemClock; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mozilla.gecko.background.testhelpers.TestRunner; import org.mozilla.gecko.sync.CryptoRecord; import org.mozilla.gecko.sync.HTTPFailureException; import org.mozilla.gecko.sync.InfoCollections; import org.mozilla.gecko.sync.InfoConfiguration; import org.mozilla.gecko.sync.net.SyncResponse; import org.mozilla.gecko.sync.net.SyncStorageCollectionRequest; import org.mozilla.gecko.sync.net.SyncStorageResponse; +import org.mozilla.gecko.sync.repositories.RepositorySession; +import org.mozilla.gecko.sync.repositories.Server11RepositorySession; import org.mozilla.gecko.sync.repositories.Server11Repository; -import org.mozilla.gecko.sync.repositories.Server11RepositorySession; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate; import org.mozilla.gecko.sync.repositories.domain.Record; import java.net.URI; import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; import ch.boye.httpclientandroidlib.ProtocolVersion; import ch.boye.httpclientandroidlib.client.ClientProtocolException; import ch.boye.httpclientandroidlib.message.BasicHttpResponse; import ch.boye.httpclientandroidlib.message.BasicStatusLine; import static org.junit.Assert.*; @RunWith(TestRunner.class) public class BatchingDownloaderDelegateTest { - private Server11Repository server11Repository; private Server11RepositorySession repositorySession; private MockDownloader mockDownloader; private String DEFAULT_COLLECTION_URL = "http://dummy.url/"; class MockDownloader extends BatchingDownloader { public boolean isSuccess = false; public boolean isFetched = false; public boolean isFailure = false; public Exception ex; - public MockDownloader(Server11Repository repository, Server11RepositorySession repositorySession) { - super(repository, repositorySession); + public MockDownloader(RepositorySession repositorySession) { + super( + null, + Uri.EMPTY, + SystemClock.elapsedRealtime() + TimeUnit.MINUTES.toMillis(30), + true, + repositorySession + ); } @Override public void onFetchCompleted(SyncStorageResponse response, final RepositorySessionFetchRecordsDelegate fetchRecordsDelegate, final SyncStorageCollectionRequest request, long l, long newerTimestamp, boolean full, String sort, String ids) { this.isSuccess = true; @@ -67,17 +77,17 @@ public class BatchingDownloaderDelegateT public void onFetchedRecord(CryptoRecord record, RepositorySessionFetchRecordsDelegate fetchRecordsDelegate) { this.isFetched = true; } } class SimpleSessionFetchRecordsDelegate implements RepositorySessionFetchRecordsDelegate { @Override - public void onFetchFailed(Exception ex, Record record) { + public void onFetchFailed(Exception ex) { } @Override public void onFetchedRecord(Record record) { } @@ -94,29 +104,36 @@ public class BatchingDownloaderDelegateT @Override public RepositorySessionFetchRecordsDelegate deferredFetchDelegate(ExecutorService executor) { return null; } } @Before public void setUp() throws Exception { - server11Repository = new Server11Repository( + repositorySession = new Server11RepositorySession(new Server11Repository( "dummyCollection", + SystemClock.elapsedRealtime() + TimeUnit.MINUTES.toMillis(30), DEFAULT_COLLECTION_URL, null, new InfoCollections(), - new InfoConfiguration()); - repositorySession = new Server11RepositorySession(server11Repository); - mockDownloader = new MockDownloader(server11Repository, repositorySession); + new InfoConfiguration()) + ); + mockDownloader = new MockDownloader(repositorySession); } @Test public void testIfUnmodifiedSince() throws Exception { - BatchingDownloader downloader = new BatchingDownloader(server11Repository, repositorySession); + BatchingDownloader downloader = new BatchingDownloader( + null, + Uri.EMPTY, + SystemClock.elapsedRealtime() + TimeUnit.MINUTES.toMillis(30), + true, + repositorySession + ); RepositorySessionFetchRecordsDelegate delegate = new SimpleSessionFetchRecordsDelegate(); BatchingDownloaderDelegate downloaderDelegate = new BatchingDownloaderDelegate(downloader, delegate, new SyncStorageCollectionRequest(new URI(DEFAULT_COLLECTION_URL)), 0, 0, true, null, null); String lastModified = "12345678"; SyncStorageResponse response = makeSyncStorageResponse(200, lastModified); downloaderDelegate.handleRequestSuccess(response); assertEquals(lastModified, downloaderDelegate.ifUnmodifiedSince()); }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderTest.java +++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloaderTest.java @@ -464,34 +464,16 @@ public class BatchingDownloaderTest { assertNotEquals(lmHeader, mockDownloader.getLastModified()); assertTrue(mockDownloader.abort); assertFalse(sessionFetchRecordsDelegate.isSuccess); assertFalse(sessionFetchRecordsDelegate.isFetched); assertTrue(sessionFetchRecordsDelegate.isFailure); } @Test - public void testFailureMissingLMMultiBatch() throws Exception { - assertNull(mockDownloader.getLastModified()); - - String offsetHeader = "100"; - SyncStorageResponse response = makeSyncStorageResponse(200, null, offsetHeader, null); - SyncStorageCollectionRequest request = new SyncStorageCollectionRequest(new URI("http://dummy.url")); - mockDownloader.onFetchCompleted(response, sessionFetchRecordsDelegate, request, DEFAULT_NEWER, - 1, true, DEFAULT_SORT, DEFAULT_IDS); - - // Last modified header somehow missing from response. - assertNull(null, mockDownloader.getLastModified()); - assertTrue(mockDownloader.abort); - assertFalse(sessionFetchRecordsDelegate.isSuccess); - assertFalse(sessionFetchRecordsDelegate.isFetched); - assertTrue(sessionFetchRecordsDelegate.isFailure); - } - - @Test public void testFailureException() throws Exception { Exception ex = new IllegalStateException(); SyncStorageCollectionRequest request = new SyncStorageCollectionRequest(new URI("http://dummy.url")); mockDownloader.onFetchFailed(ex, sessionFetchRecordsDelegate, request); assertFalse(sessionFetchRecordsDelegate.isSuccess); assertFalse(sessionFetchRecordsDelegate.isFetched); assertTrue(sessionFetchRecordsDelegate.isFailure);
deleted file mode 100644 --- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/test/TestSafeConstrainedServer11Repository.java +++ /dev/null @@ -1,144 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -package org.mozilla.gecko.sync.repositories.test; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mozilla.android.sync.test.helpers.HTTPServerTestHelper; -import org.mozilla.android.sync.test.helpers.MockServer; -import org.mozilla.gecko.background.testhelpers.TestRunner; -import org.mozilla.gecko.background.testhelpers.WaitHelper; -import org.mozilla.gecko.sync.InfoCollections; -import org.mozilla.gecko.sync.InfoConfiguration; -import org.mozilla.gecko.sync.JSONRecordFetcher; -import org.mozilla.gecko.sync.net.AuthHeaderProvider; -import org.mozilla.gecko.sync.repositories.RepositorySession; -import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate; -import org.mozilla.gecko.sync.stage.SafeConstrainedServer11Repository; -import org.simpleframework.http.Request; -import org.simpleframework.http.Response; - -import java.net.URISyntaxException; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; - -@RunWith(TestRunner.class) -public class TestSafeConstrainedServer11Repository { - private static final int TEST_PORT = HTTPServerTestHelper.getTestPort(); - private static final String TEST_SERVER = "http://localhost:" + TEST_PORT + "/"; - private static final String TEST_USERNAME = "c6o7dvmr2c4ud2fyv6woz2u4zi22bcyd"; - private static final String TEST_BASE_PATH = "/1.1/" + TEST_USERNAME + "/"; - - public AuthHeaderProvider getAuthHeaderProvider() { - return null; - } - - protected final InfoCollections infoCollections = new InfoCollections(); - protected final InfoConfiguration infoConfiguration = new InfoConfiguration(); - - private class CountsMockServer extends MockServer { - public final AtomicInteger count = new AtomicInteger(0); - public final AtomicBoolean error = new AtomicBoolean(false); - - @Override - public void handle(Request request, Response response) { - final String path = request.getPath().getPath(); - if (error.get()) { - super.handle(request, response, 503, "Unavailable."); - return; - } - - if (!path.startsWith(TEST_BASE_PATH)) { - super.handle(request, response, 404, "Not Found"); - return; - } - - if (path.endsWith("info/collections")) { - super.handle(request, response, 200, "{\"rotary\": 123456789}"); - return; - } - - if (path.endsWith("info/collection_counts")) { - super.handle(request, response, 200, "{\"rotary\": " + count.get() + "}"); - return; - } - - super.handle(request, response); - } - } - - /** - * Ensure that a {@link SafeConstrainedServer11Repository} will advise - * skipping if the reported collection counts are higher than its limit. - */ - @Test - public void testShouldSkip() throws URISyntaxException { - final HTTPServerTestHelper data = new HTTPServerTestHelper(); - final CountsMockServer server = new CountsMockServer(); - data.startHTTPServer(server); - - try { - String countsURL = TEST_SERVER + TEST_BASE_PATH + "info/collection_counts"; - JSONRecordFetcher countFetcher = new JSONRecordFetcher(countsURL, getAuthHeaderProvider()); - String sort = "sortindex"; - String collection = "rotary"; - - final int TEST_LIMIT = 1000; - final SafeConstrainedServer11Repository repo = new SafeConstrainedServer11Repository( - collection, getCollectionURL(collection), null, infoCollections, infoConfiguration, - TEST_LIMIT, TEST_LIMIT, sort, countFetcher); - - final AtomicBoolean shouldSkipLots = new AtomicBoolean(false); - final AtomicBoolean shouldSkipFew = new AtomicBoolean(true); - final AtomicBoolean shouldSkip503 = new AtomicBoolean (false); - - WaitHelper.getTestWaiter().performWait(2000, new Runnable() { - @Override - public void run() { - repo.createSession(new RepositorySessionCreationDelegate() { - @Override - public void onSessionCreated(RepositorySession session) { - // Try with too many items. - server.count.set(TEST_LIMIT + 1); - shouldSkipLots.set(session.shouldSkip()); - - // … and few enough that we should sync. - server.count.set(TEST_LIMIT - 1); - shouldSkipFew.set(session.shouldSkip()); - - // Now try with an error response. We advise skipping if we can't - // fetch counts, because we'll try again later. - server.error.set(true); - shouldSkip503.set(session.shouldSkip()); - - WaitHelper.getTestWaiter().performNotify(); - } - - @Override - public void onSessionCreateFailed(Exception ex) { - WaitHelper.getTestWaiter().performNotify(ex); - } - - @Override - public RepositorySessionCreationDelegate deferredCreationDelegate() { - return this; - } - }, null); - } - }); - - Assert.assertTrue(shouldSkipLots.get()); - Assert.assertFalse(shouldSkipFew.get()); - Assert.assertTrue(shouldSkip503.get()); - - } finally { - data.stopHTTPServer(); - } - } - - protected String getCollectionURL(String collection) { - return TEST_BASE_PATH + "/storage/" + collection; - } -}