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

jackson-kotlin dependency causes ClassNotFoundException when finding Jackson modules via service loader #11133

Closed
gbrehmer opened this Issue Nov 24, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@gbrehmer
Copy link

gbrehmer commented Nov 24, 2017

2.0.0.M6: If I use web-starter and activate findModulesViaServiceLoader on Jackson2ObjectMapperBuilder the application raises an java.lang.ClassNotFoundException: kotlin.jvm.internal.DefaultConstructorMarker on startup.

If I exclude jackson-kotlin dependency from web-starter, the error does not occur.

See example project attached:
sample.zip

@sdeleuze

This comment has been minimized.

Copy link
Contributor

sdeleuze commented Nov 24, 2017

Indeed, Jackson does not perform defensive check about the presence of Kotlin in the classpath before registering KotlinModule. I am going to send a PR on Jackson to register the Kotlin module only when Kotlin and jackson-module-kotlin are on the classpath if that's possible.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Nov 24, 2017

@gbrehmer Thanks for opening a new issue for this. Out of interest, what prompted you to activate findModulesViaServiceLoader?

@sdeleuze Please do open a Jackson pull request, but I think there's a chance that it'll be declined. Our starter's "breaking" jackson-module-kotlin by excluding its dependency on kotlin-reflect. We may need to consider removing jackson-module-kotlin from the JSON starter.

@sdeleuze

This comment has been minimized.

Copy link
Contributor

sdeleuze commented Nov 24, 2017

After a deeper look, the NoClassDefFoundError is raised by Java service loader that try to load KotlinModule which is itself a Kotlin class, so I am not sure we can fix that on Jackson side. Jackson could catch the NoClassDefFoundError but even by doing that, I guess this is not super clean and would require logging at least an INFO log message.

It is true that jackson-module-kotlin is different from jackson-datatype-jsr310 and jackson-datatype-jdk8 since it is not part of the mandatory minimum requirement, so I understand it would maybe need to be removed.

But that will raise again the question about how we could handle the creation of Kotlin web project on start.spring.io that works out of the box. I am not sure yet about how to handle that.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Nov 24, 2017

Looking at the code for ServiceLoader, I don't think it's possible for a service registered in META-INF/services to be optional. If the service is registered its class will be loaded and that class load has to succeed.

For this to work, com.fasterxml.jackson.module.kotlin.KotlinModule would have to be written in such a way that it can be loaded without Kotlin on the class path. Given that it's written in Kotlin, that's going to require a significant change.

@sdeleuze If you find a way to get this to work, please do let us know. I can only see two options at the moment:

  1. Document that you can't use service loader-based module discover with spring-boot-starter-json unless you exclude the Kotlin module dependency
  2. Remove the Kotlin module dependency from the starter

1 doesn't really feel like a reasonable option as people who shouldn't have to care about Kotlin and its Jackson module now have to deal with it. I fear that the only realistic option here is 2.

@wilkinsona wilkinsona changed the title jackson-kotlin dependency causes ClassNotFoundException jackson-kotlin dependency causes ClassNotFoundException when finding Jackson modules via service loader Nov 24, 2017

@wilkinsona wilkinsona added this to the 2.0.0.M7 milestone Nov 24, 2017

@sdeleuze

This comment has been minimized.

Copy link
Contributor

sdeleuze commented Nov 24, 2017

Yeah, I also think that the only realistic option is to remove jackson-module-kotlin from spring-boot-starter-json, but we can't just make the vast majority of Spring Boot Kotlin projects broken like it was before it was added for such basic REST webservices use case.

I don't want to do again all the discussion we had previously on that topic, but I think there is a reasonable solution: acknowledge that Kotlin is a language but also a technology that deserves its own starter.

We currently add directly kotlin-stdlib-jre8 and kotlin-stdlib-reflect dependencies when we create a new Spring Boot project on start.spring.io, to me this jackson-module-kotlin dependency is another mandatory dependency for most projects (web and web reactive at least).

Instead of adding jackson-module-kotlin to spring-boot-starter-json which generates this issue and could make Java developers puzzled like raised by @olivergierke in #9803, I think Kotlin developers will be perfectly fine to have this jackson-module-kotlin in the classpath even if it is not used for 100% of the project.

A spring-boot-starter-kotlin starter would take in account that in addition to a language, Kotlin is a technology that requires a set of library in the classpath to work correctly. It would contains kotlin-stdlib-jre8, kotlin-reflect and jackson-module-kotlin. If Jackson is not in the classpath, jackson-module-kotlin will just not be used, like it was the case for spring-boot-starter-json + Java but this time we won't have the same kind of issues like the one raised here since Jackson won't be present in the classpath. This solution would make the build configuration shorter, require trivial modification to start.spring.io (replace the 2 Kotlin dependencies by the starter), and would provide a significant added value for all Spring Boot Kotlin developers.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Nov 24, 2017

@sdeleuze Thanks for the suggestion. I'm not keen on the idea of having spring-boot-starter-kotlin. We don't have any other language-specific starters, and I don't think making an exception for Kotlin is the right thing to do.

Setting up a Spring Boot project to use Kotlin should, ideally, look the same as setting up any other project in terms of the core dependencies that you need to declare and the build plugins that need to be configured. Spring Initializr takes care of that boilerplate, but what it produces is hopefully idiomatic and commonly-used. I think that's important.

Starting a Kotlin app that has Jackson on the class path but does not have jackson-module-kotlin on the class path, logs the following:

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::             (v2.0.0.M6)

2017-11-24 16:06:51.294  INFO 65334 --- [           main] com.example.demo.DemoApplicationKt       : Starting DemoApplicationKt on aw-rmbp.local with PID 65334 (/Users/awilkinson/Downloads/demo/target/classes started by awilkinson in /Users/awilkinson/Downloads/demo)
2017-11-24 16:06:51.299  INFO 65334 --- [           main] com.example.demo.DemoApplicationKt       : No active profile set, falling back to default profiles: default
2017-11-24 16:06:51.351  INFO 65334 --- [           main] ConfigServletWebServerApplicationContext : Refreshing org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext@11ff9a8b: startup date [Fri Nov 24 16:06:51 GMT 2017]; root of context hierarchy
2017-11-24 16:06:51.403  WARN 65334 --- [kground-preinit] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:51.623  WARN 65334 --- [kground-preinit] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.280  INFO 65334 --- [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat initialized with port(s): 8080 (http)
2017-11-24 16:06:52.289  INFO 65334 --- [           main] o.apache.catalina.core.StandardService   : Starting service [Tomcat]
2017-11-24 16:06:52.290  INFO 65334 --- [           main] org.apache.catalina.core.StandardEngine  : Starting Servlet Engine: Apache Tomcat/8.5.23
2017-11-24 16:06:52.304  INFO 65334 --- [ost-startStop-1] o.a.catalina.core.AprLifecycleListener   : The APR based Apache Tomcat Native library which allows optimal performance in production environments was not found on the java.library.path: [/Users/awilkinson/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:/System/Library/Java/Extensions:/usr/lib/java:.]
2017-11-24 16:06:52.356  INFO 65334 --- [ost-startStop-1] o.a.c.c.C.[Tomcat].[localhost].[/]       : Initializing Spring embedded WebApplicationContext
2017-11-24 16:06:52.357  INFO 65334 --- [ost-startStop-1] o.s.web.context.ContextLoader            : Root WebApplicationContext: initialization completed in 1009 ms
2017-11-24 16:06:52.439  WARN 65334 --- [ost-startStop-1] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.459  INFO 65334 --- [ost-startStop-1] o.s.b.w.servlet.ServletRegistrationBean  : Mapping servlet: 'dispatcherServlet' to [/]
2017-11-24 16:06:52.463  INFO 65334 --- [ost-startStop-1] o.s.b.w.servlet.FilterRegistrationBean   : Mapping filter: 'characterEncodingFilter' to: [/*]
2017-11-24 16:06:52.463  INFO 65334 --- [ost-startStop-1] o.s.b.w.servlet.FilterRegistrationBean   : Mapping filter: 'hiddenHttpMethodFilter' to: [/*]
2017-11-24 16:06:52.464  INFO 65334 --- [ost-startStop-1] o.s.b.w.servlet.FilterRegistrationBean   : Mapping filter: 'httpPutFormContentFilter' to: [/*]
2017-11-24 16:06:52.464  INFO 65334 --- [ost-startStop-1] o.s.b.w.servlet.FilterRegistrationBean   : Mapping filter: 'requestContextFilter' to: [/*]
2017-11-24 16:06:52.547  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.652  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.689  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.689  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.724  INFO 65334 --- [           main] s.w.s.m.m.a.RequestMappingHandlerAdapter : Looking for @ControllerAdvice: org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext@11ff9a8b: startup date [Fri Nov 24 16:06:51 GMT 2017]; root of context hierarchy
2017-11-24 16:06:52.794  INFO 65334 --- [           main] s.w.s.m.m.a.RequestMappingHandlerMapping : Mapped "{[/error]}" onto public org.springframework.http.ResponseEntity<java.util.Map<java.lang.String, java.lang.Object>> org.springframework.boot.autoconfigure.web.servlet.error.BasicErrorController.error(javax.servlet.http.HttpServletRequest)
2017-11-24 16:06:52.795  INFO 65334 --- [           main] s.w.s.m.m.a.RequestMappingHandlerMapping : Mapped "{[/error],produces=[text/html]}" onto public org.springframework.web.servlet.ModelAndView org.springframework.boot.autoconfigure.web.servlet.error.BasicErrorController.errorHtml(javax.servlet.http.HttpServletRequest,javax.servlet.http.HttpServletResponse)
2017-11-24 16:06:52.821  INFO 65334 --- [           main] o.s.w.s.handler.SimpleUrlHandlerMapping  : Mapped URL path [/webjars/**] onto handler of type [class org.springframework.web.servlet.resource.ResourceHttpRequestHandler]
2017-11-24 16:06:52.822  INFO 65334 --- [           main] o.s.w.s.handler.SimpleUrlHandlerMapping  : Mapped URL path [/**] onto handler of type [class org.springframework.web.servlet.resource.ResourceHttpRequestHandler]
2017-11-24 16:06:52.830  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath
2017-11-24 16:06:52.851  INFO 65334 --- [           main] o.s.w.s.handler.SimpleUrlHandlerMapping  : Mapped URL path [/**/favicon.ico] onto handler of type [class org.springframework.web.servlet.resource.ResourceHttpRequestHandler]
2017-11-24 16:06:52.985  INFO 65334 --- [           main] o.s.j.e.a.AnnotationMBeanExporter        : Registering beans for JMX exposure on startup
2017-11-24 16:06:53.050  INFO 65334 --- [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat started on port(s): 8080 (http)
2017-11-24 16:06:53.054  INFO 65334 --- [           main] com.example.demo.DemoApplicationKt       : Started DemoApplicationKt in 2.134 seconds (JVM running for 5.173)

There are no fewer than 8 lines telling me that I need to add the jackson-module-kotlin dependency, each similar to the following:

2017-11-24 16:06:52.830  WARN 65334 --- [           main] o.s.h.c.j.Jackson2ObjectMapperBuilder    : For Jackson Kotlin classes support please add "com.fasterxml.jackson.module:jackson-module-kotlin" to the classpath

While this isn't ideal, I think it's preferable to introducing a language-specific starter. I also think that this is good enough for Spring Boot 2.0 given the problems that this issue has identified with our preferred solution/workaround.

In all likelihood, the ideal solution to this problem belongs in Spring Initializr. When time is available, I can foresee it having more sophisticated support for automatically adding a dependency, such as jackson-module-kotlin, when the other dependencies that have been selected indicate that it's required.

@philwebb

This comment has been minimized.

Copy link
Member

philwebb commented Nov 24, 2017

com.fasterxml.jackson.databind.Module looks pretty simple. Perhaps a Java shim could be added that would delegate to KotlinModule only if Kotlin is available? This would still need to be done on the Jackson side, since the services file is there.

@sdeleuze

This comment has been minimized.

Copy link
Contributor

sdeleuze commented Nov 24, 2017

Technically it could be possible but based on the various feedback we had, I now tend to think that adding jackson-module-kotlin to spring-boot-starter-json is not a good path, and I would not be comfortable asking Jackson such change. I think a service detected is expected to be loaded correctly or throwing an exception, not something in between.

@wilkinsona wilkinsona self-assigned this Nov 27, 2017

mle-enso referenced this issue in evainga/rememberbrall Nov 27, 2017

Eva Goettert
@unlimitedsola

This comment has been minimized.

Copy link
Contributor

unlimitedsola commented Dec 1, 2017

just broke my little app when compiling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment