Handle errors during repo creation, don't leave bad repos around

Closes #165

repomaker/tests/views/test_repository.py

@@ -4,6 +4,8 @@ from unittest.mock import patch
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.urls import reverse
+from django.utils.translation import ugettext_lazy as _
+from fdroidserver.exception import BuildException
 from repomaker.models import App, Apk, ApkPointer, Repository
 from repomaker.views.repository import RepositoryCreateView, RepositoryForm, RepositoryView
 
@@ -85,6 +87,27 @@ class RepositoryTestCase(RmTestCase):
         self.assertTrue(isinstance(response.context['form'], RepositoryForm))
         self.assertFormError(response, 'form', 'description', 'This field is required.')
 
+    @patch('fdroidserver.common.genkeystore')
+    def test_create_fails(self, genkeystore):
+        # remember how many repositories we have
+        repo_count = Repository.objects.all().count()
+
+        # creating the key store will fail
+        genkeystore.side_effect = BuildException('keystore failure')
+
+        # post data for a new repository to be created
+        query = {'name': 'TestRepo', 'description': 'TestDescription'}
+        response = self.client.post(reverse('add_repo'), query)
+
+        # assert that error message is shown
+        self.assertContains(response, 'Error')
+        self.assertContains(response, 'keystore failure')
+        self.assertContains(response,
+                            _('There was an error creating the repository. Please try again!'))
+
+        # assert that repository was deleted
+        self.assertEqual(repo_count, Repository.objects.all().count())
+
     def test_list(self):
         response = self.client.get(reverse('index'))
         self.assertEqual(200, response.status_code)

repomaker/views/__init__.py

@@ -131,13 +131,17 @@ class AppScrollListView(ListView):
         raise NotImplementedError()
 
 
-class DatabaseLockedView(TemplateView):
+class ErrorView(TemplateView):
     request = None
-    template_name = 'repomaker/db_locked.html'
+    template_name = "repomaker/error.html"
 
     def post(self, request, *args, **kwargs):
         return self.get(request, *args, **kwargs)
 
+
+class DatabaseLockedView(ErrorView):
+    template_name = 'repomaker/db_locked.html'
+
     def get(self, request, *args, **kwargs):
         context = self.get_context_data(**kwargs)
         return self.render_to_response(context, status=408)

repomaker/views/repository.py

@@ -12,7 +12,7 @@ from django.views.generic.edit import CreateView, UpdateView, DeleteView
 
 from repomaker.models import Repository, App, Apk
 from repomaker.models.storage import StorageManager
-from . import BaseModelForm, AppScrollListView, LoginOrSingleUserRequiredMixin
+from . import BaseModelForm, AppScrollListView, LoginOrSingleUserRequiredMixin, ErrorView
 
 
 class RepositoryAuthorizationMixin(LoginOrSingleUserRequiredMixin, UserPassesTestMixin):
@@ -124,7 +124,13 @@ class RepositoryCreateView(LoginOrSingleUserRequiredMixin, CreateView):
         if len(storage) > 0:
             form.instance.url = storage[0].get_repo_url()
 
-        form.instance.create()  # generate repo, QR Code, etc. on disk
+        try:
+            form.instance.create()  # generate repo, QR Code, etc. on disk
+        except Exception as e:
+            logging.error('Creating repo failed: %s', e)
+            form.instance.delete()
+            error = _('There was an error creating the repository. Please try again!')
+            return ErrorView().dispatch(self.request, error=error + ' ' + str(e))
         return result