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

Provide a WebMvc.fn / WebFlux.fn functional DSL #891

Closed
sdeleuze opened this issue Oct 6, 2020 · 15 comments
Closed

Provide a WebMvc.fn / WebFlux.fn functional DSL #891

sdeleuze opened this issue Oct 6, 2020 · 15 comments
Labels
enhancement New feature or request

Comments

@sdeleuze
Copy link

sdeleuze commented Oct 6, 2020

Hey,

Thanks for creating this project. As a follow-up of #546 created by @nlochschmidt, where you added support for Spring web functional DSLs with a mix of Visitor patterns and annotations like @RouterOperations and @RouterOperation, I am wondering if you would be interested to explore the possibility to provide an alternative functional API to these annotations that could be used with the router DSL.

I am not an OpenAPI guru, but I tend to think that programmatic APIs and method references could allow springdoc-openapi to provide an API more consistent with WebMvc.fn / WebFlux.fn mindset, with more compile-time checks and avoiding mixing too much annotation and functional paradigms. That would also allow functional Spring applications like the one incubated in https://github.com/spring-projects-experimental/spring-fu that I created to use springdoc-openapi.

I am not sure yet what it would look like, but if you are interested to move forward this idea, I will be happy to provide some insights on the Spring Framework side of things, on Kotlin related DSLs, or even discuss potential missing extension points. It could also be potentially interesting to compare external versus "in the router" OpenAPI meta information definition.

Any thoughts?

@bnasslahsen
Copy link
Contributor

Hi @sdeleuze (Ou bien plutôt bonsoir Sébastien!),

The @RouterOperation annotations are the simplest workaround that we find out, to answer the rising demand to support the functional approach:

  • This annotations allow developers to add some OpenAPI metadata to describe their REST endpoints.

Ideally, the springdoc-openapi should be only the bridge between spring-projects and OpenAPI 3 projects: Without even adding extra annotations for the developer:

  • This is quite simple in spring-mvc/webflux projects that relies on the annotations (@RestController, ...). We have all the information available to generate the documentation even the developer doesn't add any OpenAPI annotation.
  • But as you know, it's much more difficult with the functional approach as the routes don't contain by definition all the metadata about their inputs/outputs: Lambdas are passed as arguments which makes introspection difficult to get their concrete inputs/outputs parameters.

I would say that providing an end to end functional API, will be more interesting for projects, choosing the functional paradigm because it:

  • Will make the life of the developer easier
  • Will make the code simpler to maintain and more consistent
  • Will make the maintenance of springdoc-openapi project simpler as well!

What we need is an extension to the existing router DSL, that allows developers to add more metadata (that are not available on the router defintion) about their REST endpoints, without any extra annotations.

And Ideally, even the developer doesn't add this metadata, it should be possible to generate automatically minimal API description as we propose on the projects that are not using the Functional Endpoints. (But i am not sure this point is quite simple to achieve)

It's a great pleasure to know that you are interested in the project.
I am open to discuss more in details this idea. If you need more inputs, i would be more than happy to help.

@poutsma
Copy link

poutsma commented Oct 9, 2020

Hi @bnasslahsen, I would also be willing to explore the idea of an extension of the router function model that adds the required metadata that you need. I wonder if you could show me, in pseudo-code, what you have in mind, i.e. what such an extension would look like?

@bnasslahsen
Copy link
Contributor

@poutsma,

Thank for your interest to this subject as well.

This a sample pseudo-code, even i would say that @sdeleuze would have better idea from functional API extension, point of view: The list of metadata used in this sample is not exhaustive.

