Refactor datasource.py
Move 'get_resource_id_from_name()' from datasoure to utils. Fixed policy.py and corresponding unit tests Change-Id: Ie4c6c64cc04fbf07f17831d9f7ab73389bee5ea9
This commit is contained in:
parent
c7b0d22ef2
commit
594f1b22c8
@ -102,3 +102,28 @@ def get_dict_properties(item, fields, mixed_case_fields=[], formatters={}):
|
||||
else:
|
||||
row.append(data)
|
||||
return tuple(row)
|
||||
|
||||
|
||||
def get_resource_id_from_name(name, results):
|
||||
# FIXME(arosen): move to common lib and add tests...
|
||||
name_match = None
|
||||
id_match = None
|
||||
double_name_match = False
|
||||
for result in results['results']:
|
||||
if result['id'] == name:
|
||||
id_match = result['id']
|
||||
if result['name'] == name:
|
||||
if name_match:
|
||||
double_name_match = True
|
||||
name_match = result['id']
|
||||
if not double_name_match and name_match:
|
||||
return name_match
|
||||
if double_name_match and not id_match:
|
||||
# NOTE(arosen): this should only occur is using congress
|
||||
# as admin and multiple tenants use the same datsource name.
|
||||
raise exceptions.Conflict(
|
||||
"Multiple resources have this name %s. Delete by id." % name)
|
||||
if id_match:
|
||||
return id_match
|
||||
|
||||
raise exceptions.NotFound("Resource %s not found" % name)
|
||||
|
@ -19,38 +19,12 @@ import logging
|
||||
from cliff import command
|
||||
from cliff import lister
|
||||
from cliff import show
|
||||
from keystoneclient.openstack.common.apiclient import exceptions
|
||||
import six
|
||||
|
||||
from congressclient.common import parseractions
|
||||
from congressclient.common import utils
|
||||
|
||||
|
||||
def get_resource_id_from_name(name, results):
|
||||
# FIXME(arosen): move to common lib and add tests...
|
||||
name_match = None
|
||||
id_match = None
|
||||
double_name_match = False
|
||||
for result in results['results']:
|
||||
if result['id'] == name:
|
||||
id_match = result['id']
|
||||
if result['name'] == name:
|
||||
if name_match:
|
||||
double_name_match = True
|
||||
name_match = result['id']
|
||||
if not double_name_match and name_match:
|
||||
return name_match
|
||||
if double_name_match and not id_match:
|
||||
# NOTE(arosen): this should only occur is using congress
|
||||
# as admin and multiple tenants use the same datsource name.
|
||||
raise exceptions.Conflict(
|
||||
"Multiple resources have this name %s. Delete by id." % name)
|
||||
if id_match:
|
||||
return id_match
|
||||
|
||||
raise exceptions.NotFound("Resource %s not found" % name)
|
||||
|
||||
|
||||
class ListDatasources(lister.Lister):
|
||||
"""List Datasources."""
|
||||
|
||||
@ -115,8 +89,8 @@ class ShowDatasourceStatus(show.ShowOne):
|
||||
client = self.app.client_manager.congressclient
|
||||
|
||||
results = client.list_datasources()
|
||||
datasource_id = get_resource_id_from_name(parsed_args.datasource_name,
|
||||
results)
|
||||
datasource_id = utils.get_resource_id_from_name(
|
||||
parsed_args.datasource_name, results)
|
||||
|
||||
data = client.list_datasource_status(datasource_id)
|
||||
return zip(*sorted(six.iteritems(data)))
|
||||
@ -139,8 +113,8 @@ class ShowDatasourceSchema(lister.Lister):
|
||||
self.log.debug('take_action(%s)' % parsed_args)
|
||||
client = self.app.client_manager.congressclient
|
||||
results = client.list_datasources()
|
||||
datasource_id = get_resource_id_from_name(parsed_args.datasource_name,
|
||||
results)
|
||||
datasource_id = utils.get_resource_id_from_name(
|
||||
parsed_args.datasource_name, results)
|
||||
data = client.show_datasource_schema(datasource_id)
|
||||
formatters = {'columns': utils.format_long_dict_list}
|
||||
newdata = [{'table': x['table_id'],
|
||||
@ -204,8 +178,8 @@ class ListDatasourceRows(lister.Lister):
|
||||
client = self.app.client_manager.congressclient
|
||||
|
||||
results = client.list_datasources()
|
||||
datasource_id = get_resource_id_from_name(parsed_args.datasource_name,
|
||||
results)
|
||||
datasource_id = utils.get_resource_id_from_name(
|
||||
parsed_args.datasource_name, results)
|
||||
results = client.list_datasource_rows(datasource_id,
|
||||
parsed_args.table)['results']
|
||||
if results:
|
||||
@ -286,20 +260,20 @@ class CreateDatasource(show.ShowOne):
|
||||
class DeleteDatasource(command.Command):
|
||||
"""Delete a datasource."""
|
||||
|
||||
log = logging.getLogger(__name__ + '.DeletePolicyRule')
|
||||
log = logging.getLogger(__name__ + '.DeleteDatasource')
|
||||
|
||||
def get_parser(self, prog_name):
|
||||
parser = super(DeleteDatasource, self).get_parser(prog_name)
|
||||
parser.add_argument(
|
||||
'datasource',
|
||||
metavar="<datasource-name>",
|
||||
help="Name of the policy to delete")
|
||||
help="Name of the datasource to delete")
|
||||
return parser
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug('take_action(%s)' % parsed_args)
|
||||
client = self.app.client_manager.congressclient
|
||||
results = client.list_datasources()
|
||||
datasource_id = get_resource_id_from_name(parsed_args.datasource,
|
||||
results)
|
||||
datasource_id = utils.get_resource_id_from_name(
|
||||
parsed_args.datasource, results)
|
||||
client.delete_datasource(datasource_id)
|
||||
|
@ -104,13 +104,10 @@ class DeletePolicyRule(command.Command):
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug('take_action(%s)' % parsed_args)
|
||||
client = self.app.client_manager.congressclient
|
||||
try:
|
||||
client.delete_policy_rule(parsed_args.policy_name,
|
||||
parsed_args.rule_id)
|
||||
except exceptions.NotFound:
|
||||
rule_id = get_rule_id_from_name(client, parsed_args)
|
||||
if rule_id is not None:
|
||||
client.delete_policy_rule(parsed_args.policy_name, rule_id)
|
||||
results = client.list_policy_rules(parsed_args.policy_name)
|
||||
rule_id = utils.get_resource_id_from_name(
|
||||
parsed_args.rule_id, results)
|
||||
client.delete_policy_rule(parsed_args.policy_name, rule_id)
|
||||
|
||||
|
||||
class ListPolicyRules(command.Command):
|
||||
@ -370,14 +367,10 @@ class ShowPolicyRule(show.ShowOne):
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug('take_action(%s)' % parsed_args)
|
||||
client = self.app.client_manager.congressclient
|
||||
try:
|
||||
data = client.show_policy_rule(parsed_args.policy_name,
|
||||
parsed_args.rule_id)
|
||||
except exceptions.NotFound:
|
||||
rule_id = get_rule_id_from_name(client, parsed_args)
|
||||
if rule_id is not None:
|
||||
data = client.show_policy_rule(parsed_args.policy_name,
|
||||
rule_id)
|
||||
results = client.list_policy_rules(parsed_args.policy_name)
|
||||
rule_id = utils.get_resource_id_from_name(
|
||||
parsed_args.rule_id, results)
|
||||
data = client.show_policy_rule(parsed_args.policy_name, rule_id)
|
||||
return zip(*sorted(six.iteritems(data)))
|
||||
|
||||
|
||||
@ -424,5 +417,8 @@ class ShowPolicy(show.ShowOne):
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug('take_action(%s)' % parsed_args)
|
||||
client = self.app.client_manager.congressclient
|
||||
data = client.show_policy(parsed_args.policy_name)
|
||||
results = client.list_policy()
|
||||
policy_id = utils.get_resource_id_from_name(
|
||||
parsed_args.policy_name, results)
|
||||
data = client.show_policy(policy_id)
|
||||
return zip(*sorted(six.iteritems(data)))
|
||||
|
@ -13,6 +13,7 @@
|
||||
|
||||
import mock
|
||||
|
||||
from congressclient.common import utils
|
||||
from congressclient.osc.v1 import datasource
|
||||
from congressclient.tests import common
|
||||
|
||||
@ -85,7 +86,7 @@ class TestListDatasourceStatus(common.TestCongressBase):
|
||||
cmd = datasource.ShowDatasourceStatus(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
with mock.patch.object(datasource, "get_resource_id_from_name",
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = list(cmd.take_action(parsed_args))
|
||||
|
||||
@ -121,7 +122,7 @@ class TestShowDatasourceSchema(common.TestCongressBase):
|
||||
cmd = datasource.ShowDatasourceSchema(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
with mock.patch.object(datasource, "get_resource_id_from_name",
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = cmd.take_action(parsed_args)
|
||||
|
||||
@ -189,7 +190,7 @@ class TestListDatasourceRows(common.TestCongressBase):
|
||||
cmd = datasource.ListDatasourceRows(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
with mock.patch.object(datasource, "get_resource_id_from_name",
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = cmd.take_action(parsed_args)
|
||||
|
||||
@ -255,7 +256,7 @@ class TestDeleteDatasourceDriver(common.TestCongressBase):
|
||||
self.app.client_manager.congressclient.list_datasources = mock.Mock()
|
||||
cmd = datasource.DeleteDatasource(self.app, self.namespace)
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
with mock.patch.object(datasource, "get_resource_id_from_name",
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = cmd.take_action(parsed_args)
|
||||
mocker.assert_called_with("id")
|
||||
|
@ -13,6 +13,7 @@
|
||||
|
||||
import mock
|
||||
|
||||
from congressclient.common import utils
|
||||
from congressclient.osc.v1 import policy
|
||||
from congressclient.tests import common
|
||||
|
||||
@ -63,10 +64,12 @@ class TestShowPolicy(common.TestCongressBase):
|
||||
|
||||
mocker = mock.Mock(return_value=response)
|
||||
self.app.client_manager.congressclient.show_policy = mocker
|
||||
self.app.client_manager.congressclient.list_policy = mock.Mock()
|
||||
cmd = policy.ShowPolicy(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
result = list(cmd.take_action(parsed_args))
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="name"):
|
||||
result = list(cmd.take_action(parsed_args))
|
||||
filtered = [('abbreviation', 'description', 'id', 'kind', 'name',
|
||||
'owner'),
|
||||
(policy_name, '', policy_id, 'nonrecursive',
|
||||
@ -85,10 +88,13 @@ class TestDeletePolicy(common.TestCongressBase):
|
||||
]
|
||||
mocker = mock.Mock(return_value=None)
|
||||
self.app.client_manager.congressclient.delete_policy = mocker
|
||||
self.app.client_manager.congressclient.list_policy = mock.Mock()
|
||||
cmd = policy.DeletePolicy(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
result = cmd.take_action(parsed_args)
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = cmd.take_action(parsed_args)
|
||||
|
||||
mocker.assert_called_with(policy_id)
|
||||
self.assertEqual(None, result)
|
||||
@ -158,10 +164,13 @@ class TestDeletePolicyRule(common.TestCongressBase):
|
||||
]
|
||||
mocker = mock.Mock(return_value=None)
|
||||
self.app.client_manager.congressclient.delete_policy_rule = mocker
|
||||
self.app.client_manager.congressclient.list_policy_rules = mock.Mock()
|
||||
cmd = policy.DeletePolicyRule(self.app, self.namespace)
|
||||
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
result = cmd.take_action(parsed_args)
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value=rule_id):
|
||||
result = cmd.take_action(parsed_args)
|
||||
|
||||
mocker.assert_called_with(policy_name, rule_id)
|
||||
self.assertEqual(None, result)
|
||||
@ -416,9 +425,12 @@ class TestGet(common.TestCongressBase):
|
||||
|
||||
mocker = mock.Mock(return_value=response)
|
||||
self.app.client_manager.congressclient.show_policy_rule = mocker
|
||||
self.app.client_manager.congressclient.list_policy_rules = mock.Mock()
|
||||
cmd = policy.ShowPolicyRule(self.app, self.namespace)
|
||||
parsed_args = self.check_parser(cmd, arglist, verifylist)
|
||||
result = list(cmd.take_action(parsed_args))
|
||||
with mock.patch.object(utils, "get_resource_id_from_name",
|
||||
return_value="id"):
|
||||
result = list(cmd.take_action(parsed_args))
|
||||
filtered = [('comment', 'id', 'rule'),
|
||||
('None', id, rule)]
|
||||
self.assertEqual(filtered, result)
|
||||
|
Loading…
Reference in New Issue
Block a user