mirror of
https://github.com/block/buzz.git
synced 2026-11-14 05:12:27 +00:00
fix(mobile): skip a taken starter only when its holder is unusable
A refused starter create was skipped whenever the directory did not show the name's holder, so a starter another client had just created, whose kind:39000 had not arrived, was treated as finished setup. Skip only a holder the directory shows cannot serve as the starter, and apply that to an archived channel at the deterministic starter id too. A holder the directory does not show keeps the setup recoverable. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
@@ -123,9 +123,9 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
/// All starter work is fenced to the community and identity that started it.
|
||||
/// A partial setup remains recoverable, so a failed operation deliberately
|
||||
/// propagates to the invite flow instead of being reported as success. The
|
||||
/// one exception is a starter name the relay reports as already held by a
|
||||
/// channel that cannot serve as the starter: that is permanent, so the
|
||||
/// starter is skipped instead of blocking the join.
|
||||
/// one exception is a starter whose name or id the relay reports as taken
|
||||
/// by a channel the directory shows cannot serve as the starter: that is
|
||||
/// permanent, so the starter is skipped instead of blocking the join.
|
||||
@override
|
||||
Future<String?> ensureStarterChannels() async {
|
||||
_ensureScopeCurrent();
|
||||
@@ -156,17 +156,20 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
_ensureScopeCurrent();
|
||||
channels = await _loadChannels();
|
||||
_ensureScopeCurrent();
|
||||
channel =
|
||||
final holder =
|
||||
_findStarterChannel(channels, starter.slug) ??
|
||||
channels
|
||||
.where((candidate) => candidate.id == channelId)
|
||||
.firstOrNull;
|
||||
// The relay reserves a listed channel's name, archived ones
|
||||
// included, so a taken name will never become creatable on retry.
|
||||
// An archived or non-stream namesake cannot serve as the starter, so
|
||||
// skip it rather than strand the invite in permanent recovery.
|
||||
if (channel == null && nameTaken) continue;
|
||||
if (channel == null) rethrow;
|
||||
.firstOrNull ??
|
||||
(nameTaken ? _findListedNamesake(channels, starter.slug) : null);
|
||||
// A holder the directory does not show yet may still be usable, so
|
||||
// keep the setup recoverable.
|
||||
if (holder == null) rethrow;
|
||||
// The relay keeps a channel's id and a listed channel's name, archived
|
||||
// ones included, so an unusable holder never frees the starter. Skip
|
||||
// it rather than strand the invite in permanent recovery.
|
||||
if (!_isUsableStarter(holder)) continue;
|
||||
channel = holder;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -192,18 +195,20 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
}
|
||||
}
|
||||
|
||||
Channel? _findStarterChannel(List<Channel> channels, String name) {
|
||||
final normalizedName = name.trim().toLowerCase();
|
||||
return channels
|
||||
.where(
|
||||
(channel) =>
|
||||
channel.name.trim().toLowerCase() == normalizedName &&
|
||||
channel.isStream &&
|
||||
channel.visibility == 'open' &&
|
||||
!channel.isArchived,
|
||||
)
|
||||
.firstOrNull;
|
||||
}
|
||||
Channel? _findStarterChannel(List<Channel> channels, String name) => channels
|
||||
.where((channel) => _hasName(channel, name) && _isUsableStarter(channel))
|
||||
.firstOrNull;
|
||||
|
||||
/// The open channel that holds [name] in the relay's listed-name rule.
|
||||
Channel? _findListedNamesake(List<Channel> channels, String name) => channels
|
||||
.where((channel) => _hasName(channel, name) && channel.visibility == 'open')
|
||||
.firstOrNull;
|
||||
|
||||
bool _hasName(Channel channel, String name) =>
|
||||
channel.name.trim().toLowerCase() == name.trim().toLowerCase();
|
||||
|
||||
bool _isUsableStarter(Channel channel) =>
|
||||
channel.isStream && channel.visibility == 'open' && !channel.isArchived;
|
||||
|
||||
bool _isDuplicateChannelError(Object error) =>
|
||||
error.toString().contains('duplicate: channel already exists');
|
||||
|
||||
@@ -368,26 +368,33 @@ void main() {
|
||||
test(
|
||||
'taken starter names are skipped or converged instead of failing the invite',
|
||||
() async {
|
||||
const origin = 'https://relay.example.com';
|
||||
const refusal = 'invalid: an open channel named "general" already exists';
|
||||
final archivedGeneral = _channel(
|
||||
id: 'old-general',
|
||||
name: 'General',
|
||||
archivedAt: DateTime.utc(2025),
|
||||
const duplicateId = 'duplicate: channel already exists';
|
||||
final generalId = desktopStarterChannelId(
|
||||
relayHttpOrigin: origin,
|
||||
slug: 'general',
|
||||
);
|
||||
final forumGeneral = _channel(
|
||||
id: 'forum-general',
|
||||
name: 'general',
|
||||
channelType: 'forum',
|
||||
final welcomeId = desktopStarterChannelId(
|
||||
relayHttpOrigin: origin,
|
||||
slug: 'welcome-everyone',
|
||||
);
|
||||
for (final namesake in [archivedGeneral, forumGeneral]) {
|
||||
({
|
||||
List<String> created,
|
||||
List<String> joined,
|
||||
MobileInviteJoinRecovery recovery,
|
||||
int Function() loads,
|
||||
})
|
||||
starterSetup({
|
||||
required List<Channel> Function(int load) channelsOnLoad,
|
||||
required String generalError,
|
||||
bool Function()? isScopeCurrent,
|
||||
}) {
|
||||
final created = <String>[];
|
||||
final joined = <String>[];
|
||||
var loadCount = 0;
|
||||
final recovery = MobileInviteJoinRecovery(
|
||||
loadChannels: () async {
|
||||
loadCount++;
|
||||
return [namesake];
|
||||
},
|
||||
loadChannels: () async => channelsOnLoad(++loadCount),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -398,76 +405,103 @@ void main() {
|
||||
ttlSeconds,
|
||||
}) async {
|
||||
created.add(name);
|
||||
if (name == 'general') throw Exception(refusal);
|
||||
if (name == 'general') throw Exception(generalError);
|
||||
return _channel(id: channelId, name: name, isMember: true);
|
||||
},
|
||||
joinChannel: (channelId) async => joined.add(channelId),
|
||||
relayHttpOrigin: 'https://relay.example.com',
|
||||
relayHttpOrigin: origin,
|
||||
isScopeCurrent: isScopeCurrent,
|
||||
);
|
||||
|
||||
final focusChannelId = await recovery.ensureStarterChannels();
|
||||
|
||||
// The unusable namesake is neither joined nor retried; the other
|
||||
// starter is still created and focused.
|
||||
expect(created, ['general', 'welcome-everyone'], reason: namesake.id);
|
||||
expect(joined, isEmpty, reason: namesake.id);
|
||||
expect(loadCount, 2, reason: namesake.id);
|
||||
expect(
|
||||
focusChannelId,
|
||||
desktopStarterChannelId(
|
||||
relayHttpOrigin: 'https://relay.example.com',
|
||||
slug: 'welcome-everyone',
|
||||
),
|
||||
return (
|
||||
created: created,
|
||||
joined: joined,
|
||||
recovery: recovery,
|
||||
loads: () => loadCount,
|
||||
);
|
||||
}
|
||||
|
||||
// A holder the directory shows cannot serve as the starter is neither
|
||||
// joined nor retried; the other starter is still created and focused.
|
||||
final unusableHolders = [
|
||||
(
|
||||
_channel(
|
||||
id: 'old-general',
|
||||
name: 'General',
|
||||
archivedAt: DateTime.utc(2025),
|
||||
),
|
||||
refusal,
|
||||
),
|
||||
(
|
||||
_channel(id: 'forum-general', name: 'general', channelType: 'forum'),
|
||||
refusal,
|
||||
),
|
||||
(
|
||||
_channel(
|
||||
id: generalId,
|
||||
name: 'general',
|
||||
archivedAt: DateTime.utc(2025),
|
||||
),
|
||||
duplicateId,
|
||||
),
|
||||
];
|
||||
for (final (holder, error) in unusableHolders) {
|
||||
final setup = starterSetup(
|
||||
channelsOnLoad: (_) => [holder],
|
||||
generalError: error,
|
||||
);
|
||||
|
||||
final focusChannelId = await setup.recovery.ensureStarterChannels();
|
||||
|
||||
expect(setup.created, [
|
||||
'general',
|
||||
'welcome-everyone',
|
||||
], reason: holder.id);
|
||||
expect(setup.joined, isEmpty, reason: holder.id);
|
||||
expect(setup.loads(), 2, reason: holder.id);
|
||||
expect(focusChannelId, welcomeId, reason: holder.id);
|
||||
}
|
||||
|
||||
// A taken name whose holder the directory does not show yet stays
|
||||
// recoverable: it may be a usable starter another client just created.
|
||||
final unseen = starterSetup(
|
||||
channelsOnLoad: (_) => const [],
|
||||
generalError: refusal,
|
||||
);
|
||||
await expectLater(
|
||||
unseen.recovery.ensureStarterChannels(),
|
||||
throwsException,
|
||||
);
|
||||
expect(unseen.created, ['general']);
|
||||
expect(unseen.joined, isEmpty);
|
||||
expect(unseen.loads(), 2);
|
||||
|
||||
// A usable open channel that appeared after the first load (lost race)
|
||||
// is joined rather than skipped.
|
||||
var loadCount = 0;
|
||||
final joined = <String>[];
|
||||
final raced = MobileInviteJoinRecovery(
|
||||
loadChannels: () async => ++loadCount == 1
|
||||
? const []
|
||||
: [_channel(id: 'raced', name: 'general')],
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
required name,
|
||||
required channelType,
|
||||
required visibility,
|
||||
description,
|
||||
ttlSeconds,
|
||||
}) async => name == 'general'
|
||||
? throw Exception(refusal)
|
||||
: _channel(id: channelId, name: name, isMember: true),
|
||||
joinChannel: (channelId) async => joined.add(channelId),
|
||||
relayHttpOrigin: 'https://relay.example.com',
|
||||
final raced = starterSetup(
|
||||
channelsOnLoad: (load) =>
|
||||
load == 1 ? const [] : [_channel(id: 'raced', name: 'general')],
|
||||
generalError: refusal,
|
||||
);
|
||||
await raced.ensureStarterChannels();
|
||||
expect(joined, ['raced']);
|
||||
await raced.recovery.ensureStarterChannels();
|
||||
expect(raced.joined, ['raced']);
|
||||
|
||||
// The scope fence still holds after a refusal.
|
||||
// The scope fence still holds after a refusal and its reload.
|
||||
var current = true;
|
||||
final fenced = MobileInviteJoinRecovery(
|
||||
loadChannels: () async {
|
||||
final channels = [archivedGeneral];
|
||||
if (loadCount++ > 1) current = false;
|
||||
return channels;
|
||||
final fenced = starterSetup(
|
||||
channelsOnLoad: (load) {
|
||||
if (load == 2) current = false;
|
||||
return [unusableHolders.first.$1];
|
||||
},
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
required name,
|
||||
required channelType,
|
||||
required visibility,
|
||||
description,
|
||||
ttlSeconds,
|
||||
}) async => throw Exception(refusal),
|
||||
joinChannel: (_) async => fail('scope changed'),
|
||||
relayHttpOrigin: 'https://relay.example.com',
|
||||
generalError: refusal,
|
||||
isScopeCurrent: () => current,
|
||||
);
|
||||
await expectLater(fenced.ensureStarterChannels(), throwsStateError);
|
||||
await expectLater(
|
||||
fenced.recovery.ensureStarterChannels(),
|
||||
throwsStateError,
|
||||
);
|
||||
expect(fenced.created, ['general']);
|
||||
expect(fenced.loads(), 2);
|
||||
expect(fenced.joined, isEmpty);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user