22

Issue

Summary: Multiple LiveData Observers are being triggered in a Fragment after navigating to a new Fragment, popping the new Fragment, and returning to the original Fragment.

Details: The architecture consists of MainActivity that hosts a HomeFragment as the start destination in the MainActivity's navigation graph. Within HomeFragment is a programmatically inflated PriceGraphFragment. The HomeFragment is using the navigation component to launch a new child Fragment ProfileFragment. On back press the ProfileFragment is popped and the app returns to the HomeFragment hosting the PriceGraphFragment. The PriceGraphFragment is where the Observer is being called multiple times.

I'm logging the hashcode of the HashMap being emitted by the Observer and it is showing 2 unique hashcodes when I go to the profile Fragment, pop the profile Fragment, and return to the price Fragment. This is opposed to the one hashcode seen from the HashMap when I rotate the screen without launching the profile Fragment.

Implementation

  1. Navigation component to launch new ProfileFragment within HomeFragment.

    view.setOnClickListener(Navigation.createNavigateOnClickListener( R.id.action_homeFragment_to_profileFragment, null))

  2. ViewModel creation in Fragment (PriceGraphFragment). The ViewModel has been logged and the data that has multiple Observers only has data initialized in the ViewModel once.

    override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) priceViewModel = ViewModelProviders.of(this).get(PriceDataViewModel::class.java) }

  3. Listen for data from ViewModel in original Fragment (PriceGraphFragment). This is being called multiple times, however it is only expected to have one Observer when the Fragment is loaded.

    priceViewModel.graphLiveData.observe( this, Observer { priceGraphDataMap: HashMap<Exchange, PriceGraphLiveData>? -> // This is being called multiple times. })

Attempted Solutions

  1. Creating the Fragment's ViewModel in the onCreate() method. priceViewModel = ViewModelProviders.of(this).get(PriceDataViewModel::class.java)
  2. Creating the ViewModel using the Fragment's activity and the child Fragment's parent Fragment.
    priceViewModel = ViewModelProviders.of(activity!!).get(PriceDataViewModel::class.java)

    priceViewModel = ViewModelProviders.of(parentFragment!!).get(PriceDataViewModel::class.java)

  3. Moving methods that create the Observers to the Fragment's onCreate() and onActivityCreated() methods.
  4. Using viewLifecycleOwner instead of this for the LifecycleOwner in the method observe(@NonNull LifecycleOwner owner, @NonNull Observer<? super T> observer).
  5. Storing the HashMap data that is showing as duplicates in the ViewModel opposed to the Fragment.
  6. Launching the child Fragment using the ChildFragmentManager and the SupportFragmentManager (on the Activity level).

Similar Issues and Proposed Solutions

Next Steps

  • Perhaps the issue is related to creating the nested ChildFragment (PriceGraphFragment) in the ParentFragment's (HomeFragment) onViewCreated()?

ParentFragment

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    super.onViewCreated(view, savedInstanceState)
    user = viewModel.getCurrentUser()
     if (savedInstanceState == null) {
         fragmentManager
                ?.beginTransaction()
                ?.replace(binding.priceDataContainer.id, 
                   PriceGraphFragment.newInstance())
                ?.commit()
}
  • Test replacing the LiveData objects with RxJava observables.
AdamHurwitz
  • 9,758
  • 10
  • 72
  • 134
  • 1
    Regarding 1: why are you storing View references in your ViewModel ? The ViewModel must not have any reference to a View. If you're using Data Binding, you can either set the click listener by code when you create the View, or you put a method returning a click listener in your ViewModel and bind in to the button in your layout. – BladeCoder Aug 17 '18 at 11:25
  • 1
    Regarding 3: where are you registering your observer? Make sure it's done in `onActivityCreated()` and use the `viewLifeCycleOwner` as lifecycle instead of `this`. If the observer is still called mutiple times after that, it may just be because your `LiveData` is updated multiple times. – BladeCoder Aug 17 '18 at 11:31
  • @BladeCoder 1 - I'm passing the button view to the ViewModel in order to implement the click listener. No other views are held in the ViewModel. Implementing the click directly in the layout or in a **BindingAdapter** is not an option in this case as the alternative use case of the click uses **startActivityForResult()** which requires Fragment access. 3 - I've logged when the LiveData is updated and it is printing out once as expected. Currently, the Observer is registered in **onCreate()**. I re-tested moving them to **onActivityCreated()** with the same result. – AdamHurwitz Aug 17 '18 at 18:10
  • Perhaps the issue is related to creating the nested ChildFragment (PriceGraphFragment) in the ParentFragment's (HomeFragment) `onViewCreated()`? – AdamHurwitz Aug 17 '18 at 18:25
  • @BladeCoder - You're correct about the ViewModel. It doesn't fix the issue above, but I removed the one view that was passed through there. – AdamHurwitz Aug 17 '18 at 22:22
  • 1
    You're calling `PriceGraphFragment.newInstance()` only if there's no `savedInstanceState` and you're using `add` instead of `replace` on the `FragmentTransaction` - is there a reason for that? Also have you checked whether this isn't called every single time you're getting back to the `ParentFragment`, because if it is, you're basically just adding a new `PriceGraphFragment` on top of the old one every single time, which is why you're getting multiple calls to the observer, because all `PriceGraphFragment`-instances are observing the same LiveData, thus multiple signals are sent. – Darwind Aug 17 '18 at 22:35
  • @Darwind - There is not a thoughtful reason for `.add()` over `.replace()`. I tried switching it to `.replace()` now but unfortunately I am still seeing the multiple observers in the ChildFragment. – AdamHurwitz Aug 17 '18 at 22:59
  • @Darwind - Every time I go to the new ProfileFragment and back to the HomeFragment parent) I am seeing the Observer twice even with using `.replace()` for the Fragments so I think the issue of multiple Fragments being created is resolved. I know have `if (savedInstanceState == null) { fragmentManager ?.beginTransaction() ?.replace(priceDataContainer.id, PriceGraphFragment.newInstance()) ?.commit()}` – AdamHurwitz Aug 17 '18 at 23:13
  • @Darwind - For good measure I logged the `onActivityCreated()` of the child Fragment (_PriceGraphFragment_) where the observers are created to ensure only one Fragment instance is created when I go press the back button. – AdamHurwitz Aug 18 '18 at 03:35
  • 1
    To add child fragments you need to use the childFragmentManager, not the fragmentManager – BladeCoder Aug 18 '18 at 17:05
  • I've replaced my existing code with the ChildFragmentManager. The Duplicate data is more delayed but now seeing the view created 2 additional times. Before, I was just seeing additional Observers, not view creations. `childFragmentManager .beginTransaction() .replace(priceDataContainer.id, PriceGraphFragment.newInstance()) .commit()` Here's a video of the behavior: https://photos.app.goo.gl/6Q1jPgTc5z7kHf916. I can provide GitHub access to the project if interested. – AdamHurwitz Aug 18 '18 at 17:37
  • 1
    If possible could you provide a small working project with the issue on Github for instance? It's very difficult to figure out the issue when there are multiple minor issues all the time. My guess is that you have something holding on to the `ViewModel` and then indirectly holding on to the `Fragment` in the end and when a new instance of the fragment comes along and calls `observe` on your `ViewModel` it also calls back to the original `ViewModel`. – Darwind Aug 20 '18 at 09:24
  • 1
    @Darwind, I'm thinking the same thing in regards to the ViewModel. I will refactor the Fragment + ViewModel to store the graph data showing twice in the ViewModel instead of the Fragment which could likely be adding to the problem. If this refactor does not work I'll provide a basic sample app illustrating the issue. – AdamHurwitz Aug 20 '18 at 23:30
  • 1
    Great, let us know how it went no matter what :-) if you fix it, remember to provide an answer yourself :-) – Darwind Aug 21 '18 at 07:50
  • 1
    You are adding the viewmodel with 'ViewModelProviders.of(this).get(PriceDataViewModel::class.java)' instead of 'ViewModelProviders.of(getActivity()).get(PriceDataViewModel::class.java)'. The viewmodel of the fragment is different than the viewModel of the activity? and of the parentFragment? – KvdLingen Aug 21 '18 at 18:04
  • Thanks @KvdLingen, I gave this a try for both the parent and child Fragments. I'm still getting the additional Observer. I am going to log the hashcode to see if this is the same Observer or a new one relating to my Fragment creation. – AdamHurwitz Aug 21 '18 at 19:04

3 Answers3

10

This is basically a bug in the architecture. You can read more about it here. You can solve it by using getViewLifecycleOwner instead of this in the observer.

Like this:

mViewModel.methodToObserve().observe(getViewLifecycleOwner(), new Observer<Type>() {
        @Override
        public void onChanged(@Nullable Type variable) {

And put this code in onActivityCreated() as the use of getViewLifecycleOwner requires a view.

Karthikeyan
  • 196
  • 2
  • 10
KvdLingen
  • 1,230
  • 2
  • 14
  • 22
  • 3
    Thanks @KvidLingen, from my initial post you can see that I've already tried the getViewLifecycleOwner solution. I have updated my initial post to also show that I've tried the `onActivityCreated()` solution in addition without success. – AdamHurwitz Aug 20 '18 at 23:27
  • 1
    I even use this in my child fragment, after pop all observes starts from beginning in child fragment. – Mahdi Dec 06 '19 at 00:32
7

First off, thank you to everyone who posted here. It was a combination of your advice and pointers that helped me solve this bug over the past 5 days as there were multiple issues involved.

Issues Resolved

  1. Creating nested Fragments properly in parent Fragment (HomeFragment).

Before:

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
                          savedInstanceState: Bundle?): View? {

        if (savedInstanceState == null) {
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.priceDataContainer.id, PriceGraphFragment.newInstance())
                ?.commit()
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.contentFeedContainer.id, ContentFeedFragment.newInstance())
                ?.commit()
    }
...
}

After:

override fun onActivityCreated(savedInstanceState: Bundle?) {
    super.onActivityCreated(savedInstanceState)

    if (savedInstanceState == null
            && childFragmentManager.findFragmentByTag(PRICEGRAPH_FRAGMENT_TAG) == null
            && childFragmentManager.findFragmentByTag(CONTENTFEED_FRAGMENT_TAG) == null) {
        childFragmentManager.beginTransaction()
                .replace(priceDataContainer.id, PriceGraphFragment.newInstance(),
                        PRICEGRAPH_FRAGMENT_TAG)
                .commit()
        childFragmentManager.beginTransaction()
                .replace(contentFeedContainer.id, ContentFeedFragment.newInstance(),
                        CONTENTFEED_FRAGMENT_TAG)
                .commit()
    }
...
}
  1. Creating ViewModels in onCreate() as opposed to onCreateView() for both the parent and child Fragments.

  2. Initializing request for data (Firebase Firestore query) data of child Fragment (PriceFragment) in onCreate() rather than onViewCreated() but still doing so only when saveInstanceState is null.

Non Factors

A couple items were suggested but turned out to not have an impact in solving this bug.

  1. Creating Observers in onActivityCreated(). I'm keeping mine in onViewCreated() of the child Fragment (PriceFragment).

  2. Using viewLifecycleOwner in the Observer creation. I was using the child Fragment (PriceFragment)'s this before. Even though viewLifecycleOwner does not impact this bug it seems to be best practice overall so I'm keeping this new implementation.

Community
  • 1
  • 1
AdamHurwitz
  • 9,758
  • 10
  • 72
  • 134
0

It's better to initialize the view model and observe live data objects in onCreate.

override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        viewModel = ViewModelProvider(this).get(MyFragmentViewModel::class.java)

        // 'viewLifecycleOwner' is not available here, so use 'this'
        viewModel.myLiveData.observe(this) {
            // Do something
        }
    }

However, no matter where you initialize the view model, whether in onCreate or onViewCreated, it will still give you the same view model object as it's created only once for the lifecycle of the Fragment.

The important part is observing the live data in onCreate. Because onCreate is called only on fragment creation, you're calling observe only once.

onViewCreated is called both when the fragment is created and when it is brought back from the back stack (after popping the fragment on top of it). If you observe live data in onViewCreated it will get the existing data that your live data is holding from the previous call immediately on returning from the back stack.

Instead, use onViewCreated only to fetch data from the view model. So whenever the fragment appears, either on the creation or returning from the back stack, it will always fetch the latest data.

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)

        viewModel.fetchData()

        ...
    }
Vikram Gupta
  • 6,496
  • 5
  • 34
  • 47