From 720dbf4b77e0ab7332d09b18c78e896a79b0675f Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Wed, 18 Sep 2024 12:23:41 +0000 Subject: [PATCH 01/18] Add config section for reader performance query option --- datagateway_api/config.yaml.example | 5 +++++ datagateway_api/src/common/config.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/datagateway_api/config.yaml.example b/datagateway_api/config.yaml.example index 0634e1ac..38170d43 100644 --- a/datagateway_api/config.yaml.example +++ b/datagateway_api/config.yaml.example @@ -8,6 +8,11 @@ datagateway_api: db_url: "mysql+pymysql://icatdbuser:icatdbuserpw@localhost:3306/icatdb" icat_url: "https://localhost:8181" icat_check_cert: false + use_reader_for_performance: + enabled: false + reader_mechanism: simple + reader_username: reader + reader_password: readerpw search_api: extension: "/search-api" icat_url: "https://localhost:8181" diff --git a/datagateway_api/src/common/config.py b/datagateway_api/src/common/config.py index 245ffc68..dbc96571 100644 --- a/datagateway_api/src/common/config.py +++ b/datagateway_api/src/common/config.py @@ -38,6 +38,13 @@ def validate_extension(extension): return extension +class UseReaderForPerformance(BaseModel): + enabled: StrictBool + reader_mechanism: StrictStr + reader_username: StrictStr + reader_password: StrictStr + + class DataGatewayAPI(BaseModel): """ Configuration model class that implements pydantic's BaseModel class to allow for @@ -54,6 +61,7 @@ class DataGatewayAPI(BaseModel): extension: StrictStr icat_check_cert: Optional[StrictBool] icat_url: Optional[StrictStr] + use_reader_for_performance: Optional[UseReaderForPerformance] _validate_extension = validator("extension", allow_reuse=True)(validate_extension) From c9449f4088f8b2403dbddde4d48aab5772a66d56 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 19 Sep 2024 15:02:08 +0000 Subject: [PATCH 02/18] Add class to handle queries which should be done by reader account --- .../icat/reader_query_handler.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 datagateway_api/src/datagateway_api/icat/reader_query_handler.py diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py new file mode 100644 index 00000000..24f0703e --- /dev/null +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -0,0 +1,84 @@ +import logging + +from datagateway_api.src.common.config import Config +from datagateway_api.src.common.filter_order_handler import FilterOrderHandler +from datagateway_api.src.datagateway_api.icat.filters import PythonICATWhereFilter +from datagateway_api.src.datagateway_api.icat.query import ICATQuery + +log = logging.getLogger() + + +class ReaderQueryHandler: + # TODO - better names and comments on dicts to explain what they're for + # TODO - add docstrings + entity_filter_check = {"Datafile": "dataset.id", "Dataset": "investigation.id"} + entity_type_check = {"Datafile": "Dataset", "Dataset": "Investigation"} + + def __init__(self, entity_type, filters): + self.entity_type = entity_type + self.filters = filters + log.debug( + "Instance of ReaderQueryHandler created for a '%s' request", + self.entity_type, + ) + self.reader_query_eligible = self.check_eligibility() + + def check_eligibility(self): + if not Config.config.datagateway_api.use_reader_for_performance.enabled: + return False + + log.info("Checking whether query is eligible to go via reader account") + if self.entity_type in ReaderQueryHandler.entity_filter_check.keys(): + if self.get_where_filter_for_entity_id_check(): + return True + + return False + + def is_query_eligible_for_reader_performance(self): + return self.reader_query_eligible + + def get_where_filter_for_entity_id_check(self): + for query_filter in self.filters: + if ( + isinstance(query_filter, PythonICATWhereFilter) + and query_filter.field + == ReaderQueryHandler.entity_filter_check[self.entity_type] + and query_filter.operation == "eq" + ): + log.debug( + "WHERE filter relevant for reader query checking: %s", query_filter, + ) + self.where_filter_entity_id = query_filter.value + return query_filter + + return None + + def is_user_authorised_to_see_entity_id(self, client): + log.info( + "Checking to see if user '%s' can see '%s' = %s", + client.getUserName(), + ReaderQueryHandler.entity_filter_check[self.entity_type], + self.where_filter_entity_id, + ) + access_query = ICATQuery(client, self.entity_type) + id_check = PythonICATWhereFilter("id", self.where_filter_entity_id, "eq") + access_filter_handler = FilterOrderHandler() + access_filter_handler.manage_icat_filters([id_check], access_query.query) + results = access_query.execute_query(client) + + if results: + log.debug( + "User is authorised to see '%s' '%s'", + ReaderQueryHandler.entity_filter_check[self.entity_type], + self.where_filter_entity_id, + ) + user_authorised = True + else: + log.debug( + "User is NOT authorised to see '%s' '%s'", + ReaderQueryHandler.entity_filter_check[self.entity_type], + self.where_filter_entity_id, + ) + user_authorised = False + + return user_authorised From 60c5254dd6006d213934c4cb48f4ac37fab32ea3 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 19 Sep 2024 15:02:39 +0000 Subject: [PATCH 03/18] feat: Implement `ReaderQueryHandler` for entity and count endpoints --- .../src/datagateway_api/icat/helpers.py | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index d3471e3e..254230fd 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -13,6 +13,7 @@ ICATValidationError, ) +from datagateway_api.src.common.config import Config from datagateway_api.src.common.date_handler import DateHandler from datagateway_api.src.common.exceptions import ( AuthenticationError, @@ -25,8 +26,12 @@ PythonICATLimitFilter, PythonICATWhereFilter, ) +from datagateway_api.src.datagateway_api.icat.icat_client_pool import ICATClient from datagateway_api.src.datagateway_api.icat.lru_cache import ExtendedLRUCache from datagateway_api.src.datagateway_api.icat.query import ICATQuery +from datagateway_api.src.datagateway_api.icat.reader_query_handler import ( + ReaderQueryHandler, +) log = logging.getLogger() @@ -298,15 +303,7 @@ def get_entity_with_filters(client, entity_type, filters): result of the query """ log.info("Getting entity using request's filters") - - query = ICATQuery(client, entity_type) - - filter_handler = FilterOrderHandler() - filter_handler.manage_icat_filters(filters, query.query) - - data = query.execute_query(client, True) - - return data + return get_data_with_filters(client, entity_type, filters) def get_count_with_filters(client, entity_type, filters): @@ -329,15 +326,52 @@ def get_count_with_filters(client, entity_type, filters): entity_type, ) - query = ICATQuery(client, entity_type, aggregate="COUNT") + data = get_data_with_filters(client, entity_type, filters, aggregate="COUNT") + # Only ever 1 element in a count query result + return data[0] + + +def get_data_with_filters(client, entity_type, filters, aggregate=None): + reader_query = ReaderQueryHandler(entity_type, filters) + if reader_query.is_query_eligible_for_reader_performance(): + log.info("Query is eligible to be passed as reader acount") + if reader_query.is_user_authorised_to_see_entity_id(client): + # TODO - make reader client reuseable + reader_client = ICATClient("datagateway_api") + reader_config = Config.config.datagateway_api.use_reader_for_performance + login_credentals = { + "username": reader_config.reader_username, + "password": reader_config.reader_password, + } + reader_client.login(reader_config.reader_mechanism, login_credentals) + log.info("Query to be executed as reader account") + return execute_entity_query( + reader_client, entity_type, filters, aggregate=aggregate, + ) + else: + raise AuthenticationError( + "Not authorised to access the" + f" {ReaderQueryHandler.entity_filter_check[entity_type]}" + " you have filtered on", + ) + else: + log.info("Query to be executed as user from request: %s", client.getUserName()) + return execute_entity_query(client, entity_type, filters, aggregate=aggregate) + + +def execute_entity_query(client, entity_type, filters, aggregate=None): + query = ICATQuery(client, entity_type, aggregate=aggregate) filter_handler = FilterOrderHandler() filter_handler.manage_icat_filters(filters, query.query) - data = query.execute_query(client, True) - - # Only ever 1 element in a count query result - return data[0] + log.debug( + "Query on entity '%s' (aggregate: %s), executed as user: %s", + entity_type, + aggregate, + client.getUserName(), + ) + return query.execute_query(client, True) def get_first_result_with_filters(client, entity_type, filters): From 350480ba7ea4fdfa86035b3a96770bbc2c2ffbcd Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 19 Sep 2024 15:03:14 +0000 Subject: [PATCH 04/18] Add repr for WHERE filter to make logging them more readable --- datagateway_api/src/common/filters.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/datagateway_api/src/common/filters.py b/datagateway_api/src/common/filters.py index bd0e636c..3b5270c3 100644 --- a/datagateway_api/src/common/filters.py +++ b/datagateway_api/src/common/filters.py @@ -39,6 +39,9 @@ def __init__(self, field, value, operation): "must contain two values e.g. [1, 2]", ) + def __repr__(self): + return f"Field: {self.field}, Operation: {self.operation}, Value: {self.value}" + class DistinctFieldFilter(QueryFilter): precedence = 0 From 292cfef83aa472e1555d1688aaeac801245fb744 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 20 Sep 2024 13:25:59 +0000 Subject: [PATCH 05/18] Ensure API works when reader config isn't present in config file --- .../src/datagateway_api/icat/reader_query_handler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index 24f0703e..fd2b2aa4 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -24,7 +24,10 @@ def __init__(self, entity_type, filters): self.reader_query_eligible = self.check_eligibility() def check_eligibility(self): - if not Config.config.datagateway_api.use_reader_for_performance.enabled: + reader_config = Config.config.datagateway_api.use_reader_for_performance + if not reader_config: + return False + if not reader_config.enabled: return False log.info("Checking whether query is eligible to go via reader account") From b47391adf891549c809be6cd0c629198cc4bc007 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 20 Sep 2024 13:48:35 +0000 Subject: [PATCH 06/18] Use clients from the pool for reader queries --- .../src/datagateway_api/icat/backend.py | 8 ++++++-- .../src/datagateway_api/icat/helpers.py | 19 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/backend.py b/datagateway_api/src/datagateway_api/icat/backend.py index 83990117..1fd8cebe 100644 --- a/datagateway_api/src/datagateway_api/icat/backend.py +++ b/datagateway_api/src/datagateway_api/icat/backend.py @@ -88,7 +88,9 @@ def logout(self, session_id, **kwargs): @requires_session_id @queries_records def get_with_filters(self, session_id, entity_type, filters, **kwargs): - return get_entity_with_filters(kwargs.get("client"), entity_type, filters) + return get_entity_with_filters( + kwargs.get("client"), entity_type, filters, kwargs.get("client_pool"), + ) @requires_session_id @queries_records @@ -108,7 +110,9 @@ def get_one_with_filters(self, session_id, entity_type, filters, **kwargs): @requires_session_id @queries_records def count_with_filters(self, session_id, entity_type, filters, **kwargs): - return get_count_with_filters(kwargs.get("client"), entity_type, filters) + return get_count_with_filters( + kwargs.get("client"), entity_type, filters, kwargs.get("client_pool"), + ) @requires_session_id @queries_records diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index 254230fd..285a01d2 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -289,7 +289,7 @@ def update_entity_by_id(client, entity_type, id_, new_data): return get_entity_by_id(client, entity_type, id_, True) -def get_entity_with_filters(client, entity_type, filters): +def get_entity_with_filters(client, entity_type, filters, client_pool=None): """ Gets all the records of a given entity, based on the filters provided in the request @@ -303,10 +303,10 @@ def get_entity_with_filters(client, entity_type, filters): result of the query """ log.info("Getting entity using request's filters") - return get_data_with_filters(client, entity_type, filters) + return get_data_with_filters(client, entity_type, filters, client_pool=client_pool) -def get_count_with_filters(client, entity_type, filters): +def get_count_with_filters(client, entity_type, filters, client_pool=None): """ Get the number of results of a given entity, based on the filters provided in the request. This acts very much like `get_entity_with_filters()` but returns the number @@ -326,18 +326,25 @@ def get_count_with_filters(client, entity_type, filters): entity_type, ) - data = get_data_with_filters(client, entity_type, filters, aggregate="COUNT") + data = get_data_with_filters( + client, entity_type, filters, aggregate="COUNT", client_pool=client_pool, + ) # Only ever 1 element in a count query result return data[0] -def get_data_with_filters(client, entity_type, filters, aggregate=None): +def get_data_with_filters( + client, entity_type, filters, aggregate=None, client_pool=None, +): reader_query = ReaderQueryHandler(entity_type, filters) if reader_query.is_query_eligible_for_reader_performance(): log.info("Query is eligible to be passed as reader acount") if reader_query.is_user_authorised_to_see_entity_id(client): # TODO - make reader client reuseable - reader_client = ICATClient("datagateway_api") + if client_pool: + reader_client = get_cached_client(None, client_pool) + else: + reader_client = ICATClient("datagateway_api") reader_config = Config.config.datagateway_api.use_reader_for_performance login_credentals = { "username": reader_config.reader_username, From 6eed052f01d0bdfde8065748a6a5ac6660680692 Mon Sep 17 00:00:00 2001 From: Kevin Phipps Date: Tue, 24 Sep 2024 11:39:34 +0100 Subject: [PATCH 07/18] Add entity type lookup when checking user access --- .../src/datagateway_api/icat/reader_query_handler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index fd2b2aa4..9bc08632 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -63,7 +63,10 @@ def is_user_authorised_to_see_entity_id(self, client): ReaderQueryHandler.entity_filter_check[self.entity_type], self.where_filter_entity_id, ) - access_query = ICATQuery(client, self.entity_type) + access_query = ICATQuery( + client, + ReaderQueryHandler.entity_type_check[self.entity_type], + ) id_check = PythonICATWhereFilter("id", self.where_filter_entity_id, "eq") access_filter_handler = FilterOrderHandler() access_filter_handler.manage_icat_filters([id_check], access_query.query) From 6a5d1e5a0715f04697f680b168b961d8bbd264d4 Mon Sep 17 00:00:00 2001 From: kevinphippsstfc Date: Wed, 25 Sep 2024 08:25:59 +0000 Subject: [PATCH 08/18] Move reader_client into ReaderQueryHandler --- .../src/datagateway_api/icat/helpers.py | 26 +++++++++---------- .../icat/reader_query_handler.py | 18 +++++++++++++ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index 285a01d2..3cffcdad 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -340,21 +340,19 @@ def get_data_with_filters( if reader_query.is_query_eligible_for_reader_performance(): log.info("Query is eligible to be passed as reader acount") if reader_query.is_user_authorised_to_see_entity_id(client): - # TODO - make reader client reuseable - if client_pool: - reader_client = get_cached_client(None, client_pool) - else: - reader_client = ICATClient("datagateway_api") - reader_config = Config.config.datagateway_api.use_reader_for_performance - login_credentals = { - "username": reader_config.reader_username, - "password": reader_config.reader_password, - } - reader_client.login(reader_config.reader_mechanism, login_credentals) + reader_client = ReaderQueryHandler.reader_client log.info("Query to be executed as reader account") - return execute_entity_query( - reader_client, entity_type, filters, aggregate=aggregate, - ) + try: + results = execute_entity_query( + reader_client, entity_type, filters, aggregate=aggregate, + ) + except ICATSessionError: + # re-login as reader and try the query again + reader_client = reader_query.create_reader_client() + results = execute_entity_query( + reader_client, entity_type, filters, aggregate=aggregate, + ) + return results else: raise AuthenticationError( "Not authorised to access the" diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index 9bc08632..f83c22a9 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -3,6 +3,7 @@ from datagateway_api.src.common.config import Config from datagateway_api.src.common.filter_order_handler import FilterOrderHandler from datagateway_api.src.datagateway_api.icat.filters import PythonICATWhereFilter +from datagateway_api.src.datagateway_api.icat.icat_client_pool import ICATClient from datagateway_api.src.datagateway_api.icat.query import ICATQuery log = logging.getLogger() @@ -13,6 +14,8 @@ class ReaderQueryHandler: # TODO - add docstrings entity_filter_check = {"Datafile": "dataset.id", "Dataset": "investigation.id"} entity_type_check = {"Datafile": "Dataset", "Dataset": "Investigation"} + # keep a cached reader_client for faster queries + reader_client = None def __init__(self, entity_type, filters): self.entity_type = entity_type @@ -22,6 +25,21 @@ def __init__(self, entity_type, filters): self.entity_type, ) self.reader_query_eligible = self.check_eligibility() + self.create_reader_client() + + def create_reader_client(self): + log.info("Creating reader_client") + ReaderQueryHandler.reader_client = ICATClient("datagateway_api") + reader_config = Config.config.datagateway_api.use_reader_for_performance + login_credentals = { + "username": reader_config.reader_username, + "password": reader_config.reader_password, + } + ReaderQueryHandler.reader_client.login( + reader_config.reader_mechanism, + login_credentals, + ) + return ReaderQueryHandler.reader_client def check_eligibility(self): reader_config = Config.config.datagateway_api.use_reader_for_performance From 11f4a7b778788b4e0365357dd4ff3e0112086872 Mon Sep 17 00:00:00 2001 From: kevinphippsstfc Date: Wed, 25 Sep 2024 08:52:30 +0000 Subject: [PATCH 09/18] Remove unused imports --- datagateway_api/src/datagateway_api/icat/helpers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index 3cffcdad..b209ec21 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -13,7 +13,6 @@ ICATValidationError, ) -from datagateway_api.src.common.config import Config from datagateway_api.src.common.date_handler import DateHandler from datagateway_api.src.common.exceptions import ( AuthenticationError, @@ -26,7 +25,6 @@ PythonICATLimitFilter, PythonICATWhereFilter, ) -from datagateway_api.src.datagateway_api.icat.icat_client_pool import ICATClient from datagateway_api.src.datagateway_api.icat.lru_cache import ExtendedLRUCache from datagateway_api.src.datagateway_api.icat.query import ICATQuery from datagateway_api.src.datagateway_api.icat.reader_query_handler import ( From 7b4d5afe34be3f740c803f33ebbbfe63d063eb1e Mon Sep 17 00:00:00 2001 From: kevinphippsstfc Date: Thu, 26 Sep 2024 08:33:09 +0000 Subject: [PATCH 10/18] Ensure reader client only initialised once --- .../src/datagateway_api/icat/reader_query_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index f83c22a9..71429d95 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -25,7 +25,8 @@ def __init__(self, entity_type, filters): self.entity_type, ) self.reader_query_eligible = self.check_eligibility() - self.create_reader_client() + if not ReaderQueryHandler.reader_client: + self.create_reader_client() def create_reader_client(self): log.info("Creating reader_client") From dca658f56bac66a715c6920ef7566186f4ae5ba7 Mon Sep 17 00:00:00 2001 From: kevinphippsstfc Date: Thu, 26 Sep 2024 15:18:02 +0000 Subject: [PATCH 11/18] Do config check before ReaderQueryHandler creation --- .../src/datagateway_api/icat/helpers.py | 21 +++++++++++++++++++ .../icat/reader_query_handler.py | 6 ------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index b209ec21..1d3ccdc3 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -4,6 +4,7 @@ from cachetools import cached from dateutil.tz import tzlocal +from datagateway_api.src.common.config import Config from icat.exception import ( ICATInternalError, ICATNoObjectError, @@ -334,6 +335,12 @@ def get_count_with_filters(client, entity_type, filters, client_pool=None): def get_data_with_filters( client, entity_type, filters, aggregate=None, client_pool=None, ): + if not is_use_reader_for_performance_enabled(): + # just execute the query as normal + return execute_entity_query(client, entity_type, filters, aggregate=aggregate) + + # otherwise see if this query is eligible to benefit from running + # faster using the reader account reader_query = ReaderQueryHandler(entity_type, filters) if reader_query.is_query_eligible_for_reader_performance(): log.info("Query is eligible to be passed as reader acount") @@ -377,6 +384,20 @@ def execute_entity_query(client, entity_type, filters, aggregate=None): return query.execute_query(client, True) +def is_use_reader_for_performance_enabled(): + """ + Returns true is the 'use_reader_for_performance' section is present in the + config file and 'enabled' in that section is set to true + """ + reader_config = Config.config.datagateway_api.use_reader_for_performance + if not reader_config: + return False + if not reader_config.enabled: + return False + + return True + + def get_first_result_with_filters(client, entity_type, filters): """ Using filters in the request, get results of the given entity, but only show the diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index 71429d95..38fd7404 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -43,12 +43,6 @@ def create_reader_client(self): return ReaderQueryHandler.reader_client def check_eligibility(self): - reader_config = Config.config.datagateway_api.use_reader_for_performance - if not reader_config: - return False - if not reader_config.enabled: - return False - log.info("Checking whether query is eligible to go via reader account") if self.entity_type in ReaderQueryHandler.entity_filter_check.keys(): if self.get_where_filter_for_entity_id_check(): From c5325b01b82014b82e104432c53c4fb43f613759 Mon Sep 17 00:00:00 2001 From: kevinphippsstfc Date: Thu, 26 Sep 2024 15:27:50 +0000 Subject: [PATCH 12/18] Move import and remove whitespace --- datagateway_api/src/datagateway_api/icat/helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index 1d3ccdc3..0fc3ffa0 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -4,7 +4,6 @@ from cachetools import cached from dateutil.tz import tzlocal -from datagateway_api.src.common.config import Config from icat.exception import ( ICATInternalError, ICATNoObjectError, @@ -14,6 +13,7 @@ ICATValidationError, ) +from datagateway_api.src.common.config import Config from datagateway_api.src.common.date_handler import DateHandler from datagateway_api.src.common.exceptions import ( AuthenticationError, @@ -386,7 +386,7 @@ def execute_entity_query(client, entity_type, filters, aggregate=None): def is_use_reader_for_performance_enabled(): """ - Returns true is the 'use_reader_for_performance' section is present in the + Returns true is the 'use_reader_for_performance' section is present in the config file and 'enabled' in that section is set to true """ reader_config = Config.config.datagateway_api.use_reader_for_performance From 74185f971ec4a9addf9914744c61fceda8a1714c Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 30 Sep 2024 13:14:58 +0000 Subject: [PATCH 13/18] Improve exception handling on reader account login - If there's an issue when logging in (e.g. due to invalid credentials), the user will now return a 500, not a 403 --- .../datagateway_api/icat/reader_query_handler.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index 38fd7404..bec78ab3 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -13,7 +13,7 @@ class ReaderQueryHandler: # TODO - better names and comments on dicts to explain what they're for # TODO - add docstrings entity_filter_check = {"Datafile": "dataset.id", "Dataset": "investigation.id"} - entity_type_check = {"Datafile": "Dataset", "Dataset": "Investigation"} + parent_entity_lookup = {"Datafile": "Dataset", "Dataset": "Investigation"} # keep a cached reader_client for faster queries reader_client = None @@ -36,10 +36,13 @@ def create_reader_client(self): "username": reader_config.reader_username, "password": reader_config.reader_password, } - ReaderQueryHandler.reader_client.login( - reader_config.reader_mechanism, - login_credentals, - ) + try: + ReaderQueryHandler.reader_client.login( + reader_config.reader_mechanism, login_credentals, + ) + except ICATSessionError: + log.error("User credentials for reader account aren't valid") + raise PythonICATError("Internal error with reader account configuration") return ReaderQueryHandler.reader_client def check_eligibility(self): @@ -77,8 +80,7 @@ def is_user_authorised_to_see_entity_id(self, client): self.where_filter_entity_id, ) access_query = ICATQuery( - client, - ReaderQueryHandler.entity_type_check[self.entity_type], + client, ReaderQueryHandler.parent_entity_lookup[self.entity_type], ) id_check = PythonICATWhereFilter("id", self.where_filter_entity_id, "eq") access_filter_handler = FilterOrderHandler() From a36b607626829d800fcdac8fae36a7e9a7aa491a Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 30 Sep 2024 15:40:41 +0000 Subject: [PATCH 14/18] Code cleanup - Add docstrings - Remove client_pool kwargs - Add type hints - Fix any outstanding linting issues --- .../src/datagateway_api/icat/backend.py | 8 +- .../src/datagateway_api/icat/helpers.py | 33 +++++--- .../icat/reader_query_handler.py | 75 ++++++++++++++++--- 3 files changed, 91 insertions(+), 25 deletions(-) diff --git a/datagateway_api/src/datagateway_api/icat/backend.py b/datagateway_api/src/datagateway_api/icat/backend.py index 1fd8cebe..83990117 100644 --- a/datagateway_api/src/datagateway_api/icat/backend.py +++ b/datagateway_api/src/datagateway_api/icat/backend.py @@ -88,9 +88,7 @@ def logout(self, session_id, **kwargs): @requires_session_id @queries_records def get_with_filters(self, session_id, entity_type, filters, **kwargs): - return get_entity_with_filters( - kwargs.get("client"), entity_type, filters, kwargs.get("client_pool"), - ) + return get_entity_with_filters(kwargs.get("client"), entity_type, filters) @requires_session_id @queries_records @@ -110,9 +108,7 @@ def get_one_with_filters(self, session_id, entity_type, filters, **kwargs): @requires_session_id @queries_records def count_with_filters(self, session_id, entity_type, filters, **kwargs): - return get_count_with_filters( - kwargs.get("client"), entity_type, filters, kwargs.get("client_pool"), - ) + return get_count_with_filters(kwargs.get("client"), entity_type, filters) @requires_session_id @queries_records diff --git a/datagateway_api/src/datagateway_api/icat/helpers.py b/datagateway_api/src/datagateway_api/icat/helpers.py index 0fc3ffa0..35922cf1 100644 --- a/datagateway_api/src/datagateway_api/icat/helpers.py +++ b/datagateway_api/src/datagateway_api/icat/helpers.py @@ -288,7 +288,7 @@ def update_entity_by_id(client, entity_type, id_, new_data): return get_entity_by_id(client, entity_type, id_, True) -def get_entity_with_filters(client, entity_type, filters, client_pool=None): +def get_entity_with_filters(client, entity_type, filters): """ Gets all the records of a given entity, based on the filters provided in the request @@ -302,10 +302,10 @@ def get_entity_with_filters(client, entity_type, filters, client_pool=None): result of the query """ log.info("Getting entity using request's filters") - return get_data_with_filters(client, entity_type, filters, client_pool=client_pool) + return get_data_with_filters(client, entity_type, filters) -def get_count_with_filters(client, entity_type, filters, client_pool=None): +def get_count_with_filters(client, entity_type, filters): """ Get the number of results of a given entity, based on the filters provided in the request. This acts very much like `get_entity_with_filters()` but returns the number @@ -325,16 +325,24 @@ def get_count_with_filters(client, entity_type, filters, client_pool=None): entity_type, ) - data = get_data_with_filters( - client, entity_type, filters, aggregate="COUNT", client_pool=client_pool, - ) + data = get_data_with_filters(client, entity_type, filters, aggregate="COUNT") # Only ever 1 element in a count query result return data[0] -def get_data_with_filters( - client, entity_type, filters, aggregate=None, client_pool=None, -): +def get_data_with_filters(client, entity_type, filters, aggregate=None): + """ + Gets all the records of a given entity, based on the filters and an optional + aggregate provided in the request. This function is called by + `get_entity_with_filters()` and `get_count_with_filters()` that deal with GET entity + and GET /count entity endpoints respectively + + This function uses the reader performance query functionality IF it is enabled in + the config. Checks are done to see whether this functionality has been enabled and + whether the query is suitable to be completed with the reader account. There are + more details about the inner workings in ReaderQueryHandler + """ + if not is_use_reader_for_performance_enabled(): # just execute the query as normal return execute_entity_query(client, entity_type, filters, aggregate=aggregate) @@ -370,6 +378,11 @@ def get_data_with_filters( def execute_entity_query(client, entity_type, filters, aggregate=None): + """ + Assemble a query object with the user's query filters and execute the query by + passing it to ICAT, returning them in this function + """ + query = ICATQuery(client, entity_type, aggregate=aggregate) filter_handler = FilterOrderHandler() @@ -384,7 +397,7 @@ def execute_entity_query(client, entity_type, filters, aggregate=None): return query.execute_query(client, True) -def is_use_reader_for_performance_enabled(): +def is_use_reader_for_performance_enabled() -> bool: """ Returns true is the 'use_reader_for_performance' section is present in the config file and 'enabled' in that section is set to true diff --git a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py index bec78ab3..7101c80d 100644 --- a/datagateway_api/src/datagateway_api/icat/reader_query_handler.py +++ b/datagateway_api/src/datagateway_api/icat/reader_query_handler.py @@ -1,7 +1,12 @@ import logging +from typing import List, Optional + +from icat.exception import ICATSessionError from datagateway_api.src.common.config import Config +from datagateway_api.src.common.exceptions import PythonICATError from datagateway_api.src.common.filter_order_handler import FilterOrderHandler +from datagateway_api.src.common.filters import QueryFilter from datagateway_api.src.datagateway_api.icat.filters import PythonICATWhereFilter from datagateway_api.src.datagateway_api.icat.icat_client_pool import ICATClient from datagateway_api.src.datagateway_api.icat.query import ICATQuery @@ -10,14 +15,38 @@ class ReaderQueryHandler: - # TODO - better names and comments on dicts to explain what they're for - # TODO - add docstrings + """ + This class handles the mechanism that allows 'performance queries' to occur on + particular endpoints. These queries are to improve performance on requests that have + a WHERE filter on the ID of the parent entity where passing the query directly to + ICAT can cause performance issues. This is due to the complexity of the ICAT rules, + meaning a relatively simple SQL query is a long paragraph of SQL. The rules are + bypassed by performing an equivalent check to see if the user can see the parent + entity by querying for it directly. Once permissions have been verified, the user's + original query is executed using a configurable reader account. + + On a production instance where this functionality is needed, the reader account will + have been setup with appropriate ICAT rules to view the entities. + + Example workflow: + - User sends request to /datafiles with a WHERE filter of dataset.id = 4 + - Query is determined as eligble + - Dataset query is sent to ICAT with a WHERE filter of id = 4 + - If the appropriate dataset is returned, the user's original query is executed, but + as the reader account, not the user's account + - If no dataset is found (i.e. the user doesn't have permission is view the dataset) + the API responds with a 403 + """ + + # Lookup to determine which field to search whether a user has permission to view entity_filter_check = {"Datafile": "dataset.id", "Dataset": "investigation.id"} parent_entity_lookup = {"Datafile": "Dataset", "Dataset": "Investigation"} - # keep a cached reader_client for faster queries + # Keep a cached reader_client for faster queries. A reader client is created when + # the first instance of this class is created and is refreshed when a login attempt + # fails (due to an expired session ID) reader_client = None - def __init__(self, entity_type, filters): + def __init__(self, entity_type: str, filters: List[QueryFilter]) -> None: self.entity_type = entity_type self.filters = filters log.debug( @@ -28,7 +57,13 @@ def __init__(self, entity_type, filters): if not ReaderQueryHandler.reader_client: self.create_reader_client() - def create_reader_client(self): + def create_reader_client(self) -> ICATClient: + """ + Create a new client (assigning it as a class variable) and login using the + reader's credentials. If the credentials aren't valid, a PythonICATError is + raised (resulting in a 500). The client object is returned + """ + log.info("Creating reader_client") ReaderQueryHandler.reader_client = ICATClient("datagateway_api") reader_config = Config.config.datagateway_api.use_reader_for_performance @@ -45,7 +80,13 @@ def create_reader_client(self): raise PythonICATError("Internal error with reader account configuration") return ReaderQueryHandler.reader_client - def check_eligibility(self): + def check_eligibility(self) -> bool: + """ + This function checks whether the input query can be executed as a 'reader + performance query'. The entity of the query needs to be in `entity_filter_check` + and an appropriate WHERE filter needs to be sought + (using `get_where_filter_for_entity_id_check()`) + """ log.info("Checking whether query is eligible to go via reader account") if self.entity_type in ReaderQueryHandler.entity_filter_check.keys(): if self.get_where_filter_for_entity_id_check(): @@ -53,10 +94,19 @@ def check_eligibility(self): return False - def is_query_eligible_for_reader_performance(self): + def is_query_eligible_for_reader_performance(self) -> bool: + """ + Getter that returns a boolean regarding query eligibility + """ return self.reader_query_eligible - def get_where_filter_for_entity_id_check(self): + def get_where_filter_for_entity_id_check(self) -> Optional[PythonICATWhereFilter]: + """ + Iterate through the instance's query filters and return a WHERE filter for a + relevant parent entity (e.g. dataset.id or datafile.id). The WHERE filter must + use the 'eq' operator + """ + for query_filter in self.filters: if ( isinstance(query_filter, PythonICATWhereFilter) @@ -72,7 +122,14 @@ def get_where_filter_for_entity_id_check(self): return None - def is_user_authorised_to_see_entity_id(self, client): + def is_user_authorised_to_see_entity_id(self, client) -> bool: + """ + This function checks whether the user is authorised to see a parent entity (e.g. + if they query /datafiles, whether they can see a particular dataset). A query is + created and sent to ICAT for execution - the query is performed using the + session ID provided in the request + """ + log.info( "Checking to see if user '%s' can see '%s' = %s", client.getUserName(), From 642ea8b766eb2640bf8602a9ec9bb3afbe96f0a0 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 1 Oct 2024 07:13:19 +0000 Subject: [PATCH 15/18] Upgrade dependencies and ignore vulnerabilities - As ever, the vulnerabilities that are being ignored are because we cannot upgrade due to being tied to 3.6 --- noxfile.py | 4 ++++ poetry.lock | 12 ++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/noxfile.py b/noxfile.py index 1dbf2a60..2eba6629 100644 --- a/noxfile.py +++ b/noxfile.py @@ -181,6 +181,10 @@ def safety(session): "71681", "--ignore", "71684", + "--ignore", + "72132", + "--ignore", + "72731", ) try: diff --git a/poetry.lock b/poetry.lock index f61ee920..9840a3bb 100644 --- a/poetry.lock +++ b/poetry.lock @@ -114,7 +114,7 @@ python-versions = "~=3.5" [[package]] name = "certifi" -version = "2023.11.17" +version = "2024.8.30" description = "Python package for providing Mozilla's CA Bundle." category = "main" optional = false @@ -426,7 +426,7 @@ dotenv = ["python-dotenv"] [[package]] name = "flask-cors" -version = "4.0.1" +version = "4.0.2" description = "A Flask extension adding a decorator for CORS support" category = "main" optional = false @@ -1341,8 +1341,8 @@ cachetools = [ {file = "cachetools-4.2.4.tar.gz", hash = "sha256:89ea6f1b638d5a73a4f9226be57ac5e4f399d22770b92355f92dcb0f7f001693"}, ] certifi = [ - {file = "certifi-2023.11.17-py3-none-any.whl", hash = "sha256:e036ab49d5b79556f99cfc2d9320b34cfbe5be05c5871b51de9329f0603b0474"}, - {file = "certifi-2023.11.17.tar.gz", hash = "sha256:9b469f3a900bf28dc19b8cfbf8019bf47f7fdd1a65a1d4ffb98fc14166beb4d1"}, + {file = "certifi-2024.8.30-py3-none-any.whl", hash = "sha256:922820b53db7a7257ffbda3f597266d435245903d80737e34f8a45ff3e3230d8"}, + {file = "certifi-2024.8.30.tar.gz", hash = "sha256:bec941d2aa8195e248a60b31ff9f0558284cf01a52591ceda73ea9afffd69fd9"}, ] cffi = [ {file = "cffi-1.15.1-cp27-cp27m-macosx_10_9_x86_64.whl", hash = "sha256:a66d3508133af6e8548451b25058d5812812ec3798c886bf38ed24a98216fab2"}, @@ -1572,8 +1572,8 @@ flask = [ {file = "Flask-2.0.3.tar.gz", hash = "sha256:e1120c228ca2f553b470df4a5fa927ab66258467526069981b3eb0a91902687d"}, ] flask-cors = [ - {file = "Flask_Cors-4.0.1-py2.py3-none-any.whl", hash = "sha256:f2a704e4458665580c074b714c4627dd5a306b333deb9074d0b1794dfa2fb677"}, - {file = "flask_cors-4.0.1.tar.gz", hash = "sha256:eeb69b342142fdbf4766ad99357a7f3876a2ceb77689dc10ff912aac06c389e4"}, + {file = "Flask_Cors-4.0.2-py2.py3-none-any.whl", hash = "sha256:38364faf1a7a5d0a55bd1d2e2f83ee9e359039182f5e6a029557e1f56d92c09a"}, + {file = "flask_cors-4.0.2.tar.gz", hash = "sha256:493b98e2d1e2f1a4720a7af25693ef2fe32fbafec09a2f72c59f3e475eda61d2"}, ] flask-restful = [ {file = "Flask-RESTful-0.3.10.tar.gz", hash = "sha256:fe4af2ef0027df8f9b4f797aba20c5566801b6ade995ac63b588abf1a59cec37"}, From 88d63909a92a34d52fabfd5ad233440a38c58ce4 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 1 Oct 2024 13:26:16 +0000 Subject: [PATCH 16/18] build: Fix installation issues in modern Python 3.8+ --- .github/workflows/ci-build.yml | 10 ++++++++++ noxfile.py | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index b4d4903e..4b557896 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -138,6 +138,10 @@ jobs: - name: Create search_api_mapping.json run: cp datagateway_api/search_api_mapping.json.example datagateway_api/search_api_mapping.json + # See comment in noxfile.py for explanation regarding this step + - name: Downgrade setuptools + run: poetry run pip install --upgrade setuptools==70.0.0 + if: matrix.python-version == '3.8' || matrix.python-version == '3.9' || matrix.python-version == '3.10' - name: Install dependencies run: poetry install @@ -284,6 +288,9 @@ jobs: - name: Install Requests run: pip install 'requests<2.30' + # See comment in noxfile.py for explanation regarding this step + - name: Downgrade setuptools + run: poetry run pip install --upgrade setuptools==70.0.0 - name: Install dependencies run: poetry install @@ -339,6 +346,9 @@ jobs: - name: Create search_api_mapping.json run: cd /home/runner/work/datagateway-api/datagateway-api; cp datagateway_api/search_api_mapping.json.example datagateway_api/search_api_mapping.json + # See comment in noxfile.py for explanation regarding this step + - name: Downgrade setuptools + run: poetry run pip install --upgrade setuptools==70.0.0 - name: Install dependencies run: poetry install diff --git a/noxfile.py b/noxfile.py index 2eba6629..0bdcb160 100644 --- a/noxfile.py +++ b/noxfile.py @@ -198,6 +198,13 @@ def safety(session): @nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"], reuse_venv=True) def unit_tests(session): args = session.posargs + # Explicitly installing/downgrading setuptools is done to fix poetry install on 3.8+ + # as per https://github.com/pypa/setuptools/issues/4483#issuecomment-2236327987. + # A cleaner fix would be to upgrade the packaging library to 22.0+ (as per + # https://github.com/pypa/setuptools/issues/4483#issuecomment-2236339726) but this + # cannot be done on this repo until support for Python 3.6 is dropped + if session.python in ["3.8", "3.9", "3.10"]: + session.run("pip", "install", "--upgrade", "setuptools==70.0.0") session.run("poetry", "install", external=True) session.run("pytest", "test/unit", *args) @@ -205,5 +212,9 @@ def unit_tests(session): @nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"], reuse_venv=True) def integration_tests(session): args = session.posargs + # Explicit downgrade of setuptools also performed here. See explanation in + # `unit_tests()` + if session.python in ["3.8", "3.9", "3.10"]: + session.run("pip", "install", "--upgrade", "setuptools==70.0.0") session.run("poetry", "install", external=True) session.run("pytest", "test/integration", *args) From b7b2b072947a0f2e62790bbbb6530eff86855482 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 1 Oct 2024 13:26:56 +0000 Subject: [PATCH 17/18] Add columns to drop list for SQL dump diff - The final diff in the CI will still fail because `main` doesn't contain these dropped columns, but the initial diff to check that two runs produce identical data should pass --- util/columns_to_drop.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/util/columns_to_drop.sql b/util/columns_to_drop.sql index 918493da..d678bf20 100644 --- a/util/columns_to_drop.sql +++ b/util/columns_to_drop.sql @@ -14,6 +14,8 @@ ALTER TABLE DATACOLLECTIONPARAMETER DROP COLUMN `MOD_TIME`; ALTER TABLE DATAFILE DROP COLUMN `CREATE_TIME`; ALTER TABLE DATAFILE DROP COLUMN `MOD_TIME`; + ALTER TABLE DATAFILE DROP COLUMN `DATAFILECREATETIME`; + ALTER TABLE DATAFILE DROP COLUMN `DATAFILEMODTIME`; ALTER TABLE DATAFILEFORMAT DROP COLUMN `CREATE_TIME`; ALTER TABLE DATAFILEFORMAT DROP COLUMN `MOD_TIME`; ALTER TABLE DATAFILEPARAMETER DROP COLUMN `CREATE_TIME`; From e361e2bc7bfa6d3ca72f3e8285c349911df451f4 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Wed, 2 Oct 2024 15:55:48 +0000 Subject: [PATCH 18/18] Add tests for reader performance queries --- .../icat/test_reader_performance.py | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 test/integration/datagateway_api/icat/test_reader_performance.py diff --git a/test/integration/datagateway_api/icat/test_reader_performance.py b/test/integration/datagateway_api/icat/test_reader_performance.py new file mode 100644 index 00000000..0ab9d0d4 --- /dev/null +++ b/test/integration/datagateway_api/icat/test_reader_performance.py @@ -0,0 +1,125 @@ +from unittest.mock import patch + +import pytest + +from datagateway_api.src.common.config import Config +from datagateway_api.src.datagateway_api.icat.filters import ( + PythonICATLimitFilter, + PythonICATOrderFilter, + PythonICATWhereFilter, +) +from datagateway_api.src.datagateway_api.icat.helpers import ( + get_data_with_filters, + is_use_reader_for_performance_enabled, +) +from datagateway_api.src.datagateway_api.icat.icat_client_pool import ICATClient +from datagateway_api.src.datagateway_api.icat.reader_query_handler import ( + ReaderQueryHandler, +) + + +class TestReaderPerformance: + @pytest.mark.parametrize( + "test_entity_type, test_query_filters, expected_eligbility", + [ + pytest.param( + "Dataset", + [PythonICATWhereFilter("investigation.id", 3, "eq")], + True, + id="Typical use case for dataset", + ), + pytest.param( + "Datafile", + [PythonICATWhereFilter("dataset.id", 3, "eq")], + True, + id="Typical use case for datafile", + ), + pytest.param( + "Datafile", + [ + PythonICATLimitFilter(50), + PythonICATOrderFilter("id", "asc"), + PythonICATWhereFilter("dataset.id", 3, "eq"), + ], + True, + id="Typical use case with multiple query filters", + ), + pytest.param( + "Datafile", + [PythonICATLimitFilter(25)], + False, + id="Query with no relevant filters", + ), + pytest.param( + "User", + [PythonICATWhereFilter("studies.id", 3, "eq")], + False, + id="Query on an entity type that isn't relevant for reader performance", + ), + ], + ) + @patch( + "datagateway_api.src.common.config.Config.config.datagateway_api" + ".use_reader_for_performance.enabled", + return_value=True, + ) + def test_eligbility( + self, _, test_entity_type, test_query_filters, expected_eligbility, + ): + reader_performance_enabled = is_use_reader_for_performance_enabled() + assert reader_performance_enabled + + # Check eligbility method is executed in init + test_handler = ReaderQueryHandler(test_entity_type, test_query_filters) + query_eligbility = test_handler.is_query_eligible_for_reader_performance() + assert query_eligbility == expected_eligbility + + def test_reader_client(self): + ReaderQueryHandler("Datafile", []) + reader_client = ReaderQueryHandler.reader_client + assert isinstance(reader_client, ICATClient) + + reader_config = Config.config.datagateway_api.use_reader_for_performance + assert ( + reader_client.getUserName() + == f"{reader_config.reader_mechanism}/{reader_config.reader_username}" + ) + + @pytest.mark.parametrize( + "test_entity_type, test_query_filters, expected_authorisation", + [ + pytest.param( + "Datafile", + [PythonICATWhereFilter("dataset.id", 3, "eq")], + True, + id="Typical positive use case", + ), + pytest.param( + "Datafile", + [PythonICATWhereFilter("dataset.id", 99999, "eq")], + False, + id="Typical negative use case", + ), + ], + ) + def test_is_user_authorised( + self, icat_client, test_entity_type, test_query_filters, expected_authorisation, + ): + test_handler = ReaderQueryHandler(test_entity_type, test_query_filters) + is_authorised = test_handler.is_user_authorised_to_see_entity_id(icat_client) + assert is_authorised == expected_authorisation + + @patch("datagateway_api.src.datagateway_api.icat.helpers.execute_entity_query") + @patch( + "datagateway_api.src.common.config.Config.config.datagateway_api" + ".use_reader_for_performance.enabled", + return_value=True, + ) + def test_execute_query_as_reader(self, _, mock_execute_entity_query, icat_client): + get_data_with_filters( + icat_client, "Datafile", [PythonICATWhereFilter("dataset.id", 3, "eq")], + ) + assert mock_execute_entity_query.call_count == 1 + + client = mock_execute_entity_query.call_args_list[0][0][0] + assert client.getUserName() == "simple/reader"