fix(sync): Pull down and associate metadata during initial sync

Summary:
Snooze should wait for categories on all accounts

Fix authPlugin to rembmer `plugin+accountId`, not pluginId, add specs

categories() returned [], categories(acctId) returned {}

dry up sync worker, fetch metadata before anything else

Test Plan: Run tests

Reviewers: drew, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D2693

internal_packages/thread-snooze/lib/snooze-utils.js

@@ -58,8 +58,8 @@ const SnoozeUtils = {
     })
   },
 
-  whenCategoriesReady() {
-    const categoriesReady = ()=> CategoryStore.categories().length > 0
+  whenCategoriesReady(accountId) {
+    const categoriesReady = ()=> CategoryStore.categories(accountId).length > 0;
     if (!categoriesReady()) {
       return new Promise((resolve)=> {
         const unsubscribe = CategoryStore.listen(()=> {
@@ -74,7 +74,7 @@ const SnoozeUtils = {
   },
 
   getSnoozeCategory(accountId, categoryName = SNOOZE_CATEGORY_NAME) {
-    return SnoozeUtils.whenCategoriesReady()
+    return SnoozeUtils.whenCategoriesReady(accountId)
     .then(()=> {
       const allCategories = CategoryStore.categories(accountId)
       const category = _.findWhere(allCategories, {displayName: categoryName})

internal_packages/worker-sync/lib/nylas-sync-worker.coffee

@@ -106,23 +106,51 @@ class NylasSyncWorker
     # we'll backoff and restart the timer.
     @_resumeTimer.cancel()
 
-    @fetchCollection('threads')
-    if @_account.usesLabels()
-      @fetchCollection('labels', {initialPageSize: 1000})
-    if @_account.usesFolders()
-      @fetchCollection('folders', {initialPageSize: 1000})
-    @fetchCollection('drafts')
-    @fetchCollection('contacts')
-    @fetchCollection('calendars')
-    @fetchCollection('events')
+    needed = [
+      {model: 'threads'},
+      {model: "#{@_account.organizationUnit}s", initialPageSize: 1000}
+      {model: 'drafts'},
+      {model: 'contacts'},
+      {model: 'calendars'},
+      {model: 'events'},
+    ].filter ({model}) =>
+      @shouldFetchCollection(model)
 
-  fetchCollection: (model, options = {}) ->
-    return unless @_state
+    return if needed.length is 0
+
+    @fetchAllMetadata =>
+      needed.forEach ({model, initialPageSize}) =>
+        @fetchCollection(model, initialPageSize)
+
+  fetchAllMetadata: (finished) ->
+    @_metadata = {}
+    makeMetadataRequest = (offset) =>
+      limit = 200
+      @_fetchWithErrorHandling
+        path: "/metadata"
+        qs: {limit, offset}
+        success: (data) =>
+          for metadatum in data
+            @_metadata[metadatum.object_id] ?= []
+            @_metadata[metadatum.object_id].push(metadatum)
+          if data.length is limit
+            makeMetadataRequest(offset + limit)
+          else
+            console.log("Retrieved #{offset + data.length} metadata objects")
+            finished()
+
+    makeMetadataRequest(0)
+
+  shouldFetchCollection: (model) ->
+    return false unless @_state
     state = @_state[model] ? {}
 
-    return if state.complete and not options.force?
-    return if state.busy
+    return false if state.complete
+    return false if state.busy
+    return true
 
+  fetchCollection: (model, initialPageSize = INITIAL_PAGE_SIZE) ->
+    state = @_state[model] ? {}
     state.complete = false
     state.error = null
     state.busy = true
@@ -138,7 +166,7 @@ class NylasSyncWorker
       @fetchCollectionPage(model, {limit, offset})
     else
       @fetchCollectionPage(model, {
-        limit: options.initialPageSize ? INITIAL_PAGE_SIZE,
+        limit: initialPageSize,
         offset: 0
       })
 
@@ -146,23 +174,17 @@ class NylasSyncWorker
     @writeState()
 
   fetchCollectionCount: (model) ->
-    @_api.makeRequest
-      accountId: @_account.id
+    @_fetchWithErrorHandling
       path: "/#{model}"
-      returnsModel: false
-      qs:
-        view: 'count'
+      qs: {view: 'count'}
       success: (response) =>
-        return if @_terminated
         @updateTransferState(model, count: response.count)
-      error: (err) =>
-        return if @_terminated
-        @_resumeTimer.backoff()
-        @_resumeTimer.start()
 
   fetchCollectionPage: (model, params = {}) ->
     requestStartTime = Date.now()
     requestOptions =
+      metadataToAttach: @_metadata
+
       error: (err) =>
         return if @_terminated
         @_fetchCollectionPageError(model, params, err)
@@ -203,6 +225,21 @@ class NylasSyncWorker
   _hasNoInbox: (json) ->
     return not _.any(json, (obj) -> obj.name is "inbox")
 
+  _fetchWithErrorHandling: ({path, qs, success, error}) ->
+    @_api.makeRequest
+      accountId: @_account.id
+      returnsModel: false
+      path: path
+      qs: qs
+      success: (response) =>
+        return if @_terminated
+        success(response) if success
+      error: (err) =>
+        return if @_terminated
+        @_resumeTimer.backoff()
+        @_resumeTimer.start()
+        error(err) if error
+
   _fetchCollectionPageError: (model, params, err) ->
     @_resumeTimer.backoff()
     @_resumeTimer.start()

internal_packages/worker-sync/spec/nylas-sync-worker-spec.coffee

@@ -17,6 +17,7 @@ describe "NylasSyncWorker", ->
         @apiRequests.push({account, model:'threads', params, requestOptions})
 
     @apiCursorStub = undefined
+    spyOn(NylasSyncWorker.prototype, 'fetchAllMetadata').andCallFake (cb) -> cb()
     spyOn(DatabaseTransaction.prototype, 'persistJSONBlob').andReturn(Promise.resolve())
     spyOn(DatabaseStore, 'findJSONBlob').andCallFake (key) =>
       if key is "NylasSyncWorker:#{TEST_ACCOUNT_ID}"
@@ -37,6 +38,7 @@ describe "NylasSyncWorker", ->
 
     @account = new Account(clientId: TEST_ACCOUNT_CLIENT_ID, serverId: TEST_ACCOUNT_ID, organizationUnit: 'label')
     @worker = new NylasSyncWorker(@api, @account)
+    @worker._metadata = {"a": [{"id":"b"}]}
     @connection = @worker.connection()
     spyOn(@connection, 'start')
     advanceClock()
@@ -177,35 +179,58 @@ describe "NylasSyncWorker", ->
       expect(nextState.threads.count).toEqual(1001)
 
   describe "resumeFetches", ->
-    it "should fetch collections", ->
+    it "should fetch metadata first and fetch other collections when metadata is ready", ->
+      fetchAllMetadataCallback = null
+      jasmine.unspy(NylasSyncWorker.prototype, 'fetchAllMetadata')
+      spyOn(NylasSyncWorker.prototype, 'fetchAllMetadata').andCallFake (cb) =>
+        fetchAllMetadataCallback = cb
       spyOn(@worker, 'fetchCollection')
+      @worker._state = {}
       @worker.resumeFetches()
-      expect(@worker.fetchCollection.calls.map (call) -> call.args[0]).toEqual(['threads', 'labels', 'drafts', 'contacts', 'calendars', 'events'])
+      expect(@worker.fetchAllMetadata).toHaveBeenCalled()
+      expect(@worker.fetchCollection.calls.length).toBe(0)
+      fetchAllMetadataCallback()
+      expect(@worker.fetchCollection.calls.length).not.toBe(0)
+
+    it "should fetch collections for which `shouldFetchCollection` returns true", ->
+      spyOn(@worker, 'fetchCollection')
+      spyOn(@worker, 'shouldFetchCollection').andCallFake (collection) =>
+        return collection in ['threads', 'labels', 'drafts']
+      @worker.resumeFetches()
+      expect(@worker.fetchCollection.calls.map (call) -> call.args[0]).toEqual(['threads', 'labels', 'drafts'])
 
     it "should be called when Actions.retryInitialSync is received", ->
       spyOn(@worker, 'resumeFetches').andCallThrough()
       Actions.retryInitialSync()
       expect(@worker.resumeFetches).toHaveBeenCalled()
 
-  describe "fetchCollection", ->
-    beforeEach ->
-      @apiRequests = []
-
-    it "should not start if the collection sync is already in progress", ->
+  describe "shouldFetchCollection", ->
+    it "should return false if the collection sync is already in progress", ->
       @worker._state.threads = {
         'busy': true
         'complete': false
       }
-      @worker.fetchCollection('threads')
-      expect(@apiRequests.length).toBe(0)
+      expect(@worker.shouldFetchCollection('threads')).toBe(false)
 
-    it "should not start if the collection sync is already complete", ->
+    it "should return false if the collection sync is already complete", ->
       @worker._state.threads = {
         'busy': false
         'complete': true
       }
-      @worker.fetchCollection('threads')
-      expect(@apiRequests.length).toBe(0)
+      expect(@worker.shouldFetchCollection('threads')).toBe(false)
+
+    it "should return true otherwise", ->
+      @worker._state.threads = {
+        'busy': false
+        'complete': false
+      }
+      expect(@worker.shouldFetchCollection('threads')).toBe(true)
+      @worker._state.threads = undefined
+      expect(@worker.shouldFetchCollection('threads')).toBe(true)
+
+  describe "fetchCollection", ->
+    beforeEach ->
+      @apiRequests = []
 
     it "should start the request for the model count", ->
       @worker._state.threads = {
@@ -216,7 +241,16 @@ describe "NylasSyncWorker", ->
       expect(@apiRequests[0].requestOptions.path).toBe('/threads')
       expect(@apiRequests[0].requestOptions.qs.view).toBe('count')
 
-    describe "when there is no errorRequestRange saved", ->
+    it "should pass any metadata it preloaded", ->
+      @worker._state.threads = {
+        'busy': false
+        'complete': false
+      }
+      @worker.fetchCollection('threads')
+      expect(@apiRequests[1].model).toBe('threads')
+      expect(@apiRequests[1].requestOptions.metadataToAttach).toBe(@worker._metadata)
+
+    describe "when there is not a previous page failure (`errorRequestRange`)", ->
       it "should start the first request for models", ->
         @worker._state.threads = {
           'busy': false
@@ -226,7 +260,7 @@ describe "NylasSyncWorker", ->
         expect(@apiRequests[1].model).toBe('threads')
         expect(@apiRequests[1].params.offset).toBe(0)
 
-    describe "when there is an errorRequestRange saved", ->
+    describe "when there is a previous page failure (`errorRequestRange`)", ->
       beforeEach ->
         @worker._state.threads =
           'count': 1200

spec/nylas-api-spec.coffee

@@ -8,6 +8,110 @@ DatabaseStore = require '../src/flux/stores/database-store'
 DatabaseTransaction = require '../src/flux/stores/database-transaction'
 
 describe "NylasAPI", ->
+  describe "authPlugin", ->
+    beforeEach ->
+      NylasAPI.pluginsSupported = true
+      @authGetResponse = null
+      @authPostResponse = null
+      @error = null
+      @resolved = false
+      spyOn(NylasEnv.config, 'set')
+      spyOn(NylasEnv.config, 'get').andReturn(null)
+      spyOn(NylasAPI, 'makeRequest').andCallFake (options) =>
+        return @authGetResponse if options.method is 'GET' and @authGetResponse
+        return @authPostResponse if options.method is 'POST' and @authPostResponse
+        return new Promise (resolve, reject) -> #never respond
+
+    it "should reject if the current environment does not support plugins", ->
+      NylasAPI.pluginsSupported = false
+      NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).catch (err) => @error = err
+      waitsFor =>
+        @error
+      runs =>
+        expect(@error.message).toEqual('Sorry, this feature is only available when N1 is running against the hosted version of the Nylas Sync Engine.')
+
+    it "should reject if no account can be found for the given accountOrId", ->
+      NylasAPI.authPlugin('PID', 'PSECRET', 'randomAccountId').catch (err) => @error = err
+      waitsFor =>
+        @error
+      runs =>
+        expect(@error.message).toEqual('Invalid account')
+
+    it "should resolve if the plugin has been successfully authed with accountOrId already", ->
+      jasmine.unspy(NylasEnv.config, 'get')
+      spyOn(NylasEnv.config, 'get').andCallFake (key) =>
+        return Date.now() if key is "plugins.PID.lastAuth.#{TEST_ACCOUNT_ID}"
+        return null
+      NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).then (err) =>
+        @resolved = true
+      waitsFor =>
+        @resolved
+      expect(NylasAPI.makeRequest).not.toHaveBeenCalled()
+
+    describe "check for existing auth", ->
+      it "should GET /auth/plugin to check if the plugin has been authed", ->
+        @authGetResponse = Promise.resolve({authed: true})
+        NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID)
+        advanceClock()
+        expect(NylasAPI.makeRequest).toHaveBeenCalledWith({
+          returnsModel: false,
+          method: 'GET',
+          accountId: 'test-account-server-id',
+          path: '/auth/plugin?client_id=PID'
+        })
+
+      it "should record a successful auth in the config and resolve without making a POST", ->
+        @authGetResponse = Promise.resolve({authed: true})
+        @authPostResponse = null
+        NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).then => @resolved = true
+        waitsFor =>
+          @resolved
+        runs =>
+          expect(NylasAPI.makeRequest).toHaveBeenCalled()
+          expect(NylasEnv.config.set.mostRecentCall.args[0]).toEqual("plugins.PID.lastAuth.#{TEST_ACCOUNT_ID}")
+
+      it "should propagate any network errors back to the caller", ->
+        @authGetResponse = Promise.reject(new Error("Network failure!"))
+        NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).catch (err) => @error = err
+        advanceClock()
+        advanceClock()
+        expect(@error.message).toBe("Network failure!")
+        expect(NylasEnv.config.set).not.toHaveBeenCalled()
+
+    describe "request for auth", ->
+      it "should POST to /auth/plugin with the client id and record a successful auth", ->
+        @authGetResponse = Promise.resolve({authed: false})
+        @authPostResponse = Promise.resolve({authed: true})
+        NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).then => @resolved = true
+        waitsFor =>
+          @resolved
+        runs =>
+          expect(NylasAPI.makeRequest.calls[0].args[0]).toEqual({
+            returnsModel: false,
+            method: 'GET',
+            accountId: 'test-account-server-id',
+            path: '/auth/plugin?client_id=PID'
+          })
+          expect(NylasAPI.makeRequest.calls[1].args[0]).toEqual({
+            returnsModel: false,
+            method: 'POST',
+            accountId: 'test-account-server-id',
+            path: '/auth/plugin',
+            body: {client_id: 'PID'},
+            json: true
+          })
+          setCall = NylasEnv.config.set.mostRecentCall
+          expect(setCall.args[0]).toEqual("plugins.PID.lastAuth.#{TEST_ACCOUNT_ID}")
+
+      it "should propagate any network errors back to the caller", ->
+        @authGetResponse = Promise.resolve({authed: false})
+        @authPostResponse = Promise.reject(new Error("Network failure!"))
+        NylasAPI.authPlugin('PID', 'PSECRET', TEST_ACCOUNT_ID).catch (err) => @error = err
+        waitsFor =>
+          @error
+        runs =>
+          expect(@error.message).toBe("Network failure!")
+
   describe "handleModel404", ->
     it "should unpersist the model from the cache that was requested", ->
       model = new Thread(id: 'threadidhere')

src/flux/models/model-with-metadata.es6

@@ -23,6 +23,13 @@ class PluginMetadata extends Model {
     this.version = this.version || 0;
   }
 
+  fromJSON(json) {
+    super.fromJSON(json);
+
+    // application_id is used in JSON coming down from the API
+    this.pluginId = this.pluginId || json.application_id;
+  }
+
   get id() {
     return this.pluginId
   }

src/flux/nylas-api.coffee

@@ -121,7 +121,6 @@ class NylasAPI
   SampleTemporaryErrorCode: SampleTemporaryErrorCode
 
   constructor: ->
-    @_workers = []
     @_lockTracker = new NylasAPIChangeLockTracker()
 
     NylasEnv.config.onDidChange('env', @_onConfigChanged)
@@ -131,6 +130,7 @@ class NylasAPI
     prev = {@AppID, @APIRoot, @APITokens}
 
     if NylasEnv.inSpecMode()
+      @pluginsSupported = true
       env = "testing"
     else
       env = NylasEnv.config.get('env')
@@ -143,12 +143,15 @@ class NylasAPI
     if env in ['production']
       @AppID = 'eco3rpsghu81xdc48t5qugwq7'
       @APIRoot = 'https://api.nylas.com'
+      @pluginsSupported = true
     else if env in ['staging', 'development']
       @AppID = '54miogmnotxuo5st254trcmb9'
       @APIRoot = 'https://api-staging.nylas.com'
+      @pluginsSupported = true
     else if env in ['experimental']
       @AppID = 'c5dis00do2vki9ib6hngrjs18'
       @APIRoot = 'https://api-staging-experimental.nylas.com'
+      @pluginsSupported = true
     else if env in ['local']
       @AppID = NylasEnv.config.get('syncEngine.AppID') or 'n/a'
       @APIRoot = 'http://localhost:5555'
@@ -321,6 +324,12 @@ class NylasAPI
       .then ->
         return Promise.resolve(responseModels)
 
+  _attachMetadataToResponse: (jsons, metadataToAttach) ->
+    return unless metadataToAttach
+    for obj in jsons
+      if metadataToAttach[obj.id]
+        obj.metadata = metadataToAttach[obj.id]
+
   _apiObjectToClassMap:
     "file": require('./models/file')
     "event": require('./models/event')
@@ -341,6 +350,7 @@ class NylasAPI
         if result.messages
           messages = messages.concat(result.messages)
       if messages.length > 0
+        @_attachMetadataToResponse(messages, requestOptions.metadataToAttach)
         @_handleModelResponse(messages)
       if requestSuccess
         requestSuccess(json)
@@ -350,11 +360,17 @@ class NylasAPI
 
   getCollection: (accountId, collection, params={}, requestOptions={}) ->
     throw (new Error "getCollection requires accountId") unless accountId
+    requestSuccess = requestOptions.success
     @makeRequest _.extend requestOptions,
       path: "/#{collection}"
       accountId: accountId
       qs: params
-      returnsModel: true
+      returnsModel: false
+      success: (jsons) =>
+        @_attachMetadataToResponse(jsons, requestOptions.metadataToAttach)
+        @_handleModelResponse(jsons)
+        if requestSuccess
+          requestSuccess(jsons)
 
   incrementRemoteChangeLock: (klass, id) ->
     @_lockTracker.increment(klass, id)
@@ -385,14 +401,19 @@ class NylasAPI
   #    the plugin server couldn't be reached or failed to respond properly when authing
   #    the account, or that the Nylas API couldn't be reached.
   authPlugin: (pluginId, pluginName, accountOrId) ->
-    account = if accountOrId instanceof Account
-      accountOrId
+    unless @pluginsSupported
+      return Promise.reject(new Error('Sorry, this feature is only available when N1 is running against the hosted version of the Nylas Sync Engine.'))
+
+    if accountOrId instanceof Account
+      account = accountOrId
     else
       AccountStore ?= require './stores/account-store'
-      AccountStore.accountForId(accountOrId)
-    Promise.reject(new Error('Invalid account')) unless account
+      account = AccountStore.accountForId(accountOrId)
 
-    cacheKey = "plugins.#{pluginId}.lastAuthTimestamp"
+    unless account
+      return Promise.reject(new Error('Invalid account'))
+
+    cacheKey = "plugins.#{pluginId}.lastAuth.#{account.id}"
     if NylasEnv.config.get(cacheKey)
       return Promise.resolve()
 
@@ -401,12 +422,12 @@ class NylasAPI
       method: "GET",
       accountId: account.id,
       path: "/auth/plugin?client_id=#{pluginId}"
-    })
-    .then (result) =>
+
+    }).then (result) =>
       if result.authed
         NylasEnv.config.set(cacheKey, Date.now())
         return Promise.resolve()
-      else
+
       # Enable to show a prompt to the user
       # return @_requestPluginAuth(pluginName, account).then =>
       return @makeRequest({
@@ -416,7 +437,9 @@ class NylasAPI
         path: "/auth/plugin",
         body: {client_id: pluginId},
         json: true
-        })
+      }).then (result) =>
+        NylasEnv.config.set(cacheKey, Date.now()) if result.authed
+        return Promise.resolve()
 
   _requestPluginAuth: (pluginName, account) ->
     {dialog} = require('electron').remote

src/flux/stores/category-store.coffee

@@ -32,7 +32,8 @@ class CategoryStore extends NylasStore
       .subscribe(@_onCategoriesChanged)
 
   byId: (accountOrId, categoryId) ->
-    @categories(accountOrId)[categoryId]
+    categories = @_categoryCache[asAccountId(accountOrId)] ? {}
+    categories[categoryId]
 
   # Public: Returns an array of all categories for an account, both
   # standard and user generated. The items returned by this function will be
@@ -40,7 +41,7 @@ class CategoryStore extends NylasStore
   #
   categories: (accountOrId = null) ->
     if accountOrId
-      @_categoryCache[asAccountId(accountOrId)] ? {}
+      _.values(@_categoryCache[asAccountId(accountOrId)]) ? []
     else
       all = []
       for accountId, categories of @_categoryCache