checkout / frames-android

Frames Android: making native card payments simple
https://www.checkout.com/docs/integrate/sdks/android-sdk
MIT License
47 stars 32 forks source link

PIMOB:2459 - Added moshi codegen dependency #266

Closed chintan-soni-cko closed 3 months ago

chintan-soni-cko commented 3 months ago

Issue

PIMOB-2459

Proposed changes

Context: Merchant issue Error: java.lang.IllegalArgumentException: No property for required constructor parameter #0 p0 of fun(kotlin.String, kotlin.String, kotlin.String, kotlin.String, kotlin.String?, kotlin.String?, com.checkout.tokenization.entity.AddressEntity?, com.checkout.tokenization.entity.PhoneEntity?)

Problem/Merchant issue: Starbuck is migrating from legacy Frames to the latest version. However, while migrating they use Dexguard to generate the signed apk. The problem is when they obfuscate their code using it, results in for the app not being able to find the network models from the SDK which are generated by Moshi using @JsonClass(generateAdapter = true). This annotation is doing the serialisation/deserialisation of the objects while processing network calls and we don't need to do it manual parsing. The main reason of the crash is that we are not using Moshi codegen reflection which has the functionality to not remove those generated classes during obfuscation instead we are using kotlin serialisation using Moshi.

References to understand the issues: Moshi developers introduce new improvements using moshi reflection library as codegen to not remove generated adapters during obfuscation. Reference Moshi PR

Trial and error solution: Initially we tried different solutions comparing this PR which were validated by me and Starbuck (reference app to reproduce the issue shared by a merchant). Explaining it, we have implemented manual the serialisation/deserialisation of network objects which does not take part in kotlin reflections and also adds runtime cost to perform it. Reference branch

Final solution: In this PR, we tried to optimise the code and extra parsing operations we were performing in the previous solution by adding a new library as codegen for reflection and updating the moshi version.

Test Steps

I have validated the below snapshot versions in the merchant sample app and it's working fine. Additionally, I will do full regression testing going forward. implementation("com.github.checkout.frames-android:frames-android:c8a870c")

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you choose the solution you did and what alternatives you considered, etc...

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud