Fix bug in StorageByKeyName

[roat, 2012-03-08 22:17:04]

Thanks for the update, and adding the test!

On Thu, Mar 8, 2012 at 1:31 PM, <jcgregorio@google.com> wrote:

> Reviewers: aiuto,
>
>
>
> Please review this at
http://codereview.appspot.com/**5782058/<http://codereview.appspot.com/5782058/>
>
> Affected files:
>  M apiclient/discovery.py
>  M oauth2client/appengine.py
>  M runtests.py
>  M tests/test_oauth2client_**appengine.py
>
>
> Index: apiclient/discovery.py
> ==============================**==============================**=======
> --- a/apiclient/discovery.py
> +++ b/apiclient/discovery.py
> @@ -55,6 +55,7 @@
>  from email.mime.nonmultipart import MIMENonMultipart
>  from oauth2client.anyjson import simplejson
>
> +logger = logging.getLogger(__name__)
>
>  URITEMPLATE = re.compile('{[^}]*}')
>  VARNAME = re.compile('[a-zA-Z0-9_-]+')
> @@ -175,7 +176,7 @@
>   if 'REMOTE_ADDR' in os.environ:
>     requested_url = _add_query_parameter(**requested_url, 'userIp',
>                                          os.environ['REMOTE_ADDR'])
> -  logging.info('URL being requested: %s' % requested_url)
> +  logger.info('URL being requested: %s' % requested_url)
>
>   resp, content = http.request(requested_url)
>
> @@ -188,7 +189,7 @@
>   try:
>     service = simplejson.loads(content)
>   except ValueError, e:
> -    logging.error('Failed to parse as JSON: ' + content)
> +    logger.error('Failed to parse as JSON: ' + content)
>     raise InvalidJsonError()
>
>   filename = os.path.join(os.path.dirname(_**_file__), 'contrib',
> @@ -536,7 +537,7 @@
>                                        'boundary="%s"') %
> multipart_boundary
>             url = _add_query_parameter(url, 'uploadType', 'multipart')
>
> -      logging.info('URL being requested: %s' % url)
> +      logger.info('URL being requested: %s' % url)
>       return self._requestBuilder(self._**http,
>                                   model.response,
>                                   url,
> @@ -616,7 +617,7 @@
>       headers = {}
>       headers, params, query, body = self._model.request(headers, {}, {},
> None)
>
> -      logging.info('URL being requested: %s' % url)
> +      logger.info('URL being requested: %s' % url)
>       resp, content = self._http.request(url, method='GET',
> headers=headers)
>
>       return self._requestBuilder(self._**http,
> @@ -663,7 +664,7 @@
>
>       request.uri = uri
>
> -      logging.info('URL being requested: %s' % uri)
> +      logger.info('URL being requested: %s' % uri)
>
>       return request
>
> Index: oauth2client/appengine.py
> ==============================**==============================**=======
> --- a/oauth2client/appengine.py
> +++ b/oauth2client/appengine.py
> @@ -224,7 +224,7 @@
>       if credential and hasattr(credential, 'set_store'):
>         credential.set_store(self)
>         if self._cache:
> -          self._cache.set(self._key_**name, credentials.to_json())
> +          self._cache.set(self._key_**name, credential.to_json())
>
>     return credential
>
> Index: runtests.py
> ==============================**==============================**=======
> --- a/runtests.py
> +++ b/runtests.py
> @@ -7,6 +7,8 @@
>  import unittest
>  from trace import fullmodname
>
> +logging.basicConfig(level=**logging.CRITICAL)
> +
>  APP_ENGINE_PATH='../google_**appengine'
>
>  # Conditional import of cleanup function
> Index: tests/test_oauth2client_**appengine.py
> ==============================**==============================**=======
> --- a/tests/test_oauth2client_**appengine.py
> +++ b/tests/test_oauth2client_**appengine.py
> @@ -23,6 +23,7 @@
>  __author__ = 'jcgregorio@google.com (Joe Gregorio)'
>
>  import base64
> +import datetime
>  import httplib2
>  import time
>  import unittest
> @@ -42,15 +43,20 @@
>  from google.appengine.api import apiproxy_stub_map
>  from google.appengine.api import app_identity
>  from google.appengine.api import users
> +from google.appengine.api import memcache
>  from google.appengine.api.memcache import memcache_stub
> +from google.appengine.ext import db
>  from google.appengine.ext import testbed
>  from google.appengine.runtime import apiproxy_errors
>  from oauth2client.anyjson import simplejson
>  from oauth2client.appengine import AppAssertionCredentials
> +from oauth2client.appengine import CredentialsModel
>  from oauth2client.appengine import OAuth2Decorator
>  from oauth2client.appengine import OAuth2Handler
> +from oauth2client.appengine import StorageByKeyName
>  from oauth2client.client import AccessTokenRefreshError
>  from oauth2client.client import FlowExchangeError
> +from oauth2client.client import OAuth2Credentials
>  from webtest import TestApp
>
>  class UserMock(object):
> @@ -132,6 +138,75 @@
>     self.assertEqual('a_token_123'**, credentials.access_token)
>
>
> +def _http_request(*args, **kwargs):
> +  resp = httplib2.Response({'status': '200'})
> +  content = simplejson.dumps({'access_**token': 'bar'})
> +
> +  return resp, content
> +
> +
> +class StorageByKeyNameTest(unittest.**TestCase):
> +
> +  def setUp(self):
> +    self.testbed = testbed.Testbed()
> +    self.testbed.activate()
> +    self.testbed.init_datastore_**v3_stub()
> +    self.testbed.init_memcache_**stub()
> +    self.testbed.init_user_stub()
> +
> +    access_token = "foo"
> +    client_id = "some_client_id"
> +    client_secret = "cOuDdkfjxxnv+"
> +    refresh_token = "1/0/a.df219fjls0"
> +    token_expiry = datetime.datetime.utcnow()
> +    token_uri =
"https://www.google.com/**accounts/o8/oauth2/token<https://www.google.com/accounts/o8/oauth2/token>
> "
> +    user_agent = "refresh_checker/1.0"
> +    self.credentials = OAuth2Credentials(
> +      access_token, client_id, client_secret,
> +      refresh_token, token_expiry, token_uri,
> +      user_agent)
> +
> +  def tearDown(self):
> +    self.testbed.deactivate()
> +
> +  def test_get_and_put_simple(self):
> +    storage = StorageByKeyName(
> +      CredentialsModel, 'foo', 'credentials')
> +
> +    self.assertEqual(None, storage.get())
> +    self.credentials.set_store(**storage)
> +
> +    self.credentials._refresh(_**http_request)
> +    credmodel = CredentialsModel.get_by_key_**name('foo')
> +    self.assertEqual('bar', credmodel.credentials.access_**token)
> +
> +  def test_get_and_put_cached(self):
> +    storage = StorageByKeyName(
> +      CredentialsModel, 'foo', 'credentials', cache=memcache)
> +
> +    self.assertEqual(None, storage.get())
> +    self.credentials.set_store(**storage)
> +
> +    self.credentials._refresh(_**http_request)
> +    credmodel = CredentialsModel.get_by_key_**name('foo')
> +    self.assertEqual('bar', credmodel.credentials.access_**token)
> +
> +    # Now remove the item from the cache.
> +    memcache.delete('foo')
> +
> +    # Check that getting refreshes the cache.
> +    credentials = storage.get()
> +    self.assertEqual('bar', credentials.access_token)
> +    self.assertNotEqual(None, memcache.get('foo'))
> +
> +    # Deleting should clear the cache.
> +    storage.delete()
> +    credentials = storage.get()
> +    self.assertEqual(None, credentials)
> +    self.assertEqual(None, memcache.get('foo'))
> +
> +
> +
>  class DecoratorTests(unittest.**TestCase):
>
>   def setUp(self):
>
>
>

[aiuto, 2012-03-12 01:25:11]

lgtm

[jcgregorio_google, 2012-03-12 12:46:39]

On 2012/03/12 01:25:11, aiuto wrote:
> lgtm

Committed in
http://code.google.com/p/google-api-python-client/source/detail?r=fcf98d8a195...

[jcgregorio_google, 2012-03-12 13:30:13]

For future reference please follow the guidelines for submitting code:

  
http://code.google.com/p/google-api-python-client/wiki/BecomingAContributor#S...

Which includes adding a reference to the codereview in the commit message.

On 2012/03/12 12:46:39, jcgregorio_google wrote:
> On 2012/03/12 01:25:11, aiuto wrote:
> > lgtm
> 
> Committed in
>
http://code.google.com/p/google-api-python-client/source/detail?r=fcf98d8a195...

[jcgregorio_google, 2012-03-12 13:37:50]

D'oh, wrong code review for that comment. Please ignore.

On Mon, Mar 12, 2012 at 9:30 AM, <jcgregorio@google.com> wrote:

> For future reference please follow the guidelines for submitting code:
>
>
> http://code.google.com/p/**google-api-python-client/wiki/**
>
BecomingAContributor#**Submitting_Your_Approved_Code<http://code.google.com/p/google-api-python-client/wiki/BecomingAContributor#Submitting_Your_Approved_Code>
>
> Which includes adding a reference to the codereview in the commit
> message.
>
>
> On 2012/03/12 12:46:39, jcgregorio_google wrote:
>
>> On 2012/03/12 01:25:11, aiuto wrote:
>> > lgtm
>>
>
>  Committed in
>>
>
> http://code.google.com/p/**google-api-python-client/**source/detail?r=**
>
fcf98d8a1955e78cf349178999e072**aeec148110<http://code.google.com/p/google-api-python-client/source/detail?r=fcf98d8a1955e78cf349178999e072aeec148110>
>
>
>
http://codereview.appspot.com/**5782058/<http://codereview.appspot.com/5782058/>
>