Skip to content

Commit

Permalink
Change cache key definitiion in environment
Browse files Browse the repository at this point in the history
In 6671b97 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, pallets#485, as
the get_source method (in the FileStoreLoader) loads the template
(causing IO).

The purpose of pallets#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.
  • Loading branch information
pgjones committed May 19, 2016
1 parent b927c9f commit 7d8ec0f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
10 changes: 1 addition & 9 deletions jinja2/environment.py
Expand Up @@ -769,15 +769,7 @@ def join_path(self, template, parent):
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
Expand Down
28 changes: 20 additions & 8 deletions tests/test_loader.py
Expand Up @@ -78,19 +78,31 @@ def get_source(self, environment, template):
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"}
Expand Down

0 comments on commit 7d8ec0f

Please sign in to comment.