Add "name" filter in "list" call when retrieving a single register

Resource.find() function accepts both the ID or the name of a register.
First tries to retrieve the register by calling the server API using
the identifier given. By default, all OpenStack API will accept the ID
of the register (which is unique). If this call fails, the function
retrieves the list of all registers in the table.

This last call is very inefficient if the number of elements is high.
The bug description documents the time needed to retrieve a list of
1000 ports.

Instead of this, this patch uses the filtering options available in the
servers APIs. This filter will be applied in the database call and the
number of registers returned will be limited to only those ones
matching the "name" field.

In case the API doesn't support this filtering parameter, a
"InvalidResourceQuery" will be risen and the function will retrieve
again the list of registers in the database without filtering by "name".

Change-Id: I8bb6f46f66249aeae0649c8f6596f9269e3518a5
Related-Bug: #1779882
This commit is contained in:
Rodolfo Alonso Hernandez 2019-04-12 15:24:22 +00:00
parent f198347552
commit 790fefbccd
5 changed files with 127 additions and 68 deletions

View File

@ -1587,6 +1587,10 @@ class Resource(dict):
except exceptions.NotFoundException:
pass
if ('name' in cls._query_mapping._mapping.keys()
and 'name' not in params):
params['name'] = name_or_id
data = cls.list(session, **params)
result = cls._get_one_match(name_or_id, data)

View File

