Skip to content

Commit

Permalink
fix: change state initialization lifecycle
Browse files Browse the repository at this point in the history
Request change for android.

Activity is calling save instance state on child
fragments before the view was attached
when the currentState was uninitialized which
leads to crash.

We are using currentState in onSaveInstanceState to keep
state in android.

Presenter's initializeState gives flexibility to init a state
depending on platform specific requirements.

Also fixed type, expose exceptions classes to give possibility to
handle exceptions from library.
  • Loading branch information
Arkadiusz Pałka committed Dec 22, 2020
1 parent a49f4f9 commit 79ece5a
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package pl.valueadd.mvi.exception

internal class ViewNotAttachedException :
class ViewNotAttachedException internal constructor() :
RuntimeException("View was called before that has been attached to presenter")
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
package pl.valueadd.mvi.exception

import java.lang.RuntimeException

internal class ViewWasNotDetachedException : RuntimeException("Detach previous view first.")
class ViewWasNotDetachedException internal constructor() :
RuntimeException("Detach previous view first.")
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI

/**
* Returns [view][IBaseView] but may throw [ViewNotAttachedException] if called in wrong place.
* @throws ViewNotAttachedException if called in wrong place
*/
@get:Throws(ViewNotAttachedException::class)
protected val view: V
get() = internalView ?: throw ViewNotAttachedException()

Expand All @@ -53,12 +55,12 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI
private var internalView: V? = null

/**
* Use to determine when a intents have to be binded.
* Use to determine when a intents have to be bound.
*/
private var wasViewAttachedOnce = false

/**
* A disposable container of temporarily binded view intents.
* A disposable container of temporarily bound view intents.
*/
private var currentViewIntentsDisposable: Disposable? = null

Expand All @@ -68,12 +70,12 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI
private var viewStateReducerDisposable: Disposable? = null

/**
* A disposable of currently binded consumer for emission of wrapped intents.
* A disposable of currently bound consumer for emission of wrapped intents.
*/
private var viewStateConsumerDisposable: Disposable? = null

