Merge lp:~brian.curtin/ubuntuone-dev-tools/py3-getPage-replacement into lp:ubuntuone-dev-tools

[Brian Curtin]

- Move to a standard library based HTTPWebSever and HTTPSWebServer implementation due to lack of twisted support for those implementations on Python 3.

As twisted.web.client.getPage is not currently available for Python 3 (nor is it in scope for the current porting efforts), we need to replace it with a standard equivalent, which is available via urllib.

While moving towards that urllib implementation, it was pointed out that the server would have to be moved away from twisted as well, so HTTPWebServer and HTTPSWebServer were moved from twisted implementations to similar standard library equivalents using socketserver.ThreadedMixIn, socketserver.TCPServer, and the ssl module.

With the exception of HTTPSWebSever, everything here works on Python 2 and 3. HTTPS is currently untested within devtools, but *is* tested within all of the projects that use it. In the interest of progressing on SSO porting, I haven't added any HTTPS specific tests but intend to bring some back from SSO and the other projects as I encounter them (and fix them).

[Alejandro J. Cura (alecu)]

When running the sso tests with this branch I get a lot of errors similar to this:

test_ssl_fail_dialog_user_accepts_via_proxy ... Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/alecu/canonical/ubuntu-sso-client/trunk/ubuntu_sso/utils/webclient/tests/test_webclient.py", line 969, in setUp
    self.ws = HTTPSMockWebServer(self.ssl_settings)
  File "/home/alecu/canonical/ubuntu-sso-client/trunk/ubuntu_sso/utils/webclient/tests/test_webclient.py", line 239, in __init__
    super(HTTPSMockWebServer, self).__init__(root, ssl_settings)
exceptions.TypeError: must be type, not classobj

review: Needs Fixing

[lp:~brian.curtin/ubuntuone-dev-tools/py3-getPage-replacement]

102. By Brian Curtin on 2012-09-07

Shouldn't inherit from the mixin/TCPServer - instantiate them in the BaseWebSever and then use super all around. Should fix alecu's comment.

[Roberto Alsina (ralsina)]

Looks good to me.

review: Approve

[Brian Curtin (brian.curtin)]

Don't mark the branch as approved yet - it breaks SSO horribly so I'm working on making the changes for it to work with this branch.

Unmerged revisions

102. By Brian Curtin on 2012-09-07

Shouldn't inherit from the mixin/TCPServer - instantiate them in the BaseWebSever and then use super all around. Should fix alecu's comment.

101. By Brian Curtin on 2012-09-07

Remove commented out lines

100. By Brian Curtin on 2012-09-07

Restructure BaseWebSever to ease HTTPSWebServer implementation. Remove all Twisted code and imports

99. By Brian Curtin on 2012-09-06

Suppress HTTPServer output by overriding the log_message method. This removes the various output that gets mixed in with the test runner output.

98. By Brian Curtin on 2012-09-06

Merge trunk

97. By Brian Curtin on 2012-09-04

Various pylint fixes

96. By Brian Curtin on 2012-09-04

Initial implementation of stdlib HTTPServer

=== modified file 'ubuntuone/devtools/testing/tests/test_txwebserver.py'
--- ubuntuone/devtools/testing/tests/test_txwebserver.py	2012-08-28 20:28:43 +0000
+++ ubuntuone/devtools/testing/tests/test_txwebserver.py	2012-09-07 18:28:19 +0000
@@ -30,9 +30,17 @@
 
 from __future__ import unicode_literals
 
+# pylint: disable=F0401,E0611
+try:
+    from http.server import SimpleHTTPRequestHandler
+    from urllib.request import urlopen
+except ImportError:
+    from SimpleHTTPServer import SimpleHTTPRequestHandler
+    from urllib import urlopen
+# pylint: disable=F0401,E0611
+
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
-from twisted.web import client, resource
 
 from ubuntuone.devtools.testing.txwebserver import HTTPWebServer
 
@@ -50,15 +58,31 @@
 # pylint: disable=W0232
 
 
-class SimpleResource(resource.Resource):
-    """A simple web resource."""
+# pylint: disable=C0103
+def getPage(url):
+    """Simple standard library replacement for twisted.web.client.getPage."""
+    data = urlopen(url)
+    text = data.read()
+    return text
+# pylint: enable=C0103
+
+
+class RequestHandler(SimpleHTTPRequestHandler):
+    """A HTTP request handler."""
 
     # pylint: disable=C0103
-    def render_GET(self, request):
-        """Make a bit of html out of these resource's content."""
-        return SAMPLE_RESOURCE
+    def do_GET(self):
+        """Simply return SAMPLE_RESOURCE for all GET requests."""
+        self.send_response(200)
+        self.send_header("Content-type", "text/html")
+        self.end_headers()
+        self.wfile.write(SAMPLE_RESOURCE)
     # pylint: enable=C0103
 
+    def log_message(self, *args):
+        """Override the default log message so we don't output anything."""
+        return
+
 
 class WebServerTestCase(TestCase):
     """Test the web server that will allow to have connections."""
@@ -68,34 +92,24 @@
         """Set the different tests."""
         yield super(WebServerTestCase, self).setUp()
         # create the root to be used by the webserver
-        root = resource.Resource()
-        root.putChild(SIMPLERESOURCE, SimpleResource())
-        root.putChild(OTHER_SIMPLERESOURCE, SimpleResource())
-        root.putChild(THROWERROR, resource.NoResource())
-
-        unauthorized_resource = resource.ErrorPage(resource.http.UNAUTHORIZED,
-                                                "Unauthorized", "Unauthorized")
-        root.putChild(UNAUTHORIZED, unauthorized_resource)
-        self.server = HTTPWebServer(root)
+        self.server = HTTPWebServer(RequestHandler)
         self.server.start()
         self.uri = "http://127.0.0.1:{port}/".format(
                         port=self.server.get_port()).encode("utf8")
         self.addCleanup(self.server.stop)
 
-    @defer.inlineCallbacks
     def test_single_request(self):
         """Test performing a single request to get the data."""
         url = self.uri + SIMPLERESOURCE
-        result = yield client.getPage(url)
+        result = getPage(url.decode("utf8"))
         self.assertEqual(SAMPLE_RESOURCE, result)
 
-    @defer.inlineCallbacks
     def test_multiple_requests(self):
         """Test performing multiple requests."""
         simple_url = self.uri + SIMPLERESOURCE
         other_simple_url = self.uri + OTHER_SIMPLERESOURCE
-        simple_result = yield client.getPage(simple_url)
-        other_result = yield client.getPage(other_simple_url)
+        simple_result = getPage(simple_url.decode("utf8"))
+        other_result = getPage(other_simple_url.decode("utf8"))
         self.assertEqual(SAMPLE_RESOURCE, simple_result)
         self.assertEqual(SAMPLE_RESOURCE, other_result)
 
@@ -112,7 +126,7 @@
     def test_get_port(self):
         """Test getting the port."""
         # pylint: disable=W0212
-        port = self.server.port.getHost().port
+        port = self.server.port
         # pylint: enable=W0212
         self.assertEqual(port, self.server.get_port())
 
@@ -124,51 +138,41 @@
     def setUp(self):
         """Set the diff tests."""
         yield super(MultipleWebServersTestCase, self).setUp()
-        self.root = resource.Resource()
-        self.root.putChild(SIMPLERESOURCE, SimpleResource())
-        self.root.putChild(OTHER_SIMPLERESOURCE, SimpleResource())
-        self.root.putChild(THROWERROR, resource.NoResource())
-
-        unauthorized_resource = resource.ErrorPage(resource.http.UNAUTHORIZED,
-                                                "Unauthorized", "Unauthorized")
-        self.root.putChild(UNAUTHORIZED, unauthorized_resource)
 
     def get_uri(self, server):
         """Return the uri for the server."""
         url = "http://127.0.0.1:{port}/"
         return url.format(port=server.get_port()).encode("utf8")
 
-    @defer.inlineCallbacks
     def test_single_request(self):
         """Test performing a single request to get the data."""
-        first_server = HTTPWebServer(self.root)
+        first_server = HTTPWebServer(RequestHandler)
         first_server.start()
         self.addCleanup(first_server.stop)
 
-        second_server = HTTPWebServer(self.root)
+        second_server = HTTPWebServer(RequestHandler)
         second_server.start()
         self.addCleanup(second_server.stop)
 
         for server in [first_server, second_server]:
             url = self.get_uri(server) + SIMPLERESOURCE
-            result = yield client.getPage(url)
+            result = getPage(url.decode("utf8"))
             self.assertEqual(SAMPLE_RESOURCE, result)
 
-    @defer.inlineCallbacks
     def test_multiple_requests(self):
         """Test performing multiple requests."""
-        first_server = HTTPWebServer(self.root)
+        first_server = HTTPWebServer(RequestHandler)
         first_server.start()
         self.addCleanup(first_server.stop)
 
-        second_server = HTTPWebServer(self.root)
+        second_server = HTTPWebServer(RequestHandler)
         second_server.start()
         self.addCleanup(second_server.stop)
 
         for server in [first_server, second_server]:
             simple_url = self.get_uri(server) + SIMPLERESOURCE
             other_simple_url = self.get_uri(server) + OTHER_SIMPLERESOURCE
-            simple_result = yield client.getPage(simple_url)
-            other_result = yield client.getPage(other_simple_url)
+            simple_result = getPage(simple_url.decode("utf8"))
+            other_result = getPage(other_simple_url.decode("utf8"))
             self.assertEqual(SAMPLE_RESOURCE, simple_result)
             self.assertEqual(SAMPLE_RESOURCE, other_result)

