-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[16.0][IMP] database_cleanup: purge orphaned attachments #3566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Aldeigja
wants to merge
1
commit into
OCA:16.0
Choose a base branch
from
cetmix:16.0-t5258-database_cleanup-orphaned-attachments-clean
base: 16.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # Copyright 2026 Cetmix | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
|
|
||
| import os | ||
|
|
||
| from odoo import _, api, fields, models | ||
| from odoo.exceptions import AccessError, UserError, ValidationError | ||
|
|
||
| REASON_MISSING_FILE = "missing_file" | ||
|
|
||
|
|
||
| class CleanupPurgeLineAttachment(models.TransientModel): | ||
| _inherit = "cleanup.purge.line" | ||
| _name = "cleanup.purge.line.attachment" | ||
| _description = "Cleanup Purge Line Attachment" | ||
|
|
||
| attachment_id = fields.Many2one("ir.attachment") | ||
| reason = fields.Selection( | ||
| [ | ||
| (REASON_MISSING_FILE, "File missing in filestore"), | ||
| ], | ||
| ) | ||
| error_message = fields.Char(readonly=True) | ||
| wizard_id = fields.Many2one("cleanup.purge.wizard.attachment", readonly=True) | ||
|
|
||
| def purge(self): | ||
| """Unlink orphaned attachment records upon manual confirmation. | ||
|
|
||
| Filters unpurged lines with attachment_id. Unlinks each attachment | ||
| individually; failures are logged and skipped so the batch continues. | ||
| Only successfully removed attachments get their lines marked purged. | ||
|
|
||
| :return: result of write({"purged": True}) on successfully purged lines, | ||
| or True if none were purged | ||
| """ | ||
| if self: | ||
| objs = self | ||
| else: | ||
| objs = self.env["cleanup.purge.line.attachment"].browse( | ||
| self._context.get("active_ids") | ||
| ) | ||
| to_unlink = objs.filtered(lambda x: not x.purged and x.attachment_id) | ||
| self.logger.info("Purging attachments: %s", to_unlink.mapped("name")) | ||
| purged_line_ids = [] | ||
| for line in to_unlink: | ||
| attach = line.attachment_id | ||
| try: | ||
| attach.unlink() | ||
| purged_line_ids.append(line.id) | ||
| except (UserError, ValidationError, AccessError) as exc: | ||
| self.logger.warning( | ||
| "Attachment #%s cannot be deleted: %s", | ||
| attach.id, | ||
| str(exc), | ||
| ) | ||
| line.error_message = str(exc) | ||
| if not purged_line_ids: | ||
| return True | ||
| return ( | ||
| self.env["cleanup.purge.line.attachment"] | ||
| .browse(purged_line_ids) | ||
| .write({"purged": True}) | ||
| ) | ||
|
|
||
|
|
||
| class CleanupPurgeWizardAttachment(models.TransientModel): | ||
| _inherit = "cleanup.purge.wizard" | ||
| _name = "cleanup.purge.wizard.attachment" | ||
| _description = "Purge attachments" | ||
|
|
||
| @api.model | ||
| def find(self): | ||
| """Collect ir.attachment records whose backing files are missing on disk. | ||
|
|
||
| Requires file storage. Searches binary attachments with store_fname, | ||
| checks each file exists via os.path.isfile(_full_path(store_fname)). | ||
|
|
||
| :raises UserError: if storage != "file" or no orphaned entries found | ||
| """ | ||
| if self.env["ir.attachment"]._storage() != "file": | ||
| raise UserError( | ||
| _( | ||
| "Attachment storage is not 'file'. " | ||
| "Purge of orphaned attachments only works with file storage." | ||
| ) | ||
| ) | ||
| res = [] | ||
| attachments = self.env["ir.attachment"].search( | ||
| [ | ||
| ("store_fname", "!=", False), | ||
| ("type", "=", "binary"), | ||
| ] | ||
| ) | ||
| for attach in attachments: | ||
| full_path = self.env["ir.attachment"]._full_path(attach.store_fname) | ||
| if not os.path.isfile(full_path): | ||
| res.append( | ||
| fields.Command.create( | ||
| { | ||
| "attachment_id": attach.id, | ||
| "name": attach.store_fname or attach.name or str(attach.id), | ||
| "reason": REASON_MISSING_FILE, | ||
| } | ||
| ) | ||
| ) | ||
| if not res: | ||
| raise UserError(_("No orphaned attachment entries found")) | ||
| return res | ||
|
|
||
| purge_line_ids = fields.One2many("cleanup.purge.line.attachment", "wizard_id") | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| # Copyright 2026 Cetmix | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
|
|
||
| import base64 | ||
| import os | ||
| from unittest.mock import patch | ||
|
|
||
| from odoo.exceptions import UserError | ||
| from odoo.fields import Command | ||
| from odoo.tests.common import tagged | ||
|
|
||
| from .common import Common, environment | ||
|
|
||
|
|
||
| # Use post_install to get all models loaded more info: odoo/odoo#13458 | ||
| @tagged("post_install", "-at_install") | ||
| class TestCleanupPurgeLineAttachment(Common): | ||
| def setUp(self): | ||
| """Create two ir.attachment records; delete backing file of one (orphan). | ||
|
|
||
| :var orphan: ir.attachment with backing file removed via os.unlink | ||
| :var valid: ir.attachment with file intact | ||
| :return: None | ||
| """ | ||
| super().setUp() | ||
| with environment() as env: | ||
| IrAttachment = env["ir.attachment"] | ||
| datas = base64.b64encode(b"test_orphan").decode("ascii") | ||
| orphan = IrAttachment.create( | ||
| { | ||
| "name": "test_orphan_attachment.txt", | ||
| "type": "binary", | ||
| "datas": datas, | ||
| } | ||
| ) | ||
| datas_valid = base64.b64encode(b"test_valid").decode("ascii") | ||
| valid = IrAttachment.create( | ||
| { | ||
| "name": "test_valid_attachment.txt", | ||
| "type": "binary", | ||
| "datas": datas_valid, | ||
| } | ||
| ) | ||
| # Delete backing file to create orphan | ||
| full_path = IrAttachment._full_path(orphan.store_fname) | ||
| os.unlink(full_path) | ||
| self.orphan_attach_id = orphan.id | ||
| self.valid_attach_id = valid.id | ||
|
|
||
| def test_find_orphaned_attachments(self): | ||
| """Assert wizard find() includes orphan in purge lines, excludes valid. | ||
|
|
||
| :var wizard: cleanup.purge.wizard.attachment | ||
| :var line_attachment_ids: ids from purge_line_ids.mapped("attachment_id") | ||
| :return: None | ||
| """ | ||
| with environment() as env: | ||
| wizard = env["cleanup.purge.wizard.attachment"].create({}) | ||
| line_attachment_ids = wizard.purge_line_ids.mapped("attachment_id").ids | ||
| self.assertIn(self.orphan_attach_id, line_attachment_ids) | ||
| self.assertNotIn(self.valid_attach_id, line_attachment_ids) | ||
|
|
||
| def test_purge_orphaned_attachments(self): | ||
| """Assert purge_all() removes orphan record, leaves valid intact. | ||
|
|
||
| :var wizard: cleanup.purge.wizard.attachment | ||
| :var orphan: ir.attachment browsed by self.orphan_attach_id | ||
| :var valid: ir.attachment browsed by self.valid_attach_id | ||
| :return: None | ||
| """ | ||
| with environment() as env: | ||
| wizard = env["cleanup.purge.wizard.attachment"].create({}) | ||
| wizard.purge_all() | ||
| orphan = env["ir.attachment"].browse(self.orphan_attach_id) | ||
| valid = env["ir.attachment"].browse(self.valid_attach_id) | ||
| self.assertFalse(orphan.exists()) | ||
| self.assertTrue(valid.exists()) | ||
|
|
||
| def test_purge_skips_protected_attachment(self): | ||
| """When unlink raises UserError on one line, purge others and skip. | ||
|
|
||
| :var wizard: cleanup.purge.wizard.attachment | ||
| :return: None | ||
| """ | ||
| with environment() as env: | ||
| IrAttachment = env["ir.attachment"] | ||
| datas_a = base64.b64encode(b"test_protected").decode("ascii") | ||
| orphan_protected = IrAttachment.create( | ||
| { | ||
| "name": "test_protected_orphan.txt", | ||
| "type": "binary", | ||
| "datas": datas_a, | ||
| } | ||
| ) | ||
| datas_b = base64.b64encode(b"test_unprotected").decode("ascii") | ||
| orphan_other = IrAttachment.create( | ||
| { | ||
| "name": "test_unprotected_orphan.txt", | ||
| "type": "binary", | ||
| "datas": datas_b, | ||
| } | ||
| ) | ||
| os.unlink(IrAttachment._full_path(orphan_protected.store_fname)) | ||
| os.unlink(IrAttachment._full_path(orphan_other.store_fname)) | ||
|
|
||
| protected_id = orphan_protected.id | ||
| other_id = orphan_other.id | ||
|
|
||
| IrModel = env.registry["ir.attachment"] | ||
| original_unlink = IrModel.unlink | ||
|
|
||
| def patched_unlink(self): | ||
|
StefanRijnhart marked this conversation as resolved.
|
||
| if protected_id in self.ids: | ||
| # Dynamic message avoids translation lint on test-only UserError. | ||
| raise UserError(str(protected_id)) | ||
| return original_unlink(self) | ||
|
|
||
| wizard = env["cleanup.purge.wizard.attachment"].create( | ||
| { | ||
| "purge_line_ids": [ | ||
| Command.create( | ||
| { | ||
| "attachment_id": protected_id, | ||
| "name": orphan_protected.store_fname | ||
| or str(protected_id), | ||
| } | ||
| ), | ||
| Command.create( | ||
| { | ||
| "attachment_id": other_id, | ||
| "name": orphan_other.store_fname or str(other_id), | ||
| } | ||
| ), | ||
| ], | ||
| } | ||
| ) | ||
| protected_line = wizard.purge_line_ids.filtered( | ||
| lambda l: l.attachment_id.id == protected_id | ||
| ) | ||
| other_line = wizard.purge_line_ids.filtered( | ||
| lambda l: l.attachment_id.id == other_id | ||
| ) | ||
| protected_line_id = protected_line.id | ||
| other_line_id = other_line.id | ||
|
|
||
| with patch.object(IrModel, "unlink", patched_unlink): | ||
| wizard.purge_line_ids.purge() | ||
|
|
||
| Line = env["cleanup.purge.line.attachment"] | ||
| self.assertFalse(Line.browse(protected_line_id).purged) | ||
| self.assertTrue(Line.browse(other_line_id).purged) | ||
| self.assertTrue(IrAttachment.browse(protected_id).exists()) | ||
| self.assertFalse(IrAttachment.browse(other_id).exists()) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.