Merge branch 'master' into JPBergsma_check_response_single_entry

.github/workflows/ci.yml

@@ -241,15 +241,15 @@ jobs:
 
     - name: Upload coverage to Codecov
       if: matrix.python-version == 3.8 && github.repository == 'Materials-Consortia/optimade-python-tools'
-      uses: codecov/codecov-action@v2
+      uses: codecov/codecov-action@v3
       with:
         name: project
         file: ./coverage.xml
         flags: project
 
     - name: Upload validator coverage to Codecov
       if: matrix.python-version == 3.8 && github.repository == 'Materials-Consortia/optimade-python-tools'
-      uses: codecov/codecov-action@v2
+      uses: codecov/codecov-action@v3
       with:
         name: validator
         file: ./validator_cov.xml

.pre-commit-config.yaml

@@ -9,7 +9,7 @@ repos:
       name: Blacken
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.2.0
     hooks:
     - id: check-symlinks
     - id: check-yaml

INSTALL.md

@@ -85,7 +85,7 @@ will run the index meta-database server at <http://localhost:5001/v1>.
 
 In order to run the test suite for a specific backend, the
 `OPTIMADE_DATABASE_BACKEND` [environment variable (or config
-option)](https://www.optimade.org/optimade-python-tools/configuration/) can be
+option)](https://www.optimade.org/optimade-python-tools/latest/configuration/) can be
 set to one of `'mongodb'`, `'mongomock'` or `'elastic'` (see
 [`ServerConfig.database_backend`][optimade.server.config.ServerConfig.database_backend]).
 Tests for the two "real" database backends, MongoDB and Elasticsearch, require a writable, temporary database to be accessible.

optimade/filtertransformers/mongo.py

@@ -141,9 +141,9 @@ def property_first_comparison(self, quantity, query):
 
     def constant_first_comparison(self, arg):
         # constant_first_comparison: constant OPERATOR ( non_string_value | not_implemented_string )
-        return {
-            arg[2]: {self.operator_map[self._reversed_operator_map[arg[1]]]: arg[0]}
-        }
+        return self.property_first_comparison(
+            arg[2], {self.operator_map[self._reversed_operator_map[arg[1]]]: arg[0]}
+        )
 
     @v_args(inline=True)
     def value_op_rhs(self, operator, value):

optimade/server/config.py

@@ -267,6 +267,10 @@ class ServerConfig(BaseSettings):
         Path("/var/log/optimade/"),
         description="Folder in which log files will be saved.",
     )
+    validate_query_parameters: Optional[bool] = Field(
+        True,
+        description="If True, the server will check whether the query parameters given in the request are correct.",
+    )
 
     @validator("implementation", pre=True)
     def set_implementation_version(cls, v):

optimade/server/data/test_references.json

@@ -58,5 +58,25 @@
     "title": "Dummy reference that should remain orphaned from all structures for testing purposes",
     "journal": "JACS",
     "doi": "10.1038/00000"
+  },
+  {
+    "_id": {
+      "$oid": "98fb441f053b1744107019e3"
+    },
+    "id": "dummy/2022",
+    "last_modified": {
+      "$date": "2022-01-23T14:24:37.332Z"
+    },
+    "authors": [
+      {
+        "name": "A Nother",
+        "firstname": "A",
+        "lastname": "Nother"
+      }
+    ],
+    "year": "2019",
+    "note": "Dummy reference",
+    "title": "Just another title",
+    "journal": "JACS"
   }
 ]

optimade/server/entry_collections/mongo.py

