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