[IMP] move stuff around, try building flatter top-down API with read_combined as utility 'do all the things' hook

bzr revid: xmo@openerp.com-20130424095114-4nw0nfuxzx4n1ltv

openerp/addons/base/ir/ir_ui_view.py

@@ -165,34 +165,47 @@ class view(osv.osv):
         if not cr.fetchone():
             cr.execute('CREATE INDEX ir_ui_view_model_type_inherit_id ON ir_ui_view (model, inherit_id)')
 
-    def read_combined(self, cr, uid, view_id, fields=None, model=None, context=None):
+    def read_combined(self, cr, uid, view_id, view_type, model,
+                      fields=None, context=None):
         """
-        Reads the specified view and returns it after applying all inheriting
+        Utility function stringing together all method calls necessary to get
-        views upon it (essentially reads the "currently final" view)
+        a full, final view:
 
-        Returns `False` if the provided id is not a valid view, or if `False`
+        * Gets the default view if no view_id is provided
-        is provided as an id.
+        * Gets the top of the view tree if a sub-view is requested
+        * Applies all inherited archs on the root view
+        * Applies post-processing both generic (ir.ui.view) and specific
+          (ir.ui.view.$type)
+        * Returns the view with all requested fields
 
-        The `arch` of the view is always read (regardless of its presence in
+          .. note:: ``arch`` is always added to the fields list even if not
-        `fields`), and is returned as an lxml tree
+                    requested (similar to ``id``)
 
-        :param list(str) fields: same as in BaseModel.read()
+        If no view is available (no view_id or invalid view_id provided, or
-        :param str model:
+        no view stored for (model, view_type)) a view record will be fetched
+        from the ``defaults`` mapping.
         """
+        if not view_id:
+            view_id = self.default_view(cr, uid, model, view_type, context=context)
+        root_id = self.root_ancestor(cr, uid, view_id, context=context)
+
         if fields and 'arch' not in fields:
             fields = list(itertools.chain(['arch'], fields))
-        if not view_id: return False
-        views = self.read(cr, uid, [view_id], fields=fields, context=context)
-        if not views: return False
 
-        base_arch = views[0]['arch']
+        [view] = self.read(cr, uid, [root_id], fields=fields, context=context)
-        base_arch = base_arch.encode('utf-8') if isinstance(base_arch, unicode) else base_arch
+
-        return dict(views[0], arch=reduce(
+        arch_tree = etree.fromstring(
-            lambda current_arch, descendant: self.apply_inheritance_specs(
+            view['arch'].encode('utf-8') if isinstance(view['arch'], unicode)
-                cr, uid, model, view_id, current_arch, *descendant, context=context),
+            else view['arch'])
-            self.iter(cr, uid, view_id, model, exclude_base=True, context=context),
+        descendants = self.iter(
-            etree.fromstring(base_arch)
+            cr, uid, view['id'], model, exclude_base=True, context=context)
-        ))
+        arch = self.apply_inherited_archs(
+            cr, uid, arch_tree, descendants,
+            model, view['id'], context=context)
+
+        # TODO: post-processing
+
+        return dict(view, arch=arch)#etree.tostring(arch, 'utf-8'))
 
     def locate_node(self, arch, spec):
         """ Locate a node in a source (parent) architecture.
@@ -262,18 +275,18 @@ class view(osv.osv):
                 if not (view.groups_id and user_groups.isdisjoint(view.groups_id))]
 
     def iter(self, cr, uid, view_id, model, exclude_base=False, context=None):
-        """ iterates on all of `view_id`'s descendants tree depth-first.
+        """ iterates on all of ``view_id``'s descendants tree depth-first.
 
-        If `exclude_base` is `False`, also yields `view_id` itself. It is
+        If ``exclude_base`` is ``False``, also yields ``view_id`` itself. It is
-        `False` by default to match the behavior of etree's Element.iter.
+        ``False`` by default to match the behavior of etree's Element.iter.
 
         :param int view_id: database id of the root view
         :param str model: name of the view's related model (for filtering)
-        :param boolean exclude_base: whether `view_id` should be excluded from the iteration
+        :param bool exclude_base: whether ``view_id`` should be excluded
-        :param context:
+                                  from the iteration
-        :return: iterator of (database_id, arch_field) pairs for all
+        :return: iterator of (database_id, arch_string) pairs for all
-                 descendants of `view_id` (including `view_id` itself if
+                 descendants of ``view_id`` (including ``view_id`` itself if
-                 `exclude_base` is `False`, the default)
+                 ``exclude_base`` is ``False``, the default)
         """
         if not exclude_base:
             base = self.browse(cr, uid, view_id, context=context)
@@ -286,36 +299,32 @@ class view(osv.osv):
                     cr, uid, id, model, exclude_base=True, context=None):
                 yield info
 
