Refactor media sync handling

proto/anki/collection.proto

@@ -8,6 +8,7 @@ option java_multiple_files = true;
 package anki.collection;
 
 import "anki/generic.proto";
+import "anki/sync.proto";
 
 service CollectionService {
   rpc CheckDatabase(generic.Empty) returns (CheckDatabaseResponse);
@@ -100,12 +101,6 @@ message OpChangesAfterUndo {
 }
 
 message Progress {
-  message MediaSync {
-    string checked = 1;
-    string added = 2;
-    string removed = 3;
-  }
-
   message FullSync {
     uint32 transferred = 1;
     uint32 total = 2;
@@ -136,7 +131,7 @@ message Progress {
 
   oneof value {
     generic.Empty none = 1;
-    MediaSync media_sync = 2;
+    sync.MediaSyncProgress media_sync = 2;
     string media_check = 3;
     FullSync full_sync = 4;
     NormalSync normal_sync = 5;

proto/anki/sync.proto

@@ -9,17 +9,19 @@ package anki.sync;
 
 import "anki/generic.proto";
 
-/// Syncing methods are only available with a Backend handle.
+// Syncing methods are only available with a Backend handle.
 service SyncService {}
 
 service BackendSyncService {
   rpc SyncMedia(SyncAuth) returns (generic.Empty);
   rpc AbortMediaSync(generic.Empty) returns (generic.Empty);
+  // Can be used by the frontend to detect an active sync. If the sync aborted
+  // with an error, the next call to this method will return the error.
+  rpc MediaSyncStatus(generic.Empty) returns (MediaSyncStatusResponse);
   rpc SyncLogin(SyncLoginRequest) returns (SyncAuth);
   rpc SyncStatus(SyncAuth) returns (SyncStatusResponse);
-  rpc SyncCollection(SyncAuth) returns (SyncCollectionResponse);
-  rpc FullUpload(SyncAuth) returns (generic.Empty);
-  rpc FullDownload(SyncAuth) returns (generic.Empty);
+  rpc SyncCollection(SyncCollectionRequest) returns (SyncCollectionResponse);
+  rpc FullUploadOrDownload(FullUploadOrDownloadRequest) returns (generic.Empty);
   rpc AbortSync(generic.Empty) returns (generic.Empty);
 }
 
@@ -45,6 +47,11 @@ message SyncStatusResponse {
   optional string new_endpoint = 4;
 }
 
+message SyncCollectionRequest {
+  SyncAuth auth = 1;
+  bool sync_media = 2;
+}
+
 message SyncCollectionResponse {
   enum ChangesRequired {
     NO_CHANGES = 0;
@@ -60,4 +67,23 @@ message SyncCollectionResponse {
   string server_message = 2;
   ChangesRequired required = 3;
   optional string new_endpoint = 4;
+  int32 server_media_usn = 5;
+}
+
+message MediaSyncStatusResponse {
+  bool active = 1;
+  MediaSyncProgress progress = 2;
+}
+
+message MediaSyncProgress {
+  string checked = 1;
+  string added = 2;
+  string removed = 3;
+}
+
+message FullUploadOrDownloadRequest {
+  SyncAuth auth = 1;
+  bool upload = 2;
+  // if not provided, media syncing will be skipped
+  optional int32 server_usn = 3;
 }

pylib/anki/collection.py

@@ -16,6 +16,7 @@
     links_pb2,
     search_pb2,
     stats_pb2,
+    sync_pb2,
 )
 from anki._legacy import DeprecatedNamesMixin, deprecated
 from anki.sync_pb2 import SyncLoginRequest
@@ -49,6 +50,7 @@
 GetImageOcclusionNoteResponse = image_occlusion_pb2.GetImageOcclusionNoteResponse
 AddonInfo = ankiweb_pb2.AddonInfo
 CheckForUpdateResponse = ankiweb_pb2.CheckForUpdateResponse
+MediaSyncStatus = sync_pb2.MediaSyncStatusResponse
 
 import copy
 import os
@@ -1246,11 +1248,14 @@ def abort_media_sync(self) -> None:
     def abort_sync(self) -> None:
         self._backend.abort_sync()
 
-    def full_upload(self, auth: SyncAuth) -> None:
-        self._backend.full_upload(auth)
-
-    def full_download(self, auth: SyncAuth) -> None:
-        self._backend.full_download(auth)
+    def full_upload_or_download(
+        self, *, auth: SyncAuth, server_usn: int | None, upload: bool
+    ) -> None:
+        self._backend.full_upload_or_download(
+            sync_pb2.FullUploadOrDownloadRequest(
+                auth=auth, server_usn=server_usn, upload=upload
+            )
+        )
 
     def sync_login(
         self, username: str, password: str, endpoint: str | None
@@ -1259,15 +1264,19 @@ def sync_login(
             SyncLoginRequest(username=username, password=password, endpoint=endpoint)
         )
 
-    def sync_collection(self, auth: SyncAuth) -> SyncOutput:
-        return self._backend.sync_collection(auth)
+    def sync_collection(self, auth: SyncAuth, sync_media: bool) -> SyncOutput:
+        return self._backend.sync_collection(auth=auth, sync_media=sync_media)
 
     def sync_media(self, auth: SyncAuth) -> None:
         self._backend.sync_media(auth)
 
     def sync_status(self, auth: SyncAuth) -> SyncStatus:
         return self._backend.sync_status(auth)
 
+    def media_sync_status(self) -> MediaSyncStatus:
+        "This will throw if the sync failed with an error."
+        return self._backend.media_sync_status()
+
     def get_preferences(self) -> Preferences:
         return self._backend.get_preferences()
 

qt/aqt/forms/synclog.ui

@@ -6,21 +6,24 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>557</width>
-    <height>295</height>
+    <width>482</width>
+    <height>90</height>
    </rect>
   </property>
   <property name="windowTitle">
    <string/>
   </property>
   <layout class="QVBoxLayout" name="verticalLayout">
    <item>
-    <widget class="QPlainTextEdit" name="plainTextEdit">
-     <property name="readOnly">
-      <bool>true</bool>
+    <widget class="QLabel" name="log_label">
+     <property name="text">
+      <string notr="true">TextLabel</string>
      </property>
-     <property name="plainText">
-      <string notr="true"/>
+     <property name="textFormat">
+      <enum>Qt::PlainText</enum>
+     </property>
+     <property name="alignment">
+      <set>Qt::AlignCenter</set>
      </property>
     </widget>
    </item>

qt/aqt/main.py

@@ -1019,9 +1019,6 @@ def _refresh_after_sync(self) -> None:
 
     def _sync_collection_and_media(self, after_sync: Callable[[], None]) -> None:
         "Caller should ensure auth available."
-        # start media sync if not already running
-        if not self.media_syncer.is_syncing():
-            self.media_syncer.start()
 
         def on_collection_sync_finished() -> None:
             self.col.clear_python_undo()

qt/aqt/mediasync.py

@@ -5,110 +5,93 @@
 
 import time
 from concurrent.futures import Future
-from dataclasses import dataclass
-from typing import Any, Callable, Union
+from datetime import datetime
+from typing import Any, Callable
 
 import aqt
 import aqt.forms
 import aqt.main
-from anki.collection import Progress
+from anki.collection import Collection
 from anki.errors import Interrupted
-from anki.types import assert_exhaustive
 from anki.utils import int_time
 from aqt import gui_hooks
-from aqt.qt import QDialog, QDialogButtonBox, QPushButton, QTextCursor, QTimer, qconnect
-from aqt.utils import disable_help_button, tr
-
-LogEntry = Union[Progress.MediaSync, str]
-
-
-@dataclass
-class LogEntryWithTime:
-    time: int
-    entry: LogEntry
+from aqt.operations import QueryOp
+from aqt.qt import QDialog, QDialogButtonBox, QPushButton, Qt, QTimer, qconnect
+from aqt.utils import disable_help_button, show_info, tr
 
 
 class MediaSyncer:
     def __init__(self, mw: aqt.main.AnkiQt) -> None:
         self.mw = mw
         self._syncing: bool = False
-        self._log: list[LogEntryWithTime] = []
-        self._progress_timer: QTimer | None = None
+        self.last_progress = ""
+        self._last_progress_at = 0
         gui_hooks.media_sync_did_start_or_stop.append(self._on_start_stop)
 
-    def _on_progress(self) -> None:
-        progress = self.mw.col.latest_progress()
-        if not progress.HasField("media_sync"):
-            return
-        sync_progress = progress.media_sync
-        self._log_and_notify(sync_progress)
-
     def start(self) -> None:
         "Start media syncing in the background, if it's not already running."
-        if self._syncing:
+        if not self.mw.pm.media_syncing_enabled() or not (
+            auth := self.mw.pm.sync_auth()
+        ):
             return
 
-        if not self.mw.pm.media_syncing_enabled():
-            self._log_and_notify(tr.sync_media_disabled())
-            return
+        def run(col: Collection) -> None:
+            col.sync_media(auth)
 
-        auth = self.mw.pm.sync_auth()
-        if auth is None:
-            return
+        # this will exit after the thread is spawned, but may block if there's an existing
+        # backend lock
+        QueryOp(parent=aqt.mw, op=run, success=lambda _: 1).run_in_background()
+
+        self.start_monitoring()
 
-        self._log_and_notify(tr.sync_media_starting())
+    def start_monitoring(self) -> None:
+        if self._syncing:
+            return
         self._syncing = True
-        self._progress_timer = self.mw.progress.timer(
-            1000, self._on_progress, True, True, parent=self.mw
-        )
         gui_hooks.media_sync_did_start_or_stop(True)
+        self._update_progress(tr.sync_media_starting())
 
-        def run() -> None:
-            self.mw.col.sync_media(auth)
+        def monitor() -> None:
+            while True:
+                resp = self.mw.col.media_sync_status()
+                if not resp.active:
+                    return
+                if p := resp.progress:
+                    self._update_progress(f"{p.added}, {p.removed}, {p.checked}")
 
-        self.mw.taskman.run_in_background(run, self._on_finished)
+                time.sleep(0.25)
 
-    def _log_and_notify(self, entry: LogEntry) -> None:
-        entry_with_time = LogEntryWithTime(time=int_time(), entry=entry)
-        self._log.append(entry_with_time)
-        self.mw.taskman.run_on_main(
-            lambda: gui_hooks.media_sync_did_progress(entry_with_time)
-        )
+        self.mw.taskman.run_in_background(monitor, self._on_finished)
+
+    def _update_progress(self, progress: str) -> None:
+        self.last_progress = progress
+        self.mw.taskman.run_on_main(lambda: gui_hooks.media_sync_did_progress(progress))
 
     def _on_finished(self, future: Future) -> None:
         self._syncing = False
-        if self._progress_timer:
-            self._progress_timer.stop()
-            self._progress_timer.deleteLater()
-            self._progress_timer = None
+        self._last_progress_at = int_time()
         gui_hooks.media_sync_did_start_or_stop(False)
 
         exc = future.exception()
         if exc is not None:
             self._handle_sync_error(exc)
         else:
-            self._log_and_notify(tr.sync_media_complete())
+            self._update_progress(tr.sync_media_complete())
 
     def _handle_sync_error(self, exc: BaseException) -> None:
         if isinstance(exc, Interrupted):
-            self._log_and_notify(tr.sync_media_aborted())
+            self._update_progress(tr.sync_media_aborted())
             return
         else:
-            # Avoid popups for errors; they can cause a deadlock if
-            # a modal window happens to be active, or a duplicate auth
-            # failed message if the password is changed.
-            self._log_and_notify(str(exc))
+            show_info(str(exc), modality=Qt.WindowModality.NonModal)
             return
 
-    def entries(self) -> list[LogEntryWithTime]:
-        return self._log
-
     def abort(self) -> None:
         if not self.is_syncing():
             return
-        self._log_and_notify(tr.sync_media_aborting())
         self.mw.col.set_wants_abort()
         self.mw.col.abort_media_sync()
+        self._update_progress(tr.sync_media_aborting())
 
     def is_syncing(self) -> bool:
         return self._syncing
@@ -140,11 +123,7 @@ def seconds_since_last_sync(self) -> int:
         if self.is_syncing():
             return 0
 
-        if self._log:
-            last = self._log[-1].time
-        else:
-            last = 0
-        return int_time() - last
+        return int_time() - self._last_progress_at
 
 
 class MediaSyncDialog(QDialog):
@@ -172,10 +151,7 @@ def __init__(
         gui_hooks.media_sync_did_progress.append(self._on_log_entry)
         gui_hooks.media_sync_did_start_or_stop.append(self._on_start_stop)
 
-        self.form.plainTextEdit.setPlainText(
-            "\n".join(self._entry_to_text(x) for x in syncer.entries())
-        )
-        self.form.plainTextEdit.moveCursor(QTextCursor.MoveOperation.End)
+        self._on_log_entry(syncer.last_progress)
         self.show()
 
     def reject(self) -> None:
@@ -197,24 +173,11 @@ def _on_abort(self, *_args: Any) -> None:
         self._syncer.abort()
         self.abort_button.setHidden(True)
 
-    def _time_and_text(self, stamp: int, text: str) -> str:
-        asctime = time.asctime(time.localtime(stamp))
-        return f"{asctime}: {text}"
-
-    def _entry_to_text(self, entry: LogEntryWithTime) -> str:
-        if isinstance(entry.entry, str):
-            txt = entry.entry
-        elif isinstance(entry.entry, Progress.MediaSync):
-            txt = self._logentry_to_text(entry.entry)
-        else:
-            assert_exhaustive(entry.entry)
-        return self._time_and_text(entry.time, txt)
-
-    def _logentry_to_text(self, e: Progress.MediaSync) -> str:
-        return f"{e.added}, {e.removed}, {e.checked}"
-
-    def _on_log_entry(self, entry: LogEntryWithTime) -> None:
-        self.form.plainTextEdit.appendPlainText(self._entry_to_text(entry))
+    def _on_log_entry(self, entry: str) -> None:
+        dt = datetime.fromtimestamp(int_time())
+        time = dt.strftime("%H:%M:%S")
+        text = f"{time}: {entry}"
+        self.form.log_label.setText(text)
         if not self._syncer.is_syncing():
             self.abort_button.setHidden(True)