Repository: Materials-Consortia/optimade-python-tools (python)

Issue:
Implement `warnings`
It would be cool to add something similar to the `Exception` handlers for `Warnings`.

Perhaps it could be done with `middleware`? Or something similar.

What I have in mind is to catch `from warnings import warn` warnings and add them, as [per the OPTiMaDe spec](https://github.com/Materials-Consortia/OPTiMaDe/blob/develop/optimade.rst#warnings), under `meta.warnings`.

Pull request:
New middleware to catch any `OptimadeWarning`s
This is put as the first added middleware, since the order in which middleware is added to the application matters.
See https://www.starlette.io/middleware/

The middleware is a bit hacky, since it's based on Starlette's `BaseHTTPMiddleware`, which uses the `call_next` function that creates a `StreamingResponse`.
This middleware will temporarily stitch the response back together, add the warnings, and chop up the response again, returning a new `StreamingResponse`.

Another thing that's a bit hacky about this is the way in which warnings are collected. I currently use an in-memory attribute from the middleware class, however, I am not sure if this is thread-safe.

Note, I have tested this by raising a simple warning in a router and it seemed to work :)

Closes #105

---

Missing:
- [x] Tests
- [x] Add in all "TODO" warnings littered around in the code.
- [x] Tests for ^these warnings.

Patch:
diff --git a/optimade/adapters/structures/proteindatabank.py b/optimade/adapters/structures/proteindatabank.py
index 1b6d2643..caf7eb49 100644
--- a/optimade/adapters/structures/proteindatabank.py
+++ b/optimade/adapters/structures/proteindatabank.py
@@ -84,7 +84,7 @@ def get_pdbx_mmcif(  # pylint: disable=too-many-locals
                 cartesian_positions=attributes.cartesian_site_positions,
             )
 
-    # TODO: The following lines are perhaps needed to create a "valid" PDBx/mmCIF file.
+    # NOTE: The following lines are perhaps needed to create a "valid" PDBx/mmCIF file.
     # However, at the same time, the information here is "default" and will for all structures "at this moment in time"
     # be the same. I.e., no information is gained by adding this now.
     # If it is found that they indeed are needed to create a "valid" PDBx/mmCIF file, they should be included in the output.
