diff --git a/src/octoprint/plugin/__init__.py b/src/octoprint/plugin/__init__.py index 686c7836..c98b1cf9 100644 --- a/src/octoprint/plugin/__init__.py +++ b/src/octoprint/plugin/__init__.py @@ -295,19 +295,16 @@ class PluginSettings(object): self.set_preprocessors = dict(plugins=dict()) self.set_preprocessors["plugins"][plugin_key] = set_preprocessors - def prefix_path(path): - return ['plugins', self.plugin_key] + path - def prefix_path_in_args(args, index=0): result = [] if index == 0: - result.append(prefix_path(args[0])) + result.append(self._prefix_path(args[0])) result.extend(args[1:]) else: args_before = args[:index - 1] args_after = args[index + 1:] result.extend(args_before) - result.append(prefix_path(args[index])) + result.append(self._prefix_path(args[index])) result.extend(args_after) return result @@ -326,6 +323,7 @@ class PluginSettings(object): return kwargs self.access_methods = dict( + has =("has", prefix_path_in_args, add_getter_kwargs), get =("get", prefix_path_in_args, add_getter_kwargs), get_int =("getInt", prefix_path_in_args, add_getter_kwargs), get_float =("getFloat", prefix_path_in_args, add_getter_kwargs), @@ -333,7 +331,8 @@ class PluginSettings(object): set =("set", prefix_path_in_args, add_setter_kwargs), set_int =("setInt", prefix_path_in_args, add_setter_kwargs), set_float =("setFloat", prefix_path_in_args, add_setter_kwargs), - set_boolean=("setBoolean", prefix_path_in_args, add_setter_kwargs) + set_boolean=("setBoolean", prefix_path_in_args, add_setter_kwargs), + remove =("remove", prefix_path_in_args) ) self.deprecated_access_methods = dict( getInt ="get_int", @@ -344,6 +343,17 @@ class PluginSettings(object): setBoolean="set_boolean" ) + def _prefix_path(self, path=None): + if path is None: + path = list() + return ['plugins', self.plugin_key] + path + + def global_has(self, path, **kwargs): + return self.settings.has(path, **kwargs) + + def global_remove(self, path, **kwargs): + return self.settings.remove(path, **kwargs) + def global_get(self, path, **kwargs): """ Getter for retrieving settings not managed by the plugin itself from the core settings structure. Use this @@ -437,6 +447,12 @@ class PluginSettings(object): os.makedirs(path) return path + def get_all_data(self, **kwargs): + return self.settings.get(self._prefix_path(), **kwargs) + + def clean_all_data(self): + self.settings.remove(self._prefix_path()) + def __getattr__(self, item): all_access_methods = self.access_methods.keys() + self.deprecated_access_methods.keys() if item in all_access_methods: diff --git a/src/octoprint/plugin/types.py b/src/octoprint/plugin/types.py index c7acb288..b48c4394 100644 --- a/src/octoprint/plugin/types.py +++ b/src/octoprint/plugin/types.py @@ -1189,6 +1189,9 @@ class SettingsPlugin(OctoPrintPlugin): the plugin core system upon initialization of the implementation. """ + config_version_key = "_config_version" + """Key of the field in the settings that holds the configuration format version.""" + def on_settings_load(self): """ Loads the settings for the plugin, called by the Settings API view in order to retrieve all settings from @@ -1207,8 +1210,8 @@ class SettingsPlugin(OctoPrintPlugin): :return: the current settings of the plugin, as a dictionary """ data = self._settings.get([], asdict=True, merged=True) - if "_config_version" in data: - del data["_config_version"] + if self.config_version_key in data: + del data[self.config_version_key] return data def on_settings_save(self, data): @@ -1220,7 +1223,8 @@ class SettingsPlugin(OctoPrintPlugin): .. note:: The default implementation will persist your plugin's settings as is, so just in the structure and in the - types that were received by the Settings API view. + types that were received by the Settings API view. Values identical to the default settings values + will *not* be persisted. If you need more granular control here, e.g. over the used data types, you'll need to override this method and iterate yourself over all your settings, retrieving them (if set) from the supplied received ``data`` @@ -1228,15 +1232,27 @@ class SettingsPlugin(OctoPrintPlugin): Arguments: data (dict): The settings dictionary to be saved for the plugin + + Returns: + dict: The settings that differed from the defaults and were actually saved. """ import octoprint.util - if "_config_version" in data: - del data["_config_version"] + if self.config_version_key in data: + del data[self.config_version_key] - current = self._settings.get([], asdict=True, merged=True) - merged = octoprint.util.dict_merge(current, data) - self._settings.set([], merged) + # determine diff dict that contains minimal set of changes against the + # default settings - we only want to persist that, not everything + diff = octoprint.util.dict_diff(self.get_settings_defaults(), data) + + version = self.get_settings_version() + + to_persist = dict(diff) + if version: + to_persist[self.config_version_key] = version + self._settings.set([], to_persist) + + return diff def get_settings_defaults(self): """ @@ -1319,6 +1335,52 @@ class SettingsPlugin(OctoPrintPlugin): """ pass + def on_settings_cleanup(self): + """ + Called after migration and initialization but before call to :func:`on_settings_initialized`. + + Plugins may overwrite this method to perform additional clean up tasks. + + The default implementation just minimizes the data persisted on disk to only contain + the differences to the defaults (in case the current data was persisted with an older + version of OctoPrint that still duplicated default data). + """ + import octoprint.util + from octoprint.settings import NoSuchSettingsPath + + try: + # let's fetch the current persisted config (so only the data on disk, + # without the defaults) + config = self._settings.get([], asdict=True, incl_defaults=False, error_on_path=True) + except NoSuchSettingsPath: + # no config persisted, nothing to do => get out of here + return + + if config is None: + # config is set to None, that doesn't make sense, kill it and leave + self._settings.clean_all_data() + return + + if self.config_version_key in config and config[self.config_version_key] is None: + # delete None entries for config version - it's the default, no need + del config[self.config_version_key] + + # calculate a minimal diff between the settings and the current config - + # anything already in the settings will be removed from the persisted + # config, no need to duplicate it + defaults = self.get_settings_defaults() + diff = octoprint.util.dict_diff(defaults, config) + + if not diff: + # no diff to defaults, no need to have anything persisted + self._settings.clean_all_data() + else: + # diff => persist only that + self._settings.set([], diff) + + # finally save everything + self._settings.save() + def on_settings_initialized(self): """ Called after the settings have been initialized and - if necessary - also been migrated through a call to diff --git a/src/octoprint/server/__init__.py b/src/octoprint/server/__init__.py index b0b3ac6c..973a1472 100644 --- a/src/octoprint/server/__init__.py +++ b/src/octoprint/server/__init__.py @@ -224,7 +224,7 @@ class Server(object): set_preprocessors=set_preprocessors) return dict(settings=plugin_settings) - def settings_plugin_config_migration(name, implementation): + def settings_plugin_config_migration_and_cleanup(name, implementation): if not isinstance(implementation, octoprint.plugin.SettingsPlugin): return @@ -232,12 +232,13 @@ class Server(object): settings_migrator = implementation.on_settings_migrate if settings_version is not None and settings_migrator is not None: - stored_version = implementation._settings.get_int(["_config_version"]) + stored_version = implementation._settings.get_int([octoprint.plugin.SettingsPlugin.config_version_key]) if stored_version is None or stored_version < settings_version: settings_migrator(settings_version, stored_version) - implementation._settings.set_int(["_config_version"], settings_version) + implementation._settings.set_int([octoprint.plugin.SettingsPlugin.config_version_key], settings_version) implementation._settings.save() + implementation.on_settings_cleanup() implementation.on_settings_initialized() pluginManager.implementation_inject_factories=[octoprint_plugin_inject_factory, settings_plugin_inject_factory] @@ -246,11 +247,11 @@ class Server(object): settingsPlugins = pluginManager.get_implementations(octoprint.plugin.SettingsPlugin) for implementation in settingsPlugins: try: - settings_plugin_config_migration(implementation._identifier, implementation) + settings_plugin_config_migration_and_cleanup(implementation._identifier, implementation) except: self._logger.exception("Error while trying to migrate settings for plugin {}, ignoring it".format(implementation._identifier)) - pluginManager.implementation_post_inits=[settings_plugin_config_migration] + pluginManager.implementation_post_inits=[settings_plugin_config_migration_and_cleanup] pluginManager.log_all_plugins() diff --git a/src/octoprint/settings.py b/src/octoprint/settings.py index 75ee8142..65ffae0c 100644 --- a/src/octoprint/settings.py +++ b/src/octoprint/settings.py @@ -312,6 +312,11 @@ default_settings = { valid_boolean_trues = [True, "true", "yes", "y", "1"] """ Values that are considered to be equivalent to the boolean ``True`` value, used for type conversion in various places.""" + +class NoSuchSettingsPath(BaseException): + pass + + class Settings(object): """ The :class:`Settings` class allows managing all of OctoPrint's settings. It takes care of initializing the settings @@ -832,13 +837,13 @@ class Settings(object): stat = os.stat(self._configfile) return stat.st_mtime - #~~ getter + ##~~ Internal getter - def get(self, path, asdict=False, config=None, defaults=None, preprocessors=None, merged=False, incl_defaults=True): + def _get_value(self, path, asdict=False, config=None, defaults=None, preprocessors=None, merged=False, incl_defaults=True): import octoprint.util as util if len(path) == 0: - return None + raise NoSuchSettingsPath() if config is None: config = self._config @@ -856,7 +861,7 @@ class Settings(object): config = {} defaults = defaults[key] else: - return None + raise NoSuchSettingsPath() if preprocessors and isinstance(preprocessors, dict) and key in preprocessors: preprocessors = preprocessors[key] @@ -880,7 +885,7 @@ class Settings(object): elif incl_defaults and key in defaults: value = defaults[key] else: - value = None + raise NoSuchSettingsPath() if preprocessors and isinstance(preprocessors, dict) and key in preprocessors and callable(preprocessors[key]): value = preprocessors[key](value) @@ -898,8 +903,34 @@ class Settings(object): else: return results - def getInt(self, path, config=None, defaults=None, preprocessors=None, incl_defaults=True): - value = self.get(path, config=config, defaults=defaults, preprocessors=preprocessors, incl_defaults=incl_defaults) + #~~ has + + def has(self, path, **kwargs): + try: + self._get_value(path, **kwargs) + except NoSuchSettingsPath: + return False + else: + return True + + #~~ getter + + def get(self, path, **kwargs): + error_on_path = kwargs.get("error_on_path", False) + new_kwargs = dict(kwargs) + if "error_on_path" in new_kwargs: + del new_kwargs["error_on_path"] + + try: + return self._get_value(path, **new_kwargs) + except NoSuchSettingsPath: + if error_on_path: + raise + else: + return None + + def getInt(self, path, **kwargs): + value = self.get(path, **kwargs) if value is None: return None @@ -909,8 +940,8 @@ class Settings(object): self._logger.warn("Could not convert %r to a valid integer when getting option %r" % (value, path)) return None - def getFloat(self, path, config=None, defaults=None, preprocessors=None, incl_defaults=True): - value = self.get(path, config=config, defaults=defaults, preprocessors=preprocessors, incl_defaults=incl_defaults) + def getFloat(self, path, **kwargs): + value = self.get(path, **kwargs) if value is None: return None @@ -920,8 +951,8 @@ class Settings(object): self._logger.warn("Could not convert %r to a valid integer when getting option %r" % (value, path)) return None - def getBoolean(self, path, config=None, defaults=None, preprocessors=None, incl_defaults=True): - value = self.get(path, config=config, defaults=defaults, preprocessors=preprocessors, incl_defaults=incl_defaults) + def getBoolean(self, path, **kwargs): + value = self.get(path, **kwargs) if value is None: return None if isinstance(value, bool): @@ -974,6 +1005,23 @@ class Settings(object): return script + #~~ remove + + def remove(self, path, config=None): + if config is None: + config = self._config + + while len(path) > 1: + key = path.pop(0) + if not isinstance(config, dict) or key not in config: + return + config = config[key] + + key = path.pop(0) + if isinstance(config, dict) and key in config: + del config[key] + self._dirty = True + #~~ setter def set(self, path, value, force=False, defaults=None, config=None, preprocessors=None): @@ -1020,9 +1068,9 @@ class Settings(object): config[key] = value self._dirty = True - def setInt(self, path, value, force=False, defaults=None, config=None, preprocessors=None): + def setInt(self, path, value, **kwargs): if value is None: - self.set(path, None, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, None, **kwargs) return try: @@ -1031,11 +1079,11 @@ class Settings(object): self._logger.warn("Could not convert %r to a valid integer when setting option %r" % (value, path)) return - self.set(path, intValue, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, intValue, **kwargs) - def setFloat(self, path, value, force=False, defaults=None, config=None, preprocessors=None): + def setFloat(self, path, value, **kwargs): if value is None: - self.set(path, None, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, None, **kwargs) return try: @@ -1044,15 +1092,15 @@ class Settings(object): self._logger.warn("Could not convert %r to a valid integer when setting option %r" % (value, path)) return - self.set(path, floatValue, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, floatValue, **kwargs) - def setBoolean(self, path, value, force=False, defaults=None, config=None, preprocessors=None): + def setBoolean(self, path, value, **kwargs): if value is None or isinstance(value, bool): - self.set(path, value, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, value, **kwargs) elif value.lower() in valid_boolean_trues: - self.set(path, True, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, True, **kwargs) else: - self.set(path, False, config=config, force=force, defaults=defaults, preprocessors=preprocessors) + self.set(path, False, **kwargs) def setBaseFolder(self, type, path, force=False): if type not in default_settings["folder"].keys(): diff --git a/src/octoprint/util/__init__.py b/src/octoprint/util/__init__.py index 9bfb0040..3cf932ce 100644 --- a/src/octoprint/util/__init__.py +++ b/src/octoprint/util/__init__.py @@ -432,6 +432,67 @@ def dict_clean(a, b): return result +def dict_diff(a, b): + """ + Recursively calculates the minimal dict that would be needed to be deep merged with + a in order to produce the same result as deep merging a and b. + + Example:: + + >>> a = dict(foo=dict(a=1, b=2), bar=dict(c=3, d=4)) + >>> b = dict(bar=dict(c=3, d=5), fnord=None) + >>> c = dict_diff(a, b) + >>> c == dict(bar=dict(d=5), fnord=None) + True + >>> dict_merge(a, c) == dict_merge(a, b) + True + + Arguments: + a (dict): Source dictionary + b (dict): Dictionary to compare to source dictionary and derive diff for + + Returns: + dict: The minimal dictionary to deep merge on a to get the same result + as deep merging b on a. + """ + + if not isinstance(a, dict) or not isinstance(b, dict): + raise ValueError("a and b must be dictionaries") + + if a == b: + # shortcut: if both are equal, we return an empty dict as result + return dict() + + from copy import deepcopy + + all_keys = set(a.keys() + b.keys()) + result = dict() + for k in all_keys: + if k not in b: + # key not contained in b => not contained in result + continue + + if k in a: + # key is present in both dicts, we have to take a look at the value + value_a = a[k] + value_b = b[k] + + if value_a != value_b: + # we only need to look further if the values are not equal + + if isinstance(value_a, dict) and isinstance(value_b, dict): + # both are dicts => deeper down it goes into the rabbit hole + result[k] = dict_diff(value_a, value_b) + else: + # new b wins over old a + result[k] = deepcopy(value_b) + + else: + # key is new, add it + result[k] = deepcopy(b[k]) + return result + + def dict_contains_keys(a, b): """ Recursively deep-checks if ``a`` contains all keys found in ``b``.