fix(migration-path): Very basic database versioning with re-fetch

Summary: The diff adds very basic versioning to the database via sqlite's built-in `user_version`. If the version is bumped in DatabaseStore, it means that all existing data should be blown away and the user should have to refetch the entire cache. Critically, this does not log the user out.

Test Plan: Run no new tests :-(

Reviewers: evan

Reviewed By: evan

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

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

@@ -1,6 +1,7 @@
 _ = require 'underscore'
 NylasLongConnection = require '../src/flux/nylas-long-connection'
 NylasSyncWorker = require '../src/flux/nylas-sync-worker'
+Namespace = require '../src/flux/models/namespace'
 Thread = require '../src/flux/models/thread'
 
 describe "NylasSyncWorker", ->
@@ -15,7 +16,7 @@ describe "NylasSyncWorker", ->
         @apiRequests.push({namespace, model:'threads', params, requestOptions})
 
     spyOn(atom.config, 'get').andCallFake (key) =>
-      expected = "nylas.namespace-id.worker-state"
+      expected = "nylas.sync-state.namespace-id"
       return throw new Error("Not stubbed! #{key}") unless key is expected
       return _.extend {}, {
         "contacts":
@@ -29,7 +30,8 @@ describe "NylasSyncWorker", ->
     spyOn(atom.config, 'set').andCallFake (key, val) =>
       return
 
-    @worker = new NylasSyncWorker(@api, 'namespace-id')
+    @namespace = new Namespace(id: 'namespace-id', organizationUnit: 'label')
+    @worker = new NylasSyncWorker(@api, @namespace)
     @connection = @worker.connection()
 
   it "should reset `busy` to false when reading state from disk", ->
@@ -44,29 +46,29 @@ describe "NylasSyncWorker", ->
 
     it "should start querying for model collections and counts that haven't been fully cached", ->
       @worker.start()
-      expect(@apiRequests.length).toBe(6)
+      expect(@apiRequests.length).toBe(8)
       modelsRequested = _.compact _.map @apiRequests, ({model}) -> model
-      expect(modelsRequested).toEqual(['threads', 'contacts', 'files'])
+      expect(modelsRequested).toEqual(['threads', 'contacts', 'files', 'labels'])
 
       countsRequested = _.compact _.map @apiRequests, ({requestOptions}) ->
         if requestOptions.qs?.view is 'count'
           return requestOptions.path
 
-      expect(modelsRequested).toEqual(['threads', 'contacts', 'files'])
-      expect(countsRequested).toEqual(['/n/namespace-id/threads', '/n/namespace-id/contacts', '/n/namespace-id/files'])
+      expect(modelsRequested).toEqual(['threads', 'contacts', 'files', 'labels'])
+      expect(countsRequested).toEqual(['/n/namespace-id/threads', '/n/namespace-id/contacts', '/n/namespace-id/files', '/n/namespace-id/labels'])
 
     it "should mark incomplete collections as `busy`", ->
       @worker.start()
       nextState = @worker.state()
 
-      for collection in ['contacts','threads','files']
+      for collection in ['contacts','threads','files', 'labels']
         expect(nextState[collection].busy).toEqual(true)
 
     it "should initialize count and fetched to 0", ->
       @worker.start()
       nextState = @worker.state()
 
-      for collection in ['contacts','threads','files']
+      for collection in ['contacts','threads','files', 'labels']
         expect(nextState[collection].fetched).toEqual(0)
         expect(nextState[collection].count).toEqual(0)
 
@@ -91,7 +93,7 @@ describe "NylasSyncWorker", ->
     it "should fetch collections", ->
       spyOn(@worker, 'fetchCollection')
       @worker.resumeFetches()
-      expect(@worker.fetchCollection.calls.map (call) -> call.args[0]).toEqual(['threads', 'calendars', 'contacts', 'files'])
+      expect(@worker.fetchCollection.calls.map (call) -> call.args[0]).toEqual(['threads', 'calendars', 'contacts', 'files', 'labels'])
 
   describe "fetchCollection", ->
     beforeEach ->

src/browser/application.coffee

@@ -83,7 +83,7 @@ class Application
     @nylasProtocolHandler = new NylasProtocolHandler(@resourcePath, @safeMode)
 
     @databaseManager = new DatabaseManager({@resourcePath})
-    @databaseManager.on "setup-error", @_logout
+    @databaseManager.on "setup-error", @_rebuildDatabase
 
     @listenForArgumentsFromNewProcess()
     @setupJavaScriptArguments()
@@ -131,9 +131,20 @@ class Application
     app.commandLine.appendSwitch 'js-flags', '--harmony'
 
   _logout: =>
-    @databaseManager.deleteAllDatabases()
-    @config.set('nylas', null)
-    @config.set('edgehill', null)
+    @databaseManager.deleteAllDatabases().then =>
+      @config.set('nylas', null)
+      @config.set('edgehill', null)
+
+  _rebuildDatabase: =>
+    @windowManager.closeMainWindow()
+    dialog.showMessageBox
+      type: 'info'
+      message: 'Uprading Nylas'
+      detail: 'Welcome back to Nylas! We need to rebuild your mailbox to support new features. Please wait a few moments while we re-sync your mail.'
+      buttons: ['OK']
+    @databaseManager.deleteAllDatabases().then =>
+      @config.set("nylas.sync-state", {})
+      @windowManager.showMainWindow()
 
   # Registers basic application commands, non-idempotent.
   # Note: If these events are triggered while an application window is open, the window

src/browser/database-manager.coffee

@@ -27,13 +27,13 @@ class DatabaseManager
   #    return a promise because they don't work across the IPC bridge.
   #
   # Returns nothing
-  prepare: (databasePath, callback) =>
+  prepare: (databasePath, databaseVersion, callback) =>
     if @_databases[databasePath]
       callback()
     else
-      @_prepPromises[databasePath] ?= @_createNewDatabase(databasePath)
+      @_prepPromises[databasePath] ?= @_createNewDatabase(databasePath, databaseVersion)
       @_prepPromises[databasePath].then(callback).catch (err) ->
-        console.error "Error preparing the database"
+        console.error "DatabaseManager: Error in prepare:"
         console.error err
 
     return
@@ -54,13 +54,19 @@ class DatabaseManager
     for path, val of @_databases
       @closeDatabaseConnection(path)
 
-  deleteAllDatabases: ->
-    Object.keys(@_databases).forEach (path) =>
-      db = @_databases[path]
-      db.on 'close', -> fs.unlinkSync(path)
-      db.close()
+  deleteDatabase: (db, path) =>
+    new Promise (resolve, reject) =>
       delete @_databases[path]
       delete @_prepPromises[path]
+      db.on 'close', ->
+        if fs.existsSync(path)
+          fs.unlinkSync(path)
+        resolve()
+      db.close()
+
+  deleteAllDatabases: ->
+    Promise.all(_.map(@_databases, @deleteDatabase)).catch (err) ->
+      console.error(err)
 
   onIPCDatabaseQuery: (event, {databasePath, queryKey, query, values}) =>
     db = @_databases[databasePath]
@@ -77,8 +83,12 @@ class DatabaseManager
 
   # Resolves when a new database has been created and the initial setup
   # migration has run successfuly.
-  _createNewDatabase: (databasePath) ->
+  # Rejects with an Error if setup fails or if the database is too old.
+  #
+  _createNewDatabase: (databasePath, databaseVersion) ->
     @_getDBAdapter().then (dbAdapter) =>
+      creating = not fs.existsSync(databasePath)
+
       # Create a new database for the requested path
       db = dbAdapter(databasePath)
 
@@ -87,15 +97,36 @@ class DatabaseManager
       # still allow queries to be made.
       db.ignoreErrors = true
 
-      # Resolves when the DB has been initalized
-      @_runSetupQueries(db, @_setupQueries[databasePath])
+      cleanupAfterError = (err) =>
+        @deleteDatabase(db, databasePath).then =>
+          @emit("setup-error", err)
+          return Promise.reject(err)
+
+      if creating
+        versionCheck = @_setDatabaseVersion(db, databaseVersion)
+      else
+        versionCheck = @_checkDatabaseVersion(db, databaseVersion)
+
+      versionCheck
+      .catch(cleanupAfterError)
       .then =>
-        @_databases[databasePath] = db
-        return Promise.resolve()
-      .catch (err) =>
-        console.error("DatabaseManager: Error running setup queries: #{err?.message}")
-        @emit("setup-error", err)
-        return Promise.reject(err)
+        @_runSetupQueries(db, @_setupQueries[databasePath])
+        .catch(cleanupAfterError)
+        .then =>
+          @_databases[databasePath] = db
+          return Promise.resolve()
+
+  _setDatabaseVersion: (db, databaseVersion) ->
+    new Promise (resolve, reject) =>
+      db.query("PRAGMA user_version=#{databaseVersion}", [], null, resolve)
+
+  _checkDatabaseVersion: (db, databaseVersion) ->
+    new Promise (resolve, reject) ->
+      db.query "PRAGMA user_version", [], null, (currentVersion) ->
+        if currentVersion/1 isnt databaseVersion/1
+          reject(new Error("Incorrect database schema version: #{currentVersion} not #{databaseVersion}"))
+        else
+          resolve()
 
   # Takes a set of queries to initialize the database with
   #

src/flux/nylas-api.coffee

@@ -45,12 +45,6 @@ class NylasAPIRequest
     @
 
   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 =>
@@ -134,10 +128,10 @@ class NylasAPI
     @_workers
 
   workerForNamespace: (namespace) =>
-    worker = _.find @_workers, (c) -> c.namespaceId() is namespace.id
+    worker = _.find @_workers, (c) -> c.namespace().id is namespace.id
     return worker if worker
 
-    worker = new NylasSyncWorker(@, namespace.id)
+    worker = new NylasSyncWorker(@, namespace)
     connection = worker.connection()
 
     connection.onStateChange (state) ->
@@ -184,8 +178,8 @@ class NylasAPI
       return Promise.resolve()
 
     if not @APIToken
-      console.log('Cannot make Nylas request without auth token.')
-      return Promise.reject()
+      err = new APIError(statusCode: 400, body: 'Cannot make Nylas request without auth token.')
+      return Promise.reject(err)
 
     success = (body) =>
       if options.beforeProcessing

src/flux/nylas-sync-worker.coffee

@@ -12,20 +12,20 @@ class NylasSyncWorker
   @include: CoffeeHelpers.includeModule
   @include Publisher
 
-  constructor: (api, namespaceId) ->
+  constructor: (api, namespace) ->
     @_api = api
-    @_namespaceId = namespaceId
+    @_namespace = namespace
 
     @_terminated = false
-    @_connection = new NylasLongConnection(api, namespaceId)
-    @_state = atom.config.get("nylas.#{namespaceId}.worker-state") ? {}
+    @_connection = new NylasLongConnection(api, namespace.id)
+    @_state = atom.config.get("nylas.sync-state.#{namespace.id}") ? {}
     for model, modelState of @_state
       modelState.busy = false
 
     @
 
-  namespaceId: ->
-    @_namespaceId
+  namespace: ->
+    @_namespace
 
   connection: ->
     @_connection
@@ -55,6 +55,10 @@ class NylasSyncWorker
     @fetchCollection('calendars')
     @fetchCollection('contacts')
     @fetchCollection('files')
+    if @_namespace.usesLabels()
+      @fetchCollection('labels')
+    if @_namespace.usesFolders()
+      @fetchCollection('folders')
 
   fetchCollection: (model, options = {}) ->
     return if @_state[model]?.complete and not options.force?
@@ -73,7 +77,7 @@ class NylasSyncWorker
 
   fetchCollectionCount: (model) ->
     @_api.makeRequest
-      path: "/n/#{@_namespaceId}/#{model}"
+      path: "/n/#{@_namespace.id}/#{model}"
       returnsModel: false
       qs:
         view: 'count'
@@ -99,9 +103,9 @@ class NylasSyncWorker
           @updateTransferState(model, {fetched: lastReceivedIndex, busy: false, complete: true})
 
     if model is 'threads'
-      @_api.getThreads(@_namespaceId, params, requestOptions)
+      @_api.getThreads(@_namespace.id, params, requestOptions)
     else
-      @_api.getCollection(@_namespaceId, model, params, requestOptions)
+      @_api.getCollection(@_namespace.id, model, params, requestOptions)
 
   updateTransferState: (model, {busy, error, complete, fetched, count}) ->
     @_state[model] = _.defaults({busy, error, complete, fetched, count}, @_state[model])
@@ -109,7 +113,7 @@ class NylasSyncWorker
 
   writeState: ->
     @_writeState ?= _.debounce =>
-      atom.config.set("nylas.#{@_namespaceId}.worker-state", @_state)
+      atom.config.set("nylas.sync-state.#{@_namespace.id}", @_state)
     ,100
     @_writeState()
     @trigger()

src/flux/stores/category-store.coffee

@@ -10,10 +10,8 @@ class CategoryStore extends NylasStore
   constructor: ->
     @_categoryCache = {}
     @listenTo DatabaseStore, @_onDBChanged
-    @listenTo NamespaceStore, @_onNamespaceChanged
-
+    @listenTo NamespaceStore, @_refreshCacheFromDB
     @_refreshCacheFromDB()
-    @_onNamespaceChanged()
 
   # and labels: an extended version of [RFC-6154]
   # (http://tools.ietf.org/html/rfc6154), returned as the name of the
@@ -32,9 +30,28 @@ class CategoryStore extends NylasStore
   AllMailName: "all"
 
   byId: (id) -> @_categoryCache[id]
+
   categories: -> _.values @_categoryCache
 
-  categoryLabel: -> @_categoryLabel
+  categoryLabel: ->
+    namespace = NamespaceStore.current()
+    return "Unknown" unless namespace
+
+    if namespace.usesFolders()
+      return "Folders"
+    else if namespace.usesLabels()
+      return "Labels"
+    return "Unknown"
+
+  categoryClass: ->
+    namespace = NamespaceStore.current()
+    return null unless namespace
+
+    if namespace.usesFolders()
+      return Folder
+    else if namespace.usesLabels()
+      return Label
+    return null
 
   # It's possible for this to return `null`. For example, Gmail likely
   # doesn't have an `archive` label.
@@ -44,34 +61,19 @@ class CategoryStore extends NylasStore
     return _.findWhere @_categoryCache, {name}
 
   _onDBChanged: (change) ->
-    return unless @_klass and change?.objectClass == @_klass.name
-    @_refreshCacheFromDB()
+    categoryClass = @categoryClass()
+    return unless categoryClass
 
-  _refreshDBFromAPI: ->
-    NylasAPI.getCollection @_namespace.id, @_endpoint
+    if change and change.objectClass is categoryClass.name
+      @_refreshCacheFromDB()
 
   _refreshCacheFromDB: ->
-    return unless @_klass
-    DatabaseStore.findAll(@_klass).then (categories=[]) =>
+    categoryClass = @categoryClass()
+    return unless categoryClass
+
+    DatabaseStore.findAll(categoryClass).then (categories=[]) =>
       @_categoryCache = {}
       @_categoryCache[category.id] = category for category in categories
       @trigger()
 
-  _onNamespaceChanged: ->
-    @_namespace = NamespaceStore.current()
-    return unless @_namespace
-
-    if @_namespace.usesFolders()
-      @_klass = Folder
-      @_endpoint = "folders"
-      @_categoryLabel = "Folders"
-    else if @_namespace.usesLabels()
-      @_klass = Label
-      @_endpoint = "labels"
-      @_categoryLabel = "Labels"
-    else
-      throw new Error("Invalid organizationUnit")
-
-    @_refreshDBFromAPI()
-
 module.exports = new CategoryStore()

src/flux/stores/database-connection.coffee

@@ -16,7 +16,7 @@ DEBUG_TO_LOG = false
 # currently running and fires promise callbacks when complete.
 #
 class DatabaseConnection
-  constructor: (@_databasePath) ->
+  constructor: (@_databasePath, @_databaseVersion) ->
     @_queryId = 0
     @_windowId = remote.getCurrentWindow().id
     @_isConnected = false
@@ -38,7 +38,7 @@ class DatabaseConnection
     # all have `IF NOT EXISTS` clauses in them.
     databaseManager.addSetupQueries(@_databasePath, @_setupQueries())
 
-    databaseManager.prepare @_databasePath, =>
+    databaseManager.prepare @_databasePath, @_databaseVersion, =>
       @_isConnected = true
       @_flushPendingQueries()
 
@@ -50,7 +50,7 @@ class DatabaseConnection
   # handlers
   query: (query, values=[], options={}) =>
     if not query
-      throw new Error("no query")
+      throw new Error("DatabaseConnection: You need to provide a query string.")
 
     return new Promise (resolve, reject) =>
       @_queryId += 1
@@ -125,15 +125,6 @@ class DatabaseConnection
     console.error("DatabaseStore: Query #{query}, #{JSON.stringify(values)} failed #{message ? ""}")
 
 
-
-
-
-
-
-
-
-
-
   ## TODO: Make these a nicer migration-based system
   _setupQueries: ->
     queries = []

src/flux/stores/database-store.coffee

@@ -14,6 +14,8 @@ DatabaseConnection = require './database-connection'
  generateTempId,
  isTempId} = require '../models/utils'
 
+DatabaseVersion = 4
+
 ###
 Public: Nylas Mail is built on top of a custom database layer modeled after
 ActiveRecord. For many parts of the application, the database is the source
@@ -69,7 +71,7 @@ class DatabaseStore extends NylasStore
     else
       @_databasePath = path.join(atom.getConfigDirPath(),'edgehill.db')
 
-    @_dbConnection = new DatabaseConnection(@_databasePath)
+    @_dbConnection = new DatabaseConnection(@_databasePath, DatabaseVersion)
 
     # It's important that this defer is here because we can't let queries
     # commence while the app is in its `require` phase. We'll queue all of

src/flux/stores/namespace-store.coffee

@@ -29,7 +29,7 @@ class NamespaceStore
     if saveState and _.isObject(saveState)
       savedNamespace = (new Namespace).fromJSON(saveState)
       if savedNamespace.usesLabels() or savedNamespace.usesFolders()
-        @_current = savedNamespace
+        @_setCurrent(savedNamespace)
         @_namespaces = [@_current]
 
     @listenTo Actions.selectNamespaceId, @onSelectNamespaceId
@@ -42,18 +42,18 @@ class NamespaceStore
       current = _.find namespaces, (n) -> n.id is @_current?.id
       current = namespaces?[0] unless current
 
-      if current and (current.usesLabels() or current.usesFolders())
-        if not _.isEqual(current, @_current) or not _.isEqual(namespaces, @_namespaces)
-          atom.config.set(saveStateKey, current)
-          @_current = current
-          @_namespaces = namespaces
-          @trigger(@)
-      else
-        DatabaseStore.unpersistModel(current) if current
-        atom.config.unset(saveStateKey)
+      if not _.isEqual(current, @_current) or not _.isEqual(namespaces, @_namespaces)
+        @_setCurrent(current)
+        @_namespaces = namespaces
+        @trigger(@)
+
     .catch (err) =>
       console.warn("Request for Namespaces failed. #{err}", err.stack)
 
+  _setCurrent: (current) =>
+    atom.config.set(saveStateKey, current)
+    @_current = current
+
   # Inbound Events
 
   onDataChanged: (change) =>

src/tasks/ship-logs-task.coffee

@@ -2,13 +2,17 @@ fs = require 'fs'
 path = require 'path'
 request = require 'request'
 
+detailedLogging = false
+detailedLog = (msg) ->
+  console.log(msg) if detailedLogging
+
 module.exports = (dir, regexPattern) ->
   callback = @async()
 
   console.log("Running log ship: #{dir}, #{regexPattern}")
 
   fs.readdir dir, (err, files) ->
-    console.log("readdir error: #{err}") if err
+    log("readdir error: #{err}") if err
     logs = []
     logFilter = new RegExp(regexPattern)
     for file in files
@@ -24,13 +28,10 @@ module.exports = (dir, regexPattern) ->
         callback()
 
     if logs.length is 0
-      console.log("No logs found to upload.")
+      detailedLog("No logs found to upload.")
       callback()
-      console.log("Callback.")
       return
 
-    console.log("Uploading #{logs} to S3")
-
     # The AWS Module does some really interesting stuff - it loads it's configuration
     # from JSON files. Unfortunately, when the app is built into an ASAR bundle, child
     # processes forked from the main process can't seem to access files inside the archive,
@@ -57,8 +58,8 @@ module.exports = (dir, regexPattern) ->
       remaining += 1
       bucket.upload params, (err, data) ->
         if err
-          console.log("Error uploading #{key}: #{err.toString()}")
+          detailedLog("Error uploading #{key}: #{err.toString()}")
         else
-          console.log("Successfully uploaded #{key}")
+          detailedLog("Successfully uploaded #{key}")
         fs.truncate(log)
         finished()