From 5b910b557e4ec09603161ff2e72da61c67607397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 23 Nov 2016 11:34:58 +0100 Subject: [PATCH] Fixed a rare race condition causing a "settings changed" popup in case of local changes If the SettingsUpdated event for whatever reason got slightly delayed and arrived AFTER the save request was already processed, in rare situations it could happen that the "Settings Changed" popup was triggered even though the settings had already been successfully saved. Modified such that we now keep track of if we already saw the SettingsUpdated event associated with our save request and if not we ignore the next one. To ensure that we don't get out of sync due to that when a settings update request is sent, but no settings are actually change, we also now always trigger the SettingsUpdated event, even in such cases. Clients can use the hashs in the payload to test if something actually changed - if necessary. --- src/octoprint/server/api/settings.py | 13 +++++++------ .../static/js/app/viewmodels/settings.js | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/octoprint/server/api/settings.py b/src/octoprint/server/api/settings.py index 339a77a2..52ca7c32 100644 --- a/src/octoprint/server/api/settings.py +++ b/src/octoprint/server/api/settings.py @@ -359,9 +359,10 @@ def _saveSettings(data): except: logger.exception("Could not save settings for plugin {name} ({version})".format(version=plugin._plugin_version, name=plugin._plugin_name)) - if s.save(): - payload = dict( - config_hash=s.config_hash, - effective_hash=s.effective_hash - ) - eventManager().fire(Events.SETTINGS_UPDATED, payload=payload) + s.save() + + payload = dict( + config_hash=s.config_hash, + effective_hash=s.effective_hash + ) + eventManager().fire(Events.SETTINGS_UPDATED, payload=payload) diff --git a/src/octoprint/static/js/app/viewmodels/settings.js b/src/octoprint/static/js/app/viewmodels/settings.js index 63407add..102a9a90 100644 --- a/src/octoprint/static/js/app/viewmodels/settings.js +++ b/src/octoprint/static/js/app/viewmodels/settings.js @@ -16,6 +16,10 @@ $(function() { }); self.outstanding = []; + self.active = false; + self.sawUpdateEventWhileActive = false; + self.ignoreNextUpdateEvent = false; + self.settingsDialog = undefined; self.settings_dialog_update_detected = undefined; self.translationManagerDialog = undefined; @@ -728,6 +732,7 @@ $(function() { self.settingsDialog.trigger("beforeSave"); + self.sawUpdateEventWhileSending = false; self.sending(data == undefined || options.sending || false); if (data == undefined) { @@ -735,10 +740,15 @@ $(function() { data = getOnlyChangedData(self.getLocalData(), self.lastReceivedSettings); } + self.active = true; return OctoPrint.settings.save(data) .done(function(data, status, xhr) { + self.ignoreNextUpdateEvent = !self.sawUpdateEventWhileSending; + self.active = false; + self.receiving(true); self.sending(false); + try { self.fromResponse(data); if (options.success) options.success(data, status, xhr); @@ -748,6 +758,7 @@ $(function() { }) .fail(function(xhr, status, error) { self.sending(false); + self.active = false; if (options.error) options.error(xhr, status, error); }) .always(function(xhr, status) { @@ -756,6 +767,10 @@ $(function() { }; self.onEventSettingsUpdated = function() { + if (self.active) { + self.sawUpdateEventWhileActive = true; + } + var preventSettingsRefresh = _.any(self.allViewModels, function(viewModel) { if (viewModel.hasOwnProperty("onSettingsPreventRefresh")) { try { @@ -776,7 +791,8 @@ $(function() { if (self.isDialogActive()) { // dialog is open and not currently busy... - if (self.sending() || self.receiving()) { + if (self.sending() || self.receiving() || self.active || self.ignoreNextUpdateEvent) { + self.ignoreNextUpdateEvent = false; return; }