Bug 57282

Created attachment 32236 [details]
Page 2 step

Created attachment 32237 [details]
Page 1 step

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.

(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 ?

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.

This isn't going to stop a release so mark as an enhancement.

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!

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.

(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

Created attachment 32344 [details]
request process(revised)

Created attachment 32345 [details]
authenticate and realm

Hi Mark, please help to review the revised&advise request process and Authenticator&Realm diagrams, thanks!

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.

Created attachment 32368 [details]
Revised(V3) request process according to Mark's comment

Created attachment 32369 [details]
Revised(V3) authenticate process according to Mark's comment

(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.

Looking better. Some of the method calls are missing method names which makes it hard to review and less useful to the reader.

Created attachment 32417 [details]
add missed methods

Request process V4

Created attachment 32418 [details]
add missed methods

authentication diagram V4

Mark, any update ?

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.

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.

Created attachment 32751 [details]
request process V5

Created attachment 32752 [details]
authenticate v5

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.

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.

(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.

Created attachment 32813 [details]
request process pdf version

Created attachment 32814 [details]
Authenticate pdf

I prefer the old versions.