minetest: Use privileged decorator for actions

Tests:

- Functional tests work
- Updating the configuration values works
  - Enable/disable works
  - Editing the max players works
  - Changing all of them together and one at a time

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

actions/minetest

@@ -1,96 +0,0 @@
-#!/usr/bin/python3
-# SPDX-License-Identifier: AGPL-3.0-or-later
-"""
-Configuration helper for Minetest server.
-"""
-
-import argparse
-
-import augeas
-
-from plinth import action_utils
-
-CONFIG_FILE = '/etc/minetest/minetest.conf'
-AUG_PATH = '/files' + CONFIG_FILE + '/.anon'
-
-
-def parse_arguments():
-    """Return parsed command line arguments as dictionary"""
-    parser = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    configure = subparsers.add_parser('configure', help='Configure Minetest')
-    configure.add_argument('--max_players',
-                           help='Set maximum number of players')
-    configure.add_argument('--creative_mode', choices=['true', 'false'],
-                           help='Set creative mode true/false')
-    configure.add_argument('--enable_pvp', choices=['true', 'false'],
-                           help='Set player Vs player true/false')
-    configure.add_argument('--enable_damage', choices=['true', 'false'],
-                           help='Set damage true/false')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
-def subcommand_configure(arguments):
-    """Configure Minetest."""
-    aug = load_augeas()
-
-    if arguments.max_players:
-        set_max_players(aug, arguments.max_players)
-    if arguments.creative_mode:
-        set_creative_mode(aug, arguments.creative_mode)
-    if arguments.enable_pvp:
-        enable_pvp(aug, arguments.enable_pvp)
-    if arguments.enable_damage:
-        enable_damage(aug, arguments.enable_damage)
-
-    action_utils.service_restart('minetest-server')
-
-
-def set_max_players(aug, max_players):
-    """Sets the number of max players"""
-    aug.set(AUG_PATH + '/max_users', max_players)
-    aug.save()
-
-
-def enable_pvp(aug, choice):
-    """Enables pvp"""
-    aug.set(AUG_PATH + '/enable_pvp', choice)
-    aug.save()
-
-
-def set_creative_mode(aug, choice):
-    """Enables or disables creative mode"""
-    aug.set(AUG_PATH + '/creative_mode', choice)
-    aug.save()
-
-
-def enable_damage(aug, choice):
-    """Enables damage"""
-    aug.set(AUG_PATH + '/enable_damage', choice)
-    aug.save()
-
-
-def load_augeas():
-    """Initialize Augeas."""
-    aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD +
-                        augeas.Augeas.NO_MODL_AUTOLOAD)
-    aug.set('/augeas/load/Php/lens', 'Php.lns')
-    aug.set('/augeas/load/Php/incl[last() + 1]', CONFIG_FILE)
-    aug.load()
-    return aug
-
-
-def main():
-    """Parse arguments and perform all duties"""
-    arguments = parse_arguments()
-
-    subcommand = arguments.subcommand.replace('-', '_')
-    subcommand_method = globals()['subcommand_' + subcommand]
-    subcommand_method(arguments)
-
-
-if __name__ == '__main__':
-    main()

plinth/modules/minetest/__init__.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app for Minetest server."""
-FreedomBox app for Minetest server.
-"""
 
 import augeas
 from django.urls import reverse_lazy

plinth/modules/minetest/privileged.py

@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Configure Minetest server."""
+
+from typing import Optional
+
+import augeas
+
+from plinth import action_utils
+from plinth.actions import privileged
+
+CONFIG_FILE = '/etc/minetest/minetest.conf'
+AUG_PATH = '/files' + CONFIG_FILE + '/.anon'
+
+
+@privileged
+def configure(max_players: Optional[int] = None,
+              enable_pvp: Optional[bool] = None,
+              creative_mode: Optional[bool] = None,
+              enable_damage: Optional[bool] = None):
+    """Update configuration file and restart daemon if necessary."""
+    aug = load_augeas()
+    if max_players is not None:
+        aug.set(AUG_PATH + '/max_users', str(max_players))
+
+    if enable_pvp is not None:
+        aug.set(AUG_PATH + '/enable_pvp', str(enable_pvp).lower())
+
+    if creative_mode is not None:
+        aug.set(AUG_PATH + '/creative_mode', str(creative_mode).lower())
+
+    if enable_damage is not None:
+        aug.set(AUG_PATH + '/enable_damage', str(enable_damage).lower())
+
+    aug.save()
+    action_utils.service_try_restart('minetest-server')
+
+
+def load_augeas():
+    """Initialize Augeas."""
+    aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD +
+                        augeas.Augeas.NO_MODL_AUTOLOAD)
+    aug.set('/augeas/load/Php/lens', 'Php.lns')
+    aug.set('/augeas/load/Php/incl[last() + 1]', CONFIG_FILE)
+    aug.load()
+    return aug

plinth/modules/minetest/views.py

@@ -1,21 +1,19 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Views for minetest module."""
-Views for minetest module.
-"""
 
 from django.contrib import messages
 from django.utils.translation import gettext_lazy as _
 
-from plinth import actions
 from plinth.modules import names
 from plinth.views import AppView
 
-from . import get_configuration
+from . import get_configuration, privileged
 from .forms import MinetestForm
 
 
 class MinetestAppView(AppView):  # pylint: disable=too-many-ancestors
     """A specialized view for configuring minetest."""
+
     app_id = 'minetest'
     template_name = 'minetest.html'
     form_class = MinetestForm
@@ -37,35 +35,23 @@ class MinetestAppView(AppView):  # pylint: disable=too-many-ancestors
         """Change the configurations of Minetest service."""
         data = form.cleaned_data
         old_config = get_configuration()
-        updated = False
 
+        changes = {}
         if old_config['max_players'] != data['max_players'] \
            and data['max_players'] is not None:
-            actions.superuser_run(
+            changes['max_players'] = data['max_players']
-                'minetest',
-                ['configure', '--max_players',
-                 str(data['max_players'])])
-            updated = True
 
         if old_config['creative_mode'] != data['creative_mode']:
-            value = 'true' if data['creative_mode'] else 'false'
+            changes['creative_mode'] = data['creative_mode']
-            actions.superuser_run('minetest',
-                                  ['configure', '--creative_mode', value])
-            updated = True
 
         if old_config['enable_pvp'] != data['enable_pvp']:
-            value = 'true' if data['enable_pvp'] else 'false'
+            changes['enable_pvp'] = data['enable_pvp']
-            actions.superuser_run('minetest',
-                                  ['configure', '--enable_pvp', value])
-            updated = True
 
         if old_config['enable_damage'] != data['enable_damage']:
-            value = 'true' if data['enable_damage'] else 'false'
+            changes['enable_damage'] = data['enable_damage']
-            actions.superuser_run('minetest',
-                                  ['configure', '--enable_damage', value])
-            updated = True
 
-        if updated:
+        if changes:
+            privileged.configure(**changes)
             messages.success(self.request, _('Configuration updated'))
 
         return super().form_valid(form)