Refactor ElasticBase to expose framework elements (#2438)

dbutenhof · web-flow · commit 5f355374d3a7 · 2021-09-03T15:31:11.000-04:00
Refactor ElasticBase to expose framework elements

I want to reuse Schema for a new API outside of the Elasticsearch
query context. I've pulled the conversion/normalization framework
up a level to server.api.resources, and reworked the post/put/get
delegation to (hopefully) support more general interfaces. (I expect
to use GET, PUT, DELETE, and possibly POST for metadata.)
diff --git a/lib/pbench/server/api/resources/__init__.py b/lib/pbench/server/api/resources/__init__.py
diff --git a/lib/pbench/server/api/resources/query_apis/__init__.py b/lib/pbench/server/api/resources/query_apis/__init__.py
diff --git a/lib/pbench/server/api/resources/query_apis/controllers_list.py b/lib/pbench/server/api/resources/query_apis/controllers_list.py
@@ -3,15 +3,14 @@
 from logging import Logger
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
-    CONTEXT,
-    ElasticBase,
+from pbench.server.api.resources import (
     JSON,
     Schema,
     Parameter,
     ParamType,
     PostprocessError,
 )
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 
 
 class ControllersList(ElasticBase):
diff --git a/lib/pbench/server/api/resources/query_apis/datasets_detail.py b/lib/pbench/server/api/resources/query_apis/datasets_detail.py
@@ -3,15 +3,14 @@
 from logging import Logger
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
-    CONTEXT,
-    ElasticBase,
+from pbench.server.api.resources import (
     JSON,
     Schema,
     Parameter,
     ParamType,
     PostprocessError,
 )
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 
 
 class DatasetsDetail(ElasticBase):
diff --git a/lib/pbench/server/api/resources/query_apis/datasets_list.py b/lib/pbench/server/api/resources/query_apis/datasets_list.py
@@ -2,14 +2,8 @@
 from logging import Logger
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
-    CONTEXT,
-    ElasticBase,
-    JSON,
-    Schema,
-    Parameter,
-    ParamType,
-)
+from pbench.server.api.resources import JSON, Schema, Parameter, ParamType
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 
 
 class DatasetsList(ElasticBase):
diff --git a/lib/pbench/server/api/resources/query_apis/datasets_publish.py b/lib/pbench/server/api/resources/query_apis/datasets_publish.py
@@ -5,16 +5,15 @@
 from flask_restful import abort
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
+from pbench.server.api.resources import (
     API_OPERATION,
-    CONTEXT,
-    ElasticBase,
     JSON,
-    PostprocessError,
     Schema,
     Parameter,
     ParamType,
+    PostprocessError,
 )
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 from pbench.server.database.models.datasets import Dataset, Metadata
 from pbench.server.database.models.users import User
 
diff --git a/lib/pbench/server/api/resources/query_apis/elasticsearch_api.py b/lib/pbench/server/api/resources/query_apis/elasticsearch_api.py
@@ -1,14 +1,8 @@
 from logging import Logger
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
-    CONTEXT,
-    ElasticBase,
-    JSON,
-    Schema,
-    Parameter,
-    ParamType,
-)
+from pbench.server.api.resources import JSON, Schema, Parameter, ParamType
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 
 
 class Elasticsearch(ElasticBase):
