From b609abe7e5495fc00e008045e8b94acc984d4059 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 18 Nov 2021 14:39:22 -0800 Subject: [PATCH] *: Use the App's state management API Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/middleware.py | 4 +++- plinth/module_loader.py | 3 +-- plinth/modules/backups/api.py | 9 ++++--- plinth/modules/backups/tests/test_api.py | 16 ++++++------- plinth/modules/backups/tests/test_schedule.py | 9 ++++--- plinth/modules/cockpit/__init__.py | 6 ++--- plinth/modules/diagnostics/__init__.py | 4 +--- plinth/modules/ejabberd/__init__.py | 15 ++++-------- .../ejabberd/tests/test_turn_config.py | 9 ++++--- plinth/modules/first_boot/__init__.py | 2 +- plinth/modules/gitweb/__init__.py | 3 +-- plinth/modules/matrixsynapse/__init__.py | 3 +-- .../matrixsynapse/tests/test_turn_config.py | 19 +++++++-------- plinth/modules/pagekite/__init__.py | 3 +-- plinth/modules/security/__init__.py | 2 +- plinth/modules/shadowsocks/__init__.py | 2 +- plinth/modules/sso/__init__.py | 5 +--- plinth/modules/storage/forms.py | 15 ++++++------ plinth/modules/tor/__init__.py | 5 ++-- plinth/setup.py | 24 +++++++++++-------- plinth/templates/setup.html | 10 ++++---- plinth/tests/test_middleware.py | 7 ++++-- plinth/views.py | 4 ++-- 23 files changed, 83 insertions(+), 96 deletions(-) diff --git a/plinth/middleware.py b/plinth/middleware.py index 963990cc2..9e610160f 100644 --- a/plinth/middleware.py +++ b/plinth/middleware.py @@ -16,6 +16,7 @@ from django.utils.translation import gettext_lazy as _ from stronghold.utils import is_view_func_public import plinth +from plinth import app as app_module from plinth import setup from plinth.package import PackageException from plinth.utils import is_user_admin @@ -80,7 +81,8 @@ class SetupMiddleware(MiddlewareMixin): _collect_setup_result(request, module) # Check if application is up-to-date - if module.setup_helper.get_state() == 'up-to-date': + if module.app.get_setup_state() == \ + app_module.App.SetupState.UP_TO_DATE: return if not is_admin: diff --git a/plinth/module_loader.py b/plinth/module_loader.py index 561415507..9bbe8cf98 100644 --- a/plinth/module_loader.py +++ b/plinth/module_loader.py @@ -147,8 +147,7 @@ def apps_post_init(): try: module.app.post_init() - if module.setup_helper.get_state( - ) != 'needs-setup' and module.app.is_enabled(): + if not module.app.needs_setup() and module.app.is_enabled(): module.app.set_enabled(True) except Exception as exception: logger.exception('Exception while running post init for %s: %s', diff --git a/plinth/modules/backups/api.py b/plinth/modules/backups/api.py index 37c3b6892..5b4208840 100644 --- a/plinth/modules/backups/api.py +++ b/plinth/modules/backups/api.py @@ -10,7 +10,6 @@ TODO: - Implement unit tests. """ -import importlib import logging from plinth import action_utils, actions @@ -176,8 +175,9 @@ def _install_apps_before_restore(components): """ modules_to_setup = [] for component in components: - module = importlib.import_module(component.app.__class__.__module__) - if module.setup_helper.get_state() in ('needs-setup', 'needs-update'): + if component.app.get_setup_state() in ( + app_module.App.SetupState.NEEDS_SETUP, + app_module.App.SetupState.NEEDS_UPDATE): modules_to_setup.append(component.app.app_id) setup.run_setup_on_modules(modules_to_setup) @@ -198,8 +198,7 @@ def get_all_components_for_backup(): for app_ in app_module.App.list(): try: - module = importlib.import_module(app_.__class__.__module__) - if module.setup_helper.get_state() != 'needs-setup': + if not app_.needs_setup(): components.append(_get_backup_restore_component(app_)) except TypeError: # Application not available for backup/restore pass diff --git a/plinth/modules/backups/tests/test_api.py b/plinth/modules/backups/tests/test_api.py index fe5019311..395e2f271 100644 --- a/plinth/modules/backups/tests/test_api.py +++ b/plinth/modules/backups/tests/test_api.py @@ -95,23 +95,23 @@ class TestBackupProcesses: @staticmethod @patch('plinth.modules.backups.api._install_apps_before_restore') - @patch('plinth.module_loader.loaded_modules.items') - def test_restore_apps(mock_install, modules): + def test_restore_apps(mock_install): """Test that restore_handler is called.""" - modules.return_value = [('a', MagicMock())] restore_handler = MagicMock() api.restore_apps(restore_handler) restore_handler.assert_called_once() @staticmethod - @patch('importlib.import_module') + @patch('plinth.app.App.get_setup_state') @patch('plinth.app.App.list') - def test_get_all_components_for_backup(apps_list, import_module): + def test_get_all_components_for_backup(apps_list, get_setup_state): """Test listing components supporting backup and needing backup.""" - modules = [MagicMock(), MagicMock(), MagicMock()] - import_module.side_effect = modules + get_setup_state.side_effect = [ + App.SetupState.UP_TO_DATE, + App.SetupState.NEEDS_SETUP, + App.SetupState.UP_TO_DATE, + ] apps = [_get_test_app('a'), _get_test_app('b'), _get_test_app('c')] - modules[1].setup_helper.get_state.side_effect = ['needs-setup'] apps_list.return_value = apps returned_components = api.get_all_components_for_backup() diff --git a/plinth/modules/backups/tests/test_schedule.py b/plinth/modules/backups/tests/test_schedule.py index 9fb87b476..dd6ae0847 100644 --- a/plinth/modules/backups/tests/test_schedule.py +++ b/plinth/modules/backups/tests/test_schedule.py @@ -15,8 +15,6 @@ from plinth.app import App from ..components import BackupRestore from ..schedule import Schedule -setup_helper = MagicMock() - class AppTest(App): """Sample App for testing.""" @@ -426,11 +424,12 @@ cases = [ @pytest.mark.parametrize( 'schedule_params,archives_data,test_now,run_periods,cleanups', cases) +@patch('plinth.app.App.get_setup_state') @patch('plinth.modules.backups.repository.get_instance') -def test_run_schedule(get_instance, schedule_params, archives_data, test_now, - run_periods, cleanups): +def test_run_schedule(get_instance, get_setup_state, schedule_params, + archives_data, test_now, run_periods, cleanups): """Test that backups are run at expected time.""" - setup_helper.get_state.return_value = 'up-to-date' + get_setup_state.return_value = App.SetupState.UP_TO_DATE repository = MagicMock() repository.list_archives.side_effect = \ diff --git a/plinth/modules/cockpit/__init__.py b/plinth/modules/cockpit/__init__.py index f5817894d..a3f0d160a 100644 --- a/plinth/modules/cockpit/__init__.py +++ b/plinth/modules/cockpit/__init__.py @@ -118,8 +118,7 @@ def setup(helper, old_version=None): def on_domain_added(sender, domain_type, name, description='', services=None, **kwargs): """Handle addition of a new domain.""" - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() != 'needs-setup': + if not app.needs_setup(): if name not in utils.get_domains(): actions.superuser_run('cockpit', ['add-domain', name]) actions.superuser_run('service', @@ -128,8 +127,7 @@ def on_domain_added(sender, domain_type, name, description='', services=None, def on_domain_removed(sender, domain_type, name, **kwargs): """Handle removal of a domain.""" - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() != 'needs-setup': + if not app.needs_setup(): if name in utils.get_domains(): actions.superuser_run('cockpit', ['remove-domain', name]) actions.superuser_run('service', diff --git a/plinth/modules/diagnostics/__init__.py b/plinth/modules/diagnostics/__init__.py index 5cb2b202e..f11476ce2 100644 --- a/plinth/modules/diagnostics/__init__.py +++ b/plinth/modules/diagnostics/__init__.py @@ -4,7 +4,6 @@ FreedomBox app for system diagnostics. """ import collections -import importlib import logging import pathlib import threading @@ -115,8 +114,7 @@ def run_on_all_enabled_modules(): for app in app_module.App.list(): # Don't run diagnostics on apps have not been setup yet. # However, run on apps that need an upgrade. - module = importlib.import_module(app.__class__.__module__) - if module.setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): continue if not app.is_enabled(): diff --git a/plinth/modules/ejabberd/__init__.py b/plinth/modules/ejabberd/__init__.py index e35cd6367..4bde2c9ac 100644 --- a/plinth/modules/ejabberd/__init__.py +++ b/plinth/modules/ejabberd/__init__.py @@ -168,8 +168,7 @@ def get_domains(): XXX: Retrieve the list from ejabberd configuration. """ - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): return [] domain_name = config.get_domainname() @@ -185,8 +184,7 @@ def on_pre_hostname_change(sender, old_hostname, new_hostname, **kwargs): """ del sender # Unused del kwargs # Unused - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): return actions.superuser_run('ejabberd', [ @@ -199,8 +197,7 @@ def on_post_hostname_change(sender, old_hostname, new_hostname, **kwargs): """Update ejabberd config after hostname change.""" del sender # Unused del kwargs # Unused - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): return actions.superuser_run('ejabberd', [ @@ -212,8 +209,7 @@ def on_post_hostname_change(sender, old_hostname, new_hostname, **kwargs): def on_domain_added(sender, domain_type, name, description='', services=None, **kwargs): """Update ejabberd config after domain name change.""" - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): return conf = actions.superuser_run('ejabberd', ['get-configuration']) @@ -226,8 +222,7 @@ def on_domain_added(sender, domain_type, name, description='', services=None, def update_turn_configuration(config: TurnConfiguration, managed=True, force=False): """Update ejabberd's STUN/TURN server configuration.""" - setup_helper = globals()['setup_helper'] - if not force and setup_helper.get_state() == 'needs-setup': + if app.needs_setup(): return params = ['configure-turn'] diff --git a/plinth/modules/ejabberd/tests/test_turn_config.py b/plinth/modules/ejabberd/tests/test_turn_config.py index 21de15441..b48ddf67d 100644 --- a/plinth/modules/ejabberd/tests/test_turn_config.py +++ b/plinth/modules/ejabberd/tests/test_turn_config.py @@ -5,7 +5,7 @@ Test module for ejabberd STUN/TURN configuration. import pathlib import shutil -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest @@ -58,10 +58,9 @@ def fixture_test_configuration(call_action, conf_file): Patches actions.superuser_run with the fixture call_action. The module state is patched to be 'up-to-date'. """ - with patch('plinth.actions.superuser_run', call_action): - helper = MagicMock() - helper.get_state.return_value = 'up-to-date' - ejabberd.setup_helper = helper + with patch('plinth.actions.superuser_run', + call_action), patch('plinth.modules.ejabberd.app') as app: + app.needs_setup.return_value = False yield diff --git a/plinth/modules/first_boot/__init__.py b/plinth/modules/first_boot/__init__.py index e90729193..c0b49f751 100644 --- a/plinth/modules/first_boot/__init__.py +++ b/plinth/modules/first_boot/__init__.py @@ -74,7 +74,7 @@ def _get_steps(): modules = module_loader.loaded_modules for module_object in modules.values(): if getattr(module_object, 'first_boot_steps', None): - if module_object.setup_helper.get_state() != 'needs-setup': + if not module_object.app.needs_setup(): steps.extend(module_object.first_boot_steps) _all_first_boot_steps = sorted(steps, key=operator.itemgetter('order')) diff --git a/plinth/modules/gitweb/__init__.py b/plinth/modules/gitweb/__init__.py index 68a8f1de6..a68797849 100644 --- a/plinth/modules/gitweb/__init__.py +++ b/plinth/modules/gitweb/__init__.py @@ -97,8 +97,7 @@ class GitwebApp(app_module.App): def post_init(self): """Perform post initialization operations.""" - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() != 'needs-setup': + if not self.needs_setup(): self.update_service_access() def set_shortcut_login_required(self, login_required): diff --git a/plinth/modules/matrixsynapse/__init__.py b/plinth/modules/matrixsynapse/__init__.py index dda2493ac..6c7e33af4 100644 --- a/plinth/modules/matrixsynapse/__init__.py +++ b/plinth/modules/matrixsynapse/__init__.py @@ -226,8 +226,7 @@ def get_certificate_status(): def update_turn_configuration(config: TurnConfiguration, managed=True, force=False): """Update the STUN/TURN server configuration.""" - setup_helper = globals()['setup_helper'] - if not force and setup_helper.get_state() == 'needs-setup': + if not force and app.needs_setup(): return params = ['configure-turn'] diff --git a/plinth/modules/matrixsynapse/tests/test_turn_config.py b/plinth/modules/matrixsynapse/tests/test_turn_config.py index 2a9d75c10..28f5bb834 100644 --- a/plinth/modules/matrixsynapse/tests/test_turn_config.py +++ b/plinth/modules/matrixsynapse/tests/test_turn_config.py @@ -3,7 +3,7 @@ Test module for Matrix Synapse STUN/TURN configuration. """ -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest @@ -45,15 +45,14 @@ def fixture_test_configuration(call_action, managed_turn_conf_file, Overrides TURN configuration files and patches actions.superuser_run with the fixture call_action """ - with patch('plinth.modules.matrixsynapse.TURN_CONF_PATH', - managed_turn_conf_file), \ - patch('plinth.modules.matrixsynapse.OVERRIDDEN_TURN_CONF_PATH', - overridden_turn_conf_file), \ - patch('plinth.modules.matrixsynapse.is_setup', return_value=True), \ - patch('plinth.actions.superuser_run', call_action): - helper = MagicMock() - helper.get_state.return_value = 'up-to-date' - matrixsynapse.setup_helper = helper + with (patch('plinth.modules.matrixsynapse.TURN_CONF_PATH', + managed_turn_conf_file), + patch('plinth.modules.matrixsynapse.OVERRIDDEN_TURN_CONF_PATH', + overridden_turn_conf_file), + patch('plinth.modules.matrixsynapse.is_setup', return_value=True), + patch('plinth.actions.superuser_run', call_action), + patch('plinth.modules.matrixsynapse.app') as app): + app.needs_setup.return_value = False yield diff --git a/plinth/modules/pagekite/__init__.py b/plinth/modules/pagekite/__init__.py index 531f5393e..0b7cf5694 100644 --- a/plinth/modules/pagekite/__init__.py +++ b/plinth/modules/pagekite/__init__.py @@ -90,8 +90,7 @@ class PagekiteApp(app_module.App): def post_init(self): """Perform post initialization operations.""" # Register kite name with Name Services module. - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() != 'needs-setup' and self.is_enabled(): + if not self.needs_setup() and self.is_enabled(): utils.update_names_module(is_enabled=True) def enable(self): diff --git a/plinth/modules/security/__init__.py b/plinth/modules/security/__init__.py index ed32c4b95..b98f7e3cc 100644 --- a/plinth/modules/security/__init__.py +++ b/plinth/modules/security/__init__.py @@ -145,7 +145,7 @@ def get_apps_report(): services.append(component.unit) # filter out apps not setup yet - if module.setup_helper.get_state() == 'needs-setup': + if module.app.needs_setup(): continue apps[module_name] = { diff --git a/plinth/modules/shadowsocks/__init__.py b/plinth/modules/shadowsocks/__init__.py index a851cb600..566c599c1 100644 --- a/plinth/modules/shadowsocks/__init__.py +++ b/plinth/modules/shadowsocks/__init__.py @@ -87,6 +87,6 @@ class ShadowsocksApp(app_module.App): def setup(helper, old_version=None): """Install and configure the module.""" - helper.install(['shadowsocks-libev']) + app.setup(old_version) helper.call('post', actions.superuser_run, 'shadowsocks', ['setup']) helper.call('post', app.enable) diff --git a/plinth/modules/sso/__init__.py b/plinth/modules/sso/__init__.py index 60286f30b..049e92a88 100644 --- a/plinth/modules/sso/__init__.py +++ b/plinth/modules/sso/__init__.py @@ -32,10 +32,7 @@ class SSOApp(app_module.App): self.add(info) packages = Packages('packages-sso', [ - 'libapache2-mod-auth-pubtkt', - 'openssl', - 'python3-openssl', - 'flite', + 'libapache2-mod-auth-pubtkt', 'openssl', 'python3-openssl', 'flite' ]) self.add(packages) diff --git a/plinth/modules/storage/forms.py b/plinth/modules/storage/forms.py index c40b6e82d..3b7cab766 100644 --- a/plinth/modules/storage/forms.py +++ b/plinth/modules/storage/forms.py @@ -10,14 +10,15 @@ from django import forms from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ -from plinth import actions, module_loader +from plinth import actions +from plinth import app as app_module from plinth.modules import storage def get_available_samba_shares(): """Get available samba shares.""" available_shares = [] - if is_module_enabled('samba'): + if _is_app_enabled('samba'): samba_shares = json.loads( actions.superuser_run('samba', ['get-shares'])) if samba_shares: @@ -30,13 +31,11 @@ def get_available_samba_shares(): return available_shares -def is_module_enabled(name): +def _is_app_enabled(app_id): """Check whether a module is enabled.""" - if name in module_loader.loaded_modules: - module = module_loader.loaded_modules['samba'] - if module.setup_helper.get_state( - ) != 'needs-setup' and module.app.is_enabled(): - return True + app = app_module.App.get(app_id) + if not app.needs_setup() and app.is_enabled(): + return True return False diff --git a/plinth/modules/tor/__init__.py b/plinth/modules/tor/__init__.py index 3c58bd758..262cd7212 100644 --- a/plinth/modules/tor/__init__.py +++ b/plinth/modules/tor/__init__.py @@ -94,9 +94,8 @@ class TorApp(app_module.App): def post_init(self): """Perform post initialization operations.""" # Register hidden service name with Name Services module. - setup_helper = globals()['setup_helper'] - if setup_helper.get_state() != 'needs-setup' and \ - self.is_enabled() and app_is_running(self): + if (not app.needs_setup() and self.is_enabled() + and app_is_running(self)): status = utils.get_status(initialized=False) hostname = status['hs_hostname'] services = [int(port['virtport']) for port in status['hs_ports']] diff --git a/plinth/setup.py b/plinth/setup.py index ac73f94b7..39dd2be30 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -12,6 +12,7 @@ from collections import defaultdict import apt import plinth +from plinth import app as app_module from plinth.package import Packages from plinth.signals import post_setup @@ -64,8 +65,9 @@ class Helper(object): if self.current_operation: return - current_version = self.get_setup_version() - if current_version >= self.module.version: + app = self.module.app + current_version = app.get_setup_version() + if current_version >= app.info.version: return self.allow_install = allow_install @@ -83,7 +85,7 @@ class Helper(object): logger.exception('Error running setup - %s', exception) raise exception else: - self.set_setup_version(self.module.version) + app.set_setup_version(app.info.version) post_setup.send_robust(sender=self.__class__, module_name=self.module_name) finally: @@ -262,11 +264,12 @@ def _get_modules_for_regular_setup(): 1. essential modules that are not up-to-date 2. non-essential modules that are installed and need updates """ - if _is_module_essential(module) and \ - not _module_state_matches(module, 'up-to-date'): + if (_is_module_essential(module) and module.app.get_setup_state() != + app_module.App.SetupState.UP_TO_DATE): return True - if _module_state_matches(module, 'needs-update'): + if (module.app.get_setup_state() == + app_module.App.SetupState.NEEDS_UPDATE): return True return False @@ -288,9 +291,9 @@ def _set_is_first_setup(): """Set whether all essential modules have been setup at least once.""" global _is_first_setup modules = plinth.module_loader.loaded_modules.values() - _is_first_setup = any((module for module in modules - if _is_module_essential(module) - and _module_state_matches(module, 'needs-setup'))) + _is_first_setup = any( + (module for module in modules + if _is_module_essential(module) and module.app.needs_setup())) def run_setup_on_modules(module_list, allow_install=True): @@ -537,7 +540,8 @@ class ForceUpgrader(): # App does not implement force upgrade continue - if not _module_state_matches(module, 'up-to-date'): + if (module.app.get_setup_state() != + app_module.App.SetupState.UP_TO_DATE): # App is not installed. # Or needs an update, let it update first. continue diff --git a/plinth/templates/setup.html b/plinth/templates/setup.html index f0f07821f..37cb5ea22 100644 --- a/plinth/templates/setup.html +++ b/plinth/templates/setup.html @@ -13,18 +13,18 @@ {% include "toolbar.html" %} - {% if setup_state == 'up-to-date' %} + {% if setup_state.value == 'up-to-date' %} {% trans "Application installed." %} {% elif not setup_current_operation %}

- {% if setup_state == 'needs-setup' %} + {% if setup_state.value == 'needs-setup' %} {% blocktrans trimmed %} Install this application? {% endblocktrans %} - {% elif setup_state == 'needs-update' %} + {% elif setup_state.value == 'needs-update' %} {% blocktrans trimmed %} This application needs an update. Update now? {% endblocktrans %} @@ -67,9 +67,9 @@ {% if package_manager_is_busy or has_unavailable_packages %} disabled="disabled" {% endif %} - {% if setup_state == 'needs-setup' %} + {% if setup_state.value == 'needs-setup' %} value="{% trans "Install" %}" - {% elif setup_state == 'needs-update' %} + {% elif setup_state.value == 'needs-update' %} value="{% trans "Update" %}" {% endif %} /> diff --git a/plinth/tests/test_middleware.py b/plinth/tests/test_middleware.py index f3dc3157f..29a0bbe74 100644 --- a/plinth/tests/test_middleware.py +++ b/plinth/tests/test_middleware.py @@ -12,6 +12,7 @@ from django.http import HttpResponse from django.test.client import RequestFactory from stronghold.decorators import public +from plinth import app as app_module from plinth.middleware import AdminRequiredMiddleware, SetupMiddleware @@ -60,7 +61,8 @@ class TestSetupMiddleware: resolve.return_value.namespaces = ['mockapp'] module = Mock() module.setup_helper.is_finished = None - module.setup_helper.get_state.return_value = 'up-to-date' + module.app.get_setup_state.return_value = \ + app_module.App.SetupState.UP_TO_DATE loaded_modules.__getitem__.return_value = module request = RequestFactory().get('/plinth/mockapp') @@ -130,7 +132,8 @@ class TestSetupMiddleware: module.is_essential = False module.setup_helper.is_finished = True module.setup_helper.collect_result.return_value = None - module.setup_helper.get_state.return_value = 'up-to-date' + module.app.get_setup_state.return_value = \ + app_module.App.SetupState.UP_TO_DATE loaded_modules.__getitem__.return_value = module # Admin user can't collect result diff --git a/plinth/views.py b/plinth/views.py index 8a441bc39..5a381c96c 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -281,7 +281,7 @@ class SetupView(TemplateView): context['package_conflicts_action'] = package_conflicts_action # Reuse the value of setup_state throughout the view for consistency. - context['setup_state'] = setup_helper.get_state() + context['setup_state'] = setup_helper.module.app.get_setup_state() context['setup_current_operation'] = setup_helper.current_operation # Perform expensive operation only if needed. @@ -293,7 +293,7 @@ class SetupView(TemplateView): setup_helper.module.app) context['refresh_page_sec'] = None - if context['setup_state'] == 'up-to-date': + if context['setup_state'] == app.App.SetupState.UP_TO_DATE: context['refresh_page_sec'] = 0 elif context['setup_current_operation']: context['refresh_page_sec'] = 3