I shall review this code and send the comments
off-list. Once the review is complete, please send
an e-mail to the list indicating the same.
Thanks,
Kedar
(Others, Please see:
http://wiki.java.net/bin/view/Projects/AdminDeveloperResources#CodeReviewProcess)
Jane Young-Lau wrote:
> 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.
>
>