[17.0][IMP] auto_backup_fs_file: GC-based cleanup, folder visibility, regression test

auto_backup_fs_file/README.rst

@@ -1,7 +1,3 @@
-.. image:: https://odoo-community.org/readme-banner-image
-   :target: https://odoo-community.org/get-involved?utm_source=readme
-   :alt: Odoo Community Association
-
 ===================
 Auto Backup Fs File
 ===================
@@ -17,7 +13,7 @@ Auto Backup Fs File
 .. |badge1| image:: https://img.shields.io/badge/maturity-Alpha-red.png
     :target: https://odoo-community.org/page/development-status
     :alt: Alpha
-.. |badge2| image:: https://img.shields.io/badge/license-AGPL--3-blue.png
+.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png
     :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
     :alt: License: AGPL-3
 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fserver--tools-lightgray.png?logo=github
@@ -69,12 +65,12 @@ network drives, or other custom filesystems supported by ``fsspec``.
 
 Practical examples include:
 
-- Backing up Odoo data to cloud storage providers like AWS S3, Google
-  Cloud Storage, or Azure Blob Storage.
-- Storing backups on a secure local or remote filesystem for disaster
-  recovery purposes.
-- Automating backup processes in multi-environment setups, such as
-  multi-company or multi-website configurations.
+-  Backing up Odoo data to cloud storage providers like AWS S3, Google
+   Cloud Storage, or Azure Blob Storage.
+-  Storing backups on a secure local or remote filesystem for disaster
+   recovery purposes.
+-  Automating backup processes in multi-environment setups, such as
+   multi-company or multi-website configurations.
 
 APPROACH: The module extends the backup functionality from the
 ``auto_backup`` module by introducing a method that allows storing the
@@ -87,18 +83,27 @@ exporting Odoo instance data and storing it in the specified filesystem.
 Additionally, it allows users to download the backups for local storage
 or further processing.
 
+Backup file cleanup is handled automatically based on the **Days to
+Keep** configuration. When expired backup records are removed, the
+physical backup files are not deleted synchronously. Instead, the module
+delegates file deletion to the ``fs_attachment`` garbage collector (GC),
+which marks files for deferred removal and physically deletes them
+during Odoo's autovacuum cron cycle. This two-phase approach ensures
+transactional safety: files are only removed once the GC confirms no
+database record still references them.
+
 USEFUL INFORMATION:
 
-- **Dependencies**: This module depends on the ``fsspec`` library, its
-  relevant filesystem implementations, and the ``fs_file`` addon from
-  OCA/storage. Ensure the required ``fsspec`` plugins are installed for
-  your target filesystem.
+-  **Dependencies**: This module depends on the ``fsspec`` library, its
+   relevant filesystem implementations, and the ``fs_file`` addon from
+   OCA/storage. Ensure the required ``fsspec`` plugins are installed for
+   your target filesystem.
 
 Installation
 ============
 
 This addon itself does not introduce any dependencies, but its
-dependencies may require additional packages.:wa
+dependencies may require additional packages.
 
 Configuration
 =============
@@ -107,37 +112,36 @@ Configuration
    module, ensure you have reviewed the documentation for the following
    modules:
 
-- ``fs_attachment``
-- ``fs_storage`` These modules provide the necessary setup for file
-  storage and attachment handling.
+-  ``fs_attachment``
+-  ``fs_storage`` These modules provide the necessary setup for file
+   storage and attachment handling.
 
 2. **Configure File Storage**
 
-- Navigate to **Settings** > **Technical** > **FS Storage**.
-- Create or select an existing storage configuration.
-- Ensure the storage is properly set up and tested for accessibility.
+-  Navigate to **Settings** > **Technical** > **FS Storage**.
+-  Create or select an existing storage configuration.
+-  Ensure the storage is properly set up and tested for accessibility.
 
 3. **Link Backup File field to Storage**
 
-- While configuring the file storage in **Settings** > **Technical** >
-  **FS Storage**, ensure that the ``backup_file`` from the
-  ``db.backup.fs.file`` model is listed under the ``Field`` field.
-- This step is part of the storage configuration process.
-- Save the changes after verifying the setup.
+-  While configuring the file storage in **Settings** > **Technical** >
+   **FS Storage**, ensure that the ``backup_file`` from the
+   ``db.backup.fs.file`` model is listed under the ``Field`` field.
+-  This step is part of the storage configuration process.
+-  Save the changes after verifying the setup.
 
