Correcting reviewed code

arteria/handlers/arteria_runfolder_handlers.py

@@ -1,4 +1,3 @@
-import os
 from aiohttp import web
 from pathlib import Path
 from arteria.models.state import State
@@ -17,26 +16,40 @@ async def post_runfolders(request):
     the state of the runfolder is set to STARTED
     """
     data = await request.post()
+    try:
+        runfolder_name = request.match_info['runfolder']
+        for runfolder_path in Config().get('monitored_directories'):
+            runfolder_cls = Runfolder(Path(runfolder_path) / runfolder_name)
+            state = data["state"]
+            if state not in State.__members__:
+                raise web.HTTPBadRequest(reason=f"The state '{state}' is not valid")
 
-    runfolder_path = get_runfolders_path_from_query(request)
-    runfolder_cls = Runfolder(runfolder_path, request)
+            runfolder_cls.state = [s for s in State if s.name == state][0]
 
-    runfolder_cls.state = data["state"]
-    return web.json_response(status=200)
+        return web.json_response(status=200)
+    except Exception as e:
+        set_exceptions(e, kwargs={"state": data["state"]})
 
 
 @routes.get("/runfolders/path/{runfolder}")
 async def get_runfolders(request):
     """
     Returns some information about the runfolder as json
     """
-    runfolder_path = get_runfolders_path_from_query(request)
-    runfolder_dict = serialize_runfolder_path(Runfolder(runfolder_path, request))
+    try:
+        runfolder_name = request.match_info['runfolder']
+        runfolders = [
 
-    return web.json_response(
-        data=runfolder_dict,
-        status=200
-    )
+            serialize_runfolder_path(Runfolder(Path(runfolder_path) / runfolder_name), request)
+            for runfolder_path in Config().get('monitored_directories')
+        ]
+
+        return web.json_response(
+            data=runfolders,
+            status=200
+        )
+    except Exception as e:
+        set_exceptions(e)
 
 
 @routes.get("/runfolders/next")
@@ -45,47 +58,52 @@ async def get_next_runfolder(request):
     Finds unprocessed runfolder (state=ready) and then
     returns some information about this runfolder.
     """
-    runfolder_cls = list_runfolders(
-        Config()['monitored_directories'],
-        filter_key=lambda r: r.state == State.READY,
-        request=request
-    )
-
-    if len(runfolder_cls) > 0:
-        runfolder_dict0 = serialize_runfolder_path(runfolder_cls[0])
-        return web.json_response(
-            data=runfolder_dict0,
-            status=200
-        )
-    else:
-        raise web.HTTPNoContent(
-            reason="No ready runfolders available."
+    try:
+        runfolder_cls = list_runfolders(
+            Config()['monitored_directories'],
+            filter_key=lambda r: r.state == State.READY
         )
 
+        if len(runfolder_cls) > 0:
+            runfolder_dict0 = serialize_runfolder_path(runfolder_cls[0], request)
+
+            return web.json_response(
+                data=runfolder_dict0,
+                status=200
+            )
+        else:
+            raise web.HTTPNoContent(
+                reason="No ready runfolders available."
+            )
+    except Exception as e:
+        set_exceptions(e)
+
 
 @routes.get("/runfolders/pickup")
 async def get_pickup_runfolder(request):
     """
     Used to start processing runfolders and also sets the runfolder to PENDING state.
     """
-    runfolder_cls = list_runfolders(
-        Config()['monitored_directories'],
-        filter_key=lambda r: r.state == State.READY,
-        request=request
-    )
-
-    if len(runfolder_cls) > 0:
-        runfolder_cls[0].state = State.PENDING.name
-        runfolder_dict0 = serialize_runfolder_path(runfolder_cls[0])
-        return web.json_response(
-            data=runfolder_dict0,
-            status=200
-        )
-    else:
-        raise web.HTTPNoContent(
-            reason="No ready runfolders available."
+    try:
+        runfolder_cls = list_runfolders(
+            Config()['monitored_directories'],
+            filter_key=lambda r: r.state == State.READY
         )
 
+        if len(runfolder_cls) > 0:
+            runfolder_cls[0].state = State.PENDING
+            runfolder_dict0 = serialize_runfolder_path(runfolder_cls[0], request)
+            return web.json_response(
+                data=runfolder_dict0,
+                status=200
+            )
+        else:
+            raise web.HTTPNoContent(
+                reason="No ready runfolders available."
+            )
+    except Exception as e:
+        set_exceptions(e)
+
 
 @routes.get("/runfolders")
 async def get_all_runfolders(request):
@@ -94,37 +112,45 @@ async def get_all_runfolders(request):
     match the state specified (or all runfolders when state
     is not specified)
     """
-    runfolders = list_runfolders(
-        Config()['monitored_directories'],
-        filter_key=lambda r: r.state == State.READY,
-        request=request
-    )
+    try:
+        runfolders = list_runfolders(
+            Config()['monitored_directories'],
+            filter_key=lambda r: r.state == State.READY
+        )
 
-    for runfolder_count, runfolder in enumerate(runfolders):
-        runfolders[runfolder_count] = serialize_runfolder_path(runfolder)
+        for runfolder_count, runfolder in enumerate(runfolders):
+            runfolders[runfolder_count] = serialize_runfolder_path(runfolder, request)
+
+        return web.json_response(
+            data={"runfolders": runfolders},
+            status=200
+        )
+    except Exception as e:
+        set_exceptions(e)
 
-    return web.json_response(
-        data={"runfolders": runfolders},
-        status=200
-    )
 
+def get_host_link(request):
+    host = request.url.raw_host
+    link = f"{request.scheme}://{host}/api/1.0"
+    link_path = f"{link}{request.path}"
 
-def get_runfolders_path_from_query(request):
-    """
-    Returns directory of the runfolder
-    """
-    return os.path.join(
-        Path(Config()['monitored_directories'][0]),
-        request.match_info['runfolder']
-    )
+    return host, link_path
 
 
-def serialize_runfolder_path(runfolder_cls):
+def serialize_runfolder_path(runfolder_cls, request):
     """
     Get the path uri as web.json_response gives an error when
     self.path is of type Path
     """
-    runfolder_dict = runfolder_cls.__repr__()
+    runfolder_dict = runfolder_cls.to_dict()
     runfolder_dict['path'] = Path(runfolder_dict['path']).as_uri()
+    runfolder_dict['host'], runfolder_dict['link'] = get_host_link(request)
 
     return runfolder_dict
+
+
+def set_exceptions(e, kwargs=None):
+    if type(e) is AssertionError:
+        raise web.HTTPNotFound(reason=e.args[0] if len(e.args) > 0 else e)
+    else:
+        raise e

arteria/models/runfolder_utils.py

@@ -4,7 +4,6 @@
 import logging
 import xmltodict
 
-from aiohttp import web
 from pathlib import Path
 from arteria import __version__
 from arteria.models.state import State
@@ -17,7 +16,7 @@
 }
 
 
-def list_runfolders(monitored_directories, filter_key=lambda r: True, request=None):
+def list_runfolders(monitored_directories, filter_key=lambda r: True):
     """
     Returns list of Runfolders in the monitored_directories
     according to the state filter provided (filter_key), or all
@@ -28,9 +27,9 @@ def list_runfolders(monitored_directories, filter_key=lambda r: True, request=No
         monitored_dir_path = Path(monitored_directory)
         for subdir in monitored_dir_path.iterdir():
             try:
-                if filter_key(runfolder := Runfolder(monitored_dir_path / subdir, request)):
+                if filter_key(runfolder := Runfolder(monitored_dir_path / subdir)):
                     runfolders.append(runfolder)
-            except web.HTTPNotFound as e:
+            except AssertionError as e:
                 if e == f"File [Rr]unParameters.xml not found in runfolder {subdir}":
                     continue
 
@@ -41,19 +40,13 @@ class Runfolder():
     """
     A class to manipulate runfolders on disk
     """
-    def __init__(self, path, request=None):
+    def __init__(self, path):
         self.config = Config(DEFAULT_CONFIG)
         self.path = Path(path)
 
         runfolder_name = os.path.basename(self.path)
         if not self.path.is_dir():
-            raise web.HTTPNotFound(
-                reason=f"Runfolder '{runfolder_name}' does not exist"
-            )
-        if request:
-            self.host = request.url.raw_host
-            link = f"{request.scheme}://{self.host}/api/1.0"
-            self.link = f"{link}{request.path}"
+            raise AssertionError(f"Runfolder '{runfolder_name}' does not exist")
 
         try:
             run_parameter_file = next(
@@ -66,8 +59,8 @@ def __init__(self, path, request=None):
             )
             self.run_parameters = xmltodict.parse(run_parameter_file.read_text())["RunParameters"]
         except StopIteration as exc:
-            raise web.HTTPNotFound(
-                reason=f"File [Rr]unParameters.xml not found in runfolder {path}"
+            raise AssertionError(
+                f"File [Rr]unParameters.xml not found in runfolder {path}"
             ) from exc
 
         marker_file_name = Instrument(self.run_parameters).completed_marker_file
@@ -92,9 +85,8 @@ def state(self):
 
     @state.setter
     def state(self, new_state):
-        if new_state not in State.__members__:
-            raise web.HTTPBadRequest(reason=f"The state '{new_state}' is not valid")
-        self._state_file.write_text(State[new_state].value)
+        assert new_state in State
+        self._state_file.write_text(new_state.value)
 
     @property
     def metadata(self):
@@ -133,14 +125,15 @@ def metadata(self):
 
         return metadata
 
-    def __repr__(self):
+    def __setitem__(self, key, value):
+        return setattr(self, key, value)
+
+    def to_dict(self):
         return {
-            "host": self.host if hasattr(self, 'host') else '',
-            "link": self.link if hasattr(self, 'link') else '',
             "metadata": self.metadata,
             "path": self.path,
             "service_version": __version__,
-            "state": self.state.name,
+            "state": self.state.name
         }
 
 

tests/integration/test_arteria_runfolder.py

@@ -15,7 +15,7 @@ def config():
     """
     Setup a temporary directory to be monitored by the service.
     """
-    with tempfile.TemporaryDirectory(delete=False) as monitored_dir:
+    with tempfile.TemporaryDirectory() as monitored_dir:
         config_dict = {
             "monitored_directories": [monitored_dir],
             "port": 8080,
@@ -124,9 +124,9 @@ async def test_get_runfolder_path(client, config, runfolder):
         assert resp.status == 200
         expected_runfolder = get_expected_runfolder(runfolder, resp)
         content = await resp.json()
-        content['path'] = expected_runfolder.get("path")
+        content[0]['path'] = expected_runfolder.get("path")
 
-        assert content == expected_runfolder
+        assert content == [expected_runfolder]
 
 
 @pytest.mark.parametrize("runfolder", [{"state": State.READY.name}], indirect=True)

tests/unit/test_runfolder_utils.py

@@ -6,7 +6,6 @@
 import xmltodict
 import pytest
 
-from aiohttp.web_exceptions import HTTPNotFound
 from arteria.models.state import State
 from arteria.models.config import Config
 from arteria.models.runfolder_utils import list_runfolders, Runfolder, Instrument
@@ -84,7 +83,7 @@ def test_list_runfolders_filtered(monitored_directory):
 
 class TestRunfolder():
     def test_init_regular_folder(self):
-        with pytest.raises(HTTPNotFound):
+        with pytest.raises(AssertionError):
             with tempfile.TemporaryDirectory() as regular_folder:
                 Runfolder(regular_folder)
 
@@ -102,7 +101,7 @@ def test_get_state(self, runfolder):
         assert runfolder.state == State.STARTED
 
     def test_set_state(self, runfolder):
-        runfolder.state = State.DONE.name
+        runfolder.state = State.DONE
         assert runfolder.state == State.DONE
 
     def get_path(self, runfolder):