diff --git a/CHANGELOG.md b/CHANGELOG.md index 4403a2571..7fbdfabed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## [Unreleased] +### Added + +- Backends: (internal) add support for multiple authority queries + ### Removed - Drop support for Python 3.8 diff --git a/src/ralph/backends/lrs/base.py b/src/ralph/backends/lrs/base.py index bc6fc8fe0..22a37a166 100644 --- a/src/ralph/backends/lrs/base.py +++ b/src/ralph/backends/lrs/base.py @@ -117,7 +117,9 @@ class RalphStatementsQuery(LRSStatementsQuery): agent: Optional[AgentParameters] = AgentParameters.model_construct() search_after: Optional[str] = None pit_id: Optional[str] = None - authority: Optional[AgentParameters] = AgentParameters.model_construct() + authority: Optional[Union[AgentParameters, list[AgentParameters]]] = ( + AgentParameters.model_construct() + ) ignore_order: Optional[bool] = None diff --git a/src/ralph/backends/lrs/clickhouse.py b/src/ralph/backends/lrs/clickhouse.py index 3abadf63f..309acaf62 100644 --- a/src/ralph/backends/lrs/clickhouse.py +++ b/src/ralph/backends/lrs/clickhouse.py @@ -1,7 +1,8 @@ """ClickHouse LRS backend for Ralph.""" import logging -from typing import Generator, Iterator, List, Optional +from functools import reduce +from typing import Generator, Iterator, List, Optional, Union from pydantic_settings import SettingsConfigDict @@ -162,48 +163,80 @@ def chunk_id_list(chunk_size: int = self.settings.IDS_CHUNK_SIZE) -> Generator: def _add_agent_filters( ch_params: dict, where: list, - agent_params: AgentParameters, + agent_params: Union[AgentParameters, list[AgentParameters]], target_field: str, ) -> None: """Add filters relative to agents to `where`.""" - if not agent_params: - return - if not isinstance(agent_params, dict): - agent_params = agent_params.model_dump() + def _get_agent_filters( + _params: AgentParameters, idx: Optional[int] = None + ) -> Union[tuple[list[str], dict], None]: + if not _params: + return None - if agent_params.get("mbox"): - ch_params[f"{target_field}__mbox"] = agent_params.get("mbox") - where.append( - f"JSONExtractString(event, '{target_field}', 'mbox') = " - f"{{{target_field}__mbox:String}}" - ) - elif agent_params.get("mbox_sha1sum"): - ch_params[f"{target_field}__mbox_sha1sum"] = agent_params.get( - "mbox_sha1sum" - ) - where.append( - f"JSONExtractString(event, '{target_field}', 'mbox_sha1sum') = " - f"{{{target_field}__mbox_sha1sum:String}}" - ) - elif agent_params.get("openid"): - ch_params[f"{target_field}__openid"] = agent_params.get("openid") - where.append( - f"JSONExtractString(event, '{target_field}', 'openid') = " - f"{{{target_field}__openid:String}}" - ) - elif agent_params.get("account__name"): - ch_params[f"{target_field}__account__name"] = agent_params.get( - "account__name" - ) - where.append( - f"JSONExtractString(event, '{target_field}', 'account', 'name') = " - f"{{{target_field}__account__name:String}}" - ) - ch_params[f"{target_field}__account__home_page"] = agent_params.get( - "account__home_page" - ) - where.append( - f"JSONExtractString(event, '{target_field}', 'account', 'homePage') = " - f"{{{target_field}__account__home_page:String}}" - ) + if not isinstance(_params, dict): + _params = _params.model_dump() + + target_param = f"{target_field}_{idx}" if idx is not None else target_field + + if _params.get("mbox"): + return ( + [ + f"JSONExtractString(event, '{target_field}', 'mbox') = " + f"{{{target_param}__mbox:String}}" + ], + {f"{target_param}__mbox": _params.get("mbox")}, + ) + elif _params.get("mbox_sha1sum"): + return ( + [ + f"JSONExtractString(event, '{target_field}', 'mbox_sha1sum') = " + f"{{{target_param}__mbox_sha1sum:String}}" + ], + {f"{target_param}__mbox_sha1sum": _params.get("mbox_sha1sum")}, + ) + elif _params.get("openid"): + return ( + [ + f"JSONExtractString(event, '{target_field}', 'openid') = " + f"{{{target_param}__openid:String}}" + ], + {f"{target_param}__openid": _params.get("openid")}, + ) + elif _params.get("account__name"): + return ( + [ + f"JSONExtractString(event, '{target_field}', 'account'," + f" 'name') = " + f"{{{target_param}__account__name:String}}", + f"JSONExtractString(event, '{target_field}', 'account'," + f" 'homePage') = " + f"{{{target_param}__account__home_page:String}}", + ], + { + f"{target_param}__account__name": _params.get("account__name"), + f"{target_param}__account__home_page": _params.get( + "account__home_page" + ), + }, + ) + return None + + if not agent_params: + return + elif not isinstance(agent_params, list): + filters = _get_agent_filters(agent_params) + if filters: + _where, _ch_params = filters + ch_params.update(_ch_params) + where.extend(_where) + else: + filters = [ + _get_agent_filters(params, idx=idx) + for idx, params in enumerate(agent_params) + if params + ] + _ch_params = reduce(lambda acc, el: acc | el[1], filters, {}) + _where = [" OR ".join([" AND ".join(el[0]) for el in filters])] + ch_params.update(_ch_params) + where.extend(_where) diff --git a/src/ralph/backends/lrs/es.py b/src/ralph/backends/lrs/es.py index 165752af3..918a5e764 100644 --- a/src/ralph/backends/lrs/es.py +++ b/src/ralph/backends/lrs/es.py @@ -1,7 +1,7 @@ """Elasticsearch LRS backend for Ralph.""" import logging -from typing import Iterator, List, Optional +from typing import Iterator, List, Optional, Union from pydantic_settings import SettingsConfigDict @@ -112,28 +112,51 @@ def get_query(params: RalphStatementsQuery) -> ESQuery: @staticmethod def _add_agent_filters( - es_query_filters: list, agent_params: AgentParameters, target_field: str + es_query_filters: list, + agent_params: Union[AgentParameters, list[AgentParameters]], + target_field: str, ) -> None: """Add filters relative to agents to `es_query_filters`.""" + + def _get_agent_filters(_params: AgentParameters) -> Union[dict, None]: + if not _params: + return None + + if not isinstance(_params, dict): + _params = _params.model_dump() + + if _params.get("mbox"): + field = f"{target_field}.mbox.keyword" + return {"term": {field: _params.get("mbox")}} + elif _params.get("mbox_sha1sum"): + field = f"{target_field}.mbox_sha1sum.keyword" + return {"term": {field: _params.get("mbox_sha1sum")}} + elif _params.get("openid"): + field = f"{target_field}.openid.keyword" + return {"term": {field: _params.get("openid")}} + elif _params.get("account__name"): + field_name = f"{target_field}.account.name.keyword" + field_homepage = f"{target_field}.account.homePage.keyword" + return { + "bool": { + "filter": [ + {"term": {field_name: _params.get("account__name")}}, + { + "term": { + field_homepage: _params.get("account__home_page") + } + }, + ] + } + } + return None + if not agent_params: return - - if not isinstance(agent_params, dict): - agent_params = agent_params.model_dump() - - if agent_params.get("mbox"): - field = f"{target_field}.mbox.keyword" - es_query_filters += [{"term": {field: agent_params.get("mbox")}}] - elif agent_params.get("mbox_sha1sum"): - field = f"{target_field}.mbox_sha1sum.keyword" - es_query_filters += [{"term": {field: agent_params.get("mbox_sha1sum")}}] - elif agent_params.get("openid"): - field = f"{target_field}.openid.keyword" - es_query_filters += [{"term": {field: agent_params.get("openid")}}] - elif agent_params.get("account__name"): - field = f"{target_field}.account.name.keyword" - es_query_filters += [{"term": {field: agent_params.get("account__name")}}] - field = f"{target_field}.account.homePage.keyword" - es_query_filters += [ - {"term": {field: agent_params.get("account__home_page")}} - ] + elif not isinstance(agent_params, list): + filters = _get_agent_filters(agent_params) + if filters: + es_query_filters += [filters] + else: + filters = [_get_agent_filters(params) for params in agent_params if params] + es_query_filters += [{"bool": {"should": filters}}] diff --git a/src/ralph/backends/lrs/fs.py b/src/ralph/backends/lrs/fs.py index 497223793..a095849ad 100644 --- a/src/ralph/backends/lrs/fs.py +++ b/src/ralph/backends/lrs/fs.py @@ -136,29 +136,43 @@ def _add_filter_by_agent( @staticmethod def _add_filter_by_authority( filters: list, - authority: Optional[AgentParameters], + authority: Optional[Union[AgentParameters, list[AgentParameters]]], ) -> None: """Add authority filters to `filters` if `authority` is set.""" + + def _add_filter_by_authority_single( + _filter: list, _params: AgentParameters + ) -> None: + if not _params: + return + if not isinstance(_params, dict): + _params = _params.model_dump() + + FSLRSBackend._add_filter_by_mbox( + _filter, _params.get("mbox", None), field="authority" + ) + FSLRSBackend._add_filter_by_sha1sum( + _filter, _params.get("mbox_sha1sum", None), field="authority" + ) + FSLRSBackend._add_filter_by_openid( + _filter, _params.get("openid", None), field="authority" + ) + FSLRSBackend._add_filter_by_account( + _filter, + _params.get("account__name", None), + _params.get("account__home_page", None), + field="authority", + ) + if not authority: return - - if not isinstance(authority, dict): - authority = authority.model_dump() - FSLRSBackend._add_filter_by_mbox( - filters, authority.get("mbox", None), field="authority" - ) - FSLRSBackend._add_filter_by_sha1sum( - filters, authority.get("mbox_sha1sum", None), field="authority" - ) - FSLRSBackend._add_filter_by_openid( - filters, authority.get("openid", None), field="authority" - ) - FSLRSBackend._add_filter_by_account( - filters, - authority.get("account__name", None), - authority.get("account__home_page", None), - field="authority", - ) + elif not isinstance(authority, list): + _add_filter_by_authority_single(filters, authority) + else: + or_filters = [] + for params in authority: + _add_filter_by_authority_single(or_filters, params) + FSLRSBackend._add_or_filter(filters, or_filters=or_filters) @staticmethod def _add_filter_by_id(filters: list, statement_id: Optional[str]) -> None: @@ -334,6 +348,14 @@ def match_related_object_id(statement: dict) -> bool: if object_id: filters.append(match_related_object_id if related else match_object_id) + @staticmethod + def _add_or_filter(filters: list, or_filters: Optional[list]) -> None: + """Add a filter that passes if the input passes any of `or_filters`.""" + if or_filters: + filters.append( + lambda statement: any(filter(statement) for filter in or_filters) + ) + def _add_filter_by_timestamp_since( self, filters: list, timestamp: Optional[datetime] ) -> None: diff --git a/src/ralph/backends/lrs/mongo.py b/src/ralph/backends/lrs/mongo.py index 0325011c9..5c17393f1 100644 --- a/src/ralph/backends/lrs/mongo.py +++ b/src/ralph/backends/lrs/mongo.py @@ -1,7 +1,7 @@ """MongoDB LRS backend for Ralph.""" import logging -from typing import Iterator, List, Optional +from typing import Iterator, List, Optional, Union from bson.objectid import ObjectId from pydantic_settings import SettingsConfigDict @@ -122,7 +122,9 @@ def get_query(params: RalphStatementsQuery) -> MongoQuery: @staticmethod def _add_agent_filters( - mongo_query_filters: dict, agent_params: AgentParameters, target_field: str + mongo_query_filters: dict, + agent_params: Union[AgentParameters, list[AgentParameters]], + target_field: str, ) -> None: """Add filters relative to agents to mongo_query_filters. @@ -131,26 +133,42 @@ def _add_agent_filters( agent_params (AgentParameters): Agent query parameters to search for. target_field (str): The target agent field name to perform the search. """ - if not agent_params: - return - - if not isinstance(agent_params, dict): - agent_params = agent_params.model_dump() - - if agent_params.get("mbox"): - key = f"_source.{target_field}.mbox" - mongo_query_filters.update({key: agent_params.get("mbox")}) - if agent_params.get("mbox_sha1sum"): - key = f"_source.{target_field}.mbox_sha1sum" - mongo_query_filters.update({key: agent_params.get("mbox_sha1sum")}) + def _get_agent_filters( + _params: AgentParameters, + ) -> Union[dict, None]: + if not _params: + return None + if not isinstance(_params, dict): + _params = _params.model_dump() + + if _params.get("mbox"): + key = f"_source.{target_field}.mbox" + return {key: _params.get("mbox")} + + if _params.get("mbox_sha1sum"): + key = f"_source.{target_field}.mbox_sha1sum" + return {key: _params.get("mbox_sha1sum")} + + if _params.get("openid"): + key = f"_source.{target_field}.openid" + return {key: _params.get("openid")} + + if _params.get("account__name"): + key_name = f"_source.{target_field}.account.name" + key_homepage = f"_source.{target_field}.account.homePage" + return { + key_name: _params.get("account__name"), + key_homepage: _params.get("account__home_page"), + } + return None - if agent_params.get("openid"): - key = f"_source.{target_field}.openid" - mongo_query_filters.update({key: agent_params.get("openid")}) - - if agent_params.get("account__name"): - key = f"_source.{target_field}.account.name" - mongo_query_filters.update({key: agent_params.get("account__name")}) - key = f"_source.{target_field}.account.homePage" - mongo_query_filters.update({key: agent_params.get("account__home_page")}) + if not agent_params: + return + elif not isinstance(agent_params, list): + filters = _get_agent_filters(agent_params) + if filters: + mongo_query_filters.update(filters) + else: + filters = [_get_agent_filters(params) for params in agent_params if params] + mongo_query_filters.update({"$or": filters}) diff --git a/tests/backends/lrs/test_async_es.py b/tests/backends/lrs/test_async_es.py index 8ff411ea9..d9da5782d 100644 --- a/tests/backends/lrs/test_async_es.py +++ b/tests/backends/lrs/test_async_es.py @@ -150,12 +150,24 @@ def test_backends_lrs_async_es_default_instantiation(monkeypatch, fs): "bool": { "filter": [ {"term": {"_id": "statementId"}}, - {"term": {"actor.account.name.keyword": ("13936749")}}, { - "term": { - "actor.account.homePage.keyword": ( - "http://www.example.com" - ) + "bool": { + "filter": [ + { + "term": { + "actor.account.name.keyword": ( + "13936749" + ) + } + }, + { + "term": { + "actor.account.homePage.keyword": ( + "http://www.example.com" + ) + } + }, + ] } }, ] diff --git a/tests/backends/lrs/test_clickhouse.py b/tests/backends/lrs/test_clickhouse.py index fe895223f..40745debe 100644 --- a/tests/backends/lrs/test_clickhouse.py +++ b/tests/backends/lrs/test_clickhouse.py @@ -257,6 +257,36 @@ def test_backends_lrs_clickhouse_default_instantiation(monkeypatch, fs): "sort": "emission_time DESCENDING, event_id DESCENDING", }, ), + # 9. Query by multiple authorities with OpenID. + ( + { + "authority": [ + {"openid": "http://toby.openid.example.org/"}, + {"openid": "http://alex.openid.example.org/"}, + ] + }, + { + "where": [ + "JSONExtractString(event, 'authority', 'openid') = {" + "authority_0__openid:String}" + " OR " + "JSONExtractString(event, 'authority', 'openid') = {" + "authority_1__openid:String}", + ], + "params": { + "authority_0__openid": "http://toby.openid.example.org/", + "authority_1__openid": "http://alex.openid.example.org/", + "ascending": False, + "attachments": False, + "format": "exact", + "limit": 0, + "related_activities": False, + "related_agents": False, + }, + "limit": 0, + "sort": "emission_time DESCENDING, event_id DESCENDING", + }, + ), ], ) def test_backends_database_clickhouse_query_statements_query( diff --git a/tests/backends/lrs/test_es.py b/tests/backends/lrs/test_es.py index b97bfa53b..3805a3130 100644 --- a/tests/backends/lrs/test_es.py +++ b/tests/backends/lrs/test_es.py @@ -150,12 +150,24 @@ def test_backends_lrs_es_default_instantiation(monkeypatch, fs): "bool": { "filter": [ {"term": {"_id": "statementId"}}, - {"term": {"actor.account.name.keyword": ("13936749")}}, { - "term": { - "actor.account.homePage.keyword": ( - "http://www.example.com" - ) + "bool": { + "filter": [ + { + "term": { + "actor.account.name.keyword": ( + "13936749" + ) + } + }, + { + "term": { + "actor.account.homePage.keyword": ( + "http://www.example.com" + ) + } + }, + ] } }, ] @@ -263,6 +275,76 @@ def test_backends_lrs_es_default_instantiation(monkeypatch, fs): "track_total_hits": False, }, ), + # 10. Query by Authority with openid IFI. + ( + { + "authority": {"openid": "http://toby.openid.example.org/"}, + }, + { + "pit": {"id": None, "keep_alive": None}, + "q": None, + "query": { + "bool": { + "filter": [ + { + "term": { + "authority.openid.keyword": ( + "http://toby.openid.example.org/" + ) + } + }, + ] + } + }, + "search_after": None, + "size": 0, + "sort": [{"timestamp": {"order": "desc"}}], + "track_total_hits": False, + }, + ), + # 11. Query by Authority with multiple openid IFI. + ( + { + "authority": [ + {"openid": "http://toby.openid.example.org/"}, + {"openid": "http://alex.openid.example.org/"}, + ] + }, + { + "pit": {"id": None, "keep_alive": None}, + "q": None, + "query": { + "bool": { + "filter": [ + { + "bool": { + "should": [ + { + "term": { + "authority.openid.keyword": ( + "http://toby.openid.example.org/" + ) + }, + }, + { + "term": { + "authority.openid.keyword": ( + "http://alex.openid.example.org/" + ) + } + }, + ] + }, + }, + ] + } + }, + "search_after": None, + "size": 0, + "sort": [{"timestamp": {"order": "desc"}}], + "track_total_hits": False, + }, + ), ], ) def test_backends_lrs_es_query_statements_query( diff --git a/tests/backends/lrs/test_fs.py b/tests/backends/lrs/test_fs.py index 046f43a20..59f9a9bfa 100644 --- a/tests/backends/lrs/test_fs.py +++ b/tests/backends/lrs/test_fs.py @@ -151,6 +151,22 @@ def test_backends_lrs_fs_default_instantiation(monkeypatch, fs): }, [], ), + # 32. Query by multiple authorities with OpenID + ({"authority": [{"openid": "bar_openid"}, {"openid": "foo_openid"}]}, ["6"]), + # 33. Query by multiple authorities with OpenID and ifi + ( + { + "authority": [ + {"openid": "bar_openid"}, + { + "account__home_page": "foo_home", + "account__name": "foo_name", + }, + {"openid": "foo_openid"}, + ] + }, + ["2", "6"], + ), ], ) def test_backends_lrs_fs_query_statements_query( diff --git a/tests/backends/lrs/test_mongo.py b/tests/backends/lrs/test_mongo.py index dec20a39b..f1e1ca543 100644 --- a/tests/backends/lrs/test_mongo.py +++ b/tests/backends/lrs/test_mongo.py @@ -226,6 +226,77 @@ def test_backends_lrs_mongo_default_instantiation(monkeypatch, fs): ], }, ), + # 11. Query by authority with openid IFI. + ( + { + "authority": {"openid": "http://toby.openid.example.org/"}, + }, + { + "filter": { + "_source.authority.openid": "http://toby.openid.example.org/", + }, + "limit": 0, + "projection": None, + "sort": [ + ("_source.timestamp", DESCENDING), + ("_id", DESCENDING), + ], + }, + ), + # 11. Query by multiple authority (OR) with openid IFI. + ( + { + "statementId": "statementId", + "authority": [ + {"openid": "http://toby.openid.example.org/"}, + {"openid": "http://alex.openid.example.org/"}, + ], + }, + { + "filter": { + "_source.id": "statementId", + "$or": [ + {"_source.authority.openid": "http://toby.openid.example.org/"}, + {"_source.authority.openid": "http://alex.openid.example.org/"}, + ], + }, + "limit": 0, + "projection": None, + "sort": [ + ("_source.timestamp", DESCENDING), + ("_id", DESCENDING), + ], + }, + ), + ( + { + "statementId": "62b9ce922c26b46b68ffc68f", + "agent": { + "account__name": "test_name", + "account__home_page": "http://example.com", + }, + "verb": "https://xapi-example.com/verb-id", + "activity": "http://example.com", + "since": "2020-01-01T00:00:00.000000+00:00", + "until": "2022-12-01T15:36:50", + }, + { + "filter": { + "_source.id": "62b9ce922c26b46b68ffc68f", + "_source.actor.account.name": "test_name", + "_source.actor.account.homePage": "http://example.com", + "_source.verb.id": "https://xapi-example.com/verb-id", + "_source.object.id": "http://example.com", + "_source.timestamp": { + "$gt": "2020-01-01T00:00:00.000000+00:00", + "$lte": "2022-12-01T15:36:50", + }, + }, + "limit": 0, + "projection": None, + "sort": [("_source.timestamp", DESCENDING), ("_id", DESCENDING)], + }, + ), ], ) def test_backends_lrs_mongo_query_statements_query(