Improves code quality of block storage ordering code

Kevin McDonald · Kevin McDonald · commit 653b0da59dab · 2016-04-25T11:57:40.000-05:00
diff --git a/SoftLayer/CLI/block/order.py b/SoftLayer/CLI/block/order.py
@@ -22,11 +22,11 @@
 @click.option('--iops',
               type=int,
               help='Performance Storage IOPs,'
-              + ' between 100 and 6000 in multiples of 100'
-              + '  [required for storage-type performance]')
+              ' between 100 and 6000 in multiples of 100'
+              '  [required for storage-type performance]')
 @click.option('--tier',
               help='Endurance Storage Tier (IOP per GB)'
-              + '  [required for storage-type endurance]',
+              '  [required for storage-type endurance]',
               type=click.Choice(['0.25', '2', '4']))
 @click.option('--os-type',
               help='Operating System',
@@ -62,26 +62,32 @@ def cli(env, storage_type, size, iops, tier, os_type, location):
                 'Option --iops must be a multiple of 100'
             )
 
-        order = block_manager.order_block_volume(
-            storage_type='performance_storage_iscsi',
-            location=location,
-            size=size,
-            iops=iops,
-            os_type=os_type
-        )
+        try:
+            order = block_manager.order_block_volume(
+                storage_type='performance_storage_iscsi',
+                location=location,
+                size=size,
+                iops=iops,
+                os_type=os_type
+            )
+        except ValueError as ex:
+            raise exceptions.ArgumentError(str(ex))
 
     if storage_type == 'endurance':
         if tier is None:
             raise exceptions.CLIAbort(
                 'Option --tier required with Endurance in IOPS/GB [0.25,2,4]')
 
