Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30197 closed New feature (wontfix)

Template variable resolution with class objects — at Version 4

Reported by: Alex Epshteyn Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords: template, variable, resolve
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Alex Epshteyn)

The behavior of if callable(current): ... current = current() in django.template.base.Variable._resolve_lookup prevents using a function (or a class!) as a value in a template.

This behavior only makes sense when the previous bit in self.lookups was an instance and the current bit is a method that should be invoked on that instance. I would argue that it never makes sense in any other case (for example, https://stackoverflow.com/q/6861601/1965404).

Example

Suppose you want to write a filter that returns the name of a class:

@register.filter()
def type_name(someClass):
    return someClass.__name__

Using this filter in a template like:

<div>The class name is {{ foo.bar|type_name }}</div>

might seem valid, but it turns out that this expression will never work as intended, due to the implementation of Variable._resolve_lookup — the filter we defined in this example will pretty much always return an empty string (or whatever value you configured for your app's string_if_invalid setting).

For example, if foo.bar is <type 'basestring'>, invoking it as a callable will raise TypeError: The basestring type cannot be instantiated, and if foo.bar is some other type that is actually instantiable (e.g. <type 'str'>, or a user-defined class), our filter will receive an instance of that class (rather than the class itself), which will almost-certainly lead to an exception like AttributeError: 'str' object has no attribute '__name__'

Approaches tried in the past

The workaround introduced by #15791 allows setting an attribute named do_not_call_in_templates on a callable object to be able to use the object itself as the value (rather than the value returned from invoking it).

However, this seems like a dirty hack, and furthermore, in many cases you may not want to (or even be able to) set an attribute on that object (e.g. if this object is a class that comes from a library and you have no control of it).

Current implementation of Variable._resolve_lookup*:

def _resolve_lookup(self, context):
    """
    Perform resolution of a real variable (i.e. not a literal) against the
    given context.
    """
    current = context
    try:  # catch-all for silent variable failures
        for bit in self.lookups:
            try:  # dictionary lookup
                current = current[bit]
            # (followed by nested try/except blocks for attribute lookup and list-index lookup)
            # ...
            # *** and at last ***:
            if callable(current):
                if getattr(current, 'do_not_call_in_templates', False):
                    pass
                elif getattr(current, 'alters_data', False):
                    current = context.template.engine.string_if_invalid
                else:
                    try:  # method call (assuming no args required)
                        current = current()
                    # etc...
    # etc...
    return current

* abbreviated for clarity

Proposed solution

At the very least, I think it would make sense to add a check for inspect.isclass to the if callable(current): ... current = current() block of this method.

For example:

if callable(current) and not inspect.isclass(current):
    if getattr(current, 'do_not_call_in_templates', False):
        pass
    # etc...

or

if callable(current):
    if getattr(current, 'do_not_call_in_templates', False) or inspect.isclass(current):
        pass
    # etc...

However, ideally, it would be even better to check whether the previous bit in self.lookups was an instance and the current bit is a method supported by the class of that instance before trying to invoke current as a callable.

Change History (4)

comment:1 by Tim Graham, 6 years ago

Easy pickings: unset

Could you detail your use case a little more? What's Foo.Bar some nested class? Also, the discussion of <type 'basestring'> is outdated since basestring is removed in Python 3. If you can present a patch with a test, we can take a look, although I'm not sure it'll be backwards compatible. I guess a documentation change may be needed to describe the new behavior.

comment:2 by Carlton Gibson, 6 years ago

Resolution: duplicate
Status: newclosed

I think we have to close this as a duplicate of #15791, which introduced do_not_call_in_templates, and just say "Use that".
(See also #29382, #29306, ...)

In particular, people will be passing classes to templates and relying on their being called such that ...or inspect.isclass(current): pass would be a breaking change.

The alternative natural approach to setting do_not_call_in_templates is to move the logic resolving the class name up into the scope where you prepare the context data.

in reply to:  1 comment:3 by Alex Epshteyn, 6 years ago

Replying to Tim Graham:

Could you detail your use case a little more? What's Foo.Bar some nested class? Also, the discussion of <type 'basestring'> is outdated since basestring is removed in Python 3. If you can present a patch with a test, we can take a look, although I'm not sure it'll be backwards compatible. I guess a documentation change may be needed to describe the new behavior.

"Foo.Bar" is just a label used in the example template -- it could be anything, really (I'll update the example to avoid this confusion). The actual class is not known until run-time (otherwise we wouldn't need to use our type_name filter in the first place :)). Let's just say that foo is some arbitrary object that has an attribute named "bar", whose value is a class.

In my particular use case, the code is running on the Python 2.7 runtime of Google App Engine and foo is an instance of google.appengine.ext.db.Property, and bar is its "data_type" attribute (whose value is the Python class representing this datastore property's storage type). My template is listing all the properties defined for a particular datastore model class, including their storage type and other metadata. For many subclasses of db.Property, the data_type is basestring, but that's not really the point -- this template wouldn't work regardless of the data type and regardless of whether it's running on Python 2 or 3, because de-referencing this variable in a template returns a new instance of the data type rather than the data type itself.

comment:4 by Alex Epshteyn, 6 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top