Merge pull request #17 from bag-cnag/dev

0.4.2 patch -> fix doc engine for apispec + minor changes.

docs/developer_manual.rst

@@ -251,21 +251,65 @@ combination with ``@overload_docstrings``, made to overload docstrings of contro
         @overload_docstring
         async def create(**kwargs):
             """
+            requestBody:
+                description: payload.
+                required: true
+                content:
+                    application/json:
+                        schema: DatasetSchema
             responses:
-              201:
+            201:
                 description: Create Dataset.
                 examples: |
-                  {"name": "ds_test", "owner": {"username": "my_team_member"}}
-              204:
+                    # TODO:
+                    {"name": "instant_sc_1234", ""}
+                content:
+                    application/json:
+                        schema: DatasetSchema
+            204:
                 description: Empty Payload.
             """
-
+        ...
+        class TagController(ResourceController):
+            @overload_docstring
+            async def read(**kwargs):
+                """
+                parameters:
+                - in: path
+                  name: name
+                  description: Tag name
+                responses:
+                  200:
+                    description: Found matching Tag.
+                    examples: |
+                      {"name": "epidemiology"}
+                    content:
+                      application/json:
+                        schema: TagSchema
+                  404:
+                    description: Tag not found.
+                """
         ...
 
 .. warning::
 
-    ``@overload_docstrings`` returns the parent class method, hence if you use the latter variant,
-    be sure to use it first even if you do not wish to document that endpoint.
+    ``@overload_docstrings`` returns a wrapper pointing to the parent class method,
+    hence if you use the latter variant, be sure to this decorator first even if you do not wish to
+    document that endpoint.
+
+**Docstrings Guide**
+Docstrings are parsed by `apispec <https://github.com/marshmallow-code/apispec/>`_ and shall
+comply with their specification. In particular you have to be precise with parameters,
+and marshmallow schema specifications.
+This is required in order to output specification in ``OpenAPISchema`` format,
+which enables support for ``swagger-ui`` and the rest of the ecosystem.
+
+.. note::
+
+    The core patches abstract method documentation at runtime for endpoints that are left
+    undocumented. However, if you are using ``@overload_docstrings`` ensuring that it works is up
+    to you.
+
 
 .. _dev-user-permissions:
 

src/biodm/__init__.py

@@ -1,5 +1,5 @@
 """BioDM framework."""
-__version__ = '0.4.0'
+__version__ = '0.4.2'
 __version_info__ = ([int(num) for num in __version__.split('.')])
 
 

src/biodm/basics/__init__.py

@@ -1,6 +1,17 @@
 from .rootcontroller import RootController
-from .usercontroller import UserController
-from .groupcontroller import GroupController
-from .k8scontroller import K8sController
+from .k8scontroller import K8sController # Not below, because started conditionally.
+
+from biodm.components.controllers import ResourceController
+
+
+class UserController(ResourceController):
+    """User Controller."""
+    pass
+
+
+class GroupController(ResourceController):
+    """Group Controller."""
+    pass
+
 
 CORE_CONTROLLERS = [RootController, UserController, GroupController]

src/biodm/components/controllers/resourcecontroller.py

@@ -1,7 +1,9 @@
 """Controller class for Tables acting as a Resource."""
 
 from __future__ import annotations
+from copy import copy
 from functools import partial
+from types import MethodType
 from typing import TYPE_CHECKING, List, Set, Any, Dict, Type
 
 from marshmallow.schema import RAISE
