ERP5Type/tests: review requests in test

The general idea of this patch is that now that we are using
zope.globalrequest, we no longer need to patch get_request, we can
simply call zope.globalrequest.setRequest with the request from the
test and restore the "real" request afterwards.

To achieve this, we reuse Testing.ZopeTestCase.connections.registry,
which already has the logic of cleaning up resources in the right place
and use a "Request" resource that calls setRequest(test_request) and
setRequest(real_request) when closed, so that:
 - test runs with an independant request
 - this test request is closed at the end
 - the real request is restored at the end

This also fixes a bug with self.publish when runnning
ERP5TypeLiveTestCase from portal_components of a running instance,
after a call to self.publish the current request was lost.

The testing for this revealed that ERP5TypeLiveTestCase.publish way
of dealing with zope.security interaction was not always correct: when
running a live test inside runUnitTest (like we do here in
testDynamicClassGeneration), there is no security interaction. This
was reviewed to use the high level API instead of changing directly the
internal storage.

bt5/erp5_paypal_secure_payment/TestTemplateItem/portal_components/test.erp5.testERP5PaypalSecurePayment.py

@@ -26,7 +26,8 @@
 ##############################################################################
 
 import random
-from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase, get_request
+from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
+from zope.globalrequest import getRequest
 
 
 def getMessageList(o):
