Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't find ResourceBundle with JPMS #1420

Closed
tasogare3710 opened this issue Aug 30, 2021 · 31 comments
Closed

Can't find ResourceBundle with JPMS #1420

tasogare3710 opened this issue Aug 30, 2021 · 31 comments
Labels
status: help-wanted 🆘 theme: module An issue or change related to JPMS modules type: doc 📘
Milestone

Comments

@tasogare3710
Copy link

ResourceBundle fails to load if the ResourceBundle is specified using the resourceBundle element of the CommandLine.Command annotation.

info.picocli/CommandLine$Model$Messages#createBundle is calling ResourceBundle#getBundle(String), so it seems to be looking for ResourceBundle from info.picocli module. Is there a way to specify the module in which the ResourceBundle or to use the ResourceBundleProvider?

@remkop remkop added the theme: module An issue or change related to JPMS modules label Aug 31, 2021
@remkop
Copy link
Owner

remkop commented Aug 31, 2021

Thank you for raising this!

Looking at the javadoc for Resource Bundles in Named Modules, and ResourceBundleProvider, no, this is currently not yet supported in picocli as of version 4.6.1.

I am not opposed to adding support for this. One thing to note is that in order to continue to only require Java 5 or higher at runtime, adding support for resource bundles in modules will require the use of reflection. It is likely that only the CommandLine.Model.Messages class needs to be modified, but I have not looked at the details of what the implementation would look like.

A potential issue is that picocli currently only requires Java 8 or higher to build the project.
So how do we test this functionality?

When running in continuous integration environments, picocli is built and all tests are executed on Java versions 8-16.
One idea is to have some tests that are only executed when running on Java 9 or higher.
Also, we could create a small pre-built picocli-based application that is a JPMS module, include the binary JAR of that application to the resources of the picocli project, and reference this application from the test (again, only when running on Java 9 or higher).

Do you think you will be able to provide a pull request for this?

@tasogare3710
Copy link
Author

Do you think you will be able to provide a pull request for this?

I'm so sorry, but I not know picocli inside out to send a pull request.

A potential issue is that picocli currently only requires Java 8 or higher to build the project.
So how do we test this functionality?

I'm not sure if this will work with picocli, but how about using a multi release jar files and including picocli for java8 and picocli for java9 and higher in single jar file?

@tasogare3710
Copy link
Author

tasogare3710 commented Aug 31, 2021

This is the smallest gradle project that can reproduce this issue.

unzip, cd and run:

gradlew run
  1. java with JPMS is required.
  2. This will fail to run.
  3. Uncomment line 17 of app\build.gradle and comment out lines 18 and 19 to successfully run.

@remkop
Copy link
Owner

remkop commented Sep 2, 2021

Thank you for providing the sample project.

I'm so sorry, but I not know picocli inside out to send a pull request.

Understood, no problem.
I am extremely pressed for time and I don't see myself working on this issue in the near future.

Let's leave this ticket open for someone who is willing to work on it.

@tasogare3710
Copy link
Author

Of course. no problem too.

Thank you.

@aalmiray
Copy link
Contributor

Interested in seeing this issue moving forward. How can we keep backward compatibility while supporting Java 9+ and modules?

@remkop remkop added this to the 4.7.2 milestone Feb 12, 2023
@tasogare3710
Copy link
Author

JPMS is not backward compatibility. upgrade path called --illegal-access option was removed in java17, but you can still use multi-release-jar.
And next LTS will be released this September.

https://openjdk.org/jeps/238
https://openjdk.org/jeps/403

@aalmiray
Copy link
Contributor

Might be much to ask to use MR-JARs as most of picocli is implemented as an annotation with embedded types. For this to work the class responsible for loading resources would have to move to a top level class, then a Java9+ compatible implementation may be placed in /META-INF/versions/9.

@tasogare3710
Copy link
Author

Mhh, divide into smaller milestones step by step.

@remkop
Copy link
Owner

remkop commented Feb 14, 2023

I’m assuming that the programmatic API CommandLine.setResourceBundle works correctly with modular applications, is that right?

(only the annotations give errors?)

@aalmiray
Copy link
Contributor

Yes. Using the programmatic API works with modules.

@aalmiray
Copy link
Contributor

FWIW These are the changes I had to make to Jarviz to turn it into a full modular CLI kordamp/jarviz@fd21b1f

@remkop
Copy link
Owner

remkop commented Feb 15, 2023

My initial thoughts on solving this would be:

  • First, we should document that for modular CLI applications, when using picocli 4.7.1 or earlier, there is a problem with the annotations and they need to call the programmatic API CommandLine.setResourceBundle. This needs to be mentioned in the User Manual and in the javadoc for resource-bundle related methods and annotation elements.
  • the fix for this problem with the @Command(resourceBundle="xxx") annotation may be relatively straight-forward: One idea is to use reflection to call the ResourceBundle.getBundle method with the Module of the user object of the command. The "user object" here is the class that was annotated with @Command. We can obtain the user object's Module via Class.getModule by using reflection.

Thoughts?


Note to self: relevant documentation below:

Resource Bundles in Named Modules

When resource bundles are deployed in named modules, the following module-specific requirements and restrictions are applied.

  • Code in a named module that calls getBundle(String, Locale) will locate resource bundles in the caller's module (caller module).
  • If resource bundles are deployed in named modules separate from the caller module, those resource bundles need to be loaded from service providers of ResourceBundleProvider. The caller module must declare "uses" and the service interface name is the concatenation of the package name of the base name, string ".spi.", the simple class name of the base name, and the string "Provider". The bundle provider modules containing resource bundles must declare "provides" with the service interface name and its implementation class name. For example, if the base name is "com.example.app.MyResources", the caller module must declare "uses com.example.app.spi.MyResourcesProvider;" and a module containing resource bundles must declare "provides com.example.app.spi.MyResourcesProvider with com.example.app.internal.MyResourcesProviderImpl;" where com.example.app.internal.MyResourcesProviderImpl is an implementation class of com.example.app.spi.MyResourcesProvider.
  • If you want to use non-standard formats in named modules, such as XML, ResourceBundleProvider needs to be used.
  • The getBundle method with a ClassLoader may not be able to find resource bundles using the given ClassLoader in named modules. The getBundle method with a Module can be used, instead.
  • ResourceBundle.Control is not supported in named modules. If the getBundle method with a ResourceBundle.Control is called in a named module, the method will throw an UnsupportedOperationException. Any service providers of ResourceBundleControlProvider are ignored in named modules.

ResourceBundleProvider Service Providers

The getBundle factory methods load service providers of ResourceBundleProvider, if available, using ServiceLoader. The service type is designated by <package name> + ".spi." + <simple name> + "Provider". For example, if the base name is "com.example.app.MyResources", the service type is com.example.app.spi.MyResourcesProvider.
In named modules, the loaded service providers for the given base name are used to load resource bundles. If no service provider is available, or if none of the service providers returns a resource bundle and the caller module doesn't have its own service provider, the getBundle factory method searches for resource bundles that are local in the caller module and that are visible to the class loader of the caller module. The resource bundle formats for local module searching are "java.class" and "java.properties".

Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
--- a/src/main/java/picocli/CommandLine.java	
+++ b/src/main/java/picocli/CommandLine.java	(date 1676436268171)
@@ -6508,7 +6508,7 @@
              * @see #addSubcommand(String, CommandLine)
              * @since 4.0 */
             public CommandSpec resourceBundleBaseName(String resourceBundleBaseName) {
-                ResourceBundle bundle = empty(resourceBundleBaseName) ? null : ResourceBundle.getBundle(resourceBundleBaseName);
+                ResourceBundle bundle = Messages.createBundle(resourceBundleBaseName, this);
                 setBundle(resourceBundleBaseName, bundle);
                 return this;
             }
@@ -8416,7 +8416,7 @@
                         messages(new Messages(commandSpec, cmd.resourceBundle()));
                     } else {
                         ResourceBundle rb = null; // from annotation processor, we may not be able to load the ResourceBundle
-                        try { rb = ResourceBundle.getBundle(cmd.resourceBundle()); } catch (MissingResourceException ignored) {}
+                        try { rb = Messages.createBundle(cmd.resourceBundle(), commandSpec); } catch (MissingResourceException ignored) {}
                         messages(new Messages(commandSpec, cmd.resourceBundle(), rb));
                     }
                 }