-|Example of File Storage Configuration|
+.. image:: https://raw.githubusercontent.com/OCA/server-tools/17.0/auto_backup_fs_file/images/file_storage_configuration.png
+   :alt: Example of File Storage Configuration
 
 4. **Verify Configuration**
 
-- Perform a test backup to ensure the files are being stored in the
-  correct location.
-- Check the logs for any errors or warnings.
+-  Perform a test backup to ensure the files are being stored in the
+   correct location.
+-  Check the logs for any errors or warnings.
 
 By following these steps, you will ensure that the module is properly
 configured for storing backups in the desired file storage system.
 
-.. |Example of File Storage Configuration| image:: https://raw.githubusercontent.com/OCA/server-tools/17.0/auto_backup_fs_file/images/file_storage_configuration.png
-
 Usage
 =====
 
@@ -179,43 +183,68 @@ How to Use the Module
 4. Manage Fs File Backups
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- In the Fs File backups list view, you can see details such as the
-  backup filename and associated database backup configuration.
-- Use this view to manage or download backups as needed.
+-  In the Fs File backups list view, you can see details such as the
+   backup filename and associated database backup configuration.
+-  Use this view to manage or download backups as needed.
+
+5. Cleanup and File Deletion
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Backup retention is controlled by the **Days to Keep** field on the
+backup configuration. When this value is greater than 0, the automatic
+cleanup process removes expired backup records during each backup run.
+
+When a backup record is deleted (either by automatic cleanup or manually
+from the list view), the physical backup file in the filesystem storage
+is **not removed immediately**. Instead, the file is marked for deferred
+deletion by the ``fs_attachment`` garbage collector (GC), which runs
+periodically via Odoo's autovacuum cron. Physical files are only removed
+from the storage backend once the GC confirms no database record
+references them.
+
+This means:
+
+-  **Immediately after deletion**: the database record is gone, but the
+   file may still exist in the storage backend for a short period.
+-  **After the next autovacuum cycle**: the file is permanently deleted
+   from the storage backend.
+
+This behavior requires the storage's ``autovacuum_gc`` flag to be
+enabled (the default). If disabled, files must be managed manually.
 
 Screenshots
 ~~~~~~~~~~~
 
-- **Backup Configuration Form View** |Backup Configuration Form|
+-  **Backup Configuration Form View** |Backup Configuration Form|
 
-- **Fs File Backups List View** |Fs File Backups List|
+-  **Fs File Backups List View** |Fs File Backups List|
 
 Notes
 ~~~~~
 
-- Ensure that the FSSPEC storage is properly configured before using the
-  **Fs File** method.
-- This module adds a new stat button in the backup configuration form
-  view to quickly access Fs File backups.
+-  Ensure that the FSSPEC storage is properly configured before using
+   the **Fs File** method.
+-  This module adds a new stat button in the backup configuration form
+   view to quickly access Fs File backups.
 
 .. |Backup Configuration Form| image:: https://raw.githubusercontent.com/OCA/server-tools/17.0/auto_backup_fs_file/static/description/db_backup_form_view.png
 .. |Fs File Backups List| image:: https://raw.githubusercontent.com/OCA/server-tools/17.0/auto_backup_fs_file/static/description/db_backup_fs_file_tree_view.png
 
 Known issues / Roadmap
 ======================
 
-- **Folder field behavior**: The ``folder`` field on the ``db.backup``
-  model specifies the backup storage directory. For records using the
-  ``fs_file`` method, storage is actually controlled by the ``fs_file``
-  field's settings. However, since ``folder`` is currently a required
-  non-computed field in the ``auto_backup`` addon, modifications to sync
-  these two fields are not performed. Future versions may add this
-  synchronization support.
+-  **Folder field behavior**: The ``folder`` field on the ``db.backup``
+   model specifies the backup storage directory. For records using the
+   ``fs_file`` method, storage is controlled by the ``fs_file`` field's
+   settings. The ``folder`` field is hidden (``invisible``) and no
+   longer required when ``method='fs_file'``, so it no longer interferes
+   with ``fs_file`` configurations. No auto-sync between both fields is
+   performed since the methods are mutually exclusive.
 