@Bean
public RouterFunction<ServerResponse> routes(PostHandler postHandler) {
	return route(GET("/posts").and(queryParam("key1", "value1")).and(queryParam("key2", "value2"))
			.withMetadata("operation", new Operation().operationId("findPostsByKey1AndKey2"))
			.withMetadata("parameter", new Parameter().name("key1").description("My key1 description"))
			.withMetadata("parameter", new Parameter().name("key2").description("My key2 description"))
		        .withMetadata("response", new ApiResponse().responseCode("200").description("This is an operation description").type(Post.class))
			.withMetadata("response", new ApiResponse().responseCode("404").description("item not found"))
		, postHandler::all)
		.andRoute(PUT("/posts/{id}")
			.withMetadata("operation", new Operation().operationId("updatePost")
			.addParametersItem(new Parameter().name("id").in(ParameterIn.PATH.toString()).description("My id description"))
			.responses(new ApiResponses()
                           .addApiResponse("202", new ApiResponse().description("OK"))
			   .addApiResponse("404", new ApiResponse().description("item to update, not found"))))
		, postHandler::update);
	}

And if we would like the router defintion to be less verbose:

@Bean
public RouterFunction<ServerResponse> routes(PostHandler postHandler) {
	return route(GET("/posts").and(queryParam("key1", "value1")).and(queryParam("key2", "value2")).withMetadata("operation", findPostsByKey1AndKey2()), postHandler::all)
			.andRoute(PUT("/posts/{id}").withMetadata("operation", updatePost()), postHandler::update);
}

private Operation findPostsByKey1AndKey2() {
	return new Operation().operationId("findPostsByKey1AndKey2")
			.addParametersItem(new Parameter().name("key1").description("My key1 description"))
			.addParametersItem(new Parameter().name("key2").description("My key2 description"))
			.responses(new ApiResponses()
					.addApiResponse("200", new ApiResponse().description("This is an operation description").type(Post.class))
					.addApiResponse("404", new ApiResponse().description("item not found")));

}

private Operation updatePost() {
	return new Operation().operationId("updatePost")
			.addParametersItem(new Parameter().name("id").in(ParameterIn.PATH.toString()).description("My id description"))
			.responses(new ApiResponses()
					.addApiResponse("202", new ApiResponse().description("OK"))
					.addApiResponse("404", new ApiResponse().description("item to update, not found")));

}

@poutsma
Copy link

poutsma commented Oct 16, 2020

Hi @bnasslahsen. Thank you for providing that example. It seems that having route meta-data (or attributes) is a useful feature to have.

I have pushed a version of WebFlux.fn that adds the capability to define attributes for RouterFunction objects. The attributes are exposed via the visitor, just like other router function information. See the branch here: poutsma/spring-framework@abfbe20

I've also provided a sample how you (and others) can use this feature to expand on the router function builder, and add the required operation data as part of the function registration. The idea being that your users would use the builder API in springdoc that adds these features, instead of the standard builder API. This would allow your users to register routes like this:

RouterFunction<ServerResponse> routerFunction =
	route()
	.GET("/foo", request -> ServerResponse.ok().build(), ops -> ops
			.parameter("key1", "My key1 description")
			.parameter("key1", "My key1 description")
			.response(200, "This is normal response description")
			.response(404, "This is response description")
		)
	.build();

which I think is quite compelling.

Let me know what you think (but please do so quickly, so that we can get this in 5.3 GA, which releases in less than two weeks).

@bnasslahsen
Copy link
Contributor

bnasslahsen commented Oct 16, 2020

Great proposal @poutsma , which seems from a first point of view very promising.

The RouterFunctions API is getting more Open this way, which will be very useful for the community: It will help projects when they need to enhance the RouterFunction attributes (or metadata).

I will test the integration of your branch and get back to you, Sunday 18th October at the latest to confirm you the result.

This way, we can be ready, to release a new version of springdoc-openapi once the 5.3 GA, will be available, in order to support this great enhancement asap.

@bnasslahsen
Copy link
Contributor

@poutsma,

The integration is going well with simple routes.

Some of the springdoc-openapi tests are failing, because they are based on composed routing functions, with andRoute :

  • I was not able to link the attributes related to each route for this case.

