Skip to content

Commit

Permalink
Androapp 5769 application not responding anr for at least 5000 ms (#3457
Browse files Browse the repository at this point in the history
)

* fix: [ANDROAPP-5769] re-adapt data set groups request to coroutines, modify double call to data set detail fragment

* fix: [ANDROAPP-5769] fix double call to data set detail fragment

* fix: [ANDROAPP-5769] adapt DataSetListViewModelTest to new coroutine flow

* fix: [ANDROAPP-5769] remove unnecessary import

* fix: [ANDROAPP-5769] fix code smell
  • Loading branch information
xavimolloy authored Jan 16, 2024
1 parent 87cd3d8 commit 47dfd5f
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fun dataSetDetailRobot(dataSetDetailRobot: DataSetDetailRobot.() -> Unit) {
class DataSetDetailRobot : BaseRobot() {

fun clickOnAddDataSet() {
onView(withId(R.id.addDatasetButton)).perform(click())
waitForView(withId(R.id.addDatasetButton)).perform(click())
}

fun checkDataSetInList(period: String, orgUnit: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public class DataSetDetailActivity extends ActivityGlobalAbstract implements Dat
private String dataSetUid;
public DataSetDetailComponent dataSetDetailComponent;

private Fragment fragment;

@Inject
DataSetDetailPresenter presenter;

Expand Down Expand Up @@ -104,19 +106,21 @@ protected void onCreate(Bundle savedInstanceState) {
private void configureBottomNavigation() {
boolean accessWriteData = Boolean.parseBoolean(getIntent().getStringExtra(Constants.ACCESS_DATA));
viewModel.getPageConfiguration().observe(this, binding.navigationBar::pageConfiguration);
binding.navigationBar.setOnNavigationItemSelectedListener(item -> {
Fragment fragment = null;

binding.navigationBar.setOnItemSelectedListener(item -> {
Fragment newFragment = null;
switch (item.getItemId()) {
case R.id.navigation_list_view:
fragment = DataSetListFragment.newInstance(dataSetUid, accessWriteData);
newFragment = DataSetListFragment.newInstance(dataSetUid, accessWriteData);
break;
case R.id.navigation_analytics:
presenter.trackDataSetAnalytics();
fragment = GroupAnalyticsFragment.Companion.forDataSet(dataSetUid);
newFragment = GroupAnalyticsFragment.Companion.forDataSet(dataSetUid);
break;
}
FragmentTransaction transaction = getSupportFragmentManager().beginTransaction();
if (fragment != null) {
if (fragment == null ||(newFragment != null && !fragment.getClass().toString().equals(newFragment.getClass().toString()))) {
fragment = newFragment;
transaction.replace(R.id.fragmentContainer, fragment).commit();
}
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.dhis2.usescases.datasets.datasetDetail;


import static org.dhis2.data.dhislogic.AuthoritiesKt.AUTH_DATAVALUE_ADD;

import org.dhis2.data.dhislogic.DhisPeriodUtils;
import org.hisp.dhis.android.core.D2;
import org.hisp.dhis.android.core.arch.helpers.UidsHelper;
Expand All @@ -13,21 +11,19 @@
import org.hisp.dhis.android.core.dataset.DataSetCompleteRegistration;
import org.hisp.dhis.android.core.dataset.DataSetEditableStatus;
import org.hisp.dhis.android.core.dataset.DataSetInstanceCollectionRepository;
import org.hisp.dhis.android.core.event.EventEditableStatus;
import org.hisp.dhis.android.core.organisationunit.OrganisationUnit;
import org.hisp.dhis.android.core.period.DatePeriod;
import org.hisp.dhis.android.core.period.Period;
import org.hisp.dhis.android.core.period.PeriodType;
import org.hisp.dhis.android.core.settings.AnalyticsDhisVisualizationsSetting;

import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Locale;
import java.util.Objects;

import dhis2.org.analytics.charts.Charts;
import io.reactivex.Flowable;
import timber.log.Timber;

public class DataSetDetailRepositoryImpl implements DataSetDetailRepository {

Expand Down Expand Up @@ -58,7 +54,6 @@ public Flowable<List<DataSetDetailModel>> dataSetGroups(List<String> orgUnits, L
repo = repo.byPeriodStartDate().inDatePeriods(periodFilter);
if (!catOptComboFilters.isEmpty())
repo = repo.byAttributeOptionComboUid().in(UidsHelper.getUids(catOptComboFilters));

d2.dataSetModule().dataSets().uid(dataSetUid).blockingGet();
int dataSetOrgUnitNumber = d2.organisationUnitModule().organisationUnits()
.byDataSetUids(Collections.singletonList(dataSetUid))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.dhis2.commons.di.dagger.PerFragment
import org.dhis2.commons.filters.FilterManager
import org.dhis2.commons.matomo.MatomoAnalyticsController
import org.dhis2.commons.schedulers.SchedulerProvider
import org.dhis2.commons.viewmodel.DispatcherProvider
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailRepository

@PerFragment
Expand All @@ -24,10 +25,12 @@ class DataSetListModule {
schedulerProvider: SchedulerProvider,
filterManager: FilterManager,
matomoAnalyticsController: MatomoAnalyticsController,
dispatcher: DispatcherProvider,
): DataSetListViewModelFactory = DataSetListViewModelFactory(
dataSetDetailRepository,
schedulerProvider,
filterManager,
matomoAnalyticsController,
dispatcher,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,30 @@ package org.dhis2.usescases.datasets.datasetDetail.datasetList
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import io.reactivex.disposables.CompositeDisposable
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.async
import kotlinx.coroutines.launch
import org.dhis2.commons.filters.FilterManager
import org.dhis2.commons.matomo.Actions
import org.dhis2.commons.matomo.Categories
import org.dhis2.commons.matomo.Labels
import org.dhis2.commons.matomo.MatomoAnalyticsController
import org.dhis2.commons.schedulers.SchedulerProvider
import org.dhis2.commons.viewmodel.DispatcherProvider
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailModel
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailRepository
import org.dhis2.utils.Action
import timber.log.Timber

class DataSetListViewModel(
val dataSetDetailRepository: DataSetDetailRepository,
private val dataSetDetailRepository: DataSetDetailRepository,
schedulerProvider: SchedulerProvider,
val filterManager: FilterManager,
val matomoAnalyticsController: MatomoAnalyticsController,
private val dispatcher: DispatcherProvider,

) : ViewModel() {

var disposable: CompositeDisposable = CompositeDisposable()
private val _datasets = MutableLiveData<List<DataSetDetailModel>>()
val datasets: LiveData<List<DataSetDetailModel>> = _datasets

Expand All @@ -34,28 +38,38 @@ class DataSetListViewModel(
val selectedSync = MutableLiveData<Action<DataSetDetailModel>>()

init {
disposable.add(
filterManager.asFlowable()
.startWith(filterManager)
.flatMap { filterManager: FilterManager ->
dataSetDetailRepository.dataSetGroups(
filterManager.orgUnitUidsFilters,
filterManager.periodFilters,
filterManager.stateFilters,
filterManager.catOptComboFilters,
)
}
.subscribeOn(schedulerProvider.io())
.observeOn(schedulerProvider.ui())
.subscribe({ _datasets.value = it }) { t: Throwable? -> Timber.d(t) },
)

disposable.add(
dataSetDetailRepository.canWriteAny()
.subscribeOn(schedulerProvider.io())
.observeOn(schedulerProvider.ui())
.subscribe({ _canWrite.value = it }) { t: Throwable? -> Timber.e(t) },
)
viewModelScope.launch(dispatcher.io()) {

val datasets = async {
filterManager.asFlowable()
.startWith(filterManager)
.flatMap { filterManager: FilterManager ->

dataSetDetailRepository.dataSetGroups(
filterManager.orgUnitUidsFilters,
filterManager.periodFilters,
filterManager.stateFilters,
filterManager.catOptComboFilters,
)
}
.subscribeOn(schedulerProvider.io())
.observeOn(schedulerProvider.ui())
.subscribe({
_datasets.value = it
}) { t: Throwable? -> Timber.d(t) }
}
val permissions = async {
dataSetDetailRepository.canWriteAny()
.subscribeOn(schedulerProvider.io())
.observeOn(schedulerProvider.ui())
.subscribe({
_canWrite.value = it
}) { t: Throwable? -> Timber.e(t) }
}
datasets.await()
permissions.await()
}
}

fun openDataSet(dataset: DataSetDetailModel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.lifecycle.ViewModelProvider
import org.dhis2.commons.filters.FilterManager
import org.dhis2.commons.matomo.MatomoAnalyticsController
import org.dhis2.commons.schedulers.SchedulerProvider
import org.dhis2.commons.viewmodel.DispatcherProvider
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailRepository

@Suppress("UNCHECKED_CAST")
Expand All @@ -13,6 +14,8 @@ class DataSetListViewModelFactory(
val schedulerProvider: SchedulerProvider,
val filterManager: FilterManager,
val matomoAnalyticsController: MatomoAnalyticsController,
private val dispatchers: DispatcherProvider,

) :
ViewModelProvider.Factory {
override fun <T : ViewModel> create(modelClass: Class<T>): T {
Expand All @@ -21,6 +24,7 @@ class DataSetListViewModelFactory(
schedulerProvider,
filterManager,
matomoAnalyticsController,
dispatchers,
) as T
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import io.reactivex.Flowable
import io.reactivex.processors.FlowableProcessor
import io.reactivex.processors.PublishProcessor
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.setMain
import org.dhis2.commons.filters.FilterManager
import org.dhis2.commons.matomo.Actions
import org.dhis2.commons.matomo.Categories
import org.dhis2.commons.matomo.Labels
import org.dhis2.commons.matomo.MatomoAnalyticsController
import org.dhis2.commons.viewmodel.DispatcherProvider
import org.dhis2.data.schedulers.TrampolineSchedulerProvider
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailModel
import org.dhis2.usescases.datasets.datasetDetail.DataSetDetailRepository
Expand All @@ -25,30 +31,51 @@ import org.mockito.kotlin.whenever
import java.util.Date

class DataSetListViewModelTest {

@get:Rule
val rule = InstantTaskExecutorRule()
val instantTaskExecutorRule = InstantTaskExecutorRule()

private lateinit var viewModel: DataSetListViewModel
private val repository: DataSetDetailRepository = mock()
private val scheduler = TrampolineSchedulerProvider()
private val filterManager: FilterManager = mock()
private val matomoAnalyticsController: MatomoAnalyticsController = mock()

@OptIn(ExperimentalCoroutinesApi::class)
private val testingDispatcher = StandardTestDispatcher()

private val filterProcessor: FlowableProcessor<FilterManager> = PublishProcessor.create()
private val filterManagerFlowable = Flowable.just(filterManager).startWith(filterProcessor)

@OptIn(ExperimentalCoroutinesApi::class)
@Before
fun setUp() {
Dispatchers.setMain(testingDispatcher)

whenever(filterManager.asFlowable()) doReturn filterManagerFlowable
whenever(repository.canWriteAny()) doReturn Flowable.just(true)
viewModel = DataSetListViewModel(
repository,
scheduler,
filterManager,
matomoAnalyticsController,
object : DispatcherProvider {
override fun io(): CoroutineDispatcher {
return testingDispatcher
}

override fun computation(): CoroutineDispatcher {
return testingDispatcher
}

override fun ui(): CoroutineDispatcher {
return testingDispatcher
}
},
)
}

@OptIn(ExperimentalCoroutinesApi::class)
@Test
fun `Should get the list of dataSet`() {
val dataSets = listOf(dummyDataSet(), dummyDataSet(), dummyDataSet())
Expand All @@ -60,13 +87,29 @@ class DataSetListViewModelTest {
scheduler,
filterManager,
matomoAnalyticsController,
object : DispatcherProvider {
override fun io(): CoroutineDispatcher {
return testingDispatcher
}

override fun computation(): CoroutineDispatcher {
return testingDispatcher
}

override fun ui(): CoroutineDispatcher {
return testingDispatcher
}
},
)
testingDispatcher.scheduler.advanceUntilIdle()
assert(viewModel.datasets.value == dataSets)
}

@OptIn(ExperimentalCoroutinesApi::class)
@Test
fun `Should get write permissions`() {
whenever(repository.canWriteAny()) doReturn Flowable.just(true)
testingDispatcher.scheduler.advanceUntilIdle()
assert(viewModel.canWrite.value == true)
}

Expand Down

0 comments on commit 47dfd5f

Please sign in to comment.