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

Feature/webflux #2608

Merged
merged 46 commits into from Aug 18, 2018
Merged

Feature/webflux #2608

merged 46 commits into from Aug 18, 2018

Conversation

ligasgr
Copy link
Contributor

@ligasgr ligasgr commented Aug 9, 2018

What's this PR do/fix?

This PR provides support for webflux.

Are there unit tests? If not how should this be manually tested?

Existing unit tests were preserved and are still working. Added simplest contract tests for webflux.

Any background context you want to provide?

This PR supersedes PR #2233. It includes all the original changes plus necessary fixes and tests.

Not all of the tests moved in the original PR into the new project were easy to move back. The remaining ones would require creating a new layer of mixins that would not be tied to specific implementation (mvc/webflux). Unfortunately I don’t have that much time available to work on it.

Existing annotation was renamed @EnableSwagger2 -> @ EnableSwagger2WebMvc
and new annotation @EnableSwagger2WebFlux was added.

Please note that it is no longer enough to add dependency on:

<dependency>
    <groupId>io.springfox</groupId>
    <artifactId>springfox-swagger2</artifactId>
    <version>2.9.2</version>
</dependency>

you now also have to include either:

<dependency>
    <groupId>io.springfox</groupId>
    <artifactId>springfox-spring-webmvc</artifactId>
    <version>2.9.2</version>
</dependency>

or

<dependency>
    <groupId>io.springfox</groupId>
    <artifactId>springfox-spring-webflux</artifactId>
    <version>2.9.2</version>
</dependency>

Documentation needs a lot of updates about dependencies and an example for web flux. The section 4.2. Component Model also needs an update.

What are the relevant issues?

#1773
#2233

Thomas Deblock and others added 30 commits January 19, 2018 16:45
Add two interfaces :
   - NameValueExpression
   - PatternsRequestCondition

Class from spring will be wrapped onto this classes
there is the first usable version of webflux project.
there are a error while building swagger-ui webjar. the jar is empty.
We must use old webjar before fix.
…ature/webflux

Conflicts:
	.version
	gradle/dependencies.gradle
	gradle/wrapper/gradle-wrapper.properties
	springfox-spi/src/main/java/springfox/documentation/spi/service/contexts/Defaults.java
	springfox-spi/src/main/java/springfox/documentation/spi/service/contexts/Orderings.java
	springfox-spring-web/src/main/java/springfox/documentation/spring/web/plugins/CombinedRequestHandler.java
	springfox-spring-web/src/test/groovy/springfox/documentation/spring/web/scanners/ApiListingScannerSpec.groovy
	springfox-spring-webmvc/src/test/groovy/springfox/documentation/spring/web/plugins/DefaultRequestHandlerCombinerSpec.groovy
	springfox-spring-webmvc/src/test/groovy/springfox/documentation/spring/web/plugins/PathAndParametersEquivalenceSpec.groovy
	springfox-spring-webmvc/src/test/groovy/springfox/documentation/spring/web/readers/ApiModelReaderSpec.groovy
	springfox-swagger-ui/build.gradle
	springfox-swagger2/build.gradle
	springfox-swagger2/src/main/java/springfox/documentation/swagger2/annotations/EnableSwagger2WebMvc.java
	springfox-swagger2/src/main/java/springfox/documentation/swagger2/configuration/Swagger2DocumentationWebMvcConfiguration.java
	swagger-contract-tests/src/test/groovy/springfox/test/contract/swaggertests/Swagger2TestConfig.groovy
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@springfox springfox deleted a comment Aug 9, 2018
@codecov
Copy link

codecov bot commented Aug 9, 2018

Codecov Report

Merging #2608 into master will decrease coverage by 0.39%.
The diff coverage is 78.78%.

@@             Coverage Diff             @@
##             master    #2608     +/-   ##
===========================================
- Coverage     95.15%   94.75%   -0.4%     
- Complexity     3105     3175     +70     
===========================================
  Files           343      359     +16     
  Lines          7863     8030    +167     
  Branches        599      614     +15     
