Skip to content

Commit bafd84b

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Optional filters parameters should be passed only once"
2 parents 61fec71 + 624b444 commit bafd84b

4 files changed

Lines changed: 136 additions & 21 deletions

File tree

‎cinderclient/shell.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,33 @@
5454
HINT_HELP_MSG = (" [hint: use '--os-volume-api-version' flag to show help "
5555
"message for proper version]")
5656

57+
FILTER_CHECK = ["type-list",
58+
"backup-list",
59+
"get-pools",
60+
"list",
61+
"group-list",
62+
"group-snapshot-list",
63+
"message-list",
64+
"snapshot-list",
65+
"attachment-list"]
66+
67+
RESOURCE_FILTERS = {
68+
"list": ["name", "status", "metadata",
69+
"bootable", "migration_status", "availability_zone",
70+
"group_id", "size"],
71+
"backup-list": ["name", "status", "volume_id"],
72+
"snapshot-list": ["name", "status", "volume_id", "metadata",
73+
"availability_zone"],
74+
"group-list": ["name"],
75+
"group-snapshot-list": ["name", "status", "group_id"],
76+
"attachment-list": ["volume_id", "status", "instance_id", "attach_status"],
77+
"message-list": ["resource_uuid", "resource_type", "event_id",
78+
"request_id", "message_level"],
79+
"get-pools": ["name", "volume_type"],
80+
"type-list": ["is_public"]
81+
}
82+
83+
5784
logging.basicConfig()
5885
logger = logging.getLogger(__name__)
5986

@@ -521,8 +548,28 @@ def downgrade_warning(requested, discovered):
521548
logger.warning("downgrading to %s based on server support." %
522549
discovered.get_string())
523550

551+
def check_duplicate_filters(self, argv, filter):
552+
resource = RESOURCE_FILTERS[filter]
553+
filters = []
554+
for opt in range(len(argv)):
555+
if argv[opt].startswith('--'):
556+
if argv[opt] == '--filters':
557+
key, __ = argv[opt + 1].split('=')
558+
if key in resource:
559+
filters.append(key)
560+
elif argv[opt][2:] in resource:
561+
filters.append(argv[opt][2:])
562+
563+
if len(set(filters)) != len(filters):
564+
raise exc.CommandError(
565+
"Filters are only allowed to be passed once.")
566+
524567
def main(self, argv):
525568
# Parse args once to find version and debug settings
569+
for filter in FILTER_CHECK:
570+
if filter in argv:
571+
self.check_duplicate_filters(argv, filter)
572+
break
526573
parser = self.get_base_parser()
527574
(options, args) = parser.parse_known_args(argv)
528575
self.setup_debugging(options.debug)

‎cinderclient/tests/unit/test_shell.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,12 @@ def list_volumes_on_service(self, count, mocker):
201201
_shell = shell.OpenStackCinderShell()
202202
_shell.main(['list'])
203203

204+
def test_duplicate_filters(self):
205+
_shell = shell.OpenStackCinderShell()
206+
self.assertRaises(exceptions.CommandError,
207+
_shell.main,
208+
['list', '--name', 'abc', '--filters', 'name=xyz'])
209+
204210
@unittest.skip("Skip cuz I broke it")
205211
def test_cinder_service_name(self):
206212
# Failing with 'No mock address' means we are not

‎cinderclient/tests/unit/v3/test_shell.py‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ def test_list_filters(self, resource, query_url):
145145
u'list --filters name~=Σ',
146146
'expected':
147147
'/volumes/detail?name~=%CE%A3'},
148+
{'command':
149+
u'list --filters name=abc --filters size=1',
150+
'expected':
151+
'/volumes/detail?name=abc&size=1'},
148152
# testcases for list group
149153
{'command':
150154
'group-list --filters name=456',
@@ -158,6 +162,10 @@ def test_list_filters(self, resource, query_url):
158162
'group-list --filters name~=456',
159163
'expected':
160164
'/groups/detail?name~=456'},
165+
{'command':
166+
'group-list --filters name=abc --filters status=available',
167+
'expected':
168+
'/groups/detail?name=abc&status=available'},
161169
# testcases for list group-snapshot
162170
{'command':
163171
'group-snapshot-list --status=error --filters status=available',
@@ -171,6 +179,11 @@ def test_list_filters(self, resource, query_url):
171179
'group-snapshot-list --filters status~=available',
172180
'expected':
173181
'/group_snapshots/detail?status~=available'},
182+
{'command':
183+
'group-snapshot-list --filters status=available '
184+
'--filters availability_zone=123',
185+
'expected':
186+
'/group_snapshots/detail?availability_zone=123&status=available'},
174187
# testcases for list message
175188
{'command':
176189
'message-list --event_id=123 --filters event_id=456',
@@ -184,6 +197,10 @@ def test_list_filters(self, resource, query_url):
184197
'message-list --filters request_id~=123',
185198
'expected':
186199
'/messages?request_id~=123'},
200+
{'command':
201+
'message-list --filters request_id=123 --filters event_id=456',
202+
'expected':
203+
'/messages?event_id=456&request_id=123'},
187204
# testcases for list attachment
188205
{'command':
189206
'attachment-list --volume-id=123 --filters volume_id=456',
@@ -197,6 +214,11 @@ def test_list_filters(self, resource, query_url):
197214
'attachment-list --filters volume_id~=456',
198215
'expected':
199216
'/attachments?volume_id~=456'},
217+
{'command':
218+
'attachment-list --filters volume_id=123 '
219+
'--filters mountpoint=456',
220+
'expected':
221+
'/attachments?mountpoint=456&volume_id=123'},
200222
# testcases for list backup
201223
{'command':
202224
'backup-list --volume-id=123 --filters volume_id=456',
@@ -210,6 +232,10 @@ def test_list_filters(self, resource, query_url):
210232
'backup-list --filters volume_id~=456',
211233
'expected':
212234
'/backups/detail?volume_id~=456'},
235+
{'command':
236+
'backup-list --filters volume_id=123 --filters name=456',
237+
'expected':
238+
'/backups/detail?name=456&volume_id=123'},
213239
# testcases for list snapshot
214240
{'command':
215241
'snapshot-list --volume-id=123 --filters volume_id=456',
@@ -223,6 +249,10 @@ def test_list_filters(self, resource, query_url):
223249
'snapshot-list --filters volume_id~=456',
224250
'expected':
225251
'/snapshots/detail?volume_id~=456'},
252+
{'command':
253+
'snapshot-list --filters volume_id=123 --filters name=456',
254+
'expected':
255+
'/snapshots/detail?name=456&volume_id=123'},
226256
# testcases for get pools
227257
{'command':
228258
'get-pools --filters name=456 --detail',
@@ -231,7 +261,11 @@ def test_list_filters(self, resource, query_url):
231261
{'command':
232262
'get-pools --filters name=456',
233263
'expected':
234-
'/scheduler-stats/get_pools?name=456'}
264+
'/scheduler-stats/get_pools?name=456'},
265+
{'command':
266+
'get-pools --filters name=456 --filters detail=True',
267+
'expected':
268+
'/scheduler-stats/get_pools?detail=True&name=456'}
235269
)
236270
@ddt.unpack
237271
def test_list_with_filters_mixed(self, command, expected):

0 commit comments

Comments
 (0)