@@ -102,5 +103,5 @@ business=business@sample.com""")
     self.tic()
     script_absolute_url = script.absolute_url()
     self.service.edit(link_url_string=script_absolute_url)
-    response = self.service.reportPaymentStatus(get_request())
+    response = self.service.reportPaymentStatus(getRequest())
     self.assertTrue(response)

product/ERP5Type/tests/ERP5TypeLiveTestCase.py

@@ -33,15 +33,19 @@ import imp
 import re
 
 from zope.site.hooks import setSite
+from zope.globalrequest import getRequest
 from Acquisition import aq_base
 from Testing import ZopeTestCase
-from Testing.ZopeTestCase import PortalTestCase, user_name
+from Testing.ZopeTestCase import connections, PortalTestCase, user_name
 from Products.CMFCore.utils import getToolByName
 from Products.ERP5Type.tests.ProcessingNodeTestCase import ProcessingNodeTestCase
 from Products.ERP5Type.tests.SecurityTestCase import SecurityTestCase
-from Products.ERP5Type.Globals import get_request
-from Products.ERP5Type.tests.ERP5TypeTestCase import \
-  ERP5TypeTestCaseMixin, ERP5TypeTestCase, ERP5ReportTestCase
+from Products.ERP5Type.tests.ERP5TypeTestCase import (
+  ERP5TypeTestCase,
+  ERP5TypeTestCaseMixin,
+  ERP5TypeTestCaseRequestConnection,
+  ERP5ReportTestCase,
+)
 from glob import glob
 import transaction
 
@@ -101,22 +105,12 @@ class ERP5TypeLiveTestCase(ERP5TypeTestCaseMixin):
         makerequest(aq_base(site.aq_parent), environ=environ),
         site.getId())
 
-      # Make the various get_request patches return this request.
-      # TODO: check this is still needed
-      # This is for ERP5TypeTestCase patch
-      from Testing.ZopeTestCase.connections import registry
-      if registry:
-        registry._conns[-1] = portal
-
-      # This is for Localizer patch
-      from zope.globalrequest import setRequest
       request = portal.REQUEST
-      setRequest(request)
-
-      # Make live tests run under the same server URL than the host instance.
       if _request_server_url:
         request['SERVER_URL'] = _request_server_url
         request._resetURLS()
+      self._request_connection = ERP5TypeTestCaseRequestConnection(request)
+      connections.register(self._request_connection)
 
       portal.setupCurrentSkin(request)
       setSite(portal)
@@ -144,7 +138,11 @@ class ERP5TypeLiveTestCase(ERP5TypeTestCaseMixin):
         self.portal.portal_activities.subscribe()
         self.commit()
       if self.portal is not None:
+        # With a live test, we don't close all connections, because the ZODB connection
+        # was not opened by us. 
         self.portal.REQUEST.close()
+        self.portal = None
+        connections.close(self._request_connection)
 
     def _setup(self):
         '''Change some site properties in order to be ready for live test
@@ -177,13 +175,12 @@ class ERP5TypeLiveTestCase(ERP5TypeTestCaseMixin):
       pass
 
     def publish(self, *args, **kw):
-      from zope.security.management import thread_local
-      interaction = thread_local.interaction
+      from zope.security.management import endInteraction, restoreInteraction
+      endInteraction()
       try:
-        del thread_local.interaction
         return super(ERP5TypeLiveTestCase, self).publish(*args, **kw)
       finally:
-        thread_local.interaction = interaction
+        restoreInteraction()
 
 from Products.ERP5Type.dynamic.component_package import ComponentDynamicPackage
 from Products.ERP5Type.tests.runUnitTest import ERP5TypeTestLoader

product/ERP5Type/tests/ERP5TypeTestCase.py

@@ -32,34 +32,16 @@ from DateTime import DateTime
 import mock
 import Products.ZMySQLDA.DA
 from Products.ZMySQLDA.DA import Connection as ZMySQLDA_Connection
-
-# XXX make sure that get_request works.
-from new import function
+from zope.globalrequest import clearRequest
 from zope.globalrequest import getRequest
+from zope.globalrequest import setRequest
 import six
-original_get_request = function(getRequest.__code__, getRequest.__globals__)
-
-from Testing.ZopeTestCase.connections import registry
-def get_context():
-  if registry:
-    return registry._conns[-1]
-
-def get_request():
-  request = original_get_request()
-  if request is not None:
-    return request
-  current_app = get_context()
-  if current_app is not None:
-    return current_app.REQUEST
-
-sys.modules[getRequest.__module__].get_request = get_request
-getRequest.__code__ = (lambda: get_request()).__code__
 
 from zope.site.hooks import setSite
 
 from Testing import ZopeTestCase
 from Testing.makerequest import makerequest
-from Testing.ZopeTestCase import PortalTestCase, user_name, ZopeLite, functional
+from Testing.ZopeTestCase import connections, PortalTestCase, user_name, ZopeLite, functional
 from Products.ERP5Type.Core.Workflow import ValidationFailed
 from Products.PythonScripts.PythonScript import PythonScript
 from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
@@ -218,6 +200,21 @@ def _parse_args(self, *args, **kw):
 _parse_args._original = DateTime._original_parse_args
 DateTime._parse_args = _parse_args
 
+
+class ERP5TypeTestCaseRequestConnection(object):
+  """A "Connection" to ensure the test runs with an independent request
+  and that the original request is properly restored at the end of test.
+
+  This is used with Testing.ZopeTestCase.connection.registry
+  """
+  def __init__(self, request):
+    self._current_request = getRequest()
+    setRequest(request)
+
+  def close(self):
+    setRequest(self._current_request)
+
+
 class ERP5TypeTestCaseMixin(ProcessingNodeTestCase, PortalTestCase, functional.Functional):
     """Mixin class for ERP5 based tests.
     """
@@ -773,6 +770,7 @@ class ERP5TypeTestCaseMixin(ProcessingNodeTestCase, PortalTestCase, functional.F
       else:
          user_context = nullcontext()
 
+      request = getRequest()
       try:
         with user_context:
           return super(ERP5TypeTestCaseMixin, self).publish(
@@ -788,6 +786,7 @@ class ERP5TypeTestCaseMixin(ProcessingNodeTestCase, PortalTestCase, functional.F
         # Make sure that the skin cache does not have objects that were
         # loaded with the connection used by the requested url.
         self.changeSkin(self.portal.getCurrentSkinName())
+        setRequest(request)
 
     def getConsistencyMessageList(self, obj):
         return sorted([ str(message.getMessage())
@@ -960,6 +959,8 @@ class ERP5TypeCommandLineTestCase(ERP5TypeTestCaseMixin):
       http://nohost, but we prefer to create directly a connection to the
       app wrapped in a request to our web server.
       '''
+      if hasattr(self, 'app'):
+        return self.app
       app = ZopeLite.app()
       environ = {}
       if self._server_address:
@@ -968,8 +969,11 @@ class ERP5TypeCommandLineTestCase(ERP5TypeTestCaseMixin):
         environ['SERVER_PORT'] = str(port)
 
       app = makerequest(app, environ=environ)
-      registry.register(app)
-      app.REQUEST['HTTP_ACCEPT_CHARSET'] = 'utf-8'
+      connections.register(app)
+
+      request = app.REQUEST
+      request['HTTP_ACCEPT_CHARSET'] = 'utf-8'
+      connections.register(ERP5TypeTestCaseRequestConnection(request))
       return app
 
     def __onConnect(self, connector):

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -3183,6 +3183,13 @@ class Test(ERP5TypeTestCase):
     request.set('foo', 'bar')
     self.assertEqual(request.get('foo'), 'bar')
 
+    from zope.globalrequest import getRequest
+    self.assertEqual(getRequest().get('foo'), 'bar')
+
+    self.publish(self.portal.getId())
+    self.assertEqual(self.portal.REQUEST.get('foo'), 'bar')
+    self.assertEqual(getRequest().get('foo'), 'bar')
+
   def afterClear(self):
     super(Test, self).afterClear()