feat: remove after method (#393)

* fix: azure reading in stream monkey patch documents
This commit is contained in:
Vincent Renaville
2022-11-11 15:17:03 +01:00
committed by GitHub
co-authored by GitHub
parent 988d4906bf
commit fc452c6a2a
5 changed files with 194 additions and 122 deletions
+1
View File
@@ -2,3 +2,4 @@
# Copyright 2021 Open Source Integrators # Copyright 2021 Open Source Integrators
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
from . import ir_attachment from . import ir_attachment
from . import ir_binary
+9 -12
View File
@@ -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 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") running_env = os.environ.get("RUNNING_ENV", "dev")
storage_name = os.environ.get('AZURE_STORAGE_NAME', r'{env}-{db}') storage_name = os.environ.get("AZURE_STORAGE_NAME", r"{env}-{db}")
storage_name = storage_name.format( storage_name = storage_name.format(env=running_env, db=self.env.cr.dbname)
env=running_env,
db=self.env.cr.dbname
)
# replace invalid characters by _ # replace invalid characters by _
storage_name = re.sub(r"[\W_]+", "-", storage_name) storage_name = re.sub(r"[\W_]+", "-", storage_name)
# lowercase, max 63 chars # lowercase, max 63 chars
@@ -136,7 +133,7 @@ class IrAttachment(models.Model):
except exceptions.UserError: except exceptions.UserError:
_logger.exception( _logger.exception(
"error accessing to storage '%s' please check credentials ", "error accessing to storage '%s' please check credentials ",
container_name container_name,
) )
return False return False
container_client = blob_service_client.get_container_client(container_name) 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): def _store_file_read(self, fname, bin_size=False):
if fname.startswith("azure://"): if fname.startswith("azure://"):
key = fname.replace("azure://", "", 1).lower() key = fname.replace("azure://", "", 1).lower()
if '/' in key: if "/" in key:
container_name, key = key.split('/', 1) container_name, key = key.split("/", 1)
else: else:
container_name = None container_name = None
container_client = self._get_azure_container(container_name) container_client = self._get_azure_container(container_name)
# if container cannot be retrived, abort reading from azure storage # if container cannot be retrived, abort reading from azure storage
if not container_client: if not container_client:
return '' return ""
try: try:
blob_client = container_client.get_blob_client(key) blob_client = container_client.get_blob_client(key)
read = blob_client.download_blob().readall() read = blob_client.download_blob().readall()
@@ -200,13 +197,13 @@ class IrAttachment(models.Model):
def _store_file_delete(self, fname): def _store_file_delete(self, fname):
if fname.startswith("azure://"): if fname.startswith("azure://"):
key = fname.replace("azure://", "", 1).lower() key = fname.replace("azure://", "", 1).lower()
if '/' in key: if "/" in key:
container_name, key = key.split('/', 1) container_name, key = key.split("/", 1)
else: else:
container_name = None container_name = None
container_client = self._get_azure_container(container_name) container_client = self._get_azure_container(container_name)
if not container_client: if not container_client:
return '' return ""
# delete the file only if it is on the current configured container # delete the file only if it is on the current configured container
# otherwise, we might delete files used on a different environment # otherwise, we might delete files used on a different environment
try: try:
+65
View File
@@ -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
@@ -20,38 +20,36 @@ _logger = logging.getLogger(__name__)
def is_true(strval): def is_true(strval):
return bool(strtobool(strval or '0')) return bool(strtobool(strval or "0"))
def clean_fs(files): def clean_fs(files):
_logger.info('cleaning old files from filestore') _logger.info("cleaning old files from filestore")
for full_path in files: for full_path in files:
if os.path.exists(full_path): if os.path.exists(full_path):
try: try:
os.unlink(full_path) os.unlink(full_path)
except OSError: except OSError:
_logger.info( _logger.info(
"_file_delete could not unlink %s", "_file_delete could not unlink %s", full_path, exc_info=True
full_path, exc_info=True
) )
except IOError: except IOError:
# Harmless and needed for race conditions # Harmless and needed for race conditions
_logger.info( _logger.info(
"_file_delete could not unlink %s", "_file_delete could not unlink %s", full_path, exc_info=True
full_path, exc_info=True
) )
class IrAttachment(models.Model): class IrAttachment(models.Model):
_inherit = 'ir.attachment' _inherit = "ir.attachment"
@staticmethod @staticmethod
def is_storage_disabled(storage=None, log=True): def is_storage_disabled(storage=None, log=True):
msg = _("Storages are disabled (see environment configuration).") msg = _("Storages are disabled (see environment configuration).")
if storage: if storage:
msg = _( msg = _("Storage '%s' is disabled (see environment configuration).") % (
"Storage '%s' is disabled (see environment configuration)." storage,
) % (storage,) )
is_disabled = is_true(os.environ.get("DISABLE_ATTACHMENT_STORAGE")) is_disabled = is_true(os.environ.get("DISABLE_ATTACHMENT_STORAGE"))
if is_disabled and log: if is_disabled and log:
_logger.warning(msg) _logger.warning(msg)
@@ -59,7 +57,7 @@ class IrAttachment(models.Model):
def _register_hook(self): def _register_hook(self):
super()._register_hook() 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 # ignore if we are not using an object storage
if location not in self._get_stores(): if location not in self._get_stores():
return return
@@ -73,7 +71,7 @@ class IrAttachment(models.Model):
# done during the initialization. We need to move the attachments that # done during the initialization. We need to move the attachments that
# could have been created or updated in other addons before this addon # could have been created or updated in other addons before this addon
# was loaded # 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 # We need to call the migration on the loading of the model because
# when we are upgrading addons, some of them might add attachments. # 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 # Typical example is images of ir.ui.menu which are updated in
# ir.attachment at every upgrade of the addons # ir.attachment at every upgrade of the addons
if update_module: if update_module:
self.env['ir.attachment'].sudo()._force_storage_to_object_storage() self.env["ir.attachment"].sudo()._force_storage_to_object_storage()
@property @property
def _object_storage_default_force_db_config(self): def _object_storage_default_force_db_config(self):
return {"image/": 51200, "application/javascript": 0, "text/css": 0} return {"image/": 51200, "application/javascript": 0, "text/css": 0}
def _get_storage_force_db_config(self): def _get_storage_force_db_config(self):
param = self.env['ir.config_parameter'].sudo().get_param( param = (
'ir_attachment.storage.force.database', self.env["ir.config_parameter"]
.sudo()
.get_param(
"ir_attachment.storage.force.database",
)
) )
storage_config = None storage_config = None
if param: if param:
@@ -100,7 +102,8 @@ class IrAttachment(models.Model):
_logger.exception( _logger.exception(
"Could not parse system parameter" "Could not parse system parameter"
" 'ir_attachment.storage.force.database', reverting to the" " 'ir_attachment.storage.force.database', reverting to the"
" default configuration.") " default configuration."
)
if not storage_config: if not storage_config:
storage_config = self._object_storage_default_force_db_config storage_config = self._object_storage_default_force_db_config
@@ -180,17 +183,17 @@ class IrAttachment(models.Model):
return False return False
def _get_datas_related_values(self, data, mimetype): 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 data and storage in self._get_stores():
if self._store_in_db_instead_of_object_storage(data, mimetype): if self._store_in_db_instead_of_object_storage(data, mimetype):
# compute the fields that depend on datas # compute the fields that depend on datas
bin_data = data bin_data = data
values = { values = {
'file_size': len(bin_data), "file_size": len(bin_data),
'checksum': self._compute_checksum(bin_data), "checksum": self._compute_checksum(bin_data),
'index_content': self._index(bin_data, mimetype), "index_content": self._index(bin_data, mimetype),
'store_fname': False, "store_fname": False,
'db_datas': data, "db_datas": data,
} }
return values return values
return super()._get_datas_related_values(data, mimetype) return super()._get_datas_related_values(data, mimetype)
@@ -203,28 +206,22 @@ class IrAttachment(models.Model):
return super()._file_read(fname) return super()._file_read(fname)
def _store_file_read(self, fname): def _store_file_read(self, fname):
storage = fname.partition('://')[0] storage = fname.partition("://")[0]
raise NotImplementedError( raise NotImplementedError("No implementation for %s" % (storage,))
'No implementation for %s' % (storage,)
)
def _store_file_write(self, key, bin_data): def _store_file_write(self, key, bin_data):
storage = self.storage() storage = self.storage()
raise NotImplementedError( raise NotImplementedError("No implementation for %s" % (storage,))
'No implementation for %s' % (storage,)
)
def _store_file_delete(self, fname): def _store_file_delete(self, fname):
storage = fname.partition('://')[0] storage = fname.partition("://")[0]
raise NotImplementedError( raise NotImplementedError("No implementation for %s" % (storage,))
'No implementation for %s' % (storage,)
)
@api.model @api.model
def _file_write(self, bin_data, checksum): 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(): 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: if not key:
key = self._compute_checksum(bin_data) key = self._compute_checksum(bin_data)
filename = self._store_file_write(key, bin_data) filename = self._store_file_write(key, bin_data)
@@ -238,8 +235,9 @@ class IrAttachment(models.Model):
cr = self.env.cr cr = self.env.cr
# using SQL to include files hidden through unlink or due to record # using SQL to include files hidden through unlink or due to record
# rules # rules
cr.execute("SELECT COUNT(*) FROM ir_attachment " cr.execute(
"WHERE store_fname = %s", (fname,)) "SELECT COUNT(*) FROM ir_attachment " "WHERE store_fname = %s", (fname,)
)
count = cr.fetchone()[0] count = cr.fetchone()[0]
if not count: if not count:
self._store_file_delete(fname) self._store_file_delete(fname)
@@ -251,7 +249,7 @@ class IrAttachment(models.Model):
for store_name in self._get_stores(): for store_name in self._get_stores():
if self.is_storage_disabled(store_name): if self.is_storage_disabled(store_name):
continue continue
uri = '{}://'.format(store_name) uri = "{}://".format(store_name)
if fname.startswith(uri): if fname.startswith(uri):
return True return True
return False return False
@@ -264,9 +262,7 @@ class IrAttachment(models.Model):
""" """
with api.Environment.manage(): with api.Environment.manage():
if new_cr: if new_cr:
registry = odoo.modules.registry.Registry.new( registry = odoo.modules.registry.Registry.new(self.env.cr.dbname)
self.env.cr.dbname
)
with closing(registry.cursor()) as cr: with closing(registry.cursor()) as cr:
try: try:
yield self.env(cr=cr) yield self.env(cr=cr)
@@ -283,33 +279,38 @@ class IrAttachment(models.Model):
def _move_attachment_to_store(self): def _move_attachment_to_store(self):
self.ensure_one() 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 fname = self.store_fname
storage = fname.partition('://')[0] storage = fname.partition("://")[0]
if self.is_storage_disabled(storage): if self.is_storage_disabled(storage):
fname = False fname = False
if fname: if fname:
# migrating from filesystem filestore # migrating from filesystem filestore
# or from the old 'store_fname' without the bucket name # or from the old 'store_fname' without the bucket name
_logger.info('moving %s on the object storage', fname) _logger.info("moving %s on the object storage", fname)
self.write({'datas': self.datas, self.write(
{
"datas": self.datas,
# this is required otherwise the # this is required otherwise the
# mimetype gets overriden with # mimetype gets overriden with
# 'application/octet-stream' # 'application/octet-stream'
# on assets # on assets
'mimetype': self.mimetype}) "mimetype": self.mimetype,
_logger.info('moved %s on the object storage', fname) }
)
_logger.info("moved %s on the object storage", fname)
return self._full_path(fname) return self._full_path(fname)
elif self.db_datas: elif self.db_datas:
_logger.info('moving on the object storage from database') _logger.info("moving on the object storage from database")
self.write({'datas': self.datas}) self.write({"datas": self.datas})
@api.model @api.model
def force_storage(self): 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( raise exceptions.AccessError(
_('Only administrators can execute this action.')) _("Only administrators can execute this action.")
location = self.env.context.get('storage_location') or self._storage() )
location = self.env.context.get("storage_location") or self._storage()
if location not in self._get_stores(): if location not in self._get_stores():
return super().force_storage() return super().force_storage()
self._force_storage_to_object_storage() self._force_storage_to_object_storage()
@@ -335,30 +336,32 @@ class IrAttachment(models.Model):
if storage not in self._get_stores(): if storage not in self._get_stores():
return return
domain = AND(( domain = AND(
(
normalize_domain( normalize_domain(
[('store_fname', '=like', '{}://%'.format(storage)), [
("store_fname", "=like", "{}://%".format(storage)),
# for res_field, see comment in # for res_field, see comment in
# _force_storage_to_object_storage # _force_storage_to_object_storage
'|', "|",
('res_field', '=', False), ("res_field", "=", False),
('res_field', '!=', False), ("res_field", "!=", False),
] ]
), ),
normalize_domain(self._store_in_db_instead_of_object_storage_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: with self.do_in_new_env(new_cr=new_cr) as new_env:
model_env = new_env['ir.attachment'].with_context( model_env = new_env["ir.attachment"].with_context(prefetch_fields=False)
prefetch_fields=False
)
attachment_ids = model_env.search(domain).ids attachment_ids = model_env.search(domain).ids
if not attachment_ids: if not attachment_ids:
return return
total = len(attachment_ids) total = len(attachment_ids)
start_time = time.time() start_time = time.time()
_logger.info('Moving %d attachments from %s to' _logger.info(
' DB for fast access', total, storage) "Moving %d attachments from %s to" " DB for fast access", total, storage
)
current = 0 current = 0
for attachment_id in attachment_ids: for attachment_id in attachment_ids:
current += 1 current += 1
@@ -370,21 +373,22 @@ class IrAttachment(models.Model):
# this write will read the datas from the Object Storage and # this write will read the datas from the Object Storage and
# write them back in the DB (the logic for location to write is # write them back in the DB (the logic for location to write is
# in the 'datas' inverse computed field) # 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, # as the file will potentially be dropped on the bucket,
# we should commit the changes here # we should commit the changes here
new_env.cr.commit() new_env.cr.commit()
if current % 100 == 0 or total - current == 0: if current % 100 == 0 or total - current == 0:
_logger.info( _logger.info(
'attachment %s/%s after %.2fs', "attachment %s/%s after %.2fs",
current, total, current,
time.time() - start_time total,
time.time() - start_time,
) )
@api.model @api.model
def _force_storage_to_object_storage(self, new_cr=False): def _force_storage_to_object_storage(self, new_cr=False):
_logger.info('migrating files to the object storage') _logger.info("migrating files to the object storage")
storage = self.env.context.get('storage_location') or self._storage() storage = self.env.context.get("storage_location") or self._storage()
if self.is_storage_disabled(storage): if self.is_storage_disabled(storage):
return return
# The weird "res_field = False OR res_field != False" domain # 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 # which adds ('res_field', '=', False) when the domain does not
# contain 'res_field'. # contain 'res_field'.
# https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/odoo/addons/base/ir/ir_attachment.py#L344-L347 # https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/odoo/addons/base/ir/ir_attachment.py#L344-L347
domain = ['!', ('store_fname', '=like', '{}://%'.format(storage)), domain = [
'|', "!",
('res_field', '=', False), ("store_fname", "=like", "{}://%".format(storage)),
('res_field', '!=', False)] "|",
("res_field", "=", False),
("res_field", "!=", False),
]
# We do a copy of the environment so we can workaround the cache issue # 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 # below. We do not create a new cursor by default because it causes
# serialization issues due to concurrent updates on attachments during # serialization issues due to concurrent updates on attachments during
# the installation # the installation
with self.do_in_new_env(new_cr=new_cr) as new_env: 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 ids = model_env.search(domain).ids
files_to_clean = [] files_to_clean = []
for attachment_id in ids: for attachment_id in ids:
@@ -410,12 +417,14 @@ class IrAttachment(models.Model):
# check that no other transaction has # check that no other transaction has
# locked the row, don't send a file to storage # locked the row, don't send a file to storage
# in that case # in that case
self.env.cr.execute("SELECT id " self.env.cr.execute(
"SELECT id "
"FROM ir_attachment " "FROM ir_attachment "
"WHERE id = %s " "WHERE id = %s "
"FOR UPDATE NOWAIT", "FOR UPDATE NOWAIT",
(attachment_id,), (attachment_id,),
log_exceptions=False) log_exceptions=False,
)
# This is a trick to avoid having the 'datas' # This is a trick to avoid having the 'datas'
# function fields computed for every attachment on # function fields computed for every attachment on
@@ -428,16 +437,15 @@ class IrAttachment(models.Model):
if path: if path:
files_to_clean.append(path) files_to_clean.append(path)
except psycopg2.OperationalError: except psycopg2.OperationalError:
_logger.error('Could not migrate attachment %s to S3', _logger.error(
attachment_id) "Could not migrate attachment %s to S3", attachment_id
)
def clean():
clean_fs(files_to_clean)
# delete the files from the filesystem once we know the changes # delete the files from the filesystem once we know the changes
# have been committed in ir.attachment # have been committed in ir.attachment
if files_to_clean: if files_to_clean:
new_env.cr.after('commit', clean) new_env.cr.commit()
clean_fs(files_to_clean)
def _get_stores(self): def _get_stores(self):
"""To get the list of stores activated in the system""" """To get the list of stores activated in the system"""
+12 -11
View File
@@ -1,12 +1,13 @@
azure-storage-blob==12.8.1 azure-storage-blob==12.14.1
azure-identity==1.6.0 azure-identity==1.12.0
boto3==1.9.102 boto3==1.26.7
redis==2.10.5 redis==4.3.4
python-json-logger==0.1.5 python-json-logger==2.0.4
statsd==3.2.1 statsd==4.0.1
python-swiftclient==3.9.0 python-swiftclient==4.1.0
python-keystoneclient==3.22.0 python-keystoneclient==5.0.1
keystoneauth1==3.14.0 keystoneauth1==5.0.0
# error with 5.x (ConstructorError: could not determine a constructor for the tag '!record') # error with 5.x (ConstructorError: could not determine a constructor for the tag '!record')
PyYAML==4.2b4 PyYAML==6.0
prometheus_client==0.11.0 prometheus_client==0.15.0
pyopenssl==22.1.0