Re: [PEPr] Comment on Web Services::OAuth

Hi Till, To confirm, there is a bit of work left to do to complete the component to the point I'd be comfortable calling for votes. It's not a huge amount - mostly down to some small bug fixes and updates surrounding the use of other PEAR dependencies (like Net_URL2 and the OAuth client itself).. A lot of this is down to refactoring that occured outside of the PEAR library I need to push in. On Curl, if you do the grunt work ;), I can add you to the Google project to commit it. I have wanted to remove the HTTP_Request references in favour of a proxy interface since I don't necessarily always use HTTP_Request either. >Then I was wondering if you'd provide a build in way to override >OAuth_Consumer::redirect() (I don't like the "header()-call" in there, I'd >rather provide a screen to the user stating, 'You are redirect because of >...'.) or would you rather prefer people to extend your code and override >there? The redirect() method is mainly a convenience method. It's easy to create your own process using the getRedirectUrl() accessor as a base - which is the only piece of data of importance generated there. >Aside, some of your code could use some inline documentations. At the same >time I also want to add that I didn't have too much trouble finding my way >around. Great work. And I bet you are working on that before you call for >votes. ;) My inline documentation generally is done last. I find on these types of libraries it's easy to spend as much time migrating comments than the refactoring that sparked to need to do so :). I only comment once the code has stabilised to the point that refactoring is unlikely to effect the overall general design which as of the last commit was a fairly new condition - so fewer comments than even I would like. I think most of the lacking comments on more internal classes. I'm glad it was readable at least! >The reason why it dies is because your example does "new >OAuth_Consumer($consumer_key, $consumer_key_secret, $options);", while the >definition of the __construct() states a single parameter "$options". I >figured out that if I included the keys 'consumerKey' and 'consumerSecret' >in the $options it works. One of the most recent refactorings was to recognise the duplication of options across multiple classes. Rather than put up with that uncertainty I introduced a configuration interface and container and pushed those options into it - so they are no longer constructor parameters by themselves. Guess I need to update the examples to reflect that! >To cut a long story short, once that was fixed it also worked on Pownce. :) Cool! More services it's tested against the more edge cases might turn up. However Pownce has been on the OAuth bandwagon for a long time so they weren't a priority. Surprisingly it was Google who ended up needing some extra work since their Java Oauth Server did some odd stuff. Any other questions feel free to email me. Won't be long before I get back to the final bits and pieces and call a vote. I'm off work for another two weeks in September ;). Paddy Pádraic Brady http://blog.astrumfutura.com http://www.patternsforphp.com OpenID Europe Foundation ----- Original Message ---- From: Till Klampaeckel <till@php.net> To: PEAR developer mailinglist <pear-dev@lists.php.net> Cc: Till Klampaeckel <till@php.net>; Pádraic Brady <padraic.brady@yahoo.com> Sent: Monday, August 18, 2008 11:24:52 PM Subject: [PEAR-DEV] [PEPr] Comment on Web Services::OAuth Till Klampaeckel (http://pear.php.net/user/till) has commented on the proposal for Web Services::OAuth. Comment: Hey, I saw your updates, but the code I recently checked out (r38) from Google Code still had Net_URL in it (vs. you talking about Net_URL2), or is this my misunderstanding? Also, speaking of PHP4 vs 5 - I was wondering if you'd like to do a cURL-based driver for all HTTP instead of HTTP_Request, I'd contribute code also if you are too busy. Just let me know. Then I was wondering if you'd provide a build in way to override OAuth_Consumer::redirect() (I don't like the "header()-call" in there, I'd rather provide a screen to the user stating, 'You are redirect because of ..'.) or would you rather prefer people to extend your code and override there? Aside, some of your code could use some inline documentations. At the same time I also want to add that I didn't have too much trouble finding my way around. Great work. And I bet you are working on that before you call for votes. ;) Last but not least, a bug report: In OAuth_Consumer::setRequestScheme() your example dies. "'e' is an unsupported request scheme" I couldn't figure out at first why the in_array() with the constants did not work. (Besides, maybe make the array() 'static'?) When you comment it out, it moves on but the same issue is brought up by OAuth_Http_RequestToken::_attemptRequest(). Due to it being unable to match the scheme, the $httpClient var is null. The reason why it dies is because your example does "new OAuth_Consumer($consumer_key, $consumer_key_secret, $options);", while the definition of the __construct() states a single parameter "$options". I figured out that if I included the keys 'consumerKey' and 'consumerSecret' in the $options it works. To cut a long story short, once that was fixed it also worked on Pownce. :) If you do any updates, let me know. I'd try it out right away. Cheers, Till Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=512 -- Sent by PEPr, the automatic proposal system at http://pear.php.net -- PEAR Development Mailing List (http://pear.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php