From 339f33177663d50fc779dc67913da2686b8a22f0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 29 Dec 2014 23:52:41 -0800 Subject: [PATCH] Debounce saving and loading of config --- spec/config-spec.coffee | 68 ++++++++++++++++++++++++++++------------- spec/spec-helper.coffee | 5 +-- src/config.coffee | 12 +++++--- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index e6114823b..4b698b625 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -102,18 +102,20 @@ describe "Config", -> expect(atom.config.set("foo.bar.baz", 42)).toBe true expect(atom.config.get("foo.bar.baz")).toBe 42 - it "updates observers and saves when a key path is set", -> - observeHandler = jasmine.createSpy "observeHandler" - atom.config.observe "foo.bar.baz", observeHandler - observeHandler.reset() - + it "saves the user's config to disk after it stops changing", -> atom.config.set("foo.bar.baz", 42) - + advanceClock(50) + expect(atom.config.save).not.toHaveBeenCalled() + atom.config.set("foo.bar.baz", 43) + advanceClock(50) + expect(atom.config.save).not.toHaveBeenCalled() + atom.config.set("foo.bar.baz", 44) + advanceClock(150) expect(atom.config.save).toHaveBeenCalled() - expect(observeHandler).toHaveBeenCalledWith 42 it "does not save when a non-default 'source' is given", -> atom.config.set("foo.bar.baz", 42, source: 'some-other-source', scopeSelector: '.a') + advanceClock(500) expect(atom.config.save).not.toHaveBeenCalled() it "does not allow a 'source' option without a 'scopeSelector'", -> @@ -197,6 +199,7 @@ describe "Config", -> atom.config.save.reset() atom.config.unset('a.c') + advanceClock(500) expect(atom.config.save.callCount).toBe 1 describe "when a 'source' and no 'scopeSelector' is given", -> @@ -234,6 +237,7 @@ describe "Config", -> atom.config.save.reset() atom.config.unset('foo.bar.baz', scopeSelector: '.source.coffee') + advanceClock(150) expect(atom.config.save.callCount).toBe 1 it "allows removing settings for a specific source and scope selector", -> @@ -267,26 +271,32 @@ describe "Config", -> it "removes the scoped value when it was the only set value on the object", -> spyOn(CSON, 'writeFileSync') - jasmine.unspy atom.config, 'save' + atom.config.save.andCallThrough() atom.config.setDefaults("foo", bar: baz: 10) atom.config.set('foo.bar.baz', 55, scopeSelector: '.source.coffee') atom.config.set('foo.bar.ok', 20, scopeSelector: '.source.coffee') - CSON.writeFileSync.reset() expect(atom.config.get('foo.bar.baz', scope: ['.source.coffee'])).toBe 55 + advanceClock(150) + CSON.writeFileSync.reset() + atom.config.unset('foo.bar.baz', scopeSelector: '.source.coffee') expect(atom.config.get('foo.bar.baz', scope: ['.source.coffee'])).toBe 10 expect(atom.config.get('foo.bar.ok', scope: ['.source.coffee'])).toBe 20 + + advanceClock(150) expect(CSON.writeFileSync).toHaveBeenCalled() properties = CSON.writeFileSync.mostRecentCall.args[1] expect(properties['.coffee.source']).toEqual foo: bar: ok: 20 - CSON.writeFileSync.reset() + atom.config.unset('foo.bar.ok', scopeSelector: '.source.coffee') + + advanceClock(150) expect(CSON.writeFileSync).toHaveBeenCalled() properties = CSON.writeFileSync.mostRecentCall.args[1] expect(properties['.coffee.source']).toBeUndefined() @@ -692,6 +702,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) + beforeEach -> atom.config.setSchema 'foo', type: 'object' @@ -728,7 +747,7 @@ describe "Config", -> describe "when the config file changes to contain valid cson", -> it "updates the config data", -> - fs.writeFileSync(atom.config.configFilePath, "foo: { bar: 'quux', baz: 'bar'}") + writeConfigFile("foo: { bar: 'quux', baz: 'bar'}") waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> expect(atom.config.get('foo.bar')).toBe 'quux' @@ -737,8 +756,9 @@ describe "Config", -> it "does not fire a change event for paths that did not change", -> atom.config.onDidChange 'foo.bar', noChangeSpy = jasmine.createSpy() - fs.writeFileSync(atom.config.configFilePath, "foo: { bar: 'baz', baz: 'ok'}") + writeConfigFile("foo: { bar: 'baz', baz: 'ok'}") waitsFor 'update event', -> updatedHandler.callCount > 0 + runs -> expect(noChangeSpy).not.toHaveBeenCalled() expect(atom.config.get('foo.bar')).toBe 'baz' @@ -750,7 +770,8 @@ describe "Config", -> type: 'array' items: type: 'string' - fs.writeFileSync(atom.config.configFilePath, "foo: { bar: ['baz', 'ok']}") + + writeConfigFile("foo: { bar: ['baz', 'ok']}") waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> updatedHandler.reset() @@ -758,8 +779,9 @@ describe "Config", -> noChangeSpy = jasmine.createSpy() atom.config.onDidChange('foo.bar', noChangeSpy) - fs.writeFileSync(atom.config.configFilePath, "foo: { bar: ['baz', 'ok'], baz: 'another'}") + writeConfigFile("foo: { bar: ['baz', 'ok'], baz: 'another'}") waitsFor 'update event', -> updatedHandler.callCount > 0 + runs -> expect(noChangeSpy).not.toHaveBeenCalled() expect(atom.config.get('foo.bar')).toEqual ['baz', 'ok'] @@ -770,12 +792,13 @@ describe "Config", -> scopedSpy = jasmine.createSpy() atom.config.onDidChange('foo.scoped', scope: ['.source.ruby'], scopedSpy) - fs.writeFileSync atom.config.configFilePath, """ + writeConfigFile """ global: foo: scoped: false """ waitsFor 'update event', -> updatedHandler.callCount > 0 + runs -> expect(scopedSpy).toHaveBeenCalled() expect(atom.config.get('foo.scoped', scope: ['.source.ruby'])).toBe false @@ -784,7 +807,7 @@ describe "Config", -> noChangeSpy = jasmine.createSpy() atom.config.onDidChange('foo.scoped', scope: ['.source.ruby'], noChangeSpy) - fs.writeFileSync atom.config.configFilePath, """ + writeConfigFile """ global: foo: bar: 'baz' @@ -793,6 +816,7 @@ describe "Config", -> scoped: true """ waitsFor 'update event', -> updatedHandler.callCount > 0 + runs -> expect(noChangeSpy).not.toHaveBeenCalled() expect(atom.config.get('foo.bar', scope: ['.source.ruby'])).toBe 'baz' @@ -800,7 +824,7 @@ describe "Config", -> describe "when the config file changes to omit a setting with a default", -> it "resets the setting back to the default", -> - fs.writeFileSync(atom.config.configFilePath, "foo: { baz: 'new'}") + writeConfigFile("foo: { baz: 'new'}") waitsFor 'update event', -> updatedHandler.callCount > 0 runs -> expect(atom.config.get('foo.bar')).toBe 'def' @@ -808,19 +832,20 @@ describe "Config", -> describe "when the config file changes to be empty", -> beforeEach -> - fs.writeFileSync(atom.config.configFilePath, "") + writeConfigFile("") 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() describe "when the config file subsequently changes again to contain configuration", -> beforeEach -> updatedHandler.reset() - fs.writeFileSync(atom.config.configFilePath, "foo: bar: 'newVal'") + writeConfigFile("foo: bar: 'newVal'") waitsFor 'update event', -> updatedHandler.callCount > 0 it "sets the setting to the value specified in the config file", -> @@ -829,7 +854,7 @@ describe "Config", -> describe "when the config file changes to contain invalid cson", -> beforeEach -> spyOn(console, 'error') - fs.writeFileSync(atom.config.configFilePath, "}}}") + writeConfigFile("}}}") waitsFor "error to be logged", -> console.error.callCount > 0 it "logs a warning and does not update config data", -> @@ -840,11 +865,12 @@ describe "Config", -> describe "when the config file subsequently changes again to contain valid cson", -> beforeEach -> - fs.writeFileSync(atom.config.configFilePath, "foo: bar: 'newVal'") + writeConfigFile("foo: bar: 'newVal'") 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() describe ".initializeConfigDirectory()", -> diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index ddb728686..e726d98f9 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -106,9 +106,9 @@ beforeEach -> spyOn(atom.menu, 'sendToBrowserProcess') # reset config before each spec; don't load or save from/to `config.json` + spyOn(Config::, 'load') + spyOn(Config::, 'save') config = new Config({resourcePath, configDirPath: atom.getConfigDirPath()}) - spyOn(config, 'load') - spyOn(config, 'save') atom.config = config atom.loadConfig() config.set "core.destroyEmptyPanes", false @@ -118,6 +118,7 @@ beforeEach -> config.set "core.disabledPackages", ["package-that-throws-an-exception", "package-with-broken-package-json", "package-with-broken-keymap"] config.set "editor.useShadowDOM", true + advanceClock(1000) config.load.reset() config.save.reset() diff --git a/src/config.coffee b/src/config.coffee index a28beaca8..3a2dd16a3 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -318,6 +318,9 @@ class Config @configFilePath ?= path.join(@configDirPath, 'config.cson') @transactDepth = 0 + @debouncedSave = _.debounce(@save, 100) + @debouncedLoad = _.debounce(@loadUserConfig, 100) + ### Section: Config Subscription ### @@ -564,7 +567,7 @@ class Config else @setRawValue(keyPath, value) - @save() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors + @debouncedSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors true # Essential: Restore the setting at `keyPath` to its default value. @@ -594,16 +597,15 @@ class Config _.setValueForKeyPath(settings, keyPath, undefined) settings = withoutEmptyObjects(settings) @set(null, settings, {scopeSelector, source, priority: @priorityForSource(source)}) if settings? - @save() unless @configFileHasErrors + @debouncedSave() else - @scopedSettingsStore.removePropertiesForSource(source) + @scopedSettingsStore.removePropertiesForSourceAndSelector(source, scopeSelector) @emitChangeEvent() else @scopedSettingsStore.removePropertiesForSource(source) if keyPath? @set(keyPath, _.valueForKeyPath(@defaultSettings, keyPath)) - # Extended: Get an {Array} of all of the `source` {String}s with which # settings have been added via {::set}. getSources: -> @@ -797,7 +799,7 @@ class Config observeUserConfig: -> try @watchSubscription ?= pathWatcher.watch @configFilePath, (eventType) => - @loadUserConfig() if eventType is 'change' and @watchSubscription? + @debouncedLoad() if eventType is 'change' and @watchSubscription? catch error @notifyFailure('Failed to watch user config', error)