Skip to content

Commit 2db40f4

Browse files
PIG208PIG208
committed
actions [nfc]: Move logOutAccount and unregisterToken under lib/model
plus moving and refactoring the tests to match Signed-off-by: Zixuan James Li <[email protected]>
1 parent 2347aee commit 2db40f4

File tree

7 files changed

+227
-197
lines changed

7 files changed

+227
-197
lines changed

‎lib/model/actions.dart

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import 'dart:async';
2+
3+
import '../notifications/receive.dart';
4+
import 'store.dart';
5+
6+
// TODO: Make this a part of GlobalStore
7+
Future<void> logOutAccount(GlobalStore globalStore, int accountId) async {
8+
final account = globalStore.getAccount(accountId);
9+
if (account == null) return; // TODO(log)
10+
11+
// Unawaited, to not block removing the account on this request.
12+
unawaited(unregisterToken(globalStore, accountId));
13+
14+
await globalStore.removeAccount(accountId);
15+
}
16+
17+
Future<void> unregisterToken(GlobalStore globalStore, int accountId) async {
18+
final account = globalStore.getAccount(accountId);
19+
if (account == null) return; // TODO(log)
20+
21+
// TODO(#322) use actual acked push token; until #322, this is just null.
22+
final token = account.ackedPushToken
23+
// Try the current token as a fallback; maybe the server has registered
24+
// it and we just haven't recorded that fact in the client.
25+
?? NotificationService.instance.token.value;
26+
if (token == null) return;
27+
28+
final connection = globalStore.apiConnectionFromAccount(account);
29+
try {
30+
await NotificationService.unregisterToken(connection, token: token);
31+
} catch (e) {
32+
// TODO retry? handle failures?
33+
} finally {
34+
connection.close();
35+
}
36+
}

‎lib/widgets/actions.dart

-33
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,9 @@ import '../api/model/narrow.dart';
1515
import '../api/route/messages.dart';
1616
import '../generated/l10n/zulip_localizations.dart';
1717
import '../model/narrow.dart';
18-
import '../model/store.dart';
19-
import '../notifications/receive.dart';
2018
import 'dialog.dart';
2119
import 'store.dart';
2220

23-
Future<void> logOutAccount(GlobalStore globalStore, int accountId) async {
24-
final account = globalStore.getAccount(accountId);
25-
if (account == null) return; // TODO(log)
26-
27-
// Unawaited, to not block removing the account on this request.
28-
unawaited(unregisterToken(globalStore, accountId));
29-
30-
await globalStore.removeAccount(accountId);
31-
}
32-
33-
Future<void> unregisterToken(GlobalStore globalStore, int accountId) async {
34-
final account = globalStore.getAccount(accountId);
35-
if (account == null) return; // TODO(log)
36-
37-
// TODO(#322) use actual acked push token; until #322, this is just null.
38-
final token = account.ackedPushToken
39-
// Try the current token as a fallback; maybe the server has registered
40-
// it and we just haven't recorded that fact in the client.
41-
?? NotificationService.instance.token.value;
42-
if (token == null) return;
43-
44-
final connection = globalStore.apiConnectionFromAccount(account);
45-
try {
46-
await NotificationService.unregisterToken(connection, token: token);
47-
} catch (e) {
48-
// TODO retry? handle failures?
49-
} finally {
50-
connection.close();
51-
}
52-
}
53-
5421
Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
5522
final store = PerAccountStoreWidget.of(context);
5623
final connection = store.connection;

‎lib/widgets/app.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import 'package:flutter/scheduler.dart';
66

77
import '../generated/l10n/zulip_localizations.dart';
88
import '../log.dart';
9+
import '../model/actions.dart';
910
import '../model/localizations.dart';
1011
import '../model/store.dart';
1112
import '../notifications/display.dart';
1213
import 'about_zulip.dart';
13-
import 'actions.dart';
1414
import 'dialog.dart';
1515
import 'home.dart';
1616
import 'login.dart';

‎test/model/actions_test.dart