diff --git a/optimade/server/exception_handlers.py b/optimade/server/exception_handlers.py
index 42f66c2c..72972b54 100644
--- a/optimade/server/exception_handlers.py
+++ b/optimade/server/exception_handlers.py
@@ -47,24 +47,16 @@ def general_exception(
     if errors is None:
         errors = [OptimadeError(detail=detail, status=http_response_code, title=title)]
 
-    try:
-        response = ErrorResponse(
-            meta=meta_values(
-                url=str(request.url),
-                data_returned=0,
-                data_available=0,
-                more_data_available=False,
-                **debug_info,
-            ),
-            errors=errors,
-        )
-    except Exception:
-        # This was introduced due to the original raise of an HTTPException if the
-        # path prefix could not be found, e.g., `/v0`.
-        # However, due to the testing, this error cannot be raised anymore.
-        # Instead, an OPTIMADE warning should be issued.
-        # Having this try and except is still good practice though.
-        response = ErrorResponse(errors=errors)
+    response = ErrorResponse(
+        meta=meta_values(
+            url=str(request.url),
+            data_returned=0,
+            data_available=0,
+            more_data_available=False,
+            **debug_info,
+        ),
+        errors=errors,
+    )
 
     return JSONResponse(
         status_code=http_response_code,
diff --git a/optimade/server/main.py b/optimade/server/main.py
index 212fb8ed..2e97e6a5 100644
--- a/optimade/server/main.py
+++ b/optimade/server/main.py
@@ -12,7 +12,11 @@ import optimade.server.exception_handlers as exc_handlers
 
 from .entry_collections import MongoCollection
 from .config import CONFIG
-from .middleware import EnsureQueryParamIntegrity, CheckWronglyVersionedBaseUrls
+from .middleware import (
+    AddWarnings,
+    CheckWronglyVersionedBaseUrls,
+    EnsureQueryParamIntegrity,
+)
 from .routers import info, links, references, structures, landing, versions
 from .routers.utils import get_providers, BASE_URL_PREFIXES
 
@@ -56,6 +60,7 @@ if not CONFIG.use_real_mongo:
 
 
 # Add various middleware
+app.add_middleware(AddWarnings)
 app.add_middleware(CORSMiddleware, allow_origins=["*"])
 app.add_middleware(EnsureQueryParamIntegrity)
 app.add_middleware(CheckWronglyVersionedBaseUrls)
diff --git a/optimade/server/main_index.py b/optimade/server/main_index.py
index 33acce68..c60c2057 100644
--- a/optimade/server/main_index.py
+++ b/optimade/server/main_index.py
@@ -13,8 +13,9 @@ import optimade.server.exception_handlers as exc_handlers
 
 from optimade.server.config import CONFIG
 from optimade.server.middleware import (
-    EnsureQueryParamIntegrity,
+    AddWarnings,
     CheckWronglyVersionedBaseUrls,
+    EnsureQueryParamIntegrity,
 )
 from optimade.server.routers import index_info, links, versions
 from optimade.server.routers.utils import BASE_URL_PREFIXES
@@ -62,6 +63,7 @@ if not CONFIG.use_real_mongo and CONFIG.index_links_path.exists():
 
 
 # Add various middleware
+app.add_middleware(AddWarnings)
 app.add_middleware(CORSMiddleware, allow_origins=["*"])
 app.add_middleware(EnsureQueryParamIntegrity)
 app.add_middleware(CheckWronglyVersionedBaseUrls)
diff --git a/optimade/server/middleware.py b/optimade/server/middleware.py
index 44fb8055..63de66d5 100644
--- a/optimade/server/middleware.py
+++ b/optimade/server/middleware.py
@@ -1,8 +1,12 @@
+from typing import Optional, IO, Type, Generator
 import urllib.parse
+import warnings
 
 from starlette.middleware.base import BaseHTTPMiddleware
 from starlette.requests import Request
 
+from optimade.server.warnings import OptimadeWarning
+
 
 class EnsureQueryParamIntegrity(BaseHTTPMiddleware):
     """Ensure all query parameters are followed by an equal sign (`=`)"""
@@ -54,7 +58,8 @@ class CheckWronglyVersionedBaseUrls(BaseHTTPMiddleware):
                 version_prefix = re.findall(r"(/v[0-9]+(\.[0-9]+){0,2})", optimade_path)
                 raise VersionNotSupported(
                     detail=(
-                        f"The parsed versioned base URL {version_prefix[0][0]!r} from {urllib.parse.urlunparse(parsed_url)!r} is not supported by this implementation. "
+                        f"The parsed versioned base URL {version_prefix[0][0]!r} from "
+                        f"{urllib.parse.urlunparse(parsed_url)!r} is not supported by this implementation. "
                         f"Supported versioned base URLs are: {', '.join(BASE_URL_PREFIXES.values())}"
                     )
                 )
@@ -65,3 +70,191 @@ class CheckWronglyVersionedBaseUrls(BaseHTTPMiddleware):
             self.check_url(parsed_url)
         response = await call_next(request)
         return response
+
+
+class AddWarnings(BaseHTTPMiddleware):
+    """
+    Add [`OptimadeWarning`][optimade.server.warnings.OptimadeWarning]s to the response.
+
+    All sub-classes of [`OptimadeWarning`][optimade.server.warnings.OptimadeWarning]
+    will also be added to the response's `meta.warnings` list.
+
+    By overriding the `warnings.showwarning()` function with the
+    [showwarning method][optimade.server.middleware.AddWarnings.showwarning],
+    all usages of `warnings.warn()` will result in the regular printing of the
+    warning message to `stderr`, but also its addition to an in-memory list of
+    warnings.
+    This middleware will, after the URL request has been handled, add the list of
+    accumulated warnings to the JSON response under the `meta.warnings` field.
+
+    To make sure the last part happens correctly and a Starlette `StreamingResponse`
+    is returned, as is expected from a `BaseHTTPMiddleware` sub-class, one is
+    instantiated with the updated `Content-Length` header, as well as making sure
+    the response's body content is actually streamable, by breaking it down into
+    chunks of the original response's chunk size.
+
+    Attributes:
+        _warnings (List[Warnings]): List of [Warnings][optimade.models.optimade_json.Warnings]
+            added through usages of `warnings.warn()` via [showwarning][optimade.server.middleware.AddWarnings.showwarning].
+
+    """
+
+    def showwarning(
+        self,
+        message: Warning,
+        category: Type[Warning],
+        filename: str,
+        lineno: int,
+        file: Optional[IO] = None,
+        line: Optional[str] = None,
+    ) -> None:
+        """
+        Hook to write a warning to a file using the built-in `warnings` lib.
+
+        In [the documentation](https://docs.python.org/3/library/warnings.html)
+        for the built-in `warnings` library, there are a few recommended ways of
+        customizing the printing of warning messages.
+
+        This method can override the `warnings.showwarning` function,
+        which is called as part of the `warnings` library's workflow to print
+        warning messages, e.g., when using `warnings.warn()`.
+        Originally, it prints warning messages to `stderr`.
+        This method will also print warning messages to `stderr` by calling
+        `warnings._showwarning_orig()` or `warnings._showwarnmsg_impl()`.
+        The first function will be called if the issued warning is not recognized
+        as an [`OptimadeWarning`][optimade.server.warnings.OptimadeWarning].'
+        This is equivalent to "standard behaviour".
+        The second function will be called _after_ an
+        [`OptimadeWarning`][optimade.server.warnings.OptimadeWarning] has been handled.
+
+        An [`OptimadeWarning`][optimade.server.warnings.OptimadeWarning] will be
+        translated into an OPTIMADE Warnings JSON object in accordance with
+        [the specification](https://github.com/Materials-Consortia/OPTIMADE/blob/v1.0.0/optimade.rst#json-response-schema-common-fields).
+        This process is similar to the [Exception handlers][optimade.server.exception_handlers].
+
+        Parameters:
+            message (Warning): The `Warning` object to show and possibly handle.
+            category (Type[Warning]): `Warning` type being warned about. This amounts to `type(message)`.
+            filename (str): Name of the file, where the warning was issued.
+            lineno (int): Line number in the file, where the warning was issued.
+            file (Optional[IO]): A file-like object to which the warning should be written.
+            line (Optional[str]): Source content of the line that issued the warning.
+
+        """
+        from optimade.models import Warnings
+        from optimade.server.config import CONFIG
+
+        assert isinstance(
+            message, Warning
+        ), "'message' is expected to be a Warning or subclass thereof."
+
+        if not isinstance(message, OptimadeWarning):
+            # If the Warning is not an OptimadeWarning or subclass thereof,
+            # use the regular 'showwarning' function.
+            warnings._showwarning_orig(message, category, filename, lineno, file, line)
+            return
+
+        # Format warning
+        try:
+            title = str(message.title)
+        except AttributeError:
+            title = str(message.__class__.__name__)
+
+        try:
+            detail = str(message.detail)
+        except AttributeError:
+            detail = str(message)
+
+        if CONFIG.debug:
+            if line is None:
+                # All this is taken directly from the warnings library.
+                # See 'warnings._formatwarnmsg_impl()' for the original code.
+                try:
+                    import linecache
+
+                    line = linecache.getline(filename, lineno)
+                except Exception:
+                    # When a warning is logged during Python shutdown, linecache
+                    # and the import machinery don't work anymore
+                    line = None
+                    linecache = None
+            meta = {
+                "filename": filename,
+                "lineno": lineno,
+            }
+            if line:
+                meta["line"] = line.strip()
+
+        if CONFIG.debug:
+            new_warning = Warnings(title=title, detail=detail, meta=meta)
+        else:
+            new_warning = Warnings(title=title, detail=detail)
+
+        # Add new warning to self._warnings
+        self._warnings.append(new_warning.dict(exclude_unset=True))
+
+        # Show warning message as normal in sys.stderr
+        warnings._showwarnmsg_impl(
+            warnings.WarningMessage(message, category, filename, lineno, file, line)
+        )
+
+    @staticmethod
+    def chunk_it_up(content: str, chunk_size: int) -> Generator:
+        """
+        Return generator for string in chunks of size `chunk_size`
+
+        Parameters:
+            content (str): String-content to separate into chunks.
+            chunk_size (int): The size of the chunks, i.e. the length of the string-chunks.
+
+        """
+        return (content[i : chunk_size + i] for i in range(0, len(content), chunk_size))
+
+    async def dispatch(self, request: Request, call_next):
+        from starlette.responses import StreamingResponse
+
+        try:
+            import simplejson as json
+        except ImportError:
+            import json
+
+        self._warnings = []
+
+        warnings.simplefilter(action="default", category=OptimadeWarning)
+        warnings.showwarning = self.showwarning
+
+        response = await call_next(request)
+
+        status = response.status_code
+        headers = response.headers
+        media_type = response.media_type
+        background = response.background
+        charset = response.charset
+
+        body = b""
+        first_run = True
+        async for chunk in response.body_iterator:
+            if first_run:
+                first_run = False
+                chunk_size = len(chunk)
+            if not isinstance(chunk, bytes):
+                chunk = chunk.encode(charset)
+            body += chunk
+        body = body.decode(charset)
+
+        if self._warnings:
+            response = json.loads(body)
+            response.get("meta", {})["warnings"] = self._warnings
+            body = json.dumps(response)
+            if "content-length" in headers:
+                headers["content-length"] = str(len(body))
+
+        response = StreamingResponse(
+            content=self.chunk_it_up(body, chunk_size),
+            status_code=status,
+            headers=headers,
+            media_type=media_type,
+            background=background,
+        )
+
+        return response
diff --git a/optimade/server/routers/utils.py b/optimade/server/routers/utils.py
index d2d7efa2..56a8361f 100644
--- a/optimade/server/routers/utils.py
+++ b/optimade/server/routers/utils.py
@@ -144,8 +144,8 @@ def get_included_relationships(
             if entry_relationship is not None:
                 refs = entry_relationship.get("data", [])
                 for ref in refs:
-                    # could check here and raise a warning if any IDs clash
-                    endpoint_includes[entry_type][ref["id"]] = ref
+                    if ref["id"] not in endpoint_includes[entry_type]:
+                        endpoint_includes[entry_type][ref["id"]] = ref
 
     included = {}
     for entry_type in endpoint_includes:
diff --git a/optimade/server/warnings.py b/optimade/server/warnings.py
new file mode 100644
index 00000000..55d1c4e6
--- /dev/null
+++ b/optimade/server/warnings.py
@@ -0,0 +1,35 @@
+class OptimadeWarning(Warning):
+    """Base Warning for the `optimade` package"""
+
+    def __init__(self, detail: str = None, title: str = None, *args) -> None:
+        detail = detail if detail else self.__doc__
+        super().__init__(detail, *args)
+        self.detail = detail
+        self.title = title if title else self.__class__.__name__
+
+    def __repr__(self) -> str:
+        attrs = {
+            "detail": self.detail,
+            "title": self.title,
+        }
+        return "<{:s}({:s})>".format(
+            self.__class__.__name__,
+            " ".join(
+                [
+                    f"{attr}={value!r}"
+                    for attr, value in attrs.items()
+                    if value is not None
+                ]
+            ),
+        )
+
+    def __str__(self) -> str:
+        return self.detail if self.detail is not None else ""
+
+
+class FieldNotCreated(OptimadeWarning):
+    """A non-essential field could not be created"""
+
+
+class UnmatchedValues(OptimadeWarning):
+    """Values of the same field or resource differ, where they should be equal"""


Test patch:
diff --git a/tests/server/test_middleware.py b/tests/server/test_middleware.py
index 9e362d89..301d4093 100644
--- a/tests/server/test_middleware.py
+++ b/tests/server/test_middleware.py
@@ -151,3 +151,79 @@ def test_multiple_versions_in_path(both_clients):
     finally:
         if org_base_url:
             CONFIG.base_url = org_base_url
+
+
+def test_showwarning_overload(both_clients, recwarn):
+    """Make sure warnings.showwarning can be overloaded correctly"""
+    import warnings
+
+    from optimade.server.middleware import AddWarnings
+    from optimade.server.warnings import OptimadeWarning
+
+    add_warning_middleware = AddWarnings(both_clients.app)
+    # Set up things that are setup usually in `dispatch()`
+    add_warning_middleware._warnings = []
+
+    warnings.showwarning = add_warning_middleware.showwarning
+
+    warning_message = "It's all gone awry!"
+
+    warnings.warn(OptimadeWarning(detail=warning_message))
+
+    assert add_warning_middleware._warnings == [
+        {"title": OptimadeWarning.__name__, "detail": warning_message}
+    ]
+
+    # Make sure a "normal" warning is treated as usual
+    warnings.warn(warning_message, UserWarning)
+    assert len(add_warning_middleware._warnings) == 1
+    assert len(recwarn.list) == 2
+    assert recwarn.pop(OptimadeWarning)
+    assert recwarn.pop(UserWarning)
+
+
+def test_showwarning_debug(both_clients, recwarn):
+    """Make sure warnings.showwarning adds 'meta' field in DEBUG MODE"""
+    import warnings
+
+    from optimade.server.config import CONFIG
+    from optimade.server.middleware import AddWarnings
+    from optimade.server.warnings import OptimadeWarning
+
+    add_warning_middleware = AddWarnings(both_clients.app)
+    # Set up things that are setup usually in `dispatch()`
+    add_warning_middleware._warnings = []
+
+    warnings.showwarning = add_warning_middleware.showwarning
+
+    warning_message = "It's all gone awry!"
+
+    org_debug = CONFIG.debug
+    try:
+        CONFIG.debug = True
+
+        warnings.warn(OptimadeWarning(detail=warning_message))
+
+        assert add_warning_middleware._warnings != [
+            {"title": OptimadeWarning.__name__, "detail": warning_message}
+        ]
+        warning = add_warning_middleware._warnings[0]
+        assert "meta" in warning
+        for meta_field in ("filename", "lineno", "line"):
+            assert meta_field in warning["meta"]
+        assert len(recwarn.list) == 1
+        assert recwarn.pop(OptimadeWarning)
+    finally:
+        CONFIG.debug = org_debug
+
+
+def test_chunk_it_up():
+    """Make sure all content is return from `chunk_it_up` generator"""
+    from optimade.server.middleware import AddWarnings
+
+    content = "Oh what a sad and tragic waste of a young attractive life!"
+    chunk_size = 16
+    assert len(content) % chunk_size != 0, "We want a rest bit, this isn't helpful!"
+
+    generator = AddWarnings.chunk_it_up(content, chunk_size)
+    assert "".join(generator) == content