This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8500 in repository https://gitbox.apache.org/repos/asf/allura.git
commit a55b1db165c23eb63b541c2e04f925e3bece499f Author: Dave Brondsema <[email protected]> AuthorDate: Thu Feb 23 13:42:15 2023 -0500 [#8500] turbogears upgrade: ensure TG registry globals are cleaned up --- Allura/allura/config/middleware.py | 8 +++++--- Allura/allura/tests/functional/test_root.py | 20 ++++++++++++++++++++ Allura/allura/tests/functional/test_site_admin.py | 1 + AlluraTest/alluratest/validation.py | 13 +++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Allura/allura/config/middleware.py b/Allura/allura/config/middleware.py index e10a14613..99b88645f 100644 --- a/Allura/allura/config/middleware.py +++ b/Allura/allura/config/middleware.py @@ -180,9 +180,11 @@ def _make_core_app(root, global_conf: dict, **app_conf): app = MingTaskSessionSetupMiddleware(app) app = MingMiddleware(app) # Set up the registry for stacked object proxies (SOPs). - # streaming=true ensures they won't be cleaned up till - # the WSGI application's iterator is exhausted - app = RegistryManager(app, streaming=True) + app = RegistryManager(app, + # streaming=True causes cleanup problems when StatusCodeRedirect does an extra request + streaming=False, + preserve_exceptions=asbool(config['debug']), # allow inspecting them when debugging errors + ) # "task" wsgi would get a 2nd request to /error/document if we used this middleware if config.get('override_root') not in ('task', 'basetest_project_root'): diff --git a/Allura/allura/tests/functional/test_root.py b/Allura/allura/tests/functional/test_root.py index 7ee73c600..a69dbf550 100644 --- a/Allura/allura/tests/functional/test_root.py +++ b/Allura/allura/tests/functional/test_root.py @@ -30,6 +30,7 @@ import os import re from unittest import skipIf +import pytest from tg import tmpl_context as c from allura.tests.decorators import patch_middleware_config @@ -45,6 +46,20 @@ from allura.lib import helpers as h from alluratest.controller import setup_trove_categories +def assert_globals_are_reset(): + # in normal tests the globals 'stacks' are cleared + assert tg.config._object_stack() == [] + + # and similarly, 'c' and 'request' TG globals are not even accessible at all + with pytest.raises(AttributeError): + c._object_stack() + assert 'allura.websetup.schema.EmptyClass' in repr(c) # can't use isinstance/type because it's a proxy + + with pytest.raises(AttributeError) as exc_info: + tg.request._object_stack() + assert str(exc_info.value) == 'request', exc_info + + class TestRootController(TestController): def setup_method(self, method): @@ -62,6 +77,8 @@ class TestRootController(TestController): response = self.app.get('/') assert response.location == 'http://localhost/dashboard' + assert_globals_are_reset() # should be ok, but just to make sure what "normal" is (might change with upgrades) + def test_neighborhood(self): response = self.app.get('/neighborhood') assert response.html.find('h2', {'class': 'dark title'}).find('span').contents[ @@ -259,6 +276,9 @@ class TestErrorMiddleware(TestController): r.mustcontain('404 Error has Occurred') # in error.html r.mustcontain('This project is powered by') # in master template + # Ensure 404 handling with StatusCodeRedirect doesn't leave anything behind + assert_globals_are_reset() + def test_error_nodebug(self): with mock.patch.object(M.Project, 'ordered_mounts') as function_nbhd_controller_calls: function_nbhd_controller_calls.side_effect = Exception('forced an error!') diff --git a/Allura/allura/tests/functional/test_site_admin.py b/Allura/allura/tests/functional/test_site_admin.py index f47245fab..f54228795 100644 --- a/Allura/allura/tests/functional/test_site_admin.py +++ b/Allura/allura/tests/functional/test_site_admin.py @@ -18,6 +18,7 @@ import json import datetime as dt import bson +import pytest from mock import patch, MagicMock from ming.odm import ThreadLocalODMSession diff --git a/AlluraTest/alluratest/validation.py b/AlluraTest/alluratest/validation.py index 37cc972dc..3102f3e6a 100644 --- a/AlluraTest/alluratest/validation.py +++ b/AlluraTest/alluratest/validation.py @@ -362,3 +362,16 @@ class ValidatingTestApp(PostParamCheckingTestApp): if not self.validate_skip and not val_params['validate_skip']: self._validate(resp, 'delete', val_params) return resp + + def do_request(self, *args, **kwargs): + # middleware should do this already, but be sure that no global c/config/request etc remains between tests + resp = super().do_request(*args, **kwargs) + tgGlobalsRegistry = resp.request.environ['paste.registry'] + try: + tgGlobalsRegistry.cleanup() + except IndexError: + # already cleaned up + pass + except Exception: + log.warning('Error cleaning up TG Registry', exc_info=True) + return resp
