Merge lp:~openerp-dev/openobject-server/6.0-opw-17601-rgo into lp:openobject-server/6.0

[Ravi Gohil (OpenERP)]

Hello,

I have done the suggested changes.

Thanks

[Anup(SerpentCS) (anup-serpent)]

Hello Ravi,

I agree with you. I would definitely admit that any method which is being called by xmlrpc should not return None as 'None' is not being marshaled and raises an error.

I agree with your fix but I think won't be feasible to do it on a single method or it would require lot's of changes with other methods which are returning None.

So IMHO there should be some generic way to get rid of the issue. Whenever xmlrpc calls a method.

We still need to discuss with experts before merging it.

@Raphael,Olivier Can you please share your views?

Thanks.

[Olivier Dony (Odoo) (odo-openerp)]

ir.model.access.check() is not meant to be called in RPC, but it's better to fix it indeed, as it is technically RPC-exposed.

For the fix, wouldn't it be better to put `or False` on the return line instead, to catch future changes/mistakes.

As for a global solution, as you know XML-RPC support for `null` is not properly standardized so we shouldn't waste too much time with this. Introspecting returned data structure to catch None values seems like overkill. It's not very important, we'll gradually move over to JSON-RPC instead, which supports `null`, and in the mean time, developers should just be aware of it and be careful.

review: Needs Fixing

[Ravi Gohil (OpenERP) (rgo-openerp)]

review: Needs Resubmitting

[Anup(SerpentCS) (anup-serpent)]

Hello,

Olivier, I do agree with you that this definitely is a better way.

Thanks.

review: Approve

[Olivier Dony (Odoo) (odo-openerp)]

Looks good to me now

review: Approve

[Raphael Collet (OpenERP) (rco-openerp)]

review: Approve

=== modified file 'bin/addons/base/ir/ir_model.py'
--- bin/addons/base/ir/ir_model.py	2011-04-06 15:05:25 +0000
+++ bin/addons/base/ir/ir_model.py	2011-09-29 06:09:26 +0000
@@ -512,7 +512,7 @@
             }
 
             raise except_orm(_('AccessError'), msgs[mode] % (model_name, groups) )
-        return r
+        return r or False
 
     check = tools.cache()(check)