On Fri, Nov 20, 2009 at 9:50 PM, Tobias McNulty <tob...@caktusgroup.com> wrote: > Hi Russell, thanks for all your feedback. I'll review the small issues in a > separate email (or just fix them), but here are my responses to the two > bigger problems. > > On Fri, Nov 20, 2009 at 3:29 AM, Russell Keith-Magee > <freakboy3...@gmail.com> wrote: >> >> * I think there's a circular reference problem in the middleware with >> the setup for request.messages. The messages middleware actually >> creates request.messages, but the auth middleware creates a lazy >> evaluator that can return request.messages. This means that as a >> request comes in, the request is always given a lazy evaluator for >> user.messages_set, which is then overwritten by the messages >> middleware. The consequence of this is that any middleware that is >> executed between the auth and messages processors will get >> user.message_set if they try to set a message. > > The auth middleware now returns the same thing the messages middleware does, > so that may change the problem a little. Still, I'm not sure what the > consequence would be of using messages between the two middlewares. I'll > give it some thought and propose/implement a fix if it looks problematic.
Looking at this again, it isn't as bad as I first thought, due to a brainfart on my part. I was looking at the context processor code for auth and mentally mapping it to the auth middleware for some reason. However, there is still a lesser version of the same problem - request.messages.add() will fail outright if you try to invoke it before the messages middleware has executed, but compat_add_message() function won't fail in the same circumstances. >> * I have some concerns about the thread-safety (or, rather, >> request-safety) of the various backends. If I'm reading the code >> right, if you have two parallel requests, a message written in one >> response will overwrite a message written in the second, which will >> result in the first message never being displayed to the user. This >> wasn't a problem with the old API because messages were written to the >> database on response, and read on request; the first request would get >> all the messages. If we're using cookies and sessions, there's all >> sorts of clobbering problems. > > True. One simple solution would be to offer another "thread safe" backend > that used the database as a storage (independent of the User model of > course). > > As long as we note the lack of thread safety in the cookie and session > backends I think they'll still be alright for most cases; hopefully this is > something that folks using sessions and cookies are prepared to deal with. While I agree that this problem is at least semi-obvious to anyone experienced with sessions and cookies, I can imagine it being extremely surprising to anybody looking at contrib.messages as a "messaging system" independent of internals. A big notice that says "messages aren't guaranteed to be delivered, or guaranteed to be delivered into the window that created them" is the very least we can do here. It would be nice if we could do more, but I'm not sure there is a practical solution (other than providing a database-backed store). Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=.