RUM-2708: Enable Resource Endpoint by default#1858
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1858 +/- ##
===========================================
+ Coverage 83.32% 83.37% +0.05%
===========================================
Files 479 477 -2
Lines 16928 16875 -53
Branches 2527 2511 -16
===========================================
- Hits 14104 14069 -35
+ Misses 2135 2121 -14
+ Partials 689 685 -4
🚀 New features to boost your workflow:
|
4d660bf to
ea5f4a5
Compare
| } | ||
|
|
||
| internal data class CacheData(val base64Encoding: ByteArray, var resourceId: ByteArray?) { | ||
| internal data class CacheData(val resourceId: ByteArray) { |
There was a problem hiding this comment.
do we need a class with only single property? seems like we can get rid of it.
There was a problem hiding this comment.
Good point. Removing the base64 indeed removes our need for this container class
ef39f54 to
7dcc78c
Compare
ea5f4a5 to
7c2cf69
Compare
7dcc78c to
79cacc9
Compare
|
|
||
| @Suppress("ReturnCount") | ||
| @WorkerThread | ||
| private fun serializeBitmap( |
There was a problem hiding this comment.
why is this called serializeBitmap ?
There was a problem hiding this comment.
This is renamed in the next PR to resolveResourceHash in the context of the refactor
| return | ||
| } | ||
|
|
||
| if (!resourceIdsSeen.contains(resourceId)) { |
There was a problem hiding this comment.
shouldn't we do the check before ? Are we computing the byteArray for this resource for nothing if the resource was already seen ?
There was a problem hiding this comment.
We're calculating the md5 hash from the bytearray, so I'm not sure we can move this earlier in the flow
|
|
||
| @Suppress("TooManyFunctions") | ||
| internal class Base64Serializer private constructor( | ||
| internal class ResourcesSerializer private constructor( |
There was a problem hiding this comment.
I think this class should not be called a serializer ? I think the scope change into something different. For me this class has too many responsibilities now and it is so hard to track the logic inside. We are using it to check if the resource already exists in cache was already solved before and also in the same time to update the queue and mark the wireframe as resolved. These are 2 different responsibilities and should be extracted in separated components. Also there is nothing that suggests that this is a serializer for me here.
There was a problem hiding this comment.
I do not mind if we do this in another PR but I really think we need to do it now and simplify this logic a bit more.
There was a problem hiding this comment.
It's renamed in the next pr to ResourceResolver and some responsibilities are extracted out
7c2cf69 to
c162aea
Compare
c162aea to
7725929
Compare
7725929 to
098ea45
Compare
5 participants
What does this PR do?
• Renames
base64packages toresources• Renames resource classes
• Removes base64 from the lrucache
• Remove base64 from places where we populated it (now it will be null)
• Removes the Resource Endpoint feature flag.
NOTE: This is Part 1 of this pr. Part 2 will have everything related to the refactor/splitting of classes
Motivation
Final PR for the Resource Endpoint
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)