RequestContext does not apply context processors [regression]

[direx]

Prior to Django 1.8 django.template.RequestContext always had the context from all of the TEMPLATE_CONTEXT_PROCESSORS applied. In Django 1.8 this is no longer the case, which breaks the API. As this has neither been announced nor documented anywhere I'd consider this a critical regression.

The following view code no longer works with the default TEMPLATE_CONTEXT_PROCESSORS setting:

from django.template import RequestContext

def my_view(request):
    context = RequestContext(request)
    print context['user']
    #...

Actually user should be put there by django.contrib.auth.context_processors.auth. It is also present in templates rendered by render(), but not in RequestContext. Of course this does not only apply to the user context variable, but to all context variables set by context processors.

[Tim Graham]

Bisected to 37505b6397058bcc3460f23d48a7de9641cd6ef0. The test I used to bisect is attached.

[Carl Meyer]

As part of the multiple-template-engines feature, Django 1.8 refactored the Django template language to remove the direct coupling to global settings, instead making it possible to instantiate multiple independently-configured Engine instances (and configure multiple such instances via the new TEMPLATES setting). Because of this, it's no longer possible for RequestContext to automatically fetch the list of configured context processors from settings (the TEMPLATES setting can define multiple instances of the DTL with different lists of context processors, so how could RequestContext possibly know which list of context processors to use?). This was handled by adding a new context-manager method RequestContext.bind_template which temporarily binds a RequestContext to a template instance, in the process applying all the context processors from that template's engine configuration.

This solution is fully backwards-compatible for the only documented usage of a RequestContext, which is to pass it into a template render and access its values in the template. It's true that it's not backwards-compatible for the usage you demonstrate, directly accessing values from the context as a dictionary without rendering a template, but this is not a documented or supported mode of use for RequestContext. If you need this usage, you must now first bind the context to a specific template:

from django.template import engines

engine = engines['default']
tpl = engine.get_template('some/template.html')
ctx = RequestContext(request)
with ctx.bind_template(tpl):
    print(ctx['user'])

It may be possible to restore the old behavior for this usage using Engine.get_default() (it would only work if there's only a single instance of the DTL configured in settings); personally I don't think that's worth doing, or that this use of RequestContext should be supported.

Alternatively, we could add a warning to the 1.8 release notes about this, but this usage is so unusual that I'm not even sure that's worth doing.

[Aymeric Augustin]

Thanks Carl, you've explained the situation more clearly than I would have been able to. This is the intended behavior. It does not break the public API.

[direx]

By saying that this is not part of the public API or not a supported usage of RequestContext you are making this a little too easy for you.

After looking at the Django 1.7 documentation these are facts:

1. RequestContext is a special Context class. One of the differences „is that it automatically populates the context with a few variables, according to your TEMPLATE_CONTEXT_PROCESSORS setting.“ (quote from documentation)

2. Another quote from the documentation: „Most of the time, you’ll instantiate Context objects by passing in a fully-populated dictionary to Context(). But you can add and delete items from a Context object once it’s been instantiated, too, using standard dictionary syntax“. Note the standard dictionary syntax here.

So I don't see why the behavior described in this bug should not be considered part of the public API. The usage as dictionary is actually even documented and encouraged (also in 1.8, section Playing with Context objects). So I'd still consider this a public API breakage.

[Aymeric Augustin]

Can you submit a patch?

[Aymeric Augustin]

(In case that's not clear enough, saying "you are making this a little too easy for you" to a person who works in their free time guarantees their non-cooperation. My involvement with this issue stops here.)

[Carl Meyer]

That's a good point; I looked through the RequestContext docs but didn't notice that dictionary-style access was documented higher up in the Context docs. I think that does make it a backwards-incompatible change to a public API that should have been documented in the release notes.

However, I still don't think it's a valuable enough usage to fix at this point. If we had run across this case while building multiple-template-engines, I think we'd have documented it as a minor backwards-incompatibility, not attempted to accommodate it -- the usage just doesn't belong in a multiple-template-engines world.

I'd be willing to look at a proposed patch, but I think a patch will need to introduce too much complexity. Specifically, a patch to reintroduce the old behavior would need to introduce a scheme for lazy-loading of the "default" context processors (that is, the ones configured for the "default" DTL instance -- which doesn't exist if there's more than one DTL instance configured) that takes place only if someone tries to dictionary-access a RequestContext that hasn't been bound to a template. This would be an entirely new code path that exists only to support this usage. I don't think that added complexity is justified by the limited-to-nonexistent need for this usage (note that the docs don't ever recommend it for any real purpose, just for "playing around" to understand Context objects).

So I think this ticket should be fixed the same way we'd have fixed it if we'd noticed it before the release of 1.8: by making a note of the issue in the backwards-incompatible changes section of the 1.8 release notes, and perhaps also clarifying the RequestContext docs to note that values from context processors are only available in a RequestContext once it has been bound to a template (which Django does automatically when you render a template using a RequestContext).

[direx]

Replying to aaugustin:

(In case that's not clear enough, saying "you are making this a little too easy for you" to a person who works in their free time guarantees their non-cooperation. My involvement with this issue stops here.)

I don't get your point. I was neither rude, nor was my comment meant to be offensive. I am also reporting bugs in my free time, hoping to help other users who might stumble upon the same issue. Sorry if you got me wrong there.

Replying to carljm:

So I think this ticket should be fixed the same way we'd have fixed it if we'd noticed it before the release of 1.8: by making a note of the issue in the backwards-incompatible changes section of the 1.8 release notes, and perhaps also clarifying the RequestContext docs to note that values from context processors are only available in a RequestContext once it has been bound to a template (which Django does automatically when you render a template using a RequestContext).

To me this entire issue is actually not a biggie. Some third-party apps I am using rely on that behavior, but I can fix that. So I'd agree with you here that adding it to the list of backwards-incompatible changes and maybe a small note in the docs would probably be sufficient.

[Aymeric Augustin]

It took me on the order of magnitude of one week of work to come up with the patch discussed here in order to maximize backwards compatibility — most likely because I'm a poor developer. Regardless I didn't try to cut corners and this was certainly not "easy for me". I'm sorry it didn't work for you.

[Carl Meyer]

@direx Thanks for reporting the issue, and following up with your observations about what behaviors were documented! I'm glad we're agreed on the best path forward.

FWIW I didn't take the "too easy" comment personally on an individual level, but rather on the project level, which includes everyone commenting here. As a project, the initial response here was to disavow any responsibility at all for this change on a backwards-compatibility front, and I do indeed (in retrospect) think that was a bit "making it too easy," purely in a what-do-we-need-to-do-now-about-this-issue-as-a-project sense. Fortunately I don't think the documentation-based resolution is "making it too hard" either :-) I certainly didn't see the comment as a criticism of the (large amount of excellent) work that went into the initial multiple-template-engines change; unintentional minor regressions happen, even with much smaller feature additions than that one.

@direx, I'm curious if you could point me to the third-party app(s) making use of this behavior? I doubt it will change my mind on what we should do, but I am curious to see how they found it useful.

[Tim Graham]

As 1.8 is nearly a year old and we only have a single report of the issue, I say it's not worth documenting unless someone offers a patch. If so, please reopen. Thanks!