+188
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:flutter/foundation.dart';
3+
import 'package:flutter_test/flutter_test.dart';
4+
import 'package:http/http.dart' as http;
5+
import 'package:zulip/api/exception.dart';
6+
import 'package:zulip/model/actions.dart';
7+
import 'package:zulip/model/store.dart';
8+
import 'package:zulip/notifications/receive.dart';
9+
10+
import '../api/fake_api.dart';
11+
import '../example_data.dart' as eg;
12+
import '../fake_async.dart';
13+
import '../model/binding.dart';
14+
import '../model/store_checks.dart';
15+
import '../model/test_store.dart';
16+
import '../stdlib_checks.dart';
17+
import 'store_test.dart';
18+
19+
void main() {
20+
TestZulipBinding.ensureInitialized();
21+
22+
late PerAccountStore store;
23+
late FakeApiConnection connection;
24+
25+
Future<void> prepare({String? ackedPushToken = '123'}) async {
26+
addTearDown(testBinding.reset);
27+
final selfAccount = eg.selfAccount.copyWith(ackedPushToken: Value(ackedPushToken));
28+
await testBinding.globalStore.add(selfAccount, eg.initialSnapshot());
29+
store = await testBinding.globalStore.perAccount(selfAccount.id);
30+
connection = store.connection as FakeApiConnection;
31+
}
32+
33+
/// Creates and caches a new [FakeApiConnection] in [TestGlobalStore].
34+
///
35+
/// In live code, [unregisterToken] makes a new [ApiConnection] for the
36+
/// unregister-token request instead of reusing the store's connection.
37+
/// To enable callers to prepare responses for that request, this function
38+
/// creates a new [FakeApiConnection] and caches it in [TestGlobalStore]
39+
/// for [unregisterToken] to pick up.
40+
///
41+
/// Call this instead of just turning on
42+
/// [TestGlobalStore.useCachedApiConnections] so that [unregisterToken]
43+
/// doesn't try to call `close` twice on the same connection instance,
44+
/// which isn't allowed. (Once by the unregister-token code
45+
/// and once as part of removing the account.)
46+
FakeApiConnection separateConnection() {
47+
testBinding.globalStore
48+
..clearCachedApiConnections()
49+
..useCachedApiConnections = true;
50+
return testBinding.globalStore
51+
.apiConnectionFromAccount(eg.selfAccount) as FakeApiConnection;
52+
}
53+
54+
String unregisterApiPathForPlatform(TargetPlatform platform) {
55+
return switch (platform) {
56+
TargetPlatform.android => '/api/v1/users/me/android_gcm_reg_id',
57+
TargetPlatform.iOS => '/api/v1/users/me/apns_device_token',
58+
_ => throw Error(),
59+
};
60+
}
61+
62+
void checkSingleUnregisterRequest(
63+
FakeApiConnection connection, {
64+
String? expectedToken,
65+
}) {
66+
final subject = check(connection.takeRequests()).single.isA<http.Request>()
67+
..method.equals('DELETE')
68+
..url.path.equals(unregisterApiPathForPlatform(defaultTargetPlatform));
69+
if (expectedToken != null) {
70+
subject.bodyFields.deepEquals({'token': expectedToken});
71+
}
72+
}
73+
74+
group('logOutAccount', () {
75+
test('smoke', () => awaitFakeAsync((async) async {
76+
await prepare();
77+
check(testBinding.globalStore).accountIds.single.equals(eg.selfAccount.id);
78+
const unregisterDelay = Duration(seconds: 5);
79+
assert(unregisterDelay > TestGlobalStore.removeAccountDuration);
80+
final newConnection = separateConnection()
81+
..prepare(delay: unregisterDelay, json: {'msg': '', 'result': 'success'});
82+
83+
final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id);
84+
// Unregister-token request and account removal dispatched together
85+
checkSingleUnregisterRequest(newConnection);
86+
check(testBinding.globalStore.takeDoRemoveAccountCalls())
87+
.single.equals(eg.selfAccount.id);
88+
89+
async.elapse(TestGlobalStore.removeAccountDuration);
90+
await future;
91+
// Account removal not blocked on unregister-token response
92+
check(testBinding.globalStore).accountIds.isEmpty();
93+
check(connection.isOpen).isFalse();
94+
check(newConnection.isOpen).isTrue(); // still busy with unregister-token
95+
96+
async.elapse(unregisterDelay - TestGlobalStore.removeAccountDuration);
97+
check(newConnection.isOpen).isFalse();
98+
}));
99+
100+
test('unregister request has an error', () => awaitFakeAsync((async) async {
101+
await prepare();
102+
check(testBinding.globalStore).accountIds.single.equals(eg.selfAccount.id);
103+
const unregisterDelay = Duration(seconds: 5);
104+
assert(unregisterDelay > TestGlobalStore.removeAccountDuration);
105+
final exception = ZulipApiException(
106+
httpStatus: 401,
107+
code: 'UNAUTHORIZED',
108+
data: {"result": "error", "msg": "Invalid API key", "code": "UNAUTHORIZED"},
109+
routeName: 'removeEtcEtcToken',
110+
message: 'Invalid API key',
111+
);
112+
final newConnection = separateConnection()
113+
..prepare(delay: unregisterDelay, exception: exception);
114+
115+
final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id);
116+
// Unregister-token request and account removal dispatched together
117+
checkSingleUnregisterRequest(newConnection);
118+
check(testBinding.globalStore.takeDoRemoveAccountCalls())
119+
.single.equals(eg.selfAccount.id);
120+
121+
async.elapse(TestGlobalStore.removeAccountDuration);
122+
await future;
123+
// Account removal not blocked on unregister-token response
124+
check(testBinding.globalStore).accountIds.isEmpty();
125+
check(connection.isOpen).isFalse();
126+
check(newConnection.isOpen).isTrue(); // for the unregister-token request
127+
128+
async.elapse(unregisterDelay - TestGlobalStore.removeAccountDuration);
129+
check(newConnection.isOpen).isFalse();
130+
}));
131+
});
132+
133+
group('unregisterToken', () {
134+
testAndroidIos('smoke, happy path', () => awaitFakeAsync((async) async {
135+
await prepare(ackedPushToken: '123');
136+
137+
final newConnection = separateConnection()
138+
..prepare(json: {'msg': '', 'result': 'success'});
139+
final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id);
140+
async.elapse(Duration.zero);
141+
await future;
142+
checkSingleUnregisterRequest(newConnection, expectedToken: '123');
143+
check(newConnection.isOpen).isFalse();
144+
}));
145+
146+
test('fallback to current token if acked is missing', () => awaitFakeAsync((async) async {
147+
await prepare(ackedPushToken: null);
148+
NotificationService.instance.token = ValueNotifier('asdf');
149+
150+
final newConnection = separateConnection()
151+
..prepare(json: {'msg': '', 'result': 'success'});
152+
final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id);
153+
async.elapse(Duration.zero);
154+
await future;
155+
checkSingleUnregisterRequest(newConnection, expectedToken: 'asdf');
156+
check(newConnection.isOpen).isFalse();
157+
}));
158+
159+
test('no error if acked token and current token both missing', () => awaitFakeAsync((async) async {
160+
await prepare(ackedPushToken: null);
161+
NotificationService.instance.token = ValueNotifier(null);
162+
163+
final newConnection = separateConnection();
164+
final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id);
165+
async.flushTimers();
166+
await future;
167+
check(newConnection.takeRequests()).isEmpty();
168+
}));
169+
170+
test('connection closed if request errors', () => awaitFakeAsync((async) async {
171+
await prepare(ackedPushToken: '123');
172+
173+
final newConnection = separateConnection()
174+
..prepare(exception: ZulipApiException(
175+
httpStatus: 401,
176+
code: 'UNAUTHORIZED',
177+
data: {"result": "error", "msg": "Invalid API key", "code": "UNAUTHORIZED"},
178+
routeName: 'removeEtcEtcToken',
179+
message: 'Invalid API key',
180+
));
181+
final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id);
182+
async.elapse(Duration.zero);
183+
await future;
184+
checkSingleUnregisterRequest(newConnection, expectedToken: '123');
185+
check(newConnection.isOpen).isFalse();
186+
}));
187+
});
188+
}

0 commit comments

Comments
 (0)