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

Create router map doesnt work well with properties #137

Closed
fa1k3n opened this issue Oct 16, 2020 · 7 comments
Closed

Create router map doesnt work well with properties #137

fa1k3n opened this issue Oct 16, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@fa1k3n
Copy link
Contributor

fa1k3n commented Oct 16, 2020

If you have a class containing properties the getters are called when the ocpp library is building its route map since it iterates att the attributes in the object

in router.py

 for attr_name in dir(obj):
       attrib = getattr(obj, attr_name)    

The problem is with the getattr call, if this is a property it will then actually call the getter method, which in most cases is unwanted.

One solution that I have found is to have a global list in router.py that contains all the routables. The on and after decorators can then append the method name to this list. Then during the call to create router map instead of iterating the object attributes this list can be iterated

@OrangeTux
Copy link
Contributor

Hello. thanks for the report. I tried reproduce the problem as you described in a minimal example, but I failed. Can you show some code that show the problem?

@fa1k3n
Copy link
Contributor Author

fa1k3n commented Oct 19, 2020 via email

@fa1k3n
Copy link
Contributor Author

fa1k3n commented Oct 20, 2020

Here is a minimal unittest that will provoke the error and thus fail

import unittest                                                                                                                               
from ocpp.v20 import ChargePoint as cp                                                                                                        
                                                                                                                                          
class ChargePoint(cp):                                                                                                                        
    @property                                                                                                                                 
    def foo(self):                                                                                                                            
        raise Exception("this will be raised")                                                                                                     
                                                                                                                                          
class TestChargePoint(unittest.TestCase):                                                                                                     
    def test_getters_should_not_be_called_during_routemap_setup(self):                                                                        
        try:                                                                                                                                  
            ChargePoint("blah", None)                                                                                                         
        except Exception as e:                                                                                                                
            self.assertEqual(str(e), "this will be raised”)                                                                                        
            self.fail("Getter was called during ChargePoint creation")                                                                        
        self.assertTrue(True)                                                                                                                 
                                                                                                                                          
if __name__ == '__main__':                                                                                                                    
    unittest.main()

@OrangeTux
Copy link
Contributor

Thanks for the code. I was able to reproduce the error.

@OrangeTux OrangeTux added the bug Something isn't working label Oct 20, 2020
@fa1k3n
Copy link
Contributor Author

fa1k3n commented Oct 20, 2020

Thanks for the code. I was able to reproduce the error.

excellent, do you want me to do a PR for my solution?

@OrangeTux
Copy link
Contributor

Sure! It would be great.

@fa1k3n
Copy link
Contributor Author

fa1k3n commented Oct 21, 2020

just did it, all tests passed locally, CI is running right now

ajmirsky pushed a commit to ajmirsky/ocpp that referenced this issue Nov 26, 2024
For subclasses of `ChargePoint` using `@property` generating the route map could have unintended
side effects. The function `ocpp.routing.create_route_map()` iterates over all attributes of the subclass, thus
executing the method decorated with `@property`. 

This commit addressed that issue. 

Fixed: mobilityhouse#137 

Co-authored-by: Auke Willem Oosterhoff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants