[FIX] fields: use function to check manual name

[jjmaksoud]

The use of IrModelFields method is_manual_name was reverted for the sake of efficiency and avoiding instanciating models when not really needed.
The use of an overridable method is still needed (reference).
Here is a fix using a simple function that can still be patched.

---

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[robodoo]

[kmagusiak]

Please check with @rco-odoo.
I don't see a reason to have this other than "some customers might want to" - which does not explain why.
The function must have a docstring explaining why this function is needed, otherwise it might be dropped again on the future.

[jjmaksoud]

@kmagusiak The override is also needed internally in the context of upgrades for all databases that have custom modules. Some of the integrity checks that are done during migration validate that views are still accessible, and those views may include fields that are not loaded, so the upgrade tries to load them as manual fields just for those checks. The patch needed in upgrade after this is resolved: odoo/upgrade-util#309

[jjmaksoud]

@Xavier-Do @beledouxdenis This didnt make the freeze but it is still needed for upgrades

[beledouxdenis]

If I follow correctly what @Xavier-Do is trying to achieve in his pull request #212579, he just want to use the registry rather than records in a maximum of places.

When he did this

-    if not (is_class_field or model.env['ir.model.fields']._is_manual_name(name)):
+    if not (is_class_field or name.startswith('x_')):

I believe the main issue was the use of model.env['ir.model.fields'], but using model.env.registry['ir.model.fields']._is_manual_name whould be fine I believe.

So I believe you could simplify your pull request with simply

Suggested change

	if not (is_class_field or is_manual_field_name(name)):
	if not (is_class_field or model.env.registry['ir.model.fields']._is_manual_name(name)):

and no need of this additional fields.is_manual_field_name.

However I would still ask Xavier guidance to make sure this is still ok to reach its performance goal.

[Xavier-Do]

The main goal was to speedup the setup as much as possible.
One of the slow point was to get model class in the registry and to instantiate it as a model.
The second part was partially an issue but not only, the getitem was visible and a part of the change was to avoid to call it on each field, but rather once per model when possible.

For this particular change, since no override were found in the code and the change was in master we decided to remove the method completely to cleanup a little. No necessary additional method call, easier to read, ... after a discussion with rco we decided to remove it, we didn't notice that it was patched in the upgrades.

Since the upgrade part just patch the method, it doesn't need to be a class method so I suggested to make a function, patchable but easier for me to assume that no performance regression will be introduced. If you think that a class method is better I can start a profiler on the branch to check if the change invisible or not.

[Xavier-Do]

IMHO the is_manual_field_name could be defined immediately in the model_classes file, no need to import it it is only used there.

[jjmaksoud]

To use model_cls.pool['ir.model.fields']._is_manual_name I would still need to edit _is_manual_name signature to remove the self, which I think is not okay in stable

IMHO the is_manual_field_name could be defined immediately in the model_classes file, no need to import it it is only used there.

It is also used in base/ir_model which is modified in this PR as well