Mailspring/spec/nylas-api-spec.coffee
Ben Gotow ea76b7c442 feat(transactions): Explicit (and faster) database transactions
Summary:
Until now, we've been hiding transactions beneath the surface. When you call persistModel, you're implicitly creating a transaction.
You could explicitly create them with `atomically`..., but there were several critical problems that are fixed in this diff:

- Calling persistModel / unpersistModel within a transaction could cause the DatabaseStore to trigger. This could result in other parts of the app making queries /during/
  the transaction, potentially before the COMMIT occurred and saved the changes. The new, explicit inTransaction syntax holds all changes until after COMMIT and then triggers.

- Calling atomically and then calling persistModel inside that resulted in us having to check whether a transaction was present and was gross.

- Many parts of the code ran extensive logic inside a promise chained within `atomically`:

  BAD:

```
  DatabaseStore.atomically =>
   DatabaseStore.persistModel(draft) =>
     GoMakeANetworkRequestThatReturnsAPromise
```

OVERWHELMINGLY BETTER:

```
  DatabaseStore.inTransaction (t) =>
     t.persistModel(draft)
  .then =>
    GoMakeANetworkRequestThatReturnsAPromise
```

Having explicit transactions also puts us on equal footing with Sequelize and other ORMs. Note that you /have/ to call DatabaseStore.inTransaction (t) =>. There is no other way to access the methods that let you alter the database. :-)

Other changes:
- This diff removes Message.labels and the Message-Labels table. We weren't using Message-level labels anywhere, and the table could grow very large.
- This diff changes the page size during initial sync from 250 => 200 in an effort to make transactions a bit faster.

Test Plan: Run tests!

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D2353
2015-12-17 11:46:05 -08:00

228 lines
9.2 KiB
CoffeeScript

