Skip to content

Commit 931fb8d

Browse files
lfkellogglfkellogg
authored
Refactor notification tests to be more focused to the classes under test (#3274)
* Refactor tests around notifications to be more focused to the classes under test * Change field initialization order * Make postUpdateProgress private
1 parent 6a0b405 commit 931fb8d

File tree

7 files changed

+285
-217
lines changed

7 files changed

+285
-217
lines changed

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

Lines changed: 26 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -41,35 +41,40 @@ class ApkUpdater {
4141
private static final int UPDATE_INTERVAL_MS = 250;
4242
private static final String TAG = "UpdateApkClient:";
4343
private static final String REQUEST_METHOD = "GET";
44-
private final FirebaseAppDistributionNotificationsManager appDistributionNotificationsManager;
4544

4645
private TaskCompletionSource<File> downloadTaskCompletionSource;
4746
private final Executor taskExecutor; // Executor to run task listeners on a background thread
4847
private final FirebaseApp firebaseApp;
4948
private final ApkInstaller apkInstaller;
49+
private final FirebaseAppDistributionNotificationsManager appDistributionNotificationsManager;
5050

5151
@GuardedBy("updateTaskLock")
5252
private UpdateTaskImpl cachedUpdateTask;
5353

5454
private final Object updateTaskLock = new Object();
5555

5656
public ApkUpdater(@NonNull FirebaseApp firebaseApp, @NonNull ApkInstaller apkInstaller) {
57-
this(Executors.newSingleThreadExecutor(), firebaseApp, apkInstaller);
57+
this(
58+
Executors.newSingleThreadExecutor(),
59+
firebaseApp,
60+
apkInstaller,
61+
new FirebaseAppDistributionNotificationsManager(firebaseApp.getApplicationContext()));
5862
}
5963

64+
@VisibleForTesting
6065
public ApkUpdater(
6166
@NonNull Executor taskExecutor,
6267
@NonNull FirebaseApp firebaseApp,
63-
@NonNull ApkInstaller apkInstaller) {
64-
this.appDistributionNotificationsManager =
65-
new FirebaseAppDistributionNotificationsManager(firebaseApp);
68+
@NonNull ApkInstaller apkInstaller,
69+
@NonNull FirebaseAppDistributionNotificationsManager appDistributionNotificationsManager) {
6670
this.taskExecutor = taskExecutor;
6771
this.firebaseApp = firebaseApp;
6872
this.apkInstaller = apkInstaller;
73+
this.appDistributionNotificationsManager = appDistributionNotificationsManager;
6974
}
7075

7176
UpdateTaskImpl updateApk(
72-
@NonNull AppDistributionReleaseInternal newRelease, boolean showDownloadNotificationManager) {
77+
@NonNull AppDistributionReleaseInternal newRelease, boolean showNotification) {
7378
synchronized (updateTaskLock) {
7479
if (cachedUpdateTask != null && !cachedUpdateTask.isComplete()) {
7580
return cachedUpdateTask;
@@ -78,7 +83,7 @@ UpdateTaskImpl updateApk(
7883
cachedUpdateTask = new UpdateTaskImpl();
7984
}
8085

81-
downloadApk(newRelease, showDownloadNotificationManager)
86+
downloadApk(newRelease, showNotification)
8287
// Using onSuccess task to ensure that all install errors get cascaded to the Failure
8388
// listener down below
8489
.addOnSuccessListener(
@@ -94,7 +99,7 @@ UpdateTaskImpl updateApk(
9499
file.length(),
95100
file.length(),
96101
UpdateStatus.INSTALL_FAILED,
97-
showDownloadNotificationManager);
102+
showNotification);
98103
setUpdateTaskCompletionErrorWithDefault(
99104
e,
100105
new FirebaseAppDistributionException(
@@ -128,20 +133,20 @@ UpdateTaskImpl updateApk(
128133
@VisibleForTesting
129134
@NonNull
130135
Task<File> downloadApk(
131-
@NonNull AppDistributionReleaseInternal newRelease, boolean showDownloadNotificationManager) {
136+
@NonNull AppDistributionReleaseInternal newRelease, boolean showNotification) {
132137
if (downloadTaskCompletionSource != null
133138
&& !downloadTaskCompletionSource.getTask().isComplete()) {
134139
return downloadTaskCompletionSource.getTask();
135140
}
136141

137142
downloadTaskCompletionSource = new TaskCompletionSource<>();
138143

139-
makeApkDownloadRequest(newRelease, showDownloadNotificationManager);
144+
makeApkDownloadRequest(newRelease, showNotification);
140145
return downloadTaskCompletionSource.getTask();
141146
}
142147

143148
private void makeApkDownloadRequest(
144-
@NonNull AppDistributionReleaseInternal newRelease, boolean showDownloadNotificationManager) {
149+
@NonNull AppDistributionReleaseInternal newRelease, boolean showNotification) {
145150
taskExecutor.execute(
146151
() -> {
147152
try {
@@ -154,16 +159,12 @@ private void makeApkDownloadRequest(
154159
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
155160
} else {
156161
long responseLength = connection.getContentLength();
157-
postUpdateProgress(
158-
responseLength, 0, UpdateStatus.PENDING, showDownloadNotificationManager);
162+
postUpdateProgress(responseLength, 0, UpdateStatus.PENDING, showNotification);
159163
String fileName = getApplicationName() + ".apk";
160164
LogWrapper.getInstance().v(TAG + "Attempting to download to disk");
161165

162166
downloadToDisk(
163-
connection.getInputStream(),
164-
responseLength,
165-
fileName,
166-
showDownloadNotificationManager);
167+
connection.getInputStream(), responseLength, fileName, showNotification);
167168
}
168169
} catch (IOException | FirebaseAppDistributionException e) {
169170
setDownloadTaskCompletionErrorWithDefault(
@@ -176,7 +177,7 @@ private void makeApkDownloadRequest(
176177
}
177178

178179
private void downloadToDisk(
179-
InputStream input, long totalSize, String fileName, boolean showDownloadNotificationManager) {
180+
InputStream input, long totalSize, String fileName, boolean showNotification) {
180181

181182
File apkFile = getApkFileForApp(fileName);
182183
apkFile.delete();
@@ -205,19 +206,13 @@ private void downloadToDisk(
205206
if (currentTimeMs - lastMsUpdated > UPDATE_INTERVAL_MS) {
206207
lastMsUpdated = currentTimeMs;
207208
postUpdateProgress(
208-
totalSize,
209-
bytesDownloaded,
210-
UpdateStatus.DOWNLOADING,
211-
showDownloadNotificationManager);
209+
totalSize, bytesDownloaded, UpdateStatus.DOWNLOADING, showNotification);
212210
}
213211
}
214212

215213
} catch (IOException e) {
216214
postUpdateProgress(
217-
totalSize,
218-
bytesDownloaded,
219-
UpdateStatus.DOWNLOAD_FAILED,
220-
showDownloadNotificationManager);
215+
totalSize, bytesDownloaded, UpdateStatus.DOWNLOAD_FAILED, showNotification);
221216
setDownloadTaskCompletionError(
222217
new FirebaseAppDistributionException(
223218
Constants.ErrorMessages.NETWORK_ERROR,
@@ -230,19 +225,15 @@ private void downloadToDisk(
230225

231226
} catch (Exception e) {
232227
postUpdateProgress(
233-
totalSize,
234-
bytesDownloaded,
235-
UpdateStatus.DOWNLOAD_FAILED,
236-
showDownloadNotificationManager);
228+
totalSize, bytesDownloaded, UpdateStatus.DOWNLOAD_FAILED, showNotification);
237229
setDownloadTaskCompletionError(
238230
new FirebaseAppDistributionException(
239231
Constants.ErrorMessages.NETWORK_ERROR,
240232
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
241233
}
242234

243235
// completion
244-
postUpdateProgress(
245-
totalSize, totalSize, UpdateStatus.DOWNLOADED, showDownloadNotificationManager);
236+
postUpdateProgress(totalSize, totalSize, UpdateStatus.DOWNLOADED, showNotification);
246237

247238
safeSetTaskResult(downloadTaskCompletionSource, apkFile);
248239
}
@@ -304,12 +295,8 @@ private void setUpdateTaskCompletionErrorWithDefault(
304295
}
305296
}
306297

307-
@VisibleForTesting
308-
void postUpdateProgress(
309-
long totalBytes,
310-
long downloadedBytes,
311-
UpdateStatus status,
312-
boolean showDownloadNotificationManager) {
298+
private void postUpdateProgress(
299+
long totalBytes, long downloadedBytes, UpdateStatus status, boolean showNotification) {
313300
synchronized (updateTaskLock) {
314301
cachedUpdateTask.updateProgress(
315302
UpdateProgress.builder()
@@ -318,7 +305,7 @@ void postUpdateProgress(
318305
.setUpdateStatus(status)
319306
.build());
320307
}
321-
if (showDownloadNotificationManager) {
308+
if (showNotification) {
322309
appDistributionNotificationsManager.updateNotification(totalBytes, downloadedBytes, status);
323310
}
324311
}

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2021 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.app.distribution;
16+
17+
import android.content.Context;
18+
import android.content.res.Resources;
19+
import android.graphics.drawable.AdaptiveIconDrawable;
20+
import android.graphics.drawable.Drawable;
21+
import android.os.Build;
22+
import android.os.Build.VERSION;
23+
import androidx.core.content.ContextCompat;
24+
import com.google.firebase.app.distribution.internal.LogWrapper;
25+
26+
class AppIconSource {
27+
private static final String TAG = "AppIconSource: ";
28+
29+
private static final int DEFAULT_ICON = android.R.drawable.sym_def_app_icon;
30+
31+
/**
32+
* Get an icon for the given app context, or a default icon if the app icon is adaptive or does
33+
* not exist.
34+
*
35+
* <p>This is useful for showing an icon in notifications, since adaptive icons cause a crash loop
36+
* in the NotificationsManager (b/69969749).
37+
*
38+
* @return A drawable resource identifier (in the package's resources) of the icon.
39+
*/
40+
int getNonAdaptiveIconOrDefault(Context context) {
41+
int iconId = context.getApplicationInfo().icon;
42+
if (iconId == 0) {
43+
return DEFAULT_ICON;
44+
}
45+
46+
Drawable icon;
47+
try {
48+
icon = ContextCompat.getDrawable(context, iconId);
49+
} catch (Resources.NotFoundException ex) {
50+
return DEFAULT_ICON;
51+
}
52+
53+
if (isAdaptiveIcon(icon)) {
54+
LogWrapper.getInstance()
55+
.e(TAG + "Adaptive icons cannot be used in notifications. Ignoring icon id: " + iconId);
56+
return DEFAULT_ICON;
57+
}
58+
59+
return iconId;
60+
}
61+
62+
private boolean isAdaptiveIcon(Drawable icon) {
63+
// AdaptiveIcons were introduced in API 26
64+
return VERSION.SDK_INT >= Build.VERSION_CODES.O && icon instanceof AdaptiveIconDrawable;
65+
}
66+
}

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

Lines changed: 32 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,15 @@
1414

1515
package com.google.firebase.app.distribution;
1616

17+
import android.app.Notification;
1718
import android.app.NotificationChannel;
1819
import android.app.NotificationManager;
1920
import android.app.PendingIntent;
2021
import android.content.Context;
2122
import android.content.Intent;
22-
import android.content.res.Resources;
23-
import android.graphics.drawable.AdaptiveIconDrawable;
24-
import android.graphics.drawable.Drawable;
2523
import android.os.Build;
26-
import android.os.Build.VERSION;
2724
import androidx.annotation.VisibleForTesting;
2825
import androidx.core.app.NotificationCompat;
29-
import androidx.core.content.ContextCompat;
30-
import com.google.firebase.FirebaseApp;
3126
import com.google.firebase.app.distribution.internal.LogWrapper;
3227

3328
class FirebaseAppDistributionNotificationsManager {
@@ -38,28 +33,43 @@ class FirebaseAppDistributionNotificationsManager {
3833
@VisibleForTesting
3934
static final String NOTIFICATION_TAG = "com.google.firebase.app.distribution.tag";
4035

41-
private final FirebaseApp firebaseApp;
36+
private final Context context;
37+
private final AppIconSource appIconSource;
4238

43-
FirebaseAppDistributionNotificationsManager(FirebaseApp firebaseApp) {
44-
this.firebaseApp = firebaseApp;
39+
FirebaseAppDistributionNotificationsManager(Context context) {
40+
this(context, new AppIconSource());
41+
}
42+
43+
@VisibleForTesting
44+
FirebaseAppDistributionNotificationsManager(Context context, AppIconSource appIconSource) {
45+
this.context = context;
46+
this.appIconSource = appIconSource;
4547
}
4648

4749
void updateNotification(long totalBytes, long downloadedBytes, UpdateStatus status) {
48-
Context context = firebaseApp.getApplicationContext();
4950
NotificationManager notificationManager = createNotificationManager(context);
50-
NotificationCompat.Builder builder = createNotificationBuilder();
51+
Notification notification =
52+
new NotificationCompat.Builder(context, NOTIFICATION_CHANNEL_ID)
53+
.setOnlyAlertOnce(true)
54+
.setSmallIcon(appIconSource.getNonAdaptiveIconOrDefault(context))
55+
.setContentIntent(createPendingIntent())
56+
.setContentTitle(context.getString(getNotificationContentTitleId(status)))
57+
.setProgress(
58+
100,
59+
(int) (((float) downloadedBytes / (float) totalBytes) * 100),
60+
/*indeterminate = */ false)
61+
.build();
62+
notificationManager.notify(NOTIFICATION_TAG, /*id =*/ 0, notification);
63+
}
64+
65+
int getNotificationContentTitleId(UpdateStatus status) {
5166
if (isErrorState(status)) {
52-
builder.setContentTitle(context.getString(R.string.download_failed));
67+
return R.string.download_failed;
5368
} else if (status.equals(UpdateStatus.DOWNLOADED)) {
54-
builder.setContentTitle(context.getString(R.string.download_completed));
69+
return R.string.download_completed;
5570
} else {
56-
builder.setContentTitle(context.getString(R.string.downloading_app_update));
71+
return R.string.downloading_app_update;
5772
}
58-
builder.setProgress(
59-
100,
60-
(int) (((float) downloadedBytes / (float) totalBytes) * 100),
61-
/*indeterminate = */ false);
62-
notificationManager.notify(NOTIFICATION_TAG, /*id =*/ 0, builder.build());
6373
}
6474

6575
// CHECK THIS LATER
@@ -81,60 +91,21 @@ private NotificationManager createNotificationManager(Context context) {
8191
channel.setDescription(context.getString(R.string.notifications_channel_description));
8292
// Register the channel with the system; you can't change the importance
8393
// or other notification behaviors after this
84-
NotificationManager notificationManager = context.getSystemService(NotificationManager.class);
94+
NotificationManager notificationManager =
95+
(NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
8596
notificationManager.createNotificationChannel(channel);
8697
return notificationManager;
8798
} else {
8899
return (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
89100
}
90101
}
91102

92-
private NotificationCompat.Builder createNotificationBuilder() {
93-
Context context = firebaseApp.getApplicationContext();
94-
return new NotificationCompat.Builder(context, NOTIFICATION_CHANNEL_ID)
95-
.setOnlyAlertOnce(true)
96-
.setSmallIcon(getSmallIcon())
97-
.setContentIntent(createPendingIntent());
98-
}
99-
100103
private PendingIntent createPendingIntent() {
101104
// Query the package manager for the best launch intent for the app
102-
Context context = firebaseApp.getApplicationContext();
103105
Intent intent = context.getPackageManager().getLaunchIntentForPackage(context.getPackageName());
104106
if (intent == null) {
105107
LogWrapper.getInstance().w(TAG + "No activity found to launch app");
106108
}
107-
return PendingIntent.getActivity(
108-
firebaseApp.getApplicationContext(), 0, intent, PendingIntent.FLAG_ONE_SHOT);
109-
}
110-
111-
@VisibleForTesting
112-
int getSmallIcon() {
113-
Context context = firebaseApp.getApplicationContext();
114-
int iconId = context.getApplicationInfo().icon;
115-
116-
if (iconId == 0 || isAdaptiveIcon(iconId)) {
117-
// fallback to default icon
118-
return android.R.drawable.sym_def_app_icon;
119-
}
120-
121-
return iconId;
122-
}
123-
124-
/** Adaptive icons cause a crash loop in the Notifications Manager. See b/69969749. */
125-
private boolean isAdaptiveIcon(int iconId) {
126-
try {
127-
Drawable icon = ContextCompat.getDrawable(firebaseApp.getApplicationContext(), iconId);
128-
if (VERSION.SDK_INT >= Build.VERSION_CODES.O && icon instanceof AdaptiveIconDrawable) {
129-
LogWrapper.getInstance()
130-
.e(TAG + "Adaptive icons cannot be used in notifications. Ignoring icon id: " + iconId);
131-
return true;
132-
} else {
133-
// AdaptiveIcons were introduced in API 26
134-
return false;
135-
}
136-
} catch (Resources.NotFoundException ex) {
137-
return true;
138-
}
109+
return PendingIntent.getActivity(context, 0, intent, PendingIntent.FLAG_ONE_SHOT);
139110
}
140111
}

0 commit comments

Comments
 (0)