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

Support multiple nesting of JAR files #293

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Conversation

dmlloyd
Copy link
Collaborator

@dmlloyd dmlloyd commented Mar 25, 2024

  • Make sure that jar:file:///... are properly formed (the sub-URL must have a scheme of file)
  • When there is no ! in the JAR URL, treat the file as a JAR and process its root path
  • Otherwise recursively resolve each segment which is !-terminated

Fixes #289

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

One thing I am unsure of is whether it is correct to treat URLs of form jar:file:///some/path/something.jar as valid (current behavior) or invalid.

magicprinc added a commit to magicprinc/SmallRyeConfig-SpringBoot that referenced this pull request Mar 25, 2024
@magicprinc
Copy link

magicprinc commented Mar 25, 2024

TestApp still fails 😭
https://github.com/magicprinc/SmallRyeConfig-SpringBoot/tree/fix2

I have put your classes inside the app and removed jar with original classes from dependencies.
You can debug and see details.

Exception in thread "main" java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:49)
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:108)
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:58)
	at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:65)
Caused by: java.lang.ExceptionInInitializerError
	at fink.demo.smallryeconfigspringboot.ExampleApplication.main(ExampleApplication.java:13)
	... 8 more
Caused by: java.nio.file.ProviderNotFoundException: Provider not found
	at java.base/java.nio.file.FileSystems.newFileSystem(FileSystems.java:545)
	at java.base/java.nio.file.FileSystems.newFileSystem(FileSystems.java:400)
	at io.smallrye.common.classloader.ClassPathUtils.processAsJarPath(ClassPathUtils.java:163)
	at io.smallrye.common.classloader.ClassPathUtils.processAsJarPath(ClassPathUtils.java:169)
	at io.smallrye.common.classloader.ClassPathUtils.processAsPath(ClassPathUtils.java:149)
	at io.smallrye.common.classloader.ClassPathUtils.consumeAsPath(ClassPathUtils.java:102)
	at io.smallrye.common.classloader.ClassPathUtils.consumeAsPaths(ClassPathUtils.java:86)
	at io.smallrye.config.AbstractLocationConfigSourceLoader.tryClassPath(AbstractLocationConfigSourceLoader.java:141)
	at io.smallrye.config.AbstractLocationConfigSourceLoader.loadConfigSources(AbstractLocationConfigSourceLoader.java:104)
	at io.smallrye.config.AbstractLocationConfigSourceLoader.loadConfigSources(AbstractLocationConfigSourceLoader.java:87)
	at io.smallrye.config.PropertiesConfigSourceProvider.<init>(PropertiesConfigSourceProvider.java:37)
	at fink.config.spring.SmallRyeConfigAutoConf.<clinit>(SmallRyeConfigAutoConf.java:75)
	... 9 more

FAILURE: Build failed with an exception.

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

Do you know what URL was input into the method?

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

jar:file:/D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar!/BOOT-INF/classes!/application.properties

BTW if you simply replace lastIndexOf with indexOf - everything works (spring magic 🤷‍♀️)

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

Well, at least I was correct about it being a two-level nesting. I have an idea for a fix, but I'm going to add some additional unit tests first.

@magicprinc
Copy link

With IntelliJ IDEA you can debug jar file: you build spring jar with gradle bootJar, then you add Run/Debug configuration - JAR Application with path to your jar (e.g. D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar)

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

https://github.com/magicprinc/SmallRyeConfig-SpringBoot/tree/fixBundled

url: jar:file:/D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar!/BOOT-INF/classes!/application.properties

file: file:/D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar!/BOOT-INF/classes!/application.properties

jar: D:\TEMP\SmallRyeConfig-SpringBoot\build\libs\SmallRyeConfig-SpringBoot-1.0.jar

localPath: /BOOT-INF/classes!/application.properties


io.smallrye.common.classloader.ClassPathUtils#readStream

url: jar:file:///D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar!/BOOT-INF/classes!/application.properties

can be read

LOG
[main] DEBUG io.smallrye.config - SRCFG01006: Loaded ConfigSource PropertiesConfigSource[source=jar:file:///D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar!/BOOT-INF/classes!/application.properties] with ordinal 100

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

Can you determine whether the sub-entry /BOOT-INF/classes inside of file:///D:/TEMP/SmallRyeConfig-SpringBoot/build/libs/SmallRyeConfig-SpringBoot-1.0.jar is a JAR directory, or is it a nested JAR file? I am assuming that ! means to treat that entry as a JAR file but Spring Boot might not agree with that, but this would be an easy way to check.

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

image

Usual jar/zip directory

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

image

original/as-is dependency jars in a usual jar/zip directory

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

image

META-INF has an extra folder for itself :-)

- "dependencies":
  - "BOOT-INF/lib/"
- "spring-boot-loader":
  - "org/"
- "snapshot-dependencies":
- "application":
  - "BOOT-INF/classes/"
  - "BOOT-INF/classpath.idx"
  - "BOOT-INF/layers.idx"
  - "META-INF/"

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

OK, this version should be able to detect whether a nested entry is a JAR or a directory and act accordingly.

* Make sure that `jar:file:///...` are properly formed (the sub-URL must have a scheme of `file`)
* When there is no `!` in the JAR URL, treat the file as a JAR and process its root path
* Otherwise recursively resolve each segment which is `!`-terminated
@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 25, 2024

One more fix to prevent wrong path lookups if the ! is followed by a /.

@magicprinc
Copy link

magicprinc commented Mar 25, 2024

Works!

jarPath: D:\TEMP\SmallRyeConfig-SpringBoot\build\libs\SmallRyeConfig-SpringBoot-1.0.jar
path: /BOOT-INF/classes!/application.properties

localPath: /BOOT-INF/classes
path: /BOOT-INF/classes!/application.properties

The code is quite complicated, so I hope you would have enough unit tests 😅

@magicprinc
Copy link

magicprinc commented Mar 26, 2024

BTW, sorry for a noob question...
Why do you need such heavy machinery (FileSystems, path parsing)
As far as I know Spring uses only ClassLoader.getResources, .getResourceAsStream, etc

E.g.
https://github.com/spring-projects/spring-framework/blob/main/spring-core/src/main/java/org/springframework/core/io/ClassPathResource.java

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Mar 26, 2024

That's a more complex question than it seems to be ;)

There are several answers though:

  • Sometimes there is no class loader for the target path
  • Sometimes the class loader does not have the necessary capabilities
  • Sometimes you are using an API that is based on Path
  • Probably more cases I'm not thinking of right now

@magicprinc
Copy link

magicprinc commented Mar 26, 2024

When is it possible to have snapshot/RC version in maven repo? 😥🙏

PS: IDEA recommends flipping couple of .equals
→ p -> "jar".equals(p.getScheme())
→ "file".equals(fileUrl.getProtocol()

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Apr 2, 2024

I'm just doing some final testing with Quarkus to make sure nothing has broken.

@dmlloyd
Copy link
Collaborator Author

dmlloyd commented Apr 2, 2024

Looks good.

@dmlloyd dmlloyd merged commit 9a34989 into smallrye:main Apr 2, 2024
4 checks passed
@dmlloyd dmlloyd deleted the jar branch April 2, 2024 16:48
@github-actions github-actions bot added this to the 2.3.1 milestone Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doesn't work in Spring Boot jar
2 participants