Skip to content

Commit 30cb335

Browse files
gnpricechrisbobbe
gnprice
authored and
chrisbobbe
committed
push_device [nfc]: Move registering push token off of UpdateMachine
This makes a more comfortable home for adding further logic in this area.
1 parent 55df456 commit 30cb335

File tree

5 files changed

+202
-160
lines changed

5 files changed

+202
-160
lines changed

‎lib/model/push_device.dart‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import '../notifications/receive.dart';
2+
import 'store.dart';
3+
4+
/// Manages telling the server this device's push token,
5+
/// and tracking the server's responses on the status of push devices.
6+
// TODO(#1764) do that tracking of responses
7+
class PushDeviceManager extends PerAccountStoreBase {
8+
PushDeviceManager({required super.core});
9+
10+
bool _disposed = false;
11+
12+
/// Cleans up resources and tells the instance not to make new API requests.
13+
///
14+
/// After this is called, the instance is not in a usable state
15+
/// and should be abandoned.
16+
void dispose() {
17+
assert(!_disposed);
18+
NotificationService.instance.token.removeListener(_registerToken);
19+
_disposed = true;
20+
}
21+
22+
/// Send this client's notification token to the server, now and if it changes.
23+
///
24+
/// TODO The returned future isn't especially meaningful (it may or may not
25+
/// mean we actually sent the token). Make it just `void` once we fix the
26+
/// one test that relies on the future.
27+
// TODO(#322) save acked token, to dedupe updating it on the server
28+
// TODO(#323) track the addFcmToken/etc request, warn if not succeeding
29+
Future<void> registerToken() async {
30+
if (!debugEnableRegisterToken) {
31+
return;
32+
}
33+
NotificationService.instance.token.addListener(_registerToken);
34+
await _registerToken();
35+
}
36+
37+
Future<void> _registerToken() async {
38+
// TODO it would be nice to register the token before even registerQueue:
39+
// https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807
40+
await NotificationService.instance.registerToken(connection);
41+
}
42+
43+
/// In debug mode, controls whether [registerToken] should
44+
/// have its normal effect.
45+
///
46+
/// Outside of debug mode, this is always true and the setter has no effect.
47+
static bool get debugEnableRegisterToken {
48+
bool result = true;
49+
assert(() {
50+
result = _debugEnableRegisterToken;
51+
return true;
52+
}());
53+
return result;
54+
}
55+
static bool _debugEnableRegisterToken = true;
56+
static set debugEnableRegisterToken(bool value) {
57+
assert(() {
58+
_debugEnableRegisterToken = value;
59+
return true;
60+
}());
61+
}
62+
}

‎lib/model/store.dart‎

Lines changed: 7 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ import '../api/route/events.dart';
1717
import '../api/backoff.dart';
1818
import '../api/route/realm.dart';
1919
import '../log.dart';
20-
import '../notifications/receive.dart';
2120
import 'actions.dart';
2221
import 'autocomplete.dart';
2322
import 'database.dart';
2423
import 'emoji.dart';
2524
import 'localizations.dart';
2625
import 'message.dart';
2726
import 'presence.dart';
27+
import 'push_device.dart';
2828
import 'realm.dart';
2929
import 'recent_dm_conversations.dart';
3030
import 'recent_senders.dart';
@@ -529,6 +529,7 @@ class PerAccountStore extends PerAccountStoreBase with
529529
emoji: EmojiStoreImpl(core: core,
530530
allRealmEmoji: initialSnapshot.realmEmoji),
531531
userSettings: initialSnapshot.userSettings,
532+
pushDevices: PushDeviceManager(core: core),
532533
savedSnippets: SavedSnippetStoreImpl(core: core,
533534
savedSnippets: initialSnapshot.savedSnippets ?? []),
534535
typingNotifier: TypingNotifier(realm: realm),
@@ -552,6 +553,7 @@ class PerAccountStore extends PerAccountStoreBase with
552553
required RealmStoreImpl realm,
553554
required EmojiStoreImpl emoji,
554555
required this.userSettings,
556+
required this.pushDevices,
555557
required SavedSnippetStoreImpl savedSnippets,
556558
required this.typingNotifier,
557559
required UserStoreImpl users,
@@ -623,6 +625,8 @@ class PerAccountStore extends PerAccountStoreBase with
623625

624626
final UserSettings userSettings;
625627

