LiveData not updating when data changes

Question

I'm using LiveData to fetch data from a server and observe it. My onChanged() method just gets called the first time, and does not get called when data in the server gets updated.

UserFragment:

UserViewModel userViewModel = ViewModelProviders.of(this).get(UserViewModel.class);
userViewModel.getUser().observe(this, new Observer<User>() {
    @Override
    public void onChanged(User user) {
        //Set UI
    }
});

UserViewModel:

public class UserViewModel extends AndroidViewModel {
    private LiveData<User> user;

    public UserViewModel(Application application) {
        super(application);
        user = UserRepository.getInstance().fetchUser();
    }

    public LiveData<User> getUser() {
        return user;
    }    
}

UserRepository:

public class UserRepository {    
    private ApiService apiService;
    private static UserRepository userRepository;

    private UserRepository() {
        apiService = RestClient.getClient().create(ApiService.class);
    }

    public synchronized static UserRepository getInstance() {
        if (userRepository == null) userRepository = new UserRepository();
        return userRepository;
    }

    public LiveData<User> fetchUser() {
        final MutableLiveData<User> data = new MutableLiveData<>();
        Call<User> call = apiService.getUser();
        call.enqueue(new Callback<User>() {
            @Override
            public void onResponse(@NonNull Call<User> call, @NonNull Response<User> response) {
                if (response.body() != null) {
                    data.postValue(response.body());
                }
            }

            @Override
            public void onFailure(@NonNull Call<User> call, @NonNull Throwable t) {
                data.postValue(null);
                t.printStackTrace();
            }
        });
        return data;
    }
}

Solution

The issue is that fetchUser creates a new LiveData<> every time you call it.

This means that your first one will never receive an update.

Please take a look at these...

Repository

public class UserRepository {    
    private ApiService apiService;
    private static UserRepository userRepository;

    private UserRepository() {
        apiService = RestClient.getClient().create(ApiService.class);
    }

    public synchronized static UserRepository getInstance() {
        if (userRepository == null) userRepository = new UserRepository();
        return userRepository;
    }

    // Your example code
    public LiveData<User> fetchUser() {
        // Your problem lies here. Every time you fetch user data, you create a new LiveData.
        // Instead, fetch user should update the data on a pre-existing LiveData.
        final MutableLiveData<User> data = new MutableLiveData<>();
        Call<User> call = apiService.getUser();
        call.enqueue(new Callback<User>() {
            @Override
            public void onResponse(@NonNull Call<User> call, @NonNull Response<User> response) {
                if (response.body() != null) {
                    data.postValue(response.body());
                }
            }

            @Override
            public void onFailure(@NonNull Call<User> call, @NonNull Throwable t) {
                data.postValue(null);
                t.printStackTrace();
            }
        });
        return data;
    }

    // My alterations below:
    private MutableLiveData<User> userLiveData = new MutableLiveData<>();

    public LiveData<User> getUser() {
        return userLiveData;
    }

    public LiveData<User> fetchUser2() {
        Call<User> call = apiService.getUser();
        call.enqueue(new Callback<User>() {
            @Override
            public void onResponse(@NonNull Call<User> call, @NonNull Response<User> response) {
                if (response.body() != null) {
                    userLiveData.postValue(response.body());
                }
                // TODO: Consider a fallback response to the LiveData here, in the case that bad data is returned. Perhaps null?
            }

            @Override
            public void onFailure(@NonNull Call<User> call, @NonNull Throwable t) {
                userLiveData.postValue(null);
                t.printStackTrace();
            }
        });
        return userLiveData;
    }
}

ViewModel

I would also change this slightly. Instead of observing fetch, I would observe the LiveData directly.

user = UserRepository.getInstance().getUser();

Later, you can request updated data from the server at any point.

UserRepository.getInstance().fetchUser2();

You could also call fetchUser2() on the first construction of UserRepository. Then only updates would call fetchUser2() directly.

private UserRepository() {
    apiService = RestClient.getClient().create(ApiService.class);
    fetchUser2();
}

Fragment

Also, in your Fragment, do not observe on this. Instead use getViewLifecycleOwner()

userViewModel.getUser().observe(getViewLifecycleOwner(), new Observer<User>() {
    @Override
    public void onChanged(User user) {
        //Set UI
    }
});

Comments

[jackycflau]

would you mind to explain what's the difference of using this vs getViewLifecycleOwner() ?

[Knossos]

It changes what the LiveData binds to. The problem is that Fragment lives much longer than you might expect. This can lead to observing the same LiveData multiple times. getViewLifecycleOwner() will actually bind to the lifecycle of the fragments View. So when the view hierarchy is destroyed, the LiveData observers will be unbound too.

[Knossos]

For a more official explanation, there is also this link from the docs: developer.android.com/reference/androidx/fragment/app/…

[Saurabh Thorat]

It still doesn't update. And with this code, even after the fragment is destroyed and finished, it still shows the old data.

[Knossos]

You might consider adding logs to the code in all places. Log in your repository under fetchUser2() to ensure that the LiveData is being transmitted to, for example. So log in fetchUser2() directly, in onResponse() and in onFailure(). Double check that you aren't still using fetchUser() in your code, if you just copy and pasted.

[Knossos]

Additionally, about showing old data, that is part of the point of having a ViewModel. It retains state through rebuilding. If that is a problem for you, you will need to invalidate that yourself when starting the Fragment.

[Saurabh Thorat]

is fetchUser2() really required? Can't it be done with just one method? And regarding old data still being shown, afaik ViewModel clears data after observers lifecycle has ended. Here it still showed old data.

[Saurabh Thorat]

I also debugged it, the code simply doesn't go in any repository method.

[Knossos]

Well, you can delete the old fetchUser() method, and rename fetchUser2(). You can also just observe fetchUser() and not getUser() if you want to. I personally prefer to separate concerns where possible. Where are you trying to refresh the data? Do you have a button in your Fragment?

[Saurabh Thorat]

Let us continue this discussion in chat.

[IgorGanapolsky]

onChanged is not necessary.