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