*: Use the App's state management API

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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:

plinth/module_loader.py

@@ -147,8 +147,7 @@ def apps_post_init():
 
         try:
             module.app.post_init()
-            if module.setup_helper.get_state(
+            if not module.app.needs_setup() and module.app.is_enabled():
-            ) != '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',

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 component.app.get_setup_state() in (
-        if module.setup_helper.get_state() in ('needs-setup', 'needs-update'):
+                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 not app_.needs_setup():
-            if module.setup_helper.get_state() != 'needs-setup':
                 components.append(_get_backup_restore_component(app_))
         except TypeError:  # Application not available for backup/restore
             pass

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):
-    def test_restore_apps(mock_install, modules):
         """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()]
+        get_setup_state.side_effect = [
-        import_module.side_effect = modules
+            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()

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,
+def test_run_schedule(get_instance, get_setup_state, schedule_params,
-                      run_periods, cleanups):
+                      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 = \

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 not app.needs_setup():
-    if setup_helper.get_state() != '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 not app.needs_setup():
-    if setup_helper.get_state() != 'needs-setup':
         if name in utils.get_domains():
             actions.superuser_run('cockpit', ['remove-domain', name])
             actions.superuser_run('service',

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 app.needs_setup():
-            if module.setup_helper.get_state() == 'needs-setup':
                 continue
 
             if not app.is_enabled():

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 app.needs_setup():
-    if setup_helper.get_state() == '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 app.needs_setup():
-    if setup_helper.get_state() == '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 app.needs_setup():
-    if setup_helper.get_state() == '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 app.needs_setup():
-    if setup_helper.get_state() == '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 app.needs_setup():
-    if not force and setup_helper.get_state() == 'needs-setup':
         return
 
     params = ['configure-turn']

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):
+    with patch('plinth.actions.superuser_run',
-        helper = MagicMock()
+               call_action), patch('plinth.modules.ejabberd.app') as app:
-        helper.get_state.return_value = 'up-to-date'
+        app.needs_setup.return_value = False
-        ejabberd.setup_helper = helper
         yield
 
 

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'))

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 not self.needs_setup():
-        if setup_helper.get_state() != 'needs-setup':
             self.update_service_access()
 
     def set_shortcut_login_required(self, login_required):

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 app.needs_setup():
-    if not force and setup_helper.get_state() == 'needs-setup':
         return
 
     params = ['configure-turn']

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',
+    with (patch('plinth.modules.matrixsynapse.TURN_CONF_PATH',
-               managed_turn_conf_file), \
+                managed_turn_conf_file),
-        patch('plinth.modules.matrixsynapse.OVERRIDDEN_TURN_CONF_PATH',
+          patch('plinth.modules.matrixsynapse.OVERRIDDEN_TURN_CONF_PATH',
-              overridden_turn_conf_file), \
+                overridden_turn_conf_file),
-        patch('plinth.modules.matrixsynapse.is_setup', return_value=True), \
+          patch('plinth.modules.matrixsynapse.is_setup', return_value=True),
-            patch('plinth.actions.superuser_run', call_action):
+          patch('plinth.actions.superuser_run', call_action),
-        helper = MagicMock()
+          patch('plinth.modules.matrixsynapse.app') as app):
-        helper.get_state.return_value = 'up-to-date'
+        app.needs_setup.return_value = False
-        matrixsynapse.setup_helper = helper
         yield
 
 

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 not self.needs_setup() and self.is_enabled():
-        if setup_helper.get_state() != 'needs-setup' and self.is_enabled():
             utils.update_names_module(is_enabled=True)
 
     def enable(self):

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] = {

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)

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',
+            'libapache2-mod-auth-pubtkt', 'openssl', 'python3-openssl', 'flite'
-            'openssl',
-            'python3-openssl',
-            'flite',
         ])
         self.add(packages)
 

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:
+    app = app_module.App.get(app_id)
-        module = module_loader.loaded_modules['samba']
+    if not app.needs_setup() and app.is_enabled():
-        if module.setup_helper.get_state(
+        return True
-        ) != 'needs-setup' and module.app.is_enabled():
-            return True
 
     return False
 

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 (not app.needs_setup() and self.is_enabled()
-        if setup_helper.get_state() != 'needs-setup' and \
+                and app_is_running(self)):
-           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']]

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()
+        app = self.module.app
-        if current_version >= self.module.version:
+        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 \
+        if (_is_module_essential(module) and module.app.get_setup_state() !=
-           not _module_state_matches(module, 'up-to-date'):
+                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
+    _is_first_setup = any(
-                           if _is_module_essential(module)
+        (module for module in modules
-                           and _module_state_matches(module, 'needs-setup')))
+         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

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 %}
 
     <p>
-      {% 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 %} />
 

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

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