Provide guidelines concerning the usage of `@property` in ORM models

[AaronBeaudoin]

I've noticed that throughout the entire Odoo codebase there is almost no usage anywhere of the Python @property decorator, which I found rather surprising considering how "clean" it tends to be. It has lead me to wonder whether using @property is discouraged for some reason or another.

Here is an example of where I would find it to be rather intuitive:

class ResPartner(models.Model):
    _inherit = "res.partner"

    @property
    def stripe_api_key(self):
        IrConfigParameter = self.env["ir.config_parameter"].sudo()
        stripe_api_key = IrConfigParameter.get_param("stripe.api_key")

        if not stripe_api_key: raise Exception
        return stripe_api_key

In this case I wish to store a global ir.config_parameter containing my Stripe API key, and use @property to conveniently access it at self.stripe_api_key anywhere in the model so I can use it for requests to the Stripe API, and avoid duplicating the self.env["..."].get_param("...") code throughout all my model methods. Of course, I could also simply create a self.get_stripe_api_key() method, but as I mentioned, I think the @property style is much cleaner.

Is there any reason that this type of code is not recommended? I understand that in some cases you might have expensive code that you would want to ensure programmers are explicitly choosing to run, but in this case ir.config_parameter is cached anyways so I don't think we're looking at any real performance concern here compared to just using a regular function.

What are the guidelines concerning usage of @property in Odoo ORM models?

[Julien00859]

TL;DR: @properties work well for regular python instances, not record sets. Use compute-fields in Odoo and if it doesn't feel right then it shouldn't had been a property in the first place.

A good python property is a property which usage feels like using a regular attribute: it works on a specific instance and it is fast to get/set. The thing is... you never manipulate a specific instance when using Odoo models, you instead manipulate a record set, a container that can have no, one or multiple records. The assumption "work on a specific instance and is fast" fall short because (1) the instance (self) is the record-set and not a specific record, (2) due to its container nature, the operation you could do on the record-set are a magnitude slower than O(1).

In your example it feels like every user has his own stripe api key (partner.strike_api_key) but in reality there is a single API Key and it is stored in the database parameters. When considering RPC, it doesn't feel right either: you have to browse a user before you can use the property (maybe there is a missing @api.model decorator).

The equivalent to properties in regular python object are compute-fields in odoo models.

[AaronBeaudoin]

Thanks for the detailed response! When you have the time, it does prompt quite a few follow-up questions though:

