Create base_attachment_object_storage to extract common code to store implementations

attachment_swift/README.rst

@@ -0,0 +1,34 @@
+Attachments on Swift storage
+============================
+
+This addon allows to store the attachments (documents and assets) on 
+OpenStack Object Storage (Swift)
+
+Configuration
+-------------
+
+Activate Swift storage:
+
+* Create or set the system parameter with the key ``ir_attachment.location``
+  and the value in the form ``swift``.
+
+Configure accesses with environment variables:
+
+* ``SWIFT_HOST``
+* ``SWIFT_ACCOUNT``
+* ``SWIFT_PASSWORD``
+* ``SWIFT_WRITE_CONTAINER``
+
+Read-only mode:
+
+The container name and the key are stored in the attachment. So if you change the
+``SWIFT_WRITE_CONTAINER`` or the ``ir_attachment.location``, the existing attachments
+will still be read on their former container. But as soon as they are written over
+or new attachments are created, they will be created on the new container or on
+the other location (db or filesystem). This is a convenient way to be able to
+read the production attachments on a replication (since you have the
+credentials) without any risk to alter the production data.
+
+This addon must be added in the server wide addons with (``--load`` option):
+
+``--load=web,web_kanban,attachment_swift``

attachment_swift/__init__.py

@@ -1 +1 @@
-import models
+from . import models

attachment_swift/__manifest__.py

@@ -4,12 +4,15 @@
 
 
 {'name': 'Attachments on Swift storage',
- 'summary': 'Store assets and attachments on a Swift compatible object storage',
+ 'summary': 'Store assets and attachments on a Swift compatible object store',
- 'version': '10.0.0.0',
+ 'version': '10.0.1.1.0',
  'author': 'Camptocamp,Odoo Community Association (OCA)',
  'license': 'AGPL-3',
  'category': 'Knowledge Management',
- 'depends': ['base'],
+ 'depends': ['base_attachment_object_storage'],
+ 'external_dependencies': {
+     'python': ['swiftclient'],
+ },
  'website': 'http://www.camptocamp.com',
  'data': [],
  'installable': True,

attachment_swift/models/ir_attachment.py

@@ -6,23 +6,25 @@
 import base64
 import logging
 import os
-import swiftclient
-from swiftclient.exceptions import ClientException
 from ..swift_uri import SwiftUri
 
 from odoo import api, exceptions, models, _
 
 _logger = logging.getLogger(__name__)
 
+try:
+    import swiftclient
+    from swiftclient.exceptions import ClientException
+except ImportError:
+    swiftclient = None
+    ClientException = None
+    _logger.debug("Cannot 'import swiftclient'.")
+
 
 class IrAttachment(models.Model):
     _inherit = 'ir.attachment'
 
-    @api.multi
+    _SWIFT_STORAGE = 'swift'
-    def _store_in_db_when_swift(self):
-        # For testing lets save everything in Swift object store
-        # TODO: same than in attachment_s3
-        return False
 
     @api.model
     def _get_swift_connection(self):
@@ -31,86 +33,71 @@ class IrAttachment(models.Model):
         account = os.environ.get('SWIFT_ACCOUNT')
         password = os.environ.get('SWIFT_PASSWORD')
         if not (host and account and password):
-            raise exceptions.UserError(
+            raise exceptions.UserError(_(
-                _('Problem connecting to Swift store, are not the env variables set ?'))
+                '''Problem connecting to Swift store, are the env variables
-        print 'Connection to host: {}, account: {}, password: {}'.format(host, account, password)
+                   (SWIFT_HOST, SWIFT_ACCOUNT, SWIFT_PASSWORD) properly set ?
+                '''))
         try:
-            conn = swiftclient.client.Connection(authurl=host, user=account, key=password)
+            conn = swiftclient.client.Connection(authurl=host,
+                                                 user=account,
+                                                 key=password)
         except ClientException:
             _logger.exception('Error connecting to Swift object store')
-            raise exceptions.UserError('Error connection to Swift')
+            raise exceptions.UserError(_('Error on Swift connection'))
         return conn
 
     @api.model
-    def _file_read_swift(self, fname, bin_size=False):
+    def _store_file_read(self, fname, bin_size=False):
-        swifturi = SwiftUri(fname)
-        conn = self._get_swift_connection()
-        print 'Swift reading on {} of {} '.format(swifturi.container(), swifturi.item())
-        try:
-            resp_headers, obj_content = conn.get_object(swifturi.container(), swifturi.item())
-            read = base64.b64encode(obj_content)
-        except ClientException:
-            _logger.exception('Error reading object from Swift object store');
-            #raise exceptions.UserError('Error reading to Swift')
-            return ''
-        return read
-
-    @api.model
-    def _file_read(self, fname, bin_size=False):
         if fname.startswith('swift://'):
-            return self._file_read_swift(fname, bin_size=bin_size)
+            swifturi = SwiftUri(fname)
+            conn = self._get_swift_connection()
+            try:
+                resp, obj_content = conn.get_object(swifturi.container(),
+                                                    swifturi.item())
+                read = base64.b64encode(obj_content)
+            except ClientException:
+                _logger.exception(
+                    'Error reading object from Swift object store')
+                raise exceptions.UserError(_('Error reading on Swift'))
+            return read
+        else:
+            return super(IrAttachment, self)._store_file_read(fname, bin_size)
+
+    def _store_file_write(self, value, checksum):
+        if self._storage() == self._SWIFT_STORAGE:
+            container = os.environ.get('SWIFT_WRITE_CONTAINER')
+            conn = self._get_swift_connection()
+            conn.put_container(container)
+            bin_data = value.decode('base64')
+            key = self._compute_checksum(bin_data)
+            filename = 'swift://{}/{}'.format(container, key)
+            try:
+                conn.put_object(container, key, bin_data)
+            except ClientException:
+                _logger.exception('Error writing to Swift object store')
+                raise exceptions.UserError(_('Error writing to Swift'))
         else:
             _super = super(IrAttachment, self)
-            return _super._file_read(fname, bin_size=bin_size)
+            filename = _super._store_file_write(value, checksum)
-
-    def _file_write_swift(self, value, checksum):
-        container = os.environ.get('SWIFT_WRITE_CONTAINER')
-        conn = self._get_swift_connection()
-        conn.put_container(container)
-        bin_data = value.decode('base64')
-        # No keys given by the store, use checksum !?
-        key = self._compute_checksum(bin_data)
-        filename = 'swift://{}/{}'.format(container, key)
-        print 'Saving {}'.format(filename)
-        try:
-            conn.put_object(container, key, bin_data)
-        except ClientException:
-            _logger.exception('Error connecting to Swift object store')
-            raise exceptions.UserError('Error writting to Swift')
-        return filename
-
-    def _file_write(self, value, checksum):
-        storage = self._storage()
-        if storage == 'swift':
-            filename = self._file_write_swift(value, checksum)
-        else:
-            filename = super(IrAttachment, self)._file_write(value, checksum)
         return filename
 
     @api.model
-    def _file_delete(self, fname):
+    def _store_file_delete(self, fname):
         if fname.startswith('swift://'):
             swifturi = SwiftUri(fname)
             container = swifturi.container()
-            print 'Deleting... container: {} | filename: {}'.format(container, swifturi.item())
             if container == os.environ.get('SWIFT_WRITE_CONTAINER'):
                 conn = self._get_swift_connection()
                 try:
                     conn.delete_object(container, swifturi.item())
-                except ClientException as error:
+                except ClientException:
-                    _logger.exception('Error connecting to Swift object store');
+                    _logger.exception(
-                    raise exceptions.UserError('Error deleting in Swift')
+                        _('Error deleting an object on the Swift store'))
+                    raise exceptions.UserError(_('Error deleting on Swift'))
         else:
             super(IrAttachment, self)._file_delete(fname)
 
-    @api.model
+    def _get_stores(self):
-    def _force_storage_swift(self, new_cr=False):
+        l = [self._SWIFT_STORAGE]
-        return
+        l += super(IrAttachment, self)._get_stores()
-
+        return l
-    @api.model
-    def force_storage(self):
-        storage = self._storage()
-        if storage == 'swift':
-            self._force_storage_swift()
-        else:
-            return super(IrAttachment, self).force_storage()

attachment_swift/swift_uri.py

@@ -7,7 +7,8 @@ import re
 
 class SwiftUri(object):
 
-    _url_re = re.compile("^swift:///*([^/]*)/?(.*)", re.IGNORECASE | re.UNICODE)
+    _url_re = re.compile("^swift:///*([^/]*)/?(.*)",
+                         re.IGNORECASE | re.UNICODE)
 
     def __init__(self, uri):
         match = self._url_re.match(uri)

attachment_swift/tests/tests.py

@@ -1,8 +1,8 @@
 # -*- coding: utf-8 -*-
+# Copyright 2016 Camptocamp SA
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
 
-import unittest
 from odoo.addons.base.tests.test_ir_attachment import TestIrAttachment
-from ..models import ir_attachment
 from ..swift_uri import SwiftUri
 from swiftclient.exceptions import ClientException
 
@@ -11,7 +11,8 @@ class TestAttachmentSwift(TestIrAttachment):
 
     def setup(self):
         super(TestAttachmentSwift, self).setUp()
-        self.env['ir.config_parameter'].set_param('ir_attachment.location', 'swift')
+        self.env['ir.config_parameter'].set_param('ir_attachment.location',
+                                                  'swift')
 
     def test_connection(self):
         """ Test the connection to the Swift object store """
@@ -19,15 +20,19 @@ class TestAttachmentSwift(TestIrAttachment):
         self.assertNotEquals(conn, False)
 
     def test_store_file_on_swift(self):
+        (self.env['ir.config_parameter'].
+            set_param('ir_attachment.location', 'swift'))
         a5 = self.Attachment.create({'name': 'a5', 'datas': self.blob1_b64})
         a5bis = self.Attachment.browse(a5.id)[0]
+        self.assertEquals(a5.datas, a5bis.datas)
 
     def test_delete_file_on_swift(self):
-        self.env['ir.config_parameter'].set_param('ir_attachment.location', 'swift')
+        (self.env['ir.config_parameter'].
+            set_param('ir_attachment.location', 'swift'))
         a5 = self.Attachment.create({'name': 'a5', 'datas': self.blob1_b64})
         uri = SwiftUri(a5.store_fname)
         con = self.Attachment._get_swift_connection()
-        data = con.get_object(uri.container(), uri.item())
+        con.get_object(uri.container(), uri.item())
         a5.unlink()
         with self.assertRaises(ClientException):
             con.get_object(uri.container(), uri.item())

base_attachment_object_storage/README.rst

@@ -0,0 +1,7 @@
+Base class for attachments on external object store
+===================================================
+
+This is a base addon that regroup common code used by addons targeting specific object store 
+
+
+

base_attachment_object_storage/__init__.py

@@ -0,0 +1 @@
+from . import models

base_attachment_object_storage/__manifest__.py

@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+# Copyright 2017 Camptocamp SA
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
+
+
+{'name': 'Base Attachment Object Store',
+ 'summary': 'Base module for the implementation of external object store.',
+ 'version': '10.0.1.1.0',
+ 'author': 'Camptocamp,Odoo Community Association (OCA)',
+ 'license': 'AGPL-3',
+ 'category': 'Knowledge Management',
+ 'depends': ['base'],
+ 'website': 'http://www.camptocamp.com',
+ 'data': [],
+ 'installable': True,
+ 'auto_install': True,
+ }

base_attachment_object_storage/models/__init__.py

@@ -0,0 +1 @@
+from . import ir_attachment

base_attachment_object_storage/models/ir_attachment.py

@@ -0,0 +1,226 @@
+# -*- coding: utf-8 -*-
+# Copyright 2017 Camptocamp SA
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
+
+
+import logging
+import os
+import psycopg2
+import odoo
+
+from contextlib import closing, contextmanager
+from odoo import api, exceptions, models, _
+
+
+_logger = logging.getLogger(__name__)
+
+
+class IrAttachment(models.Model):
+    _inherit = 'ir.attachment'
+
+    _local_fields = ('image_small', 'image_medium', 'web_icon_data')
+
+    @api.multi
+    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
+        because the object storage is slower than the database/filesystem.
+
+        We store image_small and image_medium from 'Binary' fields
+        because they should be fast to read as they are often displayed
+        in kanbans / lists. The same for web_icon_data.
+
+        We store the assets locally as well. Not only for performance,
+        but also because it improves the portability of the database:
+        when assets are invalidated, they are deleted so we don't have
+        an old database with attachments pointing to deleted assets.
+
+        """
+        self.ensure_one()
+        # assets
+        if self.res_model == 'ir.ui.view':
+            # assets are stored in 'ir.ui.view'
+            return True
+
+        # 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._storage()
+        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 = value and value.decode('base64') or ''
+                vals = {
+                    'file_size': len(bin_data),
+                    'checksum': self._compute_checksum(bin_data),
+                    'db_datas': value,
+                    # we seriously don't need index content on those fields
+                    'index_content': False,
+                    'store_fname': False,
+                }
+                fname = attach.store_fname
+                # write as superuser, as user probably does not
+                # 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):
+        if self._is_file_from_a_store(fname):
+            return self._store_file_read(fname, bin_size=bin_size)
+        else:
+            _super = super(IrAttachment, self)
+            return _super._file_read(fname, bin_size=bin_size)
+
+    @api.model
+    def _file_write(self, value, checksum):
+        if self._storage() in self._get_stores():
+            filename = self._store_file_write(value, checksum)
+        else:
+            filename = super(IrAttachment, self)._file_write(value, checksum)
+        return filename
+
+    @api.model
+    def _file_delete(self, fname):
+        if self._is_file_from_a_store(fname):
+            cr = self.env.cr
+            cr.execute("SELECT COUNT(*) FROM ir_attachment "
+                       "WHERE store_fname = %s", (fname,))
+            count = cr.fetchone()[0]
+            if not count:
+                self._store_file_delete(fname)
+        else:
+            super(IrAttachment, self)._file_delete(fname)
+
+    @api.model
+    def _is_file_from_a_store(self, fname):
+        for store_name in self._get_stores():
+            uri = '{}://'.format(store_name)
+            if fname.startswith(uri):
+                return True
+        return False
+
+    @contextmanager
+    def do_in_new_env(self, new_cr=False):
+        """ Context manager that yields a new environment
+
+        Using a new Odoo Environment thus a new PG transaction.
+        """
+        with api.Environment.manage():
+            if new_cr:
+                registry = odoo.modules.registry.RegistryManager.get(
+                    self.env.cr.dbname
+                )
+                with closing(registry.cursor()) as cr:
+                    try:
+                        yield self.env(cr=cr)
+                    except:
+                        cr.rollback()
+                        raise
+                    else:
+                        # disable pylint error because this is a valid commit,
+                        # we are in a new env
+                        cr.commit()  # pylint: disable=invalid-commit
+            else:
+                # make a copy
+                yield self.env()
+
+    @api.multi
+    def _move_attachment_to_store(self):
+        self.ensure_one()
+        _logger.info('inspecting attachment %s (%d)', self.name, self.id)
+        fname = self.store_fname
+        if fname:
+            # migrating from filesystem filestore
+            # or from the old 'store_fname' without the bucket name
+            _logger.info('moving %s on the object storage', fname)
+            self.write({'datas': self.datas,
+                        # this is required otherwise the
+                        # mimetype gets overriden with
+                        # 'application/octet-stream'
+                        # on assets
+                        'mimetype': self.mimetype})
+            _logger.info('moved %s on the object storage', fname)
+            full_path = self._full_path(fname)
+            _logger.info('cleaning fs self')
+            if os.path.exists(full_path):
+                try:
+                    os.unlink(full_path)
+                except OSError:
+                    _logger.info(
+                        "_file_delete could not unlink %s",
+                        full_path, exc_info=True
+                    )
+                except IOError:
+                    # Harmless and needed for race conditions
+                    _logger.info(
+                        "_file_delete could not unlink %s",
+                        full_path, exc_info=True
+                    )
+        elif self.db_datas:
+            _logger.info('moving on the object storage from database')
+            self.write({'datas': self.datas})
+
+    @api.model
+    def force_storage(self):
+        if not self.env['res.users'].browse(self.env.uid)._is_admin():
+            raise exceptions.AccessError(
+                _('Only administrators can execute this action.'))
+        storage = self._storage()
+        if storage not in self._get_stores():
+            return super(IrAttachment, self).force_storage()
+        _logger.info('migrating files to the object storage')
+        domain = ['!', ('store_fname', '=like', '{}://%'.format(storage)),
+                  '|',
+                  ('res_field', '=', False),
+                  ('res_field', '!=', False)]
+        # We do a copy of the environment so we can workaround the
+        # cache issue below. We do not create a new cursor because
+        # it causes serialization issues due to concurrent updates on
+        # attachments during the installation
+        with self.do_in_new_env() as new_env:
+            model_env = new_env['ir.attachment']
+            ids = model_env.search(domain).ids
+            for attachment_id in ids:
+                try:
+                    with new_env.cr.savepoint():
+                        # check that no other transaction has
+                        # locked the row, don't send a file to S3
+                        # in that case
+                        self.env.cr.execute("SELECT id "
+                                            "FROM ir_attachment "
+                                            "WHERE id = %s "
+                                            "FOR UPDATE NOWAIT",
+                                            (attachment_id,),
+                                            log_exceptions=False)
+
+                        # This is a trick to avoid having the 'datas'
+                        # function fields computed for every attachment on
+                        # each iteration of the loop. The former issue
+                        # being that it reads the content of the file of
+                        # ALL the attachments on each loop.
+                        new_env.clear()
+                        attachment = model_env.browse(attachment_id)
+                        attachment._move_attachment_to_store()
+                except psycopg2.OperationalError:
+                    _logger.error('Could not migrate attachment %s to S3',
+                                  attachment_id)
+
+    def _get_stores(self):
+        """ To get the list of stores activated in the system  """
+        return []