Refactor remove unnecessary export.Service injection

The export.getService() function allows to obtain the export service
instance wherever necessary without needing to inject it through all
the parent widgets.

This is a double-edged sword, as it makes the injection more implicit,
but it is a pattern that is commonly used for singletons, e.g. loggers.
Use thoughtfully, and remember that injecting the export service
explicitly in some widgets is always possible!

Next steps: as soon as the export.Device is not longer necessary, then
the export.Service type doesn't need to be exported anymore.

securedrop_client/app.py

@@ -249,7 +249,7 @@ def start_app(args, qt_args) -> NoReturn:  # type: ignore [no-untyped-def]
         export_service.moveToThread(export_service_thread)
         export_service_thread.start()
 
-        gui = Window(app_state, export_service)
+        gui = Window(app_state)
 
         controller = Controller(
             "http://localhost:8081/",

securedrop_client/gui/main.py

@@ -27,7 +27,7 @@
 from PyQt5.QtGui import QClipboard, QGuiApplication, QIcon, QKeySequence
 from PyQt5.QtWidgets import QAction, QApplication, QHBoxLayout, QMainWindow, QVBoxLayout, QWidget
 
-from securedrop_client import __version__, export, state
+from securedrop_client import __version__, state
 from securedrop_client.db import Source, User
 from securedrop_client.gui.auth import LoginDialog
 from securedrop_client.gui.widgets import LeftPane, MainView, TopPane
@@ -48,7 +48,6 @@ class Window(QMainWindow):
     def __init__(
         self,
         app_state: Optional[state.State] = None,
-        export_service: Optional[export.Service] = None,
     ) -> None:
         """
         Create the default start state. The window contains a root widget into
@@ -77,7 +76,7 @@ def __init__(
         layout.setSpacing(0)
         self.main_pane.setLayout(layout)
         self.left_pane = LeftPane()
-        self.main_view = MainView(self.main_pane, app_state, export_service)
+        self.main_view = MainView(self.main_pane, app_state)
         layout.addWidget(self.left_pane)
         layout.addWidget(self.main_view)
 

securedrop_client/gui/widgets.py

@@ -563,12 +563,10 @@ def __init__(
         self,
         parent: QObject,
         app_state: Optional[state.State] = None,
-        export_service: Optional[export.Service] = None,
     ) -> None:
         super().__init__(parent)
 
         self._state = app_state
-        self._export_service = export_service
 
         # Set id and styles
         self.setObjectName("MainView")
@@ -666,7 +664,7 @@ def on_source_changed(self) -> None:
                 conversation_wrapper.conversation_view.update_conversation(source.collection)
             else:
                 conversation_wrapper = SourceConversationWrapper(
-                    source, self.controller, self._state, self._export_service
+                    source, self.controller, self._state
                 )
                 self.source_conversations[source.uuid] = conversation_wrapper
 
@@ -2192,7 +2190,6 @@ def __init__(
         file_missing: pyqtBoundSignal,
         index: int,
         container_width: int,
-        export_service: Optional[export.Service] = None,
     ) -> None:
         """
         Given some text and a reference to the controller, make something to display a file.
@@ -2201,12 +2198,7 @@ def __init__(
 
         self.controller = controller
 
-        if export_service is None:
-            # Note that injecting an export service that runs in a separate
-            # thread is greatly encouraged! But it is optional because strictly
-            # speaking it is not a dependency of this FileWidget.
-            export_service = export.Service()
-
+        export_service = export.getService()
         self._export_device = conversation.ExportDevice(controller, export_service)
 
         self.file = self.controller.get_file(file_uuid)
@@ -2618,12 +2610,9 @@ def __init__(
         self,
         source_db_object: Source,
         controller: Controller,
-        export_service: Optional[export.Service] = None,
     ) -> None:
         super().__init__()
 
-        self._export_service = export_service
-
         self.source = source_db_object
         self.source_uuid = source_db_object.uuid
         self.controller = controller
@@ -2809,7 +2798,6 @@ def add_file(self, file: File, index: int) -> None:
             self.controller.file_missing,
             index,
             self.scroll.widget().width(),
-            self._export_service,
         )
         self.scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignLeft)
         self.current_messages[file.uuid] = conversation_item
@@ -2917,7 +2905,6 @@ def __init__(
         source: Source,
         controller: Controller,
         app_state: Optional[state.State] = None,
-        export_service: Optional[export.Service] = None,
     ) -> None:
         super().__init__()
 
@@ -2943,7 +2930,7 @@ def __init__(
 
         # Create widgets
         self.conversation_title_bar = SourceProfileShortWidget(source, controller, app_state)
-        self.conversation_view = ConversationView(source, controller, export_service)
+        self.conversation_view = ConversationView(source, controller)
         self.reply_box = ReplyBoxWidget(source, controller)
         self.deletion_indicator = SourceDeletionIndicator()
         self.conversation_deletion_indicator = ConversationDeletionIndicator()

tests/conftest.py

@@ -141,15 +141,16 @@ def export_service():
 
 @pytest.fixture(scope="function")
 def functional_test_app_started_context(
-    homedir, reply_status_codes, session, config, qtbot, export_service
+    mocker, homedir, reply_status_codes, session, config, qtbot, export_service
 ):
     """
     Returns a tuple containing the gui window and controller of a configured client. This should be
     used to for tests that need to start from the login dialog before the main application window
     is visible.
     """
+    mocker.patch("securedrop_client.export.getService", return_value=export_service)
     app_state = state.State()
-    gui = Window(app_state, export_service)
+    gui = Window(app_state)
     create_gpg_test_context(homedir)  # Configure test keys
     session_maker = make_session_maker(homedir)  # Configure and create the database
     controller = Controller(HOSTNAME, gui, session_maker, homedir, app_state, False, False)

tests/gui/test_main.py

@@ -40,11 +40,11 @@ def test_init(mocker):
     load_css = mocker.patch("securedrop_client.gui.main.load_css")
 
     app_state = state.State()
-    w = Window(app_state, export_service)
+    w = Window(app_state)
 
     mock_li.assert_called_once_with(w.icon)
     mock_lp.assert_called_once_with()
-    mock_mv.assert_called_once_with(w.main_pane, app_state, export_service)
+    mock_mv.assert_called_once_with(w.main_pane, app_state)
     assert mock_lo().addWidget.call_count == 2
     load_css.assert_called_once_with("sdclient.css")
 

tests/integration/conftest.py

@@ -11,7 +11,8 @@
 
 
 @pytest.fixture(scope="function")
-def main_window(mocker, homedir):
+def main_window(mocker, homedir, export_service):
+    mocker.patch("securedrop_client.export.getService", return_value=export_service)
     # Setup
     app = QApplication([])
     gui = Window()
@@ -63,7 +64,8 @@ def main_window(mocker, homedir):
 
 
 @pytest.fixture(scope="function")
-def main_window_no_key(mocker, homedir):
+def main_window_no_key(mocker, homedir, export_service):
+    mocker.patch("securedrop_client.export.getService", return_value=export_service)
     # Setup
     app = QApplication([])
     gui = Window()
@@ -161,7 +163,7 @@ def export_service():
 @pytest.fixture(scope="function")
 def print_dialog(mocker, homedir, export_service):
     app = QApplication([])
-    gui = Window(export_service=export_service)
+    gui = Window()
     app.setActiveWindow(gui)
     gui.show()
     with threads(3) as [sync_thread, main_queue_thread, file_download_thread]:
@@ -196,7 +198,7 @@ def print_dialog(mocker, homedir, export_service):
 @pytest.fixture(scope="function")
 def export_dialog(mocker, homedir, export_service):
     app = QApplication([])
-    gui = Window(export_service=export_service)
+    gui = Window()
     app.setActiveWindow(gui)
     gui.show()
     with threads(3) as [sync_thread, main_queue_thread, file_download_thread]:

tests/test_app.py

@@ -7,7 +7,7 @@
 
 import pytest
 
-from securedrop_client import export, state
+from securedrop_client import state
 from securedrop_client.app import (
     DEFAULT_SDC_HOME,
     ENCODING,
@@ -22,8 +22,6 @@
 )
 from tests.helper import app  # noqa: F401
 
-export_service = export.getService()
-
 
 def test_application_sets_en_as_default_language_code(mocker):
     mocker.patch("locale.getdefaultlocale", return_value=(None, None))
@@ -141,7 +139,6 @@ def test_start_app(homedir, mocker):
     mock_args.proxy = False
     app_state = state.State()
     mocker.patch("securedrop_client.state.State", return_value=app_state)
-    mocker.patch("securedrop_client.export.Service", return_value=export_service)
 
     mocker.patch("securedrop_client.app.configure_logging")
     mock_app = mocker.patch("securedrop_client.app.QApplication")
@@ -155,7 +152,7 @@ def test_start_app(homedir, mocker):
     start_app(mock_args, mock_qt_args)
 
     mock_app.assert_called_once_with(mock_qt_args)
-    mock_win.assert_called_once_with(app_state, export_service)
+    mock_win.assert_called_once_with(app_state)
     mock_controller.assert_called_once_with(
         "http://localhost:8081/",
         mock_win(),