RUM-1654 Add resourceId to ImageWireframe#1690
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
b4016ad to
6e3e917
Compare
6e3e917 to
f570425
Compare
f570425 to
c4f1807
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #1690 +/- ##
===========================================
- Coverage 83.32% 83.24% -0.08%
===========================================
Files 466 467 +1
Lines 16192 16238 +46
Branches 2413 2425 +12
===========================================
+ Hits 13491 13517 +26
- Misses 2043 2052 +9
- Partials 658 669 +11
|
| base64SerializerCallback: Base64SerializerCallback? | ||
| ) { | ||
| if (hash.isNotEmpty()) { | ||
| wireframe.resourceId = hash |
There was a problem hiding this comment.
So what backend does if there is no resourceId, but other payload like base64, etc. is there. How resourceId is used?
There was a problem hiding this comment.
at the moment backend is using base64, and eventually will map rssId as the key to pull the binary for use in the replays. In the future when the rss endpoint is done, there will have to be some logic there for ending support for b64 and moving over entirely to rssId (we'll start to send b64 as an empty field in the wireframes and only populate rssId)
There was a problem hiding this comment.
So what will be our logic then if we cannot generate resourceId? We won't send a resource itself then? This 99.999% won't happen (all devices for sure can generate MD5 hash), but I'm just curious.
There was a problem hiding this comment.
If we can't generate the hash then I think we'll need to throw away the binary because backend won't be able to use it in a replay. We can check for this in the future by having the rssId inside the recordedDataQueueItem holding the binary and marking it as invalid if the field isn't populated, then the dataQueueHandler can purge the record
c4f1807 to
7bc7710
Compare
e8da3bb to
3031a68
Compare
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
lgtm! I left some comments, but they are not blocking.
| shouldCacheBitmap = shouldCacheBitmap | ||
| ) | ||
| val resourceId = md5HashGenerator.generate(byteArray) | ||
| val hashes = Pair(base64String, resourceId) |
There was a problem hiding this comment.
To be precise, base64 is not a hash, but encoding.
2ed95ce to
b76e2f4
Compare
7 participants
What does this PR do?
Adds the resourceId property to ImageWireframe, and additionally removes base64 from the MutationResolver diffing.
Motivation
Removing base64 from diffing improved performance on ios and in this PR we align on this change. Additionally, resourceId is a necessary prerequisite for the resource endpoint change.
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)