зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1140810 - Upload material (non-status) Reading List modifications. r=rnewman
========575d80fddbAuthor: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 16:35:26 2015 -0700 Bug 1140810 - Part 2: Upload material (non-status) modifications. ========a86e734ef1Author: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 10:56:07 2015 -0700 Bug 1140810 - Part 1: Add storage test for material (non-status) modifications. ========2259378d08Author: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 13:33:13 2015 -0700 Bug 1140810 - Part 0: Add and use HTTP PATCH. ========0222d53d98Author: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 14:49:19 2015 -0700 Bug 1140810 - Pre: Don't fail in status upload when there are no failures. ========7f2feede3bAuthor: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 14:53:45 2015 -0700 Bug 1140810 - Pre: convert 4 spaces to 2 spaces. ========49e80d271eAuthor: Nick Alexander <nalexander@mozilla.com> Date: Wed Mar 25 10:40:26 2015 -0700 Bug 1140810 - Pre: Fix whitespace.
This commit is contained in:
Nick Alexander
Родитель
cef3c3aa64
Коммит
9c2b6b157d
|
|
@ -266,15 +266,17 @@ public class LocalReadingListStorage implements ReadingListStorage {
|
|||
ReadingListItems.RESOLVED_TITLE,
|
||||
ReadingListItems.RESOLVED_URL,
|
||||
ReadingListItems.EXCERPT,
|
||||
// TODO: ReadingListItems.IS_ARTICLE,
|
||||
// TODO: ReadingListItems.WORD_COUNT,
|
||||
};
|
||||
|
||||
|
||||
try {
|
||||
return client.query(URI_WITHOUT_DELETED, projection, selection, null, null);
|
||||
} catch (RemoteException e) {
|
||||
throw new IllegalStateException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Cursor getModified() {
|
||||
final String selection = ReadingListItems.SYNC_STATUS + " = " + ReadingListItems.SYNC_STATUS_MODIFIED;
|
||||
|
|
|
|||
|
|
@ -482,7 +482,7 @@ public class ReadingListClient {
|
|||
if (ReadingListConstants.DEBUG) {
|
||||
Logger.info(LOG_TAG, "Patching record " + guid + ": " + body.toJSONString());
|
||||
}
|
||||
r.post(body);
|
||||
r.patch(body);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ public class ReadingListConstants {
|
|||
public static final String GLOBAL_LOG_TAG = "FxReadingList";
|
||||
public static final String USER_AGENT = "Firefox-Android-FxReader/" + AppConstants.MOZ_APP_VERSION + " (" + AppConstants.MOZ_APP_DISPLAYNAME + ")";
|
||||
public static final String DEFAULT_DEV_ENDPOINT = "https://readinglist.dev.mozaws.net/v1/";
|
||||
public static final String DEFAULT_PROD_ENDPOINT = null; // TODO
|
||||
public static final String DEFAULT_PROD_ENDPOINT = "https://readinglist.services.mozilla.com/v1/";
|
||||
|
||||
public static final String OAUTH_ENDPOINT_PROD = "https://oauth.accounts.firefox.com/v1";
|
||||
|
||||
|
|
|
|||
|
|
@ -248,11 +248,10 @@ public class ReadingListSynchronizer {
|
|||
}
|
||||
|
||||
if (failures == 0) {
|
||||
try {
|
||||
next.next();
|
||||
} catch (Exception e) {
|
||||
}
|
||||
next.next();
|
||||
return;
|
||||
}
|
||||
|
||||
next.fail();
|
||||
}
|
||||
}
|
||||
|
|
@ -305,6 +304,149 @@ public class ReadingListSynchronizer {
|
|||
}
|
||||
}
|
||||
|
||||
private static class ModifiedUploadDelegate implements ReadingListRecordUploadDelegate {
|
||||
private final ReadingListChangeAccumulator acc;
|
||||
|
||||
public volatile int failures = 0;
|
||||
private final StageDelegate next;
|
||||
|
||||
ModifiedUploadDelegate(ReadingListChangeAccumulator acc, StageDelegate next) {
|
||||
this.acc = acc;
|
||||
this.next = next;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onInvalidUpload(ClientReadingListRecord up,
|
||||
ReadingListResponse response) {
|
||||
recordFailed(up);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onConflict(ClientReadingListRecord up,
|
||||
ReadingListResponse response) {
|
||||
// This can happen for a material change.
|
||||
failures++;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onSuccess(ClientReadingListRecord up,
|
||||
ReadingListRecordResponse response,
|
||||
ServerReadingListRecord down) {
|
||||
if (!TextUtils.equals(up.getGUID(), down.getGUID())) {
|
||||
// Uh oh!
|
||||
// This should never occur. We should get an onConflict instead,
|
||||
// so this would imply a server bug, or something like a truncated
|
||||
// over-long GUID string.
|
||||
//
|
||||
// Should we wish to recover from this case, probably the right approach
|
||||
// is to ensure that the GUID is overwritten locally (given that we know
|
||||
// the numeric ID).
|
||||
}
|
||||
|
||||
// We could upload our material changes but get back additional status
|
||||
// changes from the server. Apply them.
|
||||
acc.addChangedRecord(up.givenServerRecord(down));
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onBadRequest(ClientReadingListRecord up, MozResponse response) {
|
||||
recordFailed(up);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onFailure(ClientReadingListRecord up, Exception ex) {
|
||||
recordFailed(up);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onFailure(ClientReadingListRecord up, MozResponse response) {
|
||||
// Since we download and apply remote changes before uploading local changes, the conflict
|
||||
// window is very small. We should essentially never see true conflicts here.
|
||||
if (response.getStatusCode() == 404) {
|
||||
// We shouldn't see a 404; we should see a record with deleted=true when
|
||||
// we fetch remote changes.
|
||||
Logger.warn(LOG_TAG, "Ignoring 404 response patching record with guid: " + up.getGUID());
|
||||
} else if (response.getStatusCode() == 409) {
|
||||
// A 409 indicates that resolved_url has collided with an existing
|
||||
// record. Not much to be done here.
|
||||
Logger.info(LOG_TAG, "409 response seen; deleting record with guid: " + up.getGUID());
|
||||
acc.addDeletion(up);
|
||||
} else {
|
||||
// We should never see a 412 since we race to upload our changes (and
|
||||
// accept whatever the server gives us back).
|
||||
recordFailed(up);
|
||||
}
|
||||
}
|
||||
|
||||
private void recordFailed(ClientReadingListRecord up) {
|
||||
++failures;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onBatchDone() {
|
||||
try {
|
||||
acc.finish();
|
||||
} catch (Exception e) {
|
||||
next.fail(e);
|
||||
return;
|
||||
}
|
||||
|
||||
if (failures == 0) {
|
||||
next.next();
|
||||
return;
|
||||
}
|
||||
|
||||
next.fail();
|
||||
}
|
||||
}
|
||||
|
||||
private Queue<ClientReadingListRecord> collectModifiedFromCursor(final Cursor cursor) {
|
||||
try {
|
||||
final Queue<ClientReadingListRecord> toUpload = new LinkedList<>();
|
||||
|
||||
final int columnGUID = cursor.getColumnIndexOrThrow(ReadingListItems.GUID);
|
||||
final int columnExcerpt = cursor.getColumnIndexOrThrow(ReadingListItems.EXCERPT);
|
||||
final int columnResolvedURL = cursor.getColumnIndexOrThrow(ReadingListItems.RESOLVED_URL);
|
||||
final int columnResolvedTitle = cursor.getColumnIndexOrThrow(ReadingListItems.RESOLVED_TITLE);
|
||||
// TODO: final int columnIsArticle = cursor.getColumnIndexOrThrow(ReadingListItems.IS_ARTICLE);
|
||||
// TODO: final int columnWordCount = cursor.getColumnIndexOrThrow(ReadingListItems.WORD_COUNT);
|
||||
|
||||
while (cursor.moveToNext()) {
|
||||
final String guid = cursor.getString(columnGUID);
|
||||
if (guid == null) {
|
||||
// Nothing we can do here, but this should never happen: we should
|
||||
// have uploaded this record as new before trying to upload a
|
||||
// material modification!
|
||||
continue;
|
||||
}
|
||||
|
||||
final ExtendedJSONObject o = new ExtendedJSONObject();
|
||||
o.put("id", guid);
|
||||
final String excerpt = cursor.getString(columnExcerpt); // Can be NULL.
|
||||
final String resolvedURL = cursor.getString(columnResolvedURL); // Can be NULL.
|
||||
final String resolvedTitle = cursor.getString(columnResolvedTitle); // Can be NULL.
|
||||
if (excerpt == null && resolvedURL == null && resolvedTitle == null) {
|
||||
// Nothing material to upload, so skip this record.
|
||||
continue;
|
||||
}
|
||||
o.put("excerpt", excerpt);
|
||||
o.put("resolved_url", resolvedURL);
|
||||
o.put("resolved_title", resolvedTitle);
|
||||
// TODO: o.put("is_article", cursor.getInt(columnIsArticle) == 1);
|
||||
// TODO: o.put("word_count", cursor.getInt(columnWordCount));
|
||||
|
||||
final ClientMetadata cm = null;
|
||||
final ServerMetadata sm = new ServerMetadata(guid, -1L);
|
||||
final ClientReadingListRecord record = new ClientReadingListRecord(sm, cm, o);
|
||||
toUpload.add(record);
|
||||
}
|
||||
|
||||
return toUpload;
|
||||
} finally {
|
||||
cursor.close();
|
||||
}
|
||||
}
|
||||
|
||||
private Queue<ClientReadingListRecord> accumulateNewItems(Cursor cursor) {
|
||||
try {
|
||||
final Queue<ClientReadingListRecord> toUpload = new LinkedList<>();
|
||||
|
|
@ -335,8 +477,11 @@ public class ReadingListSynchronizer {
|
|||
|
||||
// Nothing to do.
|
||||
if (toUpload.isEmpty()) {
|
||||
Logger.debug(LOG_TAG, "No new unread changes to upload. Skipping.");
|
||||
delegate.next();
|
||||
return;
|
||||
} else {
|
||||
Logger.debug(LOG_TAG, "Uploading " + toUpload.size() + " new unread changes.");
|
||||
}
|
||||
|
||||
// Upload each record. This looks like batching, but it's really chained serial requests.
|
||||
|
|
@ -368,6 +513,8 @@ public class ReadingListSynchronizer {
|
|||
Logger.debug(LOG_TAG, "No new items to upload. Skipping.");
|
||||
delegate.next();
|
||||
return;
|
||||
} else {
|
||||
Logger.debug(LOG_TAG, "Uploading " + toUpload.size() + " new items.");
|
||||
}
|
||||
|
||||
final ReadingListChangeAccumulator acc = this.local.getChangeAccumulator();
|
||||
|
|
@ -420,9 +567,79 @@ public class ReadingListSynchronizer {
|
|||
}
|
||||
}
|
||||
|
||||
private void uploadModified(final StageDelegate delegate) {
|
||||
// TODO
|
||||
delegate.next();
|
||||
protected void uploadModified(final StageDelegate delegate) {
|
||||
try {
|
||||
// This looks strange because modified includes material changes and
|
||||
// status changes, but this is called after status changes have been
|
||||
// uploaded and removed from local storage. So what's left should be
|
||||
// material changes. Even so, it should be safe to upload status changes
|
||||
// here.
|
||||
final Cursor cursor = this.local.getModified();
|
||||
|
||||
if (cursor == null) {
|
||||
delegate.fail(new RuntimeException("Unable to get modified item cursor."));
|
||||
return;
|
||||
}
|
||||
|
||||
final Queue<ClientReadingListRecord> toUpload = collectModifiedFromCursor(cursor);
|
||||
|
||||
// Nothing to do.
|
||||
if (toUpload.isEmpty()) {
|
||||
Logger.debug(LOG_TAG, "No modified items to upload. Skipping.");
|
||||
delegate.next();
|
||||
return;
|
||||
} else {
|
||||
Logger.debug(LOG_TAG, "Uploading " + toUpload.size() + " modified items.");
|
||||
}
|
||||
|
||||
final ReadingListChangeAccumulator acc = this.local.getChangeAccumulator();
|
||||
final ModifiedUploadDelegate uploadDelegate = new ModifiedUploadDelegate(acc, new StageDelegate() {
|
||||
private boolean tryFlushChanges() {
|
||||
Logger.debug(LOG_TAG, "Flushing post-upload changes.");
|
||||
try {
|
||||
acc.finish();
|
||||
return true;
|
||||
} catch (Exception e) {
|
||||
Logger.warn(LOG_TAG, "Flushing changes failed! This sync went wrong.", e);
|
||||
delegate.fail(e);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void next() {
|
||||
Logger.debug(LOG_TAG, "Modified items uploaded successfully.");
|
||||
|
||||
if (tryFlushChanges()) {
|
||||
delegate.next();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void fail() {
|
||||
Logger.warn(LOG_TAG, "Couldn't upload modified items.");
|
||||
if (tryFlushChanges()) {
|
||||
delegate.fail();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void fail(Exception e) {
|
||||
Logger.warn(LOG_TAG, "Couldn't upload modified items.", e);
|
||||
if (tryFlushChanges()) {
|
||||
delegate.fail(e);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// Handle 201 for success, 400 for invalid, 303 for redirect.
|
||||
// TODO: 200 == "was already on the server, we didn't touch it, here it is."
|
||||
// ... we need to apply it locally.
|
||||
this.remote.patch(toUpload, executor, uploadDelegate);
|
||||
} catch (Exception e) {
|
||||
delegate.fail(e);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
private void downloadIncoming(final long since, final StageDelegate delegate) {
|
||||
|
|
|
|||
|
|
@ -28,4 +28,8 @@ public class ServerReadingListRecord extends ReadingListRecord {
|
|||
public String getAddedBy() {
|
||||
return this.fields.getString("added_by");
|
||||
}
|
||||
}
|
||||
|
||||
public String getExcerpt() {
|
||||
return this.fields.getString("excerpt");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ import ch.boye.httpclientandroidlib.client.AuthCache;
|
|||
import ch.boye.httpclientandroidlib.client.ClientProtocolException;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpDelete;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpGet;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpPatch;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpPost;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpPut;
|
||||
import ch.boye.httpclientandroidlib.client.methods.HttpRequestBase;
|
||||
|
|
@ -341,6 +342,14 @@ public class BaseResource implements Resource {
|
|||
this.go(request);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void patch(HttpEntity body) {
|
||||
Logger.debug(LOG_TAG, "HTTP PATCH " + this.uri.toASCIIString());
|
||||
HttpPatch request = new HttpPatch(this.uri);
|
||||
request.setEntity(body);
|
||||
this.go(request);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void put(HttpEntity body) {
|
||||
Logger.debug(LOG_TAG, "HTTP PUT " + this.uri.toASCIIString());
|
||||
|
|
@ -463,4 +472,16 @@ public class BaseResource implements Resource {
|
|||
public void post(JSONObject jsonObject) throws UnsupportedEncodingException {
|
||||
post(jsonEntity(jsonObject));
|
||||
}
|
||||
|
||||
public void patch(JSONArray jsonArray) throws UnsupportedEncodingException {
|
||||
patch(jsonEntity(jsonArray));
|
||||
}
|
||||
|
||||
public void patch(ExtendedJSONObject o) {
|
||||
patch(jsonEntity(o));
|
||||
}
|
||||
|
||||
public void patch(JSONObject jsonObject) throws UnsupportedEncodingException {
|
||||
patch(jsonEntity(jsonObject));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,5 +15,6 @@ public interface Resource {
|
|||
public abstract void get();
|
||||
public abstract void delete();
|
||||
public abstract void post(HttpEntity body);
|
||||
public abstract void patch(HttpEntity body);
|
||||
public abstract void put(HttpEntity body);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -190,6 +190,11 @@ public class SyncStorageRequest implements Resource {
|
|||
this.resource.post(body);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void patch(HttpEntity body) {
|
||||
this.resource.patch(body);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void put(HttpEntity body) {
|
||||
this.resource.put(body);
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче