From fc452c6a2abbae4ab7d4bd041c5d2212691a8096 Mon Sep 17 00:00:00 2001 From: Vincent Renaville Date: Fri, 11 Nov 2022 15:17:03 +0100 Subject: [PATCH] feat: remove after method (#393) * fix: azure reading in stream monkey patch documents --- attachment_azure/models/__init__.py | 1 + attachment_azure/models/ir_attachment.py | 21 +- attachment_azure/models/ir_binary.py | 65 ++++++ .../models/ir_attachment.py | 206 +++++++++--------- requirements.txt | 23 +- 5 files changed, 194 insertions(+), 122 deletions(-) create mode 100644 attachment_azure/models/ir_binary.py diff --git a/attachment_azure/models/__init__.py b/attachment_azure/models/__init__.py index cb9b196..c4b1a34 100644 --- a/attachment_azure/models/__init__.py +++ b/attachment_azure/models/__init__.py @@ -2,3 +2,4 @@ # Copyright 2021 Open Source Integrators # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) from . import ir_attachment +from . import ir_binary diff --git a/attachment_azure/models/ir_attachment.py b/attachment_azure/models/ir_attachment.py index a9e2113..2538817 100644 --- a/attachment_azure/models/ir_attachment.py +++ b/attachment_azure/models/ir_attachment.py @@ -117,11 +117,8 @@ class IrAttachment(models.Model): https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#container-names """ running_env = os.environ.get("RUNNING_ENV", "dev") - storage_name = os.environ.get('AZURE_STORAGE_NAME', r'{env}-{db}') - storage_name = storage_name.format( - env=running_env, - db=self.env.cr.dbname - ) + storage_name = os.environ.get("AZURE_STORAGE_NAME", r"{env}-{db}") + storage_name = storage_name.format(env=running_env, db=self.env.cr.dbname) # replace invalid characters by _ storage_name = re.sub(r"[\W_]+", "-", storage_name) # lowercase, max 63 chars @@ -136,7 +133,7 @@ class IrAttachment(models.Model): except exceptions.UserError: _logger.exception( "error accessing to storage '%s' please check credentials ", - container_name + container_name, ) return False container_client = blob_service_client.get_container_client(container_name) @@ -153,14 +150,14 @@ class IrAttachment(models.Model): def _store_file_read(self, fname, bin_size=False): if fname.startswith("azure://"): key = fname.replace("azure://", "", 1).lower() - if '/' in key: - container_name, key = key.split('/', 1) + if "/" in key: + container_name, key = key.split("/", 1) else: container_name = None container_client = self._get_azure_container(container_name) # if container cannot be retrived, abort reading from azure storage if not container_client: - return '' + return "" try: blob_client = container_client.get_blob_client(key) read = blob_client.download_blob().readall() @@ -200,13 +197,13 @@ class IrAttachment(models.Model): def _store_file_delete(self, fname): if fname.startswith("azure://"): key = fname.replace("azure://", "", 1).lower() - if '/' in key: - container_name, key = key.split('/', 1) + if "/" in key: + container_name, key = key.split("/", 1) else: container_name = None container_client = self._get_azure_container(container_name) if not container_client: - return '' + return "" # delete the file only if it is on the current configured container # otherwise, we might delete files used on a different environment try: diff --git a/attachment_azure/models/ir_binary.py b/attachment_azure/models/ir_binary.py new file mode 100644 index 0000000..7c99ffa --- /dev/null +++ b/attachment_azure/models/ir_binary.py @@ -0,0 +1,65 @@ +# Copyright 2016-2019 Camptocamp SA +# Copyright 2021 Open Source Integrators +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) +from odoo import models +from odoo.http import Stream + + +class IrBinary(models.AbstractModel): + _inherit = "ir.binary" + _description = "File streaming helper model for controllers" + + def _azure_stream(self, attachment): + # we will create or own tream and return it + stream_data = self.env["ir.attachment"]._store_file_read(attachment.store_fname) + azurestream = Stream( + type="data", + data=stream_data, + path=None, + url=None, + mimetype=attachment.mimetype or None, + download_name=attachment.name, + size=len(stream_data), + etag=attachment.checksum, + ) + return azurestream + + def _record_to_stream(self, record, field_name): + """ + Low level method responsible for the actual conversion from a + model record to a stream. This method is an extensible hook for + other modules. It is not meant to be directly called from + outside or the ir.binary model. + + :param record: the record where to load the data from. + :param str field_name: the binary field where to load the data + from. + :rtype: odoo.http.Stream + """ + if ( + record._name == "ir.attachment" + and record.store_fname + and record.store_fname.startswith("azure://") + ): + # we will create or own tream and return it + return self._azure_stream(record) + elif ( + record._name == "documents.document" + and record.attachment_id + and record.attachment_id.store_fname + and record.attachment_id.store_fname.startswith("azure://") + ): + return self._azure_stream(record.attachment_id) + + else: + return super()._record_to_stream(record, field_name) + + +# This part is used if the customer install tne enterprise module documents +try: + from odoo.addons import documents + + documents.models.ir_binary.IrBinary._record_to_stream = IrBinary._record_to_stream +except ImportError: + # document enterprise module if not installed, we just ignore + pass diff --git a/base_attachment_object_storage/models/ir_attachment.py b/base_attachment_object_storage/models/ir_attachment.py index 425bb9e..a4e3ec9 100644 --- a/base_attachment_object_storage/models/ir_attachment.py +++ b/base_attachment_object_storage/models/ir_attachment.py @@ -20,38 +20,36 @@ _logger = logging.getLogger(__name__) def is_true(strval): - return bool(strtobool(strval or '0')) + return bool(strtobool(strval or "0")) def clean_fs(files): - _logger.info('cleaning old files from filestore') + _logger.info("cleaning old files from filestore") for full_path in files: 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 + "_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 + "_file_delete could not unlink %s", full_path, exc_info=True ) class IrAttachment(models.Model): - _inherit = 'ir.attachment' + _inherit = "ir.attachment" @staticmethod def is_storage_disabled(storage=None, log=True): msg = _("Storages are disabled (see environment configuration).") if storage: - msg = _( - "Storage '%s' is disabled (see environment configuration)." - ) % (storage,) + msg = _("Storage '%s' is disabled (see environment configuration).") % ( + storage, + ) is_disabled = is_true(os.environ.get("DISABLE_ATTACHMENT_STORAGE")) if is_disabled and log: _logger.warning(msg) @@ -59,7 +57,7 @@ class IrAttachment(models.Model): def _register_hook(self): super()._register_hook() - location = self.env.context.get('storage_location') or self._storage() + location = self.env.context.get("storage_location") or self._storage() # ignore if we are not using an object storage if location not in self._get_stores(): return @@ -73,7 +71,7 @@ class IrAttachment(models.Model): # done during the initialization. We need to move the attachments that # could have been created or updated in other addons before this addon # was loaded - update_module = load_modules_frame.f_locals.get('update_module') + update_module = load_modules_frame.f_locals.get("update_module") # We need to call the migration on the loading of the model because # when we are upgrading addons, some of them might add attachments. @@ -82,15 +80,19 @@ class IrAttachment(models.Model): # Typical example is images of ir.ui.menu which are updated in # ir.attachment at every upgrade of the addons if update_module: - self.env['ir.attachment'].sudo()._force_storage_to_object_storage() + self.env["ir.attachment"].sudo()._force_storage_to_object_storage() @property 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', + param = ( + self.env["ir.config_parameter"] + .sudo() + .get_param( + "ir_attachment.storage.force.database", + ) ) storage_config = None if param: @@ -100,7 +102,8 @@ class IrAttachment(models.Model): _logger.exception( "Could not parse system parameter" " 'ir_attachment.storage.force.database', reverting to the" - " default configuration.") + " default configuration." + ) if not storage_config: storage_config = self._object_storage_default_force_db_config @@ -128,7 +131,7 @@ class IrAttachment(models.Model): return domain def _store_in_db_instead_of_object_storage(self, data, mimetype): - """ Return whether an attachment must be stored in db + """Return whether an attachment must be stored in db When we are using an Object Storage. This is sometimes required because the object storage is slower than the database/filesystem. @@ -180,17 +183,17 @@ class IrAttachment(models.Model): return False def _get_datas_related_values(self, data, mimetype): - storage = self.env.context.get('storage_location') or self._storage() + storage = self.env.context.get("storage_location") or self._storage() if data and storage in self._get_stores(): if self._store_in_db_instead_of_object_storage(data, mimetype): # compute the fields that depend on datas bin_data = data values = { - 'file_size': len(bin_data), - 'checksum': self._compute_checksum(bin_data), - 'index_content': self._index(bin_data, mimetype), - 'store_fname': False, - 'db_datas': data, + "file_size": len(bin_data), + "checksum": self._compute_checksum(bin_data), + "index_content": self._index(bin_data, mimetype), + "store_fname": False, + "db_datas": data, } return values return super()._get_datas_related_values(data, mimetype) @@ -203,28 +206,22 @@ class IrAttachment(models.Model): return super()._file_read(fname) def _store_file_read(self, fname): - storage = fname.partition('://')[0] - raise NotImplementedError( - 'No implementation for %s' % (storage,) - ) + storage = fname.partition("://")[0] + raise NotImplementedError("No implementation for %s" % (storage,)) def _store_file_write(self, key, bin_data): storage = self.storage() - raise NotImplementedError( - 'No implementation for %s' % (storage,) - ) + raise NotImplementedError("No implementation for %s" % (storage,)) def _store_file_delete(self, fname): - storage = fname.partition('://')[0] - raise NotImplementedError( - 'No implementation for %s' % (storage,) - ) + storage = fname.partition("://")[0] + raise NotImplementedError("No implementation for %s" % (storage,)) @api.model def _file_write(self, bin_data, checksum): - location = self.env.context.get('storage_location') or self._storage() + location = self.env.context.get("storage_location") or self._storage() if location in self._get_stores(): - key = self.env.context.get('force_storage_key') + key = self.env.context.get("force_storage_key") if not key: key = self._compute_checksum(bin_data) filename = self._store_file_write(key, bin_data) @@ -238,8 +235,9 @@ class IrAttachment(models.Model): cr = self.env.cr # using SQL to include files hidden through unlink or due to record # rules - cr.execute("SELECT COUNT(*) FROM ir_attachment " - "WHERE store_fname = %s", (fname,)) + cr.execute( + "SELECT COUNT(*) FROM ir_attachment " "WHERE store_fname = %s", (fname,) + ) count = cr.fetchone()[0] if not count: self._store_file_delete(fname) @@ -251,22 +249,20 @@ class IrAttachment(models.Model): for store_name in self._get_stores(): if self.is_storage_disabled(store_name): continue - uri = '{}://'.format(store_name) + 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 + """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.Registry.new( - self.env.cr.dbname - ) + registry = odoo.modules.registry.Registry.new(self.env.cr.dbname) with closing(registry.cursor()) as cr: try: yield self.env(cr=cr) @@ -283,33 +279,38 @@ class IrAttachment(models.Model): def _move_attachment_to_store(self): self.ensure_one() - _logger.info('inspecting attachment %s (%d)', self.name, self.id) + _logger.info("inspecting attachment %s (%d)", self.name, self.id) fname = self.store_fname - storage = fname.partition('://')[0] + storage = fname.partition("://")[0] if self.is_storage_disabled(storage): fname = False 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) + _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) return self._full_path(fname) elif self.db_datas: - _logger.info('moving on the object storage from database') - self.write({'datas': self.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(): + if not self.env["res.users"].browse(self.env.uid)._is_admin(): raise exceptions.AccessError( - _('Only administrators can execute this action.')) - location = self.env.context.get('storage_location') or self._storage() + _("Only administrators can execute this action.") + ) + location = self.env.context.get("storage_location") or self._storage() if location not in self._get_stores(): return super().force_storage() self._force_storage_to_object_storage() @@ -335,30 +336,32 @@ class IrAttachment(models.Model): if storage not in self._get_stores(): return - domain = AND(( - normalize_domain( - [('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._store_in_db_instead_of_object_storage_domain()) - )) + domain = AND( + ( + normalize_domain( + [ + ("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._store_in_db_instead_of_object_storage_domain()), + ) + ) with self.do_in_new_env(new_cr=new_cr) as new_env: - model_env = new_env['ir.attachment'].with_context( - prefetch_fields=False - ) + model_env = new_env["ir.attachment"].with_context(prefetch_fields=False) attachment_ids = model_env.search(domain).ids if not attachment_ids: return total = len(attachment_ids) start_time = time.time() - _logger.info('Moving %d attachments from %s to' - ' DB for fast access', total, storage) + _logger.info( + "Moving %d attachments from %s to" " DB for fast access", total, storage + ) current = 0 for attachment_id in attachment_ids: current += 1 @@ -370,21 +373,22 @@ class IrAttachment(models.Model): # this write will read the datas from the Object Storage and # write them back in the DB (the logic for location to write is # in the 'datas' inverse computed field) - attachment.write({'datas': attachment.datas}) + attachment.write({"datas": attachment.datas}) # as the file will potentially be dropped on the bucket, # we should commit the changes here new_env.cr.commit() if current % 100 == 0 or total - current == 0: _logger.info( - 'attachment %s/%s after %.2fs', - current, total, - time.time() - start_time + "attachment %s/%s after %.2fs", + current, + total, + time.time() - start_time, ) @api.model def _force_storage_to_object_storage(self, new_cr=False): - _logger.info('migrating files to the object storage') - storage = self.env.context.get('storage_location') or self._storage() + _logger.info("migrating files to the object storage") + storage = self.env.context.get("storage_location") or self._storage() if self.is_storage_disabled(storage): return # The weird "res_field = False OR res_field != False" domain @@ -392,16 +396,19 @@ class IrAttachment(models.Model): # which adds ('res_field', '=', False) when the domain does not # contain 'res_field'. # https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/odoo/addons/base/ir/ir_attachment.py#L344-L347 - domain = ['!', ('store_fname', '=like', '{}://%'.format(storage)), - '|', - ('res_field', '=', False), - ('res_field', '!=', False)] + 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 by default because it causes # serialization issues due to concurrent updates on attachments during # the installation with self.do_in_new_env(new_cr=new_cr) as new_env: - model_env = new_env['ir.attachment'] + model_env = new_env["ir.attachment"] ids = model_env.search(domain).ids files_to_clean = [] for attachment_id in ids: @@ -410,12 +417,14 @@ class IrAttachment(models.Model): # check that no other transaction has # locked the row, don't send a file to storage # in that case - self.env.cr.execute("SELECT id " - "FROM ir_attachment " - "WHERE id = %s " - "FOR UPDATE NOWAIT", - (attachment_id,), - log_exceptions=False) + 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 @@ -428,17 +437,16 @@ class IrAttachment(models.Model): if path: files_to_clean.append(path) except psycopg2.OperationalError: - _logger.error('Could not migrate attachment %s to S3', - attachment_id) - - def clean(): - clean_fs(files_to_clean) + _logger.error( + "Could not migrate attachment %s to S3", attachment_id + ) # delete the files from the filesystem once we know the changes # have been committed in ir.attachment if files_to_clean: - new_env.cr.after('commit', clean) + new_env.cr.commit() + clean_fs(files_to_clean) def _get_stores(self): - """ To get the list of stores activated in the system """ + """To get the list of stores activated in the system""" return [] diff --git a/requirements.txt b/requirements.txt index 0525441..4ac6ee1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,12 +1,13 @@ -azure-storage-blob==12.8.1 -azure-identity==1.6.0 -boto3==1.9.102 -redis==2.10.5 -python-json-logger==0.1.5 -statsd==3.2.1 -python-swiftclient==3.9.0 -python-keystoneclient==3.22.0 -keystoneauth1==3.14.0 +azure-storage-blob==12.14.1 +azure-identity==1.12.0 +boto3==1.26.7 +redis==4.3.4 +python-json-logger==2.0.4 +statsd==4.0.1 +python-swiftclient==4.1.0 +python-keystoneclient==5.0.1 +keystoneauth1==5.0.0 # error with 5.x (ConstructorError: could not determine a constructor for the tag '!record') -PyYAML==4.2b4 -prometheus_client==0.11.0 +PyYAML==6.0 +prometheus_client==0.15.0 +pyopenssl==22.1.0