minidlna: Use privileged decorator for actions

Tests:

- Functional tests work
- Setting the media directory updates the configuration file. Newly set
  directory is shown on the app page after update.

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

plinth/modules/minidlna/__init__.py

@@ -4,7 +4,6 @@ FreedomBox app to configure minidlna.
 """
 from django.utils.translation import gettext_lazy as _
 
-from plinth import actions
 from plinth import app as app_module
 from plinth import frontpage, menu
 from plinth.daemon import Daemon
@@ -15,7 +14,7 @@ from plinth.modules.users.components import UsersAndGroups
 from plinth.package import Packages, install
 from plinth.utils import Version
 
-from . import manifest
+from . import manifest, privileged
 
 _description = [
     _('MiniDLNA is a simple media server software, with the aim of being '
@@ -30,13 +29,14 @@ _description = [
 
 
 class MiniDLNAApp(app_module.App):
-    """Freedombox app managing miniDlna"""
+    """Freedombox app managing miniDlna."""
+
     app_id = 'minidlna'
 
     _version = 2
 
     def __init__(self):
-        """Initialize the app components"""
+        """Initialize the app components."""
         super().__init__()
 
         groups = {'minidlna': _('Media streaming server')}
@@ -92,7 +92,7 @@ class MiniDLNAApp(app_module.App):
     def setup(self, old_version):
         """Install and configure the app."""
         super().setup(old_version)
-        actions.superuser_run('minidlna', ['setup'])
+        privileged.setup()
         if not old_version:
             self.enable()
 
@@ -106,18 +106,8 @@ class MiniDLNAApp(app_module.App):
         if Version(package['new_version']) > Version('1.4~'):
             return False
 
-        media_dir = get_media_dir()
+        media_dir = privileged.get_media_dir()
         install(['minidlna'], force_configuration='new')
-        set_media_dir(media_dir)
+        privileged.set_media_dir(media_dir)
 
         return True
-
-
-def get_media_dir():
-    """Return the currently set media directory."""
-    return actions.superuser_run('minidlna', ['get-media-dir'])
-
-
-def set_media_dir(media_dir):
-    """Set the media directory from which files will be scanned for sharing."""
-    actions.superuser_run('minidlna', ['set-media-dir', '--dir', media_dir])

plinth/modules/minidlna/privileged.py

@@ -1,9 +1,6 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
-Configuration actions for the minidlna server.
-"""
-import argparse
+"""Configure minidlna server."""
+
 import subprocess
 from os import chmod, fdopen, remove, stat
 from shutil import move
@@ -12,6 +9,7 @@ from tempfile import mkstemp
 import augeas
 
 from plinth import action_utils
+from plinth.actions import privileged
 from plinth.utils import grep
 
 CONFIG_PATH = '/etc/minidlna.conf'
@@ -22,23 +20,6 @@ fs.inotify.max_user_watches = 100000
 '''
 
 
-def parse_arguments():
-    """Return parsed command line arguments as dictionary."""
-    parser = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    subparsers.add_parser('setup', help='Setup SSH server')
-
-    subparsers.add_parser('get-media-dir', help='Get media directory')
-
-    set_media_dir = subparsers.add_parser('set-media-dir',
-                                          help='Set custom media directory')
-    set_media_dir.add_argument('--dir')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
 def _undo_old_configuration_changes():
     """Restore /etc/sysctl.conf to before our changes.
 
@@ -59,10 +40,11 @@ def _undo_old_configuration_changes():
         aug.save()
 
 
-def subcommand_setup(_):
-    """
-    Increase inotify watches per folder to allow minidlna to
-    monitor changes in large media-dirs.
+@privileged
+def setup():
+    """Increase inotify watches per folder.
+
+    This is to allow minidlna to monitor changes in large media-dirs.
     """
     _undo_old_configuration_changes()
     with open('/etc/sysctl.d/50-freedombox-minidlna.conf', 'w',
@@ -72,29 +54,31 @@ def subcommand_setup(_):
     subprocess.run(['systemctl', 'restart', 'systemd-sysctl'], check=True)
 
 
-def subcommand_get_media_dir(_):
-    """Retrieve media directory from minidlna.conf"""
+@privileged
+def get_media_dir() -> str:
+    """Retrieve media directory from minidlna.conf."""
     line = grep('^media_dir=', CONFIG_PATH)
-
-    print(line[0].split("=")[1])
+    return line[0].split('=')[1]
 
 
-def subcommand_set_media_dir(arguments):
-    """Set media directory in minidlna.conf"""
+@privileged
+def set_media_dir(media_dir: str):
+    """Set media directory in minidlna.conf."""
     line = grep('^media_dir=', CONFIG_PATH)[0]
 
-    new_line = 'media_dir=%s\n' % arguments.dir
+    new_line = 'media_dir=%s\n' % media_dir
     replace_in_config_file(CONFIG_PATH, line, new_line)
     if action_utils.service_is_running('minidlna'):
         action_utils.service_restart('minidlna')
 
 
 def replace_in_config_file(file_path, pattern, subst):
-    """
-    Create a temporary minidlna.conf file,
-    replace the media dir config,
-    remove original one and move the temporary file.
-    Preserve permissions as the original file.
+    """Replace a directive in configuration file.
+
+    - Create a temporary minidlna.conf file
+    - Replace the media dir config
+    - Remove original one and move the temporary file
+    - Preserve permissions as the original file
     """
     temp_file, temp_file_path = mkstemp()
     with fdopen(temp_file, 'w') as new_file:
@@ -106,16 +90,3 @@ def replace_in_config_file(file_path, pattern, subst):
     remove(file_path)
     move(temp_file_path, file_path)
     chmod(file_path, old_st_mode)
-
-
-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/minidlna/views.py

@@ -1,31 +1,31 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
-Views for the minidlna module
-"""
+"""Views for the minidlna module."""
+
 import os
 
 from django.contrib import messages
 from django.utils.translation import gettext_lazy as _
 
-from plinth.modules import minidlna
 from plinth.views import AppView
 
+from . import privileged
 from .forms import MiniDLNAServerForm
 
 
 class MiniDLNAAppView(AppView):
+    """Show minidlna app view."""
+
     app_id = 'minidlna'
     form_class = MiniDLNAServerForm
 
     def get_initial(self):
-        """Initial form value as found in the minidlna.conf"""
+        """Return initial values of the form."""
         initial = super().get_initial()
-        initial.update({'media_dir': minidlna.get_media_dir()})
-
+        initial.update({'media_dir': privileged.get_media_dir()})
         return initial
 
     def form_valid(self, form):
-        """Apply changes from the form"""
+        """Apply changes from the form."""
         old_config = form.initial
         new_config = form.cleaned_data
 
@@ -34,7 +34,7 @@ class MiniDLNAAppView(AppView):
                 messages.error(self.request,
                                _('Specified directory does not exist.'))
             else:
-                minidlna.set_media_dir(new_config['media_dir'])
+                privileged.set_media_dir(new_config['media_dir'])
                 messages.success(self.request, _('Updated media directory'))
 
         return super().form_valid(form)