diff --git a/package.json b/package.json index 416bbf6c8..4235069fe 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "scandal": "^3.1.0", "scoped-property-store": "^0.17.0", "scrollbar-style": "^3.2", - "season": "^6.0.0", + "season": "^6.0.1", "semver": "^4.3.3", "service-hub": "^0.7.4", "sinon": "1.17.4", diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index 552f44bf9..bcf50c268 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -13,6 +13,8 @@ describe "Config", -> dotAtomPath = temp.path('atom-spec-config') atom.config.configDirPath = dotAtomPath atom.config.enablePersistence = true + atom.config.settingsLoaded = true + atom.config.pendingOperations = [] atom.config.configFilePath = path.join(atom.config.configDirPath, "atom.config.cson") afterEach -> @@ -877,7 +879,7 @@ describe "Config", -> beforeEach -> atom.notifications.onDidAddNotification addErrorHandler = jasmine.createSpy() - spyOn(fs, "existsSync").andCallFake -> + spyOn(fs, "makeTreeSync").andCallFake -> error = new Error() error.code = 'EPERM' throw error @@ -895,16 +897,15 @@ describe "Config", -> describe ".observeUserConfig()", -> updatedHandler = null - writeConfigFile = (data) -> - previousSetTimeoutCallCount = setTimeout.callCount - runs -> - fs.writeFileSync(atom.config.configFilePath, data) - waitsFor "debounced config file load", -> - setTimeout.callCount > previousSetTimeoutCallCount - runs -> - advanceClock(1000) + writeConfigFile = (data, secondsInFuture = 0) -> + fs.writeFileSync(atom.config.configFilePath, data) + + future = (Date.now() / 1000) + secondsInFuture + fs.utimesSync(atom.config.configFilePath, future, future) beforeEach -> + jasmine.useRealClock() + atom.config.setSchema 'foo', type: 'object' properties: @@ -920,7 +921,7 @@ describe "Config", -> default: 12 expect(fs.existsSync(atom.config.configDirPath)).toBeFalsy() - fs.writeFileSync atom.config.configFilePath, """ + writeConfigFile """ '*': foo: bar: 'baz' @@ -930,26 +931,32 @@ describe "Config", -> scoped: true """ atom.config.loadUserConfig() - atom.config.observeUserConfig() - updatedHandler = jasmine.createSpy("updatedHandler") - atom.config.onDidChange updatedHandler + + waitsForPromise -> atom.config.observeUserConfig() + + runs -> + updatedHandler = jasmine.createSpy "updatedHandler" + atom.config.onDidChange updatedHandler afterEach -> atom.config.unobserveUserConfig() fs.removeSync(dotAtomPath) describe "when the config file changes to contain valid cson", -> + it "updates the config data", -> - writeConfigFile("foo: { bar: 'quux', baz: 'bar'}") + writeConfigFile "foo: { bar: 'quux', baz: 'bar'}", 2 + waitsFor 'update event', -> updatedHandler.callCount > 0 + runs -> expect(atom.config.get('foo.bar')).toBe 'quux' expect(atom.config.get('foo.baz')).toBe 'bar' it "does not fire a change event for paths that did not change", -> - atom.config.onDidChange 'foo.bar', noChangeSpy = jasmine.createSpy() + atom.config.onDidChange 'foo.bar', noChangeSpy = jasmine.createSpy "unchanged" - writeConfigFile("foo: { bar: 'baz', baz: 'ok'}") + writeConfigFile "foo: { bar: 'baz', baz: 'ok'}", 2 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> @@ -964,15 +971,16 @@ describe "Config", -> items: type: 'string' - writeConfigFile("foo: { bar: ['baz', 'ok']}") + updatedHandler.reset() + writeConfigFile "foo: { bar: ['baz', 'ok']}", 4 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> updatedHandler.reset() it "does not fire a change event for paths that did not change", -> - noChangeSpy = jasmine.createSpy() + noChangeSpy = jasmine.createSpy "unchanged" atom.config.onDidChange('foo.bar', noChangeSpy) - writeConfigFile("foo: { bar: ['baz', 'ok'], baz: 'another'}") + writeConfigFile "foo: { bar: ['baz', 'ok'], baz: 'another'}", 2 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> @@ -989,7 +997,7 @@ describe "Config", -> '*': foo: scoped: false - """ + """, 2 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> @@ -997,7 +1005,7 @@ describe "Config", -> expect(atom.config.get('foo.scoped', scope: ['.source.ruby'])).toBe false it "does not fire a change event for paths that did not change", -> - noChangeSpy = jasmine.createSpy() + noChangeSpy = jasmine.createSpy "no change" atom.config.onDidChange('foo.scoped', scope: ['.source.ruby'], noChangeSpy) writeConfigFile """ @@ -1007,7 +1015,7 @@ describe "Config", -> '.source.ruby': foo: scoped: true - """ + """, 2 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> @@ -1017,7 +1025,7 @@ describe "Config", -> describe "when the config file changes to omit a setting with a default", -> it "resets the setting back to the default", -> - writeConfigFile("foo: { baz: 'new'}") + writeConfigFile "foo: { baz: 'new'}", 2 waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> expect(atom.config.get('foo.bar')).toBe 'def' @@ -1025,20 +1033,20 @@ describe "Config", -> describe "when the config file changes to be empty", -> beforeEach -> - writeConfigFile("") + updatedHandler.reset() + writeConfigFile "", 4 waitsFor 'update event', -> updatedHandler.callCount > 0 it "resets all settings back to the defaults", -> expect(updatedHandler.callCount).toBe 1 expect(atom.config.get('foo.bar')).toBe 'def' atom.config.set("hair", "blonde") # trigger a save - advanceClock(500) - expect(atom.config.save).toHaveBeenCalled() + waitsFor 'save', -> atom.config.save.callCount > 0 describe "when the config file subsequently changes again to contain configuration", -> beforeEach -> updatedHandler.reset() - writeConfigFile("foo: bar: 'newVal'") + writeConfigFile "foo: bar: 'newVal'", 2 waitsFor 'update event', -> updatedHandler.callCount > 0 it "sets the setting to the value specified in the config file", -> @@ -1047,25 +1055,26 @@ describe "Config", -> describe "when the config file changes to contain invalid cson", -> addErrorHandler = null beforeEach -> - atom.notifications.onDidAddNotification addErrorHandler = jasmine.createSpy() - writeConfigFile("}}}") + atom.notifications.onDidAddNotification addErrorHandler = jasmine.createSpy "error handler" + writeConfigFile "}}}", 4 waitsFor "error to be logged", -> addErrorHandler.callCount > 0 it "logs a warning and does not update config data", -> expect(updatedHandler.callCount).toBe 0 expect(atom.config.get('foo.bar')).toBe 'baz' + atom.config.set("hair", "blonde") # trigger a save expect(atom.config.save).not.toHaveBeenCalled() describe "when the config file subsequently changes again to contain valid cson", -> beforeEach -> - writeConfigFile("foo: bar: 'newVal'") + updatedHandler.reset() + writeConfigFile "foo: bar: 'newVal'", 6 waitsFor 'update event', -> updatedHandler.callCount > 0 it "updates the config data and resumes saving", -> atom.config.set("hair", "blonde") - advanceClock(500) - expect(atom.config.save).toHaveBeenCalled() + waitsFor 'save', -> atom.config.save.callCount > 0 describe ".initializeConfigDirectory()", -> beforeEach -> @@ -1741,3 +1750,35 @@ describe "Config", -> expect(atom.config.set('foo.bar.str_options', 'One')).toBe false expect(atom.config.get('foo.bar.str_options')).toEqual 'two' + + describe "when .set/.unset is called prior to .loadUserConfig", -> + beforeEach -> + atom.config.settingsLoaded = false + fs.writeFileSync atom.config.configFilePath, """ + '*': + foo: + bar: 'baz' + do: + ray: 'me' + """ + + it "ensures that early set and unset calls are replayed after the config is loaded from disk", -> + atom.config.unset 'foo.bar' + atom.config.set 'foo.qux', 'boo' + + expect(atom.config.get('foo.bar')).toBeUndefined() + expect(atom.config.get('foo.qux')).toBe 'boo' + expect(atom.config.get('do.ray')).toBeUndefined() + + advanceClock 100 + expect(atom.config.save).not.toHaveBeenCalled() + + atom.config.loadUserConfig() + + advanceClock 100 + waitsFor -> atom.config.save.callCount > 0 + + runs -> + expect(atom.config.get('foo.bar')).toBeUndefined() + expect(atom.config.get('foo.qux')).toBe 'boo' + expect(atom.config.get('do.ray')).toBe 'me' diff --git a/src/config.coffee b/src/config.coffee index bbcdad01f..01cb732ca 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -4,7 +4,7 @@ fs = require 'fs-plus' CSON = require 'season' path = require 'path' async = require 'async' -pathWatcher = require 'pathwatcher' +{watchPath} = require './path-watcher' { getValueAtKeyPath, setValueAtKeyPath, deleteValueAtKeyPath, pushKeyPath, splitKeyPath, @@ -417,17 +417,24 @@ class Config @defaultSettings = {} @settings = {} @scopedSettingsStore = new ScopedPropertyStore + + @settingsLoaded = false + @savePending = false @configFileHasErrors = false @transactDepth = 0 - @savePending = false - @requestLoad = _.debounce(@loadUserConfig, 100) - @requestSave = => - @savePending = true - debouncedSave.call(this) - save = => + @pendingOperations = [] + + @requestLoad = _.debounce => + @loadUserConfig() + , 100 + + debouncedSave = _.debounce => @savePending = false @save() - debouncedSave = _.debounce(save, 100) + , 100 + @requestSave = => + @savePending = true + debouncedSave() shouldNotAccessFileSystem: -> not @enablePersistence @@ -647,6 +654,10 @@ class Config # * `false` if the value was not able to be coerced to the type specified in the setting's schema. set: -> [keyPath, value, options] = arguments + + unless @settingsLoaded + @pendingOperations.push => @set.call(this, keyPath, value, options) + scopeSelector = options?.scopeSelector source = options?.source shouldSave = options?.save ? true @@ -667,7 +678,8 @@ class Config else @setRawValue(keyPath, value) - @requestSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors + if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors and @settingsLoaded + @requestSave() true # Essential: Restore the setting at `keyPath` to its default value. @@ -677,6 +689,9 @@ class Config # * `scopeSelector` (optional) {String}. See {::set} # * `source` (optional) {String}. See {::set} unset: (keyPath, options) -> + unless @settingsLoaded + @pendingOperations.push => @unset.call(this, keyPath, options) + {scopeSelector, source} = options ? {} source ?= @getUserConfigPath() @@ -688,7 +703,8 @@ class Config setValueAtKeyPath(settings, keyPath, undefined) settings = withoutEmptyObjects(settings) @set(null, settings, {scopeSelector, source, priority: @priorityForSource(source)}) if settings? - @requestSave() + if source is @getUserConfigPath() and not @configFileHasErrors and @settingsLoaded + @requestSave() else @scopedSettingsStore.removePropertiesForSourceAndSelector(source, scopeSelector) @emitChangeEvent() @@ -848,21 +864,26 @@ class Config loadUserConfig: -> return if @shouldNotAccessFileSystem() + return if @savePending try - unless fs.existsSync(@configFilePath) - fs.makeTreeSync(path.dirname(@configFilePath)) - CSON.writeFileSync(@configFilePath, {}) + fs.makeTreeSync(path.dirname(@configFilePath)) + CSON.writeFileSync(@configFilePath, {}, {flag: 'wx'}) # fails if file exists catch error - @configFileHasErrors = true - @notifyFailure("Failed to initialize `#{path.basename(@configFilePath)}`", error.stack) - return + if error.code isnt 'EEXIST' + @configFileHasErrors = true + @notifyFailure("Failed to initialize `#{path.basename(@configFilePath)}`", error.stack) + return try - unless @savePending - userConfig = CSON.readFileSync(@configFilePath) - @resetUserSettings(userConfig) - @configFileHasErrors = false + userConfig = CSON.readFileSync(@configFilePath) + userConfig = {} if userConfig is null + + unless isPlainObject(userConfig) + throw new Error("`#{path.basename(@configFilePath)}` must contain valid JSON or CSON") + + @resetUserSettings(userConfig) + @configFileHasErrors = false catch error @configFileHasErrors = true message = "Failed to load `#{path.basename(@configFilePath)}`" @@ -880,8 +901,10 @@ class Config return if @shouldNotAccessFileSystem() try - @watchSubscription ?= pathWatcher.watch @configFilePath, (eventType) => - @requestLoad() if eventType is 'change' and @watchSubscription? + @watchSubscriptionPromise ?= watchPath @configFilePath, {}, (events) => + for {action} in events + if action in ['created', 'modified', 'renamed'] and @watchSubscriptionPromise? + @requestLoad() catch error @notifyFailure """ Unable to watch path: `#{path.basename(@configFilePath)}`. Make sure you have permissions to @@ -890,9 +913,11 @@ class Config [watches]:https://github.com/atom/atom/blob/master/docs/build-instructions/linux.md#typeerror-unable-to-watch-path """ + @watchSubscriptionPromise + unobserveUserConfig: -> - @watchSubscription?.close() - @watchSubscription = null + @watchSubscriptionPromise?.then (watcher) -> watcher?.dispose() + @watchSubscriptionPromise = null notifyFailure: (errorMessage, detail) -> @notificationManager?.addError(errorMessage, {detail, dismissable: true}) @@ -915,11 +940,6 @@ class Config ### resetUserSettings: (newSettings) -> - unless isPlainObject(newSettings) - @settings = {} - @emitChangeEvent() - return - if newSettings.global? newSettings['*'] = newSettings.global delete newSettings.global @@ -932,8 +952,11 @@ class Config @transact => @settings = {} + @settingsLoaded = true @set(key, value, save: false) for key, value of newSettings - return + if @pendingOperations.length + op() for op in @pendingOperations + @pendingOperations = [] getRawValue: (keyPath, options) -> unless options?.excludeSources?.indexOf(@getUserConfigPath()) >= 0