Skip to content

Commit 1dce165

Browse files
andrewbibiloniandrewbibiloni
authored
FAD refactor UpdateApp and UpdateTaskImpl (#2886)
* Remove task completion from updateApp * Refactor updateTaskImpl to have completionsource and task as local fields * Address docStubs and clean up UpdateAppClient * Add multiple task test to updateappclienttest * Add unit tests for updateApkClient * Move null release test from FADTest to UpdateAppClientTest * Address lint error in updateTask * Return same download task, use thread.sleep() in tests, make https failure in test explicit
1 parent b0f2783 commit 1dce165

File tree

7 files changed

+321
-116
lines changed

7 files changed

+321
-116
lines changed

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

Lines changed: 8 additions & 15 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_FAILURE;
18-
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE;
1918

2019
import android.app.Activity;
2120
import android.app.AlertDialog;
@@ -45,6 +44,7 @@ public class FirebaseAppDistribution implements Application.ActivityLifecycleCal
4544

4645
private Task<Void> cachedUpdateToLatestReleaseTask;
4746
private Task<AppDistributionRelease> cachedCheckForUpdateTask;
47+
private UpdateTaskImpl cachedUpdateAppTask;
4848
private AppDistributionReleaseInternal cachedLatestRelease;
4949
private final SignInStorage signInStorage;
5050

@@ -172,24 +172,17 @@ public synchronized Task<AppDistributionRelease> checkForUpdate() {
172172
* new release is cached from checkForUpdate
173173
*/
174174
@NonNull
175-
public UpdateTask updateApp() throws FirebaseAppDistributionException {
175+
public synchronized UpdateTask updateApp() throws FirebaseAppDistributionException {
176176

177177
if (!isTesterSignedIn()) {
178-
return new UpdateTaskImpl(
179-
Tasks.forException(
180-
new FirebaseAppDistributionException(
181-
Constants.ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE)));
182-
}
183-
184-
AppDistributionReleaseInternal cachedRelease = getCachedLatestRelease();
185-
if (cachedRelease == null) {
186-
return new UpdateTaskImpl(
187-
Tasks.forException(
188-
new FirebaseAppDistributionException(
189-
Constants.ErrorMessages.NOT_FOUND_ERROR, UPDATE_NOT_AVAILABLE)));
178+
UpdateTaskImpl updateTask = new UpdateTaskImpl();
179+
updateTask.setException(
180+
new FirebaseAppDistributionException(
181+
Constants.ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE));
182+
return updateTask;
190183
}
191184

192-
return this.updateAppClient.getUpdateTask(cachedLatestRelease, currentActivity);
185+
return this.updateAppClient.updateApp(cachedLatestRelease, currentActivity);
193186
}
194187

195188
/** Returns true if the App Distribution tester is signed in */

‎firebase-app-distribution/src/main/java/com/google/firebase/appdistribution/UpdateApkClient.java

Lines changed: 46 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@
1919
import android.app.Activity;
2020
import android.content.Context;
2121
import android.content.Intent;
22-
import android.os.Handler;
23-
import android.os.Looper;
2422
import android.util.Log;
2523
import androidx.annotation.NonNull;
26-
import androidx.core.os.HandlerCompat;
24+
import androidx.annotation.VisibleForTesting;
2725
import com.google.android.gms.tasks.CancellationTokenSource;
2826
import com.google.android.gms.tasks.Task;
2927
import com.google.android.gms.tasks.TaskCompletionSource;
@@ -45,73 +43,65 @@ class UpdateApkClient {
4543
private static final String TAG = "FADUpdateAppClient";
4644
private static final String REQUEST_METHOD = "GET";
4745
private TaskCompletionSource<File> downloadTaskCompletionSource;
48-
private CancellationTokenSource downloadCancellationTokenSource;
4946
private final Executor downloadExecutor;
50-
private final Handler downloadHandler;
5147
private TaskCompletionSource<Void> installTaskCompletionSource;
5248
private final FirebaseApp firebaseApp;
53-
private UpdateTaskImpl updateTask;
54-
private TaskCompletionSource updateAppTaskCompletionSource;
5549

5650
public UpdateApkClient(@NonNull FirebaseApp firebaseApp) {
5751
this.downloadExecutor = Executors.newSingleThreadExecutor();
58-
this.downloadHandler = HandlerCompat.createAsync(Looper.getMainLooper());
5952
this.firebaseApp = firebaseApp;
6053
}
6154

6255
public void updateApk(
63-
@NonNull String downloadUrl,
64-
@NonNull Activity currentActivity,
6556
@NonNull UpdateTaskImpl updateTask,
66-
@NonNull TaskCompletionSource updateAppTaskCompletionSource) {
67-
this.updateTask = updateTask;
68-
this.updateAppTaskCompletionSource = updateAppTaskCompletionSource;
69-
downloadApk(downloadUrl)
57+
@NonNull String downloadUrl,
58+
@NonNull Activity currentActivity) {
59+
60+
downloadApk(downloadUrl, updateTask)
7061
.addOnSuccessListener(
62+
downloadExecutor,
7163
file ->
7264
install(file.getPath(), currentActivity)
7365
.addOnSuccessListener(
74-
downloadExecutor,
75-
Void -> {
76-
updateTask.updateProgress(
77-
UpdateProgress.builder()
78-
.setApkFileTotalBytes(file.length())
79-
.setApkBytesDownloaded(file.length())
80-
.setUpdateStatus(UpdateStatus.DOWNLOADED)
81-
.build());
82-
updateAppTaskCompletionSource.setResult(null);
66+
unused -> {
67+
postUpdateProgress(
68+
updateTask, file.length(), file.length(), UpdateStatus.DOWNLOADED);
69+
updateTask.setResult();
8370
})
8471
.addOnFailureListener(
8572
e ->
86-
setUpdateAppErrorWithDefault(
73+
setTaskCompletionErrorWithDefault(
74+
updateTask,
8775
e,
8876
new FirebaseAppDistributionException(
8977
Constants.ErrorMessages.NETWORK_ERROR,
9078
FirebaseAppDistributionException.Status.INSTALLATION_FAILURE))))
9179
.addOnFailureListener(
80+
downloadExecutor,
9281
e ->
93-
setUpdateAppErrorWithDefault(
82+
setTaskCompletionErrorWithDefault(
83+
updateTask,
9484
e,
9585
new FirebaseAppDistributionException(
9686
Constants.ErrorMessages.NETWORK_ERROR,
9787
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE)));
9888
}
9989

100-
private @NonNull Task<File> downloadApk(@NonNull String downloadUrl) {
90+
@VisibleForTesting
91+
@NonNull
92+
Task<File> downloadApk(@NonNull String downloadUrl, UpdateTaskImpl updateTask) {
10193
if (downloadTaskCompletionSource != null
10294
&& !downloadTaskCompletionSource.getTask().isComplete()) {
103-
downloadCancellationTokenSource.cancel();
95+
return downloadTaskCompletionSource.getTask();
10496
}
10597

106-
downloadCancellationTokenSource = new CancellationTokenSource();
107-
downloadTaskCompletionSource =
108-
new TaskCompletionSource<>(downloadCancellationTokenSource.getToken());
98+
downloadTaskCompletionSource = new TaskCompletionSource<>();
10999

110-
makeApkDownloadRequest(downloadUrl);
100+
makeApkDownloadRequest(downloadUrl, updateTask);
111101
return downloadTaskCompletionSource.getTask();
112102
}
113103

114-
private void makeApkDownloadRequest(@NonNull String downloadUrl) {
104+
private void makeApkDownloadRequest(@NonNull String downloadUrl, UpdateTaskImpl updateTask) {
115105
downloadExecutor.execute(
116106
() -> {
117107
try {
@@ -124,9 +114,9 @@ private void makeApkDownloadRequest(@NonNull String downloadUrl) {
124114
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
125115
} else {
126116
long responseLength = connection.getContentLength();
127-
postUpdateProgress(responseLength, 0, UpdateStatus.PENDING);
117+
postUpdateProgress(updateTask, responseLength, 0, UpdateStatus.PENDING);
128118
String fileName = getApplicationName() + ".apk";
129-
downloadToDisk(connection.getInputStream(), responseLength, fileName);
119+
downloadToDisk(connection.getInputStream(), responseLength, fileName, updateTask);
130120
}
131121
} catch (IOException | FirebaseAppDistributionException e) {
132122
setDownloadTaskCompletionErrorWithDefault(
@@ -138,7 +128,8 @@ private void makeApkDownloadRequest(@NonNull String downloadUrl) {
138128
});
139129
}
140130

141-
private void downloadToDisk(InputStream input, long totalSize, String fileName) {
131+
private void downloadToDisk(
132+
InputStream input, long totalSize, String fileName, UpdateTaskImpl updateTask) {
142133

143134
File apkFile = getApkFileForApp(fileName);
144135
apkFile.delete();
@@ -160,11 +151,11 @@ private void downloadToDisk(InputStream input, long totalSize, String fileName)
160151
long currentTimeMs = System.currentTimeMillis();
161152
if (currentTimeMs - lastMsUpdated > UPDATE_INTERVAL_MS) {
162153
lastMsUpdated = currentTimeMs;
163-
postUpdateProgress(totalSize, downloadedSize, UpdateStatus.DOWNLOADING);
154+
postUpdateProgress(updateTask, totalSize, downloadedSize, UpdateStatus.DOWNLOADING);
164155
}
165156
}
166157
// completion
167-
postUpdateProgress(totalSize, downloadedSize, UpdateStatus.DOWNLOADED);
158+
postUpdateProgress(updateTask, totalSize, downloadedSize, UpdateStatus.DOWNLOADED);
168159

169160
} catch (IOException e) {
170161
setDownloadTaskCompletionError(
@@ -257,30 +248,31 @@ void setInstallationResult(int resultCode) {
257248
}
258249
}
259250

260-
private void setUpdateAppTaskCompletionError(FirebaseAppDistributionException e) {
261-
if (updateAppTaskCompletionSource != null
262-
&& !updateAppTaskCompletionSource.getTask().isComplete()) {
263-
updateAppTaskCompletionSource.setException(e);
251+
private void setTaskCompletionError(
252+
UpdateTaskImpl updateTask, FirebaseAppDistributionException e) {
253+
if (updateTask != null && !updateTask.isComplete()) {
254+
updateTask.setException(e);
264255
}
265256
}
266257

267-
private void setUpdateAppErrorWithDefault(
268-
Exception e, FirebaseAppDistributionException defaultFirebaseException) {
258+
private void setTaskCompletionErrorWithDefault(
259+
UpdateTaskImpl updateTask,
260+
Exception e,
261+
FirebaseAppDistributionException defaultFirebaseException) {
269262
if (e instanceof FirebaseAppDistributionException) {
270-
setUpdateAppTaskCompletionError((FirebaseAppDistributionException) e);
263+
setTaskCompletionError(updateTask, (FirebaseAppDistributionException) e);
271264
} else {
272-
setUpdateAppTaskCompletionError(defaultFirebaseException);
265+
setTaskCompletionError(updateTask, defaultFirebaseException);
273266
}
274267
}
275268

276-
private void postUpdateProgress(long totalBytes, long downloadedBytes, UpdateStatus status) {
277-
downloadHandler.post(
278-
() ->
279-
updateTask.updateProgress(
280-
UpdateProgress.builder()
281-
.setApkFileTotalBytes(totalBytes)
282-
.setApkBytesDownloaded(downloadedBytes)
283-
.setUpdateStatus(status)
284-
.build()));
269+
private void postUpdateProgress(
270+
UpdateTaskImpl updateTask, long totalBytes, long downloadedBytes, UpdateStatus status) {
271+
updateTask.updateProgress(
272+
UpdateProgress.builder()
273+
.setApkFileTotalBytes(totalBytes)
274+
.setApkBytesDownloaded(downloadedBytes)
275+
.setUpdateStatus(status)
276+
.build());
285277
}
286278
}

‎firebase-app-distribution/src/main/java/com/google/firebase/appdistribution/UpdateAppClient.java

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@
1414

1515
package com.google.firebase.appdistribution;
1616

17+
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE;
18+
1719
import android.app.Activity;
1820
import android.content.Intent;
1921
import android.net.Uri;
2022
import androidx.annotation.NonNull;
21-
import com.google.android.gms.tasks.CancellationTokenSource;
2223
import com.google.android.gms.tasks.TaskCompletionSource;
2324
import com.google.firebase.FirebaseApp;
2425
import com.google.firebase.appdistribution.internal.AppDistributionReleaseInternal;
@@ -27,51 +28,44 @@
2728
public class UpdateAppClient {
2829

2930
private TaskCompletionSource<Void> updateAppTaskCompletionSource = null;
30-
private CancellationTokenSource updateAppCancellationSource;
31-
private UpdateTaskImpl updateTask;
32-
33-
private FirebaseApp firebaseApp;
3431
private UpdateApkClient updateApkClient;
32+
private UpdateTaskImpl cachedUpdateAppTask;
3533

3634
public UpdateAppClient(@NonNull FirebaseApp firebaseApp) {
37-
this.firebaseApp = firebaseApp;
3835
this.updateApkClient = new UpdateApkClient(firebaseApp);
3936
}
4037

4138
@NonNull
42-
public UpdateTask getUpdateTask(
39+
synchronized UpdateTask updateApp(
4340
@NonNull AppDistributionReleaseInternal latestRelease, @NonNull Activity currentActivity)
4441
throws FirebaseAppDistributionException {
4542

46-
if (this.updateTask != null && !updateTask.isComplete()) {
47-
return this.updateTask;
43+
if (cachedUpdateAppTask != null && !cachedUpdateAppTask.isComplete()) {
44+
return cachedUpdateAppTask;
4845
}
4946

50-
updateAppCancellationSource = new CancellationTokenSource();
51-
updateAppTaskCompletionSource =
52-
new TaskCompletionSource<>(updateAppCancellationSource.getToken());
53-
this.updateTask = new UpdateTaskImpl(updateAppTaskCompletionSource.getTask());
47+
cachedUpdateAppTask = new UpdateTaskImpl();
48+
49+
if (latestRelease == null) {
50+
cachedUpdateAppTask.setException(
51+
new FirebaseAppDistributionException(
52+
Constants.ErrorMessages.NOT_FOUND_ERROR, UPDATE_NOT_AVAILABLE));
53+
return cachedUpdateAppTask;
54+
}
5455

5556
if (latestRelease.getBinaryType() == BinaryType.AAB) {
56-
updateTask.updateProgress(
57-
UpdateProgress.builder()
58-
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
59-
.setApkBytesDownloaded(-1)
60-
.setApkFileTotalBytes(-1)
61-
.build());
62-
redirectToPlayForAabUpdate(latestRelease.getDownloadUrl(), currentActivity);
57+
redirectToPlayForAabUpdate(
58+
cachedUpdateAppTask, latestRelease.getDownloadUrl(), currentActivity);
6359
} else {
6460
this.updateApkClient.updateApk(
65-
latestRelease.getDownloadUrl(),
66-
currentActivity,
67-
updateTask,
68-
updateAppTaskCompletionSource);
61+
cachedUpdateAppTask, latestRelease.getDownloadUrl(), currentActivity);
6962
}
7063

71-
return this.updateTask;
64+
return cachedUpdateAppTask;
7265
}
7366

74-
private void redirectToPlayForAabUpdate(String downloadUrl, Activity currentActivity)
67+
private void redirectToPlayForAabUpdate(
68+
UpdateTaskImpl updateTask, String downloadUrl, Activity currentActivity)
7569
throws FirebaseAppDistributionException {
7670
if (downloadUrl == null) {
7771
throw new FirebaseAppDistributionException(
@@ -82,7 +76,13 @@ private void redirectToPlayForAabUpdate(String downloadUrl, Activity currentActi
8276
updateIntent.setData(uri);
8377
updateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
8478
currentActivity.startActivity(updateIntent);
85-
updateAppTaskCompletionSource.setResult(null);
79+
updateTask.updateProgress(
80+
UpdateProgress.builder()
81+
.setApkBytesDownloaded(-1)
82+
.setApkFileTotalBytes(-1)
83+
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
84+
.build());
85+
updateTask.setResult();
8686
}
8787

8888
void setInstallationResult(int resultCode) {

0 commit comments

Comments
 (0)