Retry fixing blocked UI on client start.

securedrop_client/gui/widgets.py

@@ -696,6 +696,7 @@ def setup(self, controller):
         """
         self.controller = controller
         self.source_list.setup(controller)
+        self.source_list.delete_source_by_uuid.connect(self.delete_conversation)
 
     def show_sources(self, sources: List[Source]):
         """
@@ -709,10 +710,15 @@ def show_sources(self, sources: List[Source]):
             self.empty_conversation_view.show_no_sources_message()
             self.empty_conversation_view.show()
 
-        deleted_sources = self.source_list.update(sources)
-        for source_uuid in deleted_sources:
-            # Then call the function to remove the wrapper and its children.
-            self.delete_conversation(source_uuid)
+        if self.source_list.source_widgets:
+            # The source list already contains sources.
+            deleted_sources = self.source_list.update(sources)
+            for source_uuid in deleted_sources:
+                # Then call the function to remove the wrapper and its children.
+                self.delete_conversation(source_uuid)
+        else:
+            # We have an empty source list, so do an initial update.
+            self.source_list.initial_update(sources)
 
     def on_source_changed(self):
         """
@@ -903,6 +909,8 @@ class SourceList(QListWidget):
     }
     '''
 
+    delete_source_by_uuid = pyqtSignal(str)
+
     def __init__(self):
         super().__init__()
 
@@ -977,6 +985,44 @@ def update(self, sources: List[Source]) -> List[str]:
 
         return deleted_uuids
 
+    def initial_update(self, sources: List[Source]):
+        """
+        Initialise the list with the passed in list of sources.
+        """
+        sources.reverse()
+        self.add_source(sources)
+
+    def add_source(self, sources, slice_size=1):
+        """
+        Add a slice of sources, and if necessary, reschedule the addition of
+        more sources.
+        """
+
+        def schedule_source_management(slice_size=slice_size):
+            if not sources:
+                # Nothing more to do.
+                return
+            # Process the remaining "slice_size" number of sources.
+            sources_slice = sources[:slice_size]
+            for source in sources_slice:
+                new_source = SourceWidget(self.controller, source)
+                self.source_widgets[source.uuid] = new_source
+                list_item = QListWidgetItem(self)
+                list_item.setSizeHint(new_source.sizeHint())
+
+                self.addItem(list_item)
+                self.setItemWidget(list_item, new_source)
+            # ATTENTION! 32 is an arbitrary number arrived at via
+            # experimentation. It adds plenty of sources, but doesn't block
+            # for a noticable amount of time.
+            new_slice_size = min(32, slice_size * 2)
+            # Call add_source again for the remaining sources.
+            self.add_source(sources[slice_size:], new_slice_size)
+
+        # Schedule the closure defined above in the next iteration of the
+        # Qt event loop (thus unblocking the UI).
+        QTimer.singleShot(1, schedule_source_management)
+
     def get_selected_source(self):
         if not self.selectedItems():
             return None

tests/gui/test_widgets.py

@@ -11,6 +11,7 @@
 from PyQt5.QtTest import QTest
 from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \
     QLineEdit
+import sqlalchemy
 import sqlalchemy.orm.exc
 from sqlalchemy.orm import attributes, scoped_session, sessionmaker
 
@@ -479,6 +480,7 @@ def test_MainView_setup(mocker):
 
     assert mv.controller == controller
     mv.source_list.setup.assert_called_once_with(controller)
+    mv.source_list.delete_source_by_uuid.connect.assert_called_once_with(mv.delete_conversation)
 
 
 def test_MainView_show_sources_with_none_selected(mocker):
@@ -496,6 +498,20 @@ def test_MainView_show_sources_with_none_selected(mocker):
     mv.empty_conversation_view.show.assert_called_once_with()
 
 
+def test_MainView_show_sources_from_cold_start(mocker):
+    """
+    Ensure the initial_update is called if no sources exist in the UI.
+    """
+    mv = MainView(None)
+    mv.source_list = mocker.MagicMock()
+    mv.source_list.source_widgets = []
+    mv.empty_conversation_view = mocker.MagicMock()
+
+    mv.show_sources([])
+
+    mv.source_list.initial_update.assert_called_once_with([])
+
+
 def test_MainView_show_sources_with_no_sources_at_all(mocker):
     """
     Ensure the sources list is passed to the source list widget to be updated.
@@ -770,6 +786,24 @@ def test_SourceList_update_adds_new_sources(mocker):
     mock_sw.deleteLater.assert_not_called()
 
 
+def test_SourceList_initial_update_adds_new_sources(mocker):
+    """
+    Check a new SourceWidget for each passed-in source is created and no widgets are cleared or
+    removed.
+    """
+    sl = SourceList()
+
+    sl.clear = mocker.MagicMock()
+    sl.add_source = mocker.MagicMock()
+    sl.setItemWidget = mocker.MagicMock()
+    sl.currentRow = mocker.MagicMock(return_value=0)
+    sl.item = mocker.MagicMock()
+    sl.item().isSelected.return_value = True
+    sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ]
+    sl.initial_update(sources)
+    sl.add_source.assert_called_once_with(sources)
+
+
 def test_SourceList_update_when_source_deleted(mocker, session, session_maker, homedir):
     """
     Test that SourceWidget.update raises an exception when its source has been deleted.
@@ -810,6 +844,19 @@ def test_SourceList_update_when_source_deleted(mocker, session, session_maker, h
     assert len(sl.source_widgets) == 0
 
 
+def test_SourceList_add_source_starts_timer(mocker, session_maker, homedir):
+    """
+    When the add_source method is called it schedules the addition of a source
+    to the source list via a single-shot QTimer.
+    """
+    sl = SourceList()
+    sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ]
+    mock_timer = mocker.MagicMock()
+    with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer):
+        sl.add_source(sources)
+    assert mock_timer.singleShot.call_count == 1
+
+
 def test_SourceList_update_when_source_deleted_crash(mocker, session, session_maker, homedir):
     """
     When SourceList.update calls SourceWidget.update and that
@@ -949,6 +996,76 @@ def test_SourceList_update_removes_item_from_beginning_of_list(mocker):
     assert len(sl.source_widgets) == 2
 
 
+def test_SourceList_add_source_closure_adds_sources(mocker):
+    """
+    The closure (function created within the add_source function) behaves in
+    the expected way given the context of the call to add_source.
+    """
+    sl = SourceList()
+    sl.controller = mocker.MagicMock()
+    sl.addItem = mocker.MagicMock()
+    sl.setItemWidget = mocker.MagicMock()
+    sl.setCurrentItem = mocker.MagicMock()
+
+    mock_sw = mocker.MagicMock()
+    mock_lwi = mocker.MagicMock()
+    mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw)
+    mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi)
+    sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ]
+    mock_timer = mocker.MagicMock()
+    with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer):
+        sl.add_source(sources, 1)
+        # Now grab the function created within add_source:
+        inner_fn = mock_timer.singleShot.call_args_list[0][0][1]
+        # Ensure add_source is mocked to avoid recursion in the test and so we
+        # can spy on how it's called.
+        sl.add_source = mocker.MagicMock()
+        # Call the inner function (as if the timer had completed).
+        inner_fn()
+        assert mock_sw.call_count == 1
+        assert mock_lwi.call_count == 1
+        assert sl.addItem.call_count == 1
+        assert sl.setItemWidget.call_count == 1
+        assert len(sl.source_widgets) == 1
+        assert sl.setCurrentItem.call_count == 0
+        sl.add_source.assert_called_once_with(sources[1:], 2)
+
+
+def test_SourceList_add_source_closure_exits_on_no_more_sources(mocker):
+    """
+    The closure (function created within the add_source function) behaves in
+    the expected way given the context of the call to add_source.
+    """
+    sl = SourceList()
+    sl.controller = mocker.MagicMock()
+    sl.addItem = mocker.MagicMock()
+    sl.setItemWidget = mocker.MagicMock()
+    sl.setCurrentItem = mocker.MagicMock()
+
+    mock_sw = mocker.MagicMock()
+    mock_lwi = mocker.MagicMock()
+    mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw)
+    mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi)
+    sources = []
+    mock_timer = mocker.MagicMock()
+    with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer):
+        sl.add_source(sources, 1)
+        # Now grab the function created within add_source:
+        inner_fn = mock_timer.singleShot.call_args_list[0][0][1]
+        # Ensure add_source is mocked to avoid recursion in the test and so we
+        # can spy on how it's called.
+        sl.add_source = mocker.MagicMock()
+        # Call the inner function (as if the timer had completed).
+        assert inner_fn() is None
+        assert mock_sw.call_count == 0
+        assert mock_lwi.call_count == 0
+        assert sl.addItem.call_count == 0
+        assert sl.setItemWidget.call_count == 0
+        assert len(sl.source_widgets) == 0
+        assert sl.setCurrentItem.call_count == 0
+        assert sl.add_source.call_count == 0
+
+
 def test_SourceList_set_snippet(mocker):
     """
     Handle the emitted event in the expected manner.
@@ -1042,6 +1159,26 @@ def test_SourceWidget_update_attachment_icon(mocker):
     assert sw.paperclip.isHidden()
 
 
+def test_SourceWidget_update_raises_InvalidRequestError(mocker):
+    """
+    If the source no longer exists in the local data store, ensure the expected
+    exception is logged and re-raised.
+    """
+    controller = mocker.MagicMock()
+    source = factory.Source(document_count=1)
+    sw = SourceWidget(controller, source)
+    ex = sqlalchemy.exc.InvalidRequestError()
+    controller.session.refresh.side_effect = ex
+    mock_logger = mocker.MagicMock()
+    mocker.patch(
+        "securedrop_client.gui.widgets.logger",
+        mock_logger,
+    )
+    with pytest.raises(sqlalchemy.exc.InvalidRequestError):
+        sw.update()
+        assert mock_logger.error.call_count == 1
+
+
 def test_SourceWidget_set_snippet(mocker, session_maker, session, homedir):
     """
     Snippets are set as expected.