launchpad-reviewers team mailing list archive

[Colin Watson <mp+426610@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:inline-zope.app.wsgi into launchpad:master with ~cjwatson/launchpad:inline-zope.session as a prerequisite.

Commit message:
Inline relevant parts of zope.app.wsgi

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/426610

This removes a number of packages from the virtualenv:

  ZConfig
  ZODB
  zc.lockfile
  zdaemon
  zodbpickle
  zope.app.appsetup
  zope.app.wsgi
  zope.minmax
  zope.session
  zope.site

This saves perhaps 5% off startup time (measured by median of five runs of `bin/py -c ''`), 10% off the time taken to execute ZCML (measured by median of five runs of `bin/harness </dev/null`), and paves the way for some further improvements by generally trimming cruft from our dependency tree.

I considered instead pruning the tree by making `ZODB` an optional dependency of `zope.app.appsetup`, or something along those lines.  However, like much of `zope.app`, `zope.app.appsetup` and `zope.app.wsgi` are fundamentally built around the assumption of running the full Zope application server.  Launchpad uses the Zope Component Architecture and many Zope Toolkit components (which are intended for use in various different application server environments), but it isn't built around the Zope application server, and we've had to work around some resulting impedance mismatches for a long time.  Architecturally, I think it would be better to let `zope.app` do its own thing as appropriate for that application server environment, and remove the relatively small number of remaining uses of it from Launchpad.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:inline-zope.app.wsgi into launchpad:master.

diff --git a/lib/lp/services/webapp/wsgi.py b/lib/lp/services/webapp/wsgi.py
index 7597602..b79fc94 100644
--- a/lib/lp/services/webapp/wsgi.py
+++ b/lib/lp/services/webapp/wsgi.py
@@ -1,5 +1,17 @@
 # Copyright 2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+#
+# Portions from zope.app.wsgi, which is:
+#
+# Copyright (c) 2004 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
 
 """Main Launchpad WSGI application."""
 
@@ -9,13 +21,15 @@ __all__ = [
 
 import logging
 
-from zope.app.wsgi import WSGIPublisherApplication
+from zope.app.publication.httpfactory import HTTPPublicationRequestFactory
 from zope.component.hooks import setHooks
 from zope.configuration import xmlconfig
 from zope.configuration.config import ConfigurationMachine
 from zope.event import notify
 from zope.interface import implementer
 from zope.processlifetime import DatabaseOpened
+from zope.publisher.interfaces.logginginfo import ILoggingInfo
+from zope.publisher.publish import publish
 from zope.security.interfaces import IParticipation
 from zope.security.management import (
     endInteraction,
@@ -33,6 +47,73 @@ class SystemConfigurationParticipation:
     interaction = None
 
 
+# Based on zope.app.wsgi.WSGIPublisherApplication, but with fewer
+# dependencies.
+class WSGIPublisherApplication:
+    """A WSGI application implementation for the Zope publisher.
+
+    Instances of this class can be used as a WSGI application object.
+
+    The class relies on a properly initialized request factory.
+    """
+
+    def __init__(self, factory=HTTPPublicationRequestFactory,
+                 handle_errors=True):
+        self.requestFactory = None
+        self.handleErrors = handle_errors
+        # HTTPPublicationRequestFactory requires a "db" object, mainly for
+        # ZODB integration.  This isn't useful in Launchpad, so just pass a
+        # meaningless object.
+        self.requestFactory = factory(object())
+
+    def __call__(self, environ, start_response):
+        """Called by a WSGI server.
+
+        The ``environ`` parameter is a dictionary object, containing CGI-style
+        environment variables. This object must be a builtin Python dictionary
+        (not a subclass, UserDict or other dictionary emulation), and the
+        application is allowed to modify the dictionary in any way it
+        desires. The dictionary must also include certain WSGI-required
+        variables (described in a later section), and may also include
+        server-specific extension variables, named according to a convention
+        that will be described below.
+
+        The ``start_response`` parameter is a callable accepting two required
+        positional arguments, and one optional argument. For the sake of
+        illustration, we have named these arguments ``status``,
+        ``response_headers``, and ``exc_info``, but they are not required to
+        have these names, and the application must invoke the
+        ``start_response`` callable using positional arguments
+        (e.g. ``start_response(status, response_headers)``).
+        """
+        request = self.requestFactory(environ['wsgi.input'], environ)
+
+        # Let's support post-mortem debugging
+        handle_errors = environ.get('wsgi.handleErrors', self.handleErrors)
+
+        request = publish(request, handle_errors=handle_errors)
+        response = request.response
+        # Get logging info from principal for log use
+        logging_info = ILoggingInfo(request.principal, None)
+        if logging_info is None:
+            message = b'-'
+        else:
+            message = logging_info.getLogMessage()
+
+        # Convert message bytes to native string
+        message = message.decode('latin1')
+
+        environ['wsgi.logging_info'] = message
+        if 'REMOTE_USER' not in environ:
+            environ['REMOTE_USER'] = message
+
+        # Start the WSGI server response
+        start_response(response.getStatusString(), response.getHeaders())
+
+        # Return the result body iterable.
+        return response.consumeBodyIter()
+
+
 def get_wsgi_application():
     # Loosely based on zope.app.appsetup.appsetup.
     features = []
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index abdc224..88a4e1a 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -85,7 +85,6 @@ import transaction
 from webob.request import environ_from_url as orig_environ_from_url
 import wsgi_intercept
 from wsgi_intercept import httplib2_intercept
-from zope.app.wsgi import WSGIPublisherApplication
 from zope.component import (
     getUtility,
     globalregistry,
@@ -146,6 +145,7 @@ from lp.services.webapp.servers import (
     register_launchpad_request_publication_factories,
     )
 import lp.services.webapp.session
+from lp.services.webapp.wsgi import WSGIPublisherApplication
 from lp.testing import (
     ANONYMOUS,
     login,
@@ -1024,11 +1024,6 @@ class _FunctionalBrowserLayer(zope.testbrowser.wsgi.Layer, ZCMLFileLayer):
     from the one zope.testrunner expects.
     """
 
-    # A meaningless object passed to publication classes that just require
-    # something other than None.  In Zope this would be a ZODB connection,
-    # but we don't use ZODB in Launchpad.
-    fake_db = object()
-
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.middlewares = [
@@ -1073,7 +1068,7 @@ class _FunctionalBrowserLayer(zope.testbrowser.wsgi.Layer, ZCMLFileLayer):
 
     def make_wsgi_app(self):
         """See `zope.testbrowser.wsgi.Layer`."""
-        return self.setupMiddleware(WSGIPublisherApplication(self.fake_db))
+        return self.setupMiddleware(WSGIPublisherApplication())
 
 
 class FunctionalLayer(BaseLayer):
diff --git a/lib/lp/testing/pages.py b/lib/lp/testing/pages.py
index 6e2d360..8c4a144 100644
--- a/lib/lp/testing/pages.py
+++ b/lib/lp/testing/pages.py
@@ -1,5 +1,18 @@
 # Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+#
+# Portions from zope.app.wsgi.testlayer, which is:
+#
+# Copyright (c) 2010 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+
 
 """Testing infrastructure for page tests."""
 
@@ -29,10 +42,6 @@ import six
 from soupsieve import escape as css_escape
 import transaction
 from webtest import TestRequest
-from zope.app.wsgi.testlayer import (
-    FakeResponse,
-    NotInBrowserLayer,
-    )
 from zope.component import getUtility
 from zope.security.management import setSecurityPolicy
 from zope.security.proxy import removeSecurityProxy
@@ -95,6 +104,72 @@ SAMPLEDATA_ACCESS_SECRETS = {
     }
 
 
+class NotInBrowserLayer(Exception):
+    """The current test is not running in zope.testbrowser.wsgi.Layer."""
+
+
+# Based on zope.app.wsgi.testlayer.FakeResponse, but with fewer dependencies.
+class FakeResponse:
+    """This behaves like a Response object returned by HTTPCaller of
+    zope.app.testing.functional.
+    """
+
+    def __init__(self, response, request=None):
+        self.response = response
+        self.request = request
+
+    @property
+    def server_protocol(self):
+        protocol = None
+        if self.request is not None:
+            protocol = self.request.environ.get('SERVER_PROTOCOL')
+        if protocol is None:
+            protocol = b'HTTP/1.0'
+        if not isinstance(protocol, bytes):
+            protocol = protocol.encode('latin1')
+        return protocol
+
+    def getStatus(self):
+        return self.response.status_int
+
+    def getStatusString(self):
+        return self.response.status
+
+    def getHeader(self, name, default=None):
+        return self.response.headers.get(name, default)
+
+    def getHeaders(self):
+        return sorted(self.response.headerlist)
+
+    def getBody(self):
+        return self.response.body
+
+    def getOutput(self):
+        status = self.response.status
+        status = (
+            status.encode('latin1') if not isinstance(status, bytes)
+            else status)
+        parts = [self.server_protocol + b' ' + status]
+
+        headers = [(k.encode('latin1') if not isinstance(k, bytes) else k,
+                    v.encode('latin1') if not isinstance(v, bytes) else v)
+                   for k, v in self.getHeaders()]
+
+        parts += [k + b': ' + v for k, v in headers]
+
+        body = self.response.body
+        if body:
+            if not isinstance(body, bytes):
+                body = body.encode('utf-8')
+            parts += [b'', body]
+        return b'\n'.join(parts)
+
+    __bytes__ = getOutput
+
+    def __str__(self):
+        return self.getOutput().decode('latin-1')
+
+
 def http(string, handle_errors=True):
     """Make a test HTTP request.
 
diff --git a/lib/lp_sitecustomize.py b/lib/lp_sitecustomize.py
index 1f75c6f..0efa18b 100644
--- a/lib/lp_sitecustomize.py
+++ b/lib/lp_sitecustomize.py
@@ -27,21 +27,7 @@ from lp.services.openid.fetcher import set_default_openid_fetcher
 
 
 def add_custom_loglevels():
-    """Add out custom log levels to the Python logging package."""
-
-    # This import installs custom ZODB loglevels, which we can then
-    # override. BLATHER is between INFO and DEBUG, so we can leave it.
-    # TRACE conflicts with DEBUG6, and since we are not using ZEO, we
-    # just overwrite the level string by calling addLevelName.
-    from ZODB.loglevels import (
-        BLATHER,
-        TRACE,
-        )
-
-    # Confirm our above assumptions, and silence lint at the same time.
-    assert BLATHER == 15
-    assert TRACE == loglevels.DEBUG6
-
+    """Add our custom log levels to the Python logging package."""
     logging.addLevelName(loglevels.DEBUG2, 'DEBUG2')
     logging.addLevelName(loglevels.DEBUG3, 'DEBUG3')
     logging.addLevelName(loglevels.DEBUG4, 'DEBUG4')
diff --git a/requirements/launchpad.txt b/requirements/launchpad.txt
index 984e30f..65cc07b 100644
--- a/requirements/launchpad.txt
+++ b/requirements/launchpad.txt
@@ -192,10 +192,7 @@ zipp==1.2.0
 zope.app.http==4.0.1
 zope.app.publication==4.3.1
 zope.app.publisher==4.2.0
-zope.app.wsgi==4.3.0
 zope.publisher==6.0.2+lp1
-# lp:~launchpad-committers/zope.session:launchpad
-zope.session==4.3.0+lp1
 zope.testbrowser==5.5.1
 # lp:~launchpad-committers/zope.testrunner:launchpad
 zope.testrunner==5.3.0+lp1
diff --git a/setup.cfg b/setup.cfg
index b4e029e..83994c0 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -120,7 +120,6 @@ install_requires =
     zope.app.http
     zope.app.publication
     zope.app.publisher
-    zope.app.wsgi[testlayer]
     zope.authentication
     zope.browser
     zope.browsermenu
diff --git a/utilities/list-pages b/utilities/list-pages
index 3dd9314..36a8bfc 100755
--- a/utilities/list-pages
+++ b/utilities/list-pages
@@ -47,7 +47,6 @@ import _pythonpath  # noqa: F401
 from inspect import getmro
 import os
 
-from zope.app.wsgi.testlayer import BrowserLayer
 from zope.browserpage.simpleviewclass import simple
 from zope.component import (
     adapter,
@@ -63,6 +62,7 @@ from lp.services.config import config
 from lp.services.scripts import execute_zcml_for_scripts
 from lp.services.webapp.interfaces import ICanonicalUrlData
 from lp.services.webapp.publisher import canonical_url
+from lp.testing.layers import _FunctionalBrowserLayer
 import zcml
 
 
@@ -76,7 +76,7 @@ def load_zcml(zopeless=False):
     if zopeless:
         execute_zcml_for_scripts()
     else:
-        BrowserLayer(zcml, zcml_file='webapp.zcml').setUp()
+        _FunctionalBrowserLayer(zcml, zcml_file='webapp.zcml').setUp()
 
 
 def is_page_adapter(a):
diff --git a/zcml/zopeapp.zcml b/zcml/zopeapp.zcml
index 9ae82dd..16cc755 100644
--- a/zcml/zopeapp.zcml
+++ b/zcml/zopeapp.zcml
@@ -81,9 +81,6 @@
       provides="zope.i18n.interfaces.IModifiableUserPreferredLanguages"
       />
 
-
-  <include package="zope.app.wsgi" />
-
   <!-- Default XMLRPC pre-marshalling. -->
   <include package="zope.publisher" />