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

Fix handling immutable collections on SentryEvent and protocol objects #1468

Merged
merged 10 commits into from
May 12, 2021

Conversation

maciejwalkowiak
Copy link
Contributor

@maciejwalkowiak maciejwalkowiak commented May 10, 2021

📜 Description

Fix handling immutable collections on SentryEvent and protocol objects. With this change, when immutable collection is set on any of SentryEvent field, it is still possible to add more entries to this collection on the event.

💡 Motivation and Context

Fixes #1357

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

@maciejwalkowiak maciejwalkowiak changed the title Do not allow replacing collections in SentryEvent. Fix handling immutable collections on SentryEvent and protocol objects May 12, 2021
@marandaneto
Copy link
Contributor

worth doing the same with Scope#setFingerprint, SentryValues, Device#archs, Mehanism#meta and data, Message#params, Request#headers, I mean, every List/Array/Map that are public and mutable and are exposed i nthe public API to swap the reference with an immutable type

@maciejwalkowiak maciejwalkowiak marked this pull request as ready for review May 12, 2021 09:48
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #1468 (32ae92f) into main (112c46d) will increase coverage by 0.21%.
The diff coverage is 83.78%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1468      +/-   ##
============================================
+ Coverage     75.62%   75.83%   +0.21%     
- Complexity     1908     1922      +14     
============================================
  Files           190      190              
  Lines          6551     6555       +4     
  Branches        659      659              
============================================
+ Hits           4954     4971      +17     
+ Misses         1280     1264      -16     
- Partials        317      320       +3     
Impacted Files Coverage Δ Complexity Δ
...ry/src/main/java/io/sentry/protocol/DebugMeta.java 33.33% <0.00%> (+22.22%) 3.00 <1.00> (+2.00)
.../src/main/java/io/sentry/util/CollectionUtils.java 54.54% <33.33%> (-7.96%) 8.00 <2.00> (+2.00) ⬇️
sentry/src/main/java/io/sentry/SentryEvent.java 74.19% <50.00%> (+6.45%) 32.00 <2.00> (+3.00)
sentry/src/main/java/io/sentry/Breadcrumb.java 92.00% <100.00%> (ø) 21.00 <0.00> (ø)
sentry/src/main/java/io/sentry/Scope.java 93.15% <100.00%> (ø) 71.00 <0.00> (ø)
...entry/src/main/java/io/sentry/SentryBaseEvent.java 81.57% <100.00%> (+2.63%) 38.00 <3.00> (+2.00)
sentry/src/main/java/io/sentry/SentryValues.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
sentry/src/main/java/io/sentry/SpanContext.java 91.89% <100.00%> (ø) 16.00 <0.00> (ø)
sentry/src/main/java/io/sentry/protocol/App.java 96.55% <100.00%> (ø) 18.00 <0.00> (ø)
...ntry/src/main/java/io/sentry/protocol/Browser.java 100.00% <100.00%> (ø) 8.00 <0.00> (ø)
... and 14 more

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 112c46d...32ae92f. Read the comment docs.

@maciejwalkowiak
Copy link
Contributor Author

Device#archs is an array, so i think there is nothing to be done there. The rest is done.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

if CI is happy, I am happy :)
Thanks @maciejwalkowiak

@marandaneto marandaneto merged commit 93f3c22 into main May 12, 2021
@marandaneto marandaneto deleted the gh-1357 branch May 12, 2021 12:25
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.

Applying immutable map in SentryEvent#setExtras causes UnsupportedOperationException
4 participants