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

Scrape @imsamurai fork for enhancements/tweaks #14

Open
ProLoser opened this issue Dec 4, 2012 · 3 comments
Open

Scrape @imsamurai fork for enhancements/tweaks #14

ProLoser opened this issue Dec 4, 2012 · 3 comments

Comments

@ProLoser
Copy link
Member

ProLoser commented Dec 4, 2012

Reviewing @imsamurai's fork there are some cleaner implementations I would like to merge back into this fork for greater code-clarity and efficiency.

I'm still not sure where I sit on the approach to OAuth and field mapping as the field mapping seems a bit complex for me to easily grasp at the moment and the OAuth integration is intended to allow REST queries.
Need to poke around a bit more to see if Opauth credentials are supplemented into RESTful requests for further merger consideration.

@ProLoser
Copy link
Member Author

ProLoser commented Dec 4, 2012

@imsamurai: I was reviewing your decision to use opauth and checked out his CakePHP Auth implementation. Unfortunately it looks like he is not doing this properly because he is not extending the Auth component, does not have have any form of reusable component, and does not place information about the authenticated user in the session (for access app-wide).

In addition, adding new strategies appears to be somewhat confusing, complex and convoluted. Although opauth is a nice idea in theory, it seems to actually have less functionality than my current implementation.
In addition, there is already plans to do a proper CakePHP2.x implementation of integrated Auth as discussed on issue #5.

I would be a little frustrated if I had to learn 2 plugins, (opauth and api) in order to add more providers/strategies.

However, I would be interested in overall merging efforts as your code seems to be quite a bit cleaner (such as with logging and decoders, etc) and I may be open to expanding support for field and condition mapping.
You also (temporarily, I'm assuming) picked up some inconsistencies I had made in my own code as I was still figuring things out (such as the $options property or setting the oauth version in the constructor). I am actually concerned these may have not been versatile enough and have planned on phasing them out.

If you're interested, I can discuss with you some of the differences in our code and the reasons for my own implementations and thoughts regarding your own improvements you've added.

@imsamurai
Copy link

Yes, it is interesting to me.
About OAuth - not trying yet to make plugin for HttpSource, so I not think about hot to better set parameters, etc.
Please tell what $options did you mean?

Although opauth is a nice idea in theory, it seems to actually have less functionality than my current implementation

Maybe you right. In any case I think better to separate these two things - login and http source. So people can use other login implementation or not use at all. But I could be wrong)

@ProLoser
Copy link
Member Author

ProLoser commented Dec 4, 2012

In my version, oauth is entirely optional. It degrades to using a simple http socket. Check out my jsFiddle plugin.

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

No branches or pull requests

2 participants