Skip to content

Commit 2b88ac9

Browse files
committed
fix(sync): finish cloud audio deletion and playback guards
1 parent f8babb7 commit 2b88ac9

6 files changed

Lines changed: 123 additions & 22 deletions

File tree

app/lib/pages/conversations/private_cloud_sync_page.dart

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
2727
bool _isDeleting = false;
2828
bool _isAudioLoading = false;
2929
List<CloudAudioConversation> _conversations = [];
30+
bool? _lastPrivateCloudSyncEnabled;
31+
int _playbackGeneration = 0;
3032

3133
final AudioPlayer _audioPlayer = AudioPlayer();
3234
StreamSubscription<PlayerState>? _playerStateSubscription;
@@ -50,9 +52,40 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
5052
setState(() {});
5153
}
5254
});
53-
if (context.read<UserProvider>().privateCloudSyncEnabled) {
54-
_loadCloudAudioConversations();
55+
}
56+
57+
@override
58+
void didChangeDependencies() {
59+
super.didChangeDependencies();
60+
61+
final isEnabled = context.watch<UserProvider>().privateCloudSyncEnabled;
62+
if (_lastPrivateCloudSyncEnabled == isEnabled) return;
63+
_lastPrivateCloudSyncEnabled = isEnabled;
64+
65+
if (isEnabled) {
66+
unawaited(_loadCloudAudioConversations());
67+
return;
5568
}
69+
70+
_cancelPendingPlayback(clearConversations: true);
71+
}
72+
73+
void _cancelPendingPlayback({bool clearConversations = false}) {
74+
_playbackGeneration++;
75+
unawaited(_audioPlayer.stop());
76+
_currentPlayingConversationId = null;
77+
_isAudioLoading = false;
78+
if (clearConversations) {
79+
_conversations = [];
80+
}
81+
}
82+
83+
bool _shouldAbortPlaybackStart(String conversationId, int playbackGeneration) {
84+
return !mounted ||
85+
_isDeleting ||
86+
!context.read<UserProvider>().privateCloudSyncEnabled ||
87+
_currentPlayingConversationId != conversationId ||
88+
_playbackGeneration != playbackGeneration;
5689
}
5790

5891
@override
@@ -98,9 +131,6 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
98131
backgroundColor: Colors.green,
99132
),
100133
);
101-
if (value) {
102-
await _loadCloudAudioConversations();
103-
}
104134
} catch (e) {
105135
Logger.debug('Error toggling cloud storage: $e');
106136
if (!mounted) return;
@@ -127,6 +157,7 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
127157
}
128158

129159
await _audioPlayer.stop();
160+
final playbackGeneration = ++_playbackGeneration;
130161
if (!mounted) return;
131162
setState(() {
132163
_currentPlayingConversationId = conversation.id;
@@ -135,17 +166,19 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
135166

136167
try {
137168
final audioFileInfos = await getConversationAudioSignedUrls(conversation.id);
169+
if (_shouldAbortPlaybackStart(conversation.id, playbackGeneration)) return;
138170
if (audioFileInfos.isEmpty) {
139171
throw Exception('No audio files available to play');
140172
}
141173

142174
if (audioFileInfos.any((af) => !af.isCached)) {
143175
await precacheConversationAudio(conversation.id);
144-
if (!mounted) return;
176+
if (_shouldAbortPlaybackStart(conversation.id, playbackGeneration)) return;
145177
setState(() {
146178
_currentPlayingConversationId = null;
147179
_isAudioLoading = false;
148180
});
181+
if (!mounted) return;
149182
ScaffoldMessenger.of(context).showSnackBar(
150183
SnackBar(content: Text(context.l10n.preparingCloudAudioTryAgain), backgroundColor: Colors.orange),
151184
);
@@ -165,9 +198,15 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
165198
);
166199

167200
await _audioPlayer.setAudioSource(playlist, preload: true);
168-
if (!mounted) return;
201+
if (_shouldAbortPlaybackStart(conversation.id, playbackGeneration)) {
202+
await _audioPlayer.stop();
203+
return;
204+
}
169205
setState(() => _isAudioLoading = false);
170206
await _audioPlayer.play();
207+
if (_shouldAbortPlaybackStart(conversation.id, playbackGeneration)) {
208+
await _audioPlayer.stop();
209+
}
171210
} catch (e) {
172211
Logger.debug('Error playing conversation audio: $e');
173212
if (!mounted) return;
@@ -227,12 +266,7 @@ class _PrivateCloudSyncPageState extends State<PrivateCloudSyncPage> {
227266
if (confirmed != true) return;
228267

229268
setState(() => _isDeleting = true);
230-
await _audioPlayer.stop();
231-
if (mounted) {
232-
setState(() {
233-
_currentPlayingConversationId = null;
234-
});
235-
}
269+
_cancelPendingPlayback();
236270

237271
try {
238272
final success = await deleteAllCloudAudio();

app/lib/services/audio_download_service.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ class AudioDownloadService {
201201
.replaceAll(RegExp(r'[^a-zA-Z0-9]+'), '_')
202202
.replaceAll(RegExp(r'_+'), '_')
203203
.replaceAll(RegExp(r'^_|_$'), '');
204-
final prefix = sanitizedTitle.isEmpty ? 'omi' : sanitizedTitle.toLowerCase();
204+
final boundedTitle = sanitizedTitle.length > 80 ? sanitizedTitle.substring(0, 80) : sanitizedTitle;
205+
final prefix = boundedTitle.isEmpty ? 'omi' : boundedTitle.toLowerCase();
205206
return '${prefix}_$timestamp.wav';
206207
}
207208

backend/database/conversations.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,27 @@ def iter_all_conversations(uid: str, batch_size: int = 400, include_discarded: b
288288
offset += batch_size
289289

290290

291+
def iter_audio_metadata_conversations(uid: str, batch_size: int = 400, include_discarded: bool = False):
292+
"""Yield lightweight conversation metadata without hydrating transcript payloads."""
293+
conversations_ref = db.collection('users').document(uid).collection(conversations_collection)
294+
if not include_discarded:
295+
conversations_ref = conversations_ref.where(filter=FieldFilter('discarded', '==', False))
296+
conversations_ref = conversations_ref.select(['id', 'created_at', 'audio_files', 'structured.title', 'is_locked'])
297+
conversations_ref = conversations_ref.order_by('created_at', direction=firestore.Query.DESCENDING)
298+
offset = 0
299+
while True:
300+
batch_ref = conversations_ref.limit(batch_size).offset(offset)
301+
batch = []
302+
for doc in batch_ref.stream():
303+
conv = doc.to_dict() or {}
304+
conv.setdefault('id', doc.id)
305+
batch.append(conv)
306+
yield from batch
307+
if len(batch) < batch_size:
308+
break
309+
offset += batch_size
310+
311+
291312
def update_conversation(uid: str, conversation_id: str, update_data: dict):
292313
doc_ref = db.collection('users').document(uid).collection(conversations_collection).document(conversation_id)
293314
doc_snapshot = doc_ref.get()

backend/routers/sync.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def list_conversations_with_audio(
176176
Returns lightweight conversation info (id, title, date, audio file count, total duration).
177177
"""
178178
result = []
179-
for conv in conversations_db.iter_all_conversations(uid, include_discarded=False):
179+
for conv in conversations_db.iter_audio_metadata_conversations(uid, include_discarded=False):
180180
audio_files = conv.get('audio_files') or []
181181
if not audio_files or conv.get('is_locked', False):
182182
continue
@@ -220,7 +220,24 @@ def delete_all_cloud_audio(
220220
logger.error(f"Failed to clear audio_files for conversation {conversation_id}: {e}")
221221
raise HTTPException(status_code=503, detail="Failed to clear conversation audio metadata.") from e
222222

223-
deleted_count = delete_all_user_cloud_audio(uid)
223+
delete_result = delete_all_user_cloud_audio(uid)
224+
deleted_count = delete_result['deleted_blobs']
225+
failed_count = delete_result['failed_blobs']
226+
227+
if failed_count:
228+
logger.error(
229+
f"Failed to delete {failed_count} cloud audio blobs after clearing {cleared_conversations} "
230+
f"conversations for user {uid}"
231+
)
232+
raise HTTPException(
233+
status_code=503,
234+
detail={
235+
'message': 'Failed to delete one or more cloud audio blobs.',
236+
'deleted_blobs': deleted_count,
237+
'failed_blobs': failed_count,
238+
'cleared_conversations': cleared_conversations,
239+
},
240+
)
224241

225242
logger.info(
226243
f"Deleted {deleted_count} cloud audio blobs and cleared {cleared_conversations} conversations for user {uid}"

backend/tests/unit/test_private_cloud_sync_audio_management.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def test_list_conversations_with_audio_iterates_all_batches(monkeypatch):
2121
conversations.extend(_conversation(i, audio_files=[{'duration': 1.5}, {'duration': 2.5}]) for i in range(2, 505))
2222

2323
iter_mock = MagicMock(return_value=iter(conversations))
24-
monkeypatch.setattr(sync.conversations_db, 'iter_all_conversations', iter_mock)
24+
monkeypatch.setattr(sync.conversations_db, 'iter_audio_metadata_conversations', iter_mock)
2525

2626
result = sync.list_conversations_with_audio(uid='user-1')
2727

@@ -39,7 +39,9 @@ def test_list_conversations_with_audio_skips_locked_conversations(monkeypatch):
3939
{**_conversation(2, audio_files=[{'duration': 2.0}], discarded=False), 'is_locked': True},
4040
]
4141

42-
monkeypatch.setattr(sync.conversations_db, 'iter_all_conversations', MagicMock(return_value=iter(conversations)))
42+
monkeypatch.setattr(
43+
sync.conversations_db, 'iter_audio_metadata_conversations', MagicMock(return_value=iter(conversations))
44+
)
4345

4446
result = sync.list_conversations_with_audio(uid='user-1')
4547

@@ -52,7 +54,7 @@ def test_delete_all_cloud_audio_clears_every_matching_conversation(monkeypatch):
5254

5355
iter_mock = MagicMock(return_value=iter(conversations))
5456
update_mock = MagicMock()
55-
delete_mock = MagicMock(return_value=77)
57+
delete_mock = MagicMock(return_value={'deleted_blobs': 77, 'failed_blobs': 0})
5658

5759
monkeypatch.setattr(sync.conversations_db, 'iter_all_conversations', iter_mock)
5860
monkeypatch.setattr(sync.conversations_db, 'update_conversation', update_mock)
@@ -83,3 +85,26 @@ def test_delete_all_cloud_audio_stops_before_blob_delete_when_metadata_clear_fai
8385

8486
assert exc_info.value.status_code == 503
8587
delete_mock.assert_not_called()
88+
89+
90+
def test_delete_all_cloud_audio_returns_error_when_blob_delete_partially_fails(monkeypatch):
91+
conversations = [_conversation(1, audio_files=[{'duration': 1.0}])]
92+
93+
monkeypatch.setattr(sync.conversations_db, 'iter_all_conversations', MagicMock(return_value=iter(conversations)))
94+
monkeypatch.setattr(sync.conversations_db, 'update_conversation', MagicMock())
95+
monkeypatch.setattr(
96+
sync,
97+
'delete_all_user_cloud_audio',
98+
MagicMock(return_value={'deleted_blobs': 4, 'failed_blobs': 1}),
99+
)
100+
101+
with pytest.raises(sync.HTTPException) as exc_info:
102+
sync.delete_all_cloud_audio(uid='user-1')
103+
104+
assert exc_info.value.status_code == 503
105+
assert exc_info.value.detail == {
106+
'message': 'Failed to delete one or more cloud audio blobs.',
107+
'deleted_blobs': 4,
108+
'failed_blobs': 1,
109+
'cleared_conversations': 1,
110+
}

backend/utils/other/storage.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -649,11 +649,11 @@ def delete_conversation_audio_files(uid: str, conversation_id: str) -> None:
649649
blob.delete()
650650

651651

652-
def delete_all_user_cloud_audio(uid: str) -> int:
652+
def delete_all_user_cloud_audio(uid: str) -> dict[str, int]:
653653
"""Delete all cloud-synced audio blobs for a user.
654654
655655
Removes raw chunks, per-conversation merged audio, and cached merged assets.
656-
Returns the total number of deleted blobs.
656+
Returns the total number of deleted and failed blobs.
657657
"""
658658
bucket = storage_client.bucket(private_cloud_sync_bucket)
659659
deleted = 0
@@ -671,7 +671,10 @@ def delete_all_user_cloud_audio(uid: str) -> int:
671671
if failed:
672672
logger.warning(f'Failed to delete {failed} cloud audio blobs for user {uid}')
673673

674-
return deleted
674+
return {
675+
'deleted_blobs': deleted,
676+
'failed_blobs': failed,
677+
}
675678

676679

677680
def download_audio_chunks_and_merge(

0 commit comments

Comments
 (0)