From b5d7a910dd80e2d82ae5a04f38d7eaa4ac55db3c Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Tue, 27 Aug 2019 23:42:16 -0700
Subject: [PATCH] backups: Simplify checking repository capabilities using
 flags

Also make storage_type an abstract property so that derived classes are forced
to override it.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
---
 plinth/modules/backups/repository.py          | 22 +++++++++++++------
 plinth/modules/backups/templates/backups.html |  6 ++---
 .../backups/templates/backups_repository.inc  |  9 ++++----
 plinth/modules/backups/views.py               |  2 +-
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/plinth/modules/backups/repository.py b/plinth/modules/backups/repository.py
index 79167223f..82a541411 100644
--- a/plinth/modules/backups/repository.py
+++ b/plinth/modules/backups/repository.py
@@ -18,11 +18,11 @@
 Remote and local Borg backup repositories
 """
 
+import abc
 import io
 import json
 import logging
 import os
-from abc import ABC
 from uuid import uuid1
 
 from django.utils.translation import ugettext_lazy as _
@@ -78,9 +78,10 @@ KNOWN_ERRORS = [{
                 }]
 
 
-class BaseBorgRepository(ABC):
+class BaseBorgRepository(abc.ABC):
     """Base class for all kinds of Borg repositories."""
     uuid = None
+    flags = {}
     is_mounted = True
 
     def __init__(self, uuid=None, path=None, credentials=None, **kwargs):
@@ -107,6 +108,11 @@ class BaseBorgRepository(ABC):
     def name(self):
         return self._path
 
+    @abc.abstractmethod
+    def storage_type(self):
+        """Return the storage type of repository."""
+        raise NotImplementedError
+
     @property
     def repo_path(self):
         """Return the repository that the backups action script should use."""
@@ -137,17 +143,17 @@ class BaseBorgRepository(ABC):
         """Get archives with additional information as needed by the view"""
         repository = {
             'name': self.name,
-            'type': self.storage_type,
-            'error': ''
+            'storage_type': self.storage_type,
+            'flags': self.flags,
+            'error': None,
         }
         try:
-            error = ''
             repository['mounted'] = self.is_mounted
             if repository['mounted']:
                 repository['archives'] = self.list_archives()
         except (BorgError, ActionError) as err:
-            error = str(err)
-        repository['error'] = error
+            repository['error'] = str(err)
+
         return repository
 
     def remove_repository(self):
@@ -308,6 +314,7 @@ class BorgRepository(BaseBorgRepository):
     """General Borg repository implementation."""
     KNOWN_CREDENTIALS = ['encryption_passphrase']
     storage_type = 'disk'
+    flags = {'removable': True}
 
     @property
     def name(self):
@@ -327,6 +334,7 @@ class SshBorgRepository(BaseBorgRepository):
         'ssh_keyfile', 'ssh_password', 'encryption_passphrase'
     ]
     storage_type = 'ssh'
+    flags = {'removable': True, 'mountable': True}
 
     @property
     def repo_path(self):
diff --git a/plinth/modules/backups/templates/backups.html b/plinth/modules/backups/templates/backups.html
index c499fb85c..fe98d864d 100644
--- a/plinth/modules/backups/templates/backups.html
+++ b/plinth/modules/backups/templates/backups.html
@@ -60,14 +60,14 @@
 
   <h3>{% trans 'Existing Backups' %}</h3>
 
-  {% include "backups_repository.inc" with repository=root_repository uuid='root' storage_type='root' editable=False %}
+  {% include "backups_repository.inc" with repository=root_repository uuid='root' %}
 
   {% for uuid,repository in ssh_repositories.items %}
-    {% include "backups_repository.inc" with editable=True storage_type='ssh' %}
+    {% include "backups_repository.inc" %}
   {% endfor %}
 
   {% for uuid,repository in disk_repositories.items %}
-    {% include "backups_repository.inc" with editable=True storage_type='disk' %}
+    {% include "backups_repository.inc" %}
   {% endfor %}
 
   <a title="{% trans 'Add a backup location' %}"
diff --git a/plinth/modules/backups/templates/backups_repository.inc b/plinth/modules/backups/templates/backups_repository.inc
index 3251743d9..2ce6a73b4 100644
--- a/plinth/modules/backups/templates/backups_repository.inc
+++ b/plinth/modules/backups/templates/backups_repository.inc
@@ -34,14 +34,10 @@
         {{ repository.name }}
 
         <span class="pull-right">
-          {% if editable %}
+          {% if repository.flags.mountable %}
 
-            {% if storage_type != 'disk' %}
             {% if repository.mounted %}
 
-              <!-- With GET redirects, the browser URL points to the
-                   redirected page (bad when reloading) - use POST instead.
-              -->
               <form action="{% url 'backups:repository-umount' uuid %}" method="POST"
                     class="inline-block" >
                 {% csrf_token %}
@@ -63,8 +59,11 @@
               </form>
 
             {% endif %}
+
           {% endif %}
 
+          {% if repository.flags.removable %}
+
             <a title="{% trans 'Remove Backup Location. This will not delete the remote backup.' %}"
                role="button" class="repository-remove btn btn-sm btn-default"
                href="{% url 'backups:repository-remove' uuid %}">
diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py
index aed1668a3..6529f8053 100644
--- a/plinth/modules/backups/views.py
+++ b/plinth/modules/backups/views.py
@@ -84,7 +84,7 @@ class CreateArchiveView(SuccessMessageMixin, FormView):
     def form_valid(self, form):
         """Create the archive on valid form submission."""
         repository = create_repository(form.cleaned_data['repository'])
-        if hasattr(repository, 'mount'):
+        if repository.flags.get('mountable'):
             repository.mount()
 
         name = datetime.now().strftime('%Y-%m-%d:%H:%M')