Bug 729248 - Smarter upload of our clients record. r=rnewman

This commit is contained in:
Marina Samuel 2012-03-15 01:25:14 -07:00
Родитель 336ba9be8b
Коммит db539089b3
8 изменённых файлов: 132 добавлений и 32 удалений

Просмотреть файл

@ -190,6 +190,8 @@ public class SyncConfiguration implements CredentialsSource {
public String prefsPath; public String prefsPath;
public PrefsSource prefsSource; public PrefsSource prefsSource;
public static final String CLIENT_RECORD_TIMESTAMP = "serverClientRecordTimestamp";
/** /**
* Create a new SyncConfiguration instance. Pass in a PrefsSource to * Create a new SyncConfiguration instance. Pass in a PrefsSource to
* provide access to preferences. * provide access to preferences.
@ -369,4 +371,12 @@ public class SyncConfiguration implements CredentialsSource {
public Editor getEditor() { public Editor getEditor() {
return this.getPrefs().edit(); return this.getPrefs().edit();
} }
public void persistServerClientRecordTimestamp(long timestamp) {
getEditor().putLong(SyncConfiguration.CLIENT_RECORD_TIMESTAMP, timestamp).commit();
}
public long getPersistedServerClientRecordTimestamp() {
return getPrefs().getLong(SyncConfiguration.CLIENT_RECORD_TIMESTAMP, 0);
}
} }

Просмотреть файл

@ -9,4 +9,5 @@ public interface ClientsDataDelegate {
public String getClientName(); public String getClientName();
public void setClientsCount(int clientsCount); public void setClientsCount(int clientsCount);
public int getClientsCount(); public int getClientsCount();
public boolean isLocalGUID(String guid);
} }

Просмотреть файл

@ -11,13 +11,13 @@ import java.util.Map;
import org.mozilla.gecko.sync.repositories.NullCursorException; import org.mozilla.gecko.sync.repositories.NullCursorException;
import org.mozilla.gecko.sync.repositories.domain.ClientRecord; import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
import org.mozilla.gecko.sync.setup.Constants;
import android.content.Context; import android.content.Context;
import android.database.Cursor; import android.database.Cursor;
public class ClientsDatabaseAccessor { public class ClientsDatabaseAccessor {
public static final String PROFILE_ID = "default"; // Generic profile id for now, until multiple profiles are implemented.
public static final String LOG_TAG = "ClientsDatabaseAccessor"; public static final String LOG_TAG = "ClientsDatabaseAccessor";
private ClientsDatabase db; private ClientsDatabase db;
@ -102,7 +102,7 @@ public class ClientsDatabaseAccessor {
} }
private String getProfileId() { private String getProfileId() {
return Constants.PROFILE_ID; return ClientsDatabaseAccessor.PROFILE_ID;
} }
public void wipe() { public void wipe() {

Просмотреть файл

@ -4,14 +4,23 @@
package org.mozilla.gecko.sync.repositories.domain; package org.mozilla.gecko.sync.repositories.domain;
import org.json.simple.JSONArray;
import org.mozilla.gecko.sync.ExtendedJSONObject; import org.mozilla.gecko.sync.ExtendedJSONObject;
import org.mozilla.gecko.sync.Logger;
import org.mozilla.gecko.sync.NonArrayJSONException;
import org.mozilla.gecko.sync.Utils; import org.mozilla.gecko.sync.Utils;
import org.mozilla.gecko.sync.repositories.android.RepoUtils; import org.mozilla.gecko.sync.repositories.android.RepoUtils;
import org.mozilla.gecko.sync.setup.Constants;
public class ClientRecord extends Record { public class ClientRecord extends Record {
private static final String LOG_TAG = "ClientRecord";
public static final String COLLECTION_NAME = "clients"; public static final String CLIENT_TYPE = "mobile";
public static final String COLLECTION_NAME = "clients";
public static final String DEFAULT_CLIENT_NAME = "Default Name";
public String name = ClientRecord.DEFAULT_CLIENT_NAME;
public String type = ClientRecord.CLIENT_TYPE;
public JSONArray commands;
public ClientRecord(String guid, String collection, long lastModified, boolean deleted) { public ClientRecord(String guid, String collection, long lastModified, boolean deleted) {
super(guid, collection, lastModified, deleted); super(guid, collection, lastModified, deleted);
@ -33,13 +42,17 @@ public class ClientRecord extends Record {
this(Utils.generateGuid(), COLLECTION_NAME, 0, false); this(Utils.generateGuid(), COLLECTION_NAME, 0, false);
} }
public String name = Constants.DEFAULT_CLIENT_NAME;
public String type = Constants.CLIENT_TYPE;
@Override @Override
protected void initFromPayload(ExtendedJSONObject payload) { protected void initFromPayload(ExtendedJSONObject payload) {
this.name = (String) payload.get("name"); this.name = (String) payload.get("name");
this.type = (String) payload.get("type"); this.type = (String) payload.get("type");
try {
commands = payload.getArray("commands");
} catch (NonArrayJSONException e) {
Logger.debug(LOG_TAG, "Got non-array commands in client record " + guid, e);
commands = null;
}
} }
@Override @Override

Просмотреть файл

@ -8,7 +8,6 @@ import org.mozilla.gecko.sync.CryptoRecord;
import org.mozilla.gecko.sync.repositories.RecordFactory; import org.mozilla.gecko.sync.repositories.RecordFactory;
public class ClientRecordFactory extends RecordFactory { public class ClientRecordFactory extends RecordFactory {
@Override @Override
public Record createRecord(Record record) { public Record createRecord(Record record) {
ClientRecord r = new ClientRecord(); ClientRecord r = new ClientRecord();

Просмотреть файл

@ -17,11 +17,6 @@ public class Constants {
public static final String CLIENT_NAME = "account.clientName"; public static final String CLIENT_NAME = "account.clientName";
public static final String NUM_CLIENTS = "account.numClients"; public static final String NUM_CLIENTS = "account.numClients";
// Constants for client records.
public static final String PROFILE_ID = "default"; // Generic profile id for now, until multiple profiles are implemented.
public static final String CLIENT_TYPE = "mobile";
public static final String DEFAULT_CLIENT_NAME = "Default Name";
// Constants for Activities. // Constants for Activities.
public static final String INTENT_EXTRA_IS_SETUP = "isSetup"; public static final String INTENT_EXTRA_IS_SETUP = "isSetup";
public static final String INTENT_EXTRA_IS_PAIR = "isPair"; public static final String INTENT_EXTRA_IS_PAIR = "isPair";

Просмотреть файл

@ -7,12 +7,15 @@ package org.mozilla.gecko.sync.stage;
import java.io.UnsupportedEncodingException; import java.io.UnsupportedEncodingException;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.util.concurrent.atomic.AtomicInteger;
import org.json.simple.JSONArray;
import org.mozilla.gecko.sync.CryptoRecord; import org.mozilla.gecko.sync.CryptoRecord;
import org.mozilla.gecko.sync.GlobalSession; import org.mozilla.gecko.sync.GlobalSession;
import org.mozilla.gecko.sync.HTTPFailureException; import org.mozilla.gecko.sync.HTTPFailureException;
import org.mozilla.gecko.sync.Logger; import org.mozilla.gecko.sync.Logger;
import org.mozilla.gecko.sync.NoCollectionKeysSetException; import org.mozilla.gecko.sync.NoCollectionKeysSetException;
import org.mozilla.gecko.sync.Utils;
import org.mozilla.gecko.sync.crypto.CryptoException; import org.mozilla.gecko.sync.crypto.CryptoException;
import org.mozilla.gecko.sync.crypto.KeyBundle; import org.mozilla.gecko.sync.crypto.KeyBundle;
import org.mozilla.gecko.sync.delegates.ClientsDataDelegate; import org.mozilla.gecko.sync.delegates.ClientsDataDelegate;
@ -27,9 +30,13 @@ import org.mozilla.gecko.sync.repositories.android.RepoUtils;
import org.mozilla.gecko.sync.repositories.domain.ClientRecord; import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
import org.mozilla.gecko.sync.repositories.domain.ClientRecordFactory; import org.mozilla.gecko.sync.repositories.domain.ClientRecordFactory;
import ch.boye.httpclientandroidlib.HttpStatus;
public class SyncClientsEngineStage implements GlobalSyncStage { public class SyncClientsEngineStage implements GlobalSyncStage {
protected static final String LOG_TAG = "SyncClientsEngineStage"; public static final String LOG_TAG = "SyncClientsEngineStage";
protected static final String COLLECTION_NAME = "clients"; public static final String COLLECTION_NAME = "clients";
public static final int CLIENTS_TTL_REFRESH = 604800000; // 7 days
public static final int MAX_UPLOAD_FAILURE_COUNT = 5;
protected GlobalSession session; protected GlobalSession session;
protected final ClientRecordFactory factory = new ClientRecordFactory(); protected final ClientRecordFactory factory = new ClientRecordFactory();
@ -37,8 +44,9 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
protected ClientDownloadDelegate clientDownloadDelegate; protected ClientDownloadDelegate clientDownloadDelegate;
protected ClientsDatabaseAccessor db; protected ClientsDatabaseAccessor db;
// Account/Profile info protected volatile boolean shouldWipe;
protected boolean shouldWipe; protected volatile boolean commandsProcessedShouldUpload;
protected final AtomicInteger uploadAttemptsCount = new AtomicInteger();
/** /**
* The following two delegates, ClientDownloadDelegate and ClientUploadDelegate * The following two delegates, ClientDownloadDelegate and ClientUploadDelegate
@ -53,6 +61,9 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
*/ */
public class ClientDownloadDelegate extends WBOCollectionRequestDelegate { public class ClientDownloadDelegate extends WBOCollectionRequestDelegate {
// We use this on each WBO, so lift it out.
final ClientsDataDelegate clientsDelegate = session.getClientsDelegate();
@Override @Override
public String credentials() { public String credentials() {
return session.credentials(); return session.credentials();
@ -67,15 +78,22 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
@Override @Override
public void handleRequestSuccess(SyncStorageResponse response) { public void handleRequestSuccess(SyncStorageResponse response) {
BaseResource.consumeEntity(response); // We don't need the response at all. BaseResource.consumeEntity(response); // We don't need the response at all.
final int clientsCount;
try { try {
clientUploadDelegate = new ClientUploadDelegate(); clientsCount = db.clientsCount();
session.getClientsDelegate().setClientsCount(db.clientsCount());
checkAndUpload();
} finally { } finally {
// Close the database to clear cached readableDatabase/writableDatabase // Close the database to clear cached readableDatabase/writableDatabase
// after we've completed our last transaction (db.store()). // after we've completed our last transaction (db.store()).
db.close(); db.close();
} }
Logger.debug(LOG_TAG, "Database contains " + clientsCount + " clients.");
Logger.debug(LOG_TAG, "Server response asserts " + response.weaveRecords() + " records.");
// TODO: persist the response timestamp to know whether to download next time (Bug 726055).
clientUploadDelegate = new ClientUploadDelegate();
clientsDelegate.setClientsCount(clientsCount);
checkAndUpload();
} }
@Override @Override
@ -106,6 +124,15 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
ClientRecord r; ClientRecord r;
try { try {
r = (ClientRecord) factory.createRecord(record.decrypt()); r = (ClientRecord) factory.createRecord(record.decrypt());
if (clientsDelegate.isLocalGUID(r.guid)) {
// Oh hey! Our record is on the server. This is the authoritative
// server timestamp, so let's hang on to it to decide whether we
// need to upload.
session.config.persistServerClientRecordTimestamp(r.lastModified);
// Process commands.
processCommands(r.commands);
}
RepoUtils.logClient(r); RepoUtils.logClient(r);
} catch (Exception e) { } catch (Exception e) {
session.abort(e, "Exception handling client WBO."); session.abort(e, "Exception handling client WBO.");
@ -135,21 +162,47 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
@Override @Override
public String ifUnmodifiedSince() { public String ifUnmodifiedSince() {
// TODO last client upload time? Long timestampInMilliseconds = session.config.getPersistedServerClientRecordTimestamp();
return null;
// It's the first upload so we don't care about X-If-Unmodified-Since.
if (timestampInMilliseconds == 0) {
return null;
}
return Utils.millisecondsToDecimalSecondsString(timestampInMilliseconds);
} }
@Override @Override
public void handleRequestSuccess(SyncStorageResponse response) { public void handleRequestSuccess(SyncStorageResponse response) {
Logger.debug(LOG_TAG, "Upload succeeded.");
commandsProcessedShouldUpload = false;
uploadAttemptsCount.set(0);
session.config.persistServerClientRecordTimestamp(response.normalizedWeaveTimestamp());
BaseResource.consumeEntity(response); BaseResource.consumeEntity(response);
session.advance(); session.advance();
} }
@Override @Override
public void handleRequestFailure(SyncStorageResponse response) { public void handleRequestFailure(SyncStorageResponse response) {
Logger.info(LOG_TAG, "Client upload failed. Aborting sync."); int statusCode = response.getStatusCode();
BaseResource.consumeEntity(response); // The exception thrown should need the response body.
session.abort(new HTTPFailureException(response), "Client upload failed."); // If upload failed because of `ifUnmodifiedSince` then there are new
// commands uploaded to our record. We must download and process them first.
if (!commandsProcessedShouldUpload ||
statusCode == HttpStatus.SC_PRECONDITION_FAILED ||
uploadAttemptsCount.incrementAndGet() > MAX_UPLOAD_FAILURE_COUNT) {
Logger.debug(LOG_TAG, "Client upload failed. Aborting sync.");
BaseResource.consumeEntity(response); // The exception thrown should need the response body.
session.abort(new HTTPFailureException(response), "Client upload failed.");
return;
}
Logger.trace(LOG_TAG, "Retrying upload…");
// Preconditions:
// commandsProcessedShouldUpload == true &&
// statusCode != 412 &&
// uploadAttemptCount < MAX_UPLOAD_FAILURE_COUNT
checkAndUpload();
} }
@Override @Override
@ -200,26 +253,49 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
return true; return true;
} }
// TODO: Bug 729248 - Smarter upload of client records.
protected boolean shouldUpload() { protected boolean shouldUpload() {
return true; if (commandsProcessedShouldUpload) {
return true;
}
long lastUpload = session.config.getPersistedServerClientRecordTimestamp(); // Defaults to 0.
if (lastUpload == 0) {
return true;
}
// Note the opportunity for clock drift problems here.
// TODO: if we track download times, we can use the timestamp of most
// recent download response instead of the current time.
long now = System.currentTimeMillis();
long age = now - lastUpload;
return age >= CLIENTS_TTL_REFRESH;
}
protected void processCommands(JSONArray commands) {
if (commands == null ||
commands.size() == 0) {
return;
}
commandsProcessedShouldUpload = true;
// TODO: Bug 715792 - Process commands here.
} }
protected void checkAndUpload() { protected void checkAndUpload() {
if (!shouldUpload()) { if (!shouldUpload()) {
Logger.trace(LOG_TAG, "Not uploading client record."); Logger.debug(LOG_TAG, "Not uploading client record.");
session.advance(); session.advance();
return; return;
} }
// Generate CryptoRecord from ClientRecord to upload. // Generate CryptoRecord from ClientRecord to upload.
String encryptionFailure = "Couldn't encrypt new client record."; final String encryptionFailure = "Couldn't encrypt new client record.";
ClientRecord localClient = newLocalClientRecord(session.getClientsDelegate()); final ClientRecord localClient = newLocalClientRecord(session.getClientsDelegate());
CryptoRecord cryptoRecord = localClient.getEnvelope();
try { try {
CryptoRecord cryptoRecord = localClient.getEnvelope();
cryptoRecord.keyBundle = clientUploadDelegate.keyBundle(); cryptoRecord.keyBundle = clientUploadDelegate.keyBundle();
cryptoRecord.encrypt(); cryptoRecord.encrypt();
this.wipeAndStore(localClient);
this.uploadClientRecord(cryptoRecord); this.uploadClientRecord(cryptoRecord);
} catch (UnsupportedEncodingException e) { } catch (UnsupportedEncodingException e) {
session.abort(e, encryptionFailure + " Unsupported encoding."); session.abort(e, encryptionFailure + " Unsupported encoding.");
@ -246,6 +322,7 @@ public class SyncClientsEngineStage implements GlobalSyncStage {
} }
protected void uploadClientRecord(CryptoRecord record) { protected void uploadClientRecord(CryptoRecord record) {
Logger.debug(LOG_TAG, "Uploading client record " + record.guid);
try { try {
URI putURI = session.config.wboURI(COLLECTION_NAME, record.guid); URI putURI = session.config.wboURI(COLLECTION_NAME, record.guid);

Просмотреть файл

@ -422,6 +422,11 @@ public class SyncAdapter extends AbstractThreadedSyncAdapter implements GlobalSe
Integer.toString(clientsCount)); Integer.toString(clientsCount));
} }
@Override
public boolean isLocalGUID(String guid) {
return getAccountGUID().equals(guid);
}
@Override @Override
public synchronized int getClientsCount() { public synchronized int getClientsCount() {
String clientsCount = mAccountManager.getUserData(localAccount, Constants.NUM_CLIENTS); String clientsCount = mAccountManager.getUserData(localAccount, Constants.NUM_CLIENTS);