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

Hi Mark, Thanks for adding tests to it. I will integrate those patches to the refactor model ones - so we avoid conflicts and re-work while merging

On 01/23/2014 03:41 AM, Mark Wu wrote: > From: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > > 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 | 23 ++++++++++ > plugins/sample/model.py | 109 > ++++++++++++++++++++++++++++++------------------ > src/kimchi/model.py | 26 ++++++++++++ > 3 files changed, 117 insertions(+), 41 deletions(-) > > diff --git a/plugins/__init__.py b/plugins/__init__.py > index e69de29..4103a65 100644 > --- a/plugins/__init__.py > +++ b/plugins/__init__.py > @@ -0,0 +1,23 @@ > +# > +# 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 > + > + > diff --git a/plugins/sample/model.py b/plugins/sample/model.py > index 9a2f22f..e606176 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.model 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): > diff --git a/src/kimchi/model.py b/src/kimchi/model.py > index ea528c6..47f2d22 100644 > --- a/src/kimchi/model.py > +++ b/src/kimchi/model.py > @@ -1853,3 +1853,29 @@ class LibvirtConnection(object): > # However the values need to be considered wisely > to not affect > # hosts which are hosting a lot of virtual machines > return conn > + > + > +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)