[Kimchi-devel] [PATCH] Break the 'sample' plugin's monolithic model into several smaller ones

Aline Manera alinefm at linux.vnet.ibm.com
Wed Jan 22 16:30:34 UTC 2014


The tests are failing.

======================================================================
FAIL: test_bad_params (test_plugin.PluginTests)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "test_plugin.py", line 119, in test_bad_params
     self.assertEquals(400, resp.status)
AssertionError: 400 != 404

======================================================================
FAIL: test_rectangles (test_plugin.PluginTests)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "test_plugin.py", line 92, in test_rectangles
     self._create_rectangle_and_assert('small', 10, 8)
   File "test_plugin.py", line 77, in _create_rectangle_and_assert
     self.assertEquals(201, resp.status)
AssertionError: 201 != 404


I will fix it and resend with model refactoring patch set as Zheng Sheng 
is on vacation these days.

On 01/22/2014 01:48 PM, Aline Manera wrote:
>
> Reviewed-by: Aline Manera <alinefm at linux.vnet.ibm.com>
>
> On 01/21/2014 05:46 AM, Zhou Zheng Sheng wrote:
>> In kimchi the model is always a monolithic class. This is not good from
>> the point of modulization, and it is not necessary to put everything
>> into one big model. Unfortunately the problem will not get solved
>> unless we conduct enormous refactor against the whole controller-model
>> architecture.
>>
>> This patch tries to mitigate the problem by firstly splitting the model
>> class into some smaller ones, than automatically mapping all sub-model
>> instance methods to a RootModel instance. As a demonstration, this patch
>> splits the 'sample' plugin's model.
>>
>> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>> ---
>>   plugins/__init__.py     |  47 +++++++++++++++++++++
>>   plugins/sample/model.py | 109 
>> ++++++++++++++++++++++++++++++------------------
>>   2 files changed, 115 insertions(+), 41 deletions(-)
>>
>> diff --git a/plugins/__init__.py b/plugins/__init__.py
>> index e69de29..f0b800c 100644
>> --- a/plugins/__init__.py
>> +++ b/plugins/__init__.py
>> @@ -0,0 +1,47 @@
>> +#
>> +# Project Kimchi
>> +#
>> +# Copyright IBM, Corp. 2014
>> +#
>> +# Authors:
>> +#  Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>> +#
>> +# This library is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU Lesser General Public
>> +# License as published by the Free Software Foundation; either
>> +# version 2.1 of the License, or (at your option) any later version.
>> +#
>> +# This library is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +# Lesser General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU Lesser General Public
>> +# License along with this library; if not, write to the Free Software
>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>> 02110-1301 USA
>> +
>> +
>> +class RootModel(object):
>> +    '''
>> +    This model squashes all sub-model's public callable methods to 
>> itself.
>> +
>> +    Model methods are not limited to get_list, create, lookup, 
>> update, delete.
>> +    Controller can call generate_action_handler to generate new 
>> actions, which
>> +    call the related model methods. So all public callable methods of a
>> +    sub-model should be mapped to this model.
>> +    '''
>> +    def __init__(self, model_instances):
>> +        for model_instance in model_instances:
>> +            cls_name = model_instance.__class__.__name__
>> +            if cls_name.endswith('Model'):
>> +                method_prefix = cls_name[:-len('Model')].lower()
>> +            else:
>> +                method_prefix = cls_name.lower()
>> +
>> +            callables = [m for m in dir(model_instance)
>> +                         if not m.startswith('_') and
>> +                         callable(getattr(model_instance, m))]
>> +
>> +            for member_name in callables:
>> +                m = getattr(model_instance, member_name, None)
>> +                setattr(self, '%s_%s' % (method_prefix, 
>> member_name), m)
>> diff --git a/plugins/sample/model.py b/plugins/sample/model.py
>> index 9a2f22f..d194e67 100644
>> --- a/plugins/sample/model.py
>> +++ b/plugins/sample/model.py
>> @@ -21,79 +21,106 @@
>>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> 02110-1301 USA
>>
>>   from kimchi.exception import InvalidOperation, NotFoundError
>> +from kimchi.plugins import RootModel
>>
>>
>> -class Model(object):
>> +class CirclesModel(object):
>> +    def __init__(self):
>> +        self._circles = {}
>> +
>> +    def create(self, params):
>> +        name = params['name']
>> +        if name in self._circles:
>> +            raise InvalidOperation("Circle %s already exists" % name)
>> +        self._circles[name] = Circle(params['radius'])
>> +        return name
>>
>> +    def get_list(self):
>> +        return sorted(self._circles)
>> +
>> +
>> +class CircleModel(object):
>> +    def __init__(self, parent_model):
>> +        # Circel and Circles models are friends, it's OK to share 
>> _circles.
>> +        self._circles = parent_model._circles
>> +
>> +    def lookup(self, name):
>> +        try:
>> +            circle = self._circles[name]
>> +        except KeyError:
>> +            raise NotFoundError("Circle %s not found" % name)
>> +        return {'radius': circle.radius}
>> +
>> +    def update(self, name, params):
>> +        if name not in self._circles:
>> +            raise NotFoundError("Circle %s not found" % name)
>> +        self._circles[name].radius = params['radius']
>> +        return name
>> +
>> +    def delete(self, name):
>> +        try:
>> +            del self._circles[name]
>> +        except KeyError:
>> +            pass
>> +
>> +
>> +class RectanglesModel(object):
>>       def __init__(self):
>> -        self.rectangles = {}
>> -        self.circles = {}
>> +        self._rectangles = {}
>>
>> -    def rectangles_create(self, params):
>> +    def create(self, params):
>>           name = params['name']
>> -        if name in self.rectangles:
>> +        if name in self._rectangles:
>>               raise InvalidOperation("Rectangle %s already exists" % 
>> name)
>> -        self.rectangles[name] = Rectangle(params['length'], 
>> params['width'])
>> +        self._rectangles[name] = Rectangle(params['length'], 
>> params['width'])
>>           return name
>>
>> -    def rectangles_get_list(self):
>> -        return sorted(self.rectangles)
>> +    def get_list(self):
>> +        return sorted(self._rectangles)
>> +
>>
>> -    def rectangle_lookup(self, name):
>> +class RectangleModel(object):
>> +    def __init__(self, parent_model):
>> +        self._rectangles = parent_model._rectangles
>> +
>> +    def lookup(self, name):
>>           try:
>> -            rectangle = self.rectangles[name]
>> +            rectangle = self._rectangles[name]
>>           except KeyError:
>>               raise NotFoundError("Rectangle %s not found" % name)
>>           return {'length': rectangle.length, 'width': rectangle.width}
>>
>> -    def rectangle_update(self, name, params):
>> -        if name not in self.rectangles:
>> +    def update(self, name, params):
>> +        if name not in self._rectangles:
>>               raise NotFoundError("Rectangle %s not found" % name)
>>           try:
>> -            self.rectangles[name].length = params['length']
>> +            self._rectangles[name].length = params['length']
>>           except KeyError:
>>               pass
>>
>>           try:
>> -            self.rectangles[name].width = params['width']
>> +            self._rectangles[name].width = params['width']
>>           except KeyError:
>>               pass
>>           return name
>>
>> -    def rectangle_delete(self, name):
>> +    def delete(self, name):
>>           try:
>> -            del self.rectangles[name]
>> +            del self._rectangles[name]
>>           except KeyError:
>>               pass
>>
>> -    def circles_create(self, params):
>> -        name = params['name']
>> -        if name in self.circles:
>> -            raise InvalidOperation("Circle %s already exists" % name)
>> -        self.circles[name] = Circle(params['radius'])
>> -        return name
>>
>> -    def circles_get_list(self):
>> -        return sorted(self.circles)
>> -
>> -    def circle_lookup(self, name):
>> -        try:
>> -            circle = self.circles[name]
>> -        except KeyError:
>> -            raise NotFoundError("Circle %s not found" % name)
>> -        return {'radius': circle.radius}
>> +class Model(RootModel):
>> +    def __init__(self):
>> +        circles = CirclesModel()
>> +        circle = CircleModel(circles)
>>
>> -    def circle_update(self, name, params):
>> -        if name not in self.circles:
>> -            raise NotFoundError("Circle %s not found" % name)
>> -        self.circles[name].radius = params['radius']
>> -        return name
>> +        rectangles = RectanglesModel()
>> +        rectangle = RectangleModel(rectangles)
>>
>> -    def circle_delete(self, name):
>> -        try:
>> -            del self.circles[name]
>> -        except KeyError:
>> -            pass
>> +        return super(Model, self).__init__(
>> +            [circle, circles, rectangle, rectangles])
>>
>>
>>   class Rectangle(object):
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>




More information about the Kimchi-devel mailing list