Fix set/unset extra specs for share type update form
Currently, form of share type extra specs update does not allow several valid cases for set/unset operations. So, fix them and cover with unit tests. Change-Id: I446a7226a3154f7d475115560fe343142311973b Closes-Bug: #1481903
This commit is contained in:
Valeriy Ponomaryov
vponomaryov
parent
8fe95c64e8
commit
43f6005f20
|
|
@ -303,9 +303,9 @@ def share_type_set_extra_specs(request, share_type_id, extra_specs):
|
|||
share_type_id).set_keys(extra_specs)
|
||||
|
||||
|
||||
def share_type_unset_extra_specs(request, share_type_id, key):
|
||||
def share_type_unset_extra_specs(request, share_type_id, keys):
|
||||
return manilaclient(request).share_types.get(
|
||||
share_type_id).unset_keys(key)
|
||||
share_type_id).unset_keys(keys)
|
||||
|
||||
|
||||
def share_type_access_list(request, share_type_id):
|
||||
|
|
|
|||
|
|
@ -220,11 +220,12 @@ class UpdateShareType(forms.SelfHandlingForm):
|
|||
if unset_list:
|
||||
get = manila.share_type_get_extra_specs(
|
||||
request, self.initial["id"])
|
||||
for unset_key in unset_list:
|
||||
# NOTE(vponomaryov): skip keys that are already unset
|
||||
if unset_key in get.keys():
|
||||
manila.share_type_unset_extra_specs(
|
||||
request, self.initial["id"], unset_key)
|
||||
|
||||
# NOTE(vponomaryov): skip keys that are already unset
|
||||
to_unset = set(unset_list).intersection(set(get.keys()))
|
||||
if to_unset:
|
||||
manila.share_type_unset_extra_specs(
|
||||
request, self.initial["id"], to_unset)
|
||||
msg = _("Successfully updated extra specs for share type '%s'.")
|
||||
msg = msg % self.initial['name']
|
||||
messages.success(request, msg)
|
||||
|
|
|
|||
|
|
@ -28,28 +28,29 @@ def parse_str_meta(meta_s):
|
|||
unset_list = []
|
||||
msg = ""
|
||||
for string in strings:
|
||||
if string.count("=") == 1:
|
||||
pair = [p.strip() for p in string.split("=")]
|
||||
if string.count("=") == 0:
|
||||
# Key for unsetting
|
||||
key = string.strip('\"\'\ ')
|
||||
if len(key) not in range(1, 256):
|
||||
msg = _("Key '%s' has inproper length.") % key
|
||||
elif " " in key:
|
||||
msg = _("Key can not contain spaces. See string '%s'.") % key
|
||||
elif key not in unset_list:
|
||||
unset_list.append(key)
|
||||
else:
|
||||
# Key-value pair for setting
|
||||
pair = [p.strip('\"\'\ ') for p in string.split("=", 1)]
|
||||
if not all(len(p) in range(1, 256) for p in pair):
|
||||
msg = _("All keys and values must be in range from 1 to 255.")
|
||||
elif pair[0] in set_dict.keys():
|
||||
elif pair[0] in list(set_dict.keys()):
|
||||
msg = _("Duplicated keys '%s'.") % pair[0]
|
||||
elif any(" " in p for p in pair):
|
||||
msg = _("Keys and values should not contain spaces. "
|
||||
elif " " in pair[0]:
|
||||
msg = _("Keys should not contain spaces. "
|
||||
"Error in '%s'.") % string
|
||||
else:
|
||||
set_dict[pair[0]] = pair[1]
|
||||
elif string.count("=") == 0:
|
||||
s = string.strip()
|
||||
if len(s) not in range(1, 256):
|
||||
msg = _("Key '%s' has inproper length.") % s
|
||||
elif " " in s:
|
||||
msg = _("Key can not contain spaces. See string '%s'.") % s
|
||||
elif s not in unset_list:
|
||||
unset_list.append(s)
|
||||
else:
|
||||
msg = _("Wrong data provided in string '%s'.") % string
|
||||
duplicated_keys = [uk for uk in unset_list if uk in set_dict.keys()]
|
||||
|
||||
duplicated_keys = [uk for uk in unset_list if uk in list(set_dict.keys())]
|
||||
if duplicated_keys:
|
||||
msg = _("Duplicated keys '%s'.") % str(duplicated_keys)
|
||||
if msg:
|
||||
|
|
|
|||
|
|
@ -19,12 +19,33 @@ from manila_ui.test import helpers as base
|
|||
|
||||
class ManilaApiTests(base.APITestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(self.__class__, self).setUp()
|
||||
self.id = "fake_id"
|
||||
|
||||
def test_share_extend(self):
|
||||
fake_share_id = "fake_id"
|
||||
new_size = "123"
|
||||
|
||||
api.share_extend(self.request, fake_share_id, new_size)
|
||||
api.share_extend(self.request, self.id, new_size)
|
||||
|
||||
self.manilaclient.shares.extend.assert_called_once_with(
|
||||
fake_share_id, new_size
|
||||
self.id, new_size
|
||||
)
|
||||
|
||||
def test_share_type_set_extra_specs(self):
|
||||
data = {"foo": "bar"}
|
||||
|
||||
api.share_type_set_extra_specs(self.request, self.id, data)
|
||||
|
||||
share_types_get = self.manilaclient.share_types.get
|
||||
share_types_get.assert_called_once_with(self.id)
|
||||
share_types_get.return_value.set_keys.assert_called_once_with(data)
|
||||
|
||||
def test_share_type_unset_extra_specs(self):
|
||||
keys = ["foo", "bar"]
|
||||
|
||||
api.share_type_unset_extra_specs(self.request, self.id, keys)
|
||||
|
||||
share_types_get = self.manilaclient.share_types.get
|
||||
share_types_get.assert_called_once_with(self.id)
|
||||
share_types_get.return_value.unset_keys.assert_called_once_with(keys)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,161 @@
|
|||
# Copyright (c) 2015 Mirantis, Inc.
|
||||
# All rights reserved.
|
||||
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import ddt
|
||||
from django.core.handlers import wsgi
|
||||
from django import forms as django_forms
|
||||
from horizon import forms as horizon_forms
|
||||
import mock
|
||||
|
||||
from manila_ui.dashboards.admin.shares import forms
|
||||
from manila_ui.test import helpers as base
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class ManilaDashboardsAdminSharesUpdateShareTypeFormTests(base.APITestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(self.__class__, self).setUp()
|
||||
FAKE_ENVIRON = {'REQUEST_METHOD': 'GET', 'wsgi.input': 'fake_input'}
|
||||
self.request = wsgi.WSGIRequest(FAKE_ENVIRON)
|
||||
|
||||
def _get_form(self, initial):
|
||||
kwargs = {
|
||||
'prefix': None,
|
||||
'initial': initial,
|
||||
}
|
||||
return forms.UpdateShareType(self.request, **kwargs)
|
||||
|
||||
@ddt.data(
|
||||
({}, []),
|
||||
({'foo': 'bar', 'quuz': 'zaab'}, ["foo=bar\r\n", "quuz=zaab\r\n"]),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test___init__(self, extra_specs_dict_input, extra_specs_str_output):
|
||||
form = self._get_form({'extra_specs': extra_specs_dict_input})
|
||||
|
||||
for expected_extra_spec in extra_specs_str_output:
|
||||
self.assertIn(expected_extra_spec, form.initial['extra_specs'])
|
||||
self.assertIn('extra_specs', list(form.fields.keys()))
|
||||
self.assertTrue(
|
||||
isinstance(form.fields['extra_specs'], horizon_forms.CharField))
|
||||
|
||||
@mock.patch('horizon.messages.success')
|
||||
def test_handle_success_no_changes(self, mock_horizon_messages_success):
|
||||
initial = {'id': 'fake_id', 'name': 'fake_name', 'extra_specs': {}}
|
||||
form = self._get_form(initial)
|
||||
data = {'extra_specs': ''}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertTrue(result)
|
||||
mock_horizon_messages_success.assert_called_once_with(
|
||||
mock.ANY, mock.ANY)
|
||||
|
||||
@mock.patch('horizon.messages.success')
|
||||
def test_handle_success_only_set(self, mock_horizon_messages_success):
|
||||
initial = {
|
||||
'id': 'fake_id',
|
||||
'name': 'fake_name',
|
||||
'extra_specs': {'foo': 'bar'}
|
||||
}
|
||||
form = self._get_form(initial)
|
||||
data = {'extra_specs': 'foo=bar\r\n'}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertTrue(result)
|
||||
mock_horizon_messages_success.assert_called_once_with(
|
||||
mock.ANY, mock.ANY)
|
||||
self.manilaclient.share_types.get.assert_called_once_with(
|
||||
initial['id'])
|
||||
self.manilaclient.share_types.get.return_value.set_keys.\
|
||||
assert_called_once_with({'foo': 'bar'})
|
||||
self.assertFalse(
|
||||
self.manilaclient.share_types.get.return_value.unset_keys.called)
|
||||
|
||||
@mock.patch('horizon.messages.success')
|
||||
def test_handle_success_only_unset(self, mock_horizon_messages_success):
|
||||
initial = {
|
||||
'id': 'fake_id',
|
||||
'name': 'fake_name',
|
||||
'extra_specs': {'foo': 'bar'}
|
||||
}
|
||||
form = self._get_form(initial)
|
||||
data = {'extra_specs': 'foo\r\n'}
|
||||
share_types_get = self.manilaclient.share_types.get
|
||||
share_types_get.return_value.get_keys.return_value = {
|
||||
'foo': 'bar', 'quuz': 'zaab'}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertTrue(result)
|
||||
mock_horizon_messages_success.assert_called_once_with(
|
||||
mock.ANY, mock.ANY)
|
||||
self.manilaclient.share_types.get.assert_has_calls([
|
||||
mock.call(initial['id'])])
|
||||
share_types_get.return_value.get_keys.assert_called_once_with()
|
||||
self.assertFalse(share_types_get.return_value.set_keys.called)
|
||||
share_types_get.return_value.unset_keys.assert_called_once_with(
|
||||
{'foo'})
|
||||
|
||||
@mock.patch('horizon.messages.success')
|
||||
def test_handle_success_set_and_unset(self, mock_horizon_messages_success):
|
||||
initial = {
|
||||
'id': 'fake_id',
|
||||
'name': 'fake_name',
|
||||
'extra_specs': {'foo': 'bar'}
|
||||
}
|
||||
form = self._get_form(initial)
|
||||
data = {'extra_specs': 'foo\r\nquuz=zaab'}
|
||||
share_types_get = self.manilaclient.share_types.get
|
||||
share_types_get.return_value.get_keys.return_value = {'foo': 'bar'}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertTrue(result)
|
||||
mock_horizon_messages_success.assert_called_once_with(
|
||||
mock.ANY, mock.ANY)
|
||||
self.manilaclient.share_types.get.assert_has_calls([
|
||||
mock.call(initial['id'])])
|
||||
share_types_get.return_value.get_keys.assert_called_once_with()
|
||||
share_types_get.return_value.set_keys.assert_called_once_with(
|
||||
{'quuz': 'zaab'})
|
||||
share_types_get.return_value.unset_keys.assert_called_once_with(
|
||||
{'foo'})
|
||||
|
||||
def test_handle_validation_error(self):
|
||||
initial = {'id': 'fake_id', 'name': 'fake_name', 'extra_specs': {}}
|
||||
form = self._get_form(initial)
|
||||
form.api_error = mock.Mock()
|
||||
data = {'extra_specs': 'a b'}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertFalse(result)
|
||||
form.api_error.assert_called_once_with(mock.ANY)
|
||||
|
||||
@mock.patch('horizon.exceptions.handle')
|
||||
def test_handle_other_exception(self, mock_horizon_exceptions_handle):
|
||||
django_forms.ValidationError
|
||||
initial = {'id': 'fake_id', 'name': 'fake_name', 'extra_specs': {}}
|
||||
form = self._get_form(initial)
|
||||
data = {'extra_specs': None}
|
||||
|
||||
result = form.handle(self.request, data)
|
||||
|
||||
self.assertFalse(result)
|
||||
mock_horizon_exceptions_handle.assert_called_once_with(
|
||||
self.request, mock.ANY)
|
||||
|
|
@ -0,0 +1,63 @@
|
|||
# Copyright (c) 2015 Mirantis, Inc.
|
||||
# All rights reserved.
|
||||
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import ddt
|
||||
from django.forms import ValidationError # noqa
|
||||
|
||||
from manila_ui.dashboards import utils
|
||||
from manila_ui.test import helpers as base
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class ManilaDashboardsUtilsTests(base.TestCase):
|
||||
|
||||
@ddt.data(
|
||||
("", {}, []),
|
||||
(" ", {}, []),
|
||||
("\n", {}, []),
|
||||
("f", {}, ["f"]),
|
||||
("f=b", {"f": "b"}, []),
|
||||
("foo=bar", {"foo": "bar"}, []),
|
||||
("\nfoo \n", {}, ["foo"]),
|
||||
("'foo'=\"bar\"\n'bar'", {"foo": "bar"}, ["bar"]),
|
||||
(" foo= bar ", {"foo": "bar"}, []),
|
||||
("foo= \"<is> bar\"\n", {"foo": "<is> bar"}, []),
|
||||
("\n\nset_me_key = 'value with spaces and equality 2=2'\nunset_key ",
|
||||
{"set_me_key": "value with spaces and equality 2=2"},
|
||||
["unset_key"]),
|
||||
("f" * 255, {}, ["f" * 255]),
|
||||
("f" * 255 + "=" + "b" * 255, {"f" * 255: "b" * 255}, []),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_parse_str_meta_success(
|
||||
self, input_data, expect_set_dict, expected_unset_list):
|
||||
set_dict, unset_list = utils.parse_str_meta(input_data)
|
||||
|
||||
self.assertEqual(expect_set_dict, set_dict)
|
||||
self.assertEqual(expected_unset_list, unset_list)
|
||||
|
||||
@ddt.data(
|
||||
"a b",
|
||||
"'a b'",
|
||||
"\"a b\"",
|
||||
"f" * 256,
|
||||
"f" * 256 + "=bar",
|
||||
"foo=" + "b" * 256,
|
||||
"\"a b \"",
|
||||
"foo=bar\nfoo",
|
||||
"foo=bar\nfoo=quuz",
|
||||
)
|
||||
def test_parse_str_meta_validation_error(self, input_data):
|
||||
self.assertRaises(ValidationError, utils.parse_str_meta, input_data)
|
||||
|
|
@ -42,15 +42,9 @@ class BaseAdminViewTests(ManilaTestsMixin, helpers.BaseAdminViewTests):
|
|||
|
||||
|
||||
class APITestCase(ManilaTestsMixin, helpers.APITestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(APITestCase, self).setUp()
|
||||
|
||||
self.manilaclient = mock.Mock()
|
||||
self._original_manilaclient = api.manila.manilaclient
|
||||
api.manila.manilaclient = lambda request: self.stub_manilaclient()
|
||||
|
||||
def stub_manilaclient(self):
|
||||
# NOTE(u_glide): Create empty mock if test case doesn't have
|
||||
# predefined manilaclient
|
||||
if not hasattr(self, "manilaclient"):
|
||||
self.manilaclient = mock.Mock()
|
||||
return self.manilaclient
|
||||
api.manila.manilaclient = lambda request: self.manilaclient
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
http://tarballs.openstack.org/horizon/horizon-master.tar.gz#egg=horizon
|
||||
hacking<0.11,>=0.10.0
|
||||
coverage>=3.6
|
||||
ddt>=0.7.0
|
||||
django-nose
|
||||
discover
|
||||
mock>=1.0
|
||||
|
|
|
|||
Loading…
Reference in New Issue