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 <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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)
 
 

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,
+from . import (_backup_handler, api, get_known_hosts_path,
-               _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):
+    def __init__(self, path, credentials=None, uuid=None, **kwargs):
-        """
+        """Instantiate a new repository.
-        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:
+    @classmethod
-            uuid = str(uuid1())
+    def load(cls, uuid):
-        self.uuid = 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:
+        return cls(path, credentials, uuid, **storage)
-            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()
 
     @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
+    name = format_lazy(_('{box_name} storage'), box_name=cfg.box_name)
-    borg_path = ROOT_REPOSITORY
+    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
+        super().__init__(self.PATH, credentials, self.UUID)
-        if credentials is None:
-            credentials = {}
-
-        self._path = path
-        self.credentials = credentials
 
     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:
+    if uuid == RootBorgRepository.UUID:
-        return RootBorgRepository(path=ROOT_REPOSITORY)
+        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)

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

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,
+    repository = SshBorgRepository(repo_url, _dummy_credentials)
-                                   credentials=_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,
+    repository = SshBorgRepository(repo_url, _dummy_credentials)
-                                   credentials=_dummy_credentials)
     with pytest.raises(backups.errors.BorgError):
         repository.get_info()
 

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: