Merge lp:~alecu/ubuntu-sso-client/updated-txweb into lp:ubuntu-sso-client

[Alejandro J. Cura]

- Use HTTPClientFactory to allow replacing the reactor used to connect (LP: #929207).

- use HTTPClientFactory to allow replacing the reactor used to connect (LP: #929207)

[Diego Sarmentero (diegosarmentero)]

Looks good!
+1

review: Approve

[Eric Casteleijn (thisfred)]

Looks good

review: Approve

[Alejandro J. Cura (alecu)]

review: Abstain

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~alecu/ubuntu-sso-client/updated-txweb into lp:ubuntu-sso-client failed. Below is the output from the failed tests.

*** Running GTK test suite for ubuntu_sso ***
ubuntu_sso.xdg_base_directory.tests.test_common
  TestBaseDirectory
    test_load_config_paths_filter ... [OK]
    test_save_config_path ... [OK]
    test_xdg_cache_home_is_utf8_bytes ... [OK]
    test_xdg_config_dirs_are_bytes ... [OK]
    test_xdg_config_home_is_utf8_bytes ... [OK]
    test_xdg_data_dirs_are_bytes ... [OK]
    test_xdg_data_home_is_utf8_bytes ... [OK]
    test_xdg_home_is_utf8_bytes ... [OK]
twisted.trial.unittest
  TestCase
    runTest ... [OK]
ubuntu_sso.main.tests.test_clients
  AbstractTestCase
    test_error ... [OK]
    test_success ... [OK]
ubuntuone.devtools.testcases.dbus
  DBusTestCase
    runTest ... [OK]
ubuntu_sso.main.tests.test_clients
  ClearCredentialsTestCase
    test_error ... [OK]
    test_success ... [OK]
  CredentialsManagementProxyTestCase
    test_error ... [OK]
    test_success ... [OK]
  FindCredentialsTestCase
    test_error ... [OK]
    test_find_credentials_sync ... [OK]
    test_find_credentials_sync_error ... [OK]
    test_success ... [OK]
  GenerateCaptchaTestCase
    test_error ... [OK]
    test_success ... [OK]
  LoginEmailPasswordTestCase
    test_error ... [OK]
    test_success ... [OK]
  LoginOnlyTestCase
    test_error ... [OK]
    test_success ... [OK]
  LoginTestCase
    test_error ... [OK]
    test_error_when_setting_credentials ... [OK]
    test_not_validated ... [OK]
    test_success ... [OK]
  RegisterTestCase
    test_error ... [OK]
    test_success ... [OK]
  RegisterUse...

[lp:~alecu/ubuntu-sso-client/updated-txweb]

912. By Alejandro J. Cura on 2012-03-14

remove extra line that was annoying the pep8 filter

=== modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py'
--- ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-03-09 12:04:31 +0000
+++ ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-03-14 19:24:18 +0000
@@ -38,7 +38,7 @@
 )
 from ubuntu_sso.utils import webclient
 from ubuntu_sso.utils.ui import SSL_DETAILS_TEMPLATE
-from ubuntu_sso.utils.webclient import gsettings
+from ubuntu_sso.utils.webclient import gsettings, txweb
 from ubuntu_sso.utils.webclient.common import BaseWebClient, HeaderDict, oauth
 from ubuntu_sso.utils.webclient.tests import BaseMockWebServer
 
@@ -241,6 +241,7 @@
     """Test for the webclient."""
 
     timeout = 1
+    webclient_factory = webclient.webclient_factory
 
     @defer.inlineCallbacks
     def setUp(self):
@@ -248,7 +249,7 @@
         self.ws = MockWebServer()
         self.addCleanup(self.ws.stop)
         self.base_iri = self.ws.get_iri()
-        self.wc = webclient.webclient_factory()
+        self.wc = self.webclient_factory()
         self.addCleanup(self.wc.shutdown)
 
     @defer.inlineCallbacks
@@ -359,6 +360,57 @@
                         "The type of %r must be bytes" % result.content)
 
 
+class FakeSavingReactor(object):
+    """A fake reactor that saves connection attempts."""
+
+    def __init__(self):
+        """Initialize this fake instance."""
+        self.connections = []
+
+    def connectTCP(self, host, port, factory, *args):
+        """Fake the connection."""
+        self.connections.append((host, port, args))
+        factory.response_headers = {}
+        factory.deferred = defer.succeed("response content")
+
+    def connectSSL(self, host, port, factory, *args):
+        """Fake the connection."""
+        self.connections.append((host, port, args))
+        factory.response_headers = {}
+        factory.deferred = defer.succeed("response content")
+
+
+class TxWebClientTestCase(WebClientTestCase):
+    """Test case for txweb."""
+
+    webclient_factory = txweb.WebClient
+
+
+class TxWebClientReactorReplaceableTestCase(TestCase):
+    """In the txweb client the reactor is replaceable."""
+
+    timeout = 3
+    FAKE_HOST = u"fake"
+    FAKE_IRI_TEMPLATE = u"%%s://%s/fake_page" % FAKE_HOST
+
+    @defer.inlineCallbacks
+    def _test_replaceable_reactor(self, iri):
+        """The reactor can be replaced with the tunnel client."""
+        fake_reactor = FakeSavingReactor()
+        wc = txweb.WebClient(fake_reactor)
+        _response = yield wc.request(iri)
+        host, _port, _args = fake_reactor.connections[0]
+        self.assertEqual(host, self.FAKE_HOST)
+
+    def test_replaceable_reactor_http(self):
+        """Test the replaceable reactor with an http iri."""
+        return self._test_replaceable_reactor(self.FAKE_IRI_TEMPLATE % "http")
+
+    def test_replaceable_reactor_https(self):
+        """Test the replaceable reactor with an https iri."""
+        return self._test_replaceable_reactor(self.FAKE_IRI_TEMPLATE % "https")
+
+
 class TimestampCheckerTestCase(TestCase):
     """Tests for the timestampchecker classmethod."""
 

=== modified file 'ubuntu_sso/utils/webclient/txweb.py'
--- ubuntu_sso/utils/webclient/txweb.py	2012-02-07 19:36:50 +0000
+++ ubuntu_sso/utils/webclient/txweb.py	2012-03-14 19:24:18 +0000
@@ -16,11 +16,9 @@
 """A webclient backend that uses twisted.web.client."""
 
 import base64
-
-from StringIO import StringIO
-
-from twisted.internet import defer, protocol
-from zope.interface import implements
+import urlparse
+
+from twisted.internet import defer
 
 from ubuntu_sso.utils.webclient.common import (
     BaseWebClient,
@@ -31,64 +29,80 @@
 )
 
 
-class StringProtocol(protocol.Protocol):
-    """Hold the stuff received in a StringIO."""
-
-    # pylint: disable=C0103
-    def __init__(self):
-        """Initialize this instance."""
-        self.deferred = defer.Deferred()
-        self.content = StringIO()
-
-    def dataReceived(self, data):
-        """Some more blocks received."""
-        self.content.write(data)
-
-    def connectionLost(self, reason=protocol.connectionDone):
-        """No more bytes available."""
-        self.deferred.callback(self.content.getvalue())
-
-
-class StringProducer(object):
-    """Simple implementation of IBodyProducer."""
-
-    # delay import, otherwise a default reactor gets installed
-    from twisted.web import iweb
-
-    implements(iweb.IBodyProducer)
-
-    def __init__(self, body):
-        """Initialize this instance with some bytes."""
-        self.body = body
-        self.length = len(body)
-
-    # pylint: disable=C0103
-    def startProducing(self, consumer):
-        """Start producing to the given IConsumer provider."""
-        consumer.write(self.body)
-        return defer.succeed(None)
-
-    def pauseProducing(self):
-        """In our case, do nothing."""
-
-    def stopProducing(self):
-        """In our case, do nothing."""
+class RawResponse(object):
+    """A raw response from the webcall."""
+
+    def __init__(self, headers, content, code=200, phrase="OK"):
+        """Initialize this response."""
+        self.headers = headers
+        self.content = content
+        self.code = code
+        self.phrase = phrase
 
 
 class WebClient(BaseWebClient):
     """A simple web client that does not support proxies, yet."""
 
