openstack
/
fuel-web
Code Issues Proposed changes

Removed recursive observing MutableObject hierarchy 

The observing the whole hierarchy of MutableObject is not
trivial and the code is unmaintainable. Also it causes the write
data to database after each call of constructor.
It is better to notify about changes in second and deeper level explicitly.

Change-Id: Iebf09a944e23e0ebb8cb4f34f75a4188279b10b0
Closes-Bug: 1554229
This commit is contained in:
Bulat Gaifullin 2016-03-08 01:46:01 +03:00 committed by Igor Kalnitsky
parent 73dcf257d1
commit 1d2908e62e
17 changed files with 74 additions and 291 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nailgun/nailgun/db/sqlalchemy/models/action_logs.py
View File

@ -16,12 +16,12 @@ from sqlalchemy import Boolean
from sqlalchemy import Column
from sqlalchemy import DateTime
from sqlalchemy import Enum
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import Integer
from sqlalchemy import String
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun import consts

2
nailgun/nailgun/db/sqlalchemy/models/base.py
View File

@ -23,11 +23,11 @@ from sqlalchemy import DateTime
from sqlalchemy import Integer
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm.base import object_state
from nailgun.db import deadlock_detector as dd
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.settings import settings

2
nailgun/nailgun/db/sqlalchemy/models/cluster.py
View File

@ -24,7 +24,6 @@ from sqlalchemy import Text
from sqlalchemy import UnicodeText
from sqlalchemy.dialects import postgresql as psql
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import backref
from sqlalchemy.orm import relationship
@ -35,6 +34,7 @@ from nailgun import consts
from nailgun.db import db
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.db.sqlalchemy.models.mutable import MutableList
from nailgun.db.sqlalchemy.models.node import Node

2
nailgun/nailgun/db/sqlalchemy/models/master_node_settings.py
View File

@ -14,11 +14,11 @@
from sqlalchemy import Column
from sqlalchemy.dialects.postgresql import JSON
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import Integer
from sqlalchemy import String
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.mutable import MutableDict
class MasterNodeSettings(Base):

184
nailgun/nailgun/db/sqlalchemy/models/mutable.py
View File

@ -15,85 +15,47 @@
# under the License.
import copy
import six
from sqlalchemy.ext.mutable import Mutable as MutableBase
# TODO(vkaplov) Starting to use native sqlalchemy mutable types after bug fix
# https://bitbucket.org/zzzeek/sqlalchemy/issues/3646
class Mutable(MutableBase):
def __init__(self, *args, **kwargs):
super(Mutable, self).__init__(*args, **kwargs)
self.__setstate__(self)
def __setstate__(self, _):
"""Set state of object.
By default do nothing
:param _: new state of object
:return: None
"""
pass
@classmethod
def _coerce(cls, key, value, coerce_type):
"""Convert plain dictionaries to MutableDict.
:param key: string name of the ORM-mapped attribute being set.
:param value: the incoming value.
:param coerce_type: type, that should be coerced
:return: the method should return the coerced value
:raises ValueError: if the coercion cannot be completed.
"""
if not isinstance(value, cls):
if isinstance(value, coerce_type):
return cls(value)
# this call will raise ValueError
return MutableBase.coerce(key, value)
return value
@staticmethod
def mutable_cast_map():
yield dict, MutableDict
yield list, MutableList
def cast(self, value):
"""Cast type of value.
If value of MutableBase type, then just update the '_parents'.
If value of type from 'mutable_cast_map', then convert value to
corresponding mutable type and assign the '_parents'.
In all other cases return value without changes.
:param value: value to be casted
:return: casted value
"""
if isinstance(value, MutableBase):
value._parents = self._parents
return value
for original, mutable in self.mutable_cast_map():
if isinstance(value, original):
value = mutable(value)
value._parents = self._parents
return value
return value
class MutableCollection(MutableBase):
@classmethod
def __copy__(cls, value):
"""Create and return deepcopy of value.
Because of consistency we should to deepcopy Mutables in tree.
Because of consistency we should to deepcopy Mutable in tree.
Otherwise depending on level in tree Mutable will reference to
different structures in database.
:param value: subclass of Mutable object
"""
return copy.deepcopy(value)
def mark_dirty(self):
"""Alias for method changed."""
self.changed()
class MutableDict(Mutable, dict):
def __setitem__(self, key, value):
super(MutableCollection, self).__setitem__(key, value)
self.changed()
def __delitem__(self, key):
super(MutableCollection, self).__delitem__(key)
self.changed()
def clear(self):
super(MutableCollection, self).clear()
self.changed()
def pop(self, *args):
result = super(MutableCollection, self).pop(*args)
self.changed()
return result
class MutableDict(MutableCollection, dict):
@classmethod
def coerce(cls, key, value):
"""Convert plain dictionaries to MutableDict.
@ -102,16 +64,12 @@ class MutableDict(Mutable, dict):
:param value: the incoming value.
:return: the method should return the coerced value
"""
return cls._coerce(key, value, dict)
def __setitem__(self, key, value):
"""Detect dict set events and emit change events.
:param key: key of element
:param value: new value of element
"""
dict.__setitem__(self, key, self.cast(value))
self.changed()
if not isinstance(value, cls):
if isinstance(value, dict):
return cls(value)
# this call will raise ValueError
return MutableBase.coerce(key, value)
return value
def setdefault(self, key, default=None):
"""Detect dict setdefault events and emit change events.
@ -120,17 +78,11 @@ class MutableDict(Mutable, dict):
:param default: default value.
:return: if key is in the dictionary return value, otherwise default.
"""
result = dict.setdefault(self, key, self.cast(default))
self.changed()
return result
def __delitem__(self, key):
"""Detect dictionary del events and emit change events.
:param key: key in the dictionary
"""
dict.__delitem__(self, key)
self.changed()
try:
return self[key]
except KeyError:
self[key] = default
return default
def update(self, *args, **kwargs):
"""Detect dictionary update events and emit change events.
@ -138,13 +90,7 @@ class MutableDict(Mutable, dict):
:param args: accept the same args as builtin dict
:param kwargs: accept the same kwargs as builtin dict
"""
tmp = dict(*args, **kwargs)
dict.update(self, {k: self.cast(v) for k, v in six.iteritems(tmp)})
self.changed()
def clear(self):
"""Detect dictionary clear events and emit change events."""
dict.clear(self)
dict.update(self, *args, **kwargs)
self.changed()
def __getstate__(self):
@ -161,17 +107,6 @@ class MutableDict(Mutable, dict):
"""
self.update(state)
def pop(self, *args):
"""Pop element and emit change events if item removed.
:param args: (key, default) 'default' is optional
:return: element
:raises KeyError: if dict is empty or element not found.
"""
result = dict.pop(self, *args)
self.changed()
return result
def popitem(self):
"""Pop arbitrary element and emit change events if item removed.
@ -187,7 +122,7 @@ class MutableDict(Mutable, dict):
return MutableDict({k: _deepcopy(v, memo) for k, v in self.items()})
class MutableList(Mutable, list):
class MutableList(MutableCollection, list):
@classmethod
def coerce(cls, key, value):
"""Convert plain lists to MutableList.
@ -196,14 +131,19 @@ class MutableList(Mutable, list):
:param value: the incoming value.
:return: the method should return the coerced value
"""
return cls._coerce(key, value, list)
if not isinstance(value, cls):
if isinstance(value, list):
return cls(value)
# this call will raise ValueError
return MutableBase.coerce(key, value)
return value
def append(self, value):
"""Append value to end and emit change events.
:param value: element value
"""
list.append(self, self.cast(value))
list.append(self, value)
self.changed()
def extend(self, iterable):
@ -211,8 +151,7 @@ class MutableList(Mutable, list):
:param iterable: iterable on elements
"""
casted_elements = (self.cast(value) for value in iterable)
list.extend(self, casted_elements)
list.extend(self, iterable)
self.changed()
def insert(self, index, value):
@ -221,20 +160,9 @@ class MutableList(Mutable, list):
:param index: index of next element
:param value: value of inserting element
"""
list.insert(self, index, self.cast(value))
list.insert(self, index, value)
self.changed()
def pop(self, index=-1):
"""Pop element and emit change events if item removed.
:param index: index of element (default last)
:return: element
:raises IndexError: if list is empty or index is out of range.
"""
result = list.pop(self, index)
self.changed()
return result
def remove(self, value):
"""Remove first occurrence of value.
@ -246,23 +174,6 @@ class MutableList(Mutable, list):
list.remove(self, value)
self.changed()
def __setitem__(self, key, value):
"""Detect list set events and emit change events.
:param key: index of element
:param value: new value of element
"""
list.__setitem__(self, key, self.cast(value))
self.changed()
def __delitem__(self, key):
"""Detect list del events and emit change events.
:param key: index of element
"""
list.__delitem__(self, key)
self.changed()
def __getstate__(self):
"""Get state as builtin list
@ -284,8 +195,7 @@ class MutableList(Mutable, list):
:param last: last (not included) element index
:param sequence: elements that to be inserted
"""
casted_sequence = (self.cast(value) for value in sequence)
list.__setslice__(self, first, last, casted_sequence)
list.__setslice__(self, first, last, sequence)
self.changed()
def __delslice__(self, first, last):

2
nailgun/nailgun/db/sqlalchemy/models/network.py
View File

@ -16,7 +16,6 @@
from sqlalchemy import Boolean
from sqlalchemy import Column
from sqlalchemy.dialects import postgresql as psql
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import ForeignKey
from sqlalchemy import Integer
from sqlalchemy.orm import relationship
@ -25,6 +24,7 @@ from sqlalchemy import String
from nailgun import consts
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
class IPAddr(Base):

2
nailgun/nailgun/db/sqlalchemy/models/network_config.py
View File

@ -17,7 +17,6 @@
from sqlalchemy import Column
from sqlalchemy.dialects import postgresql as psql
from sqlalchemy import Enum
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import ForeignKey
from sqlalchemy import Integer
from sqlalchemy import String
@ -25,6 +24,7 @@ from sqlalchemy import String
from nailgun import consts
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.db.sqlalchemy.models.mutable import MutableList

2
nailgun/nailgun/db/sqlalchemy/models/node.py
View File

@ -28,12 +28,12 @@ from sqlalchemy import Unicode
from sqlalchemy import UniqueConstraint
from sqlalchemy.dialects import postgresql as psql
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import relationship
from nailgun import consts
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.db.sqlalchemy.models.mutable import MutableList
from nailgun.db.sqlalchemy.models.network import NetworkBondAssignment
from nailgun.db.sqlalchemy.models.network import NetworkNICAssignment

2
nailgun/nailgun/db/sqlalchemy/models/openstack_config.py
View File

@ -19,7 +19,6 @@ from sqlalchemy import Boolean
from sqlalchemy import Column
from sqlalchemy import DateTime
from sqlalchemy import Enum
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import ForeignKey
from sqlalchemy import Integer
from sqlalchemy import String
@ -27,6 +26,7 @@ from sqlalchemy import String
from nailgun import consts
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
class OpenstackConfig(Base):

2
nailgun/nailgun/db/sqlalchemy/models/oswl.py
View File

@ -18,7 +18,6 @@ from sqlalchemy import Column
from sqlalchemy import Date
from sqlalchemy.dialects import postgresql
from sqlalchemy import Enum
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import Integer
from sqlalchemy import Text
from sqlalchemy import Time
@ -26,6 +25,7 @@ from sqlalchemy import UniqueConstraint
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun import consts

2
nailgun/nailgun/db/sqlalchemy/models/plugins.py
View File

@ -14,7 +14,6 @@
from sqlalchemy import Boolean
from sqlalchemy import Column
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import ForeignKey
from sqlalchemy import Integer
from sqlalchemy import String
@ -25,6 +24,7 @@ from sqlalchemy.orm import relationship
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.db.sqlalchemy.models.mutable import MutableList

2
nailgun/nailgun/db/sqlalchemy/models/task.py
View File

@ -18,7 +18,6 @@ import uuid
from sqlalchemy import Column
from sqlalchemy import Enum
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy import Float
from sqlalchemy import ForeignKey
from sqlalchemy import Integer
@ -30,6 +29,7 @@ from nailgun import consts
from nailgun.db import db
from nailgun.db.sqlalchemy.models.base import Base
from nailgun.db.sqlalchemy.models.fields import JSON
from nailgun.db.sqlalchemy.models.mutable import MutableDict
class Task(Base):

2
nailgun/nailgun/objects/cluster.py
View File

@ -1172,6 +1172,8 @@ class Cluster(NailgunObject):
for vm in node.vms_conf:
if not vm.get('created'):
vm['created'] = True
# notify about changes
node.vms_conf.changed()
db().flush()
@classmethod

1
nailgun/nailgun/objects/node.py
View File

@ -1161,6 +1161,7 @@ class Node(NailgunObject):
for vm in node.vms_conf:
vm['created'] = False
node.vms_conf.changed()
@classmethod
def set_default_attributes(cls, instance):

4
nailgun/nailgun/objects/release.py
View File

@ -95,11 +95,15 @@ class Release(NailgunObject):
instance.roles_metadata[role['name']] = role['meta']
instance.volumes_metadata['volumes_roles_mapping'][role['name']] = \
role.get('volumes_roles_mapping', [])
# notify about changes
instance.volumes_metadata.changed()
@classmethod
def remove_role(cls, instance, role_name):
result = instance.roles_metadata.pop(role_name, None)
instance.volumes_metadata['volumes_roles_mapping'].pop(role_name, None)
# notify about changes
instance.volumes_metadata.changed()
return bool(result)
@classmethod

1
nailgun/nailgun/test/base.py
View File

@ -471,6 +471,7 @@ class EnvironmentManager(object):
def disable_task_deploy(self, cluster):
cluster.attributes.editable['common']['task_deploy']['value'] = False
cluster.attributes.editable.changed()
self.db().flush()
def delete_node_group(self, ng_id, status_code=200, api=True):

151
nailgun/nailgun/test/unit/test_mutable.py
View File

@ -19,42 +19,11 @@ from copy import deepcopy
from mock import Mock
from mock import patch
from nailgun.db.sqlalchemy.models.mutable import Mutable
from nailgun.db.sqlalchemy.models.mutable import MutableDict
from nailgun.db.sqlalchemy.models.mutable import MutableList
from nailgun.test.base import BaseUnitTest
class TestMutable(BaseUnitTest):
def setUp(self):
self.parents = {'parent': 'key'}
self.mutable = Mutable()
self.mutable._parents = self.parents
def _check_cast(self, value, expected_type):
result = self.mutable.cast(value)
self.assertIsInstance(result, expected_type)
self.assertIs(result._parents, self.mutable._parents)
self.assertItemsEqual(result, value)
def test_cast_list(self):
self._check_cast([1, 2, 3], MutableList)
def test_cast_dict(self):
self._check_cast({'1': 1, '2': 2}, MutableDict)
def test_cast_mutable_list(self):
self._check_cast(MutableList([1, 2, 3]), MutableList)
def test_cast_mutable_dict(self):
self._check_cast(MutableDict({'1': 1, '2': 2}), MutableDict)
def test_cast_simple_type(self):
result = self.mutable.cast(1)
self.assertIsInstance(result, int)
self.assertEqual(result, 1)
@patch('sqlalchemy.ext.mutable.Mutable.coerce')
class TestMutableDictCoerce(BaseUnitTest):
def setUp(self):
@ -122,7 +91,7 @@ class TestMutableDict(TestMutableDictBase):
def test_initialize(self):
with patch('sqlalchemy.ext.mutable.Mutable.changed') as m_changed:
MutableDict(key1='value1', key2='value2')
self._assert_call_object_changed_once(m_changed)
self._assert_object_not_changed(m_changed)
def test_setitem(self):
self._check('__setitem__', '2', 2)
@ -138,11 +107,10 @@ class TestMutableDict(TestMutableDictBase):
def test_setdefault_with_default_dict(self):
with patch('sqlalchemy.ext.mutable.Mutable.changed') as m_changed:
self.mutable_obj.setdefault('2', dict())
# 'changed' method called twice:
# - during casting default dict to MutableDict for new object
# - in setdefault method for 'mutable_obj'
self.assertEqual(m_changed.call_count, 2)
self.mutable_obj.setdefault('num', 1)
self.mutable_obj.setdefault('num', 2)
self.assertEqual(m_changed.call_count, 1)
self.assertEqual(1, self.mutable_obj['num'])
def test_setdefault_failure(self):
self._check_failure(TypeError, 'setdefault', {})
@ -207,11 +175,7 @@ class TestMutableDictIntegration(TestMutableDictBase):
m_changed.reset_mock()
clone = deepcopy(self.mutable_obj)
# changed should calls two times
# - root cloned dict (clone)
# - mutable dict element in root dict (cloned dct)
self.assertEqual(m_changed.call_count, 2)
self.assertEqual(0, m_changed.call_count)
dct['1'] = 'new_element'
self.assertEqual(clone['2']['1'], 'element1')
self.assertEqual(self.mutable_obj['2']['1'], 'new_element')
@ -256,7 +220,7 @@ class TestMutableList(TestMutableListBase):
def test_initialize(self):
with patch('sqlalchemy.ext.mutable.Mutable.changed') as m_changed:
MutableList([1, 2, 3])
self._assert_call_object_changed_once(m_changed)
self._assert_object_not_changed(m_changed)
def test_append(self):
self._check('append', 'element')
@ -373,107 +337,8 @@ class TestMutableListIntegration(TestMutableListBase):
m_changed.reset_mock()
clone = deepcopy(self.mutable_obj)
# changed should calls two times
# - root cloned list (clone)
# - mutable list element in root list (cloned lst)
self.assertEqual(m_changed.call_count, 2)
self.assertEqual(0, m_changed.call_count)
lst[0] = 'new_element'
self.assertEqual(clone[0][0], 'element1')
self.assertEqual(self.mutable_obj[0][0], 'new_element')
class TestComplexDataStructures(BaseUnitTest):
def setUp(self):
lst = [
{'1': 1, '2': 2},
MutableDict({'1': 1, '2': 2}),
MutableList([1, 2, 3]),
'123'
]
# create list with three levels of nesting
complex_list = lst + [lst + [lst]]
self.mutable_list = MutableList(complex_list)
dct = {
'1': [1, 2],
'2': MutableList([1, 3]),
'3': MutableDict({'1': 1, '2': 2}),
'4': '123'
}
# create dict with three levels of nesting
complex_dict = dict(dct)
complex_dict.setdefault('5', copy(dct))
complex_dict['5'].setdefault('5', copy(dct))
self.mutable_dict = MutableDict(complex_dict)
self.lst = [1, [1, 2, 3], {'1': 1, '2': 2}]
self.dct = {'1': 1, '2': [1, 2, 3], '3': {'1': 1, '2': 2}}
def _validate_data(self, data):
"""Iterative data validation
Check that there're no builtin list or dict in data
:param data: data to validate
"""
elements = []
while True:
self.assertNotIn(type(data), (dict, list))
if isinstance(data, dict):
self.assertIsInstance(data, MutableDict)
elements.extend([value for key, value in data.items()])
if isinstance(data, list):
self.assertIsInstance(data, MutableList)
elements.extend(data)
if not elements:
break
data = elements.pop()
def _check(self, obj, method, *args, **kwargs):
getattr(obj, method)(*args, **kwargs)
self._validate_data(obj)
def test_append_to_first_level(self):
self._check(self.mutable_list, 'append', self.lst)
def test_append_to_second_level(self):
self._check(self.mutable_list[2], 'append', self.lst)
def test_append_to_third_level(self):
self._check(self.mutable_list[4][4], 'append', self.lst)
def test_extend_first_level(self):
self._check(self.mutable_list, 'extend', self.lst)
def test_extend_second_level(self):
self._check(self.mutable_list[2], 'extend', self.lst)
def test_third_third_level(self):
self._check(self.mutable_list[4][4], 'extend', self.lst)
def test_insert_to_first_level(self):
self._check(self.mutable_list, 'insert', 2, self.lst)
def test_insert_to_second_level(self):
self._check(self.mutable_list[2], 'insert', 2, self.lst)
def test_insert_to_third_level(self):
self._check(self.mutable_list[4][4], 'insert', 2, self.lst)
def test_setitem_to_first_level(self):
self._check(self.mutable_list, '__setitem__', 1, self.lst)
def test_setitem_to_second_level(self):
self._check(self.mutable_list[2], '__setitem__', 1, self.lst)
def test_setitem_to_third_level(self):
self._check(self.mutable_list[4][4], '__setitem__', 1, self.lst)
def test_setslice_to_first_level(self):
self._check(self.mutable_list, '__setslice__', 0, 3, self.lst)
def test_setslice_to_second_level(self):
self._check(self.mutable_list[2], '__setslice__', 0, 3, self.lst)
def test_setslice_to_third_level(self):
self._check(self.mutable_list[4][4], '__setslice__', 0, 3, self.lst)