From 7467fa4553279eebd2a502f16a7f768a649c7d16 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 28 Aug 2019 16:24:05 -0700 Subject: [PATCH] backups: Separate repository loading from instantiation The constructor of the Repository object is being used for two distinct purposes. One is to load the object from database and other to instantiate it with parameter such that it can be saved to database. Separating the two usages to different methods simplifies code and parameter passing for consumers. Also turn some class specific constants from globals to class constants. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/backups/__init__.py | 8 +-- plinth/modules/backups/repository.py | 70 +++++++++----------- plinth/modules/backups/tests/test_api.py | 5 +- plinth/modules/backups/tests/test_backups.py | 10 ++- plinth/modules/backups/views.py | 8 +-- 5 files changed, 43 insertions(+), 58 deletions(-) diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index ab8bb0023..d355848b0 100644 --- a/plinth/modules/backups/__init__.py +++ b/plinth/modules/backups/__init__.py @@ -30,7 +30,6 @@ from django.utils.translation import ugettext_lazy as _ from plinth import actions from plinth import app as app_module from plinth import cfg, menu -from plinth.utils import format_lazy from . import api @@ -49,10 +48,6 @@ description = [ manual_page = 'Backups' MANIFESTS_FOLDER = '/var/lib/plinth/backups-manifests/' -ROOT_REPOSITORY = '/var/lib/freedombox/borgbackup' -ROOT_REPOSITORY_NAME = format_lazy(_('{box_name} storage'), - box_name=cfg.box_name) -ROOT_REPOSITORY_UUID = 'root' # session variable name that stores when a backup file should be deleted SESSION_PATH_VARIABLE = 'fbx-backups-upload-path' @@ -85,8 +80,9 @@ def init(): def setup(helper, old_version=None): """Install and configure the module.""" helper.install(managed_packages) + from . import repository helper.call('post', actions.superuser_run, 'backups', - ['setup', '--path', ROOT_REPOSITORY]) + ['setup', '--path', repository.RootBorgRepository.PATH]) helper.call('post', app.enable) diff --git a/plinth/modules/backups/repository.py b/plinth/modules/backups/repository.py index 49ad9d9c0..dbb80fcbb 100644 --- a/plinth/modules/backups/repository.py +++ b/plinth/modules/backups/repository.py @@ -27,11 +27,11 @@ from uuid import uuid1 from django.utils.translation import ugettext_lazy as _ -from plinth import actions +from plinth import actions, cfg from plinth.errors import ActionError +from plinth.utils import format_lazy -from . import (ROOT_REPOSITORY, ROOT_REPOSITORY_NAME, ROOT_REPOSITORY_UUID, - _backup_handler, api, get_known_hosts_path, +from . import (_backup_handler, api, get_known_hosts_path, restore_archive_handler, split_path, store) from .errors import BorgError, BorgRepositoryDoesNotExistError, SshfsError @@ -83,27 +83,27 @@ class BaseBorgRepository(abc.ABC): flags = {} is_mounted = True - def __init__(self, uuid=None, path=None, credentials=None, **kwargs): - """ - Instantiate a new repository. + def __init__(self, path, credentials=None, uuid=None, **kwargs): + """Instantiate a new repository. If only a uuid is given, load the values from kvstore. + """ + self._path = path + self.credentials = credentials or {} + self.uuid = uuid or str(uuid1()) self.kwargs = kwargs - if not uuid: - uuid = str(uuid1()) - self.uuid = uuid + @classmethod + def load(cls, uuid): + """Create instance by loading from database.""" + storage = dict(store.get(uuid)) + path = storage.pop('path') + storage.pop('uuid') + credentials = storage.setdefault('credentials', {}) + storage.pop('credentials') - if path and credentials: - self._path = path - self.credentials = credentials - else: - # Either a uuid, or both a path and credentials must be given - if not bool(uuid): - raise ValueError('Invalid arguments.') - else: - self._load_from_kvstore() + return cls(path, credentials, uuid, **storage) @property def name(self): @@ -139,14 +139,6 @@ class BaseBorgRepository(abc.ABC): return {} - def _load_from_kvstore(self): - storage = store.get(self.uuid) - try: - self.credentials = storage['credentials'] - except KeyError: - self.credentials = {} - self._path = storage['path'] - def get_info(self): """Return Borg information about a repository.""" output = self.run(['info', '--path', self.borg_path]) @@ -309,20 +301,18 @@ class BaseBorgRepository(abc.ABC): class RootBorgRepository(BaseBorgRepository): """Borg repository on the root filesystem.""" + UUID = 'root' + PATH = '/var/lib/freedombox/borgbackup' + storage_type = 'root' - name = ROOT_REPOSITORY_NAME - borg_path = ROOT_REPOSITORY + name = format_lazy(_('{box_name} storage'), box_name=cfg.box_name) + borg_path = PATH sort_order = 10 is_mounted = True - def __init__(self, path, credentials=None): + def __init__(self, credentials=None): """Initialize the repository object.""" - self.uuid = ROOT_REPOSITORY_UUID - if credentials is None: - credentials = {} - - self._path = path - self.credentials = credentials + super().__init__(self.PATH, credentials, self.UUID) def run(self, arguments): return self._run('backups', arguments) @@ -439,7 +429,7 @@ class SshBorgRepository(BaseBorgRepository): def get_repositories(): """Get all repositories of a given storage type.""" - repositories = [get_instance(ROOT_REPOSITORY_UUID)] + repositories = [get_instance(RootBorgRepository.UUID)] for uuid in store.get_storages(): repositories.append(get_instance(uuid)) @@ -448,11 +438,11 @@ def get_repositories(): def get_instance(uuid): """Create a local or SSH repository object instance.""" - if uuid == ROOT_REPOSITORY_UUID: - return RootBorgRepository(path=ROOT_REPOSITORY) + if uuid == RootBorgRepository.UUID: + return RootBorgRepository() storage = store.get(uuid) if storage['storage_type'] == 'ssh': - return SshBorgRepository(uuid=uuid) + return SshBorgRepository.load(uuid) - return BorgRepository(uuid=uuid) + return BorgRepository.load(uuid) diff --git a/plinth/modules/backups/tests/test_api.py b/plinth/modules/backups/tests/test_api.py index e760b3248..5b4bba46a 100644 --- a/plinth/modules/backups/tests/test_api.py +++ b/plinth/modules/backups/tests/test_api.py @@ -23,7 +23,7 @@ from unittest.mock import MagicMock, call, patch import pytest from django.core.files.uploadedfile import SimpleUploadedFile -from .. import ROOT_REPOSITORY, api, forms +from .. import api, forms, repository # pylint: disable=protected-access @@ -99,7 +99,8 @@ class TestBackupProcesses: def test_backup_apps(): """Test that backup_handler is called.""" backup_handler = MagicMock() - api.backup_apps(backup_handler, path=ROOT_REPOSITORY) + api.backup_apps(backup_handler, + path=repository.RootBorgRepository.PATH) backup_handler.assert_called_once() @staticmethod diff --git a/plinth/modules/backups/tests/test_backups.py b/plinth/modules/backups/tests/test_backups.py index 384a061c5..6000477b8 100644 --- a/plinth/modules/backups/tests/test_backups.py +++ b/plinth/modules/backups/tests/test_backups.py @@ -160,7 +160,7 @@ def test_sshfs_mount_password(): credentials = _get_credentials() ssh_path = test_config.backups_ssh_path - repository = SshBorgRepository(path=ssh_path, credentials=credentials) + repository = SshBorgRepository(ssh_path, credentials) repository.mount() assert repository.is_mounted repository.umount() @@ -173,7 +173,7 @@ def test_sshfs_mount_keyfile(): credentials = _get_credentials() ssh_path = test_config.backups_ssh_path - repository = SshBorgRepository(path=ssh_path, credentials=credentials) + repository = SshBorgRepository(ssh_path, credentials) repository.mount() assert repository.is_mounted repository.umount() @@ -183,8 +183,7 @@ def test_sshfs_mount_keyfile(): def test_access_nonexisting_url(): """Test accessing a non-existent URL.""" repo_url = "user@%s.com.au:~/repo" % str(uuid.uuid1()) - repository = SshBorgRepository(path=repo_url, - credentials=_dummy_credentials) + repository = SshBorgRepository(repo_url, _dummy_credentials) with pytest.raises(backups.errors.BorgRepositoryDoesNotExistError): repository.get_info() @@ -192,8 +191,7 @@ def test_access_nonexisting_url(): def test_inaccessible_repo_url(): """Test accessing an existing URL with wrong credentials.""" repo_url = 'user@heise.de:~/repo' - repository = SshBorgRepository(path=repo_url, - credentials=_dummy_credentials) + repository = SshBorgRepository(repo_url, _dummy_credentials) with pytest.raises(backups.errors.BorgError): repository.get_info() diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py index 7a01871d2..1b5fc39e8 100644 --- a/plinth/modules/backups/views.py +++ b/plinth/modules/backups/views.py @@ -264,7 +264,7 @@ class AddRepositoryView(SuccessMessageMixin, FormView): encryption_passphrase = None credentials = {'encryption_passphrase': encryption_passphrase} - repository = BorgRepository(path=path, credentials=credentials) + repository = BorgRepository(path, credentials) try: repository.get_info() except BorgRepositoryDoesNotExistError: @@ -298,7 +298,7 @@ class AddRemoteRepositoryView(SuccessMessageMixin, FormView): 'ssh_password': form.cleaned_data.get('ssh_password'), 'encryption_passphrase': encryption_passphrase } - repository = SshBorgRepository(path=path, credentials=credentials) + repository = SshBorgRepository(path, credentials) repository.save(verified=False) messages.success(self.request, _('Added new remote SSH repository.')) @@ -471,7 +471,7 @@ class RemoveRepositoryView(SuccessMessageMixin, TemplateView): def umount_repository(request, uuid): """View to unmount a remote SSH repository.""" - repository = SshBorgRepository(uuid=uuid) + repository = SshBorgRepository.load(uuid) repository.umount() if repository.is_mounted: messages.error(request, _('Unmounting failed!')) @@ -485,7 +485,7 @@ def mount_repository(request, uuid): if not get_instance(uuid).is_usable(): return redirect('backups:verify-ssh-hostkey', uuid=uuid) - repository = SshBorgRepository(uuid=uuid) + repository = SshBorgRepository.load(uuid) try: repository.mount() except Exception as err: