Store the repository ID as returned by SVNMan
authorSybren A. Stüvel <sybren@stuvel.eu>
Wed, 8 Nov 2017 11:59:09 +0000 (12:59 +0100)
committerSybren A. Stüvel <sybren@stuvel.eu>
Wed, 8 Nov 2017 11:59:09 +0000 (12:59 +0100)
This makes it possible for SVNMan to perform some manipulation on the
repository ID.

svnman/__init__.py
svnman/remote.py
svnman/routes.py
tests/test_api.py [new file with mode: 0644]
tests/test_pillar_extension.py

index 3d6f5cd..1ae4e75 100644 (file)
@@ -1,5 +1,6 @@
 import logging
 import os.path
+import string
 from urllib.parse import urljoin
 
 import flask
@@ -157,22 +158,17 @@ class SVNManExtension(PillarExtension):
 
         return bool(pprops.repo_id)
 
-    def create_repo(self, project_url: str, creator: str) -> str:
+    def create_repo(self, project: pillarsdk.Project, creator: str) -> str:
         """Creates a SVN repository with a random ID attached to the project.
 
         Saves the repository ID in the project. Is a no-op if the project
         already has a Subversion repository.
         """
 
-        import random
-        import string
-
         from . import remote, exceptions
 
-        alphabet = string.ascii_letters + string.digits
-
-        proj = proj_utils.get_project(project_url)
-        project_id = proj['_id']
+        project_id = project['_id']
+        proj = project.to_dict()
         eprops = proj.setdefault('extension_props', {}).setdefault(EXTENSION_NAME, {})
 
         repo_id = eprops.get('repo_id')
@@ -181,9 +177,6 @@ class SVNManExtension(PillarExtension):
                               project_id, repo_id)
             return repo_id
 
-        def random_id():
-            return ''.join([random.choice(alphabet) for _ in range(24)])
-
         repo_info = remote.CreateRepo(
             repo_id='',
             project_id=str(project_id),
@@ -191,10 +184,10 @@ class SVNManExtension(PillarExtension):
         )
 
         for _ in range(100):
-            repo_info.repo_id = random_id()
+            repo_info.repo_id = _random_id()
             self._log.info('creating new repository, trying out %s', repo_info)
             try:
-                self.remote.create_repo(repo_info)
+                actual_repo_id = self.remote.create_repo(repo_info)
             except exceptions.RepoAlreadyExists:
                 self._log.info('repo_id=%r already exists, trying random other one',
                                repo_info.repo_id)
@@ -207,10 +200,10 @@ class SVNManExtension(PillarExtension):
         self._log.info('created new Subversion repository: %s', repo_info)
 
         # Update the project to include the repository ID.
-        eprops['repo_id'] = repo_info.repo_id
+        eprops['repo_id'] = actual_repo_id
         proj_utils.put_project(proj)
 
-        return repo_info.repo_id
+        return actual_repo_id
 
     def delete_repo(self, project_url: str, repo_id: str):
         """Deletes an SVN repository and detaches it from the project."""
@@ -242,5 +235,19 @@ def _get_current_svnman() -> SVNManExtension:
     return flask.current_app.pillar_extensions[EXTENSION_NAME]
 
 
+def _random_id(alphabet=string.ascii_letters + string.digits) -> str:
+    """Returns a random repository ID.
+
+    IDs start with a lowercase-letters-only prefix so that any
+    prefix-based subdivision on the server doesn't need to
+    distinguish between too many different prefixes. It's
+    a bit ugly to do that here, but at least it works 🐙
+    """
+    import random
+
+    prefix = ''.join([random.choice(string.ascii_lowercase) for _ in range(2)])
+    return prefix + ''.join([random.choice(alphabet) for _ in range(22)])
+
+
 current_svnman: SVNManExtension = LocalProxy(_get_current_svnman)
 """SVNMan extension of the current app."""
index 1002ff4..8de4cfa 100644 (file)
@@ -71,11 +71,15 @@ class API:
 
         return RepoDescription(**resp.json())
 
-    def create_repo(self, create_repo: CreateRepo):
+    def create_repo(self, create_repo: CreateRepo) -> str:
         """Creates a new repository with the given ID.
 
+        Note that the repository ID may be changed by the SVNMan;
+        always use the repo ID as returned by this function.
+
         :param create_repo: info required by the API
         :raises svnman.exceptions.RepoAlreadyExists:
+        :returns: the repository ID as returned by the SVNMan.
         """
 
         self._log.info('Creating repository %r', create_repo)
@@ -84,6 +88,9 @@ class API:
             raise exceptions.RepoAlreadyExists(create_repo.repo_id)
         self._raise_for_status(resp)
 
