Merge lp:~brian.curtin/ubuntu-sso-client/remove-SyncTimestampChecker into lp:ubuntu-sso-client

[Brian Curtin]

- Remove SyncTimestampChecker as it is no longer used.

While discussing issues holding back a Python 3 branch, it was found that SyncTimestampChecker and its associated uses and tests should be removed. It has been covered by webclient for some time.

[Alejandro J. Cura (alecu)]

The branch looks very good.
Please remove also the associated imports used in this code.

review: Needs Fixing

[lp:~brian.curtin/ubuntu-sso-client/remove-SyncTimestampChecker]

991. By Brian Curtin on 2012-08-08

Remove unused code and imports

992. By Brian Curtin on 2012-08-08

Remove unused imports

993. By Brian Curtin on 2012-08-08

Patch the urlopen function from within urllib2/urllib.request itself, rather than depending on it having been imported in utils

[Brian Curtin (brian.curtin)]

The above three changes should have this taken care of. I removed a bunch of unused imports from the code and tests, and then changed the way another test was depending on a now removed import.

Note: none of those unused imports were showing up in my lint warnings, so if you find anymore, please let me know where they are.

[lp:~brian.curtin/ubuntu-sso-client/remove-SyncTimestampChecker]

994. By Brian Curtin on 2012-08-09

Removed unused time import

995. By Brian Curtin on 2012-08-09

Fix redefined name warnings, unused utils import

[Alejandro J. Cura (alecu)]

Pylint keeps complaining like this:

ubuntu_sso/tests/test_account.py:
    40: [F0401] Unable to import 'urllib.request'
    40: [E0611] No name 'request' in module 'urllib'
    107: [W0621, FakeWebClient.request] Redefining name 'url' from outer scope (line 40)
    220: [W0621, AccountTestCase.setUp.fake_urlopen] Redefining name 'url' from outer scope (line 40)

review: Needs Fixing

[lp:~brian.curtin/ubuntu-sso-client/remove-SyncTimestampChecker]

996. By Brian Curtin on 2012-08-14

Add lint warning disable/enable pairs

997. By Brian Curtin on 2012-08-14

Rename import so it doesn't cause redefinition collisions

[Mike McCracken (mikemc)]

+1 - tests pass, and after some diffing and futzing with pyflakes and pylint I determined that this branch doesn't add any pyflakes complaints that weren't in trunk, and we don't care about pylint anymore.

review: Approve

[Alejandro J. Cura (alecu)]

Looks good, and no pylint warnings now.

review: Approve

[Alejandro J. Cura (alecu)]

> +1 - tests pass, and after some diffing and futzing with pyflakes and pylint I
> determined that this branch doesn't add any pyflakes complaints that weren't
> in trunk, and we don't care about pylint anymore.

@mmcc: afaict, we still use pylint in this project; though we do use pyflakes elsewhere.

=== modified file 'ubuntu_sso/tests/test_account.py'
--- ubuntu_sso/tests/test_account.py	2012-07-25 20:56:33 +0000
+++ ubuntu_sso/tests/test_account.py	2012-08-14 16:26:21 +0000
@@ -35,11 +35,17 @@
 
 import os
 
+# pylint: disable=W0621,F0401,E0611
+try:
+    import urllib.request as url_lib
+except ImportError:
+    import urllib2 as url_lib
+# pylint: enable=W0621,F0401,E0611
+
 from twisted.trial.unittest import TestCase
 from twisted.internet import defer
 
 from ubuntu_sso import account
-from ubuntu_sso import utils
 from ubuntu_sso.account import (
     Account,
     AuthenticationError,
@@ -217,7 +223,7 @@
             self.addCleanup(f.close)
             return f
 
-        self.patch(utils, 'urlopen', fake_urlopen)  # fd to the path
+        self.patch(url_lib, 'urlopen', fake_urlopen)  # fd to the path
         self.processor = Account()
         self.register_kwargs = dict(email=EMAIL, password=PASSWORD,
                                     displayname=NAME,

=== modified file 'ubuntu_sso/utils/__init__.py'
--- ubuntu_sso/utils/__init__.py	2012-07-13 20:45:14 +0000
+++ ubuntu_sso/utils/__init__.py	2012-08-14 16:26:21 +0000
@@ -28,22 +28,10 @@
 # files in the program, then also delete it here.
 """Utility modules that may find use outside ubuntu_sso."""
 
-import cgi
 import os
 import sys
-import time
 
 from dirspec.utils import get_program_path
-from oauth import oauth
-
-try:
-    # pylint: disable=E0611,F0401
-    from urllib.parse import urlparse
-    from urllib.request import Request, urlopen
-    # pylint: enable=E0611,F0401
-except ImportError:
-    from urlparse import urlparse
-    from urllib2 import Request, urlopen
 
 from twisted.internet import defer
 
@@ -150,98 +138,6 @@
     return cmd_args
 
 
-class RequestHead(Request):
-    """A request with the method set to HEAD."""
-
-    _request_method = "HEAD"
-
-    def get_method(self):
-        """Return the desired method."""
-        return self._request_method
-
-
-class SyncTimestampChecker(object):
-    """A timestamp that's regularly checked with a server."""
-
-    CHECKING_INTERVAL = 60 * 60  # in seconds
-    ERROR_INTERVAL = 30  # in seconds
-    SERVER_URL = "http://one.ubuntu.com/api/time"
-
-    def __init__(self):
-        """Initialize this instance."""
-        self.next_check = time.time()
-        self.skew = 0
-
-    def get_server_time(self):
-        """Get the time at the server."""
-        headers = {"Cache-Control": "no-cache"}
-        request = RequestHead(self.SERVER_URL, headers=headers)
-        response = urlopen(request)
-        date_string = response.info()["Date"]
-        # delay import, otherwise a default reactor gets installed
-        from twisted.web import http
-        timestamp = http.stringToDatetime(date_string)
-        return timestamp
-
-    def get_faithful_time(self):
-        """Get an accurate timestamp."""
-        local_time = time.time()
-        if local_time >= self.next_check:
-            try:
-                server_time = self.get_server_time()
-                self.next_check = local_time + self.CHECKING_INTERVAL
-                self.skew = server_time - local_time
-                logger.debug("Calculated server-local time skew: %r",
-                             self.skew)
-            #pylint: disable=W0703
-            except Exception as server_error:
-                logger.debug("Error while verifying the server time skew: %r",
-                             server_error)
-                self.next_check = local_time + self.ERROR_INTERVAL
-
-        # delay import, otherwise a default reactor gets installed
-        from twisted.web import http
-        logger.debug("Using corrected timestamp: %r",
-                     http.datetimeToString(local_time + self.skew))
-        return int(local_time + self.skew)
-
-
-# pylint: disable=C0103
-timestamp_checker = SyncTimestampChecker()
-# pylint: enable=C0103
-
-
-def oauth_headers(url, credentials, http_method='GET'):
-    """Sign 'url' using 'credentials'.
-
-    * 'url' must be a valid unicode url.
-    * 'credentials' must be a valid OAuth token.
-
-    Return oauth headers that can be pass to any Request like object.
-
-    """
-    assert isinstance(url, unicode)
-    url = url.encode('utf-8')
-    _, _, _, _, query, _ = urlparse(url)
-    parameters = dict(cgi.parse_qsl(query))
-    parameters["oauth_timestamp"] = timestamp_checker.get_faithful_time()
-
-    consumer = oauth.OAuthConsumer(credentials['consumer_key'],
-                                   credentials['consumer_secret'])
-    token = oauth.OAuthToken(credentials['token'],
-                             credentials['token_secret'])
-    kwargs = dict(oauth_consumer=consumer, token=token,
-                  http_method=http_method, http_url=url,
-                  parameters=parameters)
-    get_request = oauth.OAuthRequest.from_consumer_and_token
-    oauth_req = get_request(**kwargs)
-    hmac_sha1 = oauth.OAuthSignatureMethod_HMAC_SHA1()
-    oauth_req.sign_request(hmac_sha1, consumer, token)
-    headers = oauth_req.to_header()
-
-    return headers
-
-
 @defer.inlineCallbacks
 def ping_url(url, email, credentials):
     """Ping the 'url' with the 'email' attached to it.

=== modified file 'ubuntu_sso/utils/tests/test_common.py'
--- ubuntu_sso/utils/tests/test_common.py	2012-07-12 15:54:13 +0000
+++ ubuntu_sso/utils/tests/test_common.py	2012-08-14 16:26:21 +0000
@@ -30,21 +30,14 @@
 
 import logging
 import sys
-import time
 
 from twisted.internet import defer
-from twisted.internet.threads import deferToThread
 from twisted.web import resource
 from ubuntuone.devtools.handlers import MementoHandler
 from ubuntuone.devtools.testing.txwebserver import HTTPWebServer
 
 from ubuntu_sso import utils
 from ubuntu_sso.tests import TestCase, EMAIL, TOKEN
-from ubuntu_sso.utils import (
-    oauth,
-    oauth_headers,
-    SyncTimestampChecker,
-)
 
 CONSTANTS_MODULE = 'ubuntu_sso.constants'
 NOT_DEFINED = object()
@@ -188,97 +181,6 @@
         self.assertEqual(expected, result)
 
 
-class SignWithCredentialsTestCase(TestCase):
-    """Test suite for the oauth_headers method."""
-
-    url = u'http://example.com'
-    fake_timestamp_value = 1
-
-    @defer.inlineCallbacks
-    def setUp(self):
-        """Initialize this test suite."""
-        yield super(SignWithCredentialsTestCase, self).setUp()
-        self.timestamp_called = False
-
-        def fake_timestamp():
-            """A fake timestamp that records the call."""
-            self.timestamp_called = True
-            return self.fake_timestamp_value
-
-        self.patch(utils.timestamp_checker, "get_faithful_time",
-                   fake_timestamp)
-
-    def build_header(self, url, http_method='GET'):
-        """Build an Oauth header for comparison."""
-        consumer = oauth.OAuthConsumer(TOKEN['consumer_key'],
-                                       TOKEN['consumer_secret'])
-        token = oauth.OAuthToken(TOKEN['token'],
-                                 TOKEN['token_secret'])
-        get_request = oauth.OAuthRequest.from_consumer_and_token
-        oauth_req = get_request(oauth_consumer=consumer, token=token,
-                                http_method=http_method, http_url=url)
-        oauth_req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
-                               consumer, token)
-        return oauth_req.to_header()
-
-    def dictify_header(self, header):
-        """Convert an OAuth header into a dict."""
-        result = {}
-        fields = header.split(', ')
-        for field in fields:
-            key, value = field.split('=')
-            result[key] = value.strip('"')
-
-        return result
-
-    def assert_header_equal(self, expected, actual):
-        """Is 'expected' equals to 'actual'?"""
-        expected = self.dictify_header(expected['Authorization'])
-        actual = self.dictify_header(actual['Authorization'])
-        for header in (expected, actual):
-            header.pop('oauth_nonce')
-            header.pop('oauth_timestamp')
-            header.pop('oauth_signature')
-
-        self.assertEqual(expected, actual)
-
-    def assert_method_called(self, path, query_str='', http_method='GET'):
-        """Assert that the url build by joining 'paths' was called."""
-        expected = (self.url, path, query_str)
-        expected = ''.join(expected).encode('utf8')
-        expected = self.build_header(expected, http_method=http_method)
-        actual = oauth_headers(url=self.url + path, credentials=TOKEN)
-        self.assert_header_equal(expected, actual)
-
-    def test_call(self):
-        """Calling 'get' triggers an OAuth signed GET request."""
-        path = u'/test/'
-        self.assert_method_called(path)
-
-    def test_quotes_path(self):
-        """Calling 'get' quotes the path."""
-        path = u'/test me more, sí!/'
-        self.assert_method_called(path)
-
-    def test_adds_parameters_to_oauth_request(self):
-        """The query string from the path is used in the oauth request."""
-        self.patch(oauth, 'OAuthRequest', FakedOAuthRequest)
-
-        path = u'/test/something?foo=bar'
-        oauth_headers(url=self.url + path, credentials=TOKEN)
-
-        self.assertIn('parameters', FakedOAuthRequest.params)
-        params = FakedOAuthRequest.params['parameters']
-        del(params["oauth_timestamp"])
-        self.assertEqual(params, {'foo': 'bar'})
-
-    def test_oauth_headers_uses_timestamp_checker(self):
-        """The oauth_headers function uses the timestamp_checker."""
-        oauth_headers(u"http://protocultura.net", TOKEN)
-        self.assertTrue(self.timestamp_called,
-                        "the timestamp MUST be requested.")
-
-
 class RootResource(resource.Resource):
     """A root resource that logs the number of calls."""
 
@@ -312,79 +214,6 @@
     """A mock, test, sample, and fake exception."""
 
 
-class TimestampCheckerTestCase(TestCase):
-    """Tests for the timestamp checker."""
-
-    @defer.inlineCallbacks
-    def setUp(self):
-        """Initialize a fake webserver."""
-        yield super(TimestampCheckerTestCase, self).setUp()
-        self.ws = MockWebServer()
-        self.ws.start()
-        self.addCleanup(self.ws.stop)
-        self.patch(SyncTimestampChecker, "SERVER_URL", self.ws.get_iri())
-
-    @defer.inlineCallbacks
-    def test_returned_value_is_int(self):
-        """The returned value is an integer."""
-        checker = SyncTimestampChecker()
-        timestamp = yield deferToThread(checker.get_faithful_time)
-        self.assertEqual(type(timestamp), int)
-
-    @defer.inlineCallbacks
-    def test_first_call_does_head(self):
-        """The first call gets the clock from our web."""
-        checker = SyncTimestampChecker()
-        yield deferToThread(checker.get_faithful_time)
-        self.assertEqual(self.ws.root.count, 1)
-
-    @defer.inlineCallbacks
-    def test_second_call_is_cached(self):
-        """For the second call, the time is cached."""
-        checker = SyncTimestampChecker()
-        yield deferToThread(checker.get_faithful_time)
-        yield deferToThread(checker.get_faithful_time)
-        self.assertEqual(self.ws.root.count, 1)
-
-    @defer.inlineCallbacks
-    def test_after_timeout_cache_expires(self):
-        """After some time, the cache expires."""
-        fake_timestamp = 1
-        self.patch(time, "time", lambda: fake_timestamp)
-        checker = SyncTimestampChecker()
-        yield deferToThread(checker.get_faithful_time)
-        fake_timestamp += SyncTimestampChecker.CHECKING_INTERVAL
-        yield deferToThread(checker.get_faithful_time)
-        self.assertEqual(self.ws.root.count, 2)
-
-    @defer.inlineCallbacks
-    def test_server_date_sends_nocache_headers(self):
-        """Getting the server date sends the no-cache headers."""
-        checker = SyncTimestampChecker()
-        yield deferToThread(checker.get_server_time)
-        assert len(self.ws.root.request_headers) == 1
-        headers = self.ws.root.request_headers[0]
-        result = headers.getRawHeaders("Cache-Control")
-        self.assertEqual(result, ["no-cache"])
-
-    @defer.inlineCallbacks
-    def test_server_error_means_skew_not_updated(self):
-        """When server can't be reached, the skew is not updated."""
-        fake_timestamp = 1
-        self.patch(time, "time", lambda: fake_timestamp)
-        checker = SyncTimestampChecker()
-
-        def failing_get_server_time():
-            """Let's fail while retrieving the server time."""
-            raise FakedError()
-
-        self.patch(checker, "get_server_time", failing_get_server_time)
-        yield deferToThread(checker.get_faithful_time)
-        self.assertEqual(checker.skew, 0)
-        self.assertEqual(checker.next_check,
-                         fake_timestamp + SyncTimestampChecker.ERROR_INTERVAL)
-
-
 class PingUrlTestCase(TestCase):
     """Test suite for the URL pinging."""