Merge lp:~therp-nl/openobject-server/[e-mail]_fix_ir_ui_view_arch_lp925840-6.1 into lp:openobject-server

[Ronald Portier (Therp)]

Fix problems caused by excessive whitespace in arch field of ir_ui_view.

As a bonus the arch view will be much more compact. Check in psql with:

# select sum(length(arch)) as sum from ir_ui_view;

Before fix: 199.472

After fix: 118.180

This is bound to have a very positive influence on performance.

[Raphael Collet (OpenERP) (rco-openerp)]

Stripping all whitespaces in views is inadequate. It is common to edit a view with the views form. There, you want to keep the indentation of the XML in the field 'arch'.

Note that the impact of reducing the size of views on performance is negligible. (The number of queries sent to the DBMS has much more impact than the size of the results.)

review: Disapprove

[Olivier Dony (Odoo) (odo-openerp)]

Hello Ronald,

I agree with Raphael, stripping whitespace from views is very annoying for customizations, and we should rather address the whitespace issue you have found (please report a bug for it if this is a problem in the OpenERP framework), rather than blindly strip whitespace. As Raphael says, the storage and network transfer difference are rather negligible.

I hope you understand our point of view...

The numerous contributions from Therp are of great quality and really appreciated, thanks a lot for your efforts!

Unmerged revisions

4000. By Ronald Portier (Therp) on 2012-02-05

[FIX] Prevent errors caused by trailing whitespace in arch field. As a bonus, stored field will be much more compact.

=== modified file 'openerp/addons/base/ir/ir_ui_view.py'
--- openerp/addons/base/ir/ir_ui_view.py	2012-01-04 13:30:27 +0000
+++ openerp/addons/base/ir/ir_ui_view.py	2012-02-05 16:15:22 +0000
@@ -19,8 +19,10 @@
 #
 ##############################################################################
 
+import sys
 from osv import fields,osv
 from lxml import etree
+from StringIO import StringIO
 from tools import graph
 from tools.safe_eval import safe_eval as eval
 import tools
@@ -110,11 +112,55 @@
                       (view_id, model))
         return cr.fetchall()
 
+    _parser = etree.XMLParser(remove_blank_text=True, encoding='utf-8')
+
+    def _strip_tree(self, arch):
+        if  'encoding' in arch:
+            return arch  # parsing no supported for strings containg encoding
+        source = etree.parse(StringIO(arch), self._parser)
+        for element in source.iter('*'):
+            # Strip white space from element:
+            if  element.text is not None:
+                strip_text = element.text.strip()
+                if  strip_text:
+                    element.text = strip_text
+                else:
+                    element.text = None
+            # Strip whitespace from attributes:
+            for atr_key, atr_value in element.attrib.items():
+                if  atr_key and atr_value:
+                    element.set(atr_key, atr_value.strip())
+        return etree.tostring(source.getroot())
+
+    '''Override create and write method to make sure that the field arch is
+    stored in as compact a way as possible. This will not only improve
+    runtime performance, but also solve bugs due to spurious whitespace.
+    '''
+    def _compute_vals(self, cr, uid, view_id, vals):
+        try:
+            # We really should have an arch field - otherwise no point in
+            # having a view, but check regardless...
+            if  'arch' in vals and vals['arch']:
+                vals['arch'] = self._strip_tree(vals['arch'])
+        except:
+            logging.getLogger('ir_ui_view').debug(
+                '_compute_vals : Error computing vals: %s' %
+                 str(sys.exc_info()[0]))
+            logging.getLogger('ir_ui_view').debug(
+                '_compute_vals : vals: %s' % str(vals))
+            raise Exception("_compute_vals abnormally ended")
+
+    def create(self, cr, uid, vals, context=None):
+        self._compute_vals(cr, uid, 0, vals)
+        view_id = super(view, self).create(cr, uid, vals, context)
+        return view_id
+
     def write(self, cr, uid, ids, vals, context=None):
         if not isinstance(ids, (list, tuple)):
             ids = [ids]
+        for view_id in ids:
+            self._compute_vals(cr, uid, view_id, vals)
         result = super(view, self).write(cr, uid, ids, vals, context)
-
         # drop the corresponding view customizations (used for dashboards for example), otherwise
         # not all users would see the updated views
         custom_view_ids = self.pool.get('ir.ui.view.custom').search(cr, uid, [('ref_id','in',ids)])