Skip to content

Spring integration #142

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

Closed
slinkydeveloper opened this issue Apr 28, 2020 · 25 comments
Closed

Spring integration #142

slinkydeveloper opened this issue Apr 28, 2020 · 25 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@slinkydeveloper
Copy link
Member

Given the ongoing work on jaxrs implementation, what it takes to integrate with spring?

@slinkydeveloper
Copy link
Member Author

Ping @bsideup

@bsideup
Copy link
Contributor

bsideup commented Apr 28, 2020

cc @olegz (Spring Cloud)

@olegz
Copy link
Contributor

olegz commented Apr 28, 2020

I guess there should be clarification on what type of integration you are looking for. Spring provides variety of ways to expose user functionality as REST endpoints. Some of them require user to no configuration at all. I can certainly elaborate further but it would help if more specifics are shared

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented Apr 28, 2020

The Jax-Rs integration will consists of a @Provider implementing MessageBodyReader and MessageBodyWriter.

@dsyer
Copy link
Contributor

dsyer commented Apr 28, 2020

There are many equivalents to MessageBodyReader and MessageBodyWriter in Spring (depending on blocking or non-blocking, HTTP, AMQP, Kafka, JMS, etc.). I think rather than try and implement them all here it would be better just to present a clean, stable API.

@slinkydeveloper
Copy link
Member Author

@olegz @dsyer draft of the jaxrs implementation is here: #145

Can it work OOTB with Spring?

@dsyer
Copy link
Contributor

dsyer commented Apr 29, 2020

Anyone who used JAX-RS with Spring I imagine could register the @Provider - I think that's the way it works with Spring Boot. The MessageUtils looks useful as well for internal use in Spring maybe (I see you used it in the Kafka binding already).

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented May 4, 2020

I've merged the jax-rs integration https://github.com/cloudevents/sdk-java/tree/master/http/restful-ws

Let me know if this is enough to work OOTB with Spring or if we need something else 😄 . If we want to leverage some features specific to Spring, we can definitely have a module like http/spring

@bsideup
Copy link
Contributor

bsideup commented May 4, 2020

@slinkydeveloper when you're saying "work OOTB with Spring" - do you mean that this is enough to call it "Spring support", or that, when Spring's JAX-RS integration is used, the module works?
Because Spring is not based on JAX-RS (but there are options to use JAX-RS with Spring)

edit: @slinkydeveloper's comment was edited and mine makes little sense now, but I will keep it here for clarity, because there seem to be some misconception

@slinkydeveloper
Copy link
Member Author

Because Spring is not based on JAX-RS (but there are options to use JAX-RS with Spring)

Ah ok, please forget my ignorance about that 😄

@cmoulliard
Copy link

For Spring, then we should add a new module within this SDK implementing Spring Web- https://github.com/spring-projects/spring-framework/tree/master/spring-web

@slinkydeveloper
Copy link
Member Author

@cmoulliard are you willing to give a try to it? 😄

@bsideup
Copy link
Contributor

bsideup commented Jul 15, 2020

@slinkydeveloper I would highly recommend to contact the Spring team before proceeding, to ensure that what you're doing here is aligned with what will be the official CloudEvents support in Spring.

@cmoulliard
Copy link

what will be the official CloudEvents support in Spring.

Where such official CloudEvents vision is defined and communicated for the spring users/developers ? @bsideup

@olegz
Copy link
Contributor

olegz commented Jul 15, 2020

@cmoulliard at the moment it is not very clear as to what does Spring support even mean, since Spring portfolio consists of many projects. As for communication . . as an example, we have request in spring-cloud-function to add Cloud Events support which most likely be a simply translation layer between Spring Message and CloudEvent, since their structures are pretty much identical.

Basically IMHO I don't think the terminology such as "CloudEvents support in Spring" or "Spring Integration" - (which is also the name of one of Spring's frameworks) is appropriate or meaningful without further clarifying which specific part of Spring one is referring to.

@cmoulliard
Copy link

support

I used the word vision and not support at all ;-) @olegz

@slinkydeveloper
Copy link
Member Author

As for communication . . as an example, we have request in spring-cloud-function to add Cloud Events support which most likely be a simply translation layer between Spring Message and CloudEvent, since their structures are pretty much identical.

