Refine subscription update notifications

Signed-off-by: Loren Eteval <loren.eteval@proton.me>

Furious/Service/SubscriptionManager.py

@@ -102,6 +102,11 @@ class SubscriptionUpdateBatch:
 class SubscriptionManager(HttpGetManager):
     """Own subscription networking, decoding, reconciliation, and persistence."""
 
+    # Presentation metadata changed for these stable subscription IDs. This
+    # deliberately does not imply that profile topology changed.
+    subscriptionStateChanged = QtCore.Signal(object)
+
+    # Subscription groups or their derived profile topology changed.
     subscriptionsChanged = QtCore.Signal()
     subscriptionCommitted = QtCore.Signal(str)
     updateCompleted = QtCore.Signal(object)
@@ -478,7 +483,20 @@ class SubscriptionManager(HttpGetManager):
         if not committedSuccess and not committedFailure:
             return
 
-        self.subscriptionsChanged.emit()
+        changedSubscriptions = tuple(
+            dict.fromkeys(
+                param.get('unique', '')
+                for param in (*committedSuccess, *committedFailure)
+                if param.get('unique')
+            )
+        )
+
+        if changedSubscriptions:
+            self.subscriptionStateChanged.emit(changedSubscriptions)
+
+        if committedSuccess:
+            self.subscriptionsChanged.emit()
+
         self.updateCompleted.emit(
             SubscriptionUpdateBatch(
                 tuple(committedSuccess),
@@ -597,67 +615,57 @@ class SubscriptionManager(HttpGetManager):
             forwardSender=True,
         )
 
-    def updateSubsByUnique(self, unique: str, **kwargs):
-        """Update one enabled subscription group by stable ID."""
-        subscription = Storage.UserSubs().get(unique)
+    def updateSubscriptions(self, uniques, **kwargs):
+        """Start eligible stable IDs as one status and completion batch."""
+        subscriptions = Storage.UserSubs()
 
-        if (
-            not subscription
-            or not subscription.get('enabled', True)
-            or not subscription.get('webURL')
-        ):
+        batch = tuple(
+            (unique, subscriptions[unique])
+            for unique in dict.fromkeys(uniques)
+            if unique in subscriptions
+            and subscriptions[unique].get('enabled', True)
+            and subscriptions[unique].get('webURL')
+        )
+
+        if not batch:
             return
 
-        group = Storage.SubscriptionGroup(unique)
+        changedSubscriptions = []
+
+        for unique, _subscription in batch:
+            group = Storage.SubscriptionGroup(unique)
+
+            if group is None:
+                continue
 
-        if group is not None:
             group.lastSyncStatus = 'syncing'
             group.lastSyncError = ''
 
             Storage.upsertSubscriptionGroup(group)
+            changedSubscriptions.append(unique)
 
-        depthMap = kwargs.get('depthMap')
-        successArgs = kwargs.get('successArgs')
-        failureArgs = kwargs.get('failureArgs')
+        if changedSubscriptions:
+            self.subscriptionStateChanged.emit(tuple(changedSubscriptions))
 
-        if depthMap is None:
-            depthMap = {'depth': 1}
+        depthMap = {'depth': len(batch)}
+        successArgs = []
+        failureArgs = []
 
-        if successArgs is None:
-            successArgs = list()
-
-        if failureArgs is None:
-            failureArgs = list()
-
-        kwargs.update(
-            depthMap=depthMap,
-            successArgs=successArgs,
-            failureArgs=failureArgs,
-            requestVersion=self._nextRequestVersion(unique),
-        )
-
-        self.updateSubsByWebGET(unique=unique, **subscription, **kwargs)
-
-    def updateSubs(self, **kwargs):
-        """Update every enabled subscription as one completion batch."""
-        enabledKeys = tuple(
-            key
-            for key, subscription in Storage.UserSubs().items()
-            if subscription.get('enabled', True) and subscription.get('webURL')
-        )
-
-        if not enabledKeys:
-            return
-
-        depthMap = {'depth': len(enabledKeys)}
-        successArgs = list()
-        failureArgs = list()
-
-        for key in enabledKeys:
-            self.updateSubsByUnique(
-                key,
+        for unique, subscription in batch:
+            self.updateSubsByWebGET(
+                unique=unique,
+                **subscription,
+                **kwargs,
                 depthMap=depthMap,
                 successArgs=successArgs,
                 failureArgs=failureArgs,
-                **kwargs,
+                requestVersion=self._nextRequestVersion(unique),
             )
+
+    def updateSubsByUnique(self, unique: str, **kwargs):
+        """Update one enabled subscription through the canonical batch path."""
+        self.updateSubscriptions((unique,), **kwargs)
+
+    def updateSubs(self, **kwargs):
+        """Update every eligible subscription through the canonical batch path."""
+        self.updateSubscriptions(tuple(Storage.UserSubs()), **kwargs)

Furious/Widget/ServerTableView.py

@@ -1922,19 +1922,19 @@ class ServerTableView(
 
     def updateSubsByUnique(self, unique: str, httpProxy: Union[str, None], **kwargs):
         """Update subs by unique."""
+        self.updateSubscriptions((unique,), httpProxy, **kwargs)
+
+    def updateSubscriptions(self, uniques, httpProxy: Union[str, None], **kwargs):
+        """Update stable subscription IDs as one manager-owned batch."""
         kwargs.pop('parent', None)
 
         self.subsManager.configureHttpProxy(httpProxy)
-        self.subsManager.updateSubsByUnique(unique, **kwargs)
+        self.subsManager.updateSubscriptions(uniques, **kwargs)
 
     def updateSubs(self, httpProxy: Union[str, None], **kwargs):
         """Update subs."""
         self.selectionModel().clearSelection()
-
-        kwargs.pop('parent', None)
-
-        self.subsManager.configureHttpProxy(httpProxy)
-        self.subsManager.updateSubs(**kwargs)
+        self.updateSubscriptions(tuple(Storage.UserSubs()), httpProxy, **kwargs)
 
     @QtCore.Slot()
     def _handleSubscriptionsChanged(self):

Furious/Widget/SubscriptionTableView.py

@@ -718,6 +718,28 @@ class SubscriptionTableView(Mixins.QTranslatable, AppQTableView):
         self.selectRow(target)
         self.groupsChanged.emit()
 
+    @QtCore.Slot(object)
+    def refreshSubscriptionState(self, uniques):
+        """Repaint metadata cells for stable subscription IDs only."""
+        rows = {unique: row for row, unique in enumerate(Storage.UserSubs())}
+
+        firstColumn, lastColumn = (
+            self.ItemKey.index('lastSyncStatus'),
+            self.ItemKey.index('profiles'),
+        )
+
+        for unique in dict.fromkeys(uniques):
+            row = rows.get(unique)
+
+            if row is None:
+                continue
+
+            self.sourceModel.dataChanged.emit(
+                self.sourceModel.index(row, firstColumn),
+                self.sourceModel.index(row, lastColumn),
+                [],
+            )
+
     def flushItem(self, row, column, item):
         """Refresh item."""
         if row < 0 or row >= self.sourceModel.rowCount():

Furious/Window/SubscriptionPage.py

@@ -290,7 +290,11 @@ class SubscriptionPage(Mixins.QTranslatable, Mixins.ThemeAware, QMainWindow):
             self.serverTable.subsManager.subscriptionsChanged.emit
         )
 
-        self.serverTable.subsManager.subscriptionsChanged.connect(self.table.flushAll)
+        # The manager and table are persistent main-window children. This direct
+        # connection intentionally shares their process-lifetime ownership.
+        self.serverTable.subsManager.subscriptionStateChanged.connect(
+            self.table.refreshSubscriptionState
+        )
 
         self.addButton.clicked.connect(self.addSubscription)
         self.editButton.clicked.connect(self.editSelected)
@@ -508,21 +512,12 @@ class SubscriptionPage(Mixins.QTranslatable, Mixins.ThemeAware, QMainWindow):
         if not keys:
             return
 
-        depthMap = {'depth': len(keys)}
-        successArgs = []
-        failureArgs = []
-        httpProxy = self._selectedProxy()
-
-        for key in keys:
-            self.serverTable.updateSubsByUnique(
-                key,
-                httpProxy,
-                depthMap=depthMap,
-                successArgs=successArgs,
-                failureArgs=failureArgs,
-                showMessageBox=True,
-                parent=self,
-            )
+        self.serverTable.updateSubscriptions(
+            keys,
+            self._selectedProxy(),
+            showMessageBox=True,
+            parent=self,
+        )
 
     @QtCore.Slot()
     def updateAll(self):

tests/test_subscription_manager.py

@@ -340,8 +340,14 @@ class SubscriptionManagerTest(TestCase):
         )
         completed = []
         committedSubscriptions = []
+        stateChanges = []
+        structuralChanges = []
         manager.updateCompleted.connect(completed.append)
         manager.subscriptionCommitted.connect(committedSubscriptions.append)
+        manager.subscriptionStateChanged.connect(
+            lambda uniques: stateChanges.append(tuple(uniques))
+        )
+        manager.subscriptionsChanged.connect(lambda: structuralChanges.append(True))
         failed = {'unique': 'group-a', 'profiles': ()}
         successful = {'unique': 'group-b', 'profiles': ()}
 
@@ -360,6 +366,37 @@ class SubscriptionManagerTest(TestCase):
         self.assertEqual(completed[0].failed[0]['unique'], 'group-a')
         self.assertIn('injected failure', completed[0].failed[0]['error'])
         self.assertEqual(committedSubscriptions, ['group-b'])
+        self.assertEqual(stateChanges, [('group-b', 'group-a')])
+        self.assertEqual(structuralChanges, [True])
+
+        manager.deleteLater()
+
+    def testFailedBatchPublishesStateWithoutStructuralChange(self):
+        """Present terminal failure metadata without resetting profile consumers."""
+        manager = self._manager()
+        stateChanges = []
+        structuralChanges = []
+        completed = []
+
+        manager.subscriptionStateChanged.connect(
+            lambda uniques: stateChanges.append(tuple(uniques))
+        )
+        manager.subscriptionsChanged.connect(lambda: structuralChanges.append(True))
+        manager.updateCompleted.connect(completed.append)
+
+        with mock.patch(
+            'Furious.Service.SubscriptionManager.Storage.SubscriptionGroup',
+            return_value=None,
+        ):
+            manager.handleSynchronizationResults(
+                successArgs=[],
+                failureArgs=[{'unique': 'group-a', 'error': 'offline'}],
+            )
+
+        self.assertEqual(stateChanges, [('group-a',)])
+        self.assertEqual(structuralChanges, [])
+        self.assertEqual(len(completed), 1)
+        self.assertEqual(completed[0].failed[0]['error'], 'offline')
 
         manager.deleteLater()
 
@@ -660,6 +697,142 @@ class SubscriptionManagerTest(TestCase):
         self.assertEqual(gettext('Updated', 'ZH'), '已更新')
         self.assertEqual(gettext('Update Failed', 'ZH'), '更新失败')
 
+    def testSubscriptionStateNotificationRepaintsOnlyAffectedMetadataRow(self):
+        """Resolve stable IDs at delivery and avoid a whole-table refresh."""
+        subscriptions = {
+            'group-a': self._subscription(),
+            'group-b': self._subscription(
+                remark='Group B',
+                webURL='https://invalid.test/b',
+            ),
+            'group-c': self._subscription(
+                remark='Group C',
+                webURL='https://invalid.test/c',
+            ),
+        }
+
+        with mock.patch.object(Storage, 'UserSubs', return_value=subscriptions):
+            table = SubscriptionTableView()
+            changes = []
+
+            def changed(topLeft, bottomRight, _roles):
+                """Capture one exact model repaint range."""
+                changes.append(
+                    (
+                        (topLeft.row(), topLeft.column()),
+                        (bottomRight.row(), bottomRight.column()),
+                    )
+                )
+
+            table.sourceModel.dataChanged.connect(changed)
+
+            with mock.patch.object(table, 'flushAll') as flushAll:
+                table.refreshSubscriptionState(('group-b',))
+
+            self.assertEqual(
+                changes,
+                [
+                    (
+                        (1, table.ItemKey.index('lastSyncStatus')),
+                        (1, table.ItemKey.index('profiles')),
+                    )
+                ],
+            )
+            flushAll.assert_not_called()
+            table.deleteLater()
+
+    def testUpdateAllPublishesOneImmediateSyncingSnapshot(self):
+        """Expose one metadata batch without publishing structural changes."""
+        subscriptions = {
+            'group-a': self._subscription(),
+            'group-b': self._subscription(
+                remark='Group B',
+                webURL='https://invalid.test/b',
+            ),
+        }
+        manager = self._manager(subscriptions)
+        snapshots = []
+        stateChanges = []
+        structuralChanges = []
+
+        def group(unique):
+            """Return one isolated persisted group."""
+            value = subscriptions.get(unique)
+
+            return (
+                SubscriptionGroup.fromMapping(unique, value)
+                if value is not None
+                else None
+            )
+
+        def upsert(value):
+            """Persist one status transition into the isolated repository."""
+            subscriptions[value.id] = value.toMapping()
+
+        def stateChanged(uniques):
+            """Capture the stable IDs and their state at notification time."""
+            stateChanges.append(tuple(uniques))
+            snapshots.append(
+                tuple(
+                    subscriptions[unique].get('lastSyncStatus', '')
+                    for unique in ('group-a', 'group-b')
+                )
+            )
+
+        manager.subscriptionStateChanged.connect(stateChanged)
+        manager.subscriptionsChanged.connect(lambda: structuralChanges.append(True))
+
+        with (
+            mock.patch.object(Storage, 'UserSubs', return_value=subscriptions),
+            mock.patch.object(Storage, 'SubscriptionGroup', side_effect=group),
+            mock.patch.object(Storage, 'upsertSubscriptionGroup', side_effect=upsert),
+            mock.patch.object(manager, 'updateSubsByWebGET') as update,
+        ):
+            manager.updateSubs()
+
+        self.assertEqual(snapshots, [('syncing', 'syncing')])
+        self.assertEqual(stateChanges, [('group-a', 'group-b')])
+        self.assertEqual(structuralChanges, [])
+        self.assertEqual(update.call_count, 2)
+        self.assertEqual(
+            {call.kwargs['unique'] for call in update.call_args_list},
+            {'group-a', 'group-b'},
+        )
+
+        manager.deleteLater()
+
+    def testTargetedUpdatePublishesImmediateSyncingState(self):
+        """Publish one narrow targeted state before network I/O."""
+        subscriptions = {'group-a': self._subscription()}
+        manager = self._manager(subscriptions)
+        group = SubscriptionGroup.fromMapping('group-a', subscriptions['group-a'])
+        snapshots = []
+        stateChanges = []
+        structuralChanges = []
+
+        def stateChanged(uniques):
+            """Capture the target and persisted state before network I/O."""
+            stateChanges.append(tuple(uniques))
+            snapshots.append(group.lastSyncStatus)
+
+        manager.subscriptionStateChanged.connect(stateChanged)
+        manager.subscriptionsChanged.connect(lambda: structuralChanges.append(True))
+
+        with (
+            mock.patch.object(Storage, 'UserSubs', return_value=subscriptions),
+            mock.patch.object(Storage, 'SubscriptionGroup', return_value=group),
+            mock.patch.object(Storage, 'upsertSubscriptionGroup'),
+            mock.patch.object(manager, 'updateSubsByWebGET') as update,
+        ):
+            manager.updateSubsByUnique('group-a')
+
+        self.assertEqual(snapshots, ['syncing'])
+        self.assertEqual(stateChanges, [('group-a',)])
+        self.assertEqual(structuralChanges, [])
+        update.assert_called_once()
+
+        manager.deleteLater()
+
     def testTargetedPolicyChangeDoesNotDisturbOtherSubscriptions(self):
         """Reconcile only the edited subscription's schedule."""
         subscriptions = {
@@ -741,6 +914,95 @@ class SubscriptionManagerTest(TestCase):
         manager._replySubscriptions.clear()
         manager.deleteLater()
 
+    def testUpdateSelectedUsesOneOrderedCanonicalBatchCall(self):
+        """Keep a real selected-update click narrow and event-loop responsive."""
+        subscriptions = {
+            'group-a': self._subscription(),
+            'group-b': self._subscription(
+                remark='Group B',
+                webURL='https://invalid.test/b',
+            ),
+            'group-c': self._subscription(
+                remark='Group C',
+                webURL='https://invalid.test/c',
+            ),
+        }
+
+        def group(unique):
+            """Return one isolated persisted group."""
+            value = subscriptions.get(unique)
+
+            return (
+                SubscriptionGroup.fromMapping(unique, value)
+                if value is not None
+                else None
+            )
+
+        def upsert(value):
+            """Persist one status transition into the isolated repository."""
+            subscriptions[value.id] = value.toMapping()
+
+        with (
+            mock.patch.object(Storage, 'UserSubs', return_value=subscriptions),
+            mock.patch.object(Storage, 'SubscriptionGroup', side_effect=group),
+            mock.patch.object(Storage, 'upsertSubscriptionGroup', side_effect=upsert),
+        ):
+            manager = SubscriptionManager()
+            stateChanges = []
+            structuralChanges = []
+
+            def updateSubscriptions(uniques, _httpProxy, **kwargs):
+                """Cross the production wrapper boundary into the real manager."""
+                kwargs.pop('parent', None)
+                manager.updateSubscriptions(uniques, **kwargs)
+
+            serverTable = SimpleNamespace(subsManager=manager)
+            serverTable.updateSubscriptions = mock.Mock(side_effect=updateSubscriptions)
+            page = SubscriptionPage(serverTable)
+            selection = page.table.selectionModel()
+            flags = (
+                QtCore.QItemSelectionModel.SelectionFlag.Select
+                | QtCore.QItemSelectionModel.SelectionFlag.Rows
+            )
+
+            selection.select(page.table.sourceModel.index(0, 0), flags)
+            selection.select(page.table.sourceModel.index(2, 0), flags)
+
+            manager.subscriptionStateChanged.connect(
+                lambda uniques: stateChanges.append(tuple(uniques))
+            )
+            manager.subscriptionsChanged.connect(lambda: structuralChanges.append(True))
+
+            page.show()
+            processQtEvents(1)
+
+            handled = []
+            QtCore.QTimer.singleShot(0, lambda: handled.append(True))
+
+            with mock.patch.object(manager, 'updateSubsByWebGET') as update:
+                QtTest.QTest.mouseClick(
+                    page.updateSelectedButton,
+                    QtCore.Qt.MouseButton.LeftButton,
+                )
+
+            processQtEvents(1)
+
+            serverTable.updateSubscriptions.assert_called_once_with(
+                ('group-a', 'group-c'),
+                None,
+                showMessageBox=True,
+                parent=page,
+            )
+            self.assertEqual(stateChanges, [('group-a', 'group-c')])
+            self.assertEqual(structuralChanges, [])
+            self.assertEqual(update.call_count, 2)
+            self.assertEqual(handled, [True])
+
+            page.deleteLater()
+            manager.deleteLater()
+
+        processQtEvents()
+
     def testPageNavigationIsPresentationOnlyForAutoUpdateScheduler(self):
         """Keep page show/hide cycles outside scheduler policy ownership."""
         subscriptions = {'group-a': self._subscription()}