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

Incompatibility with Spring BOM when activating jcloud's request/response logging - NoSuchMethodError: 'void ConstructorConstructor.<init>(Map)' at GsonModule.provideGson #271

Open
alb-i986 opened this issue Nov 30, 2022 · 6 comments

Comments

@alb-i986
Copy link

Similar to #246 (but not that much actually).

I was able to squeeze the issue in the mvn project attached: project.tgz

Run the build (mvn clean test) (or the one and only test from your IDE), and you get:

com.google.inject.CreationException: Unable to create injector, see the following errors:

1) [Guice/ErrorInCustomProvider]: NoSuchMethodError: 'void ConstructorConstructor.<init>(Map)'
  at GsonModule.provideGson(GsonModule.java:99)
      \_ installed by: JenkinsHttpApiModule -> GsonModule
  at GsonWrapper.<init>(GsonWrapper.java:38)
      \_ for 1st parameter
  at GsonWrapper.class(GsonWrapper.java:32)
  while locating GsonWrapper
  while locating Json

Learn more:
  https://github.com/google/guice/wiki/ERROR_IN_CUSTOM_PROVIDER
Caused by: NoSuchMethodError: 'void ConstructorConstructor.<init>(Map)'
	at GsonModule.provideGson(GsonModule.java:130)
	at GsonModule$$FastClassByGuice$$2722177.GUICE$TRAMPOLINE(<generated>)
	at GsonModule$$FastClassByGuice$$2722177.apply(<generated>)
	at ProviderMethod$FastClassProviderMethod.doProvision(ProviderMethod.java:260)
[..]

Turn on <classifier>all</classifier> in the pom, and you get:

method modules in class com.cdancy.jenkins.rest.JenkinsClient.Builder cannot be applied to given types;
[ERROR]   required: io.github.cdancy.jenkins.rest.shaded.com.google.inject.Module[]
[ERROR]   found: org.jclouds.logging.slf4j.config.SLF4JLoggingModule
[ERROR]   reason: varargs mismatch; org.jclouds.logging.slf4j.config.SLF4JLoggingModule cannot be converted to io.github.cdancy.jenkins.rest.shaded.com.google.inject.Module

Now comment out <classifier>all</classifier> and comment out Spring's BOM, like this:

<!--    <dependencyManagement>-->
<!--        <dependencies>-->
<!--            <dependency>-->
<!--                <groupId>org.springframework.boot</groupId>-->
<!--                <artifactId>spring-boot-dependencies</artifactId>-->
<!--                <version>2.7.6</version>-->
<!--                <type>pom</type>-->
<!--                <scope>import</scope>-->
<!--            </dependency>-->
<!--        </dependencies>-->
<!--    </dependencyManagement>-->

    <dependencies>
        <dependency>
            <groupId>io.github.cdancy</groupId>
            <artifactId>jenkins-rest</artifactId>
            <version>1.0.2</version>
<!--            <classifier>all</classifier>-->
        </dependency>

and run the build again.

The test will now pass.

The issue has to do with the setup of the request/response logging:

JenkinsClient.builder()
                .endPoint("asd")
                .credentials("asd:asd")
                .modules(new SLF4JLoggingModule())
                .build();

Your Environment

  • Apache Maven 3.8.5 (3599d3414f046de2324203b78ddcf9b5e4388aa0)
  • Java version: 11.0.17
  • jenkins-rest: 1.0.2
@anderso
Copy link

anderso commented Aug 22, 2023

Yep, the all-version is not compatible with org.apache.jclouds.driver:jclouds-slf4j because it shades com.google.inject:guice.

I guess because it rewrites the client API, in this case com.cdancy.jenkins.rest.JenkinsClient.Builder#modules it would be considered a bug.

@cdancy
Copy link
Owner

cdancy commented Aug 22, 2023

Interesting ... I currently don't have the time to look into this but if someone wants to send in a fix or has an idea feel free to do so and I can merge.

@anderso
Copy link

anderso commented Aug 23, 2023

The issue is that the intersection of what is shaded and what is part of the public api of jenkins-rest is not empty.

Specifically in this case, this line

https://github.com/cdancy/jenkins-rest/blob/3c75844300f721f913edea37778bf5cf713d0caf/gradle/additional-artifacts.gradle#L11C19-L11C19

means that each class in the com.google package is relocated to io.github.cdancy.jenkins.rest.shaded.com.google and all imports are rewritten in the artifact with the all-classifier.

And this line declares a method with a parameter of type com.google.inject.Module

https://github.com/cdancy/jenkins-rest/blob/3c75844300f721f913edea37778bf5cf713d0caf/src/main/java/com/cdancy/jenkins/rest/JenkinsClient.java#L204C35-L204C35

which means that this method has different signatures in the different artifacts.

Fixes:

  1. Deprecate the all-classifier-artifact, but I guess it's there for a reason and people are using it.

  2. Refactor the api so that the intersection becomes empty.

  3. Change what is shaded

I'm guessing 3 is the most practical but maybe not trivial in case there are interdependencies.

The gradle library plugin supports separation of api and implementation dependencies

https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_separation

a suggestion would be to use this plugin in jenkins-rest, and declare com.google.inject:guice as api dependence.

In the best of worlds this would be recognized by the shadow jar plugin and it would do the right thing, but I do not know if this is the case.

@BLishman
Copy link

@anderso Your solution worked for me using JDK 17 and Spring Boot 3.+, I was able to build the shadowjar after setting guice to api instead of implementation and my spring boot project is able to access jenkins.

@cdancy for FYI

@cdancy
Copy link
Owner

cdancy commented Feb 22, 2024

@BLishman send in a PR and I'll take a look!

@anderso
Copy link

anderso commented Feb 22, 2024

That sounds like a good solution to me. This will cause guice to become a regular dependency of the shadow jar whereas it previously was shadowed, but on the other hand the api does not need to change.

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

No branches or pull requests

4 participants