chore: use meta-data for user agent client #439
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/react-native-cdp #439 +/- ##
===========================================================
Coverage ? 51.88%
Complexity ? 289
===========================================================
Files ? 96
Lines ? 2253
Branches ? 352
===========================================================
Hits ? 1169
Misses ? 982
Partials ? 102 ☔ View full report in Codecov by Sentry. |
📏 SDK Binary Size Comparison ReportNo changes detected in SDK binary size ✅ |
|
Build available to test |
| <meta-data | ||
| android:name="io.customer.sdk.android.core.SDK_VERSION" | ||
| android:value="1.3.5" /> | ||
| </application> |
There was a problem hiding this comment.
This is only for tests. But this is how we can modify these values in wrapper SDKs.
| null | ||
| } | ||
| val appMetaData = applicationInfo?.metaData | ||
| val appMetaData = context.applicationMetaData() |
There was a problem hiding this comment.
Reused extension to fix deprecated warning
| @@ -43,8 +42,7 @@ abstract class BaseTest { | |||
| */ | |||
| private fun registerAndroidSDKComponent(testConfig: TestConfig) { | |||
| val application = testConfig.argumentOrNull<ApplicationArgument>()?.value ?: return | |||
| val client = testConfig.argumentOrNull<ClientArgument>()?.value ?: return | |||
There was a problem hiding this comment.
We now no longer need Client to register and initialize AndroidSDKComponent
| ignoreCase = true | ||
| ) -> Flutter(sdkVersion = sdkVersion) | ||
| else -> Other(source = source, sdkVersion = sdkVersion) | ||
| fun fromMetadata(metadata: Bundle?): Client { |
There was a problem hiding this comment.
what would happen if both android SDK and react native SDK manifest have same values? shouldn't we override or handle that case?
There was a problem hiding this comment.
Added values in common test to help verify manifest merger challenges
| import org.amshove.kluent.shouldBeEqualTo | ||
| import org.junit.Test | ||
|
|
||
| class AndroidManifestClientTest : AndroidTest() { |
There was a problem hiding this comment.
it would be great if we can test meta data merging here
There was a problem hiding this comment.
Added multiple manifests, test will not change because the changes have to be made when merging Manifest
core/src/main/kotlin/io/customer/sdk/core/extensions/ContextExtensions.kt
Outdated
Show resolved
Hide resolved
part of MBL-522
Changes
Clientclass out ofAndroidSDKComponentconstructor and made it a singular dependency, allowing for decouplingClientclass by removing sealed implementationClientclass to create user-agent client based on meta-data provided inAndroidManifestAndroidManifestand fixed the deprecated method warning forApplicationInfoClientArgumentfromTestArgumentas it is no longer neededBenefits of this approach
Clientis no longer dependent on SDK initializationClientwill always remain the same, regardless of which classes initialize the SDK and registerAndroidSDKComponentClientcan still be overridden inAndroidSDKComponentUser agent sent from Android SDK
User agent sent from React Native SDK (Pending changes)