Skip to content

Declare CloudEvent generic to allow automatic payload deserialization using Jackson#591

Merged
artursouza merged 6 commits into
dapr:masterfrom
mthmulders:typed-pubsub-payload
Aug 13, 2021
Merged

Declare CloudEvent generic to allow automatic payload deserialization using Jackson#591
artursouza merged 6 commits into
dapr:masterfrom
mthmulders:typed-pubsub-payload

Conversation

@mthmulders

Copy link
Copy Markdown
Contributor

Description

I've made the CloudEvent class a generic class, meaning its instance variable data is now of a generic type <T>. This allows Spring Boot controller methods to accept an argument CloudEvent<MyPayload>, which will make the code inside that controller method a lot easier as the MyPayload object will automatically be populated from a JSON payload.

At runtime, this generic type <T> resorts to Object, which I believe makes this backward compatible. The existing tests are all green which supports this claim.

Also, I did a small refactoring in the Spring Boot Rest Controller that the PubSubIT uses to make it easier to maintain.

Issue reference

This closes #576.

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@mthmulders mthmulders requested review from a team as code owners August 1, 2021 10:35
@mthmulders mthmulders force-pushed the typed-pubsub-payload branch from eb8f645 to c7dd270 Compare August 1, 2021 10:37
//Creating the DaprClient: Using the default builder client produces an HTTP Dapr Client
try (DaprClient client = new DaprClientBuilder().build()) {
for (int i = 0; i < NUM_MESSAGES; i++) {
CloudEvent cloudEvent = new CloudEvent();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required? Changes must be backwards compatible, which means that code without generics should still work as-is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not required, in the sense that it will also work without specifying the generic type. You can see that in the PubSubIT, where even the published message is declared without a generic type. On the consuming side, it can be consumed as a CloudEvent or as a CloudEvent<PubSubIT.MyObject>. See SubscriberController for an example of that.

I thought it would make sense to update the documentation. Since this file is in the examples/ folder, I consider it part of the documentation, and hence decided to update it.

@artursouza artursouza self-assigned this Aug 4, 2021

@artursouza artursouza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I can merge this after ITs are passing. Please, take a look.

Rather than returning `null` when no
messages are present for a topic.
@mthmulders

Copy link
Copy Markdown
Contributor Author

LGTM. I can merge this after ITs are passing. Please, take a look.

Welp, must've missed that. Apologies. I've updated the helper REST controller to always return a list, even if it's empty, rather than returning null when no messages are present for a topic.

@codecov

codecov Bot commented Aug 12, 2021

Copy link
Copy Markdown

Codecov Report

Merging #591 (8ab9b18) into master (df54a5d) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #591   +/-   ##
=========================================
  Coverage     81.03%   81.03%           
  Complexity      991      991           
=========================================
  Files            88       88           
  Lines          3042     3042           
  Branches        332      332           
=========================================
  Hits           2465     2465           
  Misses          409      409           
  Partials        168      168           
Impacted Files Coverage Δ
...rc/main/java/io/dapr/client/domain/CloudEvent.java 65.51% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df54a5d...8ab9b18. Read the comment docs.

@artursouza artursouza merged commit ae5c2a1 into dapr:master Aug 13, 2021
@javageek79

Copy link
Copy Markdown

Hi @artursouza, was it on purpose, that this issue has not been added to any milestone? I was happily supprised when I realised, that with 1.3.0 we now have a typed CloudEvent :-) Are there more issues released by 1.3.0 or 1.3.1?

fackee pushed a commit to fackee/java-sdk that referenced this pull request Mar 28, 2022
… using Jackson (dapr#591)

* Declare CloudEvent generic

* Fix timestamp in logging

* Declare Spring Boot version once

* Simplify administration of received events

* Update docs

* Always return a list, even if it's empty

Rather than returning `null` when no
messages are present for a topic.

Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
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

Successfully merging this pull request may close these issues.

Automatically unwrap pub/sub CloudEvent

3 participants