@@ -123,8 +123,9 @@ def _run_db_query(
         if not single_entry:
             criteria_nolimit = criteria.copy()
             criteria_nolimit.pop("limit", None)
+            skip = criteria_nolimit.pop("skip", 0)
             data_returned = self.count(**criteria_nolimit)
-            more_data_available = nresults_now < data_returned
+            more_data_available = nresults_now + skip < data_returned
         else:
             # SingleEntryQueryParams, e.g., /structures/{entry_id}
             data_returned = nresults_now

optimade/server/query_params.py

@@ -1,10 +1,88 @@
 from fastapi import Query
 from pydantic import EmailStr  # pylint: disable=no-name-in-module
-
+from typing import Iterable, List
 from optimade.server.config import CONFIG
+from warnings import warn
+from optimade.server.mappers import BaseResourceMapper
+from optimade.server.exceptions import BadRequest
+from optimade.server.warnings import UnknownProviderQueryParameter, QueryParamNotUsed
+from abc import ABC
+
 
+class BaseQueryParams(ABC):
+    """A base class for query parameters that provides validation via the `check_params` method.
+
+    Attributes:
+        unsupported_params: Any string parameter listed here will raise a warning when passed to
+            the check_params methods. Useful for disabling optional OPTIMADE query parameters that
+            are not implemented by the server, e.g., cursor-based pagination.
 
-class EntryListingQueryParams:
+    """
+
+    unsupported_params: List[str] = []
+
+    def check_params(self, query_params: Iterable[str]) -> None:
+        """This method checks whether all the query parameters that are specified
+        in the URL string are implemented in the relevant `*QueryParams` class.
+
+        This method handles four cases:
+
+        * If a query parameter is passed that is not defined in the relevant `*QueryParams` class,
+          and it is not prefixed with a known provider prefix, then a `BadRequest` is raised.
+        * If a query parameter is passed that is not defined in the relevant `*QueryParams` class,
+          that is prefixed with a known provider prefix, then the parameter is silently ignored
+        * If a query parameter is passed that is not defined in the relevant `*QueryParams` class,
+          that is prefixed with an unknown provider prefix, then a `UnknownProviderQueryParameter`
+          warning is emitted.
+        * If a query parameter is passed that is on the `unsupported_params` list for the inherited
+          class, then a `QueryParamNotUsed` warning is emitted.
+
+        Arguments:
+            query_params: An iterable of the request's string query parameters.
+
+        Raises:
+            `BadRequest`: if the query parameter was not found in the relevant class, or if it
+                does not have a valid prefix.
+
+        """
+        if not getattr(CONFIG, "validate_query_parameters", False):
+            return
+        errors = []
+        warnings = []
+        unsupported_warnings = []
+        for param in query_params:
+            if param in self.unsupported_params:
+                unsupported_warnings.append(param)
+            if not hasattr(self, param):
+                split_param = param.split("_")
+                if param.startswith("_") and len(split_param) > 2:
+                    prefix = split_param[1]
+                    if prefix in BaseResourceMapper.SUPPORTED_PREFIXES:
+                        errors.append(param)
+                    elif prefix not in BaseResourceMapper.KNOWN_PROVIDER_PREFIXES:
+                        warnings.append(param)
+                else:
+                    errors.append(param)
+
+        if warnings:
+            warn(
+                f"The query parameter(s) '{warnings}' are unrecognised and have been ignored.",
+                UnknownProviderQueryParameter,
+            )
+
+        if unsupported_warnings:
+            warn(
+                f"The query parameter(s) '{unsupported_warnings}' are not supported by this server and have been ignored.",
+                QueryParamNotUsed,
+            )
+
+        if errors:
+            raise BadRequest(
+                f"The query parameter(s) '{errors}' are not recognised by this endpoint."
+            )
+
+
+class EntryListingQueryParams(BaseQueryParams):
     """
     Common query params for all Entry listing endpoints.
 
@@ -92,6 +170,14 @@ class EntryListingQueryParams:
 
     """
 
+    # The reference server implementation only supports offset-based pagination
+    unsupported_params: List[str] = [
+        "page_number",
+        "page_cursor",
+        "page_below",
+        "page_above",
+    ]
+
     def __init__(
         self,
         *,
@@ -169,9 +255,10 @@ def __init__(
         self.page_above = page_above
         self.page_below = page_below
         self.include = include
+        self.api_hint = api_hint
 
 
-class SingleEntryQueryParams:
+class SingleEntryQueryParams(BaseQueryParams):
     """
     Common query params for single entry endpoints.
 
@@ -244,3 +331,4 @@ def __init__(
         self.email_address = email_address
         self.response_fields = response_fields
         self.include = include
+        self.api_hint = api_hint

optimade/server/routers/utils.py

@@ -235,6 +235,7 @@ def get_entries(
     """Generalized /{entry} endpoint getter"""
     from optimade.server.routers import ENTRY_COLLECTIONS
 
+    params.check_params(request.query_params)
     (
         results,
         data_returned,
@@ -284,6 +285,7 @@ def get_single_entry(
 ) -> EntryResponseOne:
     from optimade.server.routers import ENTRY_COLLECTIONS
 
+    params.check_params(request.query_params)
     params.filter = f'id="{entry_id}"'
     (
         results,

optimade/server/warnings.py

@@ -8,10 +8,7 @@ def __init__(self, detail: str = None, title: str = None, *args) -> None:
         self.title = title if title else self.__class__.__name__
 
     def __repr__(self) -> str:
-        attrs = {
-            "detail": self.detail,
-            "title": self.title,
-        }
+        attrs = {"detail": self.detail, "title": self.title}
         return "<{:s}({:s})>".format(
             self.__class__.__name__,
             " ".join(
@@ -56,3 +53,10 @@ class UnknownProviderProperty(OptimadeWarning):
     recognised by this implementation.
 
     """
+
+
+class UnknownProviderQueryParameter(OptimadeWarning):
+    """A provider-specific query parameter has been requested in the query with a prefix not
+    recognised by this implementation.
+
+    """

requirements-client.txt

@@ -4,3 +4,4 @@ jarvis-tools==2022.1.10
 numpy==1.21.5;python_version<'3.8'
 numpy==1.22.0;python_version>='3.8'
 pymatgen==2022.0.16
+upf-to-json==0.9.2 # Can be removed if aiida-core pins to a working version (0.9.4 is broken)

requirements-dev.txt

@@ -1,8 +1,8 @@
 build==0.7.0
 codecov==2.1.12
 invoke==1.7.0
-jsondiff==1.3.1
-pre-commit==2.17.0
-pylint==2.13.2
-pytest==7.1.1
+jsondiff==2.0.0
+pre-commit==2.18.1
+pylint==2.13.7
+pytest==7.1.2
 pytest-cov==3.0.0

requirements-docs.txt

@@ -2,5 +2,5 @@ markupsafe==2.0.1 # Can be removed once aiida supports Jinja2>=3, see pallets/ma
 mike==1.1.2
 mkdocs==1.3.0
 mkdocs-awesome-pages-plugin==2.7.0
-mkdocs-material==8.2.8
+mkdocs-material==8.2.10
 mkdocstrings==0.18.1

requirements.txt

@@ -1,10 +1,10 @@
 elasticsearch-dsl==6.4.0
-email_validator==1.1.3
+email_validator==1.2.0
 fastapi==0.65.2
 lark-parser==0.12.0
 mongomock==4.0.0
 pydantic==1.9.0
-pymongo==4.0.2
+pymongo==4.1.1
 pyyaml==5.4
 requests==2.27.1
 typing-extensions==4.0.0;python_version<'3.8'

setup.py

@@ -25,7 +25,10 @@
 ] + mongo_deps
 
 # Client minded
-aiida_deps = ["aiida-core~=1.6,>=1.6.4"]
+aiida_deps = [
+    "aiida-core~=1.6,>=1.6.4",
+    "upf-to-json==0.9.2",  # Can be removed if aiida-core pins to a working version
+]
 ase_deps = ["ase~=3.22"]
 cif_deps = ["numpy~=1.21"]
 pdb_deps = cif_deps
@@ -45,12 +48,12 @@
 testing_deps = [
     "build~=0.7.0",
     "codecov~=2.1",
-    "jsondiff~=1.3",
+    "jsondiff~=2.0",
     "pytest~=7.1",
     "pytest-cov~=3.0",
 ] + server_deps
 dev_deps = (
-    ["pylint~=2.13", "pre-commit~=2.17", "invoke~=1.7"]
+    ["pylint~=2.13", "pre-commit~=2.18", "invoke~=1.7"]
     + docs_deps
     + testing_deps
     + client_deps
@@ -87,7 +90,7 @@
         "lark-parser~=0.12",
         "fastapi~=0.65.2",
         "pydantic~=1.9",
-        "email_validator~=1.1",
+        "email_validator~=1.2",
         "requests~=2.27",
         "typing-extensions~=4.0;python_version<'3.8'",
     ],

tests/filtertransformers/test_mongo.py

@@ -888,3 +888,8 @@ def test_special_cases(self):
                 {"number": {"$eq": 0.0}},
             ]
         }
+
+    def test_constant_first_comparisson(self):
+        assert self.transform("nelements != 5") == self.transform("5 != nelements")
+        assert self.transform("nelements > 5") == self.transform("5 < nelements")
+        assert self.transform("nelements <= 5") == self.transform("5 >= nelements")