Skip to content

Commit

Permalink
Merge pull request getodk#5366 from seadowg/restore-crash
Browse files Browse the repository at this point in the history
Prevent most common restore crashes
  • Loading branch information
seadowg committed Dec 8, 2022
2 parents 74d87b0 + edccfee commit 9dca980
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ object DialogFragmentUtils {
args: Bundle?,
fragmentManager: FragmentManager
) {
showIfNotShowing(createNewInstance(dialogClass, args), dialogClass, fragmentManager)
if (fragmentManager.isDestroyed) {
return
}

val fragmentFactory = fragmentManager.fragmentFactory
val instance = fragmentFactory.instantiate(dialogClass.classLoader, dialogClass.name) as T
instance.arguments = args
showIfNotShowing(instance, dialogClass, fragmentManager)
}

@JvmStatic
Expand Down Expand Up @@ -79,17 +86,4 @@ object DialogFragmentUtils {
}
}
}

private fun <T : DialogFragment> createNewInstance(dialogClass: Class<T>, args: Bundle?): T {
return try {
val instance = dialogClass.newInstance()
instance.arguments = args
instance
} catch (e: IllegalAccessException) {
// These would mean we have a non zero arg constructor for a Fragment
throw RuntimeException(e)
} catch (e: InstantiationException) {
throw RuntimeException(e)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,13 @@ public void onCreate(Bundle savedInstanceState) {
sessionId = savedInstanceState.getString(KEY_SESSION_ID);
}

formEntryViewModelFactory.setSessionId(sessionId);
formSaveViewModelFactoryFactory.setSessionId(sessionId);
backgroundAudioViewModelFactory.setSessionId(sessionId);

this.getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder()
.forClass(AudioRecordingControllerFragment.class, () -> new AudioRecordingControllerFragment(sessionId))
.forClass(QuitFormDialogFragment.class, () -> new QuitFormDialogFragment(formSaveViewModelFactoryFactory))
.build());

super.onCreate(savedInstanceState);
Expand Down Expand Up @@ -442,15 +447,13 @@ private void setupViewModels() {
new BackgroundLocationViewModel.Factory(permissionsProvider, settingsProvider.getUnprotectedSettings(), formSessionRepository, sessionId)
).get(BackgroundLocationViewModel.class);

backgroundAudioViewModelFactory.setSessionId(sessionId);
backgroundAudioViewModel = new ViewModelProvider(this, backgroundAudioViewModelFactory).get(BackgroundAudioViewModel.class);
backgroundAudioViewModel.isPermissionRequired().observe(this, isPermissionRequired -> {
if (isPermissionRequired) {
showIfNotShowing(BackgroundAudioPermissionDialogFragment.class, getSupportFragmentManager());
}
});


identityPromptViewModel = new ViewModelProvider(this).get(IdentityPromptViewModel.class);
identityPromptViewModel.requiresIdentityToContinue().observe(this, requiresIdentity -> {
if (requiresIdentity) {
Expand All @@ -464,7 +467,6 @@ private void setupViewModels() {
}
});

formEntryViewModelFactory.setSessionId(sessionId);
formEntryViewModel = new ViewModelProvider(this, formEntryViewModelFactory)
.get(FormEntryViewModel.class);

Expand Down Expand Up @@ -500,7 +502,6 @@ private void setupViewModels() {
}
});

formSaveViewModelFactoryFactory.setSessionId(sessionId);
formSaveViewModel = new ViewModelProvider(this, formSaveViewModelFactoryFactory.create(this, null)).get(FormSaveViewModel.class);
formSaveViewModel.getSaveResult().observe(this, this::handleSaveResult);
formSaveViewModel.isSavingAnswerFile().observe(this, isSavingAnswerFile -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ public void onCreate(Bundle savedInstanceState) {
formEntryViewModel = new ViewModelProvider(this, formEntryViewModelFactory).get(FormEntryViewModel.class);

FormController formController = formEntryViewModel.getFormController();
if (formController == null) {
finish();
return;
}

startIndex = formController.getFormIndex();

setTitle(formController.getFormTitle());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ public class QuitFormDialogFragment extends DialogFragment {
@Inject
Scheduler scheduler;

@Inject
FormSaveViewModel.FactoryFactory formSaveViewModelFactoryFactory;

@Inject
SettingsProvider settingsProvider;

Expand All @@ -60,6 +57,12 @@ public class QuitFormDialogFragment extends DialogFragment {
private FormEntryViewModel formEntryViewModel;
private Listener listener;

private final FormSaveViewModel.FactoryFactory formSaveViewModelFactoryFactory;

public QuitFormDialogFragment(FormSaveViewModel.FactoryFactory formSaveViewModelFactoryFactory) {
this.formSaveViewModelFactoryFactory = formSaveViewModelFactoryFactory;
}

@Override
public void onAttach(@NonNull Context context) {
super.onAttach(context);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.odk.collect.android.activities

import android.content.Context
import android.content.Intent
import androidx.lifecycle.Lifecycle
import androidx.test.core.app.ActivityScenario
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.equalTo
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class FormHierarchyActivityTest {

/**
* This can happen if the app process is restored after being kicked from memory.
*/
@Test
fun whenFormHasNotLoadedYet_finishes() {
val context = ApplicationProvider.getApplicationContext<Context>()
val intent = Intent(context, FormHierarchyActivity::class.java).also {
it.putExtra(FormHierarchyActivity.EXTRA_SESSION_ID, "blah")
}

ActivityScenario.launch<FormHierarchyActivity>(intent).use { scenario ->
assertThat(scenario.state, equalTo(Lifecycle.State.DESTROYED))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.odk.collect.android.projects.CurrentProjectProvider;
import org.odk.collect.android.support.CollectHelpers;
import org.odk.collect.android.utilities.MediaUtils;
import org.odk.collect.androidshared.ui.FragmentFactoryBuilder;
import org.odk.collect.async.Scheduler;
import org.odk.collect.audiorecorder.recording.AudioRecorder;
import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule;
Expand All @@ -51,34 +52,39 @@ public class QuitFormDialogFragmentTest {
private final FormSaveViewModel formSaveViewModel = mock(FormSaveViewModel.class);
private final FormEntryViewModel formEntryViewModel = mock(FormEntryViewModel.class);

private FormSaveViewModel.FactoryFactory formSaveViewModelFactoryFactory = new FormSaveViewModel.FactoryFactory() {
@Override
public void setSessionId(String sessionId) {

}

@Override
public ViewModelProvider.Factory create(@NonNull SavedStateRegistryOwner owner, @Nullable Bundle defaultArgs) {
return new ViewModelProvider.Factory() {

@NonNull
@Override
public <T extends ViewModel> T create(@NonNull Class<T> modelClass) {
return (T) formSaveViewModel;
}
};
}
};

@Rule
public FragmentScenarioLauncherRule launcherRule = new FragmentScenarioLauncherRule(
R.style.Theme_MaterialComponents
R.style.Theme_MaterialComponents,
new FragmentFactoryBuilder()
.forClass(QuitFormDialogFragment.class, () -> new QuitFormDialogFragment(formSaveViewModelFactoryFactory))
.build()
);

@Before
public void setup() {
CollectHelpers.overrideAppDependencyModule(new AppDependencyModule() {
@Override
public FormSaveViewModel.FactoryFactory providesFormSaveViewModelFactoryFactory(Analytics analytics, Scheduler scheduler, AudioRecorder audioRecorder, CurrentProjectProvider currentProjectProvider, MediaUtils mediaUtils, FormSessionRepository formSessionRepository, EntitiesRepositoryProvider entitiesRepositoryProvider) {
return new FormSaveViewModel.FactoryFactory() {
@Override
public void setSessionId(String sessionId) {

}

@Override
public ViewModelProvider.Factory create(@NonNull SavedStateRegistryOwner owner, @Nullable Bundle defaultArgs) {
return new ViewModelProvider.Factory() {

@NonNull
@Override
public <T extends ViewModel> T create(@NonNull Class<T> modelClass) {
return (T) formSaveViewModel;
}
};
}
};
return formSaveViewModelFactoryFactory;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ class FragmentScenarioLauncherRule @JvmOverloads constructor(
fragmentClass = fragmentClass,
fragmentArgs = fragmentArgs,
themeResId = defaultThemeResId,
factory = null,
factory = defaultFactory,
initialState = initialState
)
} else {
FragmentScenario.launch(
fragmentClass = fragmentClass,
fragmentArgs = fragmentArgs,
factory = null,
factory = defaultFactory,
initialState = initialState
)
}
Expand Down

0 comments on commit 9dca980

Please sign in to comment.