gregmuellegger / django-mobile

Detect mobile browsers and serve different template flavours to them.
http://pypi.python.org/pypi/django-mobile
BSD 3-Clause "New" or "Revised" License
560 stars 170 forks source link

CachedLoader breaks application #73

Open jungornti opened 8 years ago

jungornti commented 8 years ago

Hello!

We are using django-mobile in our project.

We use Django 1.9 and experienced trouble with cached loader: it ruined the application down with exceptions:

  1. ./manage.py runserver just crashed when tried to set flavour to mobile
  2. uwsgi has a maximum recursion depth exceeded when tried to set flavour to mobile

Our templates configuration is like this:

templates/base.html templates/mobile/base.html

mobile/base.html has {% extends "base.html" %}

In this situation problem started to appear.

We had to turn off cached loaders at all, because django-mobile's loader is broken and Django's cached loader do not working with django-mobile.

As far as i see, the problem is in django-mobile's CachedLoader implementation — it does not understand what current flavour is, so it generate broken cache key for both base.html and mobile/base.html, which looks like this:

full:base.html

So the mobile template tries to extend "base.html", but gets itself content (with extend) instead of original, root base.html.

jungornti commented 8 years ago

Oh, and i should mention that we are using version from master (0.7.0dev1), because earlier version do not work with our project (django 1.9).

gregmuellegger commented 8 years ago

Hm, this sounds strange as the CachedLoader is using get_flavour to determine the cache key. Do you have a custom flavour detection middleware in place? Are you loading the templates outside of a request/response cycle?

Maybe try hooking some print statements in the CachedLoader and see what happens there in your scenario.

jungornti commented 8 years ago

Hello, and thank you for response.

We do not have custom flavour detection middleware, also we are loading template in request/response cycle.

I will hook prints into cached loader's cache_key function and send message here.

jungornti commented 8 years ago

Okay, i did some debugging:

Thats how i modified cached loader:

class CachedLoader(DjangoCachedLoader): is_usable = True

def cache_key(self, template_name, template_dirs, *args):
    if len(args) > 0:  # Django >= 1.9
        key = super(CachedLoader, self).cache_key(template_name, template_dirs, *args)
    else:
        if template_dirs:
            key = '-'.join([
                template_name,
                hashlib.sha1(force_bytes('|'.join(template_dirs))).hexdigest()
            ])
        else:
            key = template_name
    if template_name == 'base.html':
        # template origins are in args[0]
        origins = []
        for arg in args[0]:
            origins.append({
                'object': arg,
                'object.name': arg.name,
                'object.template_name': arg.template_name,
                'object.loader_name': arg.loader_name,
            })
        # just output extended debug information
        pprint.pprint({
            'template_name': template_name,
            'template_dirs': template_dirs,
            'args[0]': origins,
            'cache_key': '{0}:{1}'.format(get_flavour(), key),
        })
    return '{0}:{1}'.format(get_flavour(), key)

This code returned this output to me:

{'args[0]': [{'object': <django.template.base.Origin object at 0x7f0dffd9e0b8>,
              'object.loader_name': 'django.template.loaders.filesystem.Loader',
              'object.name': '/home/user/project/templates/home.html',
              'object.template_name': 'home.html'},
             {'object': <django.template.base.Origin object at 0x7f0dffdb7da0>,
              'object.loader_name': 'django.template.loaders.filesystem.Loader',
              'object.name': '/home/user/project/templates/mobile/base.html',
              'object.template_name': 'mobile/base.html'},

              <few hundreds of same records for an Origin object at 0x7f0dffdb7da0>

             {'object': <django.template.base.Origin object at 0x7f0dffdb7da0>,
              'object.loader_name': 'django.template.loaders.filesystem.Loader',
              'object.name': '/home/user/project/templates/mobile/base.html',
              'object.template_name': 'mobile/base.html'}],
 'cache_key': 'mobile:base.html',
 'template_dirs': None,
 'template_name': 'base.html'}
Fatal Python error: Cannot recover from stack overflow.

<stack trace>

Our configurations is like this:

templates/home.html → first request goes here home.html extends base.html

templates/base.html → desktop version templates/mobile/base.html → mobile version

So it looks like this:

Incoming request starts the homepage view, which tries to render home.html template. Template loader loads home.html template and goes to base.html. As we can see it actually founds the mobile/base.html instead of original base.html, but then it tries to parse mobile/base.html and dies by recursion.

This is how templates looks:

base.html: just a simple template

home.html:

{% extends "base.html" %}

mobile/base.html:

{% extends "base.html" %} → this recursive extend does not work with CachedLoader.

I should note that Django supports recursive loading of templates (and it is working with non cached loader version).

TEMPLATES configuration variable looks like this:

[{'BACKEND': 'django.template.backends.django.DjangoTemplates',
  'DIRS': ['/home/user/project/templates'],
  'OPTIONS': {'builtins': ['core.templatetags.builtins',
    'el_pagination.templatetags.el_pagination_tags'],
   'context_processors': ('django.contrib.auth.context_processors.auth',
    'django.template.context_processors.request',
    'django.template.context_processors.debug',
    'django.template.context_processors.i18n',
    'django.template.context_processors.media',
    'django.template.context_processors.static',
    'django.template.context_processors.tz',
    'django.contrib.messages.context_processors.messages',
    'django_mobile.context_processors.flavour',
    'articles.context_processors.from_yandex',
    'dom.context_processors.admin_index_modules'),
   'libraries': {},
   'loaders': [('django_mobile.loader.CachedLoader',
     ['django_mobile.loader.Loader',
      'django.template.loaders.filesystem.Loader',
      'django.template.loaders.app_directories.Loader'])]}}]

I will be happy if it will be fixed, or at least some workaround found.

Thanks!