Hi Bongjae,
thanks a lot!!!
WBR,
Alexey.
On 04/21/2011 02:52 PM, Bongjae Chang wrote:
> Hi Alexey,
>
> I like your changes which simplified the mapper logic of my patch for
> NamedDispatcher!
>
> At first, I thought that same servlet names could exist in according
> to different host and context path. But HttpHandler has only supported
> the "localhost" currently. So I understood that HttpHandlerChain could
> have the map which had servlet name key in your changes.
>
> I fixed trivial mistakes in your changes and I committed all patch on
> trunk and I changed Grizzly-242 issue's status.
>
> Thanks!
>
> Regards,
> Bongjae Chang
>
> From: Oleksiy Stashok <oleksiy.stashok_at_oracle.com
> <mailto:oleksiy.stashok_at_oracle.com>>
> Reply-To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
> Date: Wed, 20 Apr 2011 16:22:36 +0200
> To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
> Subject: Re: About "(GRIZZLY-242) Add support for
> Request/NamedDispatcher"issue
>
> Hi Bongjae,
>
> thanks again for your work!
> I've created a git branch "request-dispatcher", where applied your
> patch together with my updates.
> Can you pls. check them? You have commit rights, don't you?
>
> If you think changes on "request-dispatcher" branch are fine - pls.
> feel free to commit them on trunk.
>
> Thanks.
>
> WBR,
> Alexey.
>
> On 04/15/2011 02:43 PM, Bongjae Chang wrote:
>> Hi Alexey,
>>
>> About 1),
>> I agree with you.
>> I tried to make the MapperResolver interface and an empty
>> setMapperResolver() method in HttpHandler.java. So HttpHandler
>> doesn't have the Mapper object directly.
>>
>> About 2),
>> I also agree with you.
>> It was so difficult for me to understand the Mapper class because it
>> is very complex as you said.
>> As your advice, I think that I tried to avoid changing Mapper as
>> possible as I can.
>> I tried to change only methods' accessibility and methods' params
>> which have no effect on original Mapper logic because redundant
>> params are meaningless in Mapper.java.
>> But for reusing some Mapper's code and supporting for
>> NamedDispatcher, I defined new NamedMapper class which extends Mapper
>> class and implements MapperResolver. So if NamedDispatcher won't be
>> used, I think original Mapper logic will be kept.
>>
>> I attached new patch. But I am not sure that you will like new patch
>> and I am understanding your words correctly.
>>
>> Please review this patch and feel free to advise again. :-)
>>
>> Thanks!
>>
>> Regards,
>> Bongjae Chang
>>
>>
>> From: Oleksiy Stashok <oleksiy.stashok_at_oracle.com
>> <mailto:oleksiy.stashok_at_oracle.com>>
>> Reply-To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
>> Date: Wed, 13 Apr 2011 13:54:57 +0200
>> To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
>> Subject: Re: About "(GRIZZLY-242) Add support for
>> Request/NamedDispatcher"issue
>>
>> Hi Bongjae,
>>
>> thanks a lot for the patch!
>>
>> I have 2 comments:
>>
>> 1) IMO it would be good to avoid setting Mapper object per
>> HttpHandler directly.
>> I think I understand why you did that, but can we set some kind of
>> MapperResolver instead, which will be responsible for delegating
>> Mapper related operation for HttpHandler?
>> This way it might be easier to switch/update Mappers for group of
>> HttpHandlers.
>>
>> 2) Updates inside Mapper class.
>> It could be great if we can avoid changing existing code in the
>> Mapper, we can extend it for sure, but try to not change existing
>> methods. This class is very complex and difficult to support...
>> I noticed you've added one HashMap with servletName->Wrapper
>> dependencies. Didn't have time to investigate it, but it looks like
>> something we can try to improve.
>> Otherwise I'll definitely find some time today and look at the patch
>> more carefully, meanwhile if you have any suggestions, improvements
>> regarding two points above - please let me know.
>>
>> Appreciate your help!
>>
>> Thanks.
>>
>> WBR,
>> Alexey.
>>
>> On 04/13/2011 12:29 PM, Bongjae Chang wrote:
>>> Hi Alexey,
>>>
>>> I attached the proposed patch included test cases.
>>>
>>> I also referred to Glassfish v3's servlet parts.
>>>
>>> I think that this patch may be not the best solution and doesn't
>>> support full spec, but I hope it will be able to be helpful.
>>>
>>> Please review it and let me know if you have any questions.
>>>
>>> Thanks.
>>>
>>> Regards,
>>> Bongjae Chang
>>>
>>> From: Oleksiy Stashok <oleksiy.stashok_at_oracle.com
>>> <mailto:oleksiy.stashok_at_oracle.com>>
>>> Reply-To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
>>> Date: Tue, 12 Apr 2011 10:02:09 +0200
>>> To: <dev_at_grizzly.java.net <mailto:dev_at_grizzly.java.net>>
>>> Subject: Re: About "(GRIZZLY-242) Add support for
>>> Request/NamedDispatcher"issue
>>>
>>> Hi Bongjae,
>>>
>>>> I could see Alexey's comment from
>>>> http://java.net/jira/browse/GRIZZLY-242 and know the
>>>> issue(GRIZZLY-242) has not been resolved for long time.
>>>>
>>>> When I reviewed http-server and http-servlet modules in Grizzly2.0,
>>>> I thought I could support it though it seemed to be more difficult
>>>> than I thought.
>>>>
>>>> So, could I try to implement the feature in Grizzly 2.0? :-)
>>>>
>>>> If anybody has not been following the issue yet, I will attach
>>>> patches after I will make some unit tests for
>>>> Request/NamedDispatcher as well as implement it in Grizzly 2.0
>>>> within a week.
>>> It would be great!
>>> Frankly I didn't have time to think about details of possible
>>> solution, but have a feeling part of it should be applied on
>>> http-server side as well, not just servlet.
>>> Looking forward to see your patch!
>>>
>>> Thanks.
>>>
>>> WBR,
>>> Alexey.
>>>
>>>>
>>>> Thanks.
>>>>
>>>> Regards,
>>>> Bongjae Chang
>>>
>>
>