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

Add additional CloudEvent fields (pubsubname, topic, time, etc) #866

Merged
merged 23 commits into from
Jan 11, 2024
Merged

Add additional CloudEvent fields (pubsubname, topic, time, etc) #866

merged 23 commits into from
Jan 11, 2024

Conversation

siebenluke
Copy link
Contributor

@siebenluke siebenluke commented May 24, 2023

Description

Added additional CloudEvent fields (pubsubname, topic, time, traceid, traceparent, & tracestate)

Added the com.fasterxml.jackson:jackson-datatype-jsr310 dependency to handle serdes of OffsetDateTime for the CloudEvent time field via ObjectMapper settings .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false) & .findAndRegisterModules()

Updated com.fasterxml.jackson dependencies to the latest 2.15.1

Added OffsetDateTime as timeValue to test the DefaultObjectSerializer

Added more tests for new & old CloudEvent fields in CloudEventTest & DefaultObjectSerializerTest

Signed-off-by: Luke Sieben <siebenluke@gmail.com>

Issue reference

Add additional CloudEvent fields (pubsubname, topic, time, traceid, traceparent, & tracestate) per discussion in Discord chat

Checklist

  • [✓] Code compiles correctly
  • [✓] Created/updated tests
  • [N/A] Extended the documentation

@siebenluke siebenluke requested review from a team as code owners May 24, 2023 19:11
Copy link
Member

@artursouza artursouza left a comment

Choose a reason for hiding this comment

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

Please, sign-off to your commits so DCO can pass.

sdk/src/main/java/io/dapr/client/domain/CloudEvent.java Outdated Show resolved Hide resolved
sdk/src/main/java/io/dapr/client/domain/CloudEvent.java Outdated Show resolved Hide resolved
sdk/src/main/java/io/dapr/client/domain/CloudEvent.java Outdated Show resolved Hide resolved
@siebenluke
Copy link
Contributor Author

I will also look into how I can add a DCO sign-off to my commit(s).

… traceparent, & tracestate)

Added the com.fasterxml.jackson:jackson-datatype-jsr310 dependency to handle serdes of OffsetDateTime for the CloudEvent time field via ObjectMapper settings .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false) & .findAndRegisterModules()

Updated com.fasterxml.jackson dependencies to the latest 2.15.1

Added OffsetDateTime as timeValue to test the DefaultObjectSerializer

Added more tests for new & old CloudEvent fields in CloudEventTest & DefaultObjectSerializerTest

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
Removed the 2 new constructors

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
…ames as JSON/OBJECT_MAPPER are case-sensitive

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
@siebenluke siebenluke requested a review from artursouza June 8, 2023 20:22
@siebenluke siebenluke requested a review from artursouza July 11, 2023 21:11
…ency in favor of custom field level serdes for time

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
artursouza
artursouza previously approved these changes Aug 15, 2023
… end of the offending lines to a new line

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
@cicoyle
Copy link
Contributor

cicoyle commented Dec 21, 2023

@siebenluke - mind resolving the conflicts? Then I think we can merge this

Signed-off-by: Luke Sieben <siebenluke@gmail.com>
@siebenluke
Copy link
Contributor Author

Hi @cicoyle / @artursouza, I have resolved the conflicts, so if someone (Artur) could take another look that would be great. Thanks.

artursouza
artursouza previously approved these changes Dec 22, 2023
@artursouza artursouza added this to the v1.11 milestone Dec 22, 2023
@siebenluke
Copy link
Contributor Author

siebenluke commented Jan 5, 2024

Hi @cicoyle / @artursouza, Happy New Years, I believe I've added all the necessary CloudEvent test cases in 78e9575 to appease Codecov. Let me know if there are any other additional changes I need to make before this can be merged.

@siebenluke siebenluke requested a review from artursouza January 5, 2024 20:37
artursouza
artursouza previously approved these changes Jan 5, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3dc2a90) 76.29% compared to head (e6237bb) 76.85%.

Files Patch % Lines
...rc/main/java/io/dapr/client/domain/CloudEvent.java 96.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #866      +/-   ##
============================================
+ Coverage     76.29%   76.85%   +0.56%     
- Complexity     1463     1497      +34     
============================================
  Files           138      138              
  Lines          4497     4524      +27     
  Branches        524      530       +6     
============================================
+ Hits           3431     3477      +46     
+ Misses          784      773      -11     
+ Partials        282      274       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dapr-bot dapr-bot merged commit fdb4200 into dapr:master Jan 11, 2024
17 checks passed
@siebenluke siebenluke deleted the add-additional-cloudevent-fields branch January 11, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants