Move Upstream Host type-specific code into modules

* Introduce UpnstreamHost abstract base class
* Define a subclass of this for each Upstream Host type
* Define a name-type map for these in the lorrycontroller package,
* Use these classes to replace type-specific code elsewhere

Related to #5.

7 files changed, 184 insertions, 102 deletions

diff --git a/lorrycontroller/__init__.py b/lorrycontroller/__init__.py
index c00e142..ddc2f74 100644
--- a/lorrycontroller/__init__.py
+++ b/lorrycontroller/__init__.py
@@ -37,14 +37,8 @@ from .removejob import RemoveJob
 from .lsupstreams import LsUpstreams, ForceLsUpstream
 from .pretendtime import PretendTime
 from .maxjobs import GetMaxJobs, SetMaxJobs
-from .gitano import (
-    GitanoCommand,
-    LocalTroveGitanoCommand,
-    GitanoCommandFailure,
-    new_gitano_command)
 from .static import StaticFile
 from .proxy import setup_proxy
-from .gitlab import Gitlab
 from . import gerrit
 from . import gitano
 from . import gitlab
@@ -59,4 +53,14 @@ downstream_types = {
 }
 
 
+upstream_types = {
+    'gitlab': gitlab.GitlabUpstream,
+    'trove':  gitano.TroveUpstream,
+}
+
+
+def get_upstream_host(host_info):
+    return upstream_types[host_info['type']](host_info)
+
+
 __all__ = locals()
diff --git a/lorrycontroller/gitano.py b/lorrycontroller/gitano.py
index 06039b0..499bb5d 100644
--- a/lorrycontroller/gitano.py
+++ b/lorrycontroller/gitano.py
@@ -26,7 +26,7 @@ import lorrycontroller
 from . import hosts
 
 
-class GitanoCommandFailure(Exception):
+class _GitanoCommandFailure(Exception):
 
     def __init__(self, trovehost, command, stderr):
         Exception.__init__(
@@ -35,7 +35,7 @@ class GitanoCommandFailure(Exception):
             (command, trovehost, stderr))
 
 
-class GitanoCommand(object):
+class _GitanoCommand(object):
 
     '''Run a Gitano command on a Trove.'''
 
@@ -50,7 +50,7 @@ class GitanoCommand(object):
         elif protocol in ('http', 'https'):
             self._command = self._http_command
         else:
-            raise GitanoCommandFailure(
+            raise _GitanoCommandFailure(
                 self.trovehost, '__init__', 'unknown protocol %s' % protocol)
 
     def whoami(self):
@@ -99,7 +99,7 @@ class GitanoCommand(object):
             logging.error(
                 'Failed to run "%s" for %s:\n%s',
                 quoted_args, self.trovehost, stdout + stderr)
-            raise GitanoCommandFailure(
+            raise _GitanoCommandFailure(
                 self.trovehost,
                 ' '.join(gitano_args),
                 stdout + stderr)
@@ -123,13 +123,13 @@ class GitanoCommand(object):
             else:
                 response = requests.get(url)
         except (requests.exceptions.RequestException) as e:
-            raise GitanoCommandFailure(
+            raise _GitanoCommandFailure(
                 self.trovehost, ' '.join(gitano_args), str(e))
 
         return response.text
 
 
-class LocalTroveGitanoCommand(GitanoCommand):
+class _LocalTroveGitanoCommand(_GitanoCommand):
 
     '''Run commands on the local Trove's Gitano.
 
@@ -139,22 +139,13 @@ class LocalTroveGitanoCommand(GitanoCommand):
     '''
 
     def __init__(self):
-        GitanoCommand.__init__(self, 'localhost', 'ssh', '', '')
+        _GitanoCommand.__init__(self, 'localhost', 'ssh', '', '')
 
 
 
-def new_gitano_command(statedb, trovehost):
-    trove_info = statedb.get_trove_info(trovehost)
-    return lorrycontroller.GitanoCommand(
-        trovehost,
-        trove_info['protocol'],
-        trove_info['username'],
-        trove_info['password'])
-
-
 class GitanoDownstream(hosts.DownstreamHost):
     def __init__(self, app_settings):
-        self._gitano = LocalTroveGitanoCommand()
+        self._gitano = _LocalTroveGitanoCommand()
 
     def prepare_repo(self, repo_path, metadata):
         # Create repository on local Trove. If it fails, assume
@@ -163,7 +154,7 @@ class GitanoDownstream(hosts.DownstreamHost):
 
         try:
             self._gitano.create(repo_path)
-        except GitanoCommandFailure as e:
+        except _GitanoCommandFailure as e:
             logging.debug(
                 'Ignoring error creating %s on local Trove: %s',
                 repo_path, e)
@@ -183,9 +174,54 @@ class GitanoDownstream(hosts.DownstreamHost):
                 self._gitano.set_gitano_config(repo_path,
                                                'project.description',
                                                metadata['description'])
-        except GitanoCommandFailure as e:
+        except _GitanoCommandFailure as e:
+            logging.error('ERROR: %s' % str(e))
+            # FIXME: We need a good way to report these errors to the
+            # user. However, we probably don't want to fail the
+            # request, so that's not the way to do this. Needs
+            # thinking.
+
+
+class TroveUpstream(hosts.UpstreamHost):
+    def __init__(self, host_info):
+        self._host_info = host_info
+        self._gitano = _GitanoCommand(host_info['host'],
+                                      host_info['protocol'],
+                                      host_info['username'],
+                                      host_info['password'])
+
+    def list_repos(self):
+        ls_output = self._gitano.ls()
+        repo_paths = []
+        for line in ls_output.splitlines():
+            words = line.split(None, 1)
+            if words[0].startswith('R') and len(words) == 2:
+                repo_paths.append(words[1])
+        return repo_paths
+
+    def get_repo_url(self, remote_path):
+        vars = dict(self._host_info)
+        vars['remote_path'] = remote_path
+
+        patterns = {
+            'ssh':  'ssh://git@{host}/{remote_path}',
+            'https':'https://{username}:{password}@{host}/git/{remote_path}',
+            'http': 'http://{host}/git/{remote_path}',
+            }
+
+        return patterns[self._host_info['protocol']].format(**vars)
+
+    def get_repo_metadata(self, repo_path):
+        try:
+            remote_config = self._gitano.get_gitano_config(repo_path)
+            return {
+                'head':        remote_config['project.head'],
+                'description': remote_config['project.description'],
+            }
+        except _GitanoCommandFailure as e:
             logging.error('ERROR: %s' % str(e))
             # FIXME: We need a good way to report these errors to the
             # user. However, we probably don't want to fail the
             # request, so that's not the way to do this. Needs
             # thinking.
+            return {}
diff --git a/lorrycontroller/gitlab.py b/lorrycontroller/gitlab.py
index 90ba192..ab6df63 100644
--- a/lorrycontroller/gitlab.py
+++ b/lorrycontroller/gitlab.py
@@ -115,16 +115,26 @@ class GitlabDownstream(hosts.DownstreamHost):
         logging.info('Created %s project in local GitLab.', repo_path)
 
 
-class Gitlab(object):
-    def __init__(self, host, token):
-        self.gl = _init_gitlab(host, token)
+class GitlabUpstream(hosts.UpstreamHost):
+    @staticmethod
+    def check_host_type_params(validator, section):
+        validator.check_has_required_fields(section, ['private-token'])
+
+    @staticmethod
+    def get_host_type_params(section):
+        return {'private-token': section['private-token']}
 
-    def list_projects(self):
+    def __init__(self, host_info):
+        self._protocol = host_info['protocol']
+        self.gl = _init_gitlab(host_info['host'],
+                               host_info['type_params']['private-token'])
+
+    def list_repos(self):
         '''List projects on a GitLab instance.'''
 
         return [x.path_with_namespace for x in self.gl.projects.list()]
 
-    def get_project_url(self, protocol, project_path):
+    def get_repo_url(self, repo_path):
         '''Return the clone url for a GitLab project.
 
         Depending on the protocol specified, will return a suitable clone url.
@@ -139,9 +149,13 @@ class Gitlab(object):
 
         project = self.gl.projects.get(repo_path)
 
-        if protocol == 'ssh':
+        if self._protocol == 'ssh':
             return project.ssh_url_to_repo
-        elif protocol in ('http', 'https'):
+        elif self._protocol in ('http', 'https'):
             split = urllib.parse.urlsplit(project.http_url_to_repo)
             return urllib.parse.urlunsplit((
-                protocol, split.netloc, split.path, '', ''))
+                self._protocol, split.netloc, split.path, '', ''))
+
+    def get_repo_metadata(self, repo_path):
+        # TODO
+        return {}
diff --git a/lorrycontroller/givemejob.py b/lorrycontroller/givemejob.py
index 85f1818..6736b35 100644
--- a/lorrycontroller/givemejob.py
+++ b/lorrycontroller/givemejob.py
@@ -36,11 +36,7 @@ class GiveMeJob(lorrycontroller.LorryControllerRoute):
                 now = statedb.get_current_time()
                 for lorry_info in lorry_infos:
                     if self.ready_to_run(lorry_info, now):
-                        if lorry_info['from_host']:
-                            metadata = self.get_repo_metadata(statedb,
-                                                              lorry_info)
-                        else:
-                            metadata = {}
+                        metadata = self.get_repo_metadata(statedb, lorry_info)
                         downstream_type = lorrycontroller.downstream_types[
                             self.app_settings['git-server-type']]
                         downstream_type(self.app_settings) \
@@ -72,27 +68,32 @@ class GiveMeJob(lorrycontroller.LorryControllerRoute):
     def get_repo_metadata(self, statedb, lorry_info):
         '''Get repository head and description.'''
 
-        assert lorry_info['from_host']
-        assert lorry_info['from_path']
-
-        # XXX We don't know whether upstream is Trove
+        if not lorry_info['from_host']:
+            return {}
 
-        metadata = {}
-        remote = lorrycontroller.new_gitano_command(statedb, lorry_info['from_host'])
+        assert lorry_info['from_path']
 
         try:
-            remote_config = remote.get_gitano_config(lorry_info['from_path'])
-            metadata['head'] = remote_config['project.head']
+            host_info = statedb.get_host_info(lorry_info['from_host'])
+        except lorrycontroller.HostNotFoundError:
+            # XXX We don't know whether upstream is Trove.  It should be
+            # possible to set host type for single repositories.
+            host_info = {
+                'host':        lorry_info['from_host'],
+                'protocol':    'ssh',
+                'username':    None,
+                'password':    None,
+                'type':        'trove',
+                'type_params': {},
+            }
+
+        metadata = lorrycontroller.get_upstream_host(host_info) \
+                                  .get_repo_metadata(lorry_info['from_path'])
+        if 'description' in metadata:
+            # Prepend Upstream Host name
             metadata['description'] = '{host}: {desc}'.format(
                 host=lorry_info['from_host'],
-                desc=remote_config['project.description'])
-        except lorrycontroller.GitanoCommandFailure as e:
-            logging.error('ERROR: %s' % str(e))
-            # FIXME: We need a good way to report these errors to the
-            # user. However, we probably don't want to fail the
-            # request, so that's not the way to do this. Needs
-            # thinking.
-
+                desc=metadata['description'])
         return metadata
 
     def give_job_to_minion(self, statedb, lorry_info, now):
diff --git a/lorrycontroller/hosts.py b/lorrycontroller/hosts.py
index 76eeadf..b86cbad 100644
--- a/lorrycontroller/hosts.py
+++ b/lorrycontroller/hosts.py
@@ -55,3 +55,67 @@ class DownstreamHost(abc.ABC):
         - description: Short string describing the repository
         '''
         pass
+
+
+class UpstreamHost(abc.ABC):
+    @staticmethod
+    def check_host_type_params(validator, section):
+        '''Validate any type-specific fields in a CONFGIT host section.
+
+        validator is an instance of LorryControllerConfValidator that
+        may be used to check the types of configuration fields.
+
+        section is the dictionary of fields for the section.
+
+        Returns None if the configuration is valid; raises an
+        exception on error.
+        '''
+        pass
+
+    @staticmethod
+    def get_host_type_params(section):
+        '''Convert any type-specific fields in a CONFGIT host section into a
+        dictionary that will be stored in STATEDB.
+
+        section is the dictionary of fields for the section.
+
+        Returns a dictionary, which may be empty.  This will be stored
+        in STATEDB as the type_params of the host.
+        '''
+        return {}
+
+    @abc.abstractmethod
+    def __init__(self, host_info):
+        '''Construct an Upstream Host connector from the given host_info.
+        The host_info comes directly from STATEDB.
+        '''
+        pass
+
+    @abc.abstractmethod
+    def list_repos(self):
+        '''List all visible repositories on the Host.
+
+        Returns a list of path strings.
+        '''
+        pass
+
+    @abc.abstractmethod
+    def get_repo_url(self, repo_path):
+        '''Get URL for a repository.
+
+        repo_path is the path to the repository within the Host.
+
+        Returns a URL string suitable for passing to git clone.
+        '''
+        pass
+
+    @abc.abstractmethod
+    def get_repo_metadata(self, repo_path):
+        '''Get metadata for a repository.
+
+        repo_path is the path to the repository within the Host.
+
+        Returns a dictionary of metadata suitable for passing to
+        DownstreamHost.prepare_repo.
+        '''
+        pass
diff --git a/lorrycontroller/lsupstreams.py b/lorrycontroller/lsupstreams.py
index b31a385..a535174 100644
--- a/lorrycontroller/lsupstreams.py
+++ b/lorrycontroller/lsupstreams.py
@@ -54,7 +54,7 @@ class HostRepositoryLister(object):
         if self.app_settings['debug-fake-upstream-host']:
             repo_paths = self.get_fake_ls_output(host_info)
         else:
-            repo_paths = self.get_real_ls_output(statedb, host_info)
+            repo_paths = self.get_real_ls_output(host_info)
 
         return repo_paths
 
@@ -68,25 +68,8 @@ class HostRepositoryLister(object):
                 return obj['ls-output']
         return None
 
-    def get_real_ls_output(self, statedb, host_info):
-        if host_info['type'] == 'gitlab':
-            return lorrycontroller.Gitlab(host_info['host'],
-                                          host_info['type_params']
-                                          ['private-token']) \
-                                  .list_projects()
-
-        gitano = lorrycontroller.new_gitano_command(
-                statedb, host_info['host'])
-        output = gitano.ls()
-        return self.parse_ls_output(output)
-
-    def parse_ls_output(self, ls_output):
-        repo_paths = []
-        for line in ls_output.splitlines():
-            words = line.split(None, 1)
-            if words[0].startswith('R') and len(words) == 2:
-                repo_paths.append(words[1])
-        return repo_paths
+    def get_real_ls_output(self, host_info):
+        return lorrycontroller.get_upstream_host(host_info).list_repos()
 
     def skip_ignored_repos(self, host, repo_paths):
         ignored_patterns = json.loads(host['ignore'])
@@ -156,23 +139,8 @@ class HostRepositoryLister(object):
             }
 
     def construct_lorry_url(self, host_info, remote_path):
-        if host_info['type'] == 'gitlab':
-            return lorrycontroller.Gitlab(host_info['host'],
-                                          host_info['type_params']
-                                          ['private-token']) \
-                                  .get_project_url(host_info['protocol'],
-                                                   remote_path)
-
-        vars = dict(host_info)
-        vars['remote_path'] = remote_path
-
-        patterns = {
-            'ssh':  'ssh://git@{host}/{remote_path}',
-            'https':'https://{username}:{password}@{host}/git/{remote_path}',
-            'http': 'http://{host}/git/{remote_path}',
-            }
-
-        return patterns[host_info['protocol']].format(**vars)
+        return lorrycontroller.get_upstream_host(host_info) \
+                              .get_repo_url(remote_path)
 
 
 class ForceLsUpstream(lorrycontroller.LorryControllerRoute):
diff --git a/lorrycontroller/readconf.py b/lorrycontroller/readconf.py
index 875e982..3303f68 100644
--- a/lorrycontroller/readconf.py
+++ b/lorrycontroller/readconf.py
@@ -70,7 +70,7 @@ class ReadConfiguration(lorrycontroller.LorryControllerRoute):
                     added = self.add_matching_lorries_to_statedb(
                         statedb, section)
                     lorries_to_remove = lorries_to_remove.difference(added)
-                elif section['type'] in ('trove', 'gitlab'):
+                elif section['type'] in lorrycontroller.upstream_types:
                     self.add_host(statedb, section)
                     host = section.get('host') or section['trovehost']
                     if host in hosts_to_remove:
@@ -290,9 +290,8 @@ class ReadConfiguration(lorrycontroller.LorryControllerRoute):
             username = auth.get('username')
             password = auth.get('password')
 
-        type_params = {}
-        if section['type'] == 'gitlab':
-            type_params['private-token'] = section['private-token']
+        type_params = lorrycontroller.upstream_types[section['type']] \
+                                     .get_host_type_params(section)
 
         statedb.add_host(
             host=section.get('host') or section['trovehost'],
@@ -329,12 +328,12 @@ class LorryControllerConfValidator(object):
                 # Backward compatibility
                 if section['type'] == 'troves':
                     section['type'] = 'trove'
-                if section['type'] == 'trove':
+                if section['type'] in lorrycontroller.upstream_types:
                     self._check_host_section(section)
+                    lorrycontroller.upstream_types[section['type']] \
+                                   .check_host_type_params(self, section)
                 elif section['type'] == 'lorries':
                     self._check_lorries_section(section)
-                elif section['type'] == 'gitlab':
-                    self._check_gitlab_section(section)
                 else:
                     raise ValidationError(
                         'unknown section type %r' % section['type'])
@@ -353,10 +352,6 @@ class LorryControllerConfValidator(object):
             if type(item) is not dict:
                 raise ValidationError('all items must be dicts')
 
-    def _check_gitlab_section(self, section):
-        self._check_host_section(section)
-        self._check_has_required_fields(section, ['private-token'])
-
     def _check_host_section(self, section):
         if not any(i in ('trovehost', 'host') for i in section):
             self.check_has_required_fields(section, ['host'])