628+
final PushDeviceManager pushDevices;
629+
626630
@override
627631
Map<int, SavedSnippet> get savedSnippets => _savedSnippets.savedSnippets;
628632
final SavedSnippetStoreImpl _savedSnippets;
@@ -714,6 +718,7 @@ class PerAccountStore extends PerAccountStoreBase with
714718
presence.dispose();
715719
typingStatus.dispose();
716720
typingNotifier.dispose();
721+
pushDevices.dispose();
717722
updateMachine?.dispose();
718723
connection.close();
719724
_disposed = true;
@@ -1118,9 +1123,7 @@ class UpdateMachine {
11181123
// serverEmojiDataUrl are already unsupported at time of writing.)
11191124
unawaited(updateMachine.fetchEmojiData(initialSnapshot.serverEmojiDataUrl!));
11201125
}
1121-
// TODO do registerNotificationToken before registerQueue:
1122-
// https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807
1123-
unawaited(updateMachine.registerNotificationToken());
1126+
unawaited(store.pushDevices.registerToken());
11241127
store.presence.start();
11251128
return updateMachine;
11261129
}
@@ -1505,26 +1508,6 @@ class UpdateMachine {
15051508
store.realmUrl.toString(), error.toString()));
15061509
}
15071510

1508-
/// Send this client's notification token to the server, now and if it changes.
1509-
///
1510-
/// TODO The returned future isn't especially meaningful (it may or may not
1511-
/// mean we actually sent the token). Make it just `void` once we fix the
1512-
/// one test that relies on the future.
1513-
// TODO(#322) save acked token, to dedupe updating it on the server
1514-
// TODO(#323) track the addFcmToken/etc request, warn if not succeeding
1515-
Future<void> registerNotificationToken() async {
1516-
assert(!_disposed);
1517-
if (!debugEnableRegisterNotificationToken) {
1518-
return;
1519-
}
1520-
NotificationService.instance.token.addListener(_registerNotificationToken);
1521-
await _registerNotificationToken();
1522-
}
1523-
1524-
Future<void> _registerNotificationToken() async {
1525-
await NotificationService.instance.registerToken(store.connection);
1526-
}
1527-
15281511
/// Cleans up resources and tells the instance not to make new API requests.
15291512
///
15301513
/// After this is called, the instance is not in a usable state
@@ -1535,7 +1518,6 @@ class UpdateMachine {
15351518
/// requests to error. [PerAccountStore.dispose] does that.
15361519
void dispose() {
15371520
assert(!_disposed);
1538-
NotificationService.instance.token.removeListener(_registerNotificationToken);
15391521
_disposed = true;
15401522
}
15411523

@@ -1559,26 +1541,6 @@ class UpdateMachine {
15591541
}());
15601542
}
15611543

1562-
/// In debug mode, controls whether [registerNotificationToken] should
1563-
/// have its normal effect.
1564-
///
1565-
/// Outside of debug mode, this is always true and the setter has no effect.
1566-
static bool get debugEnableRegisterNotificationToken {
1567-
bool result = true;
1568-
assert(() {
1569-
result = _debugEnableRegisterNotificationToken;
1570-
return true;
1571-
}());
1572-
return result;
1573-
}
1574-
static bool _debugEnableRegisterNotificationToken = true;
1575-
static set debugEnableRegisterNotificationToken(bool value) {
1576-
assert(() {
1577-
_debugEnableRegisterNotificationToken = value;
1578-
return true;
1579-
}());
1580-
}
1581-
15821544
@override
15831545
String toString() => '${objectRuntimeType(this, 'UpdateMachine')}#${shortHash(this)}';
15841546
}

