#12502 closed Bug (fixed)
Diagram doesn't match text in Middleware documentation
Reported by: | Chris Petrilli | Owned by: | |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Shai Berger, safplusplus | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
In the [current documentation http://docs.djangoproject.com/en/dev/topics/http/middleware/], the diagram under Activate Middleware doesn't match the MIDDLEWARE_CLASSES
above it.
Attachments (13)
Change History (34)
by , 15 years ago
Attachment: | middleware_square.png added |
---|
comment:1 by , 15 years ago
I have attached a couple designs, and the OmniGraffle document they're derived from. Rather than continue with the circle, I switched to squares in one of them simply because the middleware names were becoming unreadable due to their length.
comment:2 by , 15 years ago
The attached diagram is out of date, since MessageMiddleware is now included by startproject (#12795). We may want to consider an approach that doesn't require the image to be updated each time this list is changed.
comment:3 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 15 years ago
Attachment: | middleware_arrow.png added |
---|
by , 15 years ago
Attachment: | Request Response Middleware.graffle added |
---|
comment:4 by , 15 years ago
I've updated the diagram, but I agree that it could become an issue long-term. Unfortunately, I can't think of a way to express it "visually" in words that will make it clear how the process works.
by , 15 years ago
Attachment: | 12502_image_patch.diff added |
---|
Patch updates the middleware.png to use the middleware_arrow.png provided on this ticket.
comment:5 by , 15 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
I added a patch that just does a one-for-one replacement of the existing middleware.png file with the middleware-arrow.png provided by petrilli (which is currently up-to-date).
Until a decision is reached about finding a way to avoid using an image at all, we might as well have a working patch for the docs.
comment:6 by , 15 years ago
Cc: | added |
---|
Hi,
The images provided, as well as the one currently in the docs, share a problem: They are misleading, because they over-simplify the process.
The points missed:
- There are actually two passes "going in", one using
process_request
and one usingprocess_view
- The "go in, then out" pictures create the impression that
process_response
will only be called for a given middleware class if the correspondingprocess_request
was called; the text makes it clear that this isn't the case. - Exception processing is left completely out of the picture, and that is especially misleading; in particular, it should be made clear that exception processing does not replace response processing, but is done before it; and that it is only applied to the view, not to any processing in the middlewares.
The mismatch of the list of middlewares is, in comparison, completely minor IMHO. Also, I think a simple, complete solution to the list mismatch problem is to use generic names ("MiddleWareA, MiddleWareB" etc.) in the diagram.
by , 15 years ago
Attachment: | Middleware.process_request.png added |
---|
Diagram of middleware request processing
by , 15 years ago
Attachment: | Middleware.process_view.png added |
---|
Diagram of middleware view processing
by , 15 years ago
Attachment: | Middleware.process_exception.png added |
---|
Diagram of middleware response processing
by , 15 years ago
Attachment: | Middleware.process_response.png added |
---|
Diagram of middleware response processing
comment:7 by , 15 years ago
Cc: | added |
---|
I agree with shai on that the current diagram also doesn't represent correctly how the middleware processing actually works. I took petrilli diagram and extended it with diagrams for the handling of requests, views, exceptions and responses.
Two reasons though why I'm not 100% content with my diagrams. One being that they are ugly and the second being that they may appear too complex to glance over quickly.
comment:8 by , 15 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
I'm gonna unassign this to let somebody else make the final decision during the sprint since I'm mostly not available right now.
by , 14 years ago
Attachment: | django-middleware-processing.svg added |
---|
SVG file -- diagram depicting middleware processing
by , 14 years ago
Attachment: | django-middleware-processing-bg.png added |
---|
PNG file -- diagram depicting middleware processing
comment:10 by , 14 years ago
The diagram I've just attached shows, I believe, a true picture of the middleware processing. Black arrows show the normal flow, gray arrows are other possible (non-erroneous) paths. I think the rest is self-explanatory enough.
This is the PNG, for easy viewing; a "source" (SVG) was also attached.
Shai.
comment:11 by , 14 years ago
Patch needs improvement: | set |
---|
This latest diagram is by far the most accurate, but my concern is that it's accurate at the cost of being fairly complicated. I'm not certain there's a better compromise between accuracy and complexity to be had, but I worry this will further confuse inexperienced users.
I think at the very least it should have a key in the diagram explaining what the colors mean (black = normal flow, grey = possible short-circuits, red = exception).
If anyone else can think how this could be further simplified feel free to chime in.
comment:12 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:15 by , 12 years ago
I would like to point out that with the introduction of process_template_response
, the middleware documentation has become unclear and ambiguous: I couldn't fathom, by reading it, if process_template_response
calls are all made before process_response
calls or instead of them. If the former, than my suggested diagram is no longer correct, and the true picture is even more complex.
comment:16 by , 12 years ago
We have to strike a balance between completeness and clarity. This graph is displayed at the beginning of the introduction to middleware and should be understandable at first sight. For this reason, I wouldn't attempt to show that each middleware can short-circuit the rest of the chain by returning a response in process_request/view/exception
. It's clearly explained in the text and rather intuitive.
However, there's another aspect that I'd like to represent: process_template_response
and process_response
are applied after process_exception
. The current docs describe process_exception
at the end — they start with the common case; that can be confusing. They repeat that "Response middleware is always called on every response" but the concrete result is difficult to understand (at least for me) without a visual representation.
Finally, the schema should make clear that process_exception
is only called in error cases, and that process_template_response
isn't always called. Other middleware are always called (except in the case of short circuits, but as explained above, I'm leaving that outside of the scope of the schema).
I'm attaching (yet another) SVG file that implements these ideas and tries not to be too ugly.
by , 12 years ago
Attachment: | middleware.svg added |
---|
by , 12 years ago
Attachment: | middleware-2.svg added |
---|
comment:17 by , 12 years ago
After getting some feedback on IRC, I'm going to commit the second version.
I'll include the OmniGraffle source file in the commit, even though it's in a proprietary format, because we often waste time looking for sources.
follow-up: 20 comment:18 by , 12 years ago
I agree that the second version is better, but I still think it doesn't achieve the goals you set for it in comment:16. In particular, it doesn't clarify the relationship between the three middleware functions on the way out.
Here's a quick ascii-art sketch of what I think the diagram should look like. Every block should be expanded to "middleware1, middleware2, middleware3" in the correct order as in my previous suggestion.
Question: is it -- as implied by the suggested diagrams -- that an exception middleware can return a template response to be processed by the template-response middleware? The below represents my guess that this is not the case.
# | # V # [ process_request ] # | # V # [ process_view ] # | # V # [ USER VIEW ] # / | \ # [process_exception] | [process_template_response] # \ V / # [ process_response ] # | # V
(Of course, the arrows to process_exception and process_template_response should be formed and colored to mark their optionality etc).
comment:19 by , 12 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:20 by , 12 years ago
Replying to shai:
I agree that the second version is better, but I still think it doesn't achieve the goals you set for it in comment:16.
Sorry, I committed before seeing your comment. I wanted to get it done tonight rather than let it linger for another three years :)
Question: is it -- as implied by the suggested diagrams -- that an exception middleware can return a template response to be processed by the template-response middleware?
Yes, this is possible (look at django.core.handlers.base
).
There are four possibilities for the response phase:
- exception => template_response => response
- exception => response
- template_response => response
- response
The dashes on the diagram mean that exception and template_response are optional, so these four possibilities are represented.
comment:21 by , 12 years ago
Oh, Ok, then. I stand corrected. Thanks for finally taking care of this.
Design with squares instead of circles