Skip to content

Commit 27db9e6

Browse files
lfkellogglfkellogg
committed
Refactor based on feedback
1 parent 03bcda9 commit 27db9e6

File tree

11 files changed

+178
-183
lines changed

11 files changed

+178
-183
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
9898
runAsyncInTask(executor, () -> fetchDownloadRedirectUrl(newRelease.getDownloadUrl()))
9999
.onSuccessTask(
100100
redirectUrl ->
101-
lifecycleNotifier.getForegroundActivity(
101+
lifecycleNotifier.applyToForegroundActivity(
102102
activity -> openRedirectUrlInPlay(redirectUrl, activity)))
103103
.addOnFailureListener(this::setUpdateTaskCompletionError);
104104

‎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: 46 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,27 @@
2121
import androidx.annotation.NonNull;
2222
import androidx.annotation.Nullable;
2323
import androidx.annotation.VisibleForTesting;
24+
import com.google.android.gms.tasks.SuccessContinuation;
2425
import com.google.android.gms.tasks.Task;
2526
import com.google.android.gms.tasks.TaskCompletionSource;
2627
import com.google.android.gms.tasks.Tasks;
2728
import java.util.ArrayDeque;
2829
import java.util.Queue;
30+
import java.util.concurrent.Executor;
2931

3032
class FirebaseAppDistributionLifecycleNotifier implements Application.ActivityLifecycleCallbacks {
3133

32-
/** A functional interface for a function that takes an activity and returns a value. */
33-
interface ActivityFunction<T> {
34-
T apply(Activity activity) throws FirebaseAppDistributionException;
35-
}
34+
/** An {@link Executor} that runs tasks on the current thread. */
35+
private static final Executor DIRECT_EXECUTOR = Runnable::run;
3636

37-
/** A functional interface for a function that takes an activity and returns a {@link Task}. */
38-
interface ActivityChainingFunction<T> {
39-
Task<T> apply(Activity activity) throws FirebaseAppDistributionException;
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);
4040
}
4141

42+
/** A {@link SuccessContinuation} that takes an activity and returns a new {@link Task}. */
43+
interface ActivityContinuation<T> extends SuccessContinuation<Activity, T> {}
44+
4245
private static FirebaseAppDistributionLifecycleNotifier instance;
4346
private final Object lock = new Object();
4447

@@ -96,13 +99,43 @@ interface OnActivityDestroyedListener {
9699
}
97100

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

125-
// /**
126-
// * Get a {@link Task} that will succeed with a result of the app's foregrounded {@link Activity},
127-
// * when one is available.
128-
// *
129-
// * <p>The returned task will never fail. It will instead remain pending indefinitely until some
130-
// * activity comes to the foreground.
131-
// */
132-
// Task<Activity> getForegroundActivity() {
133-
// return getForegroundActivity(activity -> {});
134-
// }
135-
136-
// /**
137-
// * Get a {@link Task} that results from applying a {@link ActivityFunction} applied to the app's
138-
// * foregrounded {@link Activity}, when one is available.
139-
// *
140-
// * <p>The returned task will fail if the {@link ActivityFunction} throws, or the task it returns
141-
// * fails. Otherwise it will never fail, and will wait indefinitely for a foreground activity
142-
// * before applying the function.
143-
// */
144-
// <T> Task<T> applyToForegroundActivityChaining(ActivityFunction<Task<T>> function) {
145-
// synchronized (lock) {
146-
// TaskCompletionSource<T> task = new TaskCompletionSource<>();
147-
// if (currentActivity != null) {
148-
// chainToActivity(task, currentActivity, function);
149-
// } else {
150-
// addOnActivityResumedListener(
151-
// new OnActivityResumedListener() {
152-
// @Override
153-
// public void onResumed(Activity activity) {
154-
// chainToActivity(task, activity, function);
155-
// removeOnActivityResumedListener(this);
156-
// }
157-
// });
158-
// }
159-
//
160-
// return task.getTask();
161-
// }
162-
// }
163-
//
164-
// <T> Task<T> chainToActivity(
165-
// TaskCompletionSource<T> task, Activity activity, ActivityFunction<Task<T>> function) {
166-
// MoreExecutors
167-
// try {
168-
// function.apply(activity)
169-
// .addOnSuccessListener(task::setResult)
170-
// .addOnFailureListener(task::setException)
171-
// .addOnCanceledListener(task::canc)
172-
// continuation.on
173-
// task.setResult(function.apply(activity));
174-
// } catch (Throwable t) {
175-
// task.setException(FirebaseAppDistributionException.wrap(t));
176-
// }
177-
// }
178-
//
179-
// /**
180-
// * Get a {@link Task} that will succeed with a result of applying an {@link ActivityFunction} to
181-
// * the app's foregrounded {@link Activity}, when one is available.
182-
// *
183-
// * <p>The returned task will fail with a {@link FirebaseAppDistributionException} if the {@link
184-
// * ActivityFunction} throws. Otherwise it will never fail, and will wait indefinitely for a
185-
// * foreground activity before applying the function.
186-
// */
187-
// <T> Task<T> applyToForegroundActivity(ActivityFunction<T> function) {
188-
// synchronized (lock) {
189-
// TaskCompletionSource<T> task = new TaskCompletionSource<>();
190-
// if (currentActivity != null) {
191-
// applyToActivityAndCompleteTask(task, currentActivity, function);
192-
// } else {
193-
// addOnActivityResumedListener(
194-
// new OnActivityResumedListener() {
195-
// @Override
196-
// public void onResumed(Activity activity) {
197-
// applyToActivityAndCompleteTask(task, activity, function);
198-
// removeOnActivityResumedListener(this);
199-
// }
200-
// });
201-
// }
202-
//
203-
// return task.getTask();
204-
// }
205-
// }
206-
//
207-
// <T> void applyToActivityAndCompleteTask(
208-
// TaskCompletionSource<T> task, Activity activity, ActivityFunction<T> function) {
209-
// try {
210-
// task.setResult(function.apply(activity));
211-
// } catch (Throwable t) {
212-
// task.setException(FirebaseAppDistributionException.wrap(t));
213-
// }
214-
// }
215-
216158
void addOnActivityCreatedListener(@NonNull OnActivityCreatedListener listener) {
217159
synchronized (lock) {
218160
this.onActivityCreatedListeners.add(listener);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ public Task<Void> signInTester() {
148148
}
149149
}
150150

151-
private Task<Activity> getForegroundActivityAndOpenSignInFlow(String fid) {
152-
return lifecycleNotifier.getForegroundActivity(
151+
private Task<Void> getForegroundActivityAndOpenSignInFlow(String fid) {
152+
return lifecycleNotifier.applyToForegroundActivity(
153153
activity -> {
154154
// Launch the intent outside of the synchronized block because we don't need to wait
155155
// for the lock, and we don't want to risk the activity leaving the foreground in

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
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;
19-
import static com.google.firebase.appdistribution.TestUtils.getForegroundActivityAnswer;
2020
import static org.junit.Assert.assertEquals;
2121
import static org.mockito.ArgumentMatchers.any;
2222
import static org.mockito.Mockito.when;
@@ -90,8 +90,8 @@ public void setup() throws IOException, FirebaseAppDistributionException {
9090
Mockito.spy(
9191
new AabUpdater(mockLifecycleNotifier, mockHttpsUrlConnectionFactory, testExecutor));
9292

93-
when(mockLifecycleNotifier.getForegroundActivity(any()))
94-
.thenAnswer(getForegroundActivityAnswer(activity));
93+
when(mockLifecycleNotifier.applyToForegroundActivity(any()))
94+
.thenAnswer(applyToForegroundActivityAnswer(activity));
9595
}
9696

9797
@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)