Merge pull request #197 from camptocamp/12.0-fix-force-storage-db

[12.0][IMP] force storage db

attachment_s3/README.rst

@@ -34,6 +34,10 @@ This addon must be added in the server wide addons with (``--load`` option):
 
 ``--load=web,attachment_s3``
 
+The System Parameter ``ir_attachment.storage.force.database`` can be customized to
+force storage of files in the database. See the documentation of the module
+``base_attachment_object_storage``.
+
 Limitations
 -----------
 

attachment_swift/README.rst

@@ -34,6 +34,10 @@ This addon must be added in the server wide addons with (``--load`` option):
 
 ``--load=web,attachment_swift``
 
+The System Parameter ``ir_attachment.storage.force.database`` can be customized to
+force storage of files in the database. See the documentation of the module
+``base_attachment_object_storage``.
+
 Python Dependencies
 -------------------
 

base_attachment_object_storage/README.rst

@@ -3,5 +3,38 @@ Base class for attachments on external object store
 
 This is a base addon that regroup common code used by addons targeting specific object store 
 
+Configuration
+-------------
 
+Object storage may be slow, and for this reason, we want to store
+some files in the database whatever.
 
+Small images (128, 256) are used in Odoo in list / kanban views. We
+want them to be fast to read.
+They are generally < 50KB (default configuration) so they don't take
+that much space in database, but they'll be read much faster than from
+the object storage.
+
+The assets (application/javascript, text/css) are stored in database
+as well whatever their size is:
+
+* a database doesn't have thousands of them
+* of course better for performance
+* better portability of a database: when replicating a production
+  instance for dev, the assets are included
+
+This storage configuration can be modified in the system parameter
+``ir_attachment.storage.force.database``, as a JSON value, for instance::
+
+    {"image/": 51200, "application/javascript": 0, "text/css": 0}
+
+Where the key is the beginning of the mimetype to configure and the
+value is the limit in size below which attachments are kept in DB.
+0 means no limit.
+
+Default configuration means:
+
+* images mimetypes (image/png, image/jpeg, ...) below 50KB are
+  stored in database
+* application/javascript are stored in database whatever their size
+* text/css are stored in database whatever their size

base_attachment_object_storage/__manifest__.py

@@ -4,13 +4,15 @@
 
 {'name': 'Base Attachment Object Store',
  'summary': 'Base module for the implementation of external object store.',
- 'version': '12.0.1.1.0',
+ 'version': '12.0.1.2.0',
  'author': 'Camptocamp,Odoo Community Association (OCA)',
  'license': 'AGPL-3',
  'category': 'Knowledge Management',
  'depends': ['base'],
  'website': 'http://www.camptocamp.com',
- 'data': [],
+ 'data': [
+     'data/res_config_settings_data.xml',
+ ],
  'installable': True,
  'auto_install': True,
  }

base_attachment_object_storage/data/res_config_settings_data.xml

@@ -0,0 +1,9 @@
+<?xml version='1.0' encoding='utf-8'?>
+<odoo noupdate="1">
+
+    <record id="ir_attachment_storage_force_database" model="ir.config_parameter">
+        <field name="key">ir_attachment.storage.force.database</field>
+        <field name="value">{"image/": 51200, "application/javascript": 0, "text/css": 0}</field>
+    </record>
+
+</odoo>

base_attachment_object_storage/models/ir_attachment.py

@@ -12,7 +12,8 @@ import odoo
 
 from contextlib import closing, contextmanager
 from odoo import api, exceptions, models, _
-from odoo.osv.expression import AND, normalize_domain
+from odoo.osv.expression import AND, OR, normalize_domain
+from odoo.tools.safe_eval import const_eval
 
 
 _logger = logging.getLogger(__name__)
@@ -70,88 +71,124 @@ class IrAttachment(models.Model):
         if update_module:
             self.env['ir.attachment'].sudo()._force_storage_to_object_storage()
 
-    @api.model
+    @property
-    def _save_in_db_domain(self):
+    def _object_storage_default_force_db_config(self):
+        return {"image/": 51200, "application/javascript": 0, "text/css": 0}
+
+    def _get_storage_force_db_config(self):
+        param = self.env['ir.config_parameter'].sudo().get_param(
+            'ir_attachment.storage.force.database',
+        )
+        storage_config = None
+        if param:
+            try:
+                storage_config = const_eval(param)
+            except (SyntaxError, TypeError, ValueError):
+                _logger.exception(
+                    "Could not parse system parameter"
+                    " 'ir_attachment.storage.force.database', reverting to the"
+                    " default configuration.")
+
+        if not storage_config:
+            storage_config = self._object_storage_default_force_db_config
+        return storage_config
+
+    def _store_in_db_instead_of_object_storage_domain(self):
         """Return a domain for attachments that must be forced to DB
 
-        Read the docstring of ``_save_in_db_anyway`` for more details.
+        Read the docstring of ``_store_in_db_instead_of_object_storage`` for
+        more details.
+
+        Used in ``force_storage_to_db_for_special_fields`` to find records
+        to move from the object storage to the database.
 
         The domain must be inline with the conditions in
-        ``_save_in_db_anyway``.
+        ``_store_in_db_instead_of_object_storage``.
         """
-        return [
+        domain = []
-            '|',
+        storage_config = self._get_storage_force_db_config()
-            # assets are stored in 'ir.ui.view'
+        for mimetype_key, limit in storage_config.items():
-            ('res_model', '=', 'ir.ui.view'),
+            part = [("mimetype", "=like", "{}%".format(mimetype_key))]
-            # Binary fields are stored with the name of the field in
+            if limit:
-            # 'res_field'
+                part = AND([part, [("file_size", "<=", limit)]])
-            # 'image' fields can be rather large and should usually
+            domain = OR([domain, part])
-            # not be requests in bulk in lists
+        return domain
-            ('res_field', 'in', self._local_fields)
-        ]
 
