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

JaxrsApplicationParser is invoked using plugin classpath rather than compile classpath when discovering types #214

Closed
fhoy opened this issue Jan 21, 2018 · 6 comments

Comments

@fhoy
Copy link

fhoy commented Jan 21, 2018

When upgrading typescript-generator from v1.16.274, I noticed a subtle change in class loading, that causes class discovery through our JAX-RS Application class, using <classesFromJaxrsApplication> to fail. Due to some issues with Jersey and Spring Boot, our Application class uses Spring's ClassPathScanningCandidateComponentProvider in the constructor to find @Path-annotated (and other) classes. When typescript-generator instantiates our Application class, no @Path-annotated classes are found, and the reason for this is that the classloader of the plugin is used, rather than the Maven compile classpath. When running the plugin with <classesFromAutomaticJaxrsApplication> instead, class discovery succeeds, since in this case, the plugin explicitly uses the classloader provided by Settings.createClassLoader(). I belive this is due to the changes in v1.17.282 in Input.fromClassNamesAndJaxrsApplication(), where previously all classes were scanned immediately, and afterwards only the Application class is scanned.

Should not typescript-generator use the Maven compile classpath for all class discovery, rather than the classpath provided by the plugin classloader?

@vojtechhabarta
Copy link
Owner

So to summarize: v1.16.274 works as expected but v1.17.282 does not? Could you please also try latest version (2.0.400)?

I am not sure what exactly causes this problem. It's strange that classpath scanner in typescript-generator is able to find resource classes but classpath scanner in Spring cannot find them. Could you also try latest Spring?

I think one classloader is used for all classes, it is created here: GenerateMojo.java#L530 and it is then set as context classloader of current thread.

@fhoy
Copy link
Author

fhoy commented Jan 25, 2018

I have tried both the latest version and v1.17.282, with the same result.

However, I have debugged my way once more through the plugin and application now, and think I have found the cause.

The underlying issue seems to be that at the time the Application class is instantiated (in JaxrsApplicationScanner.scanJaxrsApplication()), the current thread context class loader is the plugin class loader (since Input.fromClassNamesAndJaxrsApplication() resets the current thread context class loader in the finally block). Spring's ClassPathScanningCandidateComponentProvider by default uses the current thread context class loader to find classes, and consequently finds none. Manually overriding this into using Application.getClass().getClassLoader() instead, gives me the desired result.

In other words, this issue can probably be filed under user error. :) However, given that the current thread context class loader was set during classpath scanning in class Input, one could argue that it would make sense to do the same when instantiating the Application class in JaxrsApplicationScanner. Whether or not that would cause other problems, I am not able to tell.

@vojtechhabarta
Copy link
Owner

Thanks for debugging this. Great work. I think that's exactly it.

How exactly you overrode the classloader? In typescript-generator (JaxrsApplicationScanner.scanJaxrsApplication) or in your Application class?

I don't like touching class loading because it may work in tests, it may work in my use case, but it may still break something (as you said). But I think setting context classloader in scanJaxrsApplication could work well. In the end it is probably what runtime containers are doing (deducing from your situation).

What do you think? Should I add this change?

@fhoy
Copy link
Author

fhoy commented Jan 29, 2018

I overrode the class loader using existing facilities in ClassPathScanningCandidateComponentProvider (setResourceLoader()); this worked without having to change anything in typescript-generator.

I agree that class loading can be a tricky issue, and that it is difficult to identify potential problems. I also think that the issue I encountered is not really a corner case, and should probably be handled by the plugin. To be on the safe side, you could probably set the context class loader just before instantiating the Application class, and then immediately reset it after instantiation?

@vojtechhabarta
Copy link
Owner

vojtechhabarta commented Feb 5, 2018

I released v2.1.406. Could you please try it?

@fhoy
Copy link
Author

fhoy commented Feb 7, 2018

I have tried again, using v2.1.406, and now all classes are discovered as expected. Thanks!

@fhoy fhoy closed this as completed Feb 7, 2018
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

2 participants