Opened 13 years ago

Closed 13 years ago

Last modified 11 years ago

#16307 closed Bug (wontfix)

Unable to expand (resolve) template variable attribute when the variable is a python class

Reported by: sorin Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: sorin, Enrico Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you have a variable in a template that contains a python class like {type} <app.reports.MyReport>.

If you try to access an attribute in this class, from the template, like {{ myvar.verbose_name }} you will not get the attribute and Django will return you the settings.TEMPLATE_STRING_IF_INVALID value.

I debugged this, using the latest SVN code release on 1.3.x release branch and discovered that Django reaches the line #702 - https://code.djangoproject.com/browser/django/branches/releases/1.3.X/django/template/base.py#L702

This seams like a bug because current contains the right class and current.verbose_name has a string inside.

Attachments (1)

16307_test_callable_variable_attribute_parsing.diff (2.3 KB ) - added by Enrico 13 years ago.
Tests and fix for accessing attributes on callables and classes

Download all attachments as: .zip

Change History (12)

comment:1 by sorin, 13 years ago

Cc: sorin added

comment:2 by Aymeric Augustin, 13 years ago

Resolution: invalid
Status: newclosed

As far as I can tell, in your example, you're trying to access a field from a Django model. Fields are not accessible as attributes of the class; they're stored in _meta.

If your class had this attribute, it would be caught here:
https://code.djangoproject.com/browser/django/branches/releases/1.3.X/django/template/base.py#L683

comment:3 by Enrico, 13 years ago

I think this ticket was mistakenly closed.

Sorin is trying to access the verbose_name of a app.reports.MyReport class, not the verbose_name of a model.

When the template system tries to resolve a variable which is a class, it thinks the class is a callable and may fail to call (or instantiate) it due to missing arguments in the constructor.

class MyReport(object):
    verbose_name = "My Report"

render_to_string('some/template.html', {'report_class': MyReport})

And the template:

<p>Name: {{ report_class.verbose_name }}</p>

comment:4 by Carl Meyer, 13 years ago

Resolution: invalid
Status: closedreopened
Triage Stage: UnreviewedAccepted
Version: 1.3SVN

Re-opening based on rico's clarification.

I'm not sure what the solution is, but I think this is a legitimate bug. Maybe a fallback to direct attribute access if calling a callable fails? That risks masking other deeper TypeError occurrences; I wish Python had a dedicated exception for wrong-arguments. The cure for this could be worse than the disease, which might mean it has be to wontfix - but I'll leave it open for now in case anyone has good ideas.

comment:5 by Luke Plant, 13 years ago

I think catching TypeError would be a very bad idea. First, it only fixes one instance of the bigger problem, which has surfaced in other areas, namely that the template language calls classes instead of leaving them as they are. Catching the TypeError will only work for the case where the constructor takes required arguments. Just because you *can* call a class constructor with no arguments doesn't mean that that is what the template author intended. Second, catching TypeError could mask all kind of other errors in callables.

I don't think there will be a nice solution to this. The best option would probably be to treat classes differently from other callables, and not call them. I suspect this could produce worse backwards incompatibilities, though.

For the current problem, there is an easy work around: pass MyReport.__dict__ instead of MyReport. Personally, I think the answer is "don't pass classes into templates, full stop". Classes are developer oriented objects, not meant as general datastructures suitable for use in templates.

comment:6 by Enrico, 13 years ago

I think the best approach would be to not treat classes as callables.
But I agree that this may lead to more problems than it fixes.

I just don't agree that we shouldn't pass classes for templates, I think there are viable exceptions for this.

For now I'm using a wrapper because I need to call a classmethod on the template:

class NonCallable(object):

    def __init__(self, wrapped):
        self._wrapped = wrapped

    def __getattr__(self, item):
        return getattr(self._wrapped, item)

by Enrico, 13 years ago

Tests and fix for accessing attributes on callables and classes

comment:7 by Enrico, 13 years ago

Cc: Enrico added
Has patch: set

Added a patch with tests against the 1.3.1 tag of the official git repository on github.

The tests covers accessing an attribute, a class method and a static method on a class.

comment:8 by Luke Plant, 13 years ago

This will, of course, break anyone who had a class like:

class Foo(object):
    def __init__(self):
        self.bar = 123

and then did 'Foo.bar' in a template with {'Foo':Foo} passed in as the context.

Is that a silly thing to do? Well, that's debatable, but it has one massive thing in its favour: it is the status quo, and every change we've ever made in this section of code has generated 'regression' bug reports.

The docs clearly state: "If any part of the variable is callable, the template system will try calling it", which is exactly what happens. Personally I'm finding it hard to see the bug here.

comment:9 by Enrico, 13 years ago

I think that expecting to initialize a class inside the template just because is looks like a callable is wrong.

It's simple to work around this issue by initializing the class before passing to the template, but I can't do the opposite, to use a class I'll have to use a wrapper or some sort of data structure.

But I know it's a tough decision and I'll leave to the core developers.
Changing this behaviour could be a breaker for anyone using the example above.

comment:10 by Carl Meyer, 13 years ago

Resolution: wontfix
Status: reopenedclosed

I do find this a tough call, but in the end I come down with Luke. If I were designing the template language from scratch I might do things differently, but the benefit here just isn't sufficient or evident enough to warrant the backwards-compatibility problems.

comment:11 by Philippe Raoult, 11 years ago

Just for the record I was bitten by this too, and used the do_not_call_in_templates flag in my models to work around it. Maybe this should be the default for all models ?

Note: See TracTickets for help on using tickets.
Back to Top