RFR: 8294989: ResourceBundle naming convention issue in JdbcRowSetResourceBundle.java

[Naoto Sato]

On Fri, 7 Oct 2022 18:24:02 GMT, Justin Lu <duke at openjdk.org> wrote:

> Issue: Resource bundle name does not follow proper naming conventions according to [getBundle method](https://docs.oracle.com/javase/8/docs/api/java/util/ResourceBundle.html#getBundle-java.lang.String-java.util.Locale-) for base name parameter
> 
> Fix: Modified bundle name to be a fully qualified class and added regression tests.

test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 45:

> 43:     private static final String PATH_TO_BUNDLE = "com/sun/rowset/RowSetResourceBundle";
> 44:     // Default Locale
> 45:     private static final Locale DEFAULT_LOCALE = Locale.getDefault();

Is using the default locale OK? What if the test is run under some locale where JDBC does not provide the localized bundle?

test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 62:

> 60:     private static void testResourceBundleAccess(String bundleName, boolean expectBundle) {
> 61:         try {
> 62:             var bundle = (PropertyResourceBundle) ResourceBundle.getBundle(

Is casting to `PropertyResourceBundle` needed? If you intend to check the bundle type, I'd explicitly check the type.

test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 64:

> 62:             var bundle = (PropertyResourceBundle) ResourceBundle.getBundle(
> 63:                     bundleName, DEFAULT_LOCALE, JdbcRowSetResourceBundle.class.getModule());
> 64:             System.out.printf("$$$ %s found as expected!%n", bundleName);

I think `expectBundle` needs to be examined here. If it is `false`, it should throw an exception.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 42:

> 40:     private static final String invalidState = "Invalid state";
> 41:     // Expected JDBCResourceBundle message, crsreader.connecterr
> 42:     private static final String rsReaderError = "Internal Error in RowSetReader: no connection or command";

Should be tested in English environment, if the test is going to compare the result with English text.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 53:

> 51:         // is found from the resource bundle
> 52:         try {
> 53:             jrs.getMetaData();

The test should fail if the execution of `getMetaData()` succeeds.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 59:

> 57:         // Now tests via CachedRowSet
> 58:         try {
> 59:             crs.execute();

Ditto, as above.

-------------

PR: https://git.openjdk.org/jdk/pull/10612