Fix the exact filter can be filter by inexact value

Fix the ``exact`` filters (name, description) in ``shares``,
``snapshots``, ``share-networks`` list can be filter by ``inexact``
value.

Change-Id: I51e6b754f37a09c09a60e9a7fb51d3c9721f2d1f
Closes-bug: #1704971

manila/api/v1/share_snapshots.py

@@ -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."""

manila/api/v1/shares.py

@@ -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):

manila/api/v2/share_networks.py

@@ -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]

manila/api/v2/share_snapshots.py

@@ -289,6 +289,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")
@@ -297,6 +298,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)
 
 

manila/api/v2/shares.py

@@ -425,6 +425,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)
 
@@ -438,6 +439,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)
 

manila/share/api.py

@@ -1548,7 +1548,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
@@ -1592,8 +1592,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)

manila/tests/api/v2/test_share_networks.py

@@ -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.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_share_network_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())

manila/tests/db/sqlalchemy/test_api.py

@@ -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"

manila/tests/share/test_api.py

@@ -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(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'),
@@ -1911,9 +1923,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)
@@ -1922,7 +1941,12 @@ class ShareAPITestCase(test.TestCase):
 
         result = self.api.get_all_snapshots(ctx, search_opts)
 
+        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(

manila_tempest_tests/tests/api/test_shares_actions.py

@@ -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.")

manila_tempest_tests/tests/api/test_shares_actions_negative.py

@@ -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))

releasenotes/notes/bug-1704971-fix-name-description-filter-85935582e89a72a0.yaml

@@ -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.