I have reused your test RouterFunctionBuilderTests to show the problem:

  • The first test attributesWithoutAndRoute, shows that attributes are present for each routes, when andRoute is not used.
  • The second test attributesWithAndRoute, shows the difficulty of getting the attributes related to each route, when andRoute is used.

Can you confirm, how the attributes could be retrieved using andRoute?

In the meanwhile, i will have a look if there is another alternative.

@poutsma
Copy link

poutsma commented Oct 19, 2020

@bnasslahsen

The behavior you're seeing is a consequence of the way andRoute is implemented. Basically, it returns a composed router function, which then gets the withAttribute applied to it (instead of the GET /api/admin route). If you change the code to the following (i.e. change from andRoute to and(route ), it does work:

		RouterFunction<ServerResponse> routerFunction = RouterFunctions
				.route(GET("/api/user"), handlerFunction)
				.withAttribute("foo", "bar")
				.and(RouterFunctions.route(GET("/api/admin"), handlerFunction)
				.withAttribute("foo", "baz"));

I am currently not sure if this is indeed a bug, since it does follow logically from the router component model. But I do agree that it it does have strange consequences (as shown in your test), so maybe it does need fixing. I'll think about a fix for the next few days, and get back to you.

That said, I think that this issue is not a show-stopper, because there is a workaround and—more importantly—I don't think users will use this API directly. Instead, they use it through a wrapper, where usage is controlled: a custom static factory method and/or builder, similar to what RouterFunctions offers. I've tried to show this in the CustomRouteBuilder sample.

Do you agree that the API is still useful to have, despite the current andRoute behavior? I am asking because of the short time we have until 5.3 GA.

@bnasslahsen
Copy link
Contributor

@poutsma,

Thank your analysis and quick reply.

I confirm the utility of this new API:

  • I have started the integration and i can just tell you, that it's making the code much simpler, from the moment we can get additional attributes for each route directly. (There is a significant reduction of complexity)
    I frankly wish, this enhancement will make the developer experience more handy as @sdeleuze requested.

Just to share, i have tested moving withAttribute on RequestPredicates level instead of RouterFunctions, and it's working with andRoute as well:

  • This might be not the ideal design choice, but i thought it might give you a lead on the reason of the current behaviour of andRoute when withAttribute is on RouterFunctions level.

@poutsma
Copy link

poutsma commented Oct 19, 2020

Note that I created spring-projects/spring-framework#25938 to track this from Spring Framework's side of things.

I can see why attributes can appear to work while applying them on predicates, but I think it breaks down quickly when considering that not all routes have predicates. Some routes are composed, or just expose resources, without any use of predicates at all. So I don't think it fits the model properly, and attributes should be applied to routes, not predicates.

@poutsma
Copy link

poutsma commented Oct 23, 2020

The router function attributes support has been merged into master.

@bnasslahsen
Copy link
Contributor

@poutsma,

Great news! I have seen that you have also completed this feature on the webmvc side (servlet).
We will work on completing the integration, and let you know once it's completed.

@bnasslahsen
Copy link
Contributor

@poutsma, @sdeleuze,

I confirm the integration is successful, with the last spring master version.
This the link for some samples, how the integration will look like:

I will still have to make some final changes, but the most important ones are already done.

@sdeleuze
Copy link
Author

Awesome collaboration, thanks to you both @bnasslahsen and @poutsma.

@bnasslahsen
Copy link
Contributor

Thank you @sdeleuze and @poutsma for this nice feature.
All my wishes, that it will make the developer life easier and the integration smoother with WebMvc.fn / WebFlux.fn.

@BeneStem
Copy link

Hey there @sdeleuze.

Sorry to ask a question on this closed issue...

Where you able to configure spring-docs with spring-fu?

I am trying to do this as and I am not able to register the Beans manually with the spring-fu DSL.

beans {
    bean<SomeSpringDocConfigurationBean>()
}

And those are a lot of beans as well.

Do you know of some way to activate the autoconfiguration for certain beans in spring-fu again?

Need help :)

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

No branches or pull requests

4 participants