Adyen / adyen-android

Adyen Android Drop-in and Components
https://docs.adyen.com/checkout/android
MIT License
126 stars 66 forks source link

CardView crashes when attach method is called on CardComponent #83

Closed PiDyGB closed 5 years ago

PiDyGB commented 5 years ago

This is my demo implementation:

val cardConfiguration = CardConfiguration.Builder(
    Locale.getDefault(),
    resources.displayMetrics,
    Environment.TEST,
    "<publicKey>"
).build()

val response = PaymentMethodsApiResponse.SERIALIZER.deserialize(JSONObject(res))

val cardComponent = CardComponent.PROVIDER.get(this@MainActivity, response, cardConfiguration)

cardComponent.observe(this@MainActivity, Observer { state ->
    Log.d(MainActivity::class.java.simpleName, "CardComponent data: $state")
})

adyenCardView.attach(cardComponent, this@MainActivity)

but the CardView class raises an exception during the execution of the attach method.

2019-07-04 16:01:02.722 11625-11625/com.example.aydendemo E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.example.aydendemo, PID: 11625
    java.lang.NullPointerException: Attempt to invoke virtual method 'void com.adyen.checkout.card.CardListAdapter.setCards(java.util.List)' on a null object reference
        at com.adyen.checkout.card.CardView.updateNumber(CardView.java:188)
        at com.adyen.checkout.card.CardView.onChanged(CardView.java:167)
        at com.adyen.checkout.card.CardView.onChanged(CardView.java:44)
        at androidx.lifecycle.LiveData.considerNotify(LiveData.java:113)
        at androidx.lifecycle.LiveData.dispatchingValue(LiveData.java:126)
        at androidx.lifecycle.LiveData$ObserverWrapper.activeStateChanged(LiveData.java:424)
        at androidx.lifecycle.LiveData$LifecycleBoundObserver.onStateChanged(LiveData.java:376)
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:355)
        at androidx.lifecycle.LifecycleRegistry.addObserver(LifecycleRegistry.java:181)
        at androidx.lifecycle.LiveData.observe(LiveData.java:185)
        at com.adyen.checkout.base.component.BasePaymentComponent.observeOutputData(BasePaymentComponent.java:127)
        at com.adyen.checkout.card.CardComponent.observeOutputData(CardComponent.java:142)
        at com.adyen.checkout.card.CardView.attach(CardView.java:232)
        at com.example.aydendemo.MainActivity$onCreate$1.accept(MainActivity.kt:76)
        at com.example.aydendemo.MainActivity$onCreate$1.accept(MainActivity.kt:25)
        at io.reactivex.internal.observers.ConsumerSingleObserver.onSuccess(ConsumerSingleObserver.java:62)
        at io.reactivex.internal.operators.single.SingleObserveOn$ObserveOnSingleObserver.run(SingleObserveOn.java:81)
        at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:124)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

Seems that the CardListAdapter is used before have been initialized.

caiofaustino commented 5 years ago

That's weird. Doesn't the compiler throw you an error?

It seems like you are passing the whole API response to the component, when it should be just the PaymentMethod for the Card, which is called 'scheme'.

Something like this:

for (pm in response.paymentMethods!!) {
            if (pm.type == PaymentMethodTypes.SCHEME) {
                val cardComponent = CardComponent.PROVIDER.get(this@MainActivity, pm, cardConfiguration)
            }
        }

Other than that, can you try to attach before adding the observer and see if that fixes it? If it does, we will work on a fix for the next release.

PiDyGB commented 5 years ago

Sorry @caiofaustino , I missed a piece of code, I report here the complete one:

val cardConfiguration = CardConfiguration.Builder(
    Locale.getDefault(),
    resources.displayMetrics,
    Environment.TEST,
    "<publicKey>"
).build()

val response = PaymentMethodsApiResponse.SERIALIZER.deserialize(JSONObject(res))
//when res is the /paymentMethods POST response
response.paymentMethods?.firstOrNull { it.type == PaymentMethodTypes.SCHEME }?.let {
    val cardComponent = CardComponent.PROVIDER.get(this@MainActivity, it, cardConfiguration)

    adyenCardComponent.attach(cardComponent, this@MainActivity)

    cardComponent.observe(this@MainActivity, Observer { state ->
        Log.d(MainActivity::class.java.simpleName, "CardComponent data: $state")
    })
}

This is why the compiler doesn't report an error, but, unfortunately, an NPE runtime exception is raised instead, also if I move the attach method before observe.

I don't know if this can help, but, decompiling the SDK, I noticed that into the attach method this

this.mComponent.observeOutputData(lifecycleOwner, this);

is called before this

this.mCardListAdapter = new CardListAdapter(this.mComponent);

but the first one will immediately deliver an onChanged(@NonNull CardOutputData cardOutputData) event and here the exception in raised (onChanged -> updateNumber -> this.mCardListAdapter.setCards).

caiofaustino commented 5 years ago

Ah ok, that's right this was a stupid mistake, thanks for the catch. We are creating the view programmatically on our test so this issue didn't show up. We will fix this for the next release.

Also, the code is open source here, you don't need to decompile to look at it :)

caiofaustino commented 5 years ago

Should be fixed on rc02, @PiDyGB can you check?

PiDyGB commented 5 years ago

Hi @caiofaustino ,

I can confirm that now seems working as expected.