openstack
/
manila
Code Issues Proposed changes

Merge "Fix the exact filter can be filter by inexact value"

This commit is contained in:
Jenkins 2017-07-26 20:56:45 +00:00 committed by Gerrit Code Review
parent 794526df7a f906bcdd4e
commit 244284fb6e
12 changed files with 192 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
manila/api/v1/share_snapshots.py
View File

@ -74,12 +74,14 @@ class ShareSnapshotMixin(object):
"""Returns a summary list of snapshots."""
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_snapshots(req, is_detail=False)
def detail(self, req):
"""Returns a detailed list of snapshots."""
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_snapshots(req, is_detail=True)
def _get_snapshots(self, req, is_detail):
@ -100,6 +102,9 @@ class ShareSnapshotMixin(object):
# from Cinder v1 and v2 APIs.
if 'name' in search_opts:
search_opts['display_name'] = search_opts.pop('name')
if 'description' in search_opts:
search_opts['display_description'] = search_opts.pop(
'description')
# like filter
for key, db_key in (('name~', 'display_name~'),
@ -131,7 +136,8 @@ class ShareSnapshotMixin(object):
def _get_snapshots_search_options(self):
"""Return share snapshot search options allowed by non-admin."""
return ('display_name', 'name', 'status', 'share_id', 'size',
'display_name~', 'display_description~')
'display_name~', 'display_description~', 'description',
'display_description')
def update(self, req, id, body):
"""Update a snapshot."""

7
manila/api/v1/shares.py
View File

@ -104,6 +104,7 @@ class ShareMixin(object):
req.GET.pop('export_location_path', None)
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_shares(req, is_detail=False)
def detail(self, req):
@ -112,6 +113,7 @@ class ShareMixin(object):
req.GET.pop('export_location_path', None)
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_shares(req, is_detail=True)
def _get_shares(self, req, is_detail):
@ -139,6 +141,9 @@ class ShareMixin(object):
# from Cinder v1 and v2 APIs.
if 'name' in search_opts:
search_opts['display_name'] = search_opts.pop('name')
if 'description' in search_opts:
search_opts['display_description'] = search_opts.pop(
'description')
# like filter
for key, db_key in (('name~', 'display_name~'),
@ -176,7 +181,7 @@ class ShareMixin(object):
'is_public', 'metadata', 'extra_specs', 'sort_key', 'sort_dir',
'share_group_id', 'share_group_snapshot_id',
'export_location_id', 'export_location_path', 'display_name~',
'display_description~'
'display_description~', 'description', 'display_description'
)
def update(self, req, id, body):

3
manila/api/v2/share_networks.py
View File

@ -172,7 +172,8 @@ class ShareNetworkController(wsgi.Controller):
networks = [network for network in networks
if network.get(key) == value or
(value in network.get(key.rstrip('~'))
if network.get(key.rstrip('~')) else ())]
if key.endswith('~') and
network.get(key.rstrip('~')) else ())]
else:
networks = [network for network in networks
if network.get(key) == value]

2
manila/api/v2/share_snapshots.py
View File

@ -294,6 +294,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
if req.api_version_request < api_version.APIVersionRequest("2.36"):
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_snapshots(req, is_detail=False)
@wsgi.Controller.api_version("2.0")
@ -302,6 +303,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
if req.api_version_request < api_version.APIVersionRequest("2.36"):
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_snapshots(req, is_detail=True)

2
manila/api/v2/shares.py
View File

