mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
fix(mobile): stale community selection during mobile invite setup (#7951)
Buzz sets up starter channels when a person accepts an invite. If the phone already had a saved community, the first attempt could fail. Retry could then succeed. A delayed read could replace the new active community with the old one. The read now leaves the saved selection unchanged. The fix also delays channel-status updates until the channel provider finishes initialization. ### Before and after Controlled Flutter app captures against local relays, with the same delayed storage read. The before capture restores the pre-fix active-community provider in the test harness. The after capture uses the fixed provider. These are separate from the native iOS validation below. | Before: first attempt fails | After: first attempt succeeds | | --- | --- | |  |  | ### Validation - Regression tests reproduce the delayed-read ordering and passing timing control; removing the channel sequencing safeguards makes their regressions fail. - Production app against local relays passes the forced race on the first attempt. - Native iOS simulator with an existing saved community accepted the original invite using a fresh identity, joined on the first attempt, and opened the welcome channel without Retry. Signed-off-by: Tom Brow <tomb@block.xyz>
This commit is contained in:
@@ -132,6 +132,7 @@ class _ChannelRefreshFence {
|
||||
|
||||
/// Whether the refresh still owns the active scope and generation.
|
||||
bool get isCurrent =>
|
||||
_coordinator.isMounted() &&
|
||||
_generation == _coordinator.generation &&
|
||||
scope == _coordinator.currentScope();
|
||||
|
||||
@@ -303,6 +304,9 @@ Map<String, int> _memberCountsByChannelId(Iterable<NostrEvent> memberEvents) {
|
||||
/// Lives in this part file because `channels_provider.dart` sits against the
|
||||
/// repository-wide 1200-line file ceiling enforced by `just file-size-check`.
|
||||
class _ChannelRefreshCoordinator {
|
||||
/// Whether the owning provider can still read its current scope.
|
||||
final bool Function() isMounted;
|
||||
|
||||
/// Resolves the relay-and-identity scope that is active right now.
|
||||
final String Function() currentScope;
|
||||
|
||||
@@ -315,16 +319,18 @@ class _ChannelRefreshCoordinator {
|
||||
int get generation => _generation;
|
||||
|
||||
_ChannelRefreshCoordinator({
|
||||
required this.isMounted,
|
||||
required this.currentScope,
|
||||
required this.loadStatus,
|
||||
});
|
||||
|
||||
/// Binds the fence to a notifier's [Ref] so the provider needs one line.
|
||||
///
|
||||
/// Both closures read rather than watch: the fence asks what the scope is
|
||||
/// right now, and must not make the notifier depend on it.
|
||||
/// The closures read rather than watch: the fence checks the current
|
||||
/// lifecycle and scope without making the notifier depend on them.
|
||||
factory _ChannelRefreshCoordinator.forRef(Ref ref) =>
|
||||
_ChannelRefreshCoordinator(
|
||||
isMounted: () => ref.mounted,
|
||||
currentScope: () => channelDirectoryScope(
|
||||
ref.read(relayConfigProvider).baseUrl,
|
||||
ref.read(myPubkeyProvider),
|
||||
@@ -343,9 +349,16 @@ class _ChannelRefreshCoordinator {
|
||||
/// Used by callers that must carry the scope across later awaits even when
|
||||
/// they do not refresh discovery, so a membership-only refresh cannot install
|
||||
/// an old scope's list either.
|
||||
_ChannelRefreshFence beginRefresh({required bool fetchesDirectory}) {
|
||||
Future<_ChannelRefreshFence> beginRefresh({
|
||||
required bool fetchesDirectory,
|
||||
}) async {
|
||||
final scope = currentScope();
|
||||
final generation = ++_generation;
|
||||
final fence = _ChannelRefreshFence(this, scope, generation);
|
||||
// Acquire ownership synchronously, but let the provider finish building
|
||||
// before changing another provider's directory status.
|
||||
await Future<void>.value();
|
||||
fence.ensureCurrent();
|
||||
if (!fetchesDirectory) {
|
||||
final status = loadStatus();
|
||||
if (status.isLoading(scope)) {
|
||||
@@ -359,7 +372,7 @@ class _ChannelRefreshCoordinator {
|
||||
status.markError(scope);
|
||||
}
|
||||
}
|
||||
return _ChannelRefreshFence(this, scope, generation);
|
||||
return fence;
|
||||
}
|
||||
|
||||
/// Fetches the directory under [fence], or throws if the fence is retired.
|
||||
|
||||
@@ -187,7 +187,7 @@ class ChannelsNotifier extends AsyncNotifier<List<Channel>> {
|
||||
// Acquire request ownership before the first relay await. Every channel-list
|
||||
// path uses this fence so completion order cannot let an older ordinary,
|
||||
// directory, or reconnect refresh replace a newer membership list.
|
||||
final fence = _refreshCoordinator.beginRefresh(
|
||||
final fence = await _refreshCoordinator.beginRefresh(
|
||||
fetchesDirectory: fetchDirectory,
|
||||
);
|
||||
|
||||
|
||||
@@ -618,21 +618,11 @@ final activeCommunityProvider = FutureProvider<Community?>((ref) async {
|
||||
|
||||
if (communities.isEmpty) return null;
|
||||
|
||||
if (activeId == null) {
|
||||
// No active ID stored but communities exist — fall back to first.
|
||||
await storage.saveActiveId(communities.first.id);
|
||||
return communities.first;
|
||||
}
|
||||
|
||||
try {
|
||||
return communities.firstWhere((w) => w.id == activeId);
|
||||
} on StateError {
|
||||
// Active ID points to a community that no longer exists.
|
||||
// Fall back to first community.
|
||||
if (communities.isNotEmpty) {
|
||||
await storage.saveActiveId(communities.first.id);
|
||||
return communities.first;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
// This projection can outlive the list it captured above. Never persist a
|
||||
// fallback here: a newer authentication may already have selected a community
|
||||
// absent from that list. Authentication owns durable active-ID repair.
|
||||
return communities.firstWhere(
|
||||
(community) => community.id == activeId,
|
||||
orElse: () => communities.first,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -293,67 +293,88 @@ void main() {
|
||||
);
|
||||
});
|
||||
|
||||
test(
|
||||
'ordinary refresh settles a superseded directory load for retry',
|
||||
() async {
|
||||
final session = _FakeRelaySession(
|
||||
memberships: [_membership(_channelA, myPk)],
|
||||
metadata: [
|
||||
_meta(id: _channelA, name: 'general'),
|
||||
_meta(id: _channelB, name: 'discoverable'),
|
||||
],
|
||||
);
|
||||
final container = _buildContainer(session: session);
|
||||
addTearDown(container.dispose);
|
||||
for (final rebuild in [false, true]) {
|
||||
test(
|
||||
'${rebuild ? 'Same-scope rebuild' : 'Ordinary refresh'} settles a superseded directory load for retry',
|
||||
() async {
|
||||
final session = _FakeRelaySession(
|
||||
memberships: [_membership(_channelA, myPk)],
|
||||
metadata: [
|
||||
_meta(id: _channelA, name: 'general'),
|
||||
_meta(id: _channelB, name: 'discoverable'),
|
||||
],
|
||||
);
|
||||
final container = _buildContainer(session: session);
|
||||
addTearDown(container.dispose);
|
||||
|
||||
expect(
|
||||
(await container.read(
|
||||
channelsProvider.future,
|
||||
)).map((channel) => channel.id),
|
||||
[_channelA],
|
||||
);
|
||||
expect(
|
||||
(await container.read(
|
||||
channelsProvider.future,
|
||||
)).map((channel) => channel.id),
|
||||
[_channelA],
|
||||
);
|
||||
|
||||
session.pauseNextDirectoryQuery();
|
||||
final directory = container
|
||||
.read(channelsProvider.notifier)
|
||||
.retryDirectory();
|
||||
await session.nextDirectoryQueryStarted;
|
||||
session.pauseNextDirectoryQuery();
|
||||
final directory = container
|
||||
.read(channelsProvider.notifier)
|
||||
.retryDirectory();
|
||||
await session.nextDirectoryQueryStarted;
|
||||
|
||||
// A foreground, reconnect, pull-to-refresh, or membership update can
|
||||
// start an ordinary refresh while Browse is still loading discovery.
|
||||
await container.read(channelsProvider.notifier).refresh();
|
||||
if (rebuild) {
|
||||
// A same-scope configuration rebuild can overlap an in-flight
|
||||
// directory request when onboarding settles its community providers.
|
||||
final config = container.read(relayConfigProvider);
|
||||
container
|
||||
.read(relayConfigProvider.notifier)
|
||||
.update(baseUrl: config.baseUrl, nsec: config.nsec);
|
||||
await container.read(channelsProvider.future);
|
||||
} else {
|
||||
await container.read(channelsProvider.notifier).refresh();
|
||||
}
|
||||
|
||||
session.resumePausedDirectoryQuery();
|
||||
await directory;
|
||||
await _settle();
|
||||
session.resumePausedDirectoryQuery();
|
||||
await directory;
|
||||
await _settle();
|
||||
|
||||
expect(
|
||||
container.read(channelDirectoryLoadStatusProvider).status,
|
||||
ChannelDirectoryLoadStatus.error,
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.read(channelsProvider)
|
||||
.requireValue
|
||||
.map((channel) => channel.id),
|
||||
[_channelA],
|
||||
);
|
||||
expect(
|
||||
container.read(channelDirectoryLoadStatusProvider).status,
|
||||
ChannelDirectoryLoadStatus.error,
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.read(channelsProvider)
|
||||
.requireValue
|
||||
.map((channel) => channel.id),
|
||||
[_channelA],
|
||||
);
|
||||
|
||||
await container.read(channelsProvider.notifier).retryDirectory();
|
||||
await container.read(channelsProvider.notifier).retryDirectory();
|
||||
|
||||
expect(
|
||||
container.read(channelDirectoryLoadStatusProvider).status,
|
||||
ChannelDirectoryLoadStatus.loaded,
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.read(channelsProvider)
|
||||
.requireValue
|
||||
.map((channel) => channel.id),
|
||||
unorderedEquals([_channelA, _channelB]),
|
||||
);
|
||||
},
|
||||
);
|
||||
expect(
|
||||
container.read(channelDirectoryLoadStatusProvider).status,
|
||||
ChannelDirectoryLoadStatus.loaded,
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.read(channelsProvider)
|
||||
.requireValue
|
||||
.map((channel) => channel.id),
|
||||
unorderedEquals([_channelA, _channelB]),
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
test('refresh disposed before directory settlement writes nothing', () async {
|
||||
final session = _FakeRelaySession(memberships: const []);
|
||||
final container = _buildContainer(session: session);
|
||||
await container.read(channelsProvider.future);
|
||||
|
||||
final refresh = container.read(channelsProvider.notifier).refresh();
|
||||
container.dispose();
|
||||
|
||||
await expectLater(refresh, completes);
|
||||
});
|
||||
|
||||
test(
|
||||
'community switch discards a stale directory success from the old relay',
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:buzz/features/invites/invite_join_provider.dart';
|
||||
import 'package:buzz/shared/auth/auth.dart';
|
||||
import 'package:buzz/shared/deeplink/deep_link.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart';
|
||||
import 'package:nostr/nostr.dart' as nostr;
|
||||
|
||||
import '../../shared/community/community_storage_test.dart';
|
||||
|
||||
void main() {
|
||||
for (final finishReadBeforeJoin in [false, true]) {
|
||||
test('first invite setup succeeds when old active read finishes '
|
||||
'${finishReadBeforeJoin ? 'before' : 'after'} new selection', () async {
|
||||
final storage = _PausedActiveReadStorage();
|
||||
final old = Community.create(
|
||||
name: 'Existing',
|
||||
relayUrl: 'wss://old.example.com',
|
||||
nsec: nostr.Keys.generate().nsec,
|
||||
);
|
||||
await storage.save(old);
|
||||
await storage.saveActiveId(old.id);
|
||||
var recoveryCalls = 0;
|
||||
final container = ProviderContainer(
|
||||
overrides: [
|
||||
communityStorageProvider.overrideWithValue(storage),
|
||||
communitySnapshotWriterProvider.overrideWithValue((_) async {}),
|
||||
inviteJoinHttpClientProvider.overrideWithValue(
|
||||
MockClient(
|
||||
(_) async => http.Response(
|
||||
'{"status":"joined","host":"new.example.com","role":"member"}',
|
||||
200,
|
||||
),
|
||||
),
|
||||
),
|
||||
inviteJoinRecoveryProvider.overrideWithValue((_) {
|
||||
recoveryCalls++;
|
||||
return _SuccessfulRecovery();
|
||||
}),
|
||||
],
|
||||
);
|
||||
addTearDown(container.dispose);
|
||||
// The app restores authentication before displaying the invite.
|
||||
await container.read(authProvider.future);
|
||||
storage.pauseNextRead = true;
|
||||
final oldProjection = container.read(activeCommunityProvider.future);
|
||||
await storage.readStarted.future;
|
||||
if (finishReadBeforeJoin) {
|
||||
storage.resumeRead.complete();
|
||||
await oldProjection;
|
||||
}
|
||||
final join = container.read(inviteJoinProvider.notifier);
|
||||
await join.prepare(
|
||||
const InviteDeepLink(
|
||||
relayUrl: 'wss://new.example.com',
|
||||
code: 'test-invite',
|
||||
),
|
||||
);
|
||||
await join.confirmJoin();
|
||||
await oldProjection;
|
||||
|
||||
final result = container.read(inviteJoinProvider);
|
||||
expect(result.status, InviteJoinStatus.success);
|
||||
expect(result.errorMessage, isNull);
|
||||
expect(recoveryCalls, 1);
|
||||
final communities = await storage.loadAll();
|
||||
final joined = communities.singleWhere((c) => c.id != old.id);
|
||||
expect(await storage.loadActiveId(), joined.id);
|
||||
expect(joined.starterSetupIncomplete, isFalse);
|
||||
expect(
|
||||
(await container.read(activeCommunityProvider.future))?.id,
|
||||
joined.id,
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
class _SuccessfulRecovery implements InviteJoinRecovery {
|
||||
@override
|
||||
Future<String?> ensureStarterChannels() async => 'welcome';
|
||||
}
|
||||
|
||||
/// Delays the storage read after the provider captures its community list.
|
||||
/// All reads and writes succeed; only their ordering changes.
|
||||
class _PausedActiveReadStorage extends CommunityStorage {
|
||||
_PausedActiveReadStorage() : super(secure: FakeSecureStorage());
|
||||
|
||||
bool pauseNextRead = false;
|
||||
final readStarted = Completer<void>();
|
||||
final resumeRead = Completer<void>();
|
||||
|
||||
@override
|
||||
Future<String?> loadActiveId() async {
|
||||
if (pauseNextRead) {
|
||||
pauseNextRead = false;
|
||||
readStarted.complete();
|
||||
await resumeRead.future;
|
||||
}
|
||||
return super.loadActiveId();
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> saveActiveId(String id) async {
|
||||
await super.saveActiveId(id);
|
||||
if (readStarted.isCompleted && !resumeRead.isCompleted) {
|
||||
resumeRead.complete();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -686,6 +686,19 @@ void main() {
|
||||
expect(active.name, 'Test');
|
||||
});
|
||||
|
||||
test('falls back without persisting when active ID is missing', () async {
|
||||
final community = Community.create(
|
||||
name: 'Fallback',
|
||||
relayUrl: 'https://test.example.com',
|
||||
);
|
||||
await communityStorage.save(community);
|
||||
container = createContainer();
|
||||
|
||||
final active = await container.read(activeCommunityProvider.future);
|
||||
expect(active?.id, community.id);
|
||||
expect(await communityStorage.loadActiveId(), isNull);
|
||||
});
|
||||
|
||||
test('falls back to first community if active ID is invalid', () async {
|
||||
container = createContainer();
|
||||
await container.read(communityListProvider.future);
|
||||
@@ -705,6 +718,7 @@ void main() {
|
||||
final active = await container.read(activeCommunityProvider.future);
|
||||
expect(active, isNotNull);
|
||||
expect(active!.id, ws.id);
|
||||
expect(await communityStorage.loadActiveId(), 'nonexistent-id');
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user