Merge pull request #43 from labthings/improved-test-coverage

Improved test coverage

Author: jtc42

labthings/core/event.py

@@ -30,11 +30,15 @@ def wait(self, timeout: int = 5):
             self.events[ident] = [Event(), time.time()]
 
         # We have to reimplement event waiting here as we need native thread events to allow gevent context switching
+        wait_start = time.time()
         while not self.events[ident][0].is_set():
-            gevent.time.sleep(0)
+            now = time.time()
+            if now - wait_start > timeout:
+                return False
+            gevent.sleep(0)
         return True
 
-    def set(self):
+    def set(self, timeout=5):
         """Signal that a new frame is available."""
         now = time.time()
         remove = None
@@ -47,9 +51,9 @@ def set(self):
             else:
                 # if the client's event is already set, it means the client
                 # did not process a previous frame
-                # if the event stays set for more than 5 seconds, then assume
+                # if the event stays set for more than `timeout` seconds, then assume
                 # the client is gone and remove it
-                if now - event[1] > 5:
+                if now - event[1] >= timeout:
                     remove = ident
         if remove:
             del self.events[remove]
@@ -60,4 +64,6 @@ def clear(self):
         if ident not in self.events:
             logging.error(f"Mismatched ident. Current: {ident}, available:")
             logging.error(self.events.keys())
+            return False
         self.events[id(getcurrent())][0].clear()
+        return True

labthings/core/tasks/thread.py

@@ -1,5 +1,6 @@
 from gevent import Greenlet, GreenletExit
 from gevent.thread import get_ident
+from gevent.event import Event
 import datetime
 import logging
 import traceback
@@ -28,6 +29,9 @@ def __init__(self, target=None, args=None, kwargs=None):
         # A UUID for the TaskThread (not the same as the threading.Thread ident)
         self._ID = uuid.uuid4()  # Task ID
 
+        # Event to track if the task has started
+        self.started_event = Event()
+
         # Make _target, _args, and _kwargs available to the subclass
         self._target = target
         self._args = args
@@ -96,6 +100,7 @@ def wrapped(*args, **kwargs):
 
             self._status = "running"
             self._start_time = datetime.datetime.now().strftime("%Y-%m-%d %H-%M-%S")
+            self.started_event.set()
             try:
                 self._return_value = f(*args, **kwargs)
                 self._status = "success"

labthings/server/__init__.py

@@ -1,3 +0,0 @@
-import logging
-
-EXTENSION_NAME = "flask-labthings"

labthings/server/decorators.py

@@ -4,45 +4,29 @@
 from werkzeug.wrappers import Response as ResponseBase
 from http import HTTPStatus
 from marshmallow.exceptions import ValidationError
-from collections import Mapping
+from collections.abc import Mapping
+
+from marshmallow import Schema as _Schema
 
 from .spec.utilities import update_spec
 from .schema import TaskSchema, Schema, FieldSchema
 from .fields import Field
 from .view import View
 from .find import current_labthing
+from .utilities import unpack
 
 from labthings.core.tasks.pool import TaskThread
+from labthings.core.utilities import rupdate
 
 import logging
 
 # Useful externals to have included here
 from marshmallow import pre_dump, pre_load
 
 
-def unpack(value):
-    """Return a three tuple of data, code, and headers"""
-    if not isinstance(value, tuple):
-        return value, 200, {}
-
-    try:
-        data, code, headers = value
-        return data, code, headers
-    except ValueError:
-        pass
-
-    try:
-        data, code = value
-        return data, code, {}
-    except ValueError:
-        pass
-
-    return value, 200, {}
-
-
 class marshal_with:
     def __init__(self, schema, code=200):
-        """Decorator to format the response of a View with a Marshmallow schema
+        """Decorator to format the return of a function with a Marshmallow schema
 
         Args:
             schema: Marshmallow schema, field, or dict of Fields, describing
@@ -52,11 +36,11 @@ def __init__(self, schema, code=200):
         self.code = code
 
         if isinstance(self.schema, Mapping):
-            self.converter = Schema.from_dict(self.schema)().jsonify
+            self.converter = Schema.from_dict(self.schema)().dump
         elif isinstance(self.schema, Field):
