Merge pull request #2202 from softlayer/issues1949

allmightyspiff · web-flow · commit deaa4eb1552a · 2024-12-16T13:33:48.000-06:00
Added feature to iter_call to force a orderBy filter
diff --git a/SoftLayer/API.py b/SoftLayer/API.py
@@ -19,6 +19,7 @@
 from SoftLayer import consts
 from SoftLayer import exceptions
 from SoftLayer import transports
+from SoftLayer import utils
 
 LOGGER = logging.getLogger(__name__)
 API_PUBLIC_ENDPOINT = consts.API_PUBLIC_ENDPOINT
@@ -403,6 +404,7 @@ def iter_call(self, service, method, *args, **kwargs):
         kwargs['iter'] = False
         result_count = 0
         keep_looping = True
+        kwargs['filter'] = utils.fix_filter(kwargs.get('filter'))
 
         while keep_looping:
             # Get the next results
diff --git a/SoftLayer/CLI/environment.py b/SoftLayer/CLI/environment.py
@@ -111,6 +111,7 @@ def getpass(self, prompt, default=None):
         # In windows, shift+insert actually inputs the below 2 characters
         # If we detect those 2 characters, need to manually read from the clipbaord instead
         # https://stackoverflow.com/questions/101128/how-do-i-read-text-from-the-clipboard
+        # LINUX NOTICE: `apt-get install python3-tk` required to install tk
         if password == 'àR':
             # tkinter is a built in python gui, but it has clipboard reading functions.
             # pylint: disable=import-outside-toplevel
diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py
@@ -53,23 +53,21 @@ def list_block_volumes(self, datacenter=None, username=None, storage_type=None,
         _filter = utils.NestedDict(kwargs.get('filter') or {})
 
         _filter['iscsiNetworkStorage']['serviceResource']['type']['type'] = utils.query_filter('!~ ISCSI')
+        _filter['iscsiNetworkStorage']['id'] = utils.query_filter_orderby()
 
-        _filter['iscsiNetworkStorage']['storageType']['keyName'] = (
-            utils.query_filter('*BLOCK_STORAGE*'))
+        _filter['iscsiNetworkStorage']['storageType']['keyName'] = utils.query_filter('*BLOCK_STORAGE*')
         if storage_type:
             _filter['iscsiNetworkStorage']['storageType']['keyName'] = (
                 utils.query_filter('%s_BLOCK_STORAGE*' % storage_type.upper()))
 
         if datacenter:
-            _filter['iscsiNetworkStorage']['serviceResource']['datacenter'][
-                'name'] = utils.query_filter(datacenter)
+            _filter['iscsiNetworkStorage']['serviceResource']['datacenter']['name'] = utils.query_filter(datacenter)
 
         if username:
             _filter['iscsiNetworkStorage']['username'] = utils.query_filter(username)
 
         if order:
-            _filter['iscsiNetworkStorage']['billingItem']['orderItem'][
-                'order']['id'] = utils.query_filter(order)
+            _filter['iscsiNetworkStorage']['billingItem']['orderItem']['order']['id'] = utils.query_filter(order)
 
         kwargs['filter'] = _filter.to_dict()
         return self.client.call('Account', 'getIscsiNetworkStorage', iter=True, **kwargs)
diff --git a/SoftLayer/managers/dns.py b/SoftLayer/managers/dns.py
@@ -196,6 +196,7 @@ def get_records(self, zone_id, ttl=None, data=None, host=None, record_type=None)
         :returns: A list of dictionaries representing the matching records within the specified zone.
         """
         _filter = utils.NestedDict()
+        _filter['resourceRecords']['id'] = utils.query_filter_orderby()
 
         if ttl:
             _filter['resourceRecords']['ttl'] = utils.query_filter(ttl)
diff --git a/SoftLayer/managers/event_log.py b/SoftLayer/managers/event_log.py
@@ -65,11 +65,8 @@ def build_filter(date_min=None, date_max=None, obj_event=None, obj_id=None, obj_
 
         :returns: dict: The generated query filter
         """
-
-        if not any([date_min, date_max, obj_event, obj_id, obj_type]):
-            return {}
-
         request_filter = {}
+        request_filter['traceId'] = utils.query_filter_orderby()
 
         if date_min and date_max:
             request_filter['eventCreateDate'] = utils.event_log_filter_between_date(date_min, date_max, utc_offset)
diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py
@@ -47,7 +47,7 @@ def list_file_volumes(self, datacenter=None, username=None, storage_type=None, o
             kwargs['mask'] = ','.join(items)
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
-
+        _filter['nasNetworkStorage']['id'] = utils.query_filter_orderby()
         _filter['nasNetworkStorage']['serviceResource']['type']['type'] = utils.query_filter('!~ NAS')
 
         _filter['nasNetworkStorage']['storageType']['keyName'] = (
diff --git a/SoftLayer/managers/image.py b/SoftLayer/managers/image.py
@@ -57,13 +57,12 @@ def list_private_images(self, guid=None, name=None, limit=100, **kwargs):
             kwargs['mask'] = IMAGE_MASK
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
+        _filter['privateBlockDeviceTemplateGroups']['id'] = utils.query_filter_orderby()
         if name:
-            _filter['privateBlockDeviceTemplateGroups']['name'] = (
-                utils.query_filter(name))
+            _filter['privateBlockDeviceTemplateGroups']['name'] = utils.query_filter(name)
 
         if guid:
-            _filter['privateBlockDeviceTemplateGroups']['globalIdentifier'] = (
-                utils.query_filter(guid))
+            _filter['privateBlockDeviceTemplateGroups']['globalIdentifier'] = utils.query_filter(guid)
 
         kwargs['filter'] = _filter.to_dict()
 
@@ -81,6 +80,7 @@ def list_public_images(self, guid=None, name=None, limit=100, **kwargs):
             kwargs['mask'] = IMAGE_MASK
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
+        _filter['id'] = utils.query_filter_orderby()
         if name:
             _filter['name'] = utils.query_filter(name)
 
diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py
@@ -496,6 +496,7 @@ def list_subnets(self, identifier=None, datacenter=None, version=0,
             kwargs['mask'] = DEFAULT_SUBNET_MASK
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
+        _filter['subnets']['id'] = utils.query_filter_orderby()
 
         if identifier:
             _filter['subnets']['networkIdentifier'] = (
diff --git a/SoftLayer/utils.py b/SoftLayer/utils.py
@@ -6,6 +6,7 @@
 """
 
 import collections
+import copy
 import datetime
 from json import JSONDecoder
 import re
@@ -41,6 +42,32 @@ def lookup(dic, key, *keys):
     return dic.get(key)
 
 
+def has_key_value(d: dict, key: str = "operation", value: str = "orderBy") -> bool:
+    """Scan through a dictionary looking for an orderBy clause, but can be used for any key/value combo"""
+    if d.get(key) and d.get(key) == value:
+        return True
+    for x in d.values():
+        if isinstance(x, dict):
+            if has_key_value(x, key, value):
+                return True
+    return False
+
+
+def fix_filter(sl_filter: dict = None) -> dict:
+    """Forces an object filter to have an orderBy  clause if it doesn't have one already"""
+
+    if sl_filter is None:
+        sl_filter = {}
+
+    # Make a copy to prevent sl_filter from being modified by this function
+    this_filter = copy.copy(sl_filter)
+    if not has_key_value(this_filter, "operation", "orderBy"):
+        # Check to see if 'id' is already a filter, if so just skip
+        if not this_filter.get('id', False):
+            this_filter['id'] = query_filter_orderby()
+    return this_filter
+
+
 class NestedDict(dict):
     """This helps with accessing a heavily nested dictionary.
 
diff --git a/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py
@@ -125,7 +125,8 @@ def test_volume_detail_name_identifier(self):
                 'storageType': {
                     'keyName': {'operation': '*= BLOCK_STORAGE'}
                 },
-                'username': {'operation': '_= SL-12345'}
+                'username': {'operation': '_= SL-12345'},
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
             }
         }
 
diff --git a/tests/CLI/modules/event_log_tests.py b/tests/CLI/modules/event_log_tests.py
@@ -31,10 +31,9 @@ def test_get_event_log_empty(self):
         mock.return_value = None
 
         result = self.run_command(['event-log', 'get'])
-        expected = 'Event, Object, Type, Date, Username\n' \
-                   'No logs available for filter {}.\n'
+
         self.assert_no_fail(result)
-        self.assertEqual(expected, result.output)
+        self.assertIn("No logs available for filter ", result.output)
 
     def test_get_event_log_over_limit(self):
         result = self.run_command(['event-log', 'get', '-l 1'])
diff --git a/tests/CLI/modules/file_tests.py b/tests/CLI/modules/file_tests.py
@@ -210,14 +210,13 @@ def test_volume_detail_name_identifier(self):
         expected_filter = {
             'nasNetworkStorage': {
                 'serviceResource': {
-                    'type': {
-                        'type': {'operation': '!~ NAS'}
-                    }
+                    'type': {'type': {'operation': '!~ NAS'}}
                 },
-                'storageType': {
-                    'keyName': {'operation': '*= FILE_STORAGE'}
-                },
-                'username': {'operation': '_= SL-12345'}}}
+                'storageType': {'keyName': {'operation': '*= FILE_STORAGE'}},
+                'username': {'operation': '_= SL-12345'},
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
+            }
+        }
 
         self.assert_called_with('SoftLayer_Account', 'getNasNetworkStorage', filter=expected_filter)
         self.assert_called_with('SoftLayer_Network_Storage', 'getObject', identifier=1)
diff --git a/tests/api_tests.py b/tests/api_tests.py
@@ -169,8 +169,8 @@ def test_iter_call(self, _call):
 
         self.assertEqual(list(range(125)), result)
         _call.assert_has_calls([
-            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=0),
-            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=100),
+            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=0, filter=mock.ANY),
+            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=100, filter=mock.ANY),
         ])
         _call.reset_mock()
 
@@ -183,9 +183,9 @@ def test_iter_call(self, _call):
         result = list(self.client.iter_call('SERVICE', 'METHOD', iter=True))
         self.assertEqual(list(range(200)), result)
         _call.assert_has_calls([
-            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=0),
-            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=100),
-            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=200),
+            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=0, filter=mock.ANY),
+            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=100, filter=mock.ANY),
+            mock.call('SERVICE', 'METHOD', limit=100, iter=False, offset=200, filter=mock.ANY),
         ])
         _call.reset_mock()
 
@@ -194,12 +194,11 @@ def test_iter_call(self, _call):
             transports.SoftLayerListResult(range(0, 25), 30),
             transports.SoftLayerListResult(range(25, 30), 30)
         ]
-        result = list(self.client.iter_call(
-            'SERVICE', 'METHOD', iter=True, limit=25))
+        result = list(self.client.iter_call('SERVICE', 'METHOD', iter=True, limit=25))
         self.assertEqual(list(range(30)), result)
         _call.assert_has_calls([
-            mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=0),
-            mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=25),
+            mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=0, filter=mock.ANY),
+            mock.call('SERVICE', 'METHOD', iter=False, limit=25, offset=25, filter=mock.ANY),
         ])
         _call.reset_mock()
 
@@ -208,31 +207,27 @@ def test_iter_call(self, _call):
         result = list(self.client.iter_call('SERVICE', 'METHOD', iter=True))
         self.assertEqual(["test"], result)
         _call.assert_has_calls([
-            mock.call('SERVICE', 'METHOD', iter=False, limit=100, offset=0),
+            mock.call('SERVICE', 'METHOD', iter=False, limit=100, offset=0, filter=mock.ANY),
         ])
         _call.reset_mock()
 
         _call.side_effect = [
             transports.SoftLayerListResult(range(0, 25), 30),
             transports.SoftLayerListResult(range(25, 30), 30)
         ]
-        result = list(self.client.iter_call('SERVICE', 'METHOD', 'ARG',
-                                            iter=True,
-                                            limit=25,
-                                            offset=12))
+        result = list(
+            self.client.iter_call('SERVICE', 'METHOD', 'ARG', iter=True, limit=25, offset=12)
+        )
         self.assertEqual(list(range(30)), result)
         _call.assert_has_calls([
-            mock.call('SERVICE', 'METHOD', 'ARG',
-                      iter=False, limit=25, offset=12),
-            mock.call('SERVICE', 'METHOD', 'ARG',
-                      iter=False, limit=25, offset=37),
+            mock.call('SERVICE', 'METHOD', 'ARG', iter=False, limit=25, offset=12, filter=mock.ANY),
+            mock.call('SERVICE', 'METHOD', 'ARG', iter=False, limit=25, offset=37, filter=mock.ANY),
         ])
 
         # Chunk size of 0 is invalid
         self.assertRaises(
             AttributeError,
-            lambda: list(self.client.iter_call('SERVICE', 'METHOD',
-                                               iter=True, limit=0)))
+            lambda: list(self.client.iter_call('SERVICE', 'METHOD', iter=True, limit=0, filter=mock.ANY)))
 
     def test_call_invalid_arguments(self):
         self.assertRaises(
diff --git a/tests/managers/block_tests.py b/tests/managers/block_tests.py
@@ -125,19 +125,17 @@ def test_get_block_volume_details(self):
     def test_list_block_volumes(self):
         result = self.block.list_block_volumes()
 
-        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage,
-                         result)
+        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage, result)
 
         expected_filter = {
             'iscsiNetworkStorage': {
                 'storageType': {
                     'keyName': {'operation': '*= BLOCK_STORAGE'}
                 },
                 'serviceResource': {
-                    'type': {
-                        'type': {'operation': '!~ ISCSI'}
-                    }
-                }
+                    'type': {'type': {'operation': '!~ ISCSI'}}
+                },
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
             }
         }
 
@@ -161,23 +159,20 @@ def test_list_block_volumes(self):
     def test_list_block_volumes_additional_filter_order(self):
         result = self.block.list_block_volumes(order=1234567)
 
-        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage,
-                         result)
+        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage, result)
 
         expected_filter = {
             'iscsiNetworkStorage': {
                 'storageType': {
                     'keyName': {'operation': '*= BLOCK_STORAGE'}
                 },
                 'serviceResource': {
-                    'type': {
-                        'type': {'operation': '!~ ISCSI'}
-                    }
+                    'type': {'type': {'operation': '!~ ISCSI'}}
                 },
                 'billingItem': {
-                    'orderItem': {
-                        'order': {
-                            'id': {'operation': 1234567}}}}
+                    'orderItem': {'order': {'id': {'operation': 1234567}}}
+                },
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
             }
         }
 
@@ -199,27 +194,21 @@ def test_list_block_volumes_additional_filter_order(self):
         )
 
     def test_list_block_volumes_with_additional_filters(self):
-        result = self.block.list_block_volumes(datacenter="dal09",
-                                               storage_type="Endurance",
-                                               username="username")
+        result = self.block.list_block_volumes(datacenter="dal09", storage_type="Endurance", username="username")
 
-        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage,
-                         result)
+        self.assertEqual(SoftLayer_Account.getIscsiNetworkStorage, result)
 
         expected_filter = {
             'iscsiNetworkStorage': {
                 'storageType': {
                     'keyName': {'operation': '^= ENDURANCE_BLOCK_STORAGE'}
                 },
-                'username': {'operation': u'_= username'},
+                'username': {'operation': '_= username'},
                 'serviceResource': {
-                    'datacenter': {
-                        'name': {'operation': u'_= dal09'}
-                    },
-                    'type': {
-                        'type': {'operation': '!~ ISCSI'}
-                    }
-                }
+                    'datacenter': {'name': {'operation': u'_= dal09'}},
+                    'type': {'type': {'operation': '!~ ISCSI'}}
+                },
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
             }
         }
 
diff --git a/tests/managers/dedicated_host_tests.py b/tests/managers/dedicated_host_tests.py
@@ -714,7 +714,8 @@ def test_list_guests_with_filters(self):
                 'networkComponents': {'maxSpeed': {'operation': 100}},
                 'primaryIpAddress': {'operation': '_= 1.2.3.4'},
                 'primaryBackendIpAddress': {'operation': '_= 4.3.2.1'}
-            }
+            },
+            'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}
         }
         self.assert_called_with('SoftLayer_Virtual_DedicatedHost', 'getGuests',
                                 identifier=12345, filter=_filter)
diff --git a/tests/managers/dns_tests.py b/tests/managers/dns_tests.py
diff --git a/tests/managers/event_log_tests.py b/tests/managers/event_log_tests.py
diff --git a/tests/managers/file_tests.py b/tests/managers/file_tests.py
diff --git a/tests/managers/image_tests.py b/tests/managers/image_tests.py
diff --git a/tests/managers/network_tests.py b/tests/managers/network_tests.py
diff --git a/tests/managers/vs/vs_capacity_tests.py b/tests/managers/vs/vs_capacity_tests.py
diff --git a/tests/utils_tests.py b/tests/utils_tests.py

Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,8 @@ def test_volume_detail_name_identifier(self):`
`125`	`125`	`'storageType': {`
`126`	`126`	`'keyName': {'operation': '*= BLOCK_STORAGE'}`
`127`	`127`	`},`
`128`		`- 'username': {'operation': '_= SL-12345'}`
	`128`	`+ 'username': {'operation': '_= SL-12345'},`
	`129`	`+ 'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]}`
`129`	`130`	`}`
`130`	`131`	`}`
`131`	`132`