-        order = block_manager.order_block_volume(
-            storage_type='storage_service_enterprise',
-            location=location,
-            size=size,
-            tier_level=tier,
-            os_type=os_type
-        )
+        try:
+            order = block_manager.order_block_volume(
+                storage_type='storage_service_enterprise',
+                location=location,
+                size=size,
+                tier_level=tier,
+                os_type=os_type
+            )
+        except ValueError as ex:
+            raise exceptions.ArgumentError(str(ex))
 
     if 'placedOrder' in order.keys():
         click.echo("Order #{0} placed successfully!".format(
diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py
@@ -9,6 +9,13 @@
 from SoftLayer import utils
 
 
+ENDURANCE_TIERS = {
+    '0.25': '100',
+    '2': '200',
+    '4': '300'
+}
+
+
 class BlockStorageManager(utils.IdentifierMixin, object):
     """Manages Block Storage volumes."""
 
@@ -149,16 +156,27 @@ def order_block_volume(self, storage_type, location, size, os_type,
         try:
             location_id = self._get_location_id(location)
         except ValueError:
-            raise exceptions.SoftLayerError("Invalid datacenter name specified. Please provide the lower case short name (e.g.: dal09)")
+            raise exceptions.SoftLayerError(
+                "Invalid datacenter name specified. "
+                "Please provide the lower case short name (e.g.: dal09)")
 
         base_type_name = 'SoftLayer_Container_Product_Order_Network_'
         package = self._get_package(storage_type)
         if package['name'] == 'Performance':
             complex_type = base_type_name + 'PerformanceStorage_Iscsi'
-            prices = self._get_item_prices_performance(package, size, iops)
+            prices = [
+                _find_performance_block_price(package),
+                _find_performance_space_price(package, iops),
+                _find_performance_iops_price(package, size, iops),
+            ]
         elif package['name'] == 'Endurance':
             complex_type = base_type_name + 'Storage_Enterprise'
-            prices = self._get_item_prices_endurance(package, size, tier_level)
+            prices = [
+                _find_endurance_block_price(package),
+                _find_endurance_price(package),
+                _find_endurance_space_price(package, size, tier_level),
+                _find_endurance_tier_price(package, tier_level),
+            ]
         else:
             raise exceptions.SoftLayerError(
                 "storage_type must be either Performance or Endurance")
@@ -199,108 +217,6 @@ def _get_package(self, category_code, **kwargs):
         func = getattr(self.product_package, 'getAllObjects')
         return func(**kwargs).pop()
 
-    @staticmethod
-    def _get_item_prices_performance(package, size, iops):
-        """Returns a collection of prices for performance storage.
-
-        :param package: Package object
-        :param size: Size of volume in GB
-        :param iops: IOPs of volume
-        :param kwargs:
-        :return: Returns a collection of prices for performance storage.
-        """
-        parent_category_code = 'performance_storage_iscsi'
-        space_category_code = 'performance_storage_space'
-        iops_category_code = 'performance_storage_iops'
-
-        found_space_price = False
-        found_iops_price = False
-
-        prices = []
-        for item in package['items']:
-            for price in item['prices']:
-                for category in price['categories']:
-                    if price['locationGroupId'] == '':
-                        # Find the parent-level price object.
-                        if category['categoryCode'] == parent_category_code:
-                            prices.append(price)
-                        # Find the valid space price object.
-                        elif (category['categoryCode'] == space_category_code
-                              and item['capacity'] == size):
-                            prices.append(price)
-                            found_space_price = True
-                        # Find the valid iops price object.
-                        elif (category['categoryCode'] == iops_category_code
-                              and item['capacity'] == iops
-                              and (price['capacityRestrictionMinimum'] <= size
-                                   <= price['capacityRestrictionMaximum'])):
-                            prices.append(price)
-                            found_iops_price = True
-
-        if found_space_price is False or found_iops_price is False:
-            raise ValueError(
-                "No prices found for the requested size and iops.")
-
-        return prices
-
-    @staticmethod
-    def _get_item_prices_endurance(package, size, tier_level):
-        """Returns a collection of prices for a endurance storage order.
-
-        :param package: Package object
-        :param size: Size of volume in GB
-        :param iops: IOPs of volume
-        :param kwargs:
-        :return: Returns a collection of prices for a endurance storage order.
-        """
-
-        tiers = {
-            '0.25': '100',
-            '2': '200',
-            '4': '300'
-        }
-
-        endurance_parent_category_codes = [
-            'storage_service_enterprise',
-            'storage_block',
-        ]
-
-        space_category_code = 'performance_storage_space'
-        tier_category_code = 'storage_tier_level'
-
-        found_space_price = False
-        found_iops_price = False
-
-        prices = []
-        for item in package['items']:
-            for price in item['prices']:
-                for category in price['categories']:
-                    # Only collect prices from valid location groups.
-                    if price['locationGroupId'] == '':
-                        # Find the parent-level price object.
-                        if any(category['categoryCode'] in s
-                               for s in endurance_parent_category_codes):
-                            prices.append(price)
-                        # Find the valid space price object.
-                        elif (category['categoryCode'] == space_category_code
-                              and price['capacityRestrictionMinimum'] ==
-                              tiers.get(tier_level)
-                              and item['capacity'] == size):
-                            prices.append(price)
-                            found_space_price = True
-                        # Find the valid tier price object.
-                        elif (category['categoryCode'] == tier_category_code
-                              and item['attributes'][0]['value'] ==
-                              tiers.get(tier_level)):
-                            prices.append(price)
-                            found_iops_price = True
-
-        if found_space_price is False or found_iops_price is False:
-            raise ValueError(
-                "No prices found for the requested size and tier.")
-
-        return prices
-
     def _get_location_id(self, location):
         """Returns location id
 
@@ -335,3 +251,145 @@ def cancel_block_volume(self, volume_id,
             True,
             reason,
             id=billing_item_id)
+
+
+def _find_endurance_block_price(package):
+    for item in package['items']:
+        for price in item['prices']:
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'], 'storage_block'):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for block storage")
+
+
+def _find_endurance_price(package):
+    for item in package['items']:
+        for price in item['prices']:
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'],
+                                 'storage_service_enterprise'):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for endurance block storage")
+
+
+def _find_endurance_space_price(package, size, tier_level):
+    for item in package['items']:
+        if int(item['capacity']) != size:
+            continue
+
+        for price in item['prices']:
+            # Only collect prices from valid location groups.
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'],
+                                 'performance_storage_space'):
+                continue
+
+            level = ENDURANCE_TIERS.get(tier_level)
+            if price['capacityRestrictionMinimum'] != level:
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for disk space")
+
+
+def _find_endurance_tier_price(package, tier_level):
+    for item in package['items']:
+        for attribute in item['attributes']:
+            if attribute['value'] != ENDURANCE_TIERS.get(tier_level):
+                break
+        else:
+            continue
+
+        for price in item['prices']:
+            # Only collect prices from valid location groups.
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'], 'storage_tier_level'):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for tier")
+
+
+def _find_performance_block_price(package):
+    for item in package['items']:
+        for price in item['prices']:
+            # Only collect prices from valid location groups.
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'],
+                                 'performance_storage_iscsi'):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for performance storage")
+
+
+def _find_performance_space_price(package, size):
+    for item in package['items']:
+        if int(item['capacity']) != size:
+            continue
+
+        for price in item['prices']:
+            # Only collect prices from valid location groups.
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'],
+                                 'performance_storage_space'):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for disk space")
+
+
+def _find_performance_iops_price(package, size, iops):
+    for item in package['items']:
+        if int(item['capacity']) != int(iops):
+            continue
+
+        for price in item['prices']:
+            # Only collect prices from valid location groups.
+            if price['locationGroupId'] != '':
+                continue
+
+            if not _has_category(price['categories'],
+                                 'performance_storage_iops'):
+                continue
+
+            if size < int(price['capacityRestrictionMinimum']):
+                continue
+
+            if size > int(price['capacityRestrictionMaximum']):
+                continue
+
+            return price
+
+    raise ValueError("Could not find price for iops")
+
+
+def _has_category(categories, category_code):
+    return any(
+        True
+        for category
+        in categories
+        if category['categoryCode'] == category_code
+    )
