From 8a4934e6f825707c75cad8cf26b00458e580a523 Mon Sep 17 00:00:00 2001 From: Andy Werner Date: Tue, 7 Mar 2017 10:35:33 +0100 Subject: [PATCH] Fix an issue in user settings * UserSettings: crashed if overwrite an existing key with primitive value by same key with complex value. Also reduced number of calls to user.get_setting() * Updated tests (cherry picked from commit b75b53d) --- AUTHORS.md | 1 + src/octoprint/users.py | 9 ++++----- tests/users/test_user.py | 4 +++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index 917011e7..a91f18e6 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -77,6 +77,7 @@ date of first contribution): * [Noah Martin](https://github.com/noahsmartin) * [Eyal Soha](https://github.com/eyal0) * [Greg Hulands](https://github.com/ghulands) + * [Andreas Werner](https://github.com/gallore) OctoPrint started off as a fork of [Cura](https://github.com/daid/Cura) by [Daid Braam](https://github.com/daid). Parts of its communication layer and diff --git a/src/octoprint/users.py b/src/octoprint/users.py index 56ddf786..5f05bc75 100644 --- a/src/octoprint/users.py +++ b/src/octoprint/users.py @@ -314,9 +314,8 @@ class FilebasedUserManager(UserManager): raise UnknownUser(username) user = self._users[username] - current = user.get_setting(key) - if not current or current != value: - old_value = user.get_setting(key) + old_value = user.get_setting(key) + if not old_value or old_value != value: user.set_setting(key, value) self._dirty = self._dirty or old_value != value self._save() @@ -482,7 +481,7 @@ class User(UserMixin): def _get_setting(self, path): s = self._settings for p in path: - if p in s: + if isinstance(s, dict) and p in s: s = s[p] else: return None @@ -495,7 +494,7 @@ class User(UserMixin): s[p] = dict() if not isinstance(s[p], dict): - return False + s[p] = dict() s = s[p] diff --git a/tests/users/test_user.py b/tests/users/test_user.py index 4665a215..e827d3e8 100644 --- a/tests/users/test_user.py +++ b/tests/users/test_user.py @@ -63,7 +63,9 @@ class UserTestCase(unittest.TestCase): (["sub", "othersubkey"], "othersubvalue", dict(sub=dict(othersubkey="othersubvalue")), True), ("booleankey", True, dict(booleankey=True), True), (["newsub", "newsubkey"], "newsubvalue", dict(newsub=dict(newsubkey="newsubvalue")), True), - (["sub", "subkey", "wontwork"], "wontwork", dict(), False) + # ["sub", "subkey"] is already existing and gets overwritten + (["sub", "subkey", "subsubkey"], "42", dict(sub=dict(subkey=dict(subsubkey="42"))), True), + (["sub"], "overwrite", dict(sub="overwrite"), True) ) @ddt.unpack def test_set_setting_string(self, key, value, update, expected_returnvalue):