/**
* A subject to pass emission of wrapped intents to currently binded view's consumer.
* A subject to pass emission of wrapped intents to currently bound view's consumer.
*/
private val viewStateBehaviorSubject: BehaviorSubject<VS> by lazy {
BehaviorSubject.createDefault(currentState)
Expand All @@ -89,6 +91,16 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI

//region Lifecycle methods

/**
* If view hasn't attached the first time, the presenter calls [IBaseView.provideInitialViewState]
* to set initial value of [currentState]
*/
override fun initializeState(view: V) {
if (!wasViewAttachedOnce) {
currentState = view.provideInitialViewState()
}
}

/**
* If view is attached the first time, the presenter subscribes its provided intents.
* Each time subscribes to view state's consumer and bind view's intents.
Expand All @@ -97,6 +109,7 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI
*
* @throws ViewWasNotDetachedException if previous view was not detached
*/
@Throws(ViewWasNotDetachedException::class)
override fun attachView(view: V) {
if (this.internalView != null) {
throw ViewWasNotDetachedException()
Expand All @@ -105,7 +118,6 @@ abstract class BaseMviPresenter<VS : IBaseViewState, PS : IBasePartialState, VI
this.internalView = view

if (!wasViewAttachedOnce) {
currentState = view.provideInitialViewState()
startObservingCurrentViewStateSubject()
wasViewAttachedOnce = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package pl.valueadd.mvi.presenter

interface IMviPresenter<V : IBaseView<*, *>> {

fun initializeState(view: V)

fun attachView(view: V)

fun detachView()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import io.mockk.verify
import io.reactivex.Observable
import io.reactivex.schedulers.Schedulers
import io.reactivex.subjects.PublishSubject
import org.junit.jupiter.api.Assertions
/* ktlint-disable no-wildcard-imports */
import org.junit.jupiter.api.Assertions.*
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
Expand Down Expand Up @@ -47,11 +48,12 @@ class BaseMviPresenterTest {
val firstView = createMockView(Observable.never())
val secondView: IBaseView<TestViewState, TestViewIntent> = mockk()

presenter.initializeState(firstView)
presenter.attachView(firstView)

// When
// Then
Assertions.assertThrows(ViewWasNotDetachedException::class.java) {
assertThrows(ViewWasNotDetachedException::class.java) {
presenter.attachView(secondView)
}
}
Expand All @@ -61,6 +63,7 @@ class BaseMviPresenterTest {
// Given
val viewIntentsSubject = PublishSubject.create<TestViewIntent>()
val mockView = createMockView(viewIntentsSubject)
presenter.initializeState(mockView)

// When
presenter.attachView(mockView)
Expand All @@ -75,14 +78,15 @@ class BaseMviPresenterTest {
// Given
val viewIntentsSubject = PublishSubject.create<TestViewIntent>()
val mockView = createMockView(viewIntentsSubject)
presenter.initializeState(mockView)
presenter.attachView(mockView)

// When
presenter.detachView()

// Then
verify(exactly = 1) { mockView.provideViewIntents() }
assert(viewIntentsSubject.hasObservers() == false)
assertFalse(viewIntentsSubject.hasObservers())
}

@Test
Expand All @@ -94,6 +98,7 @@ class BaseMviPresenterTest {
val mockView = createMockView(Observable.never())

every { mockReducer.reduce(any(), testPresenterPartialState) } returns expectedTestViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)

// When
Expand All @@ -117,6 +122,7 @@ class BaseMviPresenterTest {
val reducedViewState = TestViewState(1)
every { mockMapper.mapViewIntentToPartialState(testViewIntent) } returns testPartialStatePublishSubject
every { mockReducer.reduce(any(), testPartialState) } returns reducedViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)
viewIntentsSubject.onNext(testViewIntent) // For example user press login button

Expand All @@ -139,6 +145,7 @@ class BaseMviPresenterTest {

every { mockMapper.mapViewIntentToPartialState(testViewIntent) } returns testPartialStatePublishSubject
every { mockReducer.reduce(any(), testPartialState) } returns reducedViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)
viewIntentsSubject.onNext(testViewIntent) // For example user press login button
presenter.detachView() // View of fragment is destroyed by system
Expand All @@ -161,6 +168,7 @@ class BaseMviPresenterTest {
val mockView = createMockView(viewIntentsSubject)
every { mockMapper.mapViewIntentToPartialState(testViewIntent) } returns testPartialStatePublishSubject
every { mockReducer.reduce(any(), testPartialState) } returns reducedViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)
viewIntentsSubject.onNext(testViewIntent) // For example user press login button

Expand All @@ -186,6 +194,7 @@ class BaseMviPresenterTest {

every { mockMapper.mapViewIntentToPartialState(testViewIntent) } returns testPartialStatePublishSubject
every { mockReducer.reduce(any(), testPartialState) } returns reducedViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)
viewIntentsSubject.onNext(testViewIntent) // For example user press login button

Expand All @@ -208,15 +217,16 @@ class BaseMviPresenterTest {

every { mockMapper.mapViewIntentToPartialState(testViewIntent) } returns testPartialStatePublishSubject
every { mockReducer.reduce(any(), testPartialState) } returns reducedViewState
presenter.initializeState(mockView)
presenter.attachView(mockView)
viewIntentsSubject.onNext(testViewIntent) // For example user press login button

// When
presenter.destroy()

// Then
assert(testPartialStatePublishSubject.hasObservers() == false)
assert(presenterPublishSubject.hasObservers() == false)
assertFalse(testPartialStatePublishSubject.hasObservers())
assertFalse(presenterPublishSubject.hasObservers())
}

@Test
Expand All @@ -228,6 +238,7 @@ class BaseMviPresenterTest {
every { mockThrowable.stackTrace } returns emptyArray()
every { mockThrowable.cause } returns null
every { mockTestLogger.logError(any()) } returns Unit
presenter.initializeState(mockView)
presenter.attachView(mockView)

// When
Expand All @@ -246,6 +257,7 @@ class BaseMviPresenterTest {
every { mockThrowable.stackTrace } returns emptyArray()
every { mockThrowable.cause } returns null
every { mockTestLogger.logError(any()) } returns Unit
presenter.initializeState(mockView)
presenter.attachView(mockView)

// When
Expand Down
1 change: 1 addition & 0 deletions mvi/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ android {
unitTests.all {
useJUnitPlatform()
}
unitTests.returnDefaultValues = true
}

buildTypes {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package pl.valueadd.mvi.fragment.delegate.fragment

import android.os.Bundle
import androidx.annotation.CallSuper
import pl.valueadd.mvi.presenter.BaseMviPresenter
import pl.valueadd.mvi.presenter.IBaseView

Expand All @@ -14,22 +15,27 @@ open class MviFragmentDelegateImpl<V : IBaseView<*, *>>(
protected val presenter: BaseMviPresenter<*, *, *, V>
) : MviFragmentDelegate {

@CallSuper
override fun onCreate(savedInstanceState: Bundle?) {
// no-op
presenter.initializeState(fragment)
}

@CallSuper
override fun onSaveInstanceState(outState: Bundle) {
// no-op
}

@CallSuper
override fun onStart() {
presenter.attachView(fragment)
}

@CallSuper
override fun onStop() {
presenter.detachView()
}

@CallSuper
override fun onDestroy() {
presenter.destroy()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ class MviFragmentSaveInstanceStateDelegateImpl<V : IBaseView<VS, *>, VS : IBaseV
private set

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
this.restoredViewState =
savedInstanceState?.getParcelable(VIEW_STATE_BUNDLE_KEY)
}

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
outState.putParcelable(
VIEW_STATE_BUNDLE_KEY,
presenter.currentState as Parcelable
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package pl.valueadd.mvi.fragment.base

import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
/* ktlint-disable no-wildcard-imports */
import io.mockk.*
import io.reactivex.Observable
import io.reactivex.schedulers.Schedulers
import kotlinx.android.parcel.Parcelize
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import pl.valueadd.mvi.IBaseViewState
Expand All @@ -27,6 +27,26 @@ class BaseMviFragmentTest {
fragment.presenter = mockPresenter
}

@AfterEach
fun tearDown() {
clearAllMocks()
}

@Test
fun `Should call initialize state on presenter on create`() {
// Given
val mockActivity = mockk<TestActivity>(relaxed = true) {
every { supportDelegate } returns mockk(relaxed = true)
}
fragment.onAttach(mockActivity)

// When
fragment.onCreate(null)

// Then
verify(exactly = 1) { mockPresenter.initializeState(fragment) }
}

@Test
fun `Should attach view to presenter on start`() {
// Given
Expand Down

0 comments on commit 79ece5a

Please sign in to comment.