-    def get_root_ancestor(self, cr, uid, view_id=None,
+    def default_view(self, cr, uid, model, view_type, context=None):
-                          model=None, view_type=None, context=None):
+        """ Fetches the default view for the provided (model, view_type) pair:
+         view with no parent (inherit_id=Fase) with the lowest priority.
+
+        :param str model:
+        :param int view_type:
+        :return: id of the default view for the (model, view_type) pair
+        :rtype: int
         """
-        Fetches the id of the root of the view tree specified by the id or
+        return self.search(cr, uid, [
-        (type, model) parameters.
+            ['model', '=', model],
+            ['type', '=', view_type],
+            ['inherit_id', '=', False],
+        ], limit=1, order='priority', context=context)[0]
+
+    def root_ancestor(self, cr, uid, view_id, context=None):
+        """
+        Fetches the id of the root of the view tree of which view_id is part
 
         If view_id is specified, view_type and model aren't needed (and the
         other way around)
 
         :param view_id: id of view to search the root ancestor of
-        :param str model: model to use the view for
-        :param str view_type: expected view type
         :return: id of the root view for the tree
         """
-        assert view_id or (model and view_type),\
-            "caller must provide either a view_id or a model and a view_type"\
-            " to be able to fetch a root view"
-
-        if not view_id:
-            ids = self.search(cr, uid, [
-                ['model', '=', model],
-                ['type', '=', view_type],
-                ['inherit_id', '=', False],
-            ], limit=1, order='priority', context=context)[:1]
-            if not ids: return False
-            [view_id] = ids
-
         view = self.browse(cr, uid, view_id, context=context)
-        if not view.exists():
-            return False
 
         # Search for a root (i.e. without any parent) view.
         while view.inherit_id:
@@ -323,6 +332,25 @@ class view(osv.osv):
 
         return view.id
 
+    def apply_inherited_archs(self, cr, uid, source, descendants,
+                              model, source_view_id, context=None):
+        """ Applies descendants to the ``source`` view, returns the result of
+        the application.
+
+        :param Element source: source arch to apply descendant on
+        :param descendants: iterable of (id, arch_string) pairs of all
+                            descendants in the view tree, depth-first,
+                            excluding the base view
+        :type descendants: iter((int, str))
+        :return: new architecture etree produced by applying all descendants
+                 on ``source``
+        :rtype: Element
+        """
+        return reduce(
+            lambda current_arch, descendant: self.apply_inheritance_specs(
+                cr, uid, model, source_view_id, current_arch,
+                *descendant, context=context),
+            descendants, source)
 
     def raise_view_error(self, cr, uid, model, error_msg, view_id, child_view_id, context=None):
         view, child_view = self.browse(cr, uid, [view_id, child_view_id], context)
@@ -337,11 +365,11 @@ class view(osv.osv):
         describing where and what changes to apply to some parent
         architecture) given by an inheriting view.
 
-        :param source: a parent architecture to modify
+        :param Element source: a parent architecture to modify
         :param descendant_id: the database id of the descendant
         :param specs_arch: a modifying architecture in an inheriting view
         :return: a modified source where the specs are applied