+        repo_info = resp.json()
+        return repo_info['repo_id']
+
     def modify_access(self,
                       repo_id: str,
                       grant: typing.List[typing.Tuple[str, str]],
index 8983007..ddf1348 100644 (file)
@@ -5,6 +5,7 @@ import werkzeug.exceptions as wz_exceptions
 
 from pillar.api.utils.authorization import require_login
 from pillar.auth import current_user
+from pillar.web.projects.routes import project_view
 from pillar.web.utils import attach_project_pictures
 from pillar.web.system_util import pillar_api
 import pillarsdk
@@ -48,16 +49,17 @@ def error_project_not_available():
 
 @blueprint.route('/<project_url>/create-repo', methods=['POST'])
 @require_login(require_cap='svn-use')
-def create_repo(project_url: str):
+@project_view()
+def create_repo(project: pillarsdk.Project):
     log.info('going to create repository for project url=%r on behalf of user %s (%s)',
-             project_url, current_user.user_id, current_user.email)
+             project.url, current_user.user_id, current_user.email)
 
     from . import exceptions
 
     # TODO(sybren): check project access
 
     try:
-        current_svnman.create_repo(project_url, f'{current_user.full_name} <{current_user.email}>')
+        current_svnman.create_repo(project, f'{current_user.full_name} <{current_user.email}>')
     except (OSError, IOError):
         log.exception('unable to reach SVNman API')
         resp = jsonify(_message='unable to reach SVNman API server')
diff --git a/tests/test_api.py b/tests/test_api.py
new file mode 100644 (file)
index 0000000..c4483a6
--- /dev/null
@@ -0,0 +1,42 @@
+import responses
+
+from abstract_svnman_test import AbstractSVNManTest
+
+
+class TestAPI(AbstractSVNManTest):
+    @responses.activate
+    def test_create_repo(self):
+        from svnman.remote import CreateRepo
+        responses.add(responses.POST,
+                      'http://svnman_api_url/api/repo',
+                      json={'repo_id': 'something-completely-different'},
+                      status=201)
+
+        cr = CreateRepo(repo_id='UPPERCASE', project_id='someproject', creator='me <here@there>')
+        resp = self.remote.create_repo(cr)
+        self.assertEqual('something-completely-different', resp)
+
+    @responses.activate
+    def test_delete_repo(self):
+        responses.add(responses.DELETE,
+                      'http://svnman_api_url/api/repo/repo-id',
+                      status=204)
+        self.remote.delete_repo('repo-id')
+
+    @responses.activate
+    def test_fetch_repo(self):
+        from svnman.remote import RepoDescription
+        responses.add(responses.GET,
+                      'http://svnman_api_url/api/repo/repo-id',
+                      json={
+                          'repo_id': 'something-completely-different',
+                          'access': ['someuser', 'otheruser'],
+                      },
+                      status=200)
+
+        resp = self.remote.fetch_repo('repo-id')
+        expect = RepoDescription(
+            repo_id='something-completely-different',
+            access=['someuser', 'otheruser'],
+        )
+        self.assertEqual(expect, resp)
index c2789b7..5d37f05 100644 (file)
@@ -1,7 +1,9 @@
 import copy
+from unittest import mock
 
 import pillarsdk
 import pillar.tests
+import werkzeug.exceptions as wz_exceptions
 
 from abstract_svnman_test import AbstractSVNManTest
 
@@ -34,3 +36,36 @@ class TestPillarExtension(AbstractSVNManTest):
         project['extension_props'] = {'svnman': {'repo_id': 'something-random'}}
         conv()
         self.assertTrue(svn.is_svnman_project(sdk_project))
+
+    @mock.patch('svnman.remote.API.create_repo')
+    @mock.patch('svnman._random_id')
+    def test_create_repo_happy(self, mock_random_id, mock_create_repo):
+        from svnman import EXTENSION_NAME
+        from svnman.remote import CreateRepo
+        from svnman.exceptions import RepoAlreadyExists
+
+        self.enter_app_context()
+        self.login_api_as(24 * 'a', roles={'admin'})
+
+        mock_create_repo.side_effect = [RepoAlreadyExists('first-random-id'), 'new-repo-id']
+        mock_random_id.side_effect = ['first-random-id', 'second-random-id']
+
+        returned_repo_id = self.svnman.create_repo(
+            self.sdk_project,
+            'tester <tester@unittests.com>',
+        )
+
+        # Not checking for exact parameters for both calls, since the objects passed
+        # to the calls are modified in-place anyway, and wouldn't match a check
+        # after the fact.
+        cr2 = CreateRepo(repo_id='second-random-id',
+                         project_id=str(self.proj_id),
+                         creator='tester <tester@unittests.com>')
+        mock_create_repo.assert_called_with(cr2)
+
+        # This is the important bit: the final repository ID returned to the system
+        # and stored in the project document.
+        self.assertEqual('new-repo-id', returned_repo_id)
+
+        db_proj = self.fetch_project_from_db(self.proj_id)
+        self.assertEqual('new-repo-id', db_proj['extension_props'][EXTENSION_NAME]['repo_id'])