mirror of
https://github.com/Foundry376/Mailspring.git
synced 2024-09-22 00:06:06 +08:00
a14a5212ac
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
62 lines
1.9 KiB
CoffeeScript
62 lines
1.9 KiB
CoffeeScript
{Label,
|
|
NylasAPI,
|
|
Folder,
|
|
DatabaseStore,
|
|
SyncbackCategoryTask,
|
|
DatabaseTransaction} = require "nylas-exports"
|
|
|
|
describe "SyncbackCategoryTask", ->
|
|
describe "performRemote", ->
|
|
pathOf = (fn) ->
|
|
fn.calls[0].args[0].path
|
|
|
|
accountIdOf = (fn) ->
|
|
fn.calls[0].args[0].accountId
|
|
|
|
nameOf = (fn) ->
|
|
fn.calls[0].args[0].body.display_name
|
|
|
|
makeTask = (CategoryClass) ->
|
|
category = new CategoryClass
|
|
displayName: "important emails"
|
|
accountId: "account 123"
|
|
clientId: "local-444"
|
|
new SyncbackCategoryTask
|
|
category: category
|
|
|
|
beforeEach ->
|
|
spyOn(NylasAPI, "makeRequest").andCallFake ->
|
|
Promise.resolve(id: "server-444")
|
|
spyOn(DatabaseTransaction.prototype, "_query").andCallFake => Promise.resolve([])
|
|
spyOn(DatabaseTransaction.prototype, "persistModel")
|
|
|
|
it "sends API req to /labels if user uses labels", ->
|
|
task = makeTask(Label)
|
|
task.performRemote({})
|
|
expect(pathOf(NylasAPI.makeRequest)).toBe "/labels"
|
|
|
|
it "sends API req to /folders if user uses folders", ->
|
|
task = makeTask(Folder)
|
|
task.performRemote({})
|
|
expect(pathOf(NylasAPI.makeRequest)).toBe "/folders"
|
|
|
|
it "sends the account id", ->
|
|
task = makeTask(Label)
|
|
task.performRemote({})
|
|
expect(accountIdOf(NylasAPI.makeRequest)).toBe "account 123"
|
|
|
|
it "sends the display name in the body", ->
|
|
task = makeTask(Label)
|
|
task.performRemote({})
|
|
expect(nameOf(NylasAPI.makeRequest)).toBe "important emails"
|
|
|
|
it "adds server id to the category, then saves the category", ->
|
|
waitsForPromise ->
|
|
task = makeTask(Label)
|
|
task.performRemote({})
|
|
.then ->
|
|
expect(DatabaseTransaction.prototype.persistModel).toHaveBeenCalled()
|
|
model = DatabaseTransaction.prototype.persistModel.calls[0].args[0]
|
|
expect(model.clientId).toBe "local-444"
|
|
expect(model.serverId).toBe "server-444"
|