admin@glassfish.java.net

code review: admin-cli/framework module

From: Jane Young-Lau <Jane.Young_at_Sun.COM>
Date: Tue, 22 Aug 2006 17:24:17 -0700

Hi,

Please review the changes for the following bug fixes:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6460545
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6459283
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6460119

I'd appreciated if someone can review this today.

Thank you!
Jane

Below is the description of the changes:

*CLIDescriptorsReader.java*
Bug: 6460545
Description: The file separator in jar file is "/". The code was
using "File.separator" when reading the single serialized file from the
jar file. It fails on Windows since "File.separator" returns "\".
Tests: Manually verified that single serialized file is recognized in
Windows environment as well as Solaris. passed QL tests.
Diffs:
Index: CLIDescriptorsReader.java
===================================================================
RCS file:
/cvs/glassfish/admin-cli/framework/src/java/com/sun/enterprise/cli/framework/CLIDescriptorsReader.java,v
retrieving revision 1.4
diff -u -r1.4 CLIDescriptorsReader.java
--- CLIDescriptorsReader.java 31 Jul 2006 23:45:20 -0000 1.4
+++ CLIDescriptorsReader.java 22 Aug 2006 23:03:37 -0000
@@ -497,9 +497,8 @@
         Vector<URL> descriptorFiles = new Vector();
         try
         {
- final String sDescriptorFile = serializeDir +
- File.separator +
-
SERIALIZED_DESCRIPTOR_FILE;
+ final String sDescriptorFile = serializeDir + "/" +
+ SERIALIZED_DESCRIPTOR_FILE;

             Enumeration<URL> urls =
CLIDescriptorsReader.class.getClassLoader().getResources(sDescriptorFile);
             if ((urls == null) || (!urls.hasMoreElements()))



*CLIMain.java*
Bug: 6459283
Description: When validating the command line, the environment
variable does not get validated. This is because of a change in the
code where clp.getOptionsList() (from CommandLineParser.java) does not
return the options list with the environment variables. Therefore, the
environment variables do not get included during command validation. As
a result, required options that are set in the environment will return
an option not found when executing the command.
Tests: Manually tested that the environment variables are validated.
Tested on both WebServer and AppServer products. passed QL tests.
passed CLI Framework unit tests.
Diffs:
Index: CLIMain.java
===================================================================
RCS file:
/cvs/glassfish/admin-cli/framework/src/java/com/sun/enterprise/cli/framework/CLIMain.java,v
retrieving revision 1.4
diff -u -r1.4 CLIMain.java
--- CLIMain.java 2 Feb 2006 00:21:46 -0000 1.4
+++ CLIMain.java 22 Aug 2006 23:03:37 -0000
@@ -137,21 +137,22 @@
 
             checkValidCommand(validCommand, args[0]);
 
- //parse the command line arguments
+ //parse the command line arguments
             final CommandLineParser clp = new CommandLineParser(args,
validCommand);
 
- //validate the command line arguments with the
validCommand object
- final CommandValidator commandValidator = new
CommandValidator();
- commandValidator.validateCommandAndOptions(validCommand,
-
clp.getOptionsList(),
- clp.getOperands());
-
- //creates the command object using command factory
+ //creates the command object using command factory
             final Command command = CommandFactory.createCommand(
                                         validCommand,
clp.getOptionsList(),
                                         clp.getValidEnvironmentOptions(),
                                         clp.getOperands());
- //invoke the command
+
+ //validate the Command with the validCommand object
+ new CommandValidator().validateCommandAndOptions(validCommand,
+
command.getOptions(),
+
command.getOperands());
+
+
+ //invoke the command
             command.runCommand();
             return;
         }

LocalStrings.properties
Bug: 6460119
Description: CLI Framework should be as generic as possible. The
LocalStrings.properties file contains reference to "asadmin" and that
should be removed.
Tests: Verify that the "asadmin" does not get displayed if command
cannot be found.
Diffs:
Index: LocalStrings.properties
===================================================================
RCS file:
/cvs/glassfish/admin-cli/framework/src/java/com/sun/enterprise/cli/framework/LocalStrings.properties,v
retrieving revision 1.2
diff -u -r1.2 LocalStrings.properties
--- LocalStrings.properties 2 Feb 2006 00:21:47 -0000 1.2
+++ LocalStrings.properties 22 Aug 2006 23:03:37 -0000
@@ -1,12 +1,12 @@
 PROMPT=Enter Level:
 Usage=Usage: {0}
-NoUsageText=There is no such command {0}. Use "asadmin help" for a
list of valid commands.
+NoUsageText=There is no such command {0}. Use "help" command for a
list of valid commands.
 OptionDeprecated=Option {0} deprecated.
 OptionDeprecatedUseNew=Option {0} deprecated, use --{1} instead.
 UnableToReadEnv=Unable to read system environment. No system
environment will be used.
 
 #Error Messages
-InvalidCommand=CLI001 Invalid Command, {0}. Use "asadmin help" for a
list of valid commands.
+InvalidCommand=CLI001 Invalid Command, {0}. Use "help" command for a
list of valid commands.
 UnableToCreateCommand=CLI002 Unable to create command object, {0}
 NoDescriptorsDefined=CLI003 No descriptors defined.
 CouldNotLoadDescriptor=CLI004 Could not load the descriptor URLs.