-- **Design limitation**: The current implementation has a design
-  constraint due to ``fs_storage`` addon limitations. Since storage
-  setting targets the ``db.backup.fs.file`` model, only one storage
-  backend can effectively be used.
+-  **Design limitation**: The current implementation has a design
+   constraint due to ``fs_storage`` addon limitations. Since storage
+   setting targets the ``db.backup.fs.file`` model, only one storage
+   backend can effectively be used.
 
 Bug Tracker
 ===========
@@ -238,14 +267,14 @@ Authors
 Contributors
 ------------
 
-- Rolando Pérez Rebollo r.perez@binhex.cloud
+-  Rolando Pérez Rebollo r.perez@binhex.cloud
 
 Other credits
 -------------
 
 The development of this module has been financially supported by:
 
-- Binhex
+-  Binhex
 
 Maintainers
 -----------

auto_backup_fs_file/models/db_backup.py

@@ -27,6 +27,17 @@ class DbBackup(models.Model):
 
     responsible_id = fields.Many2one("res.users", help="User to be notified.")
 
+    folder = fields.Char(required=False, default=False)
+
+    @api.constrains("method", "folder")
+    def _check_folder_required_for_method(self):
+        """folder is only required for local and sftp methods, not fs_file."""
+        for record in self:
+            if record.method and record.method != "fs_file" and not record.folder:
+                raise ValidationError(
+                    _("Folder is required for local and SFTP backup methods.")
+                )
+
     @api.model
     def _get_fs_storage(self):
         """Get the fs_storage to be used for fs_file backups."""
@@ -63,6 +74,7 @@ def _check_fs_file_backup_storage(self):
     def action_backup(self):
         """Override the action_backup method to add the fs_file method."""
         fs_backups = self.filtered(lambda it: it.method == "fs_file")
+        successful_fs = self.browse()
         dbname = self.env.cr.dbname
         for fs_backup in fs_backups:
             with fs_backup.backup_log():
@@ -103,7 +115,9 @@ def action_backup(self):
                         summary=_("Database backup is ready to download."),
                         user_id=user_to_notify.id,
                     )
+                successful_fs |= fs_backup
         res = super().action_backup()
+        successful_fs.cleanup()
         return res
 
     def action_open_fs_backups_view(self):
@@ -115,14 +129,18 @@ def action_open_fs_backups_view(self):
         return action
 
     def cleanup(self):
-        """Extend cleanup to fs_file backups."""
+        """Extend cleanup to fs_file backups.
+
+        Physical file removal is handled by the fs_attachment GC stack:
+        unlink() on db.backup.fs.file cascades to ir.attachment.unlink(),
+        which marks the file in fs.file.gc for deferred deletion by the
+        autovacuum job.
+        """
         for db_backup_conf in self.filtered(
             lambda record: record.method == "fs_file" and record.days_to_keep
         ):
             with db_backup_conf.cleanup_log():
                 to_delete = db_backup_conf.fs_file_backup_ids.filtered("is_expired")
-                for backup in to_delete:
-                    self._get_fs_storage().fs.rm_file(backup.get_fs_storage_filename())
                 to_delete.unlink()
         res = super().cleanup()
         return res

auto_backup_fs_file/models/db_backup_fs_file.py

@@ -22,6 +22,7 @@ class DbBackupFsFile(models.Model):
         help="Indicates whether the backup has exceeded its storage time.",
     )
 
+    @api.depends("db_backup_id.days_to_keep", "create_date")
     def _compute_is_expired(self):
         """Compute whether the backup has exceeded its storage time."""
         for record in self:
@@ -34,6 +35,25 @@ def _compute_is_expired(self):
             else:
                 record.is_expired = False
 
+    def unlink(self):
+        """Unlink backing ir.attachment records before deleting the DB record.
+
+        Odoo's base Model.unlink() does cascade-delete ir.attachment via raw
+        SQL, but we make this explicit to ensure the fs_attachment GC stack
+        (_fs_mark_for_gc) is triggered reliably. The physical file on the
+        external storage is removed by the autovacuum GC job
+        (fs_file_gc._gc_files), not synchronously here.
+        """
+        attachments = self.env["ir.attachment"].search(
+            [
+                ("res_model", "=", self._name),
+                ("res_id", "in", self.ids),
+                ("res_field", "=", "backup_file"),
+            ]
+        )
+        attachments.unlink()
+        return super().unlink()
+
     @api.model
     def fs_storage(self):
         FsStorage = self.env["fs.storage"]
@@ -51,7 +71,3 @@ def fs_storage(self):
         if fs_storage:
             return fs_storage
         return False
-
-    def get_fs_storage_filename(self):
-        self.ensure_one()
-        return self.backup_file.attachment.store_fname.split("://")[-1]

auto_backup_fs_file/readme/CONTEXT.md

@@ -9,5 +9,7 @@ Practical examples include:
 APPROACH:
 The module extends the backup functionality from the `auto_backup` module by introducing a method that allows storing the resulting backup using an `fsspec` implementation. This is achieved through the integration of the `fs_file` from [storage repository](https://github.com/OCA/storage). The module leverages the `fsspec` library to provide a flexible and extensible interface for interacting with various filesystems. It automates the backup process by exporting Odoo instance data and storing it in the specified filesystem. Additionally, it allows users to download the backups for local storage or further processing.
 
+  Backup file cleanup is handled automatically based on the **Days to Keep** configuration. When expired backup records are removed, the physical backup files are not deleted synchronously. Instead, the module delegates file deletion to the `fs_attachment` garbage collector (GC), which marks files for deferred removal and physically deletes them during Odoo's autovacuum cron cycle. This two-phase approach ensures transactional safety: files are only removed once the GC confirms no database record still references them.
+
 USEFUL INFORMATION:
 - **Dependencies**: This module depends on the `fsspec` library, its relevant filesystem implementations, and the `fs_file` addon from OCA/storage. Ensure the required `fsspec` plugins are installed for your target filesystem.

auto_backup_fs_file/readme/INSTALL.md

@@ -1 +1 @@
-This addon itself does not introduce any dependencies, but its dependencies may require additional packages.:wa
+This addon itself does not introduce any dependencies, but its dependencies may require additional packages.

auto_backup_fs_file/readme/ROADMAP.md

@@ -1,3 +1,3 @@
-- **Folder field behavior**: The `folder` field on the `db.backup` model specifies the backup storage directory. For records using the `fs_file` method, storage is actually controlled by the `fs_file` field's settings. However, since `folder` is currently a required non-computed field in the `auto_backup` addon, modifications to sync these two fields are not performed. Future versions may add this synchronization support.
+- **Folder field behavior**: The `folder` field on the `db.backup` model specifies the backup storage directory. For records using the `fs_file` method, storage is controlled by the `fs_file` field's settings. The `folder` field is hidden (`invisible`) and no longer required when `method='fs_file'`, so it no longer interferes with `fs_file` configurations. No auto-sync between both fields is performed since the methods are mutually exclusive.
 
 - **Design limitation**: The current implementation has a design constraint due to `fs_storage` addon limitations. Since storage setting targets the `db.backup.fs.file` model, only one storage backend can effectively be used.

auto_backup_fs_file/readme/USAGE.md

@@ -23,6 +23,18 @@ This module extends the functionality of the database backup system in Odoo by i
 - In the Fs File backups list view, you can see details such as the backup filename and associated database backup configuration.
 - Use this view to manage or download backups as needed.
 
+### 5. Cleanup and File Deletion
+
+Backup retention is controlled by the **Days to Keep** field on the backup configuration. When this value is greater than 0, the automatic cleanup process removes expired backup records during each backup run.
+
+When a backup record is deleted (either by automatic cleanup or manually from the list view), the physical backup file in the filesystem storage is **not removed immediately**. Instead, the file is marked for deferred deletion by the `fs_attachment` garbage collector (GC), which runs periodically via Odoo's autovacuum cron. Physical files are only removed from the storage backend once the GC confirms no database record references them.
+
+This means:
+- **Immediately after deletion**: the database record is gone, but the file may still exist in the storage backend for a short period.
+- **After the next autovacuum cycle**: the file is permanently deleted from the storage backend.
+
+This behavior requires the storage's `autovacuum_gc` flag to be enabled (the default). If disabled, files must be managed manually.
+
 ### Screenshots
 - **Backup Configuration Form View**
   ![Backup Configuration Form](../static/description/db_backup_form_view.png)