mirror of
https://github.com/block/buzz.git
synced 2026-11-03 07:51:22 +00:00
fix(mobile): keep community icons on originals and record thumbnail 404s at fetch (#8896)
Follow-up to #8879, addressing its review. ## Changes - **Community icons load the original again.** `AvatarImageContent` gained a `preferThumbnail` flag, off by default. People and agent avatars turn it on (as #8879 did for every caller); `CommunityAvatar` leaves it off. Community logos are often transparent PNGs, and the relay's JPEG thumbnail renders transparency black. This matches desktop, which shows community icons from the original. - **A missing thumbnail is recorded where its 404 arrives.** Before, only the mounted `Image`'s error builder recorded it. If the avatar scrolled away before a slow 404 came back, the thumbnail URL kept a 30s failure cooldown instead, and showing that avatar again at any size during the cooldown rendered initials and never requested the original. The fetch now records a 404 on a relay `/media/<sha256>.thumb.jpg` URL in the missing-thumbnail set and puts no cooldown on it. ## Tests Two journey tests in `avatar_image_test.dart`, both failing on main and passing here: - An avatar unmounts before its slow thumbnail 404 arrives, then mounts again at another size: it requests the original once, renders it (valid image bytes, no initials), and leaves no cooldown. - A 36pt `CommunityAvatar` with a relay-hosted icon requests and shows only the original. `flutter analyze` is clean. In the full local `flutter test` run, only 12 push and notification tests fail, and they fail the same way on main in this environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,9 @@ import 'dart:math' as math;
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/foundation.dart';
|
||||
|
||||
import '../../shared/widgets/ios_native_segmented_control.dart';
|
||||
|
||||
import 'package:flutter_hooks/flutter_hooks.dart';
|
||||
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
|
||||
@@ -311,6 +313,7 @@ class AgentAvatarPicker extends HookConsumerWidget {
|
||||
),
|
||||
child: AvatarImageContent(
|
||||
imageUrl: agentAvatarCatalog[i].url,
|
||||
preferThumbnail: true,
|
||||
fallback: const Icon(Icons.smart_toy_outlined),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -59,6 +59,7 @@ class AgentCard extends StatelessWidget {
|
||||
)
|
||||
: AvatarImageContent(
|
||||
imageUrl: poster,
|
||||
preferThumbnail: true,
|
||||
fallback: fallback,
|
||||
fit: portraitFit,
|
||||
);
|
||||
@@ -89,6 +90,7 @@ class AgentCard extends StatelessWidget {
|
||||
scale: 1.8,
|
||||
child: AvatarImageContent(
|
||||
imageUrl: poster,
|
||||
preferThumbnail: true,
|
||||
fallback: const SizedBox.shrink(),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -96,6 +96,7 @@ class SettingsProfileHeader extends HookConsumerWidget {
|
||||
: activeHandoff == null || animatedPosterUrl == null
|
||||
? AvatarImageContent(
|
||||
imageUrl: avatarUrl,
|
||||
preferThumbnail: true,
|
||||
fallback: _AvatarFallback(initial: profile?.initial),
|
||||
)
|
||||
: Stack(
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
import '../../shared/widgets/profile_action_chip.dart';
|
||||
import '../../shared/widgets/app_list_card.dart';
|
||||
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_hooks/flutter_hooks.dart';
|
||||
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
@@ -602,6 +604,7 @@ class _ProfileAvatar extends HookWidget {
|
||||
)
|
||||
: AvatarImageContent(
|
||||
imageUrl: animatedAvatar?.posterUrl ?? avatarUrl,
|
||||
preferThumbnail: true,
|
||||
fallback: _AvatarFallback(initial: initial),
|
||||
);
|
||||
return AvatarClip(
|
||||
|
||||
@@ -45,6 +45,13 @@ class MediaImageProvider extends ImageProvider<MediaImageProvider> {
|
||||
/// Excluded from equality because it is a side effect, not cache identity.
|
||||
final ValueChanged<Uint8List>? onBytesLoaded;
|
||||
|
||||
/// Set when [url] is a thumbnail [MediaImage] chose in place of an
|
||||
/// original: its 404 then records a missing thumbnail, so later builds load
|
||||
/// the original, rather than a failure cooldown. Any other 404, including a
|
||||
/// profile URL that is itself a thumbnail, keeps the cooldown. Excluded from
|
||||
/// equality: it changes how a failure is handled, not the bytes.
|
||||
final bool isThumbnailFallback;
|
||||
|
||||
/// Excluded from equality: transport, not identity.
|
||||
final http.Client client;
|
||||
|
||||
@@ -62,6 +69,7 @@ class MediaImageProvider extends ImageProvider<MediaImageProvider> {
|
||||
required this.auth,
|
||||
required this.client,
|
||||
this.onBytesLoaded,
|
||||
this.isThumbnailFallback = false,
|
||||
this.scale = 1.0,
|
||||
this.stallTimeout = const Duration(seconds: 20),
|
||||
this.maxFetchDuration = const Duration(minutes: 5),
|
||||
@@ -240,6 +248,14 @@ class MediaImageProvider extends ImageProvider<MediaImageProvider> {
|
||||
}
|
||||
rethrow;
|
||||
}
|
||||
if (response.statusCode == 404 && isThumbnailFallback) {
|
||||
// Older or externally populated media may have no generated thumbnail.
|
||||
// Record that here rather than in a widget's error builder: the widget
|
||||
// that asked may be gone by the time a slow 404 arrives, and a
|
||||
// cooldown would then keep every later build off the original.
|
||||
MediaImage._rememberMissingThumbnail(url);
|
||||
throw NetworkImageLoadException(statusCode: 404, uri: uri);
|
||||
}
|
||||
if (response.statusCode != 200) {
|
||||
_cooldownUntil[url] = debugNow().add(_cooldownFor(response));
|
||||
throw NetworkImageLoadException(
|
||||
@@ -468,11 +484,15 @@ class MediaImage extends ConsumerWidget {
|
||||
Widget build(BuildContext context, WidgetRef ref) {
|
||||
final auth = ref.watch(mediaGetAuthServiceProvider);
|
||||
final client = ref.watch(mediaHttpClientProvider);
|
||||
MediaImageProvider providerFor(String source) => MediaImageProvider(
|
||||
MediaImageProvider providerFor(
|
||||
String source, {
|
||||
bool isThumbnailFallback = false,
|
||||
}) => MediaImageProvider(
|
||||
url: source,
|
||||
auth: auth,
|
||||
client: client,
|
||||
onBytesLoaded: onBytesLoaded,
|
||||
isThumbnailFallback: isThumbnailFallback,
|
||||
);
|
||||
|
||||
if (decodeWidth != null) {
|
||||
@@ -496,17 +516,15 @@ class MediaImage extends ConsumerWidget {
|
||||
: candidate;
|
||||
if (thumbnail == null) return _image(providerFor(url), cacheWidth);
|
||||
return _image(
|
||||
providerFor(thumbnail),
|
||||
providerFor(thumbnail, isThumbnailFallback: true),
|
||||
cacheWidth,
|
||||
errorBuilder: (context, error, stackTrace) {
|
||||
if (error is! NetworkImageLoadException ||
|
||||
error.statusCode != 404) {
|
||||
return errorBuilder!(context, error, stackTrace);
|
||||
}
|
||||
// Older or externally populated media may have no generated
|
||||
// thumbnail. Remember that, so later builds go straight to the
|
||||
// original instead of hitting the thumbnail's failure cooldown.
|
||||
_rememberMissingThumbnail(thumbnail);
|
||||
// The fetch already recorded the missing thumbnail, so later
|
||||
// builds go straight to the original.
|
||||
return _image(providerFor(url), cacheWidth);
|
||||
},
|
||||
);
|
||||
@@ -520,11 +538,10 @@ class MediaImage extends ConsumerWidget {
|
||||
static final Set<String> _missingThumbnails = <String>{};
|
||||
static const _maxMissingThumbnails = 256;
|
||||
|
||||
/// Called by the fetch that got the 404, which puts no failure cooldown on
|
||||
/// the thumbnail: this set stands in for it, and an entry evicted here then
|
||||
/// costs one more 404 rather than showing the fallback.
|
||||
static void _rememberMissingThumbnail(String url) {
|
||||
// This set now stands in for the 404's failure cooldown. Drop that, or
|
||||
// nothing would fetch the URL again to clear it, and an entry evicted
|
||||
// here during its cooldown would show the fallback, not the original.
|
||||
MediaImageProvider._cooldownUntil.remove(url);
|
||||
_missingThumbnails
|
||||
..remove(url)
|
||||
..add(url);
|
||||
|
||||
@@ -49,6 +49,7 @@ class AgentArtworkPanel extends StatelessWidget {
|
||||
scale: 2,
|
||||
child: AvatarImageContent(
|
||||
imageUrl: poster,
|
||||
preferThumbnail: true,
|
||||
fallback: const SizedBox.shrink(),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -45,6 +45,7 @@ class AvatarImage extends StatelessWidget {
|
||||
child: AvatarImageContent(
|
||||
imageUrl: animatedAvatar?.posterUrl ?? imageUrl,
|
||||
fallback: fallback,
|
||||
preferThumbnail: true,
|
||||
),
|
||||
);
|
||||
final clipped = AvatarClip(
|
||||
@@ -77,11 +78,18 @@ class AvatarImageContent extends ConsumerStatefulWidget {
|
||||
final Widget fallback;
|
||||
final BoxFit fit;
|
||||
|
||||
/// Load the relay's small JPEG thumbnail for a small relay-hosted image
|
||||
/// (see [MediaImage.preferThumbnail]). People and agent avatars set this,
|
||||
/// matching desktop. Leave it off for images that may be transparent, such
|
||||
/// as community icons: the thumbnail renders transparent areas black.
|
||||
final bool preferThumbnail;
|
||||
|
||||
const AvatarImageContent({
|
||||
super.key,
|
||||
required this.imageUrl,
|
||||
required this.fallback,
|
||||
this.fit = BoxFit.cover,
|
||||
this.preferThumbnail = false,
|
||||
});
|
||||
|
||||
@override
|
||||
@@ -132,7 +140,7 @@ class _AvatarImageContentState extends ConsumerState<AvatarImageContent> {
|
||||
_NetworkAvatarSource(:final url) => MediaImage(
|
||||
url: url,
|
||||
fit: widget.fit,
|
||||
preferThumbnail: true,
|
||||
preferThumbnail: widget.preferThumbnail,
|
||||
errorBuilder: (_, _, _) => centeredFallback,
|
||||
),
|
||||
null => centeredFallback,
|
||||
|
||||
@@ -110,12 +110,17 @@ class PresetAgentAvatar extends HookWidget {
|
||||
playing: playing,
|
||||
fallback: AvatarImageContent(
|
||||
imageUrl: poster,
|
||||
preferThumbnail: true,
|
||||
fallback: fallback,
|
||||
),
|
||||
)
|
||||
: controller != null && ready.value == controller
|
||||
? VideoPlayer(controller)
|
||||
: AvatarImageContent(imageUrl: poster, fallback: fallback),
|
||||
: AvatarImageContent(
|
||||
imageUrl: poster,
|
||||
preferThumbnail: true,
|
||||
fallback: fallback,
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -80,6 +80,7 @@ class ProgressiveAnimatedAvatar extends HookWidget {
|
||||
AvatarImageContent(
|
||||
key: const ValueKey('progressive-animated-avatar-poster'),
|
||||
imageUrl: descriptor.posterUrl,
|
||||
preferThumbnail: true,
|
||||
fallback: fallback,
|
||||
fit: fit,
|
||||
),
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import 'dart:async';
|
||||
import 'dart:convert';
|
||||
|
||||
import 'package:buzz/shared/community/community_avatar.dart';
|
||||
import 'package:buzz/shared/community/community_icon_provider.dart';
|
||||
import 'package:buzz/shared/relay/media_auth.dart';
|
||||
import 'package:buzz/shared/relay/media_image.dart';
|
||||
import 'package:buzz/shared/widgets/avatar_shape.dart';
|
||||
@@ -9,9 +12,16 @@ import 'package:flutter/material.dart';
|
||||
import 'package:flutter_svg/flutter_svg.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
import 'package:hooks_riverpod/misc.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart' as http_testing;
|
||||
|
||||
// Minimal valid 1x1 transparent PNG.
|
||||
final _pngBytes = base64Decode(
|
||||
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAA'
|
||||
'AAYAAjCB0C8AAAAASUVORK5CYII=',
|
||||
);
|
||||
|
||||
void main() {
|
||||
const svg =
|
||||
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32">'
|
||||
@@ -211,6 +221,161 @@ void main() {
|
||||
);
|
||||
});
|
||||
|
||||
group('relay thumbnail journeys', () {
|
||||
const relay = 'https://relay.example';
|
||||
late List<String> requested;
|
||||
late Future<http.Response> Function(String url) respond;
|
||||
final valueOf = ValueNotifier<Widget>(const SizedBox());
|
||||
|
||||
setUp(() {
|
||||
MediaImageProvider.debugResetCooldowns();
|
||||
MediaImage.debugResetMissingThumbnails();
|
||||
requested = [];
|
||||
respond = (_) async => http.Response.bytes(_pngBytes, 200);
|
||||
valueOf.value = const SizedBox();
|
||||
});
|
||||
tearDown(() {
|
||||
MediaImageProvider.debugResetCooldowns();
|
||||
MediaImage.debugResetMissingThumbnails();
|
||||
});
|
||||
|
||||
Future<void> mount(
|
||||
WidgetTester tester, {
|
||||
List<Override> overrides = const [],
|
||||
}) async {
|
||||
tester.view.devicePixelRatio = 3;
|
||||
addTearDown(tester.view.resetDevicePixelRatio);
|
||||
final client = http_testing.MockClient((request) {
|
||||
final url = request.url.toString();
|
||||
requested.add(url);
|
||||
return respond(url);
|
||||
});
|
||||
await tester.pumpWidget(
|
||||
ProviderScope(
|
||||
overrides: [
|
||||
mediaGetAuthServiceProvider.overrideWithValue(
|
||||
MediaGetAuthService(baseUrl: relay, nsec: null),
|
||||
),
|
||||
mediaHttpClientProvider.overrideWithValue(client),
|
||||
...overrides,
|
||||
],
|
||||
child: MaterialApp(
|
||||
home: Center(
|
||||
child: ValueListenableBuilder<Widget>(
|
||||
valueListenable: valueOf,
|
||||
builder: (_, child, _) => child,
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> show(WidgetTester tester, Widget child) async {
|
||||
valueOf.value = child;
|
||||
for (var i = 0; i < 4; i++) {
|
||||
await tester.runAsync(
|
||||
() => Future<void>.delayed(const Duration(milliseconds: 20)),
|
||||
);
|
||||
await tester.pump();
|
||||
}
|
||||
}
|
||||
|
||||
String shownUrl(WidgetTester tester) {
|
||||
final image = tester.widget<Image>(find.byType(Image));
|
||||
final provider = (image.image as ResizeImage).imageProvider;
|
||||
return (provider as MediaImageProvider).url;
|
||||
}
|
||||
|
||||
testWidgets(
|
||||
'an avatar that left before its thumbnail 404 later shows the original',
|
||||
(tester) async {
|
||||
final original = '$relay/media/${'12' * 32}.png';
|
||||
final thumbnail = '$relay/media/${'12' * 32}.thumb.jpg';
|
||||
final slow404 = Completer<http.Response>();
|
||||
respond = (url) async => url == thumbnail
|
||||
? slow404.future
|
||||
: http.Response.bytes(_pngBytes, 200);
|
||||
await mount(tester);
|
||||
|
||||
// A 40pt avatar asks for the thumbnail on a slow link, then
|
||||
// scrolls away before the relay's 404 arrives.
|
||||
await show(
|
||||
tester,
|
||||
AvatarImage(
|
||||
imageUrl: original,
|
||||
radius: 20,
|
||||
fallback: const Text('M'),
|
||||
),
|
||||
);
|
||||
expect(requested, [thumbnail]);
|
||||
await show(tester, const SizedBox());
|
||||
slow404.complete(http.Response('', 404));
|
||||
await show(tester, const SizedBox());
|
||||
|
||||
// Back at another size, it loads and renders the original rather
|
||||
// than the initials.
|
||||
await show(
|
||||
tester,
|
||||
AvatarImage(
|
||||
imageUrl: original,
|
||||
radius: 16,
|
||||
fallback: const Text('M'),
|
||||
),
|
||||
);
|
||||
expect(requested.where((url) => url == original), hasLength(1));
|
||||
expect(shownUrl(tester), original);
|
||||
expect(find.text('M'), findsNothing);
|
||||
expect(MediaImageProvider.debugCooldownUrls, isEmpty);
|
||||
|
||||
// A profile URL that is itself thumbnail-shaped, on another host or
|
||||
// this relay, is not a fallback MediaImage chose: its 404 keeps the
|
||||
// cooldown, so showing it again does not request it again.
|
||||
respond = (_) async => http.Response('', 404);
|
||||
for (final (i, thumbShaped) in [
|
||||
'https://elsewhere.example/media/${'56' * 32}.thumb.jpg',
|
||||
'$relay/media/${'78' * 32}.thumb.jpg',
|
||||
].indexed) {
|
||||
MediaImageProvider.debugResetCooldowns();
|
||||
for (final radius in [16.0, 17.0]) {
|
||||
await show(
|
||||
tester,
|
||||
AvatarImage(
|
||||
key: ValueKey((i, radius)),
|
||||
imageUrl: thumbShaped,
|
||||
radius: radius,
|
||||
fallback: const Text('E'),
|
||||
),
|
||||
);
|
||||
expect(find.text('E'), findsOneWidget);
|
||||
}
|
||||
expect(requested.where((url) => url == thumbShaped), hasLength(1));
|
||||
expect(MediaImageProvider.debugCooldownUrls, [thumbShaped]);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
testWidgets('community icons load the original, which keeps transparency', (
|
||||
tester,
|
||||
) async {
|
||||
final icon = '$relay/media/${'34' * 32}.png';
|
||||
await mount(
|
||||
tester,
|
||||
overrides: [
|
||||
communityIconPresentationProvider(relay).overrideWithValue(icon),
|
||||
],
|
||||
);
|
||||
|
||||
// The channels header size, well within the thumbnail.
|
||||
await show(
|
||||
tester,
|
||||
const CommunityAvatar(name: 'Buzz', relayUrl: relay, size: 36),
|
||||
);
|
||||
expect(requested, [icon]);
|
||||
expect(shownUrl(tester), icon);
|
||||
});
|
||||
});
|
||||
|
||||
testWidgets('clips agents to the desktop squircle', (tester) async {
|
||||
await tester.pumpWidget(subject(null, isAgent: true));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user