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

Change aliasing method names in mapper and deprecate the old #746

Merged
merged 3 commits into from
Mar 23, 2021
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
10 changes: 5 additions & 5 deletions optimade/filtertransformers/mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,15 @@ def _apply_aliases(self, filter_: dict) -> dict:
return filter_

def check_for_alias(prop, expr):
return self.mapper.alias_for(prop) != prop
return self.mapper.get_backend_field(prop) != prop

def apply_alias(subdict, prop, expr):
if isinstance(subdict, dict):
subdict[self.mapper.alias_for(prop)] = self._apply_aliases(
subdict[self.mapper.get_backend_field(prop)] = self._apply_aliases(
subdict.pop(prop)
)
elif isinstance(subdict, str):
subdict = self.mapper.alias_for(subdict)
subdict = self.mapper.get_backend_field(subdict)

return subdict

Expand Down Expand Up @@ -396,7 +396,7 @@ def replace_str_id_with_objectid(subdict, prop, expr):
val = subdict[prop][operator]
if operator not in ("$eq", "$ne"):
if self.mapper is not None:
prop = self.mapper.alias_of(prop)
prop = self.mapper.get_optimade_field(prop)
raise NotImplementedError(
f"Operator {operator} not supported for query on field {prop!r}, can only test for equality"
)
Expand All @@ -417,7 +417,7 @@ def _apply_mongo_date_filter(self, filter_: dict) -> dict:
def check_for_timestamp_field(prop, _):
""" Find cases where the query dict is operating on a timestamp field. """
if self.mapper is not None:
prop = self.mapper.alias_of(prop)
prop = self.mapper.get_optimade_field(prop)
return prop == "last_modified"

def replace_str_date_with_datetime(subdict, prop, expr):
Expand Down
16 changes: 9 additions & 7 deletions optimade/server/entry_collections/elasticsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def __init__(

quantities = {}
for field in self.all_fields:
alias = self.resource_mapper.alias_for(field)
alias = self.resource_mapper.get_backend_field(field)
length_alias = self.resource_mapper.length_alias_for(field)

quantities[field] = Quantity(name=field, es_field=alias)
Expand Down Expand Up @@ -90,9 +90,9 @@ def create_optimade_index(self) -> None:

properties = {}
for field in list(body["mappings"]["doc"]["properties"].keys()):
properties[self.resource_mapper.alias_for(field)] = body["mappings"]["doc"][
"properties"
].pop(field)
properties[self.resource_mapper.get_backend_field(field)] = body[
"mappings"
]["doc"]["properties"].pop(field)
body["mappings"]["doc"]["properties"] = properties
self.client.indices.create(index=self.name, body=body, ignore=400)

Expand Down Expand Up @@ -123,7 +123,7 @@ def create_elastic_index_from_mapper(
"mappings": {
"doc": {
"properties": {
resource_mapper.alias_of(field): {"type": "keyword"}
resource_mapper.get_optimade_field(field): {"type": "keyword"}
for field in fields
}
}
Expand Down Expand Up @@ -196,7 +196,7 @@ def _run_db_query(
limit = criteria.get("limit", CONFIG.page_limit)

all_aliased_fields = [
self.resource_mapper.alias_for(field) for field in self.all_fields
self.resource_mapper.get_backend_field(field) for field in self.all_fields
]
search = search.source(includes=all_aliased_fields)

Expand All @@ -205,7 +205,9 @@ def _run_db_query(
for field, sort_dir in criteria.get("sort", {})
]
if not elastic_sort:
elastic_sort = {self.resource_mapper.alias_for("id"): {"order": "asc"}}
elastic_sort = {
self.resource_mapper.get_backend_field("id"): {"order": "asc"}
}

search = search.sort(*elastic_sort)

Expand Down
7 changes: 4 additions & 3 deletions optimade/server/entry_collections/entry_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ def handle_query_params(
cursor_kwargs["limit"] = CONFIG.page_limit

cursor_kwargs["projection"] = {
f"{self.resource_mapper.alias_for(f)}": True for f in self.all_fields
f"{self.resource_mapper.get_backend_field(f)}": True
for f in self.all_fields
}

if "_id" not in cursor_kwargs["projection"]:
Expand Down Expand Up @@ -316,13 +317,13 @@ def parse_sort_params(self, sort_params) -> List[Tuple[str, int]]:
if field.startswith("-"):
field = field[1:]
sort_dir = -1
aliased_field = self.resource_mapper.alias_for(field)
aliased_field = self.resource_mapper.get_backend_field(field)
sort_spec.append((aliased_field, sort_dir))

unknown_fields = [
field
for field, _ in sort_spec
if self.resource_mapper.alias_of(field) not in self.all_fields
if self.resource_mapper.get_optimade_field(field) not in self.all_fields
]

if unknown_fields:
Expand Down
95 changes: 85 additions & 10 deletions optimade/server/mappers/entries.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from typing import Tuple, Optional
import warnings

__all__ = ("BaseResourceMapper",)

Expand Down Expand Up @@ -37,8 +38,10 @@ class BaseResourceMapper:

@classmethod
def all_aliases(cls) -> Tuple[Tuple[str, str]]:
"""Returns all of the associated aliases for this class,
including those defined by the server config.
"""Returns all of the associated aliases for this entry type,
including those defined by the server config. The first member
of each tuple is the OPTIMADE-compliant field name, the second
is the backend-specific field name.

Returns:
A tuple of alias tuples.
Expand Down Expand Up @@ -88,37 +91,109 @@ def length_alias_for(cls, field: str) -> Optional[str]:
"""
return dict(cls.all_length_aliases()).get(field, None)

@classmethod
def get_backend_field(cls, optimade_field: str) -> str:
"""Return the field name configured for the particular
underlying database for the passed OPTIMADE field name, that would
be used in an API filter.

Aliases are read from
[`all_aliases()`][optimade.server.mappers.entries.BaseResourceMapper.all_aliases].

ml-evs marked this conversation as resolved.
Show resolved Hide resolved
If a dot-separated OPTIMADE field is provided, e.g., `species.mass`, only the first part will be mapped.
This means for an (OPTIMADE, DB) alias of (`species`, `kinds`), `get_backend_fields("species.mass")`
will return `kinds.mass`.

Arguments:
optimade_field: The OPTIMADE field to attempt to map to the backend-specific field.

Examples:
>>> get_backend_field("chemical_formula_anonymous")
'formula_anon'
>>> get_backend_field("formula_anon")
'formula_anon'
>>> get_backend_field("_exmpl_custom_provider_field")
'custom_provider_field'

Returns:
The mapped field name to be used in the query to the backend.

"""
split = optimade_field.split(".")
alias = dict(cls.all_aliases()).get(split[0], None)
if alias is not None:
return alias + ("." + ".".join(split[1:]) if len(split) > 1 else "")
return optimade_field

@classmethod
def alias_for(cls, field: str) -> str:
"""Return aliased field name.

!!! warning "Deprecated"
This method is deprecated could be removed without further warning. Please use
[`get_backend_field()`][optimade.server.mappers.entries.BaseResourceMapper.get_backend_field].

Parameters:
field: OPTIMADE field name.

Returns:
Aliased field as found in [`all_aliases()`][optimade.server.mappers.entries.BaseResourceMapper.all_aliases].

"""
split = field.split(".")
alias = dict(cls.all_aliases()).get(split[0], None)
if alias is not None:
return alias + ("." + ".".join(split[1:]) if len(split) > 1 else "")
return field
warnings.warn(
"The `.alias_for(...)` method is deprecated, please use `.get_backend_field(...)`.",
DeprecationWarning,
)
return cls.get_backend_field(field)

@classmethod
def get_optimade_field(cls, backend_field: str) -> str:
"""Return the corresponding OPTIMADE field name for the underlying database field,
ready to be used to construct the OPTIMADE-compliant JSON response.

Aliases are read from
[`all_aliases()`][optimade.server.mappers.entries.BaseResourceMapper.all_aliases].

Arguments:
backend_field: The backend field to attempt to map to an OPTIMADE field.

Examples:
>>> get_optimade_field("chemical_formula_anonymous")
'chemical_formula_anonymous'
>>> get_optimade_field("formula_anon")
'chemical_formula_anonymous'
>>> get_optimade_field("custom_provider_field")
'_exmpl_custom_provider_field'

Returns:
The mapped field name to be used in an OPTIMADE-compliant response.

"""
return {alias: real for real, alias in cls.all_aliases()}.get(
backend_field, backend_field
)

@classmethod
def alias_of(cls, field: str) -> str:
"""Return de-aliased field name, if it exists,
otherwise return the input field name.

Args:
!!! warning "Deprecated"
This method is deprecated could be removed without further warning. Please use
[`get_optimade_field()`][optimade.server.mappers.entries.BaseResourceMapper.get_optimade_field].

Parameters:
field: Field name to be de-aliased.

Returns:
De-aliased field name, falling back to returning `field`.

"""
field = field.split(".")[0]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this line from the "new" method as it is not tested anywhere and therefore should not be relied on for anything; I think it may just be left over from when alias_for was modified to make this method. If you ask for the OPTIMADE field of e.g. my_weird_field.with_dots, you would want it to return my_weird_field.with_dots rather than alias_for_my_weird_field even if the dot has some significance for that particular backend.

return {alias: real for real, alias in cls.all_aliases()}.get(field, field)
warnings.warn(
"The `.alias_of(...)` method is deprecated, please use `.get_optimade_field(...)`.",
DeprecationWarning,
)
return cls.get_optimade_field(field)

@classmethod
def get_required_fields(cls) -> set:
Expand Down
2 changes: 1 addition & 1 deletion tests/filtertransformers/test_mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ class MyStructureMapper(mapper("BaseResourceMapper")):
mapper = MyStructureMapper()
t = MongoTransformer(mapper=mapper)

assert mapper.alias_for("elements") == "my_elements"
assert mapper.get_backend_field("elements") == "my_elements"

test_filter = {"elements": {"$in": ["A", "B", "C"]}}
assert t.postprocess(test_filter) == {"my_elements": {"$in": ["A", "B", "C"]}}
Expand Down
28 changes: 16 additions & 12 deletions tests/server/test_mappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,33 +29,37 @@ class MyMapper(mapper(MAPPER)):
ALIASES = (("field", "completely_different_field"),)

mapper = MyMapper()
assert mapper.alias_for("_exmpl_dft_parameters") == "dft_parameters"
assert mapper.alias_for("_exmpl_test_field") == "test_field"
assert mapper.alias_for("field") == "completely_different_field"
assert mapper.get_backend_field("_exmpl_dft_parameters") == "dft_parameters"
assert mapper.get_backend_field("_exmpl_test_field") == "test_field"
assert mapper.get_backend_field("field") == "completely_different_field"
assert mapper.length_alias_for("_exmpl_test_field") == "test_field_len"
assert mapper.length_alias_for("test_field") is None
assert mapper.alias_for("test_field") == "test_field"
assert mapper.get_backend_field("test_field") == "test_field"
with pytest.warns(DeprecationWarning):
assert mapper.alias_for("test_field") == "test_field"

assert mapper.alias_of("dft_parameters") == "_exmpl_dft_parameters"
assert mapper.alias_of("test_field") == "_exmpl_test_field"
assert mapper.alias_of("completely_different_field") == "field"
assert mapper.alias_of("nonexistent_field") == "nonexistent_field"
assert mapper.get_optimade_field("dft_parameters") == "_exmpl_dft_parameters"
assert mapper.get_optimade_field("test_field") == "_exmpl_test_field"
assert mapper.get_optimade_field("completely_different_field") == "field"
assert mapper.get_optimade_field("nonexistent_field") == "nonexistent_field"
with pytest.warns(DeprecationWarning):
assert mapper.alias_of("nonexistent_field") == "nonexistent_field"

# nested properties
assert (
mapper.alias_for("_exmpl_dft_parameters.nested.property")
mapper.get_backend_field("_exmpl_dft_parameters.nested.property")
== "dft_parameters.nested.property"
)
assert (
mapper.alias_for("_exmpl_dft_parameters.nested_property")
mapper.get_backend_field("_exmpl_dft_parameters.nested_property")
== "dft_parameters.nested_property"
)

# test nonsensical query
assert mapper.alias_for("_exmpl_test_field.") == "test_field."
assert mapper.get_backend_field("_exmpl_test_field.") == "test_field."

# test an awkward case that has no alias
assert (
mapper.alias_for("_exmpl_dft_parameters_dft_parameters.nested.property")
mapper.get_backend_field("_exmpl_dft_parameters_dft_parameters.nested.property")
== "_exmpl_dft_parameters_dft_parameters.nested.property"
)