Repository: google/google-api-python-client
Pull request #493 (2018-04-20 19:10:36)

Remove oauth2client._helpers dependency
oauth2client is deprecated
Migrate required helper's code from oauth2client to googleapiclient to
decrease our dependency on oauth2client

_helpers.py copied from oauth2client code
https://github.com/google/oauth2client/blob/master/oauth2client/_helpers.py

helen-fornazier: Please review, I am not entirely sure if this is the appropriated solution.
But with this change and #492 I can already manipulate instances using just google-auth, after uninstalling oauth2client

More specifically, what I am trying to run with google-auth instead of oauth2client is:
[code]: https://github.com/ClusterLabs/fence-agents/blob/master/fence/agents/gce/fence_gce.py
[PR]: https://github.com/ClusterLabs/fence-agents/pull/197

Thanks
helen-fornazier: @jonparrott done, I imported the tests too as suggested. Could you please take a look in the commits?

Thanks
helen-fornazier: Also, now the only file inside `googleapiclient/` who uses oauth2client is `googleapiclient/sample_tools.py`, do you know if this module is used by external libraries?
It seems to me that this file is just an utility module used by `samples/`, so I was wondering if we could move `sample_tools.py`to the `samples/` folder and completely remove oauth2client from the `install_requires` list in the setup.py
helen-fornazier: @jonparrott lgtm, I added your suggestion in the PR, could you take a look please?

Can I remove oauth2client from the install_requires list or replace it by google-auth? https://github.com/google/google-api-python-client/blob/master/setup.py#L67


jonparrott: > Can I remove oauth2client from the install_requires list or replace it by google-auth? 

Yes, but let's two in two releases: one to have this, and then a second one to remove oauth2client.