@ -427,6 +427,7 @@ class ShareController(shares.ShareMixin,
if req.api_version_request < api_version.APIVersionRequest("2.36"):
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_shares(req, is_detail=False)
@ -440,6 +441,7 @@ class ShareController(shares.ShareMixin,
if req.api_version_request < api_version.APIVersionRequest("2.36"):
req.GET.pop('name~', None)
req.GET.pop('description~', None)
req.GET.pop('description', None)
return self._get_shares(req, is_detail=True)

7
manila/share/api.py
View File

@ -1566,7 +1566,7 @@ class API(base.Base):
for s in shares:
# values in search_opts can be only strings
if (all(s.get(k, None) == v or (v in (s.get(k.rstrip('~'))
if s.get(k.rstrip('~')) else ()))
if k.endswith('~') and s.get(k.rstrip('~')) else ()))
for k, v in search_opts.items())):
results.append(s)
shares = results
@ -1610,8 +1610,9 @@ class API(base.Base):
results = []
not_found = object()
for snapshot in snapshots:
if (all(snapshot.get(k, not_found) == v or (v in
snapshot.get(k.rstrip('~')) if
if (all(snapshot.get(k, not_found) == v or
(v in snapshot.get(k.rstrip('~'))
if k.endswith('~') and
snapshot.get(k.rstrip('~')) else ())
for k, v in search_opts.items())):
results.append(snapshot)

33
manila/tests/api/v2/test_share_networks.py
View File

@ -450,22 +450,35 @@ class ShareNetworkAPITest(test.TestCase):
result[share_networks.RESOURCES_NAME][0],
fake_sn_with_ss_shortened)
@ddt.data(('name=fo', 0), ('description=d', 0),
('name=foo&description=d', 0),
('name=foo', 1), ('description=ds', 1),
('name~=foo&description~=ds', 2),
('name=foo&description~=ds', 1),
('name~=foo&description=ds', 1))
@ddt.unpack
@mock.patch.object(db_api, 'share_network_get_all_by_project',
mock.Mock())
def test_index_filter_by_like_filter(self):
db_api.share_network_get_all_by_project.return_value = [
fake_share_network,
]
def test_index_filter_by_name_and_description(
self, filter, share_network_number):
fake_objs = [{'name': 'fo2', 'description': 'd2', 'id': 'fake1'},
{'name': 'foo', 'description': 'ds', 'id': 'fake2'},
{'name': 'foo1', 'description': 'ds1', 'id': 'fake3'}]
db_api.share_network_get_all_by_project.return_value = fake_objs
req = fakes.HTTPRequest.blank(
'/share_networks?name~=fake&description~=fake',
'/share_networks?' + filter,
use_admin_context=True, version='2.36')
result = self.controller.index(req)
db_api.share_network_get_all_by_project.assert_called_once_with(
db_api.share_network_get_all_by_project.assert_called_with(
req.environ['manila.context'], self.context.project_id)
self.assertEqual(1, len(result[share_networks.RESOURCES_NAME]))
self._check_share_network_view_shortened(
result[share_networks.RESOURCES_NAME][0],
fake_share_network_shortened)
self.assertEqual(share_network_number,
len(result[share_networks.RESOURCES_NAME]))
if share_network_number > 0:
self._check_share_network_view_shortened(
result[share_networks.RESOURCES_NAME][0], fake_objs[1])
if share_network_number > 1:
self._check_share_network_view_shortened(
result[share_networks.RESOURCES_NAME][1], fake_objs[2])
@mock.patch.object(db_api, 'share_network_get_all_by_project',
mock.Mock())

29
manila/tests/db/sqlalchemy/test_api.py
View File

@ -813,18 +813,31 @@ class ShareGroupDatabaseAPITestCase(test.TestCase):
self.assertDictMatch(dict(expected_group), dict(group))
self.assertEqual(fake_project, group['project_id'])
def test_share_group_get_all_by_like_filter(self):
expected_group = db_utils.create_share_group(
name='test1', description='test1')
db_utils.create_share_group(name='fake', description='fake')
@ddt.data(({'name': 'fo'}, 0), ({'description': 'd'}, 0),
({'name': 'foo', 'description': 'd'}, 0),
({'name': 'foo'}, 1), ({'description': 'ds'}, 1),
({'name~': 'foo', 'description~': 'ds'}, 2),
({'name': 'foo', 'description~': 'ds'}, 1),
({'name~': 'foo', 'description': 'ds'}, 1))
@ddt.unpack
def test_share_group_get_all_by_name_and_description(
self, search_opts, group_number):
db_utils.create_share_group(name='fo1', description='d1')
expected_group1 = db_utils.create_share_group(name='foo',
description='ds')
expected_group2 = db_utils.create_share_group(name='foo1',
description='ds2')
groups = db_api.share_group_get_all(
self.ctxt, detailed=True,
filters={'name~': 'test', 'description~': 'test'})
filters=search_opts)
self.assertEqual(1, len(groups))
group = groups[0]
self.assertDictMatch(dict(expected_group), dict(group))
self.assertEqual(group_number, len(groups))
if group_number == 1:
self.assertDictMatch(dict(expected_group1), dict(groups[0]))
elif group_number == 1:
self.assertDictMatch(dict(expected_group1), dict(groups[1]))
self.assertDictMatch(dict(expected_group2), dict(groups[0]))
def test_share_group_update(self):
fake_name = "my_fake_name"

42
manila/tests/share/test_api.py
View File

@ -312,11 +312,19 @@ class ShareAPITestCase(test.TestCase):
)
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[1::2], shares)
def test_get_all_admin_filter_by_inexact_filter(self):
@ddt.data(({'name': 'fo'}, 0), ({'description': 'd'}, 0),
({'name': 'foo', 'description': 'd'}, 0),
({'name': 'foo'}, 1), ({'description': 'ds'}, 1),
({'name~': 'foo', 'description~': 'ds'}, 2),
({'name': 'foo', 'description~': 'ds'}, 1),
({'name~': 'foo', 'description': 'ds'}, 1))
@ddt.unpack
def test_get_all_admin_filter_by_name_and_description(
self, search_opts, get_share_number):
ctx = context.RequestContext('fake_uid', 'fake_pid_2', is_admin=True)
self.mock_object(db_api, 'share_get_all_by_project',
mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES))
shares = self.api.get_all(ctx, {'name~': 'foo', 'description~': 'ds'})
shares = self.api.get_all(ctx, search_opts)
share_api.policy.check_policy.assert_has_calls([
mock.call(ctx, 'share', 'get_all'),
])
@ -325,7 +333,11 @@ class ShareAPITestCase(test.TestCase):
project_id='fake_pid_2',
filters={}, is_public=False
)
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[0::2], shares)
self.assertEqual(get_share_number, len(shares))
if get_share_number == 2:
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[0::2], shares)
elif get_share_number == 1:
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[:1], shares)
@ddt.data('id', 'path')
def test_get_all_admin_filter_by_export_location(self, type):
@ -353,7 +365,7 @@ class ShareAPITestCase(test.TestCase):
])
db_api.share_get_all.assert_called_once_with(
ctx, sort_dir='desc', sort_key='created_at', filters={})
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[::2], shares)
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[:1], shares)
def test_get_all_admin_filter_by_status(self):
ctx = context.RequestContext('fake_uid', 'fake_pid_2', is_admin=True)
@ -416,7 +428,7 @@ class ShareAPITestCase(test.TestCase):
# one item expected, two filtered
shares = self.api.get_all(
ctx, {'name': 'foo', 'status': constants.STATUS_AVAILABLE})
ctx, {'name': 'foo1', 'status': constants.STATUS_AVAILABLE})
self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[2::4], shares)
share_api.policy.check_policy.assert_has_calls([
mock.call(ctx, 'share', 'get_all'),
@ -1919,9 +1931,16 @@ class ShareAPITestCase(test.TestCase):
ctx, 'fakepid', sort_dir='desc', sort_key='share_id',
filters=search_opts)
def test_get_all_snapshots_not_admin_inexact_search_opts(self):
search_opts = {'name~': 'foo', 'description~': 'ds'}
fake_objs = [{'name': 'fo', 'description': 'd'},
@ddt.data(({'name': 'fo'}, 0), ({'description': 'd'}, 0),
({'name': 'foo', 'description': 'd'}, 0),
({'name': 'foo'}, 1), ({'description': 'ds'}, 1),
({'name~': 'foo', 'description~': 'ds'}, 2),
({'name': 'foo', 'description~': 'ds'}, 1),
({'name~': 'foo', 'description': 'ds'}, 1))
@ddt.unpack
def test_get_all_snapshots_filter_by_name_and_description(
self, search_opts, get_snapshot_number):
fake_objs = [{'name': 'fo2', 'description': 'd2'},
{'name': 'foo', 'description': 'ds'},
{'name': 'foo1', 'description': 'ds1'}]
ctx = context.RequestContext('fakeuid', 'fakepid', is_admin=False)
@ -1930,7 +1949,12 @@ class ShareAPITestCase(test.TestCase):
result = self.api.get_all_snapshots(ctx, search_opts)
self.assertEqual(fake_objs[1:], result)
self.assertEqual(get_snapshot_number, len(result))
if get_snapshot_number == 2:
self.assertEqual(fake_objs[1:], result)
elif get_snapshot_number == 1:
self.assertEqual(fake_objs[1:2], result)
share_api.policy.check_policy.assert_called_once_with(
ctx, 'share_snapshot', 'get_all_snapshots')
db_api.share_snapshot_get_all_by_project.assert_called_once_with(

26
manila_tempest_tests/tests/api/test_shares_actions.py
View File

@ -338,7 +338,15 @@ class SharesActionsTest(base.BaseSharesTest):
@tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_lt("2.36")
def test_list_shares_with_detail_filter_by_nonexistent_name(self):
def test_list_shares_with_detail_filter_by_existed_description(self):
# list shares by description, at least one share is expected
params = {"description": self.share_desc}
shares = self.shares_v2_client.list_shares_with_detail(params)
self.assertEqual(self.share_name, shares[0]["name"])
@tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_lt("2.36")
def test_list_shares_with_detail_filter_by_inexact_name(self):
# list shares by name, at least one share is expected
params = {"name~": 'tempest-share'}
shares = self.shares_v2_client.list_shares_with_detail(params)
@ -560,6 +568,22 @@ class SharesActionsTest(base.BaseSharesTest):
self.assertEqual(filters['status'], snap['status'])
self.assertEqual(filters['name'], snap['name'])
@tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
@testtools.skipUnless(CONF.share.run_snapshot_tests,
"Snapshot tests are disabled.")
@base.skip_if_microversion_not_supported("2.35")
def test_list_snapshots_with_detail_filter_by_description(self):
filters = {'description': self.snap_desc}
# list snapshots
snaps = self.shares_client.list_snapshots_with_detail(
params=filters)
# verify response
self.assertGreater(len(snaps), 0)
for snap in snaps:
self.assertEqual(filters['description'], snap['description'])
@tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
@testtools.skipUnless(CONF.share.run_snapshot_tests,
"Snapshot tests are disabled.")

58
manila_tempest_tests/tests/api/test_shares_actions_negative.py
View File

@ -15,6 +15,7 @@
import ddt
from tempest import config
from tempest.lib.common.utils import data_utils
from tempest.lib import exceptions as lib_exc
import testtools
from testtools import testcase as tc
@ -30,7 +31,18 @@ class SharesActionsNegativeTest(base.BaseSharesMixedTest):
def resource_setup(cls):
super(SharesActionsNegativeTest, cls).resource_setup()
cls.admin_client = cls.admin_shares_v2_client
cls.share = cls.create_share()
cls.share_name = data_utils.rand_name("tempest-share-name")
cls.share_desc = data_utils.rand_name("tempest-share-description")
cls.share = cls.create_share(
name=cls.share_name,
description=cls.share_desc)
if CONF.share.run_snapshot_tests:
# create snapshot
cls.snap_name = data_utils.rand_name("tempest-snapshot-name")
cls.snap_desc = data_utils.rand_name(
"tempest-snapshot-description")
cls.snap = cls.create_snapshot_wait_for_active(
cls.share["id"], cls.snap_name, cls.snap_desc)
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@testtools.skipUnless(
@ -167,7 +179,7 @@ class SharesActionsNegativeTest(base.BaseSharesMixedTest):
self.assertEqual(0, len(shares))
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_not_supported("2.35")
@base.skip_if_microversion_not_supported("2.36")
def test_list_shares_with_like_filter_and_invalid_version(self):
# In API versions < v2.36, querying the share API by inexact
# filter (name or description) should have no effect. Those
@ -182,7 +194,7 @@ class SharesActionsNegativeTest(base.BaseSharesMixedTest):
self.assertGreater(len(shares), 0)
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_not_supported("2.35")
@base.skip_if_microversion_not_supported("2.36")
def test_list_shares_with_like_filter_not_exist(self):
filters = {
'name~': 'fake_not_exist',
@ -191,3 +203,43 @@ class SharesActionsNegativeTest(base.BaseSharesMixedTest):
shares = self.shares_v2_client.list_shares(params=filters)
self.assertEqual(0, len(shares))
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
def test_list_shares_with_name_not_exist(self):
filters = {
'name': "tempest-share",
}
shares = self.shares_v2_client.list_shares(params=filters)
self.assertEqual(0, len(shares))
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_not_supported("2.36")
def test_list_shares_with_description_not_exist(self):
filters = {
'description': "tempest-share",
}
shares = self.shares_v2_client.list_shares(params=filters)
self.assertEqual(0, len(shares))
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@base.skip_if_microversion_not_supported("2.36")
def test_list_snapshots_with_description_not_exist(self):
filters = {
'description': "tempest-snapshot",
}
shares = self.shares_v2_client.list_snapshots_with_detail(
params=filters)
self.assertEqual(0, len(shares))
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
def test_list_snapshots_with_name_not_exist(self):
filters = {
'name': "tempest-snapshot",
}
shares = self.shares_v2_client.list_snapshots_with_detail(
params=filters)
self.assertEqual(0, len(shares))

12
releasenotes/notes/bug-1704971-fix-name-description-filter-85935582e89a72a0.yaml Normal file
View File

@ -0,0 +1,12 @@
---
fixes:
- Fix the ``exact`` filters (name, description) in ``shares``,
``snapshots``, ``share-networks`` list can be filter by ``inexact``
value.
We got the error because the ``description`` filter will be
skipped in ``shares``, ``snapshots`` list API, and we will directly
remove the ``inexact`` filter flag('~') and process the
``exact`` filters (name, description) by ``inexact`` filter logic.
Now, we added ``description`` filter in ``shares``, ``snapshots`` list,
and check whether the filter keys has the '~' in the end in ``shares``,
``snapshots``, ``share-networks`` list firstly.