Bug 57282 - request process UML diagram seems outdated
request process UML diagram seems outdated
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Documentation
7.0.56
PC All
: P2 enhancement (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-11-29 03:19 UTC by Stephen Chen
Modified: 2015-06-25 08:03 UTC (History)
0 users



Attachments
Page 2 step (54.36 KB, image/png)
2014-11-29 03:20 UTC, Stephen Chen
Details
Page 1 step (71.23 KB, image/png)
2014-11-29 03:21 UTC, Stephen Chen
Details
Tomcat 7.0 request process flow (96.77 KB, image/png)
2014-12-24 11:37 UTC, Stephen Chen
Details
request process(revised) (106.43 KB, image/png)
2015-01-06 11:40 UTC, Stephen Chen
Details
authenticate and realm (23.05 KB, image/png)
2015-01-06 11:40 UTC, Stephen Chen
Details
Revised(V3) request process according to Mark's comment (112.11 KB, image/png)
2015-01-16 00:11 UTC, Stephen Chen
Details
Revised(V3) authenticate process according to Mark's comment (26.58 KB, image/png)
2015-01-16 00:12 UTC, Stephen Chen
Details
add missed methods (51.57 KB, image/png)
2015-01-31 11:20 UTC, Stephen Chen
Details
add missed methods (14.33 KB, image/png)
2015-01-31 11:20 UTC, Stephen Chen
Details
request process V5 (106.91 KB, image/png)
2015-05-23 12:04 UTC, Stephen Chen
Details
authenticate v5 (41.68 KB, image/png)
2015-05-23 12:05 UTC, Stephen Chen
Details
request process pdf version (181.50 KB, application/pdf)
2015-06-12 14:40 UTC, Stephen Chen
Details
Authenticate pdf (175.53 KB, application/pdf)
2015-06-12 14:52 UTC, Stephen Chen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen Chen 2014-11-29 03:19:59 UTC
apache-tomcat-7.0.56/webapps/docs/architecture/requestProcess/requestProcess.pdf


it is about the request process UML diagram, I have been reading tomcat 7.0.56 source code for weeks, and along with source code reading along with debug which helps so much, and one day after I debgued the whole end-to-end request flow, I found that the document seems is derived from Tomcat 5(please check the footer of the pdf).

and there are few steps on the diagram which are different from my finding/debuging.

Please kindly check attachments, thanks!
Comment 1 Stephen Chen 2014-11-29 03:20:36 UTC
Created attachment 32236 [details]
Page 2 step
Comment 2 Stephen Chen 2014-11-29 03:21:22 UTC
Created attachment 32237 [details]
Page 1 step
Comment 3 Mark Thomas 2014-12-03 22:24:20 UTC
Yes, those diagrams are out of date.

The problem is bigger than you have identified. Page 2 is completely wrong. The call sequence is very different.
Comment 4 Stephen Chen 2014-12-04 06:47:59 UTC
(In reply to Mark Thomas from comment #3)
> Yes, those diagrams are out of date.
> 
> The problem is bigger than you have identified. Page 2 is completely wrong.
> The call sequence is very different.

Hi Mark,
community would fix that in the future? or what can I help ?
Comment 5 Mark Thomas 2014-12-04 12:57:32 UTC
You are part of community too. No-one else has stepped up to do this so if you want to fix these diagrams go for it.
Comment 6 Mark Thomas 2014-12-09 21:40:31 UTC
This isn't going to stop a release so mark as an enhancement.
Comment 7 Stephen Chen 2014-12-24 11:37:52 UTC
Created attachment 32329 [details]
Tomcat 7.0 request process flow

Hi Mark, please check the drafted request reprocess diagram, since the exported UML pdf is not that readable, please check whether the diagram meets or not first, thanks!
Comment 8 Mark Thomas 2014-12-31 18:20:26 UTC
The calls prior to CoyoteAdapter don't look right. Specifically, there is no Http11Processor and no SocketProcessor. After that things look OK. I do wonder if it would be more useful with an Authenticator + Realm configured. Maybe that should/could be in a separate diagram.
Comment 9 Stephen Chen 2015-01-02 07:11:04 UTC
(In reply to Mark Thomas from comment #8)
> The calls prior to CoyoteAdapter don't look right. Specifically, there is no
> Http11Processor and no SocketProcessor. After that things look OK. I do
> wonder if it would be more useful with an Authenticator + Realm configured.
> Maybe that should/could be in a separate diagram.

Mark, I was thinking about Http11Protocol represents both SockeProcessor(before Http11Protocol) and Http11Processor(after Http11Protocol) which is not right, the call hierarchy is very clear(at in the source code) and it should be reflected in the diagram accordingly.

thanks for your  comments, would supplement missed part, and as your said authenticator + Realm diagram would be very useful, would supplement accordingly too.

Regards
Comment 10 Stephen Chen 2015-01-06 11:40:09 UTC
Created attachment 32344 [details]
request process(revised)
Comment 11 Stephen Chen 2015-01-06 11:40:37 UTC
Created attachment 32345 [details]
authenticate and realm
Comment 12 Stephen Chen 2015-01-06 11:42:01 UTC
Hi Mark, please help to review the revised&advise request process and Authenticator&Realm diagrams, thanks!
Comment 13 Mark Thomas 2015-01-12 10:35:18 UTC
Broadly it looks OK but the detail around the Pipelines isn't right. Calls to getFirst() are missing and some method names shown don't exist. Also methods are being called on the wrong objects (e.g. getHost() on the StandardHost). 

Similar comments apply to the Authentication diagram. For example getContext() is shown as being called on StandardContext.

I'd consider ignoring the various getHost(), getPipeline() calls and just show the calls that do the real work.
Comment 14 Stephen Chen 2015-01-16 00:11:36 UTC
Created attachment 32368 [details]
Revised(V3) request process according to Mark's comment
Comment 15 Stephen Chen 2015-01-16 00:12:16 UTC
Created attachment 32369 [details]
Revised(V3) authenticate process according to Mark's comment
Comment 16 Stephen Chen 2015-01-16 00:18:19 UTC
(In reply to Mark Thomas from comment #13)
> Broadly it looks OK but the detail around the Pipelines isn't right. Calls
> to getFirst() are missing and some method names shown don't exist. Also
> methods are being called on the wrong objects (e.g. getHost() on the
> StandardHost). 
> 
> Similar comments apply to the Authentication diagram. For example
> getContext() is shown as being called on StandardContext.
> 
> I'd consider ignoring the various getHost(), getPipeline() calls and just
> show the calls that do the real work.

Mark, thanks for your valuable comment as a reader.
That diagram wasn't that strict, since in my mind, I wanted to express the process in a verbose&abstract way which is confusing people.
I have removed those unnecessary methods(getHost, getContext...) from previous diagrams, please check the latest ones and give comments, would be delighted to help.
Comment 17 Mark Thomas 2015-01-20 10:12:57 UTC
Looking better. Some of the method calls are missing method names which makes it hard to review and less useful to the reader.
Comment 18 Stephen Chen 2015-01-31 11:20:05 UTC
Created attachment 32417 [details]
add missed methods

Request process V4
Comment 19 Stephen Chen 2015-01-31 11:20:49 UTC
Created attachment 32418 [details]
add missed methods

authentication diagram V4
Comment 20 Stephen Chen 2015-02-15 12:00:50 UTC
Mark, any update ?
Comment 21 Mark Thomas 2015-05-22 11:47:22 UTC
The request processing diagram looks really good. There is one issue that needs fixing before we can use it. At the end of the chain Servlet.service() is called from ApplicationFilterChain.doFilter(), not ApplicationFilterChain.doRelease(). Get that fixed and I think it will be ready to commit.
Comment 22 Mark Thomas 2015-05-22 11:55:11 UTC
The authentication diagram also looks good. I do have a few more comments on this.

registerAuthSuccess() is specific to the LockOutRealm. It would be worth showing this Realm and the 'real' Realm (e.g. UserDataBaseRealm) on the diagram.

For BASIC auth I'd add the call to obtain the headers from the request since that is where the user name and password are called from.
Comment 23 Stephen Chen 2015-05-23 12:04:32 UTC
Created attachment 32751 [details]
request process V5
Comment 24 Stephen Chen 2015-05-23 12:05:01 UTC
Created attachment 32752 [details]
authenticate v5
Comment 25 Stephen Chen 2015-05-23 12:09:30 UTC
Hello Mark,

Good to hear from you again, for the request process diagram, that was a typo, andfixed now.

for the authenticate diagram, revised with you nice suggestions, please have a look and review, thanks.
Comment 26 Mark Thomas 2015-05-26 21:17:34 UTC
These have been added to the documentation for trunk (9.0.x), 8.0.x (for 8.0.24 onwards) and 7.0.x (for 7.0.63 onwards).

Many thanks for all your hard work.
Comment 27 Stephen Chen 2015-05-26 23:31:36 UTC
(In reply to Mark Thomas from comment #26)
> These have been added to the documentation for trunk (9.0.x), 8.0.x (for
> 8.0.24 onwards) and 7.0.x (for 7.0.63 onwards).
> 
> Many thanks for all your hard work.

Yeah, I am so honored&thrilled what I have done. 
Of course, many thanks for your great help, (In reply to Mark Thomas from comment #26)
> These have been added to the documentation for trunk (9.0.x), 8.0.x (for
> 8.0.24 onwards) and 7.0.x (for 7.0.63 onwards).
> 
> Many thanks for all your hard work.

Yeah!!! I am so thrilled&honored about what I have done for community.
of course, many thanks for your great help(valuable reviews and comments), it can't be done without your help.
Comment 28 Stephen Chen 2015-06-12 14:40:00 UTC
Created attachment 32813 [details]
request process pdf version
Comment 29 Stephen Chen 2015-06-12 14:52:32 UTC
Created attachment 32814 [details]
Authenticate pdf
Comment 30 Mark Thomas 2015-06-25 08:03:02 UTC
I prefer the old versions.