Skip to content

Commit

Permalink
Provide clearer error message for ES relationship filtering
Browse files Browse the repository at this point in the history
  • Loading branch information
ml-evs committed May 14, 2021
1 parent fe8d549 commit 805d914
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
7 changes: 5 additions & 2 deletions optimade/filtertransformers/base_transformer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import abc
from typing import Dict
from typing import Dict, Union

from lark import Transformer, v_args

Expand Down Expand Up @@ -193,10 +193,13 @@ def not_implemented_string(self, value):
"""
raise NotImplementedError("Comparing strings is not yet implemented.")

def property(self, args):
def property(self, args) -> Union[Quantity, str]:
"""property: IDENTIFIER ( "." IDENTIFIER )*"""
quantity_name = str(args[0])

# If the quantity name matches an entry type (indicating a relationship filter)
# then simply return the quantity name; the inherited property
# must then handle any futher the nested identifiers
if self.mapper and quantity_name in self.mapper.RELATIONSHIP_ENTRY_TYPES:
return quantity_name

Expand Down
13 changes: 11 additions & 2 deletions optimade/filtertransformers/elasticsearch.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Dict
from typing import Dict, Union
from lark import v_args
from elasticsearch_dsl import Q, Text, Keyword, Integer, Field

Expand Down Expand Up @@ -78,7 +78,16 @@ def __init__(

super().__init__(mapper=mapper)

def _field(self, quantity, nested=None):
def _field(self, quantity: Union[str, Quantity], nested: Quantity = None):

if (
isinstance(quantity, str)
and quantity in self.mapper.RELATIONSHIP_ENTRY_TYPES
):
raise NotImplementedError(
f"Unable to filter on relationships with type {quantity!r}"
)

if nested is not None:
return "%s.%s" % (nested.backend_field, quantity.name)

Expand Down
8 changes: 4 additions & 4 deletions tests/server/query_params/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_list_length_comparisons_aliased(check_response, check_error_response):
request,
expected_status=501,
expected_title="NotImplementedError",
expected_detail="LENGTH is not supported for nsites",
expected_detail="LENGTH is not supported for 'nsites'",
)
else:
check_response(request, expected_ids)
Expand Down Expand Up @@ -382,9 +382,9 @@ def test_filter_on_relationships(check_response, check_error_response):
if CONFIG.database_backend == SupportedBackend.ELASTIC:
check_error_response(
request,
expected_status=400,
expected_title="Bad Request",
expected_detail="references is not a searchable quantity",
expected_status=501,
expected_title="NotImplementedError",
expected_detail="Unable to filter on relationships with type 'references'",
)
pytest.xfail("Elasticsearch backend does not support relationship filtering.")
check_response(request, expected_ids)
Expand Down

0 comments on commit 805d914

Please sign in to comment.