Skip to content

Commit 8477154

Browse files
notif: Use Zulip's distinct notification sound on Android
Fixes: #340
1 parent d22c1b6 commit 8477154

File tree

6 files changed

+264
-5
lines changed

6 files changed

+264
-5
lines changed
8.62 KB
Binary file not shown.
8.28 KB
Binary file not shown.
8.66 KB
Binary file not shown.

android/app/src/main/res/raw/keep.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
https://github.com/zulip/zulip-flutter/issues/528
1313
-->
1414
<resources xmlns:tools="http://schemas.android.com/tools"
15-
tools:keep="@drawable/zulip_notification"
15+
tools:keep="@drawable/zulip_notification,@raw/chime2,@raw/chime3,@raw/chime4"
1616
/>

lib/notifications/display.dart

+126-2
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,32 @@ import '../widgets/theme.dart';
2323

2424
AndroidNotificationHostApi get _androidHost => ZulipBinding.instance.androidNotificationHost;
2525

26+
enum NotificationSound {
27+
// Any new entry here must appear in `keep.xml` too, see #528.
28+
chime2(resourceName: 'chime2', fileDisplayName: 'Zulip - Low Chime.m4a'),
29+
chime3(resourceName: 'chime3', fileDisplayName: 'Zulip - Chime.m4a'),
30+
chime4(resourceName: 'chime4', fileDisplayName: 'Zulip - High Chime.m4a');
31+
32+
const NotificationSound({
33+
required this.resourceName,
34+
required this.fileDisplayName,
35+
});
36+
final String resourceName;
37+
final String fileDisplayName;
38+
}
39+
2640
/// Service for configuring our Android "notification channel".
2741
class NotificationChannelManager {
2842
/// The channel ID we use for our one notification channel, which we use for
2943
/// all notifications.
3044
// TODO(launch) check this doesn't match zulip-mobile's current or previous
3145
// channel IDs
46+
// Previous values: 'messages-1'
47+
@visibleForTesting
48+
static const kChannelId = 'messages-2';
49+
3250
@visibleForTesting
33-
static const kChannelId = 'messages-1';
51+
static const kDefaultNotificationSound = NotificationSound.chime3;
3452

3553
/// The vibration pattern we set for notifications.
3654
// We try to set a vibration pattern that, with the phone in one's pocket,
@@ -39,6 +57,110 @@ class NotificationChannelManager {
3957
@visibleForTesting
4058
static final kVibrationPattern = Int64List.fromList([0, 125, 100, 450]);
4159

60+
/// Generates an Android resource URL for the given resource name and type.
61+
///
62+
/// For example, for a resource `@raw/chime3`, where `raw` would be the
63+
/// resource type and `chime3` would be the resource name it generates the
64+
/// following URL:
65+
/// `android.resource://com.zulip.flutter/raw/chime3`
66+
///
67+
/// Based on: https://stackoverflow.com/a/38340580
68+
static Uri _resourceUrlFromName({
69+
required String resourceTypeName,
70+
required String resourceEntryName,
71+
}) {
72+
const packageName = 'com.zulip.flutter'; // TODO(#407)
73+
74+
// URL scheme for Android resource url.
75+
// See: https://developer.android.com/reference/android/content/ContentResolver#SCHEME_ANDROID_RESOURCE
76+
const schemeAndroidResource = 'android.resource';
77+
78+
return Uri(
79+
scheme: schemeAndroidResource,
80+
host: packageName,
81+
pathSegments: <String>[resourceTypeName, resourceEntryName],
82+
);
83+
}
84+
85+
/// Prepare our notification sounds; return a URL for our default sound.
86+
///
87+
/// Where possible, this copies each of our notification sounds into shared storage
88+
/// so that the user can choose between them in the system notification settings.
89+
///
90+
/// Returns a URL for our default notification sound: either in shared storage
91+
/// if we successfully copied it there, or else as our internal resource file.
92+
static Future<String> _ensureInitNotificationSounds() async {
93+
String defaultSoundUrl = _resourceUrlFromName(
94+
resourceTypeName: 'raw',
95+
resourceEntryName: kDefaultNotificationSound.resourceName).toString();
96+
97+
final shouldUseResourceFile = switch (await ZulipBinding.instance.deviceInfo) {
98+
// Before Android 10 Q, we don't attempt to put the sounds in shared media storage.
99+
// Just use the resource file directly.
100+
// TODO(android-sdk-29): Simplify this away.
101+
AndroidDeviceInfo(:var sdkInt) => sdkInt < 29,
102+
_ => true,
103+
};
104+
if (shouldUseResourceFile) return defaultSoundUrl;
105+
106+
// First, look to see what notification sounds we've already stored,
107+
// and check against our list of sounds we have.
108+
final soundsToAdd = NotificationSound.values.toList();
109+
110+
final List<StoredNotificationSound?> storedSounds;
111+
try {
112+
storedSounds = await _androidHost.listStoredSoundsInNotificationsDirectory();
113+
} catch (e, st) {
114+
assert(debugLog('$e\n$st')); // TODO(log)
115+
return defaultSoundUrl;
116+
}
117+
for (final storedSound in storedSounds) {
118+
assert(storedSound != null); // TODO(#942)
119+
120+
// If the file is one we put there, and has the name we give to our
121+
// default sound, then use it as the default sound.
122+
if (storedSound!.fileName == kDefaultNotificationSound.fileDisplayName
123+
&& storedSound.isOwned) {
124+
defaultSoundUrl = storedSound.contentUrl;
125+
}
126+
127+
// If it has the name of any of our sounds, then don't try to add
128+
// that sound. This applies even if we didn't put it there: the
129+
// name is taken, so if we tried adding it anyway it'd get some
130+
// other name (like "Zulip - Chime (1).m4a", with " (1)" added).
131+
// Which means the *next* launch would try to add it again ad infinitum.
132+
// We could avoid this given some other way to uniquely identify the
133+
// file, but haven't found an obvious one.
134+
//
135+
// This does mean it's possible the file isn't the one we would have
136+
// put there... but it probably is, just from a debug vs. release build
137+
// of the app (because those may have different package names). And anyway,
138+
// this is a file we're supplying for the user in case they want it, not
139+
// something where the app depends on it having specific content.
140+
soundsToAdd.removeWhere((v) => v.fileDisplayName == storedSound.fileName);
141+
}
142+
143+
// If that leaves any sounds we haven't yet put into shared storage
144+
// (e.g., because this is the first run after install, or after an
145+
// upgrade that added a sound), then store those.
146+
147+
for (final sound in soundsToAdd) {
148+
try {
149+
final url = await _androidHost.copySoundResourceToMediaStore(
150+
targetFileDisplayName: sound.fileDisplayName,
151+
sourceResourceName: sound.resourceName);
152+
153+
if (sound == kDefaultNotificationSound) {
154+
defaultSoundUrl = url;
155+
}
156+
} catch (e, st) {
157+
assert(debugLog("$e\n$st")); // TODO(log)
158+
}
159+
}
160+
161+
return defaultSoundUrl;
162+
}
163+
42164
/// Create our notification channel, if it doesn't already exist.
43165
///
44166
/// Deletes obsolete channels, if present, from old versions of the app.
@@ -80,13 +202,15 @@ class NotificationChannelManager {
80202

81203
// The channel doesn't exist. Create it.
82204

205+
final defaultSoundUrl = await _ensureInitNotificationSounds();
206+
83207
await _androidHost.createNotificationChannel(NotificationChannel(
84208
id: kChannelId,
85209
name: 'Messages', // TODO(i18n)
86210
importance: NotificationImportance.high,
87211
lightsEnabled: true,
212+
soundUrl: defaultSoundUrl,
88213
vibrationPattern: kVibrationPattern,
89-
// TODO(#340) sound
90214
));
91215
}
92216
}

test/notifications/display_test.dart

+137-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:http/testing.dart' as http_testing;
1313
import 'package:zulip/api/model/model.dart';
1414
import 'package:zulip/api/notifications.dart';
1515
import 'package:zulip/host/android_notifications.dart';
16+
import 'package:zulip/model/binding.dart';
1617
import 'package:zulip/model/localizations.dart';
1718
import 'package:zulip/model/narrow.dart';
1819
import 'package:zulip/model/store.dart';
@@ -121,15 +122,16 @@ void main() {
121122
await NotificationService.instance.start();
122123
}
123124

124-
group('NotificationChannelManager', () {
125+
group('NotificationChannelManager create channel', () {
125126
test('smoke', () async {
126127
await init();
127128
check(testBinding.androidNotificationHost.takeCreatedChannels()).single
128129
..id.equals(NotificationChannelManager.kChannelId)
129130
..name.equals('Messages')
130131
..importance.equals(NotificationImportance.high)
131132
..lightsEnabled.equals(true)
132-
..soundUrl.isNull()
133+
..soundUrl.equals(testBinding.androidNotificationHost.fakeStoredNotificationSoundUrl(
134+
NotificationChannelManager.kDefaultNotificationSound.resourceName))
133135
..vibrationPattern.isNotNull().deepEquals(
134136
NotificationChannelManager.kVibrationPattern)
135137
;
@@ -211,6 +213,134 @@ void main() {
211213
});
212214
});
213215

216+
group('NotificationChannelManager sounds', () {
217+
String fakeStoredUrl(String resourceName) =>
218+
testBinding.androidNotificationHost.fakeStoredNotificationSoundUrl(resourceName);
219+
220+
test('on Android 28 (and lower) resource file is used for notification sound', () async {
221+
addTearDown(testBinding.reset);
222+
final androidNotificationHost = testBinding.androidNotificationHost;
223+
224+
testBinding.deviceInfoResult =
225+
const AndroidDeviceInfo(sdkInt: 28, release: '9');
226+
227+
// Ensure that on Android 10, notification sounds aren't being copied to
228+
// the media store, and resource file is used directly.
229+
await NotificationChannelManager.ensureChannel();
230+
check(androidNotificationHost.takeCopySoundResourceToMediaStoreCalls())
231+
.isEmpty();
232+
233+
final defaultSoundResourceName =
234+
NotificationChannelManager.kDefaultNotificationSound.resourceName;
235+
final soundUrl =
236+
'android.resource://com.zulip.flutter/raw/$defaultSoundResourceName';
237+
check(androidNotificationHost.takeCreatedChannels())
238+
.single
239+
.soundUrl.equals(soundUrl);
240+
});
241+
242+
test('notification sound resource files are being copied to the media store', () async {
243+
addTearDown(testBinding.reset);
244+
final androidNotificationHost = testBinding.androidNotificationHost;
245+
246+
await NotificationChannelManager.ensureChannel();
247+
check(androidNotificationHost.takeCopySoundResourceToMediaStoreCalls())
248+
.deepEquals(NotificationSound.values.map((e) => (
249+
sourceResourceName: e.resourceName,
250+
targetFileDisplayName: e.fileDisplayName),
251+
));
252+
253+
// Ensure the default source URL points to a file in the media store,
254+
// rather than a resource file.
255+
final defaultSoundResourceName =
256+
NotificationChannelManager.kDefaultNotificationSound.resourceName;
257+
check(androidNotificationHost.takeCreatedChannels())
258+
.single
259+
.soundUrl.equals(fakeStoredUrl(defaultSoundResourceName));
260+
});
261+
262+
test('notification sounds are not copied again if they were previously copied', () async {
263+
addTearDown(testBinding.reset);
264+
final androidNotificationHost = testBinding.androidNotificationHost;
265+
266+
// Emulate that all notifications sounds are already in the media store.
267+
androidNotificationHost.setupStoredNotificationSounds(
268+
NotificationSound.values.map((e) => StoredNotificationSound(
269+
fileName: e.fileDisplayName,
270+
isOwned: true,
271+
contentUrl: fakeStoredUrl(e.resourceName)),
272+
).toList(),
273+
);
274+
275+
await NotificationChannelManager.ensureChannel();
276+
check(androidNotificationHost.takeCopySoundResourceToMediaStoreCalls())
277+
.isEmpty();
278+
279+
final defaultSoundResourceName =
280+
NotificationChannelManager.kDefaultNotificationSound.resourceName;
281+
check(androidNotificationHost.takeCreatedChannels())
282+
.single
283+
.soundUrl.equals(fakeStoredUrl(defaultSoundResourceName));
284+
});
285+
286+
test('new notification sounds are copied to media store', () async {
287+
addTearDown(testBinding.reset);
288+
final androidNotificationHost = testBinding.androidNotificationHost;
289+
290+
// Emulate that except one sound, all other sounds are already in
291+
// media store.
292+
androidNotificationHost.setupStoredNotificationSounds(
293+
NotificationSound.values.skip(1).map((e) => StoredNotificationSound(
294+
fileName: e.fileDisplayName,
295+
isOwned: true,
296+
contentUrl: fakeStoredUrl(e.resourceName)),
297+
).toList()
298+
);
299+
300+
await NotificationChannelManager.ensureChannel();
301+
final firstSound = NotificationSound.values.first;
302+
check(androidNotificationHost.takeCopySoundResourceToMediaStoreCalls())
303+
.single
304+
..sourceResourceName.equals(firstSound.resourceName)
305+
..targetFileDisplayName.equals(firstSound.fileDisplayName);
306+
307+
final defaultSoundResourceName =
308+
NotificationChannelManager.kDefaultNotificationSound.resourceName;
309+
check(androidNotificationHost.takeCreatedChannels())
310+
.single
311+
.soundUrl.equals(fakeStoredUrl(defaultSoundResourceName));
312+
});
313+
314+
test('no recopying of existing notification sounds in the media store; default sound URL points to resource file', () async {
315+
addTearDown(testBinding.reset);
316+
final androidNotificationHost = testBinding.androidNotificationHost;
317+
318+
androidNotificationHost.setupStoredNotificationSounds(
319+
NotificationSound.values.map((e) => StoredNotificationSound(
320+
fileName: e.fileDisplayName,
321+
isOwned: false,
322+
contentUrl: fakeStoredUrl(e.resourceName)),
323+
).toList()
324+
);
325+
326+
// Ensure that if a notification sound with the same name already exists
327+
// in the media store, but it wasn't copied by us, no recopying should
328+
// happen. Additionally, the default sound URL should point to the
329+
// resource file, not the version in the media store.
330+
await NotificationChannelManager.ensureChannel();
331+
check(androidNotificationHost.takeCopySoundResourceToMediaStoreCalls())
332+
.isEmpty();
333+
334+
final defaultSoundResourceName =
335+
NotificationChannelManager.kDefaultNotificationSound.resourceName;
336+
final soundUrl =
337+
'android.resource://com.zulip.flutter/raw/$defaultSoundResourceName';
338+
check(androidNotificationHost.takeCreatedChannels())
339+
.single
340+
.soundUrl.equals(soundUrl);
341+
});
342+
});
343+
214344
group('NotificationDisplayManager show', () {
215345
void checkNotification(MessageFcmMessage data, {
216346
required List<MessageFcmMessage> messageStyleMessages,
@@ -1182,6 +1312,11 @@ void main() {
11821312
});
11831313
}
11841314

1315+
extension on Subject<CopySoundResourceToMediaStoreCall> {
1316+
Subject<String> get targetFileDisplayName => has((x) => x.targetFileDisplayName, 'targetFileDisplayName');
1317+
Subject<String> get sourceResourceName => has((x) => x.sourceResourceName, 'sourceResourceName');
1318+
}
1319+
11851320
extension NotificationChannelChecks on Subject<NotificationChannel> {
11861321
Subject<String> get id => has((x) => x.id, 'id');
11871322
Subject<int> get importance => has((x) => x.importance, 'importance');

0 commit comments

Comments
 (0)