From da5501fc8f8b65fd24152ea42ac92b5c8e31455b Mon Sep 17 00:00:00 2001
From: Jon Wayne Parrott <jon.wayne.parrott@gmail.com>
Date: Wed, 6 Jan 2016 10:19:06 -0800
Subject: [PATCH] Moving locking logic into storage base class.

---
 oauth2client/client.py                      | 16 +++++++++++++---
 oauth2client/contrib/appengine.py           |  2 ++
 oauth2client/contrib/django_orm.py          |  1 +
 oauth2client/contrib/django_util/storage.py |  1 +
 oauth2client/contrib/keyring_storage.py     | 17 +----------------
 oauth2client/file.py                        | 17 +----------------
 6 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/oauth2client/client.py b/oauth2client/client.py
index 065bd3313..15c473171 100644
--- a/oauth2client/client.py
+++ b/oauth2client/client.py
@@ -346,21 +346,31 @@ class Storage(object):
     such that multiple processes and threads can operate on a single
     store.
     """
+    def __init__(self, lock=None):
+        """Create a Storage instance.
+
+        Args:
+            lock: An optional threading.Lock-like object. Must implement at
+                  least acquire() and release(). Does not need to be re-entrant.
+        """
+        self._lock = lock
 
     def acquire_lock(self):
         """Acquires any lock necessary to access this Storage.
 
         This lock is not reentrant.
         """
-        pass
+        if self._lock is not None:
+            self._lock.acquire()
 
     def release_lock(self):
         """Release the Storage lock.
 
         Trying to release a lock that isn't held will result in a
-        RuntimeError.
+        RuntimeError in the case of a threading.Lock or multiprocessing.Lock.
         """
-        pass
+        if self._lock is not None:
+            self._lock.release()
 
     def locked_get(self):
         """Retrieve credential.
diff --git a/oauth2client/contrib/appengine.py b/oauth2client/contrib/appengine.py
index d12133692..a56cb7449 100644
--- a/oauth2client/contrib/appengine.py
+++ b/oauth2client/contrib/appengine.py
@@ -408,6 +408,8 @@ def __init__(self, model, key_name, property_name, cache=None, user=None):
             user: users.User object, optional. Can be used to grab user ID as a
                   key_name if no key name is specified.
         """
+        super(StorageByKeyName, self).__init__()
+
         if key_name is None:
             if user is None:
                 raise ValueError('StorageByKeyName called with no '
diff --git a/oauth2client/contrib/django_orm.py b/oauth2client/contrib/django_orm.py
index e6d31a602..cd22c1b71 100644
--- a/oauth2client/contrib/django_orm.py
+++ b/oauth2client/contrib/django_orm.py
@@ -124,6 +124,7 @@ def __init__(self, model_class, key_name, key_value, property_name):
             property_name: string, name of the property that is an
                            CredentialsProperty
         """
+        super(Storage, self).__init__()
         self.model_class = model_class
         self.key_name = key_name
         self.key_value = key_value
diff --git a/oauth2client/contrib/django_util/storage.py b/oauth2client/contrib/django_util/storage.py
index 72ec6cdcd..4f5637d32 100644
--- a/oauth2client/contrib/django_util/storage.py
+++ b/oauth2client/contrib/django_util/storage.py
@@ -31,6 +31,7 @@ class DjangoSessionStorage(client.Storage):
     """Storage implementation that uses Django sessions."""
 
     def __init__(self, session):
+        super(DjangoSessionStorage, self).__init__()
         self.session = session
 
     def locked_get(self):
diff --git a/oauth2client/contrib/keyring_storage.py b/oauth2client/contrib/keyring_storage.py
index 0a4c2857b..431b67b7a 100644
--- a/oauth2client/contrib/keyring_storage.py
+++ b/oauth2client/contrib/keyring_storage.py
@@ -59,24 +59,9 @@ def __init__(self, service_name, user_name):
                           credentials are stored.
             user_name: string, The name of the user to store credentials for.
         """
+        super(Storage, self).__init__(lock=threading.Lock())
         self._service_name = service_name
         self._user_name = user_name
-        self._lock = threading.Lock()
-
-    def acquire_lock(self):
-        """Acquires any lock necessary to access this Storage.
-
-        This lock is not reentrant.
-        """
-        self._lock.acquire()
-
-    def release_lock(self):
-        """Release the Storage lock.
-
-        Trying to release a lock that isn't held will result in a
-        RuntimeError.
-        """
-        self._lock.release()
 
     def locked_get(self):
         """Retrieve Credential from file.
diff --git a/oauth2client/file.py b/oauth2client/file.py
index d0dd174f9..d48235919 100644
--- a/oauth2client/file.py
+++ b/oauth2client/file.py
@@ -36,29 +36,14 @@ class Storage(BaseStorage):
     """Store and retrieve a single credential to and from a file."""
 
     def __init__(self, filename):
+        super(Storage, self).__init__(lock=threading.Lock())
         self._filename = filename
-        self._lock = threading.Lock()
 
     def _validate_file(self):
         if os.path.islink(self._filename):
             raise CredentialsFileSymbolicLinkError(
                 'File: %s is a symbolic link.' % self._filename)
 
-    def acquire_lock(self):
-        """Acquires any lock necessary to access this Storage.
-
-        This lock is not reentrant.
-        """
-        self._lock.acquire()
-
-    def release_lock(self):
-        """Release the Storage lock.
-
-        Trying to release a lock that isn't held will result in a
-        RuntimeError.
-        """
-        self._lock.release()
-
     def locked_get(self):
         """Retrieve Credential from file.