freeletics / RxRedux

Redux implementation based on RxJava
https://freeletics.engineering/2018/08/16/rxredux.html
Apache License 2.0
567 stars 34 forks source link

Maintaining state between screen changes #26

Closed DFreds closed 5 years ago

DFreds commented 5 years ago

I'm having an issue with the state being reset and I'm not sure why.

I have three views for three bottom navigation buttons. Whenever one of the views are swapped out after pressing a bottom navigation button, the previous state is lost.

For reference, I basically followed the architecture you have in your sample. In my program, the user is presented with a screen with a search text box. On typing something in, it waits 1 second, then performs the search on github and populates a recycler view with the results.

Main activity swapping controllers:

private val mOnNavigationItemSelectedListener = BottomNavigationView.OnNavigationItemSelectedListener { item ->
        when (item.itemId) {
            R.id.navigation_home -> {
                router.setRoot(RouterTransaction.with(HomeController()))
                return@OnNavigationItemSelectedListener true
            }
            R.id.navigation_dashboard -> {
                router.setRoot(RouterTransaction.with(DashboardController()))
                return@OnNavigationItemSelectedListener true
            }
            R.id.navigation_notifications -> {
                router.setRoot(RouterTransaction.with(NotificationsController()))
                return@OnNavigationItemSelectedListener true
            }
        }
        false
    }

The view is below. It uses koin to inject a new instance of the view model each time the controller is initialized.

class DashboardController : KoinViewModelBaseController() {
    private val dashboardViewModel: DashboardViewModel by viewModelByClass(
        clazz = DashboardViewModel::class,
        from = { this@DashboardController }
    )
    private val githubApi: GithubApi by inject()

    private var adapter: HomeAdapter? = null
    private val disposables = CompositeDisposable()

    override fun inflateView(inflater: LayoutInflater, container: ViewGroup): View =
        inflater.inflate(R.layout.dashboard_controller, container, false)

    override fun onViewBound(view: View) {
        super.onViewBound(view)

        adapter = HomeAdapter(LayoutInflater.from(applicationContext))

        view.dashboard_recycler_view.layoutManager = LinearLayoutManager(applicationContext)
        view.dashboard_recycler_view.adapter = adapter

        view.dashboard_search_text.textChanges()
            .skipInitialValue()
            .debounce(1, TimeUnit.SECONDS)
            .map { DashboardAction.LoadFirstPageAction(query = it.toString(), githubApi = githubApi) }
            .subscribe(dashboardViewModel.input)
            .addTo(disposables)

        dashboardViewModel.state.observe(this, Observer {
            render(it)
        })
    }

    override fun onDestroy() {
        super.onDestroy()
        disposables.dispose()
    }

    private fun render(state: DashboardState?) {
        if (state?.data != null) {
            view?.dashboard_recycler_view?.visible()
            view?.dashboard_progress_bar?.gone()

            state.data.items?.let {
                adapter?.repos = it
                adapter?.notifyDataSetChanged()
            }

            return
        }

        if (state?.isLoading == true) {
            view?.dashboard_recycler_view?.gone()
            view?.dashboard_progress_bar?.visible()

            return
        }

        view?.dashboard_recycler_view?.gone()
        view?.dashboard_progress_bar?.gone()
    }
}

The dashboard view model injects a main store singleton.

class DashboardViewModel(
    mainStore: MainStore,
    androidScheduler: Scheduler
) : ViewModel() {

    private val inputRelay: Relay<RxReduxAction> = PublishRelay.create()
    private val mutableState = MutableLiveData<DashboardState>()
    private val disposables = CompositeDisposable()

    val input: Consumer<RxReduxAction> = inputRelay
    val state: LiveData<DashboardState> = mutableState

    init {
        inputRelay
            .subscribe(mainStore.input)
            .addTo(disposables)

        mainStore.state
            .observeOn(androidScheduler)
            .subscribe { state -> mutableState.value = state.dashboardState }
            .addTo(disposables)
    }

    override fun onCleared() {
        super.onCleared()
        disposables.dispose()
    }
}

The main store is a singleton defined by Koin.

class MainStore {
    val input: Relay<RxReduxAction> = PublishRelay.create()

    val state: Observable<MainState> = input
        .doOnNext { println("Input action $it") }
        .reduxStore(
            initialState = MainState(dashboardState = null),
            reducer = ::mainReducer,
            sideEffects = listOf(::loadFirstPageSideEffect)
        )
        .distinctUntilChanged()
        .doOnNext { println("RxStore state $it") }
}

To be more clear, this is the behavior I'm seeing:

  1. User taps the bottom navigation button
  2. User sees a blank page with a search box (because data == null and isLoading == false)
  3. User types something into search box, it waits for 1 second and executes request
  4. A loading indicator appears (isLoading == true)
  5. Recycler view is populated with data (data != null)
  6. User taps on different bottom navigation button.
  7. User taps on the same bottom navigation button as before
  8. User sees a blank page again (data == null and isLoading == false)

I figured it would still show the data because I assumed the data for that state should not be null. Any ideas?

DFreds commented 5 years ago

I changed the MainStore input to a BehaviorRelay and it seems I get closer to my intended behavior. According to the relay docs, publish relays emit every subsequent observed items and behavior relays emit the most recent observed item and all subsequent observed items.

I still have an issue with it redoing the search upon reloading the page, but I think it's a separate problem related to RxBinding.

I'm going to leave this open in case you have any other suggestions, but it seems like that fixed it.

DFreds commented 5 years ago

Sorry, found something else out. I realized I'm still getting the initial state every time before I get the last state provided by the behavior relay. This causes my view to render the initial state during the onCreateView before re-rendering the actual last state. This is causing some issues with my text changed listener because it basically reruns the request, but I could see it causing issues for other reasons too. Any ideas for that?

sockeqwe commented 5 years ago

Hi @DFreds , this is a common misunderstanding how RxJava works. The "problem" is that whenever a new subscriber subscribes to MainStore.state the whole new rxjava Observable (and reduxStore is just like any other rx observable) gets created. This is not a bug nor a problem, this is by design the how rx java works.

Example:

import com.freeletics.rxredux.reduxStore
import io.reactivex.subjects.PublishSubject

class MainStore {

    val input = PublishSubject.create<Int>()

    val state = input
        .reduxStore(
            initialState = "Start",
            reducer = { state, action -> state + action },
            sideEffects = emptyList()
        )
}

fun main(args: Array<String>) {

    val store = MainStore()

    // Subscriber 1
    val d = store.state.subscribe(::println)
    store.input.onNext(1)
    store.input.onNext(2)
    store.input.onNext(3)

    d.dispose()

    // Subscriber 2
    store.state.subscribe(::println)
    store.input.onNext(4)
    store.input.onNext(5)
    store.input.onNext(6)

    // Subscriber 3
    store.state.subscribe(::println)
    store.input.onNext(7)
    store.input.onNext(8)
    store.input.onNext(9)
}

The output of this is:

Subscriber 1 --> Start Subscriber 1 --> Start1 Subscriber 1 --> Start12 Subscriber 1 --> Start123 Subscriber 2 --> Start Subscriber 2 --> Start4 Subscriber 2 --> Start45 Subscriber 2 --> Start456 Subscriber 3 --> Start Subscriber 2 --> Start4567 Subscriber 3 --> Start7 Subscriber 2 --> Start45678 Subscriber 3 --> Start78 Subscriber 2 --> Start456789 Subscriber 3 --> Start789