@@ -98,19 +100,60 @@ def __init__(
         self._replace_schema_in_docstrings()
 
     def _replace_schema_in_docstrings(self):
-        """Sets an appropriate schema annotation for Responses."""
-        # TODO: replace id annotations by precise pk elements ?
-        for f in dir(self):
-            if not f.startswith('_'):
-                fct = getattr(self, f, {})
+        """Substitutes abstract endpoint documentation bits with one targeted on this controller.
+
+        Essentially handling APIspec/Marshmallow/OpenAPISchema support for abstract endpoints.
+        """
+        for method in dir(self):
+            if not method.startswith('_'):
+                fct = getattr(self, method, {})
                 if hasattr(fct, '__annotations__'):
                     if fct.__annotations__.get('return', '') == 'Response':
-                        fct.__func__.__doc__ = (
-                            fct
-                            .__func__
-                            .__doc__
-                            .replace('schema: Schema', f"schema: {self.schema.__class__.__name__}")
-                        ) if fct.__func__.__doc__ else None
+                        abs_doc = fct.__func__.__doc__ or ""
+                        # Use intance schema.
+                        abs_doc = abs_doc.replace(
+                            'schema: Schema', f"schema: {self.schema.__class__.__name__}"
+                        )
+
+                        # Set precise primary key routes.
+                        path_key = []
+                        for key in self.pk:
+                            attr = []
+                            attr.append("- in: path")
+                            attr.append(f"name: {key}")
+                            field = self.schema.declared_fields[key]
+                            desc  = field.metadata.get("description", None)
+                            attr.append("description: " + (desc or f"{self.resource} {key}"))
+                            path_key.append(attr)
+
+                        # Replace in apispec.
+                        doc = abs_doc.split('---')
+                        if len(doc) > 1:
+                            sphinxdoc, apispec = doc
+                            apispec = apispec.split('\n')
+                            found = False
+                            # Find our convention pattern (on two lines).
+                            for i in range(len(apispec)):
+                                if '- in: path' in apispec[i-1] and 'name: id' in apispec[i]:
+                                    found = True
+                                    break
+                            if found:
+                                # Work out same indentation level in order not to break the yaml.
+                                indent = len(apispec[i-1].split('- in: path')[0])
+                                flattened = []
+                                for path_attribute in path_key:
+                                    path_attribute[0] = " " * indent + path_attribute[0]
+                                    path_attribute[1] = " " * (indent+2) + path_attribute[1]
+                                    path_attribute[2] = " " * (indent+2) + path_attribute[2]
+                                    flattened.extend(path_attribute)
+
+                                final = apispec[:i-1] + flattened + apispec[i+1:]
+                                abs_doc = sphinxdoc + "\n---\n" + "\n".join(final)
+
+                                # Create copy of the function object and patch new doc.
+                                new_f = copy(fct.__func__)
+                                new_f.__doc__ = abs_doc
+                                setattr(self, method, MethodType(new_f, self))
 
     def _infer_resource_name(self) -> str:
         """Infer entity name from controller name."""
@@ -315,7 +358,6 @@ async def read(self, request: Request) -> Response:
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_' e.g. /datasets/1_1 returns dataset with id=1 and version=1
           - in: query
             name: fields
             description: a comma separated list of fields to query only a subset of the resource e.g. /datasets/1_1?name,description,contact,files
@@ -358,7 +400,6 @@ async def update(self, request: Request) -> Response:
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'.
         responses:
             201:
                 description: Update associated resource.
@@ -403,7 +444,6 @@ async def delete(self, request: Request) -> Response:
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'
         responses:
             200:
                 description: Deleted.
@@ -460,7 +500,6 @@ async def release(self, request: Request) -> Response:
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'
         responses:
             201:
                 description: New resource version, updated values, without its nested collections.
@@ -498,14 +537,12 @@ async def release(self, request: Request) -> Response:
     async def read_nested(self, request: Request) -> Response:
         """Reads a nested collection from parent primary key.
             Call read, with attribute and serializes with child resource controller.
-
         ---
 
         description: Read nested collection from parent resource.
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'
           - in: path
             name: attribute
             description: nested collection name.

src/biodm/components/controllers/s3controller.py

@@ -43,7 +43,6 @@ async def download(self, request: Request):
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'.
         responses:
             307:
                 description: Download URL, with a redirect header.
@@ -67,7 +66,6 @@ async def upload_success(self, request: Request):
         parameters:
           - in: path
             name: id
-            description: entity primary key elements separated by '_'.
         responses:
             201:
                 description: Upload confirmation 'Uploaded'.

src/biodm/components/services/dbservice.py

@@ -1,6 +1,6 @@
 """Database service: Translates requests data into SQLA statements and execute."""
 from operator import or_
-from typing import Callable, List, Sequence, Any, Tuple, Dict, overload, Literal, Set, Type
+from typing import Callable, Iterable, List, Sequence, Any, Tuple, Dict, overload, Literal, Set, Type
 
 from sqlalchemy import insert, select, delete, or_, func
 from sqlalchemy.dialects import postgresql, sqlite
@@ -620,9 +620,9 @@ async def release(
 class CompositeEntityService(UnaryEntityService):
     """Special case for Composite Entities (i.e. containing nested entities attributes)."""
     @property
-    def runtime_relationships(self) -> Set[str]:
-        """Evaluate relationships at runtime by computing the difference with
-          self.relatioships set a instanciation time.
+    def permission_relationships(self) -> Set[str]:
+        """Get permissions relationships by computing the difference of between instanciation time
+            and runtime, since those get populated later in Base.setup_permissions().
         """
         return set(self.table.relationships().keys()) - set(self.relationships.keys())
 
@@ -655,7 +655,6 @@ async def _insert_composite(
                 .decl_class
                 .svc
             )._insert(sub, **kwargs)
-            await session.commit()
 
         # Insert main object.
         item = await self._insert(composite.item, **kwargs)
@@ -665,8 +664,6 @@ async def _insert_composite(
             await getattr(item.awaitable_attrs, key)
             setattr(item, key, sub)
 
-        await session.commit()
-
         # Populate many-to-item fields with 'delayed' (because needing item id) objects.
         for key, delay in composite.delayed.items():
             # Load attribute.
@@ -678,6 +675,8 @@ async def _insert_composite(
                 mapping = {}
                 for c in rels[key].remote_side:
                     if c.foreign_keys:
+                        # TODO: This (v) looks suspicious for some composite primary key edge cases.
+                        # TODO: check.
                         fk, = c.foreign_keys
                         mapping[c.name] = getattr(
                             item,
@@ -697,7 +696,6 @@ async def _insert_composite(
                 case list() | Insert():
                     # Insert
                     delay = await target_svc._insert_many(delay, **kwargs)
-                    await session.commit()
 
                     # Put in attribute the objects that are not already present.
                     delay, updated = partition(delay, lambda e: e not in getattr(item, key))
@@ -714,7 +712,6 @@ async def _insert_composite(
                     sub = await target_svc._insert_composite(delay, **kwargs)
                     setattr(item, key, sub)
 
-            await session.commit()
         return item
 
     async def _insert(
@@ -754,8 +751,9 @@ class based recursive tree building fashion.
         nested = {}
         delayed = {}
 
-        # Relationships declared after initial instanciation are permissions.
-        for key in self.runtime_relationships:
+        for key in self.permission_relationships:
+            # IMPORTANT: Create an entry even for empty permissions.
+            # It is necessary in order to query permissions from nested entities.
             rel = self.table.relationships()[key]
             stmt_perm = self._backend_specific_insert(rel.target.decl_class)
 

src/biodm/error.py

@@ -1,10 +1,13 @@
+import json
 from http import HTTPStatus
 
 from biodm.utils.utils import json_response
 
 from .exceptions import (
+    FailedUpdate,
     RequestError,
     FailedDelete,
+    FailedRead,
     InvalidCollectionMethod,
     PayloadEmptyError,
     UnauthorizedError,
@@ -21,11 +24,11 @@ def __init__(self, status, detail=None) -> None:
 
     @property
     def __dict__(self):
-        return {'code': self.status, 'reason': self.reason, 'message': self.detail}
+        return {"code": self.status, "reason": self.reason, "message": self.detail}
 
     @property
     def response(self):
-        return json_response(data=self.__dict__, status_code=self.status)
+        return json_response(data=json.dumps(self.__dict__), status_code=self.status)
 
 async def onerror(_, exc):
     """Error event handler.
@@ -37,7 +40,7 @@ async def onerror(_, exc):
     if issubclass(exc.__class__, RequestError):
         detail = exc.detail
         match exc:
-            case FailedDelete():
+            case FailedDelete() | FailedRead() | FailedUpdate():
                 status = 404
             case InvalidCollectionMethod():
                 status = 405

src/biodm/exceptions.py

@@ -73,19 +73,19 @@ class ManifestError(RequestError):
 
 
 ## DB
-class FailedCreate(DBError):
+class FailedCreate(RequestError):
     """Could not create record."""
 
 
-class FailedRead(DBError):
+class FailedRead(RequestError):
     """Requested record doesn't exist."""
 
 
-class FailedUpdate(DBError):
+class FailedUpdate(RequestError):
     """Raised when an update operation is not successful."""
 
 
-class FailedDelete(DBError):
+class FailedDelete(RequestError):
     """Raised when a delete operation is not successful."""