Add support for non-optimistic updates (+ API versioning)

[khamidou]

Summary: This PR adds a new way to update API objects. Instead of updating API objects before pushing the changes to the backend server (aka optimistic updates), we now push the changes to the backend and wait for the sync engine to pick up the changes. This solves a lot of problems we've had previously with folder and labels, where our datastore and the remote server would get out of sync.

For example, in the case of a folder rename, PUTting changes to /folders/my_id will return the title of the original folder. The new name will be returned only once we've carried the folder rename operation. This is a breaking API change, which could be very confusing to our long-time API users.

To keep backward compatibility, I've introduced a very basic API versioning scheme, similar to what Github does with its API. API versions are dates. You can set the API version you'd like to use by setting the "X-Api-Version" header in your request. If you don't set this header, we'll default to the version set for your developer dashboard account (changes forthcoming).

This diff is pretty close to review but I need to take a final pass to make sure everything fits together.

Test Plan: Make a final manual test pass using IMAP and Exchange accounts.

Reviewers: spang, bengotow

[bengotow]

LGTM! Just added a few comments inline it'd be good to address.

It'd also be really nice if we could change the API response for non-optimistic PUTs and POSTs. I think it'll be confusing to users that they say unread: false, and we return a Thread with unread: true still. When we implemented this in K2, we made the PUT and POST requests return a task object, which you could query to see if the operation had completed. The task objects were also in the delta stream, so you knew when a particular change had been committed to the provider. It could be a separate PR, but that would allow us to finally change the status indicator in N1 (eg "Applying labels...") so that it reflected when the sync engine finished, not the API request.

[grinich]

Do folks have strong feelings on having an integer version number versus a date? The way I've seen Stripe and Clearbit do their gatekeeper versioning is based on a date param that gets sent. More here:

https://stripe.com/docs/upgrades

http://blog.clearbit.com/versioning-apis/

[khamidou]

I have no preferences about it. The latest version of this branch uses a date field instead of an integer version.

[grinich]

Cool, I personally like the date field since it's more explicitly about when the change happened. We should probably also have an API changelog recorded somewhere.

[spang]

This in general LGTM. Can you explain the folder and calendar pieces of this? They're pretty complex and I don't quite understand how they work.

[khamidou]

Sure.

Concerning folders, the main issue we're having is that we're slow to detect folder renames/deletes (see: https://phab.nylas.com/T6988), so to save time, we simply update the folder once we've got confirmation from the IMAP server that the rename went correctly.

We have a similar problem for event updates --- because of the way ActiveSync works, we have to update the event once the server acks our request. To make this work, we store the updated fields in the ActionLog entry's extra_args fields (it's a JSON TEXT field, which is why there's some logic to make sure updates aren't bigger than 64k). Note that we're doing the same thing for Google Calendar --- we could have waited for the server to notify us but I preferred to share the same logic for event updates.

I've updated the diff to address your other comments:

• you can now set the API version by using the Api-Version HTTP header
• cleaned up the cruft

[spang]

LGTM! Let's take this to staging! :)