Merge pull request #2873 from mmerickel/defer-translation-dirs

defer config.add_translation_dirs

7 files changed, 166 insertions, 45 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index ac3989d35..ac329f574 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -140,6 +140,29 @@ Features
   ``--help`` output as well as enabling nicer documentation of their options.
   See https://github.com/Pylons/pyramid/pull/2864
 
+- Any deferred configuration action registered via ``config.action`` may now
+  depend on threadlocal state, such as asset overrides, being active when
+  the action is executed.
+  See https://github.com/Pylons/pyramid/pull/2873
+
+- Asset specifications for directories passed to
+  ``config.add_translation_dirs`` now support overriding the entire asset
+  specification, including the folder name. Previously only the package name
+  was supported and the folder would always need to have the same name.
+  See https://github.com/Pylons/pyramid/pull/2873
+
+- ``config.begin()`` will propagate the current threadlocal request through
+  as long as the registry is the same. For example:
+
+  .. code-block:: python
+
+     request = Request.blank(...)
+     config.begin(request)  # pushes a request
+     config.begin()         # propagates the previous request through unchanged
+     assert get_current_request() is request
+
+  See https://github.com/Pylons/pyramid/pull/2873
+
 Bug Fixes
 ---------
 
@@ -174,6 +197,11 @@ Bug Fixes
   style, even if a different plural function is defined in the relevant
   messages file. See https://github.com/Pylons/pyramid/pull/2859
 
+- The ``config.override_asset`` method now occurs during
+  ``pyramid.config.PHASE1_CONFIG`` such that it is ordered to execute before
+  any calls to ``config.add_translation_dirs``.
+  See https://github.com/Pylons/pyramid/pull/2873
+
 Deprecations
 ------------
 
diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst
index babfa0a98..4009ec1dc 100644
--- a/docs/narr/extconfig.rst
+++ b/docs/narr/extconfig.rst
@@ -260,6 +260,7 @@ Pre-defined Phases
 - :meth:`pyramid.config.Configurator.add_subscriber_predicate`
 - :meth:`pyramid.config.Configurator.add_view_predicate`
 - :meth:`pyramid.config.Configurator.add_view_deriver`
+- :meth:`pyramid.config.Configurator.override_asset`
 - :meth:`pyramid.config.Configurator.set_authorization_policy`
 - :meth:`pyramid.config.Configurator.set_default_csrf_options`
 - :meth:`pyramid.config.Configurator.set_default_permission`
diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py
index d4064dc78..304d3a85e 100644
--- a/pyramid/config/__init__.py
+++ b/pyramid/config/__init__.py
@@ -451,9 +451,6 @@ class Configurator(
             return filename # absolute filename
         return '%s:%s' % (package, filename)
 
-    def _split_spec(self, path_or_spec):
-        return resolve_asset_spec(path_or_spec, self.package_name)
-
     def _fix_registry(self):
         """ Fix up a ZCA component registry that is not a
         pyramid.registry.Registry by adding analogues of ``has_listeners``,
@@ -606,11 +603,15 @@ class Configurator(
         if autocommit:
             # callables can depend on the side effects of resolving a
             # deferred discriminator
-            undefer(discriminator)
-            if callable is not None:
-                callable(*args, **kw)
-            for introspectable in introspectables:
-                introspectable.register(self.introspector, action_info)
+            self.begin()
+            try:
+                undefer(discriminator)
+                if callable is not None:
+                    callable(*args, **kw)
+                for introspectable in introspectables:
+                    introspectable.register(self.introspector, action_info)
+            finally:
+                self.end()
 
         else:
             action = extra
@@ -651,7 +652,11 @@ class Configurator(
         of this error will be information about the source of the conflict,
         usually including file names and line numbers of the cause of the
         configuration conflicts."""
-        self.action_state.execute_actions(introspector=self.introspector)
+        self.begin()
+        try:
+            self.action_state.execute_actions(introspector=self.introspector)
+        finally:
+            self.end()
         self.action_state = ActionState() # old actions have been processed
 
     def include(self, callable, route_prefix=None):
@@ -885,14 +890,30 @@ class Configurator(
 
     absolute_resource_spec = absolute_asset_spec # b/w compat forever
 
-    def begin(self, request=None):
+    def begin(self, request=_marker):
         """ Indicate that application or test configuration has begun.
         This pushes a dictionary containing the :term:`application
         registry` implied by ``registry`` attribute of this
         configurator and the :term:`request` implied by the
         ``request`` argument onto the :term:`thread local` stack
         consulted by various :mod:`pyramid.threadlocal` API
-        functions."""
+        functions.
+
+        If ``request`` is not specified and the registry owned by the
+        configurator is already pushed as the current threadlocal registry
+        then this method will keep the current threadlocal request unchanged.
+
+        .. versionchanged:: 1.8
+           The current threadlocal request is propagated if the current
+           threadlocal registry remains unchanged.
+
+        """
+        if request is _marker:
+            current = self.manager.get()
+            if current['registry'] == self.registry:
+                request = current['request']
+            else:
+                request = None
         self.manager.push({'registry':self.registry, 'request':request})
 
     def end(self):
@@ -992,11 +1013,11 @@ class Configurator(
         # Push the registry onto the stack in case any code that depends on
         # the registry threadlocal APIs used in listeners subscribed to the
         # IApplicationCreated event.
-        self.manager.push({'registry': self.registry, 'request': None})
+        self.begin()
         try:
             self.registry.notify(ApplicationCreated(app))
         finally:
-            self.manager.pop()
+            self.end()
 
         return app
 
diff --git a/pyramid/config/assets.py b/pyramid/config/assets.py
index d05314384..6eafc1eb1 100644
--- a/pyramid/config/assets.py
+++ b/pyramid/config/assets.py
@@ -4,7 +4,10 @@ import sys
 
 from zope.interface import implementer
 
-from pyramid.interfaces import IPackageOverrides
+from pyramid.interfaces import (
+    IPackageOverrides,
+    PHASE1_CONFIG,
+)
 
 from pyramid.exceptions import ConfigurationError
 from pyramid.threadlocal import get_current_registry
@@ -387,6 +390,7 @@ class AssetsConfiguratorMixin(object):
             )
         intr['to_override'] = to_override
         intr['override_with'] = override_with
-        self.action(None, register, introspectables=(intr,))
+        self.action(None, register, introspectables=(intr,),
+                    order=PHASE1_CONFIG)
 
     override_resource = override_asset # bw compat
diff --git a/pyramid/config/i18n.py b/pyramid/config/i18n.py
index 69af0f9bc..9387a693b 100644
--- a/pyramid/config/i18n.py
+++ b/pyramid/config/i18n.py
@@ -1,13 +1,10 @@
-import os
-import sys
-
 from pyramid.interfaces import (
     ILocaleNegotiator,
     ITranslationDirectories,
     )
 
 from pyramid.exceptions import ConfigurationError
-from pyramid.path import package_path
+from pyramid.path import AssetResolver
 from pyramid.util import action_method
 
 class I18NConfiguratorMixin(object):
@@ -69,32 +66,32 @@ class I18NConfiguratorMixin(object):
         directories will be inserted into the beginning of the directory list
         in the order they're provided in the ``*specs`` list argument (items
         earlier in the list trump ones later in the list).
+
         """
         directories = []
         introspectables = []
-
-        for spec in specs[::-1]: # reversed
-            package_name, filename = self._split_spec(spec)
-            if package_name is None: # absolute filename
-                directory = filename
-            else:
-                __import__(package_name)
-                package = sys.modules[package_name]
-                directory = os.path.join(package_path(package), filename)
-
-            if not os.path.isdir(os.path.realpath(directory)):
-                raise ConfigurationError('"%s" is not a directory' %
-                                         directory)
-            intr = self.introspectable('translation directories', directory,
-                                       spec, 'translation directory')
-            intr['directory'] = directory
-            intr['spec'] = spec
-            introspectables.append(intr)
-            directories.append(directory)
+        resolver = AssetResolver(self.package_name)
 
         def register():
-            for directory in directories:
+            # defer spec resolution until register to allow for asset
+            # overrides to take place in an earlier config phase
+            for spec in specs[::-1]: # reversed
+                # the trailing slash helps match asset overrides for folders
+                if not spec.endswith('/'):
+                    spec += '/'
+                asset = resolver.resolve(spec)
+                directory = asset.abspath()
+                if not asset.isdir():
+                    raise ConfigurationError('"%s" is not a directory' %
+                                            directory)
+                intr = self.introspectable('translation directories', directory,
+                                        spec, 'translation directory')
+                intr['directory'] = directory
+                intr['spec'] = spec
+                introspectables.append(intr)
+                directories.append(directory)
 
+            for directory in directories:
                 tdirs = self.registry.queryUtility(ITranslationDirectories)
                 if tdirs is None:
                     tdirs = []
diff --git a/pyramid/tests/test_config/test_i18n.py b/pyramid/tests/test_config/test_i18n.py
index 71c68af8a..adfb6191c 100644
--- a/pyramid/tests/test_config/test_i18n.py
+++ b/pyramid/tests/test_config/test_i18n.py
@@ -36,9 +36,8 @@ class TestI18NConfiguratorMixin(unittest.TestCase):
     def test_add_translation_dirs_missing_dir(self):
         from pyramid.exceptions import ConfigurationError
         config = self._makeOne()
-        self.assertRaises(ConfigurationError,
-                          config.add_translation_dirs,
-                          '/wont/exist/on/my/system')
+        config.add_translation_dirs('/wont/exist/on/my/system')
+        self.assertRaises(ConfigurationError, config.commit)
 
     def test_add_translation_dirs_no_specs(self):
         from pyramid.interfaces import ITranslationDirectories
@@ -87,3 +86,30 @@ class TestI18NConfiguratorMixin(unittest.TestCase):
         self.assertEqual(config.registry.getUtility(ITranslationDirectories),
                          [locale])
 
+    def test_add_translation_dirs_uses_override_out_of_order(self):
+        from pyramid.interfaces import ITranslationDirectories
+        config = self._makeOne()
+        config.add_translation_dirs('pyramid.tests.pkgs.localeapp:locale')
+        config.override_asset('pyramid.tests.pkgs.localeapp:locale/',
+                              'pyramid.tests.pkgs.localeapp:locale2/')
+        config.commit()
+        self.assertEqual(config.registry.getUtility(ITranslationDirectories),
+                         [locale2])
+
+    def test_add_translation_dirs_doesnt_use_override_w_autocommit(self):
+        from pyramid.interfaces import ITranslationDirectories
+        config = self._makeOne(autocommit=True)
+        config.add_translation_dirs('pyramid.tests.pkgs.localeapp:locale')
+        config.override_asset('pyramid.tests.pkgs.localeapp:locale/',
+                              'pyramid.tests.pkgs.localeapp:locale2/')
+        self.assertEqual(config.registry.getUtility(ITranslationDirectories),
+                         [locale])
+
+    def test_add_translation_dirs_uses_override_w_autocommit(self):
+        from pyramid.interfaces import ITranslationDirectories
+        config = self._makeOne(autocommit=True)
+        config.override_asset('pyramid.tests.pkgs.localeapp:locale/',
+                              'pyramid.tests.pkgs.localeapp:locale2/')
+        config.add_translation_dirs('pyramid.tests.pkgs.localeapp:locale')
+        self.assertEqual(config.registry.getUtility(ITranslationDirectories),
+                         [locale2])
diff --git a/pyramid/tests/test_config/test_init.py b/pyramid/tests/test_config/test_init.py
index 7078d7e26..0d5413d16 100644
--- a/pyramid/tests/test_config/test_init.py
+++ b/pyramid/tests/test_config/test_init.py
@@ -91,13 +91,54 @@ class ConfiguratorTests(unittest.TestCase):
                          {'registry':config.registry, 'request':request})
         self.assertEqual(manager.popped, False)
 
+    def test_begin_overrides_request(self):
+        from pyramid.config import Configurator
+        config = Configurator()
+        manager = DummyThreadLocalManager()
+        req = object()
+        # set it up for auto-propagation
+        pushed = {'registry': config.registry, 'request': None}
+        manager.pushed = pushed
+        config.manager = manager
+        config.begin(req)
+        self.assertTrue(manager.pushed is not pushed)
+        self.assertEqual(manager.pushed['request'], req)
+        self.assertEqual(manager.pushed['registry'], config.registry)
+
+    def test_begin_propagates_request_for_same_registry(self):
+        from pyramid.config import Configurator
+        config = Configurator()
+        manager = DummyThreadLocalManager()
+        req = object()
+        pushed = {'registry': config.registry, 'request': req}
+        manager.pushed = pushed
+        config.manager = manager
+        config.begin()
+        self.assertTrue(manager.pushed is not pushed)
+        self.assertEqual(manager.pushed['request'], req)
+        self.assertEqual(manager.pushed['registry'], config.registry)
+
+    def test_begin_does_not_propagate_request_for_diff_registry(self):
+        from pyramid.config import Configurator
+        config = Configurator()
+        manager = DummyThreadLocalManager()
+        req = object()
+        pushed = {'registry': object(), 'request': req}
+        manager.pushed = pushed
+        config.manager = manager
+        config.begin()
+        self.assertTrue(manager.pushed is not pushed)
+        self.assertEqual(manager.pushed['request'], None)
+        self.assertEqual(manager.pushed['registry'], config.registry)
+
     def test_end(self):
         from pyramid.config import Configurator
         config = Configurator()
         manager = DummyThreadLocalManager()
+        pushed = manager.pushed
         config.manager = manager
         config.end()
-        self.assertEqual(manager.pushed, None)
+        self.assertEqual(manager.pushed, pushed)
         self.assertEqual(manager.popped, True)
 
     def test_ctor_with_package_registry(self):
@@ -1940,10 +1981,13 @@ class DummyRequest:
         self.cookies = {}
 
 class DummyThreadLocalManager(object):
-    pushed = None
-    popped = False
+    def __init__(self):
+        self.pushed = {'registry': None, 'request': None}
+        self.popped = False
     def push(self, d):
         self.pushed = d
+    def get(self):
+        return self.pushed
     def pop(self):
         self.popped = True