@ -90,7 +90,8 @@ class TestFirewallRule(FirewallTestCase):
self.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules',
name=self.firewall_rule_name),
json={'firewall_rules': [self.mock_firewall_rule]}),
dict(method='DELETE',
uri=self._make_mock_url('firewall_rules',
@ -109,7 +110,9 @@ class TestFirewallRule(FirewallTestCase):
self.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules', **filters),
uri=self._make_mock_url(
'firewall_rules',
name=self.firewall_rule_name, **filters),
json={'firewall_rules': [self.mock_firewall_rule]}, ),
dict(method='DELETE',
uri=self._make_mock_url('firewall_rules',
@ -146,7 +149,8 @@ class TestFirewallRule(FirewallTestCase):
self.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules',
name=self.firewall_rule_name),
json={'firewall_rules': [self.mock_firewall_rule,
self.mock_firewall_rule]})
])
@ -162,7 +166,8 @@ class TestFirewallRule(FirewallTestCase):
self.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules',
name=self.firewall_rule_name),
json={'firewall_rules': [self.mock_firewall_rule]})
])
r = self.cloud.get_firewall_rule(self.firewall_rule_name)
@ -176,7 +181,7 @@ class TestFirewallRule(FirewallTestCase):
uri=self._make_mock_url('firewall_rules', name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=name),
json={'firewall_rules': []})
])
self.assertIsNone(self.cloud.get_firewall_rule(name))
@ -202,7 +207,8 @@ class TestFirewallRule(FirewallTestCase):
self.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules',
name=self.firewall_rule_name),
json={'firewall_rules': [self.mock_firewall_rule]}),
dict(method='PUT',
uri=self._make_mock_url('firewall_rules',
@ -283,7 +289,9 @@ class TestFirewallPolicy(FirewallTestCase):
TestFirewallRule.firewall_rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url(
'firewall_rules',
name=TestFirewallRule.firewall_rule_name),
json={'firewall_rules': [
TestFirewallRule._mock_firewall_rule_attrs]}),
dict(method='POST',
@ -305,7 +313,9 @@ class TestFirewallPolicy(FirewallTestCase):
posted_policy['firewall_rules'][0]),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url(
'firewall_rules',
name=posted_policy['firewall_rules'][0]),
json={'firewall_rules': []})
])
@ -323,7 +333,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [self.mock_firewall_policy]}),
dict(method='DELETE',
uri=self._make_mock_url('firewall_policies',
@ -364,7 +375,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': []})
])
@ -381,7 +393,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [self.mock_firewall_policy]})
])
self.assertDictEqual(self.mock_firewall_policy,
@ -396,7 +409,7 @@ class TestFirewallPolicy(FirewallTestCase):
uri=self._make_mock_url('firewall_policies', name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies', name=name),
json={'firewall_policies': []})
])
self.assertIsNone(self.cloud.get_firewall_policy(name))
@ -448,7 +461,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [retrieved_policy]}),
dict(method='GET',
uri=self._make_mock_url('firewall_rules', lookup_rule['id']),
@ -474,7 +488,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [
deepcopy(self.mock_firewall_policy)]}),
dict(method='PUT',
@ -539,28 +554,29 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET', # get policy
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [retrieved_policy]}),
dict(method='GET', # short-circuit
uri=self._make_mock_url('firewall_rules', rule0['name']),
status_code=404),
dict(method='GET', # get rule to add
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule0['name']),
json={'firewall_rules': [rule0]}),
dict(method='GET', # short-circuit
uri=self._make_mock_url('firewall_rules', rule1['name']),
status_code=404),
dict(method='GET', # get after rule
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule1['name']),
json={'firewall_rules': [rule1]}),
dict(method='GET', # short-circuit
uri=self._make_mock_url('firewall_rules', rule2['name']),
status_code=404),
dict(method='GET', # get before rule
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule2['name']),
json={'firewall_rules': [rule2]}),
dict(method='PUT', # add rule
@ -595,14 +611,15 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': [retrieved_policy]}),
dict(method='GET', # short-circuit
uri=self._make_mock_url('firewall_rules', rule['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule['name']),
json={'firewall_rules': [rule]}),
dict(method='PUT',
@ -626,7 +643,8 @@ class TestFirewallPolicy(FirewallTestCase):
uri=self._make_mock_url('firewall_policies', policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=policy_name),
json={'firewall_policies': []})
])
@ -648,7 +666,7 @@ class TestFirewallPolicy(FirewallTestCase):
uri=self._make_mock_url('firewall_rules', rule_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule_name),
json={'firewall_rules': []})
])
self.assertRaises(exceptions.ResourceNotFound,
@ -691,14 +709,15 @@ class TestFirewallPolicy(FirewallTestCase):
uri=self._make_mock_url('firewall_policies', policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=policy_name),
json={'firewall_policies': [retrieved_policy]}),
dict(method='GET', # short-circuit
uri=self._make_mock_url('firewall_rules', rule['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule['name']),
json={'firewall_rules': [rule]}),
dict(method='PUT',
@ -719,7 +738,8 @@ class TestFirewallPolicy(FirewallTestCase):
self.firewall_policy_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.firewall_policy_name),
json={'firewall_policies': []})
])
@ -745,7 +765,7 @@ class TestFirewallPolicy(FirewallTestCase):
rule['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_rules'),
uri=self._make_mock_url('firewall_rules', name=rule['name']),
json={'firewall_rules': []})
])
r = self.cloud.remove_rule_from_policy(self.firewall_policy_id,
@ -845,7 +865,8 @@ class TestFirewallGroup(FirewallTestCase):
self.mock_egress_policy['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.mock_egress_policy['name']),
json={'firewall_policies': [self.mock_egress_policy]}),
dict(method='GET', # short-circuit
@ -853,7 +874,9 @@ class TestFirewallGroup(FirewallTestCase):
self.mock_ingress_policy['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url(
'firewall_policies',
name=self.mock_ingress_policy['name']),
json={'firewall_policies': [self.mock_ingress_policy]}),
dict(method='GET',
@ -904,7 +927,8 @@ class TestFirewallGroup(FirewallTestCase):
self.firewall_group_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_groups'),
uri=self._make_mock_url('firewall_groups',
name=self.firewall_group_name),
json={'firewall_groups': [
deepcopy(self.mock_returned_firewall_group)]}),
dict(method='DELETE',
@ -942,7 +966,8 @@ class TestFirewallGroup(FirewallTestCase):
self.firewall_group_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_groups'),
uri=self._make_mock_url('firewall_groups',
name=self.firewall_group_name),
json={'firewall_groups': []})
])
@ -960,7 +985,8 @@ class TestFirewallGroup(FirewallTestCase):
self.firewall_group_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_groups'),
uri=self._make_mock_url('firewall_groups',
name=self.firewall_group_name),
json={'firewall_groups': [returned_group]})
])
self.assertDictEqual(
@ -975,7 +1001,7 @@ class TestFirewallGroup(FirewallTestCase):
uri=self._make_mock_url('firewall_groups', name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_groups'),
uri=self._make_mock_url('firewall_groups', name=name),
json={'firewall_groups': []})
])
self.assertIsNone(self.cloud.get_firewall_group(name))
@ -1025,7 +1051,8 @@ class TestFirewallGroup(FirewallTestCase):
self.firewall_group_name),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_groups'),
uri=self._make_mock_url('firewall_groups',
name=self.firewall_group_name),
json={'firewall_groups': [returned_group]}),
dict(method='GET', # short-circuit
@ -1033,7 +1060,8 @@ class TestFirewallGroup(FirewallTestCase):
self.mock_egress_policy['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url('firewall_policies',
name=self.mock_egress_policy['name']),
json={'firewall_policies': [
deepcopy(self.mock_egress_policy)]}),
@ -1042,7 +1070,9 @@ class TestFirewallGroup(FirewallTestCase):
self.mock_ingress_policy['name']),
status_code=404),
dict(method='GET',
uri=self._make_mock_url('firewall_policies'),
uri=self._make_mock_url(
'firewall_policies',
name=self.mock_ingress_policy['name']),
json={'firewall_policies': [
deepcopy(self.mock_ingress_policy)]}),

