Skip to content

Commit fe16ad1

Browse files
rajveermalviyagnprice
authored andcommitted
notif: Use Zulip's distinct notification sound on Android
Fixes: #340
1 parent 9785936 commit fe16ad1

File tree

6 files changed

+250
-5
lines changed

6 files changed

+250
-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

+123-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,120 @@ void main() {
211213
});
212214
});
213215

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

1301+
extension on Subject<CopySoundResourceToMediaStoreCall> {
1302+
Subject<String> get targetFileDisplayName => has((x) => x.targetFileDisplayName, 'targetFileDisplayName');
1303+
Subject<String> get sourceResourceName => has((x) => x.sourceResourceName, 'sourceResourceName');
1304+
}
1305+
11851306
extension NotificationChannelChecks on Subject<NotificationChannel> {
11861307
Subject<String> get id => has((x) => x.id, 'id');
11871308
Subject<int> get importance => has((x) => x.importance, 'importance');

0 commit comments

Comments
 (0)