-    # delay import, otherwise a default reactor gets installed
-    from twisted.internet import reactor
-    from twisted.web import client, http, http_headers
-
-    # Undefined variable 'http_headers', 'client', 'reactor', 'http'
-    # pylint: disable=E0602
+    def __init__(self, connector=None, context_factory=None, **kwargs):
+        """Initialize this webclient."""
+        super(WebClient, self).__init__(**kwargs)
+
+        if connector is None:
+            from twisted.internet import reactor
+            self.connector = reactor
+        else:
+            self.connector = connector
+
+        if context_factory is None:
+            from twisted.internet import ssl
+            self.context_factory = ssl.ClientContextFactory()
+        else:
+            self.context_factory = context_factory
+
+    @defer.inlineCallbacks
+    def raw_request(self, method, uri, headers, postdata):
+        """Make a raw http request."""
+        # delay import, otherwise a default reactor gets installed
+        from twisted.web import client, error
+
+        parsed_url = urlparse.urlparse(uri)
+
+        # pylint: disable=E1101
+        https = parsed_url.scheme == "https"
+        host = parsed_url.netloc.split(":")[0]
+        # pylint: enable=E1101
+        if parsed_url.port is None:
+            port = 443 if https else 80
+        else:
+            port = parsed_url.port
+
+        factory = client.HTTPClientFactory(uri, method=method,
+                                           postdata=postdata,
+                                           headers=headers,
+                                           followRedirect=False)
+        # pylint: disable=E1103
+        if https:
+            self.connector.connectSSL(host, port, factory,
+                                      self.context_factory)
+        else:
+            self.connector.connectTCP(host, port, factory)
+        # pylint: enable=E1103
+
+        try:
+            content = yield factory.deferred
+            response = RawResponse(factory.response_headers, content)
+        except error.Error as e:
+            response = RawResponse(factory.response_headers, e.response,
+                                   int(e.status), e.message)
+        defer.returnValue(response)
 
     @defer.inlineCallbacks
     def request(self, iri, method="GET", extra_headers=None,
                 oauth_credentials=None, post_content=None):
         """Get the page, or fail trying."""
+        # delay import, otherwise a default reactor gets installed
+        from twisted.web import http
+
         uri = self.iri_to_uri(iri)
 
         if extra_headers:
@@ -107,40 +121,25 @@
             headers["Authorization"] = "Basic " + auth
 
         try:
-            request_headers = http_headers.Headers()
-            for key, value in headers.items():
-                request_headers.addRawHeader(key, value)
-            agent = client.Agent(reactor)
-            if post_content:
-                body_producer = StringProducer(post_content)
-            else:
-                body_producer = None
-            agent_response = yield agent.request(method, uri,
-                                                 headers=request_headers,
-                                                 bodyProducer=body_producer)
-            raw_headers = agent_response.headers.getAllRawHeaders()
-            response_headers = HeaderDict(raw_headers)
+            raw_response = yield self.raw_request(method, uri,
+                                                  headers=headers,
+                                                  postdata=post_content)
+            response_headers = HeaderDict(raw_response.headers)
             if method.lower() != "head":
-                response_content = yield self.get_agent_content(agent_response)
+                response_content = raw_response.content
             else:
                 response_content = ""
-            if agent_response.code == http.OK:
+            if raw_response.code == http.OK:
                 defer.returnValue(Response(response_content, response_headers))
-            if agent_response.code == http.UNAUTHORIZED:
-                raise UnauthorizedError(agent_response.phrase,
+            if raw_response.code == http.UNAUTHORIZED:
+                raise UnauthorizedError(raw_response.phrase,
                                         response_content)
-            raise WebClientError(agent_response.phrase, response_content)
+            raise WebClientError(raw_response.phrase, response_content)
         except WebClientError:
             raise
         except Exception as e:
             raise WebClientError(e.message, e)
 
-    def get_agent_content(self, agent_response):
-        """Get the contents of an agent response."""
-        string_protocol = StringProtocol()
-        agent_response.deliverBody(string_protocol)
-        return string_protocol.deferred
-
     def force_use_proxy(self, settings):
         """Setup this webclient to use the given proxy settings."""
-        # No proxy support in twisted.web.client
+        # No direct proxy support in twisted.web.client