Merge: cleanups for resources storage

9 files changed, 120 insertions, 99 deletions

diff --git a/ick2/__init__.py b/ick2/__init__.py
index 1fffe31..9af0521 100644
--- a/ick2/__init__.py
+++ b/ick2/__init__.py
@@ -15,7 +15,14 @@
 
 from .version import __version__, __version_info__
 from .logging import setup_logging, log
-from .state import ControllerState, NotFound, WrongPipelineStatus
+from .state import (
+    ControllerState,
+    NotFound,
+    WrongPipelineStatus,
+    Workers,
+    Projects,
+    PipelineInstances,
+)
 from .exceptions import (
     BadUpdate,
     ExistsAlready,
diff --git a/ick2/apibase.py b/ick2/apibase.py
index 088a2b6..c0b1aa5 100644
--- a/ick2/apibase.py
+++ b/ick2/apibase.py
@@ -21,6 +21,9 @@ class APIbase:
     def __init__(self, state):
         self._state = state
 
+    def get_state(self):
+        return self._state
+
     def get_routes(self, path):
         return [
             {
diff --git a/ick2/controllerapi.py b/ick2/controllerapi.py
index 1ef71cc..fe28c29 100644
--- a/ick2/controllerapi.py
+++ b/ick2/controllerapi.py
@@ -18,14 +18,8 @@ import ick2
 
 class ControllerAPI:
 
-    def __init__(self):
-        self._state = ick2.ControllerState()
-
-    def get_state_directory(self):
-        return self._state.get_state_directory()
-
-    def set_state_directory(self, dirname):
-        self._state.set_state_directory(dirname)
+    def __init__(self, state):
+        self._state = state
 
     def find_missing_route(self, missing_path):  # pragma: no cover
         apis = {
diff --git a/ick2/controllerapi_tests.py b/ick2/controllerapi_tests.py
index bb24e4a..89c553f 100644
--- a/ick2/controllerapi_tests.py
+++ b/ick2/controllerapi_tests.py
@@ -32,17 +32,11 @@ class ControllerAPITests(unittest.TestCase):
         shutil.rmtree(self.tempdir)
 
     def create_api(self):
-        api = ick2.ControllerAPI()
-        api.set_state_directory(self.statedir)
+        state = ick2.ControllerState()
+        state.set_state_directory(self.statedir)
+        api = ick2.ControllerAPI(state)
         return api
 
-    def test_has_no_state_directory_initially(self):
-        api = ick2.ControllerAPI()
-        statedir = api.get_state_directory()
-        self.assertTrue(statedir is None)
-
     def test_sets_and_creates_state_directory(self):
-        api = self.create_api()
-        statedir = api.get_state_directory()
-        self.assertEqual(statedir, self.statedir)
-        self.assertTrue(os.path.exists(statedir))
+        self.create_api()
+        self.assertTrue(os.path.exists(self.statedir))
diff --git a/ick2/projectapi.py b/ick2/projectapi.py
index f46fe14..6952d6d 100644
--- a/ick2/projectapi.py
+++ b/ick2/projectapi.py
@@ -20,6 +20,7 @@ class ProjectAPI(ick2.ResourceApiBase):
 
     def __init__(self, state):
         super().__init__('projects', state)
+        self._pi = ick2.PipelineInstances(self.get_state())
 
     def get_resource_name(self, resource):
         return resource['project']
@@ -53,18 +54,11 @@ class ProjectAPI(ick2.ResourceApiBase):
         if pipeline not in p['pipelines']:
             raise ick2.NotFound()
 
-        pp = self._pipeline_instance_name(project, pipeline)
-        try:
-            pl = self._state.get_resource('pipeline_instances', pp)
-        except ick2.NotFound:
-            pl = {}
+        pl = self._pi.get_instance(project, pipeline)
         return {
             'status': pl.get('status', 'idle'),
         }
 
-    def _pipeline_instance_name(self, project_name, pipeline_name):
-        return '{} {}'.format(project_name, pipeline_name)
-
     def set_pipeline_callback(
             self, body, project, pipeline, **kwargs):  # pragma: no cover
         return self.set_pipeline(body['status'], project, pipeline)
@@ -87,21 +81,12 @@ class ProjectAPI(ick2.ResourceApiBase):
             raise ick2.NotFound()
         ick2.log.log('trace', msg_text='Found project', project=p)
 
-        pp = self._pipeline_instance_name(project, pipeline)
-        try:
-            pl = self._state.get_resource('pipeline_instances', pp)
-        except ick2.NotFound:
-            pl = {
-                'name': pipeline,
-                'status': 'idle',
-            }
-            self._state.add_resource('pipeline_instances', pp, pl)
-
+        pl = self._pi.get_instance(project, pipeline)
         old_status = pl.get('status', 'idle')
         if allowed_changes[old_status] != status:
             raise ick2.WrongPipelineStatus(status)
         pl['status'] = status
-        self._state.update_resource('pipeline_instances', pp, pl)
+        self._pi.update_instance(project, pipeline, pl)
         return {'status': status}
 
     # This needs to go away as it is not protected. Once an IDP is
diff --git a/ick2/state.py b/ick2/state.py
index 548ce87..806b2a7 100644
--- a/ick2/state.py
+++ b/ick2/state.py
@@ -21,6 +21,9 @@ import os
 import yaml
 
 
+import ick2
+
+
 class ControllerState:
 
     def __init__(self):
@@ -95,3 +98,79 @@ class WrongPipelineStatus(Exception):  # pragma: no cover
 
     def __init__(self, new_state):
         super().__init__('Cannot set pipeline state to {}'.format(new_state))
+
+
+class ResourceStore:  # pragma: no cover
+
+    def __init__(self, state, category, name_field):
+        self._state = state
+        self._category = category
+        self._name_field = name_field
+
+    def list(self):
+        return self._state.get_resources(self._category)
+
+    def get(self, name):
+        try:
+            return self._state.get_resource(self._category, name)
+        except ick2.NotFound:
+            return {
+                self._name_field: name,
+            }
+
+    def add(self, name, resource):
+        self._state.add_resource(self._category, name, resource)
+
+    def update(self, resource):
+        name = resource[self._name_field]
+        try:
+            self._state.get_resource(self._category, name)
+        except ick2.NotFound:
+            raise ick2.NotFound()
+        else:
+            self._state.update_resource(self._category, name, resource)
+
+
+class Workers(ResourceStore):  # pragma: no cover
+
+    def __init__(self, state):
+        super().__init__(state, 'workers', 'worker')
+
+    def get_worker(self, name):
+        return self.get(name)
+
+    def update_worker(self, worker):
+        self.update(worker)
+
+
+class Projects(ResourceStore):  # pragma: no cover
+
+    def __init__(self, state):
+        super().__init__(state, 'projects', 'project')
+
+    def get_projects(self):
+        return self.list()
+
+    def update_project(self, project):
+        self.update(project)
+
+
+class PipelineInstances(ResourceStore):  # pragma: no cover
+
+    def __init__(self, state):
+        super().__init__(state, 'pipeline_instances', 'name')
+
+    def _name(self, project_name, pipeline_name):
+        return '{} {}'.format(project_name, pipeline_name)
+
+    def get_instance(self, project_name, pipeline_name):
+        name = self._name(project_name, pipeline_name)
+        return self.get(name)
+
+    def add_instance(self, project_name, pipeline_name, instance):
+        name = self._name(project_name, pipeline_name)
+        self.add(name, instance)
+
+    def update_instance(self, project_name, pipeline_name, instance):
+        name = self._name(project_name, pipeline_name)
+        self.add(name, instance)
diff --git a/ick2/workapi.py b/ick2/workapi.py
index f958132..bdde1a5 100644
--- a/ick2/workapi.py
+++ b/ick2/workapi.py
@@ -20,8 +20,9 @@ class WorkAPI(ick2.APIbase):
 
     def __init__(self, state):
         super().__init__(state)
-        self._workers = Workers(state)
-        self._projects = Projects(state)
+        self._workers = ick2.Workers(state)
+        self._projects = ick2.Projects(state)
+        self._pinstances = ick2.PipelineInstances(state)
         self._type_name = 'work'
 
     def get_routes(self, path):  # pragma: no cover
@@ -80,28 +81,21 @@ class WorkAPI(ick2.APIbase):
         projects = self._projects.get_projects()
         for project in projects:
             for name in project['pipelines']:
-                pp = self._pipeline_instance_name(project['project'], name)
-                try:
-                    pl = self._state.get_resource('pipeline_instances', pp)
-                except ick2.NotFound:
-                    pass
-                else:
-                    if pl.get('status') == 'triggered':
-                        pt = self._state.get_resource('pipelines', name)
-                        return project, pt
+                pl = self._pinstances.get_instance(project['project'], name)
+                if pl.get('status') == 'triggered':
+                    pt = self._state.get_resource('pipelines', name)
+                    return project, pt
         return None, None
 
-    def _pipeline_instance_name(self, project_name, pipeline_name):
-        return '{} {}'.format(project_name, pipeline_name)
-
-    def _update_pipeline(self, project, pipeline):
-        pp = self._pipeline_instance_name(project['project'], pipeline['name'])
+    def _update_pipeline(self, project, pipeline_instance):
+        project_name = project['project']
+        pipeline_name = pipeline_instance['name']
         try:
-            self._state.get_resource('pipeline_instances', pp)
+            self._pinstances.update_instance(
+                project_name, pipeline_name, pipeline_instance)
         except ick2.NotFound:  # pragma: no cover
-            self._state.add_resource('pipeline_instances', pp, pipeline)
-        else:
-            self._state.update_resource('pipeline_instances', pp, pipeline)
+            self._pinstances.add_instance(
+                project_name, pipeline_name, pipeline_instance)
 
     def _start_build(self, project, pipeline, worker, build_id):
         ick2.log.log('info', msg_text='Starting new build', build_id=build_id)
@@ -228,38 +222,3 @@ class WorkAPI(ick2.APIbase):
 
     def delete(self, *args, **kwargs):  # pragma: no cover
         pass
-
-
-class Workers:  # pragma: no cover
-
-    def __init__(self, state):
-        self._state = state
-
-    def get_worker(self, name):
-        try:
-            return self._state.get_resource('workers', name)
-        except ick2.NotFound:
-            return {
-                'worker': name,
-            }
-
-    def update_worker(self, worker):
-        try:
-            self._state.get_resource('workers', worker['worker'])
-        except ick2.NotFound:
-            raise ick2.NotFound()
-        else:
-            self._state.update_resource(
-                'workers', worker['worker'], worker)
-
-
-class Projects:  # pragma: no cover
-
-    def __init__(self, state):
-        self._state = state
-
-    def get_projects(self):
-        return self._state.get_resources('projects')
-
-    def update_project(self, project):
-        self._state.update_resource('projects', project['project'], project)
diff --git a/ick_controller.py b/ick_controller.py
index 7c611a8..85c0bca 100644
--- a/ick_controller.py
+++ b/ick_controller.py
@@ -89,12 +89,11 @@ def main():
 
         ick2.log.log('info', msg_text='Ick2 controller starts', config=config)
 
-        api = ick2.ControllerAPI()
-        ick2.log.log('info', msg_text='created ControllerAPI')
+        state = ick2.ControllerState()
+        state.set_state_directory(config['statedir'])
 
-        api.set_state_directory(config['statedir'])
-        ick2.log.log(
-            'info', msg_text='called ControllerAPI.set_state_directory')
+        api = ick2.ControllerAPI(state)
+        ick2.log.log('info', msg_text='created ControllerAPI')
 
         application = apifw.create_bottle_application(
             api, counter, dict_logger, config)
diff --git a/pylint.conf b/pylint.conf
index 3434ccd..1871f36 100644
--- a/pylint.conf
+++ b/pylint.conf
@@ -7,6 +7,7 @@ disable=
     missing-docstring,
     no-self-use,
     not-callable,
+    too-few-public-methods,
     unused-argument
 
 [REPORTS]