From 53bf7c2d2c9de683331e7b7d071093b0b35416d8 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Thu, 16 Jul 2015 17:47:02 -0700 Subject: [PATCH] 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 | 20 ++++--- src/browser/application.coffee | 19 +++++-- src/browser/database-manager.coffee | 65 ++++++++++++++++------ src/flux/nylas-api.coffee | 14 ++--- src/flux/nylas-sync-worker.coffee | 24 ++++---- src/flux/stores/category-store.coffee | 56 ++++++++++--------- src/flux/stores/database-connection.coffee | 15 +---- src/flux/stores/database-store.coffee | 4 +- src/flux/stores/namespace-store.coffee | 20 +++---- src/tasks/ship-logs-task.coffee | 15 ++--- 10 files changed, 145 insertions(+), 107 deletions(-) diff --git a/spec-nylas/nylas-sync-worker-spec.coffee b/spec-nylas/nylas-sync-worker-spec.coffee index f353617cb..6aea15c78 100644 --- a/spec-nylas/nylas-sync-worker-spec.coffee +++ b/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 -> diff --git a/src/browser/application.coffee b/src/browser/application.coffee index 3ad2ae2c5..02b1faae3 100644 --- a/src/browser/application.coffee +++ b/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 diff --git a/src/browser/database-manager.coffee b/src/browser/database-manager.coffee index 237ffbc82..e19054d2e 100644 --- a/src/browser/database-manager.coffee +++ b/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 # diff --git a/src/flux/nylas-api.coffee b/src/flux/nylas-api.coffee index 85baa3835..993ff52ad 100644 --- a/src/flux/nylas-api.coffee +++ b/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 diff --git a/src/flux/nylas-sync-worker.coffee b/src/flux/nylas-sync-worker.coffee index aba68cdb8..f039dac76 100644 --- a/src/flux/nylas-sync-worker.coffee +++ b/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() diff --git a/src/flux/stores/category-store.coffee b/src/flux/stores/category-store.coffee index 42bd9a558..e5b363ced 100644 --- a/src/flux/stores/category-store.coffee +++ b/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() diff --git a/src/flux/stores/database-connection.coffee b/src/flux/stores/database-connection.coffee index 491931599..44d8c74d3 100644 --- a/src/flux/stores/database-connection.coffee +++ b/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 = [] diff --git a/src/flux/stores/database-store.coffee b/src/flux/stores/database-store.coffee index e2a8649ea..61c7aa0da 100644 --- a/src/flux/stores/database-store.coffee +++ b/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 diff --git a/src/flux/stores/namespace-store.coffee b/src/flux/stores/namespace-store.coffee index 6f576bffa..d3b57ff3c 100644 --- a/src/flux/stores/namespace-store.coffee +++ b/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) => diff --git a/src/tasks/ship-logs-task.coffee b/src/tasks/ship-logs-task.coffee index da8cdc502..2898c774d 100644 --- a/src/tasks/ship-logs-task.coffee +++ b/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()