Is there any reason why we can't have this integration inside this community project (similar to how we've done for kafka, jax-rs and vertx)? It sounds a pretty lightweight integration that can live under some cloudevents-spring module or similar

@olegz
Copy link
Contributor

olegz commented Jul 15, 2020

@cmoulliard i was referring to the "quoted text" - what will be the official CloudEvents support in Spring.

@salaboy
Copy link

salaboy commented Jul 20, 2020

@cmoulliard @slinkydeveloper I've just created a PR with some Spring Boot (Web and WebFlux) Helpers -> #201
I will be happy to send that to Spring Boot / Spring Cloud -> @spencergibb @cmoulliard @olegz any idea where?

@olegz
Copy link
Contributor

olegz commented Jul 20, 2020

As it was suggested by Dave earlier, there are many types of abstractions in spring representing different types of readers and writers specific to the context of current execution etc. And then of course there are many projects where CloudEvents may be relevant, so I am not sure how feasible it is to manage all of them in isolation from where they are most relevant to be used.
Also, as it was mentioned before, CloudEvent is pretty much identical to Spring Message; All Spring projects understand Spring Message, so all we really need to do is to provide implementation of MessageConverter to convert CloudEvent to/from Message. Basically instead of making every Spring bit understand CloudEvent which may result in much greater effort with a lot of duplication, we provide an implementation of MessageConverter which Spring will pick up automatically and user would not even have to know that they are dealing with CloudEvents. In fact we've had the same approach for Message making it more of an option for user to use while it is still used internally.

@salaboy
Copy link

salaboy commented Jul 20, 2020

@olegz that makes a lot of sense to me .. I've provide a simple helper to get people started.. we can change the implementation to use Spring Message underneath later. I want to make sure that we unblock spring users to start using Cloud Events asap.. Do you think that is worth using Spring Message even for these simple helpers?

@olegz
Copy link
Contributor

olegz commented Jul 20, 2020

@salaboy I understand what you're trying to do and unfortunately I don't have a clear answer at the moment. That is perhaps the reason for Dave's earlier comment about simply "ensuring stable API":

There are many equivalents to MessageBodyReader and MessageBodyWriter in Spring (depending on blocking or non-blocking, HTTP, AMQP, Kafka, JMS, etc.). I think rather than try and implement them all here it would be better just to present a clean, stable API.

May be it is better to to let individual spring projects to handle it. As I mentioned before we have an open issue in spring-cloud-function which most likely will result in the implementation of the MessageConverter and then the question would be to push/migrate such implementation up to Spring Messaging so every projects can benefit from it automatically without user doing anything at all.

@cmoulliard
Copy link

As I mentioned before we have an open issue in spring-cloud-function which most likely will result in the implementation of the MessageConverter

Such MessageConverter implementation should not be tied to the function project as Cloud Events will be used from web, webflux applications too

@rstoyanchev
Copy link

rstoyanchev commented Jul 23, 2020

Looking at PR #201, this is not an idiomatic way to integrate. I would suggest implementing an HttpMessageConverter<CloudEvent> instead and then it becomes:

    @PostMapping
    public String recieveCloudEvent(CloudEvent cloudEvent) {
        // ...
    }

Or:

CloudEvent cloudEvent = ... 
restTemplate.postForEntity(url, cloudEvent, String.class)

For WebFlux, it would be a pair of Encoder<CloudEvent> and Decoder<CloudEvent> implementations that would allow the same server side signature and the below with WebClient:

CloudEvent cloudEvent = ...
webClient.post().uri(url).bodyValue(cloudEvent).retrieve()

@olegz
Copy link
Contributor

olegz commented Jul 23, 2020

@cmoulliard
The role function project plays in the context of this thread is only as the origin of the issue to support CloudEvents in Spring.
The MessageConverter approach is in no way tied to function or any other Spring project and rather a general purpose solution to provide ability to turn CloudEvent into Spring Message and vice versa. And of course as @rstoyanchev suggested the HttpMessageConverter, Encoder and Decoder would play the same role for various different style of web request.

@slinkydeveloper slinkydeveloper added enhancement New feature or request hacktoberfest help wanted Extra attention is needed labels Sep 28, 2020
@slinkydeveloper slinkydeveloper added this to the 2.0.0.RC2 milestone Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants