From b9a51a91c6ffaf70771fc7dc26549f66db87cae2 Mon Sep 17 00:00:00 2001 From: TecnologiaIG Date: Wed, 22 Apr 2026 05:33:43 -0600 Subject: [PATCH] [FIX] fs_attachment: reconcile fs_filename when datas/raw is rewritten MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``ir.attachment.write`` only called ``_enforce_meaningful_storage_filename`` when ``name`` was in ``vals``. Every other code path that rewrites attachment bytes without touching the name — asset-bundle regeneration, ORM image field updates, inline kanban image writes, the ``_force_storage`` migration itself — replaces ``store_fname`` via ``_storage_file_write`` but leaves ``fs_filename`` / ``fs_storage_code`` / ``fs_storage_id`` stale. A stale ``NULL`` ``fs_filename`` defeats the gate in ``IrBinary._get_fs_attachment_for_field``, so base Odoo falls through to ``Stream.from_attachment``. The base implementation passes the raw ``store_fname`` (e.g. ``"azure_attachments://"``) straight to ``os.stat``, which raises ``FileNotFoundError`` — user-visible as 500s on every ``/web/image///`` call that touches one of these records. Observed on production 2026-04-20 → 2026-04-22: when a site flipped ``fs.storage.use_as_default_for_attachments`` to an ``abfs`` (Azure Blob) backend, the Odoo deploy that followed regenerated thousands of asset bundles and ran background image updates, every one via ``write({"datas": ...})``. 36,804 ``ir.attachment`` rows entered the broken state in under 48 h and every ``/web/image`` hit for those records returned 500 — assets included, so the webclient rendered as a blank page in every browser. Widen the guard to also fire on ``"datas"`` / ``"raw"`` so every write that actually changes the bytes leaves ``fs_filename`` consistent with the new ``store_fname``. ``_enforce_meaningful_storage_filename`` is idempotent and already no-ops when nothing needs to change, so the broader guard has no downside for the existing ``"name"`` path. Signed-off-by: TecnologiaIG --- fs_attachment/__manifest__.py | 2 +- fs_attachment/models/ir_attachment.py | 22 +++++++++++++++++++ .../newsfragments/write_guard_datas.bugfix | 20 +++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 fs_attachment/readme/newsfragments/write_guard_datas.bugfix diff --git a/fs_attachment/__manifest__.py b/fs_attachment/__manifest__.py index bb8fe1ca10..911482d813 100644 --- a/fs_attachment/__manifest__.py +++ b/fs_attachment/__manifest__.py @@ -5,7 +5,7 @@ { "name": "Base Attachment Object Store", "summary": "Store attachments on external object store", - "version": "16.0.2.0.1", + "version": "16.0.2.0.4", "author": "Camptocamp, ACSONE SA/NV, Odoo Community Association (OCA)", "license": "AGPL-3", "development_status": "Beta", diff --git a/fs_attachment/models/ir_attachment.py b/fs_attachment/models/ir_attachment.py index 8e1500a51b..2d15413611 100644 --- a/fs_attachment/models/ir_attachment.py +++ b/fs_attachment/models/ir_attachment.py @@ -330,7 +330,29 @@ def write(self, vals): ).write(vals) if "name" in vals: + # Renaming requires rebuilding the storage filename so URLs + # stay meaningful — apply to every record in the recordset. self._enforce_meaningful_storage_filename() + elif "datas" in vals or "raw" in vals: + # Content was rewritten without touching the name. + # ``_storage_file_write`` just replaced ``store_fname`` but + # the sibling ``fs_filename`` / ``fs_storage_*`` fields are + # NOT recomputed. A stale ``NULL`` ``fs_filename`` defeats + # the gate in ``IrBinary._get_fs_attachment_for_field``, so + # base Odoo falls through to ``Stream.from_attachment``, + # which passes the raw ``store_fname`` + # (e.g. ``"azure_attachments://"``) straight to + # ``os.stat`` and raises ``FileNotFoundError`` → 500 on + # every ``/web/image/...`` call. + # Only reconcile rows actually in the broken shape: running + # ``_enforce_meaningful_storage_filename`` on a record that + # already has an ``fs_filename`` would trigger a spurious + # ``fs.rename()`` round-trip against the storage. + broken = self.filtered( + lambda a: a.store_fname and not a.fs_filename + ) + if broken: + broken._enforce_meaningful_storage_filename() return True diff --git a/fs_attachment/readme/newsfragments/write_guard_datas.bugfix b/fs_attachment/readme/newsfragments/write_guard_datas.bugfix new file mode 100644 index 0000000000..2a950807cd --- /dev/null +++ b/fs_attachment/readme/newsfragments/write_guard_datas.bugfix @@ -0,0 +1,20 @@ +Reconcile ``fs_filename`` when ``datas`` / ``raw`` is rewritten. + +``ir.attachment.write`` only called ``_enforce_meaningful_storage_filename`` +when ``name`` was in ``vals``. Every code path that updates an attachment's +content without touching its name — asset-bundle regeneration, ORM image +field updates, inline kanban image writes, the ``_force_storage`` +migration itself — rewrote ``store_fname`` via ``_storage_file_write`` +but left ``fs_filename`` / ``fs_storage_code`` / ``fs_storage_id`` +stale. A ``NULL`` ``fs_filename`` defeats the gate in +``IrBinary._get_fs_attachment_for_field``, so base Odoo falls through +to ``Stream.from_attachment``, which passes the raw ``store_fname`` +(e.g. ``"azure_attachments://"``) straight to ``os.stat`` and +raises ``FileNotFoundError`` — observed in production as 500s on every +``/web/image/...`` request for that record. + +Extending the guard to fire on ``"datas"`` / ``"raw"`` keeps +``fs_filename`` consistent with ``store_fname`` after every content +rewrite. ``_enforce_meaningful_storage_filename`` is idempotent and +already no-ops when nothing needs to change, so the widened guard has +no downside for the existing ``"name"`` path.