Hi, Jeremy
Thanks for making the changes. Please see my comments below:
1. The modified files are in deployment/admin module and not in
core/kernel.
2. For DeployCommand:
a. As we are already throwing the exception in the if block, we don't
need to put the rest of the code in the else block here.
b. For error message, let's do something else
lifecyclemodule_withsamename_exists=Lifecycle module with same name {0}
already exists, please pick a different name for the application.
3. For CreateLifecycleModuleCommand:
a. For the import statement, just import the new class you are using
(Applications) and not import everything
b. Please don't change the REST endpoints as this is not the deploy
command.
c. For the checks you made in validateTarget, you can just add the check
in the beginning of this method or even outside of the method (you don't
really need to go into each block of this method and do similar
validation as this check is not relevant to target anyways, the name has
to be unique per domain).
Application app = apps.getApplication(name);
if(app != null && !app.isLifecycleModule()){
// throw illegal exception here
}
with error string like this:
application_withsamename_exists=Application with same name {0} already
exists, please pick a different name for the lifecycle module.
- Hong
On 7/23/2012 4:27 AM, Lv Songping wrote:
> Dear Hong Zhang:
>
> I have revised the issue(GLASSFISH-18916) about the " Create a
> LifecycleModule, then deploy a application Module that has the same name
> with the LifecycleModule, NullPointerException happened " under your advices
> and reflected the modified files into
> https://github.com/LvSongping/GLASSFISH-18916
>
> please review it again.
>
>
> --best regards
> --Jeremy Lv
>
> ÒÔÉÏ
> ÂÀËÎƽ
> ----------------------------------------------------
> Lv Songping
> Development Dept.II
> No. 6 Wenzhu Road, Nanjing, 210012, China
> TEL: +86+25-86630566-8196
> COINS: 7998-8196
> FAX: +86+25-83317685
> Mail: lvsongping_at_cn.fujitsu.com
> ----------------------------------------------------
> This communication is for use by the intended recipient(s) only and may
> contain information that is privileged, confidential and exempt from
> disclosure under applicable law. If you are not an intended recipient of
> this communication, you are hereby notified that any dissemination,
> distribution or copying hereof is strictly prohibited. If you have received
> this communication in error, please notify me by reply e-mail, permanently
> delete this communication from your system, and destroy any hard copies you
> may have printed.
>
> -----Original Message-----
> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
> Sent: Thursday, July 19, 2012 10:56 PM
> To: dev_at_glassfish.java.net
> Subject: Re: About GLASSFISH-18916
>
> Hi Jeremy
> Thanks for looking into this issue. I think we need to do some
> thinking first on this issue on what is the desirable behavior here.
> Normally when you redeploy an application, it undeploys the old
> version of the application, and deploys the new version of the
> application. But here the old version of the application and the new
> version of the application are very different, one is a lifecycle module
> and the other is a regular Java EE application, and the code paths for
> deploy and undeploy lifecycle module and Java EE application are quite
> different also. Two options here, one is to delete the lifecycle module
> as part of the undeployment, and then deploy the new application. The
> other option is to disallow this type of redeployment, and print a
> meaningful error message to user.
> I prefer the second option as it is the simpler approach (without
> needing to write a lot of code), and should be sufficient to handle this
> not too common use case. The checks would need to be made from both
> sides for handling the two cases (create a lifecycle module first, then
> deploy an application with same name, or deploy an application first,
> and then create a lifecycle module with same name), so you would need to
> change both CreateLifecycleModuleCommand and DeployCommand. For
> DeployCommand, the check could be made in the handleRedeploy method
> after we get the current app. And in CreateLifecyleModuleCommand, the
> check could be made inside the validateTarget method.
>
> Thanks,
>
> - Hong
>
> On 7/18/2012 10:28 PM, Lv Songping wrote:
>> Dear Hong Zhang
>> Cc:Glassfish deployment team
>>
>> I have revised the issue(GLASSFISH-18916) about the " Create a
>> LifecycleModule, then deploy a application Module that has the same name
>> with the LifecycleModule, NullPointerException happened " and reflected
> the
>> modified files into https://github.com/LvSongping/GLASSFISH-18916 and
>> please review it and give me some advices.
>>
>> The NPE is occurred because of the LifecycleModule don't have the
> attribute
>> named "Location", when execute the " uri = new
>> URI(application.getLocation())" in UndeployCommand.java, it will throw out
>> the NPE.
>>
>> The issue url is as follows:
>> http://java.net/jira/browse/GLASSFISH-18916
>>
>> --Best Regard!
>> --Jeremy Lv
>>
>>