diff --git a/lib/pbench/server/api/resources/query_apis/index_search.py b/lib/pbench/server/api/resources/query_apis/index_search.py
@@ -4,15 +4,14 @@
 from flask import jsonify
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import (
-    CONTEXT,
-    ElasticBase,
+from pbench.server.api.resources import (
     JSON,
     Parameter,
     ParamType,
     PostprocessError,
     Schema,
 )
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 
 
 class IndexSearch(ElasticBase):
diff --git a/lib/pbench/server/api/resources/query_apis/month_indices.py b/lib/pbench/server/api/resources/query_apis/month_indices.py
@@ -2,7 +2,8 @@
 from logging import Logger
 
 from pbench.server import PbenchServerConfig
-from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase, JSON, Schema
+from pbench.server.api.resources import JSON, Schema
+from pbench.server.api.resources.query_apis import CONTEXT, ElasticBase
 from pbench.server.database.models.template import Template
 
 
diff --git a/lib/pbench/test/unit/server/conftest.py b/lib/pbench/test/unit/server/conftest.py
@@ -184,7 +184,13 @@ def login_user(client, server_config, username, password):
 
 
 @pytest.fixture()
-def create_user() -> User:
+def create_user(client) -> User:
+    """
+    Construct a test user and add it to the database.
+
+    Args:
+        client: Fixture to ensure we have a database
+    """
     user = User(
         email="test@example.com",
         password=generic_password,
@@ -197,7 +203,13 @@ def create_user() -> User:
 
 
 @pytest.fixture
-def create_admin_user() -> User:
+def create_admin_user(client) -> User:
+    """
+    Construct an admin user and add it to the database.
+
+    Args:
+        client: Fixture to ensure we have a database
+    """
     user = User(
         email=admin_email,
         password=generic_password,
diff --git a/lib/pbench/test/unit/server/query_apis/commons.py b/lib/pbench/test/unit/server/query_apis/commons.py
@@ -7,7 +7,8 @@
 from http import HTTPStatus
 from typing import AnyStr, Type
 
-from pbench.server.api.resources.query_apis import ElasticBase, JSON, ParamType
+from pbench.server.api.resources import JSON, ParamType
+from pbench.server.api.resources.query_apis import ElasticBase
 from pbench.test.unit.server.headertypes import HeaderTypes
 
 
diff --git a/lib/pbench/test/unit/server/query_apis/test_datasets_publish.py b/lib/pbench/test/unit/server/query_apis/test_datasets_publish.py
@@ -209,8 +209,8 @@ def test_partial_success(
         message = response_doc["message"]
         assert message == (
             "DatasetsPublish: the query postprocessor was unable to complete: "
-            "Postprocessing error returning 500: '1 of 4 Elasticsearch document UPDATE operations failed "
-            "[{'document_missing_exception': 1, 'ok': 3}]'"
+            "Postprocessing error returning 500: '1 of 4 Elasticsearch document UPDATE operations failed' "
+            "[{'document_missing_exception': 1, 'ok': 3}]"
         )
         summary = response_doc["data"]
         assert summary == {"ok": 3, "document_missing_exception": 1}
diff --git a/lib/pbench/test/unit/server/test_schema.py b/lib/pbench/test/unit/server/test_schema.py
@@ -1,19 +1,68 @@
+from http import HTTPStatus
+from typing import Callable
+
 import dateutil
 import pytest
-from typing import Callable
 
 from pbench.server.api.auth import Auth, UnknownUser
 from pbench.server.database.models.users import User
-from pbench.server.api.resources.query_apis import (
-    ParamType,
+from pbench.server.api.resources import (
+    API_OPERATION,
     ConversionError,
     InvalidRequestPayload,
     MissingParameters,
     Parameter,
+    ParamType,
+    PostprocessError,
     Schema,
+    SchemaError,
+    UnauthorizedAccess,
+    UnsupportedAccessMode,
+    UnverifiedUser,
 )
 
 
+class TestExceptions:
+    """
+    Test exception stringification
+    """
+
+    def test_exceptions(self, create_user):
+        e = UnauthorizedAccess(create_user, API_OPERATION.READ, "you", "public")
+        assert (
+            str(e)
+            == "User test is not authorized to READ a resource owned by you with public access"
+        )
+        assert e.user == create_user
+        e = UnauthorizedAccess(None, API_OPERATION.UPDATE, "me", "private")
+        assert (
+            str(e)
+            == "Unauthenticated client is not authorized to UPDATE a resource owned by me with private access"
+        )
+        s = SchemaError()
+        assert str(s) == "Generic schema error"
+        u = UnverifiedUser("you")
+        assert str(u) == "User you can not be verified"
+        i = InvalidRequestPayload()
+        assert str(i) == "Invalid request payload"
+        u = UnsupportedAccessMode("him", "private")
+        assert str(u) == "Unsupported mode him:private"
+        m = MissingParameters(["a", "b"])
+        assert str(m) == "Missing required parameters: a,b"
+        c = ConversionError({}, "str", "dict")
+        assert str(c) == "Value {} (dict) cannot be parsed as a str"
+        assert c.value == {}
+        p = PostprocessError(HTTPStatus.OK, "all's well", {"param": "none"})
+        assert (
+            str(p)
+            == "Postprocessing error returning 200: \"all's well\" [{'param': 'none'}]"
+        )
+        assert p.data == {"param": "none"}
+        p = PostprocessError(HTTPStatus.BAD_REQUEST, "really bad", None)
+        assert str(p) == "Postprocessing error returning 400: 'really bad' [None]"
+        assert p.status == HTTPStatus.BAD_REQUEST
+
+
 class TestParamType:
     """
     Tests on the ParamType enum
@@ -122,18 +171,17 @@ def test_invalid_required(self, test):
         assert x.invalid(json) is expected
 
     @pytest.mark.parametrize(
-        "test",
-        (
-            ({"data": "yes"}, False),
-            ({"data": None}, True),
-            ({"foo": "yes"}, False),
-            ({"foo": None}, False),
-        ),
+        "test", ({"data": "yes"}, {"data": None}, {"foo": "yes"}, {"foo": None},),
     )
     def test_invalid_optional(self, test):
+        """
+        An optional parameter is either present or not: either is OK, and the
+        value None is acceptable. (In other words, the "invalid" test isn't
+        meaningful for required=False parameters, and should always succeed.)
+        """
         x = Parameter("data", ParamType.STRING, required=False)
-        json, expected = test
-        assert x.invalid(json) is expected
+        json = test
+        assert not x.invalid(json)
 
 
 class TestSchema:
