Skip to content

Commit d131c6e

Browse files
lfkellogglfkellogg
authored
Replace getForegroundActivity with applyToForegroundActivity[Task] (#3350)
* Add getForegroundActivity overload with consumer * Refactor based on feedback * Use SuccessContinuation instead of creating new ActivityContinuation interface
1 parent 0784d9c commit d131c6e

File tree

12 files changed

+290
-134
lines changed

12 files changed

+290
-134
lines changed

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/AabUpdater.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE;
1818
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.NETWORK_FAILURE;
19-
import static com.google.firebase.appdistribution.TaskUtils.combineWithResultOf;
2019
import static com.google.firebase.appdistribution.TaskUtils.runAsyncInTask;
2120
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskException;
2221

@@ -97,10 +96,10 @@ UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
9796

9897
// On a background thread, fetch the redirect URL and open it in the Play app
9998
runAsyncInTask(executor, () -> fetchDownloadRedirectUrl(newRelease.getDownloadUrl()))
100-
.onSuccessTask(combineWithResultOf(() -> lifecycleNotifier.getForegroundActivity()))
101-
.addOnSuccessListener(
102-
urlAndActivity ->
103-
openRedirectUrlInPlay(urlAndActivity.first(), urlAndActivity.second()))
99+
.onSuccessTask(
100+
redirectUrl ->
101+
lifecycleNotifier.applyToForegroundActivity(
102+
activity -> openRedirectUrlInPlay(redirectUrl, activity)))
104103
.addOnFailureListener(this::setUpdateTaskCompletionError);
105104

106105
return cachedUpdateTask;

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/ApkUpdater.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ UpdateTaskImpl updateApk(
113113

114114
private void installApk(File file, boolean showDownloadNotificationManager) {
115115
lifeCycleNotifier
116-
.getForegroundActivity()
117-
.onSuccessTask(taskExecutor, activity -> apkInstaller.installApk(file.getPath(), activity))
116+
.applyToForegroundActivityTask(
117+
activity -> apkInstaller.installApk(file.getPath(), activity))
118118
.addOnSuccessListener(
119119
taskExecutor,
120120
unused -> {

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,7 @@ public UpdateTask updateIfNewReleaseAvailable() {
142142
}
143143

144144
lifecycleNotifier
145-
.getForegroundActivity()
146-
.onSuccessTask(this::showSignInConfirmationDialog)
145+
.applyToForegroundActivityTask(this::showSignInConfirmationDialog)
147146
// TODO(rachelprince): Revisit this comment once changes to checkForNewRelease are reviewed
148147
// Even though checkForNewRelease() calls signInTester(), we explicitly call signInTester
149148
// here both for code clarifty, and because we plan to remove the signInTester() call
@@ -173,9 +172,8 @@ public UpdateTask updateIfNewReleaseAvailable() {
173172
setCachedUpdateIfNewReleaseResult();
174173
return Tasks.forResult(null);
175174
}
176-
return lifecycleNotifier
177-
.getForegroundActivity()
178-
.onSuccessTask((activity) -> showUpdateAlertDialog(activity, release));
175+
return lifecycleNotifier.applyToForegroundActivityTask(
176+
activity -> showUpdateAlertDialog(activity, release));
179177
})
180178
.onSuccessTask(
181179
unused ->

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistributionLifecycleNotifier.java

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,25 @@
2020
import androidx.annotation.GuardedBy;
2121
import androidx.annotation.NonNull;
2222
import androidx.annotation.Nullable;
23+
import androidx.annotation.VisibleForTesting;
24+
import com.google.android.gms.tasks.SuccessContinuation;
2325
import com.google.android.gms.tasks.Task;
2426
import com.google.android.gms.tasks.TaskCompletionSource;
2527
import com.google.android.gms.tasks.Tasks;
2628
import java.util.ArrayDeque;
2729
import java.util.Queue;
30+
import java.util.concurrent.Executor;
2831

2932
class FirebaseAppDistributionLifecycleNotifier implements Application.ActivityLifecycleCallbacks {
3033

34+
/** An {@link Executor} that runs tasks on the current thread. */
35+
private static final Executor DIRECT_EXECUTOR = Runnable::run;
36+
37+
/** A functional interface for a function that takes an activity and does something with it. */
38+
interface ActivityConsumer<T> {
39+
void consume(Activity activity);
40+
}
41+
3142
private static FirebaseAppDistributionLifecycleNotifier instance;
3243
private final Object lock = new Object();
3344

@@ -54,7 +65,8 @@ class FirebaseAppDistributionLifecycleNotifier implements Application.ActivityLi
5465
@GuardedBy("lock")
5566
private final Queue<OnActivityDestroyedListener> onDestroyedListeners = new ArrayDeque<>();
5667

57-
private FirebaseAppDistributionLifecycleNotifier() {}
68+
@VisibleForTesting
69+
FirebaseAppDistributionLifecycleNotifier() {}
5870

5971
static synchronized FirebaseAppDistributionLifecycleNotifier getInstance() {
6072
if (instance == null) {
@@ -84,13 +96,43 @@ interface OnActivityDestroyedListener {
8496
}
8597

8698
/**
87-
* Get a {@link Task} that will succeed with a result of the app's foregrounded {@link Activity},
88-
* when one is available.
89-
*
90-
* <p>The returned task will never fail. It will instead remain pending indefinitely until some
91-
* activity comes to the foreground.
99+
* Apply a function to a foreground activity, when one is available, returning a {@link Task} that
100+
* will complete immediately after the function is applied.
101+
*/
102+
Task<Void> applyToForegroundActivity(ActivityConsumer consumer) {
103+
return getForegroundActivity()
104+
.onSuccessTask(
105+
// Use direct executor to ensure the consumer is called while Activity is in foreground
106+
DIRECT_EXECUTOR,
107+
activity -> {
108+
try {
109+
consumer.consume(activity);
110+
return Tasks.forResult(null);
111+
} catch (Throwable t) {
112+
return Tasks.forException(FirebaseAppDistributionException.wrap(t));
113+
}
114+
});
115+
}
116+
117+
/**
118+
* Apply a function to a foreground activity, when one is available, returning a {@link Task} that
119+
* will complete with the result of the Task returned by that function.
92120
*/
93-
Task<Activity> getForegroundActivity() {
121+
<T> Task<T> applyToForegroundActivityTask(SuccessContinuation<Activity, T> continuation) {
122+
return getForegroundActivity()
123+
.onSuccessTask(
124+
// Use direct executor to ensure the consumer is called while Activity is in foreground
125+
DIRECT_EXECUTOR,
126+
activity -> {
127+
try {
128+
return continuation.then(activity);
129+
} catch (Throwable t) {
130+
return Tasks.forException(FirebaseAppDistributionException.wrap(t));
131+
}
132+
});
133+
}
134+
135+
private Task<Activity> getForegroundActivity() {
94136
synchronized (lock) {
95137
if (currentActivity != null) {
96138
return Tasks.forResult(currentActivity);

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/TaskUtils.java

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@
1414

1515
package com.google.firebase.appdistribution;
1616

17-
import com.google.android.gms.tasks.SuccessContinuation;
1817
import com.google.android.gms.tasks.Task;
1918
import com.google.android.gms.tasks.TaskCompletionSource;
2019
import com.google.android.gms.tasks.Tasks;
21-
import com.google.auto.value.AutoValue;
2220
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
2321
import com.google.firebase.appdistribution.internal.LogWrapper;
2422
import java.util.concurrent.Executor;
@@ -34,11 +32,6 @@ interface Operation<TResult> {
3432
TResult run() throws FirebaseAppDistributionException;
3533
}
3634

37-
/** A functional interface to wrap a function that produces a {@link Task}. */
38-
interface TaskSource<TResult> {
39-
Task<TResult> get();
40-
}
41-
4235
/**
4336
* Runs a long running operation inside a {@link Task}, wrapping any errors in {@link
4437
* FirebaseAppDistributionException}.
@@ -92,58 +85,6 @@ static <TResult> Task<TResult> handleTaskFailure(Task<TResult> task) {
9285
return task;
9386
}
9487

95-
/**
96-
* An @{link AutoValue} class to hold the result of two Tasks, combined using {@link
97-
* #combineWithResultOf}.
98-
*
99-
* @param <T1> The result type of the first task
100-
* @param <T2> The result type of the second task
101-
*/
102-
@AutoValue
103-
abstract static class CombinedTaskResults<T1, T2> {
104-
abstract T1 first();
105-
106-
abstract T2 second();
107-
108-
static <T1, T2> CombinedTaskResults<T1, T2> create(T1 first, T2 second) {
109-
return new AutoValue_TaskUtils_CombinedTaskResults(first, second);
110-
}
111-
}
112-
113-
/**
114-
* Returns a {@link SuccessContinuation} to be chained off of a {@link Task}, that will run
115-
* another task in sequence and combine both results together.
116-
*
117-
* <p>This is useful when you want to run two tasks and use the results of each, but those tasks
118-
* need to be run sequentially. If they can be run in parallel, use {@link Tasks#whenAll} or one
119-
* of its variations.
120-
*
121-
* <p>Usage:
122-
*
123-
* <pre>{@code
124-
* runFirstAsyncTask()
125-
* .onSuccessTask(combineWithResultOf(executor, () -> startSecondAsyncTask())
126-
* .addOnSuccessListener(
127-
* results ->
128-
* doSomethingWithBothResults(results.result1(), results.result2()));
129-
* }</pre>
130-
*
131-
* @param secondTaskSource A {@link TaskSource} providing the next task to run
132-
* @param <T1> The result type of the first task
133-
* @param <T2> The result type of the second task
134-
* @return A {@link SuccessContinuation} that will return a new task with result type {@link
135-
* CombinedTaskResults}, combining the results of both tasks
136-
*/
137-
static <T1, T2> SuccessContinuation<T1, CombinedTaskResults<T1, T2>> combineWithResultOf(
138-
TaskSource<T2> secondTaskSource) {
139-
return firstResult ->
140-
secondTaskSource
141-
.get()
142-
.onSuccessTask(
143-
secondResult ->
144-
Tasks.forResult(CombinedTaskResults.create(firstResult, secondResult)));
145-
}
146-
14788
static void safeSetTaskException(TaskCompletionSource taskCompletionSource, Exception e) {
14889
if (taskCompletionSource != null && !taskCompletionSource.getTask().isComplete()) {
14990
taskCompletionSource.setException(e);

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/TesterSignInManager.java

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package com.google.firebase.appdistribution;
1616

1717
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
18-
import static com.google.firebase.appdistribution.TaskUtils.combineWithResultOf;
1918
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskException;
2019
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskResult;
2120

@@ -141,28 +140,30 @@ public Task<Void> signInTester() {
141140
.getId()
142141
.addOnFailureListener(
143142
handleTaskFailure(ErrorMessages.AUTHENTICATION_ERROR, Status.AUTHENTICATION_FAILURE))
144-
.onSuccessTask(combineWithResultOf(() -> lifecycleNotifier.getForegroundActivity()))
145-
.addOnSuccessListener(
146-
fidAndActivity -> {
147-
// Launch the intent outside of the synchronized block because we don't need to wait
148-
// for the lock, and we don't want to risk the activity leaving the foreground in
149-
// the meantime.
150-
openSignInFlowInBrowser(fidAndActivity.first(), fidAndActivity.second());
151-
// This synchronized block is required by the @GuardedBy annotation, but is not
152-
// practically required in this case because the only reads of this variable are on
153-
// the main thread, which this callback is also running on.
154-
synchronized (signInTaskLock) {
155-
hasBeenSentToBrowserForCurrentTask = true;
156-
}
157-
})
158-
// No failures expected here, since getForegroundActivity() will wait indefinitely for a
159-
// foreground activity, but catch any unexpected failures to be safe.
143+
.onSuccessTask(this::getForegroundActivityAndOpenSignInFlow)
144+
// Catch any unexpected failures to be safe.
160145
.addOnFailureListener(handleTaskFailure(ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN));
161146

162147
return signInTaskCompletionSource.getTask();
163148
}
164149
}
165150

151+
private Task<Void> getForegroundActivityAndOpenSignInFlow(String fid) {
152+
return lifecycleNotifier.applyToForegroundActivity(
153+
activity -> {
154+
// Launch the intent outside of the synchronized block because we don't need to wait
155+
// for the lock, and we don't want to risk the activity leaving the foreground in
156+
// the meantime.
157+
openSignInFlowInBrowser(fid, activity);
158+
// This synchronized block is required by the @GuardedBy annotation, but is not
159+
// practically required in this case because the only reads of this variable are on
160+
// the main thread, which this callback is also running on.
161+
synchronized (signInTaskLock) {
162+
hasBeenSentToBrowserForCurrentTask = true;
163+
}
164+
});
165+
}
166+
166167
private OnFailureListener handleTaskFailure(String message, Status status) {
167168
return e -> {
168169
LogWrapper.getInstance().e(TAG + message, e);

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/AabUpdaterTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,17 @@
1414
package com.google.firebase.appdistribution;
1515

1616
import static com.google.common.truth.Truth.assertThat;
17+
import static com.google.firebase.appdistribution.TestUtils.applyToForegroundActivityAnswer;
1718
import static com.google.firebase.appdistribution.TestUtils.assertTaskFailure;
1819
import static com.google.firebase.appdistribution.TestUtils.awaitAsyncOperations;
1920
import static org.junit.Assert.assertEquals;
21+
import static org.mockito.ArgumentMatchers.any;
2022
import static org.mockito.Mockito.when;
2123
import static org.robolectric.Shadows.shadowOf;
2224
import static org.robolectric.annotation.LooperMode.Mode.PAUSED;
2325

2426
import android.app.Activity;
2527
import android.net.Uri;
26-
import com.google.android.gms.tasks.Tasks;
2728
import com.google.firebase.FirebaseApp;
2829
import com.google.firebase.appdistribution.Constants.ErrorMessages;
2930
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
@@ -88,7 +89,9 @@ public void setup() throws IOException, FirebaseAppDistributionException {
8889
aabUpdater =
8990
Mockito.spy(
9091
new AabUpdater(mockLifecycleNotifier, mockHttpsUrlConnectionFactory, testExecutor));
91-
when(mockLifecycleNotifier.getForegroundActivity()).thenReturn(Tasks.forResult(activity));
92+
93+
when(mockLifecycleNotifier.applyToForegroundActivity(any()))
94+
.thenAnswer(applyToForegroundActivityAnswer(activity));
9295
}
9396

9497
@Test

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/ApkUpdaterTest.java

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.firebase.appdistribution;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static com.google.firebase.appdistribution.TestUtils.applyToForegroundActivityTaskAnswer;
1819
import static org.junit.Assert.assertThrows;
1920
import static org.mockito.ArgumentMatchers.any;
2021
import static org.mockito.Mockito.doReturn;
@@ -102,7 +103,8 @@ public void setup() throws IOException, FirebaseAppDistributionException {
102103
when(mockFile.length()).thenReturn(TEST_FILE_LENGTH);
103104
when(mockHttpsUrlConnectionFactory.openConnection(TEST_URL)).thenReturn(mockHttpsUrlConnection);
104105
when(mockHttpsUrlConnection.getResponseCode()).thenReturn(200);
105-
when(mockLifecycleNotifier.getForegroundActivity()).thenReturn(Tasks.forResult(activity));
106+
when(mockLifecycleNotifier.applyToForegroundActivityTask(any()))
107+
.thenAnswer(applyToForegroundActivityTaskAnswer(activity));
106108
onCompleteListener = new TestOnCompleteListener<>();
107109

108110
apkUpdater =
@@ -169,36 +171,6 @@ public void updateApk_whenInstallSuccessful_setsResult() throws Exception {
169171
assertThat(updateTask.isSuccessful()).isTrue();
170172
}
171173

172-
@Test
173-
public void updateApk_getForegroundActivityFails_setsError() {
174-
boolean showNotification = true;
175-
doReturn(Tasks.forResult(mockFile))
176-
.when(apkUpdater)
177-
.downloadApk(TEST_RELEASE, showNotification);
178-
TaskCompletionSource<Activity> getForegroundActivityCompletionSource =
179-
new TaskCompletionSource<>();
180-
when(mockLifecycleNotifier.getForegroundActivity())
181-
.thenReturn(getForegroundActivityCompletionSource.getTask());
182-
UpdateTask updateTask = apkUpdater.updateApk(TEST_RELEASE, showNotification);
183-
updateTask.addOnCompleteListener(testExecutor, onCompleteListener);
184-
List<UpdateProgress> progressEvents = new ArrayList<>();
185-
updateTask.addOnProgressListener(testExecutor, progressEvents::add);
186-
187-
getForegroundActivityCompletionSource.setException(
188-
new FirebaseAppDistributionException(
189-
Constants.ErrorMessages.APK_INSTALLATION_FAILED,
190-
FirebaseAppDistributionException.Status.INSTALLATION_FAILURE));
191-
192-
assertThat(updateTask.isComplete()).isFalse();
193-
FirebaseAppDistributionException e =
194-
assertThrows(FirebaseAppDistributionException.class, () -> onCompleteListener.await());
195-
assertThat(e.getErrorCode()).isEqualTo(Status.INSTALLATION_FAILURE);
196-
assertThat(progressEvents).hasSize(1);
197-
assertThat(progressEvents.get(0).getUpdateStatus()).isEqualTo(UpdateStatus.INSTALL_FAILED);
198-
assertThat(updateTask.isSuccessful()).isFalse();
199-
verify(mockNotificationsManager).updateNotification(1000, 1000, UpdateStatus.INSTALL_FAILED);
200-
}
201-
202174
@Test
203175
public void updateApk_whenInstallFailed_setsError() {
204176
boolean showNotification = true;

0 commit comments

Comments
 (0)