From c724f721a0e5905a38a48cb8b8a66c52f137c5f3 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 3 Sep 2020 10:32:29 +0000 Subject: [PATCH 01/15] #143: Add basic implementation of include filter - This filter currently accepts a single entity name, or a list of entity names. Dictionaries are not yet supported --- common/icat/filters.py | 20 ++++++++++++++++++-- common/icat/helpers.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/common/icat/filters.py b/common/icat/filters.py index 3dce4d53..5d59381d 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -36,7 +36,7 @@ def apply_filter(self, query): where_filter = self.create_condition(self.field, ">=", self.value) elif self.operation == "in": # Convert self.value into a string with brackets equivalent to tuple format. - # Cannot convert straight to tuple as single element tuples contain a + # Cannot convert straight to tuple as single element tuples contain a # trailing comma which Python ICAT/JPQL doesn't accept self.value = str(self.value).replace("[", "(").replace("]", ")") where_filter = self.create_condition(self.field, "in", self.value) @@ -160,6 +160,22 @@ def icat_set_limit(query, skip_number, limit_number): class PythonICATIncludeFilter(IncludeFilter): def __init__(self, included_filters): super().__init__(included_filters) + # TODO - Adapt JSON input from request to Python ICAT + # Might end up removing the super constructor call + + # Included entities must be in a list + if isinstance(self.included_filters, str): + self.included_filters = [self.included_filters] + + # TODO - When a dictionary is used in self.included_filters def apply_filter(self, query): - pass + log.debug( + f"Included filters: {self.included_filters}, Type: {type(self.included_filters)}" + ) + + try: + pass + query.addIncludes(self.included_filters) + except ValueError as e: + raise FilterError(e) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index adaa0a05..f762223d 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -151,6 +151,7 @@ def execute_query(self, client, return_json_formattable=False): try: query_result = client.search(self.query) + log.debug("Query Result: %s", query_result) except ICATValidationError as e: raise PythonICATError(e) @@ -182,7 +183,41 @@ def execute_query(self, client, return_json_formattable=False): dict_result = result.as_dict() distinct_result = {} + log.debug(f"Result: {result}") + log.debug(f"Dict Result: {dict_result}") + + log.debug( + f"Includes: {self.query.includes}," + f" Type: {type(self.query.includes)}" + ) + + for entity_name in self.query.includes: + included_data = getattr(result, entity_name) + dict_result[entity_name] = [] + + for included_result in included_data: + # TODO - Test that there can be >1 element in this + dict_result[entity_name].append(included_result.as_dict()) + for key in dict_result: + log.debug(f"Key: {key}, Type: {type(key)}") + + if isinstance(dict_result[key], list): + for included_result in range(len(dict_result[key])): + for inner_key in dict_result[key][included_result]: + log.debug(f"Inner Key: {inner_key}") + # TODO - Remove duplication + if isinstance( + dict_result[key][included_result][inner_key], + datetime, + ): + # Remove timezone data which isn't utilised in ICAT + dict_result[key][included_result][inner_key] = ( + dict_result[key][included_result][inner_key] + .replace(tzinfo=None) + .strftime(Constants.ACCEPTED_DATE_FORMAT) + ) + # Convert datetime objects to strings so they can be JSON # serialisable if isinstance(dict_result[key], datetime): @@ -224,6 +259,9 @@ def check_attribute_name_for_distinct(self, key, value): if value == Constants.PYTHON_ICAT_DISTNCT_CONDITION: self.attribute_names.append(key) + def make_date_json_serialisable(self, data_dict, more_params_needed): + pass + def get_python_icat_entity_name(client, database_table_name): """ From 9b8e08e528fbc145f254a05ad60b4455f338cd33 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 3 Sep 2020 12:29:13 +0000 Subject: [PATCH 02/15] #143: Allow dictionary of include filter to be converted to Python ICAT format - This change doesn't make dictionary include filter work, but they're now converted to a notation that Python ICAT accepts --- common/icat/filters.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/icat/filters.py b/common/icat/filters.py index 5d59381d..148090e6 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -162,10 +162,17 @@ def __init__(self, included_filters): super().__init__(included_filters) # TODO - Adapt JSON input from request to Python ICAT # Might end up removing the super constructor call + #self.included_filters = included_filters["include"] # Included entities must be in a list if isinstance(self.included_filters, str): self.included_filters = [self.included_filters] + elif isinstance(self.included_filters, dict): + self.included_filters = [] + for item in included_filters["include"].items(): + self.included_filters.append(".".join(item)) + + # TODO - When a dictionary is used in self.included_filters From da3ac2c49c74584f3b54185a3524a948f60424b6 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 3 Sep 2020 15:21:29 +0000 Subject: [PATCH 03/15] #143: Start to improve how include filter input is adapted for Python ICAT - This deals with strings and lists very well, work is needed on handling dictionaries --- common/icat/filters.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/common/icat/filters.py b/common/icat/filters.py index 148090e6..dbc4856a 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -159,26 +159,35 @@ def icat_set_limit(query, skip_number, limit_number): class PythonICATIncludeFilter(IncludeFilter): def __init__(self, included_filters): - super().__init__(included_filters) # TODO - Adapt JSON input from request to Python ICAT - # Might end up removing the super constructor call - #self.included_filters = included_filters["include"] - - # Included entities must be in a list - if isinstance(self.included_filters, str): - self.included_filters = [self.included_filters] - elif isinstance(self.included_filters, dict): - self.included_filters = [] - for item in included_filters["include"].items(): - self.included_filters.append(".".join(item)) - - + self.included_filters = [] + self._extract_filter_fields(included_filters["include"]) - # TODO - When a dictionary is used in self.included_filters + def _extract_filter_fields(self, field): + """ + TODO - Add docstring + """ + if isinstance(field, str): + self.included_filters.append(field) + elif isinstance(field, dict): + for item in field.items(): + # If key and value are both strings, join them together + # Else, + log.debug(f"Item: {item}, Type: {type(item)}") + self.included_filters.append(".".join(item)) + elif isinstance(field, list): + for element in field: + self._extract_filter_fields(element) + elif isinstance(field, tuple): + # Assuming that if `field` is a tuple, it must have come from a dictionary + pass + # Need to inspect each element + # Deal with just a list, and a list of str from dict values def apply_filter(self, query): log.debug( - f"Included filters: {self.included_filters}, Type: {type(self.included_filters)}" + f"Included filters: {self.included_filters}," + f" Type: {type(self.included_filters)}" ) try: From 9d5203705b40e92741a88d3d816eeec2ca5463ca Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 3 Sep 2020 16:40:47 +0000 Subject: [PATCH 04/15] #143: Checkpoint for dealing with include filter input - Input for PythonICATIncludeFilter --- common/icat/filters.py | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/common/icat/filters.py b/common/icat/filters.py index dbc4856a..9629ad7c 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -170,19 +170,33 @@ def _extract_filter_fields(self, field): if isinstance(field, str): self.included_filters.append(field) elif isinstance(field, dict): - for item in field.items(): - # If key and value are both strings, join them together - # Else, - log.debug(f"Item: {item}, Type: {type(item)}") - self.included_filters.append(".".join(item)) + for key, value in field.items(): + if not isinstance(key, str): + raise FilterError( + "Include Filter: Dictionary key should only be a string, not" + " any other type" + ) + + if isinstance(value, str): + self.included_filters.append(".".join((key, value))) + elif isinstance(value, list): + # key.value1, key.value2, key.value3 etc. + pass + elif isinstance(value, dict): + # key.value_key.value_value + pass + else: + raise FilterError( + "Include Filter: Inner field type (inside dictionary) not" + " recognised, cannot interpret input" + ) elif isinstance(field, list): for element in field: self._extract_filter_fields(element) - elif isinstance(field, tuple): - # Assuming that if `field` is a tuple, it must have come from a dictionary - pass - # Need to inspect each element - # Deal with just a list, and a list of str from dict values + else: + raise FilterError( + "Include Filter: Field type not recognised, cannot interpret input" + ) def apply_filter(self, query): log.debug( From 32572044a67ebea17913c49e16b08a07a4b29186 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 3 Sep 2020 16:57:44 +0000 Subject: [PATCH 05/15] #143: Solve warnings by VS Code - Warning were caused by missing import statements --- common/database/filters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/database/filters.py b/common/database/filters.py index 6822355e..afd88e38 100644 --- a/common/database/filters.py +++ b/common/database/filters.py @@ -6,7 +6,8 @@ LimitFilter, IncludeFilter, ) -from common.exceptions import FilterError +from common.exceptions import FilterError, MultipleIncludeError +from common.models import db_models from sqlalchemy import asc, desc From 3d0f59cfad994e62f5dfb26c51d19e9f02ff43c5 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 7 Sep 2020 09:57:18 +0000 Subject: [PATCH 06/15] #143: Convert lists and dicts into a form compatible with Python ICAT --- common/icat/filters.py | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/common/icat/filters.py b/common/icat/filters.py index 9629ad7c..f89e73e1 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -159,13 +159,23 @@ def icat_set_limit(query, skip_number, limit_number): class PythonICATIncludeFilter(IncludeFilter): def __init__(self, included_filters): - # TODO - Adapt JSON input from request to Python ICAT self.included_filters = [] self._extract_filter_fields(included_filters["include"]) def _extract_filter_fields(self, field): """ - TODO - Add docstring + Using recursion, go through the fields and add them to the filter's instance. + This means that lists within dictionaries, dictionaries within dictionaries are + supported. Where dictionaries are involved, '.' are used to join the fields + together + + Some (but not all) fields require the plural to be accepted in the include of a + Python ICAT query - e.g. 'userGroups' is valid (plural required), but 'dataset' + is also valid (plural not required). The dictionary `substnames` in Python + ICAT's query.py gives a good overview of which need to be plural. + + :param field: Which field(s) should be included in the ICAT query + :type field: :class:`str` or :class:`list` or :class:`dict` """ if isinstance(field, str): self.included_filters.append(field) @@ -178,13 +188,23 @@ def _extract_filter_fields(self, field): ) if isinstance(value, str): - self.included_filters.append(".".join((key, value))) + self._extract_filter_fields(".".join((key, value))) elif isinstance(value, list): - # key.value1, key.value2, key.value3 etc. - pass + for element in value: + # Will end up as: key.element1, key.element2, key.element3 etc. + self._extract_filter_fields(".".join((key, element))) elif isinstance(value, dict): - # key.value_key.value_value - pass + 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" + ) + + # Will end up as: key.inner_key.inner_value + self._extract_filter_fields( + {".".join((key, inner_key)): inner_value} + ) else: raise FilterError( "Include Filter: Inner field type (inside dictionary) not" @@ -192,7 +212,7 @@ def _extract_filter_fields(self, field): ) elif isinstance(field, list): for element in field: - self._extract_filter_fields(element) + self._extract_filter_fields(element) else: raise FilterError( "Include Filter: Field type not recognised, cannot interpret input" From c32dd5e4fdbc094aa5e8e5116ca2b22a920aa9cc Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 7 Sep 2020 12:17:10 +0000 Subject: [PATCH 07/15] #143: Deal with included fields which are joined by dots - Also add some comments in `execute_query()` --- common/icat/helpers.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index f762223d..744e50d0 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -150,6 +150,7 @@ def execute_query(self, client, return_json_formattable=False): """ try: + # Going to Python ICAT to perform the query query_result = client.search(self.query) log.debug("Query Result: %s", query_result) except ICATValidationError as e: @@ -179,29 +180,39 @@ def execute_query(self, client, return_json_formattable=False): if return_json_formattable: data = [] + + # Split up self.query.includes into individual fields (some are separated by + # dots for Python ICAT) + # TODO - Test this all works without include filter + included_entities = [] + for include in self.query.includes: + split_include = include.split(".") + included_entities.extend(split_include) + for result in query_result: + # Converting each row/result into its dictionary form dict_result = result.as_dict() + # Creating dictionary to store distinct fields for use later on distinct_result = {} log.debug(f"Result: {result}") log.debug(f"Dict Result: {dict_result}") - log.debug( - f"Includes: {self.query.includes}," - f" Type: {type(self.query.includes)}" - ) - - for entity_name in self.query.includes: + # Adding data from the included data to `dict_result` which stores the + # query result in dictionary form + for entity_name in included_entities: + log.debug(f"Entity Name: {entity_name}, Type: {type(entity_name)}") included_data = getattr(result, entity_name) dict_result[entity_name] = [] + log.debug(f"Included Data: {included_data}, Type: {type(included_data)}") for included_result in included_data: # TODO - Test that there can be >1 element in this dict_result[entity_name].append(included_result.as_dict()) + # Data is prepared to be used as JSON - e.g. dates are converted to a + # specific format for key in dict_result: - log.debug(f"Key: {key}, Type: {type(key)}") - if isinstance(dict_result[key], list): for included_result in range(len(dict_result[key])): for inner_key in dict_result[key][included_result]: @@ -242,6 +253,7 @@ def execute_query(self, client, return_json_formattable=False): data.append(dict_result) return data else: + # Return data as Python ICAT returned the query return query_result def check_attribute_name_for_distinct(self, key, value): From b89baea6680cf03c88e9a9a31cafedcd1db85b13 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 8 Sep 2020 13:12:13 +0000 Subject: [PATCH 08/15] #143: Checkpoint for adding included data to response - Data is now added when dictionaries are used within an include filter, however it's not nested correctly --- common/icat/helpers.py | 78 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index 744e50d0..8cd9d23a 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -1,7 +1,9 @@ from functools import wraps import logging from datetime import datetime, timedelta +from collections.abc import Iterable +from icat.entity import EntityList from icat.query import Query from icat.exception import ICATSessionError, ICATValidationError from common.exceptions import ( @@ -184,10 +186,6 @@ def execute_query(self, client, return_json_formattable=False): # Split up self.query.includes into individual fields (some are separated by # dots for Python ICAT) # TODO - Test this all works without include filter - included_entities = [] - for include in self.query.includes: - split_include = include.split(".") - included_entities.extend(split_include) for result in query_result: # Converting each row/result into its dictionary form @@ -195,20 +193,22 @@ def execute_query(self, client, return_json_formattable=False): # Creating dictionary to store distinct fields for use later on distinct_result = {} - log.debug(f"Result: {result}") + log.debug(f"Result: {result}, Type: {type(result)}, Dir: {dir(result)}") log.debug(f"Dict Result: {dict_result}") # Adding data from the included data to `dict_result` which stores the # query result in dictionary form - for entity_name in included_entities: - log.debug(f"Entity Name: {entity_name}, Type: {type(entity_name)}") - included_data = getattr(result, entity_name) - dict_result[entity_name] = [] - log.debug(f"Included Data: {included_data}, Type: {type(included_data)}") + for entity_name in self.query.includes: - for included_result in included_data: - # TODO - Test that there can be >1 element in this - dict_result[entity_name].append(included_result.as_dict()) + # TODO - Rename that variable + # split_entities_element = split_entities.pop(0) + + # TODO - Remember this style of name is used elsewhere + # TODO - Test how the split works when there's no dot + split_entities = entity_name.split(".") + log.debug(f"Split entities: {split_entities}") + + self.add_included_data(result, dict_result, split_entities) # Data is prepared to be used as JSON - e.g. dates are converted to a # specific format @@ -253,7 +253,7 @@ def execute_query(self, client, return_json_formattable=False): data.append(dict_result) return data else: - # Return data as Python ICAT returned the query + # Return data exactly as Python ICAT returned the query return query_result def check_attribute_name_for_distinct(self, key, value): @@ -272,8 +272,58 @@ def check_attribute_name_for_distinct(self, key, value): self.attribute_names.append(key) def make_date_json_serialisable(self, data_dict, more_params_needed): + """ + TODO - Add docstring and use this + """ pass + def add_included_data(self, icat_result, dict_result, split_entities): + """ + TODO - Add docstring + """ + # split_entities = entity_name.split(".") + split_entity = split_entities.pop(0) + log.debug(f"Split entity: {split_entity}") + + if isinstance(icat_result, EntityList): + # Iterate/unpack the list, would there ever be more than one item in + # EntityList + log.debug(f"Entity List: {icat_result}, Type: {type(icat_result)}") + log.debug(f"DIR: {dir(icat_result)}") + log.debug(f"Length: {len(icat_result)}") + + for entity in icat_result: + # TODO - Be aware this will overwrite + icat_data = self.second_include_funct(entity, dict_result, split_entity) + else: + log.debug(f"NO ENTITYLIST") + icat_data = self.second_include_funct( + icat_result, dict_result, split_entity + ) + + if len(split_entities) > 0: + # Recursion time + log.debug("RECURSION") + self.add_included_data(icat_data, dict_result, split_entities) + + def second_include_funct(self, icat_entity, dict_result, entity_name): + included_data = getattr(icat_entity, entity_name) + log.debug( + f"Included Data: {included_data}, Type: {type(included_data)}, Dir: {dir(included_data)}" + ) + dict_result[entity_name] = [] + + if isinstance(included_data, Iterable): + for included_result in included_data: + # TODO - Test that there can be >1 element in this + log.debug(f"Included Result: {included_result.as_dict()}") + dict_result[entity_name].append(included_result.as_dict()) + else: + log.debug(f"Included DATA: {included_data.as_dict()}") + dict_result[entity_name].append(included_data.as_dict()) + + return included_data + def get_python_icat_entity_name(client, database_table_name): """ From 0064ac8670b600a1e06950c60a4d5ecf4c3a305a Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Wed, 9 Sep 2020 14:07:02 +0000 Subject: [PATCH 09/15] #143: Checkpoint for making data nested - Written a specific solution for the data I've been looking at. - This now uncovered another issue, with the way I search dict_result to convert datetime objects to strings --- common/icat/helpers.py | 84 ++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 49 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index 8cd9d23a..953dc933 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -193,8 +193,8 @@ def execute_query(self, client, return_json_formattable=False): # Creating dictionary to store distinct fields for use later on distinct_result = {} - log.debug(f"Result: {result}, Type: {type(result)}, Dir: {dir(result)}") - log.debug(f"Dict Result: {dict_result}") + #log.debug(f"Result: {result}, Type: {type(result)}, Dir: {dir(result)}") + #log.debug(f"Dict Result: {dict_result}") # Adding data from the included data to `dict_result` which stores the # query result in dictionary form @@ -204,19 +204,24 @@ def execute_query(self, client, return_json_formattable=False): # split_entities_element = split_entities.pop(0) # TODO - Remember this style of name is used elsewhere - # TODO - Test how the split works when there's no dot split_entities = entity_name.split(".") log.debug(f"Split entities: {split_entities}") - self.add_included_data(result, dict_result, split_entities) + #self.add_included_data(result, dict_result, split_entities) + #included_entity_data = self.add_included_data(result, dict_result, split_entities) + #dict_result[entity_name].append(included_entity_data) + + self.get_included_data(result, dict_result, split_entities) + log.debug(f"dict result: {dict_result}") # Data is prepared to be used as JSON - e.g. dates are converted to a # specific format - for key in dict_result: + for key, value in dict_result.items(): + #make_date_json_serialisable() if isinstance(dict_result[key], list): for included_result in range(len(dict_result[key])): for inner_key in dict_result[key][included_result]: - log.debug(f"Inner Key: {inner_key}") + #log.debug(f"Inner Key: {inner_key}") # TODO - Remove duplication if isinstance( dict_result[key][included_result][inner_key], @@ -271,58 +276,39 @@ def check_attribute_name_for_distinct(self, key, value): if value == Constants.PYTHON_ICAT_DISTNCT_CONDITION: self.attribute_names.append(key) - def make_date_json_serialisable(self, data_dict, more_params_needed): + def make_date_json_serialisable(self, key, value): """ TODO - Add docstring and use this """ - pass + if isinstance(value, list): + for inner_value in value: + self.make_date_json_serialisable(key, inner_value) - def add_included_data(self, icat_result, dict_result, split_entities): - """ - TODO - Add docstring - """ - # split_entities = entity_name.split(".") + + + + def get_included_data(self, icat_data, dictionary_data, split_entities): split_entity = split_entities.pop(0) - log.debug(f"Split entity: {split_entity}") - - if isinstance(icat_result, EntityList): - # Iterate/unpack the list, would there ever be more than one item in - # EntityList - log.debug(f"Entity List: {icat_result}, Type: {type(icat_result)}") - log.debug(f"DIR: {dir(icat_result)}") - log.debug(f"Length: {len(icat_result)}") - - for entity in icat_result: - # TODO - Be aware this will overwrite - icat_data = self.second_include_funct(entity, dict_result, split_entity) - else: - log.debug(f"NO ENTITYLIST") - icat_data = self.second_include_funct( - icat_result, dict_result, split_entity - ) + extracted_data = getattr(icat_data, split_entity) + log.debug(f"Extracted data: {extracted_data}, Type: {type(extracted_data)}") + + + dictionary_data[split_entity] = [] + extracted_data_i = None + for i in extracted_data: + extracted_data_i = i + dictionary_data[split_entity].append(i.as_dict()) if len(split_entities) > 0: - # Recursion time - log.debug("RECURSION") - self.add_included_data(icat_data, dict_result, split_entities) - - def second_include_funct(self, icat_entity, dict_result, entity_name): - included_data = getattr(icat_entity, entity_name) - log.debug( - f"Included Data: {included_data}, Type: {type(included_data)}, Dir: {dir(included_data)}" - ) - dict_result[entity_name] = [] + split_entity_1 = split_entities.pop(0) + extracted_data_1 = getattr(extracted_data_i, split_entity_1) + log.debug(f"Extracted data1 : {extracted_data_1}, Type: {type(extracted_data_1)}") + dictionary_data[split_entity][0][split_entity_1] = [] + dictionary_data[split_entity][0][split_entity_1].append(extracted_data_1.as_dict()) + + - if isinstance(included_data, Iterable): - for included_result in included_data: - # TODO - Test that there can be >1 element in this - log.debug(f"Included Result: {included_result.as_dict()}") - dict_result[entity_name].append(included_result.as_dict()) - else: - log.debug(f"Included DATA: {included_data.as_dict()}") - dict_result[entity_name].append(included_data.as_dict()) - return included_data def get_python_icat_entity_name(client, database_table_name): From bba6a23d5bb01fa091bdf72e8b2d54d560785b96 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 10 Sep 2020 14:57:56 +0000 Subject: [PATCH 10/15] #143: Allow included fields to be added to response to n levels deep - A nice recursive function allows dictionary-based include filters to be correctly added to the request's response - Requests are still currently broken because of date conversion still not happening correctly, but the log message which shows `data` before being returned shows the data structure is correct, near identical to the way its assembled in the database backend --- common/icat/helpers.py | 89 +++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 48 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index 953dc933..47c468ed 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -3,7 +3,7 @@ from datetime import datetime, timedelta from collections.abc import Iterable -from icat.entity import EntityList +from icat.entity import Entity, EntityList from icat.query import Query from icat.exception import ICATSessionError, ICATValidationError from common.exceptions import ( @@ -183,36 +183,23 @@ def execute_query(self, client, return_json_formattable=False): if return_json_formattable: data = [] - # Split up self.query.includes into individual fields (some are separated by - # dots for Python ICAT) # TODO - Test this all works without include filter - for result in query_result: - # Converting each row/result into its dictionary form - dict_result = result.as_dict() + # TODO - Put this in the docstring + # Converting each row/result into its dictionary form if include filter + # not used - `______` will do this if include filter(s) are used in the + # request - Python ICAT's `as_dict()` doesn't do included fields but is + # a simpler function so only used when needed + dict_result = {} if self.query.includes else result.as_dict() # Creating dictionary to store distinct fields for use later on distinct_result = {} - #log.debug(f"Result: {result}, Type: {type(result)}, Dir: {dir(result)}") - #log.debug(f"Dict Result: {dict_result}") + log.debug(f"Dict Result: {dict_result}, Type: {type(dict_result)}") # Adding data from the included data to `dict_result` which stores the # query result in dictionary form - for entity_name in self.query.includes: - - # TODO - Rename that variable - # split_entities_element = split_entities.pop(0) - - # TODO - Remember this style of name is used elsewhere - split_entities = entity_name.split(".") - log.debug(f"Split entities: {split_entities}") - - #self.add_included_data(result, dict_result, split_entities) - #included_entity_data = self.add_included_data(result, dict_result, split_entities) - #dict_result[entity_name].append(included_entity_data) - - self.get_included_data(result, dict_result, split_entities) - log.debug(f"dict result: {dict_result}") + if self.query.includes: + dict_result = self.entity_to_dict(result, self.query.includes) # Data is prepared to be used as JSON - e.g. dates are converted to a # specific format @@ -256,6 +243,7 @@ def execute_query(self, client, return_json_formattable=False): data.append(distinct_result) else: data.append(dict_result) + log.debug(f"finished data: {data}") return data else: # Return data exactly as Python ICAT returned the query @@ -284,31 +272,36 @@ def make_date_json_serialisable(self, key, value): for inner_value in value: self.make_date_json_serialisable(key, inner_value) - - - - def get_included_data(self, icat_data, dictionary_data, split_entities): - split_entity = split_entities.pop(0) - extracted_data = getattr(icat_data, split_entity) - log.debug(f"Extracted data: {extracted_data}, Type: {type(extracted_data)}") - - - dictionary_data[split_entity] = [] - extracted_data_i = None - for i in extracted_data: - extracted_data_i = i - dictionary_data[split_entity].append(i.as_dict()) - - if len(split_entities) > 0: - split_entity_1 = split_entities.pop(0) - extracted_data_1 = getattr(extracted_data_i, split_entity_1) - log.debug(f"Extracted data1 : {extracted_data_1}, Type: {type(extracted_data_1)}") - dictionary_data[split_entity][0][split_entity_1] = [] - dictionary_data[split_entity][0][split_entity_1].append(extracted_data_1.as_dict()) - - - - + def entity_to_dict(self, entity, includes): + """ + TODO - Add docstring + Return a dict with the object's attributes. + """ + # Split up the fields separated by dots and flatten the resulting lists + flat_includes = [m for n in (x.split('.') for x in includes) for m in n] + + d = {} + print(f"Includes: {includes}") + print(f"Includes: {flat_includes}") + # Expand on Python ICAT's implementation of `Entity.as_dict()` to use set operators + include_set = (entity.InstRel | entity.InstMRel) & set(flat_includes) + print(f"Include set: {include_set}") + for a in entity.InstAttr | entity.MetaAttr | include_set: + if a in flat_includes: + target = getattr(entity, a) + includes_copy = flat_includes.copy() + includes_copy.remove(a) + if isinstance(target, Entity): + a_dict = self.entity_to_dict(target, includes_copy) + d[a] = a_dict + elif isinstance(target, EntityList): + d[a] = [] + for e in target: + a_dict = self.entity_to_dict(e, includes_copy) + d[a].append(a_dict) + else: + d[a] = getattr(entity, a) + return d def get_python_icat_entity_name(client, database_table_name): From 3381e82c6acd6328498867e804b0b1dc936c8b02 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 10 Sep 2020 15:36:57 +0000 Subject: [PATCH 11/15] #143: Allow dates to be converted to strings in nested structures - Nice reduction in lines :) --- common/icat/helpers.py | 52 +++++++++--------------------------------- 1 file changed, 11 insertions(+), 41 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index 47c468ed..fe990bf2 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -194,43 +194,10 @@ def execute_query(self, client, return_json_formattable=False): # Creating dictionary to store distinct fields for use later on distinct_result = {} - log.debug(f"Dict Result: {dict_result}, Type: {type(dict_result)}") - - # Adding data from the included data to `dict_result` which stores the - # query result in dictionary form if self.query.includes: dict_result = self.entity_to_dict(result, self.query.includes) - # Data is prepared to be used as JSON - e.g. dates are converted to a - # specific format for key, value in dict_result.items(): - #make_date_json_serialisable() - if isinstance(dict_result[key], list): - for included_result in range(len(dict_result[key])): - for inner_key in dict_result[key][included_result]: - #log.debug(f"Inner Key: {inner_key}") - # TODO - Remove duplication - if isinstance( - dict_result[key][included_result][inner_key], - datetime, - ): - # Remove timezone data which isn't utilised in ICAT - dict_result[key][included_result][inner_key] = ( - dict_result[key][included_result][inner_key] - .replace(tzinfo=None) - .strftime(Constants.ACCEPTED_DATE_FORMAT) - ) - - # Convert datetime objects to strings so they can be JSON - # serialisable - if isinstance(dict_result[key], datetime): - # Remove timezone data which isn't utilised in ICAT - dict_result[key] = ( - dict_result[key] - .replace(tzinfo=None) - .strftime(Constants.ACCEPTED_DATE_FORMAT) - ) - if distinct_filter_flag: # Add only the required data as per request's distinct filter # fields @@ -243,7 +210,6 @@ def execute_query(self, client, return_json_formattable=False): data.append(distinct_result) else: data.append(dict_result) - log.debug(f"finished data: {data}") return data else: # Return data exactly as Python ICAT returned the query @@ -277,15 +243,12 @@ def entity_to_dict(self, entity, includes): TODO - Add docstring Return a dict with the object's attributes. """ - # Split up the fields separated by dots and flatten the resulting lists - flat_includes = [m for n in (x.split('.') for x in includes) for m in n] - d = {} - print(f"Includes: {includes}") - print(f"Includes: {flat_includes}") + + # Split up the fields separated by dots and flatten the resulting lists + flat_includes = [m for n in (x.split(".") for x in includes) for m in n] # Expand on Python ICAT's implementation of `Entity.as_dict()` to use set operators include_set = (entity.InstRel | entity.InstMRel) & set(flat_includes) - print(f"Include set: {include_set}") for a in entity.InstAttr | entity.MetaAttr | include_set: if a in flat_includes: target = getattr(entity, a) @@ -300,7 +263,14 @@ def entity_to_dict(self, entity, includes): a_dict = self.entity_to_dict(e, includes_copy) d[a].append(a_dict) else: - d[a] = getattr(entity, a) + entity_data = getattr(entity, a) + # Convert datetime objects to strings ready to be outputted as JSON + if isinstance(entity_data, datetime): + # Remove timezone data which isn't utilised in ICAT + entity_data = entity_data.replace(tzinfo=None).strftime( + Constants.ACCEPTED_DATE_FORMAT + ) + d[a] = entity_data return d From 2210fdbaf1a80e4bc5c17db6d5f3268cd3272589 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 10 Sep 2020 18:46:52 +0000 Subject: [PATCH 12/15] #143: Make entity_to_dict() more readable --- common/icat/helpers.py | 75 ++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index fe990bf2..b6174934 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -230,47 +230,73 @@ def check_attribute_name_for_distinct(self, key, value): if value == Constants.PYTHON_ICAT_DISTNCT_CONDITION: self.attribute_names.append(key) - def make_date_json_serialisable(self, key, value): + def datetime_object_to_str(self, date_obj): """ - TODO - Add docstring and use this + Convert a datetime object to a string so it can be outputted in JSON + + There's currently no reason to make this function static, but it could be useful + in the future if a use case required this functionality. + + :param date_obj: Datetime object from data from an ICAT entity + :type date_obj: :class:`datetime.datetime` + :return: Datetime (of type string) in the agreed format """ - if isinstance(value, list): - for inner_value in value: - self.make_date_json_serialisable(key, inner_value) + return date_obj.replace(tzinfo=None).strftime(Constants.ACCEPTED_DATE_FORMAT) def entity_to_dict(self, entity, includes): """ - TODO - Add docstring - Return a dict with the object's attributes. + This expands on Python ICAT's implementation of `icat.entity.Entity.as_dict()` + to use set operators to create a version of the entity as a dictionary + + Most of this function is dedicated to recursing over included fields from a + query, since this is functionality isn't part of Python ICAT's `as_dict()`. This + function can be used when there are no include filters in the query/request + however. + + :param entity: Python ICAT entity from an ICAT query + :type entity: :class:`icat.entities.ENTITY` (implementation of + :class:`icat.entity.Entity`) or :class:`icat.entity.EntityList` + :param includes: Set of fields that have been included in the ICAT query. Where + fields have a chain of relationships, they're a single element string + separated by dots + :type includes: :class:`set` + :return: ICAT Data (of type dictionary) ready to be serialised to JSON """ + log.info("Converting entity (%s) to dictionary format", type(entity)) d = {} # Split up the fields separated by dots and flatten the resulting lists - flat_includes = [m for n in (x.split(".") for x in includes) for m in n] - # Expand on Python ICAT's implementation of `Entity.as_dict()` to use set operators + flat_includes = [m for n in (field.split(".") for field in includes) for m in n] + + # Verifying that `flat_includes` only has fields which are related to the entity include_set = (entity.InstRel | entity.InstMRel) & set(flat_includes) - for a in entity.InstAttr | entity.MetaAttr | include_set: - if a in flat_includes: - target = getattr(entity, a) + for key in entity.InstAttr | entity.MetaAttr | include_set: + if key in flat_includes: + target = getattr(entity, key) + # Copy and remove don't return values so must be done separately includes_copy = flat_includes.copy() - includes_copy.remove(a) + try: + includes_copy.remove(key) + except ValueError: + log.warning( + "Key couldn't be found to remove from include list, this could" + " cause an issue further on in the request" + ) if isinstance(target, Entity): - a_dict = self.entity_to_dict(target, includes_copy) - d[a] = a_dict + d[key] = self.entity_to_dict(target, includes_copy) + # Related fields with one-many relationships are stored as EntityLists elif isinstance(target, EntityList): - d[a] = [] + d[key] = [] for e in target: - a_dict = self.entity_to_dict(e, includes_copy) - d[a].append(a_dict) + d[key].append(self.entity_to_dict(e, includes_copy)) + # Add actual piece of data to the dictionary else: - entity_data = getattr(entity, a) + entity_data = getattr(entity, key) # Convert datetime objects to strings ready to be outputted as JSON if isinstance(entity_data, datetime): # Remove timezone data which isn't utilised in ICAT - entity_data = entity_data.replace(tzinfo=None).strftime( - Constants.ACCEPTED_DATE_FORMAT - ) - d[a] = entity_data + entity_data = self.datetime_object_to_str(entity_data) + d[key] = entity_data return d @@ -349,7 +375,8 @@ def update_attributes(old_entity, new_entity): Python ICAT :param old_entity: An existing entity record from Python ICAT - :type object: :class:`icat.entities.ENTITY` + :type object: :class:`icat.entities.ENTITY` (implementation of + :class:`icat.entity.Entity`) :param new_entity: Dictionary containing the new data to be modified :type new_entity: :class:`dict` :raises BadRequestError: If the attribute cannot be found, or if it cannot be edited From 26f303af4c59c4f7d6da90a342cb2c08d9974b06 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 10 Sep 2020 19:13:31 +0000 Subject: [PATCH 13/15] #143: Improve structure of execute_query() - This commit will also add some info logging to track what happens to the query during its lifecycle --- common/icat/filters.py | 7 ++----- common/icat/helpers.py | 23 ++++++----------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/common/icat/filters.py b/common/icat/filters.py index f89e73e1..82ca82b1 100644 --- a/common/icat/filters.py +++ b/common/icat/filters.py @@ -160,6 +160,7 @@ def icat_set_limit(query, skip_number, limit_number): class PythonICATIncludeFilter(IncludeFilter): def __init__(self, included_filters): self.included_filters = [] + log.info("Extracting fields for include filter") self._extract_filter_fields(included_filters["include"]) def _extract_filter_fields(self, field): @@ -219,13 +220,9 @@ def _extract_filter_fields(self, field): ) def apply_filter(self, query): - log.debug( - f"Included filters: {self.included_filters}," - f" Type: {type(self.included_filters)}" - ) + log.info("Applying include filter, adding fields: %s", self.included_filters) try: - pass query.addIncludes(self.included_filters) except ValueError as e: raise FilterError(e) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index b6174934..a4825a19 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -122,6 +122,7 @@ def __init__( """ try: + log.info("Creating ICATQuery for entity: %s", entity_name) self.query = Query( client, entity_name, @@ -152,17 +153,16 @@ def execute_query(self, client, return_json_formattable=False): """ try: - # Going to Python ICAT to perform the query + log.debug("Executing ICAT query") query_result = client.search(self.query) - log.debug("Query Result: %s", query_result) except ICATValidationError as e: raise PythonICATError(e) if self.query.aggregate == "DISTINCT": + log.info("Extracting the distinct fields from query's conditions") distinct_filter_flag = True # Check query's conditions for the ones created by the distinct filter self.attribute_names = [] - log.debug("Query conditions: %s", self.query.conditions) for key, value in self.query.conditions.items(): # Value can be a list if there's multiple WHERE filters for the same @@ -181,21 +181,12 @@ def execute_query(self, client, return_json_formattable=False): distinct_filter_flag = False if return_json_formattable: + log.info("Query results will be returned in a JSON format") data = [] - # TODO - Test this all works without include filter for result in query_result: - # TODO - Put this in the docstring - # Converting each row/result into its dictionary form if include filter - # not used - `______` will do this if include filter(s) are used in the - # request - Python ICAT's `as_dict()` doesn't do included fields but is - # a simpler function so only used when needed - dict_result = {} if self.query.includes else result.as_dict() - # Creating dictionary to store distinct fields for use later on distinct_result = {} - - if self.query.includes: - dict_result = self.entity_to_dict(result, self.query.includes) + dict_result = self.entity_to_dict(result, self.query.includes) for key, value in dict_result.items(): if distinct_filter_flag: @@ -204,15 +195,13 @@ def execute_query(self, client, return_json_formattable=False): if key in self.attribute_names: distinct_result[key] = dict_result[key] - # Add to the response's data depending on whether request has a distinct - # filter if distinct_filter_flag: data.append(distinct_result) else: data.append(dict_result) return data else: - # Return data exactly as Python ICAT returned the query + log.info("Query results will be returned as ICAT entities") return query_result def check_attribute_name_for_distinct(self, key, value): From 1e7572a1ae792590b506a6ceabddadbfecd3ef69 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 11 Sep 2020 10:01:29 +0000 Subject: [PATCH 14/15] #143: Minor docstring/logging changes --- common/icat/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index a4825a19..6bb5f913 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -150,6 +150,7 @@ def execute_query(self, client, return_json_formattable=False): manipulated at some point) :type return_json_formattable_data: :class:`bool` :return: Data (of type list) from the executed query + :raises PythonICATError: If an error occurs during query execution """ try: @@ -251,7 +252,6 @@ def entity_to_dict(self, entity, includes): :type includes: :class:`set` :return: ICAT Data (of type dictionary) ready to be serialised to JSON """ - log.info("Converting entity (%s) to dictionary format", type(entity)) d = {} # Split up the fields separated by dots and flatten the resulting lists From 6fa0d9ae496a4ffeb3e43aabd104aadeff4c09c2 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 11 Sep 2020 10:21:20 +0000 Subject: [PATCH 15/15] #143: Remove unused import --- common/icat/helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/common/icat/helpers.py b/common/icat/helpers.py index 6bb5f913..dc6c7dec 100644 --- a/common/icat/helpers.py +++ b/common/icat/helpers.py @@ -1,7 +1,6 @@ from functools import wraps import logging from datetime import datetime, timedelta -from collections.abc import Iterable from icat.entity import Entity, EntityList from icat.query import Query