From 2c183617a0338bf0dab85f18700dacb285088515 Mon Sep 17 00:00:00 2001 From: Gaurav Ujjwal Date: Tue, 26 Dec 2023 11:19:54 +0530 Subject: [PATCH] Refactor LiveEvent It now uses more composable approach with LiveData as data member instead of as base class - Much more clear implementation semantics - Clear & concise API - Added support for removeObserver() - Easier to test --- .../gaurav/avnc/viewmodel/LiveEventTest.kt | 71 ++++++------ .../com/gaurav/avnc/viewmodel/LiveEvent.kt | 103 +++++++----------- 2 files changed, 81 insertions(+), 93 deletions(-) diff --git a/app/src/androidTest/java/com/gaurav/avnc/viewmodel/LiveEventTest.kt b/app/src/androidTest/java/com/gaurav/avnc/viewmodel/LiveEventTest.kt index 1c2c2609..446e9a4c 100644 --- a/app/src/androidTest/java/com/gaurav/avnc/viewmodel/LiveEventTest.kt +++ b/app/src/androidTest/java/com/gaurav/avnc/viewmodel/LiveEventTest.kt @@ -8,9 +8,10 @@ package com.gaurav.avnc.viewmodel -import androidx.test.espresso.Espresso.onIdle +import androidx.lifecycle.Observer import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.gaurav.avnc.instrumentation +import com.gaurav.avnc.pollingAssert +import com.gaurav.avnc.runOnMainSync import org.junit.Assert import org.junit.Test import org.junit.runner.RunWith @@ -23,21 +24,21 @@ class LiveEventTest { * This is the main difference between [LiveEvent] & [androidx.lifecycle.LiveData]. */ @Test - fun futureObserversShouldNotBeNotified() { - val testEvent = LiveEvent() + fun exactlyOneObserverShouldBeNotified() { + val testEvent = LiveEvent() var observer1Notified = false var observer2Notified = false var futureObserverNotified = false - instrumentation.runOnMainSync { - testEvent.observeForever { observer1Notified = true } // Observer 1 - testEvent.observeForever { observer2Notified = true } // Observer 2 - testEvent.fire(true) - testEvent.observeForever { futureObserverNotified = true } // Observer installed after event was fired + runOnMainSync { + testEvent.observeForever { observer1Notified = true } + testEvent.observeForever { observer2Notified = true } + testEvent.fire(Any()) + testEvent.observeForever { futureObserverNotified = true } } Assert.assertTrue(observer1Notified) - Assert.assertTrue(observer2Notified) + Assert.assertFalse(observer2Notified) Assert.assertFalse(futureObserverNotified) } @@ -48,39 +49,45 @@ class LiveEventTest { */ @Test fun eventShouldNotBeLost() { - val testEvent = LiveEvent() + val testEvent = LiveEvent() var observerNotified = false - instrumentation.runOnMainSync { - testEvent.fire(true) + runOnMainSync { + testEvent.fire(Any()) testEvent.observeForever { observerNotified = true } } Assert.assertTrue(observerNotified) } - /** - * Working of [LiveEvent] relies on setValue() being called in response to fireAsync(). - * This test verifies that assumption. - */ @Test - fun checkSetValueIsCalledAfterFireAsync() { - val testValue = Any() - var setValueCalled = false - var setValueCalledWith: Any? = Any() - - val testEvent = object : LiveEvent() { - override fun setValue(value: Any?) { - super.setValue(value) - setValueCalled = true - setValueCalledWith = value - } + fun testObserverRemoval() { + val testEvent = LiveEvent() + var notified = false + + runOnMainSync { + val observer = Observer { notified = true } + testEvent.observeForever(observer) + testEvent.removeObserver(observer) + testEvent.fire(Any()) } - testEvent.fireAsync(testValue) - onIdle() + Assert.assertFalse(notified) + } - Assert.assertTrue(setValueCalled) - Assert.assertSame(testValue, setValueCalledWith) + @Test + fun testAsyncFiring() { + val testEvent = LiveEvent() + var observedValue = 0 + + runOnMainSync { + testEvent.observeForever { observedValue = it } + } + + testEvent.fireAsync(1) + pollingAssert { + Assert.assertEquals(1, observedValue) + Assert.assertEquals(1, testEvent.value) + } } } \ No newline at end of file diff --git a/app/src/main/java/com/gaurav/avnc/viewmodel/LiveEvent.kt b/app/src/main/java/com/gaurav/avnc/viewmodel/LiveEvent.kt index 285ea679..cca41555 100644 --- a/app/src/main/java/com/gaurav/avnc/viewmodel/LiveEvent.kt +++ b/app/src/main/java/com/gaurav/avnc/viewmodel/LiveEvent.kt @@ -8,8 +8,10 @@ package com.gaurav.avnc.viewmodel +import androidx.annotation.MainThread import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer /** @@ -18,91 +20,70 @@ import androidx.lifecycle.Observer * * Single-shot * =========== - * When this event is fired, it will notify all active observers. - * If there is no active observer, it will wait for active observers, - * so that the event is not "lost". After notifying observers, it will - * be marked as 'handled' and any future observers will NOT be notified. + * When this event is fired, it will notify exactly one active observer. + * If there is no active observer, it will wait for one so that the event + * is not "lost". * * This is the main difference between this class & [LiveData]. [LiveData] will * notify the future observers to bring them up-to date. This can happen during * Activity restarts where old observers are detached and new ones are attached. * * This class is used for events which should be handled only once. - * e.g starting a fragment. - * - * Calling [removeObserver] on [LiveEvent] is NOT supported because we wrap - * the observer given to us in a custom observer, which is currently not - * exposed to callers (see [wrapObserver]). + * E.g. starting a fragment. */ -open class LiveEvent : LiveData() { - - /** - * Whether we are currently firing the event. Observers will be notified - * only when this is true. - */ - private var firing = false - - /** - * Whether someone has handled the last event fired. - * This is used to implement the "queuing" behaviour: - * - * 1. When event is fired, set this to false - * 2. If observers are invoked, set this to true - * 3. In [onActive], if this is still false, re-fire - */ - private var handled = true +open class LiveEvent { + private class WrappedData(val data: T, var consumed: Boolean = false) + private class WrappedObserver(private val observer: Observer) : Observer> { + override fun onChanged(value: WrappedData) { + if (!value.consumed) { + value.consumed = true + observer.onChanged(value.data) + } + } + } - /** - * Fire this event with given value. - * MUST be called from Main thread. - */ - open fun fire(value: T?) = setValue(value) + private val liveData = MutableLiveData>() + private val wrappedObservers = mutableMapOf, WrappedObserver>() /** - * Same as [fire], but can be called from any thread. + * Peek current value of this event, irrespective of whether any observer has been notified. */ - fun fireAsync(value: T?) = postValue(value) + val value get() = liveData.value?.data /** - * Overridden to manage event state. + * Fire this event with given data. + * Must be called from main thread. */ - override fun setValue(value: T?) { - firing = true - handled = false - super.setValue(value) - firing = false + @MainThread + fun fire(data: T) { + liveData.value = WrappedData(data) } /** - * Overridden to check for queued event. + * Asynchronous version of [fire]. + * Can be called from any thread. */ - override fun onActive() { - super.onActive() - - if (!handled) - fire(value) + fun fireAsync(data: T) { + liveData.postValue(WrappedData(data)) } - - override fun observe(owner: LifecycleOwner, observer: Observer) { - super.observe(owner, wrapObserver(observer)) + @MainThread + fun observe(owner: LifecycleOwner, observer: Observer) { + val wrapped = WrappedObserver(observer) + wrappedObservers[observer] = wrapped + liveData.observe(owner, wrapped) } - override fun observeForever(observer: Observer) { - super.observeForever(wrapObserver(observer)) + @MainThread + fun observeForever(observer: Observer) { + val wrapped = WrappedObserver(observer) + wrappedObservers[observer] = wrapped + liveData.observeForever(wrapped) } - /** - * Observer given to us is wrapped in another Observer - * which checks current state before invoking real observer. - */ - private fun wrapObserver(real: Observer): Observer { - return Observer { - if (firing) { - real.onChanged(it) - handled = true - } - } + @MainThread + fun removeObserver(observer: Observer) { + wrappedObservers.remove(observer)?.let { liveData.removeObserver(it) } } } \ No newline at end of file