Refactor minetest server config

Only load augeas once to read the config.

CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
   and exit. It can be followed by "essential" or "optional" to only
   list those modules.
 - Added JS license web labels for LibreJS.
+- Added basic configuration form for Minetest server.
 
 ### Changed
 - frontpage: Show app logos instead of generic icons.

actions/minetest

@@ -30,16 +30,6 @@ from plinth import action_utils
 CONFIG_FILE = '/etc/minetest/minetest.conf'
 AUG_PATH = '/files' + CONFIG_FILE + '/.anon'
 
-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
-
-aug = load_augeas()
 
 def parse_arguments():
     """Return parsed command line arguments as dictionary"""
@@ -47,7 +37,8 @@ def parse_arguments():
     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('--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'],
@@ -57,42 +48,57 @@ def parse_arguments():
 
     return parser.parse_args()
 
+
 def subcommand_configure(arguments):
     """Configure Minetest."""
+    aug = load_augeas()
+
     if arguments.max_players:
-        set_max_players(arguments.max_players)
+        set_max_players(aug, arguments.max_players)
     if arguments.creative_mode:
-        set_creative_mode(arguments.creative_mode)
+        set_creative_mode(aug, arguments.creative_mode)
     if arguments.enable_pvp:
-        enable_pvp(arguments.enable_pvp)
+        enable_pvp(aug, arguments.enable_pvp)
     if arguments.enable_damage:
-        enable_damage(arguments.enable_damage)
+        enable_damage(aug, arguments.enable_damage)
+
     action_utils.service_restart('minetest-server')
 
-def set_max_players(max_players):
+
+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(choice):
+
+def enable_pvp(aug, choice):
     """Enables pvp"""
     aug.set(AUG_PATH + '/enable_pvp', choice)
-
     aug.save()
 
-def set_creative_mode(choice):
+
+def set_creative_mode(aug, choice):
     """Enables or disables creative mode"""
     aug.set(AUG_PATH + '/creative_mode', choice)
-
     aug.save()
 
-def enable_damage(choice):
+
+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()

plinth/modules/minetest/__init__.py

@@ -16,7 +16,7 @@
 #
 
 """
-Plinth module for minetest.
+Plinth module for Minetest server.
 """
 
 import augeas
@@ -44,8 +44,7 @@ managed_packages = ['minetest-server', 'minetest-mod-advspawning',
                     'minetest-mod-animalmaterials', 'minetest-mod-animals',
                     'minetest-mod-mesecons', 'minetest-mod-mobf-core',
                     'minetest-mod-mobf-trap', 'minetest-mod-moreblocks',
-                    'minetest-mod-nether', 'minetest-mod-torches',
+                    'minetest-mod-nether', 'minetest-mod-torches']
-                    ]
 
 title = _('Block Sandbox \n (Minetest)')
 
@@ -61,15 +60,6 @@ description = [
 CONFIG_FILE = '/etc/minetest/minetest.conf'
 AUG_PATH = '/files' + CONFIG_FILE + '/.anon'
 
-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 init():
     """Initialize the module."""
@@ -129,38 +119,48 @@ def diagnose():
     return results
 
 
-def get_max_players_value():
+def load_augeas():
-    """Return the current Max Players value."""
+    """Initialize Augeas."""
-    aug = load_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 get_max_players(aug):
+    """Return the maximum players allowed on the server at one time."""
     value = aug.get(AUG_PATH + '/max_users')
     if value:
         return int(value)
 
 
-def get_creative_mode_value():
+def is_creative_mode_enabled(aug):
-    """Return the current Creative mode value."""
+    """Return whether creative mode is enabled."""
-    aug = load_augeas()
     value = aug.get(AUG_PATH + '/creative_mode')
-    if value == "true":
+    return value == 'true'
-        return True
-    else:
-        return False
 
 
-def get_enable_pvp_value():
+def is_pvp_enabled(aug):
-    """Return the current Enable pvp value."""
+    """Return whether PVP is enabled."""
-    aug = load_augeas()
     value = aug.get(AUG_PATH + '/enable_pvp')
-    if value == "true":
+    return value == 'true'
-        return True
-    else:
-        return False
 
-def get_enable_damage_value():
+
-    """Return the current Enable damage value."""
+def is_damage_enabled(aug):
-    aug = load_augeas()
+    """Return whether damage is enabled."""
     value = aug.get(AUG_PATH + '/enable_damage')
-    if value == "true":
+    return value == 'true'
-        return True
+
-    else:
+
-        return False
+def get_configuration():
+    """Return the current configuration."""
+    aug = load_augeas()
+    conf = {
+        'max_players': get_max_players(aug),
+        'creative_mode': is_creative_mode_enabled(aug),
+        'enable_pvp': is_pvp_enabled(aug),
+        'enable_damage': is_damage_enabled(aug),
+    }
+    return conf

plinth/modules/minetest/forms.py

@@ -16,7 +16,7 @@
 #
 
 """
-Forms for miinetest module.
+Forms for minetest module.
 """
 
 from django import forms
@@ -24,27 +24,30 @@ from django.utils.translation import ugettext_lazy as _
 
 from plinth.forms import ServiceForm
 
+
 class MinetestForm(ServiceForm):
     """Minetest configuration form"""
     max_players = forms.IntegerField(
         label=_('Maximum number of players'),
         required=True,
         help_text=_('You can change the maximum number of players playing \
-                    minetest at a single instance of time'))
+                     minetest at a single instance of time'))
 
     creative_mode = forms.BooleanField(
         label=_('Enable creative mode'),
         required=False,
-        help_text=_('Creative Mode changes the rules of the game to make it\
+        help_text=_('Creative mode changes the rules of the game to make it \
-                    more suitable for creative gameplay, rather than challenging\
+                     more suitable for creative gameplay, rather than \
-                    “survival” gameplay.'))
+                     challenging "survival" gameplay.'))
 
     enable_pvp = forms.BooleanField(
-        label=_('Enable pvp'),
+        label=_('Enable PVP'),
         required=False,
-        help_text=_('Enabling Player Vs Player will cause players to damage \
+        help_text=_('Enabling Player Vs Player will allow players to damage \
                     other players'))
 
     enable_damage = forms.BooleanField(
         label=_('Enable damage'),
-        required=False)
+        required=False,
+        help_text=_('When disabled, players cannot die or receive damage of \
+                    any kind'))

plinth/modules/minetest/views.py

@@ -24,12 +24,9 @@ from django.utils.translation import ugettext_lazy as _
 
 from plinth import actions
 from plinth.views import ServiceView
-from plinth import action_utils
 
 
-from . import description, managed_services, get_max_players_value,\
+from . import description, managed_services, get_configuration
-              get_enable_pvp_value, get_creative_mode_value,\
-              get_enable_damage_value
 from .forms import MinetestForm
 
 
@@ -44,59 +41,44 @@ class MinetestServiceView(ServiceView): # pylint: disable=too-many-ancestors
     def get_initial(self):
         """Return the values to fill in the form."""
         initial = super().get_initial()
-
+        initial.update(get_configuration())
-        initial['max_players'] = get_max_players_value()
-        initial['creative_mode'] = get_creative_mode_value()
-        initial['enable_pvp'] = get_enable_pvp_value()
-        initial['enable_damage'] = get_enable_damage_value()
         return initial
 
     def form_valid(self, form):
         """Change the configurations of Minetest service."""
         data = form.cleaned_data
+        old_config = get_configuration()
 
-        if get_max_players_value() != data['max_players'] and data['max_players'] != None:
+        if old_config['max_players'] != data['max_players'] \
-                actions.superuser_run(
+           and data['max_players'] != None:
-                    'minetest',
+            actions.superuser_run(
-                    ['configure', '--max_players', str(data['max_players'])])
+                'minetest',
-                messages.success(self.request,
+                ['configure', '--max_players', str(data['max_players'])])
-                                 _('Maximum players configuration updated'))
+            messages.success(self.request,
+                             _('Maximum players configuration updated'))
 
-        if data['creative_mode'] is True:
+        if old_config['creative_mode'] != data['creative_mode']:
-            value = "true"
+            value = 'true' if data['creative_mode'] else 'false'
-        else:
-            value = "false"
-
-        if get_creative_mode_value() != data['creative_mode']:
             actions.superuser_run(
                 'minetest',
                 ['configure', '--creative_mode', value])
             messages.success(self.request,
                              _('Creative mode configuration updated'))
 
-        if data['enable_pvp'] is True:
+        if old_config['enable_pvp'] != data['enable_pvp']:
-            value = "true"
+            value = 'true' if data['enable_pvp'] else 'false'
-        else:
-            value = "false"
-
-        if get_enable_pvp_value() != data['enable_pvp']:
             actions.superuser_run(
                 'minetest',
                 ['configure', '--enable_pvp', value])
             messages.success(self.request,
-                             _('PvP configuration updated'))
+                             _('PVP configuration updated'))
 
-        if data['enable_damage'] is True:
+        if old_config['enable_damage'] != data['enable_damage']:
-            value = "true"
+            value = 'true' if data['enable_damage'] else 'false'
-        else:
-            value = "false"
-
-        if get_enable_damage_value() != data['enable_damage']:
             actions.superuser_run(
                 'minetest',
                 ['configure', '--enable_damage', value])
             messages.success(self.request,
                              _('Damage configuration updated'))
 
-
         return super().form_valid(form)