[modules-dev] Review request: 6560281

[Stanley M. Ho]

Hi Dave,

Looks good. A few comments inline:

- src/share/lib/module/repository.properties

   32 extension.PARENT=BOOTSTRAP
   33 extension.SOURCE=file:${java.home}/lib/module/ext
   34 extension.CLASS=sun.module.repository.LocalRepository
   35
   36 user.PARENT=extension
   37 user.SOURCE=file:${user.home}/.java/module
   38 user.CLASS=sun.module.repository.LocalRepository

The upper case attribute looks a bit funny. Perhaps lower case? CLASS 
should be classname instead - this is the convention was also used in 
module.properties.

- src/share/lib/module/module.properties

   99 #     -Drepository.properties.file=somefile.

The system property (not the property in the module.properties file) 
should be "java.module.repository.properties.file". See 
"java.module.import.override.policy.file" and 
"Djava.module.visibility.policy.file".

- src/share/classes/sun/module/repository/RepositoryConfig.java

Upper case issue described above.

151             File f = new File(replacePropertyNamesWithValues(location));
237             String repoValue = replacePropertyNamesWithValues(
370     private static String replacePropertyNamesWithValues(String s) {

You could simply use sun.security.util.PropertyExpander rather than 
writing your own replacePropertyNamesWithValues(). This is what I used 
for expanding the property names in other files. See 
src/share/classessun/module/config/DefaultPolicy.java

  153                 BufferedInputStream is = new BufferedInputStream(
  154                     new FileInputStream(f));

Should have finally block to close the stream afterwards.

206     public static Repository configRepositories(Properties 
configProps) {

Should be private.

- Stanley