View File

@ -408,7 +408,7 @@ class TestImage(base.TestCase):
self.sess.get.assert_has_calls([
mock.call('images/' + EXAMPLE['name'], microversion=None),
mock.call('/images', headers={'Accept': 'application/json'},
microversion=None, params={}),
microversion=None, params={'name': EXAMPLE['name']}),
mock.call('/images', headers={'Accept': 'application/json'},
microversion=None, params={'os_hidden': True})
])

View File

@ -2082,38 +2082,42 @@ class TestResourceActions(base.TestCase):
class TestResourceFind(base.TestCase):
result = 1
class Base(resource.Resource):
@classmethod
def existing(cls, **kwargs):
response = mock.Mock()
response.status_code = 404
raise exceptions.ResourceNotFound(
'Not Found', response=response)
@classmethod
def list(cls, session, **params):
return None
class OneResult(Base):
@classmethod
def _get_one_match(cls, *args):
return TestResourceFind.result
class NoResults(Base):
@classmethod
def _get_one_match(cls, *args):
return None
class OneResultWithQueryParams(OneResult):
_query_mapping = resource.QueryParameters('name')
def setUp(self):
super(TestResourceFind, self).setUp()
self.result = 1
class Base(resource.Resource):
@classmethod
def existing(cls, **kwargs):
response = mock.Mock()
response.status_code = 404
raise exceptions.ResourceNotFound(
'Not Found', response=response)
@classmethod
def list(cls, session):
return None
class OneResult(Base):
@classmethod
def _get_one_match(cls, *args):
return self.result
class NoResults(Base):
@classmethod
def _get_one_match(cls, *args):
return None
self.no_results = NoResults
self.one_result = OneResult
self.no_results = self.NoResults
self.one_result = self.OneResult
self.one_result_with_qparams = self.OneResultWithQueryParams
def test_find_short_circuit(self):
value = 1
@ -2139,10 +2143,24 @@ class TestResourceFind(base.TestCase):
self.no_results.find(
self.cloud.compute, "name", ignore_missing=True))
def test_find_result(self):
def test_find_result_name_not_in_query_parameters(self):
with mock.patch.object(self.one_result, 'existing',
side_effect=self.OneResult.existing) \
as mock_existing, \
mock.patch.object(self.one_result, 'list',
side_effect=self.OneResult.list) \
as mock_list:
self.assertEqual(
self.result,
self.one_result.find(self.cloud.compute, "name"))
mock_existing.assert_called_once_with(id='name',
connection=mock.ANY)
mock_list.assert_called_once_with(mock.ANY)
def test_find_result_name_in_query_parameters(self):
self.assertEqual(
self.result,
self.one_result.find(self.cloud.compute, "name"))
self.one_result_with_qparams.find(self.cloud.compute, "name"))
def test_match_empty_results(self):
self.assertIsNone(resource.Resource._get_one_match("name", []))

View File

@ -0,0 +1,7 @@
---
other:
- |
``openstack.resource.Resource.find`` now can use the database back-end to
filter by name. If the resource class has "name" in the query parameters,
this function will add this filter parameter in the "list" command, instead
of retrieving the whole list and then manually filtering.