=== modified file 'ubuntuone/devtools/testing/txwebserver.py'
--- ubuntuone/devtools/testing/txwebserver.py	2012-08-21 21:36:20 +0000
+++ ubuntuone/devtools/testing/txwebserver.py	2012-09-07 18:28:19 +0000
@@ -30,94 +30,64 @@
 
 from __future__ import unicode_literals
 
-from twisted.internet import defer, reactor, ssl
-from twisted.protocols.policies import WrappingFactory
-from twisted.web import server
-
-from ubuntuone.devtools.testcases.txsocketserver import server_protocol_factory
-
-# no init method +  twisted common warnings
-# pylint: disable=W0232, C0103, E1101
+# pylint: disable=F0401
+try:
+    from socketserver import ThreadingMixIn, TCPServer
+except ImportError:
+    from SocketServer import ThreadingMixIn, TCPServer
+# pylint: enable=F0401
+
+import ssl
+import threading
+
+
+class ThreadedWebServer(ThreadingMixIn, TCPServer):
+    """Threaded web server."""
 
 
 class BaseWebServer(object):
-    """Webserver used to perform requests in tests."""
-
-    def __init__(self, root_resource, scheme):
-        """Create and start the instance.
-
-        The ssl_settings parameter contains a dictionary with the key and cert
-        that will be used to perform ssl connections. The root_resource
-        contains the resource with all its childre.
-        """
-        self.root = root_resource
+    """Web server"""
+
+    def __init__(self, handler, scheme):
+        self.server = ThreadedWebServer(("localhost", 0), handler)
+        self.ip, self.port = self.server.server_address
         self.scheme = scheme
-        self.port = None
-        # use an http.HTTPFactory that was modified to ensure that we have
-        # clean close connections
-        self.site = server.Site(self.root, timeout=None)
-        self.wrapper = WrappingFactory(self.site)
-        self.wrapper.testserver_on_connection_lost = defer.Deferred()
-        self.wrapper.protocol = server_protocol_factory(self.wrapper.protocol)
-        self.wrapper._disconnecting = False
 
-    def listen(self, site):
-        """Listen a port to allow the tests."""
-        raise NotImplementedError('Base abstract class.')
+        self.thread = threading.Thread(target=self.server.serve_forever)
+        self.thread.daemon = True
 
     def get_iri(self):
         """Build the iri for this mock server."""
         return "{scheme}://127.0.0.1:{port}/".format(scheme=self.scheme,
-                                                   port=self.get_port())
+                                                     port=self.get_port())
 
     def get_port(self):
         """Return the port where we are listening."""
-        return self.port.getHost().port
+        return self.port
 
     def start(self):
-        """Start the service."""
-        self.port = self.listen(self.wrapper)
+        """Start the web server in another thread."""
+        self.thread.start()
 
     def stop(self):
-        """Shut it down."""
-        if self.port:
-            self.wrapper._disconnecting = True
-            connected = self.wrapper.protocols.keys()
-            if connected:
-                for con in connected:
-                    con.transport.loseConnection()
-            else:
-                self.wrapper.testserver_on_connection_lost = \
-                                                            defer.succeed(None)
-            d = defer.maybeDeferred(self.port.stopListening)
-            return defer.gatherResults([d,
-                                  self.wrapper.testserver_on_connection_lost])
-        return defer.succeed(None)
+        """Shutdown the web server."""
+        self.server.shutdown()
 
 
 class HTTPWebServer(BaseWebServer):
-    """A Webserver that listens to http connections."""
-
-    def __init__(self, root_resource):
-        """Create  a new instance."""
-        super(HTTPWebServer, self).__init__(root_resource, 'http')
-
-    def listen(self, site):
-        """Listen a port to allow the tests."""
-        return reactor.listenTCP(0, site)
-
-
-class HTTPSWebServer(BaseWebServer):
-    """A WebServer that listens to https connections."""
-
-    def __init__(self, root_resource, ssl_settings=None):
-        """Create  a new instance."""
-        super(HTTPSWebServer, self).__init__(root_resource, 'https')
-        self.ssl_settings = ssl_settings
-
-    def listen(self, site):
-        """Listen a port to allow the tests."""
-        ssl_context = ssl.DefaultOpenSSLContextFactory(
-                self.ssl_settings['key'], self.ssl_settings['cert'])
-
-        return reactor.listenSSL(0, site, ssl_context)
+    """A standard library based HTTP server."""
+
+    def __init__(self, handler):
+        super(HTTPWebServer, self).__init__(handler, scheme="http")
+
+
+class HTTPSWebServer(HTTPWebServer):
+    """A standard library based HTTPS server."""
+
+    def __init__(self, handler, ssl_settings):
+        super(HTTPSWebServer, self).__init__(handler, scheme="https")
+
+        self.socket = ssl.wrap_socket(self.socket,
+                                      keyfile=ssl_settings["key"],
+                                      certfile=ssl_settings["key"],
+                                      server_side=True)