_ = require 'underscore'
fs = require 'fs'
Actions = require '../src/flux/actions'
NylasAPI = require '../src/flux/nylas-api'
Thread = require '../src/flux/models/thread'
DatabaseStore = require '../src/flux/stores/database-store'
DatabaseTransaction = require '../src/flux/stores/database-transaction'
describe "NylasAPI", ->
beforeEach ->
spyOn(DatabaseStore, '_query').andCallFake => Promise.resolve([])
describe "handleModel404", ->
it "should unpersist the model from the cache that was requested", ->
model = new Thread(id: 'threadidhere')
spyOn(DatabaseTransaction.prototype, 'unpersistModel')
spyOn(DatabaseStore, 'find').andCallFake (klass, id) =>
return Promise.resolve(model)
NylasAPI._handleModel404("/threads/#{model.id}")
advanceClock()
expect(DatabaseStore.find).toHaveBeenCalledWith(Thread, model.id)
expect(DatabaseTransaction.prototype.unpersistModel).toHaveBeenCalledWith(model)
it "should not do anything if the model is not in the cache", ->
spyOn(DatabaseTransaction.prototype, 'unpersistModel')
spyOn(DatabaseStore, 'find').andCallFake (klass, id) =>
return Promise.resolve(null)
NylasAPI._handleModel404("/threads/1234")
advanceClock()
expect(DatabaseStore.find).toHaveBeenCalledWith(Thread, '1234')
expect(DatabaseTransaction.prototype.unpersistModel).not.toHaveBeenCalledWith()
it "should not do anything bad if it doesn't recognize the class", ->
spyOn(DatabaseStore, 'find')
spyOn(DatabaseTransaction.prototype, 'unpersistModel')
waitsForPromise ->
NylasAPI._handleModel404("/asdasdasd/1234")
runs ->
expect(DatabaseStore.find).not.toHaveBeenCalled()
expect(DatabaseTransaction.prototype.unpersistModel).not.toHaveBeenCalled()
it "should not do anything bad if the endpoint only has a single segment", ->
spyOn(DatabaseStore, 'find')
spyOn(DatabaseTransaction.prototype, 'unpersistModel')
waitsForPromise ->
NylasAPI._handleModel404("/account")
runs ->
expect(DatabaseStore.find).not.toHaveBeenCalled()
expect(DatabaseTransaction.prototype.unpersistModel).not.toHaveBeenCalled()
describe "handle401", ->
it "should post a notification", ->
spyOn(Actions, 'postNotification')
NylasAPI._handle401('/threads/1234')
expect(Actions.postNotification).toHaveBeenCalled()
expect(Actions.postNotification.mostRecentCall.args[0].message).toEqual("Nylas can no longer authenticate with your mail provider. You will not be able to send or receive mail. Please unlink your account and sign in again.")
describe "handleModelResponse", ->
beforeEach ->
spyOn(DatabaseTransaction.prototype, "persistModels").andCallFake (models) ->
Promise.resolve(models)
stubDB = ({models, testClass, testMatcher}) ->
spyOn(DatabaseStore, "findAll").andCallFake (klass) ->
testClass?(klass)
where: (matcher) ->
testMatcher?(matcher)
Promise.resolve(models)
it "should reject if no JSON is provided", ->
waitsForPromise ->
NylasAPI._handleModelResponse()
.then -> throw new Error("Should reject!")
.catch (err) ->
expect(err.message).toEqual "handleModelResponse with no JSON provided"
it "should resolve if an empty JSON array is provided", ->
waitsForPromise ->
NylasAPI._handleModelResponse([])
.then (resp) ->
expect(resp).toEqual []
describe "if JSON contains objects which are of unknown types", ->
it "should warn and resolve", ->
spyOn(console, "warn")
waitsForPromise ->
NylasAPI._handleModelResponse([{id: 'a', object: 'unknown'}])
.then (resp) ->
expect(resp).toEqual []
expect(console.warn).toHaveBeenCalled()
expect(console.warn.calls.length).toBe 1
describe "if JSON contains the same object more than once", ->
beforeEach ->
stubDB(models: [])
spyOn(console, "warn")
@dupes = [
{id: 'a', object: 'thread'}
{id: 'a', object: 'thread'}
{id: 'b', object: 'thread'}
]
it "should warn", ->
waitsForPromise =>
NylasAPI._handleModelResponse(@dupes)
.then ->
expect(console.warn).toHaveBeenCalled()
expect(console.warn.calls.length).toBe 1
it "should omit duplicates", ->
waitsForPromise =>
NylasAPI._handleModelResponse(@dupes)
.then ->
models = DatabaseTransaction.prototype.persistModels.calls[0].args[0]
expect(models.length).toBe 2
expect(models[0].id).toBe 'a'
expect(models[1].id).toBe 'b'
describe "when locked by the optimistic change tracker", ->
it "should remove locked models from the set", ->
json = [
{id: 'a', object: 'thread'}
{id: 'b', object: 'thread'}
]
spyOn(NylasAPI._optimisticChangeTracker, "acceptRemoteChangesTo").andCallFake (klass, id) ->
if id is "a" then return false
stubDB models: [new Thread(json[1])], testMatcher: (whereMatcher) ->
expect(whereMatcher.val).toEqual ['b']
waitsForPromise =>
NylasAPI._handleModelResponse(json)
.then (models) ->
expect(models.length).toBe 1
models = DatabaseTransaction.prototype.persistModels.calls[0].args[0]
expect(models.length).toBe 1
expect(models[0].id).toBe 'b'
describe "when updating models", ->
Message = require '../src/flux/models/message'
beforeEach ->
@json = [
{id: 'a', object: 'draft', unread: true}
{id: 'b', object: 'draft', starred: true}
]
@existing = new Message(id: 'b', unread: true)
stubDB models: [@existing]
verifyUpdateHappened = (responseModels) ->
changedModels = DatabaseTransaction.prototype.persistModels.calls[0].args[0]
expect(changedModels.length).toBe 2
expect(changedModels[1].id).toBe 'b'
expect(changedModels[1].starred).toBe true
# Doesn't override existing values
expect(changedModels[1].unread).toBe true
expect(responseModels.length).toBe 2
expect(responseModels[0].id).toBe 'a'
expect(responseModels[0].unread).toBe true
it "updates found models with new data", ->
waitsForPromise =>
NylasAPI._handleModelResponse(@json).then verifyUpdateHappened
it "updates if the json version is newer", ->
@existing.version = 9
@json[1].version = 10
waitsForPromise =>
NylasAPI._handleModelResponse(@json).then verifyUpdateHappened
verifyUpdateStopped = (responseModels) ->
changedModels = DatabaseTransaction.prototype.persistModels.calls[0].args[0]
expect(changedModels.length).toBe 1
expect(changedModels[0].id).toBe 'a'
expect(changedModels[0].unread).toBe true
expect(responseModels.length).toBe 2
expect(responseModels[1].id).toBe 'b'
expect(responseModels[1].starred).toBeUndefined()
it "doesn't update if the json version is older", ->
@existing.version = 10
@json[1].version = 9
waitsForPromise =>
NylasAPI._handleModelResponse(@json).then verifyUpdateStopped
it "doesn't update if it's already sent", ->
@existing.draft = false
@json[1].draft = true
waitsForPromise =>
NylasAPI._handleModelResponse(@json).then verifyUpdateStopped
describe "handling all types of objects", ->
apiObjectToClassMap =
"file": require('../src/flux/models/file')
"event": require('../src/flux/models/event')
"label": require('../src/flux/models/label')
"folder": require('../src/flux/models/folder')
"thread": require('../src/flux/models/thread')
"draft": require('../src/flux/models/message')
"account": require('../src/flux/models/account')
"message": require('../src/flux/models/message')
"contact": require('../src/flux/models/contact')
"calendar": require('../src/flux/models/calendar')
"metadata": require('../src/flux/models/metadata')
verifyUpdateHappened = (klass, responseModels) ->
changedModels = DatabaseTransaction.prototype.persistModels.calls[0].args[0]
expect(changedModels.length).toBe 2
expect(changedModels[0].id).toBe 'a'
expect(changedModels[1].id).toBe 'b'
expect(changedModels[0] instanceof klass).toBe true
expect(changedModels[1] instanceof klass).toBe true
expect(responseModels.length).toBe 2
expect(responseModels[0].id).toBe 'a'
expect(responseModels[1].id).toBe 'b'
expect(responseModels[0] instanceof klass).toBe true
expect(responseModels[1] instanceof klass).toBe true
_.forEach apiObjectToClassMap, (klass, type) ->
it "properly handle the '#{type}' type", ->
json = [
{id: 'a', object: type}
{id: 'b', object: type}
]
stubDB models: [new klass(id: 'b')]
verifyUpdate = _.partial(verifyUpdateHappened, klass)
waitsForPromise =>
NylasAPI._handleModelResponse(json).then verifyUpdate