-    @api.multi
+    def _store_in_db_instead_of_object_storage(self):
-    def _save_in_db_anyway(self):
         """ Return whether an attachment must be stored in db
 
-        When we are using an Object Store. This is sometimes required
+        When we are using an Object Storage. This is sometimes required
         because the object storage is slower than the database/filesystem.
 
-        We store image_small and image_medium from 'Binary' fields
+        Small images (128, 256) are used in Odoo in list / kanban views. We
-        because they should be fast to read as they are often displayed
+        want them to be fast to read.
-        in kanbans / lists. The same for web_icon_data.
+        They are generally < 50KB (default configuration) so they don't take
+        that much space in database, but they'll be read much faster than from
+        the object storage.
 
-        We store the assets locally as well. Not only for performance,
+        The assets (application/javascript, text/css) are stored in database
-        but also because it improves the portability of the database:
+        as well whatever their size is:
-        when assets are invalidated, they are deleted so we don't have
+
-        an old database with attachments pointing to deleted assets.
+        * a database doesn't have thousands of them
+        * of course better for performance
+        * better portability of a database: when replicating a production
+          instance for dev, the assets are included
+
+        The configuration can be modified in the ir.config_parameter
+        ``ir_attachment.storage.force.database``, as a dictionary, for
+        instance::
+
+            {"image/": 51200, "application/javascript": 0, "text/css": 0}
+
+        Where the key is the beginning of the mimetype to configure and the
+        value is the limit in size below which attachments are kept in DB.
+        0 means no limit.
+
+        Default configuration means:
+
+        * images mimetypes (image/png, image/jpeg, ...) below 51200 bytes are
+          stored in database
+        * application/javascript are stored in database whatever their size
+        * text/css are stored in database whatever their size
 
         The conditions must be inline with the domain in
-        ``_save_in_db_domain``.
+        ``_store_in_db_instead_of_object_storage_domain``.
 
         """
-        self.ensure_one()
+        storage_config = self._get_storage_force_db_config()
-        # Note: we cannot use _save_in_db_domain because we can be working
+        for mimetype_key, limit in storage_config.items():
-        # with new records here. The conditions must stay inline though.
+            if self.mimetype.startswith(mimetype_key):
-        # assets
+                if not limit:
-        if self.res_model == 'ir.ui.view':
+                    return True
-            # assets are stored in 'ir.ui.view'
+                bin_data = base64.b64decode(self.datas) if self.datas else b''
-            return True
+                return len(bin_data) <= limit
-
-        # Binary fields
-        if self.res_field:
-            # Binary fields are stored with the name of the field in
-            # 'res_field'
-            # 'image' fields can be rather large and should usually
-            # not be requests in bulk in lists
-            if self.res_field and self.res_field in self._local_fields:
-                return True
         return False
 
     def _inverse_datas(self):
         # override in order to store files that need fast access,
         # we keep them in the database instead of the object storage
-        location = self.env.context.get('storage_location') or self._storage()
+        storage = self.env.context.get('storage_location') or self._storage()
         for attach in self:
-            if location in self._get_stores() and attach._save_in_db_anyway():
+            if storage in self._get_stores():
-                # compute the fields that depend on datas
+                if self._store_in_db_instead_of_object_storage():
-                value = attach.datas
+                    # compute the fields that depend on datas
-                bin_data = base64.b64decode(value) if value else ''
+                    value = attach.datas
-                vals = {
+                    bin_data = base64.b64decode(value) if value else ''
-                    'file_size': len(bin_data),
+                    vals = {
-                    'checksum': self._compute_checksum(bin_data),
+                        'file_size': len(bin_data),
-                    'db_datas': value,
+                        'checksum': self._compute_checksum(bin_data),
-                    # we seriously don't need index content on those fields
+                        # we seriously don't need index content on those fields
-                    'index_content': False,
+                        'index_content': False,
-                    'store_fname': False,
+                        'store_fname': False,
-                }
+                        'db_datas': value,
-                fname = attach.store_fname
+                    }
-                # write as superuser, as user probably does not
+                    fname = attach.store_fname
-                # have write access
+                    # write as superuser, as user probably does not
-                super(IrAttachment, attach.sudo()).write(vals)
+                    # have write access
-                if fname:
+                    super(IrAttachment, attach.sudo()).write(vals)
-                    self._file_delete(fname)
+                    if fname:
-                continue
+                        self._file_delete(fname)
+                    continue
             super(IrAttachment, attach)._inverse_datas()
 
     @api.model
@@ -228,7 +265,7 @@ class IrAttachment(models.Model):
                 with closing(registry.cursor()) as cr:
                     try:
                         yield self.env(cr=cr)
-                    except:
+                    except Exception:
                         cr.rollback()
                         raise
                     else:
@@ -291,9 +328,15 @@ class IrAttachment(models.Model):
 
         domain = AND((
             normalize_domain(
-                [('store_fname', '=like', '{}://%'.format(storage))]
+                [('store_fname', '=like', '{}://%'.format(storage)),
+                 # for res_field, see comment in
+                 # _force_storage_to_object_storage
+                 '|',
+                 ('res_field', '=', False),
+                 ('res_field', '!=', False),
+                 ]
             ),
-            normalize_domain(self._save_in_db_domain())
+            normalize_domain(self._store_in_db_instead_of_object_storage_domain())
         ))
 
         with self.do_in_new_env(new_cr=new_cr) as new_env: