From f1be116190b353eeb20f29a691fd27bd48caa016 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= <osd@foosel.net>
Date: Fri, 19 Jun 2015 11:12:24 +0200
Subject: [PATCH] SoftwareUpdate: Don't persist more check data than necessary
 in config

The SoftwareUpdate Plugin had a bug that caused way too much check data to be stored in the configuration, leading to plugins potentially being stuck in an "update available" loop although the update had already been applied. Now only the current version of github_commit update types is persisted, not the full check configuration.

Also introduced a configuration version and made the migration function migrate old configs to remove anything that was same as the default supplied for the "octoprint" and all plugin hook checks. That should clean things up in existing installations.
---
 .../plugins/softwareupdate/__init__.py        | 78 +++++++++++++++++--
 1 file changed, 70 insertions(+), 8 deletions(-)

diff --git a/src/octoprint/plugins/softwareupdate/__init__.py b/src/octoprint/plugins/softwareupdate/__init__.py
index 76264435..f92c70ee 100644
--- a/src/octoprint/plugins/softwareupdate/__init__.py
+++ b/src/octoprint/plugins/softwareupdate/__init__.py
@@ -91,6 +91,69 @@ class SoftwareUpdatePlugin(octoprint.plugin.BlueprintPlugin,
 		super(SoftwareUpdatePlugin, self).on_settings_save(data)
 		self._version_cache_ttl = self._settings.get_int(["cache_ttl"]) * 60
 
+	def get_settings_version(self):
+		return 1
+
+	def on_settings_migrate(self, target, current=None):
+		if current is None:
+			# there might be some left over data from the time we still persisted everything to settings,
+			# even the stuff that shouldn't be persisted but always provided by the hook - let's
+			# clean up
+
+			# take care of the octoprint entry
+			configured_checks = self._settings.get(["checks"], merged=True)
+			octoprint_check = dict(configured_checks["octoprint"])
+			if "type" in octoprint_check and not octoprint_check["type"] == "github_commit":
+				deletables=["current"]
+			else:
+				deletables=[]
+			octoprint_check = self._clean_settings_check("octoprint", octoprint_check, self.get_settings_defaults()["checks"]["octoprint"], delete=deletables, save=False)
+			configured_checks["octoprint"] = octoprint_check
+
+			# and the hooks
+			update_check_hooks = self._plugin_manager.get_hooks("octoprint.plugin.softwareupdate.check_config")
+			for name, hook in update_check_hooks.items():
+				try:
+					hook_checks = hook()
+				except:
+					self._logger.exception("Error while retrieving update information from plugin {name}".format(**locals()))
+				else:
+					for key, data in hook_checks.items():
+						if key in configured_checks:
+							settings_check = dict(configured_checks[key])
+							merged = dict_merge(data, settings_check)
+							if "type" in merged and not merged["type"] == "github_commit":
+								deletables = ["current", "displayVersion"]
+							else:
+								deletables = []
+
+							self._clean_settings_check(key, settings_check, data, delete=deletables, save=False)
+
+	def _clean_settings_check(self, key, data, defaults, delete=None, save=True):
+		if delete is None:
+			delete = []
+
+		for k, v in data.items():
+			if k in defaults and defaults[k] == data[k]:
+				del data[k]
+
+		for k in delete:
+			if k in data:
+				del data[k]
+
+		dummy_defaults = dict(plugins=dict())
+		dummy_defaults["plugins"][self._identifier] = dict(checks=dict())
+		dummy_defaults["plugins"][self._identifier]["checks"][key] = defaults
+		if len(data):
+			self._settings.set(["checks", key], data, defaults=dummy_defaults)
+		else:
+			self._settings.set(["checks", key], None, defaults=dummy_defaults)
+
+		if save:
+			self._settings.save()
+
+		return data
+
 	#~~ BluePrint API
 
 	@octoprint.plugin.BlueprintPlugin.route("/check", methods=["GET"])
@@ -380,15 +443,14 @@ class SoftwareUpdatePlugin(octoprint.plugin.BlueprintPlugin,
 
 			# persist the new version if necessary for check type
 			if check["type"] == "github_commit":
-				checks = self._settings.get(["checks"], merged=True)
-				if target in checks:
-					# TODO make this cleaner, right now it saves too much to disk
-					checks[target]["current"] = target_version
-					self._settings.set(["checks"], checks)
+				dummy_default = dict(plugins=dict())
+				dummy_default["plugins"][self._identifier] = dict(checks=dict())
+				dummy_default["plugins"][self._identifier]["checks"][target] = dict(current=None)
+				self._settings.set(["checks", target, "current"], target_version, defaults=dummy_default)
 
-					# we have to save here (even though that makes us save quite often) since otherwise the next
-					# load will overwrite our changes we just made
-					self._settings.save()
+				# we have to save here (even though that makes us save quite often) since otherwise the next
+				# load will overwrite our changes we just made
+				self._settings.save()
 
 		return target_error, target_result