Secure convert_unit() function

This secures the utils.convert_unit function. Following work has been done:

* Convert all parameters to decimal.Decimal and handle all parameters the
  same way ('offset' and 'value' could not be a fraction until now,
  and there was a TypeError when 'factor' was an int).

* Set a default value to factor argument to stay consistent with
  the offset parameter.

* Pass factor and offset as ints everywhere to keep consistency across
  collectors.

Change-Id: Idd31490ff01d42530cb6b54eef042a0746d12632

cloudkitty/collector/ceilometer.py

@@ -261,8 +261,8 @@ class CeilometerCollector(collector.BaseCollector):
                 conv_data = METRICS_CONF['metrics_units']['image']
                 image_size_mb = ck_utils.convert_unit(
                     decimal.Decimal(image_stats.max),
-                    conv_data['image.size'].get('factor', '1'),
-                    conv_data['image.size'].get('offset', '0'),
+                    conv_data['image.size'].get('factor', 1),
+                    conv_data['image.size'].get('offset', 0),
                 )
             except KeyError:
                 LOG.warning('Error when trying to use yaml metrology conf.')
@@ -315,8 +315,8 @@ class CeilometerCollector(collector.BaseCollector):
                 conv_data = METRICS_CONF['metrics_units']['volume']
                 volume_stats.max = ck_utils.convert_unit(
                     decimal.Decimal(volume_stats.max),
-                    conv_data['volume.size'].get('factor', '1'),
-                    conv_data['volume.size'].get('offset', '0'),
+                    conv_data['volume.size'].get('factor', 1),
+                    conv_data['volume.size'].get('offset', 0),
                 )
 
                 volume_data.append(self.t_cloudkitty.format_item(
@@ -374,8 +374,8 @@ class CeilometerCollector(collector.BaseCollector):
                 conv = METRICS_CONF['metrics_units']['network.bw.' + direction]
                 tap_bw_mb = ck_utils.convert_unit(
                     decimal.Decimal(tap_stat.max),
-                    conv[resource_type].get('factor', '1'),
-                    conv[resource_type].get('offset', '0'),
+                    conv[resource_type].get('factor', 1),
+                    conv[resource_type].get('offset', 0),
                 )
             except KeyError:
                 LOG.warning('Error when trying to use yaml metrology conf.')
@@ -489,8 +489,8 @@ class CeilometerCollector(collector.BaseCollector):
                 conv_data = METRICS_CONF['metrics_units']['radosgw.usage']
                 rgw_size = ck_utils.convert_unit(
                     decimal.Decimal(rgw_stats.max),
-                    conv_data['radosgw.object.size'].get('factor', '1'),
-                    conv_data['radosgw.object.size'].get('offset', '0'),
+                    conv_data['radosgw.object.size'].get('factor', 1),
+                    conv_data['radosgw.object.size'].get('offset', 0),
                 )
 
                 rgw_data.append(

cloudkitty/collector/gnocchi.py

@@ -299,8 +299,8 @@ class GnocchiCollector(collector.BaseCollector):
                 if isinstance(qty, str):
                     resource_data[qty] = ck_utils.convert_unit(
                         resource_data[qty],
-                        conv_data.get('factor', '1'),
-                        conv_data.get('offset', '0'),
+                        conv_data.get('factor', 1),
+                        conv_data.get('offset', 0),
                     )
             # NOTE(mc): deprecated except part kept for backward compatibility.
             except KeyError:

cloudkitty/tests/test_utils.py

@@ -16,6 +16,9 @@
 # @author: Stéphane Albert
 #
 import datetime
+import decimal
+import fractions
+import itertools
 import unittest
 
 import mock
@@ -136,3 +139,59 @@ class UtilsTimeCalculationsTest(unittest.TestCase):
         calc_dt = ck_utils.iso2dt(self.date_iso)
         check_dt = ck_utils.ts2dt(self.date_ts)
         self.assertEqual(calc_dt, check_dt)
+
+
+class ConvertUnitTest(unittest.TestCase):
+    """Class testing the convert_unit and num2decimal function"""
+    possible_args = [
+        None,  # Use default arg
+        '2/3',
+        decimal.Decimal(1.23),
+        '1.23',
+        2,
+        '2',
+        2.3,
+    ]
+
+    def test_arg_types(self):
+        """Test function with several arg combinations of different types"""
+        for fac, off in itertools.product(self.possible_args, repeat=2):
+            factor = fac if fac else 1
+            offset = off if off else 0
+            ck_utils.convert_unit(10, factor, offset)
+
+    def test_str_str_str(self):
+        result = ck_utils.convert_unit('1/2', '1/2', '1/2')
+        self.assertEqual(result, decimal.Decimal(0.5 * 0.5 + 0.5))
+
+    def test_str_float_float(self):
+        result = ck_utils.convert_unit('1/2', 0.5, 0.5)
+        self.assertEqual(result, decimal.Decimal(0.5 * 0.5 + 0.5))
+
+    def test_convert_str_float(self):
+        result = ck_utils.num2decimal('2.0')
+        self.assertEqual(result, decimal.Decimal(2.0))
+
+    def test_convert_str_int(self):
+        result = ck_utils.num2decimal('2')
+        self.assertEqual(result, decimal.Decimal(2))
+
+    def test_convert_str_fraction(self):
+        result = ck_utils.num2decimal('2/3')
+        self.assertEqual(result, decimal.Decimal(2.0 / 3))
+
+    def test_convert_fraction(self):
+        result = ck_utils.num2decimal(fractions.Fraction(1, 2))
+        self.assertEqual(result, decimal.Decimal(1.0 / 2))
+
+    def test_convert_float(self):
+        result = ck_utils.num2decimal(0.5)
+        self.assertEqual(result, decimal.Decimal(0.5))
+
+    def test_convert_int(self):
+        result = ck_utils.num2decimal(2)
+        self.assertEqual(result, decimal.Decimal(2))
+
+    def test_convert_decimal(self):
+        result = ck_utils.num2decimal(decimal.Decimal(2))
+        self.assertEqual(result, decimal.Decimal(2))

cloudkitty/utils.py

@@ -300,18 +300,25 @@ def tempdir(**kwargs):
                       six.text_type(e))
 
 
-def convert_unit(value, factor, offset=0):
-    """Return converted value depending on the provided factor and offset."""
-    # Check if factor is a fraction
-    if '/' in factor:
-        tmp = fractions.Fraction(factor)
-        numerator = decimal.Decimal(tmp.numerator)
-        denominator = decimal.Decimal(tmp.denominator)
-        factor = numerator / denominator
-    else:
-        factor = decimal.Decimal(factor)
+def num2decimal(num):
+    """Converts a number into a decimal.Decimal.
 
-    return (decimal.Decimal(value) * factor) + decimal.Decimal(offset)
+    The number may be an str in float, int or fraction format;
+    a fraction.Fraction, a decimal.Decimal, an int or a float.
+    """
+    if isinstance(num, decimal.Decimal):
+        return num
+    if isinstance(num, str):
+        if '/' in num:
+            num = float(fractions.Fraction(num))
+    if isinstance(num, fractions.Fraction):
+        num = float(num)
+    return decimal.Decimal(num)
+
+
+def convert_unit(value, factor=1, offset=0):
+    """Return converted value depending on the provided factor and offset."""
+    return num2decimal(value) * num2decimal(factor) + num2decimal(offset)
 
 
 def flat_dict(item, parent=None):