diff --git a/openerp/addons/base/ir/ir_attachment.py b/openerp/addons/base/ir/ir_attachment.py index c636463be10..d3ecfd1b2c7 100644 --- a/openerp/addons/base/ir/ir_attachment.py +++ b/openerp/addons/base/ir/ir_attachment.py @@ -138,9 +138,9 @@ class ir_attachment(osv.osv): return fname def _file_delete(self, cr, uid, fname): - count = self.search(cr, 1, [('store_fname','=',fname)], count=True) + count = self.search_count(cr, 1, [('store_fname','=',fname)]) full_path = self._full_path(cr, uid, fname) - if count <= 1 and os.path.exists(full_path): + if not count and os.path.exists(full_path): try: os.unlink(full_path) except OSError: @@ -170,14 +170,18 @@ class ir_attachment(osv.osv): location = self._storage(cr, uid, context) file_size = len(value.decode('base64')) attach = self.browse(cr, uid, id, context=context) - if attach.store_fname: - self._file_delete(cr, uid, attach.store_fname) + fname_to_delete = attach.store_fname if location != 'db': fname = self._file_write(cr, uid, value) # SUPERUSER_ID as probably don't have write access, trigger during create super(ir_attachment, self).write(cr, SUPERUSER_ID, [id], {'store_fname': fname, 'file_size': file_size, 'db_datas': False}, context=context) else: super(ir_attachment, self).write(cr, SUPERUSER_ID, [id], {'db_datas': value, 'file_size': file_size, 'store_fname': False}, context=context) + + # After de-referencing the file in the database, check whether we need + # to garbage-collect it on the filesystem + if fname_to_delete: + self._file_delete(cr, uid, fname_to_delete) return True _name = 'ir.attachment' @@ -315,10 +319,19 @@ class ir_attachment(osv.osv): if isinstance(ids, (int, long)): ids = [ids] self.check(cr, uid, ids, 'unlink', context=context) - for attach in self.browse(cr, uid, ids, context=context): - if attach.store_fname: - self._file_delete(cr, uid, attach.store_fname) - return super(ir_attachment, self).unlink(cr, uid, ids, context) + + # First delete in the database, *then* in the filesystem if the + # database allowed it. Helps avoid errors when concurrent transactions + # are deleting the same file, and some of the transactions are + # rolled back by PostgreSQL (due to concurrent updates detection). + to_delete = [a.store_fname + for a in self.browse(cr, uid, ids, context=context) + if a.store_fname] + res = super(ir_attachment, self).unlink(cr, uid, ids, context) + for file_path in to_delete: + self._file_delete(cr, uid, file_path) + + return res def create(self, cr, uid, values, context=None): self.check(cr, uid, [], mode='write', context=context, values=values)