Skip to content
This repository has been archived by the owner on Feb 2, 2018. It is now read-only.

Commit

Permalink
models: Drop 'secure' arg from to_json() and to_dict().
Browse files Browse the repository at this point in the history
We want secure=True the vast majority of the time, but the arguments
default to False and it's too easy to forget that.
  • Loading branch information
mbarnes authored and ashcrow committed Nov 28, 2016
1 parent e68577e commit 5f236f1
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 20 deletions.
12 changes: 5 additions & 7 deletions src/commissaire/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,16 @@ def _dict_for_json(self, secure):
data[key] = getattr(self, key)
return data

def to_json(self, secure=False):
def to_json(self):
"""
Returns a JSON representation of this model.
:param secure: Include _hidden attributes in the return value.
:type secure: bool
:returns: The JSON representation.
:rtype: str
"""
return json.dumps(
self._struct_for_json(secure=secure),
default=lambda o: o._struct_for_json(secure=secure))
self._struct_for_json(secure=True),
default=lambda o: o._struct_for_json(secure=True))

def to_json_safe(self):
"""
Expand All @@ -174,7 +172,7 @@ def to_json_safe(self):
self._struct_for_json(secure=False),
default=lambda o: o._struct_for_json(secure=False))

def to_dict(self, secure=False):
def to_dict(self):
"""
Returns a dict representation of this model. This is different than
using __dict__ as the returned data will be model specific only.
Expand All @@ -186,7 +184,7 @@ def to_dict(self, secure=False):
"""
# Instead of reimplementing the logic take the performance hit of
# going between native and json
return json.loads(self.to_json(secure))
return json.loads(self.to_json())

def to_dict_safe(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion src/commissaire/storage/etcd.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def _save(self, model_instance):
:rtype: commissaire.model.Model
"""
key = self._format_key(model_instance)
self._store.write(key, model_instance.to_json(secure=True))
self._store.write(key, model_instance.to_json())
# TODO: Check if we need to update the data in the instance
return model_instance

Expand Down
16 changes: 4 additions & 12 deletions test/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,12 @@ def test_new(self):

def test_to_json(self):
"""
Verify to_json returns a sanitized json string.
Verify to_json returns a complete json string.
"""
instance = models.Host.new(ssh_priv_key='secret')
# ssh_priv_key should be hidden unless secure is True
self.assertNotIn(
'ssh_priv_key',
json.loads(instance.to_json()))
self.assertIn(
'ssh_priv_key',
json.loads(instance.to_json(secure=True)))
json.loads(instance.to_json()))

def test_to_json_safe(self):
"""
Expand All @@ -63,16 +59,12 @@ def test_to_json_safe(self):

def test_to_dict(self):
"""
Verify to_dict returns a sanitized json string.
Verify to_dict returns a complete dict.
"""
instance = models.Host.new(ssh_priv_key='secret')
# ssh_priv_key should be hidden unless secure is True
self.assertNotIn(
'ssh_priv_key',
instance.to_dict())
self.assertIn(
'ssh_priv_key',
instance.to_dict(secure=True))
instance.to_dict())

def test_to_dict_safe(self):
"""
Expand Down

0 comments on commit 5f236f1

Please sign in to comment.