As you see, although Subscriber 2 is still subscribed, Subscriber 3 starts again with the initial state because a new RxJava Observable gets created everytime a new subscriber subscribes. Again, this is by design (of RxJava not RxRedux, RxRedux just implements RxJava's design contract).

You see exactly the same output with any other RxJava operator, like scan:

class MainStore {

    val input = PublishSubject.create<Int>()

    val state = input
        .scan(0, { a, b -> a + b })
}

fun main(args: Array<String>) {

    val store = MainStore()

    val d = store.state.subscribe { println("Subscriber 1 --> $it") }
    store.input.onNext(1)
    store.input.onNext(1)
    store.input.onNext(1)

    d.dispose()

    store.state.subscribe { println("Subscriber 2 --> $it") }
    store.input.onNext(1)
    store.input.onNext(1)
    store.input.onNext(1)

    store.state.subscribe { println("Subscriber 3 --> $it") }
    store.input.onNext(1)
    store.input.onNext(1)
    store.input.onNext(1)
}

Subscriber 1 --> 0 Subscriber 1 --> 1 Subscriber 1 --> 2 Subscriber 1 --> 3 Subscriber 2 --> 0 Subscriber 2 --> 1 Subscriber 2 --> 2 Subscriber 2 --> 3 Subscriber 3 --> 0 Subscriber 2 --> 4 Subscriber 3 --> 1 Subscriber 2 --> 5 Subscriber 3 --> 2 Subscriber 2 --> 6 Subscriber 3 --> 3

To "fix" this issue you can simply use .share(). Share avoids creating a new RxJava observable. Instead it keeps one single Observable (that do multicasting) as long as there is at least 1 subscriber:

class MainStore {

    val input = PublishSubject.create<Int>()

    val state = input
        .reduxStore(
            initialState = "Start",
            reducer = { state, action -> state + action },
            sideEffects = emptyList()
        ).share()   // <-- This here does the trick
}

fun main(args: Array<String>) {

    val store = MainStore()

    val d = store.state.subscribe { println("Subscriber 1 --> $it") }
    store.input.onNext(1)
    store.input.onNext(2)
    store.input.onNext(3)

    d.dispose()

    store.state.subscribe { println("Subscriber 2 --> $it") }
    store.input.onNext(4)
    store.input.onNext(5)
    store.input.onNext(6)

    store.state.subscribe { println("Subscriber 3 --> $it") }
    store.input.onNext(7)
    store.input.onNext(8)
    store.input.onNext(9)
}

Subscriber 1 --> Start Subscriber 1 --> Start1 Subscriber 1 --> Start12 Subscriber 1 --> Start123 Subscriber 2 --> Start Subscriber 2 --> Start4 Subscriber 2 --> Start45 Subscriber 2 --> Start456 Subscriber 2 --> Start4567 Subscriber 3 --> Start4567 Subscriber 2 --> Start45678 Subscriber 3 --> Start45678 Subscriber 2 --> Start456789 Subscriber 3 --> Start456789

However, as pointed out before share() only works as expected as long as there is at least one Subscriber. You see that in the output: Subscriber 2 starts with a new Observable because Subscriber1 unsubscribed and then are 0 subscribers. Depending on your implementation, it might be possible, that for a short period of time there are no subscribers (i.e. I could imagine that switching between tab1 and tab2 could have the effect that ViewModel of tab 1 unsubscribes to MainStore before ViewModel of tab2 subscribes to same reference of MainStore.state. So there are 0 subscribers for a few milliseconds).

One could fix that like that:

class MainStore {

    val input = PublishRelay.create<Int>()

    val state = BehaviorRelay.create<String>()

    val disposable: Disposable

    init {

        disposable = input
            .reduxStore(
                initialState = "Start",
                reducer = { state, action -> state + action },
                sideEffects = emptyList()
            )
            .subscribe(state)
    }
}

fun main(args: Array<String>) {

    val store = MainStore()

    val d = store.state.subscribe { println("Subscriber 1 --> $it") }
    store.input.accept(1)
    store.input.accept(2)
    store.input.accept(3)

    d.dispose()

    store.state.subscribe { println("Subscriber 2 --> $it") }
    store.input.accept(4)
    store.input.accept(5)
    store.input.accept(6)

    store.state.subscribe { println("Subscriber 3 --> $it") }
    store.input.accept(7)
    store.input.accept(8)
    store.input.accept(9)
}

Subscriber 1 --> Start Subscriber 1 --> Start1 Subscriber 1 --> Start12 Subscriber 1 --> Start123 Subscriber 2 --> Start123 Subscriber 2 --> Start1234 Subscriber 2 --> Start12345 Subscriber 2 --> Start123456 Subscriber 3 --> Start123456 Subscriber 2 --> Start1234567 Subscriber 3 --> Start1234567 Subscriber 2 --> Start12345678 Subscriber 3 --> Start12345678 Subscriber 2 --> Start123456789 Subscriber 3 --> Start123456789

Please note that with this approach you manually have to call mainStore.disposable.dispose() because any regular subscription to MainStore.state only returns a Disposble forval state : BehaviorRelaybut NOT the disposable forreduxStore(). With that said, I don't know when the best point in time would be to callmainStore.disposable.dispose(), maybe once you destroy the koin's scope that holdsMainStore` instance ...

TL;DR: It's not an RxRedux issue but the "problem" you face is how RxJava works (by design).

DFreds commented 5 years ago

Wow, thanks for the great response. That makes a lot more sense actually. I think I can figure out a way around it now that I get what is happening. Thanks again!