Internally choosing how to process a template is inconsistent

[Keryn Knight <django@…>]

​inclusion_tag checks the following:

• Is it a Template instance?
• Or is it not a string, and is it iterable? is_iterable would allow genexps/dictionaries through
• Both the above failed? Should be a string.

​render_to_string checks the following:

• is it a list or tuple? doesn't allow genexps/dictionaries/etc
• No? It must be a string
• doesn't account for being passed a Template instance at all

​SimpleTemplateResponse checks the following:

• is it a list or a tuple (consistent with render_to_string)
• No; is it a string? (consistent with render_to_string)
• No; ok, it must be a template instance (inconsistent with render_to_string, sort of consistent with inclusion_tag)

I discovered this while trying to choose how to fix #20995, which would again be different; I don't really want to push a pull request for that one without knowing what the outcome of this is, though. It might be worth introducing a single function (resolve_template or something) which provides a consistent way to take an input and decide how to get from it to a Template instance.

[Marc Tamlyn]

+1 to improving the consistency here. I think it's safe from a compat point of view because the three cases should be independent of each other.

I like the idea of a proper function with public api for doing this, it seems common for someone wishing to implement extensions or alternatives to the API without worrying about this exact implementation.

[Keryn Knight <django@…>]

I've thought about this some, and so far I'd be inclined to introduce a function, which I'm referring to as resolve_template; the idea being to turn any given input into something which ultimately returns either a Template instance, or raises TemplateDoesNotExist. What I've come up with though, does mostly step on the toes of select_template:

• resolve_template would take one argument;
• if the argument isinstance of six.string_type, or Template, turn it into a tuple of that (allowing us to avoid needing to do the insinstance shuffle as much further in):

  input = 'mystring'
  if insinstance(input, six.string_type):
      input = (input,)
  

• otherwise, consider the argument to be an iterable of some sort.
• for each item in the iterable:
  • if it's a Template instance, return it
  • if it's a string, use get_template, returning if it resolves to a Template, or continuing if it raised TemplateDoesNotExist
• If the iterable is exhausted without finding anything, raise TemplateDoesNotExist

It's pretty much select_template, though I'm not familiar enough with the loaders behind the scenes to say whether or not there's any idiosyncrasies which would preclude the changes from being applied directly to select_template, especially WRT to the guarantee of backwards compatibility.

The larger point however, is that it should be entirely feasible for the following use cases, which aren't guaranteed right now because of the multiple implementations:

• I want to provide a list of template strings for the template loader to resolve, with a Template instance at the end (or short circuited into index N if I so desire):

  ['app/model.html', 'app/fallback.html', Template('{{ x }}')]
  

• I want to provide a genexp of templates to be resolved, because why not:

  things = (x for x in y)
  

• I have a deque object already, and I want to pass it directly to the template engine:

  things = deque(['app/model.html', 'something/else.html'])
  

I have an OrderedDict/SortedDict whose keys map to template loader locations, and whose values I'm already making use of elsewhere:

a = OrderedDict((('app/model.html', 1), ('app/model2.html', 2)))

The only 'problem' with providing this kind of flexibility is less in the implementation (I hope) than the fact that some iterables simply wouldn't be suited for template resolution, namely: dict, set and other inherently unordered objects/types. Its impossible to reasonably accept those as input, because Django can no longer guarantee order of template resolution. This kind of worry is, I expect, what led to the whitelist of iterable types for the most part, rather than a blacklist, though as the original ticket indicates, inclusion_tag might allow those through by accident at the moment.

Ultimately, this could then be used to replace many uses of get_template and select_template and the associated sniffing of types in areas outside of the template package, where it arguably belongs as an implementation detail.

[Aymeric Augustin]

The landscape changed when I merged the multiple template engines branch. Some parts may be more consistent now.

[bcail]

One option would be to add a resolve_template() method/function to the engine and loader modules. The resolve_template() function could handle choosing between select_template() and get_template().

This would allow removing resolve_template() in SimpleTemplateResponse, and Engine.render_to_string and InclusionNode could use the resolve_template() in the engine module.

If there's interest in this, I could open a PR.