Add __getstate__ to allow for pickling

[csanders-git]

The addition of getattr fixed https://oktainc.atlassian.net/browse/OKTA-384293, breaks pickling of user objects. This is because pickle will attempt to check if the object has the getstate method. Because the class doesn't define getstate and now defined getattr it will be called with getstate as the attr_name. This will cause an issue beause attr_name is passed to okta/helpers.py line 42, to_lower_camel_case() which performs a lookup on a that attr_name split by '_'.

The remaining code assumes that there is a string of length > 0 which is incorrect in the getstate scenario which behaves as follows:

['', '', 'getstate', '', '']
components[0][0].lower()
IndexError

There are two solutions to resolve this, the easiest is specifying a getstate function such that the invalid assumption of the content of components[0] is never made.

[serhiibuniak-okta]

@csanders-git Thanks for your investigation and contribution. This change should be here openapi/templates/model.py.hbs also. I believe, this is an obvious fix, but since it does change functionality and you don't mind, please sign and send a CLA over to us - https://developer.okta.com/cla/ . Meanwhile, I'll make a new branch and include your commit. Will keep you posted here.

[csanders-git]

@sergiishamrai-okta i've actually become more convinced that the unchecked assumption of components [0][0] is the real thing that should be fixed. i.e if comonents[0] has a length of 0 an attribute error should be thrown.

[serhiibuniak-okta]

@csanders-git Yes, that place is not safe, but it was designed to convert profile attributes from pythonic name style to lowerCamelCase style and wasn't supposed to used outside. Do you have any use cases? You can create separate issue for this and we'll take it into consideration too.

[csanders-git]

@serhiibuniak-okta the usecase is from above -- getstate will be passed to the function and end up failing, if the check is added and an attribute error is thrown when conditions are met you could avoid adding a getstate function.

[serhiibuniak-okta]

@csanders-git Could you please, show some code on how do you perform pickling/unpickling? I've added __getstate__ and __setstate__, then I've managed to pickle/unpickle User object and use unpickled in some api queries.

[csanders-git]

Yes, this works because it is able to find __getstate__ but in reality you likely don't want to define your code around 3rd party library support since you're not overriding your serialization with custom routines.

My suggestion would be to fix the getattr so that the boundary checking is done or you can add something like this

    if attr.startswith('__') and attr.endswith('__'):
        raise AttributeError

[serhiibuniak-okta]

@csanders-git Could you please, provide some code on how you get this error? I guess, that some profile attributes might be with double underscores? Although, nobody thought about profile attributes with double underscores, but in this case it really fails. I agree, we need to fix __getattr__, but it won't help with pickling - that's the main issue we need to fix?

[serhiibuniak-okta]

@csanders-git I think about fixing function "to_lower_camel_case":

if components[0]:
    components[0] = components[0][0].lower() + components[0][1:]

It seems better than playing with __getstate__ and __setstate__. What do you think?

[serhiibuniak-okta]

@csanders-git I've added a PR - #209. Let me know if it works for you.

[csanders-git]

This works

[serhiibuniak-okta]

@csanders-git Thanks for letting us know it works. We will include this fix into upcoming SDK release.

[serhiibuniak-okta]

@csanders-git SDK 1.7.0 released, fix included