Skip to content

Comments

#8932 : check JDK running to avoid using SpringBusFactory as default#1445

Closed
laurentschoelens wants to merge 1 commit intoapache:mainfrom
laurentschoelens:gh-8932
Closed

#8932 : check JDK running to avoid using SpringBusFactory as default#1445
laurentschoelens wants to merge 1 commit intoapache:mainfrom
laurentschoelens:gh-8932

Conversation

@laurentschoelens
Copy link

Tested OK against project sample in issue https://issues.apache.org/jira/browse/CXF-8932

Got the following output running JDK11

[INFO] --- cxf-codegen:4.0.4-SNAPSHOT:wsdl2java (generate-sources) @ issue-8932 ---
[INFO] Running code generation in fork mode...
[INFO] The java executable is /Library/Java/JavaVirtualMachines/jdk-11.0.13.jdk/Contents/Home/bin/java
[INFO] Building jar: /var/folders/m7/sbrqv5yd79z1slvj20s27wd80000gp/T/cxf-tmp-8102040999141811417/cxf-codegen15093912636962846588.jar
[WARNING] SLF4J: No SLF4J providers were found.
[WARNING] SLF4J: Defaulting to no-operation (NOP) logger implementation
[WARNING] SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.
[WARNING] sept. 21, 2023 10:44:07 AM org.apache.cxf.BusFactory getBusFactoryClass
**[WARNING] WARNING: Failed to used SpringBusFactory with current JDK running, using default CXFBusFactory.**

@reta
Copy link
Member

reta commented Sep 21, 2023

Thanks for the pull request @laurentschoelens , I don't think this is the path we should take: the unfortunate fact is that nonetheless the dependencies are declared optional, Apache Maven eagerly brings them, I've opened the pull request to fix thar here #1446

@laurentschoelens
Copy link
Author

@reta : in fact, marking a dependency optional does not do what you think it does.

Take module A / B / C :

  • B depends on A with optional tag
    • B will then have access and depends really on A (you can write code with classes under A)
  • C depends on B
    • C will have no access and does not depends on A cause A is an optional dependency of B, and then not taken as transitive dependency.

Here, codegen does have spring dependency as direct dependency, even if marked as optional (which has no effect cause final artifact). Another fix would be to check if codegen really depends on spring classes (inside his own module) and remove it if nod needed.
If needed in codegen, then my PR is the best option.

@laurentschoelens
Copy link
Author

Thanks for the pull request @laurentschoelens , I don't think this is the path we should take: the unfortunate fact is that nonetheless the dependencies are declared optional, Apache Maven eagerly brings them, I've opened the pull request to fix thar here #1446

By doing so, it will never use SpringBusFactory has default BusFactory, even on JDK17. Don't know if that you really want to do.

@reta
Copy link
Member

reta commented Sep 21, 2023

  • B will then have access and depends really on A (you can write code with classes under A)

Correct, but if the dependency is not available at runtime, the application will fail (if is not mitigated)

Here, codegen does have spring dependency as direct dependency, even if marked as optional (which has no effect cause final artifact). Another fix would be to check if codegen really depends on spring classes (inside his own module) and remove it if nod needed.

The optional part indeed was not sufficient hence the provided was added, those are never brought up by default. codegen does not depend on the Spring classes directly.

@laurentschoelens
Copy link
Author

  • B will then have access and depends really on A (you can write code with classes under A)

Correct, but if the dependency is not available at runtime, the application will fail (if is not mitigated)

Here having it with default scope but only Optional will bring it on runtime by default when execution B as application.

Here, codegen does have spring dependency as direct dependency, even if marked as optional (which has no effect cause final artifact). Another fix would be to check if codegen really depends on spring classes (inside his own module) and remove it if nod needed.

The optional part indeed was not sufficient hence the provided was added, those are never brought up by default. codegen does not depend on the Spring classes directly.

Doing so you'll always get the failing "ClassNotFoundException" with no more usage of SpringBusFactory on plugins.
Don't know if that's excepted...
It'll only use SpringBusFactory class only by adding manually Spring as dependency in the plugin...

@reta
Copy link
Member

reta commented Sep 21, 2023

Doing so you'll always get the failing "ClassNotFoundException" with no more usage of SpringBusFactory on plugins.

The are part of the codebase that works by introspection of classpath, like in this concrete example, if Spring is available on the classpath, the SpringBus would be picked.

It'll only use SpringBusFactory class only by adding manually Spring as dependency in the plugin...

... or application, which is exactly the goal

@laurentschoelens
Copy link
Author

Doing so you'll always get the failing "ClassNotFoundException" with no more usage of SpringBusFactory on plugins.

The are part of the codebase that works by introspection of classpath, like in this concrete example, if Spring is available on the classpath, the SpringBus would be picked.

Sorry to insists, just to be sure the changes made are OK

It'll only use SpringBusFactory class only by adding manually Spring as dependency in the plugin...

... or application, which is exactly the goal

Does the plugin gets dependencies of the maven module ?
For app context, ok to have Spring but in more modular approch, I get java code generated in special standalone project with minimal without spring dependencies.
What does SpringBusFactory does in code generation ?
Moreover I've just tried in debug mode (with forcing 'fork' to false and having spring-context in dependencies, and I always fail in the catch clause below

 catch (NoClassDefFoundError | ClassNotFoundException var10) {
                        busFactoryClass = "org.apache.cxf.bus.CXFBusFactory";
                    }

The only way to get rid of this, is by adding spring as direct dep of the plugin

@reta
Copy link
Member

reta commented Sep 21, 2023

Does the plugin gets dependencies of the maven module ?

It is configured by requiresDependencyResolution (see please https://maven.apache.org/guides/mini/guide-maven-classloading.html), the CXF plugin use test (default) or compile (configurable) classpath

For app context, ok to have Spring but in more modular approch, I get java code generated in special standalone project with minimal without spring dependencies.
What does SpringBusFactory does in code generation ?

The SpringBusFactory comes out of cxf-core which plugin use as the dependency, plugins do not benefit from it (SpringBusFactory). The CXF plugins require forking for JDK9+.

@laurentschoelens
Copy link
Author

Understood for jdk9 forking : that's why it always forks even if putting fork attribute to false

If spring is not used by codegen, why should you not remove the dependency ?

@reta
Copy link
Member

reta commented Sep 21, 2023

If spring is not used by codegen, why should you not remove the dependency ?

Spring is not used by codegen (and other plugins) directly but dependencies of the plugin, I sadly don't know for sure the reasons, I could only guess but if the project depends on Spring, there are different classpath scanners etc the plugin could use (this is purely speculation)

@laurentschoelens
Copy link
Author

If spring is not used by codegen, why should you not remove the dependency ?

Spring is not used by codegen (and other plugins) directly but dependencies of the plugin, I sadly don't know for sure the reasons, I could only guess but if the project depends on Spring, there are different classpath scanners etc the plugin could use (this is purely speculation)

So maybe just remove the spring dependencies of the plugins : making it optional-provided in plugin's pom is quite non-sense to me if not used

@reta
Copy link
Member

reta commented Sep 21, 2023

So maybe just remove the spring dependencies of the plugins : making it optional-provided in plugin's pom is quite non-sense to me if not used

Having these dependencies explicitly listed protects from any possible leakage of transitive Spring dependencies into the plugins, accidental or not (we may remove it the future, however I am concerned at this point to not break anyone's build in case there was reliance on Spring presence)

@laurentschoelens
Copy link
Author

Ok.
Good for me
Thanks for your support
Will close this PR

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.

2 participants