• If this property was changed to a compute field that queried ir.config_parameter the same way, how would that resolve the problem about it not feeling right because the record doesn't belong to a specific partner? It seems like it would be functionally identical to me.
• What about cases where you want to return some arbitrary object from your property/compute method, that this is no Odoo field for? Like some arbitrary custom class? How would you handle that situation?
• As I mentioned in the issue, the goal of this kind of code would be to reduce repetition by not querying ir.config_parameter in every single method on the partner model where it is needed, instead providing it in a more "global" way. Is there another better way to do that? (For example, maybe a custom decorator called @inject_stripe_api_key or something? Although that still doesn't feel "global".)
• Does adding the @api.model decorator serve any functional purpose beyond adding a hint that the method is mean't to not depend on the content of the recordset? Is a method decorated with @api.model actually treated differently in Odoo? (Other than when it is on the create method, where I am aware that it actually causes the create method to be decorated with model_create_single.)
• An alternate way to word the previous question is to flip the logic a bit. Would not adding a @api.model decorator to some method ever cause any problems by changing how it is handled/treated/called?

Also, as far as the impact I believe this conversation has on the documentation, I believe mainly it hints that two topics potentially deserve a bit more love:

• @api.model could use some further explanation (what impact does it actually have when you use it)? At the unofficial documentation here it says the purpose of the @api.model decorator is to "convert old API calls to decorated function to new API signature". Based on that description, it seems like it would be unnecessary for new code, other than to provide a minor hint concerning how the method should be used.
• What is the "Odoo way" for exposing global data that needs to be accessed on many methods, even across multiple models? (I could extend the base model, but something tells me that is definitely not the "Odoo way".)

Thanks for your help, and sorry for such a packed comment!

[Julien00859]

If this property was changed to a compute field that queried ir.config_parameter the same way, how would that resolve the problem about it not feeling right because the record doesn't belong to a specific partner? It seems like it would be functionally identical to me.

This is wrong, nor properties nor computed fields are applicable in this context. You want an @api.model dedicated method.

What about cases where you want to return some arbitrary object from your property/compute method, that this is no Odoo field for? Like some arbitrary custom class? How would you handle that situation?

For such cases, you do a dedicated private method.

As I mentioned in the issue, the goal of this kind of code would be to reduce repetition by not querying ir.config_parameter in every single method on the partner model where it is needed, instead providing it in a more "global" way. Is there another better way to do that? (For example, maybe a custom decorator called @inject_stripe_api_key or something? Although that still doesn't feel "global".)

You shouldn't worry about it, the database is requested only once to get the api key and the value is stored in cache. The cache is then consulted instead. The cache lasts for the duration of the current request.

Does adding the @api.model decorator serve any functional purpose beyond adding a hint that the method is mean't to not depend on the content of the recordset? Is a method decorated with @api.model actually treated differently in Odoo? (Other than when it is on the create method, where I am aware that it actually causes the create method to be decorated with model_create_single.)

It makes it possible to call it via XML-RPC without supplying a list of record ids. @api.model is kinda similar to @classmethod.

---

Also, as far as the impact I believe this conversation has on the documentation, I believe mainly it hints that two topics potentially deserve a bit more love:

To be fair I've never authored a single bit in the ORM documentation. ANP reached our team this morging asking if someone was interrested in taking care of the few articles related to the ORM and I was stupid enough to say I was interested. I don't know myself where this issue is going but let's continue 😅

What is the "Odoo way" for exposing global data that needs to be accessed on many methods, even across multiple models? (I could extend the base model, but something tells me that is definitely not the "Odoo way".)

Depends on the context... if it is global to all the database then it should be an ir.setting.parameter, if it is global to a company then you can extend the res.company model and add a field. "But res.company is going to be bloated with tons of fields!" yes it is 😅

[AaronBeaudoin]

I've given quite some thought to this discussion, and I have a few thoughts.

---

First, regarding @api.model. It seems like almost a useless decorator to me, then. I mean, you could always call the method XML-RPC with an empty recordset and get the same effect. It really seems like a shortcut to type a few less keystrokes.

Of course, there is the idea (as you mentioned, similar to @classmethod) that it sort of hints to anyone reading the code that the method is more stateless. That is, it doesn't depend on the contents of the recordset. But other than that, it does almost nothing. I could define the method without it, and it would work exactly the same, except for the tiny difference that I would need to pass a (probably empty) recordset when calling it via XML-RPC.

One key consequence of this is that, as a solo developer, if I never intend to use XML-RPC, I might never even bother to add the @api.model decorator because it doesn't have any meaningful effect for my use case.

I personally feel that the documentation would hugely benefit from the inclusion of an explanation like the following:

@api.model is like @classmethod, but for Odoo models. Similar to how a method decorated with @classmethod could be said to be stateless in the sense that it does not depend on an instance, a method decorated with @api.model should be stateless in the sense that it does not depend on the contents of the recordset self.

The word "should" is highlighted above because no behavior is actually enforced by the presence of the @api.model decorator. It is up to the programmer to ensure that the return values and behavior of methods decorated with @api.model do not depend on the contents of the recordset.

The only functional effect that the @api.model decorator has on the usage of the decorated method Odoo is that when calling it via XML-RPC, supplying a list of record IDs is no longer required. For this reason, we recommend making sure to include it wherever relevant, based on the meaning above.

I suspect something like this would clear up the purpose of @api.model for many developers like myself. However, it would probably also encourage many of them to just skip ever adding the decorator because they have no interest in XML-RPC. But that second issue is really more of an architectural problem than the fault of the developer.

---

Second, regarding the pattern of using a dedicated private method instead. Here is what that might look like:

class MyModel(models.Model):
    _name = "my.model"

    @api.model
    def _get_stripe_api_key(self):
        IrConfigParameter = self.env["ir.config_parameter"].sudo()
        stripe_api_key = IrConfigParameter.get_param("stripe.api_key")

        if not stripe_api_key: raise Exception
        return stripe_api_key

    def get_stripe_customer(self, customer_id):
        stripe_api_key = self._get_stripe_api_key()
        return stripe.Customer.get(customer_id, api_key=stripe_api_key)

    def get_stripe_customer_payment_methods(self, customer_id):
        stripe_api_key = self._get_stripe_api_key()
        return stripe.Customer.get_payment_methods(customer_id, api_key=stripe_api_key)

    def get_stripe_payment(self, payment_id):
        stripe_api_key = self._get_stripe_api_key()
        return stripe.Payment.get(payment_id, api_key=stripe_api_key)

Sure, I could simplify this a bit by calling the private method right where I use the API key each time:

class MyModel(models.Model):
    ...

    def get_stripe_customer(self, customer_id):
        return stripe.Customer.get(customer_id, api_key=self._get_stripe_api_key())

    def get_stripe_customer_payment_methods(self, customer_id):
        return stripe.Customer.get_payment_methods(customer_id, api_key=self._get_stripe_api_key())

    def get_stripe_payment(self, payment_id):
        return stripe.Payment.get(payment_id, api_key=self._get_stripe_api_key())

But now, what if I want to be able to get the API key like this on multiple different models? Do I have to just duplicate the _get_stripe_api_key on each model where I need it? Or is this the case where I would just put it on the base model? How would you keep things DRY in this case?

Thanks for your input and being willing to read my long responses!

[Julien00859]

/summon @rco-odoo @xmo-odoo

[Julien00859]

/summon @ryv-odoo

[ryv-odoo]

Hello @AaronBeaudoin ,

Indeed, @api.model it actually not documented correctly. @api.model (still usefull) means that your method doesn't except to get ids args from RPC call (https://github.com/odoo/odoo/blob/master/odoo/api.py#L457), in opposite of other model methods. Implicitly, in backend point of view, it means that your method doesn't depends of self (which is also empty from a RCP call).

IMO, your should put your method _get_stripe_api_key in the right model, the IrConfigParameter seems the more appropriate. And call it with self.env['ir.config_parameter']._get_stripe_api_key().

[Julien00859]

About some guidelines about using @propoerty in Odoo ORMs, I don't really understand why it should be documented. The concept of properties is independent of that of the ORM, they are not commonly used because most of the use-cases where you would use a property with pure python classes, you use compute-fields instead. In this particular example, a computed-field is not applicable. I join ryv in saying that it should be a private method on ir.config_parameter

[AaronBeaudoin]

I'm actually still going to move ahead with putting the method on the base model, even though that might not be ideal. Here's why: I don't know if I'll always want to store the API key on ir.config_parameter. At some point, it may make sense to have it be company-dependent, or I may want to use my own custom store, and in either case I'd have to refactor the code everywhere self.env["ir.config_parameter"]._get_stripe_api_key() is called. Not ideal.

I believe there are many other cases where this same scenario might apply. Part of my goal here is to make the implementation of how I get the API key completely independent from the places where I call the method to do so. I don't want the places where I call _get_stripe_api_key() to have to know anything about where that API key is actually coming from. Hopefully that makes sense.

---

As for using @property, I understand now the rationale for why it isn't really needed, but I think a note in the guidelines would still be helpful to point out that there's no active harm in using it on an ORM model. When I browsed the source code and saw it was never used I felt the bring this up over here on GitHub because I wanted to ensure that there wasn't an important reason to avoid @property for some reason.

---

Also, I still really think the documentation for @api.model should be improved, because I would never have been able to figure out what its real effect is (unless I deeply studied the source code) without this thread. Thanks so much for all the explanation concerning that, by the way!

[Julien00859]

We are not going to change the current documentation to include a § about @property. Properties work the way they should and they are compatible with our inheritance mechanisms. It happens they are not used in any of our standard modules because we don't have a use case where they are applicable. Feel free to use them in your own projects.

Also, I still really think the documentation for @api.model should be improved, because I would never have been able to figure out what its real effect is (unless I deeply studied the source code) without this thread. Thanks so much for all the explanation concerning that, by the way!

We agree.

I'm closing the issue since the matter of @property is solved, thank you for the discussion, have a nice weekend :)