Test client handler should call set_script_prefix

[Joost Cassee]

Since [8015] every HTTP handler calls urlresolvers.set_script_prefix to set the prefix for reverse URL resolving. The only except in the handler used by the test client.

The problem I am solving may seem a bit esoteric. I'm trying to remove the necessity of monkey-patching the urlresolvers.reverse function in the (third-party) localeurl app. My solution is to append to the script prefix for the request, but because the test client handler does not reset it at every request my tests fail.

The attached patch makes the test client handler call set_script_prefix at every request.

[Joost Cassee]

On second though, the test client should not depend on the environment. Just set the prefix to '/'. Updated patch.

[Malcolm Tredinnick]

I'm a little worried that this is fixing some kind of problem at the wrong level. If you do absolutely nothing, it should be the same as if the script prefix is '/'. This is because lots of code that relies on the script prefix (anything calling reverse()) can be called outside of Django's request-response loop. For example, in a cronjob.

So I'm not sure why anything at all needs to be done here. If it does need to be done, why is that so? What fails if you don't make this change?

[Joost Cassee]

I'm not sure what I can tell you that is not already in the ticket summary. The patch is trying to fix a problem with this (pseudo-)code in middleware:

def process_request(self, request):
    prefix = urlresolvers.get_script_prefix()
    prefix += <something>
    urlresolvers.set_script_prefix(prefix)

Because the real handlers reset the prefix on every request this works. The test client, however, does not. This causes tests for this middleware to fail.

I notice now, however, that these handlers set the prefix after the middleware has loaded. I now wonder how come this code snippet (which I tested and use 'in real life') work.

[Joost Cassee]

Bah, loading != processing.

[Jacob]

Looking at urlresolvers.get_script_prefix, I can't see how it could fail -- if the prefix isn't set, it'll just return / anyway. Without any more details -- and psudocode won't cut it -- I can't verify this is actually a bug. I suspect it's a problem in your code.

I'll take a shot in the dark, I suspect that it's a matter of you not cleaning up after yourself in your middleware: if you modify the script prefix in process_request, you need to clean it up at some point during the response. Otherwise the script prefix will keep growing, and that's broken.

[Jacob]

I should have also noted that if you can verify that it's a bug in Django please feel free to reopen the ticket.

[Joost Cassee]

Jacob, it's definitely a resource-cleanup problem. Thanks for pointing that out. The prefix is reset (set_script_prefix is called) at every request (at least when using FastCGI or the development server), so I got away with it. The test client does not reset it, so my code failed there.

[Jacob]

Milestone 1.0 deleted