-            self.converter = FieldSchema(self.schema).jsonify
-        elif isinstance(self.schema, Schema):
-            self.converter = self.schema.jsonify
+            self.converter = FieldSchema(self.schema).dump
+        elif isinstance(self.schema, _Schema):
+            self.converter = self.schema.dump
         else:
             raise TypeError(
                 f"Unsupported schema type {type(self.schema)} for marshal_with"
@@ -69,12 +53,15 @@ def __call__(self, f):
         @wraps(f)
         def wrapper(*args, **kwargs):
             resp = f(*args, **kwargs)
-            if isinstance(resp, tuple):
-                data, code, headers = unpack(resp)
+            if isinstance(resp, ResponseBase):
+                resp.data = self.converter(resp.data)
+                return resp
+            elif isinstance(resp, tuple):
+                resp, code, headers = unpack(resp)
+                return (self.converter(resp), code, headers)
             else:
-                data, code, headers = resp, 200, {}
-
-            return make_response(self.converter(data), code, headers)
+                resp, code, headers = resp, 200, {}
+            return (self.converter(resp), code, headers)
 
         return wrapper
 
@@ -90,15 +77,15 @@ def marshal_task(f):
     def wrapper(*args, **kwargs):
         resp = f(*args, **kwargs)
         if isinstance(resp, tuple):
-            data, code, headers = unpack(resp)
+            resp, code, headers = unpack(resp)
         else:
-            data, code, headers = resp, 200, {}
+            resp, code, headers = resp, 201, {}
 
-        if not isinstance(data, TaskThread):
+        if not isinstance(resp, TaskThread):
             raise TypeError(
-                f"Function {f.__name__} expected to return a TaskThread object, but instead returned a {type(data).__name__}. If it does not return a task, remove the @marshall_task decorator from {f.__name__}."
+                f"Function {f.__name__} expected to return a TaskThread object, but instead returned a {type(resp).__name__}. If it does not return a task, remove the @marshall_task decorator from {f.__name__}."
             )
-        return make_response(TaskSchema().jsonify(data), code, headers)
+        return (TaskSchema().dump(resp), code, headers)
 
     return wrapper
 
@@ -327,11 +314,12 @@ def __init__(self, code, description=None, mimetype=None, **kwargs):
         }
 
         if self.mimetype:
-            self.response_dict.update(
+            rupdate(
+                self.response_dict,
                 {
                     "responses": {self.code: {"content": {self.mimetype: {}}}},
                     "_content_type": self.mimetype,
-                }
+                },
             )
 
     def __call__(self, f):

labthings/server/default_views/extensions.py

@@ -17,4 +17,4 @@ def get(self):
         Returns a list of Extension representations, including basic documentation.
         Describes server methods, web views, and other relevant Lab Things metadata.
         """
-        return registered_extensions().values()
+        return registered_extensions().values() or []

labthings/server/extensions.py

@@ -65,7 +65,7 @@ def views(self):
     def add_view(self, view_class, rule, view_id=None, **kwargs):
         # Remove all leading slashes from view route
         cleaned_rule = rule
-        while cleaned_rule[0] == "/":
+        while cleaned_rule and cleaned_rule[0] == "/":
             cleaned_rule = cleaned_rule[1:]
 
         # Expand the rule to include extension name
@@ -141,7 +141,7 @@ def add_method(self, method, method_name):
         if not hasattr(self, method_name):
             setattr(self, method_name, method)
         else:
-            logging.warning(
+            raise NameError(
                 "Unable to bind method to extension. Method name already exists."
             )
 

labthings/server/find.py

@@ -1,7 +1,7 @@
 import logging
 from flask import current_app
 
-from . import EXTENSION_NAME
+from .names import EXTENSION_NAME
 
 
 def current_labthing(app=None):
@@ -19,7 +19,7 @@ def current_labthing(app=None):
     logging.debug("Active app extensions:")
     logging.debug(app.extensions)
     logging.debug("Active labthing:")
-    logging.debug(app.extensions[EXTENSION_NAME])
+    logging.debug(app.extensions.get(EXTENSION_NAME))
     return app.extensions.get(EXTENSION_NAME, None)
 
 
@@ -35,7 +35,8 @@ def registered_extensions(labthing_instance=None):
     """
     if not labthing_instance:
         labthing_instance = current_labthing()
-    return labthing_instance.extensions
+
+    return getattr(labthing_instance, "extensions", {})
 
 
 def registered_components(labthing_instance=None):

labthings/server/labthing.py

@@ -2,10 +2,14 @@
 from apispec import APISpec
 from apispec.ext.marshmallow import MarshmallowPlugin
 
-from . import EXTENSION_NAME  # TODO: Move into .names
-from .names import TASK_ENDPOINT, TASK_LIST_ENDPOINT, EXTENSION_LIST_ENDPOINT
+from .names import (
+    EXTENSION_NAME,
+    TASK_ENDPOINT,
+    TASK_LIST_ENDPOINT,
+    EXTENSION_LIST_ENDPOINT,
+)
 from .extensions import BaseExtension
-from .utilities import description_from_view
+from .utilities import description_from_view, clean_url_string
 from .exceptions import JSONExceptionHandler
 from .logging import LabThingLogger
 from .representations import LabThingsJSONEncoder
@@ -113,6 +117,8 @@ def version(self, version: str):
     # Flask stuff
 
     def init_app(self, app):
+        self.app = app
+
         app.teardown_appcontext(self.teardown)
 
         # Register Flask extension
