Skip to content

handle with tuple & list in django #6

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

watermelonlh
Copy link

since phpserializer serialize tuple & list into dict, so it might get error when use other authentication system, like https://github.com/pennersr/django-allauth . I wrapped the session_dict, hope this bridge could be more compatible with different session format.

@winhamwr
Copy link
Owner

Hello watermelon,

Thanks for the pull request.

I'm afraid I don't quite understand what this fixes. Is there any chance you could give some example session code/data that breaks in the current version and how this fixes it?

Also, it looks like your pull request uses 2 spaces for indentation. Would you mind converting it to 4 to stay compatible with PEP8?

Thanks
-Wes

@watermelonlh
Copy link
Author

Hi winhamwr,

I can tell the problem bothered me before.
My project is using https://github.com/pennersr/django-allauth as authentication system instead of the django default one. It works well before I add this bridge. The reason why I need this bridge is that I have to integrate another php project into this django project, and share the authentication. This bridge definitely helps!

After I finished the integration, I found it goes well on local account, but failed with all social account. I read the code both bridge & all-auth. The all-auth implemented social account part by itself, but just wrapped django auth for local. There is error line all-auth uses its social account session https://github.com/pennersr/django-allauth/blob/master/allauth/socialaccount/models.py#L283 . The reason this line fail, is because of the php serializer ,which imported in your bridge, encodes and decodes the python object in dict, no matter it is tuple or list. But this value of the key is a tuple, so this assignment statement get fail.

This is what my pull request fixed. I take note of what the exactly type of value in the original object( only tuple & list saved, dict is default so I just ignored, I defined '_bridge_type' is to store type of value, '_bridge_value' is to store the original value ).

So now, the decode will totally return the same session which send to the encode, not mess the tuple & list into dict.

I hope this explanation could help you to understand what my pull request do.

Thanks!

for key, value in iterable:
out[key] = self._wrap_type(value)
else:
out["_bridge_type"] = type(obj).__name__

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where most of the magic happens, every object is wrapped up and expects to know it's pythonic type as well as value. I'd be interested to see how the author proposes implementing this in PHP.

It seems that something like redis with it's own integral hash type would be a better fit than MySQL. I Realise this is all old code, but forcing python to use php serialized objects is not going to help you much and in-fact makes the whole process of migration harder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants