diff --git a/exports/nylas-exports.coffee b/exports/nylas-exports.coffee index 97ef50c66..1bf9c1b90 100644 --- a/exports/nylas-exports.coffee +++ b/exports/nylas-exports.coffee @@ -13,6 +13,7 @@ Exports = # The Task Queue Task: require '../src/flux/tasks/task' TaskQueue: require '../src/flux/stores/task-queue' + UndoRedoStore: require '../src/flux/stores/undo-redo-store' # Tasks CreateMetadataTask: require '../src/flux/tasks/create-metadata-task' diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index f4a4c7f88..108a1dec3 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -260,10 +260,14 @@ class ComposerView extends React.Component _renderBody: => if @props.mode is "inline" - @_renderBodyContenteditable() + + {@_renderBodyContenteditable()} + {@_renderAttachments()} + else @refs.scrollbar }> {@_renderBodyContenteditable()} + {@_renderAttachments()} _renderBodyContenteditable: => @@ -281,30 +285,23 @@ class ComposerView extends React.Component return
unless @props.localId
-
- {@_renderNonImageAttachmentsAndUploads()} - {@_renderImageAttachmentsAndUploads()} -
- _renderNonImageAttachmentsAndUploads: -> - @_nonImages().map (fileOrUpload) => - if fileOrUpload.object is "file" - @_attachmentComponent(fileOrUpload) - else - + _renderAttachments: -> + renderSubset = (arr, attachmentRole, UploadComponent) => + arr.map (fileOrUpload) => + if fileOrUpload.object is "file" + @_attachmentComponent(fileOrUpload, attachmentRole) + else + - _renderImageAttachmentsAndUploads: -> - @_images().map (fileOrUpload) => - if fileOrUpload.object is "file" - @_attachmentComponent(fileOrUpload, "Attachment:Image") - else - +
+ {renderSubset(@_nonImages(), 'Attachment', FileUpload)} + {renderSubset(@_images(), 'Attachment:Image', ImageFileUpload)} +
_attachmentComponent: (file, role="Attachment") => targetPath = FileUploadStore.linkedUpload(file)?.filePath @@ -317,8 +314,10 @@ class ComposerView extends React.Component targetPath: targetPath messageLocalId: @props.localId - if role is "Attachment" then className = "non-image-attachment attachment-file-wrap" - else className = "image-attachment-file-wrap" + if role is "Attachment" + className = "non-image-attachment attachment-file-wrap" + else + className = "image-attachment-file-wrap" describe "if the draft has not yet loaded", -> it "should set _focusOnUpdate and focus after the next render", -> - useDraft.call(@) - makeComposer.call(@) + @draft = new Message(draft: true, body: "") + proxy = draftStoreProxyStub(DRAFT_LOCAL_ID, @draft) + proxyResolve = null + spyOn(DraftStore, "sessionForLocalId").andCallFake -> + new Promise (resolve, reject) -> + proxyResolve = resolve - proxy = @composer._proxy - @composer._proxy = null + makeComposer.call(@) spyOn(@composer.refs['contentBody'], 'focus') @composer.focus() advanceClock(1000) expect(@composer.refs['contentBody'].focus).not.toHaveBeenCalled() - @composer._proxy = proxy - @composer._onDraftChanged() + proxyResolve(proxy) advanceClock(1000) expect(@composer.refs['contentBody'].focus).toHaveBeenCalled() @@ -550,7 +552,7 @@ describe "populated composer", -> fileSize: 1024 spyOn(Actions, "fetchFile") - spyOn(FileUploadStore, "linkedUpload") + spyOn(FileUploadStore, "linkedUpload").andReturn null spyOn(FileUploadStore, "uploadsForMessage").andReturn [@up1, @up2] useDraft.call @, files: [@file1, @file2] @@ -569,10 +571,9 @@ describe "populated composer", -> els = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, InjectedComponent, matching: role: "Attachment:Image") expect(els.length).toBe 1 - it 'renders the non image upload as a FileUpload', -> - els = ReactTestUtils.scryRenderedDOMComponentsWithClass(@composer, "file-upload") - expect(els.length).toBe 1 + it 'renders the uploads with the correct components', -> + el = ReactTestUtils.findRenderedDOMComponentWithClass(@composer, 'file-upload') + expect(el).toBeDefined() - it 'renders the image upload as an ImageFileUpload', -> - els = ReactTestUtils.scryRenderedDOMComponentsWithClass(@composer, "image-file-upload") - expect(els.length).toBe 1 + el = ReactTestUtils.findRenderedDOMComponentWithClass(@composer, 'image-file-upload') + expect(el).toBeDefined() diff --git a/internal_packages/composer/stylesheets/composer.less b/internal_packages/composer/stylesheets/composer.less index 5f962e984..0f3344016 100644 --- a/internal_packages/composer/stylesheets/composer.less +++ b/internal_packages/composer/stylesheets/composer.less @@ -147,7 +147,7 @@ cursor: text; overflow: auto; position: relative; - padding: 0 8px; + margin: 0 8px; .quoted-text-control { position: absolute; diff --git a/internal_packages/developer-bar/lib/developer-bar-task.cjsx b/internal_packages/developer-bar/lib/developer-bar-task.cjsx index b384d3ecb..2691b6f77 100644 --- a/internal_packages/developer-bar/lib/developer-bar-task.cjsx +++ b/internal_packages/developer-bar/lib/developer-bar-task.cjsx @@ -49,7 +49,7 @@ class DeveloperBarTask extends React.Component "task-local-error": qs.localError "task-remote-error": qs.remoteError "task-is-processing": qs.isProcessing - "task-success": qs.performedLocal and qs.performedRemote + "task-success": qs.localComplete and qs.remoteComplete module.exports = DeveloperBarTask diff --git a/internal_packages/message-list/spec/message-toolbar-items-spec.cjsx b/internal_packages/message-list/spec/message-toolbar-items-spec.cjsx index 0ec03b9e3..140affb7d 100644 --- a/internal_packages/message-list/spec/message-toolbar-items-spec.cjsx +++ b/internal_packages/message-list/spec/message-toolbar-items-spec.cjsx @@ -26,7 +26,7 @@ describe "MessageToolbarItem starring", -> starButton = React.findDOMNode(messageToolbarItems.refs.starButton) TestUtils.Simulate.click starButton - expect(Actions.queueTask.mostRecentCall.args[0].thread).toBe(test_thread) + expect(Actions.queueTask.mostRecentCall.args[0].threadsOrIds).toEqual([test_thread]) expect(Actions.queueTask.mostRecentCall.args[0].tagIdsToAdd).toEqual(['starred']) expect(Actions.queueTask.mostRecentCall.args[0].tagIdsToRemove).toEqual([]) @@ -39,6 +39,6 @@ describe "MessageToolbarItem starring", -> starButton = React.findDOMNode(messageToolbarItems.refs.starButton) TestUtils.Simulate.click starButton - expect(Actions.queueTask.mostRecentCall.args[0].thread).toBe(test_thread_starred) + expect(Actions.queueTask.mostRecentCall.args[0].threadsOrIds).toEqual([test_thread_starred]) expect(Actions.queueTask.mostRecentCall.args[0].tagIdsToAdd).toEqual([]) expect(Actions.queueTask.mostRecentCall.args[0].tagIdsToRemove).toEqual(['starred']) diff --git a/internal_packages/message-list/stylesheets/message-list.less b/internal_packages/message-list/stylesheets/message-list.less index d382b8f0d..f43671f52 100644 --- a/internal_packages/message-list/stylesheets/message-list.less +++ b/internal_packages/message-list/stylesheets/message-list.less @@ -78,6 +78,7 @@ max-width: @message-max-width; margin: 11px auto 10px auto; padding: 0 20px; + -webkit-user-select: text; } .message-subject { font-size: @font-size-large; diff --git a/internal_packages/thread-list/lib/empty-state.cjsx b/internal_packages/thread-list/lib/empty-state.cjsx index b3b91117e..e951f9b9c 100644 --- a/internal_packages/thread-list/lib/empty-state.cjsx +++ b/internal_packages/thread-list/lib/empty-state.cjsx @@ -87,7 +87,6 @@ class EmptyState extends React.Component @_worker = NylasAPI.workerForNamespace(namespace) @_workerUnlisten() if @_workerUnlisten @_workerUnlisten = @_worker.listen(@_onChange, @) - console.log(@_worker) @setState(syncing: @_worker.busy()) componentWillUnmount: -> diff --git a/internal_packages/thread-list/lib/thread-list-store.coffee b/internal_packages/thread-list/lib/thread-list-store.coffee index d9ad6f7e7..0383ff33b 100644 --- a/internal_packages/thread-list/lib/thread-list-store.coffee +++ b/internal_packages/thread-list/lib/thread-list-store.coffee @@ -21,7 +21,6 @@ module.exports = ThreadListStore = Reflux.createStore init: -> @_resetInstanceVars() - @_afterViewUpdate = [] @listenTo Actions.searchQueryCommitted, @_onSearchCommitted @listenTo Actions.selectLayoutMode, @_autofocusForLayoutMode @@ -53,8 +52,6 @@ ThreadListStore = Reflux.createStore @_viewUnlisten = view.listen -> @trigger(@) - fn() for fn in @_afterViewUpdate - @_afterViewUpdate = [] @_autofocusForLayoutMode() ,@ @@ -110,21 +107,20 @@ ThreadListStore = Reflux.createStore @_view.invalidateMetadataFor(threadIds) _onToggleStarSelection: -> - selected = @_view.selection.items() + selectedThreads = @_view.selection.items() focusedId = FocusedContentStore.focusedId('thread') keyboardId = FocusedContentStore.keyboardCursorId('thread') oneAlreadyStarred = false - for thread in selected + for thread in selectedThreads if thread.hasTagId('starred') oneAlreadyStarred = true - for thread in selected - if oneAlreadyStarred - task = new AddRemoveTagsTask(thread, [], ['starred']) - else - task = new AddRemoveTagsTask(thread, ['starred'], []) - Actions.queueTask(task) + if oneAlreadyStarred + task = new AddRemoveTagsTask(selectedThreads, [], ['starred']) + else + task = new AddRemoveTagsTask(selectedThreads, ['starred'], []) + Actions.queueTask(task) _onToggleStarFocused: -> focused = FocusedContentStore.focused('thread') @@ -140,18 +136,19 @@ ThreadListStore = Reflux.createStore @_archiveAndShiftBy('auto') _onArchiveSelection: -> - selected = @_view.selection.items() + selectedThreads = @_view.selection.items() + selectedThreadIds = selectedThreads.map (thread) -> thread.id focusedId = FocusedContentStore.focusedId('thread') keyboardId = FocusedContentStore.keyboardCursorId('thread') - for thread in selected - task = new AddRemoveTagsTask(thread, ['archive'], ['inbox']) - Actions.queueTask(task) - if thread.id is focusedId + task = new AddRemoveTagsTask(selectedThreads, ['archive'], ['inbox']) + task.waitForPerformLocal().then => + if focusedId in selectedThreadIds Actions.setFocus(collection: 'thread', item: null) - if thread.id is keyboardId + if keyboardId in selectedThreadIds Actions.setCursorPosition(collection: 'thread', item: null) + Actions.queueTask(task) @_view.selection.clear() _onArchiveAndPrev: -> @@ -181,10 +178,6 @@ ThreadListStore = Reflux.createStore index = Math.min(Math.max(index + offset, 0), @_view.count() - 1) nextKeyboard = nextFocus = @_view.get(index) - # Archive the current thread - task = new AddRemoveTagsTask(focused, ['archive'], ['inbox']) - Actions.queueTask(task) - # Remove the current thread from selection @_view.selection.remove(focused) @@ -194,9 +187,12 @@ ThreadListStore = Reflux.createStore if layoutMode is 'list' and not explicitOffset nextFocus = null - @_afterViewUpdate.push -> + # Archive the current thread + task = new AddRemoveTagsTask(focused, ['archive'], ['inbox']) + task.waitForPerformLocal().then -> Actions.setFocus(collection: 'thread', item: nextFocus) Actions.setCursorPosition(collection: 'thread', item: nextKeyboard) + Actions.queueTask(task) _autofocusForLayoutMode: -> layoutMode = WorkspaceStore.layoutMode() diff --git a/spec-nylas/stores/draft-store-spec.coffee b/spec-nylas/stores/draft-store-spec.coffee index 2f5628b64..4028f45ba 100644 --- a/spec-nylas/stores/draft-store-spec.coffee +++ b/spec-nylas/stores/draft-store-spec.coffee @@ -520,7 +520,7 @@ describe "DraftStore", -> it "sets the sending state when sending", -> spyOn(atom, "isMainWindow").andReturn true - spyOn(TaskQueue, "_update") + spyOn(TaskQueue, "_updateSoon") spyOn(Actions, "queueTask").andCallThrough() runs -> DraftStore._onSendDraft(draftLocalId) diff --git a/spec-nylas/stores/task-queue-spec.coffee b/spec-nylas/stores/task-queue-spec.coffee index 1489cd517..ad84cf195 100644 --- a/spec-nylas/stores/task-queue-spec.coffee +++ b/spec-nylas/stores/task-queue-spec.coffee @@ -9,7 +9,7 @@ Task = require '../../src/flux/tasks/task' TimeoutError} = require '../../src/flux/errors' class TaskSubclassA extends Task - constructor: (val) -> @aProp = val # forgot to call super + constructor: (val) -> @aProp = val; super class TaskSubclassB extends Task constructor: (val) -> @bProp = val; super @@ -17,77 +17,20 @@ class TaskSubclassB extends Task describe "TaskQueue", -> makeUnstartedTask = (task) -> - TaskQueue._initializeTask(task) - return task + task - makeLocalStarted = (task) -> - TaskQueue._initializeTask(task) + makeProcessing = (task) -> task.queueState.isProcessing = true - return task - - makeLocalFailed = (task) -> - TaskQueue._initializeTask(task) - task.queueState.performedLocal = Date.now() - return task - - makeRemoteStarted = (task) -> - TaskQueue._initializeTask(task) - task.queueState.isProcessing = true - task.queueState.remoteAttempts = 1 - task.queueState.performedLocal = Date.now() - return task - - makeRemoteSuccess = (task) -> - TaskQueue._initializeTask(task) - task.queueState.remoteAttempts = 1 - task.queueState.performedLocal = Date.now() - task.queueState.performedRemote = Date.now() - return task - - makeRemoteFailed = (task) -> - TaskQueue._initializeTask(task) - task.queueState.remoteAttempts = 1 - task.queueState.performedLocal = Date.now() - return task + task beforeEach -> @task = new Task() @unstartedTask = makeUnstartedTask(new Task()) - @localStarted = makeLocalStarted(new Task()) - @localFailed = makeLocalFailed(new Task()) - @remoteStarted = makeRemoteStarted(new Task()) - @remoteSuccess = makeRemoteSuccess(new Task()) - @remoteFailed = makeRemoteFailed(new Task()) + @processingTask = makeProcessing(new Task()) - unstartedTask = (task) -> - taks.queueState.shouldRetry = false - taks.queueState.isProcessing = false - taks.queueState.remoteAttempts = 0 - taks.queueState.perfomredLocal = false - taks.queueState.performedRemote = false - taks.queueState.notifiedOffline = false - - startedTask = (task) -> - taks.queueState.shouldRetry = false - taks.queueState.isProcessing = true - taks.queueState.remoteAttempts = 0 - taks.queueState.perfomredLocal = false - taks.queueState.performedRemote = false - taks.queueState.notifiedOffline = false - - localTask = (task) -> - taks.queueState.shouldRetry = false - taks.queueState.isProcessing = true - taks.queueState.remoteAttempts = 0 - taks.queueState.perfomredLocal = false - taks.queueState.performedRemote = false - taks.queueState.notifiedOffline = false - - localSpy = (task) -> - spyOn(task, "performLocal").andCallFake -> Promise.resolve() - - remoteSpy = (task) -> - spyOn(task, "performRemote").andCallFake -> Promise.resolve() + afterEach -> + # Flush any throttled or debounced updates + advanceClock(1000) describe "findTask", -> beforeEach -> @@ -111,287 +54,152 @@ describe "TaskQueue", -> expect(TaskQueue.findTask(TaskSubclassB, {bProp: 'B3'})).toEqual(null) describe "enqueue", -> + beforeEach -> + spyOn(@unstartedTask, 'runLocal').andCallFake => + @unstartedTask.queueState.localComplete = true + Promise.resolve() + it "makes sure you've queued a real task", -> expect( -> TaskQueue.enqueue("asamw")).toThrow() it "adds it to the queue", -> - TaskQueue.enqueue(@task) - expect(TaskQueue._queue.length).toBe 1 + spyOn(TaskQueue, '_processQueue').andCallFake -> + TaskQueue.enqueue(@unstartedTask) + advanceClock() + expect(TaskQueue._queue.length).toBe(1) + + it "immediately calls runLocal", -> + TaskQueue.enqueue(@unstartedTask) + expect(@unstartedTask.runLocal).toHaveBeenCalled() it "notifies the queue should be processed", -> - spyOn(TaskQueue, "_processTask") spyOn(TaskQueue, "_processQueue").andCallThrough() + spyOn(TaskQueue, "_processTask") - TaskQueue.enqueue(@task) - + TaskQueue.enqueue(@unstartedTask) + advanceClock() expect(TaskQueue._processQueue).toHaveBeenCalled() - expect(TaskQueue._processTask).toHaveBeenCalledWith(@task) - expect(TaskQueue._processTask.calls.length).toBe 1 + expect(TaskQueue._processTask).toHaveBeenCalledWith(@unstartedTask) + expect(TaskQueue._processTask.calls.length).toBe(1) - it "ensures all tasks have an id", -> - TaskQueue.enqueue(new TaskSubclassA()) - TaskQueue.enqueue(new TaskSubclassB()) - expect(isTempId(TaskQueue._queue[0].id)).toBe true - expect(isTempId(TaskQueue._queue[1].id)).toBe true + it "throws an exception if the task does not have a queueState", -> + task = new TaskSubclassA() + task.queueState = undefined + expect( => TaskQueue.enqueue(task)).toThrow() - it "dequeues Obsolete tasks", -> + it "throws an exception if the task does not have an ID", -> + task = new TaskSubclassA() + task.id = undefined + expect( => TaskQueue.enqueue(task)).toThrow() + + it "dequeues obsolete tasks", -> + task = new TaskSubclassA() + spyOn(TaskQueue, '_dequeueObsoleteTasks').andCallFake -> + TaskQueue.enqueue(task) + expect(TaskQueue._dequeueObsoleteTasks).toHaveBeenCalled() + + describe "_dequeueObsoleteTasks", -> + it "should dequeue tasks based on `shouldDequeueOtherTask`", -> class KillsTaskA extends Task - constructor: -> shouldDequeueOtherTask: (other) -> other instanceof TaskSubclassA + performRemote: -> new Promise (resolve, reject) -> - taskToDie = makeRemoteFailed(new TaskSubclassA()) + otherTask = new Task() + otherTask.queueState.localComplete = true + obsoleteTask = new TaskSubclassA() + obsoleteTask.queueState.localComplete = true + replacementTask = new KillsTaskA() + replacementTask.queueState.localComplete = true - spyOn(TaskQueue, "dequeue").andCallThrough() - - TaskQueue._queue = [taskToDie, @remoteFailed] - TaskQueue.enqueue(new KillsTaskA()) - - expect(TaskQueue._queue.length).toBe 2 - expect(TaskQueue.dequeue).toHaveBeenCalledWith(taskToDie, silent: true) - expect(TaskQueue.dequeue.calls.length).toBe 1 + spyOn(TaskQueue, 'dequeue').andCallThrough() + TaskQueue._queue = [obsoleteTask, otherTask] + TaskQueue._dequeueObsoleteTasks(replacementTask) + expect(TaskQueue._queue.length).toBe(1) + expect(TaskQueue.dequeue).toHaveBeenCalledWith(obsoleteTask) + expect(TaskQueue.dequeue.calls.length).toBe(1) describe "dequeue", -> beforeEach -> - TaskQueue._queue = [@unstartedTask, - @localStarted, - @remoteStarted, - @remoteFailed] + TaskQueue._queue = [@unstartedTask, @processingTask] it "grabs the task by object", -> - found = TaskQueue._parseArgs(@remoteStarted) - expect(found).toBe @remoteStarted + found = TaskQueue._resolveTaskArgument(@unstartedTask) + expect(found).toBe @unstartedTask it "grabs the task by id", -> - found = TaskQueue._parseArgs(@remoteStarted.id) - expect(found).toBe @remoteStarted + found = TaskQueue._resolveTaskArgument(@unstartedTask.id) + expect(found).toBe @unstartedTask it "throws an error if the task isn't found", -> expect( -> TaskQueue.dequeue("bad")).toThrow() - it "calls cleanup on dequeued tasks", -> - spyOn(@remoteStarted, "cleanup") - TaskQueue.dequeue(@remoteStarted, silent: true) - expect(@remoteStarted.cleanup).toHaveBeenCalled() + describe "with an unstarted task", -> + it "moves it from the queue", -> + TaskQueue.dequeue(@unstartedTask) + expect(TaskQueue._queue.length).toBe(1) + expect(TaskQueue._completed.length).toBe(1) - it "moves it from the queue", -> - TaskQueue.dequeue(@remoteStarted, silent: true) - expect(TaskQueue._queue.length).toBe 3 - expect(TaskQueue._completed.length).toBe 1 + it "notifies the queue has been updated", -> + spyOn(TaskQueue, "_processQueue") + TaskQueue.dequeue(@unstartedTask) + advanceClock(20) + expect(TaskQueue._processQueue).toHaveBeenCalled() + expect(TaskQueue._processQueue.calls.length).toBe(1) - it "marks it as no longer processing", -> - TaskQueue.dequeue(@remoteStarted, silent: true) - expect(@remoteStarted.queueState.isProcessing).toBe false - - it "notifies the queue has been updated", -> - spyOn(TaskQueue, "_processQueue") - - TaskQueue.dequeue(@remoteStarted) - - expect(TaskQueue._processQueue).toHaveBeenCalled() - expect(TaskQueue._processQueue.calls.length).toBe 1 + describe "with a processing task", -> + it "calls cancel() to allow the task to resolve or reject from runRemote()", -> + spyOn(@processingTask, 'cancel') + TaskQueue.dequeue(@processingTask) + expect(@processingTask.cancel).toHaveBeenCalled() + expect(TaskQueue._queue.length).toBe(2) + expect(TaskQueue._completed.length).toBe(0) describe "process Task", -> it "doesn't process processing tasks", -> - localSpy(@remoteStarted) - remoteSpy(@remoteStarted) - TaskQueue._processTask(@remoteStarted) - expect(@remoteStarted.performLocal).not.toHaveBeenCalled() - expect(@remoteStarted.performRemote).not.toHaveBeenCalled() + spyOn(@processingTask, "runRemote").andCallFake -> Promise.resolve() + TaskQueue._processTask(@processingTask) + expect(@processingTask.runRemote).not.toHaveBeenCalled() it "doesn't process blocked tasks", -> class BlockedByTaskA extends Task - constructor: -> shouldWaitForTask: (other) -> other instanceof TaskSubclassA - blockedByTask = new BlockedByTaskA() - localSpy(blockedByTask) - remoteSpy(blockedByTask) + taskA = new TaskSubclassA() + otherTask = new Task() + blockedByTaskA = new BlockedByTaskA() - blockingTask = makeRemoteFailed(new TaskSubclassA()) + taskA.queueState.localComplete = true + otherTask.queueState.localComplete = true + blockedByTaskA.queueState.localComplete = true - TaskQueue._queue = [blockingTask, @remoteFailed] - TaskQueue.enqueue(blockedByTask) + spyOn(taskA, "runRemote").andCallFake -> new Promise (resolve, reject) -> + spyOn(blockedByTaskA, "runRemote").andCallFake -> Promise.resolve() - expect(TaskQueue._queue.length).toBe 3 - expect(blockedByTask.performLocal).not.toHaveBeenCalled() - expect(blockedByTask.performRemote).not.toHaveBeenCalled() + TaskQueue._queue = [taskA, otherTask, blockedByTaskA] + TaskQueue._processQueue() - it "doesn't block itself", -> + advanceClock() + + expect(TaskQueue._queue.length).toBe(2) + expect(taskA.runRemote).toHaveBeenCalled() + expect(blockedByTaskA.runRemote).not.toHaveBeenCalled() + + it "doesn't block itself, even if the shouldWaitForTask method is implemented naively", -> class BlockingTask extends Task - constructor: -> shouldWaitForTask: (other) -> other instanceof BlockingTask - blockedByTask = new BlockingTask() - localSpy(blockedByTask) - remoteSpy(blockedByTask) + blockedTask = new BlockingTask() + spyOn(blockedTask, "runRemote").andCallFake -> Promise.resolve() - blockingTask = makeRemoteFailed(new BlockingTask()) - - TaskQueue._queue = [blockingTask, @remoteFailed] - TaskQueue.enqueue(blockedByTask) - - expect(TaskQueue._queue.length).toBe 3 - expect(blockedByTask.performLocal).not.toHaveBeenCalled() - expect(blockedByTask.performRemote).not.toHaveBeenCalled() + TaskQueue.enqueue(blockedTask) + advanceClock() + blockedTask.runRemote.callCount > 0 it "sets the processing bit", -> - localSpy(@unstartedTask) - TaskQueue._queue = [@unstartedTask] - TaskQueue._processTask(@unstartedTask) - expect(@unstartedTask.queueState.isProcessing).toBe true + spyOn(@unstartedTask, "runRemote").andCallFake -> Promise.resolve() + task = new Task() + task.queueState.localComplete = true + TaskQueue._queue = [task] + TaskQueue._processTask(task) + expect(task.queueState.isProcessing).toBe true - it "performs local if it's a fresh task", -> - localSpy(@unstartedTask) - TaskQueue._queue = [@unstartedTask] - TaskQueue._processTask(@unstartedTask) - expect(@unstartedTask.performLocal).toHaveBeenCalled() - - describe "performLocal", -> - it "on success it marks it as complete with the timestamp", -> - localSpy(@unstartedTask) - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.performedLocal isnt false - runs -> - expect(@unstartedTask.queueState.performedLocal).toBeGreaterThan 0 - - it "throws an error if it fails", -> - spyOn(@unstartedTask, "performLocal").andCallFake -> Promise.reject("boo") - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing == false - runs -> - expect(@unstartedTask.queueState.localError).toBe "boo" - expect(@unstartedTask.performLocal).toHaveBeenCalled() - expect(@unstartedTask.performRemote).not.toHaveBeenCalled() - - it "dequeues the task if it fails locally", -> - spyOn(@unstartedTask, "performLocal").andCallFake -> Promise.reject("boo") - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing == false - runs -> - expect(TaskQueue._queue.length).toBe 0 - expect(TaskQueue._completed.length).toBe 1 - - describe "performRemote", -> - beforeEach -> - localSpy(@unstartedTask) - - it "performs remote properly", -> - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.performedRemote isnt false - runs -> - expect(@unstartedTask.performLocal).toHaveBeenCalled() - expect(@unstartedTask.performRemote).toHaveBeenCalled() - - it "dequeues on success", -> - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing is false and - @unstartedTask.queueState.performedRemote > 0 - runs -> - expect(TaskQueue._queue.length).toBe 0 - expect(TaskQueue._completed.length).toBe 1 - - it "notifies we're offline the first time", -> - spyOn(TaskQueue, "_isOnline").andReturn false - remoteSpy(@unstartedTask) - spyOn(@unstartedTask, "onError") - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.notifiedOffline == true - runs -> - expect(@unstartedTask.performLocal).toHaveBeenCalled() - expect(@unstartedTask.performRemote).not.toHaveBeenCalled() - expect(@unstartedTask.onError).toHaveBeenCalled() - expect(@unstartedTask.queueState.isProcessing).toBe false - expect(@unstartedTask.onError.calls[0].args[0] instanceof OfflineError).toBe true - - it "doesn't notify we're offline the second+ time", -> - spyOn(TaskQueue, "_isOnline").andReturn false - localSpy(@remoteFailed) - remoteSpy(@remoteFailed) - spyOn(@remoteFailed, "onError") - @remoteFailed.queueState.notifiedOffline = true - TaskQueue._queue = [@remoteFailed] - runs -> - TaskQueue._processQueue() - waitsFor => - @remoteFailed.queueState.isProcessing is false - runs -> - expect(@remoteFailed.performLocal).not.toHaveBeenCalled() - expect(@remoteFailed.performRemote).not.toHaveBeenCalled() - expect(@remoteFailed.onError).not.toHaveBeenCalled() - - it "marks performedRemote on success", -> - remoteSpy(@unstartedTask) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.performedRemote isnt false - runs -> - expect(@unstartedTask.queueState.performedRemote).toBeGreaterThan 0 - - it "on failure it notifies of the error", -> - err = new APIError - spyOn(@unstartedTask, "performRemote").andCallFake -> Promise.reject(err) - spyOn(@unstartedTask, "onError") - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing is false - runs -> - expect(@unstartedTask.performLocal).toHaveBeenCalled() - expect(@unstartedTask.performRemote).toHaveBeenCalled() - expect(@unstartedTask.onError).toHaveBeenCalledWith(err) - - it "dequeues on failure", -> - err = new APIError - spyOn(@unstartedTask, "performRemote").andCallFake -> Promise.reject(err) - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing is false - runs -> - expect(TaskQueue._queue.length).toBe 0 - expect(TaskQueue._completed.length).toBe 1 - - it "on failure it sets the appropriate bits", -> - err = new APIError - spyOn(@unstartedTask, "performRemote").andCallFake -> Promise.reject(err) - spyOn(@unstartedTask, "onError") - runs -> - TaskQueue.enqueue(@unstartedTask) - waitsFor => - @unstartedTask.queueState.isProcessing is false - runs -> - expect(@unstartedTask.queueState.notifiedOffline).toBe false - expect(@unstartedTask.queueState.remoteError).toBe err - - describe "under stress", -> - beforeEach -> - TaskQueue._queue = [@unstartedTask, - @remoteFailed] - it "when all tasks pass it processes all items", -> - for task in TaskQueue._queue - localSpy(task) - remoteSpy(task) - runs -> - TaskQueue.enqueue(new Task) - waitsFor -> - TaskQueue._queue.length is 0 - runs -> - expect(TaskQueue._completed.length).toBe 3 diff --git a/spec-nylas/tasks/add-remove-tags-spec.coffee b/spec-nylas/tasks/add-remove-tags-spec.coffee index fea7265cd..931ceaaba 100644 --- a/spec-nylas/tasks/add-remove-tags-spec.coffee +++ b/spec-nylas/tasks/add-remove-tags-spec.coffee @@ -4,6 +4,7 @@ AddRemoveTagsTask = require '../../src/flux/tasks/add-remove-tags' DatabaseStore = require '../../src/flux/stores/database-store' Thread = require '../../src/flux/models/thread' Tag = require '../../src/flux/models/tag' +{APIError} = require '../../src/flux/errors' _ = require 'underscore' testThread = null @@ -19,20 +20,19 @@ describe "AddRemoveTagsTask", -> else throw new Error("Not stubbed!") - describe "rollbackLocal", -> - it "should perform the opposite changes to the thread", -> - testThread = new Thread - id: 'thread-id' - tags: [ - new Tag({name: 'archive', id: 'archive'}) - ] - task = new AddRemoveTagsTask(testThread, ['archive'], ['inbox']) - task._rollbackLocal() - waitsFor -> - DatabaseStore.persistModel.callCount > 0 - runs -> - testThread = DatabaseStore.persistModel.mostRecentCall.args[0] - expect(testThread.tagIds()).toEqual(['inbox']) + describe "shouldWaitForTask", -> + it "should return true if another, older AddRemoveTagsTask involves the same threads", -> + a = new AddRemoveTagsTask(['t1', 't2', 't3']) + a.creationDate = new Date(1000) + b = new AddRemoveTagsTask(['t3', 't4', 't7']) + b.creationDate = new Date(2000) + c = new AddRemoveTagsTask(['t0', 't7']) + c.creationDate = new Date(3000) + expect(a.shouldWaitForTask(b)).toEqual(false) + expect(a.shouldWaitForTask(c)).toEqual(false) + expect(b.shouldWaitForTask(a)).toEqual(true) + expect(c.shouldWaitForTask(a)).toEqual(false) + expect(c.shouldWaitForTask(b)).toEqual(true) describe "performLocal", -> beforeEach -> @@ -108,16 +108,15 @@ describe "AddRemoveTagsTask", -> expect(testThread.tagIds().length).toBe(1) expect(testThread.tagIds()[0]).toBe('archive') - describe "performRemote", -> beforeEach -> testThread = new Thread id: '1233123AEDF1' - namespaceId: 'A12ADE' + namespaceId: 'nsid' @task = new AddRemoveTagsTask(testThread, ['archive'], ['inbox']) it "should start an API request with the Draft JSON", -> - spyOn(NylasAPI, 'makeRequest') + spyOn(NylasAPI, 'makeRequest').andCallFake -> Promise.resolve() @task.performLocal() waitsFor -> DatabaseStore.persistModel.callCount > 0 @@ -130,8 +129,27 @@ describe "AddRemoveTagsTask", -> expect(options.body.remove_tags[0]).toBe('inbox') it "should pass returnsModel:true so that the draft is saved to the data store when returned", -> - spyOn(NylasAPI, 'makeRequest') + spyOn(NylasAPI, 'makeRequest').andCallFake -> Promise.resolve() @task.performLocal() @task.performRemote() options = NylasAPI.makeRequest.mostRecentCall.args[0] expect(options.returnsModel).toBe(true) + + describe "when the server responds with a permanentErrorCode", -> + beforeEach -> + spyOn(NylasAPI, 'makeRequest').andCallFake -> + Promise.reject(new APIError(statusCode: 400, message: '')) + + it "should revert the changes to the thread", -> + runs -> + testThread = new Thread + id: 'thread-id' + tags: [new Tag(name: 'inbox', id: 'inbox')] + + task = new AddRemoveTagsTask(testThread, ['archive'], ['inbox']) + task.performRemote() + waitsFor -> + DatabaseStore.persistModel.callCount is 1 + runs -> + testThread = DatabaseStore.persistModel.calls[0].args[0] + expect(testThread.tagIds()).toEqual(['inbox']) diff --git a/spec-nylas/tasks/file-upload-task-spec.coffee b/spec-nylas/tasks/file-upload-task-spec.coffee index b01052e0d..d938d6382 100644 --- a/spec-nylas/tasks/file-upload-task-spec.coffee +++ b/spec-nylas/tasks/file-upload-task-spec.coffee @@ -2,12 +2,17 @@ proxyquire = require 'proxyquire' _ = require 'underscore' NylasAPI = require '../../src/flux/nylas-api' File = require '../../src/flux/models/file' +Task = require '../../src/flux/tasks/task' Message = require '../../src/flux/models/message' Actions = require '../../src/flux/actions' NamespaceStore = require "../../src/flux/stores/namespace-store" DraftStore = require "../../src/flux/stores/draft-store" +{APIError, + OfflineError, + TimeoutError} = require '../../src/flux/errors' + FileUploadTask = proxyquire "../../src/flux/tasks/file-upload-task", fs: statSync: -> {size: 1234} @@ -19,6 +24,8 @@ test_file_paths = [ "/fake/file.jpg" ] +noop = -> + localId = "local-id_1234" fake_draft = new Message @@ -43,6 +50,7 @@ describe "FileUploadTask", -> beforeEach -> spyOn(Date, "now").andReturn DATE spyOn(FileUploadTask, "idGen").andReturn 3 + @uploadData = uploadId: 3 startedUploadingAt: DATE @@ -54,6 +62,23 @@ describe "FileUploadTask", -> @task = new FileUploadTask(test_file_paths[0], localId) + @req = jasmine.createSpyObj('req', ['abort']) + @simulateRequestSuccessImmediately = false + @simulateRequestSuccess = null + @simulateRequestFailure = null + + spyOn(NylasAPI, 'makeRequest').andCallFake (reqParams) => + new Promise (resolve, reject) => + reqParams.started?(@req) + @simulateRequestSuccess = (data) => + reqParams.success?(data) + resolve(data) + @simulateRequestFailure = (err) => + reqParams.error?(err) + reject(err) + if @simulateRequestSuccessImmediately + @simulateRequestSuccess(testResponse) + it "rejects if not initialized with a path name", (done) -> waitsForPromise -> (new FileUploadTask).performLocal().catch (err) -> @@ -80,22 +105,29 @@ describe "FileUploadTask", -> data = _.extend @uploadData, state: "pending", bytesUploaded: 0 expect(Actions.uploadStateChanged).toHaveBeenCalledWith data - it "notifies when the file upload fails", -> - spyOn(Actions, "uploadStateChanged") - spyOn(@task, "_getBytesUploaded").andReturn(0) - @task._rollbackLocal() - data = _.extend @uploadData, state: "failed", bytesUploaded: 0 - expect(Actions.uploadStateChanged).toHaveBeenCalledWith(data) + describe "when the remote API request fails with an API Error", -> + it "broadcasts uploadStateChanged", -> + runs -> + @task.performRemote().catch (err) => console.log(err) + waitsFor -> + @simulateRequestFailure + runs -> + spyOn(@task, "_getBytesUploaded").andReturn(0) + spyOn(Actions, "uploadStateChanged") + @simulateRequestFailure(new APIError()) + waitsFor -> + Actions.uploadStateChanged.callCount > 0 + runs -> + data = _.extend(@uploadData, {state: "failed", bytesUploaded: 0}) + expect(Actions.uploadStateChanged).toHaveBeenCalledWith(data) - describe "When successfully calling remote", -> + describe "when the remote API request succeeds", -> beforeEach -> - spyOn(Actions, "uploadStateChanged") - @req = jasmine.createSpyObj('req', ['abort']) - spyOn(NylasAPI, 'makeRequest').andCallFake (reqParams) => - reqParams.success(testResponse) if reqParams.success - return @req @testFiles = [] @changes = [] + @simulateRequestSuccessImmediately = true + + spyOn(Actions, "uploadStateChanged") spyOn(DraftStore, "sessionForLocalId").andCallFake => Promise.resolve( draft: => files: @testFiles @@ -122,23 +154,19 @@ describe "FileUploadTask", -> expect(@changes).toEqual [equivalentFile] describe "file upload notifications", -> - beforeEach -> - spyOn(Actions, "fileUploaded") - spyOn(@task, "_getBytesUploaded").andReturn(1000) - - runs => - @task.performRemote() - advanceClock(2000) - waitsFor -> - Actions.fileUploaded.calls.length > 0 - it "correctly fires the fileUploaded action", -> - runs => - expect(Actions.fileUploaded).toHaveBeenCalledWith - file: equivalentFile - uploadData: _.extend {}, @uploadData, - state: "completed" - bytesUploaded: 1000 + spyOn(@task, "_getBytesUploaded").andReturn(1000) + spyOn(Actions, "fileUploaded") + @task.performRemote() + advanceClock() + @simulateRequestSuccess() + advanceClock() + Actions.fileUploaded.calls.length > 0 + expect(Actions.fileUploaded).toHaveBeenCalledWith + file: equivalentFile + uploadData: _.extend {}, @uploadData, + state: "completed" + bytesUploaded: 1000 describe "when attaching a lot of files", -> it "attaches them all to the draft", -> @@ -147,6 +175,7 @@ describe "FileUploadTask", -> t3 = new FileUploadTask("3.c", localId) t4 = new FileUploadTask("4.d", localId) + @simulateRequestSuccessImmediately = true waitsForPromise => Promise.all([ t1.performRemote() t2.performRemote() @@ -155,28 +184,29 @@ describe "FileUploadTask", -> ]).then => expect(@changes.length).toBe 4 - describe "cleanup", -> + describe "cancel", -> it "should not do anything if the request has finished", -> - req = jasmine.createSpyObj('req', ['abort']) - reqSuccess = null - spyOn(NylasAPI, 'makeRequest').andCallFake (reqParams) -> - reqSuccess = reqParams.success - req - - @task.performRemote() - reqSuccess(testResponse) - @task.cleanup() - expect(req.abort).not.toHaveBeenCalled() + runs => + @task.performRemote() + waitsFor => + @simulateRequestSuccess + runs => + @simulateRequestSuccess(testResponse) + waitsFor => + @task.req is null + runs => + @task.cancel() + expect(@req.abort).not.toHaveBeenCalled() it "should cancel the request if it's in flight", -> - req = jasmine.createSpyObj('req', ['abort']) - spyOn(NylasAPI, 'makeRequest').andCallFake (reqParams) -> req spyOn(Actions, "uploadStateChanged") @task.performRemote() - @task.cleanup() + advanceClock() + @task.cancel() + advanceClock() - expect(req.abort).toHaveBeenCalled() + expect(@req.abort).toHaveBeenCalled() data = _.extend @uploadData, state: "aborted" bytesUploaded: 0 diff --git a/spec-nylas/tasks/mark-message-read-spec.coffee b/spec-nylas/tasks/mark-message-read-spec.coffee index 025b59094..a93283b8b 100644 --- a/spec-nylas/tasks/mark-message-read-spec.coffee +++ b/spec-nylas/tasks/mark-message-read-spec.coffee @@ -1,5 +1,6 @@ NylasAPI = require '../../src/flux/nylas-api' Actions = require '../../src/flux/actions' +{APIError} = require '../../src/flux/errors' MarkMessageReadTask = require '../../src/flux/tasks/mark-message-read' DatabaseStore = require '../../src/flux/stores/database-store' Message = require '../../src/flux/models/message' @@ -17,34 +18,6 @@ describe "MarkMessageReadTask", -> email: 'dummy@nylas.com' @task = new MarkMessageReadTask(@message) - describe "_rollbackLocal", -> - beforeEach -> - spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve() - - it "should not mark the message as unread if it was not unread initially", -> - message = new Message - id: '1233123AEDF1' - namespaceId: 'A12ADE' - subject: 'New Message' - unread: false - to: - name: 'Dummy' - email: 'dummy@nylas.com' - @task = new MarkMessageReadTask(message) - @task.performLocal() - @task._rollbackLocal() - expect(message.unread).toBe(false) - - it "should mark the message as unread", -> - @task.performLocal() - @task._rollbackLocal() - expect(@message.unread).toBe(true) - - it "should trigger an action to persist the change", -> - @task.performLocal() - @task._rollbackLocal() - expect(DatabaseStore.persistModel).toHaveBeenCalled() - describe "performLocal", -> it "should mark the message as read", -> @task.performLocal() @@ -57,9 +30,41 @@ describe "MarkMessageReadTask", -> describe "performRemote", -> it "should make the PUT request to the message endpoint", -> - spyOn(NylasAPI, 'makeRequest') + spyOn(NylasAPI, 'makeRequest').andCallFake => new Promise (resolve,reject) -> @task.performRemote() options = NylasAPI.makeRequest.mostRecentCall.args[0] expect(options.path).toBe("/n/#{@message.namespaceId}/messages/#{@message.id}") expect(options.method).toBe('PUT') expect(options.body.unread).toBe(false) + + describe "when the remote API request fails", -> + beforeEach -> + spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve() + spyOn(NylasAPI, 'makeRequest').andCallFake -> Promise.reject(new APIError(body: '', statusCode: 400)) + + it "should not mark the message as unread if it was not unread initially", -> + message = new Message + id: '1233123AEDF1' + namespaceId: 'A12ADE' + subject: 'New Message' + unread: false + to: + name: 'Dummy' + email: 'dummy@nylas.com' + @task = new MarkMessageReadTask(message) + @task.performLocal() + @task.performRemote() + advanceClock() + expect(message.unread).toBe(false) + + it "should mark the message as unread", -> + @task.performLocal() + @task.performRemote() + advanceClock() + expect(@message.unread).toBe(true) + + it "should trigger an action to persist the change", -> + @task.performLocal() + @task.performRemote() + advanceClock() + expect(DatabaseStore.persistModel).toHaveBeenCalled() diff --git a/spec-nylas/tasks/send-draft-spec.coffee b/spec-nylas/tasks/send-draft-spec.coffee index 8a00b0408..6524603d6 100644 --- a/spec-nylas/tasks/send-draft-spec.coffee +++ b/spec-nylas/tasks/send-draft-spec.coffee @@ -4,6 +4,7 @@ SyncbackDraftTask = require '../../src/flux/tasks/syncback-draft' SendDraftTask = require '../../src/flux/tasks/send-draft' DatabaseStore = require '../../src/flux/stores/database-store' {generateTempId} = require '../../src/flux/models/utils' +{APIError} = require '../../src/flux/errors' Message = require '../../src/flux/models/message' TaskQueue = require '../../src/flux/stores/task-queue' _ = require 'underscore' @@ -37,47 +38,6 @@ describe "SendDraftTask", -> expect(@sendA.shouldWaitForTask(@saveA)).toBe(true) - describe "When on the TaskQueue", -> - beforeEach -> - TaskQueue._queue = [] - TaskQueue._completed = [] - @saveTask = new SyncbackDraftTask('localid-A') - @saveTaskB = new SyncbackDraftTask('localid-B') - @sendTask = new SendDraftTask('localid-A') - @tasks = [@saveTask, @saveTaskB, @sendTask] - - describe "when tasks succeed", -> - beforeEach -> - for task in @tasks - spyOn(task, "performLocal").andCallFake -> Promise.resolve() - spyOn(task, "performRemote").andCallFake -> Promise.resolve() - runs -> - TaskQueue.enqueue(@saveTask, silent: true) - TaskQueue.enqueue(@saveTaskB, silent: true) - TaskQueue.enqueue(@sendTask) - waitsFor -> - @sendTask.queueState.performedRemote isnt false - - it "processes all of the items", -> - runs -> - expect(TaskQueue._queue.length).toBe 0 - expect(TaskQueue._completed.length).toBe 3 - - it "all of the tasks", -> - runs -> - expect(@saveTask.performRemote).toHaveBeenCalled() - expect(@saveTaskB.performRemote).toHaveBeenCalled() - expect(@sendTask.performRemote).toHaveBeenCalled() - - it "finishes the save before sending", -> - runs -> - save = @saveTask.queueState.performedRemote - send = @sendTask.queueState.performedRemote - expect(save).toBeGreaterThan 0 - expect(send).toBeGreaterThan 0 - expect(save <= send).toBe true - - describe "performLocal", -> it "should throw an exception if the first parameter is not a localId", -> badTasks = [new SendDraftTask()] @@ -113,7 +73,8 @@ describe "SendDraftTask", -> @draftLocalId = "local-123" @task = new SendDraftTask(@draftLocalId) spyOn(NylasAPI, 'makeRequest').andCallFake (options) => - options.success(@draft.toJSON()) if options.success + options.success?(@draft.toJSON()) + Promise.resolve(@draft.toJSON()) spyOn(DatabaseStore, 'findByLocalId').andCallFake (klass, localId) => Promise.resolve(@draft) spyOn(DatabaseStore, 'unpersistModel').andCallFake (draft) -> @@ -207,12 +168,14 @@ describe "SendDraftTask", -> it "should resend the draft without the reply_to_message_id key set", -> @draft.id = generateTempId() spyOn(DatabaseStore, 'findByLocalId').andCallFake => Promise.resolve(@draft) - spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) -> + spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) => if body.reply_to_message_id - err = new Error("Invalid message public id") - error(err) + err = new APIError(body: "Invalid message public id", statusCode: 400) + error?(err) + return Promise.reject(err) else - success(body) + success?(body) + return Promise.resolve(body) waitsForPromise => @task.performRemote().then => @@ -224,18 +187,23 @@ describe "SendDraftTask", -> it "should resend the draft without the thread_id or reply_to_message_id keys set", -> @draft.id = generateTempId() spyOn(DatabaseStore, 'findByLocalId').andCallFake => Promise.resolve(@draft) - spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) -> - if body.thread_id - err = new Error("Invalid thread public id") - error(err) - else - success(body) + spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) => + new Promise (resolve, reject) => + if body.thread_id + err = new APIError(body: "Invalid thread public id", statusCode: 400) + error?(err) + reject(err) + else + success?(body) + resolve(body) waitsForPromise => @task.performRemote().then => expect(NylasAPI.makeRequest.calls.length).toBe(2) expect(NylasAPI.makeRequest.calls[1].args[0].body.thread_id).toBe(null) expect(NylasAPI.makeRequest.calls[1].args[0].body.reply_to_message_id).toBe(null) + .catch (err) => + console.log(err.trace) it "throws an error if the draft can't be found", -> spyOn(DatabaseStore, 'findByLocalId').andCallFake (klass, localId) -> @@ -257,16 +225,3 @@ describe "SendDraftTask", -> waitsForPromise => @task.performRemote().catch (error) -> expect(error).toBe "DB error" - - it "onAPIError notifies of the error", -> - @task.onAPIError(message: "oh no") - - it "onOtherError notifies of the error", -> - @task.onOtherError() - - it "onTimeoutError notifies of the error", -> - @task.onTimeoutError() - - it "onOfflineError notifies of the error and dequeues", -> - @task.onOfflineError() - expect(Actions.dequeueTask).toHaveBeenCalledWith(@task) diff --git a/spec-nylas/tasks/syncback-draft-spec.coffee b/spec-nylas/tasks/syncback-draft-spec.coffee index e428f8b8c..6e8b2f2ff 100644 --- a/spec-nylas/tasks/syncback-draft-spec.coffee +++ b/spec-nylas/tasks/syncback-draft-spec.coffee @@ -50,7 +50,7 @@ describe "SyncbackDraftTask", -> describe "performRemote", -> beforeEach -> spyOn(NylasAPI, 'makeRequest').andCallFake (opts) -> - opts.success(remoteDraft.toJSON()) if opts.success + Promise.resolve(remoteDraft.toJSON()) it "does nothing if no draft can be found in the db", -> task = new SyncbackDraftTask("missingDraftId") @@ -108,16 +108,17 @@ describe "SyncbackDraftTask", -> describe "When the api throws a 404 error", -> beforeEach -> - spyOn(TaskQueue, "enqueue") spyOn(NylasAPI, "makeRequest").andCallFake (opts) -> - opts.error(testError(opts)) if opts.error + Promise.reject(testError(opts)) it "resets the id", -> task = new SyncbackDraftTask("remoteDraftId") - task.onAPIError(testError({})) + taskStatus = null + task.performRemote().then (status) => taskStatus = status + waitsFor -> DatabaseStore.swapModel.calls.length > 0 runs -> newDraft = DatabaseStore.swapModel.mostRecentCall.args[0].newModel expect(isTempId(newDraft.id)).toBe true - expect(TaskQueue.enqueue).toHaveBeenCalled() + expect(taskStatus).toBe(Task.Status.Retry) diff --git a/spec-nylas/tasks/task-spec.coffee b/spec-nylas/tasks/task-spec.coffee index 8b89dbc06..56bce54b6 100644 --- a/spec-nylas/tasks/task-spec.coffee +++ b/spec-nylas/tasks/task-spec.coffee @@ -1,56 +1,127 @@ -# {APIError} = require '../../src/flux/errors' -# Task = require '../../src/flux/tasks/task' -# _ = require 'underscore' -# -# describe "Task", -> -# beforeEach -> -# @task = new Task() -# -# describe "shouldRetry", -> -# -# it "should default to false if the error does not have a status code", -> -# expect(@task.shouldRetry(new Error())).toBe(false) -# -# # Should Not Retry -# -# it "should return false when the error is a 401 Unauthorized from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 401}))).toBe(false) -# -# it "should return false when the error is a 403 Forbidden from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 403}))).toBe(false) -# -# it "should return false when the error is a 404 Not Found from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 404}))).toBe(false) -# -# it "should return false when the error is a 405 Method Not Allowed from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 405}))).toBe(false) -# -# it "should return false when the error is a 406 Not Acceptable from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 406}))).toBe(false) -# -# it "should return false when the error is a 409 Conflict from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 409}))).toBe(false) -# -# # Should Retry -# -# it "should return true when the error is 0 Request Not Made from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 0}))).toBe(true) -# -# it "should return true when the error is 407 Proxy Authentication Required from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 407}))).toBe(true) -# -# it "should return true when the error is 408 Request Timeout from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 408}))).toBe(true) -# -# it "should return true when the error is 305 Use Proxy from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 305}))).toBe(true) -# -# it "should return true when the error is 502 Bad Gateway from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 502}))).toBe(true) -# -# it "should return true when the error is 503 Service Unavailable from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 503}))).toBe(true) -# -# it "should return true when the error is 504 Gateway Timeout from the API", -> -# expect(@task.shouldRetry(new APIError({statusCode: 504}))).toBe(true) -# +Actions = require '../../src/flux/actions' +TaskQueue = require '../../src/flux/stores/task-queue' +Task = require '../../src/flux/tasks/task' + +{APIError, + OfflineError, + TimeoutError} = require '../../src/flux/errors' + +noop = -> + +describe "Task", -> + describe "initial state", -> + it "should set up queue state with additional information about local/remote", -> + task = new Task() + expect(task.queueState).toEqual({ isProcessing : false, localError : null, localComplete : false, remoteError : null, remoteAttempts : 0, remoteComplete : false }) + + describe "runLocal", -> + beforeEach -> + class APITestTask extends Task + performLocal: -> Promise.resolve() + performRemote: -> Promise.resolve(Task.Status.Finished) + @task = new APITestTask() + + describe "when performLocal is not complete", -> + it "should run performLocal", -> + spyOn(@task, 'performLocal').andCallThrough() + @task.runLocal() + expect(@task.performLocal).toHaveBeenCalled() + + describe "when performLocal rejects", -> + beforeEach -> + spyOn(@task, 'performLocal').andCallFake => + Promise.reject(new Error("Oh no!")) + + it "should save the error to the queueState", -> + @task.runLocal().catch(noop) + advanceClock() + expect(@task.performLocal).toHaveBeenCalled() + expect(@task.queueState.localComplete).toBe(false) + expect(@task.queueState.localError.message).toBe("Oh no!") + + it "should reject with the error", -> + rejection = null + runs -> + @task.runLocal().catch (err) -> + rejection = err + waitsFor -> + rejection + runs -> + expect(rejection.message).toBe("Oh no!") + + describe "when performLocal resolves", -> + beforeEach -> + spyOn(@task, 'performLocal').andCallFake -> Promise.resolve('Hooray') + + it "should save that performLocal is complete", -> + @task.runLocal() + advanceClock() + expect(@task.queueState.localComplete).toBe(true) + + it "should save that there was no performLocal error", -> + @task.runLocal() + advanceClock() + expect(@task.queueState.localError).toBe(null) + + describe "runRemote", -> + beforeEach -> + @task.queueState.localComplete = true + + it "should run performRemote", -> + spyOn(@task, 'performRemote').andCallThrough() + @task.runRemote() + advanceClock() + expect(@task.performRemote).toHaveBeenCalled() + + describe "when performRemote resolves", -> + beforeEach -> + spyOn(@task, 'performRemote').andCallFake -> + Promise.resolve(Task.Status.Finished) + + it "should save that performRemote is complete with no errors", -> + @task.runRemote() + advanceClock() + expect(@task.performRemote).toHaveBeenCalled() + expect(@task.queueState.remoteError).toBe(null) + expect(@task.queueState.remoteComplete).toBe(true) + + it "should only allow the performRemote method to return a Task.Status", -> + result = null + err = null + + class OKTask extends Task + performRemote: -> Promise.resolve(Task.Status.Retry) + + @ok = new OKTask() + @ok.queueState.localComplete = true + @ok.runRemote().then (r) -> result = r + advanceClock() + expect(result).toBe(Task.Status.Retry) + + class BadTask extends Task + performRemote: -> Promise.resolve('lalal') + @bad = new BadTask() + @bad.queueState.localComplete = true + @bad.runRemote().catch (e) -> err = e + advanceClock() + expect(err.message).toBe('performRemote returned lalal, which is not a Task.Status') + + describe "when performRemote rejects", -> + beforeEach -> + @error = new APIError("Oh no!") + spyOn(@task, 'performRemote').andCallFake => Promise.reject(@error) + + it "should save the error to the queueState", -> + @task.runRemote().catch(noop) + advanceClock() + expect(@task.queueState.remoteError).toBe(@error) + + it "should increment the number of attempts", -> + runs -> + @task.runRemote().catch(noop) + waitsFor -> + @task.queueState.remoteAttempts == 1 + runs -> + @task.runRemote().catch(noop) + waitsFor -> + @task.queueState.remoteAttempts == 2 diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index 139293166..42c3d0cfc 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -97,6 +97,13 @@ ReactTestUtils.unmountAll = -> React.unmountComponentAtNode(container) ReactElementContainers = [] +# Make Bluebird use setTimeout so that it hooks into our stubs, and you can +# advance promises using `advanceClock()`. To avoid breaking any specs that +# `dont` manually call advanceClock, call it automatically on the next tick. +Promise.setScheduler (fn) -> + setTimeout(fn, 0) + process.nextTick -> advanceClock(1) + beforeEach -> Grim.clearDeprecations() if isCoreSpec ComponentRegistry._clear() @@ -251,6 +258,8 @@ jasmine.restoreDeprecationsSnapshot = -> jasmine.useRealClock = -> jasmine.unspy(window, 'setTimeout') jasmine.unspy(window, 'clearTimeout') + jasmine.unspy(window, 'setInterval') + jasmine.unspy(window, 'clearInterval') jasmine.unspy(_._, 'now') addCustomMatchers = (spec) -> diff --git a/src/atom.coffee b/src/atom.coffee index 3368a89b8..28032cd26 100644 --- a/src/atom.coffee +++ b/src/atom.coffee @@ -16,6 +16,7 @@ WindowEventHandler = require './window-event-handler' StylesElement = require './styles-element' Utils = require './flux/models/utils' +{APIError} = require './flux/errors' # Essential: Atom global for dealing with packages, themes, menus, and the window. # @@ -271,6 +272,11 @@ class Atom extends Model error.stack = convertStackTrace(error.stack, sourceMapCache) eventObject = {message: error.message, originalError: error} + # API Errors are a normal part of life and are logged to the API + # history panel. We ignore these errors and do not report them to Sentry. + if error instanceof APIError + return + if @inSpecMode() console.error(error.stack) else if @inDevMode() diff --git a/src/command-registry.coffee b/src/command-registry.coffee index b49167f50..7ce745b09 100644 --- a/src/command-registry.coffee +++ b/src/command-registry.coffee @@ -88,6 +88,9 @@ class CommandRegistry disposable.add @add(target, commandName, callback) return disposable + if not callback + throw new Error("CommandRegistry:add called without a callback") + if typeof target is 'string' @addSelectorBasedListener(target, commandName, callback) else diff --git a/src/flux/errors.coffee b/src/flux/errors.coffee index 3f020bcd1..7d89c2ce4 100644 --- a/src/flux/errors.coffee +++ b/src/flux/errors.coffee @@ -13,9 +13,6 @@ class APIError extends Error @name = "APIError" @message = @body?.message ? @body ? @error?.toString?() - notifyConsole: -> - console.error("Edgehill API Error: #{@message}", @) - class OfflineError extends Error constructor: -> diff --git a/src/flux/nylas-api.coffee b/src/flux/nylas-api.coffee index a8d9f5112..6123fe969 100644 --- a/src/flux/nylas-api.coffee +++ b/src/flux/nylas-api.coffee @@ -10,11 +10,67 @@ NylasLongConnection = require './nylas-long-connection' {modelFromJSON, modelClassMap} = require './models/utils' async = require 'async' +class NylasAPIOptimisticChangeTracker + constructor: -> + @_locks = {} + + acceptRemoteChangesTo: (klass, id) -> + @_locks["#{klass.name}-#{id}"] is undefined + + increment: (klass, id) -> + @_locks["#{klass.name}-#{id}"] ?= 0 + @_locks["#{klass.name}-#{id}"] += 1 + + decrement: (klass, id) -> + @_locks["#{klass.name}-#{id}"] -= 1 + if @_locks["#{klass.name}-#{id}"] is 0 + delete @_locks["#{klass.name}-#{id}"] + + print: -> + console.log("The following models are locked:") + console.log(@_locks) + +class NylasAPIRequest + + constructor: (@api, @options) -> + @options.method ?= 'GET' + @options.url ?= "#{@api.APIRoot}#{@options.path}" if @options.path + @options.json ?= true + @options.auth = {'user': @api.APIToken, 'pass': '', sendImmediately: true} + unless @options.method is 'GET' or @options.formData + @options.body ?= {} + @ + + run: -> + if atom.getLoadSettings().isSpec + return Promise.resolve() + + if not @api.APIToken + return Promise.reject(new Error('Cannot make Nylas request without auth token.')) + + new Promise (resolve, reject) => + req = request @options, (error, response, body) => + PriorityUICoordinator.settle.then => + Actions.didMakeAPIRequest({request: @options, response: response}) + + if error or response.statusCode > 299 + apiError = new APIError({error, response, body, requestOptions: @options}) + @options.error?(apiError) + reject(apiError) + else + @options.success?(body) + resolve(body) + req.on 'abort', -> + reject(new APIError({statusCode: 0, body: 'Request Aborted'})) + @options.started?(req) class NylasAPI + PermanentErrorCodes: [400, 404, 500] + constructor: -> @_workers = [] + @_optimisticChangeTracker = new NylasAPIOptimisticChangeTracker() atom.config.onDidChange('env', @_onConfigChanged) atom.config.onDidChange('nylas.token', @_onConfigChanged) @@ -107,47 +163,41 @@ class NylasAPI # success: (body) -> callback gets passed the returned json object # error: (apiError) -> the error callback gets passed an Nylas # APIError object. + # + # Returns a Promise, which resolves or rejects in the success / error + # scenarios, respectively. + # makeRequest: (options={}) -> - return if atom.getLoadSettings().isSpec - return console.log('Cannot make Nylas request without auth token.') unless @APIToken - options.method ?= 'GET' - options.url ?= "#{@APIRoot}#{options.path}" if options.path - options.json ?= true - options.auth = {'user': @APIToken, 'pass': '', sendImmediately: true} + if atom.getLoadSettings().isSpec + return Promise.resolve() - unless options.method is 'GET' or options.formData - options.body ?= {} + if not @APIToken + console.log('Cannot make Nylas request without auth token.') + return Promise.reject() - request options, (error, response, body) => - PriorityUICoordinator.settle.then => - Actions.didMakeAPIRequest({request: options, response: response}) - if error? or response.statusCode > 299 - if response and response.statusCode is 404 and options.returnsModel - @_handleModel404(options.url) - if response and response.statusCode is 401 - @_handle401(options.url) - options.error?(new APIError({error, response, body})) - else - if options.json - if _.isString(body) - try - body = JSON.parse(body) - catch error - options.error?(new APIError({error, response, body})) - if options.returnsModel - @_handleModelResponse(body) + success = (body) => + if options.beforeProcessing + body = options.beforeProcessing(body) + if options.returnsModel + @_handleModelResponse(body) + Promise.resolve(body) - if options.success - options.success(body) + error = (err) => + if err.response + if err.response.statusCode is 404 and options.returnsModel + @_handleModel404(options.url) + if err.response.statusCode is 401 + @_handle401(options.url) + Promise.reject(err) + + req = new NylasAPIRequest(@, options) + req.run().then(success, error) # If we make a request that `returnsModel` and we get a 404, we want to handle # it intelligently and in a centralized way. This method identifies the object # that could not be found and purges it from local cache. # - # Handles: - # - # /namespace/// - # /namespace//?thread_id= + # Handles: /namespace/// # _handleModel404: (modelUrl) -> url = require('url') @@ -164,8 +214,7 @@ class NylasAPI DatabaseStore.find(klass, klassId).then (model) -> DatabaseStore.unpersistModel(model) if model - _handle401: (url) -> - # Throw up a notification indicating that the user should log out and log back in + _handle401: (modelUrl) -> Actions.postNotification type: 'error' tag: '401' @@ -267,14 +316,16 @@ class NylasAPI Promise.resolve(true) - _shouldAcceptModelIfNewer: (klass, model = null) -> - new Promise (resolve, reject) -> - DatabaseStore = require './stores/database-store' - DatabaseStore.find(klass, model.id).then (existing) -> - if existing and existing.version >= model.version - resolve(false) - else - resolve(true) + _shouldAcceptModelIfNewer: (klass, model) -> + if @_optimisticChangeTracker.acceptRemoteChangesTo(klass, model.id) is false + return Promise.resolve(false) + + DatabaseStore = require './stores/database-store' + DatabaseStore.find(klass, model.id).then (existing) -> + if existing and existing.version >= model.version + return Promise.resolve(false) + else + return Promise.resolve(true) getThreads: (namespaceId, params = {}, requestOptions = {}) -> requestSuccess = requestOptions.success @@ -298,4 +349,10 @@ class NylasAPI qs: params returnsModel: true + incrementOptimisticChangeCount: (klass, id) -> + @_optimisticChangeTracker.increment(klass, id) + + decrementOptimisticChangeCount: (klass, id) -> + @_optimisticChangeTracker.decrement(klass, id) + module.exports = new NylasAPI() diff --git a/src/flux/stores/file-download-store.coffee b/src/flux/stores/file-download-store.coffee index b33eaa419..e58ed30b1 100644 --- a/src/flux/stores/file-download-store.coffee +++ b/src/flux/stores/file-download-store.coffee @@ -80,27 +80,28 @@ class Download else finishedAction = action - @request = NylasAPI.makeRequest + NylasAPI.makeRequest json: false path: "/n/#{namespace}/files/#{@fileId}/download" + started: (req) => + @request = req + progress(@request, {throtte: 250}) + .on "progress", (progress) => + @percent = progress.percent + @progressCallback() + .on "end", => + # Wait for the file stream to finish writing before we resolve or reject + stream.end(streamEnded) + .pipe(stream) + success: => # At this point, the file stream has not finished writing to disk. # Don't resolve yet, or the browser will load only part of the image. onStreamEnded(resolve) + error: => onStreamEnded(reject) - progress(@request, {throtte: 250}) - .on("progress", (progress) => - @percent = progress.percent - @progressCallback() - ) - .on("end", => - # Wait for the file stream to finish writing before we resolve or reject - stream.end(streamEnded) - ) - .pipe(stream) - abort: -> @request?.abort() diff --git a/src/flux/stores/metadata-store.coffee b/src/flux/stores/metadata-store.coffee index 507b449ee..c4b4e5f29 100644 --- a/src/flux/stores/metadata-store.coffee +++ b/src/flux/stores/metadata-store.coffee @@ -85,7 +85,6 @@ MetadataStore = Reflux.createStore else DatabaseStore.persistModels(metadata).then(resolve).catch(reject) error: (apiError) -> - apiError.notifyConsole() reject(apiError) _onDBChanged: (change) -> diff --git a/src/flux/stores/task-queue.coffee b/src/flux/stores/task-queue.coffee index 8498fc4b3..204cedcc3 100644 --- a/src/flux/stores/task-queue.coffee +++ b/src/flux/stores/task-queue.coffee @@ -11,7 +11,6 @@ Reflux = require 'reflux' Actions = require '../actions' {APIError, - OfflineError, TimeoutError} = require '../errors' if not atom.isMainWindow() and not atom.inSpecMode() then return @@ -79,27 +78,8 @@ class TaskQueue @listenTo(Actions.clearDeveloperConsole, @clearCompleted) - # TODO - # @listenTo(OnlineStatusStore, @_onOnlineChange) - @_onlineStatus = true @listenTo Actions.longPollConnected, => - @_onlineStatus = true - @_update() - @listenTo Actions.longPollOffline, => - @_onlineStatus = false - @_update() - - _initializeTask: (task) => - task.id ?= generateTempId() - task.queueState ?= {} - task.queueState = - localError: null - remoteError: null - isProcessing: false - remoteAttempts: 0 - performedLocal: false - performedRemote: false - notifiedOffline: false + @_processQueue() queue: => @_queue @@ -122,125 +102,97 @@ class TaskQueue match = _.find @_queue, (task) -> task.constructor.name is type and _.isMatch(task, matching) match ? null - enqueue: (task, {silent}={}) => + enqueue: (task) => if not (task instanceof Task) - throw new Error("You must queue a `Task` object") + throw new Error("You must queue a `Task` instance") + if not task.id + throw new Error("Tasks must have an ID prior to being queued. Check that your Task constructor is calling `super`") + if not task.queueState + throw new Error("Tasks must have a queueState prior to being queued. Check that your Task constructor is calling `super`") - @_initializeTask(task) @_dequeueObsoleteTasks(task) - @_queue.push(task) - @_update() if not silent + task.runLocal().then => + @_queue.push(task) + @_updateSoon() - dequeue: (taskOrId={}, {silent}={}) => - task = @_parseArgs(taskOrId) + dequeue: (taskOrId) => + task = @_resolveTaskArgument(taskOrId) if not task throw new Error("Couldn't find task in queue to dequeue") - task.queueState.isProcessing = false - task.cleanup() - - @_queue.splice(@_queue.indexOf(task), 1) - @_moveToCompleted(task) - @_update() if not silent + if task.queueState.isProcessing + # We cannot remove a task from the queue while it's running and pretend + # things have stopped. Ask the task to cancel. It's promise will resolve + # or reject, and then we'll end up back here. + task.cancel() + else + @_queue.splice(@_queue.indexOf(task), 1) + @_completed.push(task) + @_completed.shift() if @_completed.length > 1000 + @_updateSoon() dequeueAll: => for task in @_queue by -1 - @dequeue(task, silent: true) if task? - @_update() + @dequeue(task) dequeueMatching: ({type, matching}) => - toDequeue = @findTask(type, matching) + task = @findTask(type, matching) - if not toDequeue - console.warn("Could not find task: #{type}", matching) + if not task + console.warn("Could not find matching task: #{type}", matching) return - @dequeue(toDequeue, silent: true) - @_update() + @dequeue(task) clearCompleted: => @_completed = [] @trigger() + # Helper Methods + _processQueue: => for task in @_queue by -1 - @_processTask(task) if task? + continue if @_taskIsBlocked(task) + @_processTask(task) _processTask: (task) => return if task.queueState.isProcessing - return if @_taskIsBlocked(task) task.queueState.isProcessing = true - - if task.queueState.performedLocal - @_performRemote(task) - else - task.performLocal().then => - task.queueState.performedLocal = Date.now() - @_performRemote(task) - .catch @_onLocalError(task) - - _performRemote: (task) => - if @_isOnline() - task.queueState.remoteAttempts += 1 - task.performRemote().then => - task.queueState.performedRemote = Date.now() - @dequeue(task) - .catch @_onRemoteError(task) - else - @_notifyOffline(task) - - _update: => - @trigger() - @_saveQueueToDiskDebounced() - @_processQueue() + task.runRemote() + .finally => + task.queueState.isProcessing = false + @trigger() + .then (status) => + @dequeue(task) unless status is Task.Status.Retry + .catch (err) => + console.warn("Task #{task.constructor.name} threw an error: #{err}.") + @dequeue(task) _dequeueObsoleteTasks: (task) => - for otherTask in @_queue by -1 + obsolete = _.filter @_queue, (otherTask) => # Do not interrupt tasks which are currently processing - continue if otherTask.queueState.isProcessing + return false if otherTask.queueState.isProcessing # Do not remove ourselves from the queue - continue if otherTask is task + return false if otherTask is task # Dequeue tasks which our new task indicates it makes obsolete - if task.shouldDequeueOtherTask(otherTask) - @dequeue(otherTask, silent: true) + return task.shouldDequeueOtherTask(otherTask) + + for otherTask in obsolete + @dequeue(otherTask) + _taskIsBlocked: (task) => _.any @_queue, (otherTask) -> task.shouldWaitForTask(otherTask) and task isnt otherTask - _notifyOffline: (task) => - task.queueState.isProcessing = false - if not task.queueState.notifiedOffline - task.queueState.notifiedOffline = true - task.onError(new OfflineError) - - _onLocalError: (task) => (error) => - task.queueState.isProcessing = false - task.queueState.localError = error - task.onError(error) - @dequeue(task) - - _onRemoteError: (task) => (apiError) => - task.queueState.isProcessing = false - task.queueState.notifiedOffline = false - task.queueState.remoteError = apiError - task.onError(apiError) - @dequeue(task) - - _isOnline: => @_onlineStatus # TODO # OnlineStatusStore.isOnline() - _onOnlineChange: => @_processQueue() - - _parseArgs: (taskOrId) => - if taskOrId instanceof Task - task = _.find @_queue, (task) -> task is taskOrId + _resolveTaskArgument: (taskOrId) => + if not taskOrId + return null + else if taskOrId instanceof Task + return _.find @_queue, (task) -> task is taskOrId else - task = _.findWhere(@_queue, id: taskOrId) - return task - - _moveToCompleted: (task) => - @_completed.push(task) - @_completed.shift() if @_completed.length > 1000 + return _.findWhere(@_queue, id: taskOrId) _restoreQueueFromDisk: => {modelReviver} = require '../models/utils' @@ -250,25 +202,30 @@ class TaskQueue # We need to set the processing bit back to false so it gets # re-retried upon inflation for task in queue - if task.queueState?.isProcessing - task.queueState ?= {} - task.queueState.isProcessing = false + task.queueState ?= {} + task.queueState.isProcessing = false @_queue = queue catch e if not atom.inSpecMode() console.log("Queue deserialization failed with error: #{e.toString()}") - # It's very important that we debounce saving here. When the user bulk-archives - # items, they can easily process 1000 tasks at the same moment. We can't try to - # save 1000 times! (Do not remove debounce without a plan!) - _saveQueueToDisk: => - queueFile = path.join(atom.getConfigDirPath(), 'task-queue.json') - queueJSON = JSON.stringify((@_queue ? [])) - fs.writeFile(queueFile, queueJSON) + # It's very important that we debounce saving here. When the user bulk-archives + # items, they can easily process 1000 tasks at the same moment. We can't try to + # save 1000 times! (Do not remove debounce without a plan!) + @_saveDebounced ?= _.debounce => + queueFile = path.join(atom.getConfigDirPath(), 'task-queue.json') + queueJSON = JSON.stringify((@_queue ? [])) + fs.writeFile(queueFile, queueJSON) + , 150 + @_saveDebounced() - _saveQueueToDiskDebounced: => - @__saveQueueToDiskDebounced ?= _.debounce(@_saveQueueToDisk, 150) - @__saveQueueToDiskDebounced() + _updateSoon: => + @_updateSoonThrottled ?= _.throttle => + @_processQueue() + @_saveQueueToDisk() + @trigger() + , 10, {leading: false} + @_updateSoonThrottled() module.exports = new TaskQueue() diff --git a/src/flux/stores/undo-redo-store.coffee b/src/flux/stores/undo-redo-store.coffee new file mode 100644 index 000000000..018ca8519 --- /dev/null +++ b/src/flux/stores/undo-redo-store.coffee @@ -0,0 +1,47 @@ +_ = require 'underscore' + +{Listener, Publisher} = require '../modules/reflux-coffee' +CoffeeHelpers = require '../coffee-helpers' + +Task = require "../tasks/task" +Actions = require '../actions' + +class UndoRedoStore + @include: CoffeeHelpers.includeModule + + @include Publisher + @include Listener + + constructor: -> + @_undo = [] + @_redo = [] + + @listenTo(Actions.queueTask, @_onTaskQueued) + + atom.commands.add('body', {'core:undo': => @undo() }) + atom.commands.add('body', {'core:redo': => @redo() }) + + _onTaskQueued: (task) => + if task.canBeUndone() and not task.isUndo() + @_redo = [] + @_undo.push(task) + + undo: => + topTask = @_undo.pop() + return unless topTask + + Actions.queueTask(topTask.createUndoTask()) + @_redo.push(topTask.createIdenticalTask()) + + redo: => + redoTask = @_redo.pop() + return unless redoTask + Actions.queueTask(redoTask) + + print: -> + console.log("Undo Stack") + console.log(@_undo) + console.log("Redo Stack") + console.log(@_redo) + +module.exports = new UndoRedoStore() diff --git a/src/flux/tasks/add-remove-tags.coffee b/src/flux/tasks/add-remove-tags.coffee index fa263f9ca..37e2ef5fa 100644 --- a/src/flux/tasks/add-remove-tags.coffee +++ b/src/flux/tasks/add-remove-tags.coffee @@ -1,6 +1,8 @@ Task = require './task' +{APIError} = require '../errors' NylasAPI = require '../nylas-api' DatabaseStore = require '../stores/database-store' +NamespaceStore = require '../stores/namespace-store' Actions = require '../actions' Tag = require '../models/tag' Thread = require '../models/thread' @@ -10,71 +12,114 @@ async = require 'async' module.exports = class AddRemoveTagsTask extends Task - constructor: (@thread, @tagIdsToAdd = [], @tagIdsToRemove = []) -> + constructor: (@threadsOrIds, @tagIdsToAdd = [], @tagIdsToRemove = []) -> + # For backwards compatibility, allow someone to make the task with a single thread + # object or it's ID + if @threadsOrIds instanceof Thread or _.isString(@threadsOrIds) + @threadsOrIds = [@threadsOrIds] super label: -> "Applying tags..." - performLocal: (versionIncrement = 1) -> - new Promise (resolve, reject) => - if not @thread or not @thread instanceof Thread - return reject(new Error("Attempt to call AddRemoveTagsTask.performLocal without Thread")) + threadIds: -> + @threadsOrIds.map (t) -> if t instanceof Thread then t.id else t - # collect all of the models we need. - needed = {} - for id in @tagIdsToAdd - if id in ['archive', 'unread', 'inbox', 'unseen'] - needed["tag-#{id}"] = new Tag(id: id, name: id) - else - needed["tag-#{id}"] = DatabaseStore.find(Tag, id) + # Undo & Redo support - Promise.props(needed).then (objs) => - # Always apply our changes to a new copy of the thread. - # In some scenarios it may actually be frozen - thread = new Thread(@thread) + canBeUndone: -> + true - @namespaceId = thread.namespaceId + isUndo: -> + @_isUndoTask is true - # increment the thread version number - thread.version += versionIncrement + createUndoTask: -> + task = new AddRemoveTagsTask(@threadIds(), @tagIdsToRemove, @tagIdsToAdd) + task._isUndoTask = true + task - # filter the tags array to exclude tags we're removing and tags we're adding. - # Removing before adding is a quick way to make sure they're only in the set - # once. (super important) - thread.tags = _.filter thread.tags, (tag) => - @tagIdsToRemove.indexOf(tag.id) is -1 and @tagIdsToAdd.indexOf(tag.id) is -1 + # Core Behavior - # add tags in the add list - for id in @tagIdsToAdd - tag = objs["tag-#{id}"] - thread.tags.push(tag) if tag + # To ensure that complex offline actions are synced correctly, tag additions + # and removals need to be applied in order. (For example, star many threads, + # and then unstar one.) + shouldWaitForTask: (other) -> + # Only wait on other tasks that are older and also involve the same threads + return unless other instanceof AddRemoveTagsTask + otherOlder = other.creationDate < @creationDate + otherSameThreads = _.intersection(other.threadIds(), @threadIds()).length > 0 + return otherOlder and otherSameThreads - DatabaseStore.persistModel(thread).then(resolve) + performLocal: ({reverting} = {}) -> + if not @threadsOrIds or not @threadsOrIds instanceof Array + return Promise.reject(new Error("Attempt to call AddRemoveTagsTask.performLocal without threads")) + + # collect all of the tag models we need. + needed = {} + for id in @tagIdsToAdd + if id in ['archive', 'unread', 'inbox', 'unseen'] + needed["tag-#{id}"] = new Tag(id: id, name: id) + else + needed["tag-#{id}"] = DatabaseStore.find(Tag, id) + + Promise.props(needed).then (objs) => + promises = @threadsOrIds.map (item) => + getThread = Promise.resolve(item) + if _.isString(item) + getThread = DatabaseStore.find(Thread, item) + + getThread.then (thread) => + # Always apply our changes to a new copy of the thread. + # In some scenarios it may actually be frozen + thread = new Thread(thread) + + # Mark that we are optimistically changing this model. This will prevent + # inbound delta syncs from changing it back to it's old state. Only the + # operation that changes `optimisticChangeCount` back to zero will + # apply the server's version of the model to our cache. + if reverting is true + NylasAPI.decrementOptimisticChangeCount(Thread, thread.id) + else + NylasAPI.incrementOptimisticChangeCount(Thread, thread.id) + + # filter the tags array to exclude tags we're removing and tags we're adding. + # Removing before adding is a quick way to make sure they're only in the set + # once. (super important) + thread.tags = _.filter thread.tags, (tag) => + @tagIdsToRemove.indexOf(tag.id) is -1 and @tagIdsToAdd.indexOf(tag.id) is -1 + + # add tags in the add list + for id in @tagIdsToAdd + tag = objs["tag-#{id}"] + thread.tags.push(tag) if tag + + return DatabaseStore.persistModel(thread) + + Promise.all(promises) performRemote: -> - new Promise (resolve, reject) => - # queue the operation to the server + nsid = NamespaceStore.current()?.id + promises = @threadIds().map (id) => NylasAPI.makeRequest - path: "/n/#{@namespaceId}/threads/#{@thread.id}" + path: "/n/#{nsid}/threads/#{id}" method: 'PUT' body: add_tags: @tagIdsToAdd, remove_tags: @tagIdsToRemove returnsModel: true - success: resolve - error: reject + beforeProcessing: (body) -> + NylasAPI.decrementOptimisticChangeCount(Thread, id) + body - onAPIError: (apiError) -> - if apiError.response.statusCode is 404 - # Do nothing - NylasAPI will destroy the object. - else - @_rollbackLocal() - Promise.resolve() + Promise.all(promises) + .then => + return Promise.resolve(Task.Status.Finished) - _rollbackLocal: -> - # Run performLocal backwards to undo the tag changes - a = @tagIdsToAdd - @tagIdsToAdd = @tagIdsToRemove - @tagIdsToRemove = a - @performLocal(-1) + .catch APIError, (err) => + if err.statusCode in NylasAPI.PermanentErrorCodes + # Run performLocal backwards to undo the tag changes + [@tagIdsToAdd, @tagIdsToRemove] = [@tagIdsToRemove, @tagIdsToAdd] + @performLocal({reverting: true}).then => + return Promise.resolve(Task.Status.Finished) + else + return Promise.resolve(Task.Status.Retry) diff --git a/src/flux/tasks/create-metadata-task.coffee b/src/flux/tasks/create-metadata-task.coffee index 081624cde..320cf533e 100644 --- a/src/flux/tasks/create-metadata-task.coffee +++ b/src/flux/tasks/create-metadata-task.coffee @@ -1,5 +1,6 @@ _ = require 'underscore' Task = require './task' +{APIError} = require '../errors' Actions = require '../actions' Metadata = require '../models/metadata' EdgehillAPI = require '../edgehill-api' @@ -29,37 +30,22 @@ class CreateMetadataTask extends Task @metadatum = new Metadata({@type, @publicId, @key, @value}) return DatabaseStore.persistModel(@metadatum) - performRemote: -> new Promise (resolve, reject) => - EdgehillAPI.request - method: "POST" - path: "/metadata/#{NamespaceStore.current().id}/#{@type}/#{@publicId}" - body: - key: @key - value: @value - success: (args...) => - Actions.metadataCreated @type, @metadatum - resolve(args...) - error: (apiError) -> - apiError.notifyConsole() - reject(apiError) - - onAPIError: (apiError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "APIError" - error: apiError - Promise.resolve() - - onOtherError: (otherError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "OtherError" - error: otherError - Promise.resolve() - - onTimeoutError: (timeoutError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "TimeoutError" - error: timeoutError - Promise.resolve() + performRemote: -> + new Promise (resolve, reject) => + EdgehillAPI.request + method: "POST" + path: "/metadata/#{NamespaceStore.current().id}/#{@type}/#{@publicId}" + body: + key: @key + value: @value + success: => + Actions.metadataCreated @type, @metadatum + resolve(Task.Status.Finished) + error: (apiError) => + Actions.metadataError _.extend @_baseErrorData(), + errorType: "APIError" + error: apiError + reject(apiError) _baseErrorData: -> action: "create" @@ -68,6 +54,3 @@ class CreateMetadataTask extends Task publicId: @publicId key: @key value: @value - - onOfflineError: (offlineError) -> - Promise.resolve() diff --git a/src/flux/tasks/destroy-draft.coffee b/src/flux/tasks/destroy-draft.coffee index 2f3cead56..5824a813e 100644 --- a/src/flux/tasks/destroy-draft.coffee +++ b/src/flux/tasks/destroy-draft.coffee @@ -1,4 +1,5 @@ Task = require './task' +{APIError} = require '../errors' Message = require '../models/message' DatabaseStore = require '../stores/database-store' Actions = require '../actions' @@ -14,55 +15,47 @@ class DestroyDraftTask extends Task shouldDequeueOtherTask: (other) -> (other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId) or (other instanceof SendDraftTask and other.draftLocalId is @draftLocalId) or - (other instanceof FileUploadTask and other.draftLocalId is @draftLocalId) + (other instanceof FileUploadTask and other.messageLocalId is @draftLocalId) shouldWaitForTask: (other) -> (other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId) performLocal: -> - new Promise (resolve, reject) => - unless @draftLocalId? - return reject(new Error("Attempt to call DestroyDraftTask.performLocal without @draftLocalId")) + unless @draftLocalId? + return Promise.reject(new Error("Attempt to call DestroyDraftTask.performLocal without @draftLocalId")) - DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - return resolve() unless draft - @draft = draft - DatabaseStore.unpersistModel(draft).then(resolve) + DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => + return resolve() unless draft + @draft = draft + DatabaseStore.unpersistModel(draft) performRemote: -> - new Promise (resolve, reject) => - # We don't need to do anything if we weren't able to find the draft - # when we performed locally, or if the draft has never been synced to - # the server (id is still self-assigned) - return resolve() unless @draft - return resolve() unless @draft.isSaved() + # We don't need to do anything if we weren't able to find the draft + # when we performed locally, or if the draft has never been synced to + # the server (id is still self-assigned) + return Promise.resolve() unless @draft + return Promise.resolve() unless @draft.isSaved() - atom.inbox.makeRequest - path: "/n/#{@draft.namespaceId}/drafts/#{@draft.id}" - method: "DELETE" - body: - version: @draft.version - returnsModel: false - success: resolve - error: reject + atom.inbox.makeRequest + path: "/n/#{@draft.namespaceId}/drafts/#{@draft.id}" + method: "DELETE" + body: + version: @draft.version + returnsModel: false - onAPIError: (apiError) -> - inboxMsg = apiError.body?.message ? "" - if apiError.statusCode is 404 - # Draft has already been deleted, this is not really an error - return true - else if inboxMsg.indexOf("is not a draft") >= 0 - # Draft has been sent, and can't be deleted. Not much we can - # do but finish - return true - else - @_rollbackLocal() + recoverFromRemoteAPIError: (err) -> + inboxMsg = err.body?.message ? "" - onOtherError: -> Promise.resolve() - onTimeoutError: -> Promise.resolve() - onOfflineError: -> Promise.resolve() + # Draft has already been deleted, this is not really an error + if err.statusCode is 404 + return Promise.resolve(Task.Status.Finished) - _rollbackLocal: (msg) -> - msg ?= "Unable to delete this draft. Restoring..." - Actions.postNotification({message: msg, type: "error"}) - DatabaseStore.persistModel(@draft) if @draft? + if err.statusCode in NylasAPI.PermanentErrorCodes + Actions.postNotification({message: "Unable to delete this draft. Restoring...", type: "error"}) + return DatabaseStore.persistModel(@draft) + + # Draft has been sent, and can't be deleted. Not much we can do but finish + if inboxMsg.indexOf("is not a draft") >= 0 + return Promise.resolve(Task.Status.Finished) + + Promise.resolve(Task.Status.Retry) diff --git a/src/flux/tasks/destroy-metadata-task.coffee b/src/flux/tasks/destroy-metadata-task.coffee index e7b563c57..92136714f 100644 --- a/src/flux/tasks/destroy-metadata-task.coffee +++ b/src/flux/tasks/destroy-metadata-task.coffee @@ -1,5 +1,6 @@ _ = require 'underscore' Task = require './task' +{APIError} = require '../errors' Actions = require '../actions' Metadata = require '../models/metadata' EdgehillAPI = require '../edgehill-api' @@ -54,37 +55,18 @@ class DestroyMetadataTask extends Task method: "DELETE" path: "/metadata/#{NamespaceStore.current().id}/#{@type}/#{@publicId}" body: body - success: (args...) => + success: => Actions.metadataDestroyed(@type) - resolve(args...) - error: (apiError) -> - apiError.notifyConsole() + resolve(Task.Status.Finished) + error: (apiError) => + Actions.metadataError _.extend @_baseErrorData(), + errorType: "APIError" + error: apiError reject(apiError) - onAPIError: (apiError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "APIError" - error: apiError - Promise.resolve() - - onOtherError: (otherError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "OtherError" - error: otherError - Promise.resolve() - - onTimeoutError: (timeoutError) -> - Actions.metadataError _.extend @_baseErrorData(), - errorType: "TimeoutError" - error: timeoutError - Promise.resolve() - _baseErrorData: -> action: "destroy" className: @constructor.name type: @type publicId: @publicId key: @key - - onOfflineError: (offlineError) -> - Promise.resolve() diff --git a/src/flux/tasks/file-upload-task.coffee b/src/flux/tasks/file-upload-task.coffee index 95195f08b..50dff4449 100644 --- a/src/flux/tasks/file-upload-task.coffee +++ b/src/flux/tasks/file-upload-task.coffee @@ -2,6 +2,7 @@ fs = require 'fs' _ = require 'underscore' pathUtils = require 'path' Task = require './task' +{APIError} = require '../errors' File = require '../models/file' Message = require '../models/message' Actions = require '../actions' @@ -15,14 +16,16 @@ idGen = 2 class FileUploadTask extends Task + # Necessary so that tasks always get the same ID during specs @idGen: -> idGen constructor: (@filePath, @messageLocalId) -> super @_startedUploadingAt = Date.now() + @progress = null # The progress checking timer. + @_uploadId = FileUploadTask.idGen() idGen += 1 - @progress = null # The progress checking timer. performLocal: -> return Promise.reject(new Error("Must pass an absolute path to upload")) unless @filePath?.length @@ -31,75 +34,55 @@ class FileUploadTask extends Task Promise.resolve() performRemote: -> - new Promise (resolve, reject) => - Actions.uploadStateChanged @_uploadData("started") - - @req = NylasAPI.makeRequest - path: "/n/#{@_namespaceId()}/files" - method: "POST" - json: false - formData: @_formData() - error: reject - success: (rawResponseString) => - # The Nylas API returns the file json wrapped in an array. - # - # Since we requested `json:false` the response will come back as - # a raw string. - try - json = JSON.parse(rawResponseString) - file = (new File).fromJSON(json[0]) - catch error - reject(error) - @_onRemoteSuccess(file, resolve, reject) + Actions.uploadStateChanged @_uploadData("started") + started = (req) => + @req = req @progress = setInterval => Actions.uploadStateChanged(@_uploadData("progress")) , 250 - cleanup: -> - super - - # If the request is still in progress, notify observers that - # we've failed. - if @req - @req.abort() + cleanup = => clearInterval(@progress) - Actions.uploadStateChanged(@_uploadData("aborted")) - setTimeout => - # To see the aborted state for a little bit - Actions.fileAborted(@_uploadData("aborted")) - , 1000 + @req = null - onAPIError: (apiError) -> - @_rollbackLocal() + NylasAPI.makeRequest + path: "/n/#{@_namespaceId()}/files" + method: "POST" + json: false + formData: @_formData() + started: started - onOtherError: (otherError) -> - @_rollbackLocal() + .finally(cleanup) + .then(@performRemoteParseFile) + .then(@performRemoteAttachFile) + .then (file) => + Actions.uploadStateChanged @_uploadData("completed") + Actions.fileUploaded(file: file, uploadData: @_uploadData("completed")) + return Promise.resolve(Task.Status.Finished) - onTimeoutError: -> - # Do nothing. It could take a while. - Promise.resolve() + .catch APIError, (err) => + Actions.uploadStateChanged(@_uploadData("failed")) + if err.statusCode in NylasAPI.PermanentErrorCodes + msg = "There was a problem uploading this file. Please try again later." + Actions.postNotification({message: msg, type: "error"}) + return Promise.reject(err) + else + return Promise.resolve(Task.Status.Retry) - onOfflineError: (offlineError) -> - msg = "You can't upload a file while you're offline." - @_rollbackLocal(msg) - - _rollbackLocal: (msg) -> - clearInterval(@progress) - @req = null - - msg ?= "There was a problem uploading this file. Please try again later." - Actions.postNotification({message: msg, type: "error"}) - Actions.uploadStateChanged @_uploadData("failed") - - _onRemoteSuccess: (file, resolve, reject) => - clearInterval(@progress) - @req = null + performRemoteParseFile: (rawResponseString) => + # The Nylas API returns the file json wrapped in an array. + # Since we requested `json:false` the response will come back as + # a raw string. + json = JSON.parse(rawResponseString) + file = (new File).fromJSON(json[0]) + Promise.resolve(file) + performRemoteAttachFile: (file) => # The minute we know what file is associated with the upload, we need # to fire an Action to notify a popout window's FileUploadStore that # these two objects are linked. We unfortunately can't wait until - # `_attacheFileToDraft` resolves, because that will resolve after the + # `_attachFileToDraft` resolves, because that will resolve after the # DB transaction is completed AND all of the callbacks have fired. # Unfortunately in the callback chain is a render method which means # that the upload will be left on the page for a split second before @@ -110,19 +93,28 @@ class FileUploadTask extends Task # listing. Actions.linkFileToUpload(file: file, uploadData: @_uploadData("completed")) - @_attachFileToDraft(file).then => - Actions.uploadStateChanged @_uploadData("completed") - Actions.fileUploaded(file: file, uploadData: @_uploadData("completed")) - resolve() - .catch(reject) - - _attachFileToDraft: (file) -> DraftStore = require '../stores/draft-store' DraftStore.sessionForLocalId(@messageLocalId).then (session) => files = _.clone(session.draft().files) ? [] files.push(file) session.changes.add({files}) - return session.changes.commit() + session.changes.commit().then -> + Promise.resolve(file) + + cancel: -> + super + + # Note: When you call cancel, we stop the request, which causes + # NylasAPI.makeRequest to reject with an error. + return unless @req + @req.abort() + clearInterval(@progress) + + # To see the aborted state for a little bit + Actions.uploadStateChanged(@_uploadData("aborted")) + setTimeout(( => Actions.fileAborted(@_uploadData("aborted"))), 1000) + + # Helper Methods _formData: -> file: # Must be named `file` as per the Nylas API spec @@ -157,6 +149,7 @@ class FileUploadTask extends Task # http://stackoverflow.com/questions/12098713/upload-progress-request @req?.req?.connection?._bytesDispatched ? 0 - _namespaceId: -> NamespaceStore.current()?.id + _namespaceId: -> + NamespaceStore.current()?.id module.exports = FileUploadTask diff --git a/src/flux/tasks/mark-message-read.coffee b/src/flux/tasks/mark-message-read.coffee index 973f15788..68ba91e7f 100644 --- a/src/flux/tasks/mark-message-read.coffee +++ b/src/flux/tasks/mark-message-read.coffee @@ -1,4 +1,5 @@ Task = require './task' +{APIError} = require '../errors' DatabaseStore = require '../stores/database-store' Actions = require '../actions' NylasAPI = require '../nylas-api' @@ -11,37 +12,28 @@ class MarkMessageReadTask extends Task super performLocal: -> - new Promise (resolve, reject) => - # update the flag on the message - @_previousUnreadState = @message.unread - @message.unread = false + # update the flag on the message + @_previousUnreadState = @message.unread + @message.unread = false - # dispatch an action to persist it - DatabaseStore.persistModel(@message).then(resolve).catch(reject) + # dispatch an action to persist it + DatabaseStore.persistModel(@message) performRemote: -> - new Promise (resolve, reject) => - # queue the operation to the server - NylasAPI.makeRequest { - path: "/n/#{@message.namespaceId}/messages/#{@message.id}" - method: 'PUT' - body: { - unread: false - } - returnsModel: true - success: resolve - error: reject - } - - # We don't really care if this fails. - onAPIError: -> Promise.resolve() - onOtherError: -> Promise.resolve() - onTimeoutError: -> Promise.resolve() - onOfflineError: -> Promise.resolve() - - _rollbackLocal: -> - new Promise (resolve, reject) => - unless @_previousUnreadState? - reject(new Error("Cannot call rollbackLocal without previous call to performLocal")) - @message.unread = @_previousUnreadState - DatabaseStore.persistModel(@message).then(resolve).catch(reject) + # queue the operation to the server + NylasAPI.makeRequest + path: "/n/#{@message.namespaceId}/messages/#{@message.id}" + method: 'PUT' + body: + unread: false + returnsModel: true + .then => + return Promise.resolve(Task.Status.Finished) + .catch APIError, (err) => + if err.statusCode in NylasAPI.PermanentErrorCodes + # Run performLocal backwards to undo the tag changes + @message.unread = @_previousUnreadState + DatabaseStore.persistModel(@message).then => + return Promise.resolve(Task.Status.Finished) + else + return Promise.resolve(Task.Status.Retry) diff --git a/src/flux/tasks/mark-thread-read.coffee b/src/flux/tasks/mark-thread-read.coffee index 3fe8551e5..05188920e 100644 --- a/src/flux/tasks/mark-thread-read.coffee +++ b/src/flux/tasks/mark-thread-read.coffee @@ -1,4 +1,5 @@ Task = require './task' +{APIError} = require '../errors' DatabaseStore = require '../stores/database-store' AddRemoveTagsTask = require './add-remove-tags' Message = require '../models/message' diff --git a/src/flux/tasks/send-draft.coffee b/src/flux/tasks/send-draft.coffee index 169212f02..255c279e4 100644 --- a/src/flux/tasks/send-draft.coffee +++ b/src/flux/tasks/send-draft.coffee @@ -3,6 +3,7 @@ Actions = require '../actions' DatabaseStore = require '../stores/database-store' Message = require '../models/message' +{APIError} = require '../errors' Task = require './task' TaskQueue = require '../stores/task-queue' SyncbackDraftTask = require './syncback-draft' @@ -50,61 +51,38 @@ class SendDraftTask extends Task # Pass joined:true so the draft body is included body = draft.toJSON(joined: true) - return @_performRemoteSend(body) + return @_send(body) # Returns a promise which resolves when the draft is sent. There are several # failure cases where this method may call itself, stripping bad fields out of # the body. This promise only rejects when these changes have been tried. - _performRemoteSend: (body) -> - @_performRemoteAPIRequest(body) + _send: (body) -> + NylasAPI.makeRequest + path: "/n/#{@draft.namespaceId}/send" + method: 'POST' + body: body + returnsModel: true + .then (json) => message = (new Message).fromJSON(json) atom.playSound('mail_sent.ogg') Actions.sendDraftSuccess draftLocalId: @draftLocalId newMessage: message - return DatabaseStore.unpersistModel(@draft) + DatabaseStore.unpersistModel(@draft).then => + return Promise.resolve(Task.Status.Finished) - .catch (err) => + .catch APIError, (err) => if err.message?.indexOf('Invalid message public id') is 0 body.reply_to_message_id = null - return @_performRemoteSend(body) + return @_send(body) else if err.message?.indexOf('Invalid thread') is 0 body.thread_id = null body.reply_to_message_id = null - return @_performRemoteSend(body) + return @_send(body) + else if err.statusCode in NylasAPI.PermanentErrorCodes + msg = err.message ? "Your draft could not be sent." + Actions.composePopoutDraft(@draftLocalId, {errorMessage: msg}) + return Promise.resolve(Task.Status.Finished) else - return Promise.reject(err) - - _performRemoteAPIRequest: (body) -> - new Promise (resolve, reject) => - NylasAPI.makeRequest - path: "/n/#{@draft.namespaceId}/send" - method: 'POST' - body: body - returnsModel: true - success: resolve - error: reject - - onAPIError: (apiError) -> - msg = apiError.message ? "Our server is having problems. Your message has not been sent." - @_notifyError(msg) - - onOtherError: -> - msg = "We had a serious issue while sending. Your message has not been sent." - @_notifyError(msg) - - onTimeoutError: -> - msg = "The server is taking an abnormally long time to respond. Your message has not been sent." - @_notifyError(msg) - - onOfflineError: -> - msg = "You are offline. Your message has NOT been sent. Please send your message when you come back online." - @_notifyError(msg) - # For sending draft, we don't send when we come back online. - Actions.dequeueTask(@) - - _notifyError: (msg) -> - @notifyErrorMessage(msg) - if @fromPopout - Actions.composePopoutDraft(@draftLocalId, {errorMessage: msg}) + return Promise.resolve(Task.Status.Retry) diff --git a/src/flux/tasks/syncback-draft.coffee b/src/flux/tasks/syncback-draft.coffee index 5a5c26fd1..523460c70 100644 --- a/src/flux/tasks/syncback-draft.coffee +++ b/src/flux/tasks/syncback-draft.coffee @@ -6,6 +6,7 @@ DatabaseStore = require '../stores/database-store' NylasAPI = require '../nylas-api' Task = require './task' +{APIError} = require '../errors' Message = require '../models/message' FileUploadTask = require './file-upload-task' @@ -17,7 +18,6 @@ class SyncbackDraftTask extends Task constructor: (@draftLocalId) -> super - @_saveAttempts = 0 shouldDequeueOtherTask: (other) -> other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId and other.creationDate < @creationDate @@ -29,99 +29,55 @@ class SyncbackDraftTask extends Task # SyncbackDraftTask does not do anything locally. You should persist your changes # to the local database directly or using a DraftStoreProxy, and then queue a # SyncbackDraftTask to send those changes to the server. - console.log('in performLocal') - if not @draftLocalId? + if not @draftLocalId errMsg = "Attempt to call FileUploadTask.performLocal without @draftLocalId" - Promise.reject(new Error(errMsg)) - else - Promise.resolve() + return Promise.reject(new Error(errMsg)) + Promise.resolve() performRemote: -> - new Promise (resolve, reject) => - DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - # The draft may have been deleted by another task. Nothing we can do. - return resolve() unless draft - - if draft.isSaved() - path = "/n/#{draft.namespaceId}/drafts/#{draft.id}" - method = 'PUT' - else - path = "/n/#{draft.namespaceId}/drafts" - method = 'POST' - - body = draft.toJSON() - delete body['from'] - - initialId = draft.id - - @_saveAttempts += 1 - NylasAPI.makeRequest - path: path - method: method - body: body - returnsModel: false - success: (json) => - if json.id != initialId - newDraft = (new Message).fromJSON(json) - DatabaseStore.swapModel(oldModel: draft, newModel: newDraft, localId: @draftLocalId).then(resolve) - else - DatabaseStore.persistModel(draft).then(resolve) - error: reject - - onAPIError: (apiError) -> - # If we get a 404 from the server this might mean that the - # draft has been deleted from underneath us. We should retry - # again. Before we can retry we need to set the ID to a - # localID so that the next time this fires the model will - # trigger a POST instead of a PUT - if apiError.statusCode is 404 - msg = "It looks like the draft you're working on got deleted from underneath you. We're creating a new draft and saving your work." - @_retrySaveAsNewDraft(msg) - else - if @_saveAttempts <= 1 - msg = "We had a problem with the server. We're going to try and save your draft again." - @_retrySaveToExistingDraft(msg) - else - msg = "We're continuing to have issues saving your draft. It will be saved locally, but is failing to save on the server." - @notifyErrorMessage(msg) - - onOtherError: -> - msg = "We had a serious issue trying to save your draft. Please copy the text out of the composer and try again later." - @notifyErrorMessage(msg) - - onTimeoutError: -> - if @_saveAttempts <= 1 - msg = "The server is taking an abnormally long time to respond. We're going to try and save your changes again." - @_retrySaveToExistingDraft(msg) - else - msg = "We're continuing to have issues saving your draft. It will be saved locally, but is failing to save on the server." - @notifyErrorMessage(msg) - - onOfflineError: -> - msg = "WARNING: You are offline. Your edits are being saved locally. They will save to the server when you come back online" - @notifyErrorMessage(msg) - - _retrySaveAsNewDraft: (msg) -> - TaskQueue = require '../stores/task-queue' DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - if not draft? - console.log "Couldn't find draft!", @draftLocalId - @_onOtherError() + # The draft may have been deleted by another task. Nothing we can do. + return Promise.resolve() unless draft + if draft.isSaved() + path = "/n/#{draft.namespaceId}/drafts/#{draft.id}" + method = 'PUT' + else + path = "/n/#{draft.namespaceId}/drafts" + method = 'POST' + + body = draft.toJSON() + delete body['from'] + + initialId = draft.id + + NylasAPI.makeRequest + path: path + method: method + body: body + returnsModel: false + + .then (json) => + if json.id != initialId + newDraft = (new Message).fromJSON(json) + DatabaseStore.swapModel(oldModel: draft, newModel: newDraft, localId: @draftLocalId) + else + DatabaseStore.persistModel(draft) + + .catch APIError, (err) => + if err.statusCode in NylasAPI.PermanentErrorCodes + if err.requestOptions.method is 'PUT' + return @disassociateFromRemoteID().then => + Promise.resolve(Task.Status.Retry) + else + return Promise.resolve(Task.Status.Finished) + else + return Promise.resolve(Task.Status.Retry) + + disassociateFromRemoteID: -> + DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => + return Promise.resolve() unless draft newJSON = _.clone(draft.toJSON()) newJSON.id = generateTempId() unless isTempId(draft.id) newDraft = (new Message).fromJSON(newJSON) - DatabaseStore.swapModel(oldModel: draft, newModel: newDraft, localId: @draftLocalId).then => - TaskQueue.enqueue @ - - @notifyErrorMessage(msg) - - _retrySaveToExistingDraft: (msg) -> - TaskQueue = require '../stores/task-queue' - DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - if not draft? - console.log "Couldn't find draft!", @draftLocalId - @_onOtherError() - TaskQueue.enqueue @ - - @notifyErrorMessage(msg) + DatabaseStore.swapModel(oldModel: draft, newModel: newDraft, localId: @draftLocalId) diff --git a/src/flux/tasks/task.coffee b/src/flux/tasks/task.coffee index 75b443b70..1117dbf70 100644 --- a/src/flux/tasks/task.coffee +++ b/src/flux/tasks/task.coffee @@ -5,93 +5,137 @@ Actions = require '../actions' OfflineError, TimeoutError} = require '../errors' -# Tasks represent individual changes to the datastore that +TaskStatus = + Finished: 'finished' + Retry: 'retry' + +# Public: Tasks represent individual changes to the datastore that # alter the local cache and need to be synced back to the server. - -# Tasks should optimistically modify local models and trigger -# model update actions, and also make API calls which trigger -# further model updates once they're complete. - -# Subclasses implement `performLocal` and `performRemote`. # -# `performLocal` can be called directly by whoever has access to the -# class. It can only be called once. If it is not called directly, -# `performLocal` will be invoked as soon as the task is queued. Since -# performLocal is frequently asynchronous, it is sometimes necessary to -# wait for it to finish. +# To create a new task, subclass Task and implement the following methods: +# +# - performLocal: +# Return a {Promise} that does work immediately. Must resolve or the task +# will be thrown out. Generally, you should optimistically update +# the local cache here. +# +# - performRemote: +# Do work that requires dependencies to have resolved and may need to be +# tried multiple times to succeed in case of network issues. +# +# performRemote must return a {Promise}, and it should always resolve with +# Task.Status.Finished or Task.Status.Retry. Rejections are considered +# exception cases and are logged to our server. +# +# Returning Task.Status.Retry will cause the TaskQueue to leave your task +# on the queue and run it again later. You should only return Task.Status.Retry +# if your task encountered a transient error (for example, a `0` but not a `400`). +# +# - shouldWaitForTask: +# Tasks may be arbitrarily dependent on other tasks. To ensure that +# performRemote is called at the right time, subclasses should implement +# `shouldWaitForTask(other)`. For example, the `SendDraft` task is dependent +# on the draft's files' `UploadFile` tasks completing. # -# `performRemote` may be called after a delay, depending on internet -# connectivity and dependency resolution. - -# Tasks may be arbitrarily dependent on other tasks. To ensure that -# performRemote is called at the right time, subclasses should implement -# shouldWaitForTask(other). For example, the SendDraft task is dependent -# on the draft's files' UploadFile tasks completing. - # Tasks may also implement shouldDequeueOtherTask(other). Returning true # will cause the other event to be removed from the queue. This is useful in # offline mode especially, when the user might Save,Save,Save,Save,Send. # Each newly queued Save can cancel the (unstarted) save task in the queue. - -# Because tasks may be queued and performed when internet is available, -# they may need to be persisted to disk. Subclasses should implement -# serialize / deserialize to convert to / from raw JSON. - +# +# Tasks that need to support undo/redo should implement `canBeUndone`, `isUndo`, +# `createUndoTask`, and `createIdenticalTask`. +# class Task - ## These are commonly overridden ## + + @Status: TaskStatus + constructor: -> + @_performLocalCompletePromise = new Promise (resolve, reject) => + @_performLocalComplete = resolve + @id = generateTempId() @creationDate = new Date() + @queueState = + isProcessing: false + localError: null + localComplete: false + remoteError: null + remoteAttempts: 0 + remoteComplete: false + @ - performLocal: -> Promise.resolve() + runLocal: -> + if @queueState.localComplete + return Promise.resolve() + else + @performLocal() + .then => + @_performLocalComplete() + @queueState.localComplete = true + @queueState.localError = null + return Promise.resolve() + .catch (err) => + @queueState.localError = err + return Promise.reject(err) - performRemote: -> Promise.resolve() + runRemote: -> + if @queueState.localComplete is false + throw new Error("runRemote called before performLocal complete, this is an assertion failure.") + + if @queueState.remoteComplete + return Promise.resolve(Task.Status.Finished) + + @performRemote() + .catch (err) => + @queueState.remoteAttempts += 1 + @queueState.remoteError = err + .then (status) => + if not (status in _.values(Task.Status)) + throw new Error("performRemote returned #{status}, which is not a Task.Status") + @queueState.remoteAttempts += 1 + @queueState.remoteComplete = status is Task.Status.Finished + @queueState.remoteError = null + return Promise.resolve(status) + + + ## Everything beneath here may be overridden in subclasses ## + + # performLocal is called once when the task is queued. You must return + # a promise. If you resolve, the task is queued and performRemote will + # be called. If you reject, the task will not be queued. + # + performLocal: -> + Promise.resolve() + + performRemote: -> + Promise.resolve(Task.Status.Finished) + + waitForPerformLocal: -> + if not atom.isMainWindow() + throw new Error("waitForPerformLocal is only supported in the main window. In + secondary windows, tasks are serialized and sent to the main + window, and cannot be observed.") + @_performLocalCompletePromise + + cancel: -> + # We ignore requests to cancel and carry on. Subclasses that want to support + # cancellation or dequeue requests while running should implement cancel. + + canBeUndone: -> false + + isUndo: -> false + + createUndoTask: -> throw new Error("Unimplemented") + + createIdenticalTask: -> + json = @toJSON() + delete json['queueState'] + (new @.constructor).fromJSON(json) shouldDequeueOtherTask: (other) -> false shouldWaitForTask: (other) -> false - cleanup: -> true - - abort: -> Promise.resolve() - - onAPIError: (apiError) -> - msg = "We had a problem with the server. Your action was NOT completed." - Actions.postNotification({message: msg, type: "error"}) - Promise.resolve() - - onOtherError: (otherError) -> - msg = "Something went wrong. Please report this issue immediately." - Actions.postNotification({message: msg, type: "error"}) - Promise.resolve() - - onTimeoutError: (timeoutError) -> - msg = "This took too long. Check your internet connection. Your action was NOT completed." - Actions.postNotification({message: msg, type: "error"}) - Promise.resolve() - - onOfflineError: (offlineError) -> - msg = "WARNING: You are offline. This will complete when you come back online." - Actions.postNotification({message: msg, type: "error"}) - Promise.resolve() - - ## Only override if you know what you're doing ## - onError: (error) -> - if error instanceof APIError - @onAPIError(error) - else if error instanceof TimeoutError - @onTimeoutError(error) - else if error instanceof OfflineError - @onOfflineError(error) - else - if error instanceof Error - console.error "Task #{@constructor.name} threw an unknown error: #{error.message}" - console.error error.stack - @onOtherError(error) - - notifyErrorMessage: (msg) -> - Actions.postNotification({message: msg, type: "error"}) - toJSON: -> json = _.clone(@) json['object'] = @constructor.name diff --git a/src/tasks/ship-logs-task.coffee b/src/tasks/ship-logs-task.coffee index 7c82a3fb3..da8cdc502 100644 --- a/src/tasks/ship-logs-task.coffee +++ b/src/tasks/ship-logs-task.coffee @@ -40,8 +40,6 @@ module.exports = (dir, regexPattern) -> else AWSModulePath = 'aws-sdk' - console.log("Load AWS module from #{AWSModulePath}") - # Note: These credentials are only good for uploading to this # specific bucket and can't be used for anything else. AWS = require(AWSModulePath) diff --git a/static/components/spinner.less b/static/components/spinner.less index edc1fc2a7..4d1d0ad84 100644 --- a/static/components/spinner.less +++ b/static/components/spinner.less @@ -6,6 +6,7 @@ text-align: center; opacity: 1; -webkit-transition: opacity 0.2s linear; //transition + pointer-events: none; } .spinner.hidden { @@ -57,10 +58,10 @@ } @keyframes bouncedelay { - 0%, 80%, 100% { + 0%, 80%, 100% { transform: scale(0.0); -webkit-transform: scale(0.0); - } 40% { + } 40% { transform: scale(1.0); -webkit-transform: scale(1.0); }