@@ -160,7 +166,7 @@ def _create_base_routes(self):
         self.add_view(TaskView, "/tasks/<task_id>", endpoint=TASK_ENDPOINT)
 
     def _create_base_sockets(self):
-        self.sockets.add_url_rule(f"{self.url_prefix}", self._socket_handler)
+        self.sockets.add_url_rule(self._complete_url("", ""), self._socket_handler)
 
     def _socket_handler(self, ws):
         # Create a socket subscriber
@@ -231,8 +237,9 @@ def _complete_url(self, url_part, registration_prefix):
         :param registration_prefix: The part of the url contributed by the
             blueprint.  Generally speaking, BlueprintSetupState.url_prefix
         """
-        parts = [registration_prefix, self.url_prefix, url_part]
-        return "".join([part for part in parts if part])
+        parts = [self.url_prefix, registration_prefix, url_part]
+        u = "".join([clean_url_string(part) for part in parts if part])
+        return u if u else "/"
 
     def add_view(self, resource, *urls, endpoint=None, **kwargs):
         """Adds a view to the api.
@@ -280,18 +287,6 @@ def _register_view(self, app, view, *urls, endpoint=None, **kwargs):
         resource_class_args = kwargs.pop("resource_class_args", ())
         resource_class_kwargs = kwargs.pop("resource_class_kwargs", {})
 
-        # NOTE: 'view_functions' is cleaned up from Blueprint class in Flask 1.0
-        if endpoint in getattr(app, "view_functions", {}):
-            previous_view_class = app.view_functions[endpoint].__dict__["view_class"]
-
-            # If you override the endpoint with a different class,
-            # avoid the collision by raising an exception
-            if previous_view_class != view:
-                raise ValueError(
-                    "This endpoint (%s) is already set to the class %s."
-                    % (endpoint, previous_view_class.__name__)
-                )
-
         view.endpoint = endpoint
         resource_func = view.as_view(
             endpoint, *resource_class_args, **resource_class_kwargs
@@ -311,7 +306,7 @@ def _register_view(self, app, view, *urls, endpoint=None, **kwargs):
 
         # Handle resource groups listed in API spec
         view_spec = get_spec(view)
-        view_groups = view_spec.get("_groups", {})
+        view_groups = view_spec.get("_groups", [])
         if "actions" in view_groups:
             self.thing_description.action(flask_rules, view)
             self._action_views[view.endpoint] = view
@@ -324,7 +319,9 @@ def _register_view(self, app, view, *urls, endpoint=None, **kwargs):
     def url_for(self, view, **values):
         """Generates a URL to the given resource.
         Works like :func:`flask.url_for`."""
-        endpoint = view.endpoint
+        endpoint = getattr(view, "endpoint", None)
+        if not endpoint:
+            return ""
         # Default to external links
         if "_external" not in values:
             values["_external"] = True

labthings/server/names.py

@@ -1,3 +1,4 @@
 TASK_ENDPOINT = "labthing_task"
 TASK_LIST_ENDPOINT = "labthing_task_list"
 EXTENSION_LIST_ENDPOINT = "labthing_extension_list"
+EXTENSION_NAME = "flask-labthings"

labthings/server/schema.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from flask import jsonify, url_for
+from werkzeug.routing import BuildError
 import marshmallow
 
 from .names import TASK_ENDPOINT, TASK_LIST_ENDPOINT, EXTENSION_LIST_ENDPOINT
@@ -75,6 +76,9 @@ def serialize(self, value):
 
         return self.field.serialize("value", obj)
 
+    def dump(self, value):
+        return self.serialize(value)
+
     def jsonify(self, value):
         """Serialize a value to JSON
 
@@ -102,9 +106,13 @@ class TaskSchema(Schema):
 
     @marshmallow.pre_dump
     def generate_links(self, data, **kwargs):
+        try:
+            url = url_for(TASK_ENDPOINT, task_id=data.id, _external=True)
+        except BuildError:
+            url = None
         data.links = {
             "self": {
-                "href": url_for(TASK_ENDPOINT, task_id=data.id, _external=True),
+                "href": url,
                 "mimetype": "application/json",
                 **description_from_view(view_class_from_endpoint(TASK_ENDPOINT)),
             }
@@ -127,11 +135,13 @@ def generate_links(self, data, **kwargs):
         for view_id, view_data in data.views.items():
             view_cls = view_data["view"]
             view_rule = view_data["rule"]
+            # Try to build a URL
+            try:
+                url = url_for(EXTENSION_LIST_ENDPOINT, _external=True) + view_rule
+            except BuildError:
+                url = None
             # Make links dictionary if it doesn't yet exist
-            d[view_id] = {
-                "href": url_for(EXTENSION_LIST_ENDPOINT, _external=True) + view_rule,
-                **description_from_view(view_cls),
-            }
+            d[view_id] = {"href": url, **description_from_view(view_cls)}
 
         data.links = d