Skip to content

Commit

Permalink
#184: Make certain function less complex
Browse files Browse the repository at this point in the history
- I've had to increase the max complexity in .flake8 because there's not much you can do with a couple of the functions. Still made some improvements though!
  • Loading branch information
MRichards99 committed Nov 5, 2020
1 parent af816bf commit a694129
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[flake8]
select = A,B,B9,BLK,C,E,F,I,N,S,W
ignore = E203,W503,E501
max-complexity = 10
max-complexity = 14
max-line-length = 80
application-import-names = datagateway_api,test,util
import-order-style = google
Expand Down
33 changes: 24 additions & 9 deletions datagateway_api/common/icat/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ def __init__(self, field, value, operation):
self.field = field

def apply_filter(self, query):
try:
log.info("Adding ICAT where filter (for %s) to query", self.value)
query.addConditions(self.create_filter())
except ValueError:
raise FilterError(
"Something went wrong when adding WHERE filter to ICAT query",
)

def create_filter(self):
"""
Create what's needed for a where filter dependent on the operation provided
The logic in this function has been abstracted away from `apply_filter()` to
make that function used for its named purpose, and no more.
:return: A where filter (of type :class:`dict`) ready to be applied to a Query
object
:raises FilterError: If the operation provided to the instance isn't valid
"""

log.info("Creating condition for ICAT where filter")
if self.operation == "eq":
where_filter = self.create_condition(self.field, "=", self.value)
Expand All @@ -46,13 +66,8 @@ def apply_filter(self, query):
raise FilterError(f"Bad operation given to where filter: {self.operation}")

log.debug("ICAT Where Filter: %s", where_filter)
try:
log.info("Adding ICAT where filter (for %s) to query", self.value)
query.addConditions(where_filter)
except ValueError:
raise FilterError(
"Something went wrong when adding WHERE filter to ICAT query",
)

return where_filter

@staticmethod
def create_condition(attribute_name, operator, value):
Expand Down Expand Up @@ -213,8 +228,8 @@ def _extract_filter_fields(self, field):
for inner_key, inner_value in value.items():
if not isinstance(inner_key, str):
raise FilterError(
"Include Filter: Dictionary key should only be a string"
", not any other type",
"Include Filter: Dictionary key should only be a"
" string, not any other type",
)

# Will end up as: key.inner_key.inner_value
Expand Down
7 changes: 4 additions & 3 deletions datagateway_api/common/icat/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,10 @@ def create_entities(client, entity_type, data):
try:
entity_info = new_entity.getAttrInfo(client, attribute_name)
if entity_info.relType.lower() == "attribute":
if isinstance(value, str):
if DateHandler.is_str_a_date(value):
value = DateHandler.str_to_datetime_object(value)
# Short circuiting ensures is_str_date() will only be executed if
# value is a string
if isinstance(value, str) and DateHandler.is_str_a_date(value):
value = DateHandler.str_to_datetime_object(value)

setattr(new_entity, attribute_name, value)
else:
Expand Down
24 changes: 9 additions & 15 deletions datagateway_api/common/icat/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ def entity_to_dict(self, entity, includes, distinct_fields=None):
include_set = (entity.InstRel | entity.InstMRel) & set(includes)
for key in entity.InstAttr | entity.MetaAttr | include_set:
if key in includes:
# Make a copy of distinct_fields when calling this function again later
if distinct_fields is not None:
distinct_fields_copy = self.prepare_distinct_fields(
key, distinct_fields,
)
else:
distinct_fields_copy = None

target = getattr(entity, key)
# Copy and remove don't return values so must be done separately
includes_copy = includes.copy()
Expand All @@ -187,31 +195,17 @@ def entity_to_dict(self, entity, includes, distinct_fields=None):
" cause an issue further on in the request",
)
if isinstance(target, Entity):
if distinct_fields is not None:
distinct_fields_copy = self.prepare_distinct_fields(
key, distinct_fields,
)
else:
distinct_fields_copy = None

d[key] = self.entity_to_dict(
target, includes_copy, distinct_fields_copy,
)

# Related fields with one-many relationships are stored as EntityLists
elif isinstance(target, EntityList):
d[key] = []
for e in target:
if distinct_fields is not None:
distinct_fields_copy = self.prepare_distinct_fields(
key, distinct_fields,
)
else:
distinct_fields_copy = None

d[key].append(
self.entity_to_dict(e, includes_copy, distinct_fields_copy),
)

# Add actual piece of data to the dictionary
else:
entity_data = None
Expand Down

0 comments on commit a694129

Please sign in to comment.