[tor-commits] [sbws/maint-1.1] fix: state: Let json manage data types

[juga at torproject.org]

commit 26409a3d8187cc65147a25484655a7639bdb705e
Author: juga0 <juga at riseup.net>
Date:   Sat Mar 14 17:27:59 2020 +0000

    fix: state: Let json manage data types
    
    Since state uses json and json will raise an error when it can't
    decode/encode some datatype.
---
 sbws/util/state.py            | 34 +++++++---------------------------
 tests/unit/util/test_state.py | 43 -------------------------------------------
 2 files changed, 7 insertions(+), 70 deletions(-)

diff --git a/sbws/util/state.py b/sbws/util/state.py
index f460413..86cc59b 100644
--- a/sbws/util/state.py
+++ b/sbws/util/state.py
@@ -4,16 +4,12 @@ import json
 
 
 class State:
-    '''
-    State allows one to atomically access and update a simple state file on
-    disk across threads and across processes.
+    """
+    `json` wrapper to read a json file every time it gets a key and to write
+    to the file every time a key is set.
 
-    To put it blunty, due to limited developer time and his inability to
-    quickly find a way to safely access and update more complex data types
-    (namely, collections like list, set, and dict), you may only store simple
-    types of data as enumerated in _ALLOWED_TYPES. Keys must be strings.
-
-    Data is stored as JSON on disk in the provided file file.
+    Every time a key is got or set, the file is locked, to atomically access
+    and update the file across threads and across processes.
 
     >>> state = State('foo.state')
     >>> # state == {}
@@ -40,8 +36,8 @@ class State:
     >>> # We can do many of the same things with a State object as with a dict
     >>> for key in state: print(key)
     >>> # Prints 'linux', 'age', and 'name'
-    '''
-    _ALLOWED_TYPES = (int, float, str, bool, type(None))
+
+    """
 
     def __init__(self, fname):
         self._fname = fname
@@ -68,35 +64,19 @@ class State:
         Implements a dictionary ``get`` method reading and locking
         a json file.
         """
-        if not isinstance(key, str):
-            raise TypeError(
-                'Keys must be strings. %s is a %s' % (key, type(key)))
         self._state = self._read()
         return self._state.get(key, d)
 
     def __getitem__(self, key):
-        if not isinstance(key, str):
-            raise TypeError(
-                'Keys must be strings. %s is a %s' % (key, type(key)))
         self._state = self._read()
         return self._state.__getitem__(key)
 
     def __delitem__(self, key):
-        if not isinstance(key, str):
-            raise TypeError(
-                'Keys must be strings. %s is a %s' % (key, type(key)))
         self._state = self._read()
         self._state.__delitem__(key)
         self._write()
 
     def __setitem__(self, key, value):
-        if not isinstance(key, str):
-            raise TypeError(
-                'Keys must be strings. %s is a %s' % (key, type(key)))
-        if type(value) not in State._ALLOWED_TYPES:
-            raise TypeError(
-                'May only store value with type in %s, not %s' %
-                (State._ALLOWED_TYPES, type(value)))
         # NOTE: important, read the file before setting the key,
         # otherwise if other instances are creating other keys, they're lost.
         self._state = self._read()
diff --git a/tests/unit/util/test_state.py b/tests/unit/util/test_state.py
index 2443049..a57768d 100644
--- a/tests/unit/util/test_state.py
+++ b/tests/unit/util/test_state.py
@@ -11,26 +11,6 @@ def test_state_set_allowed_key_types(tmpdir):
         assert state[key] == 4
 
 
-def test_state_set_bad_key_types(tmpdir):
-    state = State(os.path.join(str(tmpdir), 'statefoo'))
-    attempt_keys = (15983, None, True, -1.2, [], {}, set())
-    for key in attempt_keys:
-        try:
-            state[key] = 4
-        except TypeError:
-            pass
-        else:
-            assert None, 'Should not have been able to use %s %s as a key' %\
-                (key, type(key))
-    try:
-        state[key]
-    except TypeError:
-        pass
-    else:
-        assert None, '%s %s is not a valid key type, so should have got '\
-            'TypeError when giving it' % (key, type(key))
-
-
 def test_state_set_allowed_value_types(tmpdir):
     state = State(os.path.join(str(tmpdir), 'statefoo'))
     attempt_vals = (15983, None, True, -1.2, 'loooooool')
@@ -39,19 +19,6 @@ def test_state_set_allowed_value_types(tmpdir):
         assert state['foo'] == val
 
 
-def test_state_set_bad_value_types(tmpdir):
-    state = State(os.path.join(str(tmpdir), 'statefoo'))
-    attempt_vals = ([], {}, set())
-    for val in attempt_vals:
-        try:
-            state['foo'] = val
-        except TypeError:
-            pass
-        else:
-            assert None, 'Should not have been able to use %s %s as a value' %\
-                (val, type(val))
-
-
 def test_state_del(tmpdir):
     state = State(os.path.join(str(tmpdir), 'statefoo'))
     d = {'a': 1, 'b': 2, 'c': 3, 'd': 4}
@@ -65,16 +32,6 @@ def test_state_del(tmpdir):
     for key in d:
         assert d[key] == state[key]
 
-    attempt_keys = (15983, None, True, -1.2, [], {}, set())
-    for key in attempt_keys:
-        try:
-            del state[key]
-        except TypeError:
-            pass
-        else:
-            assert None, 'Should not have been allowed to delete %s %s '\
-                'because it is not a valid key type' % (key, type(key))
-
     d['e'] = 5
     state['e'] = 5
     d['e'] = 5.5