Opened 14 years ago

Closed 11 years ago

Last modified 11 years ago

#12502 closed Bug (fixed)

Diagram doesn't match text in Middleware documentation

Reported by: Chris Petrilli Owned by: Aymeric Augustin <aymeric.augustin@…>
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)

middleware_square.png (25.7 KB ) - added by Chris Petrilli 14 years ago.
Design with squares instead of circles
middleware_arrow.png (31.4 KB ) - added by Chris Petrilli 14 years ago.
Request Response Middleware.graffle (73.0 KB ) - added by Chris Petrilli 14 years ago.
12502_image_patch.diff (87.3 KB ) - added by Gabriel Hurley 14 years ago.
Patch updates the middleware.png to use the middleware_arrow.png provided on this ticket.
Middleware.process_request.png (33.7 KB ) - added by SafPlusPlus 14 years ago.
Diagram of middleware request processing
Middleware.process_view.png (51.1 KB ) - added by SafPlusPlus 14 years ago.
Diagram of middleware view processing
Middleware.process_exception.png (41.9 KB ) - added by SafPlusPlus 14 years ago.
Diagram of middleware response processing
Middleware.process_response.png (30.0 KB ) - added by SafPlusPlus 14 years ago.
Diagram of middleware response processing
Middleware.zip (83.8 KB ) - added by SafPlusPlus 14 years ago.
Extension of petrilli's OnmiGraffle file
django-middleware-processing.svg (31.6 KB ) - added by Shai Berger 14 years ago.
SVG file -- diagram depicting middleware processing
django-middleware-processing-bg.png (62.3 KB ) - added by Shai Berger 14 years ago.
PNG file -- diagram depicting middleware processing
middleware.svg (11.1 KB ) - added by Aymeric Augustin 11 years ago.
middleware-2.svg (10.4 KB ) - added by Aymeric Augustin 11 years ago.

Download all attachments as: .zip

Change History (34)

by Chris Petrilli, 14 years ago

Attachment: middleware_square.png added

Design with squares instead of circles

comment:1 by Chris Petrilli, 14 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 Tim Graham, 14 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 Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

by Chris Petrilli, 14 years ago

Attachment: middleware_arrow.png added

by Chris Petrilli, 14 years ago

comment:4 by Chris Petrilli, 14 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 Gabriel Hurley, 14 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 Gabriel Hurley, 14 years ago

Has patch: set
Owner: changed from nobody to Gabriel Hurley
Status: newassigned

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 Shai Berger, 14 years ago

Cc: Shai Berger 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:

  1. There are actually two passes "going in", one using process_request and one using process_view
  2. The "go in, then out" pictures create the impression that process_response will only be called for a given middleware class if the corresponding process_request was called; the text makes it clear that this isn't the case.
  3. 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 SafPlusPlus, 14 years ago

Diagram of middleware request processing

by SafPlusPlus, 14 years ago

Attachment: Middleware.process_view.png added

Diagram of middleware view processing

by SafPlusPlus, 14 years ago

Diagram of middleware response processing

by SafPlusPlus, 14 years ago

Diagram of middleware response processing

by SafPlusPlus, 14 years ago

Attachment: Middleware.zip added

Extension of petrilli's OnmiGraffle file

comment:7 by SafPlusPlus, 14 years ago

Cc: safplusplus 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 Gabriel Hurley, 14 years ago

Owner: Gabriel Hurley removed
Status: assignednew

I'm gonna unassign this to let somebody else make the final decision during the sprint since I'm mostly not available right now.

comment:9 by Russell Keith-Magee, 14 years ago

milestone: 1.2

Deferring due to the absence of a trunk-ready patch.

by Shai Berger, 14 years ago

SVG file -- diagram depicting middleware processing

by Shai Berger, 14 years ago

PNG file -- diagram depicting middleware processing

comment:10 by Shai Berger, 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.

PNG file -- diagram depicting middleware processing

Shai.

comment:11 by Gabriel Hurley, 13 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 Matt McClanahan, 13 years ago

Severity: Normal
Type: Bug

comment:13 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:14 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:15 by Shai Berger, 11 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 Aymeric Augustin, 11 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.

I'm attaching (yet another) SVG file that implements these ideas and tries not to be too ugly.

Version 0, edited 11 years ago by Aymeric Augustin (next)

by Aymeric Augustin, 11 years ago

Attachment: middleware.svg added

by Aymeric Augustin, 11 years ago

Attachment: middleware-2.svg added

comment:17 by Aymeric Augustin, 11 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.

comment:18 by Shai Berger, 11 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 Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Owner: set to Aymeric Augustin <aymeric.augustin@…>
Resolution: fixed
Status: newclosed

In ae8e97384bbacaf4094b5f98c83b8599478b3fac:

Fixed #12502 -- Improved middleware diagram.

in reply to:  18 comment:20 by Aymeric Augustin, 11 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 Shai Berger, 11 years ago

Oh, Ok, then. I stand corrected. Thanks for finally taking care of this.

Note: See TracTickets for help on using tickets.
Back to Top