Comments below.
lvsongping wrote on 09/20/13 03:27:
> -----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.
I'd like to hear from Byron or others who know the code better.
I wonder if there isn't already a method to determine
"is this the DAS's config?"
If there's no such method, and there's already other code that checks
for the name "server-config", I suppose it's fine to do the same here.
>> 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;
That's better.
Can getParent ever return null?
Will getParent always eventually reach a Config?