admin@glassfish.java.net

Re: Code Review for Bug#6595559

From: Kedar Mhaswade <Kedar.Mhaswade_at_Sun.COM>
Date: Mon, 01 Oct 2007 22:58:15 -0700

Ana Caballero wrote:
> Please review for Bug#6595559 <http://monaco.sfbay.sun.com/detail.jsp?cr=6595559> (Negative test): get 500 status if server side deploy path is empty and type application name .
>
> A null pointer exception was being thrown if 'server side' directory path was not provided by the user. I added a check to see if
> this path is provided(see diffs below) and if it is empty an error is displayed to the user. I also added some Javascript to enable
> the server side directory components when the error is displayed. By default the 'client' directory path was enabled on page display
> and the 'server side' was disabled.
>
> Thanks,
> Ana
>
>
> Index: FileUploadHandler.java
> ===================================================================
> RCS file:
> /cvs/glassfish/admin-gui/src/java/com/sun/enterprise/tools/admingui/handlers/FileUploadHandler.java,v
> retrieving revision 1.10
> diff -r1.10 FileUploadHandler.java
> 128c128,133
> <
> ---
> >
> > if(origPath == null || origPath == "") {

I don't have enough context, but are you checking if
origPath points to an empty string (object equality)?

I thought that the pattern to be used was:
- origPath.length() == 0, or
- "".equals(origPath) (which should be enough instead of a conditional
operator)

Another suggestion: see if you can reuse StringUtils.ok(String)

[http://fisheye5.cenqua.com/browse/glassfish/appserv-commons/src/java/com/sun/enterprise/util/StringUtils.java?r=1.6]

Regards,
Kedar

> > String mesg =
> GuiUtil.getMessage("msg.deploy.nullArchiveError");
> > GuiUtil.handleError(handlerCtx, mesg);
> > return;
> > }
> 136a142
> >
>
> Index: upload.jsf
> ===================================================================
> RCS file: /cvs/glassfish/admin-gui/src/docroot/applications/upload.jsf,v
> retrieving revision 1.14
> diff -r1.14 upload.jsf
> 57c57
> < }
> ---
> > }
> 63a64
> > <sun:script url="../js/selectElements.js" />
> 66c67,74
> < <sun:body onLoad="javascript:
> disableComponent('form:title:sheet1:section1:prop1:dirPath',
> 'text');setVisible('#{appType}');synchronizeRestartRequired('#{requestScope.restartRequired}',
> '#{sessionScope.restartRequired}')">
> ---
> > <sun:body onLoad="javascript:
> disableComponent('form:title:sheet1:section1:prop1:dirPath',
> 'text');setVisible('#{appType}');synchronizeRestartRequired('#{requestScope.restartRequired}',
> '#{sessionScope.restartRequired}');
> > if(getSelectedValue('uploadRdBtn')=='serverSide'){
> > enableComponent('#{dirPathId}', 'text');
> > enableBtnComponent('#{dirSelectBtnId}');
> > enableBtnComponent('#{filSelectBtnId}');
> > disableComponent('#{fileuploadId}', 'file');
> > }
> > ">
> 93c101
> < </sun:body>
> ---
> > </sun:body>
>