Merge pull request #187 from guewen/13.0-fix-force-storage-db

[13.0] Rework and fix storage forced in database

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/data/res_config_settings_data.xml

@@ -1,16 +1,9 @@
 <?xml version='1.0' encoding='utf-8'?>
 <odoo noupdate="1">
-    <record id="default_mimes_type_storedb" model="ir.config_parameter">
+
-        <field name="key">mimetypes.list.storedb</field>
+    <record id="ir_attachment_storage_force_database" model="ir.config_parameter">
-        <field name="value">image</field>
+        <field name="key">ir_attachment.storage.force.database</field>
-    </record>
+        <field name="value">{"image/": 51200, "application/javascript": 0, "text/css": 0}</field>
-    <record id="default_file_maxsize_storedb" model="ir.config_parameter">
-        <field name="key">file.maxsize.storedb</field>
-        <field name="value">50000</field>
-    </record>
-    <record id="excluded_model_storedb" model="ir.config_parameter">
-        <field name="key">excluded.models.storedb</field>
-        <field name="value">mail.message,mail.mail</field>
     </record>
 
 </odoo>

base_attachment_object_storage/models/ir_attachment.py

@@ -12,8 +12,8 @@ import odoo
 
 from contextlib import closing, contextmanager
 from odoo import api, exceptions, models, _
-from odoo.tools.mimetypes import guess_mimetype
+from odoo.osv.expression import AND, OR, normalize_domain
-from odoo.osv.expression import AND, normalize_domain
+from odoo.tools.safe_eval import const_eval
 
 
 _logger = logging.getLogger(__name__)
@@ -68,109 +68,114 @@ 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``.
         """
-        excluded_model_settings = self.env['ir.config_parameter'].sudo().\
+        domain = []
-            get_param('excluded.models.storedb', default='')
+        storage_config = self._get_storage_force_db_config()
-        excluded_model_for_db_store = excluded_model_settings.split(',')
+        for mimetype_key, limit in storage_config.items():
-        mimetypes_settings = self.env['ir.config_parameter'].sudo().get_param(
+            part = [("mimetype", "=like", "{}%".format(mimetype_key))]
-            'mimetypes.list.storedb', default='')
+            if limit:
-        mimetypes_for_db_store = mimetypes_settings.split(',')
+                part = AND([part, [("file_size", "<=", limit)]])
-        filesize = self.env['ir.config_parameter'].sudo().get_param(
+            domain = OR([domain, part])
-            'file.maxsize.storedb', default='0')
-        domain = [
-            '|',
-            # assets are stored in 'ir.ui.view'
-            ('res_model', '=', 'ir.ui.view'),
-            '&', '&',
-            ('file_size', '<', int(filesize)),
-            ('res_model', 'not in', excluded_model_for_db_store),
-        ]
-        domain += ['|'] * (len(mimetypes_for_db_store) - 1)
-        domain += [('mimetype', '=like', mimetype) for mimetype in
-                   mimetypes_for_db_store]
         return domain
 
-    def _save_in_db_anyway(self):
+    def _store_in_db_instead_of_object_storage(self, data, mimetype):
         """ 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 mimetype.startswith(mimetype_key):
-        # assets
+                if not limit:
-        if self.res_model == 'ir.ui.view':
-            # assets are stored in 'ir.ui.view'
-            return True
-        # Check if model must never be stored on DB
-        excluded_model_settings = self.env['ir.config_parameter'].sudo().\
-            get_param('excluded.models.storedb', default='')
-        excluded_model_for_db_store = excluded_model_settings.split(',')
-        if self.res_model in excluded_model_for_db_store:
-            return False
-        # Check if file size and mimetype fit requirements
-        data_to_store = self.datas
-        bin_data = base64.b64decode(data_to_store) if data_to_store else ''
-        current_mimetype = guess_mimetype(bin_data)
-        mimetypes_settings = self.env['ir.config_parameter'].sudo().get_param(
-            'mimetypes.list.storedb', default='')
-        mimetypes_for_db_store = mimetypes_settings.split(',')
-        if any(current_mimetype.startswith(val) for val in
-               mimetypes_for_db_store):
-            # get allowed size
-            filesize = self.env['ir.config_parameter'].sudo().get_param(
-                'file.maxsize.storedb', default='0')
-            if len(bin_data) < int(filesize):
                     return True
+                bin_data = base64.b64decode(data) if data else b''
+                return len(bin_data) <= limit
         return False
 
-    def _inverse_datas(self):
+    def _get_datas_related_values(self, data, mimetype):
-        # override in order to store files that need fast access,
+        storage = self.env.context.get('storage_location') or self._storage()
-        # we keep them in the database instead of the object storage
+        if data and storage in self._get_stores():
-        location = self.env.context.get('storage_location') or self._storage()
+            if self._store_in_db_instead_of_object_storage(data, mimetype):
-        for attach in self:
-            if location in self._get_stores() and attach._save_in_db_anyway():
                 # compute the fields that depend on datas
-                value = attach.datas
+                bin_data = base64.b64decode(data) if data else b''
-                bin_data = base64.b64decode(value) if value else ''
+                values = {
-                vals = {
                     'file_size': len(bin_data),
                     'checksum': self._compute_checksum(bin_data),
-                    'db_datas': value,
+                    'index_content': self._index(bin_data, mimetype),
-                    # we seriously don't need index content on those fields
-                    'index_content': False,
                     'store_fname': False,
+                    'db_datas': data,
                 }
-                fname = attach.store_fname
+                return values
-                # write as superuser, as user probably does not
+        return super()._get_datas_related_values(data, mimetype)
-                # have write access
-                super(IrAttachment, attach.sudo()).write(vals)
-                if fname:
-                    self._file_delete(fname)
-                continue
-            super(IrAttachment, attach)._inverse_datas()
 
     @api.model
     def _file_read(self, fname, bin_size=False):
@@ -245,7 +250,7 @@ class IrAttachment(models.Model):
                 with closing(registry.cursor()) as cr:
                     try:
                         yield self.env(cr=cr)
-                    except:
+                    except Exception:
                         cr.rollback()
                         raise
                     else:
@@ -307,9 +312,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: