ZF-2141: Zend_Gdata_AuthSub::AuthSubRevokeToken() generates warning

Issue Type:
Bug
Created:
2007-11-03T10:55:04.000+0000
Last Updated:
2008-02-26T12:56:02.000+0000
Status:
Resolved
Fix version(s):
  • 1.0.4 (26/Feb/08)
Reporter:
Carsten Möhrke (donnerchen)
Assignee:
Ryan Boyd (rboyd)
Tags:
  • Zend_Gdata
Related issues:
Attachments:

Description

Calling Zend_Gdata_AuthSub::AuthSubRevokeToken() results in a warning:

Notice: Undefined variable: source in /Zend/Gdata/AuthSub.php on line 153

Comments

Posted by Ryan Boyd (rboyd) on 2007-11-04T09:45:02.000+0000

Fixed by no longer referencing the undefined variable $source. Sending out request for code review to fw-gdata allow for this to be released with the next release.

Posted by Trevor Johns (tjohns) on 2007-11-04T19:44:04.000+0000

Review for r6732:

I don't see anything wrong with the changes made. Feel free to merge this into release-1.0.

However, is there a reason that getHttpClient() isn't being called anywhere here? This looks like the sort of thing that could have easily been caught with a unit test.

Posted by Ryan Boyd (rboyd) on 2007-11-04T21:09:26.000+0000

Thanks for the review Trevor.

Agreed that getHttpClient could be called to generate the header and avoid duplicate code (in this method and others). Filed as http://framework.zend.com/issues/browse/ZF-2145.

Yes, it could be caught in a unit test, if we used mock HTTP client classes, but we haven't used those yet for Zend_Gdata. Filed as: http://framework.zend.com/issues/browse/ZF-2146

Posted by Trevor Johns (tjohns) on 2007-11-05T01:43:01.000+0000

While mock HTTP clients would certainly be nice, in this case we just would have had to check that getHttpClient() is returning a properly instantiated instance of Zend_Http_Client.

Posted by Ryan Boyd (rboyd) on 2007-11-05T08:24:57.000+0000

Gotcha, I thought you were mentioning that we needed testing on AuthSubRevokeToken(sic), which couldn't be tested fully without a mock client. Testing getHttpClient can be tested, and would have caught this problem (as you pointed out) if we were using getHttpClient in the other methods.

Thanks Trevor! -Ryan