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):

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