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

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(a)linux.vnet.ibm.com&gt; > > 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(a)linux.vnet.ibm.com&gt; >> --- >> 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(a)linux.vnet.ibm.com&gt; >> +# >> +# 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(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel