Backups: clean up exporting archives functionality

Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/backups

@@ -21,7 +21,6 @@ Configuration helper for backups.
 """
 
 import argparse
-import glob
 import json
 import os
 import shutil
@@ -60,16 +59,11 @@ def parse_arguments():
     export_tar.add_argument('--name', help='Archive name', required=True)
     export_tar.add_argument('--filename', help='Tarball file name', required=True)
 
-    list_exports = subparsers.add_parser(
+    get_apps_of_exported_archive = subparsers.add_parser(
-        'list-exports', help='List exported backup archive files')
+        'get-apps-of-exported-archive',
-    list_exports.add_argument('--location', required=True,
-                              help='location to check')
-
-    get_export_apps = subparsers.add_parser(
-        'get-export-apps',
         help='Get list of apps included in exported archive file')
-    get_export_apps.add_argument(
+    get_apps_of_exported_archive.add_argument(
-        '--filename', help='Tarball file name', required=True)
+        '--path', help='Tarball file path', required=True)
 
     get_archive_apps = subparsers.add_parser(
         'get-archive-apps',
@@ -80,9 +74,8 @@ def parse_arguments():
     restore_exported_archive = subparsers.add_parser(
         'restore-exported-archive',
         help='Restore files from an exported archive')
-    # TODO: rename filename to filepath (or just path)
+    restore_exported_archive.add_argument('--path',
-    restore_exported_archive.add_argument('--filename',
+            help='Tarball file path', required=True)
-            help='Tarball file name', required=True)
 
     restore_archive = subparsers.add_parser(
         'restore-archive', help='Restore files from an archive')
@@ -184,21 +177,6 @@ def subcommand_export_tar(arguments):
         pass
 
 
-def subcommand_list_exports(arguments):
-    """List exported backup archive files."""
-    exports = []
-    path = arguments.location
-    if path[-1] != '/':
-        path += '/'
-
-    path += 'FreedomBox-backups/'
-    if os.path.exists(path):
-        for filename in glob.glob(path + '*.tar.gz'):
-            exports.append(os.path.basename(filename))
-
-    print(json.dumps(exports))
-
-
 def _read_archive_file(archive, filepath):
     """Read the content of a file inside an archive"""
     arguments = ['borg', 'extract', archive, filepath, '--stdout']
@@ -225,10 +203,10 @@ def subcommand_get_archive_apps(arguments):
             print(app['name'])
 
 
-def subcommand_get_export_apps(arguments):
+def subcommand_get_apps_of_exported_archive(arguments):
     """Get list of apps included in exported archive file."""
     manifest = None
-    with tarfile.open(arguments.filename) as t:
+    with tarfile.open(arguments.path) as t:
         filenames = t.getnames()
         for name in filenames:
             if 'var/lib/plinth/backups-manifests/' in name \
@@ -257,7 +235,7 @@ def subcommand_restore_exported_archive(arguments):
     locations_data = ''.join(sys.stdin)
     locations = json.loads(locations_data)
 
-    with tarfile.open(arguments.filename) as tar_handle:
+    with tarfile.open(arguments.path) as tar_handle:
         for member in tar_handle.getmembers():
             path = '/' + member.name
             if path in locations['files']:

plinth/modules/backups/__init__.py

@@ -25,9 +25,7 @@ from django.utils.text import get_valid_filename
 from django.utils.translation import ugettext_lazy as _
 
 from plinth import actions
-from plinth.errors import PlinthError
 from plinth.menu import main_menu
-from plinth.modules import storage
 
 from . import api
 
@@ -43,8 +41,6 @@ description = [
 
 service = None
 
-BACKUP_FOLDER_NAME = 'FreedomBox-backups'
-DEFAULT_BACKUP_LOCATION = ('/var/lib/freedombox/', _('Root Filesystem'))
 MANIFESTS_FOLDER = '/var/lib/plinth/backups-manifests/'
 REPOSITORY = '/var/lib/freedombox/borgbackup'
 SESSION_BACKUP_VARIABLE = 'fbx-backup-filestamp'
@@ -118,58 +114,9 @@ def delete_tmp_backup_file():
         os.remove(TMP_BACKUP_PATH)
 
 
-def export_archive(name, location, tmp_dir=False):
+def export_archive(name, filepath=TMP_BACKUP_PATH):
-    # TODO: find a better solution for distinguishing exports to /tmp
+    arguments = ['export-tar', '--name', name, '--filename', filepath]
-    if tmp_dir:
+    actions.superuser_run('backups', arguments)
-        filepath = TMP_BACKUP_PATH
-    else:
-        filename = get_valid_filename(name) + '.tar.gz'
-        filepath = get_exported_archive_path(location, filename)
-    # TODO: that's a full path, not a filename; rename argument
-    actions.superuser_run('backups',
-                          ['export-tar', '--name', name, '--filename', filepath])
-
-
-def get_export_locations():
-    """Return a list of storage locations for exported backup archives."""
-    locations = [DEFAULT_BACKUP_LOCATION]
-    if storage.is_running():
-        devices = storage.udisks2.list_devices()
-        for device in devices:
-            if 'mount_points' in device and len(device['mount_points']) > 0:
-                location = {
-                    'path': device['mount_points'][0],
-                    'label': device['label'] or device['device'],
-                    'device': device['device']
-                }
-                locations.append(location)
-
-    return locations
-
-
-def get_location_path(device, locations=None):
-    """Returns the location path given a disk label"""
-    if locations is None:
-        locations = get_export_locations()
-
-    for location in locations:
-        if location['device'] == device:
-            return location['path']
-
-    raise PlinthError('Could not find path of location %s' % device)
-
-
-def get_export_files():
-    """Return a dict of exported backup archives found in storage locations."""
-    locations = get_export_locations()
-    export_files = []
-    for location in locations:
-        output = actions.superuser_run(
-            'backups', ['list-exports', '--location', location['path']])
-        location['files'] = json.loads(output)
-        export_files.append(location)
-
-    return export_files
 
 
 def get_archive_path(archive_name):
@@ -177,17 +124,6 @@ def get_archive_path(archive_name):
     return "::".join([REPOSITORY, archive_name])
 
 
-def get_exported_archive_path(location, archive_name):
-    """Get path of an exported archive"""
-    return os.path.join(location, BACKUP_FOLDER_NAME, archive_name)
-
-
-def find_exported_archive(device, archive_name):
-    """Return the full path for the exported archive file."""
-    location_path = get_location_path(device)
-    return get_exported_archive_path(location_path, archive_name)
-
-
 def get_archive_apps(path):
     """Get list of apps included in an archive."""
     output = actions.superuser_run('backups',
@@ -195,10 +131,10 @@ def get_archive_apps(path):
     return output.splitlines()
 
 
-def get_export_apps(filename):
+def get_apps_of_exported_archive(path):
     """Get list of apps included in exported archive file."""
-    output = actions.superuser_run('backups',
+    arguments = ['get-apps-of-exported-archive', '--path', path]
-                                   ['get-export-apps', '--filename', filename])
+    output = actions.superuser_run('backups', arguments)
     return output.splitlines()
 
 
@@ -224,13 +160,6 @@ def restore_from_tmp(apps=None):
                      create_subvolume=False, backup_file=TMP_BACKUP_PATH)
 
 
-def restore_exported(device, archive_name, apps=None):
-    """Restore files from exported backup archive."""
-    filename = find_exported_archive(device, archive_name)
-    api.restore_apps(_restore_exported_archive_handler, app_names=apps,
-                     create_subvolume=False, backup_file=filename)
-
-
 def restore(archive_path, apps=None):
     """Restore files from a backup archive."""
     api.restore_apps(_restore_archive_handler, app_names=apps,

plinth/modules/backups/api.py

@@ -113,6 +113,7 @@ class Packet:
         """
         self.operation = operation
         self.scope = scope
+        # TODO: do we need root if we have the path?
         self.root = root
         self.apps = apps
         # TODO: label is an archive path -- rename

plinth/modules/backups/forms.py

@@ -18,16 +18,12 @@
 Forms for backups module.
 """
 
-import os
-
 from django import forms
 from django.core import validators
 from django.core.validators import FileExtensionValidator
 from django.utils.translation import ugettext, ugettext_lazy as _
 
 from . import api
-from . import get_export_locations, get_exported_archive_path, \
-        get_location_path
 
 
 def _get_app_choices(apps):
@@ -63,17 +59,18 @@ class CreateArchiveForm(forms.Form):
         self.fields['selected_apps'].initial = [app.name for app in apps]
 
 
-class ExportArchiveForm(forms.Form):
+class RestoreForm(forms.Form):
-    disk = forms.ChoiceField(
+    selected_apps = forms.MultipleChoiceField(
-        label=_('Disk'), widget=forms.RadioSelect(),
+        label=_('Restore apps'),
-        help_text=_('Disk or removable storage where the backup archive will '
+        help_text=_('Apps data to restore from the backup'),
-                    'be saved.'))
+        widget=forms.CheckboxSelectMultiple)
 
     def __init__(self, *args, **kwargs):
-        """Initialize the form with disk choices."""
+        """Initialize the form with selectable apps."""
+        apps = kwargs.pop('apps')
         super().__init__(*args, **kwargs)
-        self.fields['disk'].choices = [(location['device'], location['label'])
+        self.fields['selected_apps'].choices = _get_app_choices(apps)
-                                       for location in get_export_locations()]
+        self.fields['selected_apps'].initial = [app.name for app in apps]
 
 
 class RestoreFromTmpForm(forms.Form):
@@ -89,56 +86,6 @@ class RestoreFromTmpForm(forms.Form):
         self.fields['selected_apps'].initial = [app.name for app in apps]
 
 
-class RestoreForm(forms.Form):
-    selected_apps = forms.MultipleChoiceField(
-        label=_('Restore apps'),
-        help_text=_('Apps data to restore from the backup'),
-        widget=forms.CheckboxSelectMultiple)
-
-    def __init__(self, *args, **kwargs):
-        """Initialize the form with selectable apps."""
-        apps = kwargs.pop('apps')
-        super().__init__(*args, **kwargs)
-        self.fields['selected_apps'].choices = _get_app_choices(apps)
-        self.fields['selected_apps'].initial = [app.name for app in apps]
-
-
-class UploadForm(forms.Form):
-    location = forms.ChoiceField(
-        choices=(), label=_('Location'), initial='', widget=forms.Select(),
-        required=True, help_text=_('Location to upload the archive to'))
-    file = forms.FileField(
-        label=_('Upload File'), required=True, validators=[
-            FileExtensionValidator(['gz'],
-                                   'Backup files have to be in .tar.gz format')
-        ], help_text=_('Select the backup file you want to upload'))
-
-    def __init__(self, *args, **kwargs):
-        """Initialize the form with location choices."""
-        super().__init__(*args, **kwargs)
-        locations = get_export_locations()
-        # users should only be able to select a location name -- don't
-        # provide paths as a form input for security reasons
-        location_labels = [(location['device'], location['label'])
-                           for location in locations]
-        self.fields['location'].choices = location_labels
-
-    def clean(self):
-        """Check that the uploaded file does not yet exist."""
-        cleaned_data = super().clean()
-        file = cleaned_data.get('file')
-        location_device = cleaned_data.get('location')
-        location_path = get_location_path(location_device)
-        # if other errors occured before, 'file' won't be in cleaned_data
-        if (file and file.name):
-            filepath = get_exported_archive_path(location_path, file.name)
-            if os.path.exists(filepath):
-                raise forms.ValidationError(
-                    "File %s already exists" % file.name)
-            else:
-                self.cleaned_data.update({'filepath': filepath})
-
-
 class UploadToTmpForm(forms.Form):
     file = forms.FileField(label=_('Upload File'), required=True,
             validators=[FileExtensionValidator(['gz'],

plinth/modules/backups/templates/backups.html

@@ -39,31 +39,7 @@
     <p>{{ paragraph|safe }}</p>
   {% endfor %}
 
-  <h3>{% trans 'Backup archives' %}</h3>
+  <h3>{% trans 'Existing backups' %}</h3>
-
-  {% if available_apps %}
-    <p>
-      <a title="{% trans 'New backup' %}"
-         role="button" class="btn btn-primary"
-         href="{% url 'backups:create' %}">
-        {% trans 'New backup' %}
-      </a>
-    </p>
-  {% else %}
-    <p>
-      <a title="{% trans 'New backup' %}"
-         role="button" class="btn btn-primary disabled"
-         href="{% url 'backups:create' %}">
-        {% trans 'New backup' %}
-      </a>
-    </p>
-    <p>
-      {% blocktrans trimmed %}
-        No apps that support backup are currently installed. Backup can be
-        created after an app supporting backups is installed.
-      {% endblocktrans %}
-    </p>
-  {% endif %}
 
   {% if not archives %}
     <p>{% trans 'No archives currently exist.' %}</p>
@@ -82,12 +58,6 @@
           <tr id="archive-{{ archive.name }}" class="archive">
             <td class="archive-name">{{ archive.name }}</td>
             <td class="archive-operations">
-              <!-- TODO: use or remove
-              <a class="archive-export btn btn-sm btn-default"
-                 href="{% url 'backups:export' archive.name %}">
-                {% trans "Export" %}
-              </a>
-              -->
               <a class="archive-export btn btn-sm btn-default" target="_blank"
                  href="{% url 'backups:export-and-download' archive.name %}">
                 {% trans "Download" %}
@@ -108,49 +78,4 @@
     </table>
   {% endif %}
 
-  <h3>{% trans 'Existing backup files' %}</h3>
-  {% if not exports %}
-    <p>{% trans 'No existing backup files were found.' %}</p>
-  {% else %}
-    <table class="table table-bordered table-condensed table-striped"
-           id="exports-list">
-      <thead>
-        <tr>
-          <th>{% trans "Location" %}</th>
-          <th>{% trans "Name" %}</th>
-          <th></th>
-        </tr>
-      </thead>
-
-      <tbody>
-        {% for export in exports %}
-          {% for name in export.files %}
-            <tr id="export-{{ label }}-{{ name }}" class="export">
-              <td class="export-label">{{ export.label }}</td>
-              <td class="export-name">{{ name }}</td>
-              <td class="export-operations">
-                <a class="download btn btn-sm btn-default" target="_blank"
-                   href="{% url 'backups:download' export.device|urlencode:'' name|urlencode:'' %}">
-                  {% trans "Download" %}
-                </a>
-                <a class="restore btn btn-sm btn-default"
-                   href="{% url 'backups:restore' export.device|urlencode:'' name|urlencode:'' %}">
-                  {% trans "Restore" %}
-                </a>
-              </td>
-            </tr>
-          {% endfor %}
-        {% endfor %}
-      </tbody>
-    </table>
-  {% endif %}
-
-  <p>
-    <a title="{% trans 'Upload a backup file' %}"
-       role="button" class="btn btn-primary"
-       href="{% url 'backups:upload' %}">
-      {% trans 'Upload backup file' %}
-    </a>
-  </p>
-
 {% endblock %}

plinth/modules/backups/templates/backups_upload.html

@@ -27,6 +27,12 @@
 {% block content %}
 
   <h3>{{ title }}</h3>
+  <p>
+  {% blocktrans %}
+  You can choose the apps you wish to import after uploading a backup file.
+  {% endblocktrans %}
+  </p>
+  <br />
 
   <form class="form" enctype="multipart/form-data" method="post">
       {% csrf_token %}

plinth/modules/backups/urls.py

@@ -20,24 +20,18 @@ URLs for the backups module.
 
 from django.conf.urls import url
 
-from .views import IndexView, CreateArchiveView, DownloadArchiveView, \
+from .views import IndexView, CreateArchiveView, DeleteArchiveView, \
-    DeleteArchiveView, ExportArchiveView, RestoreView, UploadArchiveView, \
+        UploadArchiveView, ExportAndDownloadView, RestoreArchiveView, \
-    ExportAndDownloadView, RestoreArchiveView, RestoreFromTmpView
+        RestoreFromTmpView
 
 urlpatterns = [
     url(r'^sys/backups/$', IndexView.as_view(), name='index'),
     url(r'^sys/backups/create/$', CreateArchiveView.as_view(), name='create'),
-    url(r'^sys/backups/export/(?P<name>[^/]+)/$',
-        ExportArchiveView.as_view(), name='export'),
-    url(r'^sys/backups/download/(?P<device>[^/]+)/(?P<name>[^/]+)/$',
-        DownloadArchiveView.as_view(), name='download'),
     url(r'^sys/backups/export-and-download/(?P<name>[^/]+)/$',
         ExportAndDownloadView.as_view(), name='export-and-download'),
     url(r'^sys/backups/delete/(?P<name>[^/]+)/$',
         DeleteArchiveView.as_view(), name='delete'),
     url(r'^sys/backups/upload/$', UploadArchiveView.as_view(), name='upload'),
-    url(r'^sys/backups/restore/(?P<device>[^/]+)/(?P<name>[^/]+)/$',
-        RestoreView.as_view(), name='restore'),
     url(r'^sys/backups/restore-archive/(?P<name>[^/]+)/$',
         RestoreArchiveView.as_view(), name='restore-archive'),
     url(r'^sys/backups/restore-from-tmp/$',

plinth/modules/backups/views.py

@@ -35,7 +35,7 @@ from django.views.generic import View, FormView, TemplateView
 
 from plinth.modules import backups
 
-from . import api, find_exported_archive, TMP_BACKUP_PATH, forms, \
+from . import api, TMP_BACKUP_PATH, forms, \
         SESSION_BACKUP_VARIABLE, delete_tmp_backup_file
 
 # number of seconds an uploaded backup file should be kept/stored
@@ -47,6 +47,9 @@ subsubmenu = [{
 }, {
     'url': reverse_lazy('backups:upload'),
     'text': ugettext_lazy('Upload backup')
+}, {
+    'url': reverse_lazy('backups:create'),
+    'text': ugettext_lazy('Create backup')
 }]
 
 
@@ -61,7 +64,6 @@ class IndexView(TemplateView):
         context['description'] = backups.description
         context['info'] = backups.get_info()
         context['archives'] = backups.list_archives()
-        context['exports'] = backups.get_export_files()
         context['subsubmenu'] = subsubmenu
         apps = api.get_all_apps_for_backup()
         context['available_apps'] = [app.name for app in apps]
@@ -118,15 +120,6 @@ class DeleteArchiveView(SuccessMessageMixin, TemplateView):
         return redirect(reverse_lazy('backups:index'))
 
 
-class DownloadArchiveView(View):
-    """View to download an archive."""
-    def get(self, request, device, name):
-        device = unquote(device)
-        name = unquote(name)
-        filepath = find_exported_archive(device, name)
-        return _get_file_response(filepath, name)
-
-
 def _get_file_response(path, filename):
     """Read and return a downloadable file"""
     (content_type, encoding) = mimetypes.guess_type(filename)
@@ -155,7 +148,7 @@ class create_temporary_backup_file:
         self.path = TMP_BACKUP_PATH
 
     def __enter__(self):
-        backups.export_archive(self.name, self.path, tmp_dir=True)
+        backups.export_archive(self.name, self.path)
         return self.path
 
     def __exit__(self, type, value, traceback):
@@ -171,7 +164,7 @@ class UploadArchiveView(SuccessMessageMixin, FormView):
     def get_context_data(self, **kwargs):
         """Return additional context for rendering the template."""
         context = super().get_context_data(**kwargs)
-        context['title'] = _('Upload Backup File')
+        context['title'] = _('Upload and import a backup file')
         context['subsubmenu'] = subsubmenu
         return context
 
@@ -185,74 +178,6 @@ class UploadArchiveView(SuccessMessageMixin, FormView):
         return super().form_valid(form)
 
 
-class ExportArchiveView(SuccessMessageMixin, FormView):
-    """View to export an archive."""
-    form_class = forms.ExportArchiveForm
-    prefix = 'backups'
-    template_name = 'backups_form.html'
-    success_url = reverse_lazy('backups:index')
-    success_message = _('Archive exported.')
-
-    def get_context_data(self, **kwargs):
-        """Return additional context for rendering the template."""
-        context = super().get_context_data(**kwargs)
-        context['title'] = _('Export Archive')
-        context['archive'] = backups.get_archive(self.kwargs['name'])
-        if context['archive'] is None:
-            raise Http404
-
-        return context
-
-    def form_valid(self, form):
-        """Create the archive on valid form submission."""
-        backups.export_archive(self.kwargs['name'], form.cleaned_data['disk'])
-        return super().form_valid(form)
-
-
-class RestoreView(SuccessMessageMixin, FormView):
-    """View to restore files from an exported archive."""
-    form_class = forms.RestoreForm
-    prefix = 'backups'
-    template_name = 'backups_restore.html'
-    success_url = reverse_lazy('backups:index')
-    success_message = _('Restored files from backup.')
-
-    def _get_included_apps(self):
-        """Save some data used to instantiate the form."""
-        device = unquote(self.kwargs['device'])
-        name = unquote(self.kwargs['name'])
-        if self.kwargs['use_tmp_file'] == 'true':
-            filename = TMP_BACKUP_PATH
-        else:
-            filename = backups.find_exported_archive(device, name)
-        return backups.get_export_apps(filename)
-
-    def get_form_kwargs(self):
-        """Pass additional keyword args for instantiating the form."""
-        kwargs = super().get_form_kwargs()
-        included_apps = self._get_included_apps()
-        installed_apps = api.get_all_apps_for_backup()
-        kwargs['apps'] = [
-            app for app in installed_apps if app.name in included_apps
-        ]
-        return kwargs
-
-    def get_context_data(self, **kwargs):
-        """Return additional context for rendering the template."""
-        context = super().get_context_data(**kwargs)
-        context['title'] = _('Restore from backup')
-        context['device'] = unquote(self.kwargs['device'])
-        context['name'] = self.kwargs['name']
-        return context
-
-    def form_valid(self, form):
-        """Restore files from the archive on valid form submission."""
-        backups.restore_exported(
-            unquote(self.kwargs['device']), self.kwargs['name'],
-            form.cleaned_data['selected_apps'])
-        return super().form_valid(form)
-
-
 class RestoreFromTmpView(SuccessMessageMixin, FormView):
     """View to restore files from an exported archive.
     
@@ -273,7 +198,7 @@ class RestoreFromTmpView(SuccessMessageMixin, FormView):
 
     def _get_included_apps(self):
         """Save some data used to instantiate the form."""
-        return backups.get_export_apps(TMP_BACKUP_PATH)
+        return backups.get_apps_of_exported_archive(TMP_BACKUP_PATH)
 
     def get_form_kwargs(self):
         """Pass additional keyword args for instantiating the form."""