From e4fd17cb70f8b27755a0dd4f543d30f93bb0282a Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Robles Date: Wed, 7 Jan 2015 15:16:02 +0200 Subject: [PATCH] Return the actual name value for entities It used to be the case that for secrets and containers, if the name was initially set to null, the GETing of the secret would actually return the UUID, which is not what's reflected in the database. This is now fixed. Issuing a GET for those entities will return what's actually in the database. On the other hand, there was a bug where the validator for secrets was actually turning empty strings into None, and this would be saved in the database. This was now fixed. Change-Id: I8ebcf752e47be97ecdd4cb3c5ab152b6717b5341 --- barbican/common/validators.py | 2 +- barbican/model/models.py | 4 ++-- .../api/v1/functional/test_orders.py | 4 +--- .../api/v1/functional/test_secrets.py | 3 ++- functionaltests/api/v1/models/base_models.py | 8 ++++++-- .../api/v1/smoke/test_containers.py | 18 +++++++++++++++--- functionaltests/api/v1/smoke/test_secrets.py | 14 +++++++++++++- 7 files changed, 40 insertions(+), 13 deletions(-) diff --git a/barbican/common/validators.py b/barbican/common/validators.py index 66da0e551..eb2335efa 100644 --- a/barbican/common/validators.py +++ b/barbican/common/validators.py @@ -178,7 +178,7 @@ class NewSecretValidator(ValidatorBase): def _extract_name(self, json_data): """Extracts and returns the name from the JSON data.""" name = json_data.get('name') - if name: + if isinstance(name, six.string_types): return name.strip() return None diff --git a/barbican/model/models.py b/barbican/model/models.py index 8cda27513..a76a6a6ff 100644 --- a/barbican/model/models.py +++ b/barbican/model/models.py @@ -323,7 +323,7 @@ class Secret(BASE, ModelBase): return { 'secret_id': self.id, - 'name': self.name or self.id, + 'name': self.name, 'expiration': expiration, 'algorithm': self.algorithm, 'bit_length': self.bit_length, @@ -601,7 +601,7 @@ class Container(BASE, ModelBase): def _do_extra_dict_fields(self): """Sub-class hook method: return dict of fields.""" return {'container_id': self.id, - 'name': self.name or self.id, + 'name': self.name, 'type': self.type, 'secret_refs': [ { diff --git a/functionaltests/api/v1/functional/test_orders.py b/functionaltests/api/v1/functional/test_orders.py index 552668077..25689eaf9 100644 --- a/functionaltests/api/v1/functional/test_orders.py +++ b/functionaltests/api/v1/functional/test_orders.py @@ -127,12 +127,10 @@ class OrdersTestCase(base.TestCase): # verify the new secret's name matches the name in the secret ref # in the newly created order. - secret_id_from_ref = utils.get_id_from_ref(order_resp.model.secret_ref) secret_resp = self.secret_behaviors.get_secret_metadata( order_resp.model.secret_ref) self.assertEqual(secret_resp.status_code, 200) - self.assertGreater(len(secret_id_from_ref), 0) - self.assertEqual(secret_resp.model.name, secret_id_from_ref) + self.assertEqual(secret_resp.model.name, test_model.meta['name']) @testcase.attr('positive') def test_order_and_secret_metadata_same(self): diff --git a/functionaltests/api/v1/functional/test_secrets.py b/functionaltests/api/v1/functional/test_secrets.py index 59c7e65e9..d27e4bf74 100644 --- a/functionaltests/api/v1/functional/test_secrets.py +++ b/functionaltests/api/v1/functional/test_secrets.py @@ -439,7 +439,8 @@ class SecretsTestCase(base.TestCase): 'punctuation': ['~!@#$%^&*()_+`-={}[]|:;<>,.?'], 'uuid': ['54262d9d-4bc7-4821-8df0-dc2ca8e112bb'], 'len_255': [base.TestCase.max_sized_field], - 'empty': [''] + 'empty': [''], + 'null': [None] }) @testcase.attr('positive') def test_secret_create_defaults_valid_name(self, name): diff --git a/functionaltests/api/v1/models/base_models.py b/functionaltests/api/v1/models/base_models.py index aeb72dbd3..d9da8df0e 100644 --- a/functionaltests/api/v1/models/base_models.py +++ b/functionaltests/api/v1/models/base_models.py @@ -58,11 +58,15 @@ class BaseModel(object): have been removed. """ + # NOTE(jaosorior): deleting a key from the incoming dictionary actually + # affects the model object. So we do a copy to avoid this. + resulting_dict = dictionary.copy() + # Dumping the keys to a list as we'll be changing the dict size empty_keys = [k for k, v in dictionary.iteritems() if v is None] for k in empty_keys: - del dictionary[k] - return dictionary + del resulting_dict[k] + return resulting_dict @classmethod def json_to_obj(cls, serialized_str): diff --git a/functionaltests/api/v1/smoke/test_containers.py b/functionaltests/api/v1/smoke/test_containers.py index 07ebd1a74..f05ec9c60 100644 --- a/functionaltests/api/v1/smoke/test_containers.py +++ b/functionaltests/api/v1/smoke/test_containers.py @@ -14,6 +14,7 @@ # limitations under the License. from testtools import testcase +from barbican.tests import utils from functionaltests.api import base from functionaltests.api.v1.behaviors import container_behaviors from functionaltests.api.v1.behaviors import secret_behaviors @@ -70,6 +71,7 @@ create_container_empty_data = { } +@utils.parameterized_test_case class ContainersTestCase(base.TestCase): def setUp(self): @@ -149,11 +151,21 @@ class ContainersTestCase(base.TestCase): self.assertEqual(resp.status_code, 201) self.assertGreater(len(container_ref), 0) + @utils.parameterized_dataset({ + 'alphanumeric': ['a2j3j6ll9'], + 'punctuation': ['~!@#$%^&*()_+`-={}[]|:;<>,.?'], + 'len_255': [str(bytearray().zfill(255))], + 'uuid': ['54262d9d-4bc7-4821-8df0-dc2ca8e112bb'], + 'empty': [''] + }) @testcase.attr('positive') - def test_container_get_defaults(self): + def test_container_get_defaults_w_valid_name(self, name): """Covers getting a generic container with a three secrets.""" test_model = container_models.ContainerModel( **create_container_defaults_data) + overrides = {'name': name} + test_model.override_values(**overrides) + secret_refs = [] for secret_ref in test_model.secret_refs: secret_refs.append(secret_ref['secret_ref']) @@ -166,9 +178,9 @@ class ContainersTestCase(base.TestCase): # Verify the response data self.assertEqual(get_resp.status_code, 200) - self.assertEqual(get_resp.model.name, "containername") + self.assertEqual(get_resp.model.name, test_model.name) self.assertEqual(get_resp.model.container_ref, container_ref) - self.assertEqual(get_resp.model.type, "generic") + self.assertEqual(get_resp.model.type, test_model.type) # Verify the secret refs in the response self.assertEqual(len(get_resp.model.secret_refs), 3) diff --git a/functionaltests/api/v1/smoke/test_secrets.py b/functionaltests/api/v1/smoke/test_secrets.py index 69f7caa37..f0a9fa954 100644 --- a/functionaltests/api/v1/smoke/test_secrets.py +++ b/functionaltests/api/v1/smoke/test_secrets.py @@ -16,6 +16,7 @@ import binascii from testtools import testcase +from barbican.tests import utils from functionaltests.api import base from functionaltests.api.v1.behaviors import secret_behaviors from functionaltests.api.v1.models import secret_models @@ -66,6 +67,7 @@ secret_create_two_phase_data = { } +@utils.parameterized_test_case class SecretsTestCase(base.TestCase): def setUp(self): @@ -90,10 +92,20 @@ class SecretsTestCase(base.TestCase): self.assertEqual(resp.model.status, "ACTIVE") self.assertGreater(resp.model.secret_ref, 0) + @utils.parameterized_dataset({ + 'alphanumeric': ['1f34ds'], + 'punctuation': ['~!@#$%^&*()_+`-={}[]|:;<>,.?'], + 'uuid': ['54262d9d-4bc7-4821-8df0-dc2ca8e112bb'], + 'len_255': [str(bytearray().zfill(255))], + 'empty': [''], + 'null': [None] + }) @testcase.attr('positive') - def test_secret_get_defaults_metadata(self): + def test_secret_get_defaults_metadata_w_valid_name(self, name): """Covers getting and checking a secret's metadata.""" test_model = secret_models.SecretModel(**secret_create_defaults_data) + overrides = {'name': name} + test_model.override_values(**overrides) resp, secret_ref = self.behaviors.create_secret(test_model) self.assertEqual(resp.status_code, 201)