Skip to content
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

[IMP] core: store translated fields as JSONB columns #97692

Merged
merged 10 commits into from Sep 16, 2022

Conversation

fpodoo
Copy link
Contributor

@fpodoo fpodoo commented Aug 8, 2022

[IMP] core: store translated fields as JSONB columns

Translated fields no longer use the model ir.translation.  Instead they store
all their values as JSON, and store them into JSONB columns in the model's
table.  The field's column value is either NULL or a JSON dict mapping language
codes to text (the field's value in the corresponding language), and must
contain an entry for key 'en_US' (as it is used as a fallback for all other
languages).  Empty text is allowed in translation values, but not NULL.

Here are examples for a field with translate=True:

    NULL
    {"en_US": "Foo"}
    {"en_US": "Foo", "fr_FR": "Bar", "nl_NL": "Baz"}
    {"en_US": "Foo", "fr_FR": "", "nl_NL": "Baz"}

Like before, writing False to the field makes it NULL, i.e., False in all
languages.  However, writing "" to the field makes its value empty in the
current language, but does not discard the values in the other languages.

Here are examples for a field with translate=xml_translate:

    NULL
    {"en_US": "<div>Foo<p>Bar</p></div>", "fr_FR": "<div>Fou<p>Barre</p></div>"}

Change for callable(translate) fields: one can now write any value in any
language on such a field.  The new value will be adapted in all languages, based
on the mapping of terms between languages in the old values.  Basically the
structure of the value must remain the same in all languages, like before.

Reading a translated field is now both simpler and faster than the former
implementation.  We fetch the value of the field in the current language by
coalescing its value with the 'en_US' value of the field:

    SELECT id, COALESCE(name->>'fr_FR', name->>'en_US') AS name ...

The raw cache of the field contains either None or a dict which is conceptually
a subset of the JSON value in database (except for missing languages).  For the
sake of simplicity, most cache operations deal with the dict and return the text
value in the current language.

Trigram indexes have been adapted to the new storing strategy, and should enable
to search in any language.  Before this change, only the source value of the
field ('en_US') could be indexed.

Computed stored translated fields are not supported by the framework, because of
the complexity of the computation itself: the field would need to be computed in
all active languages.  We chose to not provide any hook to compute a field in
all languages at once, and the framework always invokes a compute method once to
recompute it.

Code translations are no longer stored into the database.  They become static,
and are extracted from the PO files when needed.  The worker simply uses a cache
with extracted code translations for performance.  This is reasonable, since
fr_FR code translations for all modules takes around 2MB of memory, and the
cache can be shared among all registries in the worker.  Changing code
translations requires to update the corresponding PO file and reloading the
worker(s).

Performance summary:
(+) reading 'model' translated fields is faster
(+) reading 'model_terms' translated fields is much faster (no need to inject
     translations into the source value)
(+) searching translated fields with operator 'ilike' is much faster when the
     field is indexed with 'trigram'
(+) updating translated fields requires less ORM flushing
(-) importing translations from PO files is 2x slower

Some extra fixes:

  • make field 'name' of ir.actions.actions translated; because of the PG
       inheritance, this is necessary to make the column definition consistent in
       all models that inherit from ir.actions.actions.
  • add some backend API for the web/website client for editing translations
  • move methods get_field_string() to model ir.model.fields
  • move _load_module_terms to model ir.module.module
  • adapt tests in test_impex, test_new_api
  • because env.lang is injected into SQL queries, its returned value is
       now guaranteed to correspond to a valid active language or None
  • remove wizard to insert missing translations (no longer makes sense)

task-id: 2081307

@robodoo
Copy link
Contributor

robodoo commented Aug 8, 2022

@C3POdoo C3POdoo added the RD research & development, internal work label Aug 8, 2022
Copy link
Contributor Author

@fpodoo fpodoo left a comment

Choose a reason for hiding this comment

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

Quick review; didn't spend a lot of time, not 100% sure of recommendations.

addons/account/models/account_tax.py Outdated Show resolved Hide resolved
addons/http_routing/models/ir_http.py Outdated Show resolved Hide resolved
addons/web/controllers/webclient.py Outdated Show resolved Hide resolved
odoo/addons/base/models/ir_model.py Show resolved Hide resolved
odoo/addons/base/models/ir_translation_code.py Outdated Show resolved Hide resolved
odoo/tools/translate.py Outdated Show resolved Hide resolved
@HydrionBurst HydrionBurst force-pushed the master-lang-json-str-cwg branch 4 times, most recently from cafa90b to 0d43076 Compare August 10, 2022 16:48
@HydrionBurst HydrionBurst force-pushed the master-lang-json-str-cwg branch 22 times, most recently from 735df37 to 8374afc Compare August 28, 2022 19:26
HydrionBurst added a commit to odoo-dev/odoo that referenced this pull request Oct 26, 2022
In the PR odoo#97692, writing empty str `''` to a model term field no longer removes
all translations. Instead, only the translation for the current language is set
to empty str `''`.

Before this fix, if a web user sets a translation to emtpy str, and reopens
the translation dialog, the translation will be reverted to the 'en_US' value
which is wrong.
HydrionBurst added a commit to odoo-dev/odoo that referenced this pull request Oct 26, 2022
Module Transifex was removed in PR odoo#97692 since translations are no longer
stored in ir.translation module. This commit only reverts the old module for
review to compare the diff, and will be squashed before merge.
robodoo pushed a commit that referenced this pull request Oct 26, 2022
In the PR #97692, writing empty str `''` to a model term field no longer removes
all translations. Instead, only the translation for the current language is set
to empty str `''`.

Before this fix, if a web user sets a translation to emtpy str, and reopens
the translation dialog, the translation will be reverted to the 'en_US' value
which is wrong.

closes #103058

Signed-off-by: Samuel Degueldre <sad@odoo.com>
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Oct 26, 2022
In the PR odoo#97692, writing empty str `''` to a model term field no longer removes
all translations. Instead, only the translation for the current language is set
to empty str `''`.

Before this fix, if a web user sets a translation to emtpy str, and reopens
the translation dialog, the translation will be reverted to the 'en_US' value
which is wrong.

X-original-commit: f97ff63
robodoo pushed a commit that referenced this pull request Nov 2, 2022
In the PR #97692, writing empty str `''` to a model term field no longer removes
all translations. Instead, only the translation for the current language is set
to empty str `''`.

Before this fix, if a web user sets a translation to emtpy str, and reopens
the translation dialog, the translation will be reverted to the 'en_US' value
which is wrong.

closes #104181

X-original-commit: f97ff63
Signed-off-by: Samuel Degueldre <sad@odoo.com>
Signed-off-by: Wang Chong (cwg) <cwg@odoo.com>
return False
new_translations = old_translations
for lang, translation in translations.items():
old_value = new_translations.get(lang) or new_translations.get('en_US')
Copy link

Choose a reason for hiding this comment

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

old_value should not be the old translated text but always the original en_US text: old_value = old_translations.get('en_US')

Copy link
Contributor

Choose a reason for hiding this comment

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

the old term and the new term should be in same language, we use the mapping {old_term_fr: new_term_fr} to update the old_value_fr

Choose a reason for hiding this comment

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

Ok, right. Now, I understand. Then the problem lies somewhere in the UI. I investigate further.

HydrionBurst added a commit to odoo-dev/odoo that referenced this pull request Dec 30, 2022
Module Transifex was removed in PR odoo#97692 since translations are no longer
stored in ir.translation module. This commit only reverts the old module for
review to compare the diff, and will be squashed before merge.
HydrionBurst added a commit to odoo-dev/odoo that referenced this pull request Jan 23, 2023
Module Transifex was removed in PR odoo#97692 since translations are no longer
stored in ir.translation module. This commit only reverts the old module for
review to compare the diff, and will be squashed before merge.
KangOl pushed a commit to odoo/upgrade-util that referenced this pull request May 8, 2023
Adapt code for 16.0, where the ir.ui.view `arch_db` (fields.Text) is
now storred as a jsonb column.

Also, the `add_view` now return the `id` of the newly created view.

See: odoo/odoo#97692
Part-of: odoo/upgrade#4306
HydrionBurst added a commit to odoo-dev/odoo that referenced this pull request May 10, 2023
before this commit:
after odoo#97692 when copy a record with specified translated value, old translation
will be dropped.

after this commit:
The old translations can be kept

opw-3302537
KangOl added a commit to odoo/upgrade-util that referenced this pull request Oct 5, 2023
task-id 2081307

See odoo/odoo#97692
See odoo/enterprise#30829

Part of odoo/upgrade#3823

Signed-off-by: Christophe Simonis <chs@odoo.com>
Co-authored-by: Raphael Collet <rco@odoo.com>
Co-authored-by: Christophe Simonis <chs@odoo.com>
KangOl pushed a commit to odoo/upgrade-util that referenced this pull request Oct 5, 2023
Adapt code for 16.0, where the ir.ui.view `arch_db` (fields.Text) is
now storred as a jsonb column.

Also, the `add_view` now return the `id` of the newly created view.

See: odoo/odoo#97692
Part-of: odoo/upgrade#4306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
15.5 RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet