#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 )
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)
follow-up: 3 comment:1 by , 6 years ago
Easy pickings: | unset |
---|
comment:2 by , 6 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
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.
comment:3 by , 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 , 6 years ago
Description: | modified (diff) |
---|
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.