From 1ab10170a1f305151588b86b0677d7a5dd108039 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Wed, 3 Sep 2014 14:05:02 +0300 Subject: [PATCH] Remove some inline if/else statements Even though python syntax allows it, assignment of variables using inline if...else... statements have the nasty problem of not showing relevant information in the test coverage. Thus, by writing the assignment in a more canonical and verbose form, we are sure that we are actually covering the possible branches while testing. Change-Id: I4fbce4422f4986a76255ad468ee7dd7475e3f1cf --- barbican/common/validators.py | 7 ++++--- barbican/context.py | 4 +++- barbican/model/models.py | 33 ++++++++++++++++++++++----------- barbican/model/repositories.py | 8 ++++++-- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/barbican/common/validators.py b/barbican/common/validators.py index 640fedca4..5a2987522 100644 --- a/barbican/common/validators.py +++ b/barbican/common/validators.py @@ -168,7 +168,9 @@ class NewSecretValidator(ValidatorBase): def _extract_name(self, json_data): """Extracts and returns the name from the JSON data.""" name = json_data.get('name') - return name.strip() if name else None + if name: + return name.strip() + return None def _extract_expiration(self, json_data, schema_name): """Extracts and returns the expiration date from the JSON data.""" @@ -512,8 +514,7 @@ class ContainerValidator(ValidatorBase): if not secret_refs: return json_data - secret_refs_names = set(secret_ref['name'] - if 'name' in secret_ref else '' + secret_refs_names = set(secret_ref.get('name', '') for secret_ref in secret_refs) self._assert_validity( diff --git a/barbican/context.py b/barbican/context.py index 7e97605dc..6cc124883 100644 --- a/barbican/context.py +++ b/barbican/context.py @@ -82,7 +82,9 @@ class RequestContext(object): @property def owner(self): """Return the owner to correlate with key.""" - return self.tenant if self.owner_is_tenant else self.user + if self.owner_is_tenant: + return self.tenant + return self.user # TODO(jwood): # @property diff --git a/barbican/model/models.py b/barbican/model/models.py index 9f161fde4..ecc962009 100644 --- a/barbican/model/models.py +++ b/barbican/model/models.py @@ -74,10 +74,14 @@ class JsonBlob(sql_types.TypeDecorator): impl = sa.Text def process_bind_param(self, value, dialect): - return json.dumps(value) if value is not None else value + if value is not None: + return json.dumps(value) + return value def process_result_value(self, value, dialect): - return json.loads(value) if value is not None else value + if value is not None: + return json.loads(value) + return value class ModelBase(object): @@ -155,11 +159,15 @@ class ModelBase(object): def to_dict_fields(self): """Returns a dictionary of just the db fields of this entity.""" - created_at = (self.created_at.isoformat() - if self.created_at else self.created_at) + if self.created_at: + created_at = self.created_at.isoformat() + else: + created_at = self.created_at - updated_at = (self.updated_at.isoformat() - if self.updated_at else self.updated_at) + if self.updated_at: + updated_at = self.updated_at.isoformat() + else: + updated_at = self.updated_at dict_fields = { 'created': created_at, @@ -304,8 +312,10 @@ class Secret(BASE, ModelBase): def _do_extra_dict_fields(self): """Sub-class hook method: return dict of fields.""" - expiration = (self.expiration.isoformat() - if self.expiration else self.expiration) + if self.expiration: + expiration = self.expiration.isoformat() + else: + expiration = self.expiration return { 'secret_id': self.id, @@ -474,9 +484,10 @@ class Order(BASE, ModelBase): def _do_extra_dict_fields(self): """Sub-class hook method: return dict of fields.""" if not self.meta: - expiration = ( - self.secret_expiration.isoformat() - if self.secret_expiration else self.secret_expiration) + if self.secret_expiration: + expiration = self.secret_expiration.isoformat() + else: + expiration = self.secret_expiration ret = { 'secret': { 'name': self.secret_name or self.secret_id, diff --git a/barbican/model/repositories.py b/barbican/model/repositories.py index 3892c9dbd..d9e8ffed1 100644 --- a/barbican/model/repositories.py +++ b/barbican/model/repositories.py @@ -220,7 +220,8 @@ def clean_paging_values(offset_arg=0, limit_arg=CONF.default_limit_paging): try: offset = int(offset_arg) - offset = offset if offset >= 0 else 0 + if offset < 0: + offset = 0 except ValueError: offset = 0 @@ -342,7 +343,10 @@ class BaseRepo(object): entity.save(session=session) except sqlalchemy.exc.IntegrityError: LOG.exception('Problem saving entity for create') - values_id = values['id'] if values else None + if values: + values_id = values['id'] + else: + values_id = None raise exception.Duplicate("Entity ID {0} already exists!" .format(values_id)) LOG.debug('Elapsed repo '