Retry fixing blocked UI on client start.

securedrop_client/gui/widgets.py

@@ -709,10 +709,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):
         """
@@ -977,6 +982,43 @@ 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.
+        """
+        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.insertItem(0, 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

securedrop_client/logic.py

@@ -520,8 +520,6 @@ def update_sources(self):
         Display the updated list of sources with those found in local storage.
         """
         sources = list(storage.get_local_sources(self.session))
-        if sources:
-            sources.sort(key=lambda x: x.last_updated)
         self.gui.show_sources(sources)
 
     def on_update_star_success(self, source_uuid: str) -> None:

securedrop_client/storage.py

@@ -27,7 +27,7 @@
 from dateutil.parser import parse
 from typing import List, Tuple, Type, Union
 
-from sqlalchemy import and_, or_
+from sqlalchemy import and_, desc, or_
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.orm.session import Session
 
@@ -44,9 +44,9 @@
 
 def get_local_sources(session: Session) -> List[Source]:
     """
-    Return all source objects from the local database.
+    Return all source objects from the local database, newest first.
     """
-    return session.query(Source).all()
+    return session.query(Source).order_by(desc(Source.last_updated)).all()
 
 
 def delete_local_source_by_uuid(session: Session, uuid: str, data_dir: str) -> None:

tests/functional/test_download_file.py

@@ -28,7 +28,7 @@ def check_for_sources():
 
     qtbot.waitUntil(check_for_sources, timeout=10000)
     source_ids = list(gui.main_view.source_list.source_widgets.keys())
-    first_source_id = source_ids[2]
+    first_source_id = source_ids[0]
     first_source_widget = gui.main_view.source_list.source_widgets[first_source_id]
     qtbot.mouseClick(first_source_widget, Qt.LeftButton)
 

tests/functional/test_export_dialog.py

@@ -28,7 +28,7 @@ def check_for_sources():
 
     qtbot.waitUntil(check_for_sources, timeout=10000)
     source_ids = list(gui.main_view.source_list.source_widgets.keys())
-    first_source_id = source_ids[2]
+    first_source_id = source_ids[0]
     first_source_widget = gui.main_view.source_list.source_widgets[first_source_id]
     qtbot.mouseClick(first_source_widget, Qt.LeftButton)
 

tests/functional/test_receive_message.py

@@ -28,7 +28,7 @@ def check_for_sources():
 
     qtbot.waitUntil(check_for_sources, timeout=10000)
     source_ids = list(gui.main_view.source_list.source_widgets.keys())
-    first_source_id = source_ids[2]
+    first_source_id = source_ids[0]
     first_source_widget = gui.main_view.source_list.source_widgets[first_source_id]
     qtbot.mouseClick(first_source_widget, Qt.LeftButton)
 

tests/functional/test_star_source.py

@@ -26,7 +26,7 @@ def check_for_sources():
 
     qtbot.waitUntil(check_for_sources, timeout=10000)
     source_ids = list(gui.main_view.source_list.source_widgets.keys())
-    first_source_id = source_ids[1]
+    first_source_id = source_ids[0]
     first_source_widget = gui.main_view.source_list.source_widgets[first_source_id]
     qtbot.mouseClick(first_source_widget, Qt.LeftButton)
 

tests/functional/test_unstar_source.py

@@ -26,7 +26,7 @@ def check_for_sources():
 
     qtbot.waitUntil(check_for_sources, timeout=10000)
     source_ids = list(gui.main_view.source_list.source_widgets.keys())
-    first_source_id = source_ids[1]
+    first_source_id = source_ids[0]
     first_source_widget = gui.main_view.source_list.source_widgets[first_source_id]
     qtbot.mouseClick(first_source_widget, Qt.LeftButton)
 

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
 
@@ -496,6 +497,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 +785,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 +843,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 +995,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.insertItem = 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.insertItem.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 +1158,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.

tests/test_storage.py

@@ -203,7 +203,9 @@ def test_update_local_storage(homedir, mocker, session_maker):
     local_reply = mocker.MagicMock()
     mock_session.query().all = mocker.Mock()
     mock_session.query().all.side_effect = [
-        [local_source], [local_file], [local_message], [local_reply]]
+        [local_file], [local_message], [local_reply]]
+    mock_session.query().order_by().all = mocker.Mock()
+    mock_session.query().order_by().all.side_effect = [[local_source]]
     src_fn = mocker.patch('securedrop_client.storage.update_sources')
     rpl_fn = mocker.patch('securedrop_client.storage.update_replies')
     file_fn = mocker.patch('securedrop_client.storage.update_files')