Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add database page limit #139

Merged
merged 3 commits into from
Jan 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions openapi/index_openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -291,7 +291,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -480,7 +480,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down
18 changes: 9 additions & 9 deletions openapi/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -312,7 +312,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -553,7 +553,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -873,7 +873,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -1033,7 +1033,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -1274,7 +1274,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -1594,7 +1594,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -1754,7 +1754,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down Expand Up @@ -1995,7 +1995,7 @@
"title": "Page Limit",
"minimum": 0.0,
"type": "integer",
"default": 500
"default": 20
},
"name": "page_limit",
"in": "query"
Expand Down
3 changes: 2 additions & 1 deletion optimade/server/config.ini
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ REFERENCES_COLLECTION = references
STRUCTURES_COLLECTION = structures

[SERVER]
PAGE_LIMIT = 500
PAGE_LIMIT = 20
PAGE_LIMIT_MAX = 500
DEFAULT_DB = test_server
; BASE_URL = http://localhost:5000

Expand Down
15 changes: 11 additions & 4 deletions optimade/server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def _DEFAULTS(field: str) -> Any:
"references_collection": "references",
"structures_collection": "structures",
"page_limit": 20,
"page_limit_max": 500,
"default_db": "test_server",
"base_url": None,
"implementation": {
Expand Down Expand Up @@ -139,6 +140,9 @@ def load_from_ini(self):
self.page_limit = config.getint(
"SERVER", "PAGE_LIMIT", fallback=self._DEFAULTS("page_limit")
)
self.page_limit_max = config.getint(
"SERVER", "PAGE_LIMIT_MAX", fallback=self._DEFAULTS("page_limit_max")
)
self.default_db = config.get(
"SERVER", "DEFAULT_DB", fallback=self._DEFAULTS("default_db")
)
Expand All @@ -164,9 +168,9 @@ def load_from_ini(self):
self.provider_fields = {}
for endpoint in {"links", "references", "structures"}:
self.provider_fields[endpoint] = (
{field for field, _ in config[endpoint].items() if _ == ""}
list({field for field, _ in config[endpoint].items() if _ == ""})
if endpoint in config
else set()
else []
)

# MONGO collections
Expand Down Expand Up @@ -201,6 +205,9 @@ def load_from_json(self):
)

self.page_limit = int(config.get("page_limit", self._DEFAULTS("page_limit")))
self.page_limit_max = int(
config.get("page_limit_max", self._DEFAULTS("page_limit_max"))
)
self.default_db = config.get("default_db", self._DEFAULTS("default_db"))
self.base_url = config.get("base_url", self._DEFAULTS("base_url"))

