Skip to content

[19.0][IMP] fs_attachment: allow field keys in force db rules - #673

Open
HviorForgeFlow wants to merge 1 commit into
OCA:19.0from
ForgeFlow:19.0-imp-fs_attachment-force_db_by_field
Open

HviorForgeFlow wants to merge 1 commit into
OCA:19.0from
ForgeFlow:19.0-imp-fs_attachment-force_db_by_field

Conversation

@HviorForgeFlow

@HviorForgeFlow HviorForgeFlow commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

The force_db_for_default_attachment_rules only match on the mimetype prefix. Some binary fields are read on every page load, like the menu icons (ir.ui.menu.web_icon_data), and should be kept in the database when the default storage is a slow object storage. With mimetype rules only, the sole way to do it is to raise the "image/" limit, which keeps all the images in the database.

A key without "/" but with a "." is now read as a "." pair, e.g. {"image/": 51200, "ir.ui.menu.web_icon_data": 0}. A field rule is checked before the mimetype rules, with the same size limit semantics, and is also applied in the domain used by force_storage_to_db_for_special_fields to move existing attachments back to the database. Existing mimetype keys always contain a "/", so current configurations are not affected.

Updated after review: rules can be grouped by name, mimetype-size and field-size, e.g. {"mimetype-size": {"image/": 51200}, "field-size": {"ir.ui.menu.web_icon_data": 0}}. Keys that are not a rule name are read as mimetype rules, so current configurations keep working.

Assisted-by: Claude Opus 5.5

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @lmignon,
some modules you are maintaining are being modified, check this out!

@lmignon lmignon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HviorForgeFlow Thank you for the proposal. IMO you could achieve the same result by configuring a storage for these specific fields where you force the store into DB by default for all image mimetype.... no need for extra code. Do you have tested?

@HviorForgeFlow

Copy link
Copy Markdown
Member Author

My use case, that's why I opened the PR is when, I have images lower than 50kb, the default configuration, placed in the DB when I want them place in s3, because they are related to products and other stuff.

Then I was lookinjg for a quick way to store web_icon from icons and only needed attachments to run updates on Odoo "offline" without having access to the s3 storage.

that's how I ended adding that feature to allow configure per model and field.

Maybe another possibility could be adding the Odoo's postgres as new Protocol and configure using the model_ids and/or fields_ids. But maybe then we would need a priority on FS storage.

@lmignon

lmignon commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Once a field is linked to a storage, that storage will always have priority when storing it, so there should be no need for a sequence. My concern with the current implementation is that it feels somewhat redundant with the possibility of linking a specific field to a storage. But I may be missing something, and the code is still quite readable.
I would just like to make sure that we don't introduce additional complexity for a valuable requirement when it could potentially be achieved using the existing mechanism.

@lmignon

lmignon commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

@sbidoul It feels a nice feature. What do you thing about?

@sbidoul

sbidoul commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Sounds useful.

What itches me is the overloading mechanism which is not very extensible.

Since the field is named force_db_for_default_attachment_rules it could have a structure like this which lends itself well to new rules (rule name => rules, with the unknown rule names interpreted as mimetype: size for backward compatibility).

So something like this?

{ 
    "field-size": {
    {
       "ir.ui.menu.web_icon_data": 0
    },
    "mimetype-size": {
       "image/": 51200,
       ...
    },
    // legacy
    "image/": 51200,
    ...
}

@HviorForgeFlow
HviorForgeFlow force-pushed the 19.0-imp-fs_attachment-force_db_by_field branch from 0766c7f to 4fe532b Compare October 19, 2026 15:07
Allows to keep in DB the attachments of a specific binary field, like the menu
icons (ir.ui.menu.web_icon_data) that are read on every page load, without
raising the "image/" limit for all the images.

* rules can be grouped by name: "mimetype-size" and "field-size"
* keys that are not a rule name are read as mimetype rules, as before

Assisted-by: Claude Opus 5.5
@HviorForgeFlow
HviorForgeFlow force-pushed the 19.0-imp-fs_attachment-force_db_by_field branch from 4fe532b to 3ce89d6 Compare October 19, 2026 15:08
@HviorForgeFlow

Copy link
Copy Markdown
Member Author

Thanks for taking time to review, I think that the structure that you provide is optimal.

In the meanwhile on my use case I just kept all attachment in the db with the system parameter and add the proper rules to keep in the s3 the files without using the storage as default attachment storage, that makes the job, but still having remaining files in the db regarding mail images, etc.

So I think that allowing to identify what you want to keep as an attachment and defaulting everything else to s3 could be a good option to go.

@lmignon lmignon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this useful feature @HviorForgeFlow
The code remains maintainable (easy to read/understand).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants