From 4ec4ca4a835501c4e883a02862ad81d3dfa73e11 Mon Sep 17 00:00:00 2001
From: Julien Castiaux <juc@odoo.com>
Date: Mon, 18 Jul 2022 16:05:42 +0000
Subject: [PATCH] [IMP] base: cover filestore gc with tests

closes odoo/odoo#96242

Signed-off-by: Raphael Collet <rco@odoo.com>
---
 odoo/addons/base/models/ir_attachment.py     | 14 ++++++----
 odoo/addons/base/tests/test_ir_attachment.py | 29 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py
index 454720c6a64..a308b012264 100644
--- a/odoo/addons/base/models/ir_attachment.py
+++ b/odoo/addons/base/models/ir_attachment.py
@@ -174,6 +174,12 @@ class IrAttachment(models.Model):
         cr.execute("SET LOCAL lock_timeout TO '10s'")
         cr.execute("LOCK ir_attachment IN SHARE MODE")
 
+        self._gc_file_store_unsafe()
+
+        # commit to release the lock
+        cr.commit()
+
+    def _gc_file_store_unsafe(self):
         # retrieve the file names from the checklist
         checklist = {}
         for dirpath, _, filenames in os.walk(self._full_path('checklist')):
@@ -185,10 +191,10 @@ class IrAttachment(models.Model):
         # Clean up the checklist. The checklist is split in chunks and files are garbage-collected
         # for each chunk.
         removed = 0
-        for names in cr.split_for_in_conditions(checklist):
+        for names in self.env.cr.split_for_in_conditions(checklist):
             # determine which files to keep among the checklist
-            cr.execute("SELECT store_fname FROM ir_attachment WHERE store_fname IN %s", [names])
-            whitelist = set(row[0] for row in cr.fetchall())
+            self.env.cr.execute("SELECT store_fname FROM ir_attachment WHERE store_fname IN %s", [names])
+            whitelist = set(row[0] for row in self.env.cr.fetchall())
 
             # remove garbage files, and clean up checklist
             for fname in names:
@@ -203,8 +209,6 @@ class IrAttachment(models.Model):
                 with tools.ignore(OSError):
                     os.unlink(filepath)
 
-        # commit to release the lock
-        cr.commit()
         _logger.info("filestore gc %d checked, %d removed", len(checklist), removed)
 
     @api.depends('store_fname', 'db_datas', 'file_size')
diff --git a/odoo/addons/base/tests/test_ir_attachment.py b/odoo/addons/base/tests/test_ir_attachment.py
index 960775456c0..7146629e8f3 100644
--- a/odoo/addons/base/tests/test_ir_attachment.py
+++ b/odoo/addons/base/tests/test_ir_attachment.py
@@ -7,6 +7,7 @@ import os
 
 from PIL import Image
 
+import odoo
 from odoo.exceptions import AccessError
 from odoo.tests.common import TransactionCase
 from odoo.tools import image_to_base64
@@ -231,6 +232,34 @@ class TestIrAttachment(TransactionCase):
         self.assertEqual(document3.store_fname, self.blob1_fname)
         self.assertEqual(document3.checksum, self.blob1_hash)
 
+    def test_12_gc(self):
+        # the data needs to be unique so that no other attachment link
+        # the file so that the gc removes it
+        unique_blob = os.urandom(16)
+        a1 = self.Attachment.create({'name': 'a1', 'raw': unique_blob})
+        store_path = os.path.join(self.filestore, a1.store_fname)
+        self.assertTrue(os.path.isfile(store_path), 'file exists')
+        a1.unlink()
+        self.Attachment._gc_file_store_unsafe()
+        self.assertFalse(os.path.isfile(store_path), 'file removed')
+
+    def test_13_rollback(self):
+        self.registry.enter_test_mode(self.cr)
+        self.addCleanup(self.registry.leave_test_mode)
+        self.cr = self.registry.cursor()
+        self.addCleanup(self.cr.close)
+        self.env = odoo.api.Environment(self.cr, odoo.SUPERUSER_ID, {})
+
+        # the data needs to be unique so that no other attachment link
+        # the file so that the gc removes it
+        unique_blob = os.urandom(16)
+        a1 = self.Attachment.create({'name': 'a1', 'raw': unique_blob})
+        store_path = os.path.join(self.filestore, a1.store_fname)
+        self.assertTrue(os.path.isfile(store_path), 'file exists')
+        self.env.cr.rollback()
+        self.Attachment._gc_file_store_unsafe()
+        self.assertFalse(os.path.isfile(store_path), 'file removed')
+
 
 class TestPermissions(TransactionCase):
     def setUp(self):