‎test/model/push_device_test.dart‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:flutter/foundation.dart';
3+
import 'package:http/http.dart' as http;
4+
import 'package:test/scaffolding.dart';
5+
import 'package:zulip/model/push_device.dart';
6+
import 'package:zulip/notifications/receive.dart';
7+
8+
import '../api/fake_api.dart';
9+
import '../example_data.dart' as eg;
10+
import '../fake_async.dart';
11+
import 'binding.dart';
12+
import 'store_test.dart';
13+
import '../stdlib_checks.dart';
14+
15+
void main() {
16+
TestZulipBinding.ensureInitialized();
17+
18+
group('registerToken', () {
19+
late PushDeviceManager model;
20+
late FakeApiConnection connection;
21+
22+
void prepareStore() {
23+
final store = eg.store();
24+
model = store.pushDevices;
25+
connection = store.connection as FakeApiConnection;
26+
}
27+
28+
void checkLastRequestApns({required String token, required String appid}) {
29+
check(connection.takeRequests()).single.isA<http.Request>()
30+
..method.equals('POST')
31+
..url.path.equals('/api/v1/users/me/apns_device_token')
32+
..bodyFields.deepEquals({'token': token, 'appid': appid});
33+
}
34+
35+
void checkLastRequestFcm({required String token}) {
36+
check(connection.takeRequests()).single.isA<http.Request>()
37+
..method.equals('POST')
38+
..url.path.equals('/api/v1/users/me/android_gcm_reg_id')
39+
..bodyFields.deepEquals({'token': token});
40+
}
41+
42+
testAndroidIos('token already known', () => awaitFakeAsync((async) async {
43+
// This tests the case where [NotificationService.start] has already
44+
// learned the token before the store is created.
45+
// (This is probably the common case.)
46+
addTearDown(testBinding.reset);
47+
testBinding.firebaseMessagingInitialToken = '012abc';
48+
testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter');
49+
addTearDown(NotificationService.debugReset);
50+
await NotificationService.instance.start();
51+
52+
// On store startup, send the token.
53+
prepareStore();
54+
connection.prepare(json: {});
55+
await model.registerToken();
56+
if (defaultTargetPlatform == TargetPlatform.android) {
57+
checkLastRequestFcm(token: '012abc');
58+
} else {
59+
checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter');
60+
}
61+
62+
if (defaultTargetPlatform == TargetPlatform.android) {
63+
// If the token changes, send it again.
64+
testBinding.firebaseMessaging.setToken('456def');
65+
connection.prepare(json: {});
66+
async.flushMicrotasks();
67+
checkLastRequestFcm(token: '456def');
68+
}
69+
}));
70+
71+
testAndroidIos('token initially unknown', () => awaitFakeAsync((async) async {
72+
// This tests the case where the store is created while our
73+
// request for the token is still pending.
74+
addTearDown(testBinding.reset);
75+
testBinding.firebaseMessagingInitialToken = '012abc';
76+
testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter');
77+
addTearDown(NotificationService.debugReset);
78+
final startFuture = NotificationService.instance.start();
79+
80+
// TODO this test is a bit brittle in its interaction with asynchrony;
81+
// to fix, probably extend TestZulipBinding to control when getToken finishes.
82+
//
83+
// The aim here is to first wait for `model.registerToken`
84+
// to complete whatever it's going to do; then check no request was made;
85+
// and only after that wait for `NotificationService.start` to finish,
86+
// including its `getToken` call.
87+
88+
// On store startup, send nothing (because we have nothing to send).
89+
prepareStore();
90+
await model.registerToken();
91+
check(connection.lastRequest).isNull();
92+
93+
// When the token later appears, send it.
94+
connection.prepare(json: {});
95+
await startFuture;
96+
async.flushMicrotasks();
97+
if (defaultTargetPlatform == TargetPlatform.android) {
98+
checkLastRequestFcm(token: '012abc');
99+
} else {
100+
checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter');
101+
}
102+
103+
if (defaultTargetPlatform == TargetPlatform.android) {
104+
// If the token subsequently changes, send it again.
105+
testBinding.firebaseMessaging.setToken('456def');
106+
connection.prepare(json: {});
107+
async.flushMicrotasks();
108+
checkLastRequestFcm(token: '456def');
109+
}
110+
}));
111+
112+
test('on iOS, use provided app ID from packageInfo', () => awaitFakeAsync((async) async {
113+
final origTargetPlatform = debugDefaultTargetPlatformOverride;
114+
addTearDown(() => debugDefaultTargetPlatformOverride = origTargetPlatform);
115+
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
116+
addTearDown(testBinding.reset);
117+
testBinding.firebaseMessagingInitialToken = '012abc';
118+
testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.example.test');
119+
addTearDown(NotificationService.debugReset);
120+
await NotificationService.instance.start();
121+
122+
prepareStore();
123+
connection.prepare(json: {});
124+
await model.registerToken();
125+
checkLastRequestApns(token: '012abc', appid: 'com.example.test');
126+
}));
127+
});
128+
}

0 commit comments

Comments
 (0)