Skip to content

Fad cancel task when dialog closed #3228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 10, 2021
Merged

Fad cancel task when dialog closed #3228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c37812
Add an on cancelled listener to prevent hanging tasks when dialog is …
Dec 8, 2021
2dba16b
Merge remote-tracking branch 'origin/master' into fad-cancel-task-whe…
Dec 8, 2021
a01e43c
Add tests for dialog cancellation and dismissal
Dec 9, 2021
2bbf265
Fix test setup methods
Dec 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions ...tribution/src/main/java/com/google/firebase/app/distribution/FirebaseAppDistribution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,19 +346,11 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
updateDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.update_no_button),
(dialogInterface, i) -> {
dialogInterface.dismiss();
synchronized (updateIfNewReleaseTaskLock) {
postProgressToCachedUpdateIfNewReleaseTask(
UpdateProgress.builder()
.setApkFileTotalBytes(UNKNOWN_RELEASE_FILE_SIZE)
.setApkBytesDownloaded(UNKNOWN_RELEASE_FILE_SIZE)
.setUpdateStatus(UpdateStatus.UPDATE_CANCELED)
.build());
setCachedUpdateIfNewReleaseCompletionError(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED));
}
(dialogInterface, i) -> dismissUpdateDialogCallback());

updateDialog.setOnCancelListener(
dialogInterface -> {
dismissUpdateDialogCallback();
});

updateDialog.show();
Expand All @@ -368,6 +360,20 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
}
}

private void dismissUpdateDialogCallback() {
synchronized (updateIfNewReleaseTaskLock) {
postProgressToCachedUpdateIfNewReleaseTask(
UpdateProgress.builder()
.setApkFileTotalBytes(UNKNOWN_RELEASE_FILE_SIZE)
.setApkBytesDownloaded(UNKNOWN_RELEASE_FILE_SIZE)
.setUpdateStatus(UpdateStatus.UPDATE_CANCELED)
.build());
setCachedUpdateIfNewReleaseCompletionError(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED));
}
}

private void setCachedUpdateIfNewReleaseCompletionError(FirebaseAppDistributionException e) {
synchronized (updateIfNewReleaseTaskLock) {
safeSetTaskException(cachedUpdateIfNewReleaseTask, e);
Expand Down
14 changes: 8 additions & 6 deletions ...-app-distribution/src/main/java/com/google/firebase/app/distribution/InstallActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.io.File;

/**
* Activity opened during installation in {@link UpdateAppClient} after APK download is finished.
* Activity opened during installation in {@link UpdateApkClient} after APK download is finished.
*/
public class InstallActivity extends AppCompatActivity {
private static final String TAG = "InstallActivity: ";
Expand Down Expand Up @@ -92,15 +92,17 @@ private void showUnknownSourcesUi() {
alertDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
getString(R.string.update_no_button),
(dialogInterface, i) -> {
LogWrapper.getInstance().v(TAG + "Unknown sources dialog cancelled");
dialogInterface.dismiss();
finish();
});
(dialogInterface, i) -> dismissUnknownSourcesDialogCallback());
alertDialog.setOnCancelListener(dialogInterface -> dismissUnknownSourcesDialogCallback());

alertDialog.show();
}

private void dismissUnknownSourcesDialogCallback() {
LogWrapper.getInstance().v(TAG + "Unknown sources dialog cancelled");
finish();
}

private Intent getUnknownSourcesIntent() {
Intent intent;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Expand Down
56 changes: 25 additions & 31 deletions ...p-distribution/src/main/java/com/google/firebase/app/distribution/TesterSignInClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@
import android.app.Activity;
import android.app.AlertDialog;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
import android.content.pm.ResolveInfo;
import android.net.Uri;
import androidx.annotation.GuardedBy;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import androidx.browser.customtabs.CustomTabsIntent;
import com.google.android.gms.tasks.OnFailureListener;
import com.google.android.gms.tasks.OnSuccessListener;
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.TaskCompletionSource;
Expand Down Expand Up @@ -162,42 +160,38 @@ private AlertDialog getSignInAlertDialog(Activity currentActivity) {
alertDialog.setButton(
AlertDialog.BUTTON_POSITIVE,
context.getString(R.string.singin_yes_button),
new DialogInterface.OnClickListener() {
@Override
public void onClick(DialogInterface dialogInterface, int i) {
firebaseInstallationsApi
.getId()
.addOnSuccessListener(getFidGenerationOnSuccessListener(currentActivity))
.addOnFailureListener(
new OnFailureListener() {
@Override
public void onFailure(@NonNull Exception e) {
LogWrapper.getInstance().e(TAG + "Fid retrieval failed.", e);
setSignInTaskCompletionError(
new FirebaseAppDistributionException(
Constants.ErrorMessages.AUTHENTICATION_ERROR,
AUTHENTICATION_FAILURE,
e));
}
});
}
(dialogInterface, i) -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

firebaseInstallationsApi
.getId()
.addOnSuccessListener(getFidGenerationOnSuccessListener(currentActivity))
.addOnFailureListener(
e -> {
LogWrapper.getInstance().e(TAG + "Fid retrieval failed.", e);
setSignInTaskCompletionError(
new FirebaseAppDistributionException(
Constants.ErrorMessages.AUTHENTICATION_ERROR,
AUTHENTICATION_FAILURE,
e));
});
});

alertDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.singin_no_button),
new DialogInterface.OnClickListener() {
@Override
public void onClick(DialogInterface dialogInterface, int i) {
LogWrapper.getInstance().v("Sign in has been canceled.");
setSignInTaskCompletionError(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED));
dialogInterface.dismiss();
}
});
(dialogInterface, i) -> dismissSignInDialogCallback());

alertDialog.setOnCancelListener(dialogInterface -> dismissSignInDialogCallback());

return alertDialog;
}

private void dismissSignInDialogCallback() {
LogWrapper.getInstance().v("Sign in has been canceled.");
setSignInTaskCompletionError(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED));
}

private void setSignInTaskCompletionError(FirebaseAppDistributionException e) {
synchronized (signInTaskLock) {
safeSetTaskException(signInTaskCompletionSource, e);
Expand Down
89 changes: 52 additions & 37 deletions ...ution/src/test/java/com/google/firebase/app/distribution/FirebaseAppDistributionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ public void setup() {

activity = Robolectric.buildActivity(TestActivity.class).create().get();
when(mockLifecycleNotifier.getCurrentActivity()).thenReturn(activity);
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
}

@Test
public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() {
firebaseAppDistribution.setCachedNewRelease(null);
when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(
Tasks.forException(
Expand Down Expand Up @@ -234,55 +234,40 @@ public void updateApp_whenNotSignedIn_throwsError() {

@Test
public void updateToNewRelease_whenNewAabReleaseAvailable_showsUpdateDialog() {
// mockSignInStorage returns false then true to simulate logging in during first signIn check in
// updateIfNewReleaseAvailable
when(mockSignInStorage.getSignInStatus()).thenReturn(false).thenReturn(true);
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
firebaseAppDistribution.setCachedNewRelease(newRelease);
doReturn(new UpdateTaskImpl()).when(mockUpdateAabClient).updateAab(newRelease);
when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build())));

firebaseAppDistribution.updateIfNewReleaseAvailable();

// Update flow
verify(mockTesterSignInClient, times(1)).signInTester();
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
AlertDialog dialog = verifyUpdateAlertDialog();
assertEquals(
String.format(
"Version %s (%s) is available.\n\nRelease notes: %s",
TEST_RELEASE_NEWER_AAB.getDisplayVersion(),
TEST_RELEASE_NEWER_AAB.getVersionCode(),
TEST_RELEASE_NEWER_AAB.getReleaseNotes()),
shadowOf(updateDialog).getMessage().toString());
assertTrue(updateDialog.isShowing());
shadowOf(dialog).getMessage().toString());
}

@Test
public void updateToNewRelease_whenReleaseNotesEmpty_doesNotShowReleaseNotes() {
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
AppDistributionReleaseInternal newRelease =
TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build();
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
firebaseAppDistribution.setCachedNewRelease(newRelease);
when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build())));

firebaseAppDistribution.updateIfNewReleaseAvailable();

// Update flow
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
AlertDialog dialog = verifyUpdateAlertDialog();
assertEquals(
String.format(
"Version %s (%s) is available.",
TEST_RELEASE_NEWER_AAB.getDisplayVersion(), TEST_RELEASE_NEWER_AAB.getVersionCode()),
shadowOf(updateDialog).getMessage().toString());
shadowOf(dialog).getMessage().toString());
}

@Test
public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(null));
firebaseAppDistribution.setCachedNewRelease(null);

UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();

Expand All @@ -298,7 +283,6 @@ public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
public void updateToNewRelease_whenActivityBackgrounded_updateDialogNotShown() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(null));
firebaseAppDistribution.setCachedNewRelease(null);
when(mockLifecycleNotifier.getCurrentActivity()).thenReturn(null);

UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
Expand Down Expand Up @@ -352,6 +336,41 @@ public void updateToNewRelease_whenSignInFailed_checkForUpdateNotCalled() {
assertEquals(AUTHENTICATION_FAILURE, e.getErrorCode());
}

@Test
public void updateToNewRelease_whenDialogDismissed_taskFails() {
when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));

UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
AlertDialog updateDialog = verifyUpdateAlertDialog();
updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog

assertFalse(updateDialog.isShowing());
assertFalse(updateTask.isSuccessful());
Exception e = updateTask.getException();
assertTrue(e instanceof FirebaseAppDistributionException);
assertEquals(INSTALLATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode());
assertEquals(ErrorMessages.UPDATE_CANCELED, e.getMessage());
}

@Test
public void updateToNewRelease_whenDialogCanceled_taskFails() {
when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));

UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();

AlertDialog updateDialog = verifyUpdateAlertDialog();
updateDialog.onBackPressed(); // cancels the dialog

assertFalse(updateDialog.isShowing());
assertFalse(updateTask.isSuccessful());
Exception e = updateTask.getException();
assertTrue(e instanceof FirebaseAppDistributionException);
assertEquals(INSTALLATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode());
assertEquals(ErrorMessages.UPDATE_CANCELED, e.getMessage());
}

@Test
public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
when(mockCheckForNewReleaseClient.checkForNewRelease())
Expand All @@ -374,14 +393,6 @@ public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
assertEquals(FirebaseAppDistributionException.Status.NETWORK_FAILURE, e.getErrorCode());
}

@Test
public void updateToNewRelease_callsSignInTester() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: did you remove this test because it's only verifying a method call?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I meant to leave a note here. I felt this test was testing implementation, not behavior, so I removed it

when(mockCheckForNewReleaseClient.checkForNewRelease())
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));
firebaseAppDistribution.updateIfNewReleaseAvailable();
verify(mockTesterSignInClient, times(1)).signInTester();
}

@Test
public void signOutTester_setsSignInStatusFalse() {
firebaseAppDistribution.signOutTester();
Expand All @@ -390,10 +401,8 @@ public void signOutTester_setsSignInStatusFalse() {

@Test
public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
firebaseAppDistribution.setCachedNewRelease(newRelease);
UpdateTaskImpl mockTask = new UpdateTaskImpl();
when(mockUpdateAabClient.updateAab(newRelease)).thenReturn(mockTask);
mockTask.updateProgress(
Expand All @@ -419,10 +428,8 @@ public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {

@Test
public void taskCancelledOnScreenRotation() {
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
firebaseAppDistribution.setCachedNewRelease(newRelease);

UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();

Expand Down Expand Up @@ -475,4 +482,12 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() {

assertEquals(updateTask, updateTaskToReturn);
}

private AlertDialog verifyUpdateAlertDialog() {
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
AlertDialog dialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
assertTrue(dialog.isShowing());

return dialog;
}
}
Loading