cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rice Yeh (JIRA)" <j...@apache.org>
Subject [jira] Commented: (COCOON-2038) Implement true Object Oriented approach for handling super calls
Date Wed, 11 Jul 2007 07:46:04 GMT

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


Mime
View raw message