PRISM-10044 - Add Risk package

precious-ossai-cko commented 7 months ago

Issue

https://checkout.atlassian.net/browse/PRISM-10044

Proposed changes

Add Risk package to Frames

Test Steps

If there's any functionality change, please list a step by step guide of how to verify the changes, and/or upload a screen recording for any visible changes.

For instance:

Go to the main screen

Tap on primary button

Verify the image is shown on the screen

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.

[ ] Reviewers assigned
[ ] I have performed a self-review of my code and manual testing
[ ] Lint and unit tests pass locally with my changes
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if applicable)

Further comments

fabio-insolia-cko commented 7 months ago

@precious-ossai-cko This build is currently failing our CI. Some of the compiling issues can be solved in the EventLogger.kt file by changing the following

override fun resetSession() {
        val correlationId = UUID.randomUUID().toString()
        logger.addMetadata(MetadataKey.correlationId, correlationId)
        sentLogs.clear()
    }

Other issues that I found are related to ktlintCheck here

fabio-insolia-cko commented 6 months ago

@jheng-hao-lin-cko @chintan-soni-cko We had a couple of tests that, after the Risk implementation, failed because of kotlinx.coroutines.test.UncompletedCoroutinesError at TestBuilders.kt:349. While I was not able to associate this misbehaviour between the tests and the Risk library, I have added an UnconfinedTestDispatcher for those failing unit tests and now they should work, however, I want to hear your opinion about this fix.

jheng-hao-lin-cko commented 6 months ago

@jheng-hao-lin-cko @chintan-soni-cko We had a couple of tests that, after the Risk implementation, failed because of kotlinx.coroutines.test.UncompletedCoroutinesError at TestBuilders.kt:349. While I was not able to associate this misbehaviour between the tests and the Risk library, I have added an UnconfinedTestDispatcher for those failing unit tests and now they should work, however, I want to hear your opinion about this fix.

Have a quick check, looks like those test failed given the fact that Kotlin coroutines version is upgraded (1.4.2 -> 1.7.3) by the event logger 2.0.2, and the original composed tests wasn't applied anymore.

Currently Risk SDK uses event logger 2.0.2, while Frames uses 1.0.1, they are using different version of coroutines.

I would suggest to update the Risk SDK to use event logger 1.0.1, so it's compitable with Frames without any issue :)

jheng-hao-lin-cko commented 6 months ago

Here is some sample code to increase unit tests for RiskSdkUseCase

public class RiskSdkUseCaseTest {
    private val environment: RiskEnvironment = RiskEnvironment.SANDBOX
    private val riskInstanceProvider: RiskInstanceProvider = mockk()
    private val riskInstance: Risk = mockk()
    private val context: Context = mockk()
    private val tokenDetails: TokenDetails = mockk()
    private lateinit var useCase: RiskSdkUseCase

    @BeforeEach
    public fun setup() {
        coEvery { tokenDetails.token } returns TOKEN
        coEvery { riskInstanceProvider.provide(context, PUBLIC_KEY, environment) } returns riskInstance
        useCase = RiskSdkUseCase(
            environment = Environment.SANDBOX,
            context = context,
            publicKey = PUBLIC_KEY,
            riskInstanceProvider = riskInstanceProvider,
        )
    }

    @Test
    public fun `Success result should trigger publishData`() {
        runBlocking {
            useCase.execute(TokenResult.Success(tokenDetails))
            coVerify { riskInstanceProvider.provide(context, PUBLIC_KEY, environment) }
            coVerify { riskInstance.publishData(TOKEN) }
        }
    }

    @Test
    public fun `Failed result should not trigger publishData`() {
        runBlocking {
            launch {
                useCase.execute(TokenResult.Failure(CheckoutError("error", "message")))
                coVerify(exactly = 0) { riskInstance.publishData(any()) }
            }
        }
    }

    private companion object {
        private const val PUBLIC_KEY: String = "pk_123"
        private const val TOKEN = "TOKEN"
    }
}

chintan-soni-cko commented 6 months ago

@jheng-hao-lin-cko @chintan-soni-cko We had a couple of tests that, after the Risk implementation, failed because of kotlinx.coroutines.test.UncompletedCoroutinesError at TestBuilders.kt:349. While I was not able to associate this misbehaviour between the tests and the Risk library, I have added an UnconfinedTestDispatcher for those failing unit tests and now they should work, however, I want to hear your opinion about this fix.

Have a quick check, looks like those test failed given the fact that Kotlin coroutines version is upgraded (1.4.2 -> 1.7.3) by the event logger 2.0.2, and the original composed tests wasn't applied anymore.

Currently Risk SDK uses event logger 2.0.2, while Frames uses 1.0.1, they are using different version of coroutines.

I would suggest to update the Risk SDK to use event logger 1.0.1, so it's compitable with Frames without any issue :)

+1 to @jheng-hao-lin-cko for Logger, we intentionally used an older version and not 2.0.2 because 2.0.2 does have Certificate transparency which we don't require for frames, resulting in benefits by having reduced SDK size.

jheng-hao-lin-cko commented 6 months ago

https://github.com/checkout/frames-android/actions/runs/8204617782/job/22439672388?pr=264

Run ./gradlew :checkout:ktlint locally to verify whether the lint has been fixed or how to fix

opt + cmd + L should auto-format the code for the most cases

fabio-insolia-cko commented 6 months ago

@precious-ossai-cko please fix the Ktlint issues by doing opt + cmd + L on the file that is giving issues

or doing it manually here is the list

Task :checkout:ktlint


/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:61:1: Unexpected indentation (16) (should be 12) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:62:1: Unexpected indentation (20) (should be 16) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:63:1: Unexpected indentation (20) (should be 16) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:64:1: Unexpected indentation (20) (should be 16) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:65:1: Unexpected indentation (20) (should be 16) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:66:1: Unexpected indentation (16) (should be 12) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:68:1: Unexpected indentation (16) (should be 12) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:69:1: Unexpected indentation (20) (should be 16) (standard:indent)
/home/runner/work/frames-android/frames-android/checkout/src/main/java/com/checkout/CheckoutApiServiceFactory.kt:70:1: Unexpected indentation (16) (should be 12) (standard:indent)