mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(theme): survive full-store snapshot and preserve desktop glass on mobile pre-hydration edits
Two data-loss edge cases in the community-theme sync engine, flagged in review. Desktop: the controller captured the pre-migration appearance snapshot in its layout effect but the sync and persistence effects re-read it from storage. When a full store rejected the snapshot write, that re-read returned nothing and the appearance fallback collapsed to defaults, resetting glass/opacity/prominent-tab for the session and risking a durable overwrite. Cache the captured value in a ref and reuse it across the effects so the correct in-session value survives a failed write. Mobile: a theme edit made before relay hydration published a replaceable coordinate that omitted the desktop-only glass keys, stripping a desktop client's real values from the record. Hold an edit that carries no desktop appearance opinion until the coordinate has been observed (a valid record or a confirmed absence), and when republishing such an edit, merge the desktop-only fields the relay already holds so publishing never strips them. Bookkeeping stays on the local preference so the outbox acknowledgement contract is unchanged. Co-authored-by: kenny lopez <klopez4212@gmail.com> Reviewed-by: Mongo <mongo@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
This commit is contained in:
@@ -12,12 +12,12 @@ import {
|
||||
communityThemeScopeFallback,
|
||||
hasMigratedCommunityTheme,
|
||||
markCommunityThemeMigrated,
|
||||
readCommunityThemeAppearanceSnapshot,
|
||||
readCommunityThemeOutbox,
|
||||
readCommunityThemePreference,
|
||||
sameCommunityThemePreference,
|
||||
writeCommunityThemeOutbox,
|
||||
writeCommunityThemePreference,
|
||||
type CommunityThemeAppearance,
|
||||
type CommunityThemePreference,
|
||||
} from "./communityThemePreference";
|
||||
import {
|
||||
@@ -37,6 +37,7 @@ export function CommunityThemeController() {
|
||||
const relayUrl = activeCommunity?.relayUrl;
|
||||
const managerRef = useRef<CommunityThemeSyncManager | null>(null);
|
||||
const scopeRef = useRef("");
|
||||
const appearanceSnapshotRef = useRef<CommunityThemeAppearance | null>(null);
|
||||
const expectedAppliedRef = useRef<CommunityThemePreference | null>(null);
|
||||
const scopedPreferenceRef = useRef<CommunityThemePreference | null>(null);
|
||||
const lastRemoteRef = useRef({ createdAt: 0, eventId: "" });
|
||||
@@ -89,6 +90,7 @@ export function CommunityThemeController() {
|
||||
pubkey,
|
||||
initialPreferenceRef.current,
|
||||
);
|
||||
appearanceSnapshotRef.current = snapshot;
|
||||
const appearanceFallback = communityThemeAppearanceFallback(snapshot);
|
||||
const scopeFallback: CommunityThemePreference = {
|
||||
...communityThemeScopeFallback(
|
||||
@@ -126,7 +128,11 @@ export function CommunityThemeController() {
|
||||
useEffect(() => {
|
||||
if (!pubkey || !relayUrl) return;
|
||||
const scope = `${pubkey}:${relayUrl}`;
|
||||
const snapshot = readCommunityThemeAppearanceSnapshot(pubkey);
|
||||
// Reuse the snapshot captured by the layout effect above. Re-reading from
|
||||
// storage here would collapse to defaults when the snapshot write was
|
||||
// rejected (a full store), even though the layout effect already resolved
|
||||
// the correct in-session value.
|
||||
const snapshot = appearanceSnapshotRef.current;
|
||||
const appearanceFallback = communityThemeAppearanceFallback(snapshot);
|
||||
const local = readCommunityThemePreference(
|
||||
pubkey,
|
||||
@@ -280,9 +286,7 @@ export function CommunityThemeController() {
|
||||
const stored = readCommunityThemePreference(
|
||||
pubkey,
|
||||
relayUrl,
|
||||
communityThemeAppearanceFallback(
|
||||
readCommunityThemeAppearanceSnapshot(pubkey),
|
||||
),
|
||||
communityThemeAppearanceFallback(appearanceSnapshotRef.current),
|
||||
);
|
||||
if (stored && sameCommunityThemePreference(stored, preference)) return;
|
||||
scopedPreferenceRef.current = preference;
|
||||
|
||||
@@ -195,6 +195,48 @@ test("appearance snapshot is captured once and consumed per community", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("a full store still yields the correct in-session appearance fallback", () => {
|
||||
// The controller captures the snapshot once, then reuses the returned value
|
||||
// across its effects via a ref. This pins the contract that reuse depends on:
|
||||
// even when the snapshot write is rejected (a full store), capture returns
|
||||
// the live pre-migration appearance, so a legacy record still inherits it.
|
||||
globalThis.window = {
|
||||
localStorage: {
|
||||
getItem: () => null,
|
||||
setItem: () => {
|
||||
throw new Error("quota exceeded");
|
||||
},
|
||||
removeItem: () => {},
|
||||
},
|
||||
};
|
||||
const inherited = {
|
||||
...DEFAULT_COMMUNITY_THEME,
|
||||
glassBackground: true,
|
||||
glassOpacity: 80,
|
||||
prominentActiveTab: true,
|
||||
};
|
||||
const snapshot = captureCommunityThemeAppearanceSnapshot("alice", inherited);
|
||||
assert.deepEqual(snapshot, {
|
||||
glassBackground: true,
|
||||
glassOpacity: 80,
|
||||
prominentActiveTab: true,
|
||||
});
|
||||
// A re-read from storage collapses to defaults, which is exactly why the
|
||||
// controller must reuse the captured value rather than re-reading it.
|
||||
assert.equal(readCommunityThemeAppearanceSnapshot("alice"), null);
|
||||
const legacy = {
|
||||
version: 1,
|
||||
theme: "houston",
|
||||
accent: "#a855f7",
|
||||
followSystem: false,
|
||||
};
|
||||
const fallback = communityThemeAppearanceFallback(snapshot);
|
||||
assert.deepEqual(parseCommunityThemePreference(legacy, fallback), {
|
||||
...legacy,
|
||||
...snapshot,
|
||||
});
|
||||
});
|
||||
|
||||
test("desktop appearance limits match the shared wire contract", () => {
|
||||
const contract = JSON.parse(
|
||||
readFileSync(
|
||||
|
||||
@@ -130,6 +130,37 @@ class CommunityThemePreference {
|
||||
return findTheme(theme)?.isDark == true ? ThemeMode.dark : ThemeMode.light;
|
||||
}
|
||||
|
||||
/// Whether this preference carries a full desktop appearance opinion. A
|
||||
/// preference parsed from a legacy three-field payload, or a fresh/legacy
|
||||
/// mobile origin, omits these fields and must not replace a desktop record.
|
||||
bool get includesDesktopAppearance =>
|
||||
includesGlassBackground &&
|
||||
includesGlassOpacity &&
|
||||
includesProminentActiveTab;
|
||||
|
||||
/// Fold [source]'s desktop-only appearance into this preference. Used when
|
||||
/// republishing a mobile edit that carries no desktop opinion so the relay
|
||||
/// coordinate keeps the glass and prominent-tab values a desktop client
|
||||
/// authored. A preference that already includes the fields is returned
|
||||
/// unchanged; only the fields [source] actually carries are adopted.
|
||||
CommunityThemePreference mergeDesktopAppearanceFrom(
|
||||
CommunityThemePreference source,
|
||||
) {
|
||||
if (includesDesktopAppearance) return this;
|
||||
return CommunityThemePreference(
|
||||
version: version,
|
||||
theme: theme,
|
||||
accent: accent,
|
||||
followSystem: followSystem,
|
||||
glassBackground: source.glassBackground,
|
||||
glassOpacity: source.glassOpacity,
|
||||
prominentActiveTab: source.prominentActiveTab,
|
||||
includesGlassBackground: source.includesGlassBackground,
|
||||
includesGlassOpacity: source.includesGlassOpacity,
|
||||
includesProminentActiveTab: source.includesProminentActiveTab,
|
||||
);
|
||||
}
|
||||
|
||||
@override
|
||||
bool operator ==(Object other) =>
|
||||
other is CommunityThemePreference &&
|
||||
|
||||
@@ -60,6 +60,7 @@ class CommunityThemeSyncManager {
|
||||
int _publishRetryAttempt = 0;
|
||||
bool _publishInFlight = false;
|
||||
bool _publishRequestedWhileInFlight = false;
|
||||
bool _hydrationObserved = false;
|
||||
bool _disposed = false;
|
||||
|
||||
CommunityThemeSyncManager({
|
||||
@@ -124,6 +125,11 @@ class CommunityThemeSyncManager {
|
||||
CommunityThemeRemoteStatus.unavailable,
|
||||
);
|
||||
}
|
||||
if (result.status == CommunityThemeRemoteStatus.absent) {
|
||||
// A confirmed absence means the coordinate has been observed: there is no
|
||||
// desktop record to preserve, so a gated incomplete edit may publish.
|
||||
_observeHydration();
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -184,6 +190,10 @@ class CommunityThemeSyncManager {
|
||||
if (_disposed) return;
|
||||
if (result.status == CommunityThemeRemoteStatus.valid) {
|
||||
_accept(result.remote!);
|
||||
} else if (result.status == CommunityThemeRemoteStatus.absent) {
|
||||
// A confirmed absence here observes the coordinate too, releasing a gated
|
||||
// incomplete edit if the initial fetch had been unavailable.
|
||||
_observeHydration();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -204,6 +214,25 @@ class CommunityThemeSyncManager {
|
||||
_publishTimer = null;
|
||||
}
|
||||
|
||||
/// Record that the relay coordinate has been observed at least once (a valid
|
||||
/// record or a confirmed absence) and release an edit held by the
|
||||
/// pre-hydration gate in [flush]. Only an incomplete edit is ever gated, so
|
||||
/// a complete edit's own scheduled publish is left untouched.
|
||||
void _observeHydration() {
|
||||
if (_hydrationObserved) return;
|
||||
_hydrationObserved = true;
|
||||
final pending = _pending;
|
||||
if (!_disposed &&
|
||||
pending != null &&
|
||||
!pending.includesDesktopAppearance &&
|
||||
!_publishInFlight) {
|
||||
// Accelerate the held edit now that the coordinate is known. Any pending
|
||||
// debounce timer is cancelled by _schedulePublish. A publish in flight is
|
||||
// left to its own finally-block, which reschedules the pending edit.
|
||||
_schedulePublish(Duration.zero);
|
||||
}
|
||||
}
|
||||
|
||||
void publish(CommunityThemePreference preference) {
|
||||
stage(preference);
|
||||
publishStaged(preference);
|
||||
@@ -238,6 +267,13 @@ class CommunityThemeSyncManager {
|
||||
}
|
||||
final preference = _pending;
|
||||
if (_disposed || preference == null) return;
|
||||
// Hold an edit that carries no desktop appearance opinion until the relay
|
||||
// coordinate has been observed, so a pre-hydration edit cannot replace a
|
||||
// desktop-authored record before we have seen it. _observeHydration
|
||||
// releases it. A complete edit is never gated.
|
||||
if (!preference.includesDesktopAppearance && !_hydrationObserved) {
|
||||
return;
|
||||
}
|
||||
if (preference == _lastPublished) {
|
||||
_pending = null;
|
||||
onPublished(preference);
|
||||
@@ -245,7 +281,15 @@ class CommunityThemeSyncManager {
|
||||
}
|
||||
_publishInFlight = true;
|
||||
try {
|
||||
final content = crypto.encrypt(jsonEncode(preference.toJson()));
|
||||
// Preserve the desktop-only fields the relay already holds when the local
|
||||
// edit omits them, so publishing the replaceable coordinate does not strip
|
||||
// a desktop client's glass and prominent-tab choices. Bookkeeping stays on
|
||||
// the local `preference` so the outbox ack contract is unaffected.
|
||||
final remote = _lastRemote?.preference;
|
||||
final outgoing = remote == null
|
||||
? preference
|
||||
: preference.mergeDesktopAppearanceFrom(remote);
|
||||
final content = crypto.encrypt(jsonEncode(outgoing.toJson()));
|
||||
if (_disposed) return;
|
||||
final createdAt = max(
|
||||
DateTime.now().millisecondsSinceEpoch ~/ 1000,
|
||||
@@ -336,6 +380,7 @@ class CommunityThemeSyncManager {
|
||||
_lastCreatedAt = remote.createdAt;
|
||||
_lastEventId = remote.eventId;
|
||||
_lastRemote = remote;
|
||||
_observeHydration();
|
||||
if (_pending != null) {
|
||||
_lastPublished = null;
|
||||
return;
|
||||
|
||||
@@ -388,6 +388,93 @@ void main() {
|
||||
expect(manager.pending, isNull);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'gated incomplete edit holds until hydration then merges remote glass',
|
||||
() async {
|
||||
// A desktop client already published glass settings the mobile parser
|
||||
// omits. A mobile edit made before hydration must not strip them.
|
||||
const desktop = CommunityThemePreference(
|
||||
theme: 'buzz',
|
||||
accent: '#3b82f6',
|
||||
followSystem: true,
|
||||
glassBackground: true,
|
||||
glassOpacity: 80,
|
||||
prominentActiveTab: true,
|
||||
);
|
||||
const incompleteEdit = CommunityThemePreference(
|
||||
theme: 'dracula',
|
||||
accent: '#ef4444',
|
||||
followSystem: false,
|
||||
includesGlassBackground: false,
|
||||
includesGlassOpacity: false,
|
||||
includesProminentActiveTab: false,
|
||||
);
|
||||
final history = Completer<List<NostrEvent>>();
|
||||
final session = _FakeSession(historyFuture: history.future);
|
||||
final relay = _FakeSignedRelay();
|
||||
final manager = _manager(session, relay);
|
||||
|
||||
final initializing = manager.initialize();
|
||||
// Edit lands before the coordinate is observed: publishing is held.
|
||||
manager.publish(incompleteEdit);
|
||||
await manager.flush();
|
||||
expect(relay.submissions, isEmpty);
|
||||
expect(manager.pending, incompleteEdit);
|
||||
|
||||
// Hydration reveals the desktop record; the gate releases and the edit
|
||||
// publishes with the desktop-only fields merged back in.
|
||||
history.complete([
|
||||
_event(
|
||||
id: 'desktop',
|
||||
createdAt: 100,
|
||||
content: jsonEncode(desktop.toJson()),
|
||||
),
|
||||
]);
|
||||
await initializing;
|
||||
await _waitUntil(() => relay.submissions.isNotEmpty);
|
||||
|
||||
final published =
|
||||
jsonDecode(relay.submissions.single.content) as Map<String, dynamic>;
|
||||
expect(published['theme'], 'dracula');
|
||||
expect(published['glassBackground'], true);
|
||||
expect(published['glassOpacity'], 80);
|
||||
expect(published['prominentActiveTab'], true);
|
||||
},
|
||||
);
|
||||
|
||||
test('confirmed absence releases a gated incomplete edit', () async {
|
||||
const incompleteEdit = CommunityThemePreference(
|
||||
theme: 'dracula',
|
||||
accent: '#ef4444',
|
||||
followSystem: false,
|
||||
includesGlassBackground: false,
|
||||
includesGlassOpacity: false,
|
||||
includesProminentActiveTab: false,
|
||||
);
|
||||
final history = Completer<List<NostrEvent>>();
|
||||
final session = _FakeSession(historyFuture: history.future);
|
||||
final relay = _FakeSignedRelay();
|
||||
final manager = _manager(session, relay);
|
||||
|
||||
final initializing = manager.initialize();
|
||||
manager.publish(incompleteEdit);
|
||||
await manager.flush();
|
||||
expect(relay.submissions, isEmpty);
|
||||
|
||||
// No remote record exists; a confirmed absence is enough to release the
|
||||
// gate, and with no desktop values to preserve the edit publishes as-is.
|
||||
history.complete([]);
|
||||
await initializing;
|
||||
await _waitUntil(() => relay.submissions.isNotEmpty);
|
||||
|
||||
final published =
|
||||
jsonDecode(relay.submissions.single.content) as Map<String, dynamic>;
|
||||
expect(published['theme'], 'dracula');
|
||||
expect(published.containsKey('glassBackground'), isFalse);
|
||||
expect(published.containsKey('glassOpacity'), isFalse);
|
||||
expect(published.containsKey('prominentActiveTab'), isFalse);
|
||||
});
|
||||
}
|
||||
|
||||
CommunityThemeSyncManager _manager(
|
||||
|
||||
Reference in New Issue
Block a user