Hi Alexey,
I will check and test them as soon as possible and I will reply again.
Thanks!
Regards,
Bongjae Chang
From: Oleksiy Stashok <oleksiy.stashok_at_oracle.com>
Reply-To: <dev_at_grizzly.java.net>
Date: Wed, 20 Apr 2011 16:22:36 +0200
To: <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>
> Reply-To: <dev_at_grizzly.java.net>
> Date: Wed, 13 Apr 2011 13:54:57 +0200
> To: <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>
>> Reply-To: <dev_at_grizzly.java.net>
>> Date: Tue, 12 Apr 2011 10:02:09 +0200
>> To: <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
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>
>
>
>