Patch:
diff --git a/googleapiclient/_helpers.py b/googleapiclient/_helpers.py
new file mode 100644
index 000000000..5e8184ba5
--- /dev/null
+++ b/googleapiclient/_helpers.py
@@ -0,0 +1,204 @@
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Helper functions for commonly used utilities."""
+
+import functools
+import inspect
+import logging
+import warnings
+
+import six
+from six.moves import urllib
+
+
+logger = logging.getLogger(__name__)
+
+POSITIONAL_WARNING = 'WARNING'
+POSITIONAL_EXCEPTION = 'EXCEPTION'
+POSITIONAL_IGNORE = 'IGNORE'
+POSITIONAL_SET = frozenset([POSITIONAL_WARNING, POSITIONAL_EXCEPTION,
+                            POSITIONAL_IGNORE])
+
+positional_parameters_enforcement = POSITIONAL_WARNING
+
+_SYM_LINK_MESSAGE = 'File: {0}: Is a symbolic link.'
+_IS_DIR_MESSAGE = '{0}: Is a directory'
+_MISSING_FILE_MESSAGE = 'Cannot access {0}: No such file or directory'
+
+
+def positional(max_positional_args):
+    """A decorator to declare that only the first N arguments my be positional.
+
+    This decorator makes it easy to support Python 3 style keyword-only
+    parameters. For example, in Python 3 it is possible to write::
+
+        def fn(pos1, *, kwonly1=None, kwonly1=None):
+            ...
+
+    All named parameters after ``*`` must be a keyword::
+
+        fn(10, 'kw1', 'kw2')  # Raises exception.
+        fn(10, kwonly1='kw1')  # Ok.
+
+    Example
+    ^^^^^^^
+
+    To define a function like above, do::
+
+        @positional(1)
+        def fn(pos1, kwonly1=None, kwonly2=None):
+            ...
+
+    If no default value is provided to a keyword argument, it becomes a
+    required keyword argument::
+
+        @positional(0)
+        def fn(required_kw):
+            ...
+
+    This must be called with the keyword parameter::
+
+        fn()  # Raises exception.
+        fn(10)  # Raises exception.
+        fn(required_kw=10)  # Ok.
+
+    When defining instance or class methods always remember to account for
+    ``self`` and ``cls``::
+
+        class MyClass(object):
+
+            @positional(2)
+            def my_method(self, pos1, kwonly1=None):
+                ...
+
+            @classmethod
+            @positional(2)
+            def my_method(cls, pos1, kwonly1=None):
+                ...
+
+    The positional decorator behavior is controlled by
+    ``_helpers.positional_parameters_enforcement``, which may be set to
+    ``POSITIONAL_EXCEPTION``, ``POSITIONAL_WARNING`` or
+    ``POSITIONAL_IGNORE`` to raise an exception, log a warning, or do
+    nothing, respectively, if a declaration is violated.
+
+    Args:
+        max_positional_arguments: Maximum number of positional arguments. All
+                                  parameters after the this index must be
+                                  keyword only.
+
+    Returns:
+        A decorator that prevents using arguments after max_positional_args
+        from being used as positional parameters.
+
+    Raises:
+        TypeError: if a key-word only argument is provided as a positional
+                   parameter, but only if
+                   _helpers.positional_parameters_enforcement is set to
+                   POSITIONAL_EXCEPTION.
+    """
+
+    def positional_decorator(wrapped):
+        @functools.wraps(wrapped)
+        def positional_wrapper(*args, **kwargs):
+            if len(args) > max_positional_args:
+                plural_s = ''
+                if max_positional_args != 1:
+                    plural_s = 's'
+                message = ('{function}() takes at most {args_max} positional '
+                           'argument{plural} ({args_given} given)'.format(
+                               function=wrapped.__name__,
+                               args_max=max_positional_args,
+                               args_given=len(args),
+                               plural=plural_s))
+                if positional_parameters_enforcement == POSITIONAL_EXCEPTION:
+                    raise TypeError(message)
+                elif positional_parameters_enforcement == POSITIONAL_WARNING:
+                    logger.warning(message)
+            return wrapped(*args, **kwargs)
+        return positional_wrapper
+
+    if isinstance(max_positional_args, six.integer_types):
+        return positional_decorator
+    else:
+        args, _, _, defaults = inspect.getargspec(max_positional_args)
+        return positional(len(args) - len(defaults))(max_positional_args)
+
+
+def parse_unique_urlencoded(content):
+    """Parses unique key-value parameters from urlencoded content.
+
+    Args:
+        content: string, URL-encoded key-value pairs.
+
+    Returns:
+        dict, The key-value pairs from ``content``.
+
+    Raises:
+        ValueError: if one of the keys is repeated.
+    """
+    urlencoded_params = urllib.parse.parse_qs(content)
+    params = {}
+    for key, value in six.iteritems(urlencoded_params):
+        if len(value) != 1:
+            msg = ('URL-encoded content contains a repeated value:'
+                   '%s -> %s' % (key, ', '.join(value)))
+            raise ValueError(msg)
+        params[key] = value[0]
+    return params
+
+
+def update_query_params(uri, params):
+    """Updates a URI with new query parameters.
+
+    If a given key from ``params`` is repeated in the ``uri``, then
+    the URI will be considered invalid and an error will occur.
+
+    If the URI is valid, then each value from ``params`` will
+    replace the corresponding value in the query parameters (if
+    it exists).
+
+    Args:
+        uri: string, A valid URI, with potential existing query parameters.
+        params: dict, A dictionary of query parameters.
+
+    Returns:
+        The same URI but with the new query parameters added.
+    """
+    parts = urllib.parse.urlparse(uri)
+    query_params = parse_unique_urlencoded(parts.query)
+    query_params.update(params)
+    new_query = urllib.parse.urlencode(query_params)
+    new_parts = parts._replace(query=new_query)
+    return urllib.parse.urlunparse(new_parts)
+
+
+def _add_query_parameter(url, name, value):
+    """Adds a query parameter to a url.
+
+    Replaces the current value if it already exists in the URL.
+
+    Args:
+        url: string, url to add the query parameter to.
+        name: string, query parameter name.
+        value: string, query parameter value.
+
+    Returns:
+        Updated query parameter. Does not update the url if value is None.
+    """
+    if value is None:
+        return url
+    else:
+        return update_query_params(url, {name: value})
diff --git a/googleapiclient/channel.py b/googleapiclient/channel.py
index a38b4ffbc..0fdb080fe 100644
--- a/googleapiclient/channel.py
+++ b/googleapiclient/channel.py
@@ -61,15 +61,9 @@ import datetime
 import uuid
 
 from googleapiclient import errors
+from googleapiclient import _helpers as util
 import six
 
-# Oauth2client < 3 has the positional helper in 'util', >= 3 has it
-# in '_helpers'.
-try:
-  from oauth2client import util
-except ImportError:
-  from oauth2client import _helpers as util
-
 
 # The unix time epoch starts at midnight 1970.
 EPOCH = datetime.datetime.utcfromtimestamp(0)
diff --git a/googleapiclient/discovery.py b/googleapiclient/discovery.py
index e8b35273f..163f3c95b 100644
--- a/googleapiclient/discovery.py
+++ b/googleapiclient/discovery.py
@@ -73,14 +73,8 @@ from googleapiclient.model import MediaModel
 from googleapiclient.model import RawModel
 from googleapiclient.schema import Schemas
 
-# Oauth2client < 3 has the positional helper in 'util', >= 3 has it
-# in '_helpers'.
-try:
-  from oauth2client.util import _add_query_parameter
-  from oauth2client.util import positional
-except ImportError:
-  from oauth2client._helpers import _add_query_parameter
-  from oauth2client._helpers import positional
+from googleapiclient._helpers import _add_query_parameter
+from googleapiclient._helpers import positional
 
 
 # The client library requires a version of httplib2 that supports RETRIES.
diff --git a/googleapiclient/errors.py b/googleapiclient/errors.py
index bab14189f..8c4795c4c 100644
--- a/googleapiclient/errors.py
+++ b/googleapiclient/errors.py
@@ -23,12 +23,7 @@ __author__ = 'jcgregorio@google.com (Joe Gregorio)'
 
 import json
 
-# Oauth2client < 3 has the positional helper in 'util', >= 3 has it
-# in '_helpers'.
-try:
-  from oauth2client import util
-except ImportError:
-  from oauth2client import _helpers as util
+from googleapiclient import _helpers as util
 
 
 class Error(Exception):
diff --git a/googleapiclient/http.py b/googleapiclient/http.py
index 17c3714f7..29aef0e89 100644
--- a/googleapiclient/http.py
+++ b/googleapiclient/http.py
@@ -55,12 +55,7 @@ from email.mime.multipart import MIMEMultipart
 from email.mime.nonmultipart import MIMENonMultipart
 from email.parser import FeedParser
 
-# Oauth2client < 3 has the positional helper in 'util', >= 3 has it
-# in '_helpers'.
-try:
-  from oauth2client import util
-except ImportError:
-  from oauth2client import _helpers as util
+from googleapiclient import _helpers as util
 
 from googleapiclient import _auth
 from googleapiclient.errors import BatchError
diff --git a/googleapiclient/sample_tools.py b/googleapiclient/sample_tools.py
index 5ed632dab..21fede3ec 100644
--- a/googleapiclient/sample_tools.py
+++ b/googleapiclient/sample_tools.py
@@ -27,9 +27,13 @@ import os
 
 from googleapiclient import discovery
 from googleapiclient.http import build_http
-from oauth2client import client
-from oauth2client import file
-from oauth2client import tools
+
+try:
+    from oauth2client import client
+    from oauth2client import file
+    from oauth2client import tools
+except ImportError:
+    raise ImportError('googleapiclient.sample_tools requires oauth2client. Please install oauth2client and try again.')
 
 
 def init(argv, name, version, doc, filename, scope=None, parents=[], discovery_filename=None):
diff --git a/googleapiclient/schema.py b/googleapiclient/schema.py
index 160d388d8..10d4a1b59 100644
--- a/googleapiclient/schema.py
+++ b/googleapiclient/schema.py
@@ -65,12 +65,7 @@ __author__ = 'jcgregorio@google.com (Joe Gregorio)'
 
 import copy
 
-# Oauth2client < 3 has the positional helper in 'util', >= 3 has it
-# in '_helpers'.
-try:
-  from oauth2client import util
-except ImportError:
-  from oauth2client import _helpers as util
+from googleapiclient import _helpers as util
 
 
 class Schemas(object):


Test patch:
diff --git a/tests/test__helpers.py b/tests/test__helpers.py
new file mode 100644
index 000000000..e33ea7153
--- /dev/null
+++ b/tests/test__helpers.py
@@ -0,0 +1,161 @@
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unit tests for googleapiclient._helpers."""
+
+import unittest
+
+import mock
+
+import six
+from six.moves import urllib
+
+from googleapiclient import _helpers
+
+
+class PositionalTests(unittest.TestCase):
+
+    def test_usage(self):
+        _helpers.positional_parameters_enforcement = (
+            _helpers.POSITIONAL_EXCEPTION)
+
+        # 1 positional arg, 1 keyword-only arg.
+        @_helpers.positional(1)
+        def function(pos, kwonly=None):
+            return True
+
+        self.assertTrue(function(1))
+        self.assertTrue(function(1, kwonly=2))
+        with self.assertRaises(TypeError):
+            function(1, 2)
+
+        # No positional, but a required keyword arg.
+        @_helpers.positional(0)
+        def function2(required_kw):
+            return True
+
+        self.assertTrue(function2(required_kw=1))
+        with self.assertRaises(TypeError):
+            function2(1)
+
+        # Unspecified positional, should automatically figure out 1 positional
+        # 1 keyword-only (same as first case above).
+        @_helpers.positional
+        def function3(pos, kwonly=None):
+            return True
+
+        self.assertTrue(function3(1))
+        self.assertTrue(function3(1, kwonly=2))
+        with self.assertRaises(TypeError):
+            function3(1, 2)
+
+    @mock.patch('googleapiclient._helpers.logger')
+    def test_enforcement_warning(self, mock_logger):
+        _helpers.positional_parameters_enforcement = (
+            _helpers.POSITIONAL_WARNING)
+
+        @_helpers.positional(1)
+        def function(pos, kwonly=None):
+            return True
+
+        self.assertTrue(function(1, 2))
+        self.assertTrue(mock_logger.warning.called)
+
+    @mock.patch('googleapiclient._helpers.logger')
+    def test_enforcement_ignore(self, mock_logger):
+        _helpers.positional_parameters_enforcement = _helpers.POSITIONAL_IGNORE
+
+        @_helpers.positional(1)
+        def function(pos, kwonly=None):
+            return True
+
+        self.assertTrue(function(1, 2))
+        self.assertFalse(mock_logger.warning.called)
+
+
+class AddQueryParameterTests(unittest.TestCase):
+
+    def test__add_query_parameter(self):
+        self.assertEqual(
+            _helpers._add_query_parameter('/action', 'a', None),
+            '/action')
+        self.assertEqual(
+            _helpers._add_query_parameter('/action', 'a', 'b'),
+            '/action?a=b')
+        self.assertEqual(
+            _helpers._add_query_parameter('/action?a=b', 'a', 'c'),
+            '/action?a=c')
+        # Order is non-deterministic.
+        self.assertIn(
+            _helpers._add_query_parameter('/action?a=b', 'c', 'd'),
+            ['/action?a=b&c=d', '/action?c=d&a=b'])
+        self.assertEqual(
+            _helpers._add_query_parameter('/action', 'a', ' ='),
+            '/action?a=+%3D')
+
+
+def assertUrisEqual(testcase, expected, actual):
+    """Test that URIs are the same, up to reordering of query parameters."""
+    expected = urllib.parse.urlparse(expected)
+    actual = urllib.parse.urlparse(actual)
+    testcase.assertEqual(expected.scheme, actual.scheme)
+    testcase.assertEqual(expected.netloc, actual.netloc)
+    testcase.assertEqual(expected.path, actual.path)
+    testcase.assertEqual(expected.params, actual.params)
+    testcase.assertEqual(expected.fragment, actual.fragment)
+    expected_query = urllib.parse.parse_qs(expected.query)
+    actual_query = urllib.parse.parse_qs(actual.query)
+    for name in expected_query.keys():
+        testcase.assertEqual(expected_query[name], actual_query[name])
+    for name in actual_query.keys():
+        testcase.assertEqual(expected_query[name], actual_query[name])
+
+
+class Test_update_query_params(unittest.TestCase):
+
+    def test_update_query_params_no_params(self):
+        uri = 'http://www.google.com'
+        updated = _helpers.update_query_params(uri, {'a': 'b'})
+        self.assertEqual(updated, uri + '?a=b')
+
+    def test_update_query_params_existing_params(self):
+        uri = 'http://www.google.com?x=y'
+        updated = _helpers.update_query_params(uri, {'a': 'b', 'c': 'd&'})
+        hardcoded_update = uri + '&a=b&c=d%26'
+        assertUrisEqual(self, updated, hardcoded_update)
+
+    def test_update_query_params_replace_param(self):
+        base_uri = 'http://www.google.com'
+        uri = base_uri + '?x=a'
+        updated = _helpers.update_query_params(uri, {'x': 'b', 'y': 'c'})
+        hardcoded_update = base_uri + '?x=b&y=c'
+        assertUrisEqual(self, updated, hardcoded_update)
+
+    def test_update_query_params_repeated_params(self):
+        uri = 'http://www.google.com?x=a&x=b'
+        with self.assertRaises(ValueError):
+            _helpers.update_query_params(uri, {'a': 'c'})
+
+
+class Test_parse_unique_urlencoded(unittest.TestCase):
+
+    def test_without_repeats(self):
+        content = 'a=b&c=d'
+        result = _helpers.parse_unique_urlencoded(content)
+        self.assertEqual(result, {'a': 'b', 'c': 'd'})
+
+    def test_with_repeats(self):
+        content = 'a=b&a=d'
+        with self.assertRaises(ValueError):
+            _helpers.parse_unique_urlencoded(content)
diff --git a/tests/test_discovery.py b/tests/test_discovery.py
index 94027e3de..01c67ef66 100644
--- a/tests/test_discovery.py
+++ b/tests/test_discovery.py
@@ -78,10 +78,7 @@ from googleapiclient.model import JsonModel
 from oauth2client import GOOGLE_TOKEN_URI
 from oauth2client.client import OAuth2Credentials, GoogleCredentials
 
-try:
-  from oauth2client import util
-except ImportError:
-  from oauth2client import _helpers as util
+from googleapiclient import _helpers as util
 
 import uritemplate