Skip to content

Commit 5c778bd

Browse files
committed
store [nfc]: The event-poll loop has-a store, not is-a store
This seems to make more sense conceptually. For its most immediate practical effect, this means UpdateMachine.fromInitialSnapshot doesn't care whether PerAccountStore.fromInitialSnapshot is a generative or a factory constructor, which will give us the flexibility to make it the latter. This change will also probably be helpful for making other decouplings of the polling and registerNotificationToken logic from the data structures of PerAccountStore.
1 parent 77e2db8 commit 5c778bd

File tree

3 files changed

+32
-30
lines changed

3 files changed

+32
-30
lines changed

lib/model/store.dart

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,9 @@ class LiveGlobalStore extends GlobalStore {
418418
final AppDatabase _db;
419419

420420
@override
421-
Future<PerAccountStore> loadPerAccount(Account account) {
422-
return LivePerAccountStore.load(account);
421+
Future<PerAccountStore> loadPerAccount(Account account) async {
422+
final updateMachine = await UpdateMachine.load(account);
423+
return updateMachine.store;
423424
}
424425

425426
@override
@@ -436,26 +437,24 @@ class LiveGlobalStore extends GlobalStore {
436437
}
437438
}
438439

439-
/// A [PerAccountStore] which polls an event queue to stay up to date.
440+
/// A [PerAccountStore] plus an event-polling loop to stay up to date.
440441
// TODO decouple "live"ness from polling and registerNotificationToken;
441442
// the latter are made up of testable internal logic, not external integration
442-
class LivePerAccountStore extends PerAccountStore {
443-
LivePerAccountStore.fromInitialSnapshot({
444-
required super.account,
445-
required super.connection,
446-
required super.initialSnapshot,
443+
class UpdateMachine {
444+
UpdateMachine.fromInitialSnapshot({
445+
required this.store,
446+
required InitialSnapshot initialSnapshot,
447447
}) : queueId = initialSnapshot.queueId ?? (() {
448448
// The queueId is optional in the type, but should only be missing in the
449449
// case of unauthenticated access to a web-public realm. We authenticated.
450450
throw Exception("bad initial snapshot: missing queueId");
451451
})(),
452-
lastEventId = initialSnapshot.lastEventId,
453-
super.fromInitialSnapshot();
452+
lastEventId = initialSnapshot.lastEventId;
454453

455454
/// Load the user's data from the server, and start an event queue going.
456455
///
457456
/// In the future this might load an old snapshot from local storage first.
458-
static Future<PerAccountStore> load(Account account) async {
457+
static Future<UpdateMachine> load(Account account) async {
459458
final connection = ApiConnection.live(
460459
realmUrl: account.realmUrl, zulipFeatureLevel: account.zulipFeatureLevel,
461460
email: account.email, apiKey: account.apiKey);
@@ -466,30 +465,33 @@ class LivePerAccountStore extends PerAccountStore {
466465
// TODO log the time better
467466
if (kDebugMode) print("initial fetch time: ${t.inMilliseconds}ms");
468467

469-
final store = LivePerAccountStore.fromInitialSnapshot(
468+
final store = PerAccountStore.fromInitialSnapshot(
470469
account: account,
471470
connection: connection,
472471
initialSnapshot: initialSnapshot,
473472
);
474-
store.poll();
473+
final updateMachine = UpdateMachine.fromInitialSnapshot(
474+
store: store, initialSnapshot: initialSnapshot);
475+
updateMachine.poll();
475476
// TODO do registerNotificationToken before registerQueue:
476477
// https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807
477-
store.registerNotificationToken();
478-
return store;
478+
updateMachine.registerNotificationToken();
479+
return updateMachine;
479480
}
480481

482+
final PerAccountStore store;
481483
final String queueId;
482484
int lastEventId;
483485

484486
void poll() async {
485487
while (true) {
486-
final result = await getEvents(connection,
488+
final result = await getEvents(store.connection,
487489
queueId: queueId, lastEventId: lastEventId);
488490
// TODO handle errors on get-events; retry with backoff
489491
// TODO abort long-poll and close ApiConnection on [dispose]
490492
final events = result.events;
491493
for (final event in events) {
492-
handleEvent(event);
494+
store.handleEvent(event);
493495
}
494496
if (events.isNotEmpty) {
495497
lastEventId = events.last.id;
@@ -513,6 +515,6 @@ class LivePerAccountStore extends PerAccountStore {
513515
Future<void> _registerNotificationToken() async {
514516
final token = NotificationService.instance.token.value;
515517
if (token == null) return;
516-
await NotificationService.registerToken(connection, token: token);
518+
await NotificationService.registerToken(store.connection, token: token);
517519
}
518520
}

test/example_data.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,11 @@ PerAccountStore store({Account? account, InitialSnapshot? initialSnapshot}) {
473473
initialSnapshot: initialSnapshot ?? _initialSnapshot(),
474474
);
475475
}
476+
const _store = store;
476477

477-
LivePerAccountStore liveStore({Account? account, InitialSnapshot? initialSnapshot}) {
478-
return LivePerAccountStore.fromInitialSnapshot(
479-
account: account ?? selfAccount,
480-
connection: FakeApiConnection.fromAccount(account ?? selfAccount),
481-
initialSnapshot: initialSnapshot ?? _initialSnapshot(),
482-
);
478+
UpdateMachine updateMachine({Account? account, InitialSnapshot? initialSnapshot}) {
479+
initialSnapshot ??= _initialSnapshot();
480+
final store = _store(account: account, initialSnapshot: initialSnapshot);
481+
return UpdateMachine.fromInitialSnapshot(
482+
store: store, initialSnapshot: initialSnapshot);
483483
}

test/model/store_test.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,13 @@ void main() {
108108
check(completers(1)).length.equals(1);
109109
});
110110

111-
group('PerAccountStore.registerNotificationToken', () {
112-
late LivePerAccountStore store;
111+
group('UpdateMachine.registerNotificationToken', () {
112+
late UpdateMachine updateMachine;
113113
late FakeApiConnection connection;
114114

115115
void prepareStore() {
116-
store = eg.liveStore();
117-
connection = store.connection as FakeApiConnection;
116+
updateMachine = eg.updateMachine();
117+
connection = updateMachine.store.connection as FakeApiConnection;
118118
}
119119

120120
void checkLastRequestApns({required String token, required String appid}) {
@@ -143,7 +143,7 @@ void main() {
143143
// On store startup, send the token.
144144
prepareStore();
145145
connection.prepare(json: {});
146-
await store.registerNotificationToken();
146+
await updateMachine.registerNotificationToken();
147147
if (defaultTargetPlatform == TargetPlatform.android) {
148148
checkLastRequestFcm(token: '012abc');
149149
} else {
@@ -177,7 +177,7 @@ void main() {
177177

178178
// On store startup, send nothing (because we have nothing to send).
179179
prepareStore();
180-
await store.registerNotificationToken();
180+
await updateMachine.registerNotificationToken();
181181
check(connection.lastRequest).isNull();
182182

183183
// When the token later appears, send it.

0 commit comments

Comments
 (0)