Regression in Context

[Timothée Peignier]

There is a regression since r15601 (in the 1.2.X branch), that notably break tests :

def _new(self): 
 	return self.__class__(autoescape=self.autoescape, 
 	                                    current_app=self.current_app, 
 	                                    use_l10n=self.use_l10n)

But the use_l10nattribute doesn't exist in 1.2.

[Timothée Peignier]

I've added a patch, that actually only scratch the surface, I have a new error when trying to access request.user in a context_processor in my tests :

  File "django/template/context.py", line 23, in __copy__
    duplicate = self._new()
  File "django/template/context.py", line 172, in _new
    current_app=self.current_app)
  File "django/template/context.py", line 168, in __init__
    self.update(processor(request))
  [..]
    if request.user.is_authenticated():
AttributeError: 'HttpRequest' object has no attribute 'user'

I will try to get a regression test working.

[Russell Keith-Magee]

Ok - moving back to accepted. Also putting it on SmileyChris' radar, since he broke it :-)

[anonymous]

Yes, I thought about this earlier today before reading this ticket...

[Chris Beaven]

Actually, I've fixed the use_l10n part of this in r15618.

I'll leave this ticket open to track the issue of copying the context. The issue is that we need to make a new instance of RequestContext, but don't have any reference to request at that time so I faked it with a bare HttpRequest (and the problems around this is what I was saying I thought about earlier).

I'm thinking that perhaps I could change RequestContext to allow for a request of None which simply skips the running of the context processors during instantiation.

[Chris Beaven]

Ok, put up a patch but needs tests still.

[Luke Plant]

@SmileyChris - why do we need to use the class constructor to copy the instance? Could we not just implement __copy__ differently on RequestContext? In fact, do we need to implement __copy__() at all? You can simplify things quite a bit by implementing a copy() method which just uses copy.copy() with a few additions, and then using that copy method rather than copy.copy() on the Context object.

I'll attached some tests for the regression involving RequestContext, and my proposed fix.

[Luke Plant]

tests

[Luke Plant]

my solution

[Chris Beaven]

luke: elsewhere in core the __copy__ method had been used, so I was just following precidence. I'm also fine with an explicit .copy() or .clone() method.

[Luke Plant]

Replying to SmileyChris:

luke: elsewhere in core the __copy__ method had been used, so I was just following precidence. I'm also fine with an explicit .copy() or .clone() method.

Sure, I think it just happens that in this case it is simpler not to implement __copy__().

However, there is a way to implement __copy__() and still avoid the need for calling constructors - like this:

class NoInitArgs(object): pass

class MyClass(object):
   def __copy__(self):
       duplicate = NoInitArgs()
       duplicate.__class__ = self.__class__
       duplicate.__dict__ = self.__dict__.copy()
       return duplicate

[Luke Plant]

Taking ownership, since we didn't hear from Chris for a few days, and I'm aware of the earthquake in New Zealand (hope everything is OK with you and family and friends, Chris!).

[Luke Plant]

In [15660]:

Fixed #15368 - test failures due to regression with RequestContext

Thanks to cyberdelia for the reports on this.

[Luke Plant]

In [15661]:

[1.2.X] Fixed #15368 - test failures due to regression with RequestContext

Thanks to cyberdelia for the reports on this.

Backport of [15660] from trunk.

[Chris Beaven]

Sorry, should have mentioned here that I had a branch ready for merging :P

​https://github.com/SmileyChris/django/tree/15368-copy-requestcontext

I'll apply the 1.3 notes today some time.
Note that in my branch, I just used copy(super(...)) which works rather than requiring a placeholder object.

[Luke Plant]

Sorry, Chris, didn't mean to be impatient, but with this blocking the RC we were supposed to release this week, and with it affecting my own projects, I went ahead and committed a fix. Feel free to improve what I did!

[Russell Keith-Magee]

In [15876]:

Fixed #15368 -- Ensure that raw unittest TestCase instances can be invoked individually by the test runner. Thanks to SmileyChris for the report and patch.

[Russell Keith-Magee]

Dang - that last comment should have been for #15638.

[Jacob]

Milestone 1.3 deleted