Expand All @@ -217,8 +224,8 @@ def load_from_json(self):
self.provider = config.get("provider", self._DEFAULTS("provider"))
self.provider_fields = {}
for endpoint in {"structures", "references"}:
self.provider_fields[endpoint] = set(
config.get("provider_fields", {}).get(endpoint, [])
self.provider_fields[endpoint] = list(
set(config.get("provider_fields", {}).get(endpoint, []))
)


Expand Down
16 changes: 7 additions & 9 deletions optimade/server/entry_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ def __init__(

self.provider = CONFIG.provider["prefix"]
self.provider_fields = CONFIG.provider_fields.get(resource_mapper.ENDPOINT, [])
self.page_limit = CONFIG.page_limit
self.parser = LarkParser(
version=(0, 10, 1), variant="default"
) # The MongoTransformer only supports v0.10.0 as the latest grammar
) # The MongoTransformer only supports v0.10.1 as the latest grammar

def __len__(self):
return self.collection.estimated_document_count()
Expand Down Expand Up @@ -176,16 +175,15 @@ def _parse_params(
)

if getattr(params, "page_limit", False):
limit = self.page_limit
if params.page_limit != self.page_limit:
limit = params.page_limit
if limit > self.page_limit:
limit = params.page_limit
if limit > CONFIG.page_limit_max:
raise HTTPException(
status_code=400, detail=f"Max page_limit is {self.page_limit}"
status_code=403, # Forbidden
detail=f"Max allowed page_limit is {CONFIG.page_limit_max}, you requested {limit}",
)
if limit == 0:
limit = self.page_limit
cursor_kwargs["limit"] = limit
else:
cursor_kwargs["limit"] = CONFIG.page_limit

# All OPTiMaDe fields
fields = self.resource_mapper.TOP_LEVEL_NON_ATTRIBUTES_FIELDS.copy()
Expand Down
2 changes: 1 addition & 1 deletion optimade/server/mappers/entries.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def all_aliases(cls) -> Tuple[Tuple[str, str]]:
return (
tuple(
(CONFIG.provider["prefix"] + field, field)
for field in CONFIG.provider_fields.get(cls.ENDPOINT, {})
for field in CONFIG.provider_fields.get(cls.ENDPOINT, [])
)
+ cls.ALIASES
)
Expand Down
64 changes: 51 additions & 13 deletions tests/server/test_server.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable=no-member,wrong-import-position
# pylint: disable=no-member,wrong-import-position,import-outside-toplevel
import unittest
import abc

Expand Down Expand Up @@ -336,20 +336,37 @@ def test_page_limit(self):
expected_return = 6
self._check_response(request, expected_ids, expected_return)

def test_page_limit_max(self):
from optimade.server.config import CONFIG

request = f"/structures?page_limit={CONFIG.page_limit_max + 1}"
self._check_error_response(
request,
expected_status=403,
expected_title="HTTPException",
expected_detail=f"Max allowed page_limit is {CONFIG.page_limit_max}, you requested {CONFIG.page_limit_max + 1}",
)

def test_list_has_all(self):
request = '/structures?filter=elements HAS ALL "Ba","F","H","Mn","O","Re","Si"'
self._check_not_implemented(request)
self._check_error_response(
request, expected_status=501, expected_title="NotImplementedError"
)
# expected_ids = ["mpf_3819"]
# self._check_response(request, expected_ids, len(expected_ids))

request = '/structures?filter=elements HAS ALL "Re","Ti"'
self._check_not_implemented(request)
self._check_error_response(
request, expected_status=501, expected_title="NotImplementedError"
)
# expected_ids = ["mpf_3819"]
# self._check_response(request, expected_ids, len(expected_ids))

def test_list_has_any(self):
request = '/structures?filter=elements HAS ANY "Re","Ti"'
self._check_not_implemented(request)
self._check_error_response(
request, expected_status=501, expected_title="NotImplementedError"
)
# expected_ids = ["mpf_3819"]
# self._check_response(request, expected_ids, len(expected_ids))

Expand All @@ -364,25 +381,39 @@ def test_list_length_basic(self):
def test_list_length(self):
request = "/structures?filter=elements LENGTH >= 9"
error_detail = "Operator >= not implemented for LENGTH filter."
self._check_not_implemented(request, expected_detail=error_detail)
self._check_error_response(
request,
expected_status=501,
expected_title="NotImplementedError",
expected_detail=error_detail,
)
# expected_ids = ["mpf_3819"]
# self._check_response(request, expected_ids, len(expected_ids))

request = "/structures?filter=structure_features LENGTH > 0"
error_detail = "Operator > not implemented for LENGTH filter."
self._check_not_implemented(request, expected_detail=error_detail)
self._check_error_response(
request,
expected_status=501,
expected_title="NotImplementedError",
expected_detail=error_detail,
)
# expected_ids = []
# self._check_response(request, expected_ids, len(expected_ids))

def test_list_has_only(self):
request = '/structures?filter=elements HAS ONLY "Ac"'
self._check_not_implemented(request)
self._check_error_response(
request, expected_status=501, expected_title="NotImplementedError"
)
# expected_ids = ["mpf_1"]
# self._check_response(request, expected_ids, len(expected_ids))

def test_list_correlated(self):
request = '/structures?filter=elements:elements_ratios HAS "Ag":"0.2"'
self._check_not_implemented(request)
self._check_error_response(
request, expected_status=501, expected_title="NotImplementedError"
)
# expected_ids = ["mpf_259"]
# self._check_response(request, expected_ids, len(expected_ids))

Expand Down Expand Up @@ -482,21 +513,28 @@ def _check_response(self, request, expected_ids, expected_return):
print(f"{self.client.base_url}{request}")
raise exc

def _check_not_implemented(self, request, expected_detail: str = None):
def _check_error_response(
self,
request,
expected_status: int = 500,
expected_title: str = None,
expected_detail: str = None,
):
try:
response = self.client.get(request)
self.assertEqual(
response.status_code,
501,
msg=f"Request should have failed, but did not: {response.json()}",
expected_status,
msg=f"Request should have been an error with status code {expected_status}, "
f"but instead {response.status_code} was received.\nResponse:\n{response.json()}",
)
response = response.json()
self.assertEqual(len(response["errors"]), 1)
self.assertEqual(response["meta"]["data_returned"], 0)

error = response["errors"][0]
self.assertEqual("501", error["status"])
self.assertEqual("NotImplementedError", error["title"])
self.assertEqual(str(expected_status), error["status"])
self.assertEqual(expected_title, error["title"])

if expected_detail is None:
expected_detail = "Error trying to process rule "
Expand Down