[
https://issues.apache.org/jira/browse/COCOON-2038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12511680
]
Rice Yeh commented on COCOON-2038:
----------------------------------
>Grzegorz Kossakowski updated COCOON-2038:
>-----------------------------------------
>
>
>(In reply to comment #6)
>> Hi,
>> I am sorry for delaying so long to reply your messages because I has been too
>> busy for the past 2 weeks.
>
>No problem.
>
>> The following is the description of my implementation:
>>
>> I use the same reason as described by Daniel above to implement this
>> function.
>> However, in addition to
>> the returned status code, I also use ServletException to tell that a function
>> might be implemented in a super
>> servlet service. Lets say servlet service s1 extends servlet service s0. A
>> request r asking for functin f is handled
>> by s1 unsuccessfully because either an instance of ServletException is
>> thrown or
>> the returned status code
>> is not between 200 to 399, r is then handled by s0. The code is implemented
>> in
>> method 'forward' method of the
>> inner class PathDispatcher of ServletServiceContext.
>
>I think I have found a bug in your implementation in forward method. The
>problem is that you delegated handling of all super-related stuff to
>getNamedContext() method. This solution is quite reasonable because it's
>better to have one place that will handle super calls but then superCall in
>NamedDispatcher will have wrong value set. Basically, it will make
>NamedDispatcher unaware if it's super call or not.
I do not think this is a bug. see discussion below.
>
>Do you have any proposals how to fix it in elegant way?
>
>> However, since there is not a method getStatus() in HttpServletResponse, I
>> define a new interface called
>> StatusRetrievableResponse which just contains a method getStatus(). Then I
>> change the method 'service'
>> in DispatcherServlet to wrapp the passed-in repsonse 'rep' to make the
>> response
>> implement StatusRetrievableResponse
>> in order to let getting status possible in the method 'forward' in
>> PathDispatcher.
>
>I think that introduction of that interface is over-engineering. Where else
>can it be used? What other implementations we are likely to see?
>As for now I'm against introducing it; we can always refactor the code if we
>need to extract any interface.
>
>> On the implementation, I find 2 bugs in ServletServiceContext. The first one
>> is in the method
>> getNamedContext in ServletServiceContext. It does not look up super servlet
>> for
>> a named connection.
>
>I'm not sure if it's a bug really, see below.
I think this is a bug. getNamedContext() is not only called by NamedDispatcher
but also by method absolutizeURI(URI).
If does not look up servlets connected through super servlet,
absolutizeURI(URI) will throw URISyntaxException, which is not acceptable.
See also dicussion below.
>
>> The second one is in the constructor of the inner class NamedDispatcher of
>> the
>> ServletServiceContext.
>> I have to use an example to tell this bug. Let say the above servlet service
>> s0
>> have a link (but not a super link)
>> to another servlet service s3. When s1 invokes s3, current implementation
>> treats
>> this invocation as super invocation. I think
>> this is not right. So I remove some lines of code which doing the supperCall
>> setting in the NamedDispatcher constructor.
>
>My understanding of superCall field is that it should have "true" value if it
>make a call to super servlet or to servlet connected by super servlet. If my
>understanding is correct then previous implementation was proper.
No, I do not think it is necessary to set the superCall to true when making a
call to a serlvet connected by super servlet. To clarify this problem,
we first have to think what the superCall is used for. The superCall reflects
on the super attribute for a call frame in the call stack. The super attribute
is used in CallStackHelper.getBaseServletContext(), which returns the base
context. In terms of OO, the base context is the "this" context or "me". A
place to
call getBAseServletContext is ServletConnection where it finds the would-be
"this context" for the entering servlet.
Lets say servlet s1 extends s0 and s0 connect to s2. When s1 call s0, the "this
context" in s0 is s1 (not s0) which is decided by getBaseServletContext() by
the logic that it finds if the super attribute is true (s0.super is true),
then looks down one frame to return s1 as "this context" (s0.super is false).
When s1 calls s2, the "this context" in s2 is
s2, so the super attribute should be false, otherwise, s1 will be returned. I
have a digram below to illustare my thinking.
s2 (this context is s2. If s2.super is true, then looks down one frame to
s0, which is also true, looks down one frame further, s1 is returned)
---
s0* (this context is s1)
---
s1 (this context is s1)
(* means super is true)
>
>> I attach one file patch this time.
>
>This helped me a lot, thanks!
>
>> Implement true Object Oriented approach for handling super calls
>> ----------------------------------------------------------------
>>
>> Key: COCOON-2038
>> URL: https://issues.apache.org/jira/browse/COCOON-2038
>> Project: Cocoon
>> Issue Type: Task
>> Components: - Servlet service framework
>> Reporter: Grzegorz Kossakowski
>> Assignee: Grzegorz Kossakowski
>> Fix For: 2.2-dev (Current SVN)
>>
>> Attachments: BlockCallHttpServletResponse.java.patch,
>> cocoon-servlet-service-impl.patch, DispatcherServlet.java.patch,
>> ServletServiceContext.java.patch, StatusRetrievableResponse.java
>>
>>
>> As discussed here: http://thread.gmane.org/gmane.text.xml.cocoon.devel/72317
>> implementation of super calls should be changed to make it more OO-like.
>
>--
>This message is automatically generated by JIRA.
>-
>You can reply to this email to add a comment to the issue online.
> Implement true Object Oriented approach for handling super calls
> ----------------------------------------------------------------
>
> Key: COCOON-2038
> URL: https://issues.apache.org/jira/browse/COCOON-2038
> Project: Cocoon
> Issue Type: Task
> Components: - Servlet service framework
> Reporter: Grzegorz Kossakowski
> Assignee: Grzegorz Kossakowski
> Fix For: 2.2-dev (Current SVN)
>
> Attachments: BlockCallHttpServletResponse.java.patch,
> cocoon-servlet-service-impl.patch, DispatcherServlet.java.patch,
> ServletServiceContext.java.patch, StatusRetrievableResponse.java
>
>
> As discussed here: http://thread.gmane.org/gmane.text.xml.cocoon.devel/72317
> implementation of super calls should be changed to make it more OO-like.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
