From 8f3f15b6d1023eadbf4b18dc7755778ebb3a4dd7 Mon Sep 17 00:00:00 2001 From: ZhiQiang Fan Date: Wed, 4 Jun 2014 02:10:53 +0800 Subject: [PATCH] Fix incorrect trait initialization Currently, when we try to get trait, the api service uses storage data to directly initialize the trait model. However, the trait class init method expects value field as wsme.text type, while in storage, it could be int, float, or datetime. So, end user will get 400 error in such scenarios. The static method _convert_storage_trait() in Event class can convert a storage trait data to api trait object, we can use it to fix this bug. Also, this method should be in Trait class instead of Event class, This patch does this refactor too. Change-Id: Ia7bf667967a31a3cc6e8d457f3b6330ffc728471 Closes-Bug: #1324523 --- ceilometer/api/controllers/v2.py | 30 +++++++++---------- .../tests/api/v2/test_event_scenarios.py | 15 ++++++++-- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/ceilometer/api/controllers/v2.py b/ceilometer/api/controllers/v2.py index 4083edd8e..a1050ae0e 100644 --- a/ceilometer/api/controllers/v2.py +++ b/ceilometer/api/controllers/v2.py @@ -2185,6 +2185,19 @@ class Trait(_Base): type = wtypes.text "the type of the trait (string, integer, float or datetime)" + @staticmethod + def _convert_storage_trait(trait): + """Helper method to convert a storage model into an API trait + instance. If an API trait instance is passed in, just return it. + """ + if isinstance(trait, Trait): + return trait + value = (six.text_type(trait.value) + if not trait.dtype == storage.models.Trait.DATETIME_TYPE + else trait.value.isoformat()) + trait_type = storage.models.Trait.get_name_by_type(trait.dtype) + return Trait(name=trait.name, type=trait_type, value=value) + @classmethod def sample(cls): return cls(name='service', @@ -2207,21 +2220,8 @@ class Event(_Base): def get_traits(self): return self._traits - @staticmethod - def _convert_storage_trait(t): - """Helper method to convert a storage model into an API trait - instance. If an API trait instance is passed in, just return it. - """ - if isinstance(t, Trait): - return t - value = (six.text_type(t.value) - if not t.dtype == storage.models.Trait.DATETIME_TYPE - else t.value.isoformat()) - type = storage.models.Trait.get_name_by_type(t.dtype) - return Trait(name=t.name, type=type, value=value) - def set_traits(self, traits): - self._traits = map(self._convert_storage_trait, traits) + self._traits = map(Trait._convert_storage_trait, traits) traits = wsme.wsproperty(wtypes.ArrayType(Trait), get_traits, @@ -2293,7 +2293,7 @@ class TraitsController(rest.RestController): :param trait_name: Trait to return values for """ LOG.debug(_("Getting traits for %s") % event_type) - return [Trait(name=t.name, type=t.get_type_name(), value=t.value) + return [Trait._convert_storage_trait(t) for t in pecan.request.storage_conn .get_traits(event_type, trait_name)] diff --git a/ceilometer/tests/api/v2/test_event_scenarios.py b/ceilometer/tests/api/v2/test_event_scenarios.py index 26ba1d579..ea5794ad3 100644 --- a/ceilometer/tests/api/v2/test_event_scenarios.py +++ b/ceilometer/tests/api/v2/test_event_scenarios.py @@ -94,11 +94,20 @@ class TestTraitAPI(EventTestBase): def test_get_trait_data_for_event(self): path = (self.PATH % "Foo") + "/trait_A" data = self.get_json(path, headers=headers) - self.assertEqual(1, len(data)) + self.assertEqual("trait_A", data[0]['name']) - trait = data[0] - self.assertEqual("trait_A", trait['name']) + path = (self.PATH % "Foo") + "/trait_B" + data = self.get_json(path, headers=headers) + self.assertEqual(1, len(data)) + self.assertEqual("trait_B", data[0]['name']) + self.assertEqual("1", data[0]['value']) + + path = (self.PATH % "Foo") + "/trait_D" + data = self.get_json(path, headers=headers) + self.assertEqual(1, len(data)) + self.assertEqual("trait_D", data[0]['name']) + self.assertEqual(self.trait_time.isoformat(), data[0]['value']) def test_get_trait_data_for_non_existent_event(self): path = (self.PATH % "NO_SUCH_EVENT") + "/trait_A"