From b1ed1c1fbbb0cba32cd215d27cd5f74db0036544 Mon Sep 17 00:00:00 2001 From: Henry Borchers Date: Mon, 25 Mar 2024 17:38:11 -0500 Subject: [PATCH] refactored StartQtThreaded to use MainWindowBuilder and Validator --- speedwagon/frontend/qtwidgets/gui_startup.py | 311 +++++++++++++----- .../frontend/qtwidgets/models/common.py | 2 +- speedwagon/validators.py | 53 ++- tests/frontend/test_gui_startup.py | 79 ++--- 4 files changed, 312 insertions(+), 133 deletions(-) diff --git a/speedwagon/frontend/qtwidgets/gui_startup.py b/speedwagon/frontend/qtwidgets/gui_startup.py index d36efee6a..6ccdf3fd9 100644 --- a/speedwagon/frontend/qtwidgets/gui_startup.py +++ b/speedwagon/frontend/qtwidgets/gui_startup.py @@ -15,7 +15,17 @@ import time import types import typing -from typing import Dict, Optional, cast, List, Type, Callable, DefaultDict +from typing import ( + Dict, + Optional, + cast, + List, + Type, + Callable, + DefaultDict, + Tuple +) + import traceback as tb import webbrowser @@ -23,7 +33,7 @@ from importlib import metadata except ImportError: # pragma: no cover import importlib_metadata as metadata # type: ignore -from PySide6 import QtWidgets +from PySide6 import QtWidgets, QtCore import speedwagon.job from speedwagon.config.tabs import CustomTabsYamlConfig, TabsYamlWriter @@ -34,6 +44,7 @@ from speedwagon.utils import get_desktop_path from speedwagon.tasks import system as system_tasks from speedwagon import plugins, info +from speedwagon.validators import Validator from . import user_interaction from . import dialog from . import runners @@ -67,7 +78,7 @@ class AbsGuiStarter(speedwagon.startup.AbsStarter, abc.ABC): """Abstract base class to starting a gui application.""" - def __init__(self, app) -> None: + def __init__(self, app=None) -> None: """Create a new gui starter object.""" super().__init__() self.app = app @@ -291,6 +302,152 @@ def get_help_url() -> Optional[str]: return None +_T = typing.TypeVar("_T") + + +class MainWindowBuilder(typing.Generic[_T]): + + def __init__(self) -> None: + self.logger = logging.getLogger() + self.triggers = [] + self._attach_log_callback: Callable[ + [_T], + Optional[Callable[[logging.Logger], None]] + ] = lambda window: None + + def build(self, window: _T) -> _T: + attach_logger = self._attach_log_callback(window) + if attach_logger: + attach_logger(self.logger) + + for signal_callback, payload_callback in self.triggers: + signal: Optional[QtCore.SignalInstance] = signal_callback(window) + if signal: + self._assign(window, signal, payload_callback) + + return window + + def _assign( + self, + window: _T, + signal: QtCore.SignalInstance, + payload_callback: Callable + ) -> None: + signal.connect( + lambda *args, **kwargs: payload_callback( + window, + *args, + **kwargs + ) + ) + + def assign_trigger( + self, + signal: Callable[[_T], Optional[QtCore.SignalInstance]], + payload: Callable + ) -> None: + self.triggers.append((signal, payload)) + + def attach_logger( + self, + logger: logging.Logger, + attach_log_callback: Callable[ + [_T], + Optional[ + Callable[[logging.Logger], None] + ] + ] + ) -> None: + self._attach_log_callback = attach_log_callback + self.logger = logger + + +class WorkflowTabLoader: + def __init__(self, application: gui.MainWindow3) -> None: + super().__init__() + self.application = application + + def load_workflows_tab( + self, + name: str, + workflows: typing.Dict[str, Type[speedwagon.job.Workflow]], + ) -> None: + """Load tab that contains all workflows.""" + self.application.add_tab( + name, collections.OrderedDict(sorted(workflows.items())) + ) + + def load_all_workflows( + self, + app_settings: Dict[str, typing.Any], + ) -> Dict[str, Type[speedwagon.job.Workflow]]: + all_workflows = speedwagon.job.available_workflows() + for workflow_name, error in validate_workflows( + all_workflows, + app_settings + ): + error_message = ( + f"Unable to load workflow '{workflow_name}'. Reason: {error}" + ) + + self.application.logger.error(error_message) + self.application.console.console.add_message(error_message) + del all_workflows[workflow_name] + return all_workflows + + +def check_initialization( + candidate: typing.Type[speedwagon.job.Workflow], + app_global_settings +) -> Tuple[bool, List[str]]: + try: + candidate(global_settings=app_global_settings) + except ( + speedwagon.exceptions.SpeedwagonException, + AttributeError, + ) as error: + return False, [f"Unable to instantiate. Reason {error}"] + + return True, [] + + +def validate_workflows( + workflows: Dict[str, typing.Type[speedwagon.job.Workflow]], + global_settings +): + + workflow_validator = Validator[typing.Type[speedwagon.job.Workflow]]() + + def _check_initialization(candidate): + return check_initialization(candidate, global_settings) + + workflow_validator.add_checks(_check_initialization) + + for title, workflow in workflows.copy().items(): + valid, findings = workflow_validator.check(workflow) + if not valid: + yield title, "\n".join(findings) + + +def iter_tab_file_data( + tabs_file: str, + locate_workflows_strategy: Callable[ + [], + Dict[str, Type[speedwagon.job.Workflow]] + ] +): + loaded_workflows = locate_workflows_strategy() + tabs_file_size = os.path.getsize(tabs_file) + if tabs_file_size > 0: + for tab_name, extra_tab in speedwagon.startup.get_custom_tabs( + loaded_workflows, tabs_file + ): + yield ( + tab_name, + collections.OrderedDict(sorted(extra_tab.items())) + ) + + class StartQtThreaded(AbsGuiStarter): """Start a Qt Widgets base app using threads for job workers.""" @@ -354,7 +511,7 @@ def import_workflow_config( parent.set_active_workflow(workflow_name) parent.set_current_workflow_settings(data) - def _load_help(self) -> None: + def _load_help(self, _) -> None: try: home_page = get_help_url() if home_page: @@ -407,59 +564,24 @@ def load_workflows(self) -> None: self.logger.debug("Loading Workflows") loading_workflows_stream = io.StringIO() self.windows.clear_tabs() + workflow_tab_loader = WorkflowTabLoader(self.windows) with contextlib.redirect_stderr(loading_workflows_stream): - all_workflows = speedwagon.job.available_workflows() - - for workflow_name, error in self._find_invalid(all_workflows): - error_message = ( - f"Unable to load workflow '{workflow_name}'. Reason: {error}" + all_workflows = workflow_tab_loader.load_all_workflows( + app_settings=self.startup_settings ) - self.logger.error(error_message) - self.windows.console.add_message(error_message) - del all_workflows[workflow_name] - - # Load every user configured tab - self.load_custom_tabs( - self.windows, config_strategy.get_tabs_file(), all_workflows - ) - - # All Workflows tab - self.load_all_workflows_tab(self.windows, all_workflows) - - workflow_errors_msg = loading_workflows_stream.getvalue().strip() - if workflow_errors_msg: - for line in workflow_errors_msg.split("\n"): - self.logger.warning(line) - - def load_all_workflows_tab( - self, - application: gui.MainWindow3, - loaded_workflows: typing.Dict[str, Type[speedwagon.job.Workflow]], - ) -> None: - """Load tab that contains all workflows.""" - print("Loading Tab All") - self.logger.debug("Loading Tab All") - application.add_tab( - "All", collections.OrderedDict(sorted(loaded_workflows.items())) - ) - - def load_custom_tabs( - self, - main_window: gui.MainWindow3, - tabs_file: str, - loaded_workflows: typing.Dict[str, Type[speedwagon.job.Workflow]], - ) -> None: - """Load custom tabs.""" - tabs_file_size = os.path.getsize(tabs_file) - if tabs_file_size > 0: + # Load every user configured tab + tabs_file = config_strategy.get_tabs_file() try: - for tab_name, extra_tab in speedwagon.startup.get_custom_tabs( - loaded_workflows, tabs_file - ): - main_window.add_tab( + for tab_name, workflows in \ + iter_tab_file_data( + tabs_file, + locate_workflows_strategy=lambda: all_workflows + ): + self.logger.debug("Loading Tab %s", tab_name) + workflow_tab_loader.load_workflows_tab( tab_name, - collections.OrderedDict(sorted(extra_tab.items())), + workflows ) except speedwagon.exceptions.FileFormatError as error: self.logger.warning( @@ -468,6 +590,15 @@ def load_custom_tabs( error, ) + # All Workflows tab + self.logger.debug("Loading Tab All") + workflow_tab_loader.load_workflows_tab("All", all_workflows) + + workflow_errors_msg = loading_workflows_stream.getvalue().strip() + if workflow_errors_msg: + for line in workflow_errors_msg.split("\n"): + self.logger.warning(line) + def save_log(self, parent: QtWidgets.QWidget) -> None: """Action for user to save logs as a file.""" data = self._log_data.getvalue() @@ -573,42 +704,66 @@ def build_main_window( self, job_manager ) -> speedwagon.frontend.qtwidgets.gui.MainWindow3: """Build main window widget.""" - window = speedwagon.frontend.qtwidgets.gui.MainWindow3() - window.console.attach_logger(self.logger) + window_builder = MainWindowBuilder[ + speedwagon.frontend.qtwidgets.gui.MainWindow3 + ]() - window.action_export_logs.triggered.connect( - lambda: self.save_log(window) + window_builder.attach_logger( + self.logger, + lambda window: window.console.attach_logger(self.logger) ) - window.export_job_config.connect(save_workflow_config) + window_builder.assign_trigger( + lambda window: window.action_export_logs.triggered, + self.save_log + ) - window.action_import_job.triggered.connect( - lambda: self.import_workflow_config(window) + window_builder.assign_trigger( + lambda window: window.action_import_job.triggered, + self.import_workflow_config ) - window.action_system_info_requested.triggered.connect( - lambda: self.request_system_info(window) + window_builder.assign_trigger( + lambda window: window.export_job_config, + lambda window, *args, **kwargs: save_workflow_config( + *args, + **kwargs + ) ) - window.action_open_application_preferences.triggered.connect( - lambda: self.request_settings(window) + window_builder.assign_trigger( + lambda window: window.action_system_info_requested.triggered, + self.request_system_info ) - window.action_help_requested.triggered.connect(self._load_help) + window_builder.assign_trigger( + lambda window: + window.action_open_application_preferences.triggered, + self.request_settings + ) - window.action_about.triggered.connect( - lambda: dialog.about_dialog_box(window) + window_builder.assign_trigger( + lambda window: window.action_about.triggered, + dialog.about_dialog_box ) - window.submit_job.connect( - lambda workflow_name, options: self.submit_job( + window_builder.assign_trigger( + lambda window: window.submit_job, + lambda window, workflow_name, options: self.submit_job( + window, job_manager, workflow_name, options, - main_app=self.windows, ) ) - return window + + window_builder.assign_trigger( + lambda window: window.action_help_requested.triggered, + self._load_help + ) + return window_builder.build( + speedwagon.frontend.qtwidgets.gui.MainWindow3() + ) @staticmethod def abort_job( @@ -640,10 +795,10 @@ def request_more_info( def submit_job( self, + main_app: typing.Optional[gui.MainWindow3], job_manager: runner_strategies.BackgroundJobManager, workflow_name: str, options: Dict[str, typing.Any], - main_app: typing.Optional[gui.MainWindow3] = None, ) -> None: """Submit job.""" workflow_class = speedwagon.job.available_workflows().get( @@ -696,20 +851,6 @@ def _rejected(): ) threaded_events.started.set() - def _find_invalid( - self, workflows: typing.Dict[str, typing.Type[speedwagon.job.Workflow]] - ) -> typing.Iterable[typing.Tuple[str, str]]: - for title, workflow in workflows.copy().items(): - try: - workflow( - global_settings=self.startup_settings.get("GLOBAL", {}) - ) - except ( - speedwagon.exceptions.SpeedwagonException, - AttributeError, - ) as error: - yield title, str(error) - def report_exception_dialog( exc: BaseException, diff --git a/speedwagon/frontend/qtwidgets/models/common.py b/speedwagon/frontend/qtwidgets/models/common.py index 920c20873..64975fb91 100644 --- a/speedwagon/frontend/qtwidgets/models/common.py +++ b/speedwagon/frontend/qtwidgets/models/common.py @@ -57,7 +57,7 @@ def __init__(self, workflow: Optional[Type[Workflow]]) -> None: super().__init__() self.workflow = workflow if workflow is not None and workflow.name is not None: - self.setText(workflow.name) + self.setText(str(workflow.name)) def columnCount(self) -> int: # pylint: disable=invalid-name """Get column count.""" diff --git a/speedwagon/validators.py b/speedwagon/validators.py index b0c974e7e..065346173 100644 --- a/speedwagon/validators.py +++ b/speedwagon/validators.py @@ -2,7 +2,7 @@ import abc import os -from typing import Dict, Any +from typing import Dict, Any, TypeVar, Generic, List, Callable, Tuple class AbsOptionValidator(abc.ABC): @@ -88,3 +88,54 @@ class OptionValidator(OptionValidatorFactory): def get(self, key: str) -> AbsOptionValidator: """Get option validator.""" return self.create(key) + + +_T = TypeVar("_T") + + +class Validator(Generic[_T]): + """Generic validator.""" + + def __init__(self) -> None: + """Create a new validation object.""" + self.validation_checks: List[ + Callable[[_T], Tuple[bool, List[str]]] + ] = [] + + def check(self, candidate: _T) -> Tuple[bool, List[str]]: + """Check candidate against the validation checks. + + Args: + candidate: object being validated + + Returns: + tuple containing object's validation & list of findings discovered + + """ + all_findings: List[str] = [] + is_valid = True + for check in self.validation_checks: + valid, findings = check(candidate) + if not valid: + is_valid = False + all_findings += findings + + return is_valid, all_findings + + def add_checks( + self, + *check: Callable[[_T], Tuple[bool, List[str]]] + ) -> None: + """Add check to list of validations. + + Args: + *check: callbacks that return 2 length Tuple. + Index 0 is True if check produced is valid & False if not + Index 1 contains findings in the form of a list of strings. + + """ + self.validation_checks += check + + def reset(self) -> None: + """Remove any existing checks from the validation.""" + self.validation_checks.clear() diff --git a/tests/frontend/test_gui_startup.py b/tests/frontend/test_gui_startup.py index 781873fe4..9326b4e51 100644 --- a/tests/frontend/test_gui_startup.py +++ b/tests/frontend/test_gui_startup.py @@ -415,13 +415,6 @@ def test_load_workflow_config_cancel(self, qtbot, starter): ) assert serialization_strategy.load.called is False - def test_load_workflows_no_window(self, starter, monkeypatch): - load_custom_tabs = Mock() - starter.windows = None - monkeypatch.setattr(starter, "load_custom_tabs", load_custom_tabs) - starter.load_workflows() - assert load_custom_tabs.called is False - def test_save_log_opens_dialog(self, qtbot, monkeypatch, starter): from PySide6 import QtWidgets getSaveFileName = Mock( @@ -589,36 +582,13 @@ def test_run_opens_window(self, qtbot, monkeypatch, starter): 'get_tabs_file', lambda *_: 'dummy.yml' ) - starter.run() - assert main_window3.show.called is True - - def test_load_custom_tabs(self, qtbot, monkeypatch, starter): - tabs_file = "somefile.yml" - - loaded_workflows = Mock() - monkeypatch.setattr( os.path, "getsize", Mock(return_value=10) ) - - monkeypatch.setattr( - speedwagon.startup, - "get_custom_tabs", - Mock(return_value=[ - ("dummy", {}) - ]) - ) - main_window = Mock() - - starter.load_custom_tabs( - main_window=main_window, - tabs_file=tabs_file, - loaded_workflows=loaded_workflows - ) - - main_window.add_tab.assert_called_with("dummy", ANY) + starter.run() + assert main_window3.show.called is True def test_load_help_no_package_info( self, @@ -660,6 +630,11 @@ def test_load_help_no_package_info( 'get_tabs_file', lambda *_: 'dummy.yml' ) + monkeypatch.setattr( + os.path, + "getsize", + Mock(return_value=10) + ) starter.run() monkeypatch.setattr( @@ -707,6 +682,11 @@ def test_load_help(self, qtbot, monkeypatch, starter): 'get_tabs_file', lambda *_: 'dummy.yml' ) + monkeypatch.setattr( + os.path, + "getsize", + Mock(return_value=10) + ) starter.run() open_new = Mock() @@ -803,10 +783,10 @@ def test_submit_job_errors_on_unknown_workflow( ) starter.submit_job( + main_app, job_manager, workflow_name, options, - main_app, ) assert gui_startup.report_exception_dialog.called is True @@ -842,6 +822,7 @@ def test_submit_job_submits_to_job_manager( ) starter.submit_job( + None, job_manager, workflow_name, options @@ -880,19 +861,6 @@ def test_initialize(self, qtbot, monkeypatch): } assert actual == expected - def test_load_all_workflows_tab(self, qtbot): - start = gui_startup.StartQtThreaded(Mock()) - main_window = Mock('MainWindow3', add_tab=Mock()) - loaded_workflows = {} - start.load_all_workflows_tab(main_window, loaded_workflows) - - # Flushing because qt quits before the logging qt signals are - # propagated to the log widget. This should be fixed but for now, - # it's managed here in the tests - for handler in start.logger.handlers: - handler.flush() - - main_window.add_tab.assert_called_with("All", {}) def test_ensure_settings_files(self, qtbot, monkeypatch): start = gui_startup.StartQtThreaded(Mock()) monkeypatch.setattr( @@ -1278,3 +1246,22 @@ def test_get_help_url_malformed_data(monkeypatch): with pytest.raises(ValueError) as error: gui_startup.get_help_url() assert "malformed" in str(error) + + +class TestMainWindowBuilder: + def test_generate_windows_defaults(self, qtbot): + builder = gui_startup.MainWindowBuilder() + window = builder.build(speedwagon.frontend.qtwidgets.gui.MainWindow3()) + assert isinstance(window, QtWidgets.QMainWindow) + + def test_assign_trigger(self, qtbot): + builder = gui_startup.MainWindowBuilder() + call_something = Mock() + builder.assign_trigger( + signal=lambda w: w.export_job_config, + payload=lambda *args, **kwargs: call_something(*args, **kwargs) + ) + window = builder.build(speedwagon.frontend.qtwidgets.gui.MainWindow3()) + qtbot.addWidget(window) + window.export_job_config.emit("", {}, window) + call_something.assert_called_with(window, "", {}, window)