@@ -11606,7 +11606,7 @@
             private final Set<String> keys;
             private Messages parent;
             public Messages(CommandSpec spec, String baseName) {
-                this(spec, baseName, createBundle(baseName));
+                this(spec, baseName, createBundle(baseName, spec));
             }
             public Messages(CommandSpec spec, ResourceBundle rb) {
                 this(spec, extractName(rb), rb);
@@ -11620,10 +11620,9 @@
                     CommandLine.tracer().debug("Created Messages from resourceBundle[base=%s] for command '%s' (%s)", baseName, spec.name(), spec);
                 }
             }
-            private static ResourceBundle createBundle(String baseName) {
-                if (loadBundles) {
-                    return ResourceBundle.getBundle(baseName);
-                } else {
+            private static ResourceBundle createBundle(String baseName, CommandSpec commandSpec) {
+                if (CommandLine.empty(baseName)) { return null; }
+                if (!loadBundles) { // #1876 don't load ResourceBundles in annotation processor
                     return new ResourceBundle() {
                         @Override
                         protected Object handleGetObject(String key) { return null; }
@@ -11631,6 +11630,23 @@
                         public Enumeration<String> getKeys() { return Collections.enumeration(Collections.<String>emptyList()); }
                     };
                 }
+                try {
+                    return ResourceBundle.getBundle(baseName);
+                } catch (MissingResourceException mre) {
+                    tracer().debug("Could not load resource bundle from baseName=%s: %s%n", baseName, mre);
+                    try { // #1420 support java 9+ modular ResourceBundles
+                        Class<?> moduleClass = Class.forName("java.lang.Module");
+                        Method getModule = Class.class.getDeclaredMethod("getModule");
+                        Object moduleInstance = commandSpec.userObject.type == null ? null : getModule.invoke(commandSpec.userObject.type);
+                        tracer().debug("Found module=%s for Command-annotated type: %s.%n", moduleInstance, commandSpec.userObject.type);
+
+                        Method getBundle = ResourceBundle.class.getDeclaredMethod("getBundle", String.class, moduleClass);
+                        return (ResourceBundle) getBundle.invoke(null, baseName, moduleInstance);
+                    } catch (Exception ignored) {
+                        tracer().debug("Could not load resource bundle from other module: %s%n", ignored);
+                        throw mre;
+                    }
+                }
             }
             private static String extractName(ResourceBundle rb) {
                 try { // ResourceBundle.getBaseBundleName was introduced in Java 8
@@ -11664,7 +11680,7 @@
              * classpath and thereby cause failures. This method allows for disabling
              * loading of resource bundles during annotation processing, preventing such
              * errors.
-             * @since 4.7.2-SNAPSHOT
+             * @since 4.7.1
              * @param loadBundles true if bundles should be loaded (default), false if bundles should not be loaded
              */
             public static final void setLoadBundles(boolean loadBundles) {

@aalmiray
Copy link
Contributor

My initial thoughts on solving this would be:

* First, we should document that for modular CLI applications, when using picocli 4.7.1 or earlier, there is a problem with the annotations and they need to call the programmatic API [`CommandLine.setResourceBundle`](https://picocli.info/apidocs-all/info.picocli/picocli/CommandLine.html#setResourceBundle(java.util.ResourceBundle)). This needs to be mentioned in the User Manual and in the javadoc for resource-bundle related methods and annotation elements.

👍

* the fix for this problem with the [`@Command(resourceBundle="xxx")`](https://picocli.info/apidocs-all/info.picocli/picocli/CommandLine.Command.html#resourceBundle()) annotation may be relatively straight-forward: One idea is to use reflection to call  the [`ResourceBundle.getBundle`](https://docs.oracle.com/javase/9/docs/api/java/util/ResourceBundle.html#getBundle-java.lang.String-java.lang.Module-) method with the [`Module`](https://docs.oracle.com/javase/9/docs/api/java/lang/Module.html) of the user object of the command. The "user object" here is the class that was annotated with `@Command`. We can obtain the user object's `Module` via [Class.getModule](https://docs.oracle.com/javase/9/docs/api/java/lang/Class.html#getModule--) by using reflection.

I suppose this will work as long as the module is open to info.picocli otherwise reflection will fail.

@remkop
Copy link
Owner

remkop commented Feb 15, 2023

I suppose this will work as long as the module is open to info.picocli otherwise reflection will fail.

True; that bit is already documented .

@remkop
Copy link
Owner

remkop commented Feb 18, 2023

@aalmiray @tasogare3710 Good news:
it turns out that no changes to the picocli library are necessary; I was able to get it to work with picocli 4.7.1.

What is needed is:

  • the resource bundle properties or class file must exist in the application jar at the correct location
  • the module-info.java must use opens yourpackage; it must not use opens yourpackage to info.picocli;!
module mymodule {
    requires info.picocli;

    // Open this package for reflection to external frameworks.
    opens mycommandpackage;

    // THE BELOW DOES NOT WORK! 
    // The below syntax results in MissingResourceException: Can't find bundle for base name XXX
    //    opens mycommandpackage to info.picocli;  // AVOID THIS
}

@aalmiray
Copy link
Contributor

Strange, as I do have an explicit opens to picocli and it works. See last line at https://github.com/kordamp/jarviz/blob/main/plugins/jarviz-cli/src/main/java/module-info.java

@remkop
Copy link
Owner

remkop commented Feb 18, 2023

@aalmiray I thought you got it to work by calling the programmatic API? (So you are calling ResourceBundle.getBundle in your own code, not from another module.)

@aalmiray
Copy link
Contributor

That is correct. I'm using the programmatic API form within the same module.

@remkop
Copy link
Owner

remkop commented Feb 19, 2023

Yes, so calling the programmatic API always works regardless of module stuff.

What I am talking about is, in order to get the @Command(resourceBundle = "something") annotation to work,
your module-info.java needs to use opens X instead of opens X to info.picocli.

@tasogare3710 can you try to see if that resolves the issue for you also?

@tasogare3710
Copy link
Author

@remkop opens yourpackage says that yourpackage allows reflection for all other modules. We need to do is to give yourpackage proper list of allowed reflection.

The below syntax results in MissingResourceException: Can't find bundle for base name XXX

It looks like same as this issue.

@remkop
Copy link
Owner

remkop commented Feb 19, 2023

@tasogare3710 I tried the example project you attached to this ticket, and if I change the module-info.java it works without any error.

// picocli_i18n_test\app\src\main\java\module-info.java
module picotest {
    requires java.base;
    requires info.picocli;

    opens picotest; // <--- this was missing
}

The output:

C:\Users\remko\IdeaProjects\picocli_i18n_test>gradlew run    

> Task :app:run
false

BUILD SUCCESSFUL in 4s

I confirmed this works with either picocli 4.6.1 or 4.7.1.
Also, the below shows it successfully takes messages from the ResourceBundle:

C:\Users\remko\IdeaProjects\picocli_i18n_test>gradlew run --args=-h

> Task :app:run
Usage: c [-h]
  -h     display a this message

BUILD SUCCESSFUL in 5s

No more MissingResourceException when module-info.java has opens picotest!

I will update the picocli User Manual for Module Configuration accordingly.

What is still unclear is why opens picotest works correctly, but opens picotest to info.picocli gives MissingResourceException...

@tasogare3710
Copy link
Author

opens picotest; // <--- this was missing

I'm sorry, my mistake. But that mistake not relevant this issue, so forget it.


info.picocli/CommandLine$Model$Messages#createBundle is calling ResourceBundle#getBundle(String), so it seems to be looking for ResourceBundle from info.picocli module.

this direct cause of this isssue. This call is same as getBundle(baseName, Locale.getDefault(), callerModule). callerModule is info.picocli.
Essentially, this problem stems from the fact that code snippets that should be executed in the codebase of app are being executed in the codebase of library.

It's strange it runs without problems if you use opens picotest and allow reflection on picotest in all modules.

@tasogare3710
Copy link
Author

tasogare3710 commented Feb 20, 2023

In short, have to find ResourceBundle from picotest module, but @Command does not support JPMS, so finding for ResourceBundle from module self(info.picocli@4.7.1) belong to.

  • It caused ResourceBundle#getBundleImpl(Module,Module,String,Locale,Control) cannot find both baseBundle and bundle, and throws MissingResourceException.
  • ResourceBundle#getBundleImpl(Module,Module,String,Locale,Control) is called by ResourceBundle#getBundle(String).

@remkop
Copy link
Owner

remkop commented Feb 20, 2023

@tasogare3710 Sorry but I disagree with your analysis.

I tried what you suggested, but even calling getBundle(baseName, Locale.getDefault(), callerModule) with the callerModule of the app (not picocli) doesn't work, unless the module-info file contains opens callerPackage.

This opens callerPackage is relevant and necessary.

I added a sample subproject to the picocli repo. It has two JPMS modules, one CLI app that contains a resource bundle and one that tests the app and tests loading the resource bundle. That last test uses various APIs to load the resource bundle and none of them worked unless the module-info had opens pn. You can experiment with this to verify my analysis.

@tasogare3710
Copy link
Author

It is because the caller of method is incorrect; there is a issue with the codebase that calls getBundle.

@remkop
Copy link
Owner

remkop commented Feb 21, 2023

@tasogare3710 I don't understand...

It is because the caller of method is incorrect; there is a issue with the codebase that calls getBundle.

I believe I have evidence that disproves your hypothesis:
For example, the sample test subproject has a test class called ResourceBundleTest. This class does not use picocli (so has nothing to do with the @Command annotation), it simply tries to call ResourceBundle.getBundle from the app-it module for a resource bundle that lives in a different app module.

This is the code for one of the tests, it does not use reflection and it clearly uses the correct Module to call getBundle:

// this test lives in the app-it module
@Test
public void testLoadBundleModular() throws Exception {

    // the JpmsModularApp class lives in the app module
    Module module = JpmsModularApp.class.getModule(); // this is the app module

    // the picocli/test_jpms/modular_app/messages.properties file lives in the app module
    ResourceBundle bundle = ResourceBundle.getBundle("picocli.test_jpms.modular_app.messages", module);
    System.out.println(bundle);
    System.out.println(bundle.getString("usage.headerHeading"));
}

Now, this test only works if the module-info for the app module has opens picocli.test_jpms.modular_app;.
It is the same with the other tests in the ResourceBundleTest class.
All these ways of calling fail/succeed the same way:

  • ResourceBundle.getBundle(String)
  • ResourceBundle.getBundle(String, Locale)
  • ResourceBundle.getBundle(String, Module)

So it seems to me that it is necessary to have opens packageName in the module-info of the module containing the resource bundle, to make any variant of ResourceBundle.getBundle work.

Are you suggesting that there is a way to make ResourceBundle.getBundle(String, Module) (with the correct Module passed in) work, without having opens packageName in the module-info of the module that contains the resource bundle?
Can you provide a working example?

@tasogare3710
Copy link
Author

This is not my analysis but module mechanism. java8 and unconditionally open module do not protect a resource from this case. the MissingResourceException is caused by a security check failure in the open module. There seems to be a misunderstanding.

@tasogare3710
Copy link
Author

There also seems to be some misunderstanding about reflection: ResourceBundle uses reflection, and JLS also describes it as follows

https://docs.oracle.com/javase/specs/jls/se9/html/jls-7.html#jls-7.7.2

It also grants reflective access to all types in the package, and all their members, for code in other modules.

@remkop
Copy link
Owner

remkop commented Feb 21, 2023

The first version of this comment sounded more defensive than I intended. Sorry about that; let me delete it and try again.

Sorry @tasogare3710 I have trouble understanding your last few comments.

This was the original problem statement:

ResourceBundle fails to load if the ResourceBundle is specified using the resourceBundle element of the CommandLine.Command annotation.

I believe I provided a solution:

add opens packageName in the module-info of the module that contains the resource bundle, and picocli will be able to successfully read ResourceBundles that are specified in the resourceBundle element of the @CommandLine.Command annotation.

There are some tests (1 and 2) that I believe prove this solution works.

After that, I did not understand your replies. 😅

Can you clarify if the proposed solution meets your requirements, or if there are further requirements?

remkop added a commit that referenced this issue Feb 24, 2023
remkop added a commit that referenced this issue Feb 24, 2023
* add section Resource Bundles in JPMS Modules
* add section JPMS Modules Internationalization (link)
@remkop
Copy link
Owner

remkop commented Feb 24, 2023

I updated the user manual:
added this section: https://picocli.info/#_resource_bundles_in_jpms_modules
I believe this resolves the issue, please let me know if this does not work for you.

@remkop remkop closed this as completed Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help-wanted 🆘 theme: module An issue or change related to JPMS modules type: doc 📘
Projects
None yet
Development

No branches or pull requests

3 participants