-
+        :rtype: Element
         """
         if isinstance(specs_arch, unicode):
             specs_arch = specs_arch.encode('utf-8')

openerp/osv/orm.py

@@ -2065,11 +2065,10 @@ class BaseModel(object):
             if view_ref_res:
                 view_id = view_ref_res[0]
 
-        root_view_id = View.get_root_ancestor(
+        root_view = View.read_combined(
-            cr, user, view_id, self._name, view_type, context=context)
+            cr, user, view_id, view_type, self._name, fields=[
-        root_view = View.read_combined(cr, user, root_view_id, fields=[
+                'id', 'name', 'field_parent', 'type', 'model', 'arch'
-            'id', 'name', 'field_parent', 'type', 'model', 'arch'
+            ], context=context)
-        ], model=self._name, context=context)
         if root_view:
             result.update(
                 arch=root_view['arch'],

openerp/tests/test_views.py

@@ -2,6 +2,7 @@ from lxml import etree as ET
 from lxml.builder import E
 
 from . import common
+import unittest2
 
 Field = E.field
 
@@ -96,14 +97,14 @@ class TestNodeLocator(common.BaseCase):
         self.assertIsNone(node)
 
 class TestViewInheritance(common.TransactionCase):
-    def view_for(self, name):
+    def arch_for(self, name, view_type='form'):
-        return ET.tostring(ET.Element('form', string=name))
+        return ET.tostring(ET.Element(view_type, string=name))
 
-    def makeView(self, name, parent=None):
+    def makeView(self, name, parent=None, arch=None):
         view_id = self.View.create(self.cr, self.uid, {
             'model': self.model,
             'name': name,
-            'arch': self.view_for(name),
+            'arch': arch or self.arch_for(name),
             'inherit_id': parent,
         })
         self.ids[name] = view_id
@@ -128,15 +129,20 @@ class TestViewInheritance(common.TransactionCase):
         a22 = self.makeView("A22", a2)
         self.makeView("A221", a22)
 
+        b = self.makeView('B', arch=self.arch_for("B", 'tree'))
+        self.makeView('B1', b, arch=self.arch_for("B1", 'tree'))
+        c = self.makeView('C', arch=self.arch_for("C", 'tree'))
+        self.View.write(self.cr, self.uid, c, {'priority': 1})
+
     def tearDown(self):
         self.View.pool._init = self._init
         super(TestViewInheritance, self).tearDown()
 
-    def test_get_children(self):
+    def test_get_inheriting_views_arch(self):
         self.assertEqual(self.View.get_inheriting_views_arch(
             self.cr, self.uid, self.ids['A'], self.model), [
-            (self.view_for('A1'), self.ids['A1']),
+            (self.arch_for('A1'), self.ids['A1']),
-            (self.view_for('A2'), self.ids['A2']),
+            (self.arch_for('A2'), self.ids['A2']),
         ])
 
         self.assertEqual(self.View.get_inheriting_views_arch(
@@ -145,49 +151,69 @@ class TestViewInheritance(common.TransactionCase):
 
         self.assertEqual(self.View.get_inheriting_views_arch(
             self.cr, self.uid, self.ids['A11'], self.model),
-            [(self.view_for('A111'), self.ids['A111'])])
+            [(self.arch_for('A111'), self.ids['A111'])])
 
-    def test_iterate_descendents(self):
+    def test_iter(self):
         descendents = list(self.View.iter(self.cr, self.uid, self.ids['A1'], self.model))
         self.assertEqual(descendents, [
-            (self.ids[name], self.view_for(name))
+            (self.ids[name], self.arch_for(name))
             for name in ['A1', 'A11', 'A111', 'A12']
         ])
         descendents = list(self.View.iter(
             self.cr, self.uid, self.ids['A2'], self.model, exclude_base=True))
         self.assertEqual(descendents, [
-            (self.ids[name], self.view_for(name))
+            (self.ids[name], self.arch_for(name))
             for name in ['A21', 'A22', 'A221']
         ])
 
-    def test_find_root(self):
+    def test_root_ancestor(self):
         A_id = self.ids['A']
-        root_id = self.View.get_root_ancestor(self.cr, self.uid, view_id=A_id)
+        root_id = self.View.root_ancestor(self.cr, self.uid, view_id=A_id)
         self.assertEqual(root_id, A_id,
              "when given a root view, operation should be id")
 
-        root_id = self.View.get_root_ancestor(
+        root_id = self.View.root_ancestor(
             self.cr, self.uid, view_id=self.ids['A11'])
         self.assertEqual(root_id, A_id)
 
-        root_id = self.View.get_root_ancestor(
+        root_id = self.View.root_ancestor(
             self.cr, self.uid, view_id=self.ids['A221'])
         self.assertEqual(root_id, A_id)
 
-        # search by model
+        root_id = self.View.root_ancestor(
-        root_id = self.View.get_root_ancestor(
+            self.cr, self.uid, view_id=self.ids['B1'])
-            self.cr, self.uid, model=self.model, view_type='form')
+        self.assertEqual(root_id, self.ids['B'])
-        self.assertEqual(root_id, A_id)
 
-    def test_no_root(self):
+    @unittest2.skip("What should the behavior be when no ancestor is found "
-        root = self.View.get_root_ancestor(
+                    "because view_id is invalid?")
-            self.cr, self.uid, model='does.not.exist', view_type='form')
+    def test_no_root_ancestor(self):
-        self.assertFalse(root)
+        root = self.View.root_ancestor(
-
-        root = self.View.get_root_ancestor(
-            self.cr, self.uid, model=self.model, view_type='tree')
-        self.assertFalse(root)
-
-        root = self.View.get_root_ancestor(
             self.cr, self.uid, view_id=12345678)
         self.assertFalse(root)
+
+    def test_default_view(self):
+        default = self.View.default_view(
+            self.cr, self.uid, model=self.model, view_type='form')
+        self.assertEqual(default, self.ids['A'])
+
+        default_tree = self.View.default_view(
+            self.cr, self.uid, model=self.model, view_type='tree')
+        self.assertEqual(default_tree, self.ids['C'])
+
+    @unittest2.skip("What should the behavior be when no default is found "
+                    "because model does not exist or no view for model?")
+    def test_no_default_view(self):
+        default = self.View.default_view(
+            self.cr, self.uid, model='does.not.exist', view_type='form')
+        self.assertFalse(default)
+
+        default = self.View.default_view(
+            self.cr, self.uid, model=self.model, view_type='graph')
+        self.assertFalse(default)
+
+class TestViewCombined(common.TransactionCase):
+    """
+    Test fallback operations of View.read_combined:
+    * defaults mapping
+    * ?
+    """