dev@glassfish.java.net

RE: [Review request] About GLASSFISH-19730

From: lvsongping <lvsongping_at_cn.fujitsu.com>
Date: Fri, 20 Sep 2013 16:45:29 +0800

Hi, Marina:

Cc: dev list:

 

    Thanks for your patient comments and suggestions. I'm sorry for replying
the mail later as I'm out of my office during the last day. Here's some of
my comments:

 

From: Marina Vatkina [mailto:marina.vatkina_at_oracle.com]
Sent: Thursday, September 19, 2013 2:15 AM
To: lvsongping
Cc: shing.wai.chan_at_oracle.com; dev_at_glassfish.java.net
Subject: Re: [Review request] About GLASSFISH-19730

 

Hi Jeremy,

If you want to be called around each method invocation, there is already a
special (framework) interceptor available - see SystemInterceptorProxy in
the ejb-container. May be it can be extended instead with some proper
ordering added.
    Thanks for your clarification about this. I have look into the
SystemInterceptorProxy and found it is a fantastic class to call around each
method invocation. But I think the SystemInterceptorProxy is different from
my case, I file the issue of GLASSFISH-19730 is to add some interface so
that any of the developers can extended the ejb container and web container
easily. So please take it into consideration.

I thought you needed a common interface, but the patch contains separate
solutions for 2 containers (i.e. non-EJB or non-Web components won't have
that extra processing).
    Yes, this is used to extend the function about the ejb container and web
container, it will be easier for developers to develop any of the features
related to the ejb container and web container, for example, if we need to
develop some new features about monitoring the application's performance or
the application's availability? It will be easier to reuse the code I have
attached.

If you need to add a public interface to the EJB container to be implemented
outside the EJB container modules, please add it under
ejb-internal-api/src/main/java/org/glassfish/ejb/spi.
    I have moved the interface under
ejb-internal-api/src/main/java/org/glassfish/ejb/spi.

I'm not sure it's a good idea to share a writable EJB descriptor instance
with the outside callers.
    This EJB descriptor contains the detailed information about the related
ejb when we tried to access the ejb application, I think it will be useful
when any other developer tried to implement some features about the ejb
applications.

Please don't call anything pre/postInvoke - there are too many of those
already. Have very specific names (or use interceptors with an @AroundInvoke
annotation).
     I have changes the name form pre/postInvoke to
entering/leavingEjbContainer. If you think it will be better to use the
@AroundInvoke annotation, I will tried to make another patch about this.

 

Please review and consider the changes I have attached again whether it is
fine to check in?

Thanks a lot!

Regards

Jeremy Lv


HTH,
-marina

On 9/18/13 3:45 AM, lvsongping wrote:

Hi, Marina, Shing wai:

Cc: dev team:

 

     I have attached the revision source related to the GLASSFISH-19730 and
also passed all of the QL tests and devtests after I applied the revision of
GLASSFISH-19730.
   

Please review my changes and give feedback. If I don't hear from you in 3
weeks, by 10/7, I will go ahead and commit the changes.

Thanks a lot!

 

Regards

Jeremy Lv

 

 

 

--------------------------------------------------

Lv Songping

Software Division II

    Development Department I

Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)

ADDR.: No.6 Wenzhu Road, Software Avenue,

        Nanjing, 210012, China

TEL : +86+25-86630566-8307

COINS: 7998-8307

FAX : +86+25-83317685

MAIL : lvsongping_at_cn.fujitsu.com