From: pgjones <philip.graham.jones@googlemail.com>
Date: Sun, 8 May 2016 15:10:35 +0000 (+0100)
Subject: Change cache key definitiion in environment
X-Git-Tag: 2.9~27^2~6^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7d8ec0f3e8c731c835f59825c1ad5b92c9339886;p=thirdparty%2Fjinja.git

Change cache key definitiion in environment

In 6671b973e6de5abc46829a27fd3bbb989d68ca3a the load_template method
was altered to use a cache key other than the template name. The key
chosen was the abs path as returned from the loader get_source
method. Unless there is no path in which case the name is
used. Unfortunately this introduced a performance regression, #485, as
the get_source method (in the FileStoreLoader) loads the template
(causing IO).

The purpose of #332 was to allow the loader to change whilst ensuring
the correct template was loaded, i.e. to fix this case

    env.loader = loader1
    env.get_template('index.html') # return loader1/index.html
    env.loader = loader2
    env.get_template('index.html') # also return loader1/index.html because of cache

This commit changes the cache key to be a tuple of the id(loader) and
the template name. Therefore fixing the above case without calling the
get_source method and thereby avoiding the IO load.

A test has been added to ensure the above case works as expected, this
required a minor refactor of the caching tests.
---

diff --git a/jinja2/environment.py b/jinja2/environment.py
index 6d8c17d4..67d05dd9 100644
--- a/jinja2/environment.py
+++ b/jinja2/environment.py
@@ -769,15 +769,7 @@ class Environment(object):
     def _load_template(self, name, globals):
         if self.loader is None:
             raise TypeError('no loader for this environment specified')
-        try:
-            # use abs path for cache key
-            cache_key = self.loader.get_source(self, name)[1]
-        except RuntimeError:
-            # if loader does not implement get_source()
-            cache_key = None
-        # if template is not file, use name for cache key
-        if cache_key is None:
-            cache_key = name
+        cache_key = (id(self.loader), name)
         if self.cache is not None:
             template = self.cache.get(cache_key)
             if template is not None and (not self.auto_reload or
diff --git a/tests/test_loader.py b/tests/test_loader.py
index 6d22fad3..6b9d9bb4 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -78,19 +78,31 @@ class TestLoaders():
         assert tmpl is not env.get_template('template')
         changed = False
 
-        env = Environment(loader=TestLoader(), cache_size=0)
-        assert env.get_template('template') \
-            is not env.get_template('template')
-
-        env = Environment(loader=TestLoader(), cache_size=2)
+    def test_no_cache(self):
+        mapping = {'foo': 'one'}
+        env = Environment(loader=loaders.DictLoader(mapping), cache_size=0)
+        assert env.get_template('foo') is not env.get_template('foo')
+
+    def test_limited_size_cache(self):
+        mapping = {'one': 'foo', 'two': 'bar', 'three': 'baz'}
+        loader = loaders.DictLoader(mapping)
+        env = Environment(loader=loader, cache_size=2)
         t1 = env.get_template('one')
         t2 = env.get_template('two')
         assert t2 is env.get_template('two')
         assert t1 is env.get_template('one')
         t3 = env.get_template('three')
-        assert 'one' in env.cache
-        assert 'two' not in env.cache
-        assert 'three' in env.cache
+        assert (id(loader), 'one') in env.cache
+        assert (id(loader), 'two') not in env.cache
+        assert (id(loader), 'three') in env.cache
+
+    def test_cache_loader_change(self):
+        loader1 = loaders.DictLoader({'foo': 'one'})
+        loader2 = loaders.DictLoader({'foo': 'two'})
+        env = Environment(loader=loader1, cache_size=2)
+        assert env.get_template('foo').render() == 'one'
+        env.loader = loader2
+        assert env.get_template('foo').render() == 'two'
 
     def test_dict_loader_cache_invalidates(self):
         mapping = {'foo': "one"}