You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by David Blevins <da...@visi.com> on 2010/06/24 12:15:09 UTC
Callback Overrides
Hi Shawn!
Missed this patch in JIRA. Feel free to post anytime you have a patch for review.
On Jun 13, 2010, at 12:22 AM, Shawn Jiang (JIRA) wrote:
>> callback method search is not considering the overriding rule.
>> --------------------------------------------------------------
>>
>> Key: OPENEJB-1299
>> URL: https://issues.apache.org/jira/browse/OPENEJB-1299
>> Project: OpenEJB
>> Issue Type: Bug
>> Components: container system
>> Affects Versions: 3.1.3
>> Environment: 3.1.3-snapshot , geronimo 2.2.1 jetty release
>> Reporter: Shawn Jiang
>> Attachments: OPENEJB-1299.diff
>>
>>
>> Considering following scenario:
>> Bean is a EJB, BeanSuper is the Bean's super class.
>> BeanSuper {
>> @PostConstruct
>> private void postConstructMethodInBeanSuper() throws RuntimeException {
>> System.out.println( " post construct method in bean super")
>> }
>> }
>> @Stateless(name="Bean")
>> Bean extends BeanSuper {
>> protected void postConstructMethodInBeanSuper() throws RuntimeException {
>> System.out.println( " post construct method in bean")
>> }
>> }
>> When container invoke the @postconstruct callback method. It should call BeanSuper#postConstructMethodInBeanSuper() instead of Bean#postConstructMethodInBeanSuper() because BeanSuper#postConstructMethodInBeanSuper() is a private method that can't be overridden.
Good catch! I went a head and whipped up a little test case for this:
http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
All the existing override logic is done in the InterceptorBindingBuilder class. We could probably improve that logic to be sensitive of the fact that the target method may be a private method, in which case we don't need to apply the override concept.
-David
Re: Callback Overrides
Posted by Jonathan Gallimore <jo...@gmail.com>.
I've committed these, but if there's any problems give me a shout. Thanks
for the patches Shawn!
Jon
On Mon, Jul 5, 2010 at 6:36 PM, Jonathan Gallimore <
jonathan.gallimore@gmail.com> wrote:
> I've tried both of these patches, and they both work for me. I'm happy to
> get these committed unless anyone else has any comments?
>
> Cheers
>
> Jon
>
>
> On Mon, Jul 5, 2010 at 3:27 AM, Shawn Jiang <ge...@gmail.com> wrote:
>
>> Just found a related commit:
>>
>> URL: http://svn.apache.org/viewvc?rev=941800&view=rev
>> Log:
>> OPENEJB-1188 adding LocalBean functionality
>>
>>
>> But there are other problem besides the regression brought by the commit
>> above. I've created another patch to address this issue.
>>
>> Could you please review it ?
>>
>>
>>
>> On Mon, Jun 28, 2010 at 10:56 AM, Shawn Jiang <ge...@gmail.com>
>> wrote:
>>
>> >
>> >
>> > On Sat, Jun 26, 2010 at 2:54 AM, David Blevins <david.blevins@visi.com
>> >wrote:
>> >
>> >>
>> >> On Jun 24, 2010, at 11:15 PM, Shawn Jiang wrote:
>> >>
>> >> > Could yo please review the patch I attached in this JIRA ?
>> >>
>> >> [see below]
>> >>
>> >> > On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <
>> david.blevins@visi.com
>> >> >wrote:
>> >> >>
>> >> >> Good catch! I went a head and whipped up a little test case for
>> this:
>> >> >>
>> >> >>
>> >> >>
>> >>
>> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
>> >> >>
>> >> >> All the existing override logic is done in the
>> >> InterceptorBindingBuilder
>> >> >> class. We could probably improve that logic to be sensitive of the
>> >> fact
>> >> >> that the target method may be a private method, in which case we
>> don't
>> >> need
>> >> >> to apply the override concept.
>> >>
>> >
>> > Yes we might have a regression since G 2.2. was tested. I'll take a
>> look
>> > at this to see what's the reason of the regression before I improve the
>> > logic of the patch.
>> >
>> >
>> >>
>> >> I dug into this a little bit more and looks like we have one failing
>> TCK
>> >> test for the overrides. Seems like we might have a regression since G
>> 2.2
>> >> was tested. Or perhaps there's been a TCK update since 2.2.
>> >>
>> >
>> > As I know, there's no TCK update since 2.2.
>> >
>> >
>> >>
>> >>
>> >> -David
>> >>
>> >>
>> >
>> >
>> > --
>> > Shawn
>> >
>>
>>
>>
>> --
>> Shawn
>>
>
>
Re: Callback Overrides
Posted by Jonathan Gallimore <jo...@gmail.com>.
I've tried both of these patches, and they both work for me. I'm happy to
get these committed unless anyone else has any comments?
Cheers
Jon
On Mon, Jul 5, 2010 at 3:27 AM, Shawn Jiang <ge...@gmail.com> wrote:
> Just found a related commit:
>
> URL: http://svn.apache.org/viewvc?rev=941800&view=rev
> Log:
> OPENEJB-1188 adding LocalBean functionality
>
>
> But there are other problem besides the regression brought by the commit
> above. I've created another patch to address this issue.
>
> Could you please review it ?
>
>
>
> On Mon, Jun 28, 2010 at 10:56 AM, Shawn Jiang <ge...@gmail.com> wrote:
>
> >
> >
> > On Sat, Jun 26, 2010 at 2:54 AM, David Blevins <david.blevins@visi.com
> >wrote:
> >
> >>
> >> On Jun 24, 2010, at 11:15 PM, Shawn Jiang wrote:
> >>
> >> > Could yo please review the patch I attached in this JIRA ?
> >>
> >> [see below]
> >>
> >> > On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <
> david.blevins@visi.com
> >> >wrote:
> >> >>
> >> >> Good catch! I went a head and whipped up a little test case for
> this:
> >> >>
> >> >>
> >> >>
> >>
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
> >> >>
> >> >> All the existing override logic is done in the
> >> InterceptorBindingBuilder
> >> >> class. We could probably improve that logic to be sensitive of the
> >> fact
> >> >> that the target method may be a private method, in which case we
> don't
> >> need
> >> >> to apply the override concept.
> >>
> >
> > Yes we might have a regression since G 2.2. was tested. I'll take a
> look
> > at this to see what's the reason of the regression before I improve the
> > logic of the patch.
> >
> >
> >>
> >> I dug into this a little bit more and looks like we have one failing TCK
> >> test for the overrides. Seems like we might have a regression since G
> 2.2
> >> was tested. Or perhaps there's been a TCK update since 2.2.
> >>
> >
> > As I know, there's no TCK update since 2.2.
> >
> >
> >>
> >>
> >> -David
> >>
> >>
> >
> >
> > --
> > Shawn
> >
>
>
>
> --
> Shawn
>
Re: Callback Overrides
Posted by Shawn Jiang <ge...@gmail.com>.
Just found a related commit:
URL: http://svn.apache.org/viewvc?rev=941800&view=rev
Log:
OPENEJB-1188 adding LocalBean functionality
But there are other problem besides the regression brought by the commit
above. I've created another patch to address this issue.
Could you please review it ?
On Mon, Jun 28, 2010 at 10:56 AM, Shawn Jiang <ge...@gmail.com> wrote:
>
>
> On Sat, Jun 26, 2010 at 2:54 AM, David Blevins <da...@visi.com>wrote:
>
>>
>> On Jun 24, 2010, at 11:15 PM, Shawn Jiang wrote:
>>
>> > Could yo please review the patch I attached in this JIRA ?
>>
>> [see below]
>>
>> > On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <david.blevins@visi.com
>> >wrote:
>> >>
>> >> Good catch! I went a head and whipped up a little test case for this:
>> >>
>> >>
>> >>
>> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
>> >>
>> >> All the existing override logic is done in the
>> InterceptorBindingBuilder
>> >> class. We could probably improve that logic to be sensitive of the
>> fact
>> >> that the target method may be a private method, in which case we don't
>> need
>> >> to apply the override concept.
>>
>
> Yes we might have a regression since G 2.2. was tested. I'll take a look
> at this to see what's the reason of the regression before I improve the
> logic of the patch.
>
>
>>
>> I dug into this a little bit more and looks like we have one failing TCK
>> test for the overrides. Seems like we might have a regression since G 2.2
>> was tested. Or perhaps there's been a TCK update since 2.2.
>>
>
> As I know, there's no TCK update since 2.2.
>
>
>>
>>
>> -David
>>
>>
>
>
> --
> Shawn
>
--
Shawn
Re: Callback Overrides
Posted by Shawn Jiang <ge...@gmail.com>.
On Sat, Jun 26, 2010 at 2:54 AM, David Blevins <da...@visi.com>wrote:
>
> On Jun 24, 2010, at 11:15 PM, Shawn Jiang wrote:
>
> > Could yo please review the patch I attached in this JIRA ?
>
> [see below]
>
> > On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <david.blevins@visi.com
> >wrote:
> >>
> >> Good catch! I went a head and whipped up a little test case for this:
> >>
> >>
> >>
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
> >>
> >> All the existing override logic is done in the InterceptorBindingBuilder
> >> class. We could probably improve that logic to be sensitive of the fact
> >> that the target method may be a private method, in which case we don't
> need
> >> to apply the override concept.
>
Yes we might have a regression since G 2.2. was tested. I'll take a look
at this to see what's the reason of the regression before I improve the
logic of the patch.
>
> I dug into this a little bit more and looks like we have one failing TCK
> test for the overrides. Seems like we might have a regression since G 2.2
> was tested. Or perhaps there's been a TCK update since 2.2.
>
As I know, there's no TCK update since 2.2.
>
>
> -David
>
>
--
Shawn
Re: Callback Overrides
Posted by David Blevins <da...@visi.com>.
On Jun 24, 2010, at 11:15 PM, Shawn Jiang wrote:
> Could yo please review the patch I attached in this JIRA ?
[see below]
> On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <da...@visi.com>wrote:
>>
>> Good catch! I went a head and whipped up a little test case for this:
>>
>>
>> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
>>
>> All the existing override logic is done in the InterceptorBindingBuilder
>> class. We could probably improve that logic to be sensitive of the fact
>> that the target method may be a private method, in which case we don't need
>> to apply the override concept.
I dug into this a little bit more and looks like we have one failing TCK test for the overrides. Seems like we might have a regression since G 2.2 was tested. Or perhaps there's been a TCK update since 2.2.
-David
Re: Callback Overrides
Posted by Shawn Jiang <ge...@gmail.com>.
Could yo please review the patch I attached in this JIRA ?
On Thu, Jun 24, 2010 at 6:15 PM, David Blevins <da...@visi.com>wrote:
> Hi Shawn!
>
> Missed this patch in JIRA. Feel free to post anytime you have a patch for
> review.
>
> On Jun 13, 2010, at 12:22 AM, Shawn Jiang (JIRA) wrote:
>
> >> callback method search is not considering the overriding rule.
> >> --------------------------------------------------------------
> >>
> >> Key: OPENEJB-1299
> >> URL: https://issues.apache.org/jira/browse/OPENEJB-1299
> >> Project: OpenEJB
> >> Issue Type: Bug
> >> Components: container system
> >> Affects Versions: 3.1.3
> >> Environment: 3.1.3-snapshot , geronimo 2.2.1 jetty release
> >> Reporter: Shawn Jiang
> >> Attachments: OPENEJB-1299.diff
> >>
> >>
> >> Considering following scenario:
> >> Bean is a EJB, BeanSuper is the Bean's super class.
> >> BeanSuper {
> >> @PostConstruct
> >> private void postConstructMethodInBeanSuper() throws RuntimeException
> {
> >> System.out.println( " post construct method in bean super")
> >> }
> >> }
> >> @Stateless(name="Bean")
> >> Bean extends BeanSuper {
> >> protected void postConstructMethodInBeanSuper() throws
> RuntimeException {
> >> System.out.println( " post construct method in bean")
> >> }
> >> }
> >> When container invoke the @postconstruct callback method. It should
> call BeanSuper#postConstructMethodInBeanSuper() instead of
> Bean#postConstructMethodInBeanSuper() because
> BeanSuper#postConstructMethodInBeanSuper() is a private method that can't be
> overridden.
>
> Good catch! I went a head and whipped up a little test case for this:
>
>
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/stateful/CallbackOverridesTest.java
>
> All the existing override logic is done in the InterceptorBindingBuilder
> class. We could probably improve that logic to be sensitive of the fact
> that the target method may be a private method, in which case we don't need
> to apply the override concept.
>
>
> -David
>
>
--
Shawn