[16.0][FIX] fs_attachment: reconcile fs_filename when datas/raw is rewritten - #598
TecnologiaIG wants to merge 1 commit into
Conversation
|
Hi @lmignon, |
``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://<hash>"``) straight
to ``os.stat``, which raises ``FileNotFoundError`` — user-visible as
500s on every ``/web/image/<res_model>/<id>/<field>`` 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 <tecnologia@intensegroupgt.com>
115aec6 to
b9a51a9
Compare
lmignon
left a comment
There was a problem hiding this comment.
Thank you for the fix @TecnologiaIG . Can you avoid tu manually update the version into the manifest since it will be updated by the bot in charge of the merge. Can I also ask you to add a unittest to cover your modification?
Regards
| "name": "Base Attachment Object Store", | ||
| "summary": "Store attachments on external object store", | ||
| "version": "16.0.2.0.1", | ||
| "version": "16.0.2.0.4", |
There was a problem hiding this comment.
The version should not be updated manually.
|
GUIA-Azure-Blob-Cloud-Storage-Odoo16-losvolcanes-2026-04-21.pdf Attached the works done, as requested. |
| broken = self.filtered( | ||
| lambda a: a.store_fname and not a.fs_filename | ||
| ) | ||
| if broken: |
There was a problem hiding this comment.
@TecnologiaIG I'm not able to reproduce your error. Can you write a unittest?
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Problem
ir.attachment.writeonly called_enforce_meaningful_storage_filenamewhennamewas invals. But every other code path that rewrites an attachment's bytes without touching its name — asset-bundle regeneration, ORM image field updates, inline kanban image writes, the_force_storagemigration itself — replacesstore_fnamevia_storage_file_writebut leavesfs_filename/fs_storage_code/fs_storage_idstale.A
NULLfs_filenamedefeats the gate inIrBinary._get_fs_attachment_for_field:With
Nonereturned, base Odoo falls through toStream.from_attachment, which takes the rawstore_fname(e.g."azure_attachments://<hash>") and passes it straight toos.stat— raisingFileNotFoundError→ 500 on every/web/image/<res_model>/<id>/<field>call that touches the record.Why it was latent for most installations
When the default storage is the plain filestore,
store_fnameis just a 40-char hash — the same format_enforce_meaningful_storage_filenamewould rename it to. Re-reads work whether or notfs_filenameis set. The mismatch only becomes fatal oncestore_fnametakes the<storage_code>://…form, because base Odoo cannot parse that path.Observed incident (2026-04-20 → 2026-04-22)
A production Odoo.sh instance flipped
fs.storage.use_as_default_for_attachmentsto anabfs(Azure Blob) backend. The deploy that followed regenerated thousands of asset bundles and ran background image updates, every one viawrite({"datas": ...}). 36,804ir.attachmentrows entered the broken state in under 48 h. Every/web/imagehit for those records returned 500 — assets included — so the webclient rendered as a blank page in every browser.Recovery required an out-of-band
UPDATE+ filestore restore from the pre-deploy backup. The hash-to-filestore path mapping preserved the data, but thefs_filenamemismatch alone was enough to take the site down.Fix
_enforce_meaningful_storage_filenameis idempotent and already no-ops when nothing needs to change — the broader guard has no downside for the existing"name"path.Test plan
attachment.write({"datas": new_bytes})on a record living in anfs.storagebackend → assertfs_filenameis non-null afterwards.debug=assetson a clean DB) → confirm the resultingir.attachmentrows havefs_filenamepopulated.Stacks on
Version bumped to
16.0.2.0.4to leave room for #596 (16.0.2.0.2, cursor timeouts) and #597 (16.0.2.0.3, GC batching/infinite-loop). Happy to rebase as needed.Forward-ports
The affected write override is identical on 17.0 / 18.0 / 19.0 as of today's tip; happy to forward-port once 16.0 is reviewed.