dev@glassfish.java.net

RE: [Review request] About GLASSFISH-20696

From: lvsongping <lvsongping_at_cn.fujitsu.com>
Date: Fri, 20 Sep 2013 17:54:14 +0800

Hi, Bill, Byron:
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:

-----Original Message-----
From: Bill Shannon [mailto:bill.shannon_at_oracle.com]
Sent: Thursday, September 19, 2013 3:55 AM
To: dev_at_glassfish.java.net
Cc: lvsongping; byron.nevins_at_oracle.com
Subject: Re: [Review request] About GLASSFISH-20696

+ ConfigBeanProxy proxy = (ConfigBeanProxy)(event.getSource());
+ Config config;
+ if(proxy instanceof MonitoringService) {
+ config = (Config)proxy.getParent();
+ } else {
+ config = (Config)proxy.getParent().getParent();
+ }
+
+ String name = config.getName();
+ // We think that DAS's config is always server-config.
+ if(name.equalsIgnoreCase("server-config")) {

>Is is true that the DAS's config is required to be named "server-config"?
>Does other code depend on that?
    The default config name will be "server-config" when you create the DAS,
here's some detailed information below.

>Are config names case-independent? I wouldn't think so.
    You are right, we can create a DAS but the config name is not
"server-config", this is why I add a annotation here as " We regard that
DAS's config is always server-config.", we do this changes because we regard
the user will use the server-config to create the DAS rather than create
another config name based on the server-config. For example: if we want to
create a DAS to another config, we can follow the following steps:
1). asadmin copy-config server-config new-config
2). Then we can change the host port about the new-config
3). Create a standalone instance specified the --config option as new-config
and we can regard this instance as another DAS.

As to this situation the DAS's config name is "new-config" rather than
"server-config", The main reason why I attached the patch before to regard
the DAS's config name as "server-config" because I think this situation will
be happened quite a little so that we can ignore this situation in my
option.

If you need to take this situation into consideration, I will come up with
another patch as soon as possible to take all of the situations into
consideration.


>Why is the MonitoringService special? Shouldn't it just go up the parent
chain until it gets to something that's "instanceof Config"?
    Both of the revision as I have attached before and you have presented
can be used to fix this issue. The reason why I use the MonitoringService is
because the issue we need to fix is based on the monitoring. So I used the
MonitoringService instead of "instanceof Config". I have replaced the
related code as follows and tested the function could run successfully:

+ ConfigBeanProxy proxy = (ConfigBeanProxy)(event.getSource());
+ while(!(proxy instanceof Config)){
+ proxy = proxy.getParent();
+ }
+ Config config = (Config)proxy;


Thanks a lot!


Regards
Jeremy Lv

lvsongping wrote on 09/18/13 00:39:
> Hi, Byron:
>
> Cc: dev team:
>
>
>
> I have fixed the issue about the GLASFISH-20696which have been
> reviewed by my team and also ran the QL tests and all of the tests are
> passed after I have applied the revision of GLASFISH-20696.
>
>
> Please review my changes and give feedback. If I don't hear from you
> in 3weeks, 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 <mailto:lvsongping_at_cn.fujitsu.com>
>
>
>