===========================================
+ Hits           7482     7609    +127     
- Misses          235      268     +33     
- Partials        146      153      +7

@dilipkrish
Copy link
Member

@ligasgr this is a terrific PR! It was one of the few PR's that is so easy to follow the changes. I'll polish and pull it in 🤘

@dilipkrish
Copy link
Member

@ligasgr Trying to close this I encountered one potential problem, would love to bounce that idea with you and possibly @deblockt

The PathProvider we have now has web mvc or web flux avatars. Now when we have an application that has both, which one do we pick when used in common infrastructure code for e.g. here

@ligasgr
Copy link
Contributor Author

ligasgr commented Aug 15, 2018

@dilipkrish Thanks for your kind words :) And sorry for late response.

Regarding the PathProvider... it would be fairly simple to have the correct PathProviderFactory wired into the correct context by making it a @Bean in the correct configuration class, e.g. WebFluxRelativePathProviderFactory inside of SpringfoxWebFluxConfiguration.

This will not provide a solution to the issue of what the system should do when you have both spring-webmvc and spring-webflux on the classpath. There's a number of questions that need to be answered:

  • Should the library be able to scan from both controller types (webmvc & webflux) annotated with appropriate annotations inside of the same application?
  • Should it just assume that if there is a servlet context (which will hold true for webflux on tomcat/jetty/servlet 3.1 container) then it should use the provider which uses servlet context to obtain the path?
  • Should it somehow allow overriding that?

Currently applied solution avoids answering this stuff and requires exclusion of the "other dependency" which can be seen in contract test projects.
Please let me know your thoughts and I can help amending it to the expected behaviour as long as it is not a fundamental change.

@dilipkrish dilipkrish merged commit b2937a1 into springfox:master Aug 18, 2018
@dilipkrish dilipkrish added the PR label Aug 18, 2018
@dilipkrish
Copy link
Member

@ligasgr I've merged it to master. Thank you and @deblockt very much for your contribution and efforts!

I still have an outstanding question. I havent experimented or played with it yet, there is the Swagger2ControllerWebFlux and Swagger2ControllerWebMvc, I dont know how they'd behave when they co-exist. Will it throw a duplicate request mapping exception?

@dilipkrish dilipkrish added this to the 3.0 milestone Aug 18, 2018
@ligasgr
Copy link
Contributor Author

ligasgr commented Aug 18, 2018

@dilipkrish Thanks! Can't wait for the official release :) 👍
Right now when e.g. you bring up the context which is configured for webflux and have webmvc on the classpath, you will get an exception when starting up:

Exception in thread "main" org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'webMvcRequestHandlerProvider' defined in file [/Users/ligasgr/wrk/springfox/springfox-spring-webmvc/out/production/classes/springfox/documentation/spring/web/plugins/WebMvcRequestHandlerProvider.class]: Unsatisfied dependency expressed through constructor parameter 1; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'java.util.List<org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping>' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:732)
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:197)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1276)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1133)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:543)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:503)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:317)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:315)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:760)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:869)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:550)
	at org.springframework.context.annotation.AnnotationConfigApplicationContext.<init>(AnnotationConfigApplicationContext.java:88)
	at springfox.petstore.webflux.Server.applicationContext(Server.java:60)
	at springfox.petstore.webflux.Server.main(Server.java:21)
Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'java.util.List<org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping>' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.raiseNoMatchingBeanFound(DefaultListableBeanFactory.java:1509)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1104)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1065)
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:818)
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:724)
	... 15 more

So this is just a first step. Once that issue will be sorted out it will get to the problem that you described before - not able to find the correct PathProviderFactory - there are two so you get the appropriate exception about non unique bean:

Exception in thread "main" org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'documentationPluginsBootstrapper' defined in file [/Users/ligasgr/wrk/springfox/springfox-spring-web/out/production/classes/springfox/documentation/spring/web/plugins/DocumentationPluginsBootstrapper.class]: Unsatisfied dependency expressed through constructor parameter 6; nested exception is org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'springfox.documentation.spring.web.paths.PathProviderFactory' available: expected single matching bean but found 2: webFluxRelativePathProviderFactory,webMvcRelativePathProviderFactory
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:732)
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:197)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1276)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1133)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:543)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:503)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:317)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:315)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:760)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:869)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:550)
	at org.springframework.context.annotation.AnnotationConfigApplicationContext.<init>(AnnotationConfigApplicationContext.java:88)
	at springfox.petstore.webflux.Server.applicationContext(Server.java:60)
	at springfox.petstore.webflux.Server.main(Server.java:21)
Caused by: org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'springfox.documentation.spring.web.paths.PathProviderFactory' available: expected single matching bean but found 2: webFluxRelativePathProviderFactory,webMvcRelativePathProviderFactory
	at org.springframework.beans.factory.config.DependencyDescriptor.resolveNotUnique(DependencyDescriptor.java:215)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1116)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1065)
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:818)
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:724)
	... 15 more

Btw. I now know that in situation when e.g. you use embedded tomcat as per instructions from webflux you still will not have the ServletContext available in the context so the factory fails:

org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'javax.servlet.ServletContext' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}

If you exclude the WebMvcRelativePathProviderFactory from component scanning the context will start up fine and work fine.

If you have both webmvc and webflux on your classpath and you're trying to use spring-boot-starter-webflux the context will not start up as it will complain about various reasons depending on your config but an example will be:

org.springframework.context.ApplicationContextException: Unable to start web server; nested exception is org.springframework.context.ApplicationContextException: Unable to start ServletWebServerApplicationContext due to missing ServletWebServerFactory bean.
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.onRefresh(ServletWebServerApplicationContext.java:155) ~[spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:544) ~[spring-context-5.0.7.RELEASE.jar:5.0.7.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:140) ~[spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759) [spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:395) [spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:327) [spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1255) [spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1243) [spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at springfox.petstore.webflux.AppConfig.main(AppConfig.java:35) [classes/:na]
Caused by: org.springframework.context.ApplicationContextException: Unable to start ServletWebServerApplicationContext due to missing ServletWebServerFactory bean.
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.getWebServerFactory(ServletWebServerApplicationContext.java:204) ~[spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.createWebServer(ServletWebServerApplicationContext.java:178) ~[spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.onRefresh(ServletWebServerApplicationContext.java:152) ~[spring-boot-2.0.3.RELEASE.jar:2.0.3.RELEASE]
	... 8 common frames omitted

If you manually create a context without involving spring-boot-starter-webflux and use spring-boot-autoconfigure then your context will start up fine even when you have webmvc and webflux on the classpath.

But the moment you have both @EnableSwagger2WebFlux and @EnableSwagger2WebMvc in your config you will start getting exception like:

Exception in thread "main" org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'webHandler' available
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanDefinition(DefaultListableBeanFactory.java:686)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getMergedLocalBeanDefinition(AbstractBeanFactory.java:1210)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:291)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:204)
	at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1095)
	at org.springframework.web.server.adapter.WebHttpHandlerBuilder.applicationContext(WebHttpHandlerBuilder.java:161)
	at springfox.petstore.webflux.Server.applicationContext(Server.java:59)
	at springfox.petstore.webflux.Server.main(Server.java:17)

Hope this helps answering your questions...
I don't think that it is generally a good idea to mix the two configurations (webmvc and webflux) in one app and I can't really imagine a use case for that.
If you can come up with a reasonable sample app the works fine normally (without springfox) that uses both webmvc and webflux then I can have a go at making it work for springfox.

Thanks,
Grzegorz

@ligasgr ligasgr mentioned this pull request Aug 19, 2018
@IsaacDeLaRosa
Copy link

Hi! thank you so much for supporting webflux!
I am trying to auto generate reactive/reactor classes using springfox.
I added the required dependencies and the @EnableSwagger2WebFlux to my Configuration class. how and where can I define if the auto gen controller will be a Mono or Flux for example? because right now its neither of them.

@dilipkrish
Copy link
Member

@IsaacDeLaRosa I'll write up the documentation in a bit, but you would also need to add the spring-web dependency if you havent already added it. What are you seeing can you elaborate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants