From e8fac5eda3309a19ee2e1b36883a4e30476644a9 Mon Sep 17 00:00:00 2001 From: Navneet Singh Date: Fri, 14 Feb 2014 21:21:33 +0530 Subject: [PATCH] NetApp api fix structure conversion methods This fixes the issue in api element creation when data structures are supplied to be converted into NaElements. It could not handle multiple keys with same name and also not converted different types properly into NaElement. This change fixes the issue. Change-Id: I84f1ee97a73bbad88bbb64d4a9ac4e1b29cad741 Closes-bug: #1287643 --- cinder/tests/test_netapp.py | 131 ++++++++++++++++++++++++++++ cinder/volume/drivers/netapp/api.py | 50 +++++++---- 2 files changed, 165 insertions(+), 16 deletions(-) diff --git a/cinder/tests/test_netapp.py b/cinder/tests/test_netapp.py index ab03e4c2264..6a8c655374a 100644 --- a/cinder/tests/test_netapp.py +++ b/cinder/tests/test_netapp.py @@ -28,6 +28,8 @@ from cinder import exception from cinder.openstack.common import log as logging from cinder import test from cinder.volume import configuration as conf +from cinder.volume.drivers.netapp.api import NaElement +from cinder.volume.drivers.netapp.api import NaServer from cinder.volume.drivers.netapp import common from cinder.volume.drivers.netapp.options import netapp_7mode_opts from cinder.volume.drivers.netapp.options import netapp_basicauth_opts @@ -1203,3 +1205,132 @@ class NetAppDirect7modeISCSIDriverTestCase_WV( configuration.netapp_server_port = '80' configuration.netapp_vfiler = 'openstack' return configuration + + +class NetAppApiElementTransTests(test.TestCase): + """Test case for NetApp api element translations.""" + + def setUp(self): + super(NetAppApiElementTransTests, self).setUp() + + def test_translate_struct_dict_unique_key(self): + """Tests if dict gets properly converted to NaElements.""" + root = NaElement('root') + child = {'e1': 'v1', 'e2': 'v2', 'e3': 'v3'} + root.translate_struct(child) + self.assertEqual(len(root.get_children()), 3) + self.assertEqual(root.get_child_content('e1'), 'v1') + self.assertEqual(root.get_child_content('e2'), 'v2') + self.assertEqual(root.get_child_content('e3'), 'v3') + + def test_translate_struct_dict_nonunique_key(self): + """Tests if list/dict gets properly converted to NaElements.""" + root = NaElement('root') + child = [{'e1': 'v1', 'e2': 'v2'}, {'e1': 'v3'}] + root.translate_struct(child) + self.assertEqual(len(root.get_children()), 3) + children = root.get_children() + for c in children: + if c.get_name() == 'e1': + self.assertIn(c.get_content(), ['v1', 'v3']) + else: + self.assertEqual(c.get_content(), 'v2') + + def test_translate_struct_list(self): + """Tests if list gets properly converted to NaElements.""" + root = NaElement('root') + child = ['e1', 'e2'] + root.translate_struct(child) + self.assertEqual(len(root.get_children()), 2) + self.assertIsNone(root.get_child_content('e1')) + self.assertIsNone(root.get_child_content('e2')) + + def test_translate_struct_tuple(self): + """Tests if tuple gets properly converted to NaElements.""" + root = NaElement('root') + child = ('e1', 'e2') + root.translate_struct(child) + self.assertEqual(len(root.get_children()), 2) + self.assertIsNone(root.get_child_content('e1')) + self.assertIsNone(root.get_child_content('e2')) + + def test_translate_invalid_struct(self): + """Tests if invalid data structure raises exception.""" + root = NaElement('root') + child = 'random child element' + self.assertRaises(ValueError, root.translate_struct, child) + + def test_setter_builtin_types(self): + """Tests str, int, float get converted to NaElement.""" + root = NaElement('root') + root['e1'] = 'v1' + root['e2'] = 1 + root['e3'] = 2.0 + root['e4'] = 8l + self.assertEqual(len(root.get_children()), 4) + self.assertEqual(root.get_child_content('e1'), 'v1') + self.assertEqual(root.get_child_content('e2'), '1') + self.assertEqual(root.get_child_content('e3'), '2.0') + self.assertEqual(root.get_child_content('e4'), '8') + + def test_setter_na_element(self): + """Tests na_element gets appended as child.""" + root = NaElement('root') + root['e1'] = NaElement('nested') + self.assertEqual(len(root.get_children()), 1) + e1 = root.get_child_by_name('e1') + self.assertIsInstance(e1, NaElement) + self.assertIsInstance(e1.get_child_by_name('nested'), NaElement) + + def test_setter_child_dict(self): + """Tests dict is appended as child to root.""" + root = NaElement('root') + root['d'] = {'e1': 'v1', 'e2': 'v2'} + e1 = root.get_child_by_name('d') + self.assertIsInstance(e1, NaElement) + sub_ch = e1.get_children() + self.assertEqual(len(sub_ch), 2) + for c in sub_ch: + self.assertIn(c.get_name(), ['e1', 'e2']) + if c.get_name() == 'e1': + self.assertEqual(c.get_content(), 'v1') + else: + self.assertEqual(c.get_content(), 'v2') + + def test_setter_child_list_tuple(self): + """Tests list/tuple are appended as child to root.""" + root = NaElement('root') + root['l'] = ['l1', 'l2'] + root['t'] = ('t1', 't2') + l = root.get_child_by_name('l') + self.assertIsInstance(l, NaElement) + t = root.get_child_by_name('t') + self.assertIsInstance(t, NaElement) + for le in l.get_children(): + self.assertIn(le.get_name(), ['l1', 'l2']) + for te in t.get_children(): + self.assertIn(te.get_name(), ['t1', 't2']) + + def test_setter_no_value(self): + """Tests key with None value.""" + root = NaElement('root') + root['k'] = None + self.assertIsNone(root.get_child_content('k')) + + def test_setter_invalid_value(self): + """Tests invalid value raises exception.""" + root = NaElement('root') + try: + root['k'] = NaServer('localhost') + except Exception as e: + if not isinstance(e, TypeError): + self.fail(_('Error not a TypeError.')) + + def test_setter_invalid_key(self): + """Tests invalid value raises exception.""" + root = NaElement('root') + try: + root[None] = 'value' + except Exception as e: + if not isinstance(e, KeyError): + self.fail(_('Error not a KeyError.')) diff --git a/cinder/volume/drivers/netapp/api.py b/cinder/volume/drivers/netapp/api.py index 640ec65556f..8e99b7e6270 100644 --- a/cinder/volume/drivers/netapp/api.py +++ b/cinder/volume/drivers/netapp/api.py @@ -419,20 +419,19 @@ class NaElement(object): raise KeyError(_('No element by given name %s.') % (key)) def __setitem__(self, key, value): - """Dict setter method for NaElement.""" + """Dict setter method for NaElement. + + Accepts dict, list, tuple, str, int, float and long as valid value. + """ if key: if value: if isinstance(value, NaElement): child = NaElement(key) child.add_child_elem(value) self.add_child_elem(child) - elif isinstance(value, str): - child = self.get_child_by_name(key) - if child: - child.set_content(value) - else: - self.add_new_child(key, value) - elif isinstance(value, dict): + elif isinstance(value, (str, int, float, long)): + self.add_new_child(key, str(value)) + elif isinstance(value, (list, tuple, dict)): child = NaElement(key) child.translate_struct(value) self.add_child_elem(child) @@ -446,19 +445,38 @@ class NaElement(object): def translate_struct(self, data_struct): """Convert list, tuple, dict to NaElement and appends. - Useful for NaElement queries which have unique - query parameters. + Example usage: + 1. + + vl1 + vl2 + vl3 + + The above can be achieved by doing + root = NaElement('root') + root.translate_struct({'elem1': 'vl1', 'elem2': 'vl2', + 'elem3': 'vl3'}) + 2. + + vl1 + vl2 + vl3 + + The above can be achieved by doing + root = NaElement('root') + root.translate_struct([{'elem1': 'vl1', 'elem2': 'vl2'}, + {'elem1': 'vl3'}]) """ - - if isinstance(data_struct, list) or isinstance(data_struct, tuple): + if isinstance(data_struct, (list, tuple)): for el in data_struct: - self.add_child_elem(NaElement(el)) + if isinstance(el, (list, tuple, dict)): + self.translate_struct(el) + else: + self.add_child_elem(NaElement(el)) elif isinstance(data_struct, dict): for k in data_struct.keys(): child = NaElement(k) - if (isinstance(data_struct[k], dict) or - isinstance(data_struct[k], list) or - isinstance(data_struct[k], tuple)): + if isinstance(data_struct[k], (dict, list, tuple)): child.translate_struct(data_struct[k]) else: if data_struct[k]: