From b92b66b7e91c4caf6d8654518a003b26a8ef57fe Mon Sep 17 00:00:00 2001 From: Michael Pimmer Date: Fri, 12 Oct 2018 00:36:42 +0000 Subject: [PATCH] Backups: clean up forms, names and templates Reviewed-by: James Valleroy --- actions/backups | 18 ++- plinth/modules/backups/__init__.py | 31 ++--- plinth/modules/backups/api.py | 6 +- plinth/modules/backups/forms.py | 18 +-- plinth/modules/backups/middleware.py | 7 +- .../backups/templates/backups_restore.html | 32 ++---- .../templates/backups_restore_from_tmp.html | 45 -------- plinth/modules/backups/tests/test_api.py | 26 +---- plinth/modules/backups/urls.py | 6 +- plinth/modules/backups/views.py | 108 ++++++++---------- 10 files changed, 99 insertions(+), 198 deletions(-) delete mode 100644 plinth/modules/backups/templates/backups_restore_from_tmp.html diff --git a/actions/backups b/actions/backups index 1e5637d22..46474abe8 100755 --- a/actions/backups +++ b/actions/backups @@ -56,15 +56,14 @@ def parse_arguments(): export_tar = subparsers.add_parser('export-tar', help='Export archive contents as tarball') - export_tar.add_argument('--archive', help='Archive name', - required=True) + export_tar.add_argument('--name', help='Archive name)', required=True) export_tar.add_argument('--filepath', help='Destination tarball file path', required=True) - get_apps_of_exported_archive = subparsers.add_parser( - 'get-apps-of-exported-archive', + get_exported_archive_apps = subparsers.add_parser( + 'get-exported-archive-apps', help='Get list of apps included in exported archive file') - get_apps_of_exported_archive.add_argument( + get_exported_archive_apps.add_argument( '--path', help='Tarball file path', required=True) get_archive_apps = subparsers.add_parser( @@ -166,7 +165,7 @@ def subcommand_export_tar(arguments): os.makedirs(directory) subprocess.run([ - 'borg', 'export-tar', REPOSITORY + '::' + arguments.archive, + 'borg', 'export-tar', REPOSITORY + '::' + arguments.name, arguments.filepath ], check=True) @@ -202,8 +201,8 @@ def subcommand_get_archive_apps(arguments): print(app['name']) -def subcommand_get_apps_of_exported_archive(arguments): - """Get list of apps included in exported archive file.""" +def subcommand_get_exported_archive_apps(arguments): + """Get list of apps included in an exported archive file.""" manifest = None with tarfile.open(arguments.path) as t: filenames = t.getnames() @@ -225,8 +224,7 @@ def subcommand_restore_archive(arguments): _locations = json.loads(locations_data) locations = _locations['directories'] + _locations['files'] locations = [os.path.relpath(location, '/') for location in locations] - _extract(arguments.path, arguments.destination, - locations=locations) + _extract(arguments.path, arguments.destination, locations=locations) def subcommand_restore_exported_archive(arguments): diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index e2b81a039..ae250f8a4 100644 --- a/plinth/modules/backups/__init__.py +++ b/plinth/modules/backups/__init__.py @@ -43,10 +43,10 @@ service = None MANIFESTS_FOLDER = '/var/lib/plinth/backups-manifests/' REPOSITORY = '/var/lib/freedombox/borgbackup' -SESSION_BACKUP_VARIABLE = 'fbx-backup-filestamp' -# default backup path for temporary actions like imports or download -TMP_BACKUP_PATH = '/tmp/freedombox-backup.tar.gz' # session variable name that stores when a backup file should be deleted +SESSION_BACKUP_VARIABLE = 'fbx-backup-filestamp' +# default backup path for temporary backup files during down- or upload +UPLOAD_BACKUP_PATH = '/tmp/freedombox-backup.tar.gz' def init(): @@ -105,17 +105,20 @@ def create_archive(name, app_names): def delete_archive(name): - # TODO: is name actually a path? actions.superuser_run('backups', ['delete', '--name', name]) -def delete_tmp_backup_file(): - if os.path.isfile(TMP_BACKUP_PATH): - os.remove(TMP_BACKUP_PATH) +def delete_upload_backup_file(): + if os.path.isfile(UPLOAD_BACKUP_PATH): + os.remove(UPLOAD_BACKUP_PATH) -def export_archive(name, filepath=TMP_BACKUP_PATH): - arguments = ['export-tar', '--archive', name, '--filepath', filepath] +def export_archive(name, filepath=UPLOAD_BACKUP_PATH): + """Export an archive as .tar.gz file + + name: name of the repository (w/o path) + """ + arguments = ['export-tar', '--name', name, '--filepath', filepath] actions.superuser_run('backups', arguments) @@ -131,9 +134,9 @@ def get_archive_apps(path): return output.splitlines() -def get_apps_of_exported_archive(path): +def get_exported_archive_apps(path): """Get list of apps included in exported archive file.""" - arguments = ['get-apps-of-exported-archive', '--path', path] + arguments = ['get-exported-archive-apps', '--path', path] output = actions.superuser_run('backups', arguments) return output.splitlines() @@ -154,10 +157,10 @@ def _restore_archive_handler(packet): packet.label, '--destination', '/'], input=locations_data.encode()) -def restore_from_tmp(apps=None): - """Restore files from temporary backup file""" +def restore_from_upload(apps=None): + """Restore files from (uploaded) eported backup file""" api.restore_apps(_restore_exported_archive_handler, app_names=apps, - create_subvolume=False, backup_file=TMP_BACKUP_PATH) + create_subvolume=False, backup_file=UPLOAD_BACKUP_PATH) def restore(archive_path, apps=None): diff --git a/plinth/modules/backups/api.py b/plinth/modules/backups/api.py index 2ee54d0d9..bdacd55df 100644 --- a/plinth/modules/backups/api.py +++ b/plinth/modules/backups/api.py @@ -110,13 +110,15 @@ class Packet: All paths populated are relative to the 'root' path. The root path itself must not be stored in the backup. + label is either an archive name (w/o path), or the full path of an + exported archive. + TODO: create two variables out of it as it's distinct information. + """ 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 self.label = label self.errors = [] diff --git a/plinth/modules/backups/forms.py b/plinth/modules/backups/forms.py index f64299825..245120711 100644 --- a/plinth/modules/backups/forms.py +++ b/plinth/modules/backups/forms.py @@ -61,8 +61,7 @@ class CreateArchiveForm(forms.Form): class RestoreForm(forms.Form): selected_apps = forms.MultipleChoiceField( - label=_('Restore apps'), - help_text=_('Apps data to restore from the backup'), + label=_('Select the apps you want to restore'), widget=forms.CheckboxSelectMultiple) def __init__(self, *args, **kwargs): @@ -73,20 +72,7 @@ class RestoreForm(forms.Form): self.fields['selected_apps'].initial = [app.name for app in apps] -class RestoreFromTmpForm(forms.Form): - selected_apps = forms.MultipleChoiceField( - label=_('Restore apps'), - 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 UploadToTmpForm(forms.Form): +class UploadForm(forms.Form): file = forms.FileField(label=_('Upload File'), required=True, validators=[FileExtensionValidator(['gz'], 'Backup files have to be in .tar.gz format')], diff --git a/plinth/modules/backups/middleware.py b/plinth/modules/backups/middleware.py index 566e447cc..b9999dc36 100644 --- a/plinth/modules/backups/middleware.py +++ b/plinth/modules/backups/middleware.py @@ -16,8 +16,7 @@ # """ -Django middleware to redirect to firstboot wizard if it has not be run -yet. +Django middleware to occasionally delete temporary backup files """ import logging @@ -41,8 +40,8 @@ class BackupsMiddleware(MiddlewareMixin): if request.session.has_key(backups.SESSION_BACKUP_VARIABLE): now = time.time() if now > request.session[backups.SESSION_BACKUP_VARIABLE]: - backups.delete_tmp_backup_file() + backups.delete_upload_backup_file() del request.session[backups.SESSION_BACKUP_VARIABLE] else: - backups.delete_tmp_backup_file() + backups.delete_upload_backup_file() return diff --git a/plinth/modules/backups/templates/backups_restore.html b/plinth/modules/backups/templates/backups_restore.html index e986ef289..5a8ac624f 100644 --- a/plinth/modules/backups/templates/backups_restore.html +++ b/plinth/modules/backups/templates/backups_restore.html @@ -24,24 +24,11 @@ {% block content %}

{{ title }}

-

{% trans "Restore data from this archive?" %}

- -
-
- - - - - - - - - - - -
{% trans "Location" %}{% trans "Name" %}
{{ label }}{{ name }}
-
-
+ {% if name %} +

+ {% trans 'Restore data from' %} {{ name }} +

+ {% endif %}

@@ -49,10 +36,15 @@ {{ form|bootstrap }} - + + {% trans 'Abort' %} +

diff --git a/plinth/modules/backups/templates/backups_restore_from_tmp.html b/plinth/modules/backups/templates/backups_restore_from_tmp.html deleted file mode 100644 index 79d442438..000000000 --- a/plinth/modules/backups/templates/backups_restore_from_tmp.html +++ /dev/null @@ -1,45 +0,0 @@ -{% extends "base.html" %} -{% comment %} -# -# This file is part of FreedomBox. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as -# published by the Free Software Foundation, either version 3 of the -# License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . -# -{% endcomment %} - -{% load bootstrap %} -{% load i18n %} - -{% block content %} -

{{ title }}

- -

-

- {% csrf_token %} - - {{ form|bootstrap }} - - - - {% trans 'Abort' %} - -
-

- -{% endblock %} diff --git a/plinth/modules/backups/tests/test_api.py b/plinth/modules/backups/tests/test_api.py index e94bf14a8..b93c1c4ad 100644 --- a/plinth/modules/backups/tests/test_api.py +++ b/plinth/modules/backups/tests/test_api.py @@ -24,9 +24,8 @@ from unittest.mock import MagicMock, call, patch from django.core.files.uploadedfile import SimpleUploadedFile from plinth import cfg, module_loader -from plinth.errors import PlinthError -from .. import api, forms, get_location_path +from .. import api, forms # pylint: disable=protected-access @@ -244,35 +243,16 @@ class TestBackupProcesses(unittest.TestCase): class TestBackupModule(unittest.TestCase): """Tests of the backups django module, like views or forms.""" - def test_get_location_path(self): - """Test the 'get_location_path' method""" - locations = [{ - 'path': '/var/www', - 'device': '/dummy/device' - }, { - 'path': '/etc', - 'device': '/dangerous' - }] - location_path = get_location_path('/dummy/device', locations) - self.assertEqual(location_path, locations[0]['path']) - # verify that an unknown location raises an error - with self.assertRaises(PlinthError): - get_location_path('/unknown/device', locations) - def test_file_upload(self): - locations = get_export_locations() - location_name = locations[0]['device'] - post_data = {'location': location_name} - # posting a video should fail video_file = SimpleUploadedFile("video.mp4", b"file_content", content_type="video/mp4") - form = forms.UploadForm(post_data, {'file': video_file}) + form = forms.UploadForm({}, {'file': video_file}) self.assertFalse(form.is_valid()) # posting an archive file should work archive_file = SimpleUploadedFile("backup.tar.gz", b"file_content", content_type="application/gzip") - form = forms.UploadForm(post_data, {'file': archive_file}) + form = forms.UploadForm({}, {'file': archive_file}) form.is_valid() self.assertTrue(form.is_valid()) diff --git a/plinth/modules/backups/urls.py b/plinth/modules/backups/urls.py index 6c08cf581..de6ec073d 100644 --- a/plinth/modules/backups/urls.py +++ b/plinth/modules/backups/urls.py @@ -22,7 +22,7 @@ from django.conf.urls import url from .views import IndexView, CreateArchiveView, DeleteArchiveView, \ UploadArchiveView, ExportAndDownloadView, RestoreArchiveView, \ - RestoreFromTmpView + RestoreFromUploadView urlpatterns = [ url(r'^sys/backups/$', IndexView.as_view(), name='index'), @@ -34,6 +34,6 @@ urlpatterns = [ url(r'^sys/backups/upload/$', UploadArchiveView.as_view(), name='upload'), url(r'^sys/backups/restore-archive/(?P[^/]+)/$', RestoreArchiveView.as_view(), name='restore-archive'), - url(r'^sys/backups/restore-from-tmp/$', - RestoreFromTmpView.as_view(), name='restore-from-tmp'), + url(r'^sys/backups/restore-from-upload/$', + RestoreFromUploadView.as_view(), name='restore-from-upload'), ] diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py index 33f2adfa8..d92187302 100644 --- a/plinth/modules/backups/views.py +++ b/plinth/modules/backups/views.py @@ -35,8 +35,8 @@ from django.views.generic import View, FormView, TemplateView from plinth.modules import backups -from . import api, TMP_BACKUP_PATH, forms, \ - SESSION_BACKUP_VARIABLE, delete_tmp_backup_file +from . import api, UPLOAD_BACKUP_PATH, forms, \ + SESSION_BACKUP_VARIABLE, delete_upload_backup_file # number of seconds an uploaded backup file should be kept/stored KEEP_UPLOADED_BACKUP_FOR = 60*10 @@ -145,32 +145,32 @@ class create_temporary_backup_file: def __init__(self, name): self.name = name - self.path = TMP_BACKUP_PATH + self.path = UPLOAD_BACKUP_PATH def __enter__(self): backups.export_archive(self.name, self.path) return self.path def __exit__(self, type, value, traceback): - delete_tmp_backup_file() + delete_upload_backup_file() class UploadArchiveView(SuccessMessageMixin, FormView): - form_class = forms.UploadToTmpForm + form_class = forms.UploadForm prefix = 'backups' template_name = 'backups_upload.html' - success_url = reverse_lazy('backups:restore-from-tmp') + success_url = reverse_lazy('backups:restore-from-upload') def get_context_data(self, **kwargs): """Return additional context for rendering the template.""" context = super().get_context_data(**kwargs) - context['title'] = _('Upload and import a backup file') + context['title'] = _('Upload and restore a backup file') context['subsubmenu'] = subsubmenu return context def form_valid(self, form): """store uploaded file.""" - with open(TMP_BACKUP_PATH, 'wb+') as destination: + with open(UPLOAD_BACKUP_PATH, 'wb+') as destination: for chunk in self.request.FILES['backups-file'].chunks(): destination.write(chunk) self.request.session[SESSION_BACKUP_VARIABLE] = time.time() + \ @@ -178,51 +178,7 @@ class UploadArchiveView(SuccessMessageMixin, FormView): return super().form_valid(form) -class RestoreFromTmpView(SuccessMessageMixin, FormView): - """View to restore files from an exported archive. - - TODO: combine with RestoreView""" - # TODO: display more information about the backup, like the date - form_class = forms.RestoreFromTmpForm - prefix = 'backups' - template_name = 'backups_restore_from_tmp.html' - success_url = reverse_lazy('backups:index') - success_message = _('Restored files from backup.') - - def get(self, *args, **kwargs): - if not os.path.isfile(TMP_BACKUP_PATH): - messages.error(self.request, _('No backup file found.')) - return redirect(reverse_lazy('backups:index')) - else: - return super().get(*args, **kwargs) - - def _get_included_apps(self): - """Save some data used to instantiate the form.""" - return backups.get_apps_of_exported_archive(TMP_BACKUP_PATH) - - 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 data') - return context - - def form_valid(self, form): - """Restore files from the archive on valid form submission.""" - backups.restore_from_tmp(form.cleaned_data['selected_apps']) - return super().form_valid(form) - - -class RestoreArchiveView(SuccessMessageMixin, FormView): +class BaseRestoreView(SuccessMessageMixin, FormView): """View to restore files from an archive.""" form_class = forms.RestoreForm prefix = 'backups' @@ -230,12 +186,6 @@ class RestoreArchiveView(SuccessMessageMixin, FormView): 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.""" - name = unquote(self.kwargs['name']) - archive_path = backups.get_archive_path(name) - return backups.get_archive_apps(archive_path) - def get_form_kwargs(self): """Pass additional keyword args for instantiating the form.""" kwargs = super().get_form_kwargs() @@ -249,10 +199,46 @@ class RestoreArchiveView(SuccessMessageMixin, FormView): 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['name'] = self.kwargs['name'] + context['title'] = _('Restore') + context['name'] = self.kwargs.get('name', None) return context + +class RestoreFromUploadView(BaseRestoreView): + """View to restore files from an (uploaded) exported archive.""" + + def get(self, *args, **kwargs): + if not os.path.isfile(UPLOAD_BACKUP_PATH): + messages.error(self.request, _('No backup file found.')) + return redirect(reverse_lazy('backups:index')) + else: + return super().get(*args, **kwargs) + + def get_context_data(self, **kwargs): + """Return additional context for rendering the template.""" + context = super().get_context_data(**kwargs) + context['title'] = _('Restore from uploaded file') + return context + + def _get_included_apps(self): + """Save some data used to instantiate the form.""" + return backups.get_exported_archive_apps(UPLOAD_BACKUP_PATH) + + def form_valid(self, form): + """Restore files from the archive on valid form submission.""" + backups.restore_from_upload(form.cleaned_data['selected_apps']) + return super().form_valid(form) + + +class RestoreArchiveView(BaseRestoreView): + """View to restore files from an archive.""" + + def _get_included_apps(self): + """Save some data used to instantiate the form.""" + name = unquote(self.kwargs['name']) + archive_path = backups.get_archive_path(name) + return backups.get_archive_apps(archive_path) + def form_valid(self, form): """Restore files from the archive on valid form submission.